Skip to content

Commit

Permalink
Enhance graph find/hide expression power (kiali#1903)
Browse files Browse the repository at this point in the history
* Support more powerful graph find/hide expressions. Make better use of disjuntive selectors
to enhance handling of OR/AND.

kiali#3165
  • Loading branch information
jshaughn committed Sep 2, 2020
1 parent 568dec0 commit 2da96a4
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 36 deletions.
8 changes: 4 additions & 4 deletions src/pages/Graph/GraphHelpFind.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ export default class GraphHelpFind extends React.Component<GraphHelpFindProps> {
['!traffic', 'edges with no traffic'],
['http > 0.5', `edges with http rate > 0.5 rps`],
['rt > 500', `edges with response time > 500ms. (requires response time edge labels)`],
['%httptraffic >= 50.0', `edges with >= 50% of the outgoing http request traffic of the parent`]
['%httptraffic >= 50.0', `edges with >= 50% of the outgoing http request traffic of the parent`],
['node = svc and svc startswith det or !traffic', 'service node starting with "det" or edges with no traffic']
];
};

Expand Down Expand Up @@ -209,12 +210,11 @@ export default class GraphHelpFind extends React.Component<GraphHelpFindProps> {
};
private noteRows = (): string[][] => {
return [
['Expressions can not combine "AND" with "OR".'],
['Parentheses are not supported (or needed).'],
['The "name" operand expands internally to an "OR" expression (an "AND" when negated).'],
['Expressions can not combine node and edge criteria.'],
['Use OR to combine node and edge criteria.'],
['Use "<operand> = NaN" to test for no activity. Use "!= NaN" for any activity. (e.g. httpout = NaN)'],
[`Unary operands may optionally be prefixed with "is" or "has". (i.e. "has mtls")`],
['The "name" operand expands internally to an "OR" expression (an "AND" when negated).'],
['Abbrevations: namespace|ns, service|svc, workload|wl operation|op'],
['Abbrevations: circuitbreaker|cb, responsetime|rt, serviceentry->se, sidecar|sc, virtualservice|vs'],
['Hiding nodes will automatically hide connected edges.'],
Expand Down
61 changes: 33 additions & 28 deletions src/pages/Graph/GraphToolbar/GraphFind.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ export class GraphFind extends React.Component<GraphFindProps, GraphFindState> {
layoutChanged: boolean
) => {
const selector = this.parseValue(this.props.hideValue, false);
console.debug(`Hide selector=[${selector}]`);
let prevRemoved = this.removedElements;

cy.startBatch();
Expand Down Expand Up @@ -455,6 +456,7 @@ export class GraphFind extends React.Component<GraphFindProps, GraphFindState> {

private handleFind = (cy: any) => {
const selector = this.parseValue(this.props.findValue, true);
console.debug(`Find selector=[${selector}]`);

cy.startBatch();
// unhighlight old find-hits
Expand Down Expand Up @@ -483,30 +485,32 @@ export class GraphFind extends React.Component<GraphFindProps, GraphFindState> {
return undefined;
}

preparedVal = preparedVal.replace(/ and /gi, ' AND ');
preparedVal = preparedVal.replace(/ or /gi, ' OR ');
const conjunctive = preparedVal.includes(' AND ');
const disjunctive = preparedVal.includes(' OR ');
if (conjunctive && disjunctive) {
return this.setError(`Expression can not contain both 'AND' and 'OR'`, isFind);
}
const separator = disjunctive ? ',' : '';
const expressions = disjunctive ? preparedVal.split(' OR ') : preparedVal.split(' AND ');
let selector;
// generate separate selectors for each disjunctive clause and then stitch them together. This
// lets us mix node and edge criteria.
const orClauses = preparedVal.split(' OR ');
let orSelector;

for (const expression of expressions) {
const parsedExpression = this.parseExpression(expression, conjunctive, disjunctive, isFind);
if (!parsedExpression) {
return undefined;
}
selector = this.appendSelector(selector, parsedExpression, separator, isFind);
if (!selector) {
return undefined;
for (const clause of orClauses) {
const expressions = clause.split(' AND ');
const conjunctive = expressions.length > 1;
let selector;

for (const expression of expressions) {
const parsedExpression = this.parseExpression(expression, conjunctive, isFind);
if (!parsedExpression) {
return undefined;
}
selector = this.appendSelector(selector, parsedExpression, isFind);
if (!selector) {
return undefined;
}
}
// parsed successfully, clear any previous error message
this.setError(undefined, isFind);
orSelector = !orSelector ? selector : `${orSelector},${selector}`;
}
// parsed successfully, clear any previous error message
this.setError(undefined, isFind);
return selector;

return orSelector;
};

private prepareValue = (val: string): string => {
Expand All @@ -528,13 +532,17 @@ export class GraphFind extends React.Component<GraphFindProps, GraphFindState> {
val = val.replace(/ contains /gi, ' *= ');
val = val.replace(/ startswith /gi, ' ^= ');
val = val.replace(/ endswith /gi, ' $= ');

// uppercase conjunctions
val = val.replace(/ and /gi, ' AND ');
val = val.replace(/ or /gi, ' OR ');

return val.trim();
};

private parseExpression = (
expression: string,
conjunctive: boolean,
disjunctive: boolean,
isFind: boolean
): ParsedExpression | undefined => {
let op;
Expand Down Expand Up @@ -607,9 +615,7 @@ export class GraphFind extends React.Component<GraphFindProps, GraphFindState> {
}
case 'name': {
const isNegation = op.startsWith('!');
if (disjunctive && isNegation) {
return this.setError(`Can not use 'OR' with negated 'name' operand`, isFind);
} else if (conjunctive) {
if (conjunctive) {
return this.setError(`Can not use 'AND' with 'name' operand`, isFind);
}
const agg = `[${CyNode.aggregateValue} ${op} "${val}"]`;
Expand Down Expand Up @@ -804,16 +810,15 @@ export class GraphFind extends React.Component<GraphFindProps, GraphFindState> {
private appendSelector = (
selector: string,
parsedExpression: ParsedExpression,
separator: string,
isFind: boolean
): string | undefined => {
if (!selector) {
return parsedExpression.target + parsedExpression.selector;
}
if (!selector.startsWith(parsedExpression.target)) {
return this.setError('Invalid expression. Can not mix node and edge criteria.', isFind);
return this.setError('Invalid expression. Can not AND node and edge criteria.', isFind);
}
return selector + separator + parsedExpression.selector;
return selector + parsedExpression.selector;
};
}

Expand Down
20 changes: 16 additions & 4 deletions src/pages/Graph/GraphToolbar/__tests__/GraphFind.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,23 @@ describe('Parse find value test', () => {

// check composites
// @ts-ignore
expect(instance.parseValue('ns=foo OR ns=bar')).toEqual('node[namespace = "foo"],[namespace = "bar"]');
expect(instance.parseValue('ns=foo OR ns=bar')).toEqual('node[namespace = "foo"],node[namespace = "bar"]'); // OR same target
// @ts-ignore
expect(instance.parseValue('ns=foo AND ns=bar')).toEqual('node[namespace = "foo"][namespace = "bar"]');
expect(instance.parseValue('ns=foo AND ns=bar')).toEqual('node[namespace = "foo"][namespace = "bar"]'); // AND same target
// @ts-ignore
expect(instance.parseValue('ns=foo OR ns=bar AND app=foo')).toEqual(
'node[namespace = "foo"],node[namespace = "bar"][app = "foo"]'
); // AND and OR same target
// @ts-ignore
expect(instance.parseValue('ns=foo OR protocol=http')).toEqual('node[namespace = "foo"],edge[protocol = "http"]'); // OR different targets
// @ts-ignore
expect(instance.parseValue('ns=foo OR protocol=http OR !traffic')).toEqual(
'node[namespace = "foo"],edge[protocol = "http"],edge[^hasTraffic]'
); // OR different targets
// @ts-ignore
expect(instance.parseValue('ns=foo AND ns=bar OR protocol=http AND !traffic')).toEqual(
'node[namespace = "foo"][namespace = "bar"],edge[protocol = "http"][^hasTraffic]'
); // OR different targets, each with AND

// check find by name
// @ts-ignore
Expand All @@ -232,8 +246,6 @@ describe('Parse find value test', () => {
// @ts-ignore
expect(instance.parseValue('node = appp')).toEqual(undefined); // invalid node type
// @ts-ignore
expect(instance.parseValue('ns=foo OR ns=bar AND app=foo')).toEqual(undefined); // AND and OR
// @ts-ignore
expect(instance.parseValue('ns=foo AND http > 5.0')).toEqual(undefined); // Node and Edge
});
});

0 comments on commit 2da96a4

Please sign in to comment.