From bb3d8a1b88ce54acd36ee2f4cd321aeb8ba6ff3e Mon Sep 17 00:00:00 2001
From: Kelvin Liu <kelvin.liu@canonical.com>
Date: Wed, 4 Dec 2024 15:13:54 +1100
Subject: [PATCH] fix: we should register exporters once instead of many times
 for each export request;

---
 internal/migration/migration.go      |  5 +++--
 internal/migration/migration_test.go | 26 ++++++++++++++------------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/internal/migration/migration.go b/internal/migration/migration.go
index e96dc2bb98c..fedae3e6c2a 100644
--- a/internal/migration/migration.go
+++ b/internal/migration/migration.go
@@ -93,7 +93,7 @@ func NewModelExporter(
 	logger corelogger.Logger,
 	clock clock.Clock,
 ) *ModelExporter {
-	return &ModelExporter{
+	me := &ModelExporter{
 		operationExporter:     operationExporter,
 		legacyStateExporter:   legacyStateExporter,
 		scope:                 scope,
@@ -102,6 +102,8 @@ func NewModelExporter(
 		logger:                logger,
 		clock:                 clock,
 	}
+	me.operationExporter.ExportOperations(me.storageRegistryGetter)
+	return me
 }
 
 // ExportModelPartial partially serializes a model description from the
@@ -129,7 +131,6 @@ func (e *ModelExporter) ExportModel(ctx context.Context, leaders map[string]stri
 
 // Export serializes a model description from the database contents.
 func (e *ModelExporter) Export(ctx context.Context, model description.Model) (description.Model, error) {
-	e.operationExporter.ExportOperations(e.storageRegistryGetter)
 	if err := e.coordinator.Perform(ctx, e.scope, model); err != nil {
 		return nil, errors.Trace(err)
 	}
diff --git a/internal/migration/migration_test.go b/internal/migration/migration_test.go
index 66db682fb79..7bcd0e905b2 100644
--- a/internal/migration/migration_test.go
+++ b/internal/migration/migration_test.go
@@ -62,14 +62,6 @@ func (s *ExportSuite) TestExportValidates(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 
 	scope := modelmigration.NewScope(nil, nil, nil)
-	exporter := migration.NewModelExporter(
-		s.operationsExporter,
-		nil,
-		scope,
-		s.storageRegistryGetter,
-		s.coordinator,
-		nil, nil,
-	)
 
 	// The order of the expectations is important here. We expect that the
 	// validation is the last thing that happens.
@@ -79,6 +71,15 @@ func (s *ExportSuite) TestExportValidates(c *gc.C) {
 		s.model.EXPECT().Validate().Return(nil),
 	)
 
+	exporter := migration.NewModelExporter(
+		s.operationsExporter,
+		nil,
+		scope,
+		s.storageRegistryGetter,
+		s.coordinator,
+		nil, nil,
+	)
+
 	_, err := exporter.Export(context.Background(), s.model)
 	c.Assert(err, jc.ErrorIsNil)
 }
@@ -87,6 +88,11 @@ func (s *ExportSuite) TestExportValidationFails(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 
 	scope := modelmigration.NewScope(nil, nil, nil)
+
+	s.operationsExporter.EXPECT().ExportOperations(s.storageRegistryGetter)
+	s.model.EXPECT().Validate().Return(errors.New("boom"))
+	s.coordinator.EXPECT().Perform(gomock.Any(), scope, s.model).Return(nil)
+
 	exporter := migration.NewModelExporter(
 		s.operationsExporter,
 		nil,
@@ -96,10 +102,6 @@ func (s *ExportSuite) TestExportValidationFails(c *gc.C) {
 		nil, nil,
 	)
 
-	s.operationsExporter.EXPECT().ExportOperations(s.storageRegistryGetter)
-	s.model.EXPECT().Validate().Return(errors.New("boom"))
-	s.coordinator.EXPECT().Perform(gomock.Any(), scope, s.model).Return(nil)
-
 	_, err := exporter.Export(context.Background(), s.model)
 	c.Assert(err, gc.ErrorMatches, "boom")
 }