Skip to content

Commit

Permalink
Use filer.Filer to write template instantiation (#1911)
Browse files Browse the repository at this point in the history
## Changes

Prior to this change, the output directory was part of the `renderer`
type and passed down to every `file` it produced. Every file knew its
absolute destination path. This is incompatible with the use of a filer,
where all operations are automatically anchored to some base path.

To make this compatible, this change updates:
* the `file` type to only know its own path relative to the instantiation root,
* the `renderer` type to no longer require or pass along the output directory,
* the `persistToDisk` function to take a context and filer argument,
* the `filer.WriteMode` to represent permission bits

## Tests

* Existing tests pass.
* Manually confirmed template initialization works as expected.
  • Loading branch information
pietern authored Nov 20, 2024
1 parent 4fea021 commit 75b09ff
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 203 deletions.
13 changes: 12 additions & 1 deletion libs/filer/filer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,24 @@ import (
"io/fs"
)

// WriteMode captures intent when writing a file.
//
// The first 9 bits are reserved for the [fs.FileMode] permission bits.
// These are used only by the local filer implementation and have
// no effect for the other implementations.
type WriteMode int

// writeModePerm is a mask to extract permission bits from a WriteMode.
const writeModePerm = WriteMode(fs.ModePerm)

const (
OverwriteIfExists WriteMode = 1 << iota
// Note: these constants are defined as powers of 2 to support combining them using a bit-wise OR.
// They starts from the 10th bit (permission mask + 1) to avoid conflicts with the permission bits.
OverwriteIfExists WriteMode = (writeModePerm + 1) << iota
CreateParentDirectories
)

// DeleteMode captures intent when deleting a file.
type DeleteMode int

const (
Expand Down
12 changes: 12 additions & 0 deletions libs/filer/filer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package filer

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestWriteMode(t *testing.T) {
assert.Equal(t, 512, int(OverwriteIfExists))
assert.Equal(t, 1024, int(CreateParentDirectories))
}
13 changes: 11 additions & 2 deletions libs/filer/local_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,31 @@ func (w *LocalClient) Write(ctx context.Context, name string, reader io.Reader,
return err
}

// Retrieve permission mask from the [WriteMode], if present.
perm := fs.FileMode(0644)
for _, m := range mode {
bits := m & writeModePerm
if bits != 0 {
perm = fs.FileMode(bits)
}
}

flags := os.O_WRONLY | os.O_CREATE
if slices.Contains(mode, OverwriteIfExists) {
flags |= os.O_TRUNC
} else {
flags |= os.O_EXCL
}

f, err := os.OpenFile(absPath, flags, 0644)
f, err := os.OpenFile(absPath, flags, perm)
if errors.Is(err, fs.ErrNotExist) && slices.Contains(mode, CreateParentDirectories) {
// Create parent directories if they don't exist.
err = os.MkdirAll(filepath.Dir(absPath), 0755)
if err != nil {
return err
}
// Try again.
f, err = os.OpenFile(absPath, flags, 0644)
f, err = os.OpenFile(absPath, flags, perm)
}

if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions libs/template/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestTemplateConfigAssignValuesFromDefaultValues(t *testing.T) {
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

r, err := newRenderer(ctx, nil, nil, os.DirFS("."), "./testdata/empty/template", "./testdata/empty/library", t.TempDir())
r, err := newRenderer(ctx, nil, nil, os.DirFS("."), "./testdata/empty/template", "./testdata/empty/library")
require.NoError(t, err)

err = c.assignDefaultValues(r)
Expand All @@ -102,7 +102,7 @@ func TestTemplateConfigAssignValuesFromTemplatedDefaultValues(t *testing.T) {
c, err := newConfig(ctx, os.DirFS(testDir), "schema.json")
require.NoError(t, err)

r, err := newRenderer(ctx, nil, nil, os.DirFS("."), path.Join(testDir, "template/template"), path.Join(testDir, "template/library"), t.TempDir())
r, err := newRenderer(ctx, nil, nil, os.DirFS("."), path.Join(testDir, "template/template"), path.Join(testDir, "template/library"))
require.NoError(t, err)

// Note: only the string value is templated.
Expand Down
84 changes: 26 additions & 58 deletions libs/template/file.go
Original file line number Diff line number Diff line change
@@ -1,53 +1,36 @@
package template

import (
"bytes"
"context"
"io"
"io/fs"
"os"
"path/filepath"
"slices"

"github.com/databricks/cli/libs/filer"
)

// Interface representing a file to be materialized from a template into a project
// instance
type file interface {
// Destination path for file. This is where the file will be created when
// PersistToDisk is called.
DstPath() *destinationPath
// Path of the file relative to the root of the instantiated template.
// This is where the file is written to when persisting the template to disk.
// Must be slash-separated.
RelPath() string

// Write file to disk at the destination path.
PersistToDisk() error
Write(ctx context.Context, out filer.Filer) error

// contents returns the file contents as a byte slice.
// This is used for testing purposes.
contents() ([]byte, error)
}

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 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
}

// 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))
}

type copyFile struct {
ctx context.Context

// Permissions bits for the destination file
perm fs.FileMode

dstPath *destinationPath
// Destination path for the file.
relPath string

// [fs.FS] rooted at template root. Used to read srcPath.
srcFS fs.FS
Expand All @@ -56,55 +39,40 @@ type copyFile struct {
srcPath string
}

func (f *copyFile) DstPath() *destinationPath {
return f.dstPath
func (f *copyFile) RelPath() string {
return f.relPath
}

func (f *copyFile) PersistToDisk() error {
path := f.DstPath().absPath()
err := os.MkdirAll(filepath.Dir(path), 0755)
if err != nil {
return err
}
srcFile, err := f.srcFS.Open(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)
func (f *copyFile) Write(ctx context.Context, out filer.Filer) error {
src, err := f.srcFS.Open(f.srcPath)
if err != nil {
return err
}
defer dstFile.Close()
_, err = io.Copy(dstFile, srcFile)
return err
defer src.Close()
return out.Write(ctx, f.relPath, src, filer.CreateParentDirectories, filer.WriteMode(f.perm))
}

func (f *copyFile) contents() ([]byte, error) {
return fs.ReadFile(f.srcFS, f.srcPath)
}

type inMemoryFile struct {
dstPath *destinationPath

content []byte

// Permissions bits for the destination file
perm fs.FileMode
}

func (f *inMemoryFile) DstPath() *destinationPath {
return f.dstPath
// Destination path for the file.
relPath string

// Contents of the file.
content []byte
}

func (f *inMemoryFile) PersistToDisk() error {
path := f.DstPath().absPath()
func (f *inMemoryFile) RelPath() string {
return f.relPath
}

err := os.MkdirAll(filepath.Dir(path), 0755)
if err != nil {
return err
}
return os.WriteFile(path, f.content, f.perm)
func (f *inMemoryFile) Write(ctx context.Context, out filer.Filer) error {
return out.Write(ctx, f.relPath, bytes.NewReader(f.content), filer.CreateParentDirectories, filer.WriteMode(f.perm))
}

func (f *inMemoryFile) contents() ([]byte, error) {
Expand Down
62 changes: 22 additions & 40 deletions libs/template/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,77 +8,56 @@ import (
"runtime"
"testing"

"github.com/databricks/cli/libs/filer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func testInMemoryFile(t *testing.T, perm fs.FileMode) {
func testInMemoryFile(t *testing.T, ctx context.Context, perm fs.FileMode) {
tmpDir := t.TempDir()

f := &inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "a/b/c",
},
perm: perm,
relPath: "a/b/c",
content: []byte("123"),
}
err := f.PersistToDisk()

out, err := filer.NewLocalClient(tmpDir)
require.NoError(t, err)
err = f.Write(ctx, out)
assert.NoError(t, err)

assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123")
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
}

func testCopyFile(t *testing.T, perm fs.FileMode) {
func testCopyFile(t *testing.T, ctx context.Context, perm fs.FileMode) {
tmpDir := t.TempDir()
err := os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), perm)
require.NoError(t, err)

f := &copyFile{
ctx: context.Background(),
dstPath: &destinationPath{
root: tmpDir,
relPath: "a/b/c",
},
perm: perm,
srcPath: "source",
relPath: "a/b/c",
srcFS: os.DirFS(tmpDir),
srcPath: "source",
}
err = f.PersistToDisk()

out, err := filer.NewLocalClient(tmpDir)
require.NoError(t, err)
err = f.Write(ctx, out)
assert.NoError(t, err)

assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty")
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
}

func TestTemplateFileDestinationPath(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
f := &destinationPath{
root: `a/b/c`,
relPath: "d/e",
}
assert.Equal(t, `a/b/c/d/e`, f.absPath())
}

func TestTemplateFileDestinationPathForWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
}
f := &destinationPath{
root: `c:\a\b\c`,
relPath: "d/e",
}
assert.Equal(t, `c:\a\b\c\d\e`, f.absPath())
}

func TestTemplateInMemoryFilePersistToDisk(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
testInMemoryFile(t, 0755)
ctx := context.Background()
testInMemoryFile(t, ctx, 0755)
}

func TestTemplateInMemoryFilePersistToDiskForWindows(t *testing.T) {
Expand All @@ -87,14 +66,16 @@ func TestTemplateInMemoryFilePersistToDiskForWindows(t *testing.T) {
}
// we have separate tests for windows because of differences in valid
// fs.FileMode values we can use for different operating systems.
testInMemoryFile(t, 0666)
ctx := context.Background()
testInMemoryFile(t, ctx, 0666)
}

func TestTemplateCopyFilePersistToDisk(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
testCopyFile(t, 0644)
ctx := context.Background()
testCopyFile(t, ctx, 0644)
}

func TestTemplateCopyFilePersistToDiskForWindows(t *testing.T) {
Expand All @@ -103,5 +84,6 @@ func TestTemplateCopyFilePersistToDiskForWindows(t *testing.T) {
}
// we have separate tests for windows because of differences in valid
// fs.FileMode values we can use for different operating systems.
testCopyFile(t, 0666)
ctx := context.Background()
testCopyFile(t, ctx, 0666)
}
Loading

0 comments on commit 75b09ff

Please sign in to comment.