Refactor resource client api to be consistent with other juju client …

…apis
addyess · May 18, 2022 · cfd68c4 · cfd68c4
1 parent 83d6f60
commit cfd68c4
Show file tree

Hide file tree

Showing 42 changed files with 751 additions and 979 deletions.
diff --git a/...client/resources/private/client/client.go → api/agent/uniter/resource.go b/...client/resources/private/client/client.go → api/agent/uniter/resource.go
@@ -1,7 +1,7 @@
 // Copyright 2016 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package client
+package uniter
 
 import (
 	"context"

diff --git a/...t/resources/private/client/client_test.go → api/agent/uniter/resource_test.go b/...t/resources/private/client/client_test.go → api/agent/uniter/resource_test.go
@@ -1,7 +1,7 @@
 // Copyright 2016 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package client_test
+package uniter_test
 
 import (
 	"context"
@@ -14,8 +14,8 @@ import (
 	"github.com/juju/testing/filetesting"
 	gc "gopkg.in/check.v1"
 
+	"github.com/juju/juju/api/agent/uniter"
 	api "github.com/juju/juju/api/client/resources"
-	"github.com/juju/juju/api/client/resources/private/client"
 	"github.com/juju/juju/resource"
 	"github.com/juju/juju/resource/resourcetesting"
 	"github.com/juju/juju/rpc/params"
@@ -41,7 +41,7 @@ func (s *UnitFacadeClientSuite) TestNewUnitFacadeClient(c *gc.C) {
 	caller := &stubAPI{Stub: s.stub}
 	doer := &stubAPI{Stub: s.stub}
 
-	cl := client.NewUnitFacadeClient(context.Background(), caller, doer)
+	cl := uniter.NewUnitFacadeClient(context.Background(), caller, doer)
 
 	s.stub.CheckNoCalls(c)
 	c.Check(cl.FacadeCaller, gc.Equals, caller)
@@ -51,7 +51,7 @@ func (s *UnitFacadeClientSuite) TestNewUnitFacadeClient(c *gc.C) {
 func (s *UnitFacadeClientSuite) TestGetResource(c *gc.C) {
 	opened := resourcetesting.NewResource(c, s.stub, "spam", "a-application", "some data")
 	s.api.setResource(opened.Resource, opened)
-	cl := client.NewUnitFacadeClient(context.Background(), s.api, s.api)
+	cl := uniter.NewUnitFacadeClient(context.Background(), s.api, s.api)
 
 	info, content, err := cl.GetResource("spam")
 	c.Assert(err, jc.ErrorIsNil)
@@ -66,7 +66,7 @@ func (s *UnitFacadeClientSuite) TestUnitDoer(c *gc.C) {
 	req, err := http.NewRequest("GET", "/resources/eggs", body)
 	c.Assert(err, jc.ErrorIsNil)
 	var resp *http.Response
-	doer := client.NewUnitHTTPClient(context.Background(), s.api, "spam/1")
+	doer := uniter.NewUnitHTTPClient(context.Background(), s.api, "spam/1")
 
 	err = doer.Do(context.Background(), req, &resp)
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/api/base/mocks/httpdoer_mock.go b/api/base/mocks/httpdoer_mock.go
diff --git a/api/client/resources/base_test.go b/api/client/resources/base_test.go
@@ -0,0 +1,90 @@
+// Copyright 2015 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package resources_test
+
+import (
+	"time"
+
+	"github.com/golang/mock/gomock"
+	"github.com/juju/testing"
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/juju/api/base/mocks"
+	"github.com/juju/juju/api/client/resources"
+	"github.com/juju/juju/resource"
+	"github.com/juju/juju/resource/resourcetesting"
+	"github.com/juju/juju/rpc/params"
+)
+
+type BaseSuite struct {
+	testing.IsolationSuite
+
+	facade     *mocks.MockFacadeCaller
+	apiCaller  *mocks.MockAPICallCloser
+	httpClient *mocks.MockHTTPDoer
+	client     *resources.Client
+}
+
+func (s *BaseSuite) TearDownTest(c *gc.C) {
+	s.facade = nil
+	s.IsolationSuite.TearDownTest(c)
+}
+
+func (s *BaseSuite) setUpMocks(c *gc.C) *gomock.Controller {
+	ctrl := gomock.NewController(c)
+
+	s.httpClient = mocks.NewMockHTTPDoer(ctrl)
+	s.apiCaller = mocks.NewMockAPICallCloser(ctrl)
+	s.apiCaller.EXPECT().BestFacadeVersion(gomock.Any()).Return(3).AnyTimes()
+
+	s.facade = mocks.NewMockFacadeCaller(ctrl)
+	s.facade.EXPECT().RawAPICaller().Return(s.apiCaller).AnyTimes()
+	s.facade.EXPECT().BestAPIVersion().Return(2).AnyTimes()
+	s.client = resources.NewClientForTest(s.facade, s.httpClient)
+	return ctrl
+}
+
+func newResourceResult(c *gc.C, names ...string) ([]resource.Resource, params.ResourcesResult) {
+	var resources []resource.Resource
+	var apiResult params.ResourcesResult
+	for _, name := range names {
+		data := name + "...spamspamspam"
+		res, apiRes := newResource(c, name, "a-user", data)
+		resources = append(resources, res)
+		apiResult.Resources = append(apiResult.Resources, apiRes)
+	}
+	return resources, apiResult
+}
+
+func newResource(c *gc.C, name, username, data string) (resource.Resource, params.Resource) {
+	opened := resourcetesting.NewResource(c, nil, name, "a-application", data)
+	res := opened.Resource
+	res.Revision = 1
+	res.Username = username
+	if username == "" {
+		// Note that resourcetesting.NewResource() returns a resources
+		// with a username and timestamp set. So if the username was
+		// "un-set" then we have to also unset the timestamp.
+		res.Timestamp = time.Time{}
+	}
+
+	apiRes := params.Resource{
+		CharmResource: params.CharmResource{
+			Name:        name,
+			Description: name + " description",
+			Type:        "file",
+			Path:        res.Path,
+			Origin:      "upload",
+			Revision:    1,
+			Fingerprint: res.Fingerprint.Bytes(),
+			Size:        res.Size,
+		},
+		ID:            res.ID,
+		ApplicationID: res.ApplicationID,
+		Username:      username,
+		Timestamp:     res.Timestamp,
+	}
+
+	return res, apiRes
+}
diff --git a/api/client/resources/client/client.go → api/client/resources/client.go b/api/client/resources/client/client.go → api/client/resources/client.go
@@ -1,12 +1,10 @@
 // Copyright 2015 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package client
+package resources
 
 import (
-	"context"
 	"io"
-	"net/http"
 	"strings"
 
 	"github.com/juju/charm/v8"
@@ -15,42 +13,35 @@ import (
 	"github.com/juju/names/v4"
 	"gopkg.in/macaroon.v2"
 
-	api "github.com/juju/juju/api/client/resources"
+	"github.com/juju/juju/api"
+	"github.com/juju/juju/api/base"
 	apicharm "github.com/juju/juju/api/common/charm"
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/resource"
 	"github.com/juju/juju/rpc/params"
 )
 
-// TODO(ericsnow) Move FacadeCaller to a component-central package.
-
-// FacadeCaller has the api/base.FacadeCaller methods needed for the component.
-type FacadeCaller interface {
-	FacadeCall(request string, params, response interface{}) error
-	BestAPIVersion() int
-}
-
-// Doer
-type Doer interface {
-	Do(ctx context.Context, req *http.Request, resp interface{}) error
-}
-
 // Client is the public client for the resources API facade.
 type Client struct {
-	FacadeCaller
-	io.Closer
-	doer Doer
-	ctx  context.Context
+	base.ClientFacade
+	facade base.FacadeCaller
+
+	httpClient api.HTTPDoer
 }
 
 // NewClient returns a new Client for the given raw API caller.
-func NewClient(ctx context.Context, caller FacadeCaller, doer Doer, closer io.Closer) *Client {
-	return &Client{
-		FacadeCaller: caller,
-		Closer:       closer,
-		doer:         doer,
-		ctx:          ctx,
+func NewClient(apiCaller base.APICallCloser) (*Client, error) {
+	frontend, backend := base.NewClientFacade(apiCaller, "Resources")
+
+	httpClient, err := apiCaller.HTTPClient()
+	if err != nil {
+		return nil, errors.Trace(err)
 	}
+	return &Client{
+		ClientFacade: frontend,
+		facade:       backend,
+		httpClient:   httpClient,
+	}, nil
 }
 
 // ListResources calls the ListResources API server method with
@@ -62,22 +53,22 @@ func (c Client) ListResources(applications []string) ([]resource.ApplicationReso
 	}
 
 	var apiResults params.ResourcesResults
-	if err := c.FacadeCall("ListResources", &args, &apiResults); err != nil {
+	if err := c.facade.FacadeCall("ListResources", &args, &apiResults); err != nil {
 		return nil, errors.Trace(err)
 	}
 
 	if len(apiResults.Results) != len(applications) {
 		// We don't bother returning the results we *did* get since
 		// something bad happened on the server.
-		return nil, errors.Errorf("got invalid data from server (expected %d results, got %d)", len(applications), len(apiResults.Results))
+		return nil, errors.Errorf("expected %d results, got %d", len(applications), len(apiResults.Results))
 	}
 
 	var errs []error
 	results := make([]resource.ApplicationResources, len(applications))
 	for i := range applications {
 		apiResult := apiResults.Results[i]
 
-		result, err := api.APIResult2ApplicationResources(apiResult)
+		result, err := apiResult2ApplicationResources(apiResult)
 		if err != nil {
 			errs = append(errs, errors.Trace(err))
 		}
@@ -112,7 +103,7 @@ func newListResourcesArgs(applications []string) (params.ListResourcesArgs, erro
 
 // Upload sends the provided resource blob up to Juju.
 func (c Client) Upload(application, name, filename string, reader io.ReadSeeker) error {
-	uReq, err := api.NewUploadRequest(application, name, filename, reader)
+	uReq, err := NewUploadRequest(application, name, filename, reader)
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -122,7 +113,7 @@ func (c Client) Upload(application, name, filename string, reader io.ReadSeeker)
 	}
 
 	var response params.UploadResult // ignored
-	if err := c.doer.Do(c.ctx, req, &response); err != nil {
+	if err := c.httpClient.Do(c.facade.RawAPICaller().Context(), req, &response); err != nil {
 		return errors.Trace(err)
 	}
 
@@ -176,7 +167,7 @@ func (c Client) AddPendingResources(args AddPendingResourcesArgs) ([]string, err
 	}
 
 	var result params.AddPendingResourcesResult
-	if err := c.FacadeCall("AddPendingResources", &apiArgs, &result); err != nil {
+	if err := c.facade.FacadeCall("AddPendingResources", &apiArgs, &result); err != nil {
 		return nil, errors.Trace(err)
 	}
 	if result.Error != nil {
@@ -185,7 +176,7 @@ func (c Client) AddPendingResources(args AddPendingResourcesArgs) ([]string, err
 	}
 
 	if len(result.PendingIDs) != len(args.Resources) {
-		return nil, errors.Errorf("bad data from server: expected %d IDs, got %d", len(args.Resources), len(result.PendingIDs))
+		return nil, errors.Errorf("expected %d IDs, got %d", len(args.Resources), len(result.PendingIDs))
 	}
 	for i, id := range result.PendingIDs {
 		if id == "" {
@@ -206,7 +197,7 @@ func newAddPendingResourcesArgs(tag names.ApplicationTag, chID CharmID, csMac *m
 		if err := res.Validate(); err != nil {
 			return args, errors.Trace(err)
 		}
-		apiRes := api.CharmResource2API(res)
+		apiRes := CharmResource2API(res)
 		apiResources = append(apiResources, apiRes)
 	}
 	args.Tag = tag.String()
@@ -230,7 +221,7 @@ func newAddPendingResourcesArgsV2(tag names.ApplicationTag, chID CharmID, csMac
 		if err := res.Validate(); err != nil {
 			return args, errors.Trace(err)
 		}
-		apiRes := api.CharmResource2API(res)
+		apiRes := CharmResource2API(res)
 		apiResources = append(apiResources, apiRes)
 	}
 	args.Tag = tag.String()
@@ -270,7 +261,7 @@ func (c Client) UploadPendingResource(application string, res charmresource.Reso
 	pendingID = ids[0]
 
 	if reader != nil {
-		uReq, err := api.NewUploadRequest(application, res.Name, filename, reader)
+		uReq, err := NewUploadRequest(application, res.Name, filename, reader)
 		if err != nil {
 			return "", errors.Trace(err)
 		}
@@ -281,7 +272,7 @@ func (c Client) UploadPendingResource(application string, res charmresource.Reso
 		}
 
 		var response params.UploadResult // ignored
-		if err := c.doer.Do(c.ctx, req, &response); err != nil {
+		if err := c.httpClient.Do(c.facade.RawAPICaller().Context(), req, &response); err != nil {
 			return "", errors.Trace(err)
 		}
 	}