From 89bf1f87fd895b5b58729a74664b685d9d7db381 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Mon, 8 Jan 2024 11:32:01 +0100 Subject: [PATCH] feat(pinning): pinning existing CID with different name updates pin --- CHANGELOG.md | 2 +- pinning/pinner/dspinner/pin.go | 27 +++++++++- pinning/pinner/dspinner/pin_test.go | 76 ++++++++++++++++++++++++----- pinning/pinner/pin.go | 3 +- 4 files changed, 91 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09265e27e..41ba9a3b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ The following emojis are used to highlight certain changes: ### Added -* 🛠 `pinning/pinner`: you can now give a custom name when pinning a CID. To reflect this, the `Pinner` has been adjusted. +* 🛠 `pinning/pinner`: you can now give a custom name when pinning a CID. To reflect this, the `Pinner` has been adjusted. Note that calling `Pin` for the same CID with a different name will replace its current name by the newly given name. ### Changed diff --git a/pinning/pinner/dspinner/pin.go b/pinning/pinner/dspinner/pin.go index 47ec7fea5..bc1f61902 100644 --- a/pinning/pinner/dspinner/pin.go +++ b/pinning/pinner/dspinner/pin.go @@ -197,8 +197,15 @@ func (p *pinner) doPinRecursive(ctx context.Context, c cid.Cid, fetch bool, name if err != nil { return err } + // Do not return immediately! Just remove the recursive pins for the current CID. + // This allows the process to continue and the pin to be re-added with a new name. + // + // TODO: remove this to support multiple pins per CID if found { - return nil + _, err = p.removePinsForCid(ctx, c, ipfspinner.Recursive) + if err != nil { + return err + } } dirtyBefore := p.dirty @@ -222,7 +229,7 @@ func (p *pinner) doPinRecursive(ctx context.Context, c cid.Cid, fetch bool, name // Only look again if something has changed. if p.dirty != dirtyBefore { - found, err = p.cidRIndex.HasAny(ctx, cidKey) + found, err := p.cidRIndex.HasAny(ctx, cidKey) if err != nil { return err } @@ -264,6 +271,22 @@ func (p *pinner) doPinDirect(ctx context.Context, c cid.Cid, name string) error return fmt.Errorf("%s already pinned recursively", c.String()) } + // Remove existing direct pins for this CID. This ensures that the pin will be + // re-saved with the new name and that there aren't clashing pins for the same + // CID. + // + // TODO: remove this to support multiple pins per CID. + found, err = p.cidDIndex.HasAny(ctx, cidKey) + if err != nil { + return err + } + if found { + _, err = p.removePinsForCid(ctx, c, ipfspinner.Direct) + if err != nil { + return err + } + } + _, err = p.addPin(ctx, c, ipfspinner.Direct, name) if err != nil { return err diff --git a/pinning/pinner/dspinner/pin_test.go b/pinning/pinner/dspinner/pin_test.go index 25dd4c009..54e308cba 100644 --- a/pinning/pinner/dspinner/pin_test.go +++ b/pinning/pinner/dspinner/pin_test.go @@ -99,6 +99,16 @@ func assertUnpinned(t *testing.T, p ipfspin.Pinner, c cid.Cid, failmsg string) { } } +func allPins(t *testing.T, ch <-chan ipfspin.StreamedPin) (pins []ipfspin.Pinned) { + for val := range ch { + if val.Err != nil { + t.Fatal(val.Err) + } + pins = append(pins, val.Pin) + } + return pins +} + func TestPinnerBasic(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -201,17 +211,7 @@ func TestPinnerBasic(t *testing.T) { dk := d.Cid() assertPinned(t, p, dk, "pinned node not found.") - allPins := func(ch <-chan ipfspin.StreamedPin) (pins []ipfspin.Pinned) { - for val := range ch { - if val.Err != nil { - t.Fatal(val.Err) - } - pins = append(pins, val.Pin) - } - return pins - } - - pins := allPins(p.RecursiveKeys(ctx, true)) + pins := allPins(t, p.RecursiveKeys(ctx, true)) if len(pins) != 2 { t.Error("expected 2 recursive pins") } @@ -256,7 +256,7 @@ func TestPinnerBasic(t *testing.T) { } } - pins = allPins(p.DirectKeys(ctx, false)) + pins = allPins(t, p.DirectKeys(ctx, false)) if len(pins) != 1 { t.Error("expected 1 direct pin") } @@ -264,7 +264,7 @@ func TestPinnerBasic(t *testing.T) { t.Error("wrong direct pin") } - pins = allPins(p.InternalPins(ctx, false)) + pins = allPins(t, p.InternalPins(ctx, false)) if len(pins) != 0 { t.Error("should not have internal keys") } @@ -385,6 +385,56 @@ func TestAddLoadPin(t *testing.T) { } } +func TestPinAddOverwriteName(t *testing.T) { + makeTest := func(recursive bool) func(t *testing.T) { + return func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + dstore := dssync.MutexWrap(ds.NewMapDatastore()) + bstore := blockstore.NewBlockstore(dstore) + bserv := bs.New(bstore, offline.Exchange(bstore)) + + dserv := mdag.NewDAGService(bserv) + + p, err := New(ctx, dstore, dserv) + require.NoError(t, err) + + a, aCid := randNode() + err = dserv.Add(ctx, a) + require.NoError(t, err) + + var ( + getPins func(ctx context.Context, detailed bool) <-chan ipfspin.StreamedPin + mode ipfspin.Mode + ) + + if recursive { + getPins = p.RecursiveKeys + mode = ipfspin.Recursive + } else { + getPins = p.DirectKeys + mode = ipfspin.Direct + } + + for _, name := range []string{"", "pin label", "yet another pin label"} { + err = p.Pin(ctx, a, recursive, name) + require.NoError(t, err) + + err = p.Flush(ctx) + require.NoError(t, err) + pins := allPins(t, getPins(ctx, true)) + require.Len(t, pins, 1) + require.Equal(t, aCid, pins[0].Key) + require.Equal(t, mode, pins[0].Mode) + require.Equal(t, name, pins[0].Name) + } + } + } + + t.Run("Direct", makeTest(false)) + t.Run("Recursive", makeTest(true)) +} + func TestIsPinnedLookup(t *testing.T) { // Test that lookups work in pins which share // the same branches. For that construct this tree: diff --git a/pinning/pinner/pin.go b/pinning/pinner/pin.go index 1670735d7..b5b2fd187 100644 --- a/pinning/pinner/pin.go +++ b/pinning/pinner/pin.go @@ -94,7 +94,8 @@ type Pinner interface { // Pin the given node, optionally recursively. // Pin will make sure that the given node and its children if recursive is set - // are stored locally. + // are stored locally. Pinning with a different name for an already existing + // pin must replace the existing name. Pin(ctx context.Context, node ipld.Node, recursive bool, name string) error // Unpin the given cid. If recursive is true, removes either a recursive or