config/locator: Use constructor for ErrRepositoryNotFound

We're not using the constructor `NewRepositoryNotFoundError()` when the
repository that we're validating does not exist, but instead construct
our own error that has the full repository path as error metadata. This
has historical reasons because the function used to not have access to
the `storage.Repository` and was thus couldn't use the constructor that
accepts both the storage name and the relative path.

Refactor the function to use the constructor so that errors are more
consistent.

19 files changed, 57 insertions, 64 deletions

diff --git a/internal/git/localrepo/paths_test.go b/internal/git/localrepo/paths_test.go
index b87c56e5c..b33c1444a 100644
--- a/internal/git/localrepo/paths_test.go
+++ b/internal/git/localrepo/paths_test.go
@@ -45,7 +45,7 @@ func TestRepo_Path(t *testing.T) {
 		require.NoError(t, os.RemoveAll(repoPath))
 
 		_, err := repo.Path()
-		require.Equal(t, structerr.NewNotFound("%w", storage.ErrRepositoryNotFound).WithMetadata("repository_path", repoPath), err)
+		require.Equal(t, storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, repoProto.GetRelativePath()), err)
 	})
 
 	t.Run("non-git repository", func(t *testing.T) {
diff --git a/internal/gitaly/config/locator.go b/internal/gitaly/config/locator.go
index 031a66f46..d11392413 100644
--- a/internal/gitaly/config/locator.go
+++ b/internal/gitaly/config/locator.go
@@ -82,7 +82,7 @@ func (l *configLocator) ValidateRepository(repo storage.Repository, opts ...stor
 	if !cfg.SkipRepositoryExistenceCheck {
 		if _, err := os.Stat(path); err != nil {
 			if errors.Is(err, os.ErrNotExist) {
-				return structerr.NewNotFound("%w", storage.ErrRepositoryNotFound).WithMetadata("repository_path", path)
+				return storage.NewRepositoryNotFoundError(repo.GetStorageName(), repo.GetRelativePath())
 			}
 
 			return structerr.New("statting repository: %w", err).WithMetadata("repository_path", path)
diff --git a/internal/gitaly/config/locator_test.go b/internal/gitaly/config/locator_test.go
index be4063670..17de85070 100644
--- a/internal/gitaly/config/locator_test.go
+++ b/internal/gitaly/config/locator_test.go
@@ -71,7 +71,7 @@ func TestConfigLocator_GetRepoPath(t *testing.T) {
 		{
 			desc:   "unknown relative path",
 			repo:   &gitalypb.Repository{StorageName: storageName, RelativePath: "invalid"},
-			expErr: structerr.NewNotFound("%w", storage.ErrRepositoryNotFound).WithMetadata("repository_path", filepath.Join(cfg.Storages[0].Path, "invalid")),
+			expErr: storage.NewRepositoryNotFoundError(storageName, "invalid"),
 		},
 		{
 			desc: "path exists but not a git repository",
diff --git a/internal/gitaly/service/blob/get_blob_test.go b/internal/gitaly/service/blob/get_blob_test.go
index e9ba980fc..f61c9cd5a 100644
--- a/internal/gitaly/service/blob/get_blob_test.go
+++ b/internal/gitaly/service/blob/get_blob_test.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -192,9 +191,8 @@ func TestGetBlob_invalidRequest(t *testing.T) {
 				Oid: oid,
 			},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("repository not found"),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "path"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "path")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/commit/isancestor_test.go b/internal/gitaly/service/commit/isancestor_test.go
index 2dd6203d0..8821b29d2 100644
--- a/internal/gitaly/service/commit/isancestor_test.go
+++ b/internal/gitaly/service/commit/isancestor_test.go
@@ -63,9 +63,8 @@ func TestCommitIsAncestorFailure(t *testing.T) {
 				ChildId:    "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab",
 			},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "fake-path"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "fake-path")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/commit/stats_test.go b/internal/gitaly/service/commit/stats_test.go
index 41949351f..9310d6379 100644
--- a/internal/gitaly/service/commit/stats_test.go
+++ b/internal/gitaly/service/commit/stats_test.go
@@ -3,7 +3,6 @@
 package commit
 
 import (
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -107,9 +106,8 @@ func TestCommitStatsFailure(t *testing.T) {
 				Revision: []byte("test-do-not-touch"),
 			},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "bar.git"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "bar.git")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/diff/find_changed_paths_test.go b/internal/gitaly/service/diff/find_changed_paths_test.go
index ef31370f5..879af0eb2 100644
--- a/internal/gitaly/service/diff/find_changed_paths_test.go
+++ b/internal/gitaly/service/diff/find_changed_paths_test.go
@@ -2,7 +2,6 @@ package diff
 
 import (
 	"io"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -971,9 +970,8 @@ func TestFindChangedPathsRequest_failing(t *testing.T) {
 			repo:    &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "bar.git"},
 			commits: []string{newCommit.String(), oldCommit.String()},
 			err: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "bar.git"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "bar.git")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/diff/numstat_test.go b/internal/gitaly/service/diff/numstat_test.go
index a54d813b1..c92eddb19 100644
--- a/internal/gitaly/service/diff/numstat_test.go
+++ b/internal/gitaly/service/diff/numstat_test.go
@@ -4,7 +4,6 @@ package diff
 
 import (
 	"io"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -148,9 +147,8 @@ func TestFailedDiffStatsRequest(t *testing.T) {
 			leftCommitID:  "e4003da16c1c2c3fc4567700121b17bf8e591c6c",
 			rightCommitID: "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab",
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "bar.git"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "bar.git")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go
index 6cb07dff7..a1e4f94d7 100644
--- a/internal/gitaly/service/operations/apply_patch_test.go
+++ b/internal/gitaly/service/operations/apply_patch_test.go
@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 	"testing/iotest"
@@ -92,8 +91,8 @@ To restore the original branch and stop patching, run "git am --abort".
 			targetBranch:          git.DefaultBranch,
 			nonExistentRepository: true,
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound), "repository_path", filepath.Join(cfg.Storages[0].Path, "doesnt-exist"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "doesnt-exist")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/ref/find_refs_by_oid_test.go b/internal/gitaly/service/ref/find_refs_by_oid_test.go
index a1fa8b376..31aa0ef41 100644
--- a/internal/gitaly/service/ref/find_refs_by_oid_test.go
+++ b/internal/gitaly/service/ref/find_refs_by_oid_test.go
@@ -168,9 +168,17 @@ func TestFindRefsByOID_failure(t *testing.T) {
 				return &gitalypb.FindRefsByOIDRequest{
 						Repository: repo,
 						Oid:        oid.String(),
-					}, testhelper.WithInterceptedMetadata(
-						structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-						"repository_path", repoPath,
+					}, testhelper.GitalyOrPraefect(
+						testhelper.ToInterceptedMetadata(
+							structerr.New("%w", storage.NewRepositoryNotFoundError(repo.GetStorageName(), repo.GetRelativePath())),
+						),
+						// Note that Praefect reports the _rewritten_ repository path as not found. This is expected
+						// given that the repository does exist in Praefect, but is missing in Gitaly.
+						testhelper.ToInterceptedMetadata(
+							structerr.New("%w", storage.NewRepositoryNotFoundError(
+								repo.GetStorageName(), gittest.GetReplicaPath(t, ctx, cfg, repo),
+							)),
+						),
 					)
 			},
 		},
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index ed25198a6..6885fb032 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -5,7 +5,6 @@ package ref
 import (
 	"context"
 	"io"
-	"path/filepath"
 	"strings"
 	"testing"
 
@@ -185,9 +184,8 @@ func TestFindDefaultBranchName(t *testing.T) {
 						Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "made/up/path"},
 					},
 					expectedErr: testhelper.GitalyOrPraefect(
-						testhelper.WithInterceptedMetadata(
-							structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-							"repository_path", filepath.Join(cfg.Storages[0].Path, "made/up/path"),
+						testhelper.ToInterceptedMetadata(
+							structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "made/up/path")),
 						),
 						testhelper.ToInterceptedMetadata(
 							structerr.New(
@@ -496,9 +494,8 @@ func TestFindLocalBranches_validate(t *testing.T) {
 			desc: "repository doesn't exist on disk",
 			repo: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "made/up/path"},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "made/up/path"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "made/up/path")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/repository/apply_gitattributes_test.go b/internal/gitaly/service/repository/apply_gitattributes_test.go
index ac8d3e95d..25eb55132 100644
--- a/internal/gitaly/service/repository/apply_gitattributes_test.go
+++ b/internal/gitaly/service/repository/apply_gitattributes_test.go
@@ -273,9 +273,8 @@ func TestApplyGitattributes_failure(t *testing.T) {
 			},
 			revision: []byte("master"),
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "bar"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "bar")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/repository/fetch_bundle_test.go b/internal/gitaly/service/repository/fetch_bundle_test.go
index 139b6e020..04f1e48eb 100644
--- a/internal/gitaly/service/repository/fetch_bundle_test.go
+++ b/internal/gitaly/service/repository/fetch_bundle_test.go
@@ -156,9 +156,8 @@ func TestServer_FetchBundle_validation(t *testing.T) {
 				},
 			},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "unknown"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "unknown")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
diff --git a/internal/gitaly/service/repository/object_format_test.go b/internal/gitaly/service/repository/object_format_test.go
index 17da77d50..247a6b7f2 100644
--- a/internal/gitaly/service/repository/object_format_test.go
+++ b/internal/gitaly/service/repository/object_format_test.go
@@ -87,9 +87,8 @@ func TestObjectFormat(t *testing.T) {
 						},
 					},
 					expectedErr: testhelper.GitalyOrPraefect(
-						testhelper.WithInterceptedMetadata(
-							structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-							"repository_path", filepath.Join(cfg.Storages[0].Path, "nonexistent.git"),
+						testhelper.ToInterceptedMetadata(
+							structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "nonexistent.git")),
 						),
 						testhelper.ToInterceptedMetadata(
 							structerr.New(
diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go
index 1b549026d..1a211a9b4 100644
--- a/internal/gitaly/service/repository/optimize_test.go
+++ b/internal/gitaly/service/repository/optimize_test.go
@@ -370,9 +370,8 @@ func TestOptimizeRepository_validation(t *testing.T) {
 				},
 			},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "path/not/exist"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "path/not/exist")),
 				),
 				structerr.NewNotFound("routing repository maintenance: getting repository metadata: %w", storage.ErrRepositoryNotFound),
 			),
diff --git a/internal/gitaly/service/repository/prune_unreachable_objects_test.go b/internal/gitaly/service/repository/prune_unreachable_objects_test.go
index a921a9842..101ad074c 100644
--- a/internal/gitaly/service/repository/prune_unreachable_objects_test.go
+++ b/internal/gitaly/service/repository/prune_unreachable_objects_test.go
@@ -43,13 +43,18 @@ func TestPruneUnreachableObjects(t *testing.T) {
 		_, err := client.PruneUnreachableObjects(ctx, &gitalypb.PruneUnreachableObjectsRequest{
 			Repository: repo,
 		})
-		testhelper.RequireGrpcError(t,
-			testhelper.WithInterceptedMetadata(
-				structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-				"repository_path", repoPath,
+		testhelper.RequireGrpcError(t, testhelper.GitalyOrPraefect(
+			testhelper.ToInterceptedMetadata(
+				structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, repo.GetRelativePath())),
 			),
-			err,
-		)
+			// Note that Praefect reports the _rewritten_ repository path as not found. This is expected
+			// given that the repository does exist in Praefect, but is missing in Gitaly.
+			testhelper.ToInterceptedMetadata(
+				structerr.New("%w", storage.NewRepositoryNotFoundError(
+					cfg.Storages[0].Name, gittest.GetReplicaPath(t, ctx, cfg, repo),
+				)),
+			),
+		), err)
 	})
 
 	t.Run("empty repository", func(t *testing.T) {
diff --git a/internal/gitaly/service/repository/rename_test.go b/internal/gitaly/service/repository/rename_test.go
index 4d56d21e6..fc5e419d1 100644
--- a/internal/gitaly/service/repository/rename_test.go
+++ b/internal/gitaly/service/repository/rename_test.go
@@ -124,9 +124,8 @@ func TestRenameRepositoryInvalidRequest(t *testing.T) {
 			desc: "repository relative path doesn't exist",
 			req:  &gitalypb.RenameRepositoryRequest{Repository: &gitalypb.Repository{StorageName: repo.StorageName, RelativePath: "stub"}, RelativePath: "non-existent/directory"},
 			exp: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "stub"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "stub")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.NewNotFound("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "stub")),
diff --git a/internal/gitaly/service/repository/repository_info_test.go b/internal/gitaly/service/repository/repository_info_test.go
index b925e8314..c3a6c2d09 100644
--- a/internal/gitaly/service/repository/repository_info_test.go
+++ b/internal/gitaly/service/repository/repository_info_test.go
@@ -74,9 +74,8 @@ func TestRepositoryInfo(t *testing.T) {
 						},
 					},
 					expectedErr: testhelper.GitalyOrPraefect(
-						testhelper.WithInterceptedMetadata(
-							structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-							"repository_path", filepath.Join(cfg.Storages[0].Path, "does", "not", "exist"),
+						testhelper.ToInterceptedMetadata(
+							structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "does/not/exist")),
 						),
 						testhelper.ToInterceptedMetadata(
 							structerr.New(
diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go
index d2e894c1d..7c5e0df2d 100644
--- a/internal/gitaly/service/smarthttp/inforefs_test.go
+++ b/internal/gitaly/service/smarthttp/inforefs_test.go
@@ -151,8 +151,8 @@ func TestInfoRefsUploadPack_validate(t *testing.T) {
 				RelativePath: "doesnt/exist",
 			}},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound), "repository_path", filepath.Join(cfg.Storages[0].Path, "doesnt/exist"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "doesnt/exist")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(
@@ -345,9 +345,8 @@ func TestInfoRefsReceivePack_validate(t *testing.T) {
 				RelativePath: "testdata/scratch/another_repo",
 			}},
 			expectedErr: testhelper.GitalyOrPraefect(
-				testhelper.WithInterceptedMetadata(
-					structerr.NewNotFound("%w", storage.ErrRepositoryNotFound),
-					"repository_path", filepath.Join(cfg.Storages[0].Path, "testdata/scratch/another_repo"),
+				testhelper.ToInterceptedMetadata(
+					structerr.New("%w", storage.NewRepositoryNotFoundError(cfg.Storages[0].Name, "testdata/scratch/another_repo")),
 				),
 				testhelper.ToInterceptedMetadata(
 					structerr.New(