From cd957ff9e6fb218441d6e8659ac9034e17aa5a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Fri, 2 Aug 2024 12:12:48 +0200 Subject: [PATCH] Changes after review --- pkg/acceptance/helpers/database_client.go | 16 ++++++++---- ...fier_helpers.go => resource_identifier.go} | 0 ...rs_test.go => resource_identifier_test.go} | 0 .../secondary_database_acceptance_test.go | 19 ++++++-------- .../shared_database_acceptance_test.go | 25 +++++++++++++------ pkg/sdk/identifier_helpers.go | 12 +++++++++ pkg/sdk/identifier_parsers.go | 1 + 7 files changed, 50 insertions(+), 23 deletions(-) rename pkg/helpers/{resource_identifier_helpers.go => resource_identifier.go} (100%) rename pkg/helpers/{resource_identifier_helpers_test.go => resource_identifier_test.go} (100%) diff --git a/pkg/acceptance/helpers/database_client.go b/pkg/acceptance/helpers/database_client.go index 2e9c71cf23..bc657a1fa3 100644 --- a/pkg/acceptance/helpers/database_client.go +++ b/pkg/acceptance/helpers/database_client.go @@ -49,15 +49,21 @@ func (c *DatabaseClient) CreateDatabaseWithOptions(t *testing.T, id sdk.AccountO } func (c *DatabaseClient) DropDatabaseFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() { + t.Helper() + return func() { require.NoError(t, c.DropDatabase(t, id)) } +} + +func (c *DatabaseClient) DropDatabase(t *testing.T, id sdk.AccountObjectIdentifier) error { t.Helper() ctx := context.Background() - return func() { - err := c.client().Drop(ctx, id, &sdk.DropDatabaseOptions{IfExists: sdk.Bool(true)}) - require.NoError(t, err) - err = c.context.client.Sessions.UseSchema(ctx, c.ids.SchemaId()) - require.NoError(t, err) + if err := c.client().Drop(ctx, id, &sdk.DropDatabaseOptions{IfExists: sdk.Bool(true)}); err != nil { + return err + } + if err := c.context.client.Sessions.UseSchema(ctx, c.ids.SchemaId()); err != nil { + return err } + return nil } func (c *DatabaseClient) CreateSecondaryDatabaseWithOptions(t *testing.T, id sdk.AccountObjectIdentifier, externalId sdk.ExternalObjectIdentifier, opts *sdk.CreateSecondaryDatabaseOptions) (*sdk.Database, func()) { diff --git a/pkg/helpers/resource_identifier_helpers.go b/pkg/helpers/resource_identifier.go similarity index 100% rename from pkg/helpers/resource_identifier_helpers.go rename to pkg/helpers/resource_identifier.go diff --git a/pkg/helpers/resource_identifier_helpers_test.go b/pkg/helpers/resource_identifier_test.go similarity index 100% rename from pkg/helpers/resource_identifier_helpers_test.go rename to pkg/helpers/resource_identifier_test.go diff --git a/pkg/resources/secondary_database_acceptance_test.go b/pkg/resources/secondary_database_acceptance_test.go index df289b6e6a..71be403fc7 100644 --- a/pkg/resources/secondary_database_acceptance_test.go +++ b/pkg/resources/secondary_database_acceptance_test.go @@ -21,15 +21,14 @@ func TestAcc_CreateSecondaryDatabase_Basic(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() comment := random.Comment() - _, externalPrimaryId, primaryDatabaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ + primaryDatabase, externalPrimaryId, _ := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ acc.TestClient().Account.GetAccountIdentifier(t), }) t.Cleanup(func() { // TODO(SNOW-1562172): Create a better solution for this type of situations // Have to wait; otherwise the secondary database removal can be not registered yet, // resulting in an error in the cleanup below. - time.Sleep(time.Second) - primaryDatabaseCleanup() + require.Eventually(t, func() bool { return acc.SecondaryTestClient().Database.DropDatabase(t, primaryDatabase.ID()) == nil }, time.Second*5, time.Second) }) newId := acc.TestClient().Ids.RandomAccountObjectIdentifier() @@ -158,15 +157,14 @@ func TestAcc_CreateSecondaryDatabase_complete(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() comment := random.Comment() - _, externalPrimaryId, primaryDatabaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ + primaryDatabase, externalPrimaryId, _ := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ sdk.NewAccountIdentifierFromAccountLocator(acc.Client(t).GetAccountLocator()), }) t.Cleanup(func() { - // TODO(SNOW-1562172: Create a better solution for this type of situations + // TODO(SNOW-1562172): Create a better solution for this type of situations // Have to wait; otherwise the secondary database removal can be not registered yet, // resulting in an error in the cleanup below. - time.Sleep(time.Second) - primaryDatabaseCleanup() + require.Eventually(t, func() bool { return acc.SecondaryTestClient().Database.DropDatabase(t, primaryDatabase.ID()) == nil }, time.Second*5, time.Second) }) externalVolumeId, externalVolumeCleanup := acc.TestClient().ExternalVolume.Create(t) @@ -404,15 +402,14 @@ func TestAcc_CreateSecondaryDatabase_complete(t *testing.T) { func TestAcc_CreateSecondaryDatabase_DataRetentionTimeInDays(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() - _, externalPrimaryId, primaryDatabaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ + primaryDatabase, externalPrimaryId, _ := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ sdk.NewAccountIdentifierFromAccountLocator(acc.Client(t).GetAccountLocator()), }) t.Cleanup(func() { - // TODO(SNOW-1562172: Create a better solution for this type of situations + // TODO(SNOW-1562172): Create a better solution for this type of situations // Have to wait; otherwise the secondary database removal can be not registered yet, // resulting in an error in the cleanup below. - time.Sleep(time.Second) - primaryDatabaseCleanup() + require.Eventually(t, func() bool { return acc.SecondaryTestClient().Database.DropDatabase(t, primaryDatabase.ID()) == nil }, time.Second*5, time.Second) }) accountDataRetentionTimeInDays, err := acc.Client(t).Parameters.ShowAccountParameter(context.Background(), sdk.AccountParameterDataRetentionTimeInDays) diff --git a/pkg/resources/shared_database_acceptance_test.go b/pkg/resources/shared_database_acceptance_test.go index 72f3485c3c..e1e46e37ea 100644 --- a/pkg/resources/shared_database_acceptance_test.go +++ b/pkg/resources/shared_database_acceptance_test.go @@ -25,13 +25,6 @@ func TestAcc_CreateSharedDatabase_Basic(t *testing.T) { newId := acc.TestClient().Ids.RandomAccountObjectIdentifier() newComment := random.Comment() - t.Cleanup(func() { - // TODO(SNOW-1562172: Create a better solution for this type of situations - // It's needed, because we have to wait for Snowflake to "register" the cleanup after this test. - // Without it, the next test would fail. - time.Sleep(time.Second * 60) - }) - var ( accountExternalVolume = new(string) accountCatalog = new(string) @@ -181,6 +174,24 @@ func TestAcc_CreateSharedDatabase_complete(t *testing.T) { "enable_console_output": config.BoolVariable(true), } + // TODO(SNOW-1562172): Create a better solution for this type of situations + // We have to create database from share before the actual test to check if a new share is ready after previous test + // (there's some kind of issue or delay between cleaning up a share and creating a new one right after). + testId := acc.TestClient().Ids.RandomAccountObjectIdentifier() + err := acc.Client(t).Databases.CreateShared(context.Background(), testId, externalShareId, new(sdk.CreateSharedDatabaseOptions)) + require.NoError(t, err) + + require.Eventually(t, func() bool { + database, err := acc.TestClient().Database.Show(t, testId) + if err != nil { + return false + } + // Origin is returned as "" in those cases, because it's not valid sdk.ExternalObjectIdentifier parser sets it as nil. + // Once it turns into valid sdk.ExternalObjectIdentifier, we're ready to proceed with the actual test. + return database.Origin != nil + }, time.Minute, time.Second*6) + acc.TestClient().Database.DropDatabaseFunc(t, testId)() + resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ diff --git a/pkg/sdk/identifier_helpers.go b/pkg/sdk/identifier_helpers.go index c1463fa34f..c7632347da 100644 --- a/pkg/sdk/identifier_helpers.go +++ b/pkg/sdk/identifier_helpers.go @@ -183,6 +183,9 @@ func (i AccountObjectIdentifier) Name() string { } func (i AccountObjectIdentifier) FullyQualifiedName() string { + if i.name == "" { + return "" + } return fmt.Sprintf(`"%v"`, i.name) } @@ -219,6 +222,9 @@ func (i DatabaseObjectIdentifier) Name() string { } func (i DatabaseObjectIdentifier) FullyQualifiedName() string { + if i.name == "" && i.databaseName == "" { + return "" + } return fmt.Sprintf(`"%v"."%v"`, i.databaseName, i.name) } @@ -301,6 +307,9 @@ func (i SchemaObjectIdentifier) DatabaseId() AccountObjectIdentifier { } func (i SchemaObjectIdentifier) FullyQualifiedName() string { + if i.schemaName == "" && i.databaseName == "" && i.name == "" { + return "" + } if len(i.arguments) == 0 { return fmt.Sprintf(`"%v"."%v"."%v"`, i.databaseName, i.schemaName, i.name) } @@ -367,6 +376,9 @@ func (i TableColumnIdentifier) Name() string { } func (i TableColumnIdentifier) FullyQualifiedName() string { + if i.schemaName == "" && i.databaseName == "" && i.tableName == "" && i.columnName == "" { + return "" + } return fmt.Sprintf(`"%v"."%v"."%v"."%v"`, i.databaseName, i.schemaName, i.tableName, i.columnName) } diff --git a/pkg/sdk/identifier_parsers.go b/pkg/sdk/identifier_parsers.go index 44c6ab49fe..584761b0be 100644 --- a/pkg/sdk/identifier_parsers.go +++ b/pkg/sdk/identifier_parsers.go @@ -33,6 +33,7 @@ func ParseIdentifierString(identifier string) ([]string, error) { return nil, err } for _, part := range parts { + // TODO(SNOW-1571674): Remove the validation if strings.Contains(part, `"`) { return nil, fmt.Errorf(`unable to parse identifier: %s, currently identifiers containing double quotes are not supported in the provider`, identifier) }