Index: opensync/engine/opensync_mapping_engine.c =================================================================== --- opensync/engine/opensync_mapping_engine.c (revision 4289) +++ opensync/engine/opensync_mapping_engine.c (working copy) @@ -46,9 +46,9 @@ #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,6 +76,9 @@ 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,9 +360,10 @@ 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,7 +429,8 @@ 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,7 +442,8 @@ 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) { Index: opensync/engine/opensync_obj_engine.c =================================================================== --- opensync/engine/opensync_obj_engine.c (revision 4289) +++ opensync/engine/opensync_obj_engine.c (working copy) @@ -69,7 +69,7 @@ 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,7 +549,7 @@ 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,7 +865,9 @@ 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,6 +942,9 @@ 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)) && Index: opensync/engine/opensync_mapping_entry_engine.c =================================================================== --- opensync/engine/opensync_mapping_entry_engine.c (revision 4289) +++ opensync/engine/opensync_mapping_entry_engine.c (working copy) @@ -50,7 +50,11 @@ 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,6 +64,8 @@ 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; Index: opensync/engine/opensync_mapping_engine_internals.h =================================================================== --- opensync/engine/opensync_mapping_engine_internals.h (revision 4289) +++ opensync/engine/opensync_mapping_engine_internals.h (working copy) @@ -34,7 +34,7 @@ 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); Index: tests/mock-plugin/mock_sync.c =================================================================== --- tests/mock-plugin/mock_sync.c (revision 4289) +++ tests/mock-plugin/mock_sync.c (working copy) @@ -284,7 +284,15 @@ 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,6 +421,11 @@ 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__); Index: tests/support.c =================================================================== --- tests/support.c (revision 4289) +++ tests/support.c (working copy) @@ -25,6 +25,8 @@ unsetenv("MAINSINK_CONNECT"); unsetenv("OSYNC_NOMEMORY"); + unsetenv("MOCK_SYNC_EXPECT_COMMIT_ALWAYS_ADDED"); + unsetenv("MOCK_SYNC_ALWAYS_CHANGETYPE_MODIFIED"); } Index: tests/engine-tests/check_engine.c =================================================================== --- tests/engine-tests/check_engine.c (revision 4289) +++ tests/engine-tests/check_engine.c (working copy) @@ -11,6 +11,7 @@ #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,6 +1986,91 @@ } 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,7 +2090,9 @@ //batch commit //connect problem + //get_changes problem + create_case(s, "engine_getchanges_fixed_chtype_modified", engine_getchanges_fixed_chtype_modified); return s; }