-
Notifications
You must be signed in to change notification settings - Fork 27
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
OWLAP-51: Effectivetime restore updates #338
Conversation
... on property changes
…lationship-support' into feature/OWLAP-51_effectivetime_restore_updates
... relationship component types
&& memberToRestore.getCharacteristicTypeId().equals(previousMember.getProperties().get(SnomedRf2Headers.FIELD_CHARACTERISTIC_TYPE_ID)) | ||
&& memberToRestore.getTypeId().equals(previousMember.getProperties().get(SnomedRf2Headers.FIELD_TYPE_ID)) | ||
&& memberToRestore.getGroup() == ((Integer )previousMember.getProperties().get(SnomedRf2Headers.FIELD_RELATIONSHIP_GROUP)).intValue(); | ||
} else if(componentToRestore instanceof SnomedDescriptionTypeRefSetMember) { |
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.
Fix spacing, please! :)
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.
This doesn't look the same inside my IDE.
final String previousRuleStrengthId = (String) previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_RULE_STRENGTH_ID); | ||
final String previousContentTypeId = (String) previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_CONTENT_TYPE_ID); | ||
|
||
if (previousdGrouped != null && previousdGrouped.booleanValue() != memberToRestore.isGrouped()) { |
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.
Typo!
return false; | ||
} | ||
|
||
if (previousAtributeInGroupCardinality != null && !previousAtributeInGroupCardinality.equals(memberToRestore.getAttributeInGroupCardinality())) { |
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.
Typo!
final String previousDomainTemplateForPostcoordination = (String) previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_DOMAIN_TEMPLATE_FOR_POSTCOORDINATION); | ||
final String previousEditorialGuideReference = (String) previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_EDITORIAL_GUIDE_REFERENCE); | ||
|
||
if (previousDomainConstraint != null && !previousDomainConstraint.equals(memberToRestore.getDomainConstraint())) { |
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.
Use java.util.Objects.equals()
to eliminate unnecessary null
checks.
} | ||
|
||
// XXX: Should never happen | ||
throw new IllegalStateException(); |
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.
In this case, throw an UnsupportedOperationException
or IllegalArgumentException
with a proper message.
if (componentsByType.isEmpty()) return; | ||
|
||
final List<String> branchesForPreviousVersion = getAvailableVersionPaths(); | ||
for (String branch : branchesForPreviousVersion) { |
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.
This list of branches provided here has the following restrictions:
- the previous version of the component must exist on at least one branch. if we can't find it on any of the branches then we should propagate that upwards. Thoughts @cmark?
- it is not guaranteed that the component exists on all branches
- if we find the component on a branch then we must not look further on the next available branch
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.
I might even change the two for loops here. First iterate over types and then branches?
return memberToRestore.isActive() == previousMember.isActive() && memberToRestore.getModuleId().equals(previousMember.getModuleId()); | ||
} else if (componentToRestore instanceof SnomedMRCMAttributeDomainRefSetMember) { | ||
final SnomedMRCMAttributeDomainRefSetMember memberToRestore = (SnomedMRCMAttributeDomainRefSetMember) componentToRestore; | ||
final Boolean previousdGrouped = ClassUtils.checkAndCast(previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_GROUPED), Boolean.class); |
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.
Typo!
final SnomedMRCMAttributeDomainRefSetMember memberToRestore = (SnomedMRCMAttributeDomainRefSetMember) componentToRestore; | ||
final Boolean previousdGrouped = ClassUtils.checkAndCast(previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_GROUPED), Boolean.class); | ||
final String previousAttributeCardinality = (String) previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_ATTRIBUTE_CARDINALITY); | ||
final String previousAtributeInGroupCardinality = (String) previousMember.getProperties().get(SnomedRf2Headers.FIELD_MRCM_ATTRIBUTE_IN_GROUP_CARDINALITY); |
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.
Typo!
} | ||
} else { | ||
throw new UnexpectedTypeException("Unexpected member type '" + componentToRestore.getClass() + "'."); | ||
} |
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.
That's quite a long method but we are on a time schedule now, so I guess it should be ok.
@@ -791,8 +791,11 @@ public void preCommit() { | |||
} | |||
} | |||
} | |||
|
|||
new EffectiveTimeRestorer(getBranch()).restoreEffectiveTimes(transaction.getChangeSetData().getChangedObjects().stream() |
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.
Why not instantiate the restorer in the constructor and just pass the branch to the method?
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.
LGTM!
This pull request adds functionality to restore effective time on members and components.