opendev/gerrit
Code Issues Proposed changes

Use ETag and If-None-Match for GET /change/detail

Browse Source 
This prevents regenerating JSON response if nothing changed.

Feature: Issue 6639
Change-Id: I1d542928d7b48a049cd4328f1a2ac1dcd1017cd6
This commit is contained in:
Viktar Donich
2017-07-18 09:13:25 -07:00
parent 60e03f4aab
commit ade2e11a75
6 changed files with 263 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator.html Normal file
View File

@@ -0,0 +1,21 @@
<!--
Copyright (C) 2017 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<dom-module id="gr-etag-decorator">
<script src="gr-etag-decorator.js"></script>
</dom-module>

72
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator.js Normal file
View File

@@ -0,0 +1,72 @@
// Copyright (C) 2017 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
(function(window) {
'use strict';
// Prevent redefinition.
if (window.GrEtagDecorator) { return; }
// Limit cache size because /change/detail responses may be large.
const MAX_CACHE_SIZE = 30;
function GrEtagDecorator() {
this._etags = new Map();
this._payloadCache = new Map();
}
GrEtagDecorator.prototype.getOptions = function(url, opt_options) {
const etag = this._etags.get(url);
if (!etag) {
return opt_options;
}
const options = Object.assign({}, opt_options);
options.headers = options.headers || new Headers();
options.headers.set('If-None-Match', this._etags.get(url));
return options;
};
GrEtagDecorator.prototype.collect = function(url, response, payload) {
if (!response ||
!response.ok ||
response.status !== 200 ||
response.status === 304) {
// 304 Not Modified means etag is still valid.
return;
}
this._payloadCache.set(url, payload);
const etag = response.headers && response.headers.get('etag');
if (!etag) {
this._etags.delete(url);
} else {
this._etags.set(url, etag);
this._trunkateCache();
}
};
GrEtagDecorator.prototype.getCachedPayload = function(url) {
return this._payloadCache.get(url);
};
GrEtagDecorator.prototype._trunkateCache = function() {
for (const url of this._etags.keys()) {
if (this._etags.size <= MAX_CACHE_SIZE) {
break;
}
this._etags.delete(url);
this._payloadCache.delete(url);
}
};
window.GrEtagDecorator = GrEtagDecorator;
})(window);

96
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-etag-decorator_test.html Normal file
View File

@@ -0,0 +1,96 @@
<!DOCTYPE html>
<!--
Copyright (C) 2017 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-etag-decorator</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
<script src="gr-etag-decorator.js"></script>
<script>
suite('gr-etag-decorator', () => {
let etag;
let sandbox;
const fakeRequest = (opt_etag, opt_status) => {
const headers = new Headers();
if (opt_etag) {
headers.set('etag', opt_etag);
}
const status = opt_status || 200;
return {ok: true, status, headers};
};
setup(() => {
sandbox = sinon.sandbox.create();
etag = new GrEtagDecorator();
});
teardown(() => {
sandbox.restore();
});
test('exists', () => {
assert.isOk(etag);
});
test('works', () => {
etag.collect('/foo', fakeRequest('bar'));
const options = etag.getOptions('/foo');
assert.strictEqual(options.headers.get('If-None-Match'), 'bar');
});
test('updates etags', () => {
etag.collect('/foo', fakeRequest('bar'));
etag.collect('/foo', fakeRequest('baz'));
const options = etag.getOptions('/foo');
assert.strictEqual(options.headers.get('If-None-Match'), 'baz');
});
test('discards empty etags', () => {
etag.collect('/foo', fakeRequest('bar'));
etag.collect('/foo', fakeRequest());
const options = etag.getOptions('/foo', {headers: new Headers()});
assert.isNull(options.headers.get('If-None-Match'));
});
test('discards etags in order used', () => {
etag.collect('/foo', fakeRequest('bar'));
_.times(29, i => {
etag.collect('/qaz/' + i, fakeRequest('qaz'));
});
let options = etag.getOptions('/foo');
assert.strictEqual(options.headers.get('If-None-Match'), 'bar');
etag.collect('/zaq', fakeRequest('zaq'));
options = etag.getOptions('/foo', {headers: new Headers()});
assert.isNull(options.headers.get('If-None-Match'));
});
test('getCachedPayload', () => {
const payload = {};
etag.collect('/foo', fakeRequest('bar'), payload);
assert.strictEqual(etag.getCachedPayload('/foo'), payload);
etag.collect('/foo', fakeRequest('bar', 304), 'garbage');
assert.strictEqual(etag.getCachedPayload('/foo'), payload);
etag.collect('/foo', fakeRequest('bar', 200), 'new payload');
assert.strictEqual(etag.getCachedPayload('/foo'), 'new payload');
});
});
</script>

6
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
View File

@@ -14,10 +14,12 @@ See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="gr-etag-decorator.html">
<!-- NB: es6-promise Needed for IE11 and fetch polyfill support, see Issue 4308 -->
<script src="../../../bower_components/es6-promise/dist/es6-promise.min.js"></script>
<script src="../../../bower_components/fetch/fetch.js"></script>

78
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
View File

@@ -28,6 +28,7 @@
};
let auth = null;
const etags = new GrEtagDecorator();
Polymer({
is: 'gr-rest-api-interface',
@@ -74,14 +75,53 @@
auth = window.USE_GAPI_AUTH ? new GrGapiAuth() : new GrGerritAuth();
},
fetchJSON(url, opt_errFn, opt_cancelCondition, opt_params, opt_options) {
/**
* Fetch JSON from url provided.
* Returns a Promise that resolves to a native Response.
* Doesn't do error checking. Supports cancel condition. Performs auth.
* Validates auth expiry errors.
* @param {string} url
* @param {function(response, error)} opt_errFn
* @param {function()} opt_cancelCondition
* @param {Object=} opt_params URL params, key-value hash.
* @param {Object=} opt_options Fetch options.
*/
_fetchRawJSON(url, opt_errFn, opt_cancelCondition, opt_params,
opt_options) {
const urlWithParams = this._urlWithParams(url, opt_params);
return auth.fetch(urlWithParams, opt_options).then(response => {
if (opt_cancelCondition && opt_cancelCondition()) {
response.body.cancel();
return;
}
return response;
}).catch(err => {
if (opt_errFn) {
opt_errFn.call(null, null, err);
throw err;
} else {
this._checkAuthRedirect();
}
});
},
/**
* Fetch JSON from url provided.
* Returns a Promise that resolves to a parsed response.
* Same as {@link _fetchRawJSON}, plus error handling.
* @param {string} url
* @param {function(response, error)} opt_errFn
* @param {function()} opt_cancelCondition
* @param {Object=} opt_params URL params, key-value hash.
* @param {Object=} opt_options Fetch options.
*/
fetchJSON(url, opt_errFn, opt_cancelCondition, opt_params, opt_options) {
return this._fetchRawJSON(
url, opt_errFn, opt_cancelCondition, opt_params, opt_options)
.then(response => {
if (!response) {
return;
}
if (!response.ok) {
if (opt_errFn) {
opt_errFn.call(null, response);
@@ -90,15 +130,7 @@
this.fire('server-error', {response});
return;
}
return this.getResponseObject(response);
}).catch(err => {
if (opt_errFn) {
opt_errFn.call(null, null, err);
throw err;
} else {
this._checkAuthRedirect();
}
return response && this.getResponseObject(response);
});
},
@@ -462,20 +494,34 @@
},
getDiffChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
const options = this.listChangesOptionsToHex(
const params = this.listChangesOptionsToHex(
this.ListChangesOption.ALL_REVISIONS
);
return this._getChangeDetail(changeNum, options, opt_errFn,
return this._getChangeDetail(changeNum, params, opt_errFn,
opt_cancelCondition);
},
_getChangeDetail(changeNum, options, opt_errFn,
_getChangeDetail(changeNum, params, opt_errFn,
opt_cancelCondition) {
return this.fetchJSON(
this.getChangeActionURL(changeNum, null, '/detail'),
const url = this.getChangeActionURL(changeNum, null, '/detail');
return this._fetchRawJSON(
url,
opt_errFn,
opt_cancelCondition,
{O: options});
{O: params},
etags.getOptions(url))
.then(response => {
if (response && response.status === 304) {
return Promise.resolve(etags.getCachedPayload(url));
} else {
const payloadPromise = response ?
this.getResponseObject(response) : Promise.resolve();
payloadPromise.then(payload => {
etags.collect(url, response, payload);
});
return payloadPromise;
}
});
},
getChangeCommitInfo(changeNum, patchNum) {

2
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
View File

@@ -441,12 +441,14 @@ limitations under the License.
const authErrorStub = sandbox.stub();
element.addEventListener('auth-error', authErrorStub);
element.fetchJSON('/bar').then(r => {
flush(() => {
assert.isTrue(authErrorStub.called);
assert.isFalse(serverErrorStub.called);
assert.isNull(element._cache['/accounts/self/detail']);
done();
});
});
});
test('checkCredentials', done => {
const responses = [