diff --git a/doc/changes/changes_0.5.2.md b/doc/changes/changes_0.5.2.md index 0b12594b..16a4a70c 100644 --- a/doc/changes/changes_0.5.2.md +++ b/doc/changes/changes_0.5.2.md @@ -9,6 +9,7 @@ This release updates the upload process for the extension registry to verify tha ## Features * #129: Added verification for extension URLs before uploading to registry +* #130: Added verification that no instance exists before uninstalling an extension ## Documentation diff --git a/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/ExtensionManagerClientIT.java b/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/ExtensionManagerClientIT.java index 18dbb458..116c0c1f 100644 --- a/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/ExtensionManagerClientIT.java +++ b/extension-manager-integration-test-java/src/test/java/com/exasol/extensionmanager/itest/ExtensionManagerClientIT.java @@ -91,13 +91,9 @@ void installVersion() { } @Test - void uninstall() { - assertDoesNotThrow(() -> client.uninstall()); - } - - @Test - void uninstallWithExtensionVersion() { - assertDoesNotThrow(() -> client.uninstall(EXTENSION_VERSION)); + void uninstallFailsBecauseInstanceExists() { + client.assertRequestFails(() -> client.uninstall(), + "cannot uninstall extension because 1 instance(s) still exist: Instance 1", 400); } @Test diff --git a/pkg/extensionController/controller.go b/pkg/extensionController/controller.go index 1cc071e9..e6233e02 100644 --- a/pkg/extensionController/controller.go +++ b/pkg/extensionController/controller.go @@ -209,7 +209,22 @@ func (c *controllerImpl) UninstallExtension(txCtx *transaction.TransactionContex if err != nil { return extensionLoadingFailed(extensionId, err) } - return extension.Uninstall(c.createExtensionContext(txCtx), extensionVersion) + extensionCtx := c.createExtensionContext(txCtx) + instances, err := extension.ListInstances(extensionCtx, extensionVersion) + if err != nil { + return fmt.Errorf("failed to check existing instances: %w", err) + } + if len(instances) > 0 { + instanceNames := "" + for _, inst := range instances { + if instanceNames != "" { + instanceNames += ", " + } + instanceNames += inst.Name + } + return apiErrors.NewBadRequestErrorF("cannot uninstall extension because %d instance(s) still exist: %s", len(instances), instanceNames) + } + return extension.Uninstall(extensionCtx, extensionVersion) } /* [impl -> dsn~upgrade-extension~1]. */ diff --git a/pkg/extensionController/controller_test.go b/pkg/extensionController/controller_test.go index a2d7f1db..7a400545 100644 --- a/pkg/extensionController/controller_test.go +++ b/pkg/extensionController/controller_test.go @@ -122,6 +122,31 @@ var errorTests = []errorTest{ {testName: "null pointer", throwCommand: `({}).a.b; throw Error("mock");`, expectedStatus: -1, expectedMessage: "TypeError: Cannot read property 'b' of undefined"}, } +// GetInstalledExtensions + +func (suite *ControllerUTestSuite) TestGetAllInstallations() { + suite.writeDefaultExtension() + suite.metaDataMock.SimulateExaAllScripts([]exaMetadata.ExaScriptRow{{Schema: "schema", Name: "script"}}) + suite.dbMock.ExpectBegin() + suite.dbMock.ExpectRollback() + installations, err := suite.controller.GetInstalledExtensions(mockContext(), suite.db) + suite.NoError(err) + suite.Equal([]*extensionAPI.JsExtInstallation{{Name: "schema.script", Version: "0.1.0"}}, installations) +} + +func (suite *ControllerUTestSuite) TestGetAllInstallationsReturnsEmptyList() { + integrationTesting.CreateTestExtensionBuilder(suite.T()). + WithFindInstallationsFunc("return []"). + Build(). + WriteToFile(path.Join(suite.tempExtensionRepo, EXTENSION_ID)) + suite.metaDataMock.SimulateExaAllScripts([]exaMetadata.ExaScriptRow{{Schema: "schema", Name: "script"}}) + suite.dbMock.ExpectBegin() + suite.dbMock.ExpectRollback() + installations, err := suite.controller.GetInstalledExtensions(mockContext(), suite.db) + suite.NoError(err) + suite.Empty(installations) +} + func (suite *ControllerUTestSuite) TestGetAllInstallationsFails() { for _, t := range errorTests { suite.Run(t.testName, func() { @@ -141,6 +166,34 @@ func (suite *ControllerUTestSuite) TestGetAllInstallationsFails() { } } +// FindInstances + +func (suite *ControllerUTestSuite) TestFindInstancesReturnsEmptyList() { + integrationTesting.CreateTestExtensionBuilder(suite.T()). + WithFindInstancesFunc(`return []`). + Build(). + WriteToFile(path.Join(suite.tempExtensionRepo, EXTENSION_ID)) + suite.initDbMock() + suite.dbMock.ExpectBegin() + suite.dbMock.ExpectRollback() + extensions, err := suite.controller.FindInstances(mockContext(), suite.db, EXTENSION_ID, "ver") + suite.NoError(err) + suite.Empty(extensions) +} + +func (suite *ControllerUTestSuite) TestFindInstancesReturnsEntries() { + integrationTesting.CreateTestExtensionBuilder(suite.T()). + WithFindInstancesFunc(`return [{"id":"ext1","name":"ext-name1"},{"id":"ext2","name":"ext-name2"}]`). + Build(). + WriteToFile(path.Join(suite.tempExtensionRepo, EXTENSION_ID)) + suite.initDbMock() + suite.dbMock.ExpectBegin() + suite.dbMock.ExpectRollback() + extensions, err := suite.controller.FindInstances(mockContext(), suite.db, EXTENSION_ID, "ver") + suite.NoError(err) + suite.Equal([]*extensionAPI.JsExtInstance{{Id: "ext1", Name: "ext-name1"}, {Id: "ext2", Name: "ext-name2"}}, extensions) +} + func (suite *ControllerUTestSuite) TestFindInstancesFails() { for _, t := range errorTests { suite.Run(t.testName, func() { @@ -205,16 +258,6 @@ func (suite *ControllerUTestSuite) assertError(t errorTest, actualError error) { } } -func (suite *ControllerUTestSuite) TestGetAllInstallations() { - suite.writeDefaultExtension() - suite.metaDataMock.SimulateExaAllScripts([]exaMetadata.ExaScriptRow{{Schema: "schema", Name: "script"}}) - suite.dbMock.ExpectBegin() - suite.dbMock.ExpectRollback() - installations, err := suite.controller.GetInstalledExtensions(mockContext(), suite.db) - suite.NoError(err) - suite.Equal([]*extensionAPI.JsExtInstallation{{Name: "schema.script", Version: "0.1.0"}}, installations) -} - // InstallExtension func (suite *ControllerUTestSuite) TestInstallFailsForUnknownExtensionId() { @@ -318,6 +361,35 @@ func (suite *ControllerUTestSuite) TestUninstallFails() { } } +func (suite *ControllerUTestSuite) TestUninstallFailsWhenInstanceExists() { + integrationTesting.CreateTestExtensionBuilder(suite.T()). + WithUninstallFunc("context.sqlClient.execute(`uninstall extension version ${version}`)"). + WithFindInstancesFunc(`return [{"id":"ext1","name":"ext-name1"}, {"id":"ext2","name":"ext-name2"}]`). + Build(). + WriteToFile(path.Join(suite.tempExtensionRepo, EXTENSION_ID)) + suite.dbMock.ExpectBegin() + suite.dbMock.ExpectRollback() + err := suite.controller.UninstallExtension(mockContext(), suite.db, EXTENSION_ID, "ver") + suite.EqualError(err, "cannot uninstall extension because 2 instance(s) still exist: ext-name1, ext-name2") + if apiErr, ok := err.(*apiErrors.APIError); ok { + suite.Equal(400, apiErr.Status) + } else { + suite.Fail(fmt.Sprintf("expected an APIError but got %T: %v", err, err)) + } +} + +func (suite *ControllerUTestSuite) TestUninstallFailsListingInstances() { + integrationTesting.CreateTestExtensionBuilder(suite.T()). + WithUninstallFunc("context.sqlClient.execute(`uninstall extension version ${version}`)"). + WithFindInstancesFunc(`throw new Error('mock js error')`). + Build(). + WriteToFile(path.Join(suite.tempExtensionRepo, EXTENSION_ID)) + suite.dbMock.ExpectBegin() + suite.dbMock.ExpectRollback() + err := suite.controller.UninstallExtension(mockContext(), suite.db, EXTENSION_ID, "ver") + suite.ErrorContains(err, `failed to check existing instances: failed to list instances for extension "testing-extension.js" in version "ver": Error: mock js error`) +} + // Upgrade func (suite *ControllerUTestSuite) TestUpgradeFailsForUnknownExtensionId() {