From 09572635a9f4a94ebf7afcf1874c7e855732aa41 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 21 Jul 2023 13:17:53 +0200 Subject: [PATCH 01/11] Only treat files with .tmpl extension as templates --- libs/template/renderer.go | 46 ++++++++++++++----- libs/template/renderer_test.go | 24 ++++++---- .../template/{my_email => my_email.tmpl} | 0 .../{not-a-script => not-a-script.tmpl} | 0 .../template/{script.sh => script.sh.tmpl} | 0 .../fail/template/{hello => hello.tmpl} | 0 .../template/{file3 => file3.tmpl} | 0 .../template/dir1/{file1 => file1.tmpl} | 0 .../template/{file2 => file2.tmpl} | 0 .../template/dir1/dir2/{file3 => file3.tmpl} | 0 .../template/dir1/{file2 => file2.tmpl} | 0 .../template/{file1 => file1.tmpl} | 0 .../skip/template/{file1 => file1.tmpl} | 0 .../skip/template/{file2 => file2.tmpl} | 0 .../walk/template/dir2/{file4 => file4.tmpl} | 0 15 files changed, 50 insertions(+), 20 deletions(-) rename libs/template/testdata/email/template/{my_email => my_email.tmpl} (100%) rename libs/template/testdata/executable-bit-read/template/{not-a-script => not-a-script.tmpl} (100%) rename libs/template/testdata/executable-bit-read/template/{script.sh => script.sh.tmpl} (100%) rename libs/template/testdata/fail/template/{hello => hello.tmpl} (100%) rename libs/template/testdata/skip-all-files-in-cwd/template/{file3 => file3.tmpl} (100%) rename libs/template/testdata/skip-dir-eagerly/template/dir1/{file1 => file1.tmpl} (100%) rename libs/template/testdata/skip-dir-eagerly/template/{file2 => file2.tmpl} (100%) rename libs/template/testdata/skip-is-relative/template/dir1/dir2/{file3 => file3.tmpl} (100%) rename libs/template/testdata/skip-is-relative/template/dir1/{file2 => file2.tmpl} (100%) rename libs/template/testdata/skip-is-relative/template/{file1 => file1.tmpl} (100%) rename libs/template/testdata/skip/template/{file1 => file1.tmpl} (100%) rename libs/template/testdata/skip/template/{file2 => file2.tmpl} (100%) rename libs/template/testdata/walk/template/dir2/{file4 => file4.tmpl} (100%) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 853e3505bd..8ad74b3381 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -18,6 +18,10 @@ import ( "golang.org/x/exp/slices" ) +// TODO: add test for paths being executed + +const TemplateExtension = ".tmpl" + type inMemoryFile struct { // Root path for the project instance. This path uses the system's default // file separator. For example /foo/bar on Unix and C:\foo\bar on windows @@ -27,22 +31,31 @@ type inMemoryFile struct { // is relative to the root. Using unix like relative paths enables skip patterns // to work across both windows and unix based operating systems. relPath string - content []byte + content io.ReadCloser perm fs.FileMode } -func (f *inMemoryFile) fullPath() string { +func (f *inMemoryFile) path() string { return filepath.Join(f.root, filepath.FromSlash(f.relPath)) } func (f *inMemoryFile) persistToDisk() error { - path := f.fullPath() + path := f.path() err := os.MkdirAll(filepath.Dir(path), 0755) if err != nil { return err } - return os.WriteFile(path, f.content, f.perm) + + dstFile, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, f.perm) + if err != nil { + return err + } + _, err = io.Copy(dstFile, f.content) + if err != nil { + return err + } + return f.content.Close() } // Renders a databricks template as a project @@ -148,10 +161,6 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { if err != nil { return nil, err } - contentTemplate, err := io.ReadAll(templateReader) - if err != nil { - return nil, err - } // read file permissions info, err := r.templateFiler.Stat(r.ctx, relPathTemplate) @@ -160,7 +169,22 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { } perm := info.Mode().Perm() + // If file name does not specify the `.tmpl` extension, then it is copied + // over as is, without treating it as a template + if !strings.HasSuffix(relPathTemplate, TemplateExtension) { + return &inMemoryFile{ + root: r.instanceRoot, + relPath: relPathTemplate, + content: templateReader, + perm: perm, + }, nil + } + // execute the contents of the file as a template + contentTemplate, err := io.ReadAll(templateReader) + if err != nil { + return nil, err + } content, err := r.executeTemplate(string(contentTemplate)) // Capture errors caused by the "fail" helper function if target := (&ErrFail{}); errors.As(err, target) { @@ -171,6 +195,7 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { } // Execute relative path template to get materialized path for the file + relPathTemplate = strings.TrimSuffix(relPathTemplate, TemplateExtension) relPath, err := r.executeTemplate(relPathTemplate) if err != nil { return nil, err @@ -179,12 +204,11 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { return &inMemoryFile{ root: r.instanceRoot, relPath: relPath, - content: []byte(content), + content: io.NopCloser(strings.NewReader(content)), perm: perm, }, nil } - // This function walks the template file tree to generate an in memory representation // of a project. // @@ -282,7 +306,7 @@ func (r *renderer) persistToDisk() error { // Assert no conflicting files exist for _, file := range filesToPersist { - path := file.fullPath() + path := file.path() _, err := os.Stat(path) if err == nil { return fmt.Errorf("failed to persist to disk, conflict with existing file: %s", path) diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 468c607f4f..897bc6c044 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -3,6 +3,7 @@ package template import ( "context" "fmt" + "io" "io/fs" "os" "path/filepath" @@ -168,13 +169,13 @@ func TestRendererPersistToDisk(t *testing.T) { { root: tmpDir, relPath: "a/b/d", - content: []byte("123"), + content: io.NopCloser(strings.NewReader("123")), perm: 0444, }, { root: tmpDir, relPath: "mmnn", - content: []byte("456"), + content: io.NopCloser(strings.NewReader("456")), perm: 0444, }, }, @@ -205,7 +206,9 @@ func TestRendererWalk(t *testing.T) { getContent := func(r *renderer, path string) string { for _, f := range r.files { if f.relPath == path { - return strings.Trim(string(f.content), "\r\n") + b, err := io.ReadAll(f.content) + require.NoError(t, err) + return strings.Trim(string(b), "\r\n") } } require.FailNow(t, "file is absent: "+path) @@ -241,7 +244,10 @@ func TestRendererSkipsDirsEagerly(t *testing.T) { assert.NoError(t, err) assert.Len(t, r.files, 1) - content := string(r.files[0].content) + + b, err := io.ReadAll(r.files[0].content) + require.NoError(t, err) + content := string(b) assert.Equal(t, "I should be the only file created", strings.Trim(content, "\r\n")) } @@ -317,7 +323,7 @@ func TestRendererInMemoryFileFullPathForWindows(t *testing.T) { root: `c:\a\b\c`, relPath: "d/e", } - assert.Equal(t, `c:\a\b\c\d\e`, f.fullPath()) + assert.Equal(t, `c:\a\b\c\d\e`, f.path()) } func TestRendererInMemoryFilePersistToDiskSetsExecutableBit(t *testing.T) { @@ -329,7 +335,7 @@ func TestRendererInMemoryFilePersistToDiskSetsExecutableBit(t *testing.T) { f := &inMemoryFile{ root: tmpDir, relPath: "a/b/c", - content: []byte("123"), + content: io.NopCloser(strings.NewReader("123")), perm: 0755, } err := f.persistToDisk() @@ -348,7 +354,7 @@ func TestRendererInMemoryFilePersistToDiskForWindows(t *testing.T) { f := &inMemoryFile{ root: tmpDir, relPath: "a/b/c", - content: []byte("123"), + content: io.NopCloser(strings.NewReader("123")), perm: 0666, } err := f.persistToDisk() @@ -400,7 +406,7 @@ func TestRendererErrorOnConflictingFile(t *testing.T) { { root: tmpDir, relPath: "a", - content: []byte("123"), + content: io.NopCloser(strings.NewReader("123")), perm: 0444, }, }, @@ -425,7 +431,7 @@ func TestRendererNoErrorOnConflictingFileIfSkipped(t *testing.T) { { root: tmpDir, relPath: "a", - content: []byte("123"), + content: io.NopCloser(strings.NewReader("123")), perm: 0444, }, }, diff --git a/libs/template/testdata/email/template/my_email b/libs/template/testdata/email/template/my_email.tmpl similarity index 100% rename from libs/template/testdata/email/template/my_email rename to libs/template/testdata/email/template/my_email.tmpl diff --git a/libs/template/testdata/executable-bit-read/template/not-a-script b/libs/template/testdata/executable-bit-read/template/not-a-script.tmpl similarity index 100% rename from libs/template/testdata/executable-bit-read/template/not-a-script rename to libs/template/testdata/executable-bit-read/template/not-a-script.tmpl diff --git a/libs/template/testdata/executable-bit-read/template/script.sh b/libs/template/testdata/executable-bit-read/template/script.sh.tmpl similarity index 100% rename from libs/template/testdata/executable-bit-read/template/script.sh rename to libs/template/testdata/executable-bit-read/template/script.sh.tmpl diff --git a/libs/template/testdata/fail/template/hello b/libs/template/testdata/fail/template/hello.tmpl similarity index 100% rename from libs/template/testdata/fail/template/hello rename to libs/template/testdata/fail/template/hello.tmpl diff --git a/libs/template/testdata/skip-all-files-in-cwd/template/file3 b/libs/template/testdata/skip-all-files-in-cwd/template/file3.tmpl similarity index 100% rename from libs/template/testdata/skip-all-files-in-cwd/template/file3 rename to libs/template/testdata/skip-all-files-in-cwd/template/file3.tmpl diff --git a/libs/template/testdata/skip-dir-eagerly/template/dir1/file1 b/libs/template/testdata/skip-dir-eagerly/template/dir1/file1.tmpl similarity index 100% rename from libs/template/testdata/skip-dir-eagerly/template/dir1/file1 rename to libs/template/testdata/skip-dir-eagerly/template/dir1/file1.tmpl diff --git a/libs/template/testdata/skip-dir-eagerly/template/file2 b/libs/template/testdata/skip-dir-eagerly/template/file2.tmpl similarity index 100% rename from libs/template/testdata/skip-dir-eagerly/template/file2 rename to libs/template/testdata/skip-dir-eagerly/template/file2.tmpl diff --git a/libs/template/testdata/skip-is-relative/template/dir1/dir2/file3 b/libs/template/testdata/skip-is-relative/template/dir1/dir2/file3.tmpl similarity index 100% rename from libs/template/testdata/skip-is-relative/template/dir1/dir2/file3 rename to libs/template/testdata/skip-is-relative/template/dir1/dir2/file3.tmpl diff --git a/libs/template/testdata/skip-is-relative/template/dir1/file2 b/libs/template/testdata/skip-is-relative/template/dir1/file2.tmpl similarity index 100% rename from libs/template/testdata/skip-is-relative/template/dir1/file2 rename to libs/template/testdata/skip-is-relative/template/dir1/file2.tmpl diff --git a/libs/template/testdata/skip-is-relative/template/file1 b/libs/template/testdata/skip-is-relative/template/file1.tmpl similarity index 100% rename from libs/template/testdata/skip-is-relative/template/file1 rename to libs/template/testdata/skip-is-relative/template/file1.tmpl diff --git a/libs/template/testdata/skip/template/file1 b/libs/template/testdata/skip/template/file1.tmpl similarity index 100% rename from libs/template/testdata/skip/template/file1 rename to libs/template/testdata/skip/template/file1.tmpl diff --git a/libs/template/testdata/skip/template/file2 b/libs/template/testdata/skip/template/file2.tmpl similarity index 100% rename from libs/template/testdata/skip/template/file2 rename to libs/template/testdata/skip/template/file2.tmpl diff --git a/libs/template/testdata/walk/template/dir2/file4 b/libs/template/testdata/walk/template/dir2/file4.tmpl similarity index 100% rename from libs/template/testdata/walk/template/dir2/file4 rename to libs/template/testdata/walk/template/dir2/file4.tmpl From b3f93fbe19c23833f20ab0cfbe5ee7b6171abea7 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Sun, 23 Jul 2023 19:13:08 +0200 Subject: [PATCH 02/11] introducing the file interface --- libs/template/file.go | 104 +++++++++++++++++++++++ libs/template/renderer.go | 86 +++++++------------ libs/template/renderer_test.go | 145 +++++++++++++++++++++------------ 3 files changed, 225 insertions(+), 110 deletions(-) create mode 100644 libs/template/file.go diff --git a/libs/template/file.go b/libs/template/file.go new file mode 100644 index 0000000000..83e94ba69d --- /dev/null +++ b/libs/template/file.go @@ -0,0 +1,104 @@ +package template + +import ( + "context" + "io" + "io/fs" + "os" + "path" + "path/filepath" + + "github.com/databricks/cli/libs/filer" +) + +// Interface for an in memory representation of a file +type file interface { + // Path file will be persisted at, if PersistToDisk is called + Path() string + + // Does the path of the file, relative to the project root match any of the + // specified skip patterns + IsSkipped(patterns []string) (bool, error) + + // This function writes this file onto the disk + PersistToDisk() error +} + +type fileCommon struct { + // Root path for the project instance. This path uses the system's default + // file separator. For example /foo/bar on Unix and C:\foo\bar on windows + root string + + // Unix like relPath for the file (using '/' as the separator). This path + // is relative to the root. Using unix like relative paths enables skip patterns + // to work across both windows and unix based operating systems. + relPath string + + // Permissions bits for the file + perm fs.FileMode +} + +func (f *fileCommon) Path() string { + return filepath.Join(f.root, filepath.FromSlash(f.relPath)) +} + +func (f *fileCommon) IsSkipped(patterns []string) (bool, error) { + for _, pattern := range patterns { + isMatch, err := path.Match(pattern, f.relPath) + if err != nil { + return false, err + } + if isMatch { + return true, nil + } + } + return false, nil +} + +type copyFile struct { + *fileCommon + + ctx context.Context + + // Path of the source file that should be copied over. + srcPath string + + // Filer to use to read source path + srcFiler filer.Filer +} + +func (f *copyFile) PersistToDisk() error { + path := f.Path() + err := os.MkdirAll(filepath.Dir(path), 0755) + if err != nil { + return err + } + srcFile, err := f.srcFiler.Read(f.ctx, f.srcPath) + if err != nil { + return err + } + defer srcFile.Close() + dstFile, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, f.perm) + if err != nil { + return err + } + defer dstFile.Close() + _, err = io.Copy(dstFile, srcFile) + return err +} + +type inMemoryFile struct { + *fileCommon + + content []byte +} + +func (f *inMemoryFile) PersistToDisk() error { + path := f.Path() + + err := os.MkdirAll(filepath.Dir(path), 0755) + if err != nil { + return err + } + return os.WriteFile(path, f.content, f.perm) +} diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 8ad74b3381..c09abdac74 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "io/fs" "os" "path" "path/filepath" @@ -20,43 +19,7 @@ import ( // TODO: add test for paths being executed -const TemplateExtension = ".tmpl" - -type inMemoryFile struct { - // Root path for the project instance. This path uses the system's default - // file separator. For example /foo/bar on Unix and C:\foo\bar on windows - root string - - // Unix like relPath for the file (using '/' as the separator). This path - // is relative to the root. Using unix like relative paths enables skip patterns - // to work across both windows and unix based operating systems. - relPath string - content io.ReadCloser - perm fs.FileMode -} - -func (f *inMemoryFile) path() string { - return filepath.Join(f.root, filepath.FromSlash(f.relPath)) -} - -func (f *inMemoryFile) persistToDisk() error { - path := f.path() - - err := os.MkdirAll(filepath.Dir(path), 0755) - if err != nil { - return err - } - - dstFile, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, f.perm) - if err != nil { - return err - } - _, err = io.Copy(dstFile, f.content) - if err != nil { - return err - } - return f.content.Close() -} +const templateExtension = ".tmpl" // Renders a databricks template as a project type renderer struct { @@ -73,7 +36,7 @@ type renderer struct { baseTemplate *template.Template // List of in memory files generated from template - files []*inMemoryFile + files []file // Glob patterns for files and directories to skip. There are three possible // outcomes for skip: @@ -124,7 +87,7 @@ func newRenderer(ctx context.Context, config map[string]any, templateRoot, libra ctx: ctx, config: config, baseTemplate: tmpl, - files: make([]*inMemoryFile, 0), + files: make([]file, 0), skipPatterns: make([]string, 0), templateFiler: templateFiler, instanceRoot: instanceRoot, @@ -155,12 +118,13 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { return result.String(), nil } -func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { +func (r *renderer) computeFile(relPathTemplate string) (file, error) { // read template file contents templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate) if err != nil { return nil, err } + defer templateReader.Close() // read file permissions info, err := r.templateFiler.Stat(r.ctx, relPathTemplate) @@ -171,12 +135,16 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { // If file name does not specify the `.tmpl` extension, then it is copied // over as is, without treating it as a template - if !strings.HasSuffix(relPathTemplate, TemplateExtension) { - return &inMemoryFile{ - root: r.instanceRoot, - relPath: relPathTemplate, - content: templateReader, - perm: perm, + if !strings.HasSuffix(relPathTemplate, templateExtension) { + return ©File{ + fileCommon: &fileCommon{ + root: r.instanceRoot, + relPath: relPathTemplate, + perm: perm, + }, + ctx: r.ctx, + srcPath: relPathTemplate, + srcFiler: r.templateFiler, }, nil } @@ -195,17 +163,19 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) { } // Execute relative path template to get materialized path for the file - relPathTemplate = strings.TrimSuffix(relPathTemplate, TemplateExtension) + relPathTemplate = strings.TrimSuffix(relPathTemplate, templateExtension) relPath, err := r.executeTemplate(relPathTemplate) if err != nil { return nil, err } return &inMemoryFile{ - root: r.instanceRoot, - relPath: relPath, - content: io.NopCloser(strings.NewReader(content)), - perm: perm, + fileCommon: &fileCommon{ + root: r.instanceRoot, + relPath: relPath, + perm: perm, + }, + content: []byte(content), }, nil } @@ -280,7 +250,7 @@ func (r *renderer) walk() error { if err != nil { return err } - logger.Infof(r.ctx, "added file to list of in memory files: %s", f.relPath) + logger.Infof(r.ctx, "added file to list of in memory files: %s", f.Path()) r.files = append(r.files, f) } @@ -291,14 +261,14 @@ func (r *renderer) walk() error { func (r *renderer) persistToDisk() error { // Accumulate files which we will persist, skipping files whose path matches // any of the skip patterns - filesToPersist := make([]*inMemoryFile, 0) + filesToPersist := make([]file, 0) for _, file := range r.files { - isSkipped, err := r.isSkipped(file.relPath) + isSkipped, err := file.IsSkipped(r.skipPatterns) if err != nil { return err } if isSkipped { - log.Infof(r.ctx, "skipping file: %s", file.relPath) + log.Infof(r.ctx, "skipping file: %s", file.Path()) continue } filesToPersist = append(filesToPersist, file) @@ -306,7 +276,7 @@ func (r *renderer) persistToDisk() error { // Assert no conflicting files exist for _, file := range filesToPersist { - path := file.path() + path := file.Path() _, err := os.Stat(path) if err == nil { return fmt.Errorf("failed to persist to disk, conflict with existing file: %s", path) @@ -318,7 +288,7 @@ func (r *renderer) persistToDisk() error { // Persist files to disk for _, file := range filesToPersist { - err := file.persistToDisk() + err := file.PersistToDisk() if err != nil { return err } diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 897bc6c044..cc269e74f0 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "reflect" "runtime" "strings" "testing" @@ -153,30 +154,38 @@ func TestRendererPersistToDisk(t *testing.T) { ctx: ctx, instanceRoot: tmpDir, skipPatterns: []string{"a/b/c", "mn*"}, - files: []*inMemoryFile{ - { - root: tmpDir, - relPath: "a/b/c", + files: []file{ + &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0444, + }, content: nil, - perm: 0444, }, - { - root: tmpDir, - relPath: "mno", + &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "mno", + perm: 0444, + }, content: nil, - perm: 0444, }, - { - root: tmpDir, - relPath: "a/b/d", - content: io.NopCloser(strings.NewReader("123")), - perm: 0444, + &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/d", + perm: 0444, + }, + content: []byte("123"), }, - { - root: tmpDir, - relPath: "mmnn", - content: io.NopCloser(strings.NewReader("456")), - perm: 0444, + &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "mmnn", + perm: 0444, + }, + content: []byte("456"), }, }, } @@ -205,10 +214,25 @@ func TestRendererWalk(t *testing.T) { getContent := func(r *renderer, path string) string { for _, f := range r.files { - if f.relPath == path { - b, err := io.ReadAll(f.content) + switch reflect.TypeOf(f) { + case reflect.TypeOf(&inMemoryFile{}): + if f.(*inMemoryFile).relPath != path { + continue + } + content := string(f.(*inMemoryFile).content) + return strings.Trim(content, "\r\n") + case reflect.TypeOf(©File{}): + if f.(*copyFile).relPath != path { + continue + } + srcPath := f.(*copyFile).srcPath + r, err := r.templateFiler.Read(context.Background(), srcPath) + require.NoError(t, err) + b, err := io.ReadAll(r) require.NoError(t, err) return strings.Trim(string(b), "\r\n") + default: + require.FailNow(t, "execution should not reach here") } } require.FailNow(t, "file is absent: "+path) @@ -244,10 +268,7 @@ func TestRendererSkipsDirsEagerly(t *testing.T) { assert.NoError(t, err) assert.Len(t, r.files, 1) - - b, err := io.ReadAll(r.files[0].content) - require.NoError(t, err) - content := string(b) + content := string(r.files[0].(*inMemoryFile).content) assert.Equal(t, "I should be the only file created", strings.Trim(content, "\r\n")) } @@ -320,10 +341,12 @@ func TestRendererInMemoryFileFullPathForWindows(t *testing.T) { t.SkipNow() } f := &inMemoryFile{ - root: `c:\a\b\c`, - relPath: "d/e", + fileCommon: &fileCommon{ + root: `c:\a\b\c`, + relPath: "d/e", + }, } - assert.Equal(t, `c:\a\b\c\d\e`, f.path()) + assert.Equal(t, `c:\a\b\c\d\e`, f.Path()) } func TestRendererInMemoryFilePersistToDiskSetsExecutableBit(t *testing.T) { @@ -333,12 +356,14 @@ func TestRendererInMemoryFilePersistToDiskSetsExecutableBit(t *testing.T) { tmpDir := t.TempDir() f := &inMemoryFile{ - root: tmpDir, - relPath: "a/b/c", - content: io.NopCloser(strings.NewReader("123")), - perm: 0755, + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0755, + }, + content: []byte("123"), } - err := f.persistToDisk() + err := f.PersistToDisk() assert.NoError(t, err) assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") @@ -352,12 +377,14 @@ func TestRendererInMemoryFilePersistToDiskForWindows(t *testing.T) { tmpDir := t.TempDir() f := &inMemoryFile{ - root: tmpDir, - relPath: "a/b/c", - content: io.NopCloser(strings.NewReader("123")), - perm: 0666, + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0666, + }, + content: []byte("123"), } - err := f.persistToDisk() + err := f.PersistToDisk() assert.NoError(t, err) assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") @@ -379,8 +406,18 @@ func TestRendererReadsPermissionsBits(t *testing.T) { getPermissions := func(r *renderer, path string) fs.FileMode { for _, f := range r.files { - if f.relPath == path { - return f.perm + switch reflect.TypeOf(f) { + case reflect.TypeOf(&inMemoryFile{}): + if f.(*inMemoryFile).relPath == path { + return f.(*inMemoryFile).perm + } + + case reflect.TypeOf(©File{}): + if f.(*copyFile).relPath == path { + return f.(*copyFile).perm + } + default: + require.FailNow(t, "execution should not reach here") } } require.FailNow(t, "file is absent: "+path) @@ -402,12 +439,14 @@ func TestRendererErrorOnConflictingFile(t *testing.T) { r := renderer{ skipPatterns: []string{}, - files: []*inMemoryFile{ - { - root: tmpDir, - relPath: "a", - content: io.NopCloser(strings.NewReader("123")), - perm: 0444, + files: []file{ + &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a", + perm: 0444, + }, + content: []byte("123"), }, }, } @@ -427,12 +466,14 @@ func TestRendererNoErrorOnConflictingFileIfSkipped(t *testing.T) { r := renderer{ ctx: ctx, skipPatterns: []string{"a"}, - files: []*inMemoryFile{ - { - root: tmpDir, - relPath: "a", - content: io.NopCloser(strings.NewReader("123")), - perm: 0444, + files: []file{ + &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a", + perm: 0444, + }, + content: []byte("123"), }, }, } From c4eda87184839ee621af2ae642c9598b78eb423d Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 25 Jul 2023 18:08:47 +0200 Subject: [PATCH 03/11] fix test --- libs/template/helpers_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/template/helpers_test.go b/libs/template/helpers_test.go index 51c470efca..8e0561eb8f 100644 --- a/libs/template/helpers_test.go +++ b/libs/template/helpers_test.go @@ -20,7 +20,7 @@ func TestTemplatePrintStringWithoutProcessing(t *testing.T) { assert.NoError(t, err) assert.Len(t, r.files, 1) - cleanContent := strings.Trim(string(r.files[0].content), "\n\r") + cleanContent := strings.Trim(string(r.files[0].(*inMemoryFile).content), "\n\r") assert.Equal(t, `{{ fail "abc" }}`, cleanContent) } @@ -35,7 +35,7 @@ func TestTemplateRegexpCompileFunction(t *testing.T) { assert.NoError(t, err) assert.Len(t, r.files, 1) - content := string(r.files[0].content) + content := string(r.files[0].(*inMemoryFile).content) assert.Contains(t, content, "0:food") assert.Contains(t, content, "1:fool") } From 24f23126c4385be4d9e09134e0b39d39e3384b0d Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 25 Jul 2023 18:28:15 +0200 Subject: [PATCH 04/11] switch to type switch --- libs/template/renderer_test.go | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index cc269e74f0..072eb20666 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -7,7 +7,6 @@ import ( "io/fs" "os" "path/filepath" - "reflect" "runtime" "strings" "testing" @@ -214,19 +213,17 @@ func TestRendererWalk(t *testing.T) { getContent := func(r *renderer, path string) string { for _, f := range r.files { - switch reflect.TypeOf(f) { - case reflect.TypeOf(&inMemoryFile{}): - if f.(*inMemoryFile).relPath != path { + switch v := f.(type) { + case *inMemoryFile: + if v.relPath != path { continue } - content := string(f.(*inMemoryFile).content) - return strings.Trim(content, "\r\n") - case reflect.TypeOf(©File{}): - if f.(*copyFile).relPath != path { + return strings.Trim(string(v.content), "\r\n") + case *copyFile: + if v.relPath != path { continue } - srcPath := f.(*copyFile).srcPath - r, err := r.templateFiler.Read(context.Background(), srcPath) + r, err := r.templateFiler.Read(context.Background(), v.srcPath) require.NoError(t, err) b, err := io.ReadAll(r) require.NoError(t, err) @@ -406,15 +403,15 @@ func TestRendererReadsPermissionsBits(t *testing.T) { getPermissions := func(r *renderer, path string) fs.FileMode { for _, f := range r.files { - switch reflect.TypeOf(f) { - case reflect.TypeOf(&inMemoryFile{}): - if f.(*inMemoryFile).relPath == path { - return f.(*inMemoryFile).perm + switch v := f.(type) { + case *inMemoryFile: + if v.relPath == path { + return v.perm } - case reflect.TypeOf(©File{}): - if f.(*copyFile).relPath == path { - return f.(*copyFile).perm + case *copyFile: + if v.relPath == path { + return v.perm } default: require.FailNow(t, "execution should not reach here") From 7e8e4e2a29634a130738337aaa053cd49a65c995 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 26 Jul 2023 14:58:48 +0200 Subject: [PATCH 05/11] make isSkipped a separate function --- libs/template/file.go | 24 +++++---------- libs/template/renderer.go | 12 ++++---- libs/template/renderer_test.go | 53 +++++++++++++++++----------------- 3 files changed, 40 insertions(+), 49 deletions(-) diff --git a/libs/template/file.go b/libs/template/file.go index 83e94ba69d..41d25a9d89 100644 --- a/libs/template/file.go +++ b/libs/template/file.go @@ -5,7 +5,6 @@ import ( "io" "io/fs" "os" - "path" "path/filepath" "github.com/databricks/cli/libs/filer" @@ -13,12 +12,14 @@ import ( // Interface for an in memory representation of a file type file interface { - // Path file will be persisted at, if PersistToDisk is called + // Full path of the file, in the os native format. For example /foo/bar on + // Unix and C:\foo\bar on windows Path() string - // Does the path of the file, relative to the project root match any of the - // specified skip patterns - IsSkipped(patterns []string) (bool, error) + // Unix like file path relative to the "root" of the instantiated project. Is used to + // evaluate whether the file should be skipped by comparing it to a list of + // skip glob patterns + RelPath() string // This function writes this file onto the disk PersistToDisk() error @@ -42,17 +43,8 @@ func (f *fileCommon) Path() string { return filepath.Join(f.root, filepath.FromSlash(f.relPath)) } -func (f *fileCommon) IsSkipped(patterns []string) (bool, error) { - for _, pattern := range patterns { - isMatch, err := path.Match(pattern, f.relPath) - if err != nil { - return false, err - } - if isMatch { - return true, nil - } - } - return false, nil +func (f *fileCommon) RelPath() string { + return f.relPath } type copyFile struct { diff --git a/libs/template/renderer.go b/libs/template/renderer.go index c09abdac74..b915d7f35d 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -201,11 +201,11 @@ func (r *renderer) walk() error { if err != nil { return err } - isSkipped, err := r.isSkipped(instanceDirectory) + match, err := isSkipped(instanceDirectory, r.skipPatterns) if err != nil { return err } - if isSkipped { + if match { logger.Infof(r.ctx, "skipping directory: %s", instanceDirectory) continue } @@ -263,11 +263,11 @@ func (r *renderer) persistToDisk() error { // any of the skip patterns filesToPersist := make([]file, 0) for _, file := range r.files { - isSkipped, err := file.IsSkipped(r.skipPatterns) + match, err := isSkipped(file.RelPath(), r.skipPatterns) if err != nil { return err } - if isSkipped { + if match { log.Infof(r.ctx, "skipping file: %s", file.Path()) continue } @@ -296,8 +296,8 @@ func (r *renderer) persistToDisk() error { return nil } -func (r *renderer) isSkipped(filePath string) (bool, error) { - for _, pattern := range r.skipPatterns { +func isSkipped(filePath string, patterns []string) (bool, error) { + for _, pattern := range patterns { isMatch, err := path.Match(pattern, filePath) if err != nil { return false, err diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 072eb20666..7521735652 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -90,59 +90,58 @@ My email is {{template "email"}} } func TestRendererIsSkipped(t *testing.T) { - r := renderer{ - skipPatterns: []string{"a*", "*yz", "def", "a/b/*"}, - } + + skipPatterns := []string{"a*", "*yz", "def", "a/b/*"} // skipped paths - isSkipped, err := r.isSkipped("abc") + match, err := isSkipped("abc", skipPatterns) require.NoError(t, err) - assert.True(t, isSkipped) + assert.True(t, match) - isSkipped, err = r.isSkipped("abcd") + match, err = isSkipped("abcd", skipPatterns) require.NoError(t, err) - assert.True(t, isSkipped) + assert.True(t, match) - isSkipped, err = r.isSkipped("a") + match, err = isSkipped("a", skipPatterns) require.NoError(t, err) - assert.True(t, isSkipped) + assert.True(t, match) - isSkipped, err = r.isSkipped("xxyz") + match, err = isSkipped("xxyz", skipPatterns) require.NoError(t, err) - assert.True(t, isSkipped) + assert.True(t, match) - isSkipped, err = r.isSkipped("yz") + match, err = isSkipped("yz", skipPatterns) require.NoError(t, err) - assert.True(t, isSkipped) + assert.True(t, match) - isSkipped, err = r.isSkipped("a/b/c") + match, err = isSkipped("a/b/c", skipPatterns) require.NoError(t, err) - assert.True(t, isSkipped) + assert.True(t, match) // NOT skipped paths - isSkipped, err = r.isSkipped(".") + match, err = isSkipped(".", skipPatterns) require.NoError(t, err) - assert.False(t, isSkipped) + assert.False(t, match) - isSkipped, err = r.isSkipped("y") + match, err = isSkipped("y", skipPatterns) require.NoError(t, err) - assert.False(t, isSkipped) + assert.False(t, match) - isSkipped, err = r.isSkipped("z") + match, err = isSkipped("z", skipPatterns) require.NoError(t, err) - assert.False(t, isSkipped) + assert.False(t, match) - isSkipped, err = r.isSkipped("defg") + match, err = isSkipped("defg", skipPatterns) require.NoError(t, err) - assert.False(t, isSkipped) + assert.False(t, match) - isSkipped, err = r.isSkipped("cat") + match, err = isSkipped("cat", skipPatterns) require.NoError(t, err) - assert.False(t, isSkipped) + assert.False(t, match) - isSkipped, err = r.isSkipped("a/b/c/d") + match, err = isSkipped("a/b/c/d", skipPatterns) require.NoError(t, err) - assert.False(t, isSkipped) + assert.False(t, match) } func TestRendererPersistToDisk(t *testing.T) { From 02e62577227eda769c04f62098f853d25c20b0f8 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 26 Jul 2023 15:35:04 +0200 Subject: [PATCH 06/11] added test --- libs/template/renderer_test.go | 15 +++++++++++++++ .../copy-file-walk/template/not-a-template | 1 + 2 files changed, 16 insertions(+) create mode 100644 libs/template/testdata/copy-file-walk/template/not-a-template diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 7521735652..5ab3294106 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -479,3 +479,18 @@ func TestRendererNoErrorOnConflictingFileIfSkipped(t *testing.T) { assert.NoError(t, err) assert.Len(t, r.files, 1) } + +func TestRendererNonTemplatesAreCreatedAsCopyFiles(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + + r, err := newRenderer(ctx, nil, "./testdata/copy-file-walk/template", "./testdata/copy-file-walk/library", tmpDir) + require.NoError(t, err) + + err = r.walk() + assert.NoError(t, err) + + assert.Len(t, r.files, 1) + assert.Equal(t, r.files[0].(*copyFile).srcPath, "not-a-template") + assert.Equal(t, r.files[0].Path(), filepath.Join(tmpDir, "not-a-template")) +} diff --git a/libs/template/testdata/copy-file-walk/template/not-a-template b/libs/template/testdata/copy-file-walk/template/not-a-template new file mode 100644 index 0000000000..8baef1b4ab --- /dev/null +++ b/libs/template/testdata/copy-file-walk/template/not-a-template @@ -0,0 +1 @@ +abc From 15caf1915a6991c6b9d2e553b73ea56df479c51d Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 26 Jul 2023 18:07:01 +0200 Subject: [PATCH 07/11] added test for file.go --- libs/template/file_test.go | 137 +++++++++++++++++++++++++++++++++ libs/template/renderer_test.go | 41 ---------- 2 files changed, 137 insertions(+), 41 deletions(-) create mode 100644 libs/template/file_test.go diff --git a/libs/template/file_test.go b/libs/template/file_test.go new file mode 100644 index 0000000000..1dcd81d391 --- /dev/null +++ b/libs/template/file_test.go @@ -0,0 +1,137 @@ +package template + +import ( + "context" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/databricks/cli/libs/filer" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTemplateFileCommonPathForWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.SkipNow() + } + f := &fileCommon{ + root: `c:\a\b\c`, + relPath: "d/e", + } + assert.Equal(t, `c:\a\b\c\d\e`, f.Path()) + assert.Equal(t, `d/e`, f.RelPath()) +} + +func TestTemplateFileCommonPathForUnix(t *testing.T) { + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.SkipNow() + } + f := &fileCommon{ + root: `a/b/c`, + relPath: "d/e", + } + assert.Equal(t, `a/b/c/d/e`, f.Path()) + assert.Equal(t, `d/e`, f.RelPath()) +} + +func TestTemplateFileInMemoryFilePersistToDisk(t *testing.T) { + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.SkipNow() + } + tmpDir := t.TempDir() + + f := &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0755, + }, + content: []byte("123"), + } + err := f.PersistToDisk() + assert.NoError(t, err) + + assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") + assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0755) +} + +func TestTemplateFileCopyFilePersistToDisk(t *testing.T) { + if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.SkipNow() + } + tmpDir := t.TempDir() + + templateFiler, err := filer.NewLocalClient(tmpDir) + require.NoError(t, err) + os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), 0644) + + f := ©File{ + ctx: context.Background(), + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0644, + }, + srcPath: "source", + srcFiler: templateFiler, + } + err = f.PersistToDisk() + assert.NoError(t, err) + + assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty") + assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0644) +} + +// we have separate tests for windows because of differences in valid +// fs.FileMode values we can use for different operating systems. +func TestTemplateFileInMemoryFilePersistToDiskForWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.SkipNow() + } + tmpDir := t.TempDir() + + f := &inMemoryFile{ + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0666, + }, + content: []byte("123"), + } + err := f.PersistToDisk() + assert.NoError(t, err) + + assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") + assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0666) +} + +// we have separate tests for windows because of differences in valid +// fs.FileMode values we can use for different operating systems. +func TestTemplateFileCopyFilePersistToDiskForWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.SkipNow() + } + tmpDir := t.TempDir() + + templateFiler, err := filer.NewLocalClient(tmpDir) + require.NoError(t, err) + os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), 0666) + + f := ©File{ + ctx: context.Background(), + fileCommon: &fileCommon{ + root: tmpDir, + relPath: "a/b/c", + perm: 0666, + }, + srcPath: "source", + srcFiler: templateFiler, + } + err = f.PersistToDisk() + assert.NoError(t, err) + + assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty") + assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0666) +} diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 5ab3294106..30d067dea0 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -345,47 +345,6 @@ func TestRendererInMemoryFileFullPathForWindows(t *testing.T) { assert.Equal(t, `c:\a\b\c\d\e`, f.Path()) } -func TestRendererInMemoryFilePersistToDiskSetsExecutableBit(t *testing.T) { - if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { - t.SkipNow() - } - tmpDir := t.TempDir() - - f := &inMemoryFile{ - fileCommon: &fileCommon{ - root: tmpDir, - relPath: "a/b/c", - perm: 0755, - }, - content: []byte("123"), - } - err := f.PersistToDisk() - assert.NoError(t, err) - - assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") - assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0755) -} - -func TestRendererInMemoryFilePersistToDiskForWindows(t *testing.T) { - if runtime.GOOS != "windows" { - t.SkipNow() - } - tmpDir := t.TempDir() - - f := &inMemoryFile{ - fileCommon: &fileCommon{ - root: tmpDir, - relPath: "a/b/c", - perm: 0666, - }, - content: []byte("123"), - } - err := f.PersistToDisk() - assert.NoError(t, err) - - assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") - assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0666) -} func TestRendererReadsPermissionsBits(t *testing.T) { if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { From 14df5ee16f5c0a414126d236bef629553927b18b Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 26 Jul 2023 18:21:07 +0200 Subject: [PATCH 08/11] - --- libs/template/renderer.go | 2 -- libs/template/renderer_test.go | 1 - 2 files changed, 3 deletions(-) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index b915d7f35d..841166129b 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -17,8 +17,6 @@ import ( "golang.org/x/exp/slices" ) -// TODO: add test for paths being executed - const templateExtension = ".tmpl" // Renders a databricks template as a project diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 30d067dea0..89c2424600 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -345,7 +345,6 @@ func TestRendererInMemoryFileFullPathForWindows(t *testing.T) { assert.Equal(t, `c:\a\b\c\d\e`, f.Path()) } - func TestRendererReadsPermissionsBits(t *testing.T) { if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { t.SkipNow() From 131c4a991a344f3b66220069efe47ab77d04f94a Mon Sep 17 00:00:00 2001 From: monalisa Date: Fri, 28 Jul 2023 14:24:33 +0200 Subject: [PATCH 09/11] refactor to have dstPath --- libs/template/file.go | 65 +++++++++-------- libs/template/file_test.go | 126 +++++++++++++-------------------- libs/template/renderer.go | 30 ++++---- libs/template/renderer_test.go | 60 ++++++---------- 4 files changed, 121 insertions(+), 160 deletions(-) diff --git a/libs/template/file.go b/libs/template/file.go index 41d25a9d89..aafb1acfae 100644 --- a/libs/template/file.go +++ b/libs/template/file.go @@ -10,57 +10,55 @@ import ( "github.com/databricks/cli/libs/filer" ) -// Interface for an in memory representation of a file +// Interface representing a file to be materialized from a template into a project +// instance type file interface { - // Full path of the file, in the os native format. For example /foo/bar on - // Unix and C:\foo\bar on windows - Path() string + // Destination path for file. This is where the file will be created when + // PersistToDisk is called. + DstPath() *destinationPath - // Unix like file path relative to the "root" of the instantiated project. Is used to - // evaluate whether the file should be skipped by comparing it to a list of - // skip glob patterns - RelPath() string - - // This function writes this file onto the disk + // Write file to disk at the destination path. PersistToDisk() error } -type fileCommon struct { +type destinationPath struct { // Root path for the project instance. This path uses the system's default // file separator. For example /foo/bar on Unix and C:\foo\bar on windows root string - // Unix like relPath for the file (using '/' as the separator). This path - // is relative to the root. Using unix like relative paths enables skip patterns - // to work across both windows and unix based operating systems. + // Unix like file path relative to the "root" of the instantiated project. Is used to + // evaluate whether the file should be skipped by comparing it to a list of + // skip glob patterns. relPath string - - // Permissions bits for the file - perm fs.FileMode } -func (f *fileCommon) Path() string { +// Absolute path of the file, in the os native format. For example /foo/bar on +// Unix and C:\foo\bar on windows +func (f *destinationPath) absPath() string { return filepath.Join(f.root, filepath.FromSlash(f.relPath)) } -func (f *fileCommon) RelPath() string { - return f.relPath -} - type copyFile struct { - *fileCommon - ctx context.Context - // Path of the source file that should be copied over. - srcPath string + // Permissions bits for the destination file + perm fs.FileMode + + dstPath *destinationPath - // Filer to use to read source path + // Filer rooted at template root. Used to read srcPath. srcFiler filer.Filer + + // Relative path from template root for file to be copied. + srcPath string +} + +func (f *copyFile) DstPath() *destinationPath { + return f.dstPath } func (f *copyFile) PersistToDisk() error { - path := f.Path() + path := f.DstPath().absPath() err := os.MkdirAll(filepath.Dir(path), 0755) if err != nil { return err @@ -80,13 +78,20 @@ func (f *copyFile) PersistToDisk() error { } type inMemoryFile struct { - *fileCommon + dstPath *destinationPath content []byte + + // Permissions bits for the destination file + perm fs.FileMode +} + +func (f *inMemoryFile) DstPath() *destinationPath { + return f.dstPath } func (f *inMemoryFile) PersistToDisk() error { - path := f.Path() + path := f.DstPath().absPath() err := os.MkdirAll(filepath.Dir(path), 0755) if err != nil { diff --git a/libs/template/file_test.go b/libs/template/file_test.go index 1dcd81d391..85938895ef 100644 --- a/libs/template/file_test.go +++ b/libs/template/file_test.go @@ -2,6 +2,7 @@ package template import ( "context" + "io/fs" "os" "path/filepath" "runtime" @@ -12,68 +13,39 @@ import ( "github.com/stretchr/testify/require" ) -func TestTemplateFileCommonPathForWindows(t *testing.T) { - if runtime.GOOS != "windows" { - t.SkipNow() - } - f := &fileCommon{ - root: `c:\a\b\c`, - relPath: "d/e", - } - assert.Equal(t, `c:\a\b\c\d\e`, f.Path()) - assert.Equal(t, `d/e`, f.RelPath()) -} - -func TestTemplateFileCommonPathForUnix(t *testing.T) { - if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { - t.SkipNow() - } - f := &fileCommon{ - root: `a/b/c`, - relPath: "d/e", - } - assert.Equal(t, `a/b/c/d/e`, f.Path()) - assert.Equal(t, `d/e`, f.RelPath()) -} - -func TestTemplateFileInMemoryFilePersistToDisk(t *testing.T) { - if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { - t.SkipNow() - } +func testInMemoryFile(t *testing.T, perm fs.FileMode) { tmpDir := t.TempDir() f := &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "a/b/c", - perm: 0755, }, + perm: perm, content: []byte("123"), } err := f.PersistToDisk() assert.NoError(t, err) assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") - assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0755) + assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm) } -func TestTemplateFileCopyFilePersistToDisk(t *testing.T) { - if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { - t.SkipNow() - } +func testCopyFile(t *testing.T, perm fs.FileMode) { tmpDir := t.TempDir() templateFiler, err := filer.NewLocalClient(tmpDir) require.NoError(t, err) - os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), 0644) + err = os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), perm) + require.NoError(t, err) f := ©File{ ctx: context.Background(), - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "a/b/c", - perm: 0644, }, + perm: perm, srcPath: "source", srcFiler: templateFiler, } @@ -81,57 +53,59 @@ func TestTemplateFileCopyFilePersistToDisk(t *testing.T) { assert.NoError(t, err) assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty") - assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0644) + assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm) } -// we have separate tests for windows because of differences in valid -// fs.FileMode values we can use for different operating systems. -func TestTemplateFileInMemoryFilePersistToDiskForWindows(t *testing.T) { - if runtime.GOOS != "windows" { +func TestTemplateFileDestinationPath(t *testing.T) { + if runtime.GOOS == "windows" { t.SkipNow() } - tmpDir := t.TempDir() + f := &destinationPath{ + root: `a/b/c`, + relPath: "d/e", + } + assert.Equal(t, `a/b/c/d/e`, f.absPath()) +} - f := &inMemoryFile{ - fileCommon: &fileCommon{ - root: tmpDir, - relPath: "a/b/c", - perm: 0666, - }, - content: []byte("123"), +func TestTemplateFileDestinationPathForWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.SkipNow() } - err := f.PersistToDisk() - assert.NoError(t, err) + f := &destinationPath{ + root: `c:\a\b\c`, + relPath: "d/e", + } + assert.Equal(t, `c:\a\b\c\d\e`, f.absPath()) +} - assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123") - assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0666) +func TestTemplateInMemoryFilePersistToDisk(t *testing.T) { + if runtime.GOOS == "windows" { + t.SkipNow() + } + testInMemoryFile(t, 0755) } -// we have separate tests for windows because of differences in valid -// fs.FileMode values we can use for different operating systems. -func TestTemplateFileCopyFilePersistToDiskForWindows(t *testing.T) { +func TestTemplateInMemoryFilePersistToDiskForWindows(t *testing.T) { if runtime.GOOS != "windows" { t.SkipNow() } - tmpDir := t.TempDir() - - templateFiler, err := filer.NewLocalClient(tmpDir) - require.NoError(t, err) - os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), 0666) + // we have separate tests for windows because of differences in valid + // fs.FileMode values we can use for different operating systems. + testInMemoryFile(t, 0666) +} - f := ©File{ - ctx: context.Background(), - fileCommon: &fileCommon{ - root: tmpDir, - relPath: "a/b/c", - perm: 0666, - }, - srcPath: "source", - srcFiler: templateFiler, +func TestTemplateCopyFilePersistToDisk(t *testing.T) { + if runtime.GOOS == "windows" { + t.SkipNow() } - err = f.PersistToDisk() - assert.NoError(t, err) + testCopyFile(t, 0644) +} - assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty") - assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0666) +func TestTemplateCopyFilePersistToDiskForWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.SkipNow() + } + // we have separate tests for windows because of differences in valid + // fs.FileMode values we can use for different operating systems. + testCopyFile(t, 0666) } diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 841166129b..102dfe6b97 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -117,13 +117,6 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { } func (r *renderer) computeFile(relPathTemplate string) (file, error) { - // read template file contents - templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate) - if err != nil { - return nil, err - } - defer templateReader.Close() - // read file permissions info, err := r.templateFiler.Stat(r.ctx, relPathTemplate) if err != nil { @@ -135,17 +128,24 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { // over as is, without treating it as a template if !strings.HasSuffix(relPathTemplate, templateExtension) { return ©File{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: r.instanceRoot, relPath: relPathTemplate, - perm: perm, }, + perm: perm, ctx: r.ctx, srcPath: relPathTemplate, srcFiler: r.templateFiler, }, nil } + // read template file contents + templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate) + if err != nil { + return nil, err + } + defer templateReader.Close() + // execute the contents of the file as a template contentTemplate, err := io.ReadAll(templateReader) if err != nil { @@ -168,11 +168,11 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { } return &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: r.instanceRoot, relPath: relPath, - perm: perm, }, + perm: perm, content: []byte(content), }, nil } @@ -248,7 +248,7 @@ func (r *renderer) walk() error { if err != nil { return err } - logger.Infof(r.ctx, "added file to list of in memory files: %s", f.Path()) + logger.Infof(r.ctx, "added file to list of possible project files: %s", f.DstPath().relPath) r.files = append(r.files, f) } @@ -261,12 +261,12 @@ func (r *renderer) persistToDisk() error { // any of the skip patterns filesToPersist := make([]file, 0) for _, file := range r.files { - match, err := isSkipped(file.RelPath(), r.skipPatterns) + match, err := isSkipped(file.DstPath().relPath, r.skipPatterns) if err != nil { return err } if match { - log.Infof(r.ctx, "skipping file: %s", file.Path()) + log.Infof(r.ctx, "skipping file: %s", file.DstPath()) continue } filesToPersist = append(filesToPersist, file) @@ -274,7 +274,7 @@ func (r *renderer) persistToDisk() error { // Assert no conflicting files exist for _, file := range filesToPersist { - path := file.Path() + path := file.DstPath().absPath() _, err := os.Stat(path) if err == nil { return fmt.Errorf("failed to persist to disk, conflict with existing file: %s", path) diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 89c2424600..8cd89ae99a 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -154,35 +154,35 @@ func TestRendererPersistToDisk(t *testing.T) { skipPatterns: []string{"a/b/c", "mn*"}, files: []file{ &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "a/b/c", - perm: 0444, }, + perm: 0444, content: nil, }, &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "mno", - perm: 0444, }, + perm: 0444, content: nil, }, &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "a/b/d", - perm: 0444, }, + perm: 0444, content: []byte("123"), }, &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "mmnn", - perm: 0444, }, + perm: 0444, content: []byte("456"), }, }, @@ -212,16 +212,13 @@ func TestRendererWalk(t *testing.T) { getContent := func(r *renderer, path string) string { for _, f := range r.files { + if f.DstPath().relPath != path { + continue + } switch v := f.(type) { case *inMemoryFile: - if v.relPath != path { - continue - } return strings.Trim(string(v.content), "\r\n") case *copyFile: - if v.relPath != path { - continue - } r, err := r.templateFiler.Read(context.Background(), v.srcPath) require.NoError(t, err) b, err := io.ReadAll(r) @@ -332,19 +329,6 @@ func TestRendererSkip(t *testing.T) { assert.NoFileExists(t, filepath.Join(tmpDir, "dir2/file6")) } -func TestRendererInMemoryFileFullPathForWindows(t *testing.T) { - if runtime.GOOS != "windows" { - t.SkipNow() - } - f := &inMemoryFile{ - fileCommon: &fileCommon{ - root: `c:\a\b\c`, - relPath: "d/e", - }, - } - assert.Equal(t, `c:\a\b\c\d\e`, f.Path()) -} - func TestRendererReadsPermissionsBits(t *testing.T) { if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { t.SkipNow() @@ -360,16 +344,14 @@ func TestRendererReadsPermissionsBits(t *testing.T) { getPermissions := func(r *renderer, path string) fs.FileMode { for _, f := range r.files { + if f.DstPath().relPath != path { + continue + } switch v := f.(type) { case *inMemoryFile: - if v.relPath == path { - return v.perm - } - + return v.perm case *copyFile: - if v.relPath == path { - return v.perm - } + return v.perm default: require.FailNow(t, "execution should not reach here") } @@ -395,11 +377,11 @@ func TestRendererErrorOnConflictingFile(t *testing.T) { skipPatterns: []string{}, files: []file{ &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "a", - perm: 0444, }, + perm: 0444, content: []byte("123"), }, }, @@ -422,11 +404,11 @@ func TestRendererNoErrorOnConflictingFileIfSkipped(t *testing.T) { skipPatterns: []string{"a"}, files: []file{ &inMemoryFile{ - fileCommon: &fileCommon{ + dstPath: &destinationPath{ root: tmpDir, relPath: "a", - perm: 0444, }, + perm: 0444, content: []byte("123"), }, }, @@ -450,5 +432,5 @@ func TestRendererNonTemplatesAreCreatedAsCopyFiles(t *testing.T) { assert.Len(t, r.files, 1) assert.Equal(t, r.files[0].(*copyFile).srcPath, "not-a-template") - assert.Equal(t, r.files[0].Path(), filepath.Join(tmpDir, "not-a-template")) + assert.Equal(t, r.files[0].DstPath().absPath(), filepath.Join(tmpDir, "not-a-template")) } From 80e69cd84959051f4a31062adf0ee9e3cda1584e Mon Sep 17 00:00:00 2001 From: monalisa Date: Fri, 28 Jul 2023 14:32:09 +0200 Subject: [PATCH 10/11] - --- libs/template/renderer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 102dfe6b97..c7e79841ca 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -139,7 +139,7 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { }, nil } - // read template file contents + // read template file's content templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate) if err != nil { return nil, err From 4199ed3f9b8ab1c664c8b764103349ef930bb3d9 Mon Sep 17 00:00:00 2001 From: monalisa Date: Fri, 28 Jul 2023 14:34:54 +0200 Subject: [PATCH 11/11] - --- libs/template/helpers_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/template/helpers_test.go b/libs/template/helpers_test.go index 90631daf9b..169e06f35b 100644 --- a/libs/template/helpers_test.go +++ b/libs/template/helpers_test.go @@ -52,5 +52,5 @@ func TestTemplateUrlFunction(t *testing.T) { assert.NoError(t, err) assert.Len(t, r.files, 1) - assert.Equal(t, "https://www.databricks.com", string(r.files[0].content)) + assert.Equal(t, "https://www.databricks.com", string(r.files[0].(*inMemoryFile).content)) }