Standardize resource name extraction from self_link/uri (#939)

* Standardize resource name extraction from self_link/uri

* remove rebase artifact

* style improvement

* Fix merge issue

google/compute_operation.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"log"
-	"strings"
 	"time"
 
 	"github.com/hashicorp/terraform/helper/resource"
@@ -25,12 +24,10 @@ func (w *ComputeOperationWaiter) RefreshFunc() resource.StateRefreshFunc {
 		var err error
 
 		if w.Op.Zone != "" {
-			zoneURLParts := strings.Split(w.Op.Zone, "/")
-			zone := zoneURLParts[len(zoneURLParts)-1]
+			zone := GetResourceNameFromSelfLink(w.Op.Zone)
 			op, err = w.Service.ZoneOperations.Get(w.Project, zone, w.Op.Name).Do()
 		} else if w.Op.Region != "" {
-			regionURLParts := strings.Split(w.Op.Region, "/")
-			region := regionURLParts[len(regionURLParts)-1]
+			region := GetResourceNameFromSelfLink(w.Op.Region)
 			op, err = w.Service.RegionOperations.Get(w.Project, region, w.Op.Name).Do()
 		} else {
 			op, err = w.Service.GlobalOperations.Get(w.Project, w.Op.Name).Do()

google/resource_cloudfunctions_function.go

@@ -351,9 +351,9 @@ func resourceCloudFunctionsRead(d *schema.ResourceData, meta interface{}) error
 		switch function.EventTrigger.EventType {
 		// From https://github.com/google/google-api-go-client/blob/master/cloudfunctions/v1/cloudfunctions-gen.go#L335
 		case "providers/cloud.pubsub/eventTypes/topic.publish":
-			d.Set("trigger_topic", extractLastResourceFromUri(function.EventTrigger.Resource))
+			d.Set("trigger_topic", GetResourceNameFromSelfLink(function.EventTrigger.Resource))
 		case "providers/cloud.storage/eventTypes/object.change":
-			d.Set("trigger_bucket", extractLastResourceFromUri(function.EventTrigger.Resource))
+			d.Set("trigger_bucket", GetResourceNameFromSelfLink(function.EventTrigger.Resource))
 		}
 	}
 	d.Set("region", cloudFuncId.Region)

google/resource_compute_disk.go

@@ -361,10 +361,9 @@ func resourceComputeDiskDelete(d *schema.ResourceData, meta interface{}) error {
 			}
 			for _, disk := range i.Disks {
 				if disk.Source == self {
-					zoneParts := strings.Split(i.Zone, "/")
 					detachCalls = append(detachCalls, detachArgs{
 						project:    project,
-						zone:       zoneParts[len(zoneParts)-1],
+						zone:       GetResourceNameFromSelfLink(i.Zone),
 						instance:   i.Name,
 						deviceName: disk.DeviceName,
 					})

google/resource_compute_instance.go

@@ -761,10 +761,7 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 	}
 
 	d.Set("can_ip_forward", instance.CanIpForward)
-
-	machineTypeResource := strings.Split(instance.MachineType, "/")
-	machineType := machineTypeResource[len(machineTypeResource)-1]
-	d.Set("machine_type", machineType)
+	d.Set("machine_type", GetResourceNameFromSelfLink(instance.MachineType))
 
 	// Set the networks
 	// Use the first external IP found for the default connection info.

google/resource_compute_instance_group_manager_test.go

@@ -373,8 +373,7 @@ func testAccCheckInstanceGroupManagerUpdated(n string, size int64, targetPools [
 
 		tpNames := make([]string, 0, len(manager.TargetPools))
 		for _, targetPool := range manager.TargetPools {
-			targetPoolParts := strings.Split(targetPool, "/")
-			tpNames = append(tpNames, targetPoolParts[len(targetPoolParts)-1])
+			tpNames = append(tpNames, GetResourceNameFromSelfLink(targetPool))
 		}
 
 		sort.Strings(tpNames)
@@ -490,7 +489,7 @@ func testAccCheckInstanceGroupManagerTemplateTags(n string, tags []string) resou
 
 		// check that the instance template updated
 		instanceTemplate, err := config.clientCompute.InstanceTemplates.Get(
-			config.Project, resourceSplitter(manager.InstanceTemplate)).Do()
+			config.Project, GetResourceNameFromSelfLink(manager.InstanceTemplate)).Do()
 		if err != nil {
 			return fmt.Errorf("Error reading instance template: %s", err)
 		}
@@ -996,9 +995,3 @@ resource "google_compute_autoscaler" "foobar" {
 }
 `, template, target, igm, hck, autoscaler)
 }
-
-func resourceSplitter(resource string) string {
-	splits := strings.Split(resource, "/")
-
-	return splits[len(splits)-1]
-}

google/resource_compute_instance_migrate.go

@@ -235,8 +235,7 @@ func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terrafo
 
 			for _, disk := range instance.Disks {
 				if disk.Boot {
-					sourceUrl := strings.Split(disk.Source, "/")
-					is.Attributes["boot_disk.0.source"] = sourceUrl[len(sourceUrl)-1]
+					is.Attributes["boot_disk.0.source"] = GetResourceNameFromSelfLink(disk.Source)
 					is.Attributes["boot_disk.0.device_name"] = disk.DeviceName
 					break
 				}
@@ -443,8 +442,7 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a
 	if err != nil {
 		return nil, err
 	}
-	imgParts := strings.Split(img, "/projects/")
-	canonicalImage := imgParts[len(imgParts)-1]
+	canonicalImage := GetResourceNameFromSelfLink(img)
 
 	for i, disk := range instance.Disks {
 		if disk.Boot == true || disk.Type == "SCRATCH" {
@@ -453,8 +451,7 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a
 		}
 		if disk.AutoDelete == autoDelete {
 			// Read the disk to check if its image matches
-			sourceUrl := strings.Split(disk.Source, "/")
-			fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]]
+			fullDisk := allDisks[GetResourceNameFromSelfLink(disk.Source)]
 			sourceImage, err := getRelativePath(fullDisk.SourceImage)
 			if err != nil {
 				return nil, err
@@ -479,8 +476,7 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a
 		}
 		if disk.AutoDelete == autoDelete {
 			// Read the disk to check if its image matches
-			sourceUrl := strings.Split(disk.Source, "/")
-			fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]]
+			fullDisk := allDisks[GetResourceNameFromSelfLink(disk.Source)]
 			sourceImage, err := getRelativePath(fullDisk.SourceImage)
 			if err != nil {
 				return nil, err

google/resource_compute_instance_template.go

@@ -2,7 +2,6 @@ package google
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
@@ -640,8 +639,7 @@ func flattenDisks(disks []*computeBeta.AttachedDisk, d *schema.ResourceData) []m
 		if disk.InitializeParams != nil {
 			var source_img = fmt.Sprintf("disk.%d.source_image", i)
 			if d.Get(source_img) == nil || d.Get(source_img) == "" {
-				sourceImageUrl := strings.Split(disk.InitializeParams.SourceImage, "/")
-				diskMap["source_image"] = sourceImageUrl[len(sourceImageUrl)-1]
+				diskMap["source_image"] = GetResourceNameFromSelfLink(disk.InitializeParams.SourceImage)
 			} else {
 				diskMap["source_image"] = d.Get(source_img)
 			}

google/resource_compute_instance_test.go

@@ -1172,8 +1172,7 @@ func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.In
 				}
 			} else {
 				if disk.DiskEncryptionKey != nil {
-					sourceUrl := strings.Split(disk.Source, "/")
-					expectedKey := diskNameToEncryptionKey[sourceUrl[len(sourceUrl)-1]].Sha256
+					expectedKey := diskNameToEncryptionKey[GetResourceNameFromSelfLink(disk.Source)].Sha256
 					if disk.DiskEncryptionKey.Sha256 != expectedKey {
 						return fmt.Errorf("Disk %d has unexpected encryption key in GCP.\nExpected: %s\nActual: %s", i, expectedKey, disk.DiskEncryptionKey.Sha256)
 					}
@@ -1186,8 +1185,7 @@ func testAccCheckComputeInstanceDiskEncryptionKey(n string, instance *compute.In
 			return fmt.Errorf("Error converting value of attached_disk.#")
 		}
 		for i := 0; i < numAttachedDisks; i++ {
-			diskSourceUrl := strings.Split(rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.source", i)], "/")
-			diskName := diskSourceUrl[len(diskSourceUrl)-1]
+			diskName := GetResourceNameFromSelfLink(rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.source", i)])
 			encryptionKey := rs.Primary.Attributes[fmt.Sprintf("attached_disk.%d.disk_encryption_key_sha256", i)]
 			if key, ok := diskNameToEncryptionKey[diskName]; ok {
 				expectedEncryptionKey := key.Sha256

google/resource_compute_region_instance_group_manager_test.go

@@ -306,8 +306,7 @@ func testAccCheckRegionInstanceGroupManagerUpdated(n string, size int64, targetP
 
 		tpNames := make([]string, 0, len(manager.TargetPools))
 		for _, targetPool := range manager.TargetPools {
-			targetPoolParts := strings.Split(targetPool, "/")
-			tpNames = append(tpNames, targetPoolParts[len(targetPoolParts)-1])
+			tpNames = append(tpNames, GetResourceNameFromSelfLink(targetPool))
 		}
 
 		sort.Strings(tpNames)
@@ -423,7 +422,7 @@ func testAccCheckRegionInstanceGroupManagerTemplateTags(n string, tags []string)
 
 		// check that the instance template updated
 		instanceTemplate, err := config.clientCompute.InstanceTemplates.Get(
-			config.Project, resourceSplitter(manager.InstanceTemplate)).Do()
+			config.Project, GetResourceNameFromSelfLink(manager.InstanceTemplate)).Do()
 		if err != nil {
 			return fmt.Errorf("Error reading instance template: %s", err)
 		}

google/resource_compute_target_pool.go

@@ -400,7 +400,6 @@ func resourceComputeTargetPoolRead(d *schema.ResourceData, meta interface{}) err
 		return handleNotFoundError(err, d, fmt.Sprintf("Target Pool %q", d.Get("name").(string)))
 	}
 
-	regionUrl := strings.Split(tpool.Region, "/")
 	d.Set("self_link", tpool.SelfLink)
 	d.Set("backup_pool", tpool.BackupPool)
 	d.Set("description", tpool.Description)
@@ -412,7 +411,7 @@ func resourceComputeTargetPoolRead(d *schema.ResourceData, meta interface{}) err
 		d.Set("instances", nil)
 	}
 	d.Set("name", tpool.Name)
-	d.Set("region", regionUrl[len(regionUrl)-1])
+	d.Set("region", GetResourceNameFromSelfLink(tpool.Region))
 	d.Set("session_affinity", tpool.SessionAffinity)
 	d.Set("project", project)
 	return nil

google/resource_dataproc_cluster.go

@@ -570,7 +570,7 @@ func expandInstanceGroupConfig(cfg map[string]interface{}) *dataproc.InstanceGro
 		icg.NumInstances = int64(v.(int))
 	}
 	if v, ok := cfg["machine_type"]; ok {
-		icg.MachineTypeUri = extractLastResourceFromUri(v.(string))
+		icg.MachineTypeUri = GetResourceNameFromSelfLink(v.(string))
 	}
 
 	if dc, ok := cfg["disk_config"]; ok {
@@ -750,7 +750,7 @@ func flattenGceClusterConfig(d *schema.ResourceData, gcc *dataproc.GceClusterCon
 	gceConfig := map[string]interface{}{
 		"tags":             gcc.Tags,
 		"service_account":  gcc.ServiceAccount,
-		"zone":             extractLastResourceFromUri(gcc.ZoneUri),
+		"zone":             GetResourceNameFromSelfLink(gcc.ZoneUri),
 		"internal_ip_only": gcc.InternalIpOnly,
 	}
 
@@ -791,7 +791,7 @@ func flattenInstanceGroupConfig(d *schema.ResourceData, icg *dataproc.InstanceGr
 
 	if icg != nil {
 		data["num_instances"] = icg.NumInstances
-		data["machine_type"] = extractLastResourceFromUri(icg.MachineTypeUri)
+		data["machine_type"] = GetResourceNameFromSelfLink(icg.MachineTypeUri)
 		data["instance_names"] = icg.InstanceNames
 		if icg.DiskConfig != nil {
 			disk["boot_disk_size_gb"] = icg.DiskConfig.BootDiskSizeGb

google/resource_dataproc_cluster_test.go

@@ -582,13 +582,13 @@ func validateDataprocCluster_withConfigOverrides(n string, cluster *dataproc.Clu
 			{"cluster_config.0.master_config.0.num_instances", "3", strconv.Itoa(int(cluster.Config.MasterConfig.NumInstances))},
 			{"cluster_config.0.master_config.0.disk_config.0.boot_disk_size_gb", "10", strconv.Itoa(int(cluster.Config.MasterConfig.DiskConfig.BootDiskSizeGb))},
 			{"cluster_config.0.master_config.0.disk_config.0.num_local_ssds", "0", strconv.Itoa(int(cluster.Config.MasterConfig.DiskConfig.NumLocalSsds))},
-			{"cluster_config.0.master_config.0.machine_type", "n1-standard-1", extractLastResourceFromUri(cluster.Config.MasterConfig.MachineTypeUri)},
+			{"cluster_config.0.master_config.0.machine_type", "n1-standard-1", GetResourceNameFromSelfLink(cluster.Config.MasterConfig.MachineTypeUri)},
 			{"cluster_config.0.master_config.0.instance_names.#", "3", strconv.Itoa(len(cluster.Config.MasterConfig.InstanceNames))},
 
 			{"cluster_config.0.worker_config.0.num_instances", "3", strconv.Itoa(int(cluster.Config.WorkerConfig.NumInstances))},
 			{"cluster_config.0.worker_config.0.disk_config.0.boot_disk_size_gb", "11", strconv.Itoa(int(cluster.Config.WorkerConfig.DiskConfig.BootDiskSizeGb))},
 			{"cluster_config.0.worker_config.0.disk_config.0.num_local_ssds", "1", strconv.Itoa(int(cluster.Config.WorkerConfig.DiskConfig.NumLocalSsds))},
-			{"cluster_config.0.worker_config.0.machine_type", "n1-standard-1", extractLastResourceFromUri(cluster.Config.WorkerConfig.MachineTypeUri)},
+			{"cluster_config.0.worker_config.0.machine_type", "n1-standard-1", GetResourceNameFromSelfLink(cluster.Config.WorkerConfig.MachineTypeUri)},
 			{"cluster_config.0.worker_config.0.instance_names.#", "3", strconv.Itoa(len(cluster.Config.WorkerConfig.InstanceNames))},
 
 			{"cluster_config.0.preemptible_worker_config.0.num_instances", "1", strconv.Itoa(int(cluster.Config.SecondaryWorkerConfig.NumInstances))},

google/resource_spanner_database_test.go

@@ -215,8 +215,7 @@ func testAccCheckSpannerDatabaseExists(n string, instance *spanner.Database) res
 			return err
 		}
 
-		fName := extractInstanceNameFromUri(found.Name)
-		if fName != id.Database {
+		if fName := GetResourceNameFromSelfLink(found.Name); fName != id.Database {
 			return fmt.Errorf("Spanner database %s not found, found %s instead", id.Database, fName)
 		}
 

google/resource_spanner_instance.go

@@ -165,7 +165,7 @@ func resourceSpannerInstanceRead(d *schema.ResourceData, meta interface{}) error
 		return handleNotFoundError(err, d, fmt.Sprintf("Spanner instance %s", id.terraformId()))
 	}
 
-	d.Set("config", extractInstanceConfigFromUri(instance.Config))
+	d.Set("config", GetResourceNameFromSelfLink(instance.Config))
 	d.Set("labels", instance.Labels)
 	d.Set("display_name", instance.DisplayName)
 	d.Set("num_nodes", instance.NodeCount)
@@ -271,14 +271,6 @@ func buildSpannerInstanceId(d *schema.ResourceData, config *Config) (*spannerIns
 	}, nil
 }
 
-func extractInstanceConfigFromUri(configUri string) string {
-	return extractLastResourceFromUri(configUri)
-}
-
-func extractInstanceNameFromUri(nameUri string) string {
-	return extractLastResourceFromUri(nameUri)
-}
-
 func genSpannerInstanceName() string {
 	return resource.PrefixedUniqueId("tfgen-spanid-")[:30]
 }

google/resource_spanner_instance_test.go

@@ -17,30 +17,6 @@ import (
 
 // Unit Tests
 
-func TestExtractInstanceConfigFromUri_withFullPath(t *testing.T) {
-	actual := extractInstanceConfigFromUri("projects/project123/instanceConfigs/conf987")
-	expected := "conf987"
-	expectEquals(t, expected, actual)
-}
-
-func TestExtractInstanceConfigFromUri_withNoPath(t *testing.T) {
-	actual := extractInstanceConfigFromUri("conf987")
-	expected := "conf987"
-	expectEquals(t, expected, actual)
-}
-
-func TestExtractInstanceNameFromUri_withFullPath(t *testing.T) {
-	actual := extractInstanceNameFromUri("projects/project123/instances/instance456")
-	expected := "instance456"
-	expectEquals(t, expected, actual)
-}
-
-func TestExtractInstanceNameFromUri_withNoPath(t *testing.T) {
-	actual := extractInstanceConfigFromUri("instance456")
-	expected := "instance456"
-	expectEquals(t, expected, actual)
-}
-
 func TestSpannerInstanceId_instanceUri(t *testing.T) {
 	id := spannerInstanceId{
 		Project:  "project123",
@@ -308,8 +284,8 @@ func testAccCheckSpannerInstanceExists(n string, instance *spanner.Instance) res
 			return err
 		}
 
-		fName := extractInstanceNameFromUri(found.Name)
-		if fName != extractInstanceNameFromUri(rs.Primary.ID) {
+		fName := GetResourceNameFromSelfLink(found.Name)
+		if fName != GetResourceNameFromSelfLink(rs.Primary.ID) {
 			return fmt.Errorf("Spanner instance %s not found, found %s instead", rs.Primary.ID, fName)
 		}
 

google/self_link_helpers.go

@@ -31,12 +31,12 @@ func compareSelfLinkRelativePaths(k, old, new string, d *schema.ResourceData) bo
 // Use this method when the field accepts either a name or a self_link referencing a resource.
 // The value we store (i.e. `old` in this method), must be a self_link.
 func compareSelfLinkOrResourceName(k, old, new string, d *schema.ResourceData) bool {
-	oldParts := strings.Split(old, "/") // always a self_link
 	newParts := strings.Split(new, "/")
 
 	if len(newParts) == 1 {
-		// The `new` string is a name
-		if oldParts[len(oldParts)-1] == newParts[0] {
+		// `new` is a name
+		// `old` is always a self_link
+		if GetResourceNameFromSelfLink(old) == newParts[0] {
 			return true
 		}
 	}

google/self_link_helpers_test.go

@@ -65,3 +65,24 @@ func TestCompareSelfLinkOrResourceName(t *testing.T) {
 		}
 	}
 }
+
+func TestGetResourceNameFromSelfLink(t *testing.T) {
+	cases := map[string]struct {
+		SelfLink, ExpectedName string
+	}{
+		"name is extracted from self_link": {
+			SelfLink:     "http://something.com/one/two/three",
+			ExpectedName: "three",
+		},
+		"name is returned if the self_link only contains the name": {
+			SelfLink:     "resource_name",
+			ExpectedName: "resource_name",
+		},
+	}
+
+	for tn, tc := range cases {
+		if n := GetResourceNameFromSelfLink(tc.SelfLink); n != tc.ExpectedName {
+			t.Errorf("%s: expected resource name %q; got %q", tn, tc.ExpectedName, n)
+		}
+	}
+}

google/utils.go

@@ -172,8 +172,7 @@ func isConflictError(err error) bool {
 }
 
 func linkDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
-	parts := strings.Split(old, "/")
-	if parts[len(parts)-1] == new {
+	if GetResourceNameFromSelfLink(old) == new {
 		return true
 	}
 	return false
@@ -267,11 +266,6 @@ func convertAndMapStringArr(ifaceArr []interface{}, f func(string) string) []str
 	return arr
 }
 
-func extractLastResourceFromUri(uri string) string {
-	rUris := strings.Split(uri, "/")
-	return rUris[len(rUris)-1]
-}
-
 func convertStringArrToInterface(strs []string) []interface{} {
 	arr := make([]interface{}, len(strs))
 	for i, str := range strs {

google/utils_test.go

@@ -50,22 +50,6 @@ func TestConvertStringMap(t *testing.T) {
 	}
 }
 
-func TestExtractLastResourceFromUri_withUrl(t *testing.T) {
-	actual := extractLastResourceFromUri("http://something.com/one/two/three")
-	expected := "three"
-	if actual != expected {
-		t.Fatalf("Expected %s, but got %s", expected, actual)
-	}
-}
-
-func TestExtractLastResourceFromUri_WithStaticValue(t *testing.T) {
-	actual := extractLastResourceFromUri("three")
-	expected := "three"
-	if actual != expected {
-		t.Fatalf("Expected %s, but got %s", expected, actual)
-	}
-}
-
 func TestIpCidrRangeDiffSuppress(t *testing.T) {
 	cases := map[string]struct {
 		Old, New          string