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

fix: removed unsupported time ordering from interpolated query; cache… #323

Merged
merged 1 commit into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions src/RelativeRangeRequestCache/RelativeRangeCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { DataFrame, DataQueryRequest, DataQueryResponse, LoadingState, TimeRange
import { isTimeRangeCoveringStart } from 'timeRangeUtils';
import { SitewiseQuery } from 'types';
import { RequestCacheId, generateSiteWiseRequestCacheId } from './cacheIdUtils';
import { CachedQueryInfo, TIME_SERIES_QUERY_TYPES } from './types';
import { CachedQueryInfo, isTimeSeriesQueryType } from './types';
import { trimCachedQueryDataFramesAtStart, trimCachedQueryDataFramesEnding } from './dataFrameUtils';
import { getRefreshRequestRange, isCacheableTimeRange } from './timeRangeUtils';

Expand Down Expand Up @@ -174,7 +174,7 @@ export class RelativeRangeCache {
return {
...request,
range,
targets: targets.filter(({ queryType }) => TIME_SERIES_QUERY_TYPES.has(queryType)),
targets: targets.filter(({ queryType }) => isTimeSeriesQueryType(queryType)),
};
}
}
13 changes: 7 additions & 6 deletions src/RelativeRangeRequestCache/cacheIdUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { QueryType, SiteWiseQuality, SiteWiseResolution, SiteWiseResponseFormat, SiteWiseTimeOrder } from 'types';
import { AggregateType, QueryType, SiteWiseQuality, SiteWiseResolution, SiteWiseResponseFormat, SiteWiseTimeOrder } from 'types';
import { generateSiteWiseQueriesCacheId, generateSiteWiseRequestCacheId } from './cacheIdUtils';
import { dateTime } from '@grafana/data';
import { SitewiseQueriesUnion } from './types';
Expand Down Expand Up @@ -27,15 +27,16 @@ function createSiteWiseQuery(id: number): SitewiseQueriesUnion {
hierarchyId: `mock-hierarchy-${id}`,
modelId: `mock-model-${id}`,
filter: 'ALL',
aggregates: [AggregateType.AVERAGE],
};
}

describe('generateSiteWiseQueriesCacheId()', () => {
it('parses SiteWise Queries into cache Id', () => {
const actualId = generateSiteWiseQueriesCacheId([createSiteWiseQuery(1), createSiteWiseQuery(2)]);
const expectedId = JSON.stringify([
'["PropertyValueHistory","us-west-2","table","mock-asset-id-1",["mock-asset-id-1"],"mock-property-id-1","mock-property-alias-1","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-1","mock-model-1","ALL"]',
'["PropertyValueHistory","us-west-2","table","mock-asset-id-2",["mock-asset-id-2"],"mock-property-id-2","mock-property-alias-2","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-2","mock-model-2","ALL"]'
'["PropertyValueHistory","us-west-2","table","mock-asset-id-1",["mock-asset-id-1"],"mock-property-id-1","mock-property-alias-1","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-1","mock-model-1","ALL",["AVERAGE"]]',
'["PropertyValueHistory","us-west-2","table","mock-asset-id-2",["mock-asset-id-2"],"mock-property-id-2","mock-property-alias-2","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-2","mock-model-2","ALL",["AVERAGE"]]'
]);

expect(actualId).toEqual(expectedId);
Expand Down Expand Up @@ -82,7 +83,7 @@ describe('generateSiteWiseQueriesCacheId()', () => {
};
const actualId = generateSiteWiseQueriesCacheId([query]);
const expectedId = JSON.stringify([
'["ListAssets",null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null]',
'["ListAssets",null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null]',
]);

expect(actualId).toEqual(expectedId);
Expand Down Expand Up @@ -112,8 +113,8 @@ describe('generateSiteWiseRequestCacheId()', () => {
const expectedId = JSON.stringify([
'now-15m',
JSON.stringify([
'["PropertyValueHistory","us-west-2","table","mock-asset-id-1",["mock-asset-id-1"],"mock-property-id-1","mock-property-alias-1","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-1","mock-model-1","ALL"]',
'["PropertyValueHistory","us-west-2","table","mock-asset-id-2",["mock-asset-id-2"],"mock-property-id-2","mock-property-alias-2","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-2","mock-model-2","ALL"]'
'["PropertyValueHistory","us-west-2","table","mock-asset-id-1",["mock-asset-id-1"],"mock-property-id-1","mock-property-alias-1","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-1","mock-model-1","ALL",["AVERAGE"]]',
'["PropertyValueHistory","us-west-2","table","mock-asset-id-2",["mock-asset-id-2"],"mock-property-id-2","mock-property-alias-2","ANY","AUTO",true,true,1000,"grafana-iot-sitewise-datasource","mock-datasource-uid","ASCENDING",true,"mock-hierarchy-2","mock-model-2","ALL",["AVERAGE"]]'
])
]);

Expand Down
2 changes: 2 additions & 0 deletions src/RelativeRangeRequestCache/cacheIdUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function generateSiteWiseQueryCacheId(query: SitewiseQueriesUnion): QueryCacheId
hierarchyId,
modelId,
filter,
aggregates,
} = query;

/*
Expand All @@ -66,5 +67,6 @@ function generateSiteWiseQueryCacheId(query: SitewiseQueriesUnion): QueryCacheId
hierarchyId,
modelId,
filter,
aggregates,
]);
}
1 change: 0 additions & 1 deletion src/RelativeRangeRequestCache/dataFrameUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ describe('trimCachedQueryDataFrames', () => {

it.each([
QueryType.PropertyAggregate,
QueryType.PropertyInterpolated,
QueryType.PropertyValueHistory,
])('trims descending time series data of time-series type - "%s"', (queryType: QueryType) => {
const cachedQueryInfo = {
Expand Down
16 changes: 11 additions & 5 deletions src/RelativeRangeRequestCache/dataFrameUtils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { AbsoluteTimeRange, DataFrame } from '@grafana/data';
import { CachedQueryInfo, TIME_SERIES_QUERY_TYPES } from './types';
import { CachedQueryInfo, SitewiseQueriesUnion, isTimeOrderingQueryType, isTimeSeriesQueryType } from './types';
import { QueryType, SiteWiseTimeOrder } from 'types';
import { trimTimeSeriesDataFrame, trimTimeSeriesDataFrameReversedTime } from 'dataFrameUtils';

function isRequestTimeDescending({queryType, timeOrdering}: SitewiseQueriesUnion) {
return isTimeOrderingQueryType(queryType) && timeOrdering === SiteWiseTimeOrder.DESCENDING;
}

/**
* Trim cached query data frames based on the query type and time ordering for appending to the start of the data frame.
*
Expand All @@ -20,8 +24,10 @@ import { trimTimeSeriesDataFrame, trimTimeSeriesDataFrameReversedTime } from 'da
export function trimCachedQueryDataFramesAtStart(cachedQueryInfos: CachedQueryInfo[], cacheRange: AbsoluteTimeRange): DataFrame[] {
return cachedQueryInfos
.map((cachedQueryInfo) => {
const { query: { queryType, timeOrdering }, dataFrame } = cachedQueryInfo;
if (timeOrdering === SiteWiseTimeOrder.DESCENDING) {
const { query, dataFrame } = cachedQueryInfo;
const { queryType } = query;

if (isRequestTimeDescending(query)) {
// Descending ordering data frame are added at the end of the request to respect the ordering
// See related function - trimCachedQueryDataFramesEnding()
return {
Expand All @@ -40,7 +46,7 @@ export function trimCachedQueryDataFramesAtStart(cachedQueryInfos: CachedQueryIn
};
}

if (TIME_SERIES_QUERY_TYPES.has(queryType)) {
if (isTimeSeriesQueryType(queryType)) {
return trimTimeSeriesDataFrame({
dataFrame: cachedQueryInfo.dataFrame,
timeRange: cacheRange,
Expand Down Expand Up @@ -68,7 +74,7 @@ export function trimCachedQueryDataFramesAtStart(cachedQueryInfos: CachedQueryIn
*/
export function trimCachedQueryDataFramesEnding(cachedQueryInfos: CachedQueryInfo[], cacheRange: AbsoluteTimeRange): DataFrame[] {
return cachedQueryInfos
.filter((cachedQueryInfo) => (cachedQueryInfo.query.timeOrdering === SiteWiseTimeOrder.DESCENDING))
.filter(({query}) => (isRequestTimeDescending(query)))
.map((cachedQueryInfo) => {
return trimTimeSeriesDataFrameReversedTime({
dataFrame: cachedQueryInfo.dataFrame,
Expand Down
20 changes: 17 additions & 3 deletions src/RelativeRangeRequestCache/types.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,34 @@
import { DataFrame } from '@grafana/data';
import { AssetPropertyValueHistoryQuery, ListAssetsQuery, ListAssociatedAssetsQuery, QueryType, SitewiseQuery } from 'types';
import { AssetPropertyAggregatesQuery, AssetPropertyValueHistoryQuery, ListAssetsQuery, ListAssociatedAssetsQuery, QueryType, SitewiseQuery } from 'types';

export const TIME_SERIES_QUERY_TYPES = new Set<QueryType>([
const TIME_SERIES_QUERY_TYPES = new Set<QueryType>([
QueryType.PropertyAggregate,
QueryType.PropertyInterpolated,
QueryType.PropertyValue,
QueryType.PropertyValueHistory,
]);

export function isTimeSeriesQueryType(queryType: QueryType) {
return TIME_SERIES_QUERY_TYPES.has(queryType);
}

const TIME_ORDERING_QUERY_TYPES = new Set<QueryType>([
QueryType.PropertyAggregate,
QueryType.PropertyValueHistory,
]);

export function isTimeOrderingQueryType(queryType: QueryType) {
return TIME_ORDERING_QUERY_TYPES.has(queryType);
}

export interface CachedQueryInfo {
query: Pick<SitewiseQueriesUnion, 'queryType' | 'timeOrdering' | 'lastObservation'>;
query: SitewiseQueriesUnion;
dataFrame: DataFrame;
}

// Union of all SiteWise queries variants
export type SitewiseQueriesUnion = SitewiseQuery
& Partial<Pick<AssetPropertyAggregatesQuery, 'aggregates'>>
& Partial<Pick<AssetPropertyValueHistoryQuery, 'timeOrdering'>>
& Partial<Pick<ListAssociatedAssetsQuery, 'loadAllChildren'>>
& Partial<Pick<ListAssociatedAssetsQuery, 'hierarchyId'>>
Expand Down
67 changes: 41 additions & 26 deletions src/components/query/QualityAndOrderRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
SiteWiseResolution,
isAssetPropertyInterpolatedQuery,
SiteWiseResponseFormat,
QueryType,
} from 'types';
import { InlineField, Select } from '@grafana/ui';
import { SitewiseQueryEditorProps } from './types';
Expand Down Expand Up @@ -62,11 +63,6 @@ export class QualityAndOrderRow extends PureComponent<Props> {
onChange({ ...query, quality: sel.value });
};

onOrderChange = (sel: SelectableValue<SiteWiseTimeOrder>) => {
const { onChange, query } = this.props;
onChange({ ...query, timeOrdering: sel.value });
};

onResponseFormatChange = (sel: SelectableValue<SiteWiseResponseFormat>) => {
const { onChange, query } = this.props;
onChange({ ...query, responseFormat: sel.value });
Expand All @@ -83,6 +79,44 @@ export class QualityAndOrderRow extends PureComponent<Props> {
onChange({ ...query, maxPageAggregations: +event.currentTarget.value });
};

timeOrderField = () => {
Copy link
Member

Choose a reason for hiding this comment

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

if we want to break this into it's own function, I think it might make sense to create a separate functional component for this.

const { onChange, query } = this.props;

// PropertyInterpolated has no time ordering support
if (query.queryType === QueryType.PropertyInterpolated) {
return null;
}

const onOrderChange = (sel: SelectableValue<SiteWiseTimeOrder>) => {
onChange({ ...query, timeOrdering: sel.value });
};

return this.props.newFormStylingEnabled ? (
<EditorField label="Time" width={10} htmlFor="time">
<Select
id="time"
aria-label="Time"
options={ordering}
value={ordering.find((v) => v.value === query.timeOrdering) ?? ordering[0]}
onChange={onOrderChange}
isSearchable={true}
menuPlacement="auto"
/>
</EditorField>
) : (
<InlineField htmlFor="time" label="Time" labelWidth={8}>
<Select
inputId="time"
options={ordering}
value={ordering.find((v) => v.value === query.timeOrdering) ?? ordering[0]}
onChange={onOrderChange}
isSearchable={true}
menuPlacement="bottom"
/>
</InlineField>
);
};

render() {
const { query } = this.props;
return this.props.newFormStylingEnabled ? (
Expand All @@ -98,17 +132,7 @@ export class QualityAndOrderRow extends PureComponent<Props> {
menuPlacement="auto"
/>
</EditorField>
<EditorField label="Time" width={10} htmlFor="time">
<Select
id="time"
aria-label="Time"
options={ordering}
value={ordering.find((v) => v.value === query.timeOrdering) ?? ordering[0]}
onChange={this.onOrderChange}
isSearchable={true}
menuPlacement="auto"
/>
</EditorField>
{this.timeOrderField()}
<EditorField label="Format" width={10} htmlFor="format">
<Select
id="format"
Expand Down Expand Up @@ -145,16 +169,7 @@ export class QualityAndOrderRow extends PureComponent<Props> {
menuPlacement="bottom"
/>
</InlineField>
<InlineField htmlFor="time" label="Time" labelWidth={8}>
<Select
inputId="time"
options={ordering}
value={ordering.find((v) => v.value === query.timeOrdering) ?? ordering[0]}
onChange={this.onOrderChange}
isSearchable={true}
menuPlacement="bottom"
/>
</InlineField>
{this.timeOrderField()}

<InlineField htmlFor="format" label="Format" labelWidth={8}>
<Select
Expand Down
2 changes: 0 additions & 2 deletions src/components/query/QueryEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ describe('QueryEditor', () => {
expect(screen.getByText('Asset')).toBeInTheDocument();
expect(screen.getByText('Property')).toBeInTheDocument();
expect(screen.getByText('Quality')).toBeInTheDocument();
expect(screen.getByText('Time')).toBeInTheDocument();
expect(screen.getByText('Format')).toBeInTheDocument();
expect(screen.getByText('Resolution')).toBeInTheDocument();
});
Expand All @@ -125,7 +124,6 @@ describe('QueryEditor', () => {
await waitFor(() => {
expect(screen.getByText('Property Alias')).toBeInTheDocument();
expect(screen.getByText('Quality')).toBeInTheDocument();
expect(screen.getByText('Time')).toBeInTheDocument();
expect(screen.getByText('Resolution')).toBeInTheDocument();
expect(screen.getByText('Format')).toBeInTheDocument();
});
Expand Down
4 changes: 1 addition & 3 deletions src/queryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ export const siteWiseQueryTypes: QueryTypeInfo[] = [
label: 'Get interpolated property values',
value: QueryType.PropertyInterpolated,
description: `Gets interpolated values for an asset property.`,
defaultQuery: {
timeOrdering: 'ASCENDING',
} as AssetPropertyInterpolatedQuery,
defaultQuery: {} as AssetPropertyInterpolatedQuery,
helpURL: 'https://docs.aws.amazon.com/iot-sitewise/latest/APIReference/API_GetInterpolatedAssetPropertyValues.html',
},
{
Expand Down
1 change: 0 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ export function isAssetPropertyAggregatesQuery(q?: SitewiseQuery): q is AssetPro
*/
export interface AssetPropertyInterpolatedQuery extends SitewiseQuery {
queryType: QueryType.PropertyInterpolated;
timeOrdering?: SiteWiseTimeOrder;
}

export function isAssetPropertyInterpolatedQuery(q?: SitewiseQuery): q is AssetPropertyInterpolatedQuery {
Expand Down
Loading