Merge pull request #7848 from AnalyticalGraphicsInc/bing-usage

Tom Fili · web-flow · commit 6bda94022c53 · 2019-05-21T09:37:59.000-04:00
Reduce Bing Maps transactions and ion Bing sessions
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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;
