From 91097856b59ff5134475b2046565cb99bdb882d8 Mon Sep 17 00:00:00 2001
From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
Date: Fri, 2 Jun 2023 12:28:35 +0200
Subject: [PATCH] Add check for path is a directory in filer.ReadDir (#426)

## Tests
Integration tests
---
 internal/filer_test.go               | 22 +++++++++++++++++++++-
 libs/filer/dbfs_client.go            |  4 ++++
 libs/filer/filer.go                  | 12 ++++++++++++
 libs/filer/workspace_files_client.go |  5 +++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/internal/filer_test.go b/internal/filer_test.go
index 0dda1d1b..5037f784 100644
--- a/internal/filer_test.go
+++ b/internal/filer_test.go
@@ -159,6 +159,26 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) {
 	assert.Len(t, entries, 1)
 	assert.Equal(t, "c", entries[0].Name())
 	assert.True(t, entries[0].IsDir())
+
+	// Expect an error trying to call ReadDir on a file
+	_, err = f.ReadDir(ctx, "/hello.txt")
+	assert.ErrorIs(t, err, fs.ErrInvalid)
+
+	// Expect 0 entries for an empty directory
+	err = f.Mkdir(ctx, "empty-dir")
+	require.NoError(t, err)
+	entries, err = f.ReadDir(ctx, "empty-dir")
+	assert.NoError(t, err)
+	assert.Len(t, entries, 0)
+
+	// Expect one entry for a directory with a file in it
+	err = f.Write(ctx, "dir-with-one-file/my-file.txt", strings.NewReader("abc"), filer.CreateParentDirectories)
+	require.NoError(t, err)
+	entries, err = f.ReadDir(ctx, "dir-with-one-file")
+	assert.NoError(t, err)
+	assert.Len(t, entries, 1)
+	assert.Equal(t, entries[0].Name(), "my-file.txt")
+	assert.False(t, entries[0].IsDir())
 }
 
 func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string {
@@ -166,7 +186,7 @@ func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string {
 	me, err := w.CurrentUser.Me(ctx)
 	require.NoError(t, err)
 
-	path := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("integration-test-filer-wsfs-"))
+	path := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("integration-test-wsfs-"))
 
 	// Ensure directory exists, but doesn't exist YET!
 	// Otherwise we could inadvertently remove a directory that already exists on cleanup.
diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go
index c86a80b1..67878136 100644
--- a/libs/filer/dbfs_client.go
+++ b/libs/filer/dbfs_client.go
@@ -222,6 +222,10 @@ func (w *DbfsClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry, e
 		return nil, err
 	}
 
+	if len(res.Files) == 1 && res.Files[0].Path == absPath {
+		return nil, NotADirectory{absPath}
+	}
+
 	info := make([]fs.DirEntry, len(res.Files))
 	for i, v := range res.Files {
 		info[i] = dbfsDirEntry{dbfsFileInfo: dbfsFileInfo{fi: v}}
diff --git a/libs/filer/filer.go b/libs/filer/filer.go
index ff01ea79..58273fff 100644
--- a/libs/filer/filer.go
+++ b/libs/filer/filer.go
@@ -50,6 +50,18 @@ func (err NoSuchDirectoryError) Is(other error) bool {
 	return other == fs.ErrNotExist
 }
 
+type NotADirectory struct {
+	path string
+}
+
+func (err NotADirectory) Error() string {
+	return fmt.Sprintf("not a directory: %s", err.path)
+}
+
+func (err NotADirectory) Is(other error) bool {
+	return other == fs.ErrInvalid
+}
+
 // Filer is used to access files in a workspace.
 // It has implementations for accessing files in WSFS and in DBFS.
 type Filer interface {
diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go
index 967f9a1d..b06a2514 100644
--- a/libs/filer/workspace_files_client.go
+++ b/libs/filer/workspace_files_client.go
@@ -222,6 +222,11 @@ func (w *WorkspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.D
 	objects, err := w.workspaceClient.Workspace.ListAll(ctx, workspace.ListWorkspaceRequest{
 		Path: absPath,
 	})
+
+	if len(objects) == 1 && objects[0].Path == absPath {
+		return nil, NotADirectory{absPath}
+	}
+
 	if err != nil {
 		// If we got an API error we deal with it below.
 		var aerr *apierr.APIError