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

Add an unified storage class for data plugin #7701

Merged
merged 7 commits into from
Aug 19, 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
1 change: 1 addition & 0 deletions src/plugins/data/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export * from './query';
export * from './search';
export * from './types';
export * from './utils';
export * from './storage';

/**
* Use data plugin interface instead
Expand Down
6 changes: 6 additions & 0 deletions src/plugins/data/common/storage/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export { DataStorage, createStorage } from './storage';
68 changes: 68 additions & 0 deletions src/plugins/data/common/storage/storage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
abbyhu2000 marked this conversation as resolved.
Show resolved Hide resolved
*/

import { transform, startsWith, keys } from 'lodash';
import { parse, stringify } from '@osd/std';

export enum StorageKeys {
WIDTH = 'widths',
}

type IStorageEngine = typeof window.localStorage;
export class DataStorage {
Copy link
Member

Choose a reason for hiding this comment

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

i do like this structure. it makes it easier to not use opensearchDashboards. and just pass the key

constructor(private readonly engine: IStorageEngine, private readonly prefix: string) {}

encode(val: any) {
return stringify(val);
}

decode(val: any) {
if (typeof val === 'string') {
return parse(val);
}
}

encodeKey(key: string) {
return `${this.prefix}${key}`;
}

decodeKey(key: string) {
if (startsWith(key, this.prefix)) {
return `${key.slice(this.prefix.length)}`;
}
}

set(key: string, val: any) {
this.engine.setItem(this.encodeKey(key), this.encode(val));
return val;
}

has(key: string) {
return this.engine.getItem(this.encodeKey(key)) != null;
}

get<T>(key: string, _default?: T) {
if (this.has(key)) {
return this.decode(this.engine.getItem(this.encodeKey(key)));
} else {
return _default;
}
}

remove(key: string) {
return this.engine.removeItem(this.encodeKey(key));
}

keys(): string[] {
return transform(keys(this.engine), (ours, key) => {
const ourKey = this.decodeKey(key);
if (ourKey != null) ours.push(ourKey);
});
}
}

export function createStorage(deps: { engine: IStorageEngine; prefix: string }) {
return new DataStorage(deps.engine, deps.prefix);
}
11 changes: 4 additions & 7 deletions src/plugins/data/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,7 @@ import './index.scss';

import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public';
import { ConfigSchema } from '../config';
import {
Storage,
IStorageWrapper,
createStartServicesGetter,
} from '../../opensearch_dashboards_utils/public';
import { createStartServicesGetter } from '../../opensearch_dashboards_utils/public';
import {
DataPublicPluginSetup,
DataPublicPluginStart,
Expand Down Expand Up @@ -95,6 +91,7 @@ import { DefaultDslDataSource } from './data_sources/default_datasource';
import { DEFAULT_DATA_SOURCE_TYPE } from './data_sources/constants';
import { getSuggestions as getSQLSuggestions } from './antlr/opensearch_sql/code_completion';
import { getSuggestions as getDQLSuggestions } from './antlr/dql/code_completion';
import { createStorage, DataStorage } from '../common';

declare module '../../ui_actions/public' {
export interface ActionContextMapping {
Expand All @@ -117,15 +114,15 @@ export class DataPublicPlugin
private readonly uiService: UiService;
private readonly fieldFormatsService: FieldFormatsService;
private readonly queryService: QueryService;
private readonly storage: IStorageWrapper;
private readonly storage: DataStorage;

constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
this.searchService = new SearchService(initializerContext);
this.uiService = new UiService(initializerContext);
this.queryService = new QueryService();
this.fieldFormatsService = new FieldFormatsService();
this.autocomplete = new AutocompleteService(initializerContext);
this.storage = new Storage(window.localStorage);
this.storage = createStorage({ engine: window.localStorage, prefix: 'opensearch_dashboards.' });
}

public setup(
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/data/public/query/lib/add_to_query_log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@
*/

import { IUiSettingsClient } from 'src/core/public';
import { IStorageWrapper } from 'src/plugins/opensearch_dashboards_utils/public';
import { Query } from '../../../common';
import { DataStorage, Query } from '../../../common';
import { getQueryLog } from './get_query_log';

interface AddToQueryLogDependencies {
uiSettings: IUiSettingsClient;
storage: IStorageWrapper;
storage: DataStorage;
}

export function createAddToQueryLog({ storage, uiSettings }: AddToQueryLogDependencies) {
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/data/public/query/lib/get_query_log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@
*/

import { IUiSettingsClient } from 'src/core/public';
import { IStorageWrapper } from 'src/plugins/opensearch_dashboards_utils/public';
import { PersistedLog } from '../persisted_log';
import { UI_SETTINGS } from '../../../common';
import { DataStorage, UI_SETTINGS } from '../../../common';

/** @internal */
export function getQueryLog(
uiSettings: IUiSettingsClient,
storage: IStorageWrapper,
storage: DataStorage,
appName: string,
language: string
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import _ from 'lodash';
import * as Rx from 'rxjs';
import { map } from 'rxjs/operators';
import { IStorageWrapper } from 'src/plugins/opensearch_dashboards_utils/public';
import { DataStorage } from 'src/plugins/data/common';

const defaultIsDuplicate = (oldItem: any, newItem: any) => {
return _.isEqual(oldItem, newItem);
Expand All @@ -48,12 +48,12 @@ export class PersistedLog<T = any> {
public maxLength?: number;
public filterDuplicates?: boolean;
public isDuplicate: (oldItem: T, newItem: T) => boolean;
public storage: IStorageWrapper;
public storage: DataStorage;
public items: T[];

private update$ = new Rx.BehaviorSubject(undefined);

constructor(name: string, options: PersistedLogOptions<T> = {}, storage: IStorageWrapper) {
constructor(name: string, options: PersistedLogOptions<T> = {}, storage: DataStorage) {
this.name = name;
this.maxLength =
typeof options.maxLength === 'string'
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/data/public/query/query_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,19 @@

import { share } from 'rxjs/operators';
import { IUiSettingsClient, SavedObjectsClientContract } from 'src/core/public';
import { IStorageWrapper } from 'src/plugins/opensearch_dashboards_utils/public';
import { FilterManager } from './filter_manager';
import { createAddToQueryLog } from './lib';
import { TimefilterService, TimefilterSetup } from './timefilter';
import { createSavedQueryService } from './saved_query/saved_query_service';
import { createQueryStateObservable } from './state_sync/create_global_query_observable';
import { QueryStringManager, QueryStringContract } from './query_string';
import { DataSetContract, DataSetManager } from './dataset_manager';
import { buildOpenSearchQuery, getOpenSearchQueryConfig, IndexPatternsService } from '../../common';
import {
buildOpenSearchQuery,
DataStorage,
getOpenSearchQueryConfig,
IndexPatternsService,
} from '../../common';
import { getUiSettings } from '../services';
import { IndexPattern } from '..';

Expand All @@ -48,13 +52,13 @@ import { IndexPattern } from '..';
*/

interface QueryServiceSetupDependencies {
storage: IStorageWrapper;
storage: DataStorage;
uiSettings: IUiSettingsClient;
}

interface QueryServiceStartDependencies {
savedObjectsClient: SavedObjectsClientContract;
storage: IStorageWrapper;
storage: DataStorage;
uiSettings: IUiSettingsClient;
indexPatterns: IndexPatternsService;
}
Expand Down
64 changes: 64 additions & 0 deletions src/plugins/data/public/query/query_string/query_history.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { DataStorage, SimpleDataSet } from 'src/plugins/data/common';
import { BehaviorSubject } from 'rxjs';
import { Query, TimeRange } from '../..';

// Todo: Implement a more advanced QueryHistory class when needed for recent query history
Copy link
Member

Choose a reason for hiding this comment

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

seems pretty advanced hah nice. and only has functionality for recents. or do you mean configurable

export class QueryHistory {
constructor(private readonly storage: DataStorage) {}

private changeEmitter = new BehaviorSubject<any[]>(this.getHistory() || []);

getHistoryKeys() {
return this.storage
.keys()
.filter((key: string) => key.indexOf('query_') === 0)
Copy link
Member

Choose a reason for hiding this comment

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

could you explain the decision to go with query_ it makes sense to me but do we have to make it more specific to avoid potential collisions? did we look into the decision of the name of the key ssense

Copy link
Member Author

Choose a reason for hiding this comment

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

yea not sure about the meaning of "ssense" , but i think "_query" makes sense for saving recent query here? They full key is opensearch_dashboards.query_1723753938175, the numbers followed are the timetamp, i think it will avoid collisions?

.sort()
.reverse();
}

getHistory() {
return this.getHistoryKeys().map((key) => this.storage.get(key));
}

// This is used as an optimization mechanism so that different components
// can listen for changes to history and update because changes to history can
// be triggered from different places in the app. The alternative would be to store
// this in state so that we hook into the React model, but it would require loading history
// every time the application starts even if a user is not going to view history.
change(listener: (reqs: any[]) => void) {
const subscription = this.changeEmitter.subscribe(listener);
return () => subscription.unsubscribe();
}

addQueryToHistory(dataSet: SimpleDataSet, query: Query, dateRange?: TimeRange) {
const keys = this.getHistoryKeys();
keys.splice(0, 500); // only maintain most recent X;
keys.forEach((key) => {
this.storage.remove(key);
});

const timestamp = new Date().getTime();
const k = 'query_' + timestamp;
this.storage.set(k, {
dataSet,
time: timestamp,
query,
dateRange,
Copy link
Member

Choose a reason for hiding this comment

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

here is the current saved query saved object.

I think we should follow the same format and property names as the saved query.

you have query, but i would rename dataRange to timeFilter and i would include filters as well.

time could probaby could be submitted_at

should we consider workspaces @ashwin-pc ?

Screenshot 2024-08-15 at 3 50 14 AM

Copy link
Member Author

Choose a reason for hiding this comment

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

so inside the onQueryBarSubmit function, we only get time range as props. But time filter includes both time range and refresh interval. So do you think we need to include refresh interval and the filters here?

public onQueryBarSubmit = (queryAndDateRange: { dateRange?: TimeRange; query?: Query }) => {

});

this.changeEmitter.next(this.getHistory());
}

clearHistory() {
this.getHistoryKeys().forEach((key) => this.storage.remove(key));
}
}

export function createHistory(deps: { storage: DataStorage }) {
return new QueryHistory(deps.storage);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,16 @@
*/

import { QueryStringManager } from './query_string_manager';
import { Storage } from '../../../../opensearch_dashboards_utils/public/storage';
import { StubBrowserStorage } from 'test_utils/stub_browser_storage';
import { coreMock } from '../../../../../core/public/mocks';
import { Query } from '../../../common/query';
import { DataStorage } from '../../../../data/common';

describe('QueryStringManager', () => {
let service: QueryStringManager;

beforeEach(() => {
service = new QueryStringManager(
new Storage(new StubBrowserStorage()),
new DataStorage(window.localStorage, 'opensearch_dashboards.'),
coreMock.createSetup().uiSettings
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,28 @@
import { BehaviorSubject } from 'rxjs';
import { skip } from 'rxjs/operators';
import { CoreStart } from 'opensearch-dashboards/public';
import { IStorageWrapper } from 'src/plugins/opensearch_dashboards_utils/public';
import { Query, UI_SETTINGS } from '../../../common';
import { DataStorage, Query, SimpleDataSet, TimeRange, UI_SETTINGS } from '../../../common';
import { createHistory, QueryHistory } from './query_history';

export class QueryStringManager {
private query$: BehaviorSubject<Query>;
private queryHistory: QueryHistory;

constructor(
private readonly storage: IStorageWrapper,
private readonly storage: DataStorage,
private readonly uiSettings: CoreStart['uiSettings']
) {
this.query$ = new BehaviorSubject<Query>(this.getDefaultQuery());
this.queryHistory = createHistory({ storage });
}

private getDefaultQueryString() {
return this.storage.get('opensearchDashboards.userQueryString') || '';
return this.storage.get('userQueryString') || '';
}

private getDefaultLanguage() {
return (
this.storage.get('opensearchDashboards.userQueryLanguage') ||
this.storage.get('userQueryLanguage') ||
this.uiSettings.get(UI_SETTINGS.SEARCH_QUERY_LANGUAGE)
);
}
Expand Down Expand Up @@ -100,6 +102,25 @@ export class QueryStringManager {
public clearQuery = () => {
this.setQuery(this.getDefaultQuery());
};

// Todo: update this function to use the Query object when it is udpated, Query object should include time range and dataset
public addToQueryHistory(dataSet: SimpleDataSet, query: Query, timeRange?: TimeRange) {
if (query.query) {
this.queryHistory.addQueryToHistory(dataSet, query, timeRange);
}
}

public getQueryHistory() {
return this.queryHistory.getHistory();
}

public clearQueryHistory() {
this.queryHistory.clearHistory();
}

public changeQueryHistory(listener: (reqs: any[]) => void) {
return this.queryHistory.change(listener);
}
}

export type QueryStringContract = PublicMethodsOf<QueryStringManager>;
Loading
Loading