Skip to content
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

Jump to bottom

fix(object): add support for raw leaves in object diff #5472

Merged
merged 4 commits into from
Oct 4, 2018
Merged

fix(object): add support for raw leaves in object diff #5472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26a3ebf
fix(object): add support for raw leaves in object diff
overbool Sep 15, 2018
5b0a948
fix(object): add Diff() comment and test case
overbool Sep 17, 2018
7cda005
test(diff): separate diff test cases
overbool Sep 18, 2018
c6daf93
fix(diff): modify diff logic and comment
overbool Sep 22, 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
49 changes: 25 additions & 24 deletions dagutils/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,26 +94,24 @@ func ApplyChange(ctx context.Context, ds ipld.DAGService, nd *dag.ProtoNode, cs
return e.Finalize(ctx, ds)
}

// Diff returns a set of changes that transform node 'a' into node 'b'
// Diff returns a set of changes that transform node 'a' into node 'b'.
// 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{
&Change{
Type: Mod,
Before: a.Cid(),
After: b.Cid(),
},
}, nil
return getChange(a, b)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that much (I think), we could add a check here to verify that, if they have links, i.e., they are not raw nodes because they passed the first check (hence we would still be supporting them), they must be proto-nodes; if not, you could also return a Mod as before. Actually you could even merge this proto-node check in the previous if, WDYT?

var out []*Change
cleanA := a.Copy().(*dag.ProtoNode)
cleanB := b.Copy().(*dag.ProtoNode)
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() {
Expand All @@ -132,17 +130,7 @@ func Diff(ctx context.Context, ds ipld.DAGService, a, b ipld.Node) ([]*Change, e
return nil, err
}

anodepb, ok := anode.(*dag.ProtoNode)
if !ok {
return nil, dag.ErrNotProtobuf
}

bnodepb, ok := bnode.(*dag.ProtoNode)
if !ok {
return nil, dag.ErrNotProtobuf
}

sub, err := Diff(ctx, ds, anodepb, bnodepb)
sub, err := Diff(ctx, ds, anode, bnode)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -209,3 +197,16 @@ func MergeDiffs(a, b []*Change) ([]*Change, []Conflict) {
}
return out, conflicts
}

func getChange(a, b ipld.Node) ([]*Change, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely encapsulated!

if a.Cid().Equals(b.Cid()) {
return []*Change{}, nil
}
return []*Change{
{
Type: Mod,
Before: a.Cid(),
After: b.Cid(),
},
}, nil
}
76 changes: 74 additions & 2 deletions test/sharness/t0052-object-diff.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,22 @@ test_expect_success "create some objects for testing diffs" '
echo "stuff" > foo/bar &&
mkdir foo/baz &&
A=$(ipfs add -r -q foo | tail -n1) &&
AR=$(ipfs add --raw-leaves -r -q foo | tail -n1) &&
echo "more things" > foo/cat &&
B=$(ipfs add -r -q foo | tail -n1) &&
BR=$(ipfs add --raw-leaves -r -q foo | tail -n1) &&
echo "nested" > foo/baz/dog &&
C=$(ipfs add -r -q foo | tail -n1)
CR=$(ipfs add --raw-leaves -r -q foo | tail -n1)
echo "changed" > foo/bar &&
D=$(ipfs add -r -q foo | tail -n1) &&
DR=$(ipfs add --raw-leaves -r -q foo | tail -n1) &&
echo "" > single_file &&
SINGLE_FILE=$(ipfs add -r -q single_file | tail -n1) &&
SINGLE_FILE_RAW=$(ipfs add --raw-leaves -r -q single_file | tail -n1) &&
mkdir empty_dir
EMPTY_DIR=$(ipfs add -r -q empty_dir | tail -n1)
EMPTY_DIR_RAW=$(ipfs add --raw-leaves -r -q empty_dir | tail -n1)
'

test_expect_success "diff against self is empty" '
Expand All @@ -36,18 +42,39 @@ test_expect_success "identity diff output looks good" '
test_cmp diff_exp diff_out
'

test_expect_success "diff (raw-leaves) against self is empty" '
ipfs object diff $AR $AR > diff_raw_out
'

test_expect_success "identity diff (raw-leaves) output looks good" '
printf "" > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_expect_success "diff against self (single file) is empty" '
ipfs object diff $SINGLE_FILE $SINGLE_FILE > diff_out
ipfs object diff $SINGLE_FILE $SINGLE_FILE > diff_out &&
printf "" > diff_exp &&
test_cmp diff_exp diff_out
'

test_expect_success "diff (raw-leaves) against self (single file) is empty" '
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 &&
printf "" > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_expect_success "diff added link works" '
ipfs object diff $A $B > diff_out
'
Expand All @@ -57,6 +84,15 @@ test_expect_success "diff added link looks right" '
test_cmp diff_exp diff_out
'

test_expect_success "diff (raw-leaves) added link works" '
ipfs object diff $AR $BR > diff_raw_out
'

test_expect_success "diff (raw-leaves) added link looks right" '
echo + zb2rhmWNFDCdMjJoCZPE5b5NuU38yoRzRmEtfzb4exxk3R8g4 \"cat\" > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_expect_success "verbose diff added link works" '
ipfs object diff -v $A $B > diff_out
'
Expand All @@ -66,6 +102,15 @@ test_expect_success "verbose diff added link looks right" '
test_cmp diff_exp diff_out
'

test_expect_success "verbose diff (raw-leaves) added link works" '
ipfs object diff -v $AR $BR > diff_raw_out
'

test_expect_success "verbose diff (raw-leaves) added link looks right" '
echo Added new link \"cat\" pointing to zb2rhmWNFDCdMjJoCZPE5b5NuU38yoRzRmEtfzb4exxk3R8g4. > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_expect_success "diff removed link works" '
ipfs object diff -v $B $A > diff_out
'
Expand All @@ -75,6 +120,15 @@ test_expect_success "diff removed link looks right" '
test_cmp diff_exp diff_out
'

test_expect_success "diff (raw-leaves) removed link works" '
ipfs object diff -v $BR $AR > diff_raw_out
'

test_expect_success "diff (raw-leaves) removed link looks right" '
echo Removed link \"cat\" \(was zb2rhmWNFDCdMjJoCZPE5b5NuU38yoRzRmEtfzb4exxk3R8g4\). > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_expect_success "diff nested add works" '
ipfs object diff -v $B $C > diff_out
'
Expand All @@ -84,6 +138,15 @@ test_expect_success "diff looks right" '
test_cmp diff_exp diff_out
'

test_expect_success "diff (raw-leaves) nested add works" '
ipfs object diff -v $BR $CR > diff_raw_out
'

test_expect_success "diff (raw-leaves) looks right" '
echo Added new link \"baz/dog\" pointing to zb2rhaM8wjDfi8A22dEqk89raWtViq8pjxvKQu2eaKtWhYKgE. > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_expect_success "diff changed link works" '
ipfs object diff -v $C $D > diff_out
'
Expand All @@ -93,4 +156,13 @@ test_expect_success "diff looks right" '
test_cmp diff_exp diff_out
'

test_expect_success "diff (raw-leaves) changed link works" '
ipfs object diff -v $CR $DR > diff_raw_out
'

test_expect_success "diff（raw-leaves) looks right" '
echo Changed \"bar\" from zb2rhdUECGnPgMJNgmghaMKdqqGdpTe9GmEJiPna488ThfLBz to zb2rhfEA1M13SPoeayrsPcKhCezgMQPjguGFLH56G8qQ2qpDn. > diff_raw_exp &&
test_cmp diff_raw_exp diff_raw_out
'

test_done