ToMany: fix removal of non-persisted entities for stand-alone relations (removeStandaloneRelations removes non-persisted entities first)

greenrobot · greenrobot · commit 288d3e0415b5 · 2018-01-21T20:30:56.000+01:00
diff --git a/objectbox-java/src/main/java/io/objectbox/relation/ToMany.java b/objectbox-java/src/main/java/io/objectbox/relation/ToMany.java
@@ -727,7 +727,7 @@ public void internalApplyToDb(Cursor sourceCursor, Cursor<TARGET> targetCursor)
         TARGET[] toRemoveFromDb;
         TARGET[] toPut;
         TARGET[] addedStandalone = null;
-        TARGET[] removedStandalone = null;
+        List<TARGET> removedStandalone = null;
 
         boolean isStandaloneRelation = relationInfo.relationId != 0;
         IdGetter<TARGET> targetIdGetter = relationInfo.targetInfo.getIdGetter();
@@ -746,7 +746,7 @@ public void internalApplyToDb(Cursor sourceCursor, Cursor<TARGET> targetCursor)
                     entitiesAdded.clear();
                 }
                 if (!entitiesRemoved.isEmpty()) {
-                    removedStandalone = (TARGET[]) entitiesRemoved.keySet().toArray();
+                    removedStandalone = new ArrayList<>(entitiesRemoved.keySet());
                     entitiesRemoved.clear();
                 }
             }
@@ -777,26 +777,51 @@ public void internalApplyToDb(Cursor sourceCursor, Cursor<TARGET> targetCursor)
                 throw new IllegalStateException("Source entity has no ID (should have been put before)");
             }
 
-            checkModifyStandaloneRelation(sourceCursor, entityId, removedStandalone, targetIdGetter, true);
-            checkModifyStandaloneRelation(sourceCursor, entityId, addedStandalone, targetIdGetter, false);
+            if (removedStandalone != null) {
+                removeStandaloneRelations(sourceCursor, entityId, removedStandalone, targetIdGetter);
+            }
+            if (addedStandalone != null) {
+                addStandaloneRelations(sourceCursor, entityId, addedStandalone, targetIdGetter, false);
+            }
         }
     }
 
-    private void checkModifyStandaloneRelation(Cursor cursor, long sourceEntityId, @Nullable TARGET[] targets,
-                                               IdGetter<TARGET> targetIdGetter, boolean remove) {
-        if (targets != null) {
-            int length = targets.length;
-            long[] targetIds = new long[length];
-            for (int i = 0; i < length; i++) {
-                long targetId = targetIdGetter.getId(targets[i]);
-                if (targetId == 0) {
-                    // Paranoia
-                    throw new IllegalStateException("Target entity has no ID (should have been put before)");
-                }
-                targetIds[i] = targetId;
+    /**
+     * The list of removed entities may contain non-persisted entities, which will be ignored (removed from the list).
+     */
+    private void removeStandaloneRelations(Cursor cursor, long sourceEntityId, List<TARGET> removed,
+                                           IdGetter<TARGET> targetIdGetter) {
+        Iterator<TARGET> iterator = removed.iterator();
+        while (iterator.hasNext()) {
+            if (targetIdGetter.getId(iterator.next()) == 0) {
+                iterator.remove();
+            }
+        }
+
+        int size = removed.size();
+        if (size > 0) {
+            long[] targetIds = new long[size];
+            for (int i = 0; i < size; i++) {
+                targetIds[i] = targetIdGetter.getId(removed.get(i));
+            }
+            cursor.modifyRelations(relationInfo.relationId, sourceEntityId, targetIds, true);
+        }
+    }
+
+    /** The target array may not contain non-persisted entities. */
+    private void addStandaloneRelations(Cursor cursor, long sourceEntityId, @Nullable TARGET[] added,
+                                        IdGetter<TARGET> targetIdGetter, boolean remove) {
+        int length = added.length;
+        long[] targetIds = new long[length];
+        for (int i = 0; i < length; i++) {
+            long targetId = targetIdGetter.getId(added[i]);
+            if (targetId == 0) {
+                // Paranoia
+                throw new IllegalStateException("Target entity has no ID (should have been put before)");
             }
-            cursor.modifyRelations(relationInfo.relationId, sourceEntityId, targetIds, remove);
+            targetIds[i] = targetId;
         }
+        cursor.modifyRelations(relationInfo.relationId, sourceEntityId, targetIds, remove);
     }
 
     /** For tests */
diff --git a/tests/objectbox-java-test/src/main/java/io/objectbox/relation/ToManyStandaloneTest.java b/tests/objectbox-java-test/src/main/java/io/objectbox/relation/ToManyStandaloneTest.java
@@ -219,6 +219,17 @@ public void testRemove() {
         assertOrder2And4Removed(count, customer, toMany);
     }
 
+    @Test
+    public void testAddRemove_notPersisted() {
+        Customer customer = putCustomer();
+        ToMany<Order> toMany = customer.ordersStandalone;
+        Order order = new Order();
+        toMany.add(order);
+        toMany.remove(order);
+        customerBox.put(customer);
+        assertEquals(0, orderBox.count());
+    }
+
     @Test
     public void testRemoveAll() {
         int count = 5;
