Wait for plugins to be loaded before showing change actions

Also, refactoring.

Bug: Issue 8433
Change-Id: I0986d539fcbc6bd8fea626a3087fa30a27e45c1b
This commit is contained in:
Viktar Donich
2018-03-26 13:13:00 -07:00
parent 3d3c2178d7
commit 935b4e9ad0
12 changed files with 176 additions and 103 deletions

View File

@@ -37,7 +37,7 @@ limitations under the License.
let element;
let sandbox;
setup(() => {
setup(done => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
stub('gr-rest-api-interface', {
@@ -45,7 +45,9 @@ limitations under the License.
return Promise.resolve({});
},
});
sandbox.stub(Gerrit, 'awaitPluginsLoaded').returns(Promise.resolve());
const pluginsLoaded = Promise.resolve();
sandbox.stub(Gerrit, 'awaitPluginsLoaded').returns(pluginsLoaded);
pluginsLoaded.then(() => flush(done));
});
teardown(() => {
@@ -343,6 +345,7 @@ limitations under the License.
}));
sandbox.stub(element.$.restAPI, 'getIsGroupOwner')
.returns(Promise.resolve(true));
return element.reload();
});
test('group list', () => {

View File

@@ -385,7 +385,7 @@
ready() {
this.$.jsAPI.addElement(this.$.jsAPI.Element.CHANGE_ACTIONS, this);
this._loading = false;
this._handleLoadingComplete();
},
reload() {
@@ -398,7 +398,7 @@
if (!revisionActions) { return; }
this.revisionActions = revisionActions;
this._loading = false;
this._handleLoadingComplete();
}).catch(err => {
this.fire('show-alert', {message: ERR_REVISION_ACTIONS});
this._loading = false;
@@ -406,6 +406,10 @@
});
},
_handleLoadingComplete() {
Gerrit.awaitPluginsLoaded().then(() => this._loading = false);
},
_changeChanged() {
this.reload();
},

View File

@@ -91,8 +91,6 @@ limitations under the License.
});
teardown(() => {
Gerrit._pluginsPending = -1;
Gerrit._allPluginsPromise = undefined;
sandbox.restore();
});
@@ -111,6 +109,7 @@ limitations under the License.
suite('with plugin style', () => {
setup(done => {
Gerrit._resetPlugins();
const pluginHost = fixture('plugin-host');
pluginHost.config = {
plugin: {
@@ -146,7 +145,7 @@ limitations under the License.
new URL('test/plugin.html?' + Math.random(),
window.location.href).toString());
sandbox.stub(Gerrit, '_arePluginsLoaded').returns(true);
Gerrit._resolveAllPluginsLoaded();
Gerrit._setPluginsPending([]);
element = createElement();
sandbox.stub(element, '_computeCanDeleteVote').returns(true);

View File

@@ -110,8 +110,6 @@ limitations under the License.
});
teardown(() => {
Gerrit._pluginsPending = -1;
Gerrit._allPluginsPromise = undefined;
sandbox.restore();
});
@@ -132,12 +130,14 @@ limitations under the License.
});
test('lgtm plugin', done => {
Gerrit._resetPlugins();
const pluginHost = fixture('plugin-host');
pluginHost.config = {
plugin: {
js_resource_paths: [],
html_resource_paths: [
new URL('test/plugin.html', window.location.href).toString(),
new URL('test/plugin.html?' + Math.random(),
window.location.href).toString(),
],
},
};

View File

@@ -28,6 +28,6 @@ limitations under the License.
replyApi.setLabelValue(label, '+1');
}
});
}, '0.1', 'http://test.com/plugins/testplugin/static/test.js');
});
</script>
</dom-module>

View File

@@ -34,11 +34,13 @@
_configChanged(config) {
const plugins = config.plugin;
const htmlPlugins = plugins.html_resource_paths || [];
const jsPlugins = this._handleMigrations(plugins.js_resource_paths || [],
htmlPlugins);
const jsPlugins =
this._handleMigrations(plugins.js_resource_paths || [], htmlPlugins);
const defaultTheme = config.default_theme;
Gerrit._setPluginsCount(
jsPlugins.length + htmlPlugins.length + (defaultTheme ? 1 : 0));
const pluginsPending =
[].concat(jsPlugins, htmlPlugins, defaultTheme || []).map(
p => this._urlFor(p));
Gerrit._setPluginsPending(pluginsPending);
if (defaultTheme) {
// Make theme first to be first to load.
// Load sync to work around rare theme loading race condition.
@@ -72,7 +74,9 @@
// onload (second param) needs to be a function. When null or undefined
// were passed, plugins were not loaded correctly.
this.importHref(
this._urlFor(url), () => {}, Gerrit._pluginInstalled, async);
this._urlFor(url), () => {},
Gerrit._pluginInstallError.bind(null, `${url} import error`),
async);
}
},
@@ -86,18 +90,20 @@
const el = document.createElement('script');
el.defer = true;
el.src = url;
el.onerror = Gerrit._pluginInstalled;
el.onerror = Gerrit._pluginInstallError.bind(null, `${url} load error`);
return document.body.appendChild(el);
},
_urlFor(pathOrUrl) {
if (pathOrUrl.startsWith('http')) {
// Plugins are loaded from another domain.
return pathOrUrl;
}
if (!pathOrUrl.startsWith('/')) {
pathOrUrl = '/' + pathOrUrl;
}
return this.getBaseUrl() + pathOrUrl;
const {href, pathname} = window.location;
return href.split(pathname)[0] + this.getBaseUrl() + pathOrUrl;
},
});
})();

View File

@@ -36,12 +36,14 @@ limitations under the License.
suite('gr-diff tests', () => {
let element;
let sandbox;
let url;
setup(() => {
element = fixture('basic');
sandbox = sinon.sandbox.create();
sandbox.stub(document.body, 'appendChild');
sandbox.stub(element, 'importHref');
url = window.location.href.split(window.location.pathname)[0];
});
teardown(() => {
@@ -60,36 +62,43 @@ limitations under the License.
});
test('imports relative html plugins from config', () => {
sandbox.stub(Gerrit, '_pluginInstallError');
element.config = {
plugin: {html_resource_paths: ['foo/bar', 'baz']},
};
assert.equal(element.importHref.firstCall.args[0], '/foo/bar');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.equal(element.importHref.firstCall.args[0], url + '/foo/bar');
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0], '/baz');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.equal(element.importHref.secondCall.args[0], url + '/baz');
assert.isTrue(element.importHref.secondCall.args[3]);
assert.equal(Gerrit._pluginInstallError.callCount, 0);
element.importHref.firstCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 1);
element.importHref.secondCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 2);
});
test('imports relative html plugins from config with a base url', () => {
sandbox.stub(Gerrit, '_pluginInstallError');
sandbox.stub(element, 'getBaseUrl').returns('/the-base');
element.config = {
plugin: {html_resource_paths: ['foo/bar', 'baz']}};
assert.equal(element.importHref.firstCall.args[0], '/the-base/foo/bar');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.equal(element.importHref.firstCall.args[0],
url + '/the-base/foo/bar');
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0], '/the-base/baz');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.equal(element.importHref.secondCall.args[0],
url + '/the-base/baz');
assert.isTrue(element.importHref.secondCall.args[3]);
assert.equal(Gerrit._pluginInstallError.callCount, 0);
element.importHref.firstCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 1);
element.importHref.secondCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 2);
});
test('inportHref is not called with null callback functions', () => {
test('importHref is not called with null callback functions', () => {
const plugins = ['path/to/plugin'];
element._importHtmlPlugins(plugins);
assert.isTrue(element.importHref.calledOnce);
@@ -98,6 +107,7 @@ limitations under the License.
});
test('imports absolute html plugins from config', () => {
sandbox.stub(Gerrit, '_pluginInstallError');
element.config = {
plugin: {
html_resource_paths: [
@@ -108,15 +118,16 @@ limitations under the License.
};
assert.equal(element.importHref.firstCall.args[0],
'http://example.com/foo/bar');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0],
'https://example.com/baz');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.isTrue(element.importHref.secondCall.args[3]);
assert.equal(Gerrit._pluginInstallError.callCount, 0);
element.importHref.firstCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 1);
element.importHref.secondCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 2);
});
test('adds js plugins from config to the body', () => {
@@ -127,16 +138,18 @@ limitations under the License.
test('imports relative js plugins from config', () => {
sandbox.stub(element, '_createScriptTag');
element.config = {plugin: {js_resource_paths: ['foo/bar', 'baz']}};
assert.isTrue(element._createScriptTag.calledWith('/foo/bar'));
assert.isTrue(element._createScriptTag.calledWith('/baz'));
assert.isTrue(element._createScriptTag.calledWith(url + '/foo/bar'));
assert.isTrue(element._createScriptTag.calledWith(url + '/baz'));
});
test('imports relative html plugins from config with a base url', () => {
sandbox.stub(element, '_createScriptTag');
sandbox.stub(element, 'getBaseUrl').returns('/the-base');
element.config = {plugin: {js_resource_paths: ['foo/bar', 'baz']}};
assert.isTrue(element._createScriptTag.calledWith('/the-base/foo/bar'));
assert.isTrue(element._createScriptTag.calledWith('/the-base/baz'));
assert.isTrue(element._createScriptTag.calledWith(
url + '/the-base/foo/bar'));
assert.isTrue(element._createScriptTag.calledWith(
url + '/the-base/baz'));
});
test('imports absolute html plugins from config', () => {
@@ -156,21 +169,23 @@ limitations under the License.
});
test('default theme is loaded with html plugins', () => {
sandbox.stub(Gerrit, '_pluginInstallError');
element.config = {
default_theme: '/oof',
plugin: {
html_resource_paths: ['some'],
},
};
assert.equal(element.importHref.firstCall.args[0], '/oof');
assert.equal(element.importHref.firstCall.args[2],
Gerrit._pluginInstalled);
assert.equal(element.importHref.firstCall.args[0], url + '/oof');
assert.isFalse(element.importHref.firstCall.args[3]);
assert.equal(element.importHref.secondCall.args[0], '/some');
assert.equal(element.importHref.secondCall.args[2],
Gerrit._pluginInstalled);
assert.equal(element.importHref.secondCall.args[0], url + '/some');
assert.isTrue(element.importHref.secondCall.args[3]);
assert.equal(Gerrit._pluginInstallError.callCount, 0);
element.importHref.firstCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 1);
element.importHref.secondCall.args[2]();
assert.equal(Gerrit._pluginInstallError.callCount, 2);
});
});
</script>

View File

@@ -65,7 +65,7 @@ limitations under the License.
stub('gr-custom-plugin-header', {
ready() { customHeader = this; },
});
Gerrit._resolveAllPluginsLoaded();
Gerrit._setPluginsPending([]);
});
test('sets logo and title', done => {

View File

@@ -82,6 +82,7 @@
if (!el.content) { return; }
el.content.addEventListener('labels-changed', e => {
console.log('labels-changed', e.detail);
handler(e.detail);
});
});

View File

@@ -60,9 +60,9 @@ limitations under the License.
});
element = fixture('basic');
errorStub = sandbox.stub(console, 'error');
Gerrit._setPluginsCount(1);
Gerrit.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
Gerrit._setPluginsPending([]);
});
teardown(() => {
@@ -306,7 +306,6 @@ limitations under the License.
test('_setPluginsCount', done => {
stub('gr-reporting', {
pluginsLoaded() {
assert.equal(Gerrit._pluginsPending, 0);
done();
},
});
@@ -324,13 +323,11 @@ limitations under the License.
test('_pluginInstalled', done => {
stub('gr-reporting', {
pluginsLoaded() {
assert.equal(Gerrit._pluginsPending, 0);
done();
},
});
Gerrit._setPluginsCount(2);
Gerrit._pluginInstalled();
assert.equal(Gerrit._pluginsPending, 1);
Gerrit._pluginInstalled();
});
@@ -348,10 +345,10 @@ limitations under the License.
assert.isTrue(Gerrit._pluginInstalled.calledOnce);
});
test('install calls _pluginInstalled on error', () => {
sandbox.stub(Gerrit, '_pluginInstalled');
test('plugin install errors mark plugins as loaded', () => {
Gerrit._setPluginsCount(1);
Gerrit.install(() => {}, '0.0pre-alpha');
assert.isTrue(Gerrit._pluginInstalled.calledOnce);
return Gerrit.awaitPluginsLoaded();
});
test('installGwt calls _pluginInstalled', () => {

View File

@@ -20,13 +20,22 @@
/**
* Hash of loaded and installed plugins, name to Plugin object.
*/
const plugins = {};
const _plugins = {};
/**
* Array of plugin URLs to be loaded, name to url.
*/
let _pluginsPending = {};
let _pluginsPendingCount = -1;
const PANEL_ENDPOINTS_MAPPING = {
CHANGE_SCREEN_BELOW_COMMIT_INFO_BLOCK: 'change-view-integration',
CHANGE_SCREEN_BELOW_CHANGE_INFO_BLOCK: 'change-metadata-item',
};
const PLUGIN_LOADING_TIMEOUT_MS = 10000;
let _restAPI;
const getRestAPI = () => {
if (!_restAPI) {
@@ -80,6 +89,14 @@
window.$wnd = window;
function getPluginNameFromUrl(url) {
if (!(url instanceof URL)) {
try {
url = new URL(url);
} catch (e) {
console.warn(e);
return null;
}
}
const base = Gerrit.BaseUrlBehavior.getBaseUrl();
const pathname = url.pathname.replace(base, '');
// Site theme is server from predefined path.
@@ -393,22 +410,26 @@
const Gerrit = window.Gerrit || {};
let _resolveAllPluginsLoaded = null;
let _allPluginsPromise = null;
Gerrit._endpoints = new GrPluginEndpoints();
// Provide reset plugins function to clear installed plugins between tests.
const app = document.querySelector('#app');
if (!app) {
// No gr-app found (running tests)
Gerrit._resetPlugins = () => {
for (const k of Object.keys(plugins)) {
delete plugins[k];
_resolveAllPluginsLoaded = null;
_allPluginsPromise = null;
Gerrit._setPluginsPending([]);
Gerrit._endpoints = new GrPluginEndpoints();
for (const k of Object.keys(_plugins)) {
delete _plugins[k];
}
};
}
// Number of plugins to initialize, -1 means 'not yet known'.
Gerrit._pluginsPending = -1;
Gerrit._endpoints = new GrPluginEndpoints();
Gerrit.getPluginName = function() {
console.warn('Gerrit.getPluginName is not supported in PolyGerrit.',
'Please use plugin.getPluginName() instead.');
@@ -428,32 +449,29 @@
};
Gerrit.install = function(callback, opt_version, opt_src) {
const src = opt_src || (document.currentScript &&
(document.currentScript.src || document.currentScript.baseURI));
const name = getPluginNameFromUrl(src);
if (opt_version && opt_version !== API_VERSION) {
console.warn('Only version ' + API_VERSION +
' is supported in PolyGerrit. ' + opt_version + ' was given.');
Gerrit._pluginInstalled();
Gerrit._pluginInstallError(`Plugin ${name} install error: only version ` +
API_VERSION + ' is supported in PolyGerrit. ' + opt_version +
' was given.');
return;
}
const src = opt_src || (document.currentScript &&
(document.currentScript.src || document.currentScript.baseURI));
const name = getPluginNameFromUrl(new URL(src));
const existingPlugin = plugins[name];
const existingPlugin = _plugins[name];
const plugin = existingPlugin || new Plugin(src);
try {
callback(plugin);
plugins[name] = plugin;
if (name) {
_plugins[name] = plugin;
}
if (!existingPlugin) {
Gerrit._pluginInstalled(src);
}
} catch (e) {
console.warn(`${name} install failed: ${e.name}: ${e.message}`);
}
// Don't double count plugins that may have an html and js install.
// TODO(beckysiegel) remove name check once name issue is resolved.
// If there isn't a name, it's due to an issue with the polyfill for
// html imports in Safari/Firefox. In this case, other plugin related
// features may still be broken, but still make sure to call.
// _pluginInstalled.
if (!name || !existingPlugin) {
Gerrit._pluginInstalled();
Gerrit._pluginInstallError(`${e.name}: ${e.message}`);
}
};
@@ -502,50 +520,85 @@
* @deprecated best effort support, will be removed with GWT UI.
*/
Gerrit.installGwt = function(url) {
Gerrit._pluginInstalled();
const name = getPluginNameFromUrl(new URL(url));
const name = getPluginNameFromUrl(url);
let plugin;
try {
plugin = plugins[name] || new Plugin(url);
plugin = _plugins[name] || new Plugin(url);
plugin.deprecated.install();
Gerrit._pluginInstalled(url);
} catch (e) {
console.warn(`${name} install failed: ${e.name}: ${e.message}`);
Gerrit._pluginInstallError(`${e.name}: ${e.message}`);
}
return plugin;
};
Gerrit._allPluginsPromise = null;
Gerrit._resolveAllPluginsLoaded = null;
Gerrit.awaitPluginsLoaded = function() {
if (!Gerrit._allPluginsPromise) {
if (!_allPluginsPromise) {
if (Gerrit._arePluginsLoaded()) {
Gerrit._allPluginsPromise = Promise.resolve();
_allPluginsPromise = Promise.resolve();
} else {
Gerrit._allPluginsPromise = new Promise(resolve => {
Gerrit._resolveAllPluginsLoaded = resolve;
});
let timeoutId;
_allPluginsPromise =
Promise.race([
new Promise(resolve => _resolveAllPluginsLoaded = resolve),
new Promise(resolve => timeoutId = setTimeout(
Gerrit._pluginLoadingTimeout, PLUGIN_LOADING_TIMEOUT_MS)),
]).then(() => clearTimeout(timeoutId));
}
}
return Gerrit._allPluginsPromise;
return _allPluginsPromise;
};
Gerrit._pluginLoadingTimeout = function() {
document.dispatchEvent(new CustomEvent('show-alert', {
detail: {
message: 'Plugins loading timeout. Check the console for errors.',
},
}));
console.error(`Failed to load plugins: ${Object.keys(_pluginsPending)}`);
Gerrit._setPluginsPending([]);
};
Gerrit._setPluginsPending = function(plugins) {
_pluginsPending = plugins.reduce((o, url) => {
o[getPluginNameFromUrl(url)] = url;
return o;
}, {});
Gerrit._setPluginsCount(plugins.length);
};
Gerrit._setPluginsCount = function(count) {
Gerrit._pluginsPending = count;
_pluginsPendingCount = count;
if (Gerrit._arePluginsLoaded()) {
document.createElement('gr-reporting').pluginsLoaded();
if (Gerrit._resolveAllPluginsLoaded) {
Gerrit._resolveAllPluginsLoaded();
if (_resolveAllPluginsLoaded) {
_resolveAllPluginsLoaded();
}
}
};
Gerrit._pluginInstalled = function() {
Gerrit._setPluginsCount(Gerrit._pluginsPending - 1);
Gerrit._pluginInstallError = function(message) {
console.log(`Plugin install error: ${message}`);
Gerrit._setPluginsCount(_pluginsPendingCount - 1);
};
Gerrit._pluginInstalled = function(url) {
const name = getPluginNameFromUrl(url);
if (name && !_pluginsPending[name]) {
console.warn(`Unexpected plugin from ${url}!`);
} else {
if (name) {
delete _pluginsPending[name];
console.log(`Plugin ${name} installed`);
} else {
console.log(`Plugin installed from ${url}`);
}
Gerrit._setPluginsCount(_pluginsPendingCount - 1);
}
};
Gerrit._arePluginsLoaded = function() {
return Gerrit._pluginsPending === 0;
return _pluginsPendingCount === 0;
};
Gerrit._getPluginScreenName = function(pluginName, screenName) {

View File

@@ -49,14 +49,9 @@ limitations under the License.
(function() {
setup(() => {
if (!window.Gerrit) { return; }
Gerrit._pluginsPending = -1;
Gerrit._allPluginsPromise = undefined;
if (Gerrit._resetPlugins) {
Gerrit._resetPlugins();
}
if (Gerrit._endpoints) {
Gerrit._endpoints = new GrPluginEndpoints();
}
});
})();
</script>