Skip to content

Commit

Permalink
feat(match-expression): match expression should be evaluated on backe…
Browse files Browse the repository at this point in the history 
…nd (#1069)

* feat(match-expression): match expression should be evaluated on backend

Signed-off-by: Thuan Vo <thvo@redhat.com>

* chore(match_expr): ensure debounce for API request to /matchExpression endpoint

* fix(match-exp): optimize backend checks

* fix(topology): fix listview quicksearch with match expr

* fix(match-exp): add back error handling

* test(rules): fix create-rule tests

* fix(credentials): fix header check and expand row behaviors

* test(credetials): fix and add more tests

* perf(topology): limit match expression check for graph view

* chore(prettier): apply prettier

* fix(utils): increase typing debounce time

* chore(utils): use default debounce time for typing

* feat(match-expr): show loading helper text when processing

* feat(match-expr): use loading state immediately on keystrokes

---------

Signed-off-by: Thuan Vo <thvo@redhat.com>
  • Loading branch information
@tthvo
tthvo authored Aug 1, 2023
1 parent 4736688 commit 002506c
Show file tree
Hide file tree
Showing 15 changed files with 562 additions and 419 deletions.
113 changes: 71 additions & 42 deletions src/app/Rules/CreateRule.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ import { SelectTemplateSelectorForm } from '@app/Shared/SelectTemplateSelectorFo
import { TemplateType } from '@app/Shared/Services/Api.service';
import { ServiceContext } from '@app/Shared/Services/Services';
import { Target } from '@app/Shared/Services/Target.service';
import { SearchExprService, SearchExprServiceContext } from '@app/Topology/Shared/utils';
import { SearchExprService, SearchExprServiceContext, useExprSvc } from '@app/Topology/Shared/utils';
import { useSubscriptions } from '@app/utils/useSubscriptions';
import { evaluateTargetWithExpr, portalRoot } from '@app/utils/utils';
import { portalRoot } from '@app/utils/utils';
import {
ActionGroup,
Button,
Expand All @@ -74,8 +74,8 @@ import { HelpIcon } from '@patternfly/react-icons';
import _ from 'lodash';
import * as React from 'react';
import { useHistory, withRouter } from 'react-router-dom';
import { forkJoin, iif, of, Subject } from 'rxjs';
import { catchError, debounceTime, map, switchMap } from 'rxjs/operators';
import { combineLatest, forkJoin, iif, of, Subject } from 'rxjs';
import { catchError, debounceTime, map, switchMap, tap } from 'rxjs/operators';
import { Rule } from './Rules';

// FIXME check if this is correct/matches backend name validation
Expand All @@ -87,9 +87,11 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
const context = React.useContext(ServiceContext);
const notifications = React.useContext(NotificationsContext);
const history = useHistory();
// Note: Do not use useSearchExpression(). This causes the cursor to jump to the end due to async updates.
const matchExprService = React.useContext(SearchExprServiceContext);
const [matchExpression, setMatchExpression] = React.useState('');
// Do not use useSearchExpression hook for display.
// This causes the cursor to jump to the end due to async updates.
const matchExprService = useExprSvc();
// Use this for displaying match expression input
const [matchExpressionInput, setMatchExpressionInput] = React.useState('');
const addSubscription = useSubscriptions();

const [name, setName] = React.useState('');
Expand All @@ -109,10 +111,10 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
const [initialDelayUnits, setInitialDelayUnits] = React.useState(1);
const [preservedArchives, setPreservedArchives] = React.useState(0);
const [loading, setLoading] = React.useState(false);
const [targets, setTargets] = React.useState<Target[]>([]);
const [evaluating, setEvaluating] = React.useState(false);
const [sampleTarget, setSampleTarget] = React.useState<Target>();

const matchedTargetsRef = React.useRef(new Subject<Target[]>());
const matchedTargets = matchedTargetsRef.current;

const handleNameChange = React.useCallback(
(name) => {
Expand Down Expand Up @@ -198,7 +200,7 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
name,
description,
enabled,
matchExpression,
matchExpression: matchExpressionInput,
eventSpecifier: eventSpecifierString,
archivalPeriodSeconds: archivalPeriod * archivalPeriodUnits,
initialDelaySeconds: initialDelay * initialDelayUnits,
Expand All @@ -224,7 +226,7 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
nameValid,
description,
enabled,
matchExpression,
matchExpressionInput,
eventSpecifierString,
archivalPeriod,
archivalPeriodUnits,
Expand All @@ -238,6 +240,7 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
]);

React.useEffect(() => {
const matchedTargets = matchedTargetsRef.current;
addSubscription(
matchedTargets
.pipe(
Expand Down Expand Up @@ -274,38 +277,58 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
.subscribe((templates) => {
setTemplates(templates);
setTemplate((old) => {
const matched = templates.find((t) => t.name === old.name && t.type === t.type);
const matched = templates.find((t) => t.name === old.name && t.type === old.type);
return matched ? { name: matched.name, type: matched.type } : {};
});
})
);
}, [addSubscription, context.api, matchedTargets]);
}, [addSubscription, context.api]);

React.useEffect(() => {
addSubscription(context.targets.targets().subscribe(setTargets));
}, [addSubscription, context.targets, setTargets]);

React.useEffect(() => {
// Set validations
let validation: ValidatedOptions = ValidatedOptions.default;
let matches: Target[] = [];
if (matchExpression !== '' && targets.length > 0) {
try {
matches = targets.filter((t) => {
const res = evaluateTargetWithExpr(t, matchExpression);
if (typeof res === 'boolean') {
return res;
}
throw new Error('The expression matching failed.');
});
validation = matches.length ? ValidatedOptions.success : ValidatedOptions.warning;
} catch (err) {
validation = ValidatedOptions.error;
}
}
setMatchExpressionValid(validation);
matchedTargets.next(matches);
}, [matchExpression, targets, matchedTargets, setMatchExpressionValid]);
const matchedTargets = matchedTargetsRef.current;
addSubscription(
combineLatest([
matchExprService.searchExpression({
immediateFn: () => {
setEvaluating(true);
setMatchExpressionValid(ValidatedOptions.default);
},
}),
context.targets.targets().pipe(tap((ts) => setSampleTarget(ts[0]))),
])
.pipe(
switchMap(([input, targets]) =>
input
? context.api.matchTargetsWithExpr(input, targets).pipe(
map((ts) => [ts, undefined]),
catchError((err) => of([[], err]))
)
: of([undefined, undefined])
)
)
.subscribe(([ts, err]) => {
setEvaluating(false);
setMatchExpressionValid(
err
? ValidatedOptions.error
: !ts
? ValidatedOptions.default
: ts.length
? ValidatedOptions.success
: ValidatedOptions.warning
);
matchedTargets.next(ts || []);
})
);
}, [
matchExprService,
context.api,
context.targets,
setSampleTarget,
setMatchExpressionValid,
setEvaluating,
addSubscription,
]);

const createButtonLoadingProps = React.useMemo(
() =>
Expand Down Expand Up @@ -379,7 +402,7 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
bodyContent={
<>
Try an expression like:
<MatchExpressionHint target={targets[0]} />
<MatchExpressionHint target={sampleTarget} />
</>
}
hasAutoWidth
Expand All @@ -398,7 +421,9 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
isRequired
fieldId="rule-matchexpr"
helperText={
matchExpressionValid === ValidatedOptions.warning
evaluating
? 'Evaluating match expression...'
: matchExpressionValid === ValidatedOptions.warning
? `Warning: Match expression matches no targets.`
: `
Enter a match expression. This is a Java-like code snippet that is evaluated against each target
Expand All @@ -409,7 +434,7 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
data-quickstart-id="rule-matchexpr"
>
<TextArea
value={matchExpression}
value={matchExpressionInput}
isDisabled={loading}
isRequired
type="text"
Expand All @@ -418,7 +443,7 @@ const CreateRuleForm: React.FC<CreateRuleFormProps> = ({ ...props }) => {
resizeOrientation="vertical"
autoResize
onChange={(value) => {
setMatchExpression(value);
setMatchExpressionInput(value);
matchExprService.setSearchExpression(value);
}}
validated={matchExpressionValid}
Expand Down Expand Up @@ -612,7 +637,11 @@ enabled in the future.`}
variant="primary"
onClick={handleSubmit}
isDisabled={
loading || nameValid !== ValidatedOptions.success || !template.name || !template.type || !matchExpression
loading ||
nameValid !== ValidatedOptions.success ||
!template.name ||
!template.type ||
!matchExpressionInput
}
data-quickstart-id="rule-create-btn"
{...createButtonLoadingProps}
Expand Down
92 changes: 58 additions & 34 deletions src/app/SecurityPanel/Credentials/CreateCredentialModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ import { MatchExpressionHint } from '@app/Shared/MatchExpression/MatchExpression
import { MatchExpressionVisualizer } from '@app/Shared/MatchExpression/MatchExpressionVisualizer';
import { ServiceContext } from '@app/Shared/Services/Services';
import { Target } from '@app/Shared/Services/Target.service';
import { SearchExprService, SearchExprServiceContext } from '@app/Topology/Shared/utils';
import { SearchExprService, SearchExprServiceContext, useExprSvc } from '@app/Topology/Shared/utils';
import { useSubscriptions } from '@app/utils/useSubscriptions';
import { evaluateTargetWithExpr, portalRoot, StreamOf } from '@app/utils/utils';
import { portalRoot, StreamOf } from '@app/utils/utils';
import {
Button,
Card,
Expand All @@ -62,7 +62,7 @@ import {
} from '@patternfly/react-core';
import { FlaskIcon, HelpIcon, TopologyIcon } from '@patternfly/react-icons';
import * as React from 'react';
import { distinctUntilChanged, interval, map } from 'rxjs';
import { catchError, combineLatest, distinctUntilChanged, interval, map, of, switchMap, tap } from 'rxjs';
import { CredentialTestTable } from './CredentialTestTable';
import { CredentialContext, TestPoolContext, TestRequest, useAuthCredential } from './utils';

Expand Down Expand Up @@ -140,53 +140,75 @@ interface AuthFormProps extends Omit<CreateCredentialModalProps, 'visible'> {
export const AuthForm: React.FC<AuthFormProps> = ({ onDismiss, onPropsSave, progressChange, ...props }) => {
const context = React.useContext(ServiceContext);
const addSubscription = useSubscriptions();
const matchExprService = React.useContext(SearchExprServiceContext);
const [matchExpression, setMatchExpression] = React.useState('');
const matchExprService = useExprSvc();
const [matchExpressionInput, setMatchExpressionInput] = React.useState('');
const [matchExpressionValid, setMatchExpressionValid] = React.useState(ValidatedOptions.default);
const [_, setCredential] = useAuthCredential(true);
const testPool = React.useContext(TestPoolContext);
const [saving, setSaving] = React.useState(false);
const [isDisabled, setIsDisabled] = React.useState(false);
const [evaluating, setEvaluating] = React.useState(false);

const [targets, setTargets] = React.useState<Target[]>([]);
const [sampleTarget, setSampleTarget] = React.useState<Target>();

const onSave = React.useCallback(
(username: string, password: string) => {
setSaving(true);
addSubscription(
context.api.postCredentials(matchExpression, username, password).subscribe((ok) => {
context.api.postCredentials(matchExpressionInput, username, password).subscribe((ok) => {
setSaving(false);
if (ok) {
onPropsSave();
}
})
);
},
[addSubscription, onPropsSave, context.api, matchExpression, setSaving]
[addSubscription, onPropsSave, context.api, matchExpressionInput, setSaving]
);

React.useEffect(() => {
addSubscription(context.targets.targets().subscribe(setTargets));
}, [addSubscription, context.targets, setTargets]);

React.useEffect(() => {
let validation: ValidatedOptions = ValidatedOptions.default;
if (matchExpression !== '' && targets.length > 0) {
try {
const atLeastOne = targets.some((t) => {
const res = evaluateTargetWithExpr(t, matchExpression);
if (typeof res === 'boolean') {
return res;
}
throw new Error('The expression matching failed.');
});
validation = atLeastOne ? ValidatedOptions.success : ValidatedOptions.warning;
} catch (err) {
validation = ValidatedOptions.error;
}
}
setMatchExpressionValid(validation);
}, [matchExpression, targets, setMatchExpressionValid]);
addSubscription(
combineLatest([
matchExprService.searchExpression({
immediateFn: (_) => {
setEvaluating(true);
setMatchExpressionValid(ValidatedOptions.default);
},
}),
context.targets.targets().pipe(tap((ts) => setSampleTarget(ts[0]))),
])
.pipe(
switchMap(([input, targets]) =>
input
? context.api.matchTargetsWithExpr(input, targets).pipe(
map((ts) => [ts, undefined]),
catchError((err) => of([[], err]))
)
: of([undefined, undefined])
)
)
.subscribe(([ts, err]) => {
setEvaluating(false);
setMatchExpressionValid(
err
? ValidatedOptions.error
: !ts
? ValidatedOptions.default
: ts.length
? ValidatedOptions.success
: ValidatedOptions.warning
);
})
);
}, [
matchExprService,
context.api,
context.targets,
setSampleTarget,
setMatchExpressionValid,
setEvaluating,
addSubscription,
]);

React.useEffect(() => {
progressChange && progressChange(saving);
Expand Down Expand Up @@ -224,7 +246,7 @@ export const AuthForm: React.FC<AuthFormProps> = ({ onDismiss, onPropsSave, prog
bodyContent={
<>
Try an expression like:
<MatchExpressionHint target={targets[0]} />
<MatchExpressionHint target={sampleTarget} />
</>
}
hasAutoWidth
Expand All @@ -242,24 +264,26 @@ export const AuthForm: React.FC<AuthFormProps> = ({ onDismiss, onPropsSave, prog
isRequired
fieldId="match-expression"
helperText={
matchExpressionValid === ValidatedOptions.warning
evaluating
? 'Evaluating match expression...'
: matchExpressionValid === ValidatedOptions.warning
? `Warning: Match expression matches no targets.`
: `
Enter a match expression. This is a Java-like code snippet that is evaluated against each target
application to determine whether the rule should be applied.`
}
helperTextInvalid="IThe expression matching failed."
helperTextInvalid="The expression matching failed."
validated={matchExpressionValid}
>
<TextArea
value={matchExpression}
value={matchExpressionInput}
isDisabled={isDisabled}
isRequired
type="text"
id="rule-matchexpr"
aria-describedby="rule-matchexpr-helper"
onChange={(v) => {
setMatchExpression(v);
setMatchExpressionInput(v);
matchExprService.setSearchExpression(v);
}}
validated={matchExpressionValid}
Expand Down
Loading

0 comments on commit 002506c

Please sign in to comment.