From d1700dbe81acbe9129d39a1b237852e18a7f9374 Mon Sep 17 00:00:00 2001
From: Hector Sanjuan <code@hector.link>
Date: Wed, 24 Mar 2021 00:47:15 +0100
Subject: [PATCH] Fixes #1319: Status wrongly shows pins as REMOTE

The Allocations of a pin that has been added with default replication factor
are kept even when the replication factor turns out to be -1.

This resulted in the Status(cid) code skipping calls to a number of peers
and setting the pin directly as REMOTE.

The fix, on one side makes sure Allocations is always nil when the replication
factor is -1. On the other size, lets the globalPinInfoCid method check the
replication factor value, rather than the number of allocations to decide if
any nodes are bound to be remote.

On the plus side, the pin tracker used the IsRemotePin method, which uses the
replication factor, so things were pinned even if the Status(cid) method shows
them as remote.
---
 api/types.go                       |  7 ++++++-
 cluster.go                         | 16 ++++++++++++++--
 cmd/ipfs-cluster-ctl/formatters.go |  2 +-
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/api/types.go b/api/types.go
index 12b6c5e5..4de6b091 100644
--- a/api/types.go
+++ b/api/types.go
@@ -792,6 +792,11 @@ func (pin *Pin) String() string {
 	return b.String()
 }
 
+// IsPinEverywhere returns when the both replication factors are set to -1.
+func (pin *Pin) IsPinEverywhere() bool {
+	return pin.ReplicationFactorMin == -1 && pin.ReplicationFactorMax == -1
+}
+
 // PinPath is a wrapper for holding pin options and path of the content.
 type PinPath struct {
 	PinOptions
@@ -985,7 +990,7 @@ func (pin *Pin) Equals(pin2 *Pin) bool {
 // IsRemotePin determines whether a Pin's ReplicationFactor has
 // been met, so as to either pin or unpin it from the peer.
 func (pin *Pin) IsRemotePin(pid peer.ID) bool {
-	if pin.ReplicationFactorMax < 0 || pin.ReplicationFactorMin < 0 {
+	if pin.IsPinEverywhere() {
 		return false
 	}
 
diff --git a/cluster.go b/cluster.go
index e3787e36..237f3308 100644
--- a/cluster.go
+++ b/cluster.go
@@ -1288,6 +1288,15 @@ func (c *Cluster) setupReplicationFactor(pin *api.Pin) error {
 		pin.ReplicationFactorMax = rplMax
 	}
 
+	// When pinning everywhere, remove all allocations.
+	// Allocations may have been preset by the adder
+	// for the cases when the replication factor is > -1.
+	// Fixes part of #1319: allocations when adding
+	// are kept.
+	if pin.IsPinEverywhere() {
+		pin.Allocations = nil
+	}
+
 	return isReplicationFactorValid(rplMin, rplMax)
 }
 
@@ -1317,7 +1326,7 @@ func checkPinType(pin *api.Pin) error {
 			return errors.New("clusterDAG pins should reference a Meta pin")
 		}
 	case api.MetaType:
-		if pin.Allocations != nil && len(pin.Allocations) != 0 {
+		if len(pin.Allocations) != 0 {
 			return errors.New("meta pin should not specify allocations")
 		}
 		if pin.Reference == nil {
@@ -1427,6 +1436,8 @@ func (c *Cluster) pin(
 	// allocate() will check which peers are currently allocated
 	// and try to respect them.
 	if len(pin.Allocations) == 0 {
+		// If replication factor is -1, this will return empty
+		// allocations.
 		allocs, err := c.allocate(
 			ctx,
 			pin.Cid,
@@ -1442,6 +1453,7 @@ func (c *Cluster) pin(
 		pin.Allocations = allocs
 	}
 
+	// If this is true, replication factor should be -1.
 	if len(pin.Allocations) == 0 {
 		logger.Infof("pinning %s everywhere:", pin.Cid)
 	} else {
@@ -1747,7 +1759,7 @@ func (c *Cluster) globalPinInfoCid(ctx context.Context, comp, method string, h c
 			return nil, err
 		}
 
-		if len(pin.Allocations) > 0 {
+		if !pin.IsPinEverywhere() {
 			dests = pin.Allocations
 			remote = peersSubtract(members, dests)
 		} else {
diff --git a/cmd/ipfs-cluster-ctl/formatters.go b/cmd/ipfs-cluster-ctl/formatters.go
index d9862a19..57d86b34 100644
--- a/cmd/ipfs-cluster-ctl/formatters.go
+++ b/cmd/ipfs-cluster-ctl/formatters.go
@@ -189,7 +189,7 @@ func textFormatPrintPin(obj *api.Pin) {
 
 	fmt.Printf("%s | %s | %s | ", obj.Cid, obj.Name, t)
 
-	if obj.ReplicationFactorMin < 0 {
+	if obj.IsPinEverywhere() {
 		fmt.Printf("Repl. Factor: -1 | Allocations: [everywhere]")
 	} else {
 		sortAlloc := api.PeersToStrings(obj.Allocations)