-
Notifications
You must be signed in to change notification settings - Fork 931
NH-1262: Port of one-to-one cascade delete orphan functionality from Hibernate #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@mickfold could you please update documentation to reflect these changes? |
@hazzik I don't know how to do update the documentation on nhforge. Is there a guide on how to do this? |
@mcharalambous The reference documenation is in DocBook XML in the doc directory of the code repository. Publishing to nhforge will happen later. |
@oskarb thanks. |
I'm currently wishing this issue was fixed. However, it looks like this pull request does more than adding all-delete-orphan to one-to-one. It also introduces a new cascade, delete-orphan, to NHibernate. Is this from porting the Hibernate code? |
@dschilling Yes, everything that was added was done so because it was part of the Hibernate feature HHH-4726. When porting the feature I tried to stick as closely to the original hibernate code as possible with the aim of getting the ported unit tests to pass. |
@mickfold could you please rebase your PR on top of master branch? Thanks. |
@hazzik Sorry for the delay. I have rebased my PR on to the top of the master branch. Please let me know if it looks okay. Thanks. |
src/NHibernate/Mapping/ManyToOne.cs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please fix formatting here?
@hazzik I've fixed the formatting issue you highlighted. Please let me know if you find anything else which needs to be fixed. Thanks. |
There is probably one big things need to be done: make this work with MappingByCode as well, if it is not supported yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With capital letter, please
Can you please make sure that code follows C# code conventions? Methods and properties with capital letter. Thanks |
@hazzik Yes, those are the commits I ported but due to some implementation differences, such as classes like HbmBinder existing in Hibernate but not NHibernate, the port is not a line for line copy. I did try to make the test cases as close to the original Java code as close as possible though. I've also updated all the tests code so methods and properties have capital letters and have started to work on MappingByCode. |
@mickfold Any news on MappingByCode support? |
@hazzik I'm down to one failing test case which is the Bidirectional Foreign Key Composite Id test. From looking at the XML code generated by the MappingByCode its missing out some properties which is causing problems, i.e.
when exported to XML becomes:
but should be
I'm investigating why this is happening at the moment. |
@hazzik one-to-one cascade delete orphan is now supported in MappingByCode. Btw there are now 3 tests that are now failing which are checking for unsupported cascade styles. These are now redundant, should I delete them? I've listed the tests below. AnyMapperTest.AutoCleanInvalidCascade Finally, one strange thing I discovered is that if you specify columns in the mapper it resets a lot of other fields to the default values, which mean I had to change the order of the mapping to the following to get it to work. mc.ManyToOne(x => x.Info, m => |
Yes, please delete these tests |
3f9ac00
to
736069f
Compare
@hazzik I've removed the redundant test. Should I squash the 5 commits into a single commit? |
Great work, @mickfold! Thanks! |
LGTM! |
In general I prefer to reserve the NHSpecificTest//NHxxxx convention for specific bug tests or corner cases. For basic functionality it would be better to organize the tests by functional area. Isn't there any existing test case folders for cascade etc., where these test cases or fixtures should be added? |
@oskarb There's a cascade folder off the root test project. The tests could be moved there. What do you think? |
@mickfold sure |
a365cf6
to
2c2881a
Compare
…ne mappings from Hibernate to NHibernate. Mappings can be made in XML or MappingByCode. The following scenarios are supported: 1) Bidirectional primary key based one-to-one. 2) Unidirectional shared primary key based one-to-one 3) Bidirectional foreign key based one-to-one with orphan delete declared on the non-constrained (<one-to-one/>) side. 4) Bidirectional foreign key based one-to-one with orphan delete declared on the constrained (<many-to-one unique="true"/>) side. 5) Unidirectional foreign key based one-to-one with orphan delete declared on the constrained (<many-to-one unique="true"/>) side. 6) Unidirectional foreign key based one-to-one with orphan delete declared on the constrained side (<many-to-one unique="true"/>) where the foreign key is composite (multiple columns).
Thanks a lot @mickfold! |
Merged, thanks |
https://nhibernate.jira.com/browse/NH-1262
This is a direct port of the cascade delete orphan functionality for one-to-one mappings from Hibernate to NHibernate.
This commit supports the following cases:
Further information can be found in the original work for Hibernate https://hibernate.atlassian.net/browse/HHH-4726