Fix WMS can't load when unable access maps services (opensearch-proje…

…ct#1550) (opensearch-project#1707) * Fix WMS can't load when unable access maps services Signed-off-by: Junqiu Lei <junqiu@amazon.com> (cherry picked from commit b2cfb6e)
pjfitzgibbons · Oct 24, 2022 · 7ab24c5 · 7ab24c5
1 parent e20cf92
commit 7ab24c5
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 22 deletions.
diff --git a/.lycheeexclude b/.lycheeexclude
@@ -6,7 +6,7 @@ http://localhost
 https://localhost
 http://127.0.0.1/
 https://127.0.0.1/
-http://127.0.0.1:10002/bar 
+http://127.0.0.1:10002/bar
 http://127.0.0.1:10002/
 http://opensearch
 https://opensearch
@@ -35,6 +35,7 @@ http://noone.nowhere.none/
 http://bar
 http://foo
 http://test.com/
+https://manifest.foobar
 https://files.foobar/
 https://tiles.foobar/
 https://1.1.1.1:9200/
@@ -51,7 +52,7 @@ http://buildurl/
 https://dryrun/
 https://url/
 http://url/
-http://notfound.svg/ 
+http://notfound.svg/
 https://validurl/
 https://myopensearch-dashboardsdomain.com
 http://myopensearch-dashboardsdomain.com
@@ -111,4 +112,4 @@ https://developer.mozilla.org/
 https://a.tile.openstreetmap.org/
 http://www.creedthoughts.gov
 https://media-for-the-masses.theacademyofperformingartsandscience.org/
-https://yarnpkg.com/latest.msi
+https://yarnpkg.com/latest.msi
diff --git a/src/plugins/maps_legacy/public/common/opensearch_maps_client.js b/src/plugins/maps_legacy/public/common/opensearch_maps_client.js
@@ -20,8 +20,9 @@ export class OpenSearchMapsClient extends EMSClient {
     try {
       result = await this._fetchWithTimeout(this._manifestServiceUrl);
     } catch (e) {
-      // silently ignoring the exception and returning false.
-      return false;
+      // silently ignoring the exception and returning true to make sure
+      // OpenSearchMapsClient is still enabled when can't access OpenSearch maps service.
+      return true;
     }
     if (result.ok) {
       const resultJson = await result.json();

diff --git a/src/plugins/maps_legacy/public/common/opensearch_maps_client.test.js b/src/plugins/maps_legacy/public/common/opensearch_maps_client.test.js
@@ -0,0 +1,33 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { OpenSearchMapsClient } from './opensearch_maps_client.js';
+
+describe('opensearch_maps_client test without Internet', function () {
+  const noInternetManifestUrl = 'https://manifest.foobar';
+  const defaultClientConfig = {
+    appName: 'opensearch-dashboards',
+    osdVersion: '1.2.3',
+    language: 'en',
+    landingPageUrl: '',
+    fetchFunction: function (...args) {
+      return fetch(...args);
+    },
+  };
+
+  function makeOpenSearchMapsClient() {
+    const openSearchMapsClient = new OpenSearchMapsClient({
+      ...defaultClientConfig,
+      manifestServiceUrl: noInternetManifestUrl,
+    });
+    return openSearchMapsClient;
+  }
+
+  it('isEnabled() should return true when catch error', async function () {
+    const mapsClient = makeOpenSearchMapsClient();
+    const osmIsEnabled = await mapsClient.isEnabled();
+    expect(osmIsEnabled).toEqual(true);
+  });
+});
diff --git a/src/plugins/maps_legacy/public/map/service_settings.js b/src/plugins/maps_legacy/public/map/service_settings.js
@@ -38,6 +38,19 @@ import { ORIGIN } from '../common/constants/origin';
 
 const TMS_IN_YML_ID = 'TMS in config/opensearch_dashboards.yml';
 
+// When unable to fetch OpenSearch maps service, return default values to
+// make sure wms can be set up.
+export const DEFAULT_SERVICE = [
+  {
+    origin: 'elastic_maps_service',
+    id: 'road_map',
+    minZoom: 0,
+    maxZoom: 22,
+    attribution:
+      '<a rel="noreferrer noopener" href="https://www.openstreetmap.org/copyright">Map data © OpenStreetMap contributors</a>',
+  },
+];
+
 export class ServiceSettings {
   constructor(mapConfig, tilemapsConfig) {
     this._mapConfig = mapConfig;
@@ -153,8 +166,12 @@ export class ServiceSettings {
     }
 
     await this._setMapServices();
-    const fileLayers = await this._emsClient.getFileLayers();
-    return fileLayers.map(this._backfillSettings);
+    try {
+      const fileLayers = await this._emsClient.getFileLayers();
+      return fileLayers.map(this._backfillSettings);
+    } catch (e) {
+      return [];
+    }
   }
 
   /**
@@ -173,7 +190,12 @@ export class ServiceSettings {
 
     await this._setMapServices();
     if (this._mapConfig.includeOpenSearchMapsService) {
-      const servicesFromManifest = await this._emsClient.getTMSServices();
+      let servicesFromManifest = [];
+      try {
+        servicesFromManifest = await this._emsClient.getTMSServices();
+      } catch (e) {
+        return DEFAULT_SERVICE;
+      }
       const strippedServiceFromManifest = await Promise.all(
         servicesFromManifest
           .filter((tmsService) => tmsService.getId() === this._mapConfig.emsTileLayerId.bright)
@@ -205,12 +227,17 @@ export class ServiceSettings {
   }
 
   async getFileLayerFromConfig(fileLayerConfig) {
-    const fileLayers = await this._emsClient.getFileLayers();
-    return fileLayers.find((fileLayer) => {
-      const hasIdByName = fileLayer.hasId(fileLayerConfig.name); //legacy
-      const hasIdById = fileLayer.hasId(fileLayerConfig.id);
-      return hasIdByName || hasIdById;
-    });
+    let fileLayers = [];
+    try {
+      fileLayers = await this._emsClient.getFileLayers();
+      return fileLayers.find((fileLayer) => {
+        const hasIdByName = fileLayer.hasId(fileLayerConfig.name); //legacy
+        const hasIdById = fileLayer.hasId(fileLayerConfig.id);
+        return hasIdByName || hasIdById;
+      });
+    } catch (err) {
+      return null;
+    }
   }
 
   async getEMSHotLink(fileLayerConfig) {
@@ -226,7 +253,12 @@ export class ServiceSettings {
 
   async _getAttributesForEMSTMSLayer(isDesaturated, isDarkMode) {
     await this._setMapServices();
-    const tmsServices = await this._emsClient.getTMSServices();
+    let tmsServices = [];
+    try {
+      tmsServices = await this._emsClient.getTMSServices();
+    } catch (e) {
+      return DEFAULT_SERVICE;
+    }
     const emsTileLayerId = this._mapConfig.emsTileLayerId;
     let serviceId;
     if (isDarkMode) {

diff --git a/src/plugins/maps_legacy/public/map/service_settings.test.js b/src/plugins/maps_legacy/public/map/service_settings.test.js
@@ -42,11 +42,12 @@ import EMS_STYLE_ROAD_MAP_BRIGHT from '../__tests__/map/ems_mocks/sample_style_b
 import EMS_STYLE_ROAD_MAP_DESATURATED from '../__tests__/map/ems_mocks/sample_style_desaturated';
 import EMS_STYLE_DARK_MAP from '../__tests__/map/ems_mocks/sample_style_dark';
 import { ORIGIN } from '../common/constants/origin';
-import { ServiceSettings } from './service_settings';
+import { ServiceSettings, DEFAULT_SERVICE } from './service_settings';
 
 describe('service_settings (FKA tile_map test)', function () {
   const emsFileApiUrl = 'https://files.foobar';
   const emsTileApiUrl = 'https://tiles.foobar';
+  const noInternetManifestUrl = 'https://manifest.foobar';
 
   const defaultMapConfig = {
     emsFileApiUrl,
@@ -63,11 +64,23 @@ describe('service_settings (FKA tile_map test)', function () {
     options: {},
   };
 
-  function makeServiceSettings(mapConfigOptions = {}, tilemapOptions = {}) {
+  function makeServiceSettings(mapConfigOptions = {}, tilemapOptions = {}, otherOptions = {}) {
+    const { noInternet } = otherOptions;
     const serviceSettings = new ServiceSettings(
-      { ...defaultMapConfig, ...mapConfigOptions },
-      { ...defaultTilemapConfig, ...tilemapOptions }
+      {
+        ...defaultMapConfig,
+        ...mapConfigOptions,
+        opensearchManifestServiceUrl: noInternet ? noInternetManifestUrl : '',
+      },
+      {
+        ...defaultTilemapConfig,
+        ...tilemapOptions,
+      }
     );
+    if (noInternet) {
+      return serviceSettings;
+    }
+
     serviceSettings.__debugStubManifestCalls(async (url) => {
       //simulate network calls
       if (url.startsWith('https://tiles.foobar')) {
@@ -86,7 +99,6 @@ describe('service_settings (FKA tile_map test)', function () {
     });
     return serviceSettings;
   }
-
   describe('TMS', function () {
     it('should NOT get url from the config', async function () {
       const serviceSettings = makeServiceSettings();
@@ -281,6 +293,19 @@ describe('service_settings (FKA tile_map test)', function () {
         expect(tilemapServices).toEqual(expected);
       });
     });
+
+    describe('when unable to access OpenSearch maps service', function () {
+      const expectedDefaultTmService = DEFAULT_SERVICE[0];
+      it('should return default service', async () => {
+        const serviceSettings = makeServiceSettings({}, {}, { noInternet: true });
+        const tileMapServices = await serviceSettings.getTMSServices();
+        expect(tileMapServices[0]).toMatchObject(expectedDefaultTmService);
+        const isDesaturated = true;
+        const isDarkMode = true;
+        const attrs = await serviceSettings._getAttributesForEMSTMSLayer(isDesaturated, isDarkMode);
+        expect(attrs[0]).toMatchObject(expectedDefaultTmService);
+      });
+    });
   });
 
   describe('File layers', function () {
@@ -353,5 +378,19 @@ describe('service_settings (FKA tile_map test)', function () {
         '<a rel="noreferrer noopener" href="http://www.naturalearthdata.com/about/terms-of-use">&lt;div onclick=\'alert(1\')&gt;Made with NaturalEarth&lt;/div&gt;</a> | <a rel="noreferrer noopener">OpenSearch Maps Service</a>'
       );
     });
+
+    describe('when unable to access maps service', function () {
+      const serviceSettings = makeServiceSettings({}, {}, { noInternet: true });
+
+      it('should return empty arr', async () => {
+        const fileLayers = await serviceSettings.getFileLayers();
+        expect(fileLayers).toEqual([]);
+      });
+
+      it('should return null', async () => {
+        const fileLayer = await serviceSettings.getFileLayerFromConfig(null);
+        expect(fileLayer).toEqual(null);
+      });
+    });
   });
 });
diff --git a/src/plugins/region_map/public/region_map_visualization.js b/src/plugins/region_map/public/region_map_visualization.js
@@ -114,8 +114,10 @@ export function createRegionMapVisualization({
       const { escape } = await import('lodash');
 
       if (
-        fileLayerConfig.isEMS || //Hosted by EMS. Metadata needs to be resolved through EMS
-        (fileLayerConfig.layerId && fileLayerConfig.layerId.startsWith(`${ORIGIN.EMS}.`)) //fallback for older saved objects
+        fileLayerConfig &&
+        (fileLayerConfig.isEMS || //Hosted by EMS. Metadata needs to be resolved through EMS
+          (fileLayerConfig.layerId && fileLayerConfig.layerId.startsWith(`${ORIGIN.EMS}.`)))
+        //fallback for older saved objects
       ) {
         const serviceSettings = await getServiceSettings();
         return await serviceSettings.loadFileLayerConfig(fileLayerConfig);