fix list synchronisation problems shown by tests #16

Author: rbi

synchronizefx-core/src/main/java/de/saxsys/synchronizefx/core/metamodel/CommandListCreator.java

@@ -308,20 +308,24 @@ private void addToList(final UUID listId, final int position, final Object value
             final State state) {
         final boolean isObservableObject = createObservableObject(value, state);
 
+        final ListPropertyMetaData metaData = listMetaDataStore.getMetaDataOrFail(listId);
         ListVersionChange change;
         if (state.skipKnown) {
             // List is already known on other peers, update the version.
-            change = increaseListVersion(listId);
+            change = increaseListVersion(metaData);
         } else {
-            // Initial walk through the whole list.
-            change = new ListVersionChange(INITIAL_LIST_VERSION, INITIAL_LIST_VERSION);
+            // Initial walk through the whole list, do not update the version.
+            change = new ListVersionChange(metaData.getLocalVersion(), metaData.getLocalVersion());
         }
         final AddToList msg = new AddToList(listId, change, valueMapper.map(value, isObservableObject), position);
         state.commands.add(msg);
     }
 
     private ListVersionChange increaseListVersion(final UUID listId) {
-        final ListPropertyMetaData metaData = listMetaDataStore.getMetaDataOrFail(listId);
+        return increaseListVersion(listMetaDataStore.getMetaDataOrFail(listId));
+    }
+
+    private ListVersionChange increaseListVersion(final ListPropertyMetaData metaData) {
         final ListVersionChange change = new ListVersionChange(metaData.getLocalVersion(), UUID.randomUUID());
         metaData.setLocalVersion(change.getToVersion());
         return change;
@@ -396,11 +400,23 @@ protected boolean visitSingleValueProperty(final Property<?> fieldValue) {
                 protected boolean visitCollectionProperty(final ListProperty<?> fieldValue) {
                     final boolean listWasKnown = objectRegistry.getId(fieldValue).isPresent();
                     final UUID fieldId = registerPropertyAndParent(getCurrentField(), fieldValue);
-                    if (!state.skipKnown && !listWasKnown) {
+                    if (!listWasKnown) {
+                        // initial walk through the meta model
                         listMetaDataStore.storeMetaDataOrFail(fieldValue, new ListPropertyMetaData(
                                 INITIAL_LIST_VERSION, INITIAL_LIST_VERSION));
                     }
                     final ListIterator<?> it = fieldValue.listIterator();
+                    if (!it.hasNext()) {
+                        final ListPropertyMetaData metaData = listMetaDataStore.getMetaDataOrFail(fieldValue);
+                        if (metaData.getLocalVersion() != INITIAL_LIST_VERSION) {
+                            // This can happen when there where elements added after the list was created which where
+                            // all removed afterwards so that the list is now empty. This means that no AddToList
+                            // command will be created which would update the list version. Therefore the following
+                            // command will do this.
+                            state.commands.add(new RemoveFromList(fieldId, new ListVersionChange(INITIAL_LIST_VERSION,
+                                    metaData.getLocalVersion()), 0, 0));
+                        }
+                    }
                     int index = 0;
                     while (it.hasNext()) {
                         final Object o = it.next();

synchronizefx-core/src/main/java/de/saxsys/synchronizefx/core/metamodel/CommandListExecutor.java

@@ -21,6 +21,7 @@
 
 import java.lang.reflect.Field;
 import java.util.IdentityHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -190,8 +191,11 @@ private void execute(final CreateObservableObject command) {
     private void registerInMetaModel(final Object object, final UUID id) {
         objectRegistry.registerObject(object, id);
         if (object instanceof ListProperty) {
-            listPropertyMetaDataStore.storeMetaDataOrFail((ListProperty<?>) object, new ListPropertyMetaData(
-                    CommandListCreator.INITIAL_LIST_VERSION, CommandListCreator.INITIAL_LIST_VERSION));
+            final List<?> list = (List<?>) object;
+            if (!listPropertyMetaDataStore.hasMetaDataFor(list)) {
+                listPropertyMetaDataStore.storeMetaDataOrFail(list, new ListPropertyMetaData(
+                        CommandListCreator.INITIAL_LIST_VERSION, CommandListCreator.INITIAL_LIST_VERSION));
+            }
         }
     }
 

synchronizefx-core/src/main/java/de/saxsys/synchronizefx/core/metamodel/ListPropertyMetaDataStore.java

@@ -81,7 +81,7 @@ public ListPropertyMetaData getMetaDataOrFail(final List<?> list) {
      * 
      * <p>
      * <em>Performance hint:</em> If a reference to the list property itself is available, the method
-     * {@link #getMetaDataOrFail(Object)} should be used as this saves a lookup in a map.
+     * {@link #getMetaDataOrFail(List)} should be used as this saves a lookup in a map.
      * </p>
      * 
      * @param listId
@@ -106,14 +106,24 @@ public ListPropertyMetaData getMetaDataOrFail(final UUID listId) throws ObjectTo
      *             <code>listId</code>
      */
     public void storeMetaDataOrFail(final List<?> list, final ListPropertyMetaData metaData)
-            throws ObjectToIdMappingException {
+        throws ObjectToIdMappingException {
         if (listToData.get(list) != null) {
             throw new ObjectToIdMappingException("Meta data for a known property should be registered twice. "
                     + "The clients may no longer be synchron.");
         }
         listToData.put(list, metaData);
     }
 
+    /**
+     * Checks if this meta data store does already have meta data for a given list.
+     * 
+     * @param list The list that should be checked for already having meta data.
+     * @return <code>true</code> if meta data is known and <code>false</code> if not.
+     */
+    public boolean hasMetaDataFor(final List<?> list) {
+        return listToData.containsKey(list);
+    }
+
     /**
      * The meta data about a {@link List}.
      *

synchronizefx-core/src/main/java/de/saxsys/synchronizefx/core/metamodel/executors/lists/ListCommandVersionRepairer.java

@@ -53,9 +53,6 @@ public class ListCommandVersionRepairer {
      */
     public void repairLocalCommandsVersion(final Queue<ListCommand> localCommands,
             final ListCommand originalRemoteCommand) {
-        if (localCommands.isEmpty()) {
-            return;
-        }
 
         localCommands.add(repairCommand(localCommands.poll(), originalRemoteCommand.getListVersionChange()
                 .getToVersion(), randomUUID()));

synchronizefx-core/src/main/java/de/saxsys/synchronizefx/core/metamodel/executors/lists/ReparingListPropertyCommandExecutor.java

@@ -133,19 +133,24 @@ private void execute(final ListCommand command) {
             updateVersion(command);
             log.remove();
         } else {
-            // change local list
-            final List<? extends ListCommand> indexRepairedCommands = indexRepairer.repairCommands(
+            // repair indices
+            List<? extends ListCommand> repairedCommands = indexRepairer.repairCommands(
                     metaData.getUnapprovedCommands(), command);
-            versionRepairer.repairLocalCommandsVersion(metaData.getUnapprovedCommands(), command);
-            final List<? extends ListCommand> versionRepairedCommands = versionRepairer.repairRemoteCommandVersion(
-                    indexRepairedCommands, metaData.getUnapprovedCommandsAsList());
-            for (final ListCommand repaired : versionRepairedCommands) {
+
+            if (metaData.getUnapprovedCommands().size() > 0) {
+                // repair versions if local commands are left after repairing indices.
+                versionRepairer.repairLocalCommandsVersion(metaData.getUnapprovedCommands(), command);
+                repairedCommands = versionRepairer.repairRemoteCommandVersion(repairedCommands,
+                        metaData.getUnapprovedCommandsAsList());
+                // re-send repaired local changes
+                topologyLayerCallback.sendCommands((List) metaData.getUnapprovedCommandsAsList());
+            }
+
+            // execute repaired commands
+            for (final ListCommand repaired : repairedCommands) {
                 executeCommand(repaired);
             }
             updateVersion(command);
-
-            // re-send repaired local changes
-            topologyLayerCallback.sendCommands((List) metaData.getUnapprovedCommandsAsList());
         }
     }
 

synchronizefx-core/src/main/java/de/saxsys/synchronizefx/core/metamodel/executors/lists/ReplaceInListRepairer.java

@@ -32,7 +32,7 @@
  *
  * @author Raik Bieniek
  */
-public final class ReplaceInListRepairer {
+public class ReplaceInListRepairer {
 
     /**
      * Repairs a {@link ReplaceInList} in relation to an {@link AddToList} command.

synchronizefx-core/src/test/java/de/saxsys/synchronizefx/core/metamodel/ListPropertyMetaDataStoreTest.java

@@ -90,6 +90,17 @@ public void canRetrieveMetaDataByListId() {
 
         assertThat(cut.getMetaDataOrFail(list1Id)).isSameAs(exampleMetaData1);
     }
+    
+    /**
+     * The cut can check if meta data is already known for a list.
+     */
+    @Test
+    public void canCheckIfMetaDataIsStoredForList() {
+        cut.storeMetaDataOrFail(list1, exampleMetaData1);
+        
+        assertThat(cut.hasMetaDataFor(list1)).isTrue();
+        assertThat(cut.hasMetaDataFor(list2)).isFalse();
+    }
 
     /**
      * When {@link ListPropertyMetaData} should be stored for a property that already has meta data, the cut should

synchronizefx-core/src/test/java/de/saxsys/synchronizefx/core/testutils/SaveParameterCallback.java

@@ -25,7 +25,7 @@
 import de.saxsys.synchronizefx.core.metamodel.TopologyLayerCallback;
 import de.saxsys.synchronizefx.core.metamodel.commands.Command;
 
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.fail;
 
 /**
  * An implementation that saves the parameters of the callback function so that they can be evaluated in tests.
@@ -41,7 +41,7 @@ public void sendCommands(final List<Command> commands) {
 
     @Override
     public void onError(final SynchronizeFXException error) {
-        fail("exception occured: " + error.getMessage());
+        fail("exception occured: ", error);
     }
 
     @Override