Skip to content

Commit

Permalink
[8.7] [Maps] fixes Kibana maps shows MVT borders if the geometry bord…
Browse files Browse the repository at this point in the history 
…er style is greater than 1 (#150497) (#150758)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Maps] fixes Kibana maps shows MVT borders if the geometry border
style is greater than 1
(#150497)](#150497)

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

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

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-02-09T18:07:54Z","message":"[Maps]
fixes Kibana maps shows MVT borders if the geometry border style is
greater than 1 (#150497)\n\nFixes
https://github.com/elastic/kibana/issues/150187\r\n\r\nPR passes buffer
to kibana MVT route which passes buffer to\r\nElasticsearch vector tile
API. Buffer is set based on line width style\r\nproperty.\r\n\r\n<img
width=\"600\" alt=\"Screen Shot 2023-02-07 at 2 43 15
PM\"\r\nsrc=\"https://user-images.githubusercontent.com/373691/217373279-4d72e210-31ae-48cc-997f-dc05d330028b.png\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"f439bdc2b3fe7caa20ff5375460cc9659c6a76db","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","auto-backport","Feature:Maps","v8.7.0","v8.8.0"],"number":150497,"url":"https://github.com/elastic/kibana/pull/150497","mergeCommit":{"message":"[Maps]
fixes Kibana maps shows MVT borders if the geometry border style is
greater than 1 (#150497)\n\nFixes
https://github.com/elastic/kibana/issues/150187\r\n\r\nPR passes buffer
to kibana MVT route which passes buffer to\r\nElasticsearch vector tile
API. Buffer is set based on line width style\r\nproperty.\r\n\r\n<img
width=\"600\" alt=\"Screen Shot 2023-02-07 at 2 43 15
PM\"\r\nsrc=\"https://user-images.githubusercontent.com/373691/217373279-4d72e210-31ae-48cc-997f-dc05d330028b.png\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"f439bdc2b3fe7caa20ff5375460cc9659c6a76db"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/150497","number":150497,"mergeCommit":{"message":"[Maps]
fixes Kibana maps shows MVT borders if the geometry border style is
greater than 1 (#150497)\n\nFixes
https://github.com/elastic/kibana/issues/150187\r\n\r\nPR passes buffer
to kibana MVT route which passes buffer to\r\nElasticsearch vector tile
API. Buffer is set based on line width style\r\nproperty.\r\n\r\n<img
width=\"600\" alt=\"Screen Shot 2023-02-07 at 2 43 15
PM\"\r\nsrc=\"https://user-images.githubusercontent.com/373691/217373279-4d72e210-31ae-48cc-997f-dc05d330028b.png\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"f439bdc2b3fe7caa20ff5375460cc9659c6a76db"}}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <[email protected]>
  • Loading branch information
@kibanamachine @nreese
kibanamachine and nreese authored Feb 9, 2023
1 parent 75eed4e commit c4ebad6
Show file tree
Hide file tree
Showing 16 changed files with 127 additions and 12 deletions.
4 changes: 4 additions & 0 deletions x-pack/plugins/maps/common/mvt_request_body.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ describe('getAggsTileRequest', () => {
query: {},
};
const { path } = getAggsTileRequest({
buffer: 5,
encodedRequestBody: encodeMvtResponseBody(searchRequest),
geometryFieldName: 'my location',
gridPrecision: 8,
Expand All @@ -115,6 +116,7 @@ describe('getHitsTileRequest', () => {
query: {},
};
const { path } = getHitsTileRequest({
buffer: 5,
encodedRequestBody: encodeMvtResponseBody(searchRequest),
geometryFieldName: 'my location',
hasLabels: true,
Expand All @@ -135,6 +137,7 @@ describe('getHitsTileRequest', () => {
sort: ['timestamp'],
};
const { body } = getHitsTileRequest({
buffer: 5,
encodedRequestBody: encodeMvtResponseBody(searchRequest),
geometryFieldName: 'my location',
hasLabels: true,
Expand All @@ -153,6 +156,7 @@ describe('getHitsTileRequest', () => {
query: {},
};
const { body } = getHitsTileRequest({
buffer: 5,
encodedRequestBody: encodeMvtResponseBody(searchRequest),
geometryFieldName: 'my location',
hasLabels: true,
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/maps/common/mvt_request_body.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function encodeMvtResponseBody(unencodedRequestBody: estypes.SearchReques
}

export function getAggsTileRequest({
buffer,
encodedRequestBody,
geometryFieldName,
gridPrecision,
Expand All @@ -34,6 +35,7 @@ export function getAggsTileRequest({
y,
z,
}: {
buffer: number;
encodedRequestBody: string;
geometryFieldName: string;
gridPrecision: number;
Expand All @@ -53,6 +55,7 @@ export function getAggsTileRequest({
geometryFieldName
)}/${z}/${x}/${y}`,
body: {
buffer,
size: 0, // no hits
grid_precision: gridPrecision,
exact_bounds: false,
Expand All @@ -69,6 +72,7 @@ export function getAggsTileRequest({
}

export function getHitsTileRequest({
buffer,
encodedRequestBody,
geometryFieldName,
hasLabels,
Expand All @@ -77,6 +81,7 @@ export function getHitsTileRequest({
y,
z,
}: {
buffer: number;
encodedRequestBody: string;
geometryFieldName: string;
hasLabels: boolean;
Expand All @@ -90,6 +95,7 @@ export function getHitsTileRequest({
throw new Error('Required requestBody parameter not provided');
}
const tileRequestBody = {
buffer,
grid_precision: 0, // no aggs
exact_bounds: true,
extent: 4096, // full resolution,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/classes/layers/heatmap_layer/heatmap_layer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export class HeatmapLayer extends AbstractLayer {

async syncData(syncContext: DataRequestContext) {
await syncMvtSourceData({
buffer: 0,
hasLabels: false,
layerId: this.getId(),
layerName: await this.getDisplayName(),
Expand Down
61 changes: 61 additions & 0 deletions .../plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_source_data.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('syncMvtSourceData', () => {
const syncContext = new MockSyncContext({ dataFilters: {} });

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand Down Expand Up @@ -82,6 +83,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
});
});

Expand All @@ -99,6 +101,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -114,6 +117,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
Expand Down Expand Up @@ -144,6 +148,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -159,6 +164,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
Expand Down Expand Up @@ -186,6 +192,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -201,6 +208,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
Expand Down Expand Up @@ -236,6 +244,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -251,6 +260,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
Expand Down Expand Up @@ -278,6 +288,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -293,6 +304,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
Expand Down Expand Up @@ -320,6 +332,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -335,6 +348,7 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
Expand Down Expand Up @@ -362,6 +376,7 @@ describe('syncMvtSourceData', () => {
};

await syncMvtSourceData({
buffer: 5,
hasLabels: true,
layerId: 'layer1',
layerName: 'my layer',
Expand All @@ -377,6 +392,51 @@ describe('syncMvtSourceData', () => {
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 5,
};
},
} as unknown as DataRequest,
requestMeta: { ...prevRequestMeta },
source: mockSource,
syncContext,
});
// @ts-expect-error
sinon.assert.calledOnce(syncContext.startLoading);
// @ts-expect-error
sinon.assert.calledOnce(syncContext.stopLoading);
});

test('Should re-sync when buffer state changes', async () => {
const syncContext = new MockSyncContext({ dataFilters: {} });
const prevRequestMeta = {
...syncContext.dataFilters,
applyGlobalQuery: true,
applyGlobalTime: true,
applyForceRefresh: true,
fieldNames: [],
sourceMeta: {},
isForceRefresh: false,
isFeatureEditorOpenForLayer: false,
};

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
prevDataRequest: {
getMeta: () => {
return prevRequestMeta;
},
getData: () => {
return {
tileMinZoom: 4,
tileMaxZoom: 14,
tileSourceLayer: 'aggs',
tileUrl: 'https://example.com/{x}/{y}/{z}.pbf?token=12345',
refreshToken: '12345',
hasLabels: false,
buffer: 7,
};
},
} as unknown as DataRequest,
Expand All @@ -394,6 +454,7 @@ describe('syncMvtSourceData', () => {
const syncContext = new MockSyncContext({ dataFilters: {} });

await syncMvtSourceData({
buffer: 5,
hasLabels: false,
layerId: 'layer1',
layerName: 'my layer',
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_source_data.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ export interface MvtSourceData {
tileUrl: string;
refreshToken: string;
hasLabels: boolean;
buffer: number;
}

export async function syncMvtSourceData({
buffer,
hasLabels,
layerId,
layerName,
Expand All @@ -32,6 +34,7 @@ export async function syncMvtSourceData({
source,
syncContext,
}: {
buffer: number;
hasLabels: boolean;
layerId: string;
layerName: string;
Expand Down Expand Up @@ -62,7 +65,8 @@ export async function syncMvtSourceData({
!syncContext.forceRefreshDueToDrawing &&
noChangesInSourceState &&
noChangesInSearchState &&
prevData.hasLabels === hasLabels;
prevData.hasLabels === hasLabels &&
prevData.buffer === buffer;

if (canSkip) {
return;
Expand All @@ -78,7 +82,7 @@ export async function syncMvtSourceData({
? uuidv4()
: prevData.refreshToken;

const tileUrl = await source.getTileUrl(requestMeta, refreshToken, hasLabels);
const tileUrl = await source.getTileUrl(requestMeta, refreshToken, hasLabels, buffer);
if (source.isESSource()) {
syncContext.inspectorAdapters.vectorTiles.addLayer(layerId, layerName, tileUrl);
}
Expand All @@ -89,6 +93,7 @@ export async function syncMvtSourceData({
tileMaxZoom: source.getMaxZoom(),
refreshToken,
hasLabels,
buffer,
};
syncContext.stopLoading(SOURCE_DATA_REQUEST_ID, requestToken, sourceData, {});
} catch (error) {
Expand Down
20 changes: 19 additions & 1 deletion x-pack/plugins/maps/public/classes/layers/vector_layer/mvt_vector_layer/mvt_vector_layer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import { Feature } from 'geojson';
import { i18n } from '@kbn/i18n';
import { buildPhrasesFilter } from '@kbn/es-query';
import { VectorStyle } from '../../../styles/vector/vector_style';
import type { DynamicSizeProperty } from '../../../styles/vector/properties/dynamic_size_property';
import type { StaticSizeProperty } from '../../../styles/vector/properties/static_size_property';
import { getField } from '../../../../../common/elasticsearch_util';
import { LAYER_TYPE, SOURCE_TYPES } from '../../../../../common/constants';
import { LAYER_TYPE, SOURCE_TYPES, VECTOR_STYLES } from '../../../../../common/constants';
import {
NO_RESULTS_ICON_AND_TOOLTIPCONTENT,
AbstractVectorLayer,
Expand Down Expand Up @@ -225,7 +227,23 @@ export class MvtVectorLayer extends AbstractVectorLayer {
await this._syncSourceFormatters(syncContext, this.getSource(), this.getCurrentStyle());
await this._syncSupportsFeatureEditing({ syncContext, source: this.getSource() });

let maxLineWidth = 0;
const lineWidth = this.getCurrentStyle()
.getAllStyleProperties()
.find((styleProperty) => {
return styleProperty.getStyleName() === VECTOR_STYLES.LINE_WIDTH;
});
if (lineWidth) {
if (!lineWidth.isDynamic() && lineWidth.isComplete()) {
maxLineWidth = (lineWidth as StaticSizeProperty).getOptions().size;
} else if (lineWidth.isDynamic() && lineWidth.isComplete()) {
maxLineWidth = (lineWidth as DynamicSizeProperty).getOptions().maxSize;
}
}
const buffer = Math.ceil(3.5 * maxLineWidth);

await syncMvtSourceData({
buffer,
hasLabels: this.getCurrentStyle().hasLabels(),
layerId: this.getId(),
layerName: await this.getDisplayName(),
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,10 @@ describe('ESGeoGridSource', () => {
});

it('getTileUrl', async () => {
const tileUrl = await mvtGeogridSource.getTileUrl(vectorSourceRequestMeta, '1234', false);
const tileUrl = await mvtGeogridSource.getTileUrl(vectorSourceRequestMeta, '1234', false, 5);

expect(tileUrl).toEqual(
"rootdir/api/maps/mvt/getGridTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=foo-*&gridPrecision=8&hasLabels=false&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'1'%3A('0'%3Asize%2C'1'%3A0)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A''))%2C'6'%3A('0'%3Aaggs%2C'1'%3A())))&renderAs=heatmap&token=1234"
"rootdir/api/maps/mvt/getGridTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=foo-*&gridPrecision=8&hasLabels=false&buffer=5&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'1'%3A('0'%3Asize%2C'1'%3A0)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A''))%2C'6'%3A('0'%3Aaggs%2C'1'%3A())))&renderAs=heatmap&token=1234"
);
});
});
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,8 @@ export class ESGeoGridSource extends AbstractESAggSource implements IMvtVectorSo
async getTileUrl(
searchFilters: VectorSourceRequestMeta,
refreshToken: string,
hasLabels: boolean
hasLabels: boolean,
buffer: number
): Promise<string> {
const dataView = await this.getIndexPattern();
const searchSource = await this.makeSearchSource(searchFilters, 0);
Expand All @@ -517,6 +518,7 @@ export class ESGeoGridSource extends AbstractESAggSource implements IMvtVectorSo
&index=${dataView.getIndexPattern()}\
&gridPrecision=${this._getGeoGridPrecisionResolutionDelta()}\
&hasLabels=${hasLabels}\
&buffer=${buffer}\
&requestBody=${encodeMvtResponseBody(searchSource.getSearchRequestBody())}\
&renderAs=${this._descriptor.requestType}\
&token=${refreshToken}`;
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/maps/public/classes/sources/es_search_source/es_search_source.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ describe('ESSearchSource', () => {
geoField: geoFieldName,
indexPatternId: 'ipId',
});
const tileUrl = await esSearchSource.getTileUrl(searchFilters, '1234', false);
const tileUrl = await esSearchSource.getTileUrl(searchFilters, '1234', false, 5);
expect(tileUrl).toBe(
`rootdir/api/maps/mvt/getTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=foobar-title-*&hasLabels=false&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'1'%3A('0'%3Asize%2C'1'%3A1000)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A'tooltipField%3A%20foobar'))%2C'6'%3A('0'%3AfieldsFromSource%2C'1'%3A!(_id))%2C'7'%3A('0'%3Asource%2C'1'%3A!f)%2C'8'%3A('0'%3Afields%2C'1'%3A!(tooltipField%2CstyleField))))&token=1234`
`rootdir/api/maps/mvt/getTile/{z}/{x}/{y}.pbf?geometryFieldName=bar&index=foobar-title-*&hasLabels=false&buffer=5&requestBody=(foobar%3AES_DSL_PLACEHOLDER%2Cparams%3A('0'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'1'%3A('0'%3Asize%2C'1'%3A1000)%2C'2'%3A('0'%3Afilter%2C'1'%3A!())%2C'3'%3A('0'%3Aquery)%2C'4'%3A('0'%3Aindex%2C'1'%3A(fields%3A()%2Ctitle%3A'foobar-title-*'))%2C'5'%3A('0'%3Aquery%2C'1'%3A(language%3AKQL%2Cquery%3A'tooltipField%3A%20foobar'))%2C'6'%3A('0'%3AfieldsFromSource%2C'1'%3A!(_id))%2C'7'%3A('0'%3Asource%2C'1'%3A!f)%2C'8'%3A('0'%3Afields%2C'1'%3A!(tooltipField%2CstyleField))))&token=1234`
);
});
});
Expand Down
Loading

0 comments on commit c4ebad6

Please sign in to comment.