Ticket #884: ticket884_handle_new_changes_with_type_modified_v2.diff

opensync/engine/opensync_mapping_engine.c

@@ -46 +46 @@
 #include "opensync_mapping_engine_internals.h"
 
 
-OSyncMappingEngine *osync_mapping_engine_new(OSyncObjEngine *parent, OSyncMapping *mapping, OSyncError **error)
+OSyncMappingEngine *osync_mapping_engine_new(OSyncObjEngine *parent, OSyncMapping *mapping, OSyncChangeType initial_changetype, OSyncError **error)
 {
-        osync_trace(TRACE_ENTRY, "%s(%p, %p, %p)", __func__, parent, mapping, error);
+        osync_trace(TRACE_ENTRY, "%s(%p, %p, %d, %p)", __func__, parent, mapping, initial_changetype, error);
 
         osync_assert(parent);
         osync_assert(mapping);
@@ -76 +76 @@
                 if (!entry_engine)
                         goto error_free_engine;
 
+                OSyncChange *change = osync_entry_engine_get_change(entry_engine);
+                osync_change_set_changetype(change, initial_changetype);
+
                 engine->entries = g_list_append(engine->entries, entry_engine);
         }
         
@@ -357 +360 @@
                 if (newChangeType == OSYNC_CHANGE_TYPE_ADDED && (existChangeType != OSYNC_CHANGE_TYPE_DELETED && existChangeType != OSYNC_CHANGE_TYPE_UNKNOWN)) {
                         osync_trace(TRACE_INTERNAL, "Updating change type to MODIFIED");
                         osync_change_set_changetype(existChange, OSYNC_CHANGE_TYPE_MODIFIED);
-                /* Only adapt the change to ADDED if the existing Change got deleted. Don't update it to ADDED if existChangeType is UNKOWN.
+                /* Only adapt the change to ADDED if the existing Change got deleted.
                    The exitChangeType is at least also UNKOWN if the file-sync has only one modified entry. */
-                } else if (newChangeType == OSYNC_CHANGE_TYPE_MODIFIED && (existChangeType == OSYNC_CHANGE_TYPE_DELETED)) {
+                } else if (newChangeType == OSYNC_CHANGE_TYPE_MODIFIED &&
+                                (existChangeType == OSYNC_CHANGE_TYPE_DELETED || existChangeType == OSYNC_CHANGE_TYPE_UNKNOWN)) {
                         osync_trace(TRACE_INTERNAL, "Updating change type to ADDED");
                         osync_change_set_changetype(existChange, OSYNC_CHANGE_TYPE_ADDED);
                 }
@@ -425 +429 @@
                 if (leftchange == NULL)
                         continue;
                         
-                if (osync_change_get_changetype(leftchange) == OSYNC_CHANGE_TYPE_UNKNOWN)
+                if (osync_change_get_changetype(leftchange) == OSYNC_CHANGE_TYPE_UNMODIFIED
+                        || osync_change_get_changetype(leftchange) == OSYNC_CHANGE_TYPE_UNKNOWN)
                         continue;
                 
                 osync_mapping_engine_set_master(engine, leftentry);
@@ -437 +442 @@
                         if (rightchange == NULL)
                                 continue;
                 
-                        if (osync_change_get_changetype(rightchange) == OSYNC_CHANGE_TYPE_UNKNOWN)
+                        if (osync_change_get_changetype(rightchange) == OSYNC_CHANGE_TYPE_UNMODIFIED
+                                        || osync_change_get_changetype(rightchange) == OSYNC_CHANGE_TYPE_UNKNOWN)
                                 continue;
                         
                         if (osync_change_compare(leftchange, rightchange) != OSYNC_CONV_DATA_SAME) {

opensync/engine/opensync_obj_engine.c

@@ -69 +69 @@
                 osync_mapping_entry_unref(mapping_entry);
         }
         
-        OSyncMappingEngine *mapping_engine = osync_mapping_engine_new(engine, mapping, error);
+        OSyncMappingEngine *mapping_engine = osync_mapping_engine_new(engine, mapping, OSYNC_CHANGE_TYPE_UNKNOWN, error);
         if (!mapping_engine)
                 goto error_free_mapping;
         osync_mapping_unref(mapping);
@@ -549 +549 @@
         for (i = 0; i < osync_mapping_table_num_mappings(engine->mapping_table); i++) {
                 OSyncMapping *mapping = osync_mapping_table_nth_mapping(engine->mapping_table, i);
                 
-                OSyncMappingEngine *mapping_engine = osync_mapping_engine_new(engine, mapping, error);
+                OSyncMappingEngine *mapping_engine = osync_mapping_engine_new(engine, mapping, OSYNC_CHANGE_TYPE_UNMODIFIED, error);
                 if (!mapping_engine)
                         goto error;
                 
@@ -865 +865 @@
                                         OSyncMappingEntryEngine *entry = m->data;
                                         OSyncChange *change = entry->change;
 
-                                        if (!change)
+                                        osync_assert(change);
+
+                                        if (osync_change_get_changetype(change) == OSYNC_CHANGE_TYPE_UNMODIFIED)
                                                 continue;
 
                                         if (!osync_client_proxy_read(sinkengine->proxy, _osync_obj_engine_read_ignored_callback, sinkengine, change, error))
@@ -940 +942 @@
                                         OSyncMappingEntryEngine *entry_engine = e->data;
                                         osync_assert(entry_engine);
 
+                                        if (osync_change_get_changetype(entry_engine->change) == OSYNC_CHANGE_TYPE_UNMODIFIED)
+                                                continue;
+
                                         /* Merger - Save the entire xml and demerge */
                                         /* TODO: is here the right place to save the xml???? */
                                         if (osync_group_get_merger_enabled(osync_engine_get_group(engine->parent)) &&

opensync/engine/opensync_mapping_entry_engine.c

@@ -50 +50 @@
         engine->sink_engine = sink_engine;
         
         engine->objengine = objengine;
-        
+
+        engine->change = osync_change_new(error);
+        if (!engine->change)
+                goto error_free;
+ 
         engine->mapping_engine = osync_mapping_engine_ref(mapping_engine);
         engine->entry = osync_mapping_entry_ref(entry);
         
@@ -60 +64 @@
         osync_trace(TRACE_EXIT, "%s: %p", __func__, engine);
         return engine;
 
+error_free:
+        osync_entry_engine_unref(engine);
 error:
         osync_trace(TRACE_EXIT_ERROR, "%s: %s", __func__, osync_error_print(error));
         return NULL;

opensync/engine/opensync_mapping_engine_internals.h

@@ -34 +34 @@
         osync_bool synced;
 };
 
-OSyncMappingEngine *osync_mapping_engine_new(OSyncObjEngine *parent, OSyncMapping *mapping, OSyncError **error);
+OSyncMappingEngine *osync_mapping_engine_new(OSyncObjEngine *parent, OSyncMapping *mapping, OSyncChangeType initial_changetype, OSyncError **error);
 OSyncMappingEngine *osync_mapping_engine_ref(OSyncMappingEngine *engine);
 void osync_mapping_engine_unref(OSyncMappingEngine *engine);
 

tests/mock-plugin/mock_sync.c

@@ -284 +284 @@
 
                         OSyncChangeType type = osync_hashtable_get_changetype(directory->hashtable, change);
                         
-                        osync_change_set_changetype(change, type);
+                        /* simulate plugins which can't determine if a change is ADDED and need
+                           to call always MODIFIED instead. This should be correctly handled by
+                           the mapping engine. See #884
+                         */
+                        if (mock_get_error(info->memberid, "MOCK_SYNC_ALWAYS_CHANGETYPE_MODIFIED"))
+                                osync_change_set_changetype(change, OSYNC_CHANGE_TYPE_MODIFIED);
+                        else
+                                osync_change_set_changetype(change, type);
+
                         osync_hashtable_update_change(directory->hashtable, change);
 
                         if (type == OSYNC_CHANGE_TYPE_UNMODIFIED) {
@@ -413 +421 @@
                 osync_trace(TRACE_EXIT_ERROR, "COMMIT_TIMEOUT (mock-sync)!");
                 return;
         }
+
+        if (mock_get_error(info->memberid, "MOCK_SYNC_EXPECT_COMMIT_ALWAYS_ADDED")) {
+                /* for testcase: engine_getchanges_fixed_chtype_modified */
+                osync_assert(osync_change_get_changetype(change) == OSYNC_CHANGE_TYPE_ADDED);
+        }
         
         if (!mock_write(data, info, ctx, change)) {
                 osync_trace(TRACE_EXIT_ERROR, "%s", __func__);

tests/support.c

@@ -25 +25 @@
         unsetenv("MAINSINK_CONNECT");
 
         unsetenv("OSYNC_NOMEMORY");
+        unsetenv("MOCK_SYNC_EXPECT_COMMIT_ALWAYS_ADDED");
+        unsetenv("MOCK_SYNC_ALWAYS_CHANGETYPE_MODIFIED");
 }
 
 

tests/engine-tests/check_engine.c

@@ -11 +11 @@
 #include "opensync/engine/opensync_engine_internals.h"
 #include "opensync/engine/opensync_engine_private.h"
 
+#include "opensync/group/opensync_group_internals.h"
 #include "opensync/group/opensync_member_internals.h"
 #include "opensync/client/opensync_client_internals.h"
 
@@ -1985 +1986 @@
 }
 END_TEST
 
+/*
+ * Testing following secnarios:
+ * 
+ * Member A: will _never_ report changes as ADDED, instead those get reported
+ *           as MODIFIED. DELETED is supported as usual.
+ *
+ * Member B: supports reporting of ADDED, MODIFIED and DELETED changetypes.
+ *
+ * 0. Very first sync, no previous mappings are available.
+ * 1. Member A reports _new_ change "testdata" as MODIFIED (not ADDED!)
+ *    Expected result: Member B commit function sees "testdata" change as
+ *                     ADDED changetype. NOT modified.
+ * 2. Member A rerpots _modified_ change "testdata" as MODIFIED (as regular)
+ *    Expected result: Member B commit function sees "testdata" change as
+ *                     MODIFIED changetype (as regular).
+ *
+ * Regression testing: #884
+ */
+START_TEST (engine_getchanges_fixed_chtype_modified)
+{
+        char *testbed = setup_testbed("sync");
+        char *formatdir = g_strdup_printf("%s/formats",  testbed);
+        char *plugindir = g_strdup_printf("%s/plugins", testbed);
+
+        OSyncError *error = NULL;
+        OSyncGroup *group = osync_group_new(&error);
+        osync_group_set_schemadir(group, testbed);
+        osync_group_load(group, "configs/group", &error); 
+        fail_unless(error == NULL, NULL);
+
+        /* Engine Init */
+        OSyncEngine *engine = osync_engine_new(group, &error);
+        fail_unless(engine != NULL, NULL);
+        fail_unless(error == NULL, NULL);
+
+        osync_engine_set_schemadir(engine, testbed);
+        osync_engine_set_plugindir(engine, plugindir);
+        osync_engine_set_formatdir(engine, formatdir);
+
+        fail_unless(osync_engine_initialize(engine, &error), NULL);
+        fail_unless(error == NULL, NULL);
+        /* End of Engine Init */
+
+        /* Playground */
+
+
+        /* Step 1: Create new entry which gets reported as MODIFIED,
+                   and retrieved as ADDED */
+
+        /* Tune Member A (1st) to MODIFIED-only reporting plugin */
+        setenv("MOCK_SYNC_ALWAYS_CHANGETYPE_MODIFIED", "1", TRUE);
+
+        /* Tune Member B (2nd) to exepcted ADDED reproted change */
+        setenv("MOCK_SYNC_EXPECT_COMMIT_ALWAYS_ADDED", "2", TRUE);
+        osync_testing_system_abort("cp testdata data1/testdata");
+        
+        fail_unless(synchronize_once(engine, &error), NULL);
+        fail_unless(error == NULL, NULL);
+        fail_unless(osync_testing_diff("data1", "data2"), NULL);
+        /* (mock-sync plugin would fail if unexpected changetype got send */
+
+        /* Step 2: Modifed change which gets reported as MODIFIED and
+           retrieved as MODIFIED. Don't expected ADDED for Member B (2nd) */
+        unsetenv("MOCK_SYNC_EXPECT_COMMIT_ALWAYS_ADDED");
+
+        osync_testing_system_abort("cp new_data1 data1/testdata");
+        
+        fail_unless(synchronize_once(engine, &error), NULL);
+        fail_unless(error == NULL, NULL);
+        fail_unless(osync_testing_diff("data1", "data2"), NULL);
+
+        /* End of playground */
+
+        /* Finalize */
+        osync_engine_unref(engine);
+        osync_group_unref(group);
+
+        g_free(formatdir);
+        g_free(plugindir);
+
+        destroy_testbed(testbed);
+}
+END_TEST
+
+
 Suite *engine_suite(void)
 {
         Suite *s = suite_create("Engine");
@@ -2004 +2090 @@
         
         //batch commit
         //connect problem
+
         //get_changes problem
+        create_case(s, "engine_getchanges_fixed_chtype_modified", engine_getchanges_fixed_chtype_modified);
         
         return s;
 }