Merge pull request juju#18422 from SimonRichardson/refactor-applicati…

…on-domain

juju#18422

Now that we have the resources, the application domain was getting really hard to construct. The solution was to remove all namespaces and just have a State and a Service, which can be constructed. This has worked well for all the other domains. We can come back to this and split it up if we find pieces that can be broken off.

This is work towards the charm async downloader, as I wanted to add one extra dependency and it just got larger and larger.

In the future, we'll remove `modelUUID` and the model agent state from the constructor, so there will be even less. If adding more to the constructor, it's better to consider grouping similar things into a type `CharmDependencies` or `CharmServiceConfig`. 

-----

This pull request includes several changes focused on improving the model migration process and simplifying the service structure in the Juju codebase. The most important changes include removing unused imports, refactoring service initialization, and updating the model migration export and import operations to use new service structures.

### Codebase Simplification:

* Removed unused imports in multiple files, including `cleaner_test.go`, `export.go`, `import.go`, `application_test.go`, and `charm.go`. [[1]](diffhunk://#diff-48fb36787e1c279e1ba1ff38ed9cc5ae49a36ed40b0dd0cc22544258983ba410L21-L28) [[2]](diffhunk://#diff-ccbcc810b4f9ffaceaabc9ec2ed3eadb96e4e243df8381cddcd6b3c0dfb3aeb1R11-R35) [[3]](diffhunk://#diff-7961d740899680f58e70493a1801eb5c86cf80e665b8615b876d22e0dd14b7faR12) [[4]](diffhunk://#diff-54808e512b799a9eb1000c96054f7135a8a0fa435bcd7ab6e36f8ee6ade75922L10-L12) [[5]](diffhunk://#diff-f634e196c8c372b1bc36ad4d6db384d26713e39a92b90c981816480a2d1e91d6L15)
* Refactored the initialization of `WatchableService` in `cleaner_test.go` to use a simpler structure.
* Removed the `NoopDeleteSecretState` and updated the `exportOperation` and `importOperation` to use the new `MigrationService` structure in `export.go` and `import.go`. [[1]](diffhunk://#diff-ccbcc810b4f9ffaceaabc9ec2ed3eadb96e4e243df8381cddcd6b3c0dfb3aeb1L51-R64) [[2]](diffhunk://#diff-ccbcc810b4f9ffaceaabc9ec2ed3eadb96e4e243df8381cddcd6b3c0dfb3aeb1L76-R78) [[3]](diffhunk://#diff-7961d740899680f58e70493a1801eb5c86cf80e665b8615b876d22e0dd14b7faL49-R58) [[4]](diffhunk://#diff-7961d740899680f58e70493a1801eb5c86cf80e665b8615b876d22e0dd14b7faL74-R83)
* Simplified the `application_test.go` by removing redundant test setups and mocks. [[1]](diffhunk://#diff-54808e512b799a9eb1000c96054f7135a8a0fa435bcd7ab6e36f8ee6ade75922L1117-R1024) [[2]](diffhunk://#diff-54808e512b799a9eb1000c96054f7135a8a0fa435bcd7ab6e36f8ee6ade75922L1175-R1044) [[3]](diffhunk://#diff-54808e512b799a9eb1000c96054f7135a8a0fa435bcd7ab6e36f8ee6ade75922L1194-L1210)
* Merged `CharmService` into `Service` and updated methods in `charm.go` to reflect this change. [[1]](diffhunk://#diff-f634e196c8c372b1bc36ad4d6db384d26713e39a92b90c981816480a2d1e91d6L141-R146) [[2]](diffhunk://#diff-f634e196c8c372b1bc36ad4d6db384d26713e39a92b90c981816480a2d1e91d6L181-R166) [[3]](diffhunk://#diff-f634e196c8c372b1bc36ad4d6db384d26713e39a92b90c981816480a2d1e91d6L199-R184)

These changes aim to streamline the code, making it easier to maintain and extend in the future.

### QA Steps

```
$ juju bootstrap lxd test
$ juju add-model default
$ juju deploy ubuntu
```

apiserver/facades/controller/cleaner/cleaner_test.go

@@ -5,7 +5,6 @@ package cleaner_test
 
 import (
 	"context"
-	"io"
 
 	"github.com/juju/errors"
 	"github.com/juju/testing"
@@ -18,15 +17,9 @@ import (
 	"github.com/juju/juju/apiserver/facades/controller/cleaner"
 	apiservertesting "github.com/juju/juju/apiserver/testing"
 	"github.com/juju/juju/core/objectstore"
-	coreresource "github.com/juju/juju/core/resources"
-	corestorage "github.com/juju/juju/core/storage"
-	"github.com/juju/juju/domain/application/resource"
 	applicationservice "github.com/juju/juju/domain/application/service"
 	machineservice "github.com/juju/juju/domain/machine/service"
 	domainservicestesting "github.com/juju/juju/domain/services/testing"
-	charmresource "github.com/juju/juju/internal/charm/resource"
-	loggertesting "github.com/juju/juju/internal/logger/testing"
-	"github.com/juju/juju/internal/storage"
 	coretesting "github.com/juju/juju/internal/testing"
 	"github.com/juju/juju/rpc/params"
 	"github.com/juju/juju/state"
@@ -55,14 +48,7 @@ func (s *CleanerSuite) SetUpTest(c *gc.C) {
 
 	res := common.NewResources()
 	s.machineService = machineservice.NewWatchableService(nil, nil, nil)
-	s.applicationService = applicationservice.NewWatchableService(
-		nil, nil, nil, nil, nil, "", nil, nil,
-		corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry {
-			return storage.NotImplementedProviderRegistry{}
-		}),
-		&stubResourceStoreGetter{},
-		loggertesting.WrapCheckLog(c),
-	)
+	s.applicationService = &applicationservice.WatchableService{}
 
 	var err error
 	s.api, err = cleaner.NewCleanerAPI(facadetest.ModelContext{
@@ -171,41 +157,3 @@ func (st *mockState) Cleanup(_ context.Context, _ objectstore.ObjectStore, mr st
 	st.MethodCall(st, "Cleanup", mr, ar)
 	return st.NextErr()
 }
-
-type stubResourceStoreGetter struct{}
-
-func (stubResourceStoreGetter) AddStore(t charmresource.Type, store resource.ResourceStore) {}
-
-func (stubResourceStoreGetter) GetResourceStore(context.Context, charmresource.Type) (resource.ResourceStore, error) {
-	return &stubResourceStore{}, nil
-}
-
-type stubResourceStore struct{}
-
-// Get returns an io.ReadCloser for a resource in the resource store.
-func (f stubResourceStore) Get(
-	ctx context.Context,
-	resourceUUID coreresource.ID,
-) (r io.ReadCloser, size int64, err error) {
-	return nil, -1, errors.NotImplemented
-}
-
-// Put stores data from io.Reader in the resource store at the
-// path specified in the resource.
-func (f stubResourceStore) Put(
-	ctx context.Context,
-	resourceUUID coreresource.ID,
-	r io.Reader,
-	size int64,
-	fingerprint charmresource.Fingerprint,
-) (resource.ResourceStorageUUID, error) {
-	return nil, errors.NotImplemented
-}
-
-// Remove removes a resource from storage.
-func (f stubResourceStore) Remove(
-	ctx context.Context,
-	resourceUUID coreresource.ID,
-) error {
-	return errors.NotImplemented
-}

apiserver/root.go

@@ -940,7 +940,16 @@ func (ctx *facadeContext) ModelExporter(modelUUID model.UUID, backend facade.Leg
 		return ctx.r.objectStoreGetter.GetObjectStore(stdCtx, ctx.ModelUUID().String())
 	})
 
-	exporter := domainmodelmigration.NewExporter(coordinator, objectStoreGetter, logger, clock)
+	exporter := domainmodelmigration.NewExporter(
+		coordinator,
+		modelStorageRegistry(func(ctx context.Context) (storage.ProviderRegistry, error) {
+			storageService := domainServices.Storage()
+			return storageService.GetStorageRegistry(ctx)
+		}),
+		objectStoreGetter,
+		clock,
+		logger,
+	)
 	return migration.NewModelExporter(
 		exporter,
 		backend,

domain/application/modelmigration/export.go

@@ -8,28 +8,32 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/juju/clock"
 	"github.com/juju/description/v8"
 	"github.com/juju/errors"
 
 	corecharm "github.com/juju/juju/core/charm"
 	"github.com/juju/juju/core/logger"
 	"github.com/juju/juju/core/modelmigration"
-	coresecrets "github.com/juju/juju/core/secrets"
 	corestorage "github.com/juju/juju/core/storage"
-	"github.com/juju/juju/domain"
 	"github.com/juju/juju/domain/application/charm"
-	resourcestore "github.com/juju/juju/domain/application/resource"
 	"github.com/juju/juju/domain/application/service"
 	"github.com/juju/juju/domain/application/state"
 	internalcharm "github.com/juju/juju/internal/charm"
 	"github.com/juju/juju/internal/charm/resource"
-	"github.com/juju/juju/internal/storage"
 )
 
 // RegisterExport registers the export operations with the given coordinator.
-func RegisterExport(coordinator Coordinator, logger logger.Logger) {
+func RegisterExport(
+	coordinator Coordinator,
+	registry corestorage.ModelStorageRegistryGetter,
+	clock clock.Clock,
+	logger logger.Logger,
+) {
 	coordinator.Add(&exportOperation{
-		logger: logger,
+		registry: registry,
+		clock:    clock,
+		logger:   logger,
 	})
 }
 
@@ -48,21 +52,16 @@ type ExportService interface {
 	GetCharm(ctx context.Context, id corecharm.ID) (internalcharm.Charm, charm.CharmOrigin, error)
 }
 
-// NoopDeleteSecretState defines a secret state which does nothing.
-// When importing and exporting, we are not deleting any secrets.
-type NoopDeleteSecretState struct{}
-
-func (NoopDeleteSecretState) DeleteSecret(domain.AtomicContext, *coresecrets.URI, []int) error {
-	return nil
-}
-
 // exportOperation describes a way to execute a migration for
 // exporting applications.
 type exportOperation struct {
 	modelmigration.BaseOperation
 
-	logger  logger.Logger
 	service ExportService
+
+	registry corestorage.ModelStorageRegistryGetter
+	clock    clock.Clock
+	logger   logger.Logger
 }
 
 // Name returns the name of this operation.
@@ -73,19 +72,10 @@ func (e *exportOperation) Name() string {
 // Setup the export operation.
 // This will create a new service instance.
 func (e *exportOperation) Setup(scope modelmigration.Scope) error {
-	// TODO (stickupkid): The storage.ProviderRegistry should be passed in
-	// so that we can create the appropriate storage instances.
-	e.service = service.NewService(
-		state.NewApplicationState(scope.ModelDB(), e.logger),
-		NoopDeleteSecretState{},
-		state.NewCharmState(scope.ModelDB()),
-		state.NewResourceState(scope.ModelDB(), e.logger),
-		corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry {
-			return storage.NotImplementedProviderRegistry{}
-		}),
-		// TODO: Wire through an objectstoreGetter when implementing
-		// model migration for resources if needed.
-		resourcestore.NewResourceStoreFactory(nil),
+	e.service = service.NewMigrationService(
+		state.NewState(scope.ModelDB(), e.logger),
+		e.registry,
+		e.clock,
 		e.logger,
 	)
 	return nil

domain/application/modelmigration/import.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"strconv"
 
+	"github.com/juju/clock"
 	"github.com/juju/description/v8"
 	"github.com/juju/errors"
 	"github.com/juju/version/v2"
@@ -20,7 +21,6 @@ import (
 	corestatus "github.com/juju/juju/core/status"
 	corestorage "github.com/juju/juju/core/storage"
 	coreunit "github.com/juju/juju/core/unit"
-	resourcestore "github.com/juju/juju/domain/application/resource"
 	"github.com/juju/juju/domain/application/service"
 	"github.com/juju/juju/domain/application/state"
 	internalcharm "github.com/juju/juju/internal/charm"
@@ -35,9 +35,15 @@ type Coordinator interface {
 
 // RegisterImport register's a new model migration importer into the supplied
 // coordinator.
-func RegisterImport(coordinator Coordinator, registry corestorage.ModelStorageRegistryGetter, logger logger.Logger) {
+func RegisterImport(
+	coordinator Coordinator,
+	registry corestorage.ModelStorageRegistryGetter,
+	clock clock.Clock,
+	logger logger.Logger,
+) {
 	coordinator.Add(&importOperation{
 		registry:     registry,
+		clock:        clock,
 		logger:       logger,
 		charmOrigins: make(map[string]*corecharm.Origin),
 	})
@@ -46,10 +52,11 @@ func RegisterImport(coordinator Coordinator, registry corestorage.ModelStorageRe
 type importOperation struct {
 	modelmigration.BaseOperation
 
-	logger logger.Logger
+	service ImportService
 
-	service  ImportService
 	registry corestorage.ModelStorageRegistryGetter
+	clock    clock.Clock
+	logger   logger.Logger
 
 	charmOrigins map[string]*corecharm.Origin
 }
@@ -71,15 +78,10 @@ func (i *importOperation) Name() string {
 
 // Setup creates the service that is used to import applications.
 func (i *importOperation) Setup(scope modelmigration.Scope) error {
-	i.service = service.NewService(
-		state.NewApplicationState(scope.ModelDB(), i.logger),
-		NoopDeleteSecretState{},
-		state.NewCharmState(scope.ModelDB()),
-		state.NewResourceState(scope.ModelDB(), i.logger),
+	i.service = service.NewMigrationService(
+		state.NewState(scope.ModelDB(), i.logger),
 		i.registry,
-		// TODO: Wire through an objectstoreGetter when implementing
-		// model migration for resources if needed.
-		resourcestore.NewResourceStoreFactory(nil),
+		i.clock,
 		i.logger,
 	)
 	return nil