feat: Fir 31840 dont append account id for system engine url in go sdk (

#95)

client.go

@@ -9,7 +9,7 @@ import (
 
 type ClientImpl struct {
 	ConnectedToSystemEngine bool
-	SystemEngineURL         string
+	AccountName             string
 	AccountVersion          int
 	BaseClient
 }
@@ -31,6 +31,7 @@ func MakeClient(settings *fireboltSettings, apiEndpoint string) (*ClientImpl, er
 			ApiEndpoint:  apiEndpoint,
 			UserAgent:    ConstructUserAgentString(),
 		},
+		AccountName: settings.accountName,
 	}
 	client.parameterGetter = client.getQueryParams
 	client.accessTokenGetter = client.getAccessToken
@@ -40,10 +41,6 @@ func MakeClient(settings *fireboltSettings, apiEndpoint string) (*ClientImpl, er
 	if err != nil {
 		return nil, ConstructNestedError("error during getting account id", err)
 	}
-	client.SystemEngineURL, err = client.getSystemEngineURL(context.Background(), settings.accountName)
-	if err != nil {
-		return nil, ConstructNestedError("error during getting system engine url", err)
-	}
 	return client, nil
 }
 
@@ -86,7 +83,7 @@ func parseEngineInfoResponse(resp [][]interface{}) (string, string, string, erro
 	return engineUrl, status, dbName, nil
 }
 
-func (c *ClientImpl) getSystemEngineURL(ctx context.Context, accountName string) (string, error) {
+func (c *ClientImpl) getSystemEngineURLAndParameters(ctx context.Context, accountName string, databaseName string) (string, map[string]string, error) {
 	infolog.Printf("Get system engine URL for account '%s'", accountName)
 
 	type SystemEngineURLResponse struct {
@@ -97,23 +94,30 @@ func (c *ClientImpl) getSystemEngineURL(ctx context.Context, accountName string)
 
 	resp := c.request(ctx, "GET", url, make(map[string]string), "")
 	if resp.statusCode == 404 {
-		return "", fmt.Errorf(accountError, accountName)
+		return "", nil, fmt.Errorf(accountError, accountName)
 	}
 	if resp.err != nil {
-		return "", ConstructNestedError("error during system engine url http request", resp.err)
+		return "", nil, ConstructNestedError("error during system engine url http request", resp.err)
 	}
 
 	var systemEngineURLResponse SystemEngineURLResponse
 	if err := json.Unmarshal(resp.data, &systemEngineURLResponse); err != nil {
-		return "", ConstructNestedError("error during unmarshalling system engine URL response", errors.New(string(resp.data)))
+		return "", nil, ConstructNestedError("error during unmarshalling system engine URL response", errors.New(string(resp.data)))
 	}
-	// Ignore any query parameters provided in the URL
-	engineUrl, _, err := splitEngineEndpoint(systemEngineURLResponse.EngineUrl)
+	engineUrl, queryParams, err := splitEngineEndpoint(systemEngineURLResponse.EngineUrl)
 	if err != nil {
-		return "", ConstructNestedError("error during splitting system engine URL", err)
+		return "", nil, ConstructNestedError("error during splitting system engine URL", err)
+	}
+
+	parameters := make(map[string]string)
+	if len(databaseName) != 0 {
+		parameters["database"] = databaseName
+	}
+	for key, value := range queryParams {
+		parameters[key] = value[0]
 	}
 
-	return engineUrl, nil
+	return engineUrl, parameters, nil
 }
 
 func (c *ClientImpl) getAccountInfo(ctx context.Context, accountName string) (string, int, error) {
@@ -152,8 +156,8 @@ func (c *ClientImpl) getQueryParams(setStatements map[string]string) (map[string
 	for setKey, setValue := range setStatements {
 		params[setKey] = setValue
 	}
-	// Account id is only used when querying system engine
-	if c.ConnectedToSystemEngine {
+	// Account id is only used when querying system engine for infra v1
+	if c.ConnectedToSystemEngine && c.AccountVersion == 1 {
 		if len(c.AccountID) == 0 {
 			return nil, fmt.Errorf("Trying to run a query against system engine without account id defined")
 		}
@@ -168,9 +172,15 @@ func (c *ClientImpl) getAccessToken() (string, error) {
 	return getAccessTokenServiceAccount(c.ClientID, c.ClientSecret, c.ApiEndpoint, c.UserAgent)
 }
 
-func (c *ClientImpl) getConnectionParametersV2(ctx context.Context, engineName, databaseName string) (string, map[string]string, error) {
-	engineURL := c.SystemEngineURL
-	parameters := make(map[string]string)
+func (c *ClientImpl) getConnectionParametersV2(
+	ctx context.Context,
+	engineName,
+	databaseName,
+	systemEngineURL string,
+	systemEngineParameters map[string]string,
+) (string, map[string]string, error) {
+	engineURL := systemEngineURL
+	parameters := systemEngineParameters
 	control := connectionControl{
 		updateParameters: func(key, value string) {
 			parameters[key] = value
@@ -193,13 +203,18 @@ func (c *ClientImpl) getConnectionParametersV2(ctx context.Context, engineName,
 	return engineURL, parameters, nil
 }
 
-func (c *ClientImpl) getConnectionParametersV1(ctx context.Context, engineName, databaseName string) (string, map[string]string, error) {
+func (c *ClientImpl) getConnectionParametersV1(
+	ctx context.Context,
+	engineName,
+	databaseName,
+	systemEngineURL string,
+) (string, map[string]string, error) {
 	// If engine name is empty, assume system engine
 	if len(engineName) == 0 {
-		return c.SystemEngineURL, map[string]string{"database": databaseName}, nil
+		return systemEngineURL, map[string]string{"database": databaseName}, nil
 	}
 
-	engineUrl, status, dbName, err := c.getEngineUrlStatusDBByName(ctx, engineName, c.SystemEngineURL)
+	engineUrl, status, dbName, err := c.getEngineUrlStatusDBByName(ctx, engineName, systemEngineURL)
 	params := map[string]string{"database": dbName}
 	if err != nil {
 		return "", params, ConstructNestedError("error during getting engine info", err)
@@ -221,9 +236,15 @@ func (c *ClientImpl) getConnectionParametersV1(ctx context.Context, engineName,
 // GetConnectionParameters returns engine URL and parameters based on engineName and databaseName
 func (c *ClientImpl) GetConnectionParameters(ctx context.Context, engineName, databaseName string) (string, map[string]string, error) {
 	// Assume we are connected to a system engine in the beginning
+
+	systemEngineURL, systemEngineParameters, err := c.getSystemEngineURLAndParameters(context.Background(), c.AccountName, databaseName)
+	if err != nil {
+		return "", nil, ConstructNestedError("error during getting system engine url", err)
+	}
+
 	c.ConnectedToSystemEngine = true
 	if c.AccountVersion == 2 {
-		return c.getConnectionParametersV2(ctx, engineName, databaseName)
+		return c.getConnectionParametersV2(ctx, engineName, databaseName, systemEngineURL, systemEngineParameters)
 	}
-	return c.getConnectionParametersV1(ctx, engineName, databaseName)
+	return c.getConnectionParametersV1(ctx, engineName, databaseName, systemEngineURL)
 }

client_integration_test.go

@@ -11,7 +11,7 @@ import (
 
 // TestGetEnginePropsByName test getting system engine url, as well as engine url, status and database by name
 func TestGetEnginePropsByName(t *testing.T) {
-	systemEngineURL, err := clientMockWithAccount.getSystemEngineURL(context.TODO(), accountNameV1Mock)
+	systemEngineURL, _, err := clientMockWithAccount.getSystemEngineURLAndParameters(context.TODO(), accountNameV1Mock, "")
 	if err != nil {
 		t.Errorf("Error returned by getSystemEngineURL: %s", err)
 	}

client_test.go

@@ -222,7 +222,7 @@ func TestGetSystemEngineURLReturnsErrorOn404(t *testing.T) {
 	defer server.Close()
 
 	// Call the getSystemEngineURL method and check if it returns an error
-	_, err := client.getSystemEngineURL(context.Background(), testAccountName)
+	_, _, err := client.getSystemEngineURLAndParameters(context.Background(), testAccountName, "")
 	if err == nil {
 		t.Errorf("Expected an error, got nil")
 	}

connection_test.go

@@ -49,7 +49,6 @@ func runProcessSetStatementFail(t *testing.T, value string) {
 func TestProcessSetStatement(t *testing.T) {
 	runProcessSetStatementFail(t, "SET database=my_db")
 	runProcessSetStatementFail(t, "SET engine=my_engine")
-	runProcessSetStatementFail(t, "SET account_id=1")
 	runProcessSetStatementFail(t, "SET output_format='json'")
 }
 
@@ -73,7 +72,6 @@ func TestResetParameters(t *testing.T) {
 	connector.cachedParameters = map[string]string{
 		"database":      "db",
 		"engine":        "engine",
-		"account_id":    "account_id",
 		"output_format": "output_format",
 		"key":           "value",
 	}
@@ -83,16 +81,15 @@ func TestResetParameters(t *testing.T) {
 	fireboltConnection.parameters = map[string]string{
 		"database":      "db",
 		"engine":        "engine",
-		"account_id":    "account_id",
 		"output_format": "output_format",
 		"key":           "value",
 	}
 
 	fireboltConnection.resetParameters()
-	if len(fireboltConnection.parameters) != 4 {
+	if len(fireboltConnection.parameters) != 3 {
 		t.Errorf("resetParameters didn't remove parameters correctly")
 	}
-	if len(connector.cachedParameters) != 4 {
+	if len(connector.cachedParameters) != 3 {
 		t.Errorf("resetParameters didn't remove parameters in connector correctly")
 	}
 }

driver_integration_test.go

@@ -86,7 +86,7 @@ func init() {
 }
 
 func getEngineURL() string {
-	systemEngineURL, err := clientMockWithAccount.getSystemEngineURL(context.TODO(), accountNameV1Mock)
+	systemEngineURL, _, err := clientMockWithAccount.getSystemEngineURLAndParameters(context.TODO(), accountNameV1Mock, "")
 	if err != nil {
 		panic(fmt.Sprintf("Error returned by getSystemEngineURL: %s", err))
 	}
@@ -344,7 +344,7 @@ func createServiceAccountNoUser(t *testing.T, serviceAccountName string) (string
 		t.Errorf("failed unexpectedly with %v", err)
 	}
 	// create service account
-	createServiceAccountQuery := fmt.Sprintf("CREATE SERVICE ACCOUNT \"%s\" WITH DESCRIPTION = \"%s\"", serviceAccountName, serviceAccountDescription)
+	createServiceAccountQuery := fmt.Sprintf("CREATE SERVICE ACCOUNT \"%s\" WITH DESCRIPTION = '%s'", serviceAccountName, serviceAccountDescription)
 	_, err = db.Query(createServiceAccountQuery)
 	if err != nil {
 		t.Errorf("The query %s returned an error: %v", createServiceAccountQuery, err)

driver_options.go

@@ -22,11 +22,16 @@ func WithDatabaseName(databaseName string) driverOption {
 // WithClientParams defines client parameters for the driver
 func WithClientParams(accountID string, token string, userAgent string) driverOption {
 	return func(d *FireboltDriver) {
+		if d.cachedParams == nil {
+			d.cachedParams = map[string]string{}
+		}
+		// Put account_id in cachedParams for it to work both with engines v1 and v2
+		d.cachedParams["account_id"] = accountID
+
 		cl := &ClientImpl{
 			ConnectedToSystemEngine: true,
 		}
 
-		cl.AccountID = accountID
 		cl.UserAgent = userAgent
 
 		cl.parameterGetter = cl.getQueryParams

driver_options_integration_test.go

@@ -8,30 +8,51 @@ import (
 	"testing"
 )
 
-func TestFireboltConnectorWithOptions(t *testing.T) {
+func testFireboltConnectorWithOptions(t *testing.T, engineUrl, databaseName, accountID, token, userAgent string) {
+	conn := FireboltConnectorWithOptions(
+		WithEngineUrl(engineUrl),
+		WithDatabaseName(databaseMock),
+		WithClientParams(accountID, token, userAgent),
+	)
+
+	resp, err := conn.client.Query(context.Background(), conn.engineUrl, "SELECT 1", nil, connectionControl{})
+	if err != nil {
+		t.Errorf("failed unexpectedly with: %v", err)
+	}
+	assert(len(resp.Data), 1, t, "result data length is not 1")
+	assert(len(resp.Data[0]), 1, t, "result value is invalid")
+	assert(resp.Data[0][0].(float64), float64(1), t, "result is not 1")
+}
+
+func TestFireboltConnectorWithOptionsAccountV1(t *testing.T) {
 	accountID := clientMockWithAccount.AccountID
 	userAgent := "test user agent"
 	token, err := getAccessTokenServiceAccount(clientIdMock, clientSecretMock, GetHostNameURL(), userAgent)
 	if err != nil {
 		t.Errorf("failed to get access token: %v", err)
 	}
 
-	engineUrl, err := clientMockWithAccount.getSystemEngineURL(context.TODO(), accountNameV1Mock)
+	engineUrl, _, err := clientMockWithAccount.getSystemEngineURLAndParameters(context.TODO(), accountNameV1Mock, "")
 	if err != nil {
 		t.Errorf("failed to get system engine url: %v", err)
 	}
 
-	conn := FireboltConnectorWithOptions(
-		WithEngineUrl(engineUrl),
-		WithDatabaseName(databaseMock),
-		WithClientParams(accountID, token, userAgent),
-	)
+	testFireboltConnectorWithOptions(t, engineUrl, databaseMock, accountID, token, userAgent)
+}
 
-	resp, err := conn.client.Query(context.Background(), conn.engineUrl, "SELECT 1", nil, connectionControl{})
+func TestFireboltConnectorWithOptionsAccountV2(t *testing.T) {
+	userAgent := "test user agent"
+	token, err := getAccessTokenServiceAccount(clientIdMock, clientSecretMock, GetHostNameURL(), userAgent)
 	if err != nil {
-		t.Errorf("failed unexpectedly with: %v", err)
+		t.Errorf("failed to get access token: %v", err)
 	}
-	assert(len(resp.Data), 1, t, "result data length is not 1")
-	assert(len(resp.Data[0]), 1, t, "result value is invalid")
-	assert(resp.Data[0][0].(float64), float64(1), t, "result is not 1")
+
+	engineUrl, engineParameters, err := clientMockWithAccount.getSystemEngineURLAndParameters(context.TODO(), accountNameV2Mock, "")
+	if err != nil {
+		t.Errorf("failed to get system engine url: %v", err)
+	}
+
+	accountID, _ := engineParameters["account_id"]
+
+	testFireboltConnectorWithOptions(t, engineUrl, databaseMock, accountID, token, userAgent)
 }

driver_options_test.go

@@ -23,7 +23,8 @@ func TestDriverOptions(t *testing.T) {
 	cl, ok := conn.client.(*ClientImpl)
 	assert(ok, true, t, "client is not *ClientImpl")
 
-	assert(cl.AccountID, accountID, t, "accountID is invalid")
+	connectionAccountID := conn.cachedParameters["account_id"]
+	assert(connectionAccountID, accountID, t, "accountID is invalid")
 	assert(cl.UserAgent, userAgent, t, "userAgent is invalid")
 
 	tok, err := cl.accessTokenGetter()

utils.go

@@ -22,7 +22,7 @@ func getUseParametersList() []string {
 }
 
 func getDisallowedParametersList() []string {
-	return []string{"account_id", "output_format"}
+	return []string{"output_format"}
 }
 
 func ConstructNestedError(message string, err error) error {