From 4a8c3c5d994a9d91e69e7680237274667efb0ebf Mon Sep 17 00:00:00 2001 From: asabya Date: Tue, 10 Sep 2024 10:55:01 +0530 Subject: [PATCH 1/4] add test for persisting and removing entries from mantaray manifest --- pkg/manifest/mantaray/persist_test.go | 91 +++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/pkg/manifest/mantaray/persist_test.go b/pkg/manifest/mantaray/persist_test.go index b6a6c47e699..1842b84e0e6 100644 --- a/pkg/manifest/mantaray/persist_test.go +++ b/pkg/manifest/mantaray/persist_test.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "crypto/sha256" + "errors" "sync" "testing" @@ -62,6 +63,96 @@ func TestPersistIdempotence(t *testing.T) { } } +func TestPersistRemove(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + toAdd []mantaray.NodeEntry + toRemove [][]byte + }{ + { + name: "simple", + toAdd: []mantaray.NodeEntry{ + { + Path: []byte("/"), + Metadata: map[string]string{ + "index-document": "index.html", + }, + }, + { + Path: []byte("index.html"), + }, + { + Path: []byte("img/1.png"), + }, + { + Path: []byte("img/2.png"), + }, + { + Path: []byte("robots.txt"), + }, + }, + toRemove: [][]byte{ + []byte("img/2.png"), + }, + }, + } { + ctx := context.Background() + var ls mantaray.LoadSaver = newMockLoadSaver() + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // add and persist + n := mantaray.New() + for i := 0; i < len(tc.toAdd); i++ { + c := tc.toAdd[i].Path + e := tc.toAdd[i].Entry + if len(e) == 0 { + e = append(make([]byte, 32-len(c)), c...) + } + m := tc.toAdd[i].Metadata + err := n.Add(ctx, c, e, m, ls) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + } + err := n.Save(ctx, ls) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + ref := n.Reference() + + // reload and remove + nn := mantaray.NewNodeRef(ref) + for i := 0; i < len(tc.toRemove); i++ { + c := tc.toRemove[i] + err := nn.Remove(ctx, c, ls) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + } + err = nn.Save(ctx, ls) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + ref = nn.Reference() + + // reload and lookup removed node + nnn := mantaray.NewNodeRef(ref) + for i := 0; i < len(tc.toRemove); i++ { + c := tc.toRemove[i] + n, err = nnn.LookupNode(ctx, c, ls) + if !errors.Is(err, mantaray.ErrNotFound) { + t.Fatalf("expected not found error, got %v", err) + } + } + }) + } +} + type addr [32]byte type mockLoadSaver struct { mtx sync.Mutex From 6e5158fee031df30194118e33d1093680f135349 Mon Sep 17 00:00:00 2001 From: asabya Date: Fri, 13 Sep 2024 11:25:52 +0530 Subject: [PATCH 2/4] fix: https://github.com/ethersphere/bee/issues/4808, remove fork & releoad node --- pkg/manifest/mantaray/node.go | 18 +++++++++++++++++- pkg/manifest/mantaray/persist.go | 4 ++-- pkg/manifest/mantaray/persist_test.go | 26 +++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/pkg/manifest/mantaray/node.go b/pkg/manifest/mantaray/node.go index 9561c299db5..437eeb460da 100644 --- a/pkg/manifest/mantaray/node.go +++ b/pkg/manifest/mantaray/node.go @@ -313,10 +313,26 @@ func (n *Node) Remove(ctx context.Context, path []byte, ls LoadSaver) error { rest := path[len(f.prefix):] if len(rest) == 0 { // full path matched + + // Make the ref all zeros to indicate that this node needs to re-uploaded + n.ref = zero32 + // Set the refBytesSize to 32 so that unmarshall works properly + n.refBytesSize = len(n.ref) + + // remove the fork delete(n.forks, path[0]) return nil } - return f.Node.Remove(ctx, rest, ls) + err := f.Node.Remove(ctx, rest, ls) + if err != nil { + return err + } + // Make the ref all zeros to indicate that this node needs to re-uploaded + n.ref = zero32 + // Set the refBytesSize to 32 so that unmarshall works properly + n.refBytesSize = len(n.ref) + + return nil } func common(a, b []byte) (c []byte) { diff --git a/pkg/manifest/mantaray/persist.go b/pkg/manifest/mantaray/persist.go index 2bd07d74c94..8e4cfda6c1d 100644 --- a/pkg/manifest/mantaray/persist.go +++ b/pkg/manifest/mantaray/persist.go @@ -5,9 +5,9 @@ package mantaray import ( + "bytes" "context" "errors" - "golang.org/x/sync/errgroup" ) @@ -61,7 +61,7 @@ func (n *Node) Save(ctx context.Context, s Saver) error { } func (n *Node) save(ctx context.Context, s Saver) error { - if n != nil && n.ref != nil { + if n != nil && n.ref != nil && !bytes.Equal(n.ref, zero32) { return nil } select { diff --git a/pkg/manifest/mantaray/persist_test.go b/pkg/manifest/mantaray/persist_test.go index 1842b84e0e6..345953c941b 100644 --- a/pkg/manifest/mantaray/persist_test.go +++ b/pkg/manifest/mantaray/persist_test.go @@ -97,6 +97,29 @@ func TestPersistRemove(t *testing.T) { []byte("img/2.png"), }, }, + { + name: "nested-prefix-is-not-collapsed", + toAdd: []mantaray.NodeEntry{ + { + Path: []byte("index.html"), + }, + { + Path: []byte("img/1.png"), + }, + { + Path: []byte("img/2/test1.png"), + }, + { + Path: []byte("img/2/test2.png"), + }, + { + Path: []byte("robots.txt"), + }, + }, + toRemove: [][]byte{ + []byte("img/2/test1.png"), + }, + }, } { ctx := context.Background() var ls mantaray.LoadSaver = newMockLoadSaver() @@ -124,7 +147,6 @@ func TestPersistRemove(t *testing.T) { } ref := n.Reference() - // reload and remove nn := mantaray.NewNodeRef(ref) for i := 0; i < len(tc.toRemove); i++ { @@ -134,10 +156,12 @@ func TestPersistRemove(t *testing.T) { t.Fatalf("expected no error, got %v", err) } } + err = nn.Save(ctx, ls) if err != nil { t.Fatalf("expected no error, got %v", err) } + ref = nn.Reference() // reload and lookup removed node From 0069db309682e415132529c4bb5590bed2b5a986 Mon Sep 17 00:00:00 2001 From: asabya Date: Fri, 13 Sep 2024 11:28:55 +0530 Subject: [PATCH 3/4] chore: fix lint --- pkg/manifest/mantaray/persist_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/manifest/mantaray/persist_test.go b/pkg/manifest/mantaray/persist_test.go index 345953c941b..33538e59e8d 100644 --- a/pkg/manifest/mantaray/persist_test.go +++ b/pkg/manifest/mantaray/persist_test.go @@ -168,7 +168,7 @@ func TestPersistRemove(t *testing.T) { nnn := mantaray.NewNodeRef(ref) for i := 0; i < len(tc.toRemove); i++ { c := tc.toRemove[i] - n, err = nnn.LookupNode(ctx, c, ls) + _, err = nnn.LookupNode(ctx, c, ls) if !errors.Is(err, mantaray.ErrNotFound) { t.Fatalf("expected not found error, got %v", err) } From 8787863112d0265d56d497a4711eefccb119975e Mon Sep 17 00:00:00 2001 From: asabya Date: Mon, 4 Nov 2024 22:35:43 +0530 Subject: [PATCH 4/4] refactor: nil ref instead of all zeros --- pkg/manifest/mantaray/marshal.go | 4 +++- pkg/manifest/mantaray/node.go | 21 ++++----------------- pkg/manifest/mantaray/persist.go | 3 +-- pkg/manifest/mantaray/persist_test.go | 2 -- 4 files changed, 8 insertions(+), 22 deletions(-) diff --git a/pkg/manifest/mantaray/marshal.go b/pkg/manifest/mantaray/marshal.go index fcd19c55411..4e0324c35dd 100644 --- a/pkg/manifest/mantaray/marshal.go +++ b/pkg/manifest/mantaray/marshal.go @@ -287,6 +287,9 @@ func (n *Node) UnmarshalBinary(data []byte) error { bb.fromBytes(data[offset:]) offset += 32 // skip forks return bb.iter(func(b byte) error { + if refBytesSize == 0 { + return nil + } f := &fork{} if len(data) < offset+nodeForkTypeBytesSize { @@ -296,7 +299,6 @@ func (n *Node) UnmarshalBinary(data []byte) error { nodeType := data[offset] nodeForkSize := nodeForkPreReferenceSize + refBytesSize - if nodeTypeIsWithMetadataType(nodeType) { if len(data) < offset+nodeForkPreReferenceSize+refBytesSize+nodeForkMetadataBytesSize { return fmt.Errorf("not enough bytes for node fork: %d (%d) on byte '%x': %w", (len(data) - offset), (nodeForkPreReferenceSize + refBytesSize + nodeForkMetadataBytesSize), []byte{b}, ErrInvalidManifest) diff --git a/pkg/manifest/mantaray/node.go b/pkg/manifest/mantaray/node.go index 437eeb460da..2e2754f200b 100644 --- a/pkg/manifest/mantaray/node.go +++ b/pkg/manifest/mantaray/node.go @@ -311,28 +311,15 @@ func (n *Node) Remove(ctx context.Context, path []byte, ls LoadSaver) error { return ErrNotFound } rest := path[len(f.prefix):] + defer func() { + n.ref = nil + }() if len(rest) == 0 { - // full path matched - - // Make the ref all zeros to indicate that this node needs to re-uploaded - n.ref = zero32 - // Set the refBytesSize to 32 so that unmarshall works properly - n.refBytesSize = len(n.ref) - // remove the fork delete(n.forks, path[0]) return nil } - err := f.Node.Remove(ctx, rest, ls) - if err != nil { - return err - } - // Make the ref all zeros to indicate that this node needs to re-uploaded - n.ref = zero32 - // Set the refBytesSize to 32 so that unmarshall works properly - n.refBytesSize = len(n.ref) - - return nil + return f.Node.Remove(ctx, rest, ls) } func common(a, b []byte) (c []byte) { diff --git a/pkg/manifest/mantaray/persist.go b/pkg/manifest/mantaray/persist.go index 8e4cfda6c1d..8b18896fb61 100644 --- a/pkg/manifest/mantaray/persist.go +++ b/pkg/manifest/mantaray/persist.go @@ -5,7 +5,6 @@ package mantaray import ( - "bytes" "context" "errors" "golang.org/x/sync/errgroup" @@ -61,7 +60,7 @@ func (n *Node) Save(ctx context.Context, s Saver) error { } func (n *Node) save(ctx context.Context, s Saver) error { - if n != nil && n.ref != nil && !bytes.Equal(n.ref, zero32) { + if n != nil && n.ref != nil { return nil } select { diff --git a/pkg/manifest/mantaray/persist_test.go b/pkg/manifest/mantaray/persist_test.go index 33538e59e8d..301237eff89 100644 --- a/pkg/manifest/mantaray/persist_test.go +++ b/pkg/manifest/mantaray/persist_test.go @@ -145,7 +145,6 @@ func TestPersistRemove(t *testing.T) { if err != nil { t.Fatalf("expected no error, got %v", err) } - ref := n.Reference() // reload and remove nn := mantaray.NewNodeRef(ref) @@ -163,7 +162,6 @@ func TestPersistRemove(t *testing.T) { } ref = nn.Reference() - // reload and lookup removed node nnn := mantaray.NewNodeRef(ref) for i := 0; i < len(tc.toRemove); i++ {