diff --git a/integration/bundle/helpers_test.go b/integration/bundle/helpers_test.go index 60177297e..032b7f95e 100644 --- a/integration/bundle/helpers_test.go +++ b/integration/bundle/helpers_test.go @@ -8,13 +8,17 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/internal/testcli" "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/folders" + "github.com/databricks/cli/libs/template" "github.com/databricks/databricks-sdk-go" "github.com/stretchr/testify/require" ) @@ -27,32 +31,28 @@ func initTestTemplate(t testutil.TestingT, ctx context.Context, templateName str } func initTestTemplateWithBundleRoot(t testutil.TestingT, ctx context.Context, templateName string, config map[string]any, bundleRoot string) string { - return "" + templateRoot := filepath.Join("bundles", templateName) - // TODO: Make this function work but do not log telemetry. + configFilePath := writeConfigFile(t, config) - // templateRoot := filepath.Join("bundles", templateName) + ctx = root.SetWorkspaceClient(ctx, nil) + cmd := cmdio.NewIO(ctx, flags.OutputJSON, strings.NewReader(""), os.Stdout, os.Stderr, "", "bundles") + ctx = cmdio.InContext(ctx, cmd) - // configFilePath := writeConfigFile(t, config) + r := template.Resolver{ + TemplatePathOrUrl: templateRoot, + ConfigFile: configFilePath, + OutputDir: bundleRoot, + } - // ctx = root.SetWorkspaceClient(ctx, nil) - // cmd := cmdio.NewIO(ctx, flags.OutputJSON, strings.NewReader(""), os.Stdout, os.Stderr, "", "bundles") - // ctx = cmdio.InContext(ctx, cmd) - // ctx = telemetry.WithMockLogger(ctx) + tmpl, err := r.Resolve(ctx) + require.NoError(t, err) + defer tmpl.Reader.Close() - // out, err := filer.NewLocalClient(bundleRoot) - // require.NoError(t, err) - // tmpl := template.TemplateX{ - // TemplateOpts: template.TemplateOpts{ - // ConfigFilePath: configFilePath, - // TemplateFS: os.DirFS(templateRoot), - // OutputFiler: out, - // }, - // } + err = tmpl.Writer.Materialize(ctx, tmpl.Reader) + require.NoError(t, err) - // err = tmpl.Materialize(ctx) - // require.NoError(t, err) - // return bundleRoot + return bundleRoot } func writeConfigFile(t testutil.TestingT, config map[string]any) string { diff --git a/libs/template/builtin.go b/libs/template/builtin.go index 96cdcbb96..5b10534ef 100644 --- a/libs/template/builtin.go +++ b/libs/template/builtin.go @@ -8,15 +8,14 @@ import ( //go:embed all:templates var builtinTemplates embed.FS -// BuiltinTemplate represents a template that is built into the CLI. -type BuiltinTemplate struct { +// builtinTemplate represents a template that is built into the CLI. +type builtinTemplate struct { Name string FS fs.FS } -// Builtin returns the list of all built-in templates. -// TODO: Make private? -func Builtin() ([]BuiltinTemplate, error) { +// builtin returns the list of all built-in templates. +func builtin() ([]builtinTemplate, error) { templates, err := fs.Sub(builtinTemplates, "templates") if err != nil { return nil, err @@ -27,7 +26,7 @@ func Builtin() ([]BuiltinTemplate, error) { return nil, err } - var out []BuiltinTemplate + var out []builtinTemplate for _, entry := range entries { if !entry.IsDir() { continue @@ -38,7 +37,7 @@ func Builtin() ([]BuiltinTemplate, error) { return nil, err } - out = append(out, BuiltinTemplate{ + out = append(out, builtinTemplate{ Name: entry.Name(), FS: templateFS, }) diff --git a/libs/template/builtin_test.go b/libs/template/builtin_test.go index 79e04cb84..162a227ea 100644 --- a/libs/template/builtin_test.go +++ b/libs/template/builtin_test.go @@ -9,12 +9,12 @@ import ( ) func TestBuiltin(t *testing.T) { - out, err := Builtin() + out, err := builtin() require.NoError(t, err) assert.GreaterOrEqual(t, len(out), 3) // Create a map of templates by name for easier lookup - templates := make(map[string]*BuiltinTemplate) + templates := make(map[string]*builtinTemplate) for _, tmpl := range out { templates[tmpl.Name] = &tmpl } diff --git a/libs/template/reader.go b/libs/template/reader.go index 19d4ec243..56d264edd 100644 --- a/libs/template/reader.go +++ b/libs/template/reader.go @@ -9,10 +9,8 @@ import ( "strings" "github.com/databricks/cli/libs/cmdio" - "github.com/databricks/cli/libs/git" ) -// TODO: Add tests for all these readers. type Reader interface { // FS returns a file system that contains the template // definition files. This function is NOT thread safe. @@ -24,7 +22,7 @@ type Reader interface { } type builtinReader struct { - name TemplateName + name string fsCached fs.FS } @@ -34,19 +32,23 @@ func (r *builtinReader) FS(ctx context.Context) (fs.FS, error) { return r.fsCached, nil } - builtin, err := Builtin() + builtin, err := builtin() if err != nil { return nil, err } var templateFS fs.FS for _, entry := range builtin { - if entry.Name == string(r.name) { + if entry.Name == r.name { templateFS = entry.FS break } } + if templateFS == nil { + return nil, fmt.Errorf("builtin template %s not found", r.name) + } + r.fsCached = templateFS return r.fsCached, nil } @@ -64,6 +66,10 @@ type gitReader struct { // temporary directory where the repository is cloned tmpRepoDir string + // Function to clone the repository. This is a function pointer to allow + // mocking in tests. + cloneFunc func(ctx context.Context, url, reference, targetPath string) error + fsCached fs.FS } @@ -80,8 +86,7 @@ var gitUrlPrefixes = []string{ "git@", } -// TODO: Make private? -func IsGitRepoUrl(url string) bool { +func isRepoUrl(url string) bool { result := false for _, prefix := range gitUrlPrefixes { if strings.HasPrefix(url, prefix) { @@ -92,7 +97,6 @@ func IsGitRepoUrl(url string) bool { return result } -// TODO: Test the idempotency of this function as well. func (r *gitReader) FS(ctx context.Context) (fs.FS, error) { // If the FS has already been loaded, return it. if r.fsCached != nil { @@ -111,7 +115,7 @@ func (r *gitReader) FS(ctx context.Context) (fs.FS, error) { promptSpinner := cmdio.Spinner(ctx) promptSpinner <- "Downloading the template\n" - err = git.Clone(ctx, r.gitUrl, r.ref, repoDir) + err = r.cloneFunc(ctx, r.gitUrl, r.ref, repoDir) close(promptSpinner) if err != nil { return nil, err @@ -130,7 +134,6 @@ func (r *gitReader) Close() error { } type localReader struct { - name string // Path on the local filesystem that contains the template path string diff --git a/libs/template/reader_test.go b/libs/template/reader_test.go new file mode 100644 index 000000000..1cce2321b --- /dev/null +++ b/libs/template/reader_test.go @@ -0,0 +1,112 @@ +package template + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuiltInReader(t *testing.T) { + exists := []string{ + "default-python", + "default-sql", + "dbt-sql", + } + + for _, name := range exists { + r := &builtinReader{name: name} + fs, err := r.FS(context.Background()) + assert.NoError(t, err) + assert.NotNil(t, fs) + } + + // TODO: Read one of the files to confirm further test this reader. + r := &builtinReader{name: "doesnotexist"} + _, err := r.FS(context.Background()) + assert.EqualError(t, err, "builtin template doesnotexist not found") +} + +func TestGitUrlReader(t *testing.T) { + ctx := context.Background() + cmd := cmdio.NewIO(ctx, flags.OutputJSON, strings.NewReader(""), os.Stdout, os.Stderr, "", "bundles") + ctx = cmdio.InContext(ctx, cmd) + + var args []string + numCalls := 0 + cloneFunc := func(ctx context.Context, url, reference, targetPath string) error { + numCalls++ + args = []string{url, reference, targetPath} + err := os.MkdirAll(filepath.Join(targetPath, "a/b/c"), 0o755) + require.NoError(t, err) + testutil.WriteFile(t, filepath.Join(targetPath, "a", "b", "c", "somefile"), "somecontent") + return nil + } + r := &gitReader{ + gitUrl: "someurl", + cloneFunc: cloneFunc, + ref: "sometag", + templateDir: "a/b/c", + } + + // Assert cloneFunc is called with the correct args. + fs, err := r.FS(ctx) + require.NoError(t, err) + require.NotEmpty(t, r.tmpRepoDir) + assert.DirExists(t, r.tmpRepoDir) + assert.Equal(t, []string{"someurl", "sometag", r.tmpRepoDir}, args) + + // Assert the fs returned is rooted at the templateDir. + fd, err := fs.Open("somefile") + require.NoError(t, err) + defer fd.Close() + b, err := io.ReadAll(fd) + require.NoError(t, err) + assert.Equal(t, "somecontent", string(b)) + + // Assert the FS is cached. cloneFunc should not be called again. + _, err = r.FS(ctx) + require.NoError(t, err) + assert.Equal(t, 1, numCalls) + + // Assert Close cleans up the tmpRepoDir. + err = r.Close() + require.NoError(t, err) + assert.NoDirExists(t, r.tmpRepoDir) +} + +func TestLocalReader(t *testing.T) { + tmpDir := t.TempDir() + testutil.WriteFile(t, filepath.Join(tmpDir, "somefile"), "somecontent") + ctx := context.Background() + + r := &localReader{path: tmpDir} + fs, err := r.FS(ctx) + require.NoError(t, err) + + // Assert the fs returned is rooted at correct location. + fd, err := fs.Open("somefile") + require.NoError(t, err) + defer fd.Close() + b, err := io.ReadAll(fd) + require.NoError(t, err) + assert.Equal(t, "somecontent", string(b)) + + // Assert close does not error + assert.NoError(t, r.Close()) +} + +func TestFailReader(t *testing.T) { + r := &failReader{} + assert.Error(t, r.Close()) + _, err := r.FS(context.Background()) + assert.Error(t, err) +} diff --git a/libs/template/resolve.go b/libs/template/resolve.go index a4099e275..7d3b68bff 100644 --- a/libs/template/resolve.go +++ b/libs/template/resolve.go @@ -3,6 +3,8 @@ package template import ( "context" "errors" + + "github.com/databricks/cli/libs/git" ) type Resolver struct { @@ -48,11 +50,12 @@ func (r Resolver) Resolve(ctx context.Context) (*Template, error) { // Based on the provided template path or URL, // configure a reader for the template. tmpl = Get(Custom) - if IsGitRepoUrl(r.TemplatePathOrUrl) { + if isRepoUrl(r.TemplatePathOrUrl) { tmpl.Reader = &gitReader{ gitUrl: r.TemplatePathOrUrl, ref: ref, templateDir: r.TemplateDir, + cloneFunc: git.Clone, } } else { tmpl.Reader = &localReader{ diff --git a/libs/template/template.go b/libs/template/template.go index ec8e1ac15..75b913d87 100644 --- a/libs/template/template.go +++ b/libs/template/template.go @@ -6,28 +6,17 @@ import ( "strings" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/git" ) type Template struct { - // TODO: Make private as much as possible. Reader Reader Writer Writer - Name TemplateName - Description string - Aliases []string - Hidden bool -} - -// TODO: Make details private? -// TODO: Combine this with the generic template struct? -type NativeTemplate struct { - Name string - Description string - Aliases []string - GitUrl string - Hidden bool - IsOwnedByDatabricks bool + name TemplateName + description string + aliases []string + hidden bool } type TemplateName string @@ -43,40 +32,40 @@ const ( var allTemplates = []Template{ { - Name: DefaultPython, - Description: "The default Python template for Notebooks / Delta Live Tables / Workflows", + name: DefaultPython, + description: "The default Python template for Notebooks / Delta Live Tables / Workflows", Reader: &builtinReader{name: "default-python"}, Writer: &writerWithTelemetry{}, }, { - Name: DefaultSql, - Description: "The default SQL template for .sql files that run with Databricks SQL", + name: DefaultSql, + description: "The default SQL template for .sql files that run with Databricks SQL", Reader: &builtinReader{name: "default-sql"}, Writer: &writerWithTelemetry{}, }, { - Name: DbtSql, - Description: "The dbt SQL template (databricks.com/blog/delivering-cost-effective-data-real-time-dbt-and-databricks)", + name: DbtSql, + description: "The dbt SQL template (databricks.com/blog/delivering-cost-effective-data-real-time-dbt-and-databricks)", Reader: &builtinReader{name: "dbt-sql"}, Writer: &writerWithTelemetry{}, }, { - Name: MlopsStacks, - Description: "The Databricks MLOps Stacks template (github.com/databricks/mlops-stacks)", - Aliases: []string{"mlops-stack"}, - Reader: &gitReader{gitUrl: "https://github.com/databricks/mlops-stacks"}, + name: MlopsStacks, + description: "The Databricks MLOps Stacks template (github.com/databricks/mlops-stacks)", + aliases: []string{"mlops-stack"}, + Reader: &gitReader{gitUrl: "https://github.com/databricks/mlops-stacks", cloneFunc: git.Clone}, Writer: &writerWithTelemetry{}, }, { - Name: DefaultPydabs, - Hidden: true, - Description: "The default PyDABs template", - Reader: &gitReader{gitUrl: "https://databricks.github.io/workflows-authoring-toolkit/pydabs-template.git"}, + name: DefaultPydabs, + hidden: true, + description: "The default PyDABs template", + Reader: &gitReader{gitUrl: "https://databricks.github.io/workflows-authoring-toolkit/pydabs-template.git", cloneFunc: git.Clone}, Writer: &writerWithTelemetry{}, }, { - Name: Custom, - Description: "Bring your own template", + name: Custom, + description: "Bring your own template", Reader: &failReader{}, Writer: &defaultWriter{}, }, @@ -85,8 +74,8 @@ var allTemplates = []Template{ func HelpDescriptions() string { var lines []string for _, template := range allTemplates { - if template.Name != Custom && !template.Hidden { - lines = append(lines, fmt.Sprintf("- %s: %s", template.Name, template.Description)) + if template.name != Custom && !template.hidden { + lines = append(lines, fmt.Sprintf("- %s: %s", template.name, template.description)) } } return strings.Join(lines, "\n") @@ -95,12 +84,12 @@ func HelpDescriptions() string { func options() []cmdio.Tuple { names := make([]cmdio.Tuple, 0, len(allTemplates)) for _, template := range allTemplates { - if template.Hidden { + if template.hidden { continue } tuple := cmdio.Tuple{ - Name: string(template.Name), - Id: template.Description, + Name: string(template.name), + Id: template.description, } names = append(names, tuple) } @@ -118,8 +107,8 @@ func SelectTemplate(ctx context.Context) (TemplateName, error) { } for _, template := range allTemplates { - if template.Description == description { - return template.Name, nil + if template.description == description { + return template.name, nil } } @@ -128,7 +117,7 @@ func SelectTemplate(ctx context.Context) (TemplateName, error) { func Get(name TemplateName) *Template { for _, template := range allTemplates { - if template.Name == name { + if template.name == name { return &template } } diff --git a/libs/template/template_test.go b/libs/template/template_test.go index 6b6ca0d0e..73d818dfe 100644 --- a/libs/template/template_test.go +++ b/libs/template/template_test.go @@ -27,11 +27,11 @@ func TestTemplateOptions(t *testing.T) { } func TestBundleInitIsRepoUrl(t *testing.T) { - assert.True(t, IsGitRepoUrl("git@github.com:databricks/cli.git")) - assert.True(t, IsGitRepoUrl("https://github.com/databricks/cli.git")) + assert.True(t, isRepoUrl("git@github.com:databricks/cli.git")) + assert.True(t, isRepoUrl("https://github.com/databricks/cli.git")) - assert.False(t, IsGitRepoUrl("./local")) - assert.False(t, IsGitRepoUrl("foo")) + assert.False(t, isRepoUrl("./local")) + assert.False(t, isRepoUrl("foo")) } func TestBundleInitRepoName(t *testing.T) {