Create the charmhub refresher

cmd/juju/application/deployer/bundlehandler.go

@@ -543,7 +543,7 @@ func (h *bundleHandler) addCharm(change *bundlechanges.AddCharmChange) error {
 
 	var macaroon *macaroon.Macaroon
 	var charmOrigin commoncharm.Origin
-	url, macaroon, charmOrigin, err = store.AddCharmFromURL(h.deployAPI, h.authorizer, url, origin, h.force, series)
+	url, macaroon, charmOrigin, err = store.AddCharmWithAuthorizationFromURL(h.deployAPI, h.authorizer, url, origin, h.force, series)
 	if err != nil {
 		return errors.Annotatef(err, "cannot add charm %q", chParms.Charm)
 	}

cmd/juju/application/deployer/charm.go

@@ -440,7 +440,7 @@ func (c *charmStoreCharm) PrepareAndDeploy(ctx *cmd.Context, deployAPI DeployerA
 	}
 
 	// Store the charm in the controller
-	curl, csMac, csOrigin, err := store.AddCharmFromURL(deployAPI, macaroonGetter, storeCharmOrBundleURL, c.origin, c.force, series)
+	curl, csMac, csOrigin, err := store.AddCharmWithAuthorizationFromURL(deployAPI, macaroonGetter, storeCharmOrBundleURL, c.origin, c.force, series)
 	if err != nil {
 		if termErr, ok := errors.Cause(err).(*common.TermsRequiredError); ok {
 			return errors.Trace(termErr.UserErr())

cmd/juju/application/refresher/refresher.go

@@ -13,6 +13,7 @@ import (
 	"github.com/juju/errors"
 	"github.com/juju/featureflag"
 
+	commoncharm "github.com/juju/juju/api/common/charm"
 	"github.com/juju/juju/cmd/juju/application/store"
 	"github.com/juju/juju/cmd/juju/application/utils"
 	corecharm "github.com/juju/juju/core/charm"
@@ -42,6 +43,8 @@ type RefresherConfig struct {
 	ForceSeries     bool
 }
 
+// RefresherFn defines a function alias to create a Refresher from a given
+// function.
 type RefresherFn = func(RefresherConfig) (Refresher, error)
 
 type factory struct {
@@ -58,6 +61,7 @@ func NewRefresherFactory(deps RefresherDependencies) RefresherFactory {
 	d.refreshers = []RefresherFn{
 		d.maybeReadLocal(deps.CharmAdder, defaultCharmRepo{}),
 		d.maybeCharmStore(deps.Authorizer, deps.CharmAdder, deps.CharmResolver),
+		d.maybeCharmHub(deps.CharmAdder, deps.CharmResolver),
 	}
 	return d
 }
@@ -116,15 +120,34 @@ func (d *factory) maybeReadLocal(charmAdder store.CharmAdder, charmRepo CharmRep
 func (d *factory) maybeCharmStore(authorizer store.MacaroonGetter, charmAdder store.CharmAdder, charmResolver CharmResolver) func(RefresherConfig) (Refresher, error) {
 	return func(cfg RefresherConfig) (Refresher, error) {
 		return &charmStoreRefresher{
-			authorizer:     authorizer,
-			charmAdder:     charmAdder,
-			charmResolver:  charmResolver,
-			charmURL:       cfg.CharmURL,
-			charmRef:       cfg.CharmRef,
-			channel:        cfg.Channel,
-			deployedSeries: cfg.DeployedSeries,
-			force:          cfg.Force,
-			forceSeries:    cfg.ForceSeries,
+			baseRefresher: baseRefresher{
+				charmAdder:     charmAdder,
+				charmResolver:  charmResolver,
+				charmURL:       cfg.CharmURL,
+				charmRef:       cfg.CharmRef,
+				channel:        cfg.Channel,
+				deployedSeries: cfg.DeployedSeries,
+				force:          cfg.Force,
+				forceSeries:    cfg.ForceSeries,
+			},
+			authorizer: authorizer,
+		}, nil
+	}
+}
+
+func (d *factory) maybeCharmHub(charmAdder store.CharmAdder, charmResolver CharmResolver) func(RefresherConfig) (Refresher, error) {
+	return func(cfg RefresherConfig) (Refresher, error) {
+		return &charmHubRefresher{
+			baseRefresher: baseRefresher{
+				charmAdder:     charmAdder,
+				charmResolver:  charmResolver,
+				charmURL:       cfg.CharmURL,
+				charmRef:       cfg.CharmRef,
+				channel:        cfg.Channel,
+				deployedSeries: cfg.DeployedSeries,
+				force:          cfg.Force,
+				forceSeries:    cfg.ForceSeries,
+			},
 		}, nil
 	}
 }
@@ -181,8 +204,7 @@ func (d *localCharmRefresher) String() string {
 	return fmt.Sprintf("attempting to refresh local charm %q", d.charmRef)
 }
 
-type charmStoreRefresher struct {
-	authorizer     store.MacaroonGetter
+type baseRefresher struct {
 	charmAdder     store.CharmAdder
 	charmResolver  CharmResolver
 	charmURL       *charm.URL
@@ -193,46 +215,21 @@ type charmStoreRefresher struct {
 	forceSeries    bool
 }
 
-// Allowed will attempt to check if the charm store is allowed to refresh.
-// Depending on the charm url, will then determine if that's true or not.
-func (r *charmStoreRefresher) Allowed(cfg RefresherConfig) (bool, error) {
-	// If we're a charm hub charm reference, then skip the charm store and
-	// move onto the next
-	if featureflag.Enabled(feature.CharmHubIntegration) {
-		path, err := charm.EnsureSchema(cfg.CharmRef)
-		if err != nil {
-			return false, errors.Trace(err)
-		}
-
-		curl, err := charm.ParseURL(path)
-		if err != nil {
-			return false, errors.Trace(err)
-		}
-
-		if charm.CharmHub.Matches(curl.Schema) {
-			return false, nil
-		}
-	}
-	return true, nil
-}
-
-// Refresh a given charm store charm.
-// Bundles are not supported as there is no physical representation in Juju.
-func (r *charmStoreRefresher) Refresh() (*CharmID, error) {
+func (r baseRefresher) ResolveCharm() (*charm.URL, commoncharm.Origin, error) {
 	refURL, err := charm.ParseURL(r.charmRef)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, commoncharm.Origin{}, errors.Trace(err)
 	}
 
 	origin, err := utils.DeduceOrigin(refURL, r.channel)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, commoncharm.Origin{}, errors.Trace(err)
 	}
 
 	// Charm has been supplied as a URL so we resolve and deploy using the store.
 	newURL, origin, supportedSeries, err := r.charmResolver.ResolveCharm(refURL, origin)
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, commoncharm.Origin{}, errors.Trace(err)
 	}
 
 	_, seriesSupportedErr := charm.SeriesForCharm(r.deployedSeries, supportedSeries)
@@ -241,7 +238,7 @@ func (r *charmStoreRefresher) Refresh() (*CharmID, error) {
 		if len(supportedSeries) > 0 {
 			series = supportedSeries
 		}
-		return nil, errors.Errorf(
+		return nil, commoncharm.Origin{}, errors.Errorf(
 			"cannot upgrade from single series %q charm to a charm supporting %q. Use --force-series to override.",
 			r.deployedSeries, series,
 		)
@@ -251,15 +248,54 @@ func (r *charmStoreRefresher) Refresh() (*CharmID, error) {
 	// or Revision flags, discover the latest.
 	if *newURL == *r.charmURL {
 		if refURL.Revision != -1 {
-			return nil, errors.Errorf("already running specified charm %q", newURL)
+			return nil, commoncharm.Origin{}, errors.Errorf("already running specified charm %q", newURL)
 		}
 		// No point in trying to upgrade a charm store charm when
 		// we just determined that's the latest revision
 		// available.
-		return nil, errors.Errorf("already running latest charm %q", newURL)
+		return nil, commoncharm.Origin{}, errors.Errorf("already running latest charm %q", newURL)
 	}
 
-	curl, csMac, _, err := store.AddCharmFromURL(r.charmAdder, r.authorizer, newURL, origin, r.force, r.deployedSeries)
+	return newURL, origin, nil
+}
+
+type charmStoreRefresher struct {
+	baseRefresher
+	authorizer store.MacaroonGetter
+}
+
+// Allowed will attempt to check if the charm store is allowed to refresh.
+// Depending on the charm url, will then determine if that's true or not.
+func (r *charmStoreRefresher) Allowed(cfg RefresherConfig) (bool, error) {
+	// If we're a charm hub charm reference, then skip the charm store and
+	// move onto the next
+	if featureflag.Enabled(feature.CharmHubIntegration) {
+		path, err := charm.EnsureSchema(cfg.CharmRef)
+		if err != nil {
+			return false, errors.Trace(err)
+		}
+
+		curl, err := charm.ParseURL(path)
+		if err != nil {
+			return false, errors.Trace(err)
+		}
+
+		if charm.CharmHub.Matches(curl.Schema) {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
+// Refresh a given charm store charm.
+// Bundles are not supported as there is no physical representation in Juju.
+func (r *charmStoreRefresher) Refresh() (*CharmID, error) {
+	newURL, origin, err := r.ResolveCharm()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	curl, csMac, _, err := store.AddCharmWithAuthorizationFromURL(r.charmAdder, r.authorizer, newURL, origin, r.force, r.deployedSeries)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -280,3 +316,51 @@ type defaultCharmRepo struct{}
 func (defaultCharmRepo) NewCharmAtPathForceSeries(path, series string, force bool) (charm.Charm, *charm.URL, error) {
 	return charmrepo.NewCharmAtPathForceSeries(path, series, force)
 }
+
+type charmHubRefresher struct {
+	baseRefresher
+}
+
+// Allowed will attempt to check if the charm store is allowed to refresh.
+// Depending on the charm url, will then determine if that's true or not.
+func (r *charmHubRefresher) Allowed(cfg RefresherConfig) (bool, error) {
+	if featureflag.Enabled(feature.CharmHubIntegration) {
+		path, err := charm.EnsureSchema(cfg.CharmRef)
+		if err != nil {
+			return false, errors.Trace(err)
+		}
+
+		curl, err := charm.ParseURL(path)
+		if err != nil {
+			return false, errors.Trace(err)
+		}
+
+		if charm.CharmHub.Matches(curl.Schema) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// Refresh a given charm hub charm.
+// Bundles are not supported as there is no physical representation in Juju.
+func (r *charmHubRefresher) Refresh() (*CharmID, error) {
+	newURL, origin, err := r.ResolveCharm()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	curl, _, err := store.AddCharmFromURL(r.charmAdder, newURL, origin, r.force, r.deployedSeries)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	return &CharmID{
+		URL:    curl,
+		Origin: origin.CoreCharmOrigin(),
+	}, nil
+}
+
+func (r *charmHubRefresher) String() string {
+	return fmt.Sprintf("attempting to refresh charm hub charm %q", r.charmRef)
+}

cmd/juju/application/refresher/refresher_test.go

@@ -330,3 +330,98 @@ func (s *charmStoreCharmRefresherSuite) TestRefreshWithARevision(c *gc.C) {
 	_, err = task.Refresh()
 	c.Assert(err, gc.ErrorMatches, `already running specified charm "cs:meshuggah-1"`)
 }
+
+type charmHubCharmRefresherSuite struct{}
+
+var _ = gc.Suite(&charmHubCharmRefresherSuite{})
+
+func (s *charmHubCharmRefresherSuite) TestRefresh(c *gc.C) {
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	ref := "ch:meshuggah"
+	curl := charm.MustParseURL(ref)
+	newCurl := charm.MustParseURL(fmt.Sprintf("%s-1", ref))
+	origin := commoncharm.Origin{
+		Source: commoncharm.OriginCharmHub,
+	}
+
+	charmAdder := NewMockCharmAdder(ctrl)
+	charmAdder.EXPECT().AddCharm(newCurl, origin, false, "bionic").Return(origin, nil)
+
+	charmResolver := NewMockCharmResolver(ctrl)
+	charmResolver.EXPECT().ResolveCharm(curl, origin).Return(newCurl, origin, []string{}, nil)
+
+	cfg := RefresherConfig{
+		CharmURL:       curl,
+		CharmRef:       ref,
+		DeployedSeries: "bionic",
+	}
+
+	refresher := (&factory{}).maybeCharmHub(charmAdder, charmResolver)
+	task, err := refresher(cfg)
+	c.Assert(err, jc.ErrorIsNil)
+
+	charmID, err := task.Refresh()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(charmID, gc.DeepEquals, &CharmID{
+		URL:    newCurl,
+		Origin: origin.CoreCharmOrigin(),
+	})
+}
+
+func (s *charmHubCharmRefresherSuite) TestRefreshWithNoUpdates(c *gc.C) {
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	ref := "ch:meshuggah"
+	curl := charm.MustParseURL(ref)
+	origin := commoncharm.Origin{
+		Source: commoncharm.OriginCharmHub,
+	}
+
+	charmAdder := NewMockCharmAdder(ctrl)
+
+	charmResolver := NewMockCharmResolver(ctrl)
+	charmResolver.EXPECT().ResolveCharm(curl, origin).Return(curl, origin, []string{}, nil)
+
+	cfg := RefresherConfig{
+		CharmURL: curl,
+		CharmRef: ref,
+	}
+
+	refresher := (&factory{}).maybeCharmHub(charmAdder, charmResolver)
+	task, err := refresher(cfg)
+	c.Assert(err, jc.ErrorIsNil)
+
+	_, err = task.Refresh()
+	c.Assert(err, gc.ErrorMatches, `already running latest charm "meshuggah"`)
+}
+
+func (s *charmHubCharmRefresherSuite) TestRefreshWithARevision(c *gc.C) {
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	ref := "ch:meshuggah-1"
+	curl := charm.MustParseURL(ref)
+	origin := commoncharm.Origin{
+		Source: commoncharm.OriginCharmHub,
+	}
+
+	charmAdder := NewMockCharmAdder(ctrl)
+
+	charmResolver := NewMockCharmResolver(ctrl)
+	charmResolver.EXPECT().ResolveCharm(curl, origin).Return(curl, origin, []string{}, nil)
+
+	cfg := RefresherConfig{
+		CharmURL: curl,
+		CharmRef: ref,
+	}
+
+	refresher := (&factory{}).maybeCharmHub(charmAdder, charmResolver)
+	task, err := refresher(cfg)
+	c.Assert(err, jc.ErrorIsNil)
+
+	_, err = task.Refresh()
+	c.Assert(err, gc.ErrorMatches, `already running specified charm "meshuggah-1"`)
+}

cmd/juju/application/store/store.go

@@ -19,10 +19,23 @@ import (
 )
 
 // AddCharmFromURL calls the appropriate client API calls to add the
-// given charm URL to state. For non-public charm URLs, this function also
-// handles the macaroon authorization process using the given csClient.
+// given charm URL to state.
+func AddCharmFromURL(client CharmAdder, curl *charm.URL, origin commoncharm.Origin, force bool, series string) (*charm.URL, commoncharm.Origin, error) {
+	resultOrigin, err := client.AddCharm(curl, origin, force, series)
+	if err != nil {
+		if params.IsCodeUnauthorized(err) {
+			return nil, commoncharm.Origin{}, errors.Forbiddenf(err.Error())
+		}
+		return nil, commoncharm.Origin{}, errors.Trace(err)
+	}
+	return curl, resultOrigin, nil
+}
+
+// AddCharmWithAuthorizationFromURL calls the appropriate client API calls to
+// add the given charm URL to state. For non-public charm URLs, this function
+// also handles the macaroon authorization process using the given csClient.
 // The resulting charm URL of the added charm is displayed on stdout.
-func AddCharmFromURL(client CharmAdder, cs MacaroonGetter, curl *charm.URL, origin commoncharm.Origin, force bool, series string) (*charm.URL, *macaroon.Macaroon, commoncharm.Origin, error) {
+func AddCharmWithAuthorizationFromURL(client CharmAdder, cs MacaroonGetter, curl *charm.URL, origin commoncharm.Origin, force bool, series string) (*charm.URL, *macaroon.Macaroon, commoncharm.Origin, error) {
 	var csMac *macaroon.Macaroon
 	resultOrigin, err := client.AddCharm(curl, origin, force, series)
 	if err != nil {