From 23db807b87ee57ef70d7a0a72a89a49ea23e02db Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Fri, 1 Mar 2019 17:46:27 +0000 Subject: [PATCH] ipfsproxy: use PinPath to match IPFS behaviour License: MIT Signed-off-by: Hector Sanjuan --- api/ipfsproxy/ipfsproxy.go | 25 +++++++++------- api/ipfsproxy/ipfsproxy_test.go | 40 ++++++++++++++++++------- api/rest/client/methods_test.go | 23 ++++++++++----- api/rest/restapi.go | 2 +- api/rest/restapi_test.go | 52 ++++++++++++++++++++++++--------- rpc_api.go | 4 +-- state/mapstate/migrate.go | 2 +- test/rpc_api_mock.go | 34 ++++++++++++++------- 8 files changed, 125 insertions(+), 57 deletions(-) diff --git a/api/ipfsproxy/ipfsproxy.go b/api/ipfsproxy/ipfsproxy.go index 14e34a0b..6abb10dc 100644 --- a/api/ipfsproxy/ipfsproxy.go +++ b/api/ipfsproxy/ipfsproxy.go @@ -13,10 +13,6 @@ import ( "sync" "time" - "go.opencensus.io/plugin/ochttp" - "go.opencensus.io/plugin/ochttp/propagation/tracecontext" - "go.opencensus.io/trace" - "github.com/ipfs/ipfs-cluster/adder/adderutils" "github.com/ipfs/ipfs-cluster/api" "github.com/ipfs/ipfs-cluster/rpcutil" @@ -24,10 +20,15 @@ import ( mux "github.com/gorilla/mux" cid "github.com/ipfs/go-cid" logging "github.com/ipfs/go-log" + path "github.com/ipfs/go-path" rpc "github.com/libp2p/go-libp2p-gorpc" peer "github.com/libp2p/go-libp2p-peer" madns "github.com/multiformats/go-multiaddr-dns" manet "github.com/multiformats/go-multiaddr-net" + + "go.opencensus.io/plugin/ochttp" + "go.opencensus.io/plugin/ochttp/propagation/tracecontext" + "go.opencensus.io/trace" ) // DNSTimeout is used when resolving DNS multiaddresses in this module @@ -294,18 +295,20 @@ func (proxy *Server) pinOpHandler(op string, w http.ResponseWriter, r *http.Requ proxy.setHeaders(w.Header(), r) arg := r.URL.Query().Get("arg") - c, err := cid.Decode(arg) + p, err := path.ParsePath(arg) if err != nil { - ipfsErrorResponder(w, "Error parsing CID: "+err.Error()) + ipfsErrorResponder(w, "Error parsing IPFS Path: "+err.Error()) return } + pinPath := &api.PinPath{Path: p.String()} + var pin api.Pin err = proxy.rpcClient.Call( "", "Cluster", op, - api.PinCid(c), - &struct{}{}, + pinPath, + &pin, ) if err != nil { ipfsErrorResponder(w, err.Error()) @@ -313,7 +316,7 @@ func (proxy *Server) pinOpHandler(op string, w http.ResponseWriter, r *http.Requ } res := ipfsPinOpResp{ - Pins: []string{arg}, + Pins: []string{pin.Cid.String()}, } resBytes, _ := json.Marshal(res) w.WriteHeader(http.StatusOK) @@ -322,11 +325,11 @@ func (proxy *Server) pinOpHandler(op string, w http.ResponseWriter, r *http.Requ } func (proxy *Server) pinHandler(w http.ResponseWriter, r *http.Request) { - proxy.pinOpHandler("Pin", w, r) + proxy.pinOpHandler("PinPath", w, r) } func (proxy *Server) unpinHandler(w http.ResponseWriter, r *http.Request) { - proxy.pinOpHandler("Unpin", w, r) + proxy.pinOpHandler("UnpinPath", w, r) } func (proxy *Server) pinLsHandler(w http.ResponseWriter, r *http.Request) { diff --git a/api/ipfsproxy/ipfsproxy_test.go b/api/ipfsproxy/ipfsproxy_test.go index 5857b683..6f9b9cd6 100644 --- a/api/ipfsproxy/ipfsproxy_test.go +++ b/api/ipfsproxy/ipfsproxy_test.go @@ -87,7 +87,7 @@ func TestIPFSProxyPin(t *testing.T) { type args struct { urlPath string - testCid cid.Cid + testCid string statusCode int } tests := []struct { @@ -100,17 +100,27 @@ func TestIPFSProxyPin(t *testing.T) { "pin good cid query arg", args{ "/pin/add?arg=", - test.Cid1, + test.Cid1.String(), http.StatusOK, }, test.Cid1, false, }, + { + "pin good path query arg", + args{ + "/pin/add?arg=", + test.PathIPFS2, + http.StatusOK, + }, + test.CidResolved, + false, + }, { "pin good cid url arg", args{ "/pin/add/", - test.Cid1, + test.Cid1.String(), http.StatusOK, }, test.Cid1, @@ -120,7 +130,7 @@ func TestIPFSProxyPin(t *testing.T) { "pin bad cid query arg", args{ "/pin/add?arg=", - test.ErrorCid, + test.ErrorCid.String(), http.StatusInternalServerError, }, cid.Undef, @@ -130,7 +140,7 @@ func TestIPFSProxyPin(t *testing.T) { "pin bad cid url arg", args{ "/pin/add/", - test.ErrorCid, + test.ErrorCid.String(), http.StatusInternalServerError, }, cid.Undef, @@ -195,7 +205,7 @@ func TestIPFSProxyUnpin(t *testing.T) { type args struct { urlPath string - testCid cid.Cid + testCid string statusCode int } tests := []struct { @@ -208,17 +218,27 @@ func TestIPFSProxyUnpin(t *testing.T) { "unpin good cid query arg", args{ "/pin/rm?arg=", - test.Cid1, + test.Cid1.String(), http.StatusOK, }, test.Cid1, false, }, + { + "unpin good path query arg", + args{ + "/pin/rm?arg=", + test.PathIPFS2, + http.StatusOK, + }, + test.CidResolved, + false, + }, { "unpin good cid url arg", args{ "/pin/rm/", - test.Cid1, + test.Cid1.String(), http.StatusOK, }, test.Cid1, @@ -228,7 +248,7 @@ func TestIPFSProxyUnpin(t *testing.T) { "unpin bad cid query arg", args{ "/pin/rm?arg=", - test.ErrorCid, + test.ErrorCid.String(), http.StatusInternalServerError, }, cid.Undef, @@ -238,7 +258,7 @@ func TestIPFSProxyUnpin(t *testing.T) { "unpin bad cid url arg", args{ "/pin/rm/", - test.ErrorCid, + test.ErrorCid.String(), http.StatusInternalServerError, }, cid.Undef, diff --git a/api/rest/client/methods_test.go b/api/rest/client/methods_test.go index 242ff841..c8d7455d 100644 --- a/api/rest/client/methods_test.go +++ b/api/rest/client/methods_test.go @@ -173,34 +173,41 @@ func TestUnpin(t *testing.T) { } type pathCase struct { - path string - wantErr bool + path string + wantErr bool + expectedCid string } var pathTestCases = []pathCase{ { test.CidResolved.String(), false, + test.CidResolved.String(), }, { test.PathIPFS1, false, + "QmaNJ5acV31sx8jq626qTpAWW4DXKw34aGhx53dECLvXbY", }, { test.PathIPFS2, false, + test.CidResolved.String(), }, { test.PathIPNS1, false, + test.CidResolved.String(), }, { test.PathIPLD1, false, + "QmaNJ5acV31sx8jq626qTpAWW4DXKw34aGhx53dECLvXbY", }, { test.InvalidPath1, true, + "", }, } @@ -213,14 +220,16 @@ func TestPinPath(t *testing.T) { ReplicationFactorMin: 6, ReplicationFactorMax: 7, Name: "hello there", - UserAllocations: []string{"QmWPKsvv9VCXmnmX4YGNaYUmB4MbwKyyLsVDYxTQXkNdxt", "QmWPKsvv9VCVTomX4YbNaTUmJ4MbwgyyVsVDtxXQXkNdxt"}, + UserAllocations: []string{ + "QmWPKsvv9VCXmnmX4YGNaYUmB4MbwKyyLsVDYxTQXkNdxt", + "QmWPKsvv9VCVTomX4YbNaTUmJ4MbwgyyVsVDtxXQXkNdxt", + }, } - resultantPin := types.PinWithOpts(test.CidResolved, opts) - testF := func(t *testing.T, c Client) { - for _, testCase := range pathTestCases { + ec, _ := cid.Decode(testCase.expectedCid) + resultantPin := types.PinWithOpts(ec, opts) p := testCase.path pin, err := c.PinPath(ctx, p, opts) if err != nil { @@ -258,7 +267,7 @@ func TestUnpinPath(t *testing.T) { t.Fatalf("unepected error %s: %s", p, err) } - if !pin.Cid.Equals(test.CidResolved) { + if pin.Cid.String() != testCase.expectedCid { t.Errorf("bad resolved Cid: %s, %s", p, pin.Cid) } } diff --git a/api/rest/restapi.go b/api/rest/restapi.go index 718081df..6af83d3a 100644 --- a/api/rest/restapi.go +++ b/api/rest/restapi.go @@ -728,7 +728,7 @@ func (api *API) unpinPathHandler(w http.ResponseWriter, r *http.Request) { "", "Cluster", "UnpinPath", - pinpath.Path, + pinpath, &pin, ) api.sendResponse(w, http.StatusOK, err, pin) diff --git a/api/rest/restapi_test.go b/api/rest/restapi_test.go index d521d0c7..024ac310 100644 --- a/api/rest/restapi_test.go +++ b/api/rest/restapi_test.go @@ -14,6 +14,8 @@ import ( "testing" "time" + cid "github.com/ipfs/go-cid" + "github.com/ipfs/ipfs-cluster/api" "github.com/ipfs/ipfs-cluster/test" @@ -589,10 +591,11 @@ func TestAPIPinEndpoint(t *testing.T) { } type pathCase struct { - path string - opts api.PinOptions - wantErr bool - code int + path string + opts api.PinOptions + wantErr bool + code int + expectedCid string } func (p *pathCase) WithQuery() string { @@ -612,18 +615,21 @@ var pathTestCases = []pathCase{ testPinOpts, false, http.StatusOK, + "QmaNJ5acV31sx8jq626qTpAWW4DXKw34aGhx53dECLvXbY", }, { "/ipfs/QmbUNM297ZwxB8CfFAznK7H9YMesDoY6Tt5bPgt5MSCB2u/im.gif", testPinOpts, false, http.StatusOK, + test.CidResolved.String(), }, { "/ipfs/invalidhash", testPinOpts, true, http.StatusBadRequest, + "", }, // TODO: Test StatusNotFound and a case with trailing slash with paths // test.PathIPNS2, test.PathIPLD2, test.InvalidPath1 @@ -634,25 +640,33 @@ func TestAPIPinEndpointWithPath(t *testing.T) { rest := testAPI(t) defer rest.Shutdown(ctx) - resultantPin := api.PinWithOpts( - test.CidResolved, - testPinOpts, - ) - tf := func(t *testing.T, url urlF) { for _, testCase := range pathTestCases { + c, _ := cid.Decode(testCase.expectedCid) + resultantPin := api.PinWithOpts( + c, + testPinOpts, + ) + if testCase.wantErr { errResp := api.Error{} makePost(t, rest, url(rest)+"/pins"+testCase.WithQuery(), []byte{}, &errResp) if errResp.Code != testCase.code { - t.Errorf("expected different status code, expected: %d, actual: %d, path: %s\n", testCase.code, errResp.Code, testCase.path) + t.Errorf( + "status code: expected: %d, got: %d, path: %s\n", + testCase.code, + errResp.Code, + testCase.path, + ) } continue } pin := api.Pin{} makePost(t, rest, url(rest)+"/pins"+testCase.WithQuery(), []byte{}, &pin) if !pin.Equals(resultantPin) { - t.Errorf("expected different pin,\n expected: %+v,\n actual: %+v,\n path: %s\n", resultantPin, pin, testCase.path) + t.Errorf("pin: expected: %+v", resultantPin) + t.Errorf("pin: got: %+v", pin) + t.Errorf("path: %s", testCase.path) } } } @@ -695,14 +709,24 @@ func TestAPIUnpinEndpointWithPath(t *testing.T) { errResp := api.Error{} makeDelete(t, rest, url(rest)+"/pins"+testCase.path, &errResp) if errResp.Code != testCase.code { - t.Errorf("expected different status code, expected: %d, actual: %d, path: %s\n", testCase.code, errResp.Code, testCase.path) + t.Errorf( + "status code: expected: %d, got: %d, path: %s\n", + testCase.code, + errResp.Code, + testCase.path, + ) } continue } pin := api.Pin{} makeDelete(t, rest, url(rest)+"/pins"+testCase.path, &pin) - if !pin.Cid.Equals(test.CidResolved) { - t.Errorf("expected different cid, expected: %s, actual: %s, path: %s\n", test.CidResolved, pin.Cid, testCase.path) + if pin.Cid.String() != testCase.expectedCid { + t.Errorf( + "cid: expected: %s, got: %s, path: %s\n", + test.CidResolved, + pin.Cid, + testCase.path, + ) } } } diff --git a/rpc_api.go b/rpc_api.go index 46e75112..913f04fe 100644 --- a/rpc_api.go +++ b/rpc_api.go @@ -55,8 +55,8 @@ func (rpcapi *RPCAPI) PinPath(ctx context.Context, in *api.PinPath, out *api.Pin } // UnpinPath resolves path into a cid and runs Cluster.Unpin(). -func (rpcapi *RPCAPI) UnpinPath(ctx context.Context, in string, out *api.Pin) error { - pin, err := rpcapi.c.UnpinPath(ctx, in) +func (rpcapi *RPCAPI) UnpinPath(ctx context.Context, in *api.PinPath, out *api.Pin) error { + pin, err := rpcapi.c.UnpinPath(ctx, in.Path) if err != nil { return err } diff --git a/state/mapstate/migrate.go b/state/mapstate/migrate.go index acc169e2..87523840 100644 --- a/state/mapstate/migrate.go +++ b/state/mapstate/migrate.go @@ -199,7 +199,7 @@ func (st *mapStateV5) unmarshal(r io.Reader) error { func (st *mapStateV5) next() migrateable { v6 := NewMapState() for k, v := range st.PinMap { - logger.Infof("migrating", k, v.Cid) + logger.Infof("migrating %s", k) // we need to convert because we added codec struct fields // and thus serialization is not the same. p := &api.Pin{} diff --git a/test/rpc_api_mock.go b/test/rpc_api_mock.go index ac94d2b9..b733994e 100644 --- a/test/rpc_api_mock.go +++ b/test/rpc_api_mock.go @@ -3,16 +3,17 @@ package test import ( "context" "errors" + "strings" "testing" "time" + "github.com/ipfs/ipfs-cluster/api" + cid "github.com/ipfs/go-cid" gopath "github.com/ipfs/go-path" rpc "github.com/libp2p/go-libp2p-gorpc" host "github.com/libp2p/go-libp2p-host" peer "github.com/libp2p/go-libp2p-peer" - - "github.com/ipfs/ipfs-cluster/api" ) // ErrBadCid is returned when using ErrorCid. Operations with that CID always @@ -54,21 +55,32 @@ func (mock *mockService) Unpin(ctx context.Context, in *api.Pin, out *struct{}) } func (mock *mockService) PinPath(ctx context.Context, in *api.PinPath, out *api.Pin) error { - _, err := gopath.ParsePath(in.Path) + p, err := gopath.ParsePath(in.Path) if err != nil { return err } - *out = *api.PinWithOpts(CidResolved, in.PinOptions) + + var pin *api.Pin + if p.IsJustAKey() && !strings.HasPrefix(in.Path, "/ipns") { + c, _, err := gopath.SplitAbsPath(p) + if err != nil { + return err + } + if c.Equals(ErrorCid) { + return ErrBadCid + } + pin = api.PinWithOpts(c, in.PinOptions) + } else { + pin = api.PinWithOpts(CidResolved, in.PinOptions) + } + + *out = *pin return nil } -func (mock *mockService) UnpinPath(ctx context.Context, in string, out *api.Pin) error { - _, err := gopath.ParsePath(in) - if err != nil { - return err - } - *out = *api.PinCid(CidResolved) - return nil +func (mock *mockService) UnpinPath(ctx context.Context, in *api.PinPath, out *api.Pin) error { + // Mock-Unpin behaves exactly pin (doing nothing). + return mock.PinPath(ctx, in, out) } func (mock *mockService) Pins(ctx context.Context, in struct{}, out *[]*api.Pin) error {