Skip to content

Commit

Permalink
fix(activerecordings): use notifications to update state without query (
Browse files Browse the repository at this point in the history
#369)

* correct effect dependencies

* remove unnecessary isEmpty state

* tmp

* fix(activerecordings): use notifications to update state without query

* remove unnecessary archive query updates on actions

* correct active recording stoppage category

* remove unused imports and param
  • Loading branch information
andrewazores authored Feb 10, 2022
1 parent 45eb24e commit d4adb58
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 55 deletions.
89 changes: 65 additions & 24 deletions src/app/Recordings/ActiveRecordingsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,14 @@ import { Button, Checkbox, Text, Toolbar, ToolbarContent, ToolbarItem } from '@p
import { Tbody, Tr, Td, ExpandableRowContent } from '@patternfly/react-table';
import * as React from 'react';
import { useHistory, useRouteMatch } from 'react-router-dom';
import { merge, forkJoin, Observable } from 'rxjs';
import { combineLatest, forkJoin, Observable } from 'rxjs';
import { concatMap, filter, first } from 'rxjs/operators';
import { RecordingActions } from './RecordingActions';
import { RecordingsTable } from './RecordingsTable';
import { ReportFrame } from './ReportFrame';

export interface ActiveRecordingsTableProps {
archiveEnabled: boolean;
onArchive?: Function;
}

export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTableProps> = (props) => {
Expand All @@ -65,7 +64,6 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
const [checkedIndices, setCheckedIndices] = React.useState([] as number[]);
const [expandedRows, setExpandedRows] = React.useState([] as string[]);
const [isLoading, setIsLoading] = React.useState(false);
const [isEmpty, setIsEmpty] = React.useState(true);
const [errorMessage, setErrorMessage] = React.useState('');
const { url } = useRouteMatch();

Expand Down Expand Up @@ -99,9 +97,8 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
const handleRecordings = React.useCallback((recordings) => {
setRecordings(recordings);
setIsLoading(false);
setIsEmpty(!recordings.length);
setErrorMessage('');
}, [setRecordings, setIsLoading, setIsEmpty, setErrorMessage]);
}, [setRecordings, setIsLoading, setErrorMessage]);

const handleError = React.useCallback((error) => {
setIsLoading(false);
Expand All @@ -127,15 +124,63 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
}, [addSubscription, context, context.target, refreshRecordingList]);

React.useEffect(() => {
merge(
context.notificationChannel.messages(NotificationCategory.ActiveRecordingCreated),
context.notificationChannel.messages(NotificationCategory.ActiveRecordingStopped),
context.notificationChannel.messages(NotificationCategory.ActiveRecordingSaved),
context.notificationChannel.messages(NotificationCategory.ActiveRecordingDeleted)
).subscribe(
refreshRecordingList
addSubscription(
combineLatest([
context.target.target(),
context.notificationChannel.messages(NotificationCategory.ActiveRecordingCreated),
])
.subscribe(parts => {
const currentTarget = parts[0];
const event = parts[1];
if (currentTarget.connectUrl != event.message.target) {
return;
}
setRecordings(old => old.concat([event.message.recording]));
})
);
}, [addSubscription, context, context.notificationChannel, setRecordings]);

React.useEffect(() => {
addSubscription(
combineLatest([
context.target.target(),
context.notificationChannel.messages(NotificationCategory.ActiveRecordingDeleted),
])
.subscribe(parts => {
const currentTarget = parts[0];
const event = parts[1];
if (currentTarget.connectUrl != event.message.target) {
return;
}
setRecordings(old => old.filter(r => r.name != event.message.recording.name));
})
);
}, [context, context.notificationChannel, refreshRecordingList]);
}, [addSubscription, context, context.notificationChannel, setRecordings]);

React.useEffect(() => {
addSubscription(
combineLatest([
context.target.target(),
context.notificationChannel.messages(NotificationCategory.ActiveRecordingStopped),
])
.subscribe(parts => {
const currentTarget = parts[0];
const event = parts[1];
if (currentTarget.connectUrl != event.message.target) {
return;
}
setRecordings(old => {
const updated = [...old];
for (const r of updated) {
if (r.name === event.message.recording.name) {
r.state = RecordingState.STOPPED;
}
}
return updated;
});
})
);
}, [addSubscription, context, context.notificationChannel, setRecordings]);

React.useEffect(() => {
const sub = context.target.authFailure().subscribe(() => {
Expand All @@ -155,13 +200,9 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
}
});
addSubscription(
forkJoin(tasks).subscribe(arr => {
if (props.onArchive && arr.some(v => !!v)) {
props.onArchive();
}
}, window.console.error)
forkJoin(tasks).subscribe(() => {} /* do nothing */, window.console.error)
);
}, [recordings, checkedIndices, handleRowCheck, context.api, addSubscription, props.onArchive]);
}, [recordings, checkedIndices, handleRowCheck, context.api, addSubscription]);

const handleStopRecordings = React.useCallback(() => {
const tasks: Observable<boolean>[] = [];
Expand All @@ -176,9 +217,9 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
}
});
addSubscription(
forkJoin(tasks).subscribe(refreshRecordingList, window.console.error)
forkJoin(tasks).subscribe((() => {} /* do nothing */), window.console.error)
);
}, [recordings, checkedIndices, handleRowCheck, context.api, addSubscription, refreshRecordingList]);
}, [recordings, checkedIndices, handleRowCheck, context.api, addSubscription]);

const handleDeleteRecordings = React.useCallback(() => {
const tasks: Observable<{}>[] = [];
Expand All @@ -192,9 +233,9 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
}
});
addSubscription(
forkJoin(tasks).subscribe(refreshRecordingList, window.console.error)
forkJoin(tasks).subscribe((() => {} /* do nothing */), window.console.error)
);
}, [recordings, checkedIndices, handleRowCheck, context.reports, context.api, addSubscription, refreshRecordingList]);
}, [recordings, checkedIndices, handleRowCheck, context.reports, context.api, addSubscription]);

React.useEffect(() => {
if (!context.settings.autoRefreshEnabled()) {
Expand Down Expand Up @@ -380,7 +421,7 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
tableColumns={tableColumns}
isHeaderChecked={headerChecked}
onHeaderCheck={handleHeaderCheck}
isEmpty={isEmpty}
isEmpty={!recordings.length}
isLoading ={isLoading}
errorMessage ={errorMessage}
>
Expand Down
2 changes: 1 addition & 1 deletion src/app/Recordings/ArchiveUploadModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import * as React from 'react';
import { Prompt } from 'react-router-dom';
import { ActionGroup, Button, FileUpload, Form, FormGroup, Modal, ModalVariant } from '@patternfly/react-core';
import { first, tap } from 'rxjs/operators';
import { first } from 'rxjs/operators';
import { ServiceContext } from '@app/Shared/Services/Services';
import { NotificationsContext } from '@app/Notifications/Notifications';
import { CancelUploadModal } from './CancelUploadModal';
Expand Down
17 changes: 5 additions & 12 deletions src/app/Recordings/ArchivedRecordingsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,14 @@ import { Tbody, Tr, Td, ExpandableRowContent } from '@patternfly/react-table';
import { RecordingActions } from './RecordingActions';
import { RecordingsTable } from './RecordingsTable';
import { ReportFrame } from './ReportFrame';
import { Observable, Subject, forkJoin, merge } from 'rxjs';
import { Observable, forkJoin, merge } from 'rxjs';
import { first } from 'rxjs/operators';
import { PlusIcon } from '@patternfly/react-icons';
import { ArchiveUploadModal } from './ArchiveUploadModal';

interface ArchivedRecordingsTableProps {
updater: Subject<void>;
}
export interface ArchivedRecordingsTableProps { }

export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordingsTableProps> = (props) => {
export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordingsTableProps> = () => {
const context = React.useContext(ServiceContext);

const [recordings, setRecordings] = React.useState([] as ArchivedRecording[]);
Expand Down Expand Up @@ -110,14 +108,14 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
context.notificationChannel.messages(NotificationCategory.ActiveRecordingSaved)
).subscribe(v => setRecordings(old => old.concat(v.message.recording)))
);
}, [context, context.notificationChannel, refreshRecordingList]);
}, [addSubscription, context, context.notificationChannel, setRecordings]);

React.useEffect(() => {
addSubscription(
context.notificationChannel.messages(NotificationCategory.ArchivedRecordingDeleted)
.subscribe(v => setRecordings(old => old.filter(o => o.name != v.message.recording.name)))
)
}, [context, context.notificationChannel, refreshRecordingList]);
}, [addSubscription, context, context.notificationChannel, setRecordings]);

const handleDeleteRecordings = () => {
const tasks: Observable<any>[] = [];
Expand All @@ -140,11 +138,6 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
setExpandedRows(expandedRows => idx >= 0 ? [...expandedRows.slice(0, idx), ...expandedRows.slice(idx + 1, expandedRows.length)] : [...expandedRows, id]);
};

React.useEffect(() => {
const sub = props.updater.subscribe(refreshRecordingList);
return () => sub.unsubscribe();
}, [props.updater])

React.useEffect(() => {
if (!context.settings.autoRefreshEnabled()) {
return;
Expand Down
22 changes: 8 additions & 14 deletions src/app/Recordings/Recordings.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
/*
* Copyright The Cryostat Authors
*
*
* The Universal Permissive License (UPL), Version 1.0
*
*
* Subject to the condition set forth below, permission is hereby granted to any
* person obtaining a copy of this software, associated documentation and/or data
* (collectively the "Software"), free of charge and under any and all copyright
* rights in the Software, and any and all patent rights owned or freely
* licensable by each licensor hereunder covering either (i) the unmodified
* Software as contributed to or provided by such licensor, or (ii) the Larger
* Works (as defined below), to deal in both
*
*
* (a) the Software, and
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
* one is included with the Software (each a "Larger Work" to which the Software
* is contributed by such licensors),
*
*
* without restriction, including without limitation the rights to copy, create
* derivative works of, display, perform, and distribute the Software and make,
* use, sell, offer for sale, import, export, have made, and have sold the
* Software and the Larger Work(s), and to sublicense the foregoing rights on
* either these or other terms.
*
*
* This license is subject to the following condition:
* The above copyright notice and either this complete permission notice or at
* a minimum a reference to the UPL must be included in all copies or
* substantial portions of the Software.
*
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
Expand All @@ -41,31 +41,25 @@ import { TargetView } from '@app/TargetView/TargetView';
import { Card, CardBody, CardHeader, Tab, Tabs, Text, TextVariants } from '@patternfly/react-core';
import { ActiveRecordingsTable } from './ActiveRecordingsTable';
import { ArchivedRecordingsTable } from './ArchivedRecordingsTable';
import { Subject } from 'rxjs';

export const Recordings = () => {
const context = React.useContext(ServiceContext);
const [activeTab, setActiveTab] = React.useState(0);
const [archiveEnabled, setArchiveEnabled] = React.useState(false);
const archiveUpdate = new Subject<void>();

React.useEffect(() => {
const sub = context.api.isArchiveEnabled().subscribe(setArchiveEnabled);
return () => sub.unsubscribe();
}, [context.api]);

React.useEffect(() => {
return () => archiveUpdate.complete();
}, []);

const cardBody = React.useMemo(() => {
return archiveEnabled ? (
<Tabs activeKey={activeTab} onSelect={(evt, idx) => setActiveTab(Number(idx))}>
<Tab eventKey={0} title="Active Recordings">
<ActiveRecordingsTable archiveEnabled={true} onArchive={() => archiveUpdate.next()} />
<ActiveRecordingsTable archiveEnabled={true} />
</Tab>
<Tab eventKey={1} title="Archived Recordings">
<ArchivedRecordingsTable updater={archiveUpdate} />
<ArchivedRecordingsTable />
</Tab>
</Tabs>
) : (
Expand Down
8 changes: 4 additions & 4 deletions src/app/Shared/Services/NotificationChannel.service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export enum NotificationCategory {
WsClientActivity = 'WsClientActivity',
JvmDiscovery = 'TargetJvmDiscovery',
ActiveRecordingCreated = 'ActiveRecordingCreated',
ActiveRecordingStopped = 'RecordingStopped',
ActiveRecordingStopped = 'ActiveRecordingStopped',
ActiveRecordingSaved = 'ActiveRecordingSaved',
ActiveRecordingDeleted = 'ActiveRecordingDeleted',
ArchivedRecordingCreated = 'ArchivedRecordingCreated',
Expand Down Expand Up @@ -89,14 +89,14 @@ const messageKeys = new Map([
NotificationCategory.ActiveRecordingCreated, {
variant: AlertVariant.success,
title: 'Recording Created',
body: evt => `${evt.message.recording} created in target: ${evt.message.target}`,
body: evt => `${evt.message.recording.name} created in target: ${evt.message.target}`,
} as NotificationMessageMapper
],
[
NotificationCategory.ActiveRecordingStopped, {
variant: AlertVariant.success,
title: 'Recording Stopped',
body: evt => `${evt.message.recording} was stopped`
body: evt => `${evt.message.recording.name} was stopped`
} as NotificationMessageMapper
],
[
Expand All @@ -110,7 +110,7 @@ const messageKeys = new Map([
NotificationCategory.ActiveRecordingDeleted, {
variant: AlertVariant.success,
title: 'Recording Deleted',
body: evt => `${evt.message.recording} was deleted`
body: evt => `${evt.message.recording.name} was deleted`
} as NotificationMessageMapper
],
[
Expand Down

0 comments on commit d4adb58

Please sign in to comment.