Some tests. Mostly to check if travis/coveralls work

cluster.go

@@ -211,7 +211,7 @@ func (c *Cluster) handleOp(rpc ClusterRPC) {
 	case IPFSIsPinnedRPC:
 		data, err = c.ipfs.IsPinned(crpc.CID)
 	case RollbackRPC:
-		state, ok := grpc.Arguments.(ClusterState)
+		state, ok := grpc.Argument.(ClusterState)
 		if !ok {
 			err = errors.New("Bad RollbackRPC type")
 			break
@@ -220,7 +220,7 @@ func (c *Cluster) handleOp(rpc ClusterRPC) {
 	case LeaderRPC:
 		// Leader RPC is a RPC that needs to be run
 		// by the Consensus Leader. Arguments is a wrapped RPC.
-		rpc, ok := grpc.Arguments.(*ClusterRPC)
+		rpc, ok := grpc.Argument.(*ClusterRPC)
 		if !ok {
 			err = errors.New("Bad LeaderRPC type")
 		}

ipfs_cluster.go

@@ -23,7 +23,8 @@ var logger = logging.Logger("ipfs-cluster")
 // Current Cluster version.
 const Version = "0.0.1"
 
-// RPCMaxQueue can be used to set the size of the ClusterRPC channels.
+// RPCMaxQueue can be used to set the size of the ClusterRPC channels,
+// which will start blocking on send after reaching this number.
 var RPCMaxQueue = 128
 
 // MakeRPCRetryInterval specifies how long to wait before retrying
@@ -31,10 +32,11 @@ var RPCMaxQueue = 128
 var MakeRPCRetryInterval time.Duration = 1
 
 // ClusterComponent represents a piece of ipfscluster. Cluster components
-// usually run their own goroutines (a http server for example) which can
+// usually run their own goroutines (a http server for example). They
-// be controlled via start and stop via Start() and Stop(). A ClusterRPC
+// communicate with Cluster via a channel carrying ClusterRPC operations.
-// channel is used by Cluster to perform operations requested by the
+// These operations request tasks from other components. This way all components
-// component.
+// are independent from each other and can be swapped as long as they maintain
+// RPC compatibility with Cluster.
 type ClusterComponent interface {
 	Shutdown() error
 	RpcChan() <-chan ClusterRPC
@@ -47,7 +49,7 @@ type ClusterAPI interface {
 }
 
 // IPFSConnector is a component which allows cluster to interact with
-// an IPFS daemon.
+// an IPFS daemon. This is a base component.
 type IPFSConnector interface {
 	ClusterComponent
 	Pin(*cid.Cid) error
@@ -56,7 +58,7 @@ type IPFSConnector interface {
 }
 
 // Peered represents a component which needs to be aware of the peers
-// in the Cluster.
+// in the Cluster and of any changes to the peer set.
 type Peered interface {
 	AddPeer(p peer.ID)
 	RmPeer(p peer.ID)
@@ -67,7 +69,9 @@ type Peered interface {
 // is used by the ClusterConsensus component to keep track of
 // objects which objects are pinned. This component should be thread safe.
 type ClusterState interface {
+	// AddPin adds a pin to the ClusterState
 	AddPin(*cid.Cid) error
+	// RmPin removes a pin from the ClusterState
 	RmPin(*cid.Cid) error
 }
 
@@ -102,18 +106,19 @@ type PinTracker interface {
 	SyncAll() []Pin
 }
 
-// MakeRPC sends a ClusterRPC object over a channel and waits for an answer on
+// MakeRPC sends a ClusterRPC object over a channel and optionally waits for a
-// ClusterRPC.ResponseCh channel. It can be used by any ClusterComponent to
+// Response on the ClusterRPC.ResponseCh channel. It can be used by any
-// simplify making RPC requests to Cluster. The ctx parameter must be a
+// ClusterComponent to simplify making RPC requests to Cluster.
-// cancellable context, and can be used to timeout requests.
+// The ctx parameter must be a cancellable context, and can be used to
+// timeout requests.
 // If the message cannot be placed in the ClusterRPC channel, retries will be
-// issued every MakeRPCRetryInterval.
+// issued every MakeRPCRetryInterval seconds.
-func MakeRPC(ctx context.Context, ch chan ClusterRPC, r ClusterRPC, waitForResponse bool) RPCResponse {
+func MakeRPC(ctx context.Context, rpcCh chan ClusterRPC, r ClusterRPC, waitForResponse bool) RPCResponse {
 	logger.Debugf("Sending RPC %d", r.Op())
 	exitLoop := false
 	for !exitLoop {
 		select {
-		case ch <- r:
+		case rpcCh <- r:
 			exitLoop = true
 		case <-ctx.Done():
 			logger.Debug("Cancelling sending RPC")
@@ -134,7 +139,7 @@ func MakeRPC(ctx context.Context, ch chan ClusterRPC, r ClusterRPC, waitForRespo
 	select {
 	case resp, ok := <-r.ResponseCh():
 		logger.Debug("Response received")
-		if !ok { // Not interested in response
+		if !ok {
 			logger.Warning("Response channel closed. Ignoring")
 			return RPCResponse{
 				Data:  nil,

ipfs_cluster_test.go

@@ -0,0 +1,57 @@
+package ipfscluster
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+)
+
+var (
+	testCid  = "QmP63DkAFEnDYNjDYBpyNDfttu1fvUw99x1brscPzpqmmq"
+	testCid2 = "QmP63DkAFEnDYNjDYBpyNDfttu1fvUw99x1brscPzpqmma"
+	testCid3 = "QmP63DkAFEnDYNjDYBpyNDfttu1fvUw99x1brscPzpqmmb"
+)
+
+func TestMakeRPC(t *testing.T) {
+	testCh := make(chan ClusterRPC, 1)
+	testReq := RPC(MemberListRPC, nil)
+	testResp := RPCResponse{
+		Data:  "hey",
+		Error: nil,
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+
+	// Test wait for response
+	go func() {
+		req := <-testCh
+		if req.Op() != MemberListRPC {
+			t.Fatal("expected a MemberListRPC")
+		}
+		req.ResponseCh() <- testResp
+	}()
+
+	resp := MakeRPC(ctx, testCh, testReq, true)
+	data, ok := resp.Data.(string)
+	if !ok || data != "hey" {
+		t.Error("bad response")
+	}
+
+	// Test not waiting for response
+	resp = MakeRPC(ctx, testCh, testReq, false)
+	if resp.Data != nil || resp.Error != nil {
+		t.Error("expected empty response")
+	}
+
+	// Test full channel and cancel
+	go func() {
+		resp := MakeRPC(ctx, testCh, testReq, true)
+		if resp.Error == nil || !strings.Contains(resp.Error.Error(), "timed out") {
+			t.Fatal("the operation should have been waiting and then cancelled")
+		}
+	}()
+	// Previous request still taking the channel
+	time.Sleep(2 * time.Second)
+	cancel()
+}

ipfs_connector.go

@@ -216,7 +216,8 @@ func (ipfs *IPFSHTTPConnector) pinType(hash *cid.Cid) (string, error) {
 
 	err = json.Unmarshal(body, &resp)
 	if err != nil {
-		logger.Error("parsing pin/ls response")
+		logger.Error("parsing pin/ls response:")
+		logger.Error(string(body))
 		return "", err
 	}
 	pinObj, ok := resp.Keys[hash.String()]

ipfs_connector_test.go

@@ -0,0 +1,122 @@
+package ipfscluster
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"strings"
+	"testing"
+
+	cid "gx/ipfs/QmcTcsTvfaeEBRFo1TkFgT8sRmgi1n1LTZpecfVP8fzpGD/go-cid"
+)
+
+func testServer(t *testing.T) *httptest.Server {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		//t.Log(r.URL.String())
+		switch r.URL.Path {
+		case "/api/v0/pin/add":
+			if r.URL.RawQuery == fmt.Sprintf("arg=%s", testCid) {
+				fmt.Fprintln(w, `{ "pinned": "`+testCid+`" }`)
+			} else {
+				w.WriteHeader(http.StatusInternalServerError)
+			}
+		case "/api/v0/pin/rm":
+			if r.URL.RawQuery == fmt.Sprintf("arg=%s", testCid) {
+				fmt.Fprintln(w, `{ "unpinned": "`+testCid+`" }`)
+			} else {
+				w.WriteHeader(http.StatusInternalServerError)
+			}
+		case "/api/v0/pin/ls":
+			if r.URL.RawQuery == fmt.Sprintf("arg=%s", testCid) {
+				fmt.Fprintln(w,
+					`{"Keys":{"`+testCid+`":{"Type":"recursive"}}}`)
+			} else {
+				fmt.Fprintln(w,
+					`{"Keys":{"`+testCid2+`":{"Type":"indirect"}}}`)
+			}
+		default:
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	t.Log("test server url: ", ts.URL)
+	return ts
+}
+
+func testIPFSConnectorConfig(ts *httptest.Server) *ClusterConfig {
+	url, _ := url.Parse(ts.URL)
+	h := strings.Split(url.Host, ":")
+	i, _ := strconv.Atoi(h[1])
+
+	return &ClusterConfig{
+		IPFSHost:          h[0],
+		IPFSPort:          i,
+		IPFSAPIListenAddr: "127.0.0.1",
+		IPFSAPIListenPort: 5000,
+	}
+}
+
+func TestNewIPFSHTTPConnector(t *testing.T) {
+	ts := testServer(t)
+	cfg := testIPFSConnectorConfig(ts)
+
+	ipfs, err := NewIPFSHTTPConnector(cfg)
+	if err != nil {
+		t.Fatal("creating an IPFSConnector should work")
+	}
+
+	ch := ipfs.RpcChan()
+	if ch == nil {
+		t.Error("RpcCh should be created")
+	}
+
+	c, _ := cid.Decode(testCid)
+	c2, _ := cid.Decode(testCid2)
+	c3, _ := cid.Decode(testCid3)
+	err = ipfs.Pin(c)
+	if err != nil {
+		t.Error("expected success pinning cid")
+	}
+	err = ipfs.Pin(c2)
+	if err == nil {
+		t.Error("expected error pinning cid")
+	}
+
+	err = ipfs.Unpin(c)
+	if err != nil {
+		t.Error("expected success unpinning cid")
+	}
+
+	err = ipfs.Unpin(c2)
+	if err != nil {
+		t.Error("expected error unpinning cid")
+	}
+
+	err = ipfs.Unpin(c3)
+	if err == nil {
+		t.Error("expected error unpinning cid")
+	}
+
+	isp, err := ipfs.IsPinned(c)
+	if err != nil || !isp {
+		t.Error("c should appear pinned")
+	}
+
+	isp, err = ipfs.IsPinned(c2)
+	if err != nil || isp {
+		t.Error("c2 should appear unpinned")
+	}
+
+	res, err := http.Get("http://127.0.0.1:5000/api/v0/add?arg=" + testCid)
+	if err != nil {
+		t.Fatal("should forward requests to ipfs host: ", err)
+	}
+	if res.StatusCode != http.StatusOK {
+		t.Error("the request should have succeeded")
+	}
+
+	if err := ipfs.Shutdown(); err != nil {
+		t.Error("expected a clean shutdown")
+	}
+}

rpc.go

@@ -16,44 +16,51 @@ const (
 	LeaderRPC
 )
 
-// RPCMethod identifies which RPC-supported operation we are trying to make
+// RPCMethod identifies which RPC supported operation we are trying to make
 type RPCOp int
 
+// ClusterRPC represents an internal RPC operation. It should be implemented
+// by all RPC types.
 type ClusterRPC interface {
 	Op() RPCOp
 	ResponseCh() chan RPCResponse
 }
 
+// baseRPC implements ClusterRPC and can be included as anonymous
+// field in other types.
 type baseRPC struct {
 	method     RPCOp
 	responseCh chan RPCResponse
 }
 
-// Method returns the RPC method for this request
+// Op returns the RPC method for this request
 func (brpc *baseRPC) Op() RPCOp {
 	return brpc.method
 }
 
-// ResponseCh returns a channel to send the RPCResponse
+// ResponseCh returns a channel on which the result of the
+// RPC operation can be sent.
 func (brpc *baseRPC) ResponseCh() chan RPCResponse {
 	return brpc.responseCh
 }
 
-// GenericClusterRPC is used to let Cluster perform operations as mandated by
+// GenericClusterRPC is a ClusterRPC with generic arguments.
-// its ClusterComponents. The result is placed on the ResponseCh channel.
 type GenericClusterRPC struct {
 	baseRPC
-	Arguments interface{}
+	Argument interface{}
 }
 
+// CidClusterRPC is a ClusterRPC whose only argument is a CID.
 type CidClusterRPC struct {
 	baseRPC
 	CID *cid.Cid
 }
 
-// RPC builds a ClusterRPC request.
+// RPC builds a ClusterRPC request. It will create a
-func RPC(m RPCOp, args interface{}) ClusterRPC {
+// CidClusterRPC if the arg is of type cid.Cid. Otherwise,
-	c, ok := args.(*cid.Cid)
+// a GenericClusterRPC is returned.
+func RPC(m RPCOp, arg interface{}) ClusterRPC {
+	c, ok := arg.(*cid.Cid)
 	if ok { // Its a CID
 		r := new(CidClusterRPC)
 		r.method = m
@@ -64,12 +71,13 @@ func RPC(m RPCOp, args interface{}) ClusterRPC {
 	// Its not a cid, make a generic
 	r := new(GenericClusterRPC)
 	r.method = m
-	r.Arguments = args
+	r.Argument = arg
 	r.responseCh = make(chan RPCResponse)
 	return r
 }
 
-// RPC response carries the result of an ClusterRPC-requested operation
+// RPCResponse carries the result of a ClusterRPC requested operation and/or
+// an error to indicate if the operation was successful.
 type RPCResponse struct {
 	Data  interface{}
 	Error error

rpc_test.go

@@ -0,0 +1,41 @@
+package ipfscluster
+
+import (
+	"testing"
+
+	cid "gx/ipfs/QmcTcsTvfaeEBRFo1TkFgT8sRmgi1n1LTZpecfVP8fzpGD/go-cid"
+)
+
+func TestRPC(t *testing.T) {
+	c, err := cid.Decode(testCid)
+	if err != nil {
+		t.Fatal(err)
+	}
+	crpc := RPC(IPFSPinRPC, c)
+	_, ok := crpc.(*CidClusterRPC)
+	if !ok {
+		t.Error("expected a CidClusterRPC")
+	}
+
+	if crpc.Op() != IPFSPinRPC {
+		t.Error("unexpected Op() type")
+	}
+
+	if crpc.ResponseCh() == nil {
+		t.Error("should have made the ResponseCh")
+	}
+
+	grpc := RPC(MemberListRPC, 3)
+	_, ok = grpc.(*GenericClusterRPC)
+	if !ok {
+		t.Error("expected a GenericClusterRPC")
+	}
+
+	if grpc.Op() != MemberListRPC {
+		t.Error("unexpected Op() type")
+	}
+
+	if grpc.ResponseCh() == nil {
+		t.Error("should have created the ResponseCh")
+	}
+}