Thanks a lot, Dan.
Relationships are "first-class Domain citizens", so this is really important :-)
Just a question related to yesterday's BDD implementation (really useful links
on the documentation you wrote). If the relationships are going to be managed
by DN, there could be a lot of @unit tests (all those where bidir relationships
are used) that could fail, couldn't it ?
Just to clarify it before refactoring, an example with the Agreement - Roles
relationship would be refactored this way:
Agreement.java:
The relationship, field, getter and setter remains identical (with the same DN
annotation):
@javax.jdo.annotations.Persistent(mappedBy = "agreement")
private SortedSet<AgreementRole> roles = new TreeSet<AgreementRole>();
@MemberOrder(name = "Roles", sequence = "11")
@Disabled
@Render(Type.EAGERLY)
public SortedSet<AgreementRole> getRoles() {
return roles;
}
public void setRoles(final SortedSet<AgreementRole> actors) {
this.roles = actors;
}
Remove the following bidir contract methods:
public void addToRoles(final AgreementRole agreementRole) {
// check for no-op
if (agreementRole == null || getRoles().contains(agreementRole)) {
return;
}
// associate new
getRoles().add(agreementRole);
agreementRole.setAgreement(this);
}
public void removeFromRoles(final AgreementRole agreementRole) {
// check for no-op
if (agreementRole == null || !getRoles().contains(agreementRole)) {
return;
}
// dissociate existing
getRoles().remove(agreementRole);
agreementRole.setAgreement(null);
}
AgreementRole.java:
No changes on field, getter, setter and annotations (despite the
@javax.jdo.annotations.Column(name = "AGREEMENT_ID") is optional, just for
changing the name of the generated table column, isn't it?):
@javax.jdo.annotations.Column(name = "AGREEMENT_ID")
private Agreement<?> agreement;
@Title(sequence = "3", prepend = ":")
@MemberOrder(sequence = "1")
@Hidden(where = Where.REFERENCES_PARENT)
public Agreement<?> getAgreement() {
return agreement;
}
public void setAgreement(final Agreement<?> agreement) {
this.agreement = agreement;
}
Remove the bidir contract methods:
public void modifyAgreement(final Agreement<?> agreement) {
Agreement<?> currentAgreement = getAgreement();
// check for no-op
if (agreement == null || agreement.equals(currentAgreement)) {
return;
}
// delegate to parent to associate
if (currentAgreement != null) {
currentAgreement.removeFromRoles(this);
}
agreement.addToRoles(this);
}
public void clearAgreement() {
Agreement<?> currentAgreement = getAgreement();
// check for no-op
if (currentAgreement == null) {
return;
}
// delegate to parent to dissociate
currentAgreement.removeFromRoles(this);
}
So, basically, with just the field, getter, setter and required DN annotations
on each side would be enough for production system and integration tests (if
the collection is a Set or SortedSet), wouldn't it?
As I told at the beginning, also for unit tests? It would avoid to load all DN
stuff. I've seen your InMemoryDB class and the blog post commenting about it,
but still not sure about if it's going to be enough for testing Domain behavior
/ actions.
As you are currently making the tests on Estatio, which kind of tests must be
done as @integration with current Isis implementation, and for which ones the
faster @unit scope can be used?
We are going to start to make some tests regarding relationships (including 1-1
relationships, with needed flush() in our previous tests). I'll keep you
informed.
Thanks,
Oscar
El 17/07/2013, a las 08:20, Dan Haywood <[email protected]>
escribió:
> On 15 July 2013 12:42, Dan Haywood <[email protected]> wrote:
>
>>
>> On 12 July 2013 10:27, GESCONSULTOR - Óscar Bou
>> <[email protected]>wrote:
>>
>>>
>>>
>>
>>> Please, what do you think? If you are not in favor of implementing
>>> managed relationships, what are the reasons in order to properly understand
>>> it?
>>>
>>
>> [snip]
>> rather than get Isis to do this, perhaps all we need to do is to figure
>> out a way to make JDO do the heavy lifting for us. All the JDO docs [4]
>> are a little confusing, it does seem to me that the combination of using
>> Sets/SortedSets and calling DOC#flush() ought to do the trick.
>>
>> Could you do some experiments on that and report back?
>>
>>>
>>>
> Just wanted to follow up on this. In writing the Agreement#addRole BDD
> specs for Estatio, I hit an issue with one of them [5] whereby I appeared
> to get duplicate AgreementRole's in the Lease's roles collection. You can
> reproduce this issue by changing the setXxx() to modifyXxx() in [6], and
> also by commenting out the nextTransaction() method in [7].
>
> To explain this a little more: the nextTransaction() method causes the
> transaction for the "when" (the addRole) to be committed, so that the
> assertions in the "then" start afresh. If the nextTransaction is present,
> then everything is ok.
>
> If the nextTransaction is omitted, then the "then" assertions are performed
> in the same transaction as the "when". If the addRole does a modifyXxx()
> rather than a setXxx(), then what I found was that the DataNucleus-provided
> implementation of SortedSet was no such thing: the AgreementRole for POISON
> was actually in there twice!
>
> What I think happens is that JDO/DN *does* manage the relationship, ie,
> updates the set as a side-effect of the calling
> AgreementRole#setAgreement(...). But, when calling modifyAgreement(...)
> instead of setAgreement(...), in effect Isis also puts the object into the
> so-called "set".
>
> What is odd is that it is only this one scenario that fails in this way;
> other very similar scenarios go through smoothly. Slightly worrying.
>
> But anyway, for now my advice is this:
> a) for Sets and SortedSets, you MUST use setXxx rather than modifyXxx
> b) you can (indeed must) remove all those modify/addTo helper methods.
> (Which is what you were hoping to do in the first place...)
> c) there is no need to call DomainObjectContainer#flush(); that was a red
> herring. In my experiments DN seems to update the object immediately.
>
> If you (for some perverse reason) decide that you do want to stick to using
> modifyXxx, then make sure you don't touch the collection thereafter in the
> same transaction.
>
> Please let me know if you find something that contradicts the above.
>
> Thx
> Dan
>
>
> [4]
>> http://www.datanucleus.org/products/datanucleus/jdo/orm/relationships.html
>>
>> [5]
> https://github.com/estatio/estatio/blob/63b6df1be801a33dd73884b460b986278e1a5715/integtests/src/test/java/org/estatio/integration/specs/agreement/AgreementSpec_manageRoles.feature#L172
> [6]
> https://github.com/estatio/estatio/blob/63b6df1be801a33dd73884b460b986278e1a5715/dom/src/main/java/org/estatio/dom/agreement/Agreement.java#L479
> [7]
> https://github.com/estatio/estatio/blob/63b6df1be801a33dd73884b460b986278e1a5715/integtests/src/test/java/org/estatio/integration/specs/agreement/AgreementStepDefs.java#L228
