From afe416b52c63ff0d4d18ed879f57a6d7400a04e0 Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Sat, 7 May 2022 05:15:09 +0000 Subject: [PATCH 1/2] fixed testcases to make sure path supporting + a little polishing --- fs.go | 19 ++++++++++++++-- fs_test.go | 24 +++++++++++++++++--- internal/testdata/disk/under/sub/subfile | 1 + internal/testdata/embedded/under/sub/subfile | 1 + 4 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 internal/testdata/disk/under/sub/subfile create mode 100644 internal/testdata/embedded/under/sub/subfile diff --git a/fs.go b/fs.go index e7436c3c5..d861a0ac2 100644 --- a/fs.go +++ b/fs.go @@ -26,13 +26,28 @@ func NewFS(embed fs.ReadDirFS, dir string) FS { } } -// Open implements the FS interface. +// Open opens the named file. +// +// When Open returns an error, it should be of type *PathError with the Op +// field set to "open", the Path field set to name, and the Err field +// describing the problem. +// +// Open should reject attempts to open names that do not satisfy +// ValidPath(name), returning a *PathError with Err set to ErrInvalid or +// ErrNotExist. func (f FS) Open(name string) (fs.File, error) { if name == "embed.go" { - return nil, fs.ErrNotExist + return nil, &fs.PathError{ + Op: "open", + Path: name, + Err: fs.ErrNotExist, + } } file, err := f.getFile(name) if name == "." { + // NOTE: It always returns the root from the "disk" instead + // "embed". However, it could be fine since the the purpose + // of buffalo.FS isn't supporting full featured filesystem. return rootFile{file}, err } return file, err diff --git a/fs_test.go b/fs_test.go index 11d142766..31b6fccb5 100644 --- a/fs_test.go +++ b/fs_test.go @@ -48,6 +48,15 @@ func Test_FS_Prioritizes_Disk(t *testing.T) { r.NoError(err) r.Equal("This file is on disk.", string(b)) + + // should handle slash-separated path for all systems including Windows + f, err = fsys.Open("under/sub/subfile") + r.NoError(err) + + b, err = io.ReadAll(f) + r.NoError(err) + + r.Equal("This file is on disk/sub.", string(b)) } func Test_FS_Uses_Embed_If_No_Disk(t *testing.T) { @@ -63,6 +72,15 @@ func Test_FS_Uses_Embed_If_No_Disk(t *testing.T) { r.NoError(err) r.Equal("This file is embedded.", string(b)) + + // should handle slash-separated path for all systems including Windows + f, err = fsys.Open("under/sub/subfile") + r.NoError(err) + + b, err = io.ReadAll(f) + r.NoError(err) + + r.Equal("This file is on embedded/sub.", string(b)) } func Test_FS_ReadDirFile(t *testing.T) { @@ -87,11 +105,11 @@ func Test_FS_ReadDirFile(t *testing.T) { r.LessOrEqual(len(entries), 1, "a call to ReadDir must at most return n entries") // Second read should return at most 2 files - entries, err = dir.ReadDir(2) + entries, err = dir.ReadDir(3) r.NoError(err) - // The actual len will be 2 (file.txt & file2.txt) - r.LessOrEqual(len(entries), 2, "a call to ReadDir must at most return n entries") + // The actual len will be 2 (file.txt & file2.txt + under/) + r.LessOrEqual(len(entries), 3, "a call to ReadDir must at most return n entries") // trying to read next 2 files (none left) entries, err = dir.ReadDir(2) diff --git a/internal/testdata/disk/under/sub/subfile b/internal/testdata/disk/under/sub/subfile new file mode 100644 index 000000000..64bda002e --- /dev/null +++ b/internal/testdata/disk/under/sub/subfile @@ -0,0 +1 @@ +This file is on disk/sub. \ No newline at end of file diff --git a/internal/testdata/embedded/under/sub/subfile b/internal/testdata/embedded/under/sub/subfile new file mode 100644 index 000000000..354107de1 --- /dev/null +++ b/internal/testdata/embedded/under/sub/subfile @@ -0,0 +1 @@ +This file is on embedded/sub. \ No newline at end of file From faff9e354530afc7e508179a7ae0839da29a637b Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Sat, 7 May 2022 05:16:08 +0000 Subject: [PATCH 2/2] replace filepath with path for URL-ish internal expressions --- render/auto.go | 13 ++++++------- render/template_helpers.go | 7 +++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/render/auto.go b/render/auto.go index 51f1c2e59..63e78db7d 100644 --- a/render/auto.go +++ b/render/auto.go @@ -7,7 +7,6 @@ import ( "io" "net/http" "path" - "path/filepath" "reflect" "strings" @@ -117,10 +116,10 @@ func (ir htmlAutoRenderer) Render(w io.Writer, data Data) error { } if data["method"] == "PUT" { - return ir.HTML(filepath.Join(templatePrefix.String(), "edit.html")).Render(w, data) + return ir.HTML(path.Join(templatePrefix.String(), "edit.html")).Render(w, data) } - return ir.HTML(filepath.Join(templatePrefix.String(), "new.html")).Render(w, data) + return ir.HTML(path.Join(templatePrefix.String(), "new.html")).Render(w, data) } return nil } @@ -128,7 +127,7 @@ func (ir htmlAutoRenderer) Render(w io.Writer, data Data) error { cp, ok := data["current_path"].(string) defCase := func() error { - return ir.HTML(filepath.Join(templatePrefix.String(), "index.html")).Render(w, data) + return ir.HTML(path.Join(templatePrefix.String(), "index.html")).Render(w, data) } if !ok { @@ -136,15 +135,15 @@ func (ir htmlAutoRenderer) Render(w io.Writer, data Data) error { } if strings.HasSuffix(cp, "/edit/") { - return ir.HTML(filepath.Join(templatePrefix.String(), "edit.html")).Render(w, data) + return ir.HTML(path.Join(templatePrefix.String(), "edit.html")).Render(w, data) } if strings.HasSuffix(cp, "/new/") { - return ir.HTML(filepath.Join(templatePrefix.String(), "new.html")).Render(w, data) + return ir.HTML(path.Join(templatePrefix.String(), "new.html")).Render(w, data) } if !isPlural { - return ir.HTML(filepath.Join(templatePrefix.String(), "show.html")).Render(w, data) + return ir.HTML(path.Join(templatePrefix.String(), "show.html")).Render(w, data) } return defCase() diff --git a/render/template_helpers.go b/render/template_helpers.go index 4cd8d567e..b7d177a98 100644 --- a/render/template_helpers.go +++ b/render/template_helpers.go @@ -4,6 +4,7 @@ import ( "encoding/json" "html/template" "io" + "path" "path/filepath" ht "github.com/gobuffalo/helpers/tags" @@ -49,7 +50,7 @@ func assetPathFor(file string) string { if filePath == "" || !ok { filePath = file } - return filepath.ToSlash(filepath.Join("/assets", filePath)) + return path.Join("/assets", filePath) } func loadManifest(manifest io.Reader) error { @@ -59,7 +60,9 @@ func loadManifest(manifest io.Reader) error { return err } for k, v := range m { - assetMap.Store(k, v) + // I don't think v has backslash but if so, correct them when + // creating the map instead using the value in `assetPathFor()`. + assetMap.Store(k, filepath.ToSlash(v)) } return nil }