Skip to content

Commit d953eca

Browse files
authored
Merge pull request #726 from monopole/refactorLoader
Add more coverage for loader and strengthen type safety
2 parents 6651e48 + fd3cd47 commit d953eca

File tree

10 files changed

+278
-103
lines changed

10 files changed

+278
-103
lines changed

pkg/fs/confirmeddir.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package fs
18+
19+
import (
20+
"path/filepath"
21+
"strings"
22+
)
23+
24+
// ConfirmedDir is a clean, absolute, delinkified path
25+
// that was confirmed to point to an existing directory.
26+
type ConfirmedDir string
27+
28+
// HasPrefix returns true if the directory argument
29+
// is a prefix of self (d) from the point of view of
30+
// a file system.
31+
//
32+
// I.e., it's true if the argument equals or contains
33+
// self (d) in a file path sense.
34+
//
35+
// HasPrefix emulates the semantics of strings.HasPrefix
36+
// such that the following are true:
37+
//
38+
// strings.HasPrefix("foobar", "foobar")
39+
// strings.HasPrefix("foobar", "foo")
40+
// strings.HasPrefix("foobar", "")
41+
//
42+
// d := fSys.ConfirmDir("/foo/bar")
43+
// d.HasPrefix("/foo/bar")
44+
// d.HasPrefix("/foo")
45+
// d.HasPrefix("/")
46+
//
47+
// Not contacting a file system here to check for
48+
// actual path existence.
49+
//
50+
// This is tested on linux, but will have trouble
51+
// on other operating systems.
52+
// TODO(monopole) Refactor when #golang/go/18358 closes.
53+
// See also:
54+
// https://github.com/golang/go/issues/18358
55+
// https://github.com/golang/dep/issues/296
56+
// https://github.com/golang/dep/blob/master/internal/fs/fs.go#L33
57+
// https://codereview.appspot.com/5712045
58+
func (d ConfirmedDir) HasPrefix(path ConfirmedDir) bool {
59+
if path.String() == string(filepath.Separator) || path == d {
60+
return true
61+
}
62+
return strings.HasPrefix(
63+
string(d),
64+
string(path)+string(filepath.Separator))
65+
}
66+
67+
func (d ConfirmedDir) Join(path string) string {
68+
return filepath.Join(string(d), path)
69+
}
70+
71+
func (d ConfirmedDir) String() string {
72+
return string(d)
73+
}

pkg/fs/confirmeddir_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package fs
18+
19+
import (
20+
"testing"
21+
)
22+
23+
func TestJoin(t *testing.T) {
24+
fSys := MakeFakeFS()
25+
err := fSys.Mkdir("/foo")
26+
if err != nil {
27+
t.Fatalf("unexpected err: %v", err)
28+
}
29+
d, f, err := fSys.CleanedAbs("/foo")
30+
if err != nil {
31+
t.Fatalf("unexpected err: %v", err)
32+
}
33+
if f != "" {
34+
t.Fatalf("unexpected file: %v", f)
35+
}
36+
if d.Join("bar") != "/foo/bar" {
37+
t.Fatalf("expected join %s", d.Join("bar"))
38+
}
39+
}
40+
41+
func TestHasPrefix_Slash(t *testing.T) {
42+
d, f, err := MakeFakeFS().CleanedAbs("/")
43+
if err != nil {
44+
t.Fatalf("unexpected err: %v", err)
45+
}
46+
if f != "" {
47+
t.Fatalf("unexpected file: %v", f)
48+
}
49+
if d.HasPrefix("/hey") {
50+
t.Fatalf("should be false")
51+
}
52+
if !d.HasPrefix("/") {
53+
t.Fatalf("/ should have the prefix /")
54+
}
55+
}
56+
57+
func TestHasPrefix_SlashFoo(t *testing.T) {
58+
fSys := MakeFakeFS()
59+
err := fSys.Mkdir("/foo")
60+
if err != nil {
61+
t.Fatalf("unexpected err: %v", err)
62+
}
63+
d, _, err := fSys.CleanedAbs("/foo")
64+
if err != nil {
65+
t.Fatalf("unexpected err: %v", err)
66+
}
67+
if d.HasPrefix("/fo") {
68+
t.Fatalf("/foo does not have path prefix /fo")
69+
}
70+
if d.HasPrefix("/fod") {
71+
t.Fatalf("/foo does not have path prefix /fod")
72+
}
73+
if !d.HasPrefix("/foo") {
74+
t.Fatalf("/foo should have prefix /foo")
75+
}
76+
}
77+
78+
func TestHasPrefix_SlashFooBar(t *testing.T) {
79+
fSys := MakeFakeFS()
80+
err := fSys.MkdirAll("/foo/bar")
81+
if err != nil {
82+
t.Fatalf("unexpected err: %v", err)
83+
}
84+
d, _, err := fSys.CleanedAbs("/foo/bar")
85+
if err != nil {
86+
t.Fatalf("unexpected err: %v", err)
87+
}
88+
if d.HasPrefix("/fo") {
89+
t.Fatalf("/foo/bar does not have path prefix /fo")
90+
}
91+
if d.HasPrefix("/foobar") {
92+
t.Fatalf("/foo/bar does not have path prefix /foobar")
93+
}
94+
if !d.HasPrefix("/foo/bar") {
95+
t.Fatalf("/foo/bar should have prefix /foo/bar")
96+
}
97+
if !d.HasPrefix("/foo") {
98+
t.Fatalf("/foo/bar should have prefix /foo")
99+
}
100+
if !d.HasPrefix("/") {
101+
t.Fatalf("/foo/bar should have prefix /")
102+
}
103+
}

pkg/fs/fakefs.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,16 @@ func (fs *fakeFs) Open(name string) (File, error) {
102102
return fs.m[name], nil
103103
}
104104

105-
// CleanedAbs does nothing and cannot fail.
106-
func (fs *fakeFs) CleanedAbs(path string) (string, string, error) {
105+
// CleanedAbs cannot fail.
106+
func (fs *fakeFs) CleanedAbs(path string) (ConfirmedDir, string, error) {
107107
if fs.IsDir(path) {
108-
return path, "", nil
108+
return ConfirmedDir(path), "", nil
109109
}
110110
d := filepath.Dir(path)
111111
if d == path {
112-
return d, "", nil
112+
return ConfirmedDir(d), "", nil
113113
}
114-
return d, filepath.Base(path), nil
114+
return ConfirmedDir(d), filepath.Base(path), nil
115115
}
116116

117117
// Exists returns true if file is known.

pkg/fs/fs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type FileSystem interface {
3030
RemoveAll(name string) error
3131
Open(name string) (File, error)
3232
IsDir(name string) bool
33-
CleanedAbs(path string) (string, string, error)
33+
CleanedAbs(path string) (ConfirmedDir, string, error)
3434
Exists(name string) bool
3535
Glob(pattern string) ([]string, error)
3636
ReadFile(name string) ([]byte, error)

pkg/fs/realfs.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ func (realFS) Open(name string) (File, error) { return os.Open(name) }
6060
// and file components. If the entire path is
6161
// a directory, the file component is an empty
6262
// string.
63-
func (x realFS) CleanedAbs(path string) (string, string, error) {
63+
func (x realFS) CleanedAbs(
64+
path string) (ConfirmedDir, string, error) {
6465
absRoot, err := filepath.Abs(path)
6566
if err != nil {
6667
return "", "", fmt.Errorf(
@@ -72,7 +73,7 @@ func (x realFS) CleanedAbs(path string) (string, string, error) {
7273
"evalsymlink failure on '%s' : %v", path, err)
7374
}
7475
if x.IsDir(deLinked) {
75-
return deLinked, "", nil
76+
return ConfirmedDir(deLinked), "", nil
7677
}
7778
d := filepath.Dir(deLinked)
7879
if !x.IsDir(d) {
@@ -89,7 +90,7 @@ func (x realFS) CleanedAbs(path string) (string, string, error) {
8990
log.Fatalf("these should be equal: '%s', '%s'",
9091
filepath.Join(d, f), deLinked)
9192
}
92-
return d, f, nil
93+
return ConfirmedDir(d), f, nil
9394
}
9495

9596
// Exists returns true if os.Stat succeeds.

pkg/fs/realfs_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func TestCleanedAbs_1(t *testing.T) {
5252
if err != nil {
5353
t.Fatalf("unexpected err=%v", err)
5454
}
55-
if d != wd {
55+
if d.String() != wd {
5656
t.Fatalf("unexpected d=%s", d)
5757
}
5858
if f != "" {
@@ -90,7 +90,7 @@ func TestCleanedAbs_3(t *testing.T) {
9090
if err != nil {
9191
t.Fatalf("unexpected err=%v", err)
9292
}
93-
if d != testDir {
93+
if d.String() != testDir {
9494
t.Fatalf("unexpected d=%s", d)
9595
}
9696
if f != "foo" {
@@ -119,7 +119,7 @@ func TestCleanedAbs_4(t *testing.T) {
119119
if err != nil {
120120
t.Fatalf("unexpected err=%v", err)
121121
}
122-
if d != filepath.Join(testDir, "d1", "d2") {
122+
if d.String() != filepath.Join(testDir, "d1", "d2") {
123123
t.Fatalf("unexpected d=%s", d)
124124
}
125125
if f != "" {
@@ -131,7 +131,7 @@ func TestCleanedAbs_4(t *testing.T) {
131131
if err != nil {
132132
t.Fatalf("unexpected err=%v", err)
133133
}
134-
if d != filepath.Join(testDir, "d1", "d2") {
134+
if d.String() != filepath.Join(testDir, "d1", "d2") {
135135
t.Fatalf("unexpected d=%s", d)
136136
}
137137
if f != "bar" {

0 commit comments

Comments
 (0)