Revert "Revert "Merge branch 'wc/user-commit-files-structured-errors' into 'master'""

This reverts commit 52c5d3444a671264cda5b5725fb94e626a400a21.

Reintroduce the changes originally merged with MR 4988 that were
subsequently reverted.

3 files changed, 100 insertions, 36 deletions

diff --git a/internal/git2go/commit.go b/internal/git2go/commit.go
index 7197e5a37..2bddad956 100644
--- a/internal/git2go/commit.go
+++ b/internal/git2go/commit.go
@@ -2,44 +2,109 @@ package git2go
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 )
 
-// IndexError is an error that was produced by performing an invalid operation on the index.
-type IndexError string
+// UnknownIndexError is an unspecified error that was produced by performing an invalid operation on the index.
+type UnknownIndexError string
 
-// Error returns the error message of the index error.
-func (err IndexError) Error() string { return string(err) }
+// Error returns the error message of the unknown index error.
+func (err UnknownIndexError) Error() string { return string(err) }
 
-// InvalidArgumentError is returned when an invalid argument is provided.
-type InvalidArgumentError string
+// IndexErrorType specifies which of the known index error types has occurred.
+type IndexErrorType uint
 
-func (err InvalidArgumentError) Error() string { return string(err) }
+const (
+	// ErrDirectoryExists represent a directory exists error.
+	ErrDirectoryExists IndexErrorType = iota
+	// ErrDirectoryTraversal represent a directory traversal error.
+	ErrDirectoryTraversal
+	// ErrEmptyPath represent an empty path error.
+	ErrEmptyPath
+	// ErrFileExists represent a file exists error.
+	ErrFileExists
+	// ErrFileNotFound represent a file not found error.
+	ErrFileNotFound
+	// ErrInvalidPath represent an invalid path error.
+	ErrInvalidPath
+)
 
-// FileNotFoundError is returned when an action attempts to operate on a non-existing file.
-type FileNotFoundError string
+// IndexError is a well-defined error that was produced by performing an invalid operation on the index.
+type IndexError struct {
+	Path string
+	Type IndexErrorType
+}
 
-func (err FileNotFoundError) Error() string {
-	return fmt.Sprintf("file not found: %q", string(err))
+// Error returns the error message associated with the error type.
+func (err IndexError) Error() string {
+	switch err.Type {
+	case ErrDirectoryExists:
+		return "A directory with this name already exists"
+	case ErrDirectoryTraversal:
+		return "Path cannot include directory traversal"
+	case ErrEmptyPath:
+		return "You must provide a file path"
+	case ErrFileExists:
+		return "A file with this name already exists"
+	case ErrFileNotFound:
+		return "A file with this name doesn't exist"
+	case ErrInvalidPath:
+		return fmt.Sprintf("invalid path: '%s'", err.Path)
+	default:
+		panic(fmt.Sprintf("unhandled IndexErrorType: %v", err.Type))
+	}
 }
 
-// FileExistsError is returned when an action attempts to overwrite an existing file.
-type FileExistsError string
+// Proto returns the Protobuf representation of this error.
+func (err IndexError) Proto() *gitalypb.IndexError {
+	errType := gitalypb.IndexError_ERROR_TYPE_UNSPECIFIED
+	switch err.Type {
+	case ErrDirectoryExists:
+		errType = gitalypb.IndexError_ERROR_TYPE_DIRECTORY_EXISTS
+	case ErrDirectoryTraversal:
+		errType = gitalypb.IndexError_ERROR_TYPE_DIRECTORY_TRAVERSAL
+	case ErrEmptyPath:
+		errType = gitalypb.IndexError_ERROR_TYPE_EMPTY_PATH
+	case ErrFileExists:
+		errType = gitalypb.IndexError_ERROR_TYPE_FILE_EXISTS
+	case ErrFileNotFound:
+		errType = gitalypb.IndexError_ERROR_TYPE_FILE_NOT_FOUND
+	case ErrInvalidPath:
+		errType = gitalypb.IndexError_ERROR_TYPE_INVALID_PATH
+	}
 
-func (err FileExistsError) Error() string {
-	return fmt.Sprintf("file exists: %q", string(err))
+	return &gitalypb.IndexError{
+		Path:      []byte(err.Path),
+		ErrorType: errType,
+	}
 }
 
-// DirectoryExistsError is returned when an action attempts to overwrite a directory.
-type DirectoryExistsError string
-
-func (err DirectoryExistsError) Error() string {
-	return fmt.Sprintf("directory exists: %q", string(err))
+// GrpcError returns the error wrapped with a gRPC code.
+func (err IndexError) GrpcError() error {
+	e := errors.New(err.Error())
+	switch err.Type {
+	case ErrDirectoryExists, ErrFileExists:
+		return helper.ErrAlreadyExists(e)
+	case ErrDirectoryTraversal, ErrEmptyPath, ErrInvalidPath:
+		return helper.ErrInvalidArgument(e)
+	case ErrFileNotFound:
+		return helper.ErrNotFound(e)
+	default:
+		return helper.ErrInternal(e)
+	}
 }
 
+// InvalidArgumentError is returned when an invalid argument is provided.
+type InvalidArgumentError string
+
+func (err InvalidArgumentError) Error() string { return string(err) }
+
 // CommitCommand contains the information and the steps to build a commit.
 type CommitCommand struct {
 	// Repository is the path of the repository to operate on.
diff --git a/internal/git2go/commit_test.go b/internal/git2go/commit_test.go
index 711021a10..d153425a2 100644
--- a/internal/git2go/commit_test.go
+++ b/internal/git2go/commit_test.go
@@ -91,7 +91,7 @@ func TestExecutor_Commit(t *testing.T) {
 						CreateDirectory{Path: "directory"},
 						CreateDirectory{Path: "directory"},
 					},
-					error: DirectoryExistsError("directory"),
+					error: IndexError{Type: ErrDirectoryExists, Path: "directory"},
 				},
 			},
 		},
@@ -110,7 +110,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						CreateDirectory{Path: "directory"},
 					},
-					error: DirectoryExistsError("directory"),
+					error: IndexError{Type: ErrDirectoryExists, Path: "directory"},
 				},
 			},
 		},
@@ -129,7 +129,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						CreateDirectory{Path: "file"},
 					},
-					error: FileExistsError("file"),
+					error: IndexError{Type: ErrFileExists, Path: "file"},
 				},
 			},
 		},
@@ -154,7 +154,7 @@ func TestExecutor_Commit(t *testing.T) {
 						CreateFile{Path: "file", OID: originalFile.String()},
 						CreateFile{Path: "file", OID: updatedFile.String()},
 					},
-					error: FileExistsError("file"),
+					error: IndexError{Type: ErrFileExists, Path: "file"},
 				},
 			},
 		},
@@ -214,7 +214,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						UpdateFile{Path: "non-existing", OID: updatedFile.String()},
 					},
-					error: FileNotFoundError("non-existing"),
+					error: IndexError{Type: ErrFileNotFound, Path: "non-existing"},
 				},
 			},
 		},
@@ -240,7 +240,7 @@ func TestExecutor_Commit(t *testing.T) {
 						CreateDirectory{Path: "directory"},
 						MoveFile{Path: "directory", NewPath: "moved-directory"},
 					},
-					error: FileNotFoundError("directory"),
+					error: IndexError{Type: ErrFileNotFound, Path: "directory"},
 				},
 			},
 		},
@@ -272,7 +272,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						MoveFile{Path: "non-existing", NewPath: "destination-file"},
 					},
-					error: FileNotFoundError("non-existing"),
+					error: IndexError{Type: ErrFileNotFound, Path: "non-existing"},
 				},
 			},
 		},
@@ -285,7 +285,7 @@ func TestExecutor_Commit(t *testing.T) {
 						CreateFile{Path: "already-existing", OID: updatedFile.String()},
 						MoveFile{Path: "source-file", NewPath: "already-existing"},
 					},
-					error: FileExistsError("already-existing"),
+					error: IndexError{Type: ErrFileExists, Path: "already-existing"},
 				},
 			},
 		},
@@ -334,7 +334,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						ChangeFileMode{Path: "non-existing"},
 					},
-					error: FileNotFoundError("non-existing"),
+					error: IndexError{Type: ErrFileNotFound, Path: "non-existing"},
 				},
 			},
 		},
@@ -402,7 +402,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						MoveFile{Path: "non-existing", NewPath: "destination"},
 					},
-					error: FileNotFoundError("non-existing"),
+					error: IndexError{Type: ErrFileNotFound, Path: "non-existing"},
 				},
 			},
 		},
@@ -415,7 +415,7 @@ func TestExecutor_Commit(t *testing.T) {
 						CreateFile{Path: "file-2", OID: updatedFile.String()},
 						MoveFile{Path: "file-1", NewPath: "file-2"},
 					},
-					error: FileExistsError("file-2"),
+					error: IndexError{Type: ErrFileExists, Path: "file-2"},
 				},
 			},
 		},
@@ -426,7 +426,7 @@ func TestExecutor_Commit(t *testing.T) {
 					actions: []Action{
 						DeleteFile{Path: "non-existing"},
 					},
-					error: FileNotFoundError("non-existing"),
+					error: IndexError{Type: ErrFileNotFound, Path: "non-existing"},
 				},
 			},
 		},
diff --git a/internal/git2go/serialization.go b/internal/git2go/serialization.go
index 0f4949c5f..becc21537 100644
--- a/internal/git2go/serialization.go
+++ b/internal/git2go/serialization.go
@@ -21,14 +21,13 @@ var registeredTypes = map[reflect.Type]struct{}{
 	reflect.TypeOf(MoveFile{}):               {},
 	reflect.TypeOf(UpdateFile{}):             {},
 	reflect.TypeOf(wrapError{}):              {},
-	reflect.TypeOf(DirectoryExistsError("")): {},
-	reflect.TypeOf(FileExistsError("")):      {},
-	reflect.TypeOf(FileNotFoundError("")):    {},
+	reflect.TypeOf(IndexError{}):             {},
+	reflect.TypeOf(UnknownIndexError("")):    {},
 	reflect.TypeOf(InvalidArgumentError("")): {},
 	reflect.TypeOf(HasConflictsError{}):      {},
 	reflect.TypeOf(ConflictingFilesError{}):  {},
 	reflect.TypeOf(EmptyError{}):             {},
-	reflect.TypeOf(IndexError("")):           {},
+	reflect.TypeOf(UnknownIndexError("")):    {},
 	reflect.TypeOf(ConflictError{}):          {},
 	reflect.TypeOf(CommitNotFoundError{}):    {},
 }