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

[maps] fix geojson layer with joins and no left source matches stuck in loading state #160222

Merged
merged 13 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from 12 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { SOURCE_DATA_REQUEST_ID } from '../../../../../common/constants';
import { VectorLayerDescriptor } from '../../../../../common/descriptor_types';
import { InnerJoin } from '../../../joins/inner_join';
import { IJoinSource } from '../../../sources/join_sources';
import { IVectorSource } from '../../../sources/vector_source';
import { GeoJsonVectorLayer } from './geojson_vector_layer';

const joinDataRequestId = 'join_source_a0b0da65-5e1a-4967-9dbe-74f24391afe2';
const mockJoin = {
hasCompleteConfig: () => {
return true;
},
getSourceDataRequestId: () => {
return joinDataRequestId;
},
getRightJoinSource: () => {
return {} as unknown as IJoinSource;
},
} as unknown as InnerJoin;

describe('isLayerLoading', () => {
test('should return true when source loading has not started', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
layerDescriptor: {},
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(true);
});

test('Should return true when source data request is pending', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
layerDescriptor: {
__dataRequests: [
{
data: {},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMetaAtStart: {},
dataRequestToken: Symbol(),
},
],
},
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(true);
});

describe('no joins', () => {
test('Should return false when source data request is finished', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
layerDescriptor: {
__dataRequests: [
{
data: {},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMeta: {},
dataRequestMetaAtStart: undefined,
dataRequestToken: undefined,
},
],
},
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(false);
});
});

describe('joins', () => {
describe('source data loaded with no features', () => {
test('should return false when join loading has not started', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
joins: [mockJoin],
layerDescriptor: {
__dataRequests: [
{
data: {
features: [],
},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMeta: {},
dataRequestMetaAtStart: undefined,
dataRequestToken: undefined,
},
],
} as unknown as VectorLayerDescriptor,
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(false);
});
});

describe('source data loaded with features', () => {
const sourceDataRequestDescriptorWithFeatures = {
data: {
features: [{}],
},
dataId: SOURCE_DATA_REQUEST_ID,
dataRequestMeta: {},
dataRequestMetaAtStart: undefined,
dataRequestToken: undefined,
};

test('should return true when join loading has not started', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
joins: [mockJoin],
layerDescriptor: {
__dataRequests: [sourceDataRequestDescriptorWithFeatures],
} as unknown as VectorLayerDescriptor,
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(true);
});

test('should return true when join data request is pending', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
joins: [mockJoin],
layerDescriptor: {
__dataRequests: [
sourceDataRequestDescriptorWithFeatures,
{
dataId: joinDataRequestId,
dataRequestMetaAtStart: {},
dataRequestToken: Symbol(),
},
],
} as unknown as VectorLayerDescriptor,
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(true);
});

test('should return false when join data request is finished', () => {
const layer = new GeoJsonVectorLayer({
customIcons: [],
joins: [mockJoin],
layerDescriptor: {
__dataRequests: [
sourceDataRequestDescriptorWithFeatures,
{
data: {},
dataId: joinDataRequestId,
dataRequestMeta: {},
dataRequestMetaAtStart: undefined,
dataRequestToken: undefined,
},
],
} as unknown as VectorLayerDescriptor,
source: {} as unknown as IVectorSource,
});
expect(layer.isLayerLoading(1)).toBe(false);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,23 @@ export class GeoJsonVectorLayer extends AbstractVectorLayer {
return layerDescriptor;
}

isLayerLoading(zoom: number) {
const isSourceLoading = super.isLayerLoading(zoom);
if (isSourceLoading) {
return true;
}

// Do not check join loading status when there are no source features. Why?
// syncMeta short circuits join loading when there are no source features
// because there is no reason to fetch join results when there is nothing to join with
const featureCollection = this._getSourceFeatureCollection();
if (!featureCollection || featureCollection?.features?.length === 0) {
return false;
}

return this._isLoadingJoins();
}

_isTiled(): boolean {
// Uses untiled maplibre source 'geojson'
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ export class MvtVectorLayer extends AbstractVectorLayer {
this._source = args.source as IMvtVectorSource;
}

isLayerLoading(zoom: number) {
const isSourceLoading = super.isLayerLoading(zoom);
return isSourceLoading ? true : this._isLoadingJoins();
}

_isTiled(): boolean {
// Uses tiled maplibre source 'vector'
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,7 @@ export class AbstractVectorLayer extends AbstractLayer implements IVectorLayer {
return this.getValidJoins().length > 0;
}

isLayerLoading(zoom: number) {
const isSourceLoading = super.isLayerLoading(zoom);
if (isSourceLoading) {
return true;
}

_isLoadingJoins() {
return this.getValidJoins().some((join) => {
const joinDataRequest = this.getDataRequest(join.getSourceDataRequestId());
return !joinDataRequest || joinDataRequest.isLoading();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ export function GeoIndexPatternSelect(props: Props) {
}

const IndexPatternSelect = getIndexPatternSelectComponent();
const error = !hasGeoFields
const isDataViewInvalid = !props.dataView ? false : !hasGeoFields;
Copy link
Contributor Author

@nreese nreese Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for displaying data view select input with validation errors when no data view is selected

Screen Shot 2023-06-22 at 3 40 12 PM

const error = isDataViewInvalid
? i18n.translate('xpack.maps.noGeoFieldInIndexPattern.message', {
defaultMessage: 'Data view does not contain any geospatial fields',
})
Expand All @@ -116,9 +117,9 @@ export function GeoIndexPatternSelect(props: Props) {
<>
{_renderNoIndexPatternWarning()}

<EuiFormRow label={getDataViewLabel()} isInvalid={!hasGeoFields} error={error}>
<EuiFormRow label={getDataViewLabel()} isInvalid={isDataViewInvalid} error={error}>
<IndexPatternSelect
isInvalid={!hasGeoFields}
isInvalid={isDataViewInvalid}
isDisabled={noDataViews}
indexPatternId={props.dataView?.id ? props.dataView.id : ''}
onChange={_onIndexPatternSelect}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export class AddLayerPanel extends Component<Props, State> {
disabled={isDisabled || isLoading}
isLoading={isLoading}
onClick={addLayersAndClose}
iconSide="right"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny fix so both buttons display loading icon on same side
addLayerButtons

>
{ADD_LAYER_STEP_SECONDARY_ACTION_BUTTON_LABEL}
</EuiButton>
Expand Down