Skip to content

Commit

Permalink
Changes after review
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jcieslak committed Aug 2, 2024
1 parent b71b174 commit 98cb9b8
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 29 deletions.
16 changes: 11 additions & 5 deletions pkg/acceptance/helpers/database_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
File renamed without changes.
File renamed without changes.
25 changes: 8 additions & 17 deletions pkg/resources/secondary_database_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,12 @@ 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()
Expand Down Expand Up @@ -158,15 +155,12 @@ 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
// 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()
// TODO(SNOW-1562172): Create a better solution for this type of situations
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)
Expand Down Expand Up @@ -404,15 +398,12 @@ 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
// 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()
// TODO(SNOW-1562172): Create a better solution for this type of situations
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)
Expand Down
25 changes: 18 additions & 7 deletions pkg/resources/shared_database_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 test database from share before the actual test to check if the newly created 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 "<revoked>" 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{
Expand Down
12 changes: 12 additions & 0 deletions pkg/sdk/identifier_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ func (i AccountObjectIdentifier) Name() string {
}

func (i AccountObjectIdentifier) FullyQualifiedName() string {
if i.name == "" {
return ""
}
return fmt.Sprintf(`"%v"`, i.name)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions pkg/sdk/identifier_parsers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 98cb9b8

Please sign in to comment.