fix: schemas show by id (#2238)

pkg/resources/schema_acceptance_test.go

@@ -1,76 +1,155 @@
 package resources_test
 
 import (
+	"context"
+	"database/sql"
 	"fmt"
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
+	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
+	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"strings"
 	"testing"
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
-	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
+	"github.com/hashicorp/terraform-plugin-testing/config"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/tfversion"
 )
 
 func TestAcc_Schema(t *testing.T) {
-	schemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	comment := "Terraform acceptance test"
 
-	resource.ParallelTest(t, resource.TestCase{
-		Providers:    acc.TestAccProviders(),
-		PreCheck:     func() { acc.TestAccPreCheck(t) },
-		CheckDestroy: nil,
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: testAccCheckSchemaDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: schemaConfig(schemaName, acc.TestDatabaseName),
+				ConfigDirectory: config.TestNameDirectory(),
+				ConfigVariables: map[string]config.Variable{
+					"name":     config.StringVariable(name),
+					"database": config.StringVariable(acc.TestDatabaseName),
+					"comment":  config.StringVariable(comment),
+				},
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_schema.test", "name", schemaName),
+					resource.TestCheckResourceAttr("snowflake_schema.test", "name", name),
 					resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName),
-					resource.TestCheckResourceAttr("snowflake_schema.test", "comment", "Terraform acceptance test"),
-					checkBool("snowflake_schema.test", "is_transient", false), // this is from user_acceptance_test.go
+					resource.TestCheckResourceAttr("snowflake_schema.test", "comment", comment),
+					checkBool("snowflake_schema.test", "is_transient", false),
 					checkBool("snowflake_schema.test", "is_managed", false),
 				),
 			},
 		},
 	})
 }
 
-func TestAcc_SchemaRename(t *testing.T) {
+func TestAcc_Schema_Rename(t *testing.T) {
 	oldSchemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 	newSchemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	comment := "Terraform acceptance test"
 
-	resource.ParallelTest(t, resource.TestCase{
-		Providers:    acc.TestAccProviders(),
-		PreCheck:     func() { acc.TestAccPreCheck(t) },
-		CheckDestroy: nil,
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: testAccCheckSchemaDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: schemaConfig(oldSchemaName, acc.TestDatabaseName),
+				ConfigDirectory: config.TestNameDirectory(),
+				ConfigVariables: map[string]config.Variable{
+					"name":     config.StringVariable(oldSchemaName),
+					"database": config.StringVariable(acc.TestDatabaseName),
+					"comment":  config.StringVariable(comment),
+				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_schema.test", "name", oldSchemaName),
 					resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName),
-					resource.TestCheckResourceAttr("snowflake_schema.test", "comment", "Terraform acceptance test"),
-					checkBool("snowflake_schema.test", "is_transient", false), // this is from user_acceptance_test.go
+					resource.TestCheckResourceAttr("snowflake_schema.test", "comment", comment),
+					checkBool("snowflake_schema.test", "is_transient", false),
 					checkBool("snowflake_schema.test", "is_managed", false),
 				),
 			},
 			{
-				Config: schemaConfig(newSchemaName, acc.TestDatabaseName),
+				ConfigDirectory: config.TestNameDirectory(),
+				ConfigVariables: map[string]config.Variable{
+					"name":     config.StringVariable(newSchemaName),
+					"database": config.StringVariable(acc.TestDatabaseName),
+					"comment":  config.StringVariable(comment),
+				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_schema.test", "name", newSchemaName),
 					resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName),
-					resource.TestCheckResourceAttr("snowflake_schema.test", "comment", "Terraform acceptance test"),
-					checkBool("snowflake_schema.test", "is_transient", false), // this is from user_acceptance_test.go
+					resource.TestCheckResourceAttr("snowflake_schema.test", "comment", comment),
+					checkBool("snowflake_schema.test", "is_transient", false),
 					checkBool("snowflake_schema.test", "is_managed", false),
 				),
 			},
 		},
 	})
 }
 
-func schemaConfig(schemaName string, databaseName string) string {
-	return fmt.Sprintf(`
-resource "snowflake_schema" "test" {
-	name = "%v"
-	database = "%s"
-	comment = "Terraform acceptance test"
+// TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2209 issue.
+func TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases(t *testing.T) {
+	name := "test_schema"
+	// It seems like Snowflake orders the output of SHOW command based on names, so they do matter
+	newDatabaseName := "SELDQBXEKC"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: testAccCheckSchemaDestroy,
+		Steps: []resource.TestStep{
+			{
+				ConfigDirectory: config.TestStepDirectory(),
+				ConfigVariables: map[string]config.Variable{
+					"name":     config.StringVariable(name),
+					"database": config.StringVariable(acc.TestDatabaseName),
+				},
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_schema.test", "name", name),
+					resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName),
+				),
+			},
+			{
+				ConfigDirectory: config.TestStepDirectory(),
+				ConfigVariables: map[string]config.Variable{
+					"name":         config.StringVariable(name),
+					"database":     config.StringVariable(acc.TestDatabaseName),
+					"new_database": config.StringVariable(newDatabaseName),
+				},
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_schema.test", "name", name),
+					resource.TestCheckResourceAttr("snowflake_schema.test", "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr("snowflake_schema.test_2", "name", name),
+					resource.TestCheckResourceAttr("snowflake_schema.test_2", "database", newDatabaseName),
+				),
+			},
+		},
+	})
 }
-`, schemaName, databaseName)
+
+func testAccCheckSchemaDestroy(s *terraform.State) error {
+	db := acc.TestAccProvider.Meta().(*sql.DB)
+	client := sdk.NewClientFromDB(db)
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "snowflake_schema" {
+			continue
+		}
+		ctx := context.Background()
+		id := sdk.NewDatabaseObjectIdentifier(rs.Primary.Attributes["database"], rs.Primary.Attributes["name"])
+		schema, err := client.Schemas.ShowByID(ctx, id)
+		if err == nil {
+			return fmt.Errorf("schema %v still exists", schema.Name)
+		}
+	}
+	return nil
 }

pkg/resources/testdata/TestAcc_Schema/test.tf

@@ -0,0 +1,5 @@
+resource "snowflake_schema" "test" {
+  name = var.name
+  database = var.database
+  comment = var.comment
+}

pkg/resources/testdata/TestAcc_Schema/variables.tf

@@ -0,0 +1,11 @@
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}
+
+variable "comment" {
+  type = string
+}

pkg/resources/testdata/TestAcc_Schema_Rename/test.tf

@@ -0,0 +1,5 @@
+resource "snowflake_schema" "test" {
+  name = var.name
+  database = var.database
+  comment = var.comment
+}

pkg/resources/testdata/TestAcc_Schema_Rename/variables.tf

@@ -0,0 +1,11 @@
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}
+
+variable "comment" {
+  type = string
+}

pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/test.tf

@@ -0,0 +1,4 @@
+resource "snowflake_schema" "test" {
+  name = var.name
+  database = var.database
+}

pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/1/variables.tf

@@ -0,0 +1,7 @@
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}

pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/test.tf

@@ -0,0 +1,13 @@
+resource "snowflake_schema" "test" {
+  name = var.name
+  database = var.database
+}
+
+resource "snowflake_database" "test" {
+  name = var.new_database
+}
+
+resource "snowflake_schema" "test_2" {
+  name = var.name
+  database = snowflake_database.test.name
+}

pkg/resources/testdata/TestAcc_Schema_TwoSchemasWithTheSameNameOnDifferentDatabases/2/variables.tf

@@ -0,0 +1,11 @@
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}
+
+variable "new_database" {
+  type = string
+}

pkg/sdk/schemas.go

@@ -389,6 +389,10 @@ func (v *schemas) Show(ctx context.Context, opts *ShowSchemaOptions) ([]Schema,
 
 func (v *schemas) ShowByID(ctx context.Context, id DatabaseObjectIdentifier) (*Schema, error) {
 	schemas, err := v.client.Schemas.Show(ctx, &ShowSchemaOptions{
+		In: &SchemaIn{
+			Database: Bool(true),
+			Name:     NewAccountObjectIdentifier(id.DatabaseName()),
+		},
 		Like: &Like{
 			Pattern: String(id.Name()),
 		},