88599: WIP: Fix place algorithm

dspace-api/src/main/java/org/dspace/content/RelationshipServiceImpl.java

@@ -227,10 +227,10 @@ public class RelationshipServiceImpl implements RelationshipService {
         Item rightItem = relationship.getRightItem();
 
         List<Relationship> leftRelationships = findByItemAndRelationshipType(
-            context, leftItem, relationship.getRelationshipType(), true
+            context, leftItem, relationship.getRelationshipType(), true, -1, -1, false
         );
         List<Relationship> rightRelationships = findByItemAndRelationshipType(
-            context, rightItem, relationship.getRelationshipType(), false
+            context, rightItem, relationship.getRelationshipType(), false, -1, -1, false
         );
 
         // These relationships are only deleted from the temporary lists in case they're present in them so that we can
@@ -250,6 +250,7 @@ public class RelationshipServiceImpl implements RelationshipService {
         int oldLeftPlace = relationship.getLeftPlace();
         int oldRightPlace = relationship.getRightPlace();
 
+
         boolean movedUpLeft = resolveRelationshipPlace(
             relationship, true, leftRelationships, leftMetadata, oldLeftPlace, newLeftPlace
         );
@@ -259,14 +260,18 @@ public class RelationshipServiceImpl implements RelationshipService {
 
         context.turnOffAuthorisationSystem();
 
+        if (otherSideIsLatest(true, relationship.getLatestVersionStatus())) {
             shiftSiblings(
                 relationship, true, oldLeftPlace, movedUpLeft, insertLeft, deletedFromLeft,
                 leftRelationships, leftMetadata
             );
+        }
+        if (otherSideIsLatest(false, relationship.getLatestVersionStatus())) {
             shiftSiblings(
                 relationship, false, oldRightPlace, movedUpRight, insertRight, deletedFromRight,
                 rightRelationships, rightMetadata
             );
+        }
 
         updateItem(context, leftItem);
         updateItem(context, rightItem);
@@ -474,6 +479,22 @@ public class RelationshipServiceImpl implements RelationshipService {
         }
     }
 
+    /**
+     * Given a latest version status, check if the other side is "latest".
+     * If we look from the left, this implies BOTH and RIGHT_ONLY return true.
+     * If we look from the right, this implies BOTH and LEFT_ONLY return true.
+     * @param isLeft whether we should look from the left or right side.
+     * @param latestVersionStatus the latest version status.
+     * @return true if the other side has "latest" status, false otherwise.
+     */
+    private boolean otherSideIsLatest(boolean isLeft, LatestVersionStatus latestVersionStatus) {
+        if (latestVersionStatus == LatestVersionStatus.BOTH) {
+            return true;
+        }
+
+        return latestVersionStatus == (isLeft ? LatestVersionStatus.RIGHT_ONLY : LatestVersionStatus.LEFT_ONLY);
+    }
+
     private int getPlace(Relationship relationship, boolean isLeft) {
         if (isLeft) {
             return relationship.getLeftPlace();

dspace-api/src/main/java/org/dspace/versioning/DefaultItemVersionProvider.java

@@ -148,12 +148,8 @@ public class DefaultItemVersionProvider extends AbstractVersionProvider implemen
                     newItem,  // new item
                     oldRelationship.getRightItem(),
                     oldRelationship.getRelationshipType(),
-                    // NOTE: on the side of the new version, we start with an empty list of relationships
-                    //       => insert at the same position as the ancestral relationship
                     oldRelationship.getLeftPlace(),
-                    // NOTE: on the opposite side of the new version, the ancestral relationship already takes our
-                    //       desired place => insert AFTER the ancestral relationship
-                    oldRelationship.getRightPlace() + 1,
+                    oldRelationship.getRightPlace(),
                     oldRelationship.getLeftwardValue(),
                     oldRelationship.getRightwardValue(),
                     Relationship.LatestVersionStatus.RIGHT_ONLY // only mark the opposite side as "latest" for now
@@ -165,11 +161,7 @@ public class DefaultItemVersionProvider extends AbstractVersionProvider implemen
                     oldRelationship.getLeftItem(),
                     newItem, // new item
                     oldRelationship.getRelationshipType(),
-                    // NOTE: on the opposite side of the new version, the ancestral relationship already takes our
-                    //       desired place => insert AFTER the ancestral relationship
-                    oldRelationship.getLeftPlace() + 1,
-                    // NOTE: on the side of the new version, we start with an empty list of relationships
-                    //       => insert at the same position as the ancestral relationship
+                    oldRelationship.getLeftPlace(),
                     oldRelationship.getRightPlace(),
                     oldRelationship.getLeftwardValue(),
                     oldRelationship.getRightwardValue(),

dspace-api/src/test/java/org/dspace/content/VersioningWithRelationshipsTest.java

@@ -156,7 +156,7 @@ public class VersioningWithRelationshipsTest extends AbstractIntegrationTestWith
     }
 
     @Test
-    public void test_createNewVersionOfItemOnLeftSideOfRelationships() throws Exception {
+    public void test_createNewVersionOfItemOnLeftSideOfRelationships() throws Exception { // TODO
         ///////////////////////////////////////////////
         // create a publication with 3 relationships //
         ///////////////////////////////////////////////
@@ -430,7 +430,7 @@ public class VersioningWithRelationshipsTest extends AbstractIntegrationTestWith
     }
 
     @Test
-    public void test_createNewVersionOfItemAndModifyRelationships() throws Exception {
+    public void test_createNewVersionOfItemAndModifyRelationships() throws Exception { // TODO
         ///////////////////////////////////////////////
         // create a publication with 3 relationships //
         ///////////////////////////////////////////////
@@ -792,7 +792,7 @@ public class VersioningWithRelationshipsTest extends AbstractIntegrationTestWith
     }
 
     @Test
-    public void test_createNewVersionOfItemOnRightSideOfRelationships() throws Exception {
+    public void test_createNewVersionOfItemOnRightSideOfRelationships() throws Exception { // TODO
         //////////////////////////////////////////
         // create a person with 3 relationships //
         //////////////////////////////////////////
@@ -1066,7 +1066,7 @@ public class VersioningWithRelationshipsTest extends AbstractIntegrationTestWith
     }
 
     @Test
-    public void test_createNewVersionOfItemAndVerifyMetadataOrder() throws Exception {
+    public void test_createNewVersionOfItemAndVerifyMetadataOrder() throws Exception { // TODO
         /////////////////////////////////////////
         // create a publication with 6 authors //
         /////////////////////////////////////////
@@ -1341,7 +1341,7 @@ public class VersioningWithRelationshipsTest extends AbstractIntegrationTestWith
     }
 
     @Test
-    public void test_virtualMetadataPreserved() throws Exception {
+    public void test_virtualMetadataPreserved() throws Exception { // TODO
         //////////////////////////////////////////////
         // create a publication and link two people //
         //////////////////////////////////////////////
@@ -1477,7 +1477,7 @@ public class VersioningWithRelationshipsTest extends AbstractIntegrationTestWith
         assertEquals(2, mdvs5.size());
 
         assertTrue(mdvs5.get(0) instanceof RelationshipMetadataValue);
-        assertEquals("Smith, D.", mdvs5.get(0).getValue());
+        assertEquals("Smith, D.", mdvs5.get(0).getValue());// TODO fix
         assertEquals(0, mdvs5.get(0).getPlace());
 
         assertTrue(mdvs5.get(1) instanceof RelationshipMetadataValue);