Skip to content

Commit

Permalink
fix(targets): select TargetSelect to none when TargetJvmNotification …
Browse files Browse the repository at this point in the history
…LOST if that target is currently selected (#496)

* select target to none when notification LOST

* fixed tests

* fixed callback dependencies:

* fixed unclosed subscription
  • Loading branch information
maxcao13 authored Aug 6, 2022
1 parent ac3ce47 commit d896d86
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/app/Shared/MatchExpressionEvaluator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const MatchExpressionEvaluator: React.FunctionComponent<MatchExpressionEv
addSubscription(
context.target.target().subscribe(setTarget)
);
}, [context, context.target]);
}, [addSubscription, context, context.target]);

React.useEffect(() => {
if (!props.matchExpression || !target?.connectUrl) {
Expand Down
34 changes: 28 additions & 6 deletions src/app/TargetSelect/TargetSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { catchError, first } from 'rxjs/operators';

import { CreateTargetModal } from './CreateTargetModal';
import _ from 'lodash';
import { NotificationCategory } from '@app/Shared/Services/NotificationChannel.service';

export interface TargetSelectProps {
}
Expand All @@ -63,13 +64,34 @@ export const TargetSelect: React.FunctionComponent<TargetSelectProps> = (props)
const addSubscription = useSubscriptions();

React.useEffect(() => {
const sub = context.targets.targets().subscribe((targets) => {
setTargets(targets);
selectTargetFromCache(targets);
});
return () => sub.unsubscribe();
addSubscription(
context.targets.targets().subscribe((targets) => {
setTargets(targets);
selectTargetFromCache(targets);
})
);
}, [context, context.targets, setTargets]);

React.useEffect(() => {
addSubscription(
context.notificationChannel.messages(NotificationCategory.TargetJvmDiscovery)
.subscribe(v => {
const evt: TargetDiscoveryEvent = v.message.event;
if (evt.kind === 'LOST') {
const target: Target = {
connectUrl: evt.serviceRef.connectUrl,
alias: evt.serviceRef.alias,
}
context.target.target().pipe(first()).subscribe((currentTarget) => {
if (currentTarget.connectUrl === target.connectUrl && currentTarget.alias === target.alias) {
selectNone();
}
})
}
})
);
}, [addSubscription, context, context.notificationChannel, context.target]);

const refreshTargetList = React.useCallback(() => {
setLoading(true);
addSubscription(
Expand Down Expand Up @@ -126,7 +148,7 @@ export const TargetSelect: React.FunctionComponent<TargetSelectProps> = (props)
}
}
setExpanded(false);
}, [context, selected, notifications, setExpanded]);
}, [context, context.target, selected, notifications, setExpanded]);

const selectNone = React.useCallback(() => {
onSelect(undefined, undefined, true);
Expand Down
14 changes: 14 additions & 0 deletions src/test/Rules/CreateRule.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { ServiceContext, defaultServices } from '@app/Shared/Services/Services';
import { CreateRule } from '@app/Rules/CreateRule';
import { EventTemplate } from '@app/CreateRecording/CreateRecording';
import { Target } from '@app/Shared/Services/Target.service';
import { NotificationMessage } from '@app/Shared/Services/NotificationChannel.service';

const escapeKeyboardInput = (value: string) => {
return value.replace(/[{[]/g, '$&$&');
Expand Down Expand Up @@ -82,6 +83,18 @@ const mockRule: Rule = {
maxSizeBytes: 0
};

const mockNewTarget: Target = {
connectUrl: 'service:jmx:rmi://someUrl1',
alias: 'someAlias',
}

const mockTargetFoundNotification = {
message: {
event: { kind: 'FOUND', serviceRef: mockNewTarget }
}
} as NotificationMessage;


const history = createMemoryHistory();

jest.mock('react-router-dom', () => ({
Expand All @@ -91,6 +104,7 @@ jest.mock('react-router-dom', () => ({
}));

const createSpy = jest.spyOn(defaultServices.api, 'createRule').mockReturnValue(of(true));
jest.spyOn(defaultServices.notificationChannel, 'messages').mockReturnValue(of(mockTargetFoundNotification));
jest.spyOn(defaultServices.api, 'doGet').mockReturnValue(of([mockEventTemplate]));
jest.spyOn(defaultServices.target, 'target').mockReturnValue(of(mockTarget));
jest.spyOn(defaultServices.targets, 'targets').mockReturnValue(of([mockTarget]));
Expand Down

0 comments on commit d896d86

Please sign in to comment.