diff --git a/CHANGES.md b/CHANGES.md index 335ac8f3dbb8..e7982ba545ad 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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) diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index 9ac40dc2d241..d4be784ee5d5 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -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) { @@ -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, { @@ -697,5 +710,8 @@ define([ return result; } + // Exposed for testing + BingMapsImageryProvider._metadataCache = {}; + return BingMapsImageryProvider; }); diff --git a/Source/Scene/IonImageryProvider.js b/Source/Scene/IonImageryProvider.js index 5cd92dc73bf3..644af36f7f69 100644 --- a/Source/Scene/IonImageryProvider.js +++ b/Source/Scene/IonImageryProvider.js @@ -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. @@ -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; @@ -485,5 +497,8 @@ define([ return this._imageryProvider.pickFeatures(x, y, level, longitude, latitude); }; + //exposed for testing + IonImageryProvider._endpointCache = {}; + return IonImageryProvider; }); diff --git a/Specs/Scene/BingMapsImageryProviderSpec.js b/Specs/Scene/BingMapsImageryProviderSpec.js index 06f93430f69e..4c6afc5ceb8d 100644 --- a/Specs/Scene/BingMapsImageryProviderSpec.js +++ b/Specs/Scene/BingMapsImageryProviderSpec.js @@ -46,6 +46,7 @@ defineSuite([ beforeEach(function() { RequestScheduler.clearForSpecs(); + BingMapsImageryProvider._metadataCache = {}; }); afterEach(function() { @@ -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; diff --git a/Specs/Scene/IonImageryProviderSpec.js b/Specs/Scene/IonImageryProviderSpec.js index c40144fa74e8..d78f54b4f36b 100644 --- a/Specs/Scene/IonImageryProviderSpec.js +++ b/Specs/Scene/IonImageryProviderSpec.js @@ -59,6 +59,7 @@ defineSuite([ beforeEach(function() { RequestScheduler.clearForSpecs(); + IonImageryProvider._endpointCache = {}; }); it('conforms to ImageryProvider interface', function() { @@ -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;