Refactoring / fixing PR comments

hashicorp · Jul 26, 2017 · 2f586ce · 2f586ce
1 parent 680402f
commit 2f586ce
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 66 deletions.
diff --git a/azurerm/import_arm_dns_mx_record_test.go b/azurerm/import_arm_dns_mx_record_test.go
@@ -1,7 +1,6 @@
 package azurerm
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -12,18 +11,42 @@ func TestAccAzureRMDnsMxRecord_importBasic(t *testing.T) {
 	resourceName := "azurerm_dns_mx_record.test"
 
 	ri := acctest.RandInt()
-	config := fmt.Sprintf(testAccAzureRMDnsMxRecord_basic, ri, ri, ri)
+	config := testAccAzureRMDnsMxRecord_basic(ri)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testCheckAzureRMDnsMxRecordDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: config,
 			},
 
-			resource.TestStep{
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccAzureRMDnsMxRecord_importWithTags(t *testing.T) {
+	resourceName := "azurerm_dns_mx_record.test"
+
+	ri := acctest.RandInt()
+	config := testAccAzureRMDnsMxRecord_withTags(ri)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMDnsMxRecordDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: config,
+			},
+
+			{
 				ResourceName:      resourceName,
 				ImportState:       true,
 				ImportStateVerify: true,

diff --git a/azurerm/resource_arm_dns_mx_record.go b/azurerm/resource_arm_dns_mx_record.go
@@ -22,34 +22,35 @@ func resourceArmDnsMxRecord() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
-			"name": &schema.Schema{
+			"name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
 
-			"resource_group_name": &schema.Schema{
+			"resource_group_name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
 
-			"zone_name": &schema.Schema{
+			"zone_name": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"record": &schema.Schema{
+			"record": {
 				Type:     schema.TypeSet,
 				Required: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"preference": &schema.Schema{
+						"preference": {
+							// TODO: this should become an Int
 							Type:     schema.TypeString,
 							Required: true,
 						},
 
-						"exchange": &schema.Schema{
+						"exchange": {
 							Type:     schema.TypeString,
 							Required: true,
 						},
@@ -58,7 +59,7 @@ func resourceArmDnsMxRecord() *schema.Resource {
 				Set: resourceArmDnsMxRecordHash,
 			},
 
-			"ttl": &schema.Schema{
+			"ttl": {
 				Type:     schema.TypeInt,
 				Required: true,
 			},
@@ -69,31 +70,30 @@ func resourceArmDnsMxRecord() *schema.Resource {
 }
 
 func resourceArmDnsMxRecordCreateOrUpdate(d *schema.ResourceData, meta interface{}) error {
-	dnsClient := meta.(*ArmClient).dnsClient
+	client := meta.(*ArmClient).dnsClient
 
 	name := d.Get("name").(string)
 	resGroup := d.Get("resource_group_name").(string)
 	zoneName := d.Get("zone_name").(string)
 	ttl := int64(d.Get("ttl").(int))
-
 	tags := d.Get("tags").(map[string]interface{})
-	metadata := expandTags(tags)
-
 	records, err := expandAzureRmDnsMxRecords(d)
-	props := dns.RecordSetProperties{
-		Metadata:  metadata,
-		TTL:       &ttl,
-		MxRecords: &records,
+	if err != nil {
+		return err
 	}
 
 	parameters := dns.RecordSet{
-		Name:                &name,
-		RecordSetProperties: &props,
+		Name: &name,
+		RecordSetProperties: &dns.RecordSetProperties{
+			Metadata:  expandTags(tags),
+			TTL:       &ttl,
+			MxRecords: &records,
+		},
 	}
 
-	//last parameter is set to empty to allow updates to records after creation
-	// (per SDK, set it to '*' to prevent updates, all other values are ignored)
-	resp, err := dnsClient.CreateOrUpdate(resGroup, zoneName, name, dns.MX, parameters, "", "")
+	eTag := ""
+	ifNoneMatch := "" // set to empty to allow updates to records after creation
+	resp, err := client.CreateOrUpdate(resGroup, zoneName, name, dns.MX, parameters, eTag, ifNoneMatch)
 	if err != nil {
 		return err
 	}
@@ -108,7 +108,7 @@ func resourceArmDnsMxRecordCreateOrUpdate(d *schema.ResourceData, meta interface
 }
 
 func resourceArmDnsMxRecordRead(d *schema.ResourceData, meta interface{}) error {
-	dnsClient := meta.(*ArmClient).dnsClient
+	client := meta.(*ArmClient).dnsClient
 
 	id, err := parseAzureResourceID(d.Id())
 	if err != nil {
@@ -119,7 +119,7 @@ func resourceArmDnsMxRecordRead(d *schema.ResourceData, meta interface{}) error
 	name := id.Path["MX"]
 	zoneName := id.Path["dnszones"]
 
-	resp, err := dnsClient.Get(resGroup, zoneName, name, dns.MX)
+	resp, err := client.Get(resGroup, zoneName, name, dns.MX)
 	if err != nil {
 		return fmt.Errorf("Error reading DNS MX record %s: %v", name, err)
 	}
@@ -142,7 +142,7 @@ func resourceArmDnsMxRecordRead(d *schema.ResourceData, meta interface{}) error
 }
 
 func resourceArmDnsMxRecordDelete(d *schema.ResourceData, meta interface{}) error {
-	dnsClient := meta.(*ArmClient).dnsClient
+	client := meta.(*ArmClient).dnsClient
 
 	id, err := parseAzureResourceID(d.Id())
 	if err != nil {
@@ -153,7 +153,7 @@ func resourceArmDnsMxRecordDelete(d *schema.ResourceData, meta interface{}) erro
 	name := id.Path["MX"]
 	zoneName := id.Path["dnszones"]
 
-	resp, error := dnsClient.Delete(resGroup, zoneName, name, dns.MX, "")
+	resp, error := client.Delete(resGroup, zoneName, name, dns.MX, "")
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("Error deleting DNS MX Record %s: %+v", name, error)
 	}
@@ -207,6 +207,7 @@ func expandAzureRmDnsMxRecords(d *schema.ResourceData) ([]dns.MxRecord, error) {
 func resourceArmDnsMxRecordHash(v interface{}) int {
 	var buf bytes.Buffer
 	m := v.(map[string]interface{})
+
 	buf.WriteString(fmt.Sprintf("%s-", m["preference"].(string)))
 	buf.WriteString(fmt.Sprintf("%s-", m["exchange"].(string)))