feat: amend ResourceStoreFactory to take a ResourceStoreGetter

The ResourceStoreFactory now uses a ResourceStoreGetter for
TypeContainerImage. The ContainerImageResourceStore was pushed
up to an individual domain. Supporting functionality for the old
implemenation has been removed.

ContainerImageResourceStore is now provided to NewResourceStoreFactory
via a getter when a Resource domain is requested as a model service.

core/resource/store/store.go

@@ -52,3 +52,6 @@ func NewFingerprint(f hash.Fingerprint) Fingerprint {
 // blob. This can be an object store metadata UUID or a container image metadata
 // storage key.
 type UUID string
+
+// ResourceStoreGetter is a function which returns a ResourceStore.
+type ResourceStoreGetter func() ResourceStore

domain/containerimageresourcestore/service/service.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/juju/juju/core/resource/store"
 	"github.com/juju/juju/domain/containerimageresourcestore"
-	charmresource "github.com/juju/juju/internal/charm/resource"
 	"github.com/juju/juju/internal/docker"
 	"github.com/juju/juju/internal/errors"
 )
@@ -83,8 +82,8 @@ func (s Service) Put(
 	ctx context.Context,
 	storageKey string,
 	r io.Reader,
-	size int64,
-	fingerprint charmresource.Fingerprint,
+	_ int64,
+	_ store.Fingerprint,
 ) (store.UUID, error) {
 	respBuf := new(bytes.Buffer)
 	bytesRead, err := respBuf.ReadFrom(r)

domain/resource/service/resource.go

@@ -50,9 +50,6 @@ type State interface {
 }
 
 type ResourceStoreGetter interface {
-	// AddStore injects a ResourceStore for the given type into the ResourceStoreFactory.
-	AddStore(t charmresource.Type, store store.ResourceStore)
-
 	// GetResourceStore returns the appropriate ResourceStore for the
 	// given resource type.
 	GetResourceStore(context.Context, charmresource.Type) (store.ResourceStore, error)
@@ -72,15 +69,6 @@ func NewService(
 	resourceStoreGetter ResourceStoreGetter,
 	logger logger.Logger,
 ) *Service {
-	// Note:
-	// The store for container image resources is really a DQLite table today.
-	// Using AddStore is a compromise to avoid injecting one service into
-	// another, as would happen if NewResourceStoreFactory had a second
-	// argument to provide a containerImageResourceStore.
-	resourceStoreGetter.AddStore(
-		charmresource.TypeContainerImage,
-		nil,
-	)
 	return &Service{
 		st:     st,
 		logger: logger,

domain/resource/service/resource_test.go

@@ -293,7 +293,6 @@ func (s *resourceServiceSuite) setupMocks(c *gc.C) *gomock.Controller {
 
 	s.state = NewMockState(ctrl)
 	s.resourceStoreGetter = NewMockResourceStoreGetter(ctrl)
-	s.resourceStoreGetter.EXPECT().AddStore(charmresource.TypeContainerImage, gomock.Any())
 
 	s.service = NewService(s.state, s.resourceStoreGetter, loggertesting.WrapCheckLog(c))
 

domain/services/model.go

@@ -15,6 +15,7 @@ import (
 	"github.com/juju/juju/core/model"
 	"github.com/juju/juju/core/objectstore"
 	"github.com/juju/juju/core/providertracker"
+	coreresourcestore "github.com/juju/juju/core/resource/store"
 	corestorage "github.com/juju/juju/core/storage"
 	agentprovisionerservice "github.com/juju/juju/domain/agentprovisioner/service"
 	agentprovisionerstate "github.com/juju/juju/domain/agentprovisioner/state"
@@ -29,6 +30,8 @@ import (
 	blockdevicestate "github.com/juju/juju/domain/blockdevice/state"
 	cloudimagemetadataservice "github.com/juju/juju/domain/cloudimagemetadata/service"
 	cloudimagemetadatastate "github.com/juju/juju/domain/cloudimagemetadata/state"
+	containerimageresourcestoreservice "github.com/juju/juju/domain/containerimageresourcestore/service"
+	containerimageresourcestorestate "github.com/juju/juju/domain/containerimageresourcestore/state"
 	keymanagerservice "github.com/juju/juju/domain/keymanager/service"
 	keymanagerstate "github.com/juju/juju/domain/keymanager/state"
 	keyupdaterservice "github.com/juju/juju/domain/keyupdater/service"
@@ -345,13 +348,24 @@ func (s *ModelServices) BlockCommand() *blockcommandservice.Service {
 // Resource returns the service for persisting and retrieving application
 // resources for the current model.
 func (s *ModelServices) Resource() *resourceservice.Service {
+	containerImageResourceStoreGetter := func() coreresourcestore.ResourceStore {
+		return containerimageresourcestoreservice.NewService(
+			containerimageresourcestorestate.NewState(
+				changestream.NewTxnRunnerFactory(s.modelDB),
+				s.logger.Child("containerimageresourcestore.state"),
+			))
+	}
+	resourceStoreFactory := store.NewResourceStoreFactory(
+		s.objectstore,
+		containerImageResourceStoreGetter,
+	)
 	return resourceservice.NewService(
 		resourcestate.NewState(
 			changestream.NewTxnRunnerFactory(s.modelDB),
 			s.clock,
 			s.logger.Child("resource.state"),
 		),
-		store.NewResourceStoreFactory(s.objectstore),
+		resourceStoreFactory,
 		s.logger.Child("resource.service"),
 	)
 }

internal/resource/store/package_test.go

@@ -9,7 +9,8 @@ import (
 	gc "gopkg.in/check.v1"
 )
 
-//go:generate go run go.uber.org/mock/mockgen -typed -package resource -destination object_store_mock_test.go github.com/juju/juju/core/objectstore ObjectStore,ModelObjectStoreGetter
+//go:generate go run go.uber.org/mock/mockgen -typed -package store -destination object_store_mock_test.go github.com/juju/juju/core/objectstore ObjectStore,ModelObjectStoreGetter
+//go:generate go run go.uber.org/mock/mockgen -typed -package store -destination resource_store_mock_test.go github.com/juju/juju/core/resource/store ResourceStore
 
 func TestPackage(t *testing.T) {
 	gc.TestingT(t)

internal/resource/store/resourcestore.go

@@ -5,7 +5,6 @@ package store
 
 import (
 	"context"
-	"sync"
 
 	"github.com/juju/juju/core/objectstore"
 	"github.com/juju/juju/core/resource/store"
@@ -16,17 +15,19 @@ import (
 // ResourceStoreFactory contains the information to provide the required
 // ResourceStore.
 type ResourceStoreFactory struct {
-	objectStore objectstore.ModelObjectStoreGetter
-	mu          sync.Mutex
-	storeMap    map[resource.Type]store.ResourceStore
+	objectStore         objectstore.ModelObjectStoreGetter
+	containerImageStore store.ResourceStoreGetter
 }
 
 // NewResourceStoreFactory returns a factory which provides the appropriate
 // Resource Store for the resource type.
-func NewResourceStoreFactory(objectStore objectstore.ModelObjectStoreGetter) *ResourceStoreFactory {
+func NewResourceStoreFactory(
+	objectStore objectstore.ModelObjectStoreGetter,
+	containerImageStore store.ResourceStoreGetter,
+) *ResourceStoreFactory {
 	return &ResourceStoreFactory{
-		objectStore: objectStore,
-		storeMap:    make(map[resource.Type]store.ResourceStore),
+		objectStore:         objectStore,
+		containerImageStore: containerImageStore,
 	}
 }
 
@@ -35,33 +36,14 @@ func NewResourceStoreFactory(objectStore objectstore.ModelObjectStoreGetter) *Re
 func (f *ResourceStoreFactory) GetResourceStore(ctx context.Context, t resource.Type) (store.ResourceStore, error) {
 	switch t {
 	case resource.TypeFile:
-		store, err := f.objectStore.GetObjectStore(ctx)
+		objectStore, err := f.objectStore.GetObjectStore(ctx)
 		if err != nil {
 			return nil, errors.Errorf("getting file resource store: %w", err)
 		}
-		return fileResourceStore{objectStore: store}, nil
+		return fileResourceStore{objectStore: objectStore}, nil
+	case resource.TypeContainerImage:
+		return f.containerImageStore(), nil
 	default:
-		f.mu.Lock()
-		defer f.mu.Unlock()
-		store, ok := f.storeMap[t]
-		if !ok {
-			return nil, UnknownResourceType
-		}
-		return store, nil
+		return nil, UnknownResourceType
 	}
 }
-
-// AddStore injects a ResourceStore for the given type into the
-// ResourceStoreFactory.
-//
-// Note:
-// The store for container image resources is really a DQLite table today.
-// This method is a compromise to avoid injecting one service into another
-// if the ContainerImageResourceStore was provided as an argument to
-// NewResourceStoreFactory. If we get a new implementation of a container image
-// resource store this should be re-evaluated and hopefully removed.
-func (f *ResourceStoreFactory) AddStore(t resource.Type, store store.ResourceStore) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	f.storeMap[t] = store
-}