Skip to content

Commit

Permalink
[maps] remove ISource.getIndexPatternIds and ISource.getQueryableInde…
Browse files Browse the repository at this point in the history
…xPatternIds methods (elastic#176095)

While implementing `IESSource` interface with ES|QL source, I discovered
that `ISource` and `IESSource` interfaces produce a confusing
combination of methods. `ISource` interface has `getIndexPatternIds` and
`getQueryableIndexPatternIds` methods. The more specific `IESSource`
interface has `getIndexPatternId` method. Having `getIndexPatternIds`
and `getIndexPatternId` on the same class is confusing.

PR removes `getIndexPatternIds` and `getQueryableIndexPatternIds`
methods from `ISource` and uses type guards to call
`IESSource.getIndexPatternId` where appropriate.

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
nreese and kibanamachine authored Feb 5, 2024
1 parent 11593f0 commit ff2b79c
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,6 @@ export class CustomRasterSource implements IRasterSource {
return false;
}

getIndexPatternIds(): string[] {
return [];
}

getQueryableIndexPatternIds(): string[] {
return [];
}

// Returns function used to format value
async createFieldFormatter(field: IField): Promise<FieldFormatter | null> {
return null;
Expand Down
8 changes: 0 additions & 8 deletions x-pack/plugins/maps/public/classes/joins/inner_join.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,6 @@ export class InnerJoin {
return await this.getRightJoinSource().getTooltipProperties(properties, executionContext);
}

getIndexPatternIds() {
return this.getRightJoinSource().getIndexPatternIds();
}

getQueryableIndexPatternIds() {
return this.getRightJoinSource().getQueryableIndexPatternIds();
}

getWhereQuery(): Query | undefined {
return this.getRightJoinSource().getWhereQuery();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { HeatmapStyle } from '../../styles/heatmap/heatmap_style';
import { LAYER_TYPE } from '../../../../common/constants';
import { HeatmapLayerDescriptor } from '../../../../common/descriptor_types';
import { ESGeoGridSource } from '../../sources/es_geo_grid_source';
import { hasESSourceMethod } from '../../sources/es_source';
import {
NO_RESULTS_ICON_AND_TOOLTIPCONTENT,
syncBoundsData,
Expand Down Expand Up @@ -236,11 +237,15 @@ export class HeatmapLayer extends AbstractLayer {
}

getIndexPatternIds() {
return this.getSource().getIndexPatternIds();
const source = this.getSource();
return hasESSourceMethod(source, 'getIndexPatternId') ? [source.getIndexPatternId()] : [];
}

getQueryableIndexPatternIds() {
return this.getSource().getQueryableIndexPatternIds();
const source = this.getSource();
return source.getApplyGlobalQuery() && hasESSourceMethod(source, 'getIndexPatternId')
? [source.getIndexPatternId()]
: [];
}

async getLicensedFeatures() {
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/maps/public/classes/layers/layer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,14 @@ export interface ILayer {
ownsMbSourceId(mbSourceId: string): boolean;
syncLayerWithMB(mbMap: MbMap, timeslice?: Timeslice): void;
getLayerTypeIconName(): string;
/*
* ILayer.getIndexPatternIds returns data view ids used to populate layer data.
*/
getIndexPatternIds(): string[];
/*
* ILayer.getQueryableIndexPatternIds returns ILayer.getIndexPatternIds or a subset of ILayer.getIndexPatternIds.
* Data view ids are excluded when the global query is not applied to layer data.
*/
getQueryableIndexPatternIds(): string[];
getType(): LAYER_TYPE;
isVisible(): boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,17 +362,36 @@ export class AbstractVectorLayer extends AbstractLayer implements IVectorLayer {
}

getIndexPatternIds() {
const indexPatternIds = this.getSource().getIndexPatternIds();
const indexPatternIds = [];
const source = this.getSource();
if (hasESSourceMethod(source, 'getIndexPatternId')) {
indexPatternIds.push(source.getIndexPatternId());
}

this.getValidJoins().forEach((join) => {
indexPatternIds.push(...join.getIndexPatternIds());
const rightSource = join.getRightJoinSource();
if (hasESSourceMethod(rightSource, 'getIndexPatternId')) {
indexPatternIds.push(rightSource.getIndexPatternId());
}
});
return indexPatternIds;
}

getQueryableIndexPatternIds() {
const indexPatternIds = this.getSource().getQueryableIndexPatternIds();
const indexPatternIds = [];
const source = this.getSource();
if (source.getApplyGlobalQuery() && hasESSourceMethod(source, 'getIndexPatternId')) {
indexPatternIds.push(source.getIndexPatternId());
}

this.getValidJoins().forEach((join) => {
indexPatternIds.push(...join.getQueryableIndexPatternIds());
const rightSource = join.getRightJoinSource();
if (
rightSource.getApplyGlobalQuery() &&
hasESSourceMethod(rightSource, 'getIndexPatternId')
) {
indexPatternIds.push(rightSource.getIndexPatternId());
}
});
return indexPatternIds;
}
Expand Down
11 changes: 0 additions & 11 deletions x-pack/plugins/maps/public/classes/sources/es_source/es_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,6 @@ export class AbstractESSource extends AbstractVectorSource implements IESSource
return true;
}

getIndexPatternIds(): string[] {
return [this.getIndexPatternId()];
}

getQueryableIndexPatternIds(): string[] {
if (this.getApplyGlobalQuery()) {
return [this.getIndexPatternId()];
}
return [];
}

cloneDescriptor(): AbstractSourceDescriptor {
const clonedDescriptor = copyPersistentState(this._descriptor);
// id used as uuid to track requests in inspector
Expand Down
10 changes: 0 additions & 10 deletions x-pack/plugins/maps/public/classes/sources/source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ export interface ISource {
getApplyGlobalQuery(): boolean;
getApplyGlobalTime(): boolean;
getApplyForceRefresh(): boolean;
getIndexPatternIds(): string[];
getQueryableIndexPatternIds(): string[];
createFieldFormatter(field: IField): Promise<FieldFormatter | null>;
getValueSuggestions(field: IField, query: string): Promise<string[]>;
getMinZoom(): number;
Expand Down Expand Up @@ -134,14 +132,6 @@ export class AbstractSource implements ISource {
return false;
}

getIndexPatternIds(): string[] {
return [];
}

getQueryableIndexPatternIds(): string[] {
return [];
}

// Returns function used to format value
async createFieldFormatter(field: IField): Promise<FieldFormatter | null> {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ import { FormattedMessage } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
import type { DataView, Query } from '@kbn/data-plugin/common';
import { APP_ID } from '../../../../common/constants';
import { getIndexPatternService, getData, getSearchBar } from '../../../kibana_services';
import { getData, getSearchBar } from '../../../kibana_services';
import { GlobalFilterCheckbox } from '../../../components/global_filter_checkbox';
import { GlobalTimeCheckbox } from '../../../components/global_time_checkbox';
import { ILayer } from '../../../classes/layers/layer';
import { hasESSourceMethod } from '../../../classes/sources/es_source';
import { ForceRefreshCheckbox } from '../../../components/force_refresh_checkbox';

export interface Props {
Expand All @@ -40,48 +41,40 @@ export interface Props {

interface State {
isPopoverOpen: boolean;
indexPatterns: DataView[];
dataView?: DataView;
isSourceTimeAware: boolean;
}

export class FilterEditor extends Component<Props, State> {
private _isMounted = false;
state: State = {
isPopoverOpen: false,
indexPatterns: [],
isSourceTimeAware: false,
};

componentDidMount() {
this._isMounted = true;
this._loadIndexPatterns();
this._loadDataView();
this._loadSourceTimeAware();
}

componentWillUnmount() {
this._isMounted = false;
}

async _loadIndexPatterns() {
// Filter only effects source so only load source indices.
const indexPatternIds = this.props.layer.getSource().getIndexPatternIds();
const indexPatterns: DataView[] = [];
const getIndexPatternPromises = indexPatternIds.map(async (indexPatternId) => {
try {
const indexPattern = await getIndexPatternService().get(indexPatternId);
indexPatterns.push(indexPattern);
} catch (err) {
// unable to fetch index pattern
}
});

await Promise.all(getIndexPatternPromises);
async _loadDataView() {
const source = this.props.layer.getSource();
if (!hasESSourceMethod(source, 'getIndexPattern')) {
return;
}

const dataView = await source.getIndexPattern();

if (!this._isMounted) {
return;
}

this.setState({ indexPatterns });
this.setState({ dataView });
}

async _loadSourceTimeAware() {
Expand Down Expand Up @@ -142,7 +135,7 @@ export class FilterEditor extends Component<Props, State> {
showQueryInput={true}
query={layerQuery ? layerQuery : getData().query.queryString.getDefaultQuery()}
onQuerySubmit={this._onQueryChange}
indexPatterns={this.state.indexPatterns}
indexPatterns={this.state.dataView ? [this.state.dataView] : []}
customSubmitButton={
<EuiButton fill data-test-subj="mapFilterEditorSubmitButton">
<FormattedMessage
Expand Down
10 changes: 0 additions & 10 deletions x-pack/plugins/ml/public/maps/anomaly_source.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,6 @@ export class AnomalySource implements IVectorSource {
return null;
}

getIndexPatternIds(): string[] {
// IGNORE: This is only relevant if your source is backed by an index-pattern
return [];
}

getInspectorAdapters(): Adapters | undefined {
// IGNORE: This is only relevant if your source is backed by an index-pattern
return undefined;
Expand All @@ -331,11 +326,6 @@ export class AnomalySource implements IVectorSource {
return [];
}

getQueryableIndexPatternIds(): string[] {
// IGNORE: This is only relevant if your source is backed by an index-pattern
return [];
}

isFilterByMapBounds(): boolean {
// Only implement if you can query this data with a bounding-box
return false;
Expand Down

0 comments on commit ff2b79c

Please sign in to comment.