Update how resource lookup is done to account for duplicates

bundle/resources/completion.go

@@ -1,39 +1,16 @@
 package resources
 
-import (
+import "github.com/databricks/cli/bundle"
-	"github.com/databricks/cli/bundle"
-	"golang.org/x/exp/maps"
-)
 
-func CompletionMap(b *bundle.Bundle) map[string]string {
+// Completions returns the same as [References] except
-	out := make(map[string]string)
+// that every key maps directly to a single reference.
-	keyOnly, keyWithType := Keys(b)
+func Completions(b *bundle.Bundle) map[string]Reference {
-
+	out := make(map[string]Reference)
-	// Keep track of resources we have seen by their fully qualified key.
+	for k, refs := range References(b) {
-	seen := make(map[string]bool)
+		if len(refs) != 1 {
-
-	// First add resources that can be identified by key alone.
-	for k, v := range keyOnly {
-		// Invariant: len(v) >= 1. See [ResourceKeys].
-		if len(v) == 1 {
-			seen[v[0].key] = true
-			out[k] = v[0].resource.GetName()
-		}
-	}
-
-	// Then add resources that can only be identified by their type and key.
-	for k, v := range keyWithType {
-		// Invariant: len(v) == 1. See [ResourceKeys].
-		_, ok := seen[v[0].key]
-		if ok {
 			continue
 		}
-		out[k] = v[0].resource.GetName()
+		out[k] = refs[0]
 	}
-
 	return out
 }
-
-func Completions(b *bundle.Bundle) []string {
-	return maps.Keys(CompletionMap(b))
-}

bundle/resources/completion_test.go

@@ -1 +1,37 @@
 package resources
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/databricks-sdk-go/service/jobs"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCompletions(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"foo": {},
+					"bar": {
+						JobSettings: &jobs.JobSettings{
+							Name: "Bar job",
+						},
+					},
+				},
+				Pipelines: map[string]*resources.Pipeline{
+					"foo": {},
+				},
+			},
+		},
+	}
+
+	// Test that this skips duplicates and only includes unambiguous completions.
+	out := Completions(b)
+	if assert.Len(t, out, 1) {
+		assert.Contains(t, out, "bar")
+	}
+}

bundle/resources/keys.go

@@ -1,64 +0,0 @@
-package resources
-
-import (
-	"fmt"
-
-	"github.com/databricks/cli/bundle"
-	"github.com/databricks/cli/bundle/config"
-)
-
-type pair struct {
-	key      string
-	resource config.ConfigResource
-}
-
-// lookup maps identifiers to a list of resources that match that identifier.
-// The list can have more than 1 entry if resources of different types use the
-// same key. When this happens, the user should disambiguate between them.
-type lookup map[string][]pair
-
-// Keys computes maps that index resources by their key (e.g. `my_job`) and by their key
-// prefixed by their type (e.g. `jobs.my_job`). The resource key alone may be ambiguous (it is
-// possible for resources of different types to have the same key), but the key prefixed by
-// the type is guaranteed to be unique.
-func Keys(b *bundle.Bundle) (keyOnly lookup, keyWithType lookup) {
-	keyOnly = make(lookup)
-	keyWithType = make(lookup)
-
-	// Collect all resources by their key and prefixed key.
-	for _, group := range b.Config.Resources.AllResources() {
-		typ := group.Description.PluralName
-		for k, v := range group.Resources {
-			kt := fmt.Sprintf("%s.%s", typ, k)
-			p := pair{key: kt, resource: v}
-			keyOnly[k] = append(keyOnly[k], p)
-			keyWithType[kt] = append(keyWithType[kt], p)
-		}
-	}
-
-	return
-}
-
-// Lookup returns the resource with the given key.
-// It first attempts to find a resource with the key alone.
-// If this fails, it tries the key prefixed by the resource type.
-// If this also fails, it returns an error.
-func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) {
-	keyOnly, keyWithType := Keys(b)
-
-	// First try to find the resource by key alone.
-	if res, ok := keyOnly[key]; ok {
-		if len(res) == 1 {
-			return res[0].resource, nil
-		}
-	}
-
-	// Then try to find the resource by key and type.
-	if res, ok := keyWithType[key]; ok {
-		if len(res) == 1 {
-			return res[0].resource, nil
-		}
-	}
-
-	return nil, fmt.Errorf("resource with key %q not found", key)
-}

bundle/resources/keys_test.go

@@ -1 +0,0 @@
-package resources

bundle/resources/lookup.go

@@ -0,0 +1,59 @@
+package resources
+
+import (
+	"fmt"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+)
+
+// Reference is a reference to a resource.
+// It includes the resource key, the resource type description, and a reference to the resource itself.
+type Reference struct {
+	Key         string
+	Description config.ResourceDescription
+	Resource    config.ConfigResource
+}
+
+// Map is the core type for resource lookup and completion.
+type Map map[string][]Reference
+
+// References returns a map of resource keys to a slice of [Reference].
+// While its return type allows for multiple resources to share the same key,
+// this is confirmed not to happen in the [validate.UniqueResourceKeys]	mutator.
+func References(b *bundle.Bundle) Map {
+	output := make(Map)
+
+	// Collect map of resource references indexed by their keys.
+	for _, group := range b.Config.Resources.AllResources() {
+		for k, v := range group.Resources {
+			output[k] = append(output[k], Reference{
+				Key:         k,
+				Description: group.Description,
+				Resource:    v,
+			})
+		}
+	}
+
+	return output
+}
+
+// Lookup returns the resource with the specified key.
+// If the key maps to more than one resource, an error is returned.
+// If the key does not map to any resource, an error is returned.
+func Lookup(b *bundle.Bundle, key string) (config.ConfigResource, error) {
+	refs := References(b)
+	res, ok := refs[key]
+	if !ok {
+		return nil, fmt.Errorf("resource with key %q not found", key)
+	}
+
+	switch {
+	case len(res) == 1:
+		return res[0].Resource, nil
+	case len(res) > 1:
+		return nil, fmt.Errorf("multiple resources with key %q found", key)
+	default:
+		panic("unreachable")
+	}
+}

bundle/resources/lookup_test.go

@@ -0,0 +1,85 @@
+package resources
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/databricks/databricks-sdk-go/service/jobs"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLookup_EmptyBundle(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{},
+		},
+	}
+
+	out, err := Lookup(b, "foo")
+	require.Error(t, err)
+	assert.Nil(t, out)
+	assert.ErrorContains(t, err, "resource with key \"foo\" not found")
+}
+
+func TestLookup_NotFound(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"foo": {},
+					"bar": {},
+				},
+			},
+		},
+	}
+
+	out, err := Lookup(b, "qux")
+	require.Error(t, err)
+	assert.Nil(t, out)
+	assert.ErrorContains(t, err, `resource with key "qux" not found`)
+}
+
+func TestLookup_MultipleFound(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"foo": {},
+				},
+				Pipelines: map[string]*resources.Pipeline{
+					"foo": {},
+				},
+			},
+		},
+	}
+
+	out, err := Lookup(b, "foo")
+	require.Error(t, err)
+	assert.Nil(t, out)
+	assert.ErrorContains(t, err, `multiple resources with key "foo" found`)
+}
+
+func TestLookup_Nominal(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"foo": {
+						JobSettings: &jobs.JobSettings{
+							Name: "Foo job",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	out, err := Lookup(b, "foo")
+	require.NoError(t, err)
+	if assert.NotNil(t, out) {
+		assert.Equal(t, "Foo job", out.GetName())
+	}
+}

cmd/bundle/open.go

@@ -1,6 +1,7 @@
 package bundle
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"os"
@@ -15,10 +16,40 @@ import (
 	"github.com/databricks/cli/cmd/root"
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/spf13/cobra"
+	"golang.org/x/exp/maps"
 
 	"github.com/pkg/browser"
 )
 
+func promptOpenArgument(ctx context.Context, b *bundle.Bundle) (string, error) {
+	// Compute map of "Human readable name of resource" -> "resource key".
+	inv := make(map[string]string)
+	for k, ref := range resources.Completions(b) {
+		title := fmt.Sprintf("%s: %s", ref.Description.SingularTitle, ref.Resource.GetName())
+		inv[title] = k
+	}
+
+	key, err := cmdio.Select(ctx, inv, "Resource to open")
+	if err != nil {
+		return "", err
+	}
+
+	return key, nil
+}
+
+func resolveOpenArgument(ctx context.Context, b *bundle.Bundle, args []string) (string, error) {
+	// If no arguments are specified, prompt the user to select something to run.
+	if len(args) == 0 && cmdio.IsPromptSupported(ctx) {
+		return promptOpenArgument(ctx, b)
+	}
+
+	if len(args) < 1 {
+		return "", fmt.Errorf("expected a KEY of the resource to open")
+	}
+
+	return args[0], nil
+}
+
 func newOpenCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "open",
@@ -41,23 +72,10 @@ func newOpenCommand() *cobra.Command {
 			return err
 		}
 
-		// If no arguments are specified, prompt the user to select something to run.
+		arg, err := resolveOpenArgument(ctx, b, args)
-		if len(args) == 0 && cmdio.IsPromptSupported(ctx) {
-			// Invert completions from KEY -> NAME, to NAME -> KEY.
-			inv := make(map[string]string)
-			for k, v := range resources.CompletionMap(b) {
-				inv[v] = k
-			}
-			id, err := cmdio.Select(ctx, inv, "Resource to open")
 		if err != nil {
 			return err
 		}
-			args = append(args, id)
-		}
-
-		if len(args) < 1 {
-			return fmt.Errorf("expected a KEY of the resource to open")
-		}
 
 		cacheDir, err := terraform.Dir(ctx, b)
 		if err != nil {
@@ -87,7 +105,7 @@ func newOpenCommand() *cobra.Command {
 		}
 
 		// Locate resource to open.
-		resource, err := resources.Lookup(b, args[0])
+		resource, err := resources.Lookup(b, arg)
 		if err != nil {
 			return err
 		}
@@ -95,7 +113,7 @@ func newOpenCommand() *cobra.Command {
 		// Confirm that the resource has a URL.
 		url := resource.GetURL()
 		if url == "" {
-			return errors.New("resource does not have a URL associated with it (has it been deployed?)")
+			return fmt.Errorf("resource does not have a URL associated with it (has it been deployed?)")
 		}
 
 		return browser.OpenURL(url)
@@ -115,7 +133,8 @@ func newOpenCommand() *cobra.Command {
 		}
 
 		if len(args) == 0 {
-			return resources.Completions(b), cobra.ShellCompDirectiveNoFileComp
+			completions := resources.Completions(b)
+			return maps.Keys(completions), cobra.ShellCompDirectiveNoFileComp
 		} else {
 			return nil, cobra.ShellCompDirectiveNoFileComp
 		}