#130 Verify no instance exists before uninstalling an extension (#141)

exasol · Sep 12, 2023 · aa8a147 · aa8a147
1 parent 29bfa04
commit aa8a147
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 18 deletions.
diff --git 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/...n-test-java/src/test/java/com/exasol/extensionmanager/itest/ExtensionManagerClientIT.java b/...n-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
@@ -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
@@ -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() {