From 51a69eefafae016dbebab0de3c8ee0240d037865 Mon Sep 17 00:00:00 2001
From: Junjie Gao <junjiegao@microsoft.com>
Date: Wed, 10 Aug 2022 13:54:01 +0800
Subject: [PATCH] fix: dir package optimize

1. GetPath logic: if the given path doesn't exist, return the first
   possible in the union directories, so when create a new file the
   UnionDirFS will select user directory which was provided as the first
   directory when create the UnionDirFS
2. change the constant name

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
---
 dir/fs.go        | 21 +++++++++++++++------
 dir/fs_test.go   |  2 +-
 dir/path.go      | 26 +++++++++++---------------
 dir/path_test.go | 19 ++++---------------
 4 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/dir/fs.go b/dir/fs.go
index f958f0fa..4c6444f3 100644
--- a/dir/fs.go
+++ b/dir/fs.go
@@ -76,12 +76,12 @@ func (u unionDirFS) Open(name string) (fs.File, error) {
 // GetPath returns the path of the named file or directory under the FS
 //
 // if path exists, it returns the first existing path in union directories (dirs)
-// if path doesn't exist, it returns fs.ErrNotExist error
+//
+// if path doesn't exist, it returns the first possible path in the union directories
+// for creating new file and a fs.ErrNotExist error.
 func (u unionDirFS) GetPath(elem ...string) (string, error) {
-	var targetPath string
 	pathSuffix := path.Join(elem...)
 	for _, dir := range u.Dirs {
-		targetPath = filepath.Join(dir.Root(), pathSuffix)
 		_, err := fs.Stat(dir, pathSuffix)
 		if err != nil {
 			if errors.Is(err, fs.ErrNotExist) {
@@ -92,10 +92,19 @@ func (u unionDirFS) GetPath(elem ...string) (string, error) {
 		}
 		// got the first existing file path and break
 		// return the path with current OS separator
-		return targetPath, nil
+		return filepath.Join(dir.Root(), pathSuffix), nil
+	}
+	// if the given path does not exist, return the first possible path in the
+	// union directories for creating new file.
+	var fallbackPath string
+	if len(u.Dirs) == 0 {
+		return "", &fs.PathError{
+			Op:  "getpath",
+			Err: errors.New("the union directory is empty"),
+		}
 	}
-	// return the last possible path for creating new file
-	return targetPath, &fs.PathError{
+	fallbackPath = filepath.Join(u.Dirs[0].Root(), pathSuffix)
+	return fallbackPath, &fs.PathError{
 		Op:   "getpath",
 		Err:  fs.ErrNotExist,
 		Path: pathSuffix,
diff --git a/dir/fs_test.go b/dir/fs_test.go
index 21e2c550..456b19d2 100644
--- a/dir/fs_test.go
+++ b/dir/fs_test.go
@@ -183,7 +183,7 @@ func TestPath(t *testing.T) {
 			path:  []string{"plugin/a/a.exe"},
 		},
 		{name: "Path does not exist",
-			want:  "system/plugin/c/c.exe",
+			want:  "user/plugin/c/c.exe",
 			usrFS: fstest.MapFS{"plugin/a/a.exe": {Data: []byte("user a")}},
 			sysFS: fstest.MapFS{"plugin/a/b.exe": {Data: []byte("system b")}},
 			path:  []string{"plugin/c/c.exe"},
diff --git a/dir/path.go b/dir/path.go
index 42815077..43682a8f 100644
--- a/dir/path.go
+++ b/dir/path.go
@@ -2,21 +2,20 @@ package dir
 
 import (
 	"errors"
-	"fmt"
 	"io/fs"
 
 	"github.com/opencontainers/go-digest"
 )
 
 const (
-	// CertificateExtension defines the extension of the certificate files
-	CertificateExtension = ".crt"
-
 	// ConfigFile is the name of config file
 	ConfigFile = "config.json"
 
-	// KeyExtension defines the extension of the key files
-	KeyExtension = ".key"
+	// LocalCertificateExtension defines the extension of the certificate files
+	LocalCertificateExtension = ".crt"
+
+	// LocalKeyExtension defines the extension of the key files
+	LocalKeyExtension = ".key"
 
 	// LocalKeysDir is the directory name for local key store
 	LocalKeysDir = "localkeys"
@@ -60,17 +59,14 @@ func (p *PathManager) Config() string {
 	return path
 }
 
-// LocalKey returns path of the local private keys or certificate
+// LocalKey returns path of the local private key and it's certificate
 // in the localkeys directory
-//
-// extension: support .crt|.key
-func (p *PathManager) Localkey(name string, extension string) string {
-	if extension != KeyExtension && extension != CertificateExtension {
-		panic(fmt.Sprintf("doesn't support the extension `%s`", extension))
-	}
-	path, err := p.UserConfigFS.GetPath(LocalKeysDir, name+extension)
+func (p *PathManager) Localkey(name string) (keyPath, certPath string) {
+	keyPath, err := p.UserConfigFS.GetPath(LocalKeysDir, name+LocalKeyExtension)
 	checkError(err)
-	return path
+	certPath, err = p.UserConfigFS.GetPath(LocalKeysDir, name+LocalCertificateExtension)
+	checkError(err)
+	return keyPath, certPath
 }
 
 // SigningKeyConfig return the path of signingkeys.json files
diff --git a/dir/path_test.go b/dir/path_test.go
index f7438fb9..ce3d62ca 100644
--- a/dir/path_test.go
+++ b/dir/path_test.go
@@ -137,24 +137,13 @@ func TestPathManager_LocalKey(t *testing.T) {
 			NewRootedFS("/home/exampleuser/.config/notation/", nil),
 		),
 	}
-	localkeyPath := path.Localkey("key1", KeyExtension)
-	if localkeyPath != "/home/exampleuser/.config/notation/localkeys/key1"+KeyExtension {
+	keyPath, certPath := path.Localkey("key1")
+	if keyPath != "/home/exampleuser/.config/notation/localkeys/key1"+LocalKeyExtension {
 		t.Fatal("get Localkey() failed.")
 	}
-}
-
-func TestPathManager_LocalKeyFailed(t *testing.T) {
-	path := &PathManager{
-		UserConfigFS: NewUnionDirFS(
-			NewRootedFS("/home/exampleuser/.config/notation/", nil),
-		),
+	if certPath != "/home/exampleuser/.config/notation/localkeys/key1"+LocalCertificateExtension {
+		t.Fatal("get Localkey() failed.")
 	}
-	defer func() {
-		if d := recover(); d == nil {
-			t.Fatal("get Localkey() extension check failed.")
-		}
-	}()
-	path.Localkey("key1", ".acr")
 }
 
 func TestPathManager_SigningKeyConfig(t *testing.T) {