Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom parameters in the template url of an UrlTemplateImageryP… #5696

Merged
merged 5 commits into from
Oct 20, 2017
Merged

Allow custom parameters in the template url of an UrlTemplateImageryP… #5696

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e726b0d
Allow custom parameters in the template url of an UrlTemplateImageryP…
Jul 31, 2017
8566cf2
Remove use of curly braces
Aug 3, 2017
26ec8de
Don't override tags and pickFeaturesTags objects
Sep 20, 2017
2ed8e9f
Merge remote-tracking branch 'upstream/master' into time
Oct 20, 2017
d6abaf1
fix changes
Oct 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Change Log
* Fixed specular computation for certain models using the `KHR_materials_common` extension. [#5773](https://github.com/AnalyticalGraphicsInc/cesium/pull/5773)
* Fixed a picking bug in the `3D Tiles Interactivity` Sandcastle demo. [#5703](https://github.com/AnalyticalGraphicsInc/cesium/issues/5703)
* Updated knockout from 3.4.0 to 3.4.2 [#5703](https://github.com/AnalyticalGraphicsInc/cesium/pull/5829)
* Added `customTags` property to the UrlTemplateImageryProvider to allow custom keywords in the template URL.

### 1.36 - 2017-08-01

Expand Down
20 changes: 20 additions & 0 deletions Source/Scene/UrlTemplateImageryProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ define([
* source does not support picking features or if you don't want this provider's features to be pickable. Note
* that this can be dynamically overridden by modifying the {@link UriTemplateImageryProvider#enablePickFeatures}
* property.
* @param {Object} [options.customTags] Allow to replace custom keywords in the URL template. The object must have strings as keys and functions as values.
*
*
* @example
Expand All @@ -157,6 +158,15 @@ define([
* 'width=256&height=256',
* rectangle : Cesium.Rectangle.fromDegrees(96.799393, -43.598214999057824, 153.63925700000001, -9.2159219997013)
* });
* // Using custom tags in your template url.
* var custom = new Cesium.UrlTemplateImageryProvider({
* url : 'https://yoururl/{Time}/{z}/{y}/{x}.png',
* customTags : {
* Time: function(imageryProvider, x, y , level) {
* return '20171231'
* }
* }
* });
*
* @see ArcGisMapServerImageryProvider
* @see BingMapsImageryProvider
Expand Down Expand Up @@ -568,6 +578,16 @@ define([
}
that._credit = credit;

if (defined(properties.customTags)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general rule, we try to keep property access to a minimum, rather than have properties.customTags multiple times, extract a var customTags = properties.customTags variable and use it throughout.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

for (var tag in properties.customTags) {
if (properties.customTags.hasOwnProperty(tag)) {
var targetTag = '{' + tag + '}';
tags[targetTag] = properties.customTags[tag]; //eslint-disable-line no-use-before-define
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised these eslint directives are needed. What does it thing is being used before define?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here was the messages:

/home/travis/build/AnalyticalGraphicsInc/cesium/Source/Scene/UrlTemplateImageryProvider.js
  585:25  error  'tags' was used before it was defined              no-use-before-define
  586:25  error  'pickFeaturesTags' was used before it was defined  no-use-before-define

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the directives and simple move tags and pickFeaturesTags to the top of the file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized there is a major issue here. Since tags and pickFeaturesTags used to be readonly, the same set is shared across multiple instances. However, now we are modifying tags and pickFeaturesTags which means that the modified version is used by all instances. Shouldn't these objects be per-instance now?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tags and pickFeaturesTags are the same for all objects.
Maybe I can just append the customTags values to tags values in a new object and pass this new object to urlTemplateToParts.

pickFeaturesTags[targetTag] = properties.customTags[tag]; //eslint-disable-line no-use-before-define
}
}
}

that._urlParts = urlTemplateToParts(that._url, tags); //eslint-disable-line no-use-before-define
that._pickFeaturesUrlParts = urlTemplateToParts(that._pickFeaturesUrl, pickFeaturesTags); //eslint-disable-line no-use-before-define
return true;
Expand Down
28 changes: 28 additions & 0 deletions Specs/Scene/UrlTemplateImageryProviderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,34 @@ defineSuite([
});
});

it('uses custom tags', function() {
var provider = new UrlTemplateImageryProvider({
url: 'made/up/tms/server/{custom1}/{custom2}/{z}/{y}/{x}.PNG',
tilingScheme: new GeographicTilingScheme(),
maximumLevel: 6,
customTags: {
custom1: function() { return 'foo';},
custom2: function() { return 'bar';}
}
});

return pollToPromise(function() {
return provider.ready;
}).then(function() {
spyOn(loadImage, 'createImage').and.callFake(function(url, crossOrigin, deferred) {
expect(url).toEqual('made/up/tms/server/foo/bar/2/1/3.PNG');

// Just return any old image.
loadImage.defaultCreateImage('Data/Images/Red16x16.png', crossOrigin, deferred);
});

return provider.requestImage(3, 1, 2).then(function(image) {
expect(loadImage.createImage).toHaveBeenCalled();
expect(image).toBeInstanceOf(Image);
});
});
});

describe('pickFeatures', function() {
it('returns undefined when enablePickFeatures is false', function() {
var provider = new UrlTemplateImageryProvider({
Expand Down