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.

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

Reduce Bing Maps transactions and ion Bing sessions #7848

Merged
merged 3 commits into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Change Log
* Added syntax to delete data from existing properties via CZML. [#7818](https://github.com/AnalyticalGraphicsInc/cesium/pull/7818)
* Added `checkerboard` material to CZML. [#7845](https://github.com/AnalyticalGraphicsInc/cesium/pull/7845)
* `BingMapsImageryProvider` now uses `DiscardEmptyTileImagePolicy` by default to detect missing tiles as zero-length responses instead of inspecting pixel values. [#7810](https://github.com/AnalyticalGraphicsInc/cesium/pull/7810)
* Reduce the number of Bing transactions and ion Bing sessions used when destroying and recreating the same imagery layer to 1. [#7848](https://github.com/AnalyticalGraphicsInc/cesium/pull/7848)

##### Fixes :wrench:
* Fixed an edge case where Cesium would provide ion access token credentials to non-ion servers if the actual asset entrypoint was being hosted by ion. [#7839](https://github.com/AnalyticalGraphicsInc/cesium/pull/7839)
Expand Down
24 changes: 20 additions & 4 deletions Source/Scene/BingMapsImageryProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,14 @@ define([
for (var attributionIndex = 0, attributionLength = attributionList.length; attributionIndex < attributionLength; ++attributionIndex) {
var attribution = attributionList[attributionIndex];

attribution.credit = new Credit(attribution.attribution);
if (attribution.credit instanceof Credit) {
// If attribution.credit has already been created
// then we are using a cached value, which means
// none of the remaining processing needs to be done.
break;
}

attribution.credit = new Credit(attribution.attribution);
var coverageAreas = attribution.coverageAreas;

for (var areaIndex = 0, areaLength = attribution.coverageAreas.length; areaIndex < areaLength; ++areaIndex) {
Expand All @@ -200,12 +206,19 @@ define([
that._readyPromise.reject(new RuntimeError(message));
}

var cacheKey = metadataResource.url;
function requestMetadata() {
var metadata = metadataResource.fetchJsonp('jsonp');
when(metadata, metadataSuccess, metadataFailure);
var promise = metadataResource.fetchJsonp('jsonp');
BingMapsImageryProvider._metadataCache[cacheKey] = promise;
promise.then(metadataSuccess).otherwise(metadataFailure);
}

requestMetadata();
var promise = BingMapsImageryProvider._metadataCache[cacheKey];
if (defined(promise)) {
promise.then(metadataSuccess).otherwise(metadataFailure);
} else {
requestMetadata();
}
}

defineProperties(BingMapsImageryProvider.prototype, {
Expand Down Expand Up @@ -697,5 +710,8 @@ define([
return result;
}

// Exposed for testing
BingMapsImageryProvider._metadataCache = {};

return BingMapsImageryProvider;
});
25 changes: 20 additions & 5 deletions Source/Scene/IonImageryProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,11 @@ define([
function IonImageryProvider(options) {
options = defaultValue(options, defaultValue.EMPTY_OBJECT);

var assetId = options.assetId;
//>>includeStart('debug', pragmas.debug);
Check.typeOf.number('options.assetId', options.assetId);
Check.typeOf.number('options.assetId', assetId);
//>>includeEnd('debug');

var endpointResource = IonResource._createEndpointResource(options.assetId, options);

/**
* The default alpha blending value of this provider, with 0.0 representing fully transparent and
* 1.0 representing fully opaque.
Expand Down Expand Up @@ -156,10 +155,23 @@ define([
this._errorEvent = new Event();

var that = this;
this._readyPromise = endpointResource.fetchJson()
var endpointResource = IonResource._createEndpointResource(assetId, options);

// A simple cache to avoid making repeated requests to ion for endpoints we've
// already retrieved. This exists mainly to support Bing caching to reduce
// world imagery sessions, but provides a small boost of performance in general
// if constantly reloading assets
var cacheKey = options.assetId.toString() + options.accessToken + options.server;
var promise = IonImageryProvider._endpointCache[cacheKey];
if (!defined(promise)) {
promise = endpointResource.fetchJson();
IonImageryProvider._endpointCache[cacheKey] = promise;
}

this._readyPromise = promise
.then(function(endpoint) {
if (endpoint.type !== 'IMAGERY') {
return when.reject(new RuntimeError('Cesium ion asset ' + options.assetId + ' is not an imagery asset.'));
return when.reject(new RuntimeError('Cesium ion asset ' + assetId + ' is not an imagery asset.'));
}

var imageryProvider;
Expand Down Expand Up @@ -485,5 +497,8 @@ define([
return this._imageryProvider.pickFeatures(x, y, level, longitude, latitude);
};

//exposed for testing
IonImageryProvider._endpointCache = {};

return IonImageryProvider;
});
51 changes: 51 additions & 0 deletions Specs/Scene/BingMapsImageryProviderSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ defineSuite([

beforeEach(function() {
RequestScheduler.clearForSpecs();
BingMapsImageryProvider._metadataCache = {};
});

afterEach(function() {
Expand Down Expand Up @@ -247,6 +248,56 @@ defineSuite([
});
});

it('Uses cached metadata result', function() {
var url = 'http://fake.fake.invalid';
var mapStyle = BingMapsStyle.ROAD;

installFakeMetadataRequest(url, mapStyle);
installFakeImageRequest();
var provider = new BingMapsImageryProvider({
url : url,
mapStyle : mapStyle
});
var provider2;
var provider3;

return provider.readyPromise
.then(function(result) {
expect(result).toBe(true);
expect(provider.ready).toBe(true);

installFakeMetadataRequest(url, mapStyle);
installFakeImageRequest();
provider2 = new BingMapsImageryProvider({
url: url,
mapStyle: mapStyle
});
return provider2.readyPromise;
})
.then(function(result) {
expect(result).toBe(true);
expect(provider2.ready).toBe(true);

//These are the same instance only if the cache has been used
expect(provider._attributionList).toBe(provider2._attributionList);

installFakeMetadataRequest(url, BingMapsStyle.AERIAL);
installFakeImageRequest();
provider3 = new BingMapsImageryProvider({
url: url,
mapStyle: BingMapsStyle.AERIAL
});
return provider3.readyPromise;
})
.then(function(result) {
expect(result).toBe(true);
expect(provider3.ready).toBe(true);

// Because the road is different, a non-cached request should have happened
expect(provider3._attributionList).not.toBe(provider._attributionList);
});
});

it('resolves readyPromise with a path', function() {
var url = 'http://fake.fake.invalid/some/subdirectory';
var mapStyle = BingMapsStyle.ROAD;
Expand Down
35 changes: 35 additions & 0 deletions Specs/Scene/IonImageryProviderSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ defineSuite([

beforeEach(function() {
RequestScheduler.clearForSpecs();
IonImageryProvider._endpointCache = {};
});

it('conforms to ImageryProvider interface', function() {
Expand Down Expand Up @@ -117,6 +118,40 @@ defineSuite([
});
});

it('Uses previously fetched endpoint cache', function() {
var endpointData = {
type: 'IMAGERY',
url: 'http://test.invalid/layer',
accessToken: 'not_really_a_refresh_token',
attributions: []
};

var assetId = 12335;
var options = { assetId: assetId, accessToken: 'token', server: 'http://test.invalid' };
var endpointResource = IonResource._createEndpointResource(assetId, options);
spyOn(IonResource, '_createEndpointResource').and.returnValue(endpointResource);
spyOn(endpointResource, 'fetchJson').and.returnValue(when.resolve(endpointData));

expect(endpointResource.fetchJson.calls.count()).toBe(0);
var provider = new IonImageryProvider(options);
var provider2;
return provider.readyPromise
.then(function() {
expect(provider.ready).toBe(true);
expect(endpointResource.fetchJson.calls.count()).toBe(1);

// Same as options but in a different order to verify cache is order independant.
var options2 = { accessToken: 'token', server: 'http://test.invalid', assetId: assetId };
provider2 = new IonImageryProvider(options2);
return provider2.readyPromise;
})
.then(function() {
//Since the data is cached, fetchJson is not called again.
expect(endpointResource.fetchJson.calls.count()).toBe(1);
expect(provider2.ready).toBe(true);
});
});

it('propagates called to underlying imagery provider resolves when ready', function() {
var provider = createTestProvider();
var internalProvider;
Expand Down