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

Cb 4270 row counter add cancel button for big tables #2545

Merged
merged 41 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
d4b5296
CB-4270 adds cancel option for load total count of rows in table
sergeyteleshev Apr 10, 2024
a6da7c5
CB-4270 adds ui for load total count of rows in table
sergeyteleshev Apr 11, 2024
de7c6e4
CB-4270 fix: task cancels in a true way
sergeyteleshev Apr 11, 2024
bfcfaa0
CB-4270 cleanup
sergeyteleshev Apr 11, 2024
7c424c6
CB-4270 cancel load total count on close table tab
sergeyteleshev Apr 11, 2024
110c1d8
CB-4270 cancel load total count on close sql editor tab
sergeyteleshev Apr 11, 2024
d58b80c
CB-4270 fix: cancel close only task if it is not already cancelled
sergeyteleshev Apr 11, 2024
9eea88c
CB-4270 chore: tasks -> cancelLoadTotalCountTasks
sergeyteleshev Apr 11, 2024
9ddb968
CB-4270 stops counting rows when tab closes
sergeyteleshev Apr 11, 2024
bcc43ce
CB-4270 reverts incorrect calls of cancelLoadTotalCount
sergeyteleshev Apr 11, 2024
064a737
CB-4270 reverts ToolsActions unneeded props
sergeyteleshev Apr 12, 2024
2e5f4f8
CB-4270 pr fixes
sergeyteleshev Apr 12, 2024
96f4ea0
СB-4270 cancel load total count for refresh action
sergeyteleshev Apr 12, 2024
8417093
CB-4270 fix: place cancelLoadTotalCount to DatabaseDataSource
sergeyteleshev Apr 12, 2024
13f3eae
CB-4270 fix: cancel load rows in refresh data in data source
sergeyteleshev Apr 12, 2024
62d282f
CB-4270 cancels load on close result set tab
sergeyteleshev Apr 12, 2024
10623ce
CB-4270 refactor: move cancelLoadTotalCount to resultsetdatasource
sergeyteleshev Apr 12, 2024
1fbb20c
CB-4270 cancel load total count execute only for resultsetdatasource
sergeyteleshev Apr 12, 2024
c96508e
CB-4270 adds constraint for TableFooterRowCount
sergeyteleshev Apr 12, 2024
12c3d99
CB-4270 fix: do not load new value if cancelled rows loading
sergeyteleshev Apr 12, 2024
f8bffa2
CB-4270 cleanup
sergeyteleshev Apr 12, 2024
24f2e12
Revert "CB-4270 cleanup"
sergeyteleshev Apr 12, 2024
95fa2ed
CB-4270 pr fixes
sergeyteleshev Apr 12, 2024
8acd987
Merge branch 'devel' into CB-4270-row-counter-add-cancel-button-for-b…
sergeyteleshev Apr 15, 2024
dee0295
CB-4270 pr fixes
sergeyteleshev Apr 15, 2024
708d658
CB-4270 deletes cross icon icon
sergeyteleshev Apr 15, 2024
4a3445e
CB-4270 DataSources has correct tree of calls for cancel and dispose …
sergeyteleshev Apr 15, 2024
815cd26
CB-4270 cleanup
sergeyteleshev Apr 15, 2024
8b185d5
chore: add experimental cancellable promise implementation
Wroud Apr 15, 2024
46b89da
CB-4270 pr fixes: remove unneeded disposes + close results in close r…
sergeyteleshev Apr 15, 2024
c665a13
CB-4270 removes closeResults abstraction leak in ResultSetDataSource …
sergeyteleshev Apr 16, 2024
a37d332
CB-4270 removes unused context in data sources
sergeyteleshev Apr 16, 2024
a35fa39
Merge branch 'devel' into CB-4270-row-counter-add-cancel-button-for-b…
dariamarutkina Apr 16, 2024
44c8f5a
Merge branch 'devel' into CB-4270-row-counter-add-cancel-button-for-b…
dariamarutkina Apr 16, 2024
6581fa4
Merge branch 'devel' into CB-4270-row-counter-add-cancel-button-for-b…
sergeyteleshev Apr 18, 2024
6f4482d
CB-4270 adds loader for CancelTotalCountAction on cancel task action
sergeyteleshev Apr 18, 2024
d894010
CB-4270 cancel row count job fix
yagudin10 Apr 18, 2024
f7b9328
CB-4270 cancel row counter with notification
sergeyteleshev Apr 18, 2024
ea2a1cf
CB-4270 pr fixes
sergeyteleshev Apr 19, 2024
8e7f48b
CB-4270 pr fixes
sergeyteleshev Apr 19, 2024
c37cd9f
Merge branch 'devel' into CB-4270-row-counter-add-cancel-button-for-b…
dariamarutkina Apr 22, 2024
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
10 changes: 10 additions & 0 deletions webapp/packages/plugin-data-viewer/public/icons/data_cancel.svg
Wroud marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { action, makeObservable, observable, toJS } from 'mobx';

import type { IConnectionExecutionContext } from '@cloudbeaver/core-connections';
import type { IServiceInjector } from '@cloudbeaver/core-di';
import { Task } from '@cloudbeaver/core-executor';
import { ITask, Task } from '@cloudbeaver/core-executor';
import { ResultDataFormat } from '@cloudbeaver/core-sdk';

import { DatabaseDataActions } from './DatabaseDataActions';
Expand All @@ -33,6 +33,7 @@ export abstract class DatabaseDataSource<TOptions, TResult extends IDatabaseData
error: Error | null;
executionContext: IConnectionExecutionContext | null;
outdated: boolean;
cancelLoadTotalCountTask: ITask<number> | null;
Wroud marked this conversation as resolved.
Show resolved Hide resolved

get canCancel(): boolean {
if (this.activeTask instanceof Task) {
Expand All @@ -59,6 +60,7 @@ export abstract class DatabaseDataSource<TOptions, TResult extends IDatabaseData

constructor(serviceInjector: IServiceInjector) {
this.serviceInjector = serviceInjector;
this.cancelLoadTotalCountTask = null;
this.actions = new DatabaseDataActions(this);
this.access = DatabaseDataAccessMode.Default;
this.results = [];
Expand Down Expand Up @@ -95,6 +97,7 @@ export abstract class DatabaseDataSource<TOptions, TResult extends IDatabaseData
prevOptions: observable,
options: observable,
requestInfo: observable,
cancelLoadTotalCountTask: observable.ref,
error: observable.ref,
executionContext: observable,
disabled: observable,
Expand Down Expand Up @@ -372,7 +375,8 @@ export abstract class DatabaseDataSource<TOptions, TResult extends IDatabaseData
abstract request(prevResults: TResult[]): TResult[] | Promise<TResult[]>;
abstract save(prevResults: TResult[]): Promise<TResult[]> | TResult[];

abstract dispose(): Promise<void>;
abstract cancelLoadTotalCount(): ITask<number> | null;
abstract dispose(): Promise<void> | void;
abstract loadTotalCount(resultIndex: number): Promise<void>;
Wroud marked this conversation as resolved.
Show resolved Hide resolved

async requestDataAction(): Promise<TResult[] | null> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import type { IConnectionExecutionContext } from '@cloudbeaver/core-connections';
import type { IServiceInjector } from '@cloudbeaver/core-di';
import type { ITask } from '@cloudbeaver/core-executor';
import type { ResultDataFormat } from '@cloudbeaver/core-sdk';

import type { IDatabaseDataAction, IDatabaseDataActionClass, IDatabaseDataActionInterface } from './IDatabaseDataAction';
Expand Down Expand Up @@ -47,6 +48,7 @@ export interface IDatabaseDataSource<TOptions, TResult extends IDatabaseDataResu
readonly cancelled: boolean;
readonly serviceInjector: IServiceInjector;
readonly outdated: boolean;
readonly cancelLoadTotalCountTask: ITask<number> | null;

isLoadable: () => boolean;
isReadonly: (resultIndex: number) => boolean;
Expand Down Expand Up @@ -83,6 +85,7 @@ export interface IDatabaseDataSource<TOptions, TResult extends IDatabaseDataResu
setExecutionContext: (context: IConnectionExecutionContext | null) => this;
setTotalCount: (resultIndex: number, count: number) => this;
loadTotalCount: (resultIndex: number) => Promise<void>;
cancelLoadTotalCount: () => ITask<number> | null;

retry: () => Promise<void>;
/** Allows to perform an asynchronous action on the data source, this action will wait previous action to finish and save or load requests.
Expand All @@ -94,5 +97,5 @@ export interface IDatabaseDataSource<TOptions, TResult extends IDatabaseDataResu
cancel: () => Promise<void> | void;
clearError: () => this;
resetData: () => this;
dispose: () => Promise<void>;
dispose: () => Promise<void> | void;
Wroud marked this conversation as resolved.
Show resolved Hide resolved
}
40 changes: 34 additions & 6 deletions webapp/packages/plugin-data-viewer/src/ResultSetDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* you may not use this file except in compliance with the License.
*/
import type { IServiceInjector } from '@cloudbeaver/core-di';
import type { ITask } from '@cloudbeaver/core-executor';
import type { AsyncTaskInfoService, GraphQLService } from '@cloudbeaver/core-sdk';

import { DatabaseDataSource } from './DatabaseDataModel/DatabaseDataSource';
Expand All @@ -20,6 +21,24 @@ export abstract class ResultSetDataSource<TOptions> extends DatabaseDataSource<T
super(serviceInjector);
}

dispose(): void {
this.cancelLoadTotalCount();
}
Wroud marked this conversation as resolved.
Show resolved Hide resolved

async refreshData(): Promise<void> {
this.cancelLoadTotalCount();

await super.refreshData();
}
Wroud marked this conversation as resolved.
Show resolved Hide resolved

cancelLoadTotalCount(): ITask<number> | null {
if (!this.cancelLoadTotalCountTask?.cancelled) {
this.cancelLoadTotalCountTask?.cancel();
}
Wroud marked this conversation as resolved.
Show resolved Hide resolved

return this.cancelLoadTotalCountTask;
}

async loadTotalCount(resultIndex: number) {
const executionContext = this.executionContext;
const executionContextInfo = this.executionContext?.context;
Expand All @@ -34,7 +53,7 @@ export abstract class ResultSetDataSource<TOptions> extends DatabaseDataSource<T
throw new Error('Result id must be provided');
}

const task = this.asyncTaskInfoService.create(async () => {
const asyncTask = this.asyncTaskInfoService.create(async () => {
const { taskInfo } = await this.graphQLService.sdk.asyncSqlRowDataCount({
resultsId: result.id!,
connectionId: executionContextInfo.connectionId,
Expand All @@ -45,16 +64,25 @@ export abstract class ResultSetDataSource<TOptions> extends DatabaseDataSource<T
return taskInfo;
});

const count = await executionContext.run(
const task = executionContext.run(
async () => {
const info = await this.asyncTaskInfoService.run(task);
const info = await this.asyncTaskInfoService.run(asyncTask);
const { count } = await this.graphQLService.sdk.getSqlRowDataCountResult({ taskId: info.id });

return count;
},
() => this.asyncTaskInfoService.cancel(task.id),
() => this.asyncTaskInfoService.remove(task.id),
() => this.asyncTaskInfoService.cancel(asyncTask.id),
() => this.asyncTaskInfoService.remove(asyncTask.id),
);

this.setTotalCount(resultIndex, count);
this.cancelLoadTotalCountTask = task;

try {
const count = await task;

this.setTotalCount(resultIndex, count);
} finally {
this.cancelLoadTotalCountTask = null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Form, getComputed, ToolsPanel } from '@cloudbeaver/core-blocks';
import type { IDataContext } from '@cloudbeaver/core-data-context';
import { useService } from '@cloudbeaver/core-di';

import { ResultSetConstraintAction } from '../../DatabaseDataModel/Actions/ResultSet/ResultSetConstraintAction';
import type { IDatabaseDataModel } from '../../DatabaseDataModel/IDatabaseDataModel';
import { DataViewerSettingsService } from '../../DataViewerSettingsService';
import { AutoRefreshButton } from './AutoRefresh/AutoRefreshButton';
Expand Down Expand Up @@ -94,6 +95,7 @@ export const TableFooter = observer<Props>(function TableFooter({ resultIndex, m
}, [model.countGain]);

const disabled = getComputed(() => model.isLoading() || model.isDisabled(resultIndex));
const constraint = model.getResult(resultIndex) ? model.source.getAction(resultIndex, ResultSetConstraintAction) : null;

return styled(tableFooterStyles)(
<ToolsPanel type="secondary">
Expand All @@ -116,7 +118,7 @@ export const TableFooter = observer<Props>(function TableFooter({ resultIndex, m
/>
</Form>
</count>
<TableFooterRowCount model={model} resultIndex={resultIndex} />
{constraint?.supported && <TableFooterRowCount model={model} resultIndex={resultIndex} />}
<TableFooterMenu model={model} resultIndex={resultIndex} simple={simple} context={context} />
{model.source.requestInfo.requestMessage.length > 0 && (
<time>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,9 @@ interface Props {
}

export const TableFooterRowCount: React.FC<Props> = observer(function TableFooterRowCount({ resultIndex, model }) {
const translate = useTranslate();
const notificationService = useService(NotificationService);
const [loading, setLoading] = useState(false);

const result = model.getResult(resultIndex);

if (!result) {
return null;
}

async function loadTotalCount() {
try {
setLoading(true);
Expand All @@ -45,13 +38,63 @@ export const TableFooterRowCount: React.FC<Props> = observer(function TableFoote
}
}

function cancelTotalCount() {
try {
setLoading(false);
model.source.cancelLoadTotalCount();
} catch (e: any) {
const cancelled = model.source.cancelLoadTotalCountTask?.cancelled;

if (!cancelled) {
notificationService.logException(e, 'data_viewer_total_count_cancel_failed_title', typeof e === 'string' ? e : undefined);
Copy link
Member

Choose a reason for hiding this comment

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

why do we need typeof e === 'string' ? e : undefined here?

}
}
}

if (loading) {
return <CancelTotalCountAction onClick={cancelTotalCount} />;
}

return <TotalCountAction loading={loading} resultIndex={resultIndex} model={model} onClick={loadTotalCount} />;
});

const CancelTotalCountAction = observer(function CancelTotalCountAction({ onClick }: { onClick: VoidFunction }) {
Copy link
Member

Choose a reason for hiding this comment

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

Please do not declare multiple components in one file

const translate = useTranslate();

return styled(tableFooterMenuStyles)(
<div className={classes.wrapper} title={translate('ui_processing_cancel')}>
<ToolsAction icon="/icons/data_cancel.svg" viewBox="0 0 32 32" onClick={onClick}>
{translate('ui_processing_cancel')}
</ToolsAction>
</div>,
);
});

const TotalCountAction = observer(function TotalCountAction({
onClick,
loading,
resultIndex,
model,
}: {
onClick: VoidFunction;
loading: boolean;
resultIndex: number;
model: IDatabaseDataModel<any, IDatabaseResultSet>;
}) {
const result = model.getResult(resultIndex);
const translate = useTranslate();
const disabled = getComputed(() => model.isLoading() || model.isDisabled(resultIndex));

if (!result) {
return null;
}

const currentCount = result.loadedFully ? result.count : `${result.count}+`;
const count = result.totalCount ?? currentCount;
const disabled = getComputed(() => model.isLoading() || model.isDisabled(resultIndex));

return styled(tableFooterMenuStyles)(
<div className={classes.wrapper} title={translate('data_viewer_total_count_tooltip')}>
<ToolsAction disabled={disabled} loading={loading} icon="/icons/data_row_count.svg" viewBox="0 0 32 32" onClick={loadTotalCount}>
<ToolsAction disabled={disabled} loading={loading} icon="/icons/data_row_count.svg" viewBox="0 0 32 32" onClick={onClick}>
<span>{count}</span>
</ToolsAction>
</div>,
Expand Down
8 changes: 8 additions & 0 deletions webapp/packages/plugin-data-viewer/src/locales/en.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
/*
* CloudBeaver - Cloud Database Manager
* Copyright (C) 2020-2024 DBeaver Corp and others
*
* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
*/
export default [
['plugin_data_viewer_data_viewer_settings_group', 'Data Viewer'],
['table_header_sql_expression', 'Enter a SQL expression to filter results, e.g. column_name=10'],
Expand Down Expand Up @@ -46,6 +53,7 @@ export default [
['data_viewer_refresh_result_set', 'Refresh result set'],
['data_viewer_total_count_tooltip', 'Get total count'],
['data_viewer_total_count_failed', 'Failed to get total count'],
['data_viewer_total_count_cancel_failed_title', 'Failed to cancel getting of total count'],
['data_viewer_model_not_loaded', 'Table model is not loaded'],
['settings_data_editor', 'Data Editor'],
['settings_data_editor_disable_edit_name', 'Disable Edit'],
Expand Down
8 changes: 8 additions & 0 deletions webapp/packages/plugin-data-viewer/src/locales/it.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
/*
* CloudBeaver - Cloud Database Manager
* Copyright (C) 2020-2024 DBeaver Corp and others
*
* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
*/
export default [
['plugin_data_viewer_data_viewer_settings_group', 'Data Viewer'],
['table_header_sql_expression', 'Digita una espresione SQL per filtrare i risultati'],
Expand Down Expand Up @@ -39,6 +46,7 @@ export default [
['data_viewer_refresh_result_set', 'Refresh result set'],
['data_viewer_total_count_tooltip', 'Get total count'],
['data_viewer_total_count_failed', 'Failed to get total count'],
['data_viewer_total_count_cancel_failed_title', 'Failed to cancel getting of total count'],
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that we should create new message token as "Task was cancelled" is throw by the task itself

['data_viewer_model_not_loaded', 'Table model is not loaded'],
['settings_data_editor', 'Data Editor'],
['settings_data_editor_disable_edit_name', 'Disable Edit'],
Expand Down
8 changes: 8 additions & 0 deletions webapp/packages/plugin-data-viewer/src/locales/ru.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
/*
* CloudBeaver - Cloud Database Manager
* Copyright (C) 2020-2024 DBeaver Corp and others
*
* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
*/
export default [
['plugin_data_viewer_data_viewer_settings_group', 'Просмотр данных'],
['table_header_sql_expression', 'Введите SQL выражение чтобы отфильтровать результаты, например, column_name=10'],
Expand Down Expand Up @@ -40,6 +47,7 @@ export default [
['data_viewer_refresh_result_set', 'Обновить резалт сет'],
['data_viewer_total_count_tooltip', 'Получить количество записей'],
['data_viewer_total_count_failed', 'Не удалось получить количество записей'],
['data_viewer_total_count_cancel_failed_title', 'Не удалось отменить получение количества записей'],
['data_viewer_model_not_loaded', 'Не удалось загрузить модель таблицы'],
['settings_data_editor', 'Редактор данных'],
['settings_data_editor_disable_edit_name', 'Отключить редактирование'],
Expand Down
8 changes: 8 additions & 0 deletions webapp/packages/plugin-data-viewer/src/locales/zh.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
/*
* CloudBeaver - Cloud Database Manager
* Copyright (C) 2020-2024 DBeaver Corp and others
*
* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
*/
export default [
['plugin_data_viewer_data_viewer_settings_group', 'Data Viewer'],
['table_header_sql_expression', '输入SQL表达式以过滤结果'],
Expand Down Expand Up @@ -46,6 +53,7 @@ export default [
['data_viewer_refresh_result_set', 'Refresh result set'],
['data_viewer_total_count_failed', 'Failed to get total count'],
['data_viewer_total_count_tooltip', 'Get total count'],
['data_viewer_total_count_cancel_failed_title', 'Failed to cancel getting of total count'],
['data_viewer_model_not_loaded', 'Table model is not loaded'],
['settings_data_editor', 'Data Editor'],
['settings_data_editor_disable_edit_name', 'Disable Edit'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ export class SqlQueryResultService {

// TODO: we need to dispose table model, but don't close execution context, so now we only
const model = this.tableViewerStorageService.get(group.modelId);
model?.source.cancelLoadTotalCount();
Copy link
Contributor Author

@sergeyteleshev sergeyteleshev Apr 12, 2024

Choose a reason for hiding this comment

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

since there is no dispose, we need to cancel it in result set tab manually on close

Copy link
Member

Choose a reason for hiding this comment

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

then it's a bug cuz we need to call dispose for each unused table model
but in SQL editor we have special behavior where we can reuse the same model for different queries so check it please

Copy link
Member

Choose a reason for hiding this comment

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

you can move cancelLoadTotalCount call to cancel fn

// model?.dispose();

if (model?.isLoading()) {
Expand Down
Loading