Implement review findings by @ckunki

exasol · Oct 16, 2023 · 32b93ae · 32b93ae
1 parent dfb1453
commit 32b93ae
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 9 deletions.
diff --git a/pkg/extensionController/bfs/BucketFsApi.go b/pkg/extensionController/bfs/BucketFsApi.go
@@ -10,7 +10,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// BucketFsAPI allows access to BucketFS. Call the [BucketFsAPI.Close] method to release resources after using the BucketFS API.
+// BucketFsAPI allows access to BucketFS.
+// Users must call the [BucketFsAPI.Close] method to release resources after using the BucketFS API.
 type BucketFsAPI interface {
 	// ListFiles lists all files in the configured directory recursively.
 	ListFiles() ([]BfsFile, error)
@@ -63,12 +64,12 @@ func (bfs bucketFsAPIImpl) ListFiles() ([]BfsFile, error) {
 	t0 := time.Now()
 	statement, err := bfs.transaction.Prepare("SELECT " + bfs.udfScriptName + "(?) ORDER BY FULL_PATH") //nolint:gosec // SQL string concatenation is safe here
 	if err != nil {
-		return nil, fmt.Errorf("failed to create prepared statement for running list files UDF. Cause: %w", err)
+		return nil, fmt.Errorf("failed to create prepared statement for listing files. Cause: %w", err)
 	}
 	defer statement.Close()
 	result, err := statement.Query(bfs.bucketFsBasePath)
 	if err != nil {
-		return nil, fmt.Errorf("failed to list files in BucketFS using UDF. Cause: %w", err)
+		return nil, fmt.Errorf("failed to list files. Cause: %w", err)
 	}
 	defer result.Close()
 	files, err := readQueryResult(result)

diff --git a/pkg/extensionController/controller_test.go b/pkg/extensionController/controller_test.go
@@ -91,8 +91,8 @@ func (suite *ControllerUTestSuite) AfterTest(suiteName, testName string) {
 // GetAllExtensions
 
 /* [utest -> dsn~list-extensions~1]. */
-func (suite *ControllerUTestSuite) TestGetAllExtensions() {
-	suite.writeDefaultExtension()
+func (suite *ControllerUTestSuite) TestGetAllExtensionsAndBucketFSContainsJar() {
+	suite.registerDefaultExtensionDefinition()
 	suite.dbMock.ExpectBegin()
 	suite.bucketFsMock.SimulateFiles([]bfs.BfsFile{{Name: "my-extension.1.2.3.jar", Size: 3, Path: "path"}})
 	suite.bucketFsMock.SimulateCloseSuccess()
@@ -110,8 +110,8 @@ func (suite *ControllerUTestSuite) TestGetAllExtensionsFailsStartingTransaction(
 }
 
 /* [utest -> dsn~list-extensions~1]. */
-func (suite *ControllerUTestSuite) TestGetAllExtensionsWithMissingJar() {
-	suite.writeDefaultExtension()
+func (suite *ControllerUTestSuite) TestGetAllExtensionsButNoJarInBucketFS() {
+	suite.registerDefaultExtensionDefinition()
 	suite.dbMock.ExpectBegin()
 	suite.bucketFsMock.SimulateFiles([]bfs.BfsFile{})
 	suite.bucketFsMock.SimulateCloseSuccess()
@@ -157,7 +157,7 @@ var errorTests = []errorTest{
 // GetInstalledExtensions
 
 func (suite *ControllerUTestSuite) TestGetAllInstallations() {
-	suite.writeDefaultExtension()
+	suite.registerDefaultExtensionDefinition()
 	//nolint:exhaustruct // Non-exhaustive struct is fine here
 	suite.metaDataMock.SimulateExaAllScripts([]exaMetadata.ExaScriptRow{{Schema: "schema", Name: "script"}})
 	suite.dbMock.ExpectBegin()
@@ -612,7 +612,7 @@ func (suite *ControllerUTestSuite) TestDeleteInstanceFailsStartingTransaction()
 	suite.EqualError(err, beginTransactionFailedErrorMsg)
 }
 
-func (suite *ControllerUTestSuite) writeDefaultExtension() {
+func (suite *ControllerUTestSuite) registerDefaultExtensionDefinition() {
 	integrationTesting.CreateTestExtensionBuilder(suite.T()).
 		WithBucketFsUpload(integrationTesting.BucketFsUploadParams{Name: "extension jar", BucketFsFilename: "my-extension.1.2.3.jar", FileSize: 3, DownloadUrl: "", LicenseUrl: "", LicenseAgreementRequired: false}).
 		WithFindInstallationsFunc(`