refactoring geohash agg to not use vis (#20298) (#20510)

This commit is contained in:
Peter Pisljar 2018-07-06 10:11:40 +02:00 committed by GitHub
parent cb907b564f
commit 92ed45719e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 89 additions and 95 deletions

View file

@ -43,14 +43,29 @@ export function CoordinateMapsVisualizationProvider(Notifier, Private) {
await super._makeKibanaMap();
this.vis.sessionState.mapBounds = this._kibanaMap.getBounds();
const updateGeohashAgg = () => {
const geohashAgg = this._getGeoHashAgg();
if (!geohashAgg) return;
geohashAgg.params.mapBounds = this._kibanaMap.getBounds();
geohashAgg.params.mapZoom = this._kibanaMap.getZoomLevel();
geohashAgg.params.mapCenter = this._kibanaMap.getCenter();
};
updateGeohashAgg();
const uiState = this.vis.getUiState();
uiState.on('change', (prop) => {
if (prop === 'mapZoom' || prop === 'mapCenter') {
updateGeohashAgg();
}
});
let previousPrecision = this._kibanaMap.getGeohashPrecision();
let precisionChange = false;
this._kibanaMap.on('zoomchange', () => {
const geohashAgg = this._getGeoHashAgg();
precisionChange = (previousPrecision !== this._kibanaMap.getGeohashPrecision());
previousPrecision = this._kibanaMap.getGeohashPrecision();
const geohashAgg = this._getGeoHashAgg();
if (!geohashAgg) {
return;
}
@ -78,12 +93,12 @@ export function CoordinateMapsVisualizationProvider(Notifier, Private) {
this._kibanaMap.addDrawControl();
this._kibanaMap.on('drawCreated:rectangle', event => {
const geoAgg = this._getGeoHashAgg();
this.addSpatialFilter(geoAgg, 'geo_bounding_box', event.bounds);
const geohashAgg = this._getGeoHashAgg();
this.addSpatialFilter(geohashAgg, 'geo_bounding_box', event.bounds);
});
this._kibanaMap.on('drawCreated:polygon', event => {
const geoAgg = this._getGeoHashAgg();
this.addSpatialFilter(geoAgg, 'geo_polygon', { points: event.points });
const geohashAgg = this._getGeoHashAgg();
this.addSpatialFilter(geohashAgg, 'geo_polygon', { points: event.points });
});
}

View file

@ -38,16 +38,10 @@ describe('Geohash Agg', () => {
},
params: {
isFilteredByCollar: true,
useGeocentroid: true
useGeocentroid: true,
mapZoom: initialZoom
},
vis: {
hasUiState: () => {
return false;
},
params: {
mapZoom: initialZoom
},
sessionState: {},
aggs: []
},
type: 'geohash_grid',
@ -69,30 +63,24 @@ describe('Geohash Agg', () => {
BucketAggTypeModule.BucketAggType.restore();
});
function initVisSessionState() {
aggMock.vis.sessionState = {
mapBounds: initialMapBounds
};
}
function initAggParams() {
aggMock.params.isFilteredByCollar = true;
aggMock.params.useGeocentroid = true;
aggMock.params.mapBounds = initialMapBounds;
}
function zoomMap(zoomChange) {
aggMock.vis.params.mapZoom += zoomChange;
aggMock.params.mapZoom += zoomChange;
}
function moveMap(newBounds) {
aggMock.vis.sessionState.mapBounds = newBounds;
aggMock.params.mapBounds = newBounds;
}
function resetMap() {
aggMock.vis.params.mapZoom = initialZoom;
aggMock.vis.sessionState.mapBounds = initialMapBounds;
aggMock.vis.sessionState.mapCollar = {
aggMock.params.mapZoom = initialZoom;
aggMock.params.mapBounds = initialMapBounds;
aggMock.params.mapCollar = {
top_left: { lat: 1.5, lon: -1.5 },
bottom_right: { lat: -1.5, lon: 1.5 },
zoom: initialZoom
@ -142,12 +130,9 @@ describe('Geohash Agg', () => {
it(`zoom level ${zoomLevel} should correspond to correct geohash-precision`, () => {
const output = { params: {} };
precisionParam.write({
vis: {
hasUiState: () => true,
uiStateVal: () => zoomLevel
},
params: {
autoPrecision: true
autoPrecision: true,
mapZoom: zoomLevel
}
}, output);
expect(output.params.precision).to.equal(zoomToGeoHashPrecision[zoomLevel]);
@ -162,7 +147,6 @@ describe('Geohash Agg', () => {
describe('initial aggregation creation', () => {
let requestAggs;
beforeEach(() => {
initVisSessionState();
initAggParams();
requestAggs = geoHashBucketAgg.getRequestAggs(aggMock);
});
@ -175,15 +159,15 @@ describe('Geohash Agg', () => {
});
it('should set mapCollar in vis session state', () => {
expect(aggMock.vis.sessionState).to.have.property('mapCollar');
expect(aggMock.vis.sessionState.mapCollar).to.have.property('top_left');
expect(aggMock.vis.sessionState.mapCollar).to.have.property('bottom_right');
expect(aggMock.vis.sessionState.mapCollar).to.have.property('zoom');
expect(aggMock).to.have.property('lastMapCollar');
expect(aggMock.lastMapCollar).to.have.property('top_left');
expect(aggMock.lastMapCollar).to.have.property('bottom_right');
expect(aggMock.lastMapCollar).to.have.property('zoom');
});
// there was a bug because of an "&& mapZoom" check which excluded 0 as a valid mapZoom, but it is.
it('should create filter, geohash_grid, and geo_centroid aggregations when zoom level 0', () => {
aggMock.vis.params.mapZoom = 0;
aggMock.params.mapZoom = 0;
requestAggs = geoHashBucketAgg.getRequestAggs(aggMock);
expect(requestAggs.length).to.equal(3);
expect(requestAggs[0].type).to.equal('filter');
@ -195,7 +179,6 @@ describe('Geohash Agg', () => {
describe('aggregation options', () => {
beforeEach(() => {
initVisSessionState();
initAggParams();
});
@ -225,7 +208,7 @@ describe('Geohash Agg', () => {
resetMap();
initAggParams();
origRequestAggs = geoHashBucketAgg.getRequestAggs(aggMock);
origMapCollar = aggMock.vis.sessionState.mapCollar;
origMapCollar = JSON.stringify(aggMock.lastMapCollar, null, '');
});
it('should not change geo_bounding_box filter aggregation and vis session state when map movement is within map collar', () => {
@ -237,8 +220,8 @@ describe('Geohash Agg', () => {
const newRequestAggs = geoHashBucketAgg.getRequestAggs(aggMock);
expect(JSON.stringify(origRequestAggs[0].params, null, '')).to.equal(JSON.stringify(newRequestAggs[0].params, null, ''));
const newMapCollar = aggMock.vis.sessionState.mapCollar;
expect(JSON.stringify(origMapCollar, null, '')).to.equal(JSON.stringify(newMapCollar, null, ''));
const newMapCollar = JSON.stringify(aggMock.lastMapCollar, null, '');
expect(origMapCollar).to.equal(newMapCollar);
});
it('should change geo_bounding_box filter aggregation and vis session state when map movement is outside map collar', () => {
@ -250,18 +233,17 @@ describe('Geohash Agg', () => {
const newRequestAggs = geoHashBucketAgg.getRequestAggs(aggMock);
expect(JSON.stringify(origRequestAggs[0].params, null, '')).not.to.equal(JSON.stringify(newRequestAggs[0].params, null, ''));
const newMapCollar = aggMock.vis.sessionState.mapCollar;
expect(JSON.stringify(origMapCollar, null, '')).not.to.equal(JSON.stringify(newMapCollar, null, ''));
const newMapCollar = JSON.stringify(aggMock.lastMapCollar, null, '');
expect(origMapCollar).not.to.equal(newMapCollar);
});
it('should change geo_bounding_box filter aggregation and vis session state when map zoom level changes', () => {
zoomMap(-1);
const newRequestAggs = geoHashBucketAgg.getRequestAggs(aggMock);
expect(JSON.stringify(origRequestAggs[0].params, null, '')).not.to.equal(JSON.stringify(newRequestAggs[0].params, null, ''));
geoHashBucketAgg.getRequestAggs(aggMock);
const newMapCollar = aggMock.vis.sessionState.mapCollar;
expect(JSON.stringify(origMapCollar, null, '')).not.to.equal(JSON.stringify(newMapCollar, null, ''));
const newMapCollar = JSON.stringify(aggMock.lastMapCollar, null, '');
expect(origMapCollar).not.to.equal(newMapCollar);
});
});

View file

@ -63,14 +63,6 @@ function getPrecision(precision) {
return precision;
}
function getMapZoom(vis) {
if (vis.hasUiState() && parseInt(vis.uiStateVal('mapZoom')) >= 0) {
return parseInt(vis.uiStateVal('mapZoom'));
}
return vis.params.mapZoom;
}
function isOutsideCollar(bounds, collar) {
return bounds && collar && !geoContains(collar, bounds);
}
@ -100,10 +92,12 @@ export const geoHashBucketAgg = new BucketAggType({
},
{
name: 'mapZoom',
default: 2,
write: _.noop
},
{
name: 'mapCenter',
default: [0, 0],
write: _.noop
},
{
@ -114,29 +108,27 @@ export const geoHashBucketAgg = new BucketAggType({
controller: function () {
},
write: function (aggConfig, output) {
const vis = aggConfig.vis;
const currZoom = getMapZoom(vis);
const currZoom = aggConfig.params.mapZoom;
const autoPrecisionVal = zoomPrecision[currZoom];
output.params.precision = aggConfig.params.autoPrecision ? autoPrecisionVal : getPrecision(aggConfig.params.precision);
output.params.precision = aggConfig.params.autoPrecision ?
autoPrecisionVal : getPrecision(aggConfig.params.precision);
}
}
],
getRequestAggs: function (agg) {
const aggs = [];
const { vis, params } = agg;
if (agg.params.isFilteredByCollar && agg.getField()) {
const vis = agg.vis;
const mapBounds = vis.sessionState.mapBounds;
const mapZoom = getMapZoom(vis);
if (params.isFilteredByCollar && agg.getField()) {
const { mapBounds, mapZoom } = params;
if (mapBounds) {
const lastMapCollar = vis.sessionState.mapCollar;
let mapCollar;
if (!lastMapCollar || lastMapCollar.zoom !== mapZoom || isOutsideCollar(mapBounds, lastMapCollar)) {
if (!agg.lastMapCollar || agg.lastMapCollar.zoom !== mapZoom || isOutsideCollar(mapBounds, agg.lastMapCollar)) {
mapCollar = scaleBounds(mapBounds);
mapCollar.zoom = mapZoom;
vis.sessionState.mapCollar = mapCollar;
agg.lastMapCollar = mapCollar;
} else {
mapCollar = lastMapCollar;
mapCollar = agg.lastMapCollar;
}
const boundingBox = {
ignore_unmapped: true,
@ -145,7 +137,7 @@ export const geoHashBucketAgg = new BucketAggType({
bottom_right: mapCollar.bottom_right
}
};
aggs.push(new AggConfig(agg.vis, {
aggs.push(new AggConfig(vis, {
type: 'filter',
id: 'filter_agg',
enabled: true,
@ -161,8 +153,8 @@ export const geoHashBucketAgg = new BucketAggType({
aggs.push(agg);
if (agg.params.useGeocentroid) {
aggs.push(new AggConfig(agg.vis, {
if (params.useGeocentroid) {
aggs.push(new AggConfig(vis, {
type: 'geo_centroid',
enabled: true,
params: {

View file

@ -647,7 +647,6 @@ export class KibanaMap extends EventEmitter {
if (!centerFromUIState || centerFromMap.lon !== centerFromUIState[1] || centerFromMap.lat !== centerFromUIState[0]) {
visualization.uiStateVal('mapCenter', [centerFromMap.lat, centerFromMap.lon]);
}
visualization.sessionState.mapBounds = this.getBounds();
}
this._leafletMap.on('resize', () => {

View file

@ -109,30 +109,6 @@ export default function ({ getService, getPageObjects }) {
expect(roundedValues).to.eql(expected);
}
describe('Only request data around extent of map option', async () => {
before(async () => await PageObjects.visualize.openInspector());
it('when checked adds filters to aggregation', async () => {
const tableHeaders = await PageObjects.visualize.getInspectorTableHeaders();
expect(tableHeaders).to.eql(['filter', 'geohash_grid', 'Count', 'Geo Centroid']);
});
it('when not checked does not add filters to aggregation', async () => {
await PageObjects.visualize.toggleIsFilteredByCollarCheckbox();
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();
const tableHeaders = await PageObjects.visualize.getInspectorTableHeaders();
expect(tableHeaders).to.eql(['geohash_grid', 'Count', 'Geo Centroid']);
});
after(async () => {
await PageObjects.visualize.closeInspector();
await PageObjects.visualize.toggleIsFilteredByCollarCheckbox();
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();
});
});
describe('tile map chart', function indexPatternCreation() {
it('should have inspector enabled', async function () {
const spyToggleExists = await PageObjects.visualize.isInspectorButtonEnabled();
@ -205,12 +181,13 @@ export default function ({ getService, getPageObjects }) {
await PageObjects.visualize.clickMapZoomIn();
const mapBounds = await PageObjects.visualize.getMapBounds();
await PageObjects.visualize.closeInspector();
await PageObjects.visualize.saveVisualization(vizName1);
const afterSaveMapBounds = await PageObjects.visualize.getMapBounds();
await PageObjects.visualize.closeInspector();
// For some reason the values are slightly different, so we can't check that they are equal. But we did
// have a bug where after the save, there were _no_ map bounds. So this checks for the later case, but
// until we figure out how to make sure the map center is always the exact same, we can't comparison check.
@ -219,6 +196,35 @@ export default function ({ getService, getPageObjects }) {
});
});
describe('Only request data around extent of map option', async () => {
it('when checked adds filters to aggregation', async () => {
const vizName1 = 'Visualization TileMap';
await PageObjects.visualize.loadSavedVisualization(vizName1);
await PageObjects.visualize.openInspector();
const tableHeaders = await PageObjects.visualize.getInspectorTableHeaders();
await PageObjects.visualize.closeInspector();
expect(tableHeaders).to.eql(['filter', 'geohash_grid', 'Count', 'Geo Centroid']);
});
it('when not checked does not add filters to aggregation', async () => {
await PageObjects.visualize.toggleOpenEditor(2);
await PageObjects.visualize.toggleIsFilteredByCollarCheckbox();
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();
await PageObjects.visualize.openInspector();
const tableHeaders = await PageObjects.visualize.getInspectorTableHeaders();
await PageObjects.visualize.closeInspector();
expect(tableHeaders).to.eql(['geohash_grid', 'Count', 'Geo Centroid']);
});
after(async () => {
await PageObjects.visualize.toggleIsFilteredByCollarCheckbox();
await PageObjects.visualize.clickGo();
await PageObjects.header.waitUntilLoadingHasFinished();
});
});
});
});
}