Refactored the table component/model and wrote the tests

weaveworks · Jan 16, 2017 · 0e837b6 · 0e837b6
1 parent cc4c116
commit 0e837b6
Show file tree

Hide file tree

Showing 14 changed files with 477 additions and 144 deletions.
diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js
@@ -11,15 +11,15 @@ import { resetDocumentTitle, setDocumentTitle } from '../utils/title-utils';
 import MatchedText from './matched-text';
 import NodeDetailsControls from './node-details/node-details-controls';
 import NodeDetailsGenericTable from './node-details/node-details-generic-table';
+import NodeDetailsPropertyList from './node-details/node-details-property-list';
 import NodeDetailsHealth from './node-details/node-details-health';
 import NodeDetailsInfo from './node-details/node-details-info';
-import NodeDetailsLabels from './node-details/node-details-labels';
 import NodeDetailsRelatives from './node-details/node-details-relatives';
 import NodeDetailsTable from './node-details/node-details-table';
 import Warning from './warning';
 
 
-const logError = debug('scope:error');
+const log = debug('scope:node-details');
 
 function getTruncationText(count) {
   return 'This section was too long to be handled efficiently and has been truncated'
@@ -213,7 +213,7 @@ class NodeDetails extends React.Component {
               return (
                 <div className="node-details-content-section" key={table.id}>
                   <div className="node-details-content-section-header">
-                    {table.label.length > 0 && table.label}
+                    {table.label && table.label.length > 0 && table.label}
                     {table.truncationCount > 0 && <span
                       className="node-details-content-section-header-warning">
                       <Warning text={getTruncationText(table.truncationCount)} />
@@ -242,14 +242,14 @@ class NodeDetails extends React.Component {
       );
     } else if (isPropertyList(table)) {
       return (
-        <NodeDetailsLabels
+        <NodeDetailsPropertyList
           rows={table.rows} controls={table.controls}
-          matches={nodeMatches.get('labels')}
+          matches={nodeMatches.get('property-lists')}
         />
       );
     }
 
-    logError(`Undefined type '${table.type}' for table ${table.id}`);
+    log(`Undefined type '${table.type}' for table ${table.id}`);
     return null;
   }
 

diff --git a/client/app/scripts/components/node-details/node-details-generic-table.js b/client/app/scripts/components/node-details/node-details-generic-table.js
@@ -1,10 +1,13 @@
 import React from 'react';
+import sortBy from 'lodash/sortBy';
 import { Map as makeMap } from 'immutable';
-import { sortBy } from 'lodash';
 
 import { NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT } from '../../constants/limits';
+
 import {
-  isNumber, getTableColumnsStyles, genericTableEntryKey
+  isNumber,
+  getTableColumnsStyles,
+  genericTableEntryKey
 } from '../../utils/node-details-utils';
 import NodeDetailsTableHeaders from './node-details-table-headers';
 import MatchedText from '../matched-text';
@@ -31,7 +34,7 @@ export default class NodeDetailsGenericTable extends React.Component {
     super(props, context);
     this.state = {
       limit: NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT,
-      sortedBy: props.columns[0].id,
+      sortedBy: props.columns && props.columns[0].id,
       sortedDesc: true
     };
     this.handleLimitClick = this.handleLimitClick.bind(this);
@@ -53,8 +56,7 @@ export default class NodeDetailsGenericTable extends React.Component {
     const { columns, matches = makeMap() } = this.props;
     const expanded = this.state.limit === 0;
 
-    // Stabilize the order of rows
-    let rows = sortBy(this.props.rows || [], row => row.id);
+    let rows = this.props.rows || [];
     let notShown = 0;
 
     // If there are rows that would be hidden behind 'show more', keep them

diff --git a/...nents/node-details/node-details-labels.js → ...ode-details/node-details-property-list.js b/...nents/node-details/node-details-labels.js → ...ode-details/node-details-property-list.js
@@ -8,14 +8,13 @@ import MatchedText from '../matched-text';
 import ShowMore from '../show-more';
 
 const Controls = controls => (
-  <div className="node-details-labels-controls">
+  <div className="node-details-property-list-controls">
     {sortBy(controls, 'rank').map(control => <NodeDetailsControlButton
       nodeId={control.nodeId} control={control} key={control.id} />)}
   </div>
 );
 
-export default class NodeDetailsLabels extends React.Component {
-
+export default class NodeDetailsPropertyList extends React.Component {
   constructor(props, context) {
     super(props, context);
     this.state = {
@@ -45,16 +44,18 @@ export default class NodeDetailsLabels extends React.Component {
     }
 
     return (
-      <div className="node-details-labels">
+      <div className="node-details-property-list">
         {controls && Controls(controls)}
         {rows.map(field => (
-          <div className="node-details-labels-field" key={field.id}>
+          <div className="node-details-property-list-field" key={field.id}>
             <div
-              className="node-details-labels-field-label truncate"
+              className="node-details-property-list-field-label truncate"
               title={field.entries.label} key={field.id}>
               {field.entries.label}
             </div>
-            <div className="node-details-labels-field-value truncate" title={field.entries.value}>
+            <div
+              className="node-details-property-list-field-value truncate"
+              title={field.entries.value}>
               <MatchedText text={field.entries.value} match={matches.get(field.id)} />
             </div>
           </div>

diff --git a/client/app/scripts/utils/__tests__/search-utils-test.js b/client/app/scripts/utils/__tests__/search-utils-test.js
@@ -29,17 +29,50 @@ describe('SearchUtils', () => {
         }],
         tables: [{
           id: 'metric1',
+          type: 'property-list',
           rows: [{
+            id: 'label1',
             entries: {
-              id: 'row1',
-              label: 'Row 1',
-              value: 'Row Value 1'
+              label: 'Label 1',
+              value: 'Label Value 1'
             }
           }, {
+            id: 'label2',
             entries: {
-              id: 'row2',
-              label: 'Row 2',
-              value: 'Row Value 2'
+              label: 'Label 2',
+              value: 'Label Value 2'
+            }
+          }]
+        }, {
+          id: 'metric2',
+          type: 'multicolumn-table',
+          columns: [{
+            id: 'a',
+            label: 'A'
+          }, {
+            id: 'c',
+            label: 'C'
+          }],
+          rows: [{
+            id: 'row1',
+            entries: {
+              a: 'xxxa',
+              b: 'yyya',
+              c: 'zzz1'
+            }
+          }, {
+            id: 'row2',
+            entries: {
+              a: 'yyyb',
+              b: 'xxxb',
+              c: 'zzz2'
+            }
+          }, {
+            id: 'row3',
+            entries: {
+              a: 'Value 1',
+              b: 'Value 2',
+              c: 'Value 3'
             }
           }]
         }],
@@ -80,7 +113,7 @@ describe('SearchUtils', () => {
     it('should filter nodes that do not match a pinned searches', () => {
       let nextState = fromJS({
         nodes: nodeSets.someNodes,
-        pinnedSearches: ['row']
+        pinnedSearches: ['Label Value 1']
       });
       nextState = fun(nextState);
       expect(nextState.get('nodes').filter(node => node.get('filtered')).size).toEqual(1);
@@ -244,11 +277,31 @@ describe('SearchUtils', () => {
       expect(matches.getIn(['n1', 'metrics', 'metric1']).metric).toBeTruthy();
     });
 
-    it('should match on a tables field', () => {
+    it('should match on a property list value', () => {
       const nodes = nodeSets.someNodes;
-      const matches = fun(nodes, {query: 'Row Value 1'});
-      expect(matches.size).toEqual(1);
-      expect(matches.getIn(['n2', 'tables', 'row1']).text).toBe('Row Value 1');
+      const matches = fun(nodes, {query: 'Value 1'});
+      expect(matches.size).toEqual(2);
+      expect(matches.getIn(['n2', 'property-lists']).size).toEqual(1);
+      expect(matches.getIn(['n2', 'property-lists', 'label1']).text).toBe('Label Value 1');
+    });
+
+    it('should match on a generic table values', () => {
+      const nodes = nodeSets.someNodes;
+      const matches1 = fun(nodes, {query: 'xx'}).getIn(['n2', 'tables']);
+      const matches2 = fun(nodes, {query: 'yy'}).getIn(['n2', 'tables']);
+      const matches3 = fun(nodes, {query: 'zz'}).getIn(['n2', 'tables']);
+      const matches4 = fun(nodes, {query: 'a'}).getIn(['n2', 'tables']);
+      expect(matches1.size).toEqual(1);
+      expect(matches2.size).toEqual(1);
+      expect(matches3.size).toEqual(2);
+      expect(matches4.size).toEqual(3);
+      expect(matches1.get('row1_a').text).toBe('xxxa');
+      expect(matches2.get('row2_a').text).toBe('yyyb');
+      expect(matches3.get('row1_c').text).toBe('zzz1');
+      expect(matches3.get('row2_c').text).toBe('zzz2');
+      expect(matches4.get('row1_a').text).toBe('xxxa');
+      expect(matches4.get('row3_a').text).toBe('Value 1');
+      expect(matches4.get('row3_c').text).toBe('Value 3');
     });
   });
 

diff --git a/client/app/scripts/utils/node-details-utils.js b/client/app/scripts/utils/node-details-utils.js
@@ -9,11 +9,11 @@ export function isPropertyList(table) {
 }
 
 export function isNumber(data) {
-  return data.dataType && data.dataType === 'number';
+  return data && data.dataType && data.dataType === 'number';
 }
 
 export function isIP(data) {
-  return data.dataType && data.dataType === 'ip';
+  return data && data.dataType && data.dataType === 'ip';
 }
 
 export function genericTableEntryKey(row, column) {

diff --git a/client/app/scripts/utils/search-utils.js b/client/app/scripts/utils/search-utils.js
@@ -153,7 +153,7 @@ export function searchTopology(nodes, { prefix, query, metric, comp, value }) {
       (node.get('tables') || []).filter(isPropertyList).forEach((propertyList) => {
         (propertyList.get('rows') || []).forEach((row) => {
           const entries = row.get('entries');
-          const keyPath = [nodeId, 'labels', row.get('id')];
+          const keyPath = [nodeId, 'property-lists', row.get('id')];
           nodeMatches = findNodeMatch(nodeMatches, keyPath, entries.get('value'),
             query, prefix, entries.get('label'));
         });

diff --git a/client/app/styles/main.less b/client/app/styles/main.less
@@ -864,7 +864,7 @@ h2 {
     }
   }
 
-  &-labels {
+  &-property-list {
     &-controls {
       margin-left: -4px;
     }

diff --git a/probe/docker/container.go b/probe/docker/container.go
@@ -379,8 +379,8 @@ func (c *container) getBaseNode() report.Node {
 	}).WithParents(report.EmptySets.
 		Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))),
 	)
-	result = result.AddPrefixLabels(LabelPrefix, c.container.Config.Labels)
-	result = result.AddPrefixLabels(EnvPrefix, c.env())
+	result = result.AddPrefixPropertyList(LabelPrefix, c.container.Config.Labels)
+	result = result.AddPrefixPropertyList(EnvPrefix, c.env())
 	return result
 }
 

diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go
@@ -276,7 +276,7 @@ func (r *Reporter) containerImageTopology() report.Topology {
 		}
 		nodeID := report.MakeContainerImageNodeID(imageID)
 		node := report.MakeNodeWith(nodeID, latests)
-		node = node.AddPrefixLabels(ImageLabelPrefix, image.Labels)
+		node = node.AddPrefixPropertyList(ImageLabelPrefix, image.Labels)
 		result.AddNode(node)
 	})
 

diff --git a/probe/kubernetes/meta.go b/probe/kubernetes/meta.go
@@ -56,5 +56,5 @@ func (m meta) MetaNode(id string) report.Node {
 		Name:      m.Name(),
 		Namespace: m.Namespace(),
 		Created:   m.Created(),
-	}).AddPrefixLabels(LabelPrefix, m.Labels())
+	}).AddPrefixPropertyList(LabelPrefix, m.Labels())
 }
diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go
@@ -116,31 +116,32 @@ var (
 				WeavePluginDriver: "Driver Name",
 			},
 		},
-		WeaveConnectionsTablePrefix: {
-			ID:     WeaveConnectionsTablePrefix,
-			Label:  "Connections (old)",
-			Type:   report.PropertyListType,
-			Prefix: WeaveConnectionsTablePrefix,
-		},
 		WeaveConnectionsMulticolumnTablePrefix: {
 			ID:     WeaveConnectionsMulticolumnTablePrefix,
 			Type:   report.MulticolumnTableType,
 			Prefix: WeaveConnectionsMulticolumnTablePrefix,
 			Columns: []report.Column{
-				report.Column{
+				{
 					ID:    WeaveConnectionsConnection,
 					Label: "Connections",
 				},
-				report.Column{
+				{
 					ID:    WeaveConnectionsState,
 					Label: "State",
 				},
-				report.Column{
+				{
 					ID:    WeaveConnectionsInfo,
 					Label: "Info",
 				},
 			},
 		},
+		// Kept for backward-compatibility.
+		WeaveConnectionsTablePrefix: {
+			ID:     WeaveConnectionsTablePrefix,
+			Label:  "Connections",
+			Type:   report.PropertyListType,
+			Prefix: WeaveConnectionsTablePrefix,
+		},
 	}
 )
 
@@ -470,7 +471,7 @@ func (w *Weave) addCurrentPeerInfo(latests map[string]string, node report.Node)
 		latests[WeavePluginStatus] = "running"
 		latests[WeavePluginDriver] = "weave"
 	}
-	node = node.AddPrefixTable(WeaveConnectionsMulticolumnTablePrefix, getConnectionsTable(w.statusCache.Router))
+	node = node.AddPrefixMulticolumnTable(WeaveConnectionsMulticolumnTablePrefix, getConnectionsTable(w.statusCache.Router))
 	node = node.WithParents(report.EmptySets.Add(report.Host, report.MakeStringSet(w.hostID)))
 
 	return latests, node

diff --git a/render/detailed/tables_test.go b/render/detailed/tables_test.go
@@ -34,16 +34,18 @@ func TestNodeTables(t *testing.T) {
 			want: []report.Table{
 				{
 					ID:    docker.EnvPrefix,
+					Type:  report.PropertyListType,
 					Label: "Environment Variables",
 					Rows:  []report.Row{},
 				},
 				{
 					ID:    docker.LabelPrefix,
+					Type:  report.PropertyListType,
 					Label: "Docker Labels",
 					Rows: []report.Row{
 						{
+							ID: "label_label1",
 							Entries: map[string]string{
-								"id":    "label_label1",
 								"label": "label1",
 								"value": "label1value",
 							},
@@ -52,6 +54,7 @@ func TestNodeTables(t *testing.T) {
 				},
 				{
 					ID:    docker.ImageTableID,
+					Type:  report.PropertyListType,
 					Label: "Image",
 					Rows:  []report.Row{},
 				},