Rename getFromProjectLookup to getRepoName
ProjectLookup is the implementation detail and it's not relevant to the
caller of the interface.
It's still valuable to have some information about the implementation so
updated the documentation and mentioned it's relationship to
setInProjectLookup
Google-Bug-Id: b/297849592
Release-Notes: skip
Change-Id: Ica48efc7e66a80ddd989a7223ba64e18c772af96
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index cbc05d8..8c6c213 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -1834,7 +1834,7 @@
switch (action.__key) {
case ChangeActions.REVERT: {
const revertChangeInfo: ChangeInfo = response as unknown as ChangeInfo;
- this.restApiService.setInProjectLookup(
+ this.restApiService.addRepoNameToCache(
revertChangeInfo._number,
revertChangeInfo.project
);
@@ -1851,7 +1851,7 @@
case RevisionActions.CHERRYPICK: {
const cherrypickChangeInfo: ChangeInfo =
response as unknown as ChangeInfo;
- this.restApiService.setInProjectLookup(
+ this.restApiService.addRepoNameToCache(
cherrypickChangeInfo._number,
cherrypickChangeInfo.project
);
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index 07b66b2..ff11a06 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -474,9 +474,7 @@
test('submit change', async () => {
const showSpy = sinon.spy(element, 'showActionDialog');
- stubRestApi('getFromProjectLookup').returns(
- Promise.resolve('test' as RepoName)
- );
+ stubRestApi('getRepoName').returns(Promise.resolve('test' as RepoName));
element.change = {
...createChangeViewChange(),
revisions: {
@@ -510,9 +508,7 @@
'resetFocus'
)
.callsFake(() => submitted.resolve());
- stubRestApi('getFromProjectLookup').returns(
- Promise.resolve('test' as RepoName)
- );
+ stubRestApi('getRepoName').returns(Promise.resolve('test' as RepoName));
element.change = {
...createChangeViewChange(),
revisions: {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index ff2f7a5..a3cb0ee 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -401,7 +401,7 @@
setState(state: AppElementParams) {
// TODO: Move this logic into the change model.
if ('repo' in state && state.repo !== undefined && 'changeNum' in state)
- this.restApiService.setInProjectLookup(state.changeNum, state.repo);
+ this.restApiService.addRepoNameToCache(state.changeNum, state.repo);
this.routerModel.setState({view: state.view});
// We are trying to reset the change (view) model when navigating to other
@@ -1361,7 +1361,7 @@
const repo = ctx.params[0] as RepoName;
const commentId = ctx.params[2] as UrlEncodedCommentId;
- this.restApiService.setInProjectLookup(changeNum, repo);
+ this.restApiService.addRepoNameToCache(changeNum, repo);
const [comments, robotComments, drafts, change] = await Promise.all([
this.restApiService.getDiffComments(changeNum),
this.restApiService.getDiffRobotComments(changeNum),
@@ -1474,7 +1474,7 @@
this.show404();
return;
}
- this.restApiService.getFromProjectLookup(changeNum).then(project => {
+ this.restApiService.getRepoName(changeNum).then(project => {
// Show a 404 and terminate if the lookup request failed. Attempting
// to redirect after failing to get the project loops infinitely.
if (!project) {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index 50e506b..fbc0338 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -263,7 +263,7 @@
let urlPromise: MockPromise<string>;
setup(() => {
- stubRestApi('setInProjectLookup');
+ stubRestApi('addRepoNameToCache');
urlPromise = mockPromise<string>();
redirectStub = sinon
.stub(router, 'redirect')
@@ -374,7 +374,7 @@
}
setup(() => {
- stubRestApi('setInProjectLookup');
+ stubRestApi('addRepoNameToCache');
redirectStub = sinon.stub(router, 'redirect');
redirectToLoginStub = sinon.stub(router, 'redirectToLogin');
setStateStub = sinon.stub(router, 'setState');
@@ -863,7 +863,7 @@
test('CHANGE_LEGACY', async () => {
// CHANGE_LEGACY: /^\/c\/(\d+)\/?(.*)$/,
- stubRestApi('getFromProjectLookup').resolves('project' as RepoName);
+ stubRestApi('getRepoName').resolves('project' as RepoName);
await checkRedirect('/c/1234', '/c/project/+/1234/');
await checkRedirect(
'/c/1234/comment/6789',
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index df5c23d..03c2157 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -1211,7 +1211,7 @@
*/
_maybeInsertInLookup(change: ChangeInfo): void {
if (change?.project && change._number) {
- this.setInProjectLookup(change._number, change.project);
+ this.addRepoNameToCache(change._number, change.project);
}
}
@@ -1805,7 +1805,7 @@
queryParams.push(`${escapeAndWrapSearchOperatorValue(inputVal)}`);
}
if (canSee) {
- const project = await this.getFromProjectLookup(canSee);
+ const project = await this.getRepoName(canSee);
queryParams.push(`cansee:${project}~${canSee}`);
}
if (filterActive) {
@@ -2413,7 +2413,7 @@
// Some servers may require the project name to be provided
// alongside the change number, so resolve the project name
// first.
- return this.getFromProjectLookup(changeNum).then(project => {
+ return this.getRepoName(changeNum).then(project => {
const encodedRepoName = encodeURIComponent(project) + '~';
const url = `/accounts/self/starred.changes/${encodedRepoName}${changeNum}`;
return this._serialScheduler.schedule(() =>
@@ -2957,7 +2957,7 @@
changeNum: NumericChangeId,
revisionId?: RevisionId
): Promise<string> {
- return this.getFromProjectLookup(changeNum).then(project => {
+ return this.getRepoName(changeNum).then(project => {
let url = `/changes/${encodeURIComponent(project)}~${changeNum}`;
if (revisionId) {
url += `/revisions/${revisionId}`;
@@ -3271,12 +3271,12 @@
* Then we don't need to make a dedicated REST API call or have a fallback,
* if that fails.
*/
- setInProjectLookup(changeNum: NumericChangeId, project: RepoName) {
+ addRepoNameToCache(changeNum: NumericChangeId, project: RepoName) {
this._projectLookup[changeNum] = Promise.resolve(project);
}
- getFromProjectLookup(changeNum: NumericChangeId): Promise<RepoName> {
- // Hopefully setInProjectLookup() has already been called. Then we don't
+ getRepoName(changeNum: NumericChangeId): Promise<RepoName> {
+ // Hopefully addRepoNameToCache() has already been called. Then we don't
// have to make a dedicated REST API call to look up the project.
let projectPromise = this._projectLookup[changeNum];
if (projectPromise) return projectPromise;
@@ -3288,7 +3288,7 @@
// In the very rare case that the change index cannot provide an answer
// (e.g. stale index) we should check, if the router has called
- // setInProjectLookup() in the meantime. Then we can fall back to that.
+ // addRepoNameToCache() in the meantime. Then we can fall back to that.
const currentProjectPromise = this._projectLookup[changeNum];
if (currentProjectPromise && currentProjectPromise !== projectPromise) {
return currentProjectPromise;
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
index 1ec37ce..fec82e3 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl_test.ts
@@ -96,7 +96,7 @@
});
test('parent diff comments are properly grouped', async () => {
- element.setInProjectLookup(42 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(42 as NumericChangeId, TEST_PROJECT_NAME);
sinon.stub(element._restApiHelper, 'fetchJSON').resolves({
'/COMMIT_MSG': [],
'sieve.go': [
@@ -240,7 +240,7 @@
});
test('differing patch diff comments are properly grouped', async () => {
- sinon.stub(element, 'getFromProjectLookup').resolves('test' as RepoName);
+ sinon.stub(element, 'getRepoName').resolves('test' as RepoName);
sinon.stub(element._restApiHelper, 'fetchJSON').callsFake(async request => {
const url = request.url;
if (url === '/changes/test~42/revisions/1/comments') {
@@ -335,7 +335,7 @@
fetchStub = sinon
.stub(element._restApiHelper, 'fetch')
.resolves(new Response(makePrefixedJSON(createAccountWithId())));
- element.setInProjectLookup(
+ element.addRepoNameToCache(
testChangeNumber as NumericChangeId,
testProject as RepoName
);
@@ -675,7 +675,7 @@
suite('_failForCreate200', () => {
test('_sendDiffDraftRequest checks for 200 on create', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
sinon.stub(element._restApiHelper, 'fetch').resolves(new Response());
const failStub = sinon.stub(element, '_failForCreate200').resolves();
await element._sendDiffDraftRequest(
@@ -688,7 +688,7 @@
});
test('_sendDiffDraftRequest no checks for 200 on non create', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
sinon.stub(element._restApiHelper, 'fetch').resolves(new Response());
const failStub = sinon.stub(element, '_failForCreate200').resolves();
await element._sendDiffDraftRequest(
@@ -820,7 +820,7 @@
});
test('startWorkInProgress', async () => {
- element.setInProjectLookup(42 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(42 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetch')
.resolves(new Response());
@@ -858,7 +858,7 @@
});
test('deleteComment', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const comment = createComment();
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
@@ -895,7 +895,7 @@
});
test('queryChangeFiles', async () => {
- element.setInProjectLookup(42 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(42 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1181,7 +1181,7 @@
});
test('GrReviewerUpdatesParser.parse is used', async () => {
- element.setInProjectLookup(42 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(42 as NumericChangeId, TEST_PROJECT_NAME);
const changeInfo = createParsedChange();
const parseStub = sinon
.stub(GrReviewerUpdatesParser, 'parse')
@@ -1278,13 +1278,13 @@
});
});
- test('setInProjectLookup', async () => {
- element.setInProjectLookup(555 as NumericChangeId, 'project' as RepoName);
- const project = await element.getFromProjectLookup(555 as NumericChangeId);
+ test('addRepoNameToCache', async () => {
+ element.addRepoNameToCache(555 as NumericChangeId, 'project' as RepoName);
+ const project = await element.getRepoName(555 as NumericChangeId);
assert.deepEqual(project, 'project' as RepoName);
});
- suite('getFromProjectLookup', () => {
+ suite('getRepoName', () => {
const changeNum = 555 as NumericChangeId;
const repo = 'test-repo' as RepoName;
@@ -1292,7 +1292,7 @@
const promise = mockPromise<undefined>();
sinon.stub(element, 'getChange').returns(promise);
- const projectLookup = element.getFromProjectLookup(changeNum);
+ const projectLookup = element.getRepoName(changeNum);
promise.resolve(undefined);
const err: Error = await assertFails(projectLookup);
@@ -1306,19 +1306,19 @@
const promise = mockPromise<undefined | ChangeInfo>();
sinon.stub(element, 'getChange').returns(promise);
- const projectLookup = element.getFromProjectLookup(changeNum);
+ const projectLookup = element.getRepoName(changeNum);
promise.resolve({...createChange(), project: repo});
assert.equal(await projectLookup, repo);
assert.deepEqual(element._projectLookup, {'555': projectLookup});
});
- test('getChange fails, but a setInProjectLookup() call is used as fallback', async () => {
+ test('getChange fails, but a addRepoNameToCache() call is used as fallback', async () => {
const promise = mockPromise<undefined>();
sinon.stub(element, 'getChange').returns(promise);
- const projectLookup = element.getFromProjectLookup(changeNum);
- element.setInProjectLookup(changeNum, repo);
+ const projectLookup = element.getRepoName(changeNum);
+ element.addRepoNameToCache(changeNum, repo);
promise.resolve(undefined);
assert.equal(await projectLookup, repo);
@@ -1338,11 +1338,11 @@
// Array<Array<Object>>.
await element.getChangesForMultipleQueries(undefined, []);
assert.equal(Object.keys(element._projectLookup).length, 3);
- const project1 = await element.getFromProjectLookup(1 as NumericChangeId);
+ const project1 = await element.getRepoName(1 as NumericChangeId);
assert.equal(project1, 'test' as RepoName);
- const project2 = await element.getFromProjectLookup(2 as NumericChangeId);
+ const project2 = await element.getRepoName(2 as NumericChangeId);
assert.equal(project2, 'test' as RepoName);
- const project3 = await element.getFromProjectLookup(3 as NumericChangeId);
+ const project3 = await element.getRepoName(3 as NumericChangeId);
assert.equal(project3, 'test/test' as RepoName);
});
@@ -1356,11 +1356,11 @@
// When query !instanceof Array, fetchJSON returns Array<Object>.
await element.getChanges();
assert.equal(Object.keys(element._projectLookup).length, 3);
- const project1 = await element.getFromProjectLookup(1 as NumericChangeId);
+ const project1 = await element.getRepoName(1 as NumericChangeId);
assert.equal(project1, 'test' as RepoName);
- const project2 = await element.getFromProjectLookup(2 as NumericChangeId);
+ const project2 = await element.getRepoName(2 as NumericChangeId);
assert.equal(project2, 'test' as RepoName);
- const project3 = await element.getFromProjectLookup(3 as NumericChangeId);
+ const project3 = await element.getRepoName(3 as NumericChangeId);
assert.equal(project3, 'test/test' as RepoName);
});
});
@@ -1384,7 +1384,7 @@
});
test('setChangeTopic', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon.stub(element._restApiHelper, 'fetchJSON');
await element.setChangeTopic(123 as NumericChangeId, 'foo-bar');
assert.isTrue(fetchStub.calledOnce);
@@ -1395,7 +1395,7 @@
});
test('setChangeHashtag', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon.stub(element._restApiHelper, 'fetchJSON');
await element.setChangeHashtag(123 as NumericChangeId, {
add: ['foo-bar' as Hashtag],
@@ -1421,7 +1421,7 @@
suite('getChangeFiles', () => {
test('patch only', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1436,7 +1436,7 @@
});
test('simple range', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1456,7 +1456,7 @@
});
test('parent index', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1478,7 +1478,7 @@
suite('getDiff', () => {
test('patchOnly', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1499,7 +1499,7 @@
});
test('simple range', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1520,7 +1520,7 @@
});
test('parent index', async () => {
- element.setInProjectLookup(123 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(123 as NumericChangeId, TEST_PROJECT_NAME);
const fetchStub = sinon
.stub(element._restApiHelper, 'fetchJSON')
.resolves();
@@ -1558,7 +1558,7 @@
});
test('getFileContent', async () => {
- element.setInProjectLookup(1 as NumericChangeId, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(1 as NumericChangeId, TEST_PROJECT_NAME);
sinon.stub(element._restApiHelper, 'fetch').callsFake(() =>
Promise.resolve(
new Response(makePrefixedJSON('new content'), {
@@ -1656,7 +1656,7 @@
test('ported drafts are not requested user is not logged in', () => {
const change = createChange();
- element.setInProjectLookup(change._number, TEST_PROJECT_NAME);
+ element.addRepoNameToCache(change._number, TEST_PROJECT_NAME);
sinon.stub(element, 'getLoggedIn').resolves(false);
const getChangeURLAndFetchStub = sinon.stub(
element._restApiHelper,
@@ -1669,8 +1669,8 @@
});
test('saveChangeStarred', async () => {
- element.setInProjectLookup(123 as NumericChangeId, 'test' as RepoName);
- element.setInProjectLookup(456 as NumericChangeId, 'test' as RepoName);
+ element.addRepoNameToCache(123 as NumericChangeId, 'test' as RepoName);
+ element.addRepoNameToCache(456 as NumericChangeId, 'test' as RepoName);
const fetchStub = sinon.stub(element._restApiHelper, 'fetch').resolves();
await element.saveChangeStarred(123 as NumericChangeId, true);
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index 1728c43..8389a15 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -393,14 +393,24 @@
): Promise<IncludedInInfo | undefined>;
/**
- * Checks in projectLookup map shared across instances for the changeNum.
- * If it exists, returns the project. If not, calls the restAPI to get the
- * change, populates projectLookup with the project for that change, and
- * returns the project.
+ * Looks up repo name in which change is located.
+ *
+ * Change -> repo association is cached. This will only make restAPI call (and
+ * cache the result) if the repo name for the change is not already known.
+ *
+ * addRepoNameToCache can be used to add entry to the cache manually.
*
* If the lookup fails the promise rejects and result is not cached.
*/
- getFromProjectLookup(changeNum: NumericChangeId): Promise<RepoName>;
+ getRepoName(changeNum: NumericChangeId): Promise<RepoName>;
+
+ /**
+ * Populates cache for the future getRepoName(changeNum) lookup.
+ *
+ * The repo name is used for constructing of url for all change-based
+ * endpoints.
+ */
+ addRepoNameToCache(changeNum: NumericChangeId, repo: RepoName): void;
saveDiffDraft(
changeNum: NumericChangeId,
@@ -836,7 +846,6 @@
getTopMenus(): Promise<TopMenuEntryInfo[] | undefined>;
- setInProjectLookup(changeNum: NumericChangeId, repo: RepoName): void;
getMergeable(changeNum: NumericChangeId): Promise<MergeableInfo | undefined>;
putChangeCommitMessage(
diff --git a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
index e5b8f40..8e9ee16 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -325,8 +325,8 @@
getFileContent(): Promise<Response | Base64FileContent | undefined> {
return Promise.resolve(new Response());
},
- getFromProjectLookup(): Promise<RepoName> {
- throw new Error('getFromProjectLookup() not implemented by RestApiMock.');
+ getRepoName(): Promise<RepoName> {
+ throw new Error('getRepoName() not implemented by RestApiMock.');
},
getGroupAuditLog(): Promise<GroupAuditEventInfo[] | undefined> {
return Promise.resolve([]);
@@ -551,7 +551,7 @@
setDescription(): Promise<Response> {
return Promise.resolve(new Response());
},
- setInProjectLookup(): void {},
+ addRepoNameToCache(): void {},
setPreferredAccountEmail(): Promise<void> {
return Promise.resolve();
},
