Skip to content

Commit 62ac044

Browse files
Drop the charmstore.Client interface and export the Client struct.
1 parent a80b0a4 commit 62ac044

File tree

9 files changed

+59
-75
lines changed

9 files changed

+59
-75
lines changed

apiserver/charmrevisionupdater/testing/suite.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (s *CharmSuite) SetUpTest(c *gc.C) {
6767
s.jcSuite.PatchValue(&charmrepo.CacheDir, c.MkDir())
6868
// Patch the charm repo initializer function: it is replaced with a charm
6969
// store repo pointing to the testing server.
70-
s.jcSuite.PatchValue(&charmrevisionupdater.NewCharmStoreClient, func() jujucharmstore.Client {
70+
s.jcSuite.PatchValue(&charmrevisionupdater.NewCharmStoreClient, func() *jujucharmstore.Client {
7171
return jujucharmstore.NewClient(csclient.Params{
7272
URL: s.Server.URL,
7373
})

apiserver/charmrevisionupdater/updater.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func (api *CharmRevisionUpdaterAPI) updateLatestRevisions() error {
9191

9292
// NewCharmStoreClient instantiates a new charm store repository.
9393
// It is defined at top level for testing purposes.
94-
var NewCharmStoreClient = func(modelUUID string) (charmstore.Client, error) {
94+
var NewCharmStoreClient = func(modelUUID string) (*charmstore.Client, error) {
9595
// TODO(ericsnow) Use the Juju "HTTP context" once we have one.
9696
client := charmstore.NewDefaultClient()
9797
return client.WithMetadata(charmstore.JujuMetadata{

apiserver/charmrevisionupdater/updater_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (s *charmVersionSuite) TestEnvironmentUUIDUsed(c *gc.C) {
166166
defer srv.Close()
167167

168168
// Point the charm repo initializer to the testing server.
169-
s.PatchValue(&charmrevisionupdater.NewCharmStoreClient, func() charmstore.Client {
169+
s.PatchValue(&charmrevisionupdater.NewCharmStoreClient, func() *charmstore.Client {
170170
return charmstore.NewClient(csclient.Params{
171171
URL: srv.URL,
172172
})

charmstore/client.go

Lines changed: 20 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,6 @@ import (
1818

1919
var logger = loggo.GetLogger("juju.charmstore")
2020

21-
// Client provides the common interface for the charm store client needs
22-
// of Juju.
23-
type Client interface {
24-
BaseClient
25-
io.Closer
26-
27-
// WithMetadata returns a copy of the the client that will use the
28-
// provided metadata during client requests.
29-
WithMetadata(meta JujuMetadata) (Client, error)
30-
31-
// Metadata returns the Juju metadata set on the client.
32-
Metadata() JujuMetadata
33-
34-
// TODO(ericsnow) Add an "AsRepo() charmrepo.Interface" method.
35-
}
36-
3721
// BaseClient exposes the functionality of the charm store, as provided
3822
// by github.com/juju/charmrepo/csclient.Client.
3923
//
@@ -112,45 +96,41 @@ func (base baseClient) LatestRevisions(cURLs []*charm.URL) ([]charmrepo.CharmRev
11296
// TODO(ericsnow) Factor out a metadataClient type that embeds "client",
11397
// and move the "meta" field there?
11498

115-
// client adapts csclient.Client to the needs of Juju.
116-
type client struct {
99+
// Client adapts csclient.Client to the needs of Juju.
100+
type Client struct {
117101
BaseClient
118102
io.Closer
119103

120-
newCopy func() *client
104+
newCopy func() *Client
121105
meta JujuMetadata
122106
}
123107

124108
// NewClient returns a Juju charm store client for the given client
125109
// config.
126-
func NewClient(config csclient.Params) Client {
110+
func NewClient(config csclient.Params) *Client {
127111
base := csclient.New(config)
128112
closer := ioutil.NopCloser(nil)
129-
return newClient(base, closer)
113+
return WrapBaseClient(base, closer)
130114
}
131115

132116
// NewDefaultClient returns a Juju charm store client using a default
133117
// client config.
134-
func NewDefaultClient() Client {
118+
func NewDefaultClient() *Client {
135119
return NewClient(csclient.Params{})
136120
}
137121

138122
// WrapBaseClient returns a Juju charm store client that wraps
139123
// the provided client. The given closer is used to close resources
140124
// related to the client. If no closer is needed then pass in a no-op
141125
// closer (e.g. ioutil.NopCloser).
142-
func WrapBaseClient(base *csclient.Client, closer io.Closer) Client {
143-
return newClient(base, closer)
144-
}
145-
146-
func newClient(base *csclient.Client, closer io.Closer) *client {
147-
c := &client{
126+
func WrapBaseClient(base *csclient.Client, closer io.Closer) *Client {
127+
c := &Client{
148128
BaseClient: newBaseClient(base),
149129
Closer: closer,
150130
}
151-
c.newCopy = func() *client {
152-
newBase := *base
153-
copied := newClient(&newBase, closer)
131+
c.newCopy = func() *Client {
132+
newBase := *base // a copy
133+
copied := WrapBaseClient(&newBase, closer)
154134
copied.meta = c.meta
155135
return copied
156136
}
@@ -159,7 +139,7 @@ func newClient(base *csclient.Client, closer io.Closer) *client {
159139

160140
// WithMetadata returns a copy of the the client that will use the
161141
// provided metadata during client requests.
162-
func (c client) WithMetadata(meta JujuMetadata) (Client, error) {
142+
func (c Client) WithMetadata(meta JujuMetadata) (*Client, error) {
163143
newClient := c.newCopy()
164144
newClient.meta = meta
165145
// Note that we don't call meta.setOnClient() at this point.
@@ -172,8 +152,8 @@ func (c client) WithMetadata(meta JujuMetadata) (Client, error) {
172152
return newClient, nil
173153
}
174154

175-
// Metadata implements Client.
176-
func (c client) Metadata() JujuMetadata {
155+
// Metadata returns the Juju metadata set on the client.
156+
func (c Client) Metadata() JujuMetadata {
177157
// Note the value receiver, meaning that the returned metadata
178158
// is a copy.
179159
return c.meta
@@ -183,7 +163,7 @@ func (c client) Metadata() JujuMetadata {
183163
// identified charms. The revisions in the provided URLs are ignored.
184164
// Note that this differs from BaseClient.LatestRevisions() exclusively
185165
// due to taking into account Juju metadata (if any).
186-
func (c *client) LatestRevisions(cURLs []*charm.URL) ([]charmrepo.CharmRevision, error) {
166+
func (c *Client) LatestRevisions(cURLs []*charm.URL) ([]charmrepo.CharmRevision, error) {
187167
if !c.meta.IsZero() {
188168
c = c.newCopy()
189169
if err := c.meta.setOnClient(c); err != nil {
@@ -193,10 +173,12 @@ func (c *client) LatestRevisions(cURLs []*charm.URL) ([]charmrepo.CharmRevision,
193173
return c.BaseClient.LatestRevisions(cURLs)
194174
}
195175

176+
// TODO(ericsnow) Add an "AsRepo() charmrepo.Interface" method.
177+
196178
// LatestCharmInfo returns the most up-to-date information about each
197179
// of the identified charms at their latest revision. The revisions in
198180
// the provided URLs are ignored.
199-
func LatestCharmInfo(client Client, cURLs []*charm.URL) ([]CharmInfoResult, error) {
181+
func LatestCharmInfo(client BaseClient, cURLs []*charm.URL) ([]CharmInfoResult, error) {
200182
now := time.Now().UTC()
201183

202184
// Do a bulk call to get the revision info for all charms.
@@ -239,13 +221,13 @@ func LatestCharmInfo(client Client, cURLs []*charm.URL) ([]CharmInfoResult, erro
239221

240222
type fakeCharmStoreClient struct{}
241223

242-
// ListResources implements Client as a noop.
224+
// ListResources implements BaseClient as a noop.
243225
func (fakeCharmStoreClient) ListResources(charmURLs []*charm.URL) ([][]charmresource.Resource, error) {
244226
res := make([][]charmresource.Resource, len(charmURLs))
245227
return res, nil
246228
}
247229

248-
// GetResource implements Client as a noop.
230+
// GetResource implements BaseClient as a noop.
249231
func (fakeCharmStoreClient) GetResource(cURL *charm.URL, resourceName string, revision int) (io.ReadCloser, error) {
250232
return nil, errors.NotFoundf("resource %q", resourceName)
251233
}

charmstore/client_test.go

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -67,29 +67,29 @@ func (s *ClientSuite) TestWithMetadata(c *gc.C) {
6767
c.Check(newClient.Metadata(), jc.DeepEquals, meta)
6868
}
6969

70-
//func (s *ClientSuite) TestLatestRevisions(c *gc.C) {
71-
// cURLs := []*charm.URL{
72-
// charm.MustParseURL("cs:quantal/spam-17"),
73-
// charm.MustParseURL("cs:quantal/eggs-2"),
74-
// charm.MustParseURL("cs:quantal/ham-1"),
75-
// }
76-
// expected := []charmrepo.CharmRevision{{
77-
// Revision: 17,
78-
// }, {
79-
// Revision: 3,
80-
// }, {
81-
// Err: errors.New("not found"),
82-
// }}
83-
// s.client.ReturnLatestRevisions = expected
84-
// client := charmstore.Client{BaseClient: s.client}
85-
//
86-
// revisions, err := client.LatestRevisions(cURLs)
87-
// c.Assert(err, jc.ErrorIsNil)
88-
//
89-
// s.stub.CheckCallNames(c, "LatestRevisions")
90-
// s.stub.CheckCall(c, 0, "LatestRevisions", cURLs)
91-
// c.Check(revisions, jc.DeepEquals, expected)
92-
//}
70+
func (s *ClientSuite) TestLatestRevisions(c *gc.C) {
71+
cURLs := []*charm.URL{
72+
charm.MustParseURL("cs:quantal/spam-17"),
73+
charm.MustParseURL("cs:quantal/eggs-2"),
74+
charm.MustParseURL("cs:quantal/ham-1"),
75+
}
76+
expected := []charmrepo.CharmRevision{{
77+
Revision: 17,
78+
}, {
79+
Revision: 3,
80+
}, {
81+
Err: errors.New("not found"),
82+
}}
83+
s.client.ReturnLatestRevisions = expected
84+
client := charmstore.Client{BaseClient: s.client}
85+
86+
revisions, err := client.LatestRevisions(cURLs)
87+
c.Assert(err, jc.ErrorIsNil)
88+
89+
s.stub.CheckCallNames(c, "LatestRevisions")
90+
s.stub.CheckCall(c, 0, "LatestRevisions", cURLs)
91+
c.Check(revisions, jc.DeepEquals, expected)
92+
}
9393

9494
func (s *ClientSuite) TestLatestCharmInfo(c *gc.C) {
9595
cURLs := []*charm.URL{
@@ -172,7 +172,7 @@ func (s *ClientSuite) TestFakeGetResource(c *gc.C) {
172172
// TODO(ericsnow) Move the stub client and repo to a testing package.
173173

174174
type stubClient struct {
175-
charmstore.Client
175+
charmstore.BaseClient
176176
*testing.Stub
177177

178178
ReturnListResources [][]charmresource.Resource

charmstore/metadata.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type httpHeaderSetter interface {
4343

4444
// setOnClient sets custom HTTP headers that will be sent to the charm
4545
// store on each request.
46-
func (meta JujuMetadata) setOnClient(client Client) error {
46+
func (meta JujuMetadata) setOnClient(client BaseClient) error {
4747
setter, ok := client.(httpHeaderSetter)
4848
if !ok {
4949
return errors.NewNotValid(nil, "charm store client (missing SetHTTPHeader method)")

cmd/juju/charmcmd/store.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727
// store client.
2828
type CharmstoreSpec interface {
2929
// Connect connects to the specified charm store.
30-
Connect() (charmstore.Client, error)
30+
Connect() (*charmstore.Client, error)
3131
}
3232

3333
type charmstoreSpec struct {
@@ -47,7 +47,7 @@ func newCharmstoreSpec() CharmstoreSpec {
4747
}
4848

4949
// Connect implements CharmstoreSpec.
50-
func (cs charmstoreSpec) Connect() (charmstore.Client, error) {
50+
func (cs charmstoreSpec) Connect() (*charmstore.Client, error) {
5151
params, apiContext, err := cs.connect()
5252
if err != nil {
5353
return nil, errors.Trace(err)

cmd/juju/charmcmd/sub.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type CommandBase struct {
3434
}
3535

3636
// Connect implements CommandBase.
37-
func (c *CommandBase) Connect() (charmstore.Client, error) {
37+
func (c *CommandBase) Connect() (*charmstore.Client, error) {
3838
if c.spec == nil {
3939
return nil, errors.Errorf("missing charm store spec")
4040
}

resource/resourceadapters/charmstore.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func newCharmstoreOpener(cURL *charm.URL) *charmstoreOpener {
5757
}
5858

5959
// NewClient opens a new charm store client.
60-
func (cs *charmstoreOpener) NewClient() (charmstore.Client, error) {
60+
func (cs *charmstoreOpener) NewClient() (*CSRetryClient, error) {
6161
// TODO(ericsnow) Use a valid charm store client.
6262
base := csclient.New(csclient.Params{URL: "<not valid>"})
6363
// TODO(ericsnow) closer will be meaningful once we factor out the
@@ -67,12 +67,14 @@ func (cs *charmstoreOpener) NewClient() (charmstore.Client, error) {
6767
return newCSRetryClient(client), nil
6868
}
6969

70-
type csRetryClient struct {
71-
charmstore.Client
70+
// CSRetryClient is a wrapper around a Juju charm store client that
71+
// retries GetResource() calls.
72+
type CSRetryClient struct {
73+
*charmstore.Client
7274
retryArgs retry.CallArgs
7375
}
7476

75-
func newCSRetryClient(client charmstore.Client) *csRetryClient {
77+
func newCSRetryClient(client *charmstore.Client) *CSRetryClient {
7678
retryArgs := retry.CallArgs{
7779
// The only error that stops the retry loop should be "not found".
7880
IsFatalError: errors.IsNotFound,
@@ -86,14 +88,14 @@ func newCSRetryClient(client charmstore.Client) *csRetryClient {
8688
Delay: 1 * time.Minute,
8789
Clock: clock.WallClock,
8890
}
89-
return &csRetryClient{
91+
return &CSRetryClient{
9092
Client: client,
9193
retryArgs: retryArgs,
9294
}
9395
}
9496

9597
// GetResource returns a reader for the resource's data.
96-
func (client csRetryClient) GetResource(cURL *charm.URL, resourceName string, revision int) (io.ReadCloser, error) {
98+
func (client CSRetryClient) GetResource(cURL *charm.URL, resourceName string, revision int) (io.ReadCloser, error) {
9799
args := client.retryArgs // a copy
98100

99101
var reader io.ReadCloser

0 commit comments

Comments
 (0)