Some rest-api-interface clean-ups in preparation for service conversion

Change-Id: Id5bfcabf05b7858921a34ea235e91d0413ca1bf4
This commit is contained in:
Ben Rohlfs
2021-01-22 22:11:36 +01:00
parent a00ef28db9
commit 9ead541313
10 changed files with 53 additions and 115 deletions

View File

@@ -434,12 +434,10 @@ const reloadStub = sinon
.stub(element, '_reload')
.callsFake(() => Promise.resolve());
stub('gr-rest-api-interface', {
getDiffComments() { return Promise.resolve({}); },
getDiffRobotComments() { return Promise.resolve({}); },
getDiffDrafts() { return Promise.resolve({}); },
_fetchSharedCacheURL() { return Promise.resolve({}); },
});
stubRestApi('getDiffComments').returns(Promise.resolve({}));
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('_fetchSharedCacheURL').returns(Promise.resolve({}));
```
In such cases, validate the input and output of a stub/fake method. Quite often
@@ -451,61 +449,9 @@ const reloadStub = sinon
// GrChangeView._reload method returns an array
.callsFake(() => Promise.resolve([])); // return [] here
stub('gr-rest-api-interface', {
...
// Fix return type:
_fetchSharedCacheURL() { return Promise.resolve({} as ParsedJSON); },
});
```
If a method has multiple overloads, you can use one of 2 options:
```
// Option 1: less accurate, but shorter:
function getCommentsStub() {
return Promise.resolve({});
}
stub('gr-rest-api-interface', {
...
getDiffComments: (getCommentsStub as unknown) as RestApiService['getDiffComments'],
getDiffRobotComments: (getCommentsStub as unknown) as RestApiService['getDiffRobotComments'],
getDiffDrafts: (getCommentsStub as unknown) as RestApiService['getDiffDrafts'],
...
});
// Option 2: more accurate, but longer.
// Step 1: define the same overloads for stub:
function getDiffCommentsStub(
changeNum: NumericChangeId
): Promise<PathToCommentsInfoMap | undefined>;
function getDiffCommentsStub(
changeNum: NumericChangeId,
basePatchNum: PatchSetNum,
patchNum: PatchSetNum,
path: string
): Promise<GetDiffCommentsOutput>;
// Step 2: implement stub method for differnt input
function getDiffCommentsStub(
_: NumericChangeId,
basePatchNum?: PatchSetNum,
):
| Promise<PathToCommentsInfoMap | undefined>
| Promise<GetDiffCommentsOutput> {
if (basePatchNum) {
return Promise.resolve({
baseComments: [],
comments: [],
});
}
return Promise.resolve({});
}
// Step 3: use stubbed function:
stub('gr-rest-api-interface', {
...
getDiffComments: getDiffCommentsStub,
...
stubRestApi('_fetchSharedCacheURL').returns(Promise.resolve({} as ParsedJSON));
});
```

View File

@@ -35,10 +35,7 @@ import {
} from '../../../types/common';
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {customElement, property, observe} from '@polymer/decorators';
import {
GrAutocomplete,
AutocompleteSuggestion,
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {AutocompleteSuggestion} from '../../shared/gr-autocomplete/gr-autocomplete';
import {HttpMethod, ChangeStatus} from '../../../constants/constants';
import {hasOwnProperty} from '../../../utils/common-util';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
@@ -71,11 +68,9 @@ declare global {
}
}
// TODO(TS): add type after gr-autocomplete and gr-rest-api-interface
// is converted
export interface GrConfirmCherrypickDialog {
$: {
branchInput: GrAutocomplete;
branchInput: HTMLElement;
};
}

View File

@@ -776,9 +776,9 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
let response: Response = r;
// A call to _saveReview could fail with a server error if erroneous
// reviewers were requested. This is signalled with a 400 Bad Request
// status. The default gr-rest-api-interface error handling would
// result in a large JSON response body being displayed to the user in
// the gr-error-manager toast.
// status. The default gr-rest-api error handling would result in a large
// JSON response body being displayed to the user in the gr-error-manager
// toast.
//
// We can modify the error handling behavior by passing this function
// through to restAPI as a custom error handling function. Since we're

View File

@@ -23,7 +23,7 @@ import {SpecialFilePath} from '../../../constants/constants.js';
import {appContext} from '../../../services/app-context.js';
import {addListenerForTest} from '../../../test/test-utils.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {JSON_PREFIX} from '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
import {JSON_PREFIX} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.js';
const basicFixture = fixtureFromElement('gr-reply-dialog');

View File

@@ -168,7 +168,7 @@ function initGerritPluginsMethods(globalGerritObj: GerritGlobal) {
'Gerrit.getLoggedIn() is deprecated! ' +
'Use plugin.restApi().getLoggedIn()'
);
return document.createElement('gr-rest-api-interface').getLoggedIn();
return appContext.restApiService.getLoggedIn();
};
globalGerritObj.get = (

View File

@@ -47,6 +47,7 @@ import {HttpMethod} from '../../../constants/constants';
import {JsApiService} from './gr-js-api-types';
import {GrChangeActions} from '../../change/gr-change-actions/gr-change-actions';
import {GrChecksApi} from '../../plugins/gr-checks-api/gr-checks-api';
import {appContext} from '../../../services/app-context';
/**
* Plugin-provided custom components can affect content in extension
@@ -168,7 +169,7 @@ export class Plugin implements PluginApi {
}
getServerInfo() {
return document.createElement('gr-rest-api-interface').getConfig();
return appContext.restApiService.getConfig();
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any

View File

@@ -49,90 +49,90 @@ import {
AccountExternalIdInfo,
AccountId,
AccountInfo,
ActionNameToActionInfoMap,
AssigneeInput,
Base64File,
Base64FileContent,
Base64ImageFile,
BlameInfo,
BranchInfo,
BranchInput,
BranchName,
CapabilityInfoMap,
ChangeId,
ChangeInfo,
ChangeMessageId,
ChangeViewChangeInfo,
CommentInfo,
CommentInput,
CommitId,
CommitInfo,
ConfigInfo,
ConfigInput,
ContributorAgreementInfo,
ContributorAgreementInput,
DashboardId,
DashboardInfo,
DeleteDraftCommentsInput,
DiffPreferenceInput,
DocResult,
EditInfo,
EditPatchSetNum,
EditPreferencesInfo,
EmailAddress,
EmailInfo,
EncodedGroupId,
FileNameToFileInfoMap,
FilePathToDiffInfoMap,
FixId,
GitRef,
GpgKeyId,
GpgKeyInfo,
GpgKeysInput,
GroupAuditEventInfo,
GroupId,
GroupInfo,
GroupInput,
GroupName,
GroupNameToGroupInfoMap,
GroupOptionsInput,
Hashtag,
HashtagsInput,
ImagesForDiff,
IncludedInInfo,
MergeableInfo,
NameToProjectInfoMap,
NumericChangeId,
ParentPatchSetNum,
ParsedJSON,
Password,
PatchRange,
PatchSetNum,
PathToCommentsInfoMap,
PathToRobotCommentsInfoMap,
PluginInfo,
PreferencesInfo,
PreferencesInput,
ProjectAccessInfo,
ProjectAccessInfoMap,
ProjectAccessInput,
ProjectInfo,
ProjectInfoWithName,
ProjectInput,
ProjectWatchInfo,
RelatedChangesInfo,
RepoName,
RequestPayload,
ReviewInput,
RevisionId,
ServerInfo,
SshKeyInfo,
UrlEncodedCommentId,
EditInfo,
FileNameToFileInfoMap,
SuggestedReviewerInfo,
GroupNameToGroupInfoMap,
GroupAuditEventInfo,
RequestPayload,
Password,
ContributorAgreementInput,
ContributorAgreementInfo,
BranchInput,
IncludedInInfo,
TagInput,
PluginInfo,
GpgKeyInfo,
GpgKeysInput,
DocResult,
EmailInfo,
ProjectAccessInfo,
CapabilityInfoMap,
ProjectInfoWithName,
TagInfo,
RelatedChangesInfo,
SubmittedTogetherInfo,
NumericChangeId,
EmailAddress,
FixId,
FilePathToDiffInfoMap,
ChangeViewChangeInfo,
BlameInfo,
ActionNameToActionInfoMap,
RevisionId,
GroupName,
Hashtag,
SuggestedReviewerInfo,
TagInfo,
TagInput,
TopMenuEntryInfo,
MergeableInfo,
UrlEncodedCommentId,
} from '../../../types/common';
import {
DiffInfo,
@@ -142,9 +142,9 @@ import {
import {
CancelConditionCallback,
ErrorCallback,
RestApiService,
GetDiffCommentsOutput,
GetDiffRobotCommentsOutput,
RestApiService,
} from '../../../services/gr-rest-api/gr-rest-api';
import {
CommentSide,
@@ -158,7 +158,6 @@ import {
import {firePageError, fireServerError} from '../../../utils/event-util';
import {ParsedChangeInfo} from '../../../types/types';
export const JSON_PREFIX = ")]}'";
const MAX_PROJECT_RESULTS = 25;
// This value is somewhat arbitrary and not based on research or calculations.
const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 850;
@@ -301,8 +300,6 @@ declare global {
export class GrRestApiInterface
extends GestureEventListeners(LegacyElementMixin(PolymerElement))
implements RestApiService {
readonly JSON_PREFIX = JSON_PREFIX;
@property({type: Object})
readonly _cache = siteBasedCache; // Shared across instances.

View File

@@ -27,7 +27,7 @@ import {
parsePrefixedJSON,
readResponsePayload,
} from './gr-rest-apis/gr-rest-api-helper.js';
import {JSON_PREFIX} from './gr-rest-api-interface.js';
import {JSON_PREFIX} from './gr-rest-apis/gr-rest-api-helper.js';
const basicFixture = fixtureFromElement('gr-rest-api-interface');
@@ -946,8 +946,7 @@ suite('gr-rest-api-interface tests', () => {
setup(() => {
requestUrl = '/foo/bar';
const mockResponse = {foo: 'bar', baz: 42};
mockResponseSerial = element.JSON_PREFIX +
JSON.stringify(mockResponse);
mockResponseSerial = JSON_PREFIX + JSON.stringify(mockResponse);
sinon.stub(element._restApiHelper, 'urlWithParams')
.returns(requestUrl);
sinon.stub(element, 'getChangeActionURL')

View File

@@ -35,7 +35,7 @@ import {RpcLogEventDetail} from '../../../../types/events';
import {fireNetworkError, fireServerError} from '../../../../utils/event-util';
import {FetchRequest} from '../../../../types/types';
const JSON_PREFIX = ")]}'";
export const JSON_PREFIX = ")]}'";
export interface ResponsePayload {
// TODO(TS): readResponsePayload can assign null to the parsed property if

View File

@@ -1612,7 +1612,7 @@ export interface HashtagsInput {
}
/**
* Defines a patch ranges. Used as input for gr-rest-api-interface methods,
* Defines a patch ranges. Used as input for gr-rest-api methods,
* doesn't exist in Rest API
*/
export interface PatchRange {