diff options
author | Will Chandler <wchandler@gitlab.com> | 2022-11-30 19:11:45 +0300 |
---|---|---|
committer | Will Chandler <wchandler@gitlab.com> | 2022-11-30 19:13:01 +0300 |
commit | 594c29414c367a2176b8158fea734b7d9b10f3ec (patch) | |
tree | 9ab18e943c86a1e95a2b427106cf38ec6a9500dd /internal/git2go | |
parent | feee635a4481c884f1aae8093487360772d7c61c (diff) |
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.
Diffstat (limited to 'internal/git2go')
-rw-r--r-- | internal/git2go/commit.go | 105 | ||||
-rw-r--r-- | internal/git2go/commit_test.go | 24 | ||||
-rw-r--r-- | internal/git2go/serialization.go | 7 |
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{}): {}, } |