opendev/gerrit
Code Issues Proposed changes

Revert "Revert "Support load from ASSETS_PATH for plugins if provided""

Browse Source 
Also update `self.url` documentation to reflect the changes.

This reverts commit 964455ae6f.

Reason for revert: moving forward as misuse of plugin._url is fixed

Bug: Issue 11998
Change-Id: I8c4d888dae3e8df0f15ad210c4f40ac9704d5b37
This commit is contained in:
Tao Zhou
2019-11-29 15:34:26 +01:00
parent 2342590d94
commit 542096e72c
7 changed files with 180 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
Documentation/js-api.txt
View File

@@ -150,6 +150,13 @@ Returns a URL within the plugin's URL space. If invoked with no
parameter the URL of the plugin is returned. If passed a string
the argument is appended to the plugin URL.
A plugin's URL is where this plugin is loaded, it doesn't
necessary to be the same as the Gerrit host. Use `window.location`
if you need to access the Gerrit host info.
For preloaded plugins, the plugin url is based on a global
configuration of where to load all plugins, default to current host.
[source,javascript]
----
self.url(); // "https://gerrit-review.googlesource.com/plugins/demo/"

6
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js
View File

@@ -50,7 +50,11 @@
return url.pathname;
}
const base = Gerrit.BaseUrlBehavior.getBaseUrl();
const pathname = url.pathname.replace(base, '');
let pathname = url.pathname.replace(base, '');
// Load from ASSETS_PATH
if (window.ASSETS_PATH && url.href.includes(window.ASSETS_PATH)) {
pathname = url.href.replace(window.ASSETS_PATH, '');
}
// Site theme is server from predefined path.
if (pathname === '/static/gerrit-theme.html') {
return 'gerrit-theme';

9
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html
View File

@@ -72,6 +72,15 @@ limitations under the License.
'gerrit-theme'
);
});
test('with ASSETS_PATH', () => {
window.ASSETS_PATH = 'http://cdn.com/2';
assert.equal(
getPluginNameFromUrl(`${window.ASSETS_PATH}/plugins/a.html`),
'a'
);
window.ASSETS_PATH = undefined;
});
});
});
</script>

22
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
View File

@@ -82,6 +82,28 @@ limitations under the License.
'http://test.com/plugins/testplugin/static/test.js');
});
test('url for preloaded plugin without ASSETS_PATH', () => {
let plugin;
Gerrit.install(p => { plugin = p; }, '0.1',
'preloaded:testpluginB');
assert.equal(plugin.url(),
`${window.location.origin}/plugins/testpluginB/`);
assert.equal(plugin.url('/static/test.js'),
`${window.location.origin}/plugins/testpluginB/static/test.js`);
});
test('url for preloaded plugin without ASSETS_PATH', () => {
const oldAssetsPath = window.ASSETS_PATH;
window.ASSETS_PATH = 'http://test.com';
let plugin;
Gerrit.install(p => { plugin = p; }, '0.1',
'preloaded:testpluginC');
assert.equal(plugin.url(), `${window.ASSETS_PATH}/plugins/testpluginC/`);
assert.equal(plugin.url('/static/test.js'),
`${window.ASSETS_PATH}/plugins/testpluginC/static/test.js`);
window.ASSETS_PATH = oldAssetsPath;
});
test('_send on failure rejects with response text', () => {
sendStub.returns(Promise.resolve(
{status: 400, text() { return Promise.resolve('text'); }}));

74
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.js
View File

@@ -113,7 +113,7 @@
this._pluginListLoaded = true;
plugins.forEach(path => {
const url = this._urlFor(path);
const url = this._urlFor(path, window.ASSETS_PATH);
// Skip if preloaded, for bundling.
if (this.isPluginPreloaded(url)) return;
@@ -128,11 +128,11 @@
});
if (this._isPathEndsWith(url, '.html')) {
this._importHtmlPlugin(url, opts && opts[path]);
this._importHtmlPlugin(path, opts && opts[path]);
} else if (this._isPathEndsWith(url, '.js')) {
this._loadJsPlugin(url);
this._loadJsPlugin(path);
} else {
this._failToLoad(`Unrecognized plugin url ${url}`, url);
this._failToLoad(`Unrecognized plugin path ${path}`, path);
}
});
@@ -181,14 +181,15 @@
return;
}
const pluginObject = this.getPlugin(src);
const url = this._urlFor(src);
const pluginObject = this.getPlugin(url);
let plugin = pluginObject && pluginObject.plugin;
if (!plugin) {
plugin = new Plugin(src);
plugin = new Plugin(url);
}
try {
callback(plugin);
this._pluginInstalled(src, plugin);
this._pluginInstalled(url, plugin);
} catch (e) {
this._failToLoad(`${e.name}: ${e.message}`, src);
}
@@ -313,38 +314,79 @@
}
_importHtmlPlugin(pluginUrl, opts = {}) {
// onload (second param) needs to be a function. When null or undefined
// were passed, plugins were not loaded correctly.
const urlWithAP = this._urlFor(pluginUrl, window.ASSETS_PATH);
const urlWithoutAP = this._urlFor(pluginUrl);
let onerror = null;
if (urlWithAP !== urlWithoutAP) {
onerror = () => this._loadHtmlPlugin(urlWithoutAP, opts.sync);
}
this._loadHtmlPlugin(urlWithAP, opts.sync, onerror);
}
_loadHtmlPlugin(url, sync, onerror) {
if (!onerror) {
onerror = () => {
this._failToLoad(`${url} import error`, url);
};
}
(Polymer.importHref || Polymer.Base.importHref)(
this._urlFor(pluginUrl), () => {},
() => this._failToLoad(`${pluginUrl} import error`, pluginUrl),
!opts.sync);
url, () => {},
onerror,
!sync);
}
_loadJsPlugin(pluginUrl) {
this._createScriptTag(this._urlFor(pluginUrl));
const urlWithAP = this._urlFor(pluginUrl, window.ASSETS_PATH);
const urlWithoutAP = this._urlFor(pluginUrl);
let onerror = null;
if (urlWithAP !== urlWithoutAP) {
onerror = () => this._createScriptTag(urlWithoutAP);
}
this._createScriptTag(urlWithAP, onerror);
}
_createScriptTag(url) {
_createScriptTag(url, onerror) {
if (!onerror) {
onerror = () => this._failToLoad(`${url} load error`, url);
}
const el = document.createElement('script');
el.defer = true;
el.setAttribute('src', url);
el.onerror = () => this._failToLoad(`${url} load error`, url);
el.onerror = onerror;
return document.body.appendChild(el);
}
_urlFor(pathOrUrl) {
_urlFor(pathOrUrl, assetsPath) {
if (!pathOrUrl) {
return pathOrUrl;
}
// theme is per host, should always load from assetsPath
const isThemeFile = pathOrUrl.endsWith('static/gerrit-theme.html');
const shouldTryLoadFromAssetsPathFirst = !isThemeFile && assetsPath;
if (pathOrUrl.startsWith(PRELOADED_PROTOCOL) ||
pathOrUrl.startsWith('http')) {
// Plugins are loaded from another domain or preloaded.
if (pathOrUrl.includes(location.host)
&& shouldTryLoadFromAssetsPathFirst) {
// if is loading from host server, try replace with cdn when assetsPath provided
return pathOrUrl
.replace(location.origin, assetsPath);
}
return pathOrUrl;
}
if (!pathOrUrl.startsWith('/')) {
pathOrUrl = '/' + pathOrUrl;
}
if (shouldTryLoadFromAssetsPathFirst) {
return assetsPath + pathOrUrl;
}
return window.location.origin + getBaseUrl() + pathOrUrl;
}

78
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.html
View File

@@ -325,11 +325,11 @@ limitations under the License.
let loadJsPluginStub;
setup(() => {
importHtmlPluginStub = sandbox.stub();
sandbox.stub(Gerrit._pluginLoader, '_importHtmlPlugin', url => {
sandbox.stub(Gerrit._pluginLoader, '_loadHtmlPlugin', url => {
importHtmlPluginStub(url);
});
loadJsPluginStub = sandbox.stub();
sandbox.stub(Gerrit._pluginLoader, '_loadJsPlugin', url => {
sandbox.stub(Gerrit._pluginLoader, '_createScriptTag', url => {
loadJsPluginStub(url);
});
});
@@ -346,8 +346,8 @@ limitations under the License.
assert.isTrue(failToLoadStub.calledOnce);
assert.isTrue(failToLoadStub.calledWithExactly(
`Unrecognized plugin url ${url}/foo/bar`,
`${url}/foo/bar`
'Unrecognized plugin path foo/bar',
'foo/bar'
));
});
@@ -407,6 +407,72 @@ limitations under the License.
});
});
suite('With ASSETS_PATH', () => {
let importHtmlPluginStub;
let loadJsPluginStub;
setup(() => {
window.ASSETS_PATH = 'https://cdn.com';
importHtmlPluginStub = sandbox.stub();
sandbox.stub(Gerrit._pluginLoader, '_loadHtmlPlugin', url => {
importHtmlPluginStub(url);
});
loadJsPluginStub = sandbox.stub();
sandbox.stub(Gerrit._pluginLoader, '_createScriptTag', url => {
loadJsPluginStub(url);
});
});
teardown(() => {
window.ASSETS_PATH = '';
});
test('Should try load plugins from assets path instead', () => {
Gerrit._loadPlugins([
'foo/bar.js',
'foo/bar.html',
]);
assert.isTrue(importHtmlPluginStub.calledOnce);
assert.isTrue(
importHtmlPluginStub.calledWithExactly(`https://cdn.com/foo/bar.html`)
);
assert.isTrue(loadJsPluginStub.calledOnce);
assert.isTrue(
loadJsPluginStub.calledWithExactly(`https://cdn.com/foo/bar.js`));
});
test('Should honor original path if exists', () => {
Gerrit._loadPlugins([
'http://e.com/foo/bar.html',
'http://e.com/foo/bar.js',
]);
assert.isTrue(importHtmlPluginStub.calledOnce);
assert.isTrue(
importHtmlPluginStub.calledWithExactly(`http://e.com/foo/bar.html`)
);
assert.isTrue(loadJsPluginStub.calledOnce);
assert.isTrue(
loadJsPluginStub.calledWithExactly(`http://e.com/foo/bar.js`));
});
test('Should try replace current host with assetsPath', () => {
const host = window.location.origin;
Gerrit._loadPlugins([
`${host}/foo/bar.html`,
`${host}/foo/bar.js`,
]);
assert.isTrue(importHtmlPluginStub.calledOnce);
assert.isTrue(
importHtmlPluginStub.calledWithExactly(`https://cdn.com/foo/bar.html`)
);
assert.isTrue(loadJsPluginStub.calledOnce);
assert.isTrue(
loadJsPluginStub.calledWithExactly(`https://cdn.com/foo/bar.js`));
});
});
test('adds js plugins will call the body', () => {
Gerrit._loadPlugins([
'http://e.com/foo/bar.js',
@@ -489,12 +555,10 @@ limitations under the License.
test('installing preloaded plugin', () => {
let plugin;
window.ASSETS_PATH = 'http://blips.com/chitz';
Gerrit.install(p => { plugin = p; }, '0.1', 'preloaded:foo');
assert.strictEqual(plugin.getPluginName(), 'foo');
assert.strictEqual(plugin.url('/some/thing.html'),
'http://blips.com/chitz/plugins/foo/some/thing.html');
delete window.ASSETS_PATH;
`${window.location.origin}/plugins/foo/some/thing.html`);
});
});
});

18
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
View File

@@ -17,8 +17,6 @@
(function(window) {
'use strict';
const PRELOADED_PROTOCOL = 'preloaded:';
const PANEL_ENDPOINTS_MAPPING = {
CHANGE_SCREEN_BELOW_COMMIT_INFO_BLOCK: 'change-view-integration',
CHANGE_SCREEN_BELOW_CHANGE_INFO_BLOCK: 'change-metadata-item',
@@ -26,6 +24,7 @@
// Import utils methods
const {
PRELOADED_PROTOCOL,
getPluginNameFromUrl,
send,
} = window._apiUtils;
@@ -66,13 +65,6 @@
this._url = new URL(opt_url);
this._name = getPluginNameFromUrl(this._url);
if (this._url.protocol === PRELOADED_PROTOCOL) {
// Original plugin URL is used in plugin assets URLs calculation.
const assetsBaseUrl = window.ASSETS_PATH ||
(window.location.origin + Gerrit.BaseUrlBehavior.getBaseUrl());
this._url = new URL(assetsBaseUrl + '/plugins/' + this._name +
'/static/' + this._name + '.js');
}
}
Plugin._sharedAPIElement = document.createElement('gr-js-api-interface');
@@ -139,9 +131,15 @@
Plugin.prototype.url = function(opt_path) {
const relPath = '/plugins/' + this._name + (opt_path || '/');
const sameOriginPath = window.location.origin +
`${Gerrit.BaseUrlBehavior.getBaseUrl()}${relPath}`;
if (window.location.origin === this._url.origin) {
// Plugin loaded from the same origin as gr-app, getBaseUrl in effect.
return this._url.origin + Gerrit.BaseUrlBehavior.getBaseUrl() + relPath;
return sameOriginPath;
} else if (this._url.protocol === PRELOADED_PROTOCOL) {
// Plugin is preloaded, load plugin with ASSETS_PATH or location.origin
return window.ASSETS_PATH ? `${window.ASSETS_PATH}${relPath}`
: sameOriginPath;
} else {
// Plugin loaded from assets bundle, expect assets placed along with it.
return this._url.href.split('/plugins/' + this._name)[0] + relPath;