diff --git a/bundle/resources/completion.go b/bundle/resources/completion.go index 6e16b2b4c..2e033813b 100644 --- a/bundle/resources/completion.go +++ b/bundle/resources/completion.go @@ -1,39 +1,16 @@ package resources -import ( - "github.com/databricks/cli/bundle" - "golang.org/x/exp/maps" -) +import "github.com/databricks/cli/bundle" -func CompletionMap(b *bundle.Bundle) map[string]string { - out := make(map[string]string) - keyOnly, keyWithType := Keys(b) - - // Keep track of resources we have seen by their fully qualified key. - seen := make(map[string]bool) - - // 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 { +// Completions returns the same as [References] except +// that every key maps directly to a single reference. +func Completions(b *bundle.Bundle) map[string]Reference { + out := make(map[string]Reference) + for k, refs := range References(b) { + if len(refs) != 1 { continue } - out[k] = v[0].resource.GetName() + out[k] = refs[0] } - return out } - -func Completions(b *bundle.Bundle) []string { - return maps.Keys(CompletionMap(b)) -} diff --git a/bundle/resources/completion_test.go b/bundle/resources/completion_test.go index 18d6395a7..10c0c0ba6 100644 --- a/bundle/resources/completion_test.go +++ 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") + } +} diff --git a/bundle/resources/keys.go b/bundle/resources/keys.go deleted file mode 100644 index 90a5dbe77..000000000 --- a/bundle/resources/keys.go +++ /dev/null @@ -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) -} diff --git a/bundle/resources/keys_test.go b/bundle/resources/keys_test.go deleted file mode 100644 index 18d6395a7..000000000 --- a/bundle/resources/keys_test.go +++ /dev/null @@ -1 +0,0 @@ -package resources diff --git a/bundle/resources/lookup.go b/bundle/resources/lookup.go new file mode 100644 index 000000000..5cf8b724e --- /dev/null +++ b/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") + } +} diff --git a/bundle/resources/lookup_test.go b/bundle/resources/lookup_test.go new file mode 100644 index 000000000..20bc5189b --- /dev/null +++ b/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()) + } +} diff --git a/cmd/bundle/open.go b/cmd/bundle/open.go index c4d7e7e38..cf3df7a57 100644 --- a/cmd/bundle/open.go +++ b/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,22 +72,9 @@ func newOpenCommand() *cobra.Command { return err } - // If no arguments are specified, prompt the user to select something to run. - 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") + arg, err := resolveOpenArgument(ctx, b, args) + if err != nil { + return err } cacheDir, err := terraform.Dir(ctx, b) @@ -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 }