Merge pull request #299 from ncats/fixInverse

Allow inverse relationships to be added even when there is more than 1 with the same type
ncats · Apr 1, 2024 · 6dad1a2 · 6dad1a2
2 parents ff6a675 + 75884dd
commit 6dad1a2
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 6 deletions.
diff --git a/...le-substance-example/src/test/java/example/substance/RelationshipInvertFullStackTest.java b/...le-substance-example/src/test/java/example/substance/RelationshipInvertFullStackTest.java
@@ -659,6 +659,66 @@ public void add2SubstancesWithNoRelationshipThenAddRelationshipShouldResultInBiD
 
     }
 
+
+    @Test
+    public void add2SubstancesWithNoRelationshipThenAdd2RelationshipsOfSameTypeWithDifferentQualifiersShouldResultInBiDirectionalRelationshipsWithExpectedOriginiatorUUID()   throws Exception {
+        UUID uuid1 = UUID.randomUUID();
+        UUID uuid2 = UUID.randomUUID();
+        String foo_bar = "foo->bar";
+        String bar_foo = "bar->foo";
+
+        new SubstanceBuilder()
+                .addName("sub1")
+                .setUUID(uuid1)
+                .buildJsonAnd(this::assertCreatedAPI);
+        new SubstanceBuilder()
+            .addName("sub2")
+            .setUUID(uuid2)
+            .buildJsonAnd(this::assertCreatedAPI);
+
+
+            Substance sub1Fetched = substanceEntityService.get(uuid1).get();
+            assertEquals("1", sub1Fetched.version);
+
+            Substance sub2Fetched = substanceEntityService.get(uuid2).get();
+            assertEquals("1", sub2Fetched.version);
+
+
+            sub1Fetched.toBuilder()
+                    .addRelationshipTo(sub2Fetched, foo_bar, relnew1->relnew1.interactionType="Test123")
+                    .addRelationshipTo(sub2Fetched, foo_bar, relnew2->relnew2.interactionType="Test456")
+                    .buildJsonAnd(this::assertUpdatedAPI);
+
+
+            sub1Fetched = substanceEntityService.get(uuid1).get();
+            assertEquals("2", sub1Fetched.version);
+
+            sub2Fetched = substanceEntityService.get(uuid2).get();
+//            assertEquals("2", sub2Fetched.version);
+
+            assertEquals(2, sub1Fetched.relationships.size());
+            assertEquals(2, sub2Fetched.relationships.size());
+
+            assertEquals(uuid2.toString(), sub1Fetched.relationships.get(0).relatedSubstance.refuuid);
+            assertEquals(foo_bar, sub1Fetched.relationships.get(0).type);
+
+            assertEquals(uuid2.toString(), sub1Fetched.relationships.get(1).relatedSubstance.refuuid);
+            assertEquals(foo_bar, sub1Fetched.relationships.get(1).type);
+
+            String oid1 = sub1Fetched.relationships.get(0).originatorUuid;
+            String oid2 = sub2Fetched.relationships.get(0).originatorUuid;
+            assertEquals(uuid1.toString(), sub2Fetched.relationships.get(0).relatedSubstance.refuuid);
+            assertEquals(bar_foo, sub2Fetched.relationships.get(0).type);
+            assertEquals(oid1, oid2);
+
+            String oid1b = sub1Fetched.relationships.get(1).originatorUuid;
+            String oid2b = sub2Fetched.relationships.get(1).originatorUuid;
+            assertEquals(uuid1.toString(), sub2Fetched.relationships.get(1).relatedSubstance.refuuid);
+            assertEquals(bar_foo, sub2Fetched.relationships.get(1).type);
+            assertEquals(oid1b, oid2b);
+
+    }
+
     /*
      * 1. Create a new alternative definition and link back to some primary definition
         1.1. Confirm it makes the new alt record [yes]

diff --git a/gsrs-module-substance-example/src/test/java/example/substance/RelationshipInvertTest.java b/gsrs-module-substance-example/src/test/java/example/substance/RelationshipInvertTest.java
@@ -380,7 +380,7 @@ public void addRelationshipAfterAddingEachSubstanceShouldAddInvertedRelationship
         assertEquals(1, createInverseEvents.size());
         assertEquals(TryToCreateInverseRelationshipEvent.builder()
                 .relationshipIdToInvert(updatedSubstance.relationships.get(0).uuid)
-                .creationMode(TryToCreateInverseRelationshipEvent.CreationMode.CREATE_IF_MISSING)
+                .creationMode(TryToCreateInverseRelationshipEvent.CreationMode.CREATE_IF_MISSING_DEEP_CHECK)
                 .fromSubstance(UUID.fromString(uuidA))
                 .toSubstance(updatedSubstance.uuid)
                 .originatorUUID(updatedSubstance.relationships.get(0).uuid)

diff --git a/...substances-core/src/main/java/gsrs/module/substance/processors/RelationshipProcessor.java b/...substances-core/src/main/java/gsrs/module/substance/processors/RelationshipProcessor.java
@@ -98,7 +98,7 @@ public void prePersist(Relationship thisRelationship) {
 	            if (otherSubstanceReference != null && otherSubstanceReference.refuuid !=null) {
 	                event.setFromSubstance(UUID.fromString(otherSubstanceReference.refuuid));
 	            }
-	            event.setCreationMode(TryToCreateInverseRelationshipEvent.CreationMode.CREATE_IF_MISSING);
+	            event.setCreationMode(TryToCreateInverseRelationshipEvent.CreationMode.CREATE_IF_MISSING_DEEP_CHECK);
 	            eventPublisher.publishEvent(event);
 
 	        });

diff --git a/...le-substances-core/src/main/java/gsrs/module/substance/processors/SubstanceProcessor.java b/...le-substances-core/src/main/java/gsrs/module/substance/processors/SubstanceProcessor.java
@@ -105,7 +105,7 @@ private void addWaitingRelationships(Substance obj){
             if(owner!=null) {
                 eventPublisher.publishEvent(
                         TryToCreateInverseRelationshipEvent.builder()
-                                .creationMode(TryToCreateInverseRelationshipEvent.CreationMode.CREATE_IF_MISSING)
+                                .creationMode(TryToCreateInverseRelationshipEvent.CreationMode.CREATE_IF_MISSING_DEEP_CHECK)
                                 .originatorUUID(UUID.fromString(r.originatorUuid))
                                 .toSubstance(owner.uuid)
                                 .fromSubstance(obj.uuid)

diff --git a/...e/src/main/java/gsrs/module/substance/processors/TryToCreateInverseRelationshipEvent.java b/...e/src/main/java/gsrs/module/substance/processors/TryToCreateInverseRelationshipEvent.java
@@ -32,7 +32,7 @@ public class TryToCreateInverseRelationshipEvent {
     private UUID originatorUUID;
     private UUID relationshipIdToInvert;
 
-    private CreationMode creationMode = CreationMode.CREATE_IF_MISSING;
+    private CreationMode creationMode = CreationMode.CREATE_IF_MISSING_DEEP_CHECK;
 
     private UUID newRelationshipId;
 
@@ -42,7 +42,18 @@ public enum CreationMode{
             @Override
             public boolean shouldAdd(Relationship r, Substance from, Substance to){
                 for (Relationship rOld : from.relationships) {
-                    if (r.type.equals(rOld.type) && r.relatedSubstance.isEquivalentTo(rOld.relatedSubstance)) {
+                    if (r.isEquivalentBaseRelationship(rOld)) {
+                        return false;
+                    }
+                }
+                return true;
+            }
+        },
+        CREATE_IF_MISSING_DEEP_CHECK{
+            @Override
+            public boolean shouldAdd(Relationship r, Substance from, Substance to){
+                for (Relationship rOld : from.relationships) {
+                    if (r.isEquivalentFullRelationship(rOld)) {
                         return false;
                     }
                 }

diff --git a/...s-core/src/main/java/gsrs/module/substance/processors/UpdateInverseRelationshipEvent.java b/...s-core/src/main/java/gsrs/module/substance/processors/UpdateInverseRelationshipEvent.java
@@ -39,7 +39,7 @@ public TryToCreateInverseRelationshipEvent toCreateEvent() {
     	// set the from substance AND to substance to be the same, causing unintentional self-referencing
     	// relationships. This is fixed in 3.0.3.
         return TryToCreateInverseRelationshipEvent.builder()
-         .creationMode(CreationMode.CREATE_IF_MISSING)
+         .creationMode(CreationMode.CREATE_IF_MISSING_DEEP_CHECK)
          .relationshipIdToInvert(relationshipIdThatWasUpdated)
          .originatorUUID(originatorUUID)
          .fromSubstance(substanceIdToUpdate)

diff --git a/...module-substances-core/src/main/java/ix/ginas/modelBuilders/AbstractSubstanceBuilder.java b/...module-substances-core/src/main/java/ix/ginas/modelBuilders/AbstractSubstanceBuilder.java
@@ -421,9 +421,14 @@ public T setToPublic(){
     }
 
     public T addRelationshipTo(Substance relatedSubstance, String type){
+        return addRelationshipTo(relatedSubstance,type,(rr)->{});
+    }
+    public T addRelationshipTo(Substance relatedSubstance, String type, Consumer<Relationship> mutate){
         Relationship rel = new Relationship();
         rel.type = type;
         rel.relatedSubstance = relatedSubstance.asSubstanceReference();
+        mutate.accept(rel);
+
         return addRelationship(rel);
     }
 

diff --git a/gsrs-module-substances-core/src/main/java/ix/ginas/models/v1/Relationship.java b/gsrs-module-substances-core/src/main/java/ix/ginas/models/v1/Relationship.java
@@ -12,8 +12,12 @@
 import ix.ginas.models.utils.RelationshipUtil;
 
 import javax.persistence.*;
+
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 
 
 @JSONEntity(title = "Relationship", isFinal = true)
@@ -188,6 +192,28 @@ public boolean isEquivalentBaseRelationship(Relationship other){
     	return false;
     }
 
+    @JsonIgnore
+    public boolean isEquivalentFullRelationship(Relationship other){
+    	if (other.isEquivalentBaseRelationship(this)) {
+    		if(		(other.comments+"").equals(this.comments+"") && 
+    				(other.qualification+"").equals(this.qualification+"") && 
+    				(other.interactionType+"").equals(this.interactionType+"") && 
+    				(Optional.ofNullable(other.amount).map(oa -> oa.toString()).orElse("")
+    						.equals(
+    				 Optional.ofNullable(this.amount).map(oa -> oa.toString()).orElse("")))) {
+    			if((this.mediatorSubstance==null && other.mediatorSubstance==null)) {
+    				return true;
+    			}else if(this.mediatorSubstance!=null && other.mediatorSubstance!=null) {
+    				return this.mediatorSubstance.isEquivalentTo(other.mediatorSubstance);
+    			}else {
+    				return false;
+    			}
+
+    		}
+		}
+    	return false;
+    }
+
     public String toSimpleString(){
     	return type + ":" + relatedSubstance.refPname;
     }