From fb823761baddb2db645e18f8ace904ffe5580b0a Mon Sep 17 00:00:00 2001 From: Scott Hunter Date: Thu, 21 Mar 2019 16:03:45 -0400 Subject: [PATCH] Fix Resource template replacement. The previous implementation did not work with template keys that are numbers, because the regex syntax was not escaped properly. Instead of building a regex for each replacement, we can use a replace callback method. --- CHANGES.md | 1 + Source/Core/Resource.js | 18 +++++++++-------- Specs/Core/Iau2006XysDataSpec.js | 14 +++++++++++++ Specs/Core/ResourceSpec.js | 34 +++++++++++++++++++++++++++++++- 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a2dfb123e38c..19eac63f74a7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,7 @@ Change Log * Fixed the value for `BlendFunction.ONE_MINUS_CONSTANT_COLOR`. [#7624](https://github.com/AnalyticalGraphicsInc/cesium/pull/7624) * Fixed `HeadingPitchRoll.pitch` being `NaN` when using `.fromQuaternion` do to a rounding error for pitches close to +/- 90°. [#7654](https://github.com/AnalyticalGraphicsInc/cesium/pull/7654) +* Fixed an error in `Resource` when used with template replacements using numeric keys. [#7668](https://github.com/AnalyticalGraphicsInc/cesium/pull/7668) ### 1.55 - 2019-03-01 diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js index 9034d45d0a7b..c1662917fb4a 100644 --- a/Source/Core/Resource.js +++ b/Source/Core/Resource.js @@ -569,15 +569,17 @@ define([ // objectToQuery escapes the placeholders. Undo that. var url = uri.toString().replace(/%7B/g, '{').replace(/%7D/g, '}'); - var template = this._templateValues; - var keys = Object.keys(template); - if (keys.length > 0) { - for (var i = 0; i < keys.length; i++) { - var key = keys[i]; - var value = template[key]; - url = url.replace(new RegExp('{' + key + '}', 'g'), encodeURIComponent(value)); + var templateValues = this._templateValues; + url = url.replace(/{(.*?)}/g, function(match, key) { + var replacement = templateValues[key]; + if (defined(replacement)) { + // use the replacement value from templateValues if there is one... + return encodeURIComponent(replacement); } - } + // otherwise leave it unchanged + return match; + }); + if (proxy && defined(this.proxy)) { url = this.proxy.getURL(url); } diff --git a/Specs/Core/Iau2006XysDataSpec.js b/Specs/Core/Iau2006XysDataSpec.js index 01633034aa45..2f7d87750cc7 100644 --- a/Specs/Core/Iau2006XysDataSpec.js +++ b/Specs/Core/Iau2006XysDataSpec.js @@ -1,10 +1,12 @@ defineSuite([ 'Core/Iau2006XysData', + 'Core/buildModuleUrl', 'Core/defined', 'Core/Iau2006XysSample', 'Specs/pollToPromise' ], function( Iau2006XysData, + buildModuleUrl, defined, Iau2006XysSample, pollToPromise) { @@ -51,4 +53,16 @@ defineSuite([ it('returns undefined after the last XYS table sample', function() { expect(xys.computeXysRadians(2442396 + 27427, 0.0)).toBeUndefined(); }); + + it('allows configuring xysFileUrlTemplate', function() { + xys = new Iau2006XysData({ + // this should be the same location as the default, but specifying the value + // takes the code through a different code path. + xysFileUrlTemplate: buildModuleUrl('Assets/IAU2006_XYS/IAU2006_XYS_{0}.json') + }); + + return pollToPromise(function() { + return defined(xys.computeXysRadians(2442398, 1234.56)); + }); + }); }); diff --git a/Specs/Core/ResourceSpec.js b/Specs/Core/ResourceSpec.js index 9203cd4e9c93..d087c2038593 100644 --- a/Specs/Core/ResourceSpec.js +++ b/Specs/Core/ResourceSpec.js @@ -191,7 +191,7 @@ defineSuite([ expect(derived.url).toEqual('http://test.com/tileset/other_endpoint?a=5&a=7&a=1&a=2&a=4&b=6&b=3'); }); - it('templateValues are respected', function() { + it('replaces templateValues in the url', function() { var resource = new Resource({ url: 'http://test.com/tileset/{foo}/{bar}', templateValues: { @@ -203,6 +203,38 @@ defineSuite([ expect(resource.url).toEqual('http://test.com/tileset/test1/test2'); }); + it('replaces numeric templateValues', function() { + var resource = new Resource({ + url: 'http://test.com/tileset/{0}/{1}', + templateValues: { + '0': 'test1', + '1': 'test2' + } + }); + + expect(resource.url).toEqual('http://test.com/tileset/test1/test2'); + }); + + it('leaves templateValues unchanged that are not provided', function() { + var resource = new Resource({ + url: 'http://test.com/tileset/{foo}/{bar}' + }); + + expect(resource.url).toEqual('http://test.com/tileset/{foo}/{bar}'); + }); + + it('url encodes replacement templateValues in the url', function() { + var resource = new Resource({ + url: 'http://test.com/tileset/{foo}/{bar}', + templateValues: { + foo: 'a/b', + bar: 'x$y#' + } + }); + + expect(resource.url).toEqual('http://test.com/tileset/a%2Fb/x%24y%23'); + }); + it('getDerivedResource sets correct properties', function() { var proxy = new DefaultProxy('/proxy/'); var request = new Request();