Skip to content

Syncing Issue #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

Syncing Issue #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f5c349
Failing test to reproduce #560
beaucollins Sep 26, 2018
ed5a097
Better assertion
beaucollins Sep 27, 2018
822cf92
Renaming o_* variables to local* variables to make a little more sens…
beaucollins Sep 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Simperium/src/androidTestSupport/java/com/simperium/BucketTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,34 @@ public void testMergeLocalChangesWithUpdatedGhost()
assertEquals("Line 1\nLine 2\nLine 3\n", note.getContent());
}

// https://github.com/Automattic/simplenote-android/issues/560
public void testMergeLocalChangesWithRemoteTextDelete()
throws Exception {

Note note = mBucket.newObject();

// Write a note and let it sync with the server
note.setContent("Line 1\nLine 2\nThis line will be deleted by the remote change\n");
note.save();

note.setContent("Line 1\nLine 2\nLine A\nThis line will be deleted by the remote change\nLine B");

// Delete the last paragraph on PC, put a short sentence INSTEAD at its place
JSONObject external = new JSONObject(note.getDiffableValue().toString());
external.put("content", "Line 1\nLine 2\nLine C\n");

// build remote change based on 3rd party modification
RemoteChange change = RemoteChangesUtil.buildRemoteChange(note, external);

Ghost ghost = change.apply(note.getGhost());

mBucket.updateGhost(ghost, null);

// This is an approximation of the expected outcome, not confident yet in what it's
// supposed to be
assertEquals("Line 1\nLine 2\nLine C\nLine A\nLine B\n", note.getContent());
}

/**
* If two different notes are both saved and then both deleted, they should both be missing from persistent store
* but present in the backup store.
Expand Down
54 changes: 27 additions & 27 deletions Simperium/src/main/java/com/simperium/util/JSONDiff.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ public class JSONDiff {

private static diff_match_patch dmp = new diff_match_patch();

public static JSONObject transform(String o_diff, String diff, String source)
public static JSONObject transform(String localDiff, String diff, String source)
throws JSONException {

JSONObject transformed = new JSONObject();

LinkedList<Patch> o_patches = dmp.patch_make(source, dmp.diff_fromDelta(source, o_diff));
LinkedList<Patch> localPatches = dmp.patch_make(source, dmp.diff_fromDelta(source, localDiff));
LinkedList<Patch> patches = dmp.patch_make(source, dmp.diff_fromDelta(source, diff));

String text = (String) dmp.patch_apply(patches, source)[0];
String combined = (String) dmp.patch_apply(o_patches, text)[0];
String combined = (String) dmp.patch_apply(localPatches, text)[0];

if (text.equals(combined)) {
// text is the same, return empty diff
Expand All @@ -68,67 +68,67 @@ public static JSONObject transform(String o_diff, String diff, String source)
}

/**
* Given two object diffs (o_diff and diff) and a source object, calculate
* the transformed o_diff that merges the changes of source + diff and
* source + o_diff
* Given two object diffs (localDiff and diff) and a source object, calculate
* the transformed localDiff that merges the changes of source + diff and
* source + localDiff
*
* For reference:
* https://github.com/Simperium/jsondiff/blob/eb61ad1e4554450cc14af1938847f18513db946b/src/jsondiff.coffee#L458-L503
*/
public static JSONObject transform(JSONObject o_diff, JSONObject diff, JSONObject source)
public static JSONObject transform(JSONObject local, JSONObject remote, JSONObject source)
throws JSONException {

JSONObject transformed_diff = deepCopy(o_diff);
JSONObject transformed_diff = deepCopy(local);

// iterate the keys of o_diff
Iterator<String> keys = o_diff.keys();
// iterate the keys of localDiff
Iterator<String> keys = local.keys();
while(keys.hasNext()) {
String key = keys.next();

if (!diff.has(key)) {
if (!remote.has(key)) {
continue;
}

JSONObject o_operation = o_diff.getJSONObject(key);
JSONObject operation = diff.getJSONObject(key);
JSONObject localOperation = local.getJSONObject(key);
JSONObject operation = remote.getJSONObject(key);

String o_type = o_operation.getString(DIFF_OPERATION_KEY);
String localType = localOperation.getString(DIFF_OPERATION_KEY);
String type = operation.getString(DIFF_OPERATION_KEY);

Object o_value = o_operation.opt(DIFF_VALUE_KEY);
Object localValue = localOperation.opt(DIFF_VALUE_KEY);
Object value = operation.opt(DIFF_VALUE_KEY);

// both are inserts
if (o_type.equals(OPERATION_INSERT) && type.equals(OPERATION_INSERT)) {
if (localType.equals(OPERATION_INSERT) && type.equals(OPERATION_INSERT)) {
// both are inserting the same value
if (o_value.equals(value)) {
if (localValue.equals(value)) {
// don't duplicate what diff is doing
transformed_diff.remove(key);
} else {
transformed_diff.put(key, diff(value, o_value));
transformed_diff.put(key, diff(value, localValue));
}
} else if (o_type.equals(OPERATION_REMOVE) && type.equals(OPERATION_REMOVE)) {
} else if (localType.equals(OPERATION_REMOVE) && type.equals(OPERATION_REMOVE)) {
// we're both removing the same key
transformed_diff.remove(key);
} else if (type.equals(OPERATION_REMOVE) && !o_type.equals(OPERATION_REMOVE)) {
} else if (type.equals(OPERATION_REMOVE) && !localType.equals(OPERATION_REMOVE)) {

// they removed a key that we're replacing, insert the key
JSONObject restore_op = new JSONObject();
restore_op.put(DIFF_OPERATION_KEY, OPERATION_INSERT);

if (o_type.equals(OPERATION_REPLACE)) {
restore_op.put(DIFF_VALUE_KEY, o_value);
if (localType.equals(OPERATION_REPLACE)) {
restore_op.put(DIFF_VALUE_KEY, localValue);
} else {
// apply the patch for the value to insert
restore_op.put(DIFF_VALUE_KEY, apply(source.get(key), o_operation));
restore_op.put(DIFF_VALUE_KEY, apply(source.get(key), localOperation));
}

transformed_diff.put(key, restore_op);

} else if (o_type.equals(OPERATION_OBJECT) && type.equals(OPERATION_OBJECT)) {
operation.put(DIFF_VALUE_KEY, transform((JSONObject)o_value, (JSONObject)value, source.getJSONObject(key)));
} else if (o_type.equals(OPERATION_DIFF) && type.equals(OPERATION_DIFF)) {
JSONObject diff_operation = transform((String)o_value, (String)value, source.getString(key));
} else if (localType.equals(OPERATION_OBJECT) && type.equals(OPERATION_OBJECT)) {
operation.put(DIFF_VALUE_KEY, transform((JSONObject)localValue, (JSONObject)value, source.getJSONObject(key)));
} else if (localType.equals(OPERATION_DIFF) && type.equals(OPERATION_DIFF)) {
JSONObject diff_operation = transform((String)localValue, (String)value, source.getString(key));
if (diff_operation.length() == 0) {
transformed_diff.remove(key);
} else {
Expand Down