Review feedback

* Fix node-details-test for search
* Label spacing and matched text truncation
* Delete pinned search on backspace, add hint for metrics, escape % in URL
* Fix text-bg on node highlight
* Added tests for search-utils
* Fix matching of other topologies, added comment re quick clear
* s/cx/classnames/
* Ignore MoC keys when search in focus, blur on Esc
* Fixes search term highlighting on-hover
* Fix SVG exports
* Fine-tuned search item rendering
* Fixed search highlighting in the details panel
* Dont throb node on hover
* Hotkey for search: '/'
* Keep focus on search when tabbing away from the browser
* bring hovered node to top
* background for search results on hover
* fixed height for foreign object to prevent layout glitches
* Dont blur focused nodes on search
* More robust metric matchers
* More meaningful search hints

client/app/scripts/utils/__tests__/search-utils-test.js

@@ -0,0 +1,300 @@
+jest.dontMock('../search-utils');
+jest.dontMock('../string-utils');
+jest.dontMock('../../constants/naming'); // edge naming: 'source-target'
+
+import { fromJS } from 'immutable';
+
+const SearchUtils = require('../search-utils').testable;
+
+describe('SearchUtils', () => {
+  const nodeSets = {
+    someNodes: fromJS({
+      n1: {
+        id: 'n1',
+        label: 'node label 1',
+        metadata: [{
+          id: 'fieldId1',
+          label: 'Label 1',
+          value: 'value 1'
+        }],
+        metrics: [{
+          id: 'metric1',
+          label: 'Metric 1',
+          value: 1
+        }]
+      },
+      n2: {
+        id: 'n2',
+        label: 'node label 2',
+        metadata: [{
+          id: 'fieldId2',
+          label: 'Label 2',
+          value: 'value 2'
+        }],
+        tables: [{
+          id: 'metric1',
+          rows: [{
+            id: 'row1',
+            label: 'Row 1',
+            value: 'Row Value 1'
+          }]
+        }],
+      },
+    })
+  };
+
+  describe('applyPinnedSearches', () => {
+    const fun = SearchUtils.applyPinnedSearches;
+
+    it('should not filter anything when no pinned searches present', () => {
+      let nextState = fromJS({
+        nodes: nodeSets.someNodes,
+        pinnedSearches: []
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('nodes').filter(node => node.get('filtered')).size).toEqual(0);
+    });
+
+    it('should filter nodes if nothing matches a pinned search', () => {
+      let nextState = fromJS({
+        nodes: nodeSets.someNodes,
+        pinnedSearches: ['cantmatch']
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('nodes').filterNot(node => node.get('filtered')).size).toEqual(0);
+    });
+
+    it('should filter nodes if nothing matches a combination of pinned searches', () => {
+      let nextState = fromJS({
+        nodes: nodeSets.someNodes,
+        pinnedSearches: ['node label 1', 'node label 2']
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('nodes').filterNot(node => node.get('filtered')).size).toEqual(0);
+    });
+
+    it('should filter nodes that do not match a pinned searches', () => {
+      let nextState = fromJS({
+        nodes: nodeSets.someNodes,
+        pinnedSearches: ['row']
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('nodes').filter(node => node.get('filtered')).size).toEqual(1);
+    });
+  });
+
+  describe('findNodeMatch', () => {
+    const fun = SearchUtils.findNodeMatch;
+
+    it('does not add a non-matching field', () => {
+      let matches = fromJS({});
+      matches = fun(matches, ['node1', 'field1'],
+        'some value', 'some query', null, 'some label');
+      expect(matches.size).toBe(0);
+    });
+
+    it('adds a matching field', () => {
+      let matches = fromJS({});
+      matches = fun(matches, ['node1', 'field1'],
+        'samevalue', 'samevalue', null, 'some label');
+      expect(matches.size).toBe(1);
+      expect(matches.getIn(['node1', 'field1'])).toBeDefined();
+      const {text, label, start, length} = matches.getIn(['node1', 'field1']);
+      expect(text).toBe('samevalue');
+      expect(label).toBe('some label');
+      expect(start).toBe(0);
+      expect(length).toBe(9);
+    });
+
+    it('does not add a field when the prefix does not match the label', () => {
+      let matches = fromJS({});
+      matches = fun(matches, ['node1', 'field1'],
+        'samevalue', 'samevalue', 'some prefix', 'some label');
+      expect(matches.size).toBe(0);
+    });
+
+    it('adds a field when the prefix matches the label', () => {
+      let matches = fromJS({});
+      matches = fun(matches, ['node1', 'field1'],
+        'samevalue', 'samevalue', 'prefix', 'prefixed label');
+      expect(matches.size).toBe(1);
+    });
+  });
+
+  describe('findNodeMatchMetric', () => {
+    const fun = SearchUtils.findNodeMatchMetric;
+
+    it('does not add a non-matching field', () => {
+      let matches = fromJS({});
+      matches = fun(matches, ['node1', 'field1'],
+        1, 'metric1', 'metric2', 'lt', 2);
+      expect(matches.size).toBe(0);
+    });
+
+    it('adds a matching field', () => {
+      let matches = fromJS({});
+      matches = fun(matches, ['node1', 'field1'],
+        1, 'metric1', 'metric1', 'lt', 2);
+      expect(matches.size).toBe(1);
+      expect(matches.getIn(['node1', 'field1'])).toBeDefined();
+      const { metric } = matches.getIn(['node1', 'field1']);
+      expect(metric).toBeTruthy();
+
+      matches = fun(matches, ['node2', 'field1'],
+        1, 'metric1', 'metric1', 'gt', 0);
+      expect(matches.size).toBe(2);
+
+      matches = fun(matches, ['node3', 'field1'],
+        1, 'metric1', 'metric1', 'eq', 1);
+      expect(matches.size).toBe(3);
+
+      matches = fun(matches, ['node3', 'field1'],
+        1, 'metric1', 'metric1', 'other', 1);
+      expect(matches.size).toBe(3);
+    });
+  });
+
+  describe('makeRegExp', () => {
+    const fun = SearchUtils.makeRegExp;
+
+    it('should make a regexp from any string', () => {
+      expect(fun().source).toEqual((new RegExp).source);
+      expect(fun('que').source).toEqual((new RegExp('que')).source);
+      // invalid string
+      expect(fun('que[').source).toEqual((new RegExp('que\\[')).source);
+    });
+  });
+
+  describe('matchPrefix', () => {
+    const fun = SearchUtils.matchPrefix;
+
+    it('returns true if the prefix matches the label', () => {
+      expect(fun('label', 'prefix')).toBeFalsy();
+      expect(fun('memory', 'mem')).toBeTruthy();
+      expect(fun('mem', 'memory')).toBeFalsy();
+      expect(fun('com.domain.label', 'label')).toBeTruthy();
+      expect(fun('com.domain.Label', 'domainlabel')).toBeTruthy();
+      expect(fun('com-Domain-label', 'domainlabel')).toBeTruthy();
+      expect(fun('memory', 'mem.ry')).toBeTruthy();
+    });
+  });
+
+  describe('parseQuery', () => {
+    const fun = SearchUtils.parseQuery;
+
+    it('should parse a metric value from a string', () => {
+      expect(fun('')).toEqual(null);
+      expect(fun('text')).toEqual({query: 'text'});
+      expect(fun('prefix:text')).toEqual({prefix: 'prefix', query: 'text'});
+      expect(fun(':text')).toEqual(null);
+      expect(fun('text:')).toEqual(null);
+      expect(fun('cpu > 1')).toEqual({metric: 'cpu', value: 1, comp: 'gt'});
+      expect(fun('cpu >')).toEqual(null);
+    });
+  });
+
+  describe('parseValue', () => {
+    const fun = SearchUtils.parseValue;
+
+    it('should parse a metric value from a string', () => {
+      expect(fun('1')).toEqual(1);
+      expect(fun('1.34%')).toEqual(1.34);
+      expect(fun('10kB')).toEqual(1024 * 10);
+      expect(fun('1K')).toEqual(1024);
+      expect(fun('2KB')).toEqual(2048);
+      expect(fun('1MB')).toEqual(Math.pow(1024, 2));
+      expect(fun('1m')).toEqual(Math.pow(1024, 2));
+      expect(fun('1GB')).toEqual(Math.pow(1024, 3));
+      expect(fun('1TB')).toEqual(Math.pow(1024, 4));
+    });
+  });
+
+  describe('searchTopology', () => {
+    const fun = SearchUtils.searchTopology;
+
+    it('should return no matches on an empty topology', () => {
+      const nodes = fromJS({});
+      const matches = fun(nodes, {query: 'value'});
+      expect(matches.size).toEqual(0);
+    });
+
+    it('should match on a node label', () => {
+      const nodes = nodeSets.someNodes;
+      let matches = fun(nodes, {query: 'node label 1'});
+      expect(matches.size).toEqual(1);
+      matches = fun(nodes, {query: 'node label'});
+      expect(matches.size).toEqual(2);
+    });
+
+    it('should match on a metadata field', () => {
+      const nodes = nodeSets.someNodes;
+      const matches = fun(nodes, {query: 'value'});
+      expect(matches.size).toEqual(2);
+      expect(matches.getIn(['n1', 'metadata', 'fieldId1']).text).toEqual('value 1');
+    });
+
+    it('should match on a metric field', () => {
+      const nodes = nodeSets.someNodes;
+      const matches = fun(nodes, {metric: 'metric1', value: 1, comp: 'eq'});
+      expect(matches.size).toEqual(1);
+      expect(matches.getIn(['n1', 'metrics', 'metric1']).metric).toBeTruthy();
+    });
+
+    it('should match on a tables field', () => {
+      const nodes = nodeSets.someNodes;
+      const matches = fun(nodes, {query: 'Row Value 1'});
+      expect(matches.size).toEqual(1);
+      expect(matches.getIn(['n2', 'metadata', 'row1']).text).toBe('Row Value 1');
+    });
+  });
+
+  describe('updateNodeMatches', () => {
+    const fun = SearchUtils.updateNodeMatches;
+
+    it('should return no matches on an empty topology', () => {
+      let nextState = fromJS({
+        nodesByTopology: {},
+        searchNodeMatches: {},
+        searchQuery: ''
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('searchNodeMatches').size).toEqual(0);
+    });
+
+    it('should return no matches when no query is present', () => {
+      let nextState = fromJS({
+        nodesByTopology: {topo1: nodeSets.someNodes},
+        searchNodeMatches: {},
+        searchQuery: ''
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('searchNodeMatches').size).toEqual(0);
+    });
+
+    it('should return no matches when query matches nothing', () => {
+      let nextState = fromJS({
+        nodesByTopology: {topo1: nodeSets.someNodes},
+        searchNodeMatches: {},
+        searchQuery: 'cantmatch'
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('searchNodeMatches').size).toEqual(0);
+    });
+
+    it('should return a matches when a query matches something', () => {
+      let nextState = fromJS({
+        nodesByTopology: {topo1: nodeSets.someNodes},
+        searchNodeMatches: {},
+        searchQuery: 'value 2'
+      });
+      nextState = fun(nextState);
+      expect(nextState.get('searchNodeMatches').size).toEqual(1);
+      expect(nextState.get('searchNodeMatches').get('topo1').size).toEqual(1);
+
+      // then clear up again
+      nextState = nextState.set('searchQuery', '');
+      nextState = fun(nextState);
+      expect(nextState.get('searchNodeMatches').size).toEqual(0);
+    });
+  });
+});

client/app/scripts/utils/router-utils.js

@@ -8,13 +8,18 @@ import { route } from '../actions/app-actions';
 //
 const SLASH = '/';
 const SLASH_REPLACEMENT = '<SLASH>';
+const PERCENT = '%';
+const PERCENT_REPLACEMENT = '<PERCENT>';
 
 function encodeURL(url) {
-  return url.replace(new RegExp(SLASH, 'g'), SLASH_REPLACEMENT);
+  return url
+    .replace(new RegExp(PERCENT, 'g'), PERCENT_REPLACEMENT)
+    .replace(new RegExp(SLASH, 'g'), SLASH_REPLACEMENT);
 }
 
 function decodeURL(url) {
-  return decodeURIComponent(url.replace(new RegExp(SLASH_REPLACEMENT, 'g'), SLASH));
+  return decodeURIComponent(url.replace(new RegExp(SLASH_REPLACEMENT, 'g'), SLASH))
+    .replace(new RegExp(PERCENT_REPLACEMENT, 'g'), PERCENT);
 }
 
 function shouldReplaceState(prevState, nextState) {
@@ -71,7 +76,7 @@ export function getRouter(dispatch, initialState) {
   });
 
   page('/state/:state', (ctx) => {
-    const state = JSON.parse(ctx.params.state);
+    const state = JSON.parse(decodeURL(ctx.params.state));
     dispatch(route(state));
   });
 

client/app/scripts/utils/search-utils.js

@@ -18,6 +18,10 @@ const COMPARISONS_REGEX = new RegExp(`[${COMPARISONS.keySeq().toJS().join('')}]`
 
 const PREFIX_DELIMITER = ':';
 
+/**
+ * Returns a RegExp from a given string. If the string is not a valid regexp,
+ * it is escaped. Returned regexp is case-insensitive.
+ */
 function makeRegExp(expression, options = 'i') {
   try {
     return new RegExp(expression, options);
@@ -26,20 +30,27 @@ function makeRegExp(expression, options = 'i') {
   }
 }
 
+/**
+ * Returns the float of a metric value string, e.g. 2 KB -> 2048
+ */
 function parseValue(value) {
   let parsed = parseFloat(value);
-  if (_.endsWith(value, 'KB')) {
+  if ((/k/i).test(value)) {
     parsed *= 1024;
-  } else if (_.endsWith(value, 'MB')) {
+  } else if ((/m/i).test(value)) {
     parsed *= 1024 * 1024;
-  } else if (_.endsWith(value, 'GB')) {
+  } else if ((/g/i).test(value)) {
     parsed *= 1024 * 1024 * 1024;
-  } else if (_.endsWith(value, 'TB')) {
+  } else if ((/t/i).test(value)) {
     parsed *= 1024 * 1024 * 1024 * 1024;
   }
   return parsed;
 }
 
+/**
+ * True if a prefix matches a field label
+ * Slugifies the label (removes all non-alphanumerical chars).
+ */
 function matchPrefix(label, prefix) {
   if (label && prefix) {
     return (makeRegExp(prefix)).test(slugify(label));
@@ -47,6 +58,12 @@ function matchPrefix(label, prefix) {
   return false;
 }
 
+/**
+ * Adds a match to nodeMatches under the keyPath. The text is matched against
+ * the query. If a prefix is given, it is matched against the label (skip on
+ * no match).
+ * Returns a new instance of nodeMatches.
+ */
 function findNodeMatch(nodeMatches, keyPath, text, query, prefix, label) {
   if (!prefix || matchPrefix(label, prefix)) {
     const queryRe = makeRegExp(query);
@@ -55,7 +72,7 @@ function findNodeMatch(nodeMatches, keyPath, text, query, prefix, label) {
       const firstMatch = matches[0];
       const index = text.search(queryRe);
       nodeMatches = nodeMatches.setIn(keyPath,
-        {text, label, matches: [{start: index, length: firstMatch.length}]});
+        {text, label, start: index, length: firstMatch.length});
     }
   }
   return nodeMatches;
@@ -99,6 +116,7 @@ function findNodeMatchMetric(nodeMatches, keyPath, fieldValue, fieldLabel, metri
   return nodeMatches;
 }
 
+
 export function searchTopology(nodes, { prefix, query, metric, comp, value }) {
   let nodeMatches = makeMap();
   nodes.forEach((node, nodeId) => {
@@ -106,8 +124,10 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) {
       // top level fields
       SEARCH_FIELDS.forEach((field, label) => {
         const keyPath = [nodeId, label];
-        nodeMatches = findNodeMatch(nodeMatches, keyPath, node.get(field),
-          query, prefix, label);
+        if (node.has(field)) {
+          nodeMatches = findNodeMatch(nodeMatches, keyPath, node.get(field),
+            query, prefix, label);
+        }
       });
 
       // metadata
@@ -146,6 +166,12 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) {
   return nodeMatches;
 }
 
+/**
+ * Returns an object with fields depending on the query:
+ * parseQuery('text') -> {query: 'text'}
+ * parseQuery('p:text') -> {query: 'text', prefix: 'p'}
+ * parseQuery('cpu > 1') -> {metric: 'cpu', value: '1', comp: 'gt'}
+ */
 export function parseQuery(query) {
   if (query) {
     const prefixQuery = query.split(PREFIX_DELIMITER);
@@ -195,14 +221,17 @@ export function parseQuery(query) {
 export function updateNodeMatches(state) {
   const parsed = parseQuery(state.get('searchQuery'));
   if (parsed) {
-    state.get('topologyUrlsById').forEach((url, topologyId) => {
-      const topologyNodes = state.getIn(['nodesByTopology', topologyId]);
-      if (topologyNodes) {
-        const nodeMatches = searchTopology(topologyNodes, parsed);
-        state = state.setIn(['searchNodeMatches', topologyId], nodeMatches);
-      }
-    });
-  } else {
+    if (state.has('nodesByTopology')) {
+      state.get('nodesByTopology').forEach((nodes, topologyId) => {
+        const nodeMatches = searchTopology(nodes, parsed);
+        if (nodeMatches.size > 0) {
+          state = state.setIn(['searchNodeMatches', topologyId], nodeMatches);
+        } else {
+          state = state.deleteIn(['searchNodeMatches', topologyId]);
+        }
+      });
+    }
+  } else if (state.has('searchNodeMatches')) {
     state = state.update('searchNodeMatches', snm => snm.clear());
   }
 
@@ -235,3 +264,15 @@ export function applyPinnedSearches(state) {
 
   return state;
 }
+
+export const testable = {
+  applyPinnedSearches,
+  findNodeMatch,
+  findNodeMatchMetric,
+  matchPrefix,
+  makeRegExp,
+  parseQuery,
+  parseValue,
+  searchTopology,
+  updateNodeMatches
+};