github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[8.8] [maps] fix Map orphans sources on layer deletion (#159067) (#159160)

Browse source 
# Backport

This will backport the following commits from `main` to `8.8`:
- [[maps] fix Map orphans sources on layer deletion
(#159067)](https://github.com/elastic/kibana/pull/159067)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2023-06-06T20:34:40Z","message":"[maps]
fix Map orphans sources on layer deletion (#159067)\n\nFixes
https://github.com/elastic/kibana/issues/158133\r\n\r\nSources where not
getting removed because `return` was used instead of\r\n`continue` in
`for...in` loop. This caused the function to return\r\ninstead of
processing remaining sources.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"fcd195050f14f38c9427a9b9819baef442baf854","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","Feature:Maps","v8.9.0","v8.8.2"],"number":159067,"url":"https://github.com/elastic/kibana/pull/159067","mergeCommit":{"message":"[maps]
fix Map orphans sources on layer deletion (#159067)\n\nFixes
https://github.com/elastic/kibana/issues/158133\r\n\r\nSources where not
getting removed because `return` was used instead of\r\n`continue` in
`for...in` loop. This caused the function to return\r\ninstead of
processing remaining sources.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"fcd195050f14f38c9427a9b9819baef442baf854"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/159067","number":159067,"mergeCommit":{"message":"[maps]
fix Map orphans sources on layer deletion (#159067)\n\nFixes
https://github.com/elastic/kibana/issues/158133\r\n\r\nSources where not
getting removed because `return` was used instead of\r\n`continue` in
`for...in` loop. This caused the function to return\r\ninstead of
processing remaining sources.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"fcd195050f14f38c9427a9b9819baef442baf854"}},{"branch":"8.8","label":"v8.8.2","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
This commit is contained in:
Kibana Machine 2023-06-06 17:45:20 -04:00 • committed by GitHub
parent f6603e247b
commit 64b1138d19
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 94 additions and 61 deletions
Download patch file Download diff file Expand all files Collapse all files

2
packages/kbn-mapbox-gl/index.ts
View file

@ -13,6 +13,7 @@ import type {
GeoJSONSource,
VectorTileSource,
RasterTileSource,
SourceSpecification,
StyleSpecification,
MapEvent,
MapOptions,
@ -45,6 +46,7 @@ export { maplibregl };
export type {
Map,
LayerSpecification,
SourceSpecification,
StyleSpecification,
Source,
GeoJSONSource,

2
x-pack/plugins/maps/public/connected_components/mb_map/mb_map.tsx
View file

@ -43,7 +43,7 @@ import {
import { getCanAccessEmsFonts, getGlyphs, getKibanaFontsGlyphUrl } from './glyphs';
import { syncLayerOrder } from './sort_layers';
import { removeOrphanedSourcesAndLayers } from './utils';
import { removeOrphanedSourcesAndLayers } from './remove_orphaned';
import { RenderToolTipContent } from '../../classes/tooltips/tooltip_property';
import { TileStatusTracker } from './tile_status_tracker';
import { DrawFeatureControl } from './draw_control/draw_feature_control';

141
x-pack/plugins/maps/public/connected_components/mb_map/mb.utils.test.js → x-pack/plugins/maps/public/connected_components/mb_map/remove_orphaned.test.ts
View file

@ -5,12 +5,23 @@
* 2.0.
*/
import { removeOrphanedSourcesAndLayers } from './utils';
/* eslint-disable max-classes-per-file */
import type {
Map as MbMap,
LayerSpecification,
SourceSpecification,
StyleSpecification,
} from '@kbn/mapbox-gl';
import { removeOrphanedSourcesAndLayers } from './remove_orphaned';
import { SPATIAL_FILTERS_LAYER_ID } from '../../../common/constants';
import _ from 'lodash';
import { ILayer } from '../../classes/layers/layer';
class MockMbMap {
constructor(style) {
private _style: StyleSpecification;
constructor(style: StyleSpecification) {
this._style = _.cloneDeep(style);
}
@ -18,26 +29,11 @@ class MockMbMap {
return _.cloneDeep(this._style);
}
moveLayer(mbLayerId, nextMbLayerId) {
const indexOfLayerToMove = this._style.layers.findIndex((layer) => {
return layer.id === mbLayerId;
});
const layerToMove = this._style.layers[indexOfLayerToMove];
this._style.layers.splice(indexOfLayerToMove, 1);
const indexOfNextLayer = this._style.layers.findIndex((layer) => {
return layer.id === nextMbLayerId;
});
this._style.layers.splice(indexOfNextLayer, 0, layerToMove);
}
removeSource(sourceId) {
removeSource(sourceId: string) {
delete this._style.sources[sourceId];
}
removeLayer(layerId) {
removeLayer(layerId: string) {
const layerToRemove = this._style.layers.findIndex((layer) => {
return layer.id === layerId;
});
@ -46,14 +42,15 @@ class MockMbMap {
}
class MockLayer {
constructor(layerId, mbSourceIds, mbLayerIdsToSource) {
private readonly _mbSourceIds: string[];
private readonly _mbLayerIdsToSource: Array<{ id: string; source: string }>;
constructor(mbSourceIds: string[], mbLayerIdsToSource: Array<{ id: string; source: string }>) {
this._mbSourceIds = mbSourceIds;
this._mbLayerIdsToSource = mbLayerIdsToSource;
this._layerId = layerId;
}
getId() {
return this._layerId;
}
// Custom interface used in getMockStyle for populating maplibre style with layers and sources
getMbSourceIds() {
return this._mbSourceIds;
}
@ -61,58 +58,55 @@ class MockLayer {
return this._mbLayerIdsToSource;
}
getMbLayerIds() {
return this._mbLayerIdsToSource.map(({ id }) => id);
}
ownsMbLayerId(mbLayerId) {
// ILayer interface
ownsMbLayerId(mbLayerId: string) {
return this._mbLayerIdsToSource.some((mbLayerToSource) => {
return mbLayerToSource.id === mbLayerId;
});
}
ownsMbSourceId(mbSourceId) {
ownsMbSourceId(mbSourceId: string) {
return this._mbSourceIds.some((id) => mbSourceId === id);
}
}
function getMockStyle(orderedMockLayerList) {
function getMockStyle(orderedMockLayerList: MockLayer[]) {
const mockStyle = {
version: 8 as 8,
sources: {},
layers: [],
};
} as StyleSpecification;
orderedMockLayerList.forEach((mockLayer) => {
mockLayer.getMbSourceIds().forEach((mbSourceId) => {
mockStyle.sources[mbSourceId] = {};
mockStyle.sources[mbSourceId] = {} as unknown as SourceSpecification;
});
mockLayer.getMbLayersIdsToSource().forEach(({ id, source }) => {
mockStyle.layers.push({
id: id,
source: source,
});
id,
source,
} as unknown as LayerSpecification);
});
});
return mockStyle;
}
function makeSingleSourceMockLayer(layerId) {
function makeSingleSourceMockLayer(layerId: string) {
const source1 = layerId + '_source1';
return new MockLayer(
layerId,
[layerId],
[source1],
[
{ id: layerId + '_fill', source: layerId },
{ id: layerId + '_line', source: layerId },
{ id: source1 + '_fill', source: source1 },
{ id: source1 + '_line', source: source1 },
]
);
}
function makeMultiSourceMockLayer(layerId) {
function makeMultiSourceMockLayer(layerId: string) {
const source1 = layerId + '_source1';
const source2 = layerId + '_source2';
return new MockLayer(
layerId,
[source1, source2],
[
{ id: source1 + '_fill', source: source1 },
@ -136,14 +130,18 @@ describe('removeOrphanedSourcesAndLayers', () => {
const currentStyle = getMockStyle(currentLayerList);
const mockMbMap = new MockMbMap(currentStyle);
removeOrphanedSourcesAndLayers(mockMbMap, nextLayerList, spatialFilterLayer);
removeOrphanedSourcesAndLayers(
mockMbMap as unknown as MbMap,
nextLayerList as unknown as ILayer[],
spatialFilterLayer as unknown as ILayer
);
const removedStyle = mockMbMap.getStyle();
const nextStyle = getMockStyle(nextLayerList);
expect(removedStyle).toEqual(nextStyle);
});
test('should remove foo and bar layer (multisource)', async () => {
test('should remove foo and bar layer (multisource)', () => {
const bazLayer = makeMultiSourceMockLayer('baz');
const fooLayer = makeMultiSourceMockLayer('foo');
const barLayer = makeMultiSourceMockLayer('bar');
@ -154,14 +152,18 @@ describe('removeOrphanedSourcesAndLayers', () => {
const currentStyle = getMockStyle(currentLayerList);
const mockMbMap = new MockMbMap(currentStyle);
removeOrphanedSourcesAndLayers(mockMbMap, nextLayerList, spatialFilterLayer);
removeOrphanedSourcesAndLayers(
mockMbMap as unknown as MbMap,
nextLayerList as unknown as ILayer[],
spatialFilterLayer as unknown as ILayer
);
const removedStyle = mockMbMap.getStyle();
const nextStyle = getMockStyle(nextLayerList);
expect(removedStyle).toEqual(nextStyle);
});
test('should not remove anything', async () => {
test('should not remove anything', () => {
const bazLayer = makeSingleSourceMockLayer('baz');
const fooLayer = makeSingleSourceMockLayer('foo');
const barLayer = makeSingleSourceMockLayer('bar');
@ -172,30 +174,63 @@ describe('removeOrphanedSourcesAndLayers', () => {
const currentStyle = getMockStyle(currentLayerList);
const mockMbMap = new MockMbMap(currentStyle);
removeOrphanedSourcesAndLayers(mockMbMap, nextLayerList, spatialFilterLayer);
removeOrphanedSourcesAndLayers(
mockMbMap as unknown as MbMap,
nextLayerList as unknown as ILayer[],
spatialFilterLayer as unknown as ILayer
);
const removedStyle = mockMbMap.getStyle();
const nextStyle = getMockStyle(nextLayerList);
expect(removedStyle).toEqual(nextStyle);
});
test('should not remove spatial filter layer and sources when spatialFilterLayer is provided', async () => {
test('should not remove spatial filter layer and sources when spatialFilterLayer is provided', () => {
const styleWithSpatialFilters = getMockStyle([spatialFilterLayer]);
const mockMbMap = new MockMbMap(styleWithSpatialFilters);
removeOrphanedSourcesAndLayers(mockMbMap, [], spatialFilterLayer);
removeOrphanedSourcesAndLayers(
mockMbMap as unknown as MbMap,
[],
spatialFilterLayer as unknown as ILayer
);
expect(mockMbMap.getStyle()).toEqual(styleWithSpatialFilters);
});
test('should not remove mapbox gl draw layers and sources', async () => {
test('should remove sources after spatial filter layer', () => {
const barLayer = makeMultiSourceMockLayer('bar');
const currentLayerList = [spatialFilterLayer, barLayer];
const currentStyle = getMockStyle(currentLayerList);
const mockMbMap = new MockMbMap(currentStyle);
removeOrphanedSourcesAndLayers(
mockMbMap as unknown as MbMap,
[],
spatialFilterLayer as unknown as ILayer
);
const removedStyle = mockMbMap.getStyle();
expect(Object.keys(removedStyle.sources)).toEqual([
'SPATIAL_FILTERS_LAYER_ID_source1',
'SPATIAL_FILTERS_LAYER_ID_source2',
]);
});
test('should not remove mapbox gl draw layers and sources', () => {
const fooLayer = makeMultiSourceMockLayer('foo');
const layerList = [fooLayer];
const currentStyle = getMockStyle(layerList);
currentStyle.layers.push({ id: 'gl-draw-points' });
currentStyle.layers.push({ id: 'gl-draw-points' } as unknown as LayerSpecification);
const mockMbMap = new MockMbMap(currentStyle);
removeOrphanedSourcesAndLayers(mockMbMap, layerList, spatialFilterLayer);
removeOrphanedSourcesAndLayers(
mockMbMap as unknown as MbMap,
layerList as unknown as ILayer[],
spatialFilterLayer as unknown as ILayer
);
expect(mockMbMap.getStyle()).toEqual(currentStyle);
});
});

10
x-pack/plugins/maps/public/connected_components/mb_map/utils.ts → x-pack/plugins/maps/public/connected_components/mb_map/remove_orphaned.ts
View file

@ -19,7 +19,6 @@ export function removeOrphanedSourcesAndLayers(
return;
}
const mbLayerIdsToRemove: string[] = [];
mbStyle.layers.forEach((mbLayer) => {
// ignore mapbox layers from spatial filter layer
if (spatialFilterLayer.ownsMbLayerId(mbLayer.id)) {
@ -35,26 +34,23 @@ export function removeOrphanedSourcesAndLayers(
return layer.ownsMbLayerId(mbLayer.id);
});
if (!targetLayer) {
mbLayerIdsToRemove.push(mbLayer.id);
mbMap.removeLayer(mbLayer.id);
}
});
mbLayerIdsToRemove.forEach((mbLayerId) => mbMap.removeLayer(mbLayerId));
const mbSourcesToRemove = [];
for (const mbSourceId in mbStyle.sources) {
if (mbStyle.sources.hasOwnProperty(mbSourceId)) {
// ignore mapbox sources from spatial filter layer
if (spatialFilterLayer.ownsMbSourceId(mbSourceId)) {
return;
continue;
}
const targetLayer = layerList.find((layer) => {
return layer.ownsMbSourceId(mbSourceId);
});
if (!targetLayer) {
mbSourcesToRemove.push(mbSourceId);
mbMap.removeSource(mbSourceId);
}
}
}
mbSourcesToRemove.forEach((mbSourceId) => mbMap.removeSource(mbSourceId));
}