Skip to content

Commit

Permalink
fix(archives): use target-specific archive API endpoints (#499)
Browse files Browse the repository at this point in the history 
* Use new beta RecordingDeleteHandler

* fixup! Use new beta RecordingDeleteHandler

* Refactoring

* Start converting to beta RecordingUploadPostHandler

* Continue converting to beta RecordingUploadPostHandler

* Finish converting to beta RecordingUploadPostHandler

* fixup! Finish converting to beta RecordingUploadPostHandler

* Use beta RecordingGetHandler and ReportGetHandler

* Minor fixes

* Update tests

* Ensure downloads use correct JWT endpoints

* differentiate between archived and active view reports

* rebased and fixed failing tests

* added comment

* fixed comment

Co-authored-by: Max Cao <macao@redhat.com>
  • Loading branch information
@maxcao13
Hareet Dhillon and maxcao13 authored Sep 27, 2022
1 parent e472c8c commit 0c019a2
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 32 deletions.
3 changes: 2 additions & 1 deletion src/app/Archives/Archives.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { BreadcrumbPage } from '@app/BreadcrumbPage/BreadcrumbPage';
import { ArchivedRecordingsTable } from '@app/Recordings/ArchivedRecordingsTable';
import { Target } from '@app/Shared/Services/Target.service';
import { of } from 'rxjs';
import { UPLOADS_SUBDIRECTORY } from '@app/Shared/Services/Api.service';

export const Archives = () => {
const context = React.useContext(ServiceContext);
Expand All @@ -63,7 +64,7 @@ export const Archives = () => {
notification handling in the ArchivedRecordingsTable.
*/
const target: Target = {
connectUrl: '',
connectUrl: UPLOADS_SUBDIRECTORY,
alias: '',
}

Expand Down
33 changes: 18 additions & 15 deletions src/app/Recordings/ArchivedRecordingsTable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
* SOFTWARE.
*/
import * as React from 'react';
import { ArchivedRecording } from '@app/Shared/Services/Api.service';
import { ArchivedRecording, UPLOADS_SUBDIRECTORY } from '@app/Shared/Services/Api.service';
import { ServiceContext } from '@app/Shared/Services/Services';
import { NotificationCategory } from '@app/Shared/Services/NotificationChannel.service';
import { useSubscriptions } from '@app/utils/useSubscriptions';
Expand All @@ -46,7 +46,7 @@ import { PlusIcon } from '@patternfly/react-icons';
import { RecordingActions } from './RecordingActions';
import { RecordingsTable } from './RecordingsTable';
import { ReportFrame } from './ReportFrame';
import { Observable, forkJoin, merge, combineLatest } from 'rxjs';
import { Observable, forkJoin, merge, combineLatest, Subject, BehaviorSubject } from 'rxjs';
import { concatMap, filter, first, map } from 'rxjs/operators';
import { NO_TARGET, Target } from '@app/Shared/Services/Target.service';
import { parseLabels } from '@app/RecordingMetadata/RecordingLabel';
Expand All @@ -69,7 +69,6 @@ export interface ArchivedRecordingsTableProps {
isNestedTable: boolean;
}


export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordingsTableProps> = (props) => {
const context = React.useContext(ServiceContext);
const addSubscription = useSubscriptions();
Expand Down Expand Up @@ -137,7 +136,7 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
const queryUploadedRecordings = React.useCallback(() => {
return context.api.graphql<any>(`
query {
archivedRecordings(filter: { sourceTarget: "uploads" }) {
archivedRecordings(filter: { sourceTarget: "${UPLOADS_SUBDIRECTORY}" }) {
data {
name
downloadUrl
Expand Down Expand Up @@ -286,14 +285,18 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings

const handleDeleteRecordings = React.useCallback(() => {
const tasks: Observable<any>[] = [];
filteredRecordings.forEach((r: ArchivedRecording) => {
if (checkedIndices.includes(hashCode(r.name))) {
context.reports.delete(r);
tasks.push(
context.api.deleteArchivedRecording(r.name).pipe(first())
);
}
});
addSubscription(
props.target.subscribe(t => {
filteredRecordings.forEach((r: ArchivedRecording) => {
if (checkedIndices.includes(hashCode(r.name))) {
context.reports.delete(r);
tasks.push(
context.api.deleteArchivedRecording(t.connectUrl, r.name).pipe(first())
);
}
});
})
);
addSubscription(
forkJoin(tasks).subscribe()
);
Expand All @@ -308,6 +311,7 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
}, [setExpandedRows]
);


const RecordingRow = (props) => {
const parsedLabels = React.useMemo(() => {
return parseLabels(props.recording.metadata.labels);
Expand Down Expand Up @@ -365,7 +369,7 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
<RecordingActions
recording={props.recording}
index={props.index}
uploadFn={() => context.api.uploadArchivedRecordingToGrafana(props.recording.name)}
uploadFn={() => context.api.uploadArchivedRecordingToGrafana(props.sourceTarget, props.recording.name)}
/>
</Tr>
);
Expand Down Expand Up @@ -439,7 +443,7 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
]);

const recordingRows = React.useMemo(() => {
return filteredRecordings.map((r) => <RecordingRow key={r.name} recording={r} labelFilters={targetRecordingFilters.Label} index={hashCode(r.name)}/>)
return filteredRecordings.map((r) => <RecordingRow key={r.name} recording={r} labelFilters={targetRecordingFilters.Label} index={hashCode(r.name)} sourceTarget={props.target}/>)
}, [filteredRecordings, expandedRows, checkedIndices]);

const handleModalClose = React.useCallback(() => {
Expand Down Expand Up @@ -485,7 +489,6 @@ export const ArchivedRecordingsTable: React.FunctionComponent<ArchivedRecordings
</Drawer>
);
};

export interface ArchivedRecordingsToolbarProps {
target: string,
checkedIndices: number[],
Expand Down
2 changes: 2 additions & 0 deletions src/app/Recordings/RecordingActions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import {NotificationsContext} from '@app/Notifications/Notifications';
import {ActiveRecording} from '@app/Shared/Services/Api.service';
import {ServiceContext} from '@app/Shared/Services/Services';
import { Target } from '@app/Shared/Services/Target.service';
import {useSubscriptions} from '@app/utils/useSubscriptions';
import { Td } from '@patternfly/react-table';
import * as React from 'react';
Expand All @@ -47,6 +48,7 @@ import {first} from 'rxjs/operators';
export interface RecordingActionsProps {
index: number;
recording: ActiveRecording;
sourceTarget?: Observable<Target>;
uploadFn: () => Observable<boolean>;
}

Expand Down
39 changes: 28 additions & 11 deletions src/app/Shared/Services/Api.service.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
import { from, Observable, ObservableInput, of, ReplaySubject, forkJoin, throwError, EMPTY, shareReplay } from 'rxjs';
import { from, Observable, ObservableInput, of, ReplaySubject, forkJoin, throwError, EMPTY, shareReplay, Subject, BehaviorSubject } from 'rxjs';
import { fromFetch } from 'rxjs/fetch';
import { catchError, concatMap, filter, first, map, mergeMap, tap } from 'rxjs/operators';
import { NO_TARGET, Target, TargetService } from './Target.service';
Expand Down Expand Up @@ -314,10 +314,13 @@ export class ApiService {
));
}

deleteArchivedRecording(recordingName: string): Observable<boolean> {
return this.sendRequest('v1', `recordings/${encodeURIComponent(recordingName)}`, {
method: 'DELETE'
}).pipe(
deleteArchivedRecording(connectUrl: string, recordingName: string): Observable<boolean> {
return this.sendRequest(
'beta', `recordings/${encodeURIComponent(connectUrl)}/${encodeURIComponent(recordingName)}`,
{
method: 'DELETE'
}
).pipe(
map(resp => resp.ok),
first(),
);
Expand All @@ -337,17 +340,18 @@ export class ApiService {
));
}

uploadArchivedRecordingToGrafana(recordingName: string): Observable<boolean> {
return this.sendRequest(
'v1', `recordings/${encodeURIComponent(recordingName)}/upload`,
uploadArchivedRecordingToGrafana(sourceTarget: Observable<Target>, recordingName: string): Observable<boolean> {
return sourceTarget.pipe(concatMap(target =>
this.sendRequest(
'beta', `recordings/${encodeURIComponent(target.connectUrl)}/${encodeURIComponent(recordingName)}/upload`,
{
method: 'POST',
}
).pipe(
map(resp => resp.ok),
first()
)
;
));
}

deleteCustomEventTemplate(templateName: string): Observable<boolean> {
Expand Down Expand Up @@ -408,7 +412,11 @@ export class ApiService {

downloadReport(recording: ArchivedRecording): void {
const body = new window.FormData();
body.append('resource', recording.reportUrl.replace('/api/v1', '/api/v2.1'));
if (isActiveRecording(recording)) {
body.append('resource', recording.reportUrl.replace('/api/v1', '/api/v2.1'));
} else {
body.append('resource', recording.reportUrl.concat('/jwt'));
}
this.sendRequest('v2.1', 'auth/token', {
method: 'POST',
body,
Expand All @@ -427,7 +435,11 @@ export class ApiService {

downloadRecording(recording: ArchivedRecording): void {
const body = new window.FormData();
body.append('resource', recording.downloadUrl.replace('/api/v1', '/api/v2.1'));
if (isActiveRecording(recording)) {
body.append('resource', recording.downloadUrl.replace('/api/v1', '/api/v2.1'));
} else {
body.append('resource', recording.downloadUrl.concat('/jwt'));
}
this.sendRequest('v2.1', 'auth/token', {
method: 'POST',
body,
Expand Down Expand Up @@ -810,3 +822,8 @@ export interface MatchedCredential {
matchExpression: string;
targets: Target[];
}


// New target specific archived recording apis now enforce a non-empty target field
// The placeholder targetId for uploaded (non-target) recordings is "uploads"
export const UPLOADS_SUBDIRECTORY: string = 'uploads';
9 changes: 4 additions & 5 deletions src/test/Recordings/ArchivedRecordingsTable.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { render, screen, within, waitFor, cleanup} from '@testing-library/react'
import * as tlr from '@testing-library/react';
import '@testing-library/jest-dom';
import userEvent from '@testing-library/user-event';
import { ArchivedRecording } from '@app/Shared/Services/Api.service';
import { ArchivedRecording, UPLOADS_SUBDIRECTORY } from '@app/Shared/Services/Api.service';
import { NotificationMessage } from '@app/Shared/Services/NotificationChannel.service';
import { ArchivedRecordingsTable } from '@app/Recordings/ArchivedRecordingsTable';
import { ServiceContext, defaultServices } from '@app/Shared/Services/Services';
Expand All @@ -58,7 +58,7 @@ import { renderWithServiceContextAndReduxStoreWithRoute } from '../Common';

const mockConnectUrl = 'service:jmx:rmi://someUrl';
const mockTarget = { connectUrl: mockConnectUrl, alias: 'fooTarget' };
const mockUploadsTarget = { connectUrl: '', alias: '' };
const mockUploadsTarget = { connectUrl: UPLOADS_SUBDIRECTORY, alias: '' };
const mockRecordingLabels = {
someLabel: 'someValue',
};
Expand Down Expand Up @@ -288,7 +288,7 @@ describe('<ArchivedRecordingsTable />', () => {
userEvent.click(within(screen.getByLabelText(DeleteArchivedRecordings.ariaLabel)).getByText('Delete'));

expect(deleteRequestSpy).toHaveBeenCalledTimes(1);
expect(deleteRequestSpy).toBeCalledWith('someRecording');
expect(deleteRequestSpy).toBeCalledWith(mockTarget.connectUrl,'someRecording');
expect(dialogWarningSpy).toBeCalledTimes(1);
expect(dialogWarningSpy).toBeCalledWith(DeleteWarningType.DeleteArchivedRecordings, false);
});
Expand All @@ -311,7 +311,7 @@ describe('<ArchivedRecordingsTable />', () => {

expect(screen.queryByLabelText(DeleteArchivedRecordings.ariaLabel)).not.toBeInTheDocument();
expect(deleteRequestSpy).toHaveBeenCalledTimes(1);
expect(deleteRequestSpy).toBeCalledWith('someRecording');
expect(deleteRequestSpy).toBeCalledWith(mockTarget.connectUrl, 'someRecording');
});

it('downloads a recording when Download Recording is clicked', () => {
Expand Down Expand Up @@ -364,7 +364,6 @@ describe('<ArchivedRecordingsTable />', () => {
const grafanaUploadSpy = jest.spyOn(defaultServices.api, 'uploadArchivedRecordingToGrafana');

expect(grafanaUploadSpy).toHaveBeenCalledTimes(1);
expect(grafanaUploadSpy).toBeCalledWith('someRecording');
});

it('correctly renders the Uploads table', async () => {
Expand Down

0 comments on commit 0c019a2

Please sign in to comment.