Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 814aceb

Browse files
authored
feat(search): Make search aware of perforce changelist id mapping (#63563)
https://linear.app/sourcegraph/issue/SPLF-116/perforce-searching-by-perforce-changelist-id ## Details We have had requests from our customers using Perforce to be able to search inside of a changelist id. For a commit sha we support doing this ``` context:global repo:^perforce-sgdev-org/rhia-depot-test$@345c17c` some text ``` But for perforce they want to do the same thing but with the change list ids. Which would look like this ``` context:global repo:^perforce-sgdev-org/rhia-depot-test$@changelist/83732` some text ``` To support this, I am attempting to smartly detect when we should do a DB round trip and look up the proper mapping. I built a simple heuristic that is 1. Is perforce changelist mapping feature flag enabled 2. Is this a perforce repo? 3. Is the revision request a integer ? This mapping is just a best effort, if it fails it just falls back on current behavior. We are doing with a syntax of `@changelist/CL_ID` instead of supporting `@CL_ID` to future proof us. This lookup focuses on finding the mapping in the DB but in the future we may want to pre-create these refs in the db duing mapping of perforce CLs to git commits. ## Limitations This works well in testing however, the repo name@changelist/rev we return contains the sha ![image](https://github.com/sourcegraph/sourcegraph/assets/304410/a673b9bd-d11f-4b36-bd95-c21ab8a5c4af) I investigated changing this but it would required a larger change in resolving the stream results. While that would be nice to have, I decided to keep this minimal for now and add that later if needed ## Test plan <!-- REQUIRED; info at https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles --> ## Changelog - For perforce depots, support searching within a specific changelist by specifying a ref like `context:global repo:^repo/name$@changelist/83854`
1 parent 64ebfca commit 814aceb

File tree

13 files changed

+643
-14
lines changed

13 files changed

+643
-14
lines changed

.vscode/settings.json

-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
"editor.formatOnSave": true,
3737
"go.useLanguageServer": true,
3838
"gopls": {
39-
"build.allowImplicitNetworkAccess": true,
4039
"local": "github.com/sourcegraph/sourcegraph"
4140
},
4241
"npm.packageManager": "pnpm",

client/web/src/repo/commits/GitCommitNode.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ export const GitCommitNode: React.FunctionComponent<React.PropsWithChildren<GitC
271271

272272
const treeCanonicalURL =
273273
isPerforceChangelistMappingEnabled() && isPerforceDepot
274-
? node.tree.canonicalURL.replace(node.oid, refID)
274+
? node.tree.canonicalURL.replace(node.oid, `changelist/${refID}`)
275275
: node.tree.canonicalURL
276276

277277
const viewFilesCommitElement = node.tree && (

cmd/frontend/graphqlbackend/repository.go

+3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"net/url"
77
"strconv"
8+
"strings"
89
"sync"
910
"time"
1011

@@ -318,6 +319,8 @@ func (r *RepositoryResolver) Changelist(ctx context.Context, args *RepositoryCha
318319
attribute.String("changelist", args.CID))
319320
defer tr.EndWithErr(&err)
320321

322+
// Strip "changelist/" prefix if present since we will sometimes append it in the UI.
323+
args.CID = strings.TrimPrefix(args.CID, "changelist/")
321324
cid, err := strconv.ParseInt(args.CID, 10, 64)
322325
if err != nil {
323326
// NOTE: From the UI, the user may visit a URL like:

cmd/frontend/graphqlbackend/repository_test.go

+30
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,36 @@ func TestRepository_Changelist(t *testing.T) {
111111
"canonicalURL": "/perforce.sgdev.org/gorilla/mux/-/changelist/123",
112112
"commit": {
113113
"oid": "%s"
114+
}
115+
}
116+
}
117+
}
118+
`, exampleCommitSHA1),
119+
})
120+
121+
RunTest(t, &Test{
122+
Schema: mustParseGraphQLSchema(t, db),
123+
Query: `
124+
{
125+
repository(name: "perforce.sgdev.org/gorilla/mux") {
126+
changelist(cid: "changelist/123") {
127+
cid
128+
canonicalURL
129+
commit {
130+
oid
131+
}
132+
}
133+
}
134+
}
135+
`,
136+
ExpectedResult: fmt.Sprintf(`
137+
{
138+
"repository": {
139+
"changelist": {
140+
"cid": "123",
141+
"canonicalURL": "/perforce.sgdev.org/gorilla/mux/-/changelist/123",
142+
"commit": {
143+
"oid": "%s"
114144
}
115145
}
116146
}

internal/database/dbmocks/mocks_temp.go

+138
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/database/repo_commits_changelists.go

+67-1
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@ type RepoCommitsChangelistsStore interface {
2222
// GetLatestForRepo will return the latest commit that has been mapped in the database.
2323
GetLatestForRepo(ctx context.Context, repoID api.RepoID) (*types.RepoCommit, error)
2424

25-
// GetRepoCommit will return the mathcing row from the table for the given repo ID and the
25+
// GetRepoCommit will return the matching row from the table for the given repo ID and the
2626
// given changelist ID.
2727
GetRepoCommitChangelist(ctx context.Context, repoID api.RepoID, changelistID int64) (*types.RepoCommit, error)
28+
29+
// BatchGetRepoCommitChangelist bulk loads repo commits for given repo ids and changelistIds
30+
BatchGetRepoCommitChangelist(ctx context.Context, rcs ...RepoChangelistIDs) (map[api.RepoID]map[int64]*types.RepoCommit, error)
2831
}
2932

3033
type repoCommitsChangelistsStore struct {
@@ -119,3 +122,66 @@ func (s *repoCommitsChangelistsStore) GetRepoCommitChangelist(ctx context.Contex
119122
}
120123
return repoCommit, nil
121124
}
125+
126+
type RepoChangelistIDs struct {
127+
RepoID api.RepoID
128+
ChangelistIDs []int64
129+
}
130+
131+
var getRepoCommitFmtBatchStr = `
132+
SELECT
133+
id,
134+
repo_id,
135+
commit_sha,
136+
perforce_changelist_id
137+
FROM
138+
repo_commits_changelists
139+
WHERE
140+
%s;
141+
`
142+
143+
func (s *repoCommitsChangelistsStore) BatchGetRepoCommitChangelist(ctx context.Context, rcs ...RepoChangelistIDs) (map[api.RepoID]map[int64]*types.RepoCommit, error) {
144+
res := make(map[api.RepoID]map[int64]*types.RepoCommit, len(rcs))
145+
for _, rc := range rcs {
146+
res[rc.RepoID] = make(map[int64]*types.RepoCommit, len(rc.ChangelistIDs))
147+
}
148+
149+
var where []*sqlf.Query
150+
for _, rc := range rcs {
151+
changeListIdsLength := len(rc.ChangelistIDs)
152+
if changeListIdsLength == 0 {
153+
continue
154+
}
155+
items := make([]*sqlf.Query, changeListIdsLength)
156+
for i, id := range rc.ChangelistIDs {
157+
items[i] = sqlf.Sprintf("%d", id)
158+
}
159+
where = append(where, sqlf.Sprintf("(repo_id=%d AND perforce_changelist_id IN (%s))", rc.RepoID, sqlf.Join(items, ",")))
160+
}
161+
162+
var whereClause *sqlf.Query
163+
if len(where) > 0 {
164+
whereClause = sqlf.Join(where, "\n OR ")
165+
} else {
166+
// If input has no changeList ids just return an empty result
167+
return res, nil
168+
}
169+
170+
q := sqlf.Sprintf(getRepoCommitFmtBatchStr, whereClause)
171+
172+
rows, err := s.Query(ctx, q)
173+
if err != nil {
174+
return nil, err
175+
}
176+
defer rows.Close()
177+
178+
for rows.Next() {
179+
if repoCommit, err := scanRepoCommitRow(rows); err != nil {
180+
return nil, err
181+
} else {
182+
res[repoCommit.RepoID][repoCommit.PerforceChangelistID] = repoCommit
183+
}
184+
}
185+
186+
return res, rows.Err()
187+
}

0 commit comments

Comments
 (0)