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

[Backport 1.x] Fix wms maps can't load when can't request default map service #1702

Merged
merged 2 commits into from
Jul 27, 2022
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
7 changes: 4 additions & 3 deletions .lycheeexclude
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/
Expand All @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
});
52 changes: 42 additions & 10 deletions src/plugins/maps_legacy/public/map/service_settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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;
Expand Down Expand Up @@ -155,8 +168,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 [];
}
}

/**
Expand All @@ -175,7 +192,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)
Expand Down Expand Up @@ -207,12 +229,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) {
Expand All @@ -228,7 +255,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) {
Expand Down
49 changes: 44 additions & 5 deletions src/plugins/maps_legacy/public/map/service_settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,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,
Expand All @@ -65,11 +66,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')) {
Expand All @@ -88,7 +101,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();
Expand Down Expand Up @@ -283,6 +295,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 () {
Expand Down Expand Up @@ -355,5 +380,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);
});
});
});
});
6 changes: 4 additions & 2 deletions src/plugins/region_map/public/region_map_visualization.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,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);
Expand Down