diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap index ce2b6dcc01..2f367b384e 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap @@ -51,6 +51,101 @@ exports[` DdgNodeContent.getNodeRenderer() returns a +
+ + + + + + + + + + + + + Set focus + + + + + + + + View traces + + + + + + + + Focus paths through this node + + + + + + + + Hide node + + +
`; @@ -105,6 +200,28 @@ exports[` DdgNodeContent.getNodeRenderer() returns a focal +
+ + + + + + View traces + + +
`; @@ -253,46 +370,6 @@ exports[` omits the operation if it is null 1`] = ` Hide node - - - - - - View Parents - - - - - - - - View Children - - `; @@ -427,46 +504,6 @@ exports[` omits the operation if it is null 2`] = ` Hide node - - - - - - View Parents - - - - - - - - View Children - - `; @@ -616,46 +653,6 @@ exports[` renders correctly when isFocalNode = true and focalNod Hide node - - - - - - View Parents - - - - - - - - View Children - - `; @@ -732,51 +729,11 @@ exports[` renders correctly when isFocalNode = true and focalNod View traces - - - - - - View Parents - - - - - - - - View Children - - `; -exports[` renders correctly when not hovered 1`] = ` +exports[` renders the number of operations if there are multiple 1`] = `
renders correctly when not hovered 1`] = ` Hide node - - - - - - View Parents - - - - - - - - View Children - -
`; -exports[` renders correctly when not hovered 2`] = ` +exports[` renders the number of operations if there are multiple 2`] = `
renders correctly when not hovered 2`] = ` } } > - + + } + mouseEnterDelay={0.1} + mouseLeaveDelay={0.1} + overlayStyle={Object {}} + placement="bottom" + prefixCls="ant-popover" + title="Select Operation to Filter Graph" + transitionName="zoom-big" + trigger="hover" + > + + 4 Operations + +
+ `; diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.test.js index e0f51f4e52..83a3a4e3ce 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.test.js @@ -157,6 +157,23 @@ describe('calcPositioning', () => { ); expect(svcMarginTop).toBe(radius - lineHeight - OP_PADDING_TOP / 2); }); + + it('treats multiple operations as a single word', () => { + const maxSvcLines = 2; + const operationCount = 10; + svcMeasurements = genWidths(new Array(maxSvcLines).fill(0.5)); + opMeasurements = genWidths(new Array(operationCount).fill(3)); + const { opWidth, radius, svcWidth, svcMarginTop } = calcPositioning( + genStr(maxSvcLines), + new Array(operationCount).fill(genStr(1)) + ); + expect(measureSvc).toHaveBeenCalledTimes(maxSvcLines); + expect(measureOp).toHaveBeenCalledTimes(1); + expect(svcWidth).toBe(1 * lineHeight); + expect(opWidth).toBe(3 * lineHeight); + expect(radius).toMatchInlineSnapshot(`34.51869478592516`); + expect(svcMarginTop).toBeCloseTo(radius - radius * Math.sin(Math.acos(svcWidth / 2 / radius)), 8); + }); }); describe('neglible service rectangle', () => { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.tsx index 31d267af03..529aec5167 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/calc-positioning.tsx @@ -52,9 +52,6 @@ type TRect = { height: number; width: number }; let svcSpan: HTMLSpanElement | undefined; -// TODO remove -let log: (...args: any) => void = () => {}; - /* * Mesaurements for the words comprising a service are measured by a span that is mounted out of a view. * Using a canvas would remove the requirement that the measuring container is part of the DOM, however canvas @@ -153,12 +150,12 @@ function calcWidth(lengths: number[], lines: number, longestThusFar: number = 0) */ const calcRects = _memoize( function calcRects(str: string | string[], span: HTMLSpanElement): TRect[] { - log(str, Array.isArray(str)); - const lengths = (Array.isArray(str) ? [`${str.length} Operations}`] : (str.match(WORD_RX) || [str])).map(s => { - span.innerHTML = s; // eslint-disable-line no-param-reassign - return span.getClientRects()[0].width; - }); - log(lengths); + const lengths = (Array.isArray(str) ? [`${str.length} Operations}`] : str.match(WORD_RX) || [str]).map( + s => { + span.innerHTML = s; // eslint-disable-line no-param-reassign + return span.getClientRects()[0].width; + } + ); const rects: TRect[] = []; for (let lines = 1; lines <= lengths.length; lines++) { @@ -167,7 +164,6 @@ const calcRects = _memoize( if (!rects.length || width < rects[rects.length - 1].width) rects.push({ height, width }); if (height > width) break; } - log(rects); return rects; }, (str: string, span: HTMLSpanElement) => `${str}\t${span.style.fontWeight}` @@ -246,10 +242,12 @@ function smallestRadius(svcRects: TRect[], opRects?: TRect[]): TSmallestRadiusRV return rv; } -const calcPositioning: (service: string, operation?: string | string[] | null) => TSmallestRadiusRV = _memoize( +const calcPositioning: ( + service: string, + operation?: string | string[] | null +) => TSmallestRadiusRV = _memoize( function calcPositioningImpl(service: string, operation?: string | string[] | null) { const svcRects = calcRects(service, _initSvcSpan()); - log = Array.isArray(operation) ? console.log : () => {}; const opRects = operation ? calcRects(operation, _initOpSpan()) : undefined; return smallestRadius(svcRects, opRects); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.css b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.css index 72da283226..a15bd61cb8 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.css +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.css @@ -16,13 +16,18 @@ limitations under the License. /* * Because .plexus-Digraph--MeasurableHtmlNode is `position: absolute`, adding a z-index to DdgNodeContent - * would have no effect on layering. + * would have no effect on layering between different nodes. */ +.plexus-Digraph--MeasurableHtmlNode { + /* transition-delay must equal .DdgNodeContent--actionsWrapper transition delay plus duration */ + transition-delay: 250ms; + transition-property: z-index; + z-index: 0; +} + .plexus-Digraph--MeasurableHtmlNode:hover { + transition-delay: 0s; z-index: 1; - /* TODO ease-out 150ms */ - /* if ease-out also works for display none the hovered.state can be used only as a guard for calculating the - * parent/children vis */ } .DdgNodeContent--core { @@ -63,7 +68,19 @@ limitations under the License. bottom: 100%; box-shadow: 0 0px 4px 1px rgba(0, 0, 0, 0.1); left: 1em; + opacity: 0; + pointer-events: none; position: absolute; + /* transition delay plus duration must equal .plexus-Digraph--MeasurableHtmlNode transition-delay */ + transition-delay: 150ms; + transition-duration: 0.1s; + transition-property: opacity; +} + +.DdgNodeContent:hover .DdgNodeContent--actionsWrapper { + opacity: 1; + pointer-events: all; + transition-delay: 0s; } .DdgNodeContent--actionsItem { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js index f586efc864..9573ac5449 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js @@ -22,6 +22,7 @@ jest.mock('./calc-positioning', () => () => ({ /* eslint-disable import/first */ import React from 'react'; import { shallow } from 'enzyme'; +import { Checkbox } from 'antd'; import DdgNodeContent from '.'; import { MAX_LENGTH, MAX_LINKED_TRACES, MIN_LENGTH, PARAM_NAME_LENGTH, RADIUS } from './constants'; @@ -50,14 +51,10 @@ describe('', () => { let wrapper; beforeEach(() => { - props.getGenerationVisibility.mockImplementation(direction => - direction === EDirection.Upstream ? ECheckedStatus.Full : ECheckedStatus.Partial - ); props.getVisiblePathElems.mockReset(); props.setViewModifier.mockReset(); props.updateGenerationVisibility.mockReset(); wrapper = shallow(); - wrapper.setState({ hovered: true }); }); it('does not explode', () => { @@ -70,15 +67,15 @@ describe('', () => { expect(wrapper).toMatchSnapshot(); }); - it('renders correctly when isFocalNode = true and focalNodeUrl = null', () => { + it('renders the number of operations if there are multiple', () => { expect(wrapper).toMatchSnapshot(); - wrapper.setProps({ focalNodeUrl: null, isFocalNode: true }); + wrapper.setProps({ operation: ['op0', 'op1', 'op2', 'op3'] }); expect(wrapper).toMatchSnapshot(); }); - it('renders correctly when not hovered', () => { + it('renders correctly when isFocalNode = true and focalNodeUrl = null', () => { expect(wrapper).toMatchSnapshot(); - wrapper.setState({ hovered: false }); + wrapper.setProps({ focalNodeUrl: null, isFocalNode: true }); expect(wrapper).toMatchSnapshot(); }); @@ -93,62 +90,73 @@ describe('', () => { }); describe('hover behavior', () => { - beforeAll(() => { - jest.useFakeTimers(); + const testIndices = [4, 8, 15, 16, 23, 42]; + const testElems = testIndices.map(visibilityIdx => ({ visibilityIdx })); + + beforeEach(() => { + props.getVisiblePathElems.mockReturnValue(testElems); }); it('calls setViewModifier on mouse enter', () => { wrapper.simulate('mouseenter', { type: 'mouseenter' }); expect(props.setViewModifier).toHaveBeenCalledTimes(1); - expect(props.setViewModifier).toHaveBeenCalledWith(vertexKey, EViewModifier.Hovered, true); + expect(props.setViewModifier).toHaveBeenCalledWith(testIndices, EViewModifier.Hovered, true); }); - it('calls setViewModifier on mouse leave', () => { + it('calls setViewModifier with all modified indices on mouse leave', () => { + wrapper.simulate('mouseenter', { type: 'mouseenter' }); wrapper.simulate('mouseleave', { type: 'mouseleave' }); - expect(props.setViewModifier).toHaveBeenCalledTimes(1); - expect(props.setViewModifier).toHaveBeenCalledWith(vertexKey, EViewModifier.Hovered, false); - }); + expect(props.setViewModifier).toHaveBeenCalledTimes(2); + expect(props.setViewModifier).toHaveBeenCalledWith(testIndices, EViewModifier.Hovered, false); - it('calls setViewModifier on unmount iff state.hovered is true', () => { - wrapper.unmount(); - - expect(props.setViewModifier).toHaveBeenCalledTimes(1); - expect(props.setViewModifier).toHaveBeenCalledWith(vertexKey, EViewModifier.Hovered, false); - - // state.hovered is initially false - const unhoveredWrapper = shallow(); - unhoveredWrapper.unmount(); - - expect(props.setViewModifier).toHaveBeenCalledTimes(1); - expect(props.setViewModifier).toHaveBeenCalledWith(vertexKey, EViewModifier.Hovered, false); - }); - - it('sets state.hovered to true on mouse enter', () => { - wrapper.setState({ hovered: false }); wrapper.simulate('mouseenter', { type: 'mouseenter' }); + const moreIndices = [108]; + const moreElems = moreIndices.map(visibilityIdx => ({ visibilityIdx })); + props.getVisiblePathElems.mockReturnValue(moreElems); + wrapper.simulate('mouseenter', { type: 'mouseenter' }); + wrapper.simulate('mouseleave', { type: 'mouseleave' }); - expect(wrapper.state('hovered')).toBe(true); + expect(props.setViewModifier).toHaveBeenCalledTimes(5); + expect(props.setViewModifier).toHaveBeenCalledWith( + testIndices.concat(moreIndices), + EViewModifier.Hovered, + false + ); }); - it('sets state.hovered to false on mouse leave, after delay', () => { + it('calls setViewModifier on unmount iff any indices were hovered and not unhovered', () => { + wrapper.unmount(); + expect(props.setViewModifier).toHaveBeenCalledTimes(0); + + wrapper = shallow(); + wrapper.simulate('mouseenter', { type: 'mouseenter' }); wrapper.simulate('mouseleave', { type: 'mouseleave' }); - expect(wrapper.state('hovered')).toBe(true); + expect(props.setViewModifier).toHaveBeenCalledTimes(2); + wrapper.unmount(); + expect(props.setViewModifier).toHaveBeenCalledTimes(2); - jest.runAllTimers(); - expect(wrapper.state('hovered')).toBe(false); - }); + wrapper = shallow(); + wrapper.simulate('mouseenter', { type: 'mouseenter' }); + wrapper.unmount(); - it('cancels delayed set state if mouse re-enters before timeout runs', () => { - wrapper.simulate('mouseleave', { type: 'mouseleave' }); - expect(wrapper.instance().hoverClearDelay).toEqual(expect.any(Number)); + expect(props.setViewModifier).toHaveBeenCalledTimes(4); + expect(props.setViewModifier).toHaveBeenCalledWith(testIndices, EViewModifier.Hovered, false); + }); + it('calculates state.childrenVisibility and state.parentVisibility on mouse enter', () => { + const childrenVisibility = ECheckedStatus.Partial; + const parentVisibility = ECheckedStatus.Full; + props.getGenerationVisibility.mockImplementation((_key, direction) => + direction === EDirection.Upstream ? parentVisibility : childrenVisibility + ); wrapper.simulate('mouseenter', { type: 'mouseenter' }); - expect(wrapper.instance().hoverClearDelay).toBeUndefined(); - jest.runAllTimers(); - expect(wrapper.state('hovered')).toBe(true); + expect(wrapper.state()).toEqual({ + childrenVisibility, + parentVisibility, + }); }); }); @@ -186,10 +194,42 @@ describe('', () => { }); describe('updateChildren', () => { + it('renders children visibility status indicator iff state.childrenVisibility is provided', () => { + const initialItemCount = wrapper.find('.DdgNodeContent--actionsItem').length; + + wrapper.setState({ childrenVisibility: ECheckedStatus.Empty }); + expect(wrapper.find('.DdgNodeContent--actionsItem').length).toBe(initialItemCount + 1); + expect(wrapper.find(Checkbox).props()).toEqual( + expect.objectContaining({ + checked: false, + indeterminate: false, + }) + ); + + wrapper.setState({ childrenVisibility: ECheckedStatus.Partial }); + expect(wrapper.find('.DdgNodeContent--actionsItem').length).toBe(initialItemCount + 1); + expect(wrapper.find(Checkbox).props()).toEqual( + expect.objectContaining({ + checked: false, + indeterminate: true, + }) + ); + + wrapper.setState({ childrenVisibility: ECheckedStatus.Full }); + expect(wrapper.find('.DdgNodeContent--actionsItem').length).toBe(initialItemCount + 1); + expect(wrapper.find(Checkbox).props()).toEqual( + expect.objectContaining({ + checked: true, + indeterminate: false, + }) + ); + }); + it('calls this.props.updateGenerationVisibility with this.props.vertexKey', () => { + wrapper.setState({ childrenVisibility: ECheckedStatus.Empty }); wrapper .find('.DdgNodeContent--actionsItem') - .at(5) + .last() .simulate('click'); expect(props.updateGenerationVisibility).toHaveBeenCalledWith(props.vertexKey, EDirection.Downstream); @@ -198,10 +238,42 @@ describe('', () => { }); describe('updateParents', () => { + it('renders parent visibility status indicator iff state.parentVisibility is provided', () => { + const initialItemCount = wrapper.find('.DdgNodeContent--actionsItem').length; + + wrapper.setState({ parentVisibility: ECheckedStatus.Empty }); + expect(wrapper.find('.DdgNodeContent--actionsItem').length).toBe(initialItemCount + 1); + expect(wrapper.find(Checkbox).props()).toEqual( + expect.objectContaining({ + checked: false, + indeterminate: false, + }) + ); + + wrapper.setState({ parentVisibility: ECheckedStatus.Partial }); + expect(wrapper.find('.DdgNodeContent--actionsItem').length).toBe(initialItemCount + 1); + expect(wrapper.find(Checkbox).props()).toEqual( + expect.objectContaining({ + checked: false, + indeterminate: true, + }) + ); + + wrapper.setState({ parentVisibility: ECheckedStatus.Full }); + expect(wrapper.find('.DdgNodeContent--actionsItem').length).toBe(initialItemCount + 1); + expect(wrapper.find(Checkbox).props()).toEqual( + expect.objectContaining({ + checked: true, + indeterminate: false, + }) + ); + }); + it('calls this.props.updateGenerationVisibility with this.props.vertexKey', () => { + wrapper.setState({ parentVisibility: ECheckedStatus.Empty }); wrapper .find('.DdgNodeContent--actionsItem') - .at(4) + .last() .simulate('click'); expect(props.updateGenerationVisibility).toHaveBeenCalledWith(props.vertexKey, EDirection.Upstream); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx index c95f267acd..a6b23450bb 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx @@ -58,17 +58,18 @@ type TProps = { operation: string | string[] | null; service: string; setOperation: (operation: string) => void; - setViewModifier: (vertexKey: string, viewModifier: EViewModifier, isEnabled: boolean) => void; + setViewModifier: (visIndices: number[], viewModifier: EViewModifier, isEnabled: boolean) => void; updateGenerationVisibility: (vertexKey: string, direction: EDirection) => void; vertexKey: string; }; -export default class DdgNodeContent extends React.PureComponent { - hoverClearDelay?: number; +type TState = { + childrenVisibility?: ECheckedStatus | null; + parentVisibility?: ECheckedStatus | null; +}; - state = { - hovered: false, - }; +export default class DdgNodeContent extends React.PureComponent { + state: TState = {}; static measureNode() { const diameter = 2 * (RADIUS + 1); @@ -99,7 +100,7 @@ export default class DdgNodeContent extends React.PureComponent { getVisiblePathElems: (vertexKey: string) => PathElem[] | undefined; hideVertex: (vertexKey: string) => void; setOperation: (operation: string) => void; - setViewModifier: (vertexKey: string, viewModifier: EViewModifier, enable: boolean) => void; + setViewModifier: (visIndices: number[], viewModifier: EViewModifier, enable: boolean) => void; updateGenerationVisibility: (vertexKey: string, direction: EDirection) => void; }) { return function renderNode(vertex: TDdgVertex, _: unknown, lv: TLayoutVertex | null) { @@ -126,9 +127,12 @@ export default class DdgNodeContent extends React.PureComponent { }; } + hoveredIndices: Set = new Set(); + componentWillUnmount() { - if (this.state.hovered) { - this.props.setViewModifier(this.props.vertexKey, EViewModifier.Hovered, false); + if (this.hoveredIndices.size) { + this.props.setViewModifier(Array.from(this.hoveredIndices), EViewModifier.Hovered, false); + this.hoveredIndices.clear(); } } @@ -185,44 +189,32 @@ export default class DdgNodeContent extends React.PureComponent { }; private onMouseUx = (event: React.MouseEvent) => { - const { vertexKey, setViewModifier } = this.props; + const { getGenerationVisibility, getVisiblePathElems, setViewModifier, vertexKey } = this.props; const hovered = event.type === 'mouseenter'; - setViewModifier(vertexKey, EViewModifier.Hovered, hovered); + const visIndices = hovered + ? (getVisiblePathElems(vertexKey) || []).map(({ visibilityIdx }) => { + this.hoveredIndices.add(visibilityIdx); + return visibilityIdx; + }) + : Array.from(this.hoveredIndices); + setViewModifier(visIndices, EViewModifier.Hovered, hovered); + if (hovered) { - if (this.hoverClearDelay) { - clearTimeout(this.hoverClearDelay); - this.hoverClearDelay = undefined; - } else { - this.setState({ hovered }); - } - } else { - this.hoverClearDelay = setTimeout(() => { - this.setState({ hovered }); - this.hoverClearDelay = undefined; - }, 150); - } + this.setState({ + childrenVisibility: getGenerationVisibility(vertexKey, EDirection.Downstream), + parentVisibility: getGenerationVisibility(vertexKey, EDirection.Upstream), + }); + } else this.hoveredIndices.clear(); }; render() { - const { hovered } = this.state; - const { - focalNodeUrl, - getGenerationVisibility, - isFocalNode, - isPositioned, - operation, - service, - setOperation, - vertexKey, - } = this.props; + const { childrenVisibility, parentVisibility } = this.state; + const { focalNodeUrl, isFocalNode, isPositioned, operation, service, setOperation } = this.props; const { radius, svcWidth, opWidth, svcMarginTop } = calcPositioning(service, operation); const scaleFactor = RADIUS / radius; const transform = `translate(${RADIUS - radius}px, ${RADIUS - radius}px) scale(${scaleFactor})`; - const childrenVisibility = hovered && getGenerationVisibility(vertexKey, EDirection.Downstream); - const parentVisibility = hovered && getGenerationVisibility(vertexKey, EDirection.Upstream); - return (
{ className="DdgNodeContent--label" style={{ paddingTop: `${OP_PADDING_TOP}px`, width: `${opWidth}px` }} > - { - Array.isArray(operation) - ? {}} - options={operation} - value={null} - setValue={setOperation} - /> - } - placement="bottom" - title="Select Operation to Filter Graph" - > - {`${operation.length} Operations`} - - : - } + {Array.isArray(operation) ? ( + {}} + options={operation} + value={null} + setValue={setOperation} + /> + } + placement="bottom" + title="Select Operation to Filter Graph" + > + {`${operation.length} Operations`} + + ) : ( + + )}
)}
- - {hovered && ( -
- {focalNodeUrl && ( - - {setFocusIcon} - Set focus - - )} - + - )} + )} + {!isFocalNode && ( + + + + + Hide node + + )} + {parentVisibility && ( + + + + + View Parents + + )} + {childrenVisibility && ( + + + + + View Children + + )} +
); } diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/__snapshots__/index.test.js.snap b/packages/jaeger-ui/src/components/DeepDependencies/Graph/__snapshots__/index.test.js.snap index bcd7810b8b..79f3da9bfd 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/__snapshots__/index.test.js.snap +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/__snapshots__/index.test.js.snap @@ -18,16 +18,16 @@ exports[` render renders provided edges and vertices 1`] = ` } layers={ Array [ - Object { - "key": "nodes/find-emphasis/html", - "layerType": "html", - "renderNode": [Function], - }, Object { "key": "nodes/find-emphasis/vector-color-band", "layerType": "svg", "renderNode": null, }, + Object { + "key": "nodes/find-emphasis/html", + "layerType": "html", + "renderNode": [Function], + }, Object { "key": "nodes/vector-border", "layerType": "svg", diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/getNodeRenderers.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Graph/getNodeRenderers.test.js index cbb0d0334c..87c6fbbba5 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/getNodeRenderers.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/getNodeRenderers.test.js @@ -41,7 +41,7 @@ describe('getNodeRenderers', () => { pathHovered )} .is-pathHovered,\tand ${wvwo(focalNode)} .is-focalNode`, () => { const testLv = focalNode ? focalLv : lv; - const findMatches = new Set(findMatch ? [testLv.vertex] : undefined); + const findMatches = new Set(findMatch ? [testLv.vertex.key] : undefined); const vm = // eslint-disable-next-line no-bitwise (hovered ? EViewModifier.Hovered : 0) | (pathHovered ? EViewModifier.PathHovered : 0); @@ -60,7 +60,7 @@ describe('getNodeRenderers', () => { }); it('returns div with .is-findMatch if vertex is a findMatch', () => { - const wrapper = shallow(getNodeRenderers(new Set([lv.vertex]), new Map()).htmlEmphasis(lv)); + const wrapper = shallow(getNodeRenderers(new Set([lv.vertex.key]), new Map()).htmlEmphasis(lv)); expect(wrapper.hasClass('is-findMatch')).toBe(true); expect(wrapper.type()).toBe('div'); }); @@ -72,7 +72,9 @@ describe('getNodeRenderers', () => { }); it('returns div with .is-findMatch and .is-focalNode if vertex is a focalNode and a findMatch', () => { - const wrapper = shallow(getNodeRenderers(new Set([focalLv.vertex]), new Map()).htmlEmphasis(focalLv)); + const wrapper = shallow( + getNodeRenderers(new Set([focalLv.vertex.key]), new Map()).htmlEmphasis(focalLv) + ); expect(wrapper.hasClass('is-findMatch')).toBe(true); expect(wrapper.hasClass('is-focalNode')).toBe(true); expect(wrapper.type()).toBe('div'); @@ -92,7 +94,7 @@ describe('getNodeRenderers', () => { it('returns circle with correct size and className', () => { expect( - shallow(getNodeRenderers(new Set([lv.vertex]), new Map()).vectorFindColorBand(lv)) + shallow(getNodeRenderers(new Set([lv.vertex.key]), new Map()).vectorFindColorBand(lv)) ).toMatchSnapshot(); }); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx index e4addc5e48..1269737189 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx @@ -44,7 +44,7 @@ type TProps = { getVisiblePathElems: (vertexKey: string) => PathElem[] | undefined; hideVertex: (vertexKey: string) => void; setOperation: (operation: string) => void; - setViewModifier: (vertexKey: string, viewModifier: EViewModifier, enable: boolean) => void; + setViewModifier: (visIndices: number[], viewModifier: EViewModifier, enable: boolean) => void; uiFindMatches: Set | undefined; updateGenerationVisibility: (vertexKey: string, direction: EDirection) => void; vertices: TDdgVertex[]; diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/Selector.css b/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/Selector.css index b0204c0966..2653ace637 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/Selector.css +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/Selector.css @@ -15,7 +15,7 @@ limitations under the License. */ .HopsSelector--Selector { - color: rgba(0, 0, 0, 0.65); + color: var(--tx-color-muted); margin: 0 0.5em; padding: 0px 3px; } diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.test.js index 78cae3d562..d57966345c 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.test.js @@ -13,7 +13,7 @@ // limitations under the License. import React from 'react'; -import { Popover } from 'antd'; +import { Icon, Popover } from 'antd'; import { shallow } from 'enzyme'; import BreakableText from '../../common/BreakableText'; @@ -129,4 +129,30 @@ describe('', () => { wrapper.setState({ popoverVisible: true }); expect(fn.mock.calls.length).toBe(1); }); + + describe('clear', () => { + const clearValue = jest.fn(); + + beforeEach(() => { + clearValue.mockClear(); + wrapper.setProps({ clearValue, required: undefined, value: 'foo' }); + }); + + it('renders with clear icon when not required and with a value', () => { + expect(wrapper).toMatchSnapshot(); + }); + + it('clicking clear icon clears value when not required without opening popover', () => { + const stopPropagation = jest.fn(); + wrapper.find(Icon).simulate('click', { stopPropagation }); + + expect(clearValue).toHaveBeenCalled(); + expect(stopPropagation).toHaveBeenCalled(); + expect(wrapper.state('popoverVisible')).toBe(false); + }); + + it('throws Error when attempting to clear when required', () => { + expect(new NameSelector(props).clearValue).toThrowError('Cannot clear value of required NameSelector'); + }); + }); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.tsx index 1bf4890a12..f87a53d5ee 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.tsx @@ -22,16 +22,15 @@ import FilteredList from '../../common/FilteredList'; import './NameSelector.css'; - type TOptional = { clearValue: () => void; required?: false; -} +}; type TRequired = { clearValue?: never; required: true; -} +}; type TProps = { label: string; @@ -71,7 +70,7 @@ export default class NameSelector extends React.PureComponent { evt.stopPropagation(); this.props.clearValue(); - } + }; setValue = (value: string) => { this.props.setValue(value); @@ -127,7 +126,9 @@ export default class NameSelector extends React.PureComponent { {useLabel && {label}:} - {!required && value && } + {!required && value && ( + + )} ); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/NameSelector.test.js.snap b/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/NameSelector.test.js.snap index 1907e6b02d..b9f466ee50 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/NameSelector.test.js.snap +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/NameSelector.test.js.snap @@ -1,5 +1,58 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` clear renders with clear icon when not required and with a value 1`] = ` + + } + mouseEnterDelay={0.1} + mouseLeaveDelay={0.1} + onVisibleChange={[Function]} + overlayClassName="NameSelector--overlay u-rm-popover-content-padding" + overlayStyle={Object {}} + placement="bottomLeft" + prefixCls="ant-popover" + transitionName="zoom-big" + trigger="click" + visible={false} +> +

+ + a-label + : + + + + +

+ +`; + exports[` renders with is-invalid when required and without a value 1`] = ` renders the hops selector if distanceToPathElems is provided 1 className="DdgHeader--paramsHeader" > renders the operation selector if a service is selected 1`] = className="DdgHeader--paramsHeader" > renders the operation selector if a service is selected 1`] = value="testService" /> @@ -132,7 +131,7 @@ exports[`
renders the operation selector if a service is selected 2`] = className="DdgHeader--paramsHeader" > renders the operation selector if a service is selected 2`] = value="testService" /> @@ -203,7 +201,7 @@ exports[`
renders with minimal props 1`] = ` className="DdgHeader--paramsHeader" > ', () => { const getTooltip = () => wrapper.find(Tooltip); const hiddenUiFindMatches = new Set(['hidden', 'match', 'vertices']); const uiFindCount = 20; + const expectedFindCount = expect.stringContaining(`${uiFindCount}`); + const expectedHiddenCount = expect.stringContaining(`${hiddenUiFindMatches.size}`); + const expectedHiddenTitle = `Click to view ${hiddenUiFindMatches.size} hidden matches`; it('renders no info if count is `undefined`', () => { expect(getMatchesInfo()).toHaveLength(0); @@ -90,28 +95,59 @@ describe('
', () => { }); it('renders count if `hiddenUiFindMatches` is `undefined` or empty', () => { - const expectedText = `${uiFindCount}`; const expectedTitle = 'All matches are visible'; wrapper.setProps({ uiFindCount }); - expect(getMatchesInfo().text()).toBe(expectedText); + expect(getMatchesInfo().text()).toEqual(expectedFindCount); + expect(getMatchesInfo().text()).not.toEqual(expectedHiddenCount); expect(getTooltip().prop('title')).toBe(expectedTitle); expect(getBtn().prop('disabled')).toBe(true); + expect(wrapper.find(MdVisibility)).toHaveLength(1); + expect(wrapper.find(MdVisibilityOff)).toHaveLength(0); wrapper.setProps({ hiddenUiFindMatches: new Set() }); - expect(getMatchesInfo().text()).toBe(expectedText); + expect(getMatchesInfo().text()).toEqual(expectedFindCount); + expect(getMatchesInfo().text()).not.toEqual(expectedHiddenCount); expect(getTooltip().prop('title')).toBe(expectedTitle); expect(getBtn().prop('disabled')).toBe(true); + expect(wrapper.find(MdVisibility)).toHaveLength(1); + expect(wrapper.find(MdVisibilityOff)).toHaveLength(0); }); - it('renders count out of total if both are provided', () => { - const expectedText = `${uiFindCount} / ${uiFindCount + hiddenUiFindMatches.size}`; - const expectedTitle = 'Click to view hidden matches'; - + it('renders both visible and hidden counts if both are provided', () => { wrapper.setProps({ hiddenUiFindMatches, uiFindCount }); - expect(getMatchesInfo().text()).toBe(expectedText); + expect(getMatchesInfo().text()).toEqual(expectedFindCount); + expect(getMatchesInfo().text()).toEqual(expectedHiddenCount); + expect(getTooltip().prop('title')).toBe(expectedHiddenTitle); + expect(getBtn().prop('disabled')).toBe(false); + expect(wrapper.find(MdVisibility)).toHaveLength(1); + expect(wrapper.find(MdVisibilityOff)).toHaveLength(1); + }); + + it('renders 0 with correct tooltip if there are no matches', () => { + const expectedTitle = 'No matches'; + + wrapper.setProps({ uiFindCount: 0 }); + expect(getMatchesInfo().text()).toBe('0'); expect(getTooltip().prop('title')).toBe(expectedTitle); + expect(getBtn().prop('disabled')).toBe(true); + expect(wrapper.find(MdVisibility)).toHaveLength(0); + expect(wrapper.find(MdVisibilityOff)).toHaveLength(0); + + wrapper.setProps({ hiddenUiFindMatches }); + expect(getMatchesInfo().text()).toEqual(expect.stringContaining('0')); + expect(getMatchesInfo().text()).toEqual(expectedHiddenCount); + expect(getTooltip().prop('title')).toBe(expectedHiddenTitle); expect(getBtn().prop('disabled')).toBe(false); + expect(wrapper.find(MdVisibility)).toHaveLength(1); + expect(wrapper.find(MdVisibilityOff)).toHaveLength(1); + }); + + it('renders correct plurality in tooltip', () => { + const expectedTitle = `Click to view 1 hidden match`; + + wrapper.setProps({ hiddenUiFindMatches: new Set([Array.from(hiddenUiFindMatches)[0]]), uiFindCount }); + expect(getTooltip().prop('title')).toBe(expectedTitle); }); it('calls props.showVertices with vertices in props.hiddenUiFindMatches when clicked with hiddenUiFindMatches', () => { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx index 009621ee70..fa4697c1bc 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx @@ -14,6 +14,8 @@ import * as React from 'react'; import { Icon, Input, Tooltip } from 'antd'; +import MdVisibility from 'react-icons/lib/md/visibility'; +import MdVisibilityOff from 'react-icons/lib/md/visibility-off'; import HopsSelector from './HopsSelector'; import NameSelector from './NameSelector'; @@ -62,13 +64,19 @@ export default class Header extends React.PureComponent { if (uiFindCount === undefined) return null; - let btnText = `${uiFindCount}`; - let noMore = true; - let tipText = 'All matches are visible'; + let hasHidden = false; + let hiddenInfo: React.ReactNode = null; + let tipText = uiFindCount ? 'All matches are visible' : 'No matches'; if (hiddenUiFindMatches && hiddenUiFindMatches.size) { - noMore = false; - btnText = `${uiFindCount} / ${uiFindCount + hiddenUiFindMatches.size}`; - tipText = 'Click to view hidden matches'; + const { size } = hiddenUiFindMatches; + hasHidden = true; + tipText = `Click to view ${size} hidden match${size !== 1 ? 'es' : ''}`; + hiddenInfo = ( + + {size} + + + ); } return ( @@ -77,11 +85,13 @@ export default class Header extends React.PureComponent { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index e799b53d46..cd3974bb8a 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -178,6 +178,14 @@ describe('DeepDependencyGraphPage', () => { expect(getUrlSpy).toHaveBeenLastCalledWith(expect.objectContaining({ hash }), undefined); }); + describe('clearOperation', () => { + it('removes op from urlState', () => { + ddgPageImpl.clearOperation(); + const { operation: _o, ...urlStateWithoutOp } = props.urlState; + expect(getUrlSpy).toHaveBeenLastCalledWith(urlStateWithoutOp, undefined); + }); + }); + describe('focusPathsThroughVertex', () => { let trackFocusPathsSpy; @@ -469,12 +477,6 @@ describe('DeepDependencyGraphPage', () => { describe('view modifiers', () => { const visibilityIndices = ['visId0', 'visId1', 'visId2']; const targetVM = EViewModifier.Emphasized; - let warnSpy; - - beforeAll(() => { - props.graph.getVertexVisiblePathElems.mockReturnValue(visibilityIndices.map(setIdx)); - warnSpy = jest.spyOn(console, 'warn').mockImplementationOnce(); - }); beforeEach(() => { props.addViewModifier.mockReset(); @@ -482,12 +484,8 @@ describe('DeepDependencyGraphPage', () => { props.removeViewModifierFromIndices.mockReset(); }); - afterAll(() => { - warnSpy.mockRestore(); - }); - it('adds given viewModifier to specified pathElems', () => { - ddgPageImpl.setViewModifier(vertexKey, targetVM, true); + ddgPageImpl.setViewModifier(visibilityIndices, targetVM, true); expect(props.addViewModifier).toHaveBeenLastCalledWith({ operation: props.urlState.operation, service: props.urlState.service, @@ -496,14 +494,10 @@ describe('DeepDependencyGraphPage', () => { end: 0, start: 0, }); - expect(props.graph.getVertexVisiblePathElems).toHaveBeenCalledWith( - vertexKey, - props.urlState.visEncoding - ); }); it('removes given viewModifier from specified pathElems', () => { - ddgPageImpl.setViewModifier(vertexKey, targetVM, false); + ddgPageImpl.setViewModifier(visibilityIndices, targetVM, false); expect(props.removeViewModifierFromIndices).toHaveBeenCalledWith({ operation: props.urlState.operation, service: props.urlState.service, @@ -512,44 +506,20 @@ describe('DeepDependencyGraphPage', () => { end: 0, start: 0, }); - expect(props.graph.getVertexVisiblePathElems).toHaveBeenCalledWith( - vertexKey, - props.urlState.visEncoding - ); - }); - - it('warns error if given absent vertexKey', () => { - props.graph.getVertexVisiblePathElems.mockReturnValueOnce(undefined); - const absentVertexKey = 'absentVertexKey'; - ddgPageImpl.setViewModifier(absentVertexKey, EViewModifier.emphasized, true); - expect(warnSpy).toHaveBeenCalledTimes(1); - expect(warnSpy).toHaveBeenCalledWith( - `Invalid vertex key to set view modifier for: ${absentVertexKey}` - ); }); - it('no-ops if not given dispatch fn or graph or operation or service', () => { + it('no-ops if not given dispatch fn or graph or service', () => { const { addViewModifier: _add, ...propsWithoutAdd } = props; const ddgWithoutAdd = new DeepDependencyGraphPageImpl(propsWithoutAdd); ddgWithoutAdd.setViewModifier(vertexKey, EViewModifier.emphasized, true); - expect(props.graph.getVertexVisiblePathElems).not.toHaveBeenCalled(); const { removeViewModifierFromIndices: _remove, ...propsWithoutRemove } = props; const ddgWithoutRemove = new DeepDependencyGraphPageImpl(propsWithoutRemove); ddgWithoutRemove.setViewModifier(vertexKey, EViewModifier.emphasized, false); - expect(props.graph.getVertexVisiblePathElems).not.toHaveBeenCalled(); + expect(props.removeViewModifierFromIndices).not.toHaveBeenCalled(); ddgWithoutGraph.setViewModifier(vertexKey, EViewModifier.emphasized, true); - expect(props.graph.getVertexVisiblePathElems).not.toHaveBeenCalled(); - - const { - urlState: { operation: _operation, ...urlStateWithoutOperation }, - ...propsWithoutOperation - } = props; - propsWithoutOperation.urlState = urlStateWithoutOperation; - const ddgWithoutOperation = new DeepDependencyGraphPageImpl(propsWithoutGraph); - ddgWithoutOperation.setViewModifier(vertexKey, EViewModifier.emphasized, true); - expect(props.graph.getVertexVisiblePathElems).not.toHaveBeenCalled(); + expect(props.removeViewModifierFromIndices).not.toHaveBeenCalled(); const { urlState: { service: _service, ...urlStateWithoutService }, @@ -558,7 +528,7 @@ describe('DeepDependencyGraphPage', () => { propsWithoutService.urlState = urlStateWithoutService; const ddgWithoutService = new DeepDependencyGraphPageImpl(propsWithoutGraph); ddgWithoutService.setViewModifier(vertexKey, EViewModifier.emphasized, true); - expect(props.graph.getVertexVisiblePathElems).not.toHaveBeenCalled(); + expect(props.removeViewModifierFromIndices).not.toHaveBeenCalled(); }); }); @@ -787,23 +757,36 @@ describe('DeepDependencyGraphPage', () => { expect(getUrlStateSpy).toHaveBeenLastCalledWith(search); }); + it('calculates showOp off of urlState', () => { + [true, false, undefined].forEach(showOp => { + ['focalOperation', undefined].forEach(focalOp => { + const urlState = { + ...expected.urlState, + operation: focalOp, + showOp, + }; + getUrlStateSpy.mockReturnValue(urlState); + const result = mapStateToProps(state, ownProps); + expect(result.showOp).toBe(showOp === undefined ? focalOp !== undefined : showOp); + }); + }); + }); + it('includes graphState iff location.search has service, start, end, and optionally operation', () => { const graphState = 'testGraphState'; const graphStateWithoutOp = 'testGraphStateWithoutOp'; const reduxState = { ...state }; // TODO: Remove 0s once time buckets are implemented _set(reduxState, ['ddg', getStateEntryKey({ service, operation, start: 0, end: 0 })], graphState); - _set(reduxState, ['ddg', getStateEntryKey({ service, start, end })], graphStateWithoutOp); + _set(reduxState, ['ddg', getStateEntryKey({ service, start: 0, end: 0 })], graphStateWithoutOp); const result = mapStateToProps(reduxState, ownProps); expect(result.graphState).toEqual(graphState); - /* TODO: operation is still required, when requirement is lifted, re-enable const { operation: _op, ...rest } = expected.urlState; getUrlStateSpy.mockReturnValue(rest); const resultWithoutOp = mapStateToProps(reduxState, ownProps); expect(resultWithoutOp.graphState).toEqual(graphStateWithoutOp); - */ getUrlStateSpy.mockReturnValue({}); const resultWithoutParams = mapStateToProps(reduxState, ownProps); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index a24867e2da..75f6b0b693 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -37,7 +37,6 @@ import { EViewModifier, TDdgModelParams, TDdgSparseUrlState, - TDdgVertex, } from '../../model/ddg/types'; import { encode, encodeDistance } from '../../model/ddg/visibility-codec'; import { ReduxState } from '../../types'; @@ -115,7 +114,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { clearOperation = () => { this.updateUrlState({ operation: undefined }); - } + }; focusPathsThroughVertex = (vertexKey: string) => { const elems = this.getVisiblePathElems(vertexKey); @@ -187,20 +186,11 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { this.updateUrlState({ operation: undefined, service, visEncoding: undefined }); }; - setViewModifier = (vertexKey: string, viewModifier: EViewModifier, enable: boolean) => { + setViewModifier = (visibilityIndices: number[], viewModifier: EViewModifier, enable: boolean) => { const { addViewModifier, graph, removeViewModifierFromIndices, urlState } = this.props; const fn = enable ? addViewModifier : removeViewModifierFromIndices; - const { service, operation, visEncoding } = urlState; - if (!fn || !graph || !operation || !service) { - return; - } - const pathElems = graph.getVertexVisiblePathElems(vertexKey, visEncoding); - if (!pathElems) { - // eslint-disable-next-line no-console - console.warn(`Invalid vertex key to set view modifier for: ${vertexKey}`); - return; - } - const visibilityIndices = pathElems.map(pe => pe.visibilityIdx); + const { service, operation } = urlState; + if (!fn || !graph || !service) return; fn({ operation, service, @@ -340,7 +330,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP const { services, operationsForService } = stServices; const urlState = getUrlState(ownProps.location.search); const { density, operation, service, showOp: urlStateShowOp } = urlState; - const showOp = urlStateShowOp !== undefined ? urlStateShowOp: operation !== undefined ; + const showOp = urlStateShowOp !== undefined ? urlStateShowOp : operation !== undefined; let graphState: TDdgStateEntry | undefined; if (service) { graphState = _get(state.ddg, getStateEntryKey({ service, operation, start: 0, end: 0 })); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js b/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js index 20333e571c..9f97edbbf1 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js @@ -106,6 +106,21 @@ describe('TracesDdg', () => { expect(mapStateToProps(state, ownProps)).toEqual(expect.objectContaining({ urlState })); }); + it('calculates showOp off of urlState', () => { + [true, false, undefined].forEach(showOp => { + ['focalOperation', undefined].forEach(focalOp => { + const mockUrlState = { + ...urlState, + operation: focalOp, + showOp, + }; + getUrlStateSpy.mockReturnValue(mockUrlState); + const result = mapStateToProps(state, ownProps); + expect(result.showOp).toBe(showOp === undefined ? focalOp !== undefined : showOp); + }); + }); + }); + it('calculates graphState and graph iff service is provided', () => { expect(mapStateToProps(state, ownProps)).toEqual( expect.objectContaining({ diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx index 258c60ab2a..f751ca02c6 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx @@ -37,7 +37,7 @@ const svcOp = memoizeOne((service, operation) => ({ service, operation })); export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxProps { const urlState = getUrlState(ownProps.location.search); const { density, operation, service, showOp: urlStateShowOp } = urlState; - const showOp = urlStateShowOp !== undefined ? urlStateShowOp: operation !== undefined ; + const showOp = urlStateShowOp !== undefined ? urlStateShowOp : operation !== undefined; let graphState: TDdgStateEntry | undefined; let graph: GraphModel | undefined; if (service) { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/url.test.js b/packages/jaeger-ui/src/components/DeepDependencies/url.test.js index 26be8e5219..0386c22949 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/url.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/url.test.js @@ -133,13 +133,6 @@ describe('DeepDependencyGraph/url', () => { }); }); - it('defaults `showOp` to true', () => { - const { showOp: unused, ...rest } = expectedParams; - const { showOp: alsoUnused, ...rv } = acceptableParams; - parseSpy.mockReturnValue(rv); - expect(getUrlState(getSearch())).toEqual({ ...rest, showOp: true }); - }); - it("defaults `density` to 'ppe'", () => { const { density: unused, ...rest } = expectedParams; const { density: alsoUnused, ...rv } = acceptableParams; diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js index c333dadfcc..867cb22f7d 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js @@ -154,7 +154,8 @@ describe('lookback utils', () => { }); }); - it('creates timestamp for weeks ago', () => { + // DST/not-DST means this test will fail four 14 out of 52 weeks of the year! + xit('creates timestamp for weeks ago', () => { [1, 2, 4, 7].forEach(lookbackNum => { expect(nowInMicroseconds - lookbackToTimestamp(`${lookbackNum}w`, now)).toBe( lookbackNum * 7 * 24 * hourInMicroseconds diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.test.js b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.test.js index 28bf70d558..ce77820503 100644 --- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.test.js +++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.test.js @@ -14,11 +14,11 @@ import * as React from 'react'; import { shallow } from 'enzyme'; -// import _mapValues from 'lodash/mapValues'; import { UnconnectedTraceDiffGraph as TraceDiffGraph } from './TraceDiffGraph'; import ErrorMessage from '../../common/ErrorMessage'; import LoadingIndicator from '../../common/LoadingIndicator'; +import UiFindInput from '../../common/UiFindInput'; import { fetchedState } from '../../../constants'; describe('TraceDiffGraph', () => { @@ -147,6 +147,26 @@ describe('TraceDiffGraph', () => { expect(wrapper.children().length).toBe(0); }); + it('renders a DiGraph when it has data', () => { + expect(wrapper).toMatchSnapshot(); + }); + + it('renders current uiFind count when given uiFind', () => { + expect(wrapper.find(UiFindInput).prop('inputProps')).toEqual( + expect.objectContaining({ + suffix: undefined, + }) + ); + + wrapper.setProps({ uiFind: 'test uiFind' }); + + expect(wrapper.find(UiFindInput).prop('inputProps')).toEqual( + expect.objectContaining({ + suffix: '0', + }) + ); + }); + it('cleans up layoutManager before unmounting', () => { const layoutManager = jest.spyOn(wrapper.instance().layoutManager, 'stopAndRelease'); wrapper.unmount(); diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx index 7a4caf4185..50df1e16dc 100644 --- a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx +++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx @@ -39,21 +39,12 @@ export class UnconnectedTraceDiffGraph extends React.PureComponent { cacheAs = cacheAs.makeScope(); - static defaultProps = { - uiFind: '', - }; - componentWillUnmount() { this.layoutManager.stopAndRelease(); } render() { - const { - a, - b, - // Flow requires `= ''` because it does not interpret defaultProps - uiFind = '', - } = this.props; + const { a, b, uiFind = '' } = this.props; if (!a || !b) { return

At least two Traces are needed

; } diff --git a/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/TraceDiffGraph.test.js.snap b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/TraceDiffGraph.test.js.snap new file mode 100644 index 0000000000..cee4bc28d5 --- /dev/null +++ b/packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/__snapshots__/TraceDiffGraph.test.js.snap @@ -0,0 +1,82 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`TraceDiffGraph renders a DiGraph when it has data 1`] = ` +
+ + +
+`; diff --git a/packages/jaeger-ui/src/components/common/FilteredList/index.test.js b/packages/jaeger-ui/src/components/common/FilteredList/index.test.js index be0c85fc1a..fe34680d25 100644 --- a/packages/jaeger-ui/src/components/common/FilteredList/index.test.js +++ b/packages/jaeger-ui/src/components/common/FilteredList/index.test.js @@ -138,6 +138,14 @@ describe('', () => { expect(props.setValue.mock.calls).toEqual([[props.options[focusedIndex]]]); }); + it('enter selects the filteredOption if there is only one option', () => { + const value = words[1]; + expect(props.setValue.mock.calls.length).toBe(0); + wrapper.find('input').simulate('change', { target: { value } }); + keyDown(EKey.Enter); + expect(props.setValue.mock.calls).toEqual([[value]]); + }); + it('enter is ignored when an item is not focused', () => { expect(props.setValue.mock.calls.length).toBe(0); keyDown(EKey.Enter); diff --git a/packages/jaeger-ui/src/components/common/FilteredList/index.tsx b/packages/jaeger-ui/src/components/common/FilteredList/index.tsx index c67b68be74..55b3a7507a 100644 --- a/packages/jaeger-ui/src/components/common/FilteredList/index.tsx +++ b/packages/jaeger-ui/src/components/common/FilteredList/index.tsx @@ -82,7 +82,7 @@ export default class FilteredList extends React.PureComponent { const { cancel } = this.props; this.setState({ filterText: '', focusedIndex: null }); cancel(); - return; + break; } case EKey.ArrowUp: case EKey.ArrowDown: { @@ -102,19 +102,12 @@ export default class FilteredList extends React.PureComponent { if (listInstance && (focusedIndex < visibleStartIndex + 1 || focusedIndex > visibleStopIndex - 1)) { listInstance.scrollToItem(focusedIndex); } - return; + break; } case EKey.Enter: { const filteredOptions = this.getFilteredOptions(); - const value = stFocused !== null - ? filteredOptions[stFocused] - : filteredOptions.length === 1 - ? filteredOptions[0] - : null; - if (value === null) { - return; - } - this.setValue(value); + if (stFocused !== null) this.setValue(filteredOptions[stFocused]); + else if (filteredOptions.length === 1) this.setValue(filteredOptions[0]); break; } default: // no-op diff --git a/packages/jaeger-ui/src/components/common/UiFindInput.test.js b/packages/jaeger-ui/src/components/common/UiFindInput.test.js index 4513f22a35..ed4dc721a0 100644 --- a/packages/jaeger-ui/src/components/common/UiFindInput.test.js +++ b/packages/jaeger-ui/src/components/common/UiFindInput.test.js @@ -14,7 +14,7 @@ import * as React from 'react'; import { shallow } from 'enzyme'; -import { Input } from 'antd'; +import { Icon, Input } from 'antd'; import debounceMock from 'lodash/debounce'; import queryString from 'query-string'; @@ -32,7 +32,7 @@ describe('UiFind', () => { const uiFind = 'uiFind'; const ownInputValue = 'ownInputValue'; const props = { - uiFind: null, + uiFind: undefined, history: { replace: () => {}, }, @@ -54,6 +54,7 @@ describe('UiFind', () => { beforeEach(() => { flushMock.mockReset(); + updateUiFindSpy.mockReset(); wrapper = shallow(); }); @@ -109,6 +110,15 @@ describe('UiFind', () => { uiFind: newValue, }); }); + + it('no-ops if value is unchanged', () => { + wrapper.find(Input).simulate('change', { target: { value: '' } }); + expect(updateUiFindSpy).not.toHaveBeenCalled(); + + wrapper.setProps({ uiFind }); + wrapper.find(Input).simulate('change', { target: { value: uiFind } }); + expect(updateUiFindSpy).not.toHaveBeenCalled(); + }); }); describe('blurring input', () => { @@ -125,6 +135,45 @@ describe('UiFind', () => { }); }); + describe('clear uiFind', () => { + const findIcon = () => shallow(
{wrapper.find(Input).prop('suffix')}
); + + beforeEach(() => { + wrapper.setProps({ allowClear: true }); + }); + + it('renders clear icon iff clear is enabled and value is a string with at least one character', () => { + expect(findIcon().find(Icon)).toHaveLength(0); + + wrapper.setProps({ uiFind: '' }); + expect(findIcon().find(Icon)).toHaveLength(0); + + wrapper.setProps({ uiFind }); + expect(findIcon().find(Icon)).toHaveLength(1); + + wrapper.setProps({ allowClear: false }); + expect(findIcon().find(Icon)).toHaveLength(0); + + wrapper.setProps({ allowClear: true }); + wrapper.setState({ ownInputValue: '' }); + expect(findIcon().find(Icon)).toHaveLength(0); + }); + + it('clears value immediately when clicked', () => { + wrapper.setProps({ uiFind }); + findIcon() + .find(Icon) + .simulate('click'); + + expect(updateUiFindSpy).toHaveBeenLastCalledWith({ + history: props.history, + location: props.location, + uiFind: undefined, + }); + expect(flushMock).toHaveBeenCalledTimes(1); + }); + }); + describe('extractUiFindFromState', () => { const reduxStateValue = 'state.router.location.search'; diff --git a/packages/jaeger-ui/src/components/common/UiFindInput.tsx b/packages/jaeger-ui/src/components/common/UiFindInput.tsx index fdb6401fa4..4bf19a4c6f 100644 --- a/packages/jaeger-ui/src/components/common/UiFindInput.tsx +++ b/packages/jaeger-ui/src/components/common/UiFindInput.tsx @@ -57,7 +57,8 @@ export class UnconnectedUiFindInput extends React.PureComponent { - const { history, location, trackFindFunction } = this.props; + const { history, location, uiFind: prevUiFind, trackFindFunction } = this.props; + if (uiFind === prevUiFind || (!prevUiFind && !uiFind)) return; updateUiFind({ location, history, diff --git a/packages/jaeger-ui/src/components/common/__snapshots__/UiFindInput.test.js.snap b/packages/jaeger-ui/src/components/common/__snapshots__/UiFindInput.test.js.snap index c771d8bbd4..46852ee2d5 100644 --- a/packages/jaeger-ui/src/components/common/__snapshots__/UiFindInput.test.js.snap +++ b/packages/jaeger-ui/src/components/common/__snapshots__/UiFindInput.test.js.snap @@ -10,6 +10,5 @@ exports[`UiFind rendering renders as expected 1`] = ` prefixCls="ant-input" suffix={} type="text" - value={null} /> `; diff --git a/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.test.js b/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.test.js index 819d5d076d..6d916d5276 100644 --- a/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.test.js +++ b/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.test.js @@ -13,52 +13,93 @@ // limitations under the License. import GraphModel from './index'; +import { FOCAL_KEY } from './getPathElemHasher'; import transformDdgData from '../transformDdgData'; import { wrap } from '../sample-paths.test.resources'; import { EDdgDensity } from '../types'; -function makePayloadEntry(pairStr) { - const [service, operation] = pairStr.split(':'); - return { service, operation }; -} +describe('getPathElemHasher()', () => { + function makePayloadEntry(pairStr) { + const [service, operation] = pairStr.split(':'); + return { service, operation }; + } -const payload = ` - a:0 b:0 focal:focal c:0 d:0 - a:0 c:0 focal:focal d:0 c:0 - c:1 focal:focal b:0 d:0 - c:0 focal:focal b:0 - a:0 c:0 focal:focal b:0 -` - .trim() - .split('\n') - .map(line => - line + function makePayload(str) { + return str .trim() - .split(/\s+/g) - .map(makePayloadEntry) - ); - -const testTable = [ - // showOp, density, number of expected vertices - [false, EDdgDensity.MostConcise, 5], - [true, EDdgDensity.MostConcise, 6], - [false, EDdgDensity.UpstreamVsDownstream, 7], - [true, EDdgDensity.UpstreamVsDownstream, 8], - [false, EDdgDensity.OnePerLevel, 9], - [true, EDdgDensity.OnePerLevel, 10], - [false, EDdgDensity.PreventPathEntanglement, 11], - [true, EDdgDensity.PreventPathEntanglement, 12], - [false, EDdgDensity.ExternalVsInternal, 13], - [true, EDdgDensity.ExternalVsInternal, 14], -]; + .split('\n') + .map(line => + line + .trim() + .split(/\s+/g) + .map(makePayloadEntry) + ); + } -describe('getPathElemHasher()', () => { - const ddgModel = transformDdgData(wrap(payload), makePayloadEntry('focal:focal')); + const payloadStr = ` + a:0 b:0 focal:focal c:0 d:0 + a:0 c:0 focal:focal d:0 c:0 + c:1 focal:focal b:0 d:0 + c:0 focal:focal b:0 + a:0 c:0 focal:focal b:0 + `; + const ddgModel = transformDdgData(wrap(makePayload(payloadStr)), makePayloadEntry('focal:focal')); describe('creates vertices based on density and showOp', () => { + const testTable = [ + // showOp, density, number of expected vertices + [false, EDdgDensity.MostConcise, 5], + [true, EDdgDensity.MostConcise, 6], + [false, EDdgDensity.UpstreamVsDownstream, 7], + [true, EDdgDensity.UpstreamVsDownstream, 8], + [false, EDdgDensity.OnePerLevel, 9], + [true, EDdgDensity.OnePerLevel, 10], + [false, EDdgDensity.PreventPathEntanglement, 11], + [true, EDdgDensity.PreventPathEntanglement, 12], + [false, EDdgDensity.ExternalVsInternal, 13], + [true, EDdgDensity.ExternalVsInternal, 14], + ]; + it.each(testTable)('showOp: %p \t density: %p', (showOp, density, verticesCount) => { const gm = new GraphModel({ ddgModel, density, showOp }); expect(gm.vertices.size).toBe(verticesCount); + expect(gm.vertices.has(FOCAL_KEY)).toBe(true); + }); + }); + + describe('EDdgDensity.MostConcise focal node edge cases', () => { + const mcFocalStr = ` + focal:0 focal:0 + focal:0 focal:1 + focal:0 otherSvc:0 + `; + const mcFocalModel = transformDdgData(wrap(makePayload(mcFocalStr)), makePayloadEntry('focal:0')); + + it('uses focal vertex for non-focal PathElem if its service equals the focal service and showOp is false', () => { + const gm = new GraphModel({ ddgModel: mcFocalModel, density: EDdgDensity.MostConcise, showOp: false }); + expect(gm.vertices.size).toBe(2); + expect(gm.vertices.has(FOCAL_KEY)).toBe(true); + }); + + it('uses focal vertex for non-focal PathElem if its service and operation equal the focal service and operation and showOp is true', () => { + const gm = new GraphModel({ ddgModel: mcFocalModel, density: EDdgDensity.MostConcise, showOp: true }); + expect(gm.vertices.size).toBe(3); + expect(gm.vertices.has(FOCAL_KEY)).toBe(true); + }); + + it('uses focal vertex for non-focal PathElem if its service equals the focal service and its operation is any of the focal operations and showOp is true and focal operation is not set', () => { + const mcFocalWithCrossedOpStr = `${mcFocalStr}focal:1 focal:2`; + const mcFocalWithCrossedOpModel = transformDdgData( + wrap(makePayload(mcFocalWithCrossedOpStr)), + makePayloadEntry('focal') + ); + const gm = new GraphModel({ + ddgModel: mcFocalWithCrossedOpModel, + density: EDdgDensity.MostConcise, + showOp: true, + }); + expect(gm.vertices.size).toBe(3); + expect(gm.vertices.has(FOCAL_KEY)).toBe(true); }); }); @@ -72,6 +113,6 @@ describe('getPathElemHasher()', () => { expect(invalidDensity).toThrowError(); const missingDensity = () => new GraphModel({ ddgModel, density: undefined, showOp: true }); - expect(missingDensity).toThrowError(); + expect(missingDensity).toThrowError(/has not been implemented/); }); }); diff --git a/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.tsx b/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.tsx index cdfd6e3ae1..833062dfc2 100644 --- a/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.tsx +++ b/packages/jaeger-ui/src/model/ddg/GraphModel/getPathElemHasher.tsx @@ -48,22 +48,26 @@ export default function getPathElemHasher(this: GraphModel): TPathElemToStr { if (pe === focalElem || !this.showOp) return FOCAL_KEY; const focalVertex = this.vertices.get(FOCAL_KEY); + // istanbul ignore next : Unreachable error to appease TS, transformDdgData would have thrown if (!focalVertex) throw new Error('No focal vertex found'); const focalElems = this.vertexToPathElems.get(focalVertex); + // istanbul ignore next : Unreachable error to appease TS, transformDdgData would have thrown if (!focalElems) throw new Error('No focal elems found'); + // Because pathElems are processed in increasing order of absolute distance, the following set will // never be incomplete. const focalOps = new Set(Array.from(focalElems).map(({ operation }) => operation.name)); if (focalOps.has(pe.operation.name)) return FOCAL_KEY; } return elemToStr(pe); - } + }; } case EDdgDensity.UpstreamVsDownstream: { - return (pe: PathElem) => `${elemToStr(pe)}; direction=${Math.sign(pe.distance)}`; + return (pe: PathElem) => + pe.distance ? `${elemToStr(pe)}; direction=${Math.sign(pe.distance)}` : FOCAL_KEY; } case EDdgDensity.OnePerLevel: { - return (pe: PathElem) => `${elemToStr(pe)}; distance=${pe.distance}`; + return (pe: PathElem) => (pe.distance ? `${elemToStr(pe)}; distance=${pe.distance}` : FOCAL_KEY); } case EDdgDensity.PreventPathEntanglement: { return getPpeHasher(elemToStr); diff --git a/packages/jaeger-ui/src/model/ddg/GraphModel/index.test.js b/packages/jaeger-ui/src/model/ddg/GraphModel/index.test.js index 5d89f522ab..4651f25246 100644 --- a/packages/jaeger-ui/src/model/ddg/GraphModel/index.test.js +++ b/packages/jaeger-ui/src/model/ddg/GraphModel/index.test.js @@ -13,7 +13,9 @@ // limitations under the License. import GraphModel, { makeGraph } from './index'; +import { FOCAL_KEY } from './getPathElemHasher'; import { + almostDoubleFocalPath, convergentPaths, doubleFocalPath, generationPaths, @@ -28,6 +30,9 @@ import { encode } from '../visibility-codec'; describe('GraphModel', () => { const convergentModel = transformDdgData(wrap(convergentPaths), focalPayloadElem); const doubleFocalModel = transformDdgData(wrap([doubleFocalPath, simplePath]), focalPayloadElem); + const withoutFocalOpModel = transformDdgData(wrap([almostDoubleFocalPath, simplePath]), { + service: focalPayloadElem.service, + }); const simpleModel = transformDdgData(wrap([simplePath]), focalPayloadElem); const getIdx = ({ visibilityIdx }) => visibilityIdx; @@ -155,6 +160,44 @@ describe('GraphModel', () => { ); }); + it('creates focal vertex with multiple operations if focal operation is omitted', () => { + const withoutFocalOpGraph = new GraphModel({ + ddgModel: withoutFocalOpModel, + density: EDdgDensity.UpstreamVsDownstream, + showOp: true, + }); + validateGraph(withoutFocalOpGraph, [ + { + visIndices: [0, 1], + }, + { + visIndices: [3], + focalSideNeighbors: [0], + }, + { + visIndices: [4, 5], + focalSideNeighbors: [0], + }, + { + visIndices: [7], + focalSideNeighbors: [3], + }, + { + visIndices: [8, 9], + focalSideNeighbors: [4], + }, + { + visIndices: [2, 10], + focalSideNeighbors: [0, 7], + }, + { + visIndices: [6, 11], + focalSideNeighbors: [2], + }, + ]); + expect(withoutFocalOpGraph.vertices.get(FOCAL_KEY).operation).toHaveLength(2); + }); + describe('error cases', () => { it('errors if given model contains a pathElem that cannot be connected to the focal node', () => { const invalidModel = { @@ -435,6 +478,11 @@ describe('GraphModel', () => { density: EDdgDensity.PreventPathEntanglement, showOp: true, }); + const withoutFocalOpGraph = new GraphModel({ + ddgModel: withoutFocalOpModel, + density: EDdgDensity.UpstreamVsDownstream, + showOp: true, + }); describe('visEncoding provided', () => { it('returns just focalNode', () => { @@ -447,8 +495,8 @@ describe('GraphModel', () => { const { edges, vertices } = convergentGraph.getVisible(encode([0, 4])); expect(edges).toHaveLength(1); expect(vertices).toEqual([ - expect.objectContaining(convergentPaths[0][1]), expect.objectContaining(convergentPaths[0][0]), + expect.objectContaining(convergentPaths[0][1]), ]); }); @@ -456,8 +504,8 @@ describe('GraphModel', () => { const { edges, vertices } = convergentGraph.getVisible(encode([0, 1, 4, 5])); expect(edges).toHaveLength(1); expect(vertices).toEqual([ - expect.objectContaining(convergentPaths[0][1]), expect.objectContaining(convergentPaths[0][0]), + expect.objectContaining(convergentPaths[0][1]), ]); }); @@ -481,6 +529,21 @@ describe('GraphModel', () => { const now = victimOfMutation.getVisible(encode([idx, idx + 1])); expect(prior).toEqual(now); }); + + it('returns focal vertex with multiple operations', () => { + const { vertices } = withoutFocalOpGraph.getVisible(); + const focalOps = vertices[vertices.length - 1].operation; + + expect(focalOps).toEqual(expect.any(Array)); + expect(focalOps).toHaveLength(2); + }); + + it('only returns visible operations for focal vertex', () => { + const { vertices } = withoutFocalOpGraph.getVisible(encode([0])); + const focalOp = vertices[vertices.length - 1].operation; + + expect(focalOp).toEqual(expect.any(String)); + }); }); describe('visEncoding not provided', () => { @@ -510,6 +573,14 @@ describe('GraphModel', () => { vertices: [], }); }); + + it('returns focal vertex with multiple operations', () => { + const { vertices } = withoutFocalOpGraph.getVisible(); + const focalOps = vertices[vertices.length - 1].operation; + + expect(focalOps).toEqual(expect.any(Array)); + expect(focalOps).toHaveLength(2); + }); }); }); @@ -524,58 +595,127 @@ describe('GraphModel', () => { density: EDdgDensity.PreventPathEntanglement, showOp: false, }); + const withoutFocalOpGraph = new GraphModel({ + ddgModel: withoutFocalOpModel, + density: EDdgDensity.UpstreamVsDownstream, + showOp: true, + }); + const [ + { + operation: { name: withoutFocalOpZerothOp }, + }, + { + operation: { name: withoutFocalOpFistOp }, + }, + ] = withoutFocalOpGraph.visIdxToPathElem; const shorten = str => str.substring(0, str.length - 3); const visEncoding = encode([0, 1, 2, 3, 4, 5]); const { vertices: visibleVertices } = convergentGraph.getVisible(visEncoding); - const { service: focalService, operation: focalOperation } = visibleVertices[0]; + const { key: focalKey, service: focalService, operation: focalOperation } = visibleVertices[ + visibleVertices.length - 1 + ]; const { service: otherService } = visibleVertices[2]; - const { vertices: oplessVertices } = hideOpGraph.getVisible(visEncoding); + const { vertices: hiddenOpVertices } = hideOpGraph.getVisible(visEncoding); const { operation: { name: otherOp }, - } = Array.from(hideOpGraph.vertexToPathElems.get(oplessVertices[2]))[0]; + } = Array.from(hideOpGraph.vertexToPathElems.get(hiddenOpVertices[2]))[0]; describe('getHiddenUiFindMatches', () => { it('returns a subset of hidden vertices that match provided uiFind', () => { const uiFind = `${shorten(focalService)} ${shorten(focalOperation)} ${shorten(otherService)}`; expect(convergentGraph.getHiddenUiFindMatches(uiFind, encode([0, 1]))).toEqual( - new Set([visibleVertices[2]]) + new Set([visibleVertices[2].key]) ); }); it('matches only on service.name if showOp is false', () => { - const uiFind = `${shorten(oplessVertices[1].service)} ${shorten(otherOp)}`; - expect(hideOpGraph.getHiddenUiFindMatches(uiFind, encode([0]))).toEqual(new Set([oplessVertices[1]])); + const uiFind = `${shorten(hiddenOpVertices[1].service)} ${shorten(otherOp)}`; + expect(hideOpGraph.getHiddenUiFindMatches(uiFind, encode([0]))).toEqual( + new Set([hiddenOpVertices[1].key]) + ); }); it('returns an empty set when provided empty or undefined uiFind', () => { expect(convergentGraph.getHiddenUiFindMatches()).toEqual(new Set()); expect(convergentGraph.getHiddenUiFindMatches('')).toEqual(new Set()); + expect(convergentGraph.getVisibleUiFindMatches(' ')).toEqual(new Set()); }); it('returns an empty set when all matches are visible', () => { const uiFind = `${shorten(focalService)} ${shorten(otherService)}`; expect(convergentGraph.getHiddenUiFindMatches(uiFind, visEncoding)).toEqual(new Set()); }); + + it('only matches hidden operations of focal vertex', () => { + expect(withoutFocalOpGraph.getHiddenUiFindMatches(shorten(withoutFocalOpZerothOp))).toEqual( + new Set() + ); + expect(withoutFocalOpGraph.getHiddenUiFindMatches(shorten(withoutFocalOpFistOp))).toEqual(new Set()); + + expect( + withoutFocalOpGraph.getHiddenUiFindMatches(shorten(withoutFocalOpZerothOp), encode([0, 7])) + ).toEqual(new Set()); + expect( + withoutFocalOpGraph.getHiddenUiFindMatches(shorten(withoutFocalOpFistOp), encode([0, 7])) + ).toEqual(new Set([focalKey])); + + expect( + withoutFocalOpGraph.getHiddenUiFindMatches(shorten(withoutFocalOpZerothOp), encode([1, 7])) + ).toEqual(new Set([focalKey])); + expect( + withoutFocalOpGraph.getHiddenUiFindMatches(shorten(withoutFocalOpFistOp), encode([1])) + ).toEqual(new Set()); + }); + + it('handles empty graph', () => { + const emptyGraph = new GraphModel({ + ddgModel: { distanceToPathElems: new Map(), visIdxToPathElem: [] }, + density: EDdgDensity.PreventPathEntanglement, + }); + expect(emptyGraph.getHiddenUiFindMatches('*')).toEqual(new Set()); + }); }); describe('getVisibleUiFindMatches', () => { it('returns a subset of getVisible that match provided uiFind', () => { const uiFind = `${shorten(focalService)} ${shorten(focalOperation)} ${shorten(otherService)}`; expect(convergentGraph.getVisibleUiFindMatches(uiFind, visEncoding)).toEqual( - new Set([visibleVertices[0], visibleVertices[2]]) + new Set([focalKey, visibleVertices[2].key]) ); }); it('matches only on service.name if showOp is false', () => { const uiFind = `${shorten(focalService)} ${shorten(otherOp)}`; - expect(hideOpGraph.getVisibleUiFindMatches(uiFind, visEncoding)).toEqual( - new Set([visibleVertices[0]]) - ); + expect(hideOpGraph.getVisibleUiFindMatches(uiFind, visEncoding)).toEqual(new Set([focalKey])); }); it('returns an empty set when provided empty or undefined uiFind', () => { expect(convergentGraph.getVisibleUiFindMatches()).toEqual(new Set()); expect(convergentGraph.getVisibleUiFindMatches('')).toEqual(new Set()); + expect(convergentGraph.getVisibleUiFindMatches(' ')).toEqual(new Set()); + }); + + it('only matches visible operations of focal vertex', () => { + expect( + withoutFocalOpGraph.getVisibleUiFindMatches(shorten(withoutFocalOpZerothOp), encode([0, 1])) + ).toEqual(new Set([focalKey])); + expect( + withoutFocalOpGraph.getVisibleUiFindMatches(shorten(withoutFocalOpFistOp), encode([0, 1])) + ).toEqual(new Set([focalKey])); + + expect( + withoutFocalOpGraph.getVisibleUiFindMatches(shorten(withoutFocalOpZerothOp), encode([0])) + ).toEqual(new Set([focalKey])); + expect( + withoutFocalOpGraph.getVisibleUiFindMatches(shorten(withoutFocalOpFistOp), encode([0])) + ).toEqual(new Set()); + + expect( + withoutFocalOpGraph.getVisibleUiFindMatches(shorten(withoutFocalOpZerothOp), encode([1])) + ).toEqual(new Set()); + expect( + withoutFocalOpGraph.getVisibleUiFindMatches(shorten(withoutFocalOpFistOp), encode([1])) + ).toEqual(new Set([focalKey])); }); }); }); @@ -651,23 +791,23 @@ describe('GraphModel', () => { density: EDdgDensity.PreventPathEntanglement, showOp: true, }); - const vertices = [ - overlapGraph.pathElemToVertex.get(overlapGraph.distanceToPathElems.get(3)[0]), - overlapGraph.pathElemToVertex.get(overlapGraph.distanceToPathElems.get(-1)[0]), + const vertexKeys = [ + overlapGraph.pathElemToVertex.get(overlapGraph.distanceToPathElems.get(3)[0]).key, + overlapGraph.pathElemToVertex.get(overlapGraph.distanceToPathElems.get(-1)[0]).key, ]; it('handles absent visEncoding', () => { - expect(overlapGraph.getVisWithVertices(vertices)).toBe(encode([0, 1, 2, 3, 4, 5, 6, 7, 8])); + expect(overlapGraph.getVisWithVertices(vertexKeys)).toBe(encode([0, 1, 2, 3, 4, 5, 6, 7, 8])); }); it('uses provided visEncoding', () => { - expect(overlapGraph.getVisWithVertices(vertices, encode([0, 1, 3]))).toBe( + expect(overlapGraph.getVisWithVertices(vertexKeys, encode([0, 1, 3]))).toBe( encode([0, 1, 2, 3, 4, 5, 6, 8]) ); }); it('throws error if given absent vertex', () => { - expect(() => overlapGraph.getVisWithVertices([{}])).toThrowError(); + expect(() => overlapGraph.getVisWithVertices(['absent key'])).toThrowError(/does not exist in graph/); }); }); diff --git a/packages/jaeger-ui/src/model/ddg/GraphModel/index.tsx b/packages/jaeger-ui/src/model/ddg/GraphModel/index.tsx index 7072cfa2bc..ca6ff4f549 100644 --- a/packages/jaeger-ui/src/model/ddg/GraphModel/index.tsx +++ b/packages/jaeger-ui/src/model/ddg/GraphModel/index.tsx @@ -63,10 +63,8 @@ export default class GraphModel { // If there is a compatible vertex for this pathElem, use it, else, make a new vertex const key = hasher(pathElem); const isFocalNode = !pathElem.distance; - const operation = this.showOp ? pathElem.operation.name : null - if (isFocalNode) { - if (operation) focalOperations.add(operation); - } + const operation = this.showOp ? pathElem.operation.name : null; + if (isFocalNode && operation) focalOperations.add(operation); let vertex: TDdgVertex | undefined = this.vertices.get(key); if (!vertex) { @@ -104,21 +102,19 @@ export default class GraphModel { const from = pathElem.distance > 0 ? connectedVertex.key : vertex.key; const to = pathElem.distance > 0 ? vertex.key : connectedVertex.key; const edgeId = getEdgeId(from, to); - const existingEdge = edgesById.get(edgeId); - if (!existingEdge) { - const edge = { from, to }; + let edge = edgesById.get(edgeId); + if (!edge) { + edge = { from, to }; edgesById.set(edgeId, edge); - this.pathElemToEdge.set(pathElem, edge); - } else { - this.pathElemToEdge.set(pathElem, existingEdge); } + this.pathElemToEdge.set(pathElem, edge); } - }); + }); if (focalOperations.size > 1) { const focalVertex = this.vertices.get(FOCAL_KEY); + // istanbul ignore next : focalVertex cannot be missing if focalOperations.size is not 0 if (focalVertex) focalVertex.operation = Array.from(focalOperations); - else if (this.visIdxToPathElem.length) throw new Error('No focal vertex found'); } Object.freeze(this.distanceToPathElems); @@ -198,18 +194,22 @@ export default class GraphModel { if (this.visIdxToPathElem.length) { const focalVertex = this.vertices.get(FOCAL_KEY); + // istanbul ignore next : If there are pathElems without a focal vertex the constructor would throw if (!focalVertex) throw new Error('No focal vertex found'); const visibleFocalElems = this.getVertexVisiblePathElems(FOCAL_KEY, visEncoding); - if (visibleFocalElems) { - const visibleFocalOps = Array.from(new Set(visibleFocalElems.map(({ operation }) => operation.name))); - const potentiallyPartialFocalVertex = { - ...focalVertex, - operation: this.showOp - ? visibleFocalOps.length === 1 ? visibleFocalOps[0] : visibleFocalOps - : null, - }; - vertices.add(potentiallyPartialFocalVertex); - }; + if (visibleFocalElems && visibleFocalElems.length) { + if (!this.showOp) vertices.add(focalVertex); + else { + const visibleFocalOps = Array.from( + new Set(visibleFocalElems.map(({ operation }) => operation.name)) + ); + const potentiallyPartialFocalVertex = { + ...focalVertex, + operation: visibleFocalOps.length === 1 ? visibleFocalOps[0] : visibleFocalOps, + }; + vertices.add(potentiallyPartialFocalVertex); + } + } } return { @@ -221,16 +221,17 @@ export default class GraphModel { private static getUiFindMatches(vertices: TDdgVertex[], uiFind?: string): Set { const keySet: Set = new Set(); - if (!uiFind) return keySet; + if (!uiFind || /^\s+$/.test(uiFind)) return keySet; const uiFindArr = uiFind .trim() .toLowerCase() - .split(' '); + .split(/\s+/); for (let i = 0; i < vertices.length; i++) { const { service, operation } = vertices[i]; const svc = service.toLowerCase(); - const ops = operation && (Array.isArray(operation) ? operation : [operation]).map(op => op.toLowerCase()); + const ops = + operation && (Array.isArray(operation) ? operation : [operation]).map(op => op.toLowerCase()); for (let j = 0; j < uiFindArr.length; j++) { if (svc.includes(uiFindArr[j]) || (ops && ops.some(op => op.includes(uiFindArr[j])))) { keySet.add(vertices[i].key); @@ -245,12 +246,16 @@ export default class GraphModel { public getHiddenUiFindMatches: (uiFind?: string, visEncoding?: string) => Set = memoize(10)( (uiFind?: string, visEncoding?: string): Set => { const visible = new Set(this.getVisible(visEncoding).vertices); - const hidden: TDdgVertex[] = Array.from(this.vertices.values()).filter(vertex => !visible.has(vertex) && !vertex.isFocalNode); + const hidden: TDdgVertex[] = Array.from(this.vertices.values()).filter( + vertex => !visible.has(vertex) && !vertex.isFocalNode + ); if (this.visIdxToPathElem.length) { const focalVertex = this.vertices.get(FOCAL_KEY); + // istanbul ignore next : If there are pathElems without a focal vertex the constructor would throw if (!focalVertex) throw new Error('No focal vertex found'); const focalElems = this.vertexToPathElems.get(focalVertex); + // istanbul ignore next : If there are pathElems without a focal vertex the constructor would throw if (!focalElems) throw new Error('No focal elems found'); const visibleFocalElems = new Set(this.getVertexVisiblePathElems(FOCAL_KEY, visEncoding)); const hiddenFocalOperations = Array.from(focalElems) @@ -329,9 +334,10 @@ export default class GraphModel { const elemSet: PathElem[] = []; vertexKeys.forEach(vertexKey => { const vertex = this.vertices.get(vertexKey); - if (!vertex) throw new Error(`${vertex} does not exist in graph`); + if (!vertex) throw new Error(`${vertexKey} does not exist in graph`); const elems = this.vertexToPathElems.get(vertex); - if (!elems) throw new Error(`${vertex} does not exist in graph`); + // istanbul ignore next : If a vertex exists it must have elems + if (!elems) throw new Error(`${vertexKey} does not exist in graph`); elemSet.push(...elems); }); diff --git a/packages/jaeger-ui/src/model/ddg/sample-paths.test.resources.js b/packages/jaeger-ui/src/model/ddg/sample-paths.test.resources.js index 54fae54526..d19de5b909 100644 --- a/packages/jaeger-ui/src/model/ddg/sample-paths.test.resources.js +++ b/packages/jaeger-ui/src/model/ddg/sample-paths.test.resources.js @@ -20,7 +20,7 @@ export const simplePayloadElemMaker = label => ({ export const focalPayloadElem = simplePayloadElemMaker('focal'); const sameFocalServicePayloadElem = { - operation: `not-${focalPayloadElem.operation}`, + operation: 'someOtherOperation', service: focalPayloadElem.service, }; @@ -85,8 +85,8 @@ export const almostDoubleFocalPath = [ const divergentPayloadElem = simplePayloadElemMaker('divergentPayloadElem'); export const convergentPaths = [ - [firstPayloadElem, focalPayloadElem, midPayloadElem, afterPayloadElem, lastPayloadElem], [firstPayloadElem, focalPayloadElem, divergentPayloadElem, afterPayloadElem, lastPayloadElem], + [firstPayloadElem, focalPayloadElem, midPayloadElem, afterPayloadElem, lastPayloadElem], ]; const generationPayloadElems = { diff --git a/packages/jaeger-ui/src/utils/update-ui-find.test.js b/packages/jaeger-ui/src/utils/update-ui-find.test.js index 89721eb9c0..d0ff126c08 100644 --- a/packages/jaeger-ui/src/utils/update-ui-find.test.js +++ b/packages/jaeger-ui/src/utils/update-ui-find.test.js @@ -89,17 +89,6 @@ describe('updateUiFind', () => { expect(replaceMock).toHaveBeenCalledWith(expectedReplaceMockArgument); }); - it('no-ops when given existing value', () => { - updateUiFind({ - history, - location, - uiFind: existingUiFind, - }); - expect(queryStringParseSpy).toHaveBeenCalledWith(location.search); - expect(queryStringStringifySpy).not.toHaveBeenCalled(); - expect(replaceMock).not.toHaveBeenCalled(); - }); - describe('trackFindFunction provided', () => { const trackFindFunction = jest.fn(); @@ -125,25 +114,5 @@ describe('updateUiFind', () => { }); expect(trackFindFunction).toHaveBeenCalledWith(newUiFind); }); - - it('does not track unchanged value', () => { - updateUiFind({ - history, - location, - trackFindFunction, - uiFind: existingUiFind, - }); - expect(trackFindFunction).not.toHaveBeenCalled(); - - queryStringParseSpy.mockReturnValueOnce({ - uiFind: undefined, - }); - updateUiFind({ - history, - location, - trackFindFunction, - }); - expect(trackFindFunction).not.toHaveBeenCalled(); - }); }); }); diff --git a/packages/jaeger-ui/src/utils/update-ui-find.tsx b/packages/jaeger-ui/src/utils/update-ui-find.tsx index 1c64c77730..223685a6b0 100644 --- a/packages/jaeger-ui/src/utils/update-ui-find.tsx +++ b/packages/jaeger-ui/src/utils/update-ui-find.tsx @@ -29,14 +29,10 @@ export default function updateUiFind({ trackFindFunction?: (uiFind: string | TNil) => void; uiFind?: string | TNil; }) { - const { uiFind: oldUiFind, ...queryParams } = queryString.parse(location.search); - if (oldUiFind === uiFind) return; - if (trackFindFunction) { - trackFindFunction(uiFind); - } - if (uiFind) { - (queryParams as Record).uiFind = uiFind; - } + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { uiFind: _oldUiFind, ...queryParams } = queryString.parse(location.search); + if (trackFindFunction) trackFindFunction(uiFind); + if (uiFind) (queryParams as Record).uiFind = uiFind; history.replace({ ...location, search: `?${queryString.stringify(queryParams)}`,