From 71c6b44b212a7c904b0abc4131f777338e5d4b1e Mon Sep 17 00:00:00 2001 From: Darryn McGaw Date: Mon, 6 Feb 2023 17:45:27 +0000 Subject: [PATCH] FRI-492 Remove logic that upgrades components to latest version on RHS of merge FRI-492 Delete unused methods --- .../data/services/BranchReviewService.java | 82 ------------------- .../data/services/BranchMergeServiceTest.java | 62 ++++++-------- 2 files changed, 27 insertions(+), 117 deletions(-) diff --git a/src/main/java/org/snomed/snowstorm/core/data/services/BranchReviewService.java b/src/main/java/org/snomed/snowstorm/core/data/services/BranchReviewService.java index 590bdf05d..e29ecc281 100644 --- a/src/main/java/org/snomed/snowstorm/core/data/services/BranchReviewService.java +++ b/src/main/java/org/snomed/snowstorm/core/data/services/BranchReviewService.java @@ -170,9 +170,6 @@ public Collection getMergeReviewConflictingConcepts( if (sourceVersion != null && targetVersion != null) { // Neither deleted, auto-merge. mergeVersion.setAutoMergedConcept(autoMergeConcept(sourceVersion, targetVersion)); - - // Upgrade components if behind a release/version - mergeVersion.setTargetConcept(upgradeComponents(sourceVersion, targetVersion)); } conflicts.put(conceptId, mergeVersion); } @@ -231,85 +228,6 @@ public void applyMergeReview(MergeReview mergeReview) throws ServiceException { branchMergeService.mergeBranchSync(mergeReview.getSourcePath(), mergeReview.getTargetPath(), concepts); } - private Concept upgradeComponents(Concept sourceVersion, Concept targetVersion) { - // Copy from latest component - Concept winningConcept = sourceVersion.isReleasedMoreRecentlyThan(targetVersion) ? sourceVersion : targetVersion; - - Concept upgradedConcept = new Concept(); - upgradedConcept.setConceptId(winningConcept.getConceptId()); - upgradedConcept.setEffectiveTimeI(winningConcept.getEffectiveTimeI()); - upgradedConcept.setActive(winningConcept.isActive()); - upgradedConcept.setReleased(winningConcept.isReleased()); - upgradedConcept.setReleaseHash(winningConcept.getReleaseHash()); - upgradedConcept.setReleasedEffectiveTime(winningConcept.getReleasedEffectiveTime()); - upgradedConcept.setModuleId(winningConcept.getModuleId()); - upgradedConcept.setDefinitionStatusId(winningConcept.getDefinitionStatusId()); - upgradedConcept.setInactivationIndicator(winningConcept.getInactivationIndicator()); - upgradedConcept.setAssociationTargets(winningConcept.getAssociationTargets()); - upgradedConcept.setDescriptions(mergePublished(sourceVersion.getDescriptions(), targetVersion.getDescriptions())); - upgradedConcept.setRelationships(mergePublished(sourceVersion.getRelationships(), targetVersion.getRelationships())); - upgradedConcept.setClassAxioms(mergePublishedAxioms(sourceVersion.getClassAxioms(), targetVersion.getClassAxioms())); - upgradedConcept.setGciAxioms(mergePublishedAxioms(sourceVersion.getGciAxioms(), sourceVersion.getGciAxioms())); - - return upgradedConcept; - } - - private Set mergePublished(Set sourceComponents, Set targetComponents) { - Map mergedComponents = new HashMap<>(); - for (T sourceComponent : sourceComponents) { - // published and no further changes - if (sourceComponent.getEffectiveTimeI() != null) { - mergedComponents.put(sourceComponent.getId(), sourceComponent); - } - } - - for (T targetComponent : targetComponents) { - String targetComponentId = targetComponent.getId(); - if (mergedComponents.containsKey(targetComponentId)) { - T sourceComponent = mergedComponents.get(targetComponentId); - if (targetComponent.getEffectiveTimeI() == null) { - // target version with change but need to update old release details - targetComponent.setReleasedEffectiveTime(sourceComponent.getReleasedEffectiveTime()); - targetComponent.setReleaseHash(sourceComponent.getReleaseHash()); - mergedComponents.put(targetComponentId, targetComponent); - } - } else { - // component only exists in target - mergedComponents.put(targetComponentId, targetComponent); - } - } - - return new HashSet<>(mergedComponents.values()); - } - - private Set mergePublishedAxioms(Set sourceAxioms, Set targetAxioms) { - Map mergedAxioms = new HashMap<>(); - - for (Axiom sourceAxiom : sourceAxioms) { - if (sourceAxiom.isReleased()) { - mergedAxioms.put(sourceAxiom.getAxiomId(), sourceAxiom); - } - } - - for (Axiom targetAxiom : targetAxioms) { - String targetAxiomId = targetAxiom.getAxiomId(); - Axiom sourceAxiom = mergedAxioms.get(targetAxiomId); - - if (sourceAxiom == null) { - mergedAxioms.put(targetAxiomId, targetAxiom); - } else { - ReferenceSetMember sourceMember = sourceAxiom.getReferenceSetMember(); - ReferenceSetMember targetMember = targetAxiom.getReferenceSetMember(); - - if (!sourceMember.isReleasedMoreRecentlyThan(targetMember)) { - mergedAxioms.put(targetAxiomId, targetAxiom); - } - } - } - - return new HashSet<>(mergedAxioms.values()); - } - private void assertMergeReviewCurrent(MergeReview mergeReview) { if (mergeReview.getStatus() != ReviewStatus.CURRENT) { throw new IllegalStateException("Merge review state is not " + ReviewStatus.CURRENT); diff --git a/src/test/java/org/snomed/snowstorm/core/data/services/BranchMergeServiceTest.java b/src/test/java/org/snomed/snowstorm/core/data/services/BranchMergeServiceTest.java index 45f971ae1..9c4dbcdba 100644 --- a/src/test/java/org/snomed/snowstorm/core/data/services/BranchMergeServiceTest.java +++ b/src/test/java/org/snomed/snowstorm/core/data/services/BranchMergeServiceTest.java @@ -2270,9 +2270,7 @@ void testUpgradeExtensionWhenExtensionTaskHasNoAuthoringChanges() throws Service /* * The Extension upgrades to the latest version of International. After doing the upgrade, the Extension's task then - * does a rebase, which results in a conflict as there are authoring changes. The user selects the RHS of the merge window. - * The latest and greatest International components are present on the task, as well as the working-in-progress Extension - * components. In this particular scenario, the middle & right columns of the merge screen are identical. + * does a rebase, which results in a conflict as there are authoring changes. The user selects the middle of the merge window. * */ @Test void testUpgradeExtensionWhenExtensionHasAuthoringChangesAndUserSelectsRHS() throws ServiceException, InterruptedException { @@ -2337,23 +2335,23 @@ void testUpgradeExtensionWhenExtensionHasAuthoringChangesAndUserSelectsRHS() thr Collection conflicts = reviewService.getMergeReviewConflictingConcepts(review.getId(), new ArrayList<>()); assertEquals(1, conflicts.size()); // Extension has made an authoring change to the same overall component which results in conflict - // 9. User selects right column (International components have been upgraded) + // 9. User selects middle column for (MergeReviewConceptVersions conflict : conflicts) { Concept autoMergedConcept = conflict.getAutoMergedConcept(); Concept targetConcept = conflict.getTargetConcept(); - // 9.a For this scenario, both middle & right columns should have the same data assertEquals(autoMergedConcept.getConceptId(), targetConcept.getConceptId()); - assertEquals(autoMergedConcept.getReleasedEffectiveTime(), targetConcept.getReleasedEffectiveTime()); - assertEquals(autoMergedConcept.getEffectiveTimeI(), targetConcept.getEffectiveTimeI()); - assertEquals(autoMergedConcept.getModuleId(), targetConcept.getModuleId()); + assertEquals(20220228, autoMergedConcept.getReleasedEffectiveTime()); + assertEquals(20220131, targetConcept.getReleasedEffectiveTime()); + assertEquals(MODEL_MODULE, autoMergedConcept.getModuleId()); + assertEquals(CORE_MODULE, targetConcept.getModuleId()); assertEquals(autoMergedConcept.isReleased(), targetConcept.isReleased()); assertEquals(autoMergedConcept.isActive(), targetConcept.isActive()); - assertEquals(autoMergedConcept.getDescriptions().size(), targetConcept.getDescriptions().size()); + assertEquals(autoMergedConcept.getDescriptions().size(), targetConcept.getDescriptions().size() + 1); assertEquals(autoMergedConcept.getRelationships().size(), targetConcept.getRelationships().size()); assertEquals(autoMergedConcept.getAllOwlAxiomMembers().size(), targetConcept.getAllOwlAxiomMembers().size()); - reviewService.persistManuallyMergedConcept(review, Long.parseLong(cinnamonId), targetConcept); + reviewService.persistManuallyMergedConcept(review, Long.parseLong(cinnamonId), autoMergedConcept); } reviewService.applyMergeReview(review); @@ -2401,11 +2399,7 @@ void testUpgradeExtensionWhenExtensionHasAuthoringChangesAndUserSelectsRHS() thr /* * The Extension upgrades to the latest version of International. After doing the upgrade, the Extension's task then - * does a rebase, which results in a conflict as there are authoring changes. In between the Extension upgrading and - * the task rebasing, someone has sneakily promoted an authoring change to the CodeSystem. The user selects the RHS of - * the merge window. The latest and greatest International components are present on the task, as well as the working-in-progress - * Extension TASK components. The components sneakily promoted to the CodeSystem are rejected during the merge. In this - * particular scenario, the middle column contains an additional, unpublished Description. + * does a rebase, which results in a conflict as there are authoring changes. The user selects the right column of the merge window. * */ @Test void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndUserSelectsRHS() throws ServiceException, InterruptedException { @@ -2475,19 +2469,19 @@ void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndU Collection conflicts = reviewService.getMergeReviewConflictingConcepts(review.getId(), new ArrayList<>()); assertEquals(1, conflicts.size()); // Extension has made an authoring change to the same overall component which results in conflict - // 10. User selects right column (International components have been upgraded) + // 10. User selects right column for (MergeReviewConceptVersions conflict : conflicts) { Concept autoMergedConcept = conflict.getAutoMergedConcept(); Concept targetConcept = conflict.getTargetConcept(); - // 10.a For this scenario, both middle & right columns should have the same data assertEquals(autoMergedConcept.getConceptId(), targetConcept.getConceptId()); - assertEquals(autoMergedConcept.getReleasedEffectiveTime(), targetConcept.getReleasedEffectiveTime()); - assertEquals(autoMergedConcept.getEffectiveTimeI(), targetConcept.getEffectiveTimeI()); - assertEquals(autoMergedConcept.getModuleId(), targetConcept.getModuleId()); + assertEquals(20220228, autoMergedConcept.getReleasedEffectiveTime()); + assertEquals(20220131, targetConcept.getReleasedEffectiveTime()); + assertEquals(MODEL_MODULE, autoMergedConcept.getModuleId()); + assertEquals(CORE_MODULE, targetConcept.getModuleId()); assertEquals(autoMergedConcept.isReleased(), targetConcept.isReleased()); assertEquals(autoMergedConcept.isActive(), targetConcept.isActive()); - assertEquals(autoMergedConcept.getDescriptions().size(), targetConcept.getDescriptions().size() + 1); // CodeSystem has the "sneaky" authoring change + assertEquals(autoMergedConcept.getDescriptions().size(), targetConcept.getDescriptions().size() + 2); // CodeSystem has the "sneaky" authoring change assertEquals(autoMergedConcept.getRelationships().size(), targetConcept.getRelationships().size()); assertEquals(autoMergedConcept.getAllOwlAxiomMembers().size(), targetConcept.getAllOwlAxiomMembers().size()); @@ -2500,8 +2494,8 @@ void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndU concept = conceptService.find(cinnamonId, extTask); assertEquals(20220228, concept.getReleasedEffectiveTime()); - assertEquals(20220228, concept.getEffectiveTimeI()); - assertEquals(MODEL_MODULE, concept.getModuleId()); + assertNull(concept.getEffectiveTimeI()); // Because RHS was selected, the "old" state of the Concept becomes the "new" state + assertEquals(CORE_MODULE, concept.getModuleId()); assertTrue(concept.isReleased()); assertTrue(concept.isActive()); @@ -2525,9 +2519,10 @@ void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndU description = getDescription(concept, "Cinnamon swirl"); assertEquals(20220228, description.getReleasedEffectiveTime()); - assertEquals(20220228, description.getEffectiveTimeI()); + assertNull(description.getEffectiveTimeI()); assertTrue(description.isReleased()); - assertTrue(description.isActive()); + assertEquals(extModule, description.getModuleId()); // Because RHS was selected, Description has been inactivated in Extension module. + assertFalse(description.isActive()); description = getDescription(concept, "Kanelbulle"); assertNull(description.getEffectiveTimeI()); @@ -2542,10 +2537,7 @@ void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndU /* * The Extension upgrades to the latest version of International. After doing the upgrade, the Extension's task then - * does a rebase, which results in a conflict as there are authoring changes. In between the Extension upgrading and - * the task rebasing, someone has sneakily promoted an authoring change to the CodeSystem. The user selects the Middle of - * the merge window. The latest and greatest International components are present on the task, as well as the working-in-progress - * Extension TASK and PROJECT components. In this particular scenario, the middle column contains an additional, unpublished Description. + * does a rebase, which results in a conflict as there are authoring changes. The user selects the middle column of the merge window. * */ @Test void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndUserSelectsMiddle() throws ServiceException, InterruptedException { @@ -2615,19 +2607,19 @@ void testUpgradeExtensionWhenExtensionProjectAndTaskBothHaveAuthoringChangesAndU Collection conflicts = reviewService.getMergeReviewConflictingConcepts(review.getId(), new ArrayList<>()); assertEquals(1, conflicts.size()); // Extension has made an authoring change to the same overall component which results in conflict - // 10. User selects right column (International components have been upgraded) + // 10. User selects middle column for (MergeReviewConceptVersions conflict : conflicts) { Concept autoMergedConcept = conflict.getAutoMergedConcept(); Concept targetConcept = conflict.getTargetConcept(); - // 10.a For this scenario, both middle & right columns should have the same data assertEquals(autoMergedConcept.getConceptId(), targetConcept.getConceptId()); - assertEquals(autoMergedConcept.getReleasedEffectiveTime(), targetConcept.getReleasedEffectiveTime()); - assertEquals(autoMergedConcept.getEffectiveTimeI(), targetConcept.getEffectiveTimeI()); - assertEquals(autoMergedConcept.getModuleId(), targetConcept.getModuleId()); + assertEquals(20220228, autoMergedConcept.getReleasedEffectiveTime()); + assertEquals(20220131, targetConcept.getReleasedEffectiveTime()); + assertEquals(MODEL_MODULE, autoMergedConcept.getModuleId()); + assertEquals(CORE_MODULE, targetConcept.getModuleId()); assertEquals(autoMergedConcept.isReleased(), targetConcept.isReleased()); assertEquals(autoMergedConcept.isActive(), targetConcept.isActive()); - assertEquals(autoMergedConcept.getDescriptions().size(), targetConcept.getDescriptions().size() + 1); // CodeSystem has the "sneaky" authoring change + assertEquals(autoMergedConcept.getDescriptions().size(), targetConcept.getDescriptions().size() + 2); assertEquals(autoMergedConcept.getRelationships().size(), targetConcept.getRelationships().size()); assertEquals(autoMergedConcept.getAllOwlAxiomMembers().size(), targetConcept.getAllOwlAxiomMembers().size());