Skip to content

Commit 03dc9d9

Browse files
elastic-jasperspalger
authored andcommitted
[tilemap/config] Fix url manipulation and promise (#9780)
Backports PR #9754 **Commit 1:** [ui/vis_maps/tests] use $injector to avoid awkward naming * Original sha: 52d35fb * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-04T19:39:37Z **Commit 2:** [ui/vis_maps/tests] avoid boolean assertions for better error messages * Original sha: b118a07 * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-04T20:39:53Z **Commit 3:** [ui/vis_maps/tests] verify addQueryParams behavior over time * Original sha: dff8315 * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-05T19:31:45Z **Commit 4:** [ui/vis_maps/tests] stub tilemapsConfig rather than monkey-patching it * Original sha: 3099bb0 * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-04T20:52:30Z **Commit 5:** [ui/vis_maps] remove async/await usage because angular * Original sha: 084b914 * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-04T21:00:21Z **Commit 6:** [ui/vis_maps/tests] add failing test * Original sha: f38e7dc * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-05T20:21:20Z **Commit 7:** [ui/url] add modifyUrl() helper * Original sha: d4b9849 * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-03T19:40:20Z **Commit 8:** [ui/vis_maps] use modifyUrl() helper to extend query string * Original sha: bf4083f * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-03T19:49:56Z **Commit 9:** Merge branch 'master' of github.com:elastic/kibana into fix/tilemap-manifest-url-manipulation * Original sha: 22669aa * Authored by spalger <spalger@users.noreply.github.com> on 2017-01-06T19:21:41Z
1 parent 62480c2 commit 03dc9d9

File tree

7 files changed

+300
-136
lines changed

7 files changed

+300
-136
lines changed
+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { parse as parseUrl } from 'url';
2+
3+
import expect from 'expect.js';
4+
5+
import { modifyUrl } from '../modify_url';
6+
7+
describe('modifyUrl()', () => {
8+
it('throws an error with invalid input', () => {
9+
expect(() => modifyUrl(1, () => {})).to.throwError();
10+
expect(() => modifyUrl(undefined, () => {})).to.throwError();
11+
expect(() => modifyUrl('http://localhost')).to.throwError(); // no block
12+
});
13+
14+
it('supports returning a new url spec', () => {
15+
expect(modifyUrl('http://localhost', () => ({}))).to.eql('');
16+
});
17+
18+
it('supports modifying the passed object', () => {
19+
expect(modifyUrl('http://localhost', parsed => {
20+
parsed.port = 9999;
21+
parsed.auth = 'foo:bar';
22+
})).to.eql('http://foo:bar@localhost:9999/');
23+
});
24+
25+
it('supports changing pathname', () => {
26+
expect(modifyUrl('http://localhost/some/path', parsed => {
27+
parsed.pathname += '/subpath';
28+
})).to.eql('http://localhost/some/path/subpath');
29+
});
30+
31+
it('supports changing port', () => {
32+
expect(modifyUrl('http://localhost:5601', parsed => {
33+
parsed.port = (parsed.port * 1) + 1;
34+
})).to.eql('http://localhost:5602/');
35+
});
36+
37+
it('supports changing protocol', () => {
38+
expect(modifyUrl('http://localhost', parsed => {
39+
parsed.protocol = 'mail';
40+
parsed.slashes = false;
41+
parsed.pathname = null;
42+
})).to.eql('mail:localhost');
43+
});
44+
});

src/ui/public/url/index.js

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { KbnUrlProvider as default } from './url';
2+
export { modifyUrl } from './modify_url';

src/ui/public/url/modify_url.js

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { parse as parseUrl, format as formatUrl } from 'url';
2+
3+
/**
4+
* Takes a URL and a function that takes the meaningful parts
5+
* of the URL as a key-value object, modifies some or all of
6+
* the parts, and returns the modified parts formatted again
7+
* as a url.
8+
*
9+
* Url Parts sent:
10+
* - protocol
11+
* - slashes (does the url have the //)
12+
* - auth
13+
* - hostname (just the name of the host, no port or auth information)
14+
* - port
15+
* - pathmame (the path after the hostname, no query or hash, starts
16+
* with a slash if there was a path)
17+
* - query (always an object, even when no query on original url)
18+
* - hash
19+
*
20+
* Why?
21+
* - The default url library in node produces several conflicting
22+
* properties on the "parsed" output. Modifying any of these might
23+
* lead to the modifications being ignored (depending on which
24+
* property was modified)
25+
* - It's not always clear wither to use path/pathname, host/hostname,
26+
* so this trys to add helpful constraints
27+
*
28+
* @param {String} url - the url to parse
29+
* @param {Function<Object|undefined>} block - a function that will modify the parsed url, or return a new one
30+
* @return {String} the modified and reformatted url
31+
*/
32+
export function modifyUrl(url, block) {
33+
if (typeof block !== 'function') {
34+
throw new TypeError('You must pass a block to define the modifications desired');
35+
}
36+
37+
const parsed = parseUrl(url, true);
38+
39+
// copy over the most specific version of each
40+
// property. By default, the parsed url includes
41+
// several conflicting properties (like path and
42+
// pathname + search, or search and query) and keeping
43+
// track of which property is actually used when they
44+
// are formatted is harder than necessary
45+
const meaningfulParts = {
46+
protocol: parsed.protocol,
47+
slashes: parsed.slashes,
48+
auth: parsed.auth,
49+
hostname: parsed.hostname,
50+
port: parsed.port,
51+
pathname: parsed.pathname,
52+
query: parsed.query || {},
53+
hash: parsed.hash,
54+
};
55+
56+
// the block modifies the meaningfulParts object, or returns a new one
57+
const modifications = block(meaningfulParts) || meaningfulParts;
58+
59+
// format the modified/replaced meaningfulParts back into a url
60+
return formatUrl(modifications);
61+
}

src/ui/public/url/url.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import AppStateProvider from 'ui/state_management/app_state';
88
uiModules.get('kibana/url')
99
.service('kbnUrl', function (Private) { return Private(KbnUrlProvider); });
1010

11-
function KbnUrlProvider($injector, $location, $rootScope, $parse, Private) {
12-
let self = this;
11+
export function KbnUrlProvider($injector, $location, $rootScope, $parse, Private) {
12+
const self = this;
1313

1414
/**
1515
* Navigate to a url
@@ -198,5 +198,3 @@ function KbnUrlProvider($injector, $location, $rootScope, $parse, Private) {
198198
return (reloadOnSearch && searchSame) || !reloadOnSearch;
199199
};
200200
}
201-
202-
export default KbnUrlProvider;

src/ui/public/vis_maps/__tests__/tile_maps/tilemap_settings.js

+39-22
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,57 @@ import ngMock from 'ng_mock';
33
import url from 'url';
44

55
describe('tilemaptest - TileMapSettingsTests-deprecated', function () {
6-
let theTileMapSettings;
7-
let theTilemapsConfig;
8-
9-
beforeEach(ngMock.module('kibana'));
10-
beforeEach(ngMock.inject(function (Private, tilemapSettings, tilemapsConfig) {
11-
theTileMapSettings = tilemapSettings;
12-
theTilemapsConfig = tilemapsConfig;
13-
theTilemapsConfig.deprecated.isOverridden = true;
6+
let tilemapSettings;
7+
let tilemapsConfig;
8+
let loadSettings;
9+
10+
beforeEach(ngMock.module('kibana', ($provide) => {
11+
$provide.decorator('tilemapsConfig', () => ({
12+
manifestServiceUrl: 'https://proxy-tiles.elastic.co/v1/manifest',
13+
deprecated: {
14+
isOverridden: true,
15+
config: {
16+
url: 'https://tiles.elastic.co/v1/default/{z}/{x}/{y}.png?my_app_name=kibana_tests',
17+
options: {
18+
minZoom: 1,
19+
maxZoom: 10,
20+
attribution: '© [Elastic Tile Service](https://www.elastic.co/elastic_tile_service)'
21+
}
22+
},
23+
}
24+
}));
1425
}));
1526

27+
beforeEach(ngMock.inject(function ($injector, $rootScope) {
28+
tilemapSettings = $injector.get('tilemapSettings');
29+
tilemapsConfig = $injector.get('tilemapsConfig');
1630

17-
describe('getting settings', function () {
31+
loadSettings = () => {
32+
tilemapSettings.loadSettings();
33+
$rootScope.$digest();
34+
};
35+
}));
1836

19-
beforeEach(async function () {
20-
await theTileMapSettings.loadSettings();
37+
describe('getting settings', function () {
38+
beforeEach(function () {
39+
loadSettings();
2140
});
2241

23-
it('should get url', async function () {
42+
it('should get url', function () {
2443

25-
const mapUrl = theTileMapSettings.getUrl();
26-
expect(mapUrl.indexOf('{x}') > -1).to.be.ok();
27-
expect(mapUrl.indexOf('{y}') > -1).to.be.ok();
28-
expect(mapUrl.indexOf('{z}') > -1).to.be.ok();
44+
const mapUrl = tilemapSettings.getUrl();
45+
expect(mapUrl).to.contain('{x}');
46+
expect(mapUrl).to.contain('{y}');
47+
expect(mapUrl).to.contain('{z}');
2948

3049
const urlObject = url.parse(mapUrl, true);
31-
expect(urlObject.host.endsWith('elastic.co')).to.be.ok();
32-
expect(urlObject.query).to.have.property('my_app_name');
33-
expect(urlObject.query).to.have.property('my_app_version');
34-
expect(urlObject.query).to.have.property('elastic_tile_service_tos');
50+
expect(urlObject.hostname).to.be('tiles.elastic.co');
51+
expect(urlObject.query).to.have.property('my_app_name', 'kibana_tests');
3552

3653
});
3754

38-
it('should get options', async function () {
39-
const options = theTileMapSettings.getOptions();
55+
it('should get options', function () {
56+
const options = tilemapSettings.getOptions();
4057
expect(options).to.have.property('minZoom');
4158
expect(options).to.have.property('maxZoom');
4259
expect(options).to.have.property('attribution');

0 commit comments

Comments
 (0)