From c6daf934ea355b6115ec7222e00aef969ef1783a Mon Sep 17 00:00:00 2001
From: Overbool <overbool.xu@gmail.com>
Date: Sat, 22 Sep 2018 09:58:12 +0800
Subject: [PATCH] fix(diff): modify diff logic and comment

License: MIT
Signed-off-by: Overbool <overbool.xu@gmail.com>
---
 dagutils/diff.go                   | 47 ++++++++++++++++--------------
 test/sharness/t0052-object-diff.sh |  6 ++--
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/dagutils/diff.go b/dagutils/diff.go
index 6fc92140078..c63aacafbc5 100644
--- a/dagutils/diff.go
+++ b/dagutils/diff.go
@@ -95,29 +95,23 @@ func ApplyChange(ctx context.Context, ds ipld.DAGService, nd *dag.ProtoNode, cs
 }
 
 // Diff returns a set of changes that transform node 'a' into node 'b'.
-// It supports two nodes forms: ProtoNode and RawNode. Because we treat
-// the nodes as IPLD nodes as long as possible and only convert them
-// to ProtoNode when necessary: when we need to remove links, and at that point
-// (if they have links to remove) we know they are not raw nodes.
+// It only traverses links in the following cases:
+// 1. two node's links number are greater than 0.
+// 2. both of two nodes are ProtoNode.
+// Otherwise, it compares the cid and emits a Mod change object.
 func Diff(ctx context.Context, ds ipld.DAGService, a, b ipld.Node) ([]*Change, error) {
 	// Base case where both nodes are leaves, just compare
 	// their CIDs.
 	if len(a.Links()) == 0 && len(b.Links()) == 0 {
-		if a.Cid().Equals(b.Cid()) {
-			return []*Change{}, nil
-		}
-		return []*Change{
-			{
-				Type:   Mod,
-				Before: a.Cid(),
-				After:  b.Cid(),
-			},
-		}, nil
+		return getChange(a, b)
 	}
 
 	var out []*Change
-	cleanA := a.Copy()
-	cleanB := b.Copy()
+	cleanA, okA := a.Copy().(*dag.ProtoNode)
+	cleanB, okB := b.Copy().(*dag.ProtoNode)
+	if !okA || !okB {
+		return getChange(a, b)
+	}
 
 	// strip out unchanged stuff
 	for _, lnk := range a.Links() {
@@ -146,12 +140,8 @@ func Diff(ctx context.Context, ds ipld.DAGService, a, b ipld.Node) ([]*Change, e
 					out = append(out, subc)
 				}
 			}
-			if cleanA, ok := cleanA.(*dag.ProtoNode); ok {
-				cleanA.RemoveNodeLink(l.Name)
-			}
-			if cleanB, ok := cleanB.(*dag.ProtoNode); ok {
-				cleanB.RemoveNodeLink(l.Name)
-			}
+			cleanA.RemoveNodeLink(l.Name)
+			cleanB.RemoveNodeLink(l.Name)
 		}
 	}
 
@@ -207,3 +197,16 @@ func MergeDiffs(a, b []*Change) ([]*Change, []Conflict) {
 	}
 	return out, conflicts
 }
+
+func getChange(a, b ipld.Node) ([]*Change, error) {
+	if a.Cid().Equals(b.Cid()) {
+		return []*Change{}, nil
+	}
+	return []*Change{
+		{
+			Type:   Mod,
+			Before: a.Cid(),
+			After:  b.Cid(),
+		},
+	}, nil
+}
diff --git a/test/sharness/t0052-object-diff.sh b/test/sharness/t0052-object-diff.sh
index aa0bcd11deb..e512c4b18b8 100755
--- a/test/sharness/t0052-object-diff.sh
+++ b/test/sharness/t0052-object-diff.sh
@@ -58,19 +58,19 @@ test_expect_success "diff against self (single file) is empty" '
 '
 
 test_expect_success "diff (raw-leaves) against self (single file) is empty" '
-  ipfs object diff $SINGLE_FILE_RAW $SINGLE_FILE_RAW > diff_raw_out
+  ipfs object diff $SINGLE_FILE_RAW $SINGLE_FILE_RAW > diff_raw_out &&
   printf "" > diff_raw_exp &&
   test_cmp diff_raw_exp diff_raw_out
 '
 
 test_expect_success "diff against self (empty dir) is empty" '
-  ipfs object diff $EMPTY_DIR $EMPTY_DIR > diff_out
+  ipfs object diff $EMPTY_DIR $EMPTY_DIR > diff_out &&
   printf "" > diff_exp &&
   test_cmp diff_exp diff_out
 '
 
 test_expect_success "diff (raw-leaves) against self (empty dir) is empty" '
-  ipfs object diff $EMPTY_DIR_RAW $EMPTY_DIR_RAW > diff_raw_out
+  ipfs object diff $EMPTY_DIR_RAW $EMPTY_DIR_RAW > diff_raw_out &&
   printf "" > diff_raw_exp &&
   test_cmp diff_raw_exp diff_raw_out
 '