Fix zone record cache lookups when prefetch is enabled (#206)

This Pull Request implements the following updates:

- Corrects the method by which the prefetch configuration flag is loaded from the environment.
- Introduces concurrent read/write locking for the cache to prevent panics during simultaneous map writes.
- Adjusts the logic for searching zone records in the cache, utilizing the normalized content value rather than the initially configured value.

CHANGELOG.md

@@ -6,6 +6,12 @@ ENHANCEMENTS:
 
 - **Update Resource:** `dnsimple_domain_delegation` now has the trailing dot removed from the `name_servers` attribute entries. This is to align with the API and avoid perma diffs. (dnsimple/terraform-provider-dnsimple#203)
 
+BUG FIXES:
+
+- Corrects the method by which the prefetch configuration flag is loaded from the environment. (#206)
+- Introduces concurrent read/write locking for the cache to prevent panics during simultaneous map writes. (#206)
+- Adjusts the logic for searching zone records in the cache, utilizing the normalized content value rather than the initially configured value. (#206)
+
 NOTES:
 
 - This release is no longer compatible with Terraform versions < 1.3. This is due to the new protocol changes in the underlying terraform framework. If you are using Terraform 1.3 or later, you should be unaffected by this change.

internal/framework/common/zone_record_cache.go

@@ -2,22 +2,31 @@ package common
 
 import (
 	"context"
+	"sync"
 
 	"github.com/dnsimple/dnsimple-go/dnsimple"
 )
 
+var zoneRecordCacheMutex = &sync.RWMutex{}
+
 type ZoneRecordCache map[string][]dnsimple.ZoneRecord
 
 func NewZoneRecordCache() ZoneRecordCache {
 	return make(ZoneRecordCache)
 }
 
 func (c ZoneRecordCache) Get(zoneName string) ([]dnsimple.ZoneRecord, bool) {
+	zoneRecordCacheMutex.RLock()
+	defer zoneRecordCacheMutex.RUnlock()
+
 	records, ok := c[zoneName]
 	return records, ok
 }
 
 func (c ZoneRecordCache) Set(zoneName string, records []dnsimple.ZoneRecord) {
+	zoneRecordCacheMutex.Lock()
+	defer zoneRecordCacheMutex.Unlock()
+
 	c[zoneName] = records
 }
 

internal/framework/provider/provider.go

@@ -132,11 +132,9 @@ func (p *DnsimpleProvider) Configure(ctx context.Context, req provider.Configure
 		sandbox = data.Sandbox.ValueBool()
 	}
 
-	if data.Prefetch.IsNull() {
+	if data.Prefetch.IsNull() || data.Prefetch.IsUnknown() {
 		prefetchValue := utils.GetDefaultFromEnv("DNSIMPLE_PREFETCH", "")
-		if prefetchValue == "" {
-			prefetch = false
-		}
+		prefetch = prefetchValue != ""
 	} else {
 		prefetch = data.Prefetch.ValueBool()
 	}

internal/framework/resources/zone_record_resource.go

@@ -212,7 +212,7 @@ func (r *ZoneRecordResource) Read(ctx context.Context, req resource.ReadRequest,
 			}
 		}
 
-		cacheRecord, ok := r.config.ZoneRecordCache.Find(data.ZoneName.ValueString(), data.Name.ValueString(), data.Type.ValueString(), data.Value.ValueString())
+		cacheRecord, ok := r.config.ZoneRecordCache.Find(data.ZoneName.ValueString(), data.Name.ValueString(), data.Type.ValueString(), data.ValueNormalized.ValueString())
 		if !ok {
 			resp.Diagnostics.AddError(
 				"record not found",

internal/framework/resources/zone_record_resource_test.go

@@ -146,7 +146,10 @@ func TestAccZoneRecordResourceWithPrefetch(t *testing.T) {
 	resourceName := "dnsimple_zone_record.test"
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { test_utils.TestAccPreCheck(t) },
+		PreCheck: func() {
+			test_utils.TestAccPreCheck(t)
+			t.Setenv("DNSIMPLE_PREFETCH", "1")
+		},
 		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
 		CheckDestroy:             testAccCheckZoneRecordResourceDestroy,
 		Steps: []resource.TestStep{
@@ -165,6 +168,32 @@ func TestAccZoneRecordResourceWithPrefetch(t *testing.T) {
 	})
 }
 
+func TestAccZoneRecordResource_Prefetch_Normalized(t *testing.T) {
+	domainName := os.Getenv("DNSIMPLE_DOMAIN")
+	resourceName := "dnsimple_zone_record.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			test_utils.TestAccPreCheck(t)
+			t.Setenv("DNSIMPLE_PREFETCH", "1")
+		},
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		CheckDestroy:             testAccCheckZoneRecordResourceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccZoneRecordResourceNormalizedContentConfig(domainName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "zone_name", domainName),
+					resource.TestCheckResourceAttr(resourceName, "qualified_name", "terraform."+domainName),
+					resource.TestCheckResourceAttr(resourceName, "ttl", "3600"),
+					resource.TestCheckResourceAttrSet(resourceName, "id"),
+				),
+			},
+			// Delete testing automatically occurs in TestCase
+		},
+	})
+}
+
 func TestAccZoneRecordResource_Prefetch_ForEach(t *testing.T) {
 	// Issue: https://github.com/dnsimple/terraform-provider-dnsimple/issues/80
 	// This test is to ensure that the prefetch behaviour is deterministic
@@ -177,7 +206,7 @@ func TestAccZoneRecordResource_Prefetch_ForEach(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck: func() {
 			test_utils.TestAccPreCheck(t)
-			os.Setenv("PREFETCH", "1")
+			t.Setenv("DNSIMPLE_PREFETCH", "1")
 		},
 		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
@@ -312,6 +341,17 @@ resource "dnsimple_zone_record" "test" {
 }`, domainName)
 }
 
+func testAccZoneRecordResourceNormalizedContentConfig(domainName string) string {
+	return fmt.Sprintf(`
+resource "dnsimple_zone_record" "test" {
+	zone_name = %[1]q
+
+	name = "terraform"
+	value = "v=DMARC1; p=reject; pct=100; mailto:[email protected]; aspf=r;"
+	type = "TXT"
+}`, domainName)
+}
+
 func testAccZoneRecordResourceStandardConfig(domainName string) string {
 	return fmt.Sprintf(`
 resource "dnsimple_zone_record" "test" {