[Bug #2] Fixed most of the merging issues.

jopa-api/src/main/java/cz/cvut/kbss/jopa/sessions/CloneBuilder.java

@@ -53,12 +53,21 @@ Object buildClone(Object cloneOwner, Field clonedField, Object original,
                       Descriptor descriptor);
 
     /**
-     * Resets the clone builder. Especially resets the visited objects cache to
-     * make sure all the clones are built from scratch and are not affected by
-     * the previously built ones.
+     * Resets the clone builder.
+     * <p>
+     * Especially resets the visited objects cache to make sure all the clones are built from scratch and are not
+     * affected by the previously built ones.
      */
     void reset();
 
+    /**
+     * Removes the specified instance from the clone builder's visited entities cache.
+     *
+     * @param instance   The instance to remove (original object).
+     * @param descriptor Instance descriptor
+     */
+    void removeVisited(Object instance, Descriptor descriptor);
+
     /**
      * Merges the changes on clone into the original object.
      *

jopa-impl/src/main/java/cz/cvut/kbss/jopa/model/EntityManagerImpl.java

@@ -160,53 +160,48 @@ private <T> T mergeInternal(final T entity, final Descriptor descriptor) {
         switch (getState(entity, descriptor)) {
             case MANAGED_NEW:
             case MANAGED:
-                new OneLevelCascadeExplorer() {
+                new OneLevelMergeCascadeExplorer() {
                     @Override
-                    protected void exploreCascaded(Attribute<?, ?> at, Object o) {
-                        mergeX(at, o, descriptor);
+                    protected void exploreCascaded(Attribute<?, ?> at, Object merged, Object toMerge) {
+                        final Descriptor attDescriptor = descriptor.getAttributeDescriptor(at);
+                        mergeX(at, merged, toMerge, attDescriptor);
                     }
-                }.start(this, entity, CascadeType.MERGE);
+                }.start(this, entity, entity);
                 return entity;
             case NOT_MANAGED:
                 final T merged;
                 merged = getCurrentPersistenceContext().mergeDetached(entity, descriptor);
 
-                new OneLevelCascadeExplorer() {
+                new OneLevelMergeCascadeExplorer() {
                     @Override
-                    protected void exploreCascaded(Attribute<?, ?> at, Object o) {
+                    protected void exploreCascaded(Attribute<?, ?> at, Object merged, Object toMerge) {
                         final Descriptor attDescriptor = descriptor.getAttributeDescriptor(at);
-                        mergeX(at, o, attDescriptor);
-                    }
-
-                    @Override
-                    protected void exploreNonCascaded(Attribute<?, ?> at, Object o) {
-                        final Object attVal = EntityPropertiesUtils.getAttributeValue(at, o);
-                        EntityPropertiesUtils.setFieldValue(at.getJavaField(), o, attVal);
+                        mergeX(at, merged, toMerge, attDescriptor);
                     }
-                }.start(this, merged, CascadeType.MERGE);
+                }.start(this, merged, entity);
                 return merged;
             case REMOVED:
             default:
                 throw new IllegalArgumentException();
         }
     }
 
-    private void mergeX(Attribute<?, ?> at, Object o, Descriptor descriptor) {
-        Object attVal = EntityPropertiesUtils.getAttributeValue(at, o);
+    private void mergeX(Attribute<?, ?> at, Object merged, Object toMerge, Descriptor descriptor) {
+        Object attVal = EntityPropertiesUtils.getAttributeValue(at, toMerge);
         if (attVal == null) {
             return;
         }
         if (at.isCollection()) {
             Collection c = (Collection) attVal;
-            Collection merged = CollectionFactory.createInstance(c);
+            Collection result = CollectionFactory.createInstance(c);
             for (final Object ox2 : c) {
-                merged.add(mergeInternal(ox2, descriptor));
+                result.add(mergeInternal(ox2, descriptor));
             }
-            attVal = getCurrentPersistenceContext().createIndirectCollection(merged, o, at.getJavaField());
+            attVal = getCurrentPersistenceContext().createIndirectCollection(result, merged, at.getJavaField());
         } else {
             attVal = mergeInternal(attVal, descriptor);
         }
-        EntityPropertiesUtils.setFieldValue(at.getJavaField(), o, attVal);
+        EntityPropertiesUtils.setFieldValue(at.getJavaField(), merged, attVal);
     }
 
     @Override

jopa-impl/src/main/java/cz/cvut/kbss/jopa/model/OneLevelMergeCascadeExplorer.java

@@ -0,0 +1,39 @@
+package cz.cvut.kbss.jopa.model;
+
+import cz.cvut.kbss.jopa.exceptions.OWLPersistenceException;
+import cz.cvut.kbss.jopa.model.annotations.CascadeType;
+import cz.cvut.kbss.jopa.model.metamodel.Attribute;
+import cz.cvut.kbss.jopa.model.metamodel.EntityType;
+
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Merge cascade resolver.
+ * <p>
+ * Cascades the merge operation to attributes which support merge cascading.
+ */
+class OneLevelMergeCascadeExplorer {
+
+    void start(final AbstractEntityManager pc, final Object merged, final Object toMerge) {
+
+        final EntityType<?> a = pc.getMetamodel().entity(toMerge.getClass());
+        for (final Attribute<?, ?> at : a.getAttributes()) {
+
+            final List<CascadeType> cTypes = Arrays.asList(at.getCascadeTypes());
+
+            try {
+                if (cTypes.contains(CascadeType.ALL) || cTypes.contains(CascadeType.MERGE)) {
+                    exploreCascaded(at, merged, toMerge);
+                }
+            } catch (Exception e) {
+                throw new OWLPersistenceException(e);
+            }
+        }
+    }
+
+    void exploreCascaded(final Attribute<?, ?> at, final Object merged, final Object toMerge)
+            throws IllegalAccessException {
+        // empty body
+    }
+}

jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/CloneBuilderImpl.java

@@ -1,11 +1,11 @@
 /**
  * Copyright (C) 2016 Czech Technical University in Prague
- *
+ * <p>
  * This program is free software: you can redistribute it and/or modify it under
  * the terms of the GNU General Public License as published by the Free Software
  * Foundation, either version 3 of the License, or (at your option) any
  * later version.
- *
+ * <p>
  * This program is distributed in the hope that it will be useful, but WITHOUT
  * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
  * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
@@ -226,6 +226,11 @@ public void reset() {
         visitedEntities.clear();
     }
 
+    @Override
+    public void removeVisited(Object instance, Descriptor descriptor) {
+        visitedEntities.remove(descriptor, instance);
+    }
+
     IndirectCollection<?> createIndirectCollection(Object c, Object owner, Field f) {
         return uow.createIndirectCollection(c, owner, f);
     }

jopa-impl/src/main/java/cz/cvut/kbss/jopa/sessions/UnitOfWorkImpl.java

@@ -597,11 +597,12 @@ private <T> T mergeDetachedInternal(T entity, Descriptor descriptor) {
             if (chSet.hasChanges()) {
                 lifecycleListenerCaller.invokePreUpdateListeners(et, entity);
             }
-//            for (ChangeRecord record : chSet.getChanges().values()) {
-//                final Field field = et.getFieldSpecification(record.getAttributeName()).getJavaField();
-//                storage.merge(entity, field, descriptor);
-//            }
-            // TODO Merge changes (it will also cause change propagation into the storage)
+            for (ChangeRecord record : chSet.getChanges().values()) {
+                final Field field = et.getFieldSpecification(record.getAttributeName()).getJavaField();
+                storage.merge(entity, field, descriptor);
+            }
+            final DetachedInstanceMerger merger = new DetachedInstanceMerger(this);
+            merger.mergeChangesFromDetachedToManagedInstance(chSet, descriptor);
             if (chSet.hasChanges()) {
                 lifecycleListenerCaller.invokePostUpdateListeners(et, entity);
             }
@@ -620,9 +621,6 @@ private <T> T mergeDetachedInternal(T entity, Descriptor descriptor) {
         return et.getJavaType().cast(clone);
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     void registerEntityWithPersistenceContext(Object entity, UnitOfWorkImpl uow) {
         parent.registerEntityWithPersistenceContext(entity, uow);
@@ -729,9 +727,7 @@ private void registerNewObjectInternal(Object entity, Descriptor descriptor) {
         if (id == null) {
             EntityPropertiesUtils.verifyIdentifierIsGenerated(entity, eType);
         }
-        if (isIndividualManaged(id, entity) && !entity.getClass().isEnum()) {
-            throw individualAlreadyManaged(id);
-        }
+        verifyCanPersist(id, entity, eType, descriptor);
         storage.persist(id, entity, descriptor);
         if (id == null) {
             // If the ID was null, extract it from the entity. It is present now
@@ -748,6 +744,16 @@ private void registerNewObjectInternal(Object entity, Descriptor descriptor) {
         lifecycleListenerCaller.invokePostPersistListeners(eType, entity);
     }
 
+    private void verifyCanPersist(Object id, Object instance, EntityType<?> et, Descriptor descriptor) {
+        if (isIndividualManaged(id, instance) && !instance.getClass().isEnum()) {
+            throw individualAlreadyManaged(id);
+        }
+        if (storage.contains(id, instance.getClass(), descriptor)) {
+            throw new OWLEntityExistsException(
+                    "Individual " + id + " of type " + et.getIRI() + " already exists in storage.");
+        }
+    }
+
     private boolean isIndividualManaged(Object identifier, Object entity) {
         return keysToClones.containsKey(identifier) ||
                 newObjectsKeyToClone.containsKey(identifier) && !cloneMapping.containsKey(entity);
@@ -790,7 +796,8 @@ public void unregisterObject(Object object) {
             return;
         }
         cloneMapping.remove(object);
-        cloneToOriginals.remove(object);
+        final Object original = cloneToOriginals.remove(object);
+        keysToClones.remove(EntityPropertiesUtils.getPrimaryKey(object, getMetamodel()));
 
         getDeletedObjects().remove(object);
         if (hasNew) {
@@ -799,6 +806,9 @@ public void unregisterObject(Object object) {
                 getNewObjectsOriginalToClone().remove(newOriginal);
             }
         }
+        if (original != null) {
+            cloneBuilder.removeVisited(original, repoMap.getEntityDescriptor(object));
+        }
         unregisterObjectFromPersistenceContext(object);
     }
 

jopa-impl/src/test/java/cz/cvut/kbss/jopa/sessions/UnitOfWorkTest.java

@@ -71,6 +71,8 @@ public class UnitOfWorkTest {
 
     private ServerSessionStub serverSessionStub;
 
+    private CloneBuilder cloneBuilder;
+
     private UnitOfWorkImpl uow;
 
     @BeforeClass
@@ -93,6 +95,10 @@ public void setUp() throws Exception {
         final Field connectionField = UnitOfWorkImpl.class.getDeclaredField("storage");
         connectionField.setAccessible(true);
         connectionField.set(uow, storageMock);
+        final Field cbField = UnitOfWorkImpl.class.getDeclaredField("cloneBuilder");
+        cbField.setAccessible(true);
+        this.cloneBuilder = spy((CloneBuilder) cbField.get(uow));
+        cbField.set(uow, cloneBuilder);
         initEntities();
     }
 
@@ -443,10 +449,17 @@ public void testRemoveObjectNotRegistered() {
     public void testUnregisterObject() {
         final OWLClassA managed = (OWLClassA) uow.registerExistingObject(entityA, descriptor);
         assertTrue(uow.contains(managed));
-        this.uow.unregisterObject(managed);
+        uow.unregisterObject(managed);
         assertFalse(uow.contains(managed));
     }
 
+    @Test
+    public void unregisterObjectRemovesItFromCloneBuilderCache() {
+        final OWLClassA managed = (OWLClassA) uow.registerExistingObject(entityA, descriptor);
+        uow.unregisterObject(managed);
+        verify(cloneBuilder).removeVisited(entityA, descriptor);
+    }
+
     @Test
     public void testRevertObject() {
         final OWLClassA managed = (OWLClassA) uow.registerExistingObject(entityA, descriptor);

jopa-integration-tests/src/main/java/cz/cvut/kbss/jopa/test/environment/TestEnvironmentUtils.java

@@ -1,11 +1,11 @@
 /**
  * Copyright (C) 2016 Czech Technical University in Prague
- *
+ * <p>
  * This program is free software: you can redistribute it and/or modify it under
  * the terms of the GNU General Public License as published by the Free Software
  * Foundation, either version 3 of the License, or (at your option) any
  * later version.
- *
+ * <p>
  * This program is distributed in the hope that it will be useful, but WITHOUT
  * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
  * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
@@ -19,16 +19,15 @@
 
 public class TestEnvironmentUtils {
 
-    public static boolean arePropertiesEqual(Map<String, Set<String>> pOne,
-                                             Map<String, Set<String>> pTwo) {
+    public static <K, V> boolean arePropertiesEqual(Map<K, Set<V>> pOne, Map<K, Set<V>> pTwo) {
         if (pOne.size() != pTwo.size()) {
             return false;
         }
-        for (Map.Entry<String, Set<String>> e : pOne.entrySet()) {
+        for (Map.Entry<K, Set<V>> e : pOne.entrySet()) {
             if (!pTwo.containsKey(e.getKey())) {
                 return false;
             }
-            final Set<String> set = pTwo.get(e.getKey());
+            final Set<?> set = pTwo.get(e.getKey());
             if (!e.getValue().equals(set)) {
                 return false;
             }