From 0e837b61423776db9b4251e3805990fdb66308c7 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Wed, 4 Jan 2017 15:54:22 +0100 Subject: [PATCH] Refactored the table component/model and wrote the tests --- client/app/scripts/components/node-details.js | 12 +- .../node-details-generic-table.js | 12 +- ...abels.js => node-details-property-list.js} | 15 +- .../utils/__tests__/search-utils-test.js | 75 ++++- .../app/scripts/utils/node-details-utils.js | 4 +- client/app/scripts/utils/search-utils.js | 2 +- client/app/styles/main.less | 2 +- probe/docker/container.go | 4 +- probe/docker/reporter.go | 2 +- probe/kubernetes/meta.go | 2 +- probe/overlay/weave.go | 21 +- render/detailed/tables_test.go | 5 +- report/table.go | 197 ++++++++----- report/table_test.go | 268 ++++++++++++++++-- 14 files changed, 477 insertions(+), 144 deletions(-) rename client/app/scripts/components/node-details/{node-details-labels.js => node-details-property-list.js} (80%) diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 387565ea68..ffd99e6a17 100644 --- 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 (
- {table.label.length > 0 && table.label} + {table.label && table.label.length > 0 && table.label} {table.truncationCount > 0 && @@ -242,14 +242,14 @@ class NodeDetails extends React.Component { ); } else if (isPropertyList(table)) { return ( - ); } - 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 index 3478078bea..54e295ad80 100644 --- 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/client/app/scripts/components/node-details/node-details-labels.js b/client/app/scripts/components/node-details/node-details-property-list.js similarity index 80% rename from client/app/scripts/components/node-details/node-details-labels.js rename to client/app/scripts/components/node-details/node-details-property-list.js index e25ec4ccd8..4375248ca3 100644 --- a/client/app/scripts/components/node-details/node-details-labels.js +++ b/client/app/scripts/components/node-details/node-details-property-list.js @@ -8,14 +8,13 @@ import MatchedText from '../matched-text'; import ShowMore from '../show-more'; const Controls = controls => ( -
+
{sortBy(controls, 'rank').map(control => )}
); -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 ( -
+
{controls && Controls(controls)} {rows.map(field => ( -
+
{field.entries.label}
-
+
diff --git a/client/app/scripts/utils/__tests__/search-utils-test.js b/client/app/scripts/utils/__tests__/search-utils-test.js index 1596e3d626..dde59e6799 100644 --- 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 index 7d9c3d2aa4..a7c84bde08 100644 --- 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 index 2ea844c230..aec25f632f 100644 --- 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 index 801ad53db0..b209661cd1 100644 --- 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 index f23ed71fd3..419fb86059 100644 --- 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 index 4aacaca2da..44efb69e41 100644 --- 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 index 20fb213bb5..fc8188ca3d 100644 --- 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 index fde8ccf869..b87cb88eb6 100644 --- 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 index 634755c545..ab847d2761 100644 --- 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{}, }, diff --git a/report/table.go b/report/table.go index e8e0f8297e..1ee22fec46 100644 --- a/report/table.go +++ b/report/table.go @@ -13,98 +13,135 @@ import ( // MaxTableRows sets the limit on the table size to render // TODO: this won't be needed once we send reports incrementally const ( - MaxTableRows = 20 - TruncationCountPrefix = "table_truncation_count_" - MulticolumnTableType = "multicolumn-table" - PropertyListType = "property-list" + MaxTableRows = 20 + TableEntryKeySeparator = "___" + TruncationCountPrefix = "table_truncation_count_" + MulticolumnTableType = "multicolumn-table" + PropertyListType = "property-list" ) -// AddPrefixTable appends arbitrary rows to the Node, returning a new node. -func (node Node) AddPrefixTable(prefix string, rows []Row) Node { - count := 0 +// WithTableTruncationInformation appends table truncation info to the node, returning the new node. +func (node Node) WithTableTruncationInformation(prefix string, totalRowsCount int) Node { + if totalRowsCount > MaxTableRows { + truncationCount := fmt.Sprintf("%d", totalRowsCount-MaxTableRows) + node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) + } + return node +} + +// AddPrefixMulticolumnTable appends arbitrary rows to the Node, returning a new node. +func (node Node) AddPrefixMulticolumnTable(prefix string, rows []Row) Node { + addedRowsCount := 0 for _, row := range rows { - if count >= MaxTableRows { + if addedRowsCount >= MaxTableRows { break } - // TODO: Figure a more natural way of storing rows - for column, value := range row.Entries { - key := fmt.Sprintf("%s %s", row.ID, column) + // Add all the row values as separate entries + for columnID, value := range row.Entries { + key := strings.Join([]string{row.ID, columnID}, TableEntryKeySeparator) node = node.WithLatest(prefix+key, mtime.Now(), value) } - count++ + addedRowsCount++ } - if len(rows) > MaxTableRows { - truncationCount := fmt.Sprintf("%d", len(rows)-MaxTableRows) - node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) - } - return node + return node.WithTableTruncationInformation(prefix, len(rows)) } -// AddPrefixLabels appends arbitrary key-value pairs to the Node, returning a new node. -func (node Node) AddPrefixLabels(prefix string, labels map[string]string) Node { - count := 0 - for key, value := range labels { - if count >= MaxTableRows { +// AddPrefixPropertyList appends arbitrary key-value pairs to the Node, returning a new node. +func (node Node) AddPrefixPropertyList(prefix string, propertyList map[string]string) Node { + addedPropertiesCount := 0 + for label, value := range propertyList { + if addedPropertiesCount >= MaxTableRows { break } - node = node.WithLatest(prefix+key, mtime.Now(), value) - count++ - } - if len(labels) > MaxTableRows { - truncationCount := fmt.Sprintf("%d", len(labels)-MaxTableRows) - node = node.WithLatest(TruncationCountPrefix+prefix, mtime.Now(), truncationCount) + node = node.WithLatest(prefix+label, mtime.Now(), value) + addedPropertiesCount++ } - return node + return node.WithTableTruncationInformation(prefix, len(propertyList)) } -// ExtractTable returns the rows to build a table from this node -func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCount int) { - rows = []Row{} - switch template.Type { - case MulticolumnTableType: - keyRows := map[string]Row{} - node.Latest.ForEach(func(key string, _ time.Time, value string) { - if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { - rowID, column := "", "" - fmt.Sscanf(key[len(template.Prefix):], "%s %s", &rowID, &column) - if _, ok := keyRows[rowID]; !ok { - keyRows[rowID] = Row{ - ID: rowID, - Entries: map[string]string{}, - } +// WithoutPrefix returns the string with trimmed prefix and a +// boolean information of whether that prefix was really there. +// NOTE: Consider moving this function to utilities. +func WithoutPrefix(s string, prefix string) (string, bool) { + return strings.TrimPrefix(s, prefix), len(prefix) > 0 && strings.HasPrefix(s, prefix) +} + +// ExtractMulticolumnTable returns the rows to build a multicolumn table from this node +func (node Node) ExtractMulticolumnTable(template TableTemplate) (rows []Row) { + rowsMapByID := map[string]Row{} + + // Itearate through the whole of our map to extract all the values with the key + // with the given prefix. Since multicolumn tables don't support fixed rows (yet), + // all the table values will be stored under the table prefix. + // NOTE: It would be nice to optimize this part by only iterating through the keys + // with the given prefix. If it is possible to traverse the keys in the Latest map + // in a sorted order, then having LowerBoundEntry(key) and UpperBoundEntry(key) + // methods should be enough to implement ForEachWithPrefix(prefix) straightforwardly. + node.Latest.ForEach(func(key string, _ time.Time, value string) { + if keyWithoutPrefix, ok := WithoutPrefix(key, template.Prefix); ok { + ids := strings.Split(keyWithoutPrefix, TableEntryKeySeparator) + rowID, columnID := ids[0], ids[1] + // If the row with the given ID doesn't yet exist, we create an empty one. + if _, ok := rowsMapByID[rowID]; !ok { + rowsMapByID[rowID] = Row{ + ID: rowID, + Entries: map[string]string{}, } - keyRows[rowID].Entries[column] = value - } - }) - for _, row := range keyRows { - rows = append(rows, row) - } - // By default assume it's a property list (for backward compatibility) - default: - keyValues := map[string]string{} - node.Latest.ForEach(func(key string, _ time.Time, value string) { - if label, ok := template.FixedRows[key]; ok { - keyValues[label] = value } - if len(template.Prefix) > 0 && strings.HasPrefix(key, template.Prefix) { - label := key[len(template.Prefix):] - keyValues[label] = value - } - }) - labels := make([]string, 0, len(rows)) - for label := range keyValues { - labels = append(labels, label) + // At this point, the row with that ID always exists, so we just update the value. + rowsMapByID[rowID].Entries[columnID] = value } - sort.Strings(labels) - for _, label := range labels { - rows = append(rows, Row{ - ID: "label_" + label, - Entries: map[string]string{ - "label": label, - "value": keyValues[label], - }, - }) + }) + + // Gather a list of rows. + rows = make([]Row, 0, len(rowsMapByID)) + for _, row := range rowsMapByID { + rows = append(rows, row) + } + + // Return the rows sorted by ID. + sort.Sort(rowsByID(rows)) + return rows +} + +// ExtractPropertyList returns the rows to build a property list from this node +func (node Node) ExtractPropertyList(template TableTemplate) (rows []Row) { + valuesMapByLabel := map[string]string{} + + // Itearate through the whole of our map to extract all the values with the key + // with the given prefix as well as the keys corresponding to the fixed table rows. + node.Latest.ForEach(func(key string, _ time.Time, value string) { + if label, ok := template.FixedRows[key]; ok { + valuesMapByLabel[label] = value + } else if label, ok := WithoutPrefix(key, template.Prefix); ok { + valuesMapByLabel[label] = value } + }) + + // Gather a label-value formatted list of rows. + rows = make([]Row, 0, len(valuesMapByLabel)) + for label, value := range valuesMapByLabel { + rows = append(rows, Row{ + ID: "label_" + label, + Entries: map[string]string{ + "label": label, + "value": value, + }, + }) + } + + // Return the rows sorted by ID. + sort.Sort(rowsByID(rows)) + return rows +} + +// ExtractTable returns the rows to build either a property list or a generic table from this node +func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCount int) { + switch template.Type { + case MulticolumnTableType: + rows = node.ExtractMulticolumnTable(template) + default: // By default assume it's a property list (for backward compatibility). + rows = node.ExtractPropertyList(template) } truncationCount = 0 @@ -117,17 +154,25 @@ func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCou return rows, truncationCount } +// Column is the type for multi-column tables in the UI. type Column struct { ID string `json:"id"` Label string `json:"label"` DataType string `json:"dataType"` } +// Row is the type that holds the table data for the UI. Entries map from column ID to cell value. type Row struct { ID string `json:"id"` Entries map[string]string `json:"entries"` } +type rowsByID []Row + +func (t rowsByID) Len() int { return len(t) } +func (t rowsByID) Swap(i, j int) { t[i], t[j] = t[j], t[i] } +func (t rowsByID) Less(i, j int) bool { return t[i].ID < t[j].ID } + // Copy returns a copy of the Row. func (r Row) Copy() Row { entriesCopy := make(map[string]string, len(r.Entries)) @@ -206,18 +251,18 @@ func (t TableTemplate) Merge(other TableTemplate) TableTemplate { return s2 } + // NOTE: Consider actually merging the columns and fixed rows. fixedRows := t.FixedRows if len(other.FixedRows) > len(fixedRows) { fixedRows = other.FixedRows } - columns := t.Columns if len(other.Columns) > len(columns) { columns = other.Columns } - // TODO: Refactor the merging logic, as mixing - // the types now might result in invalid tables. + // TODO: Refactor the merging logic, as mixing the types now might result in + // invalid tables. Maybe we should return an error if the types are different? return TableTemplate{ ID: max(t.ID, other.ID), Label: max(t.Label, other.Label), diff --git a/report/table_test.go b/report/table_test.go index 017762a7a5..fa94d5d8c5 100644 --- a/report/table_test.go +++ b/report/table_test.go @@ -9,15 +9,34 @@ import ( "github.com/weaveworks/scope/report" ) -func TestPrefixTables(t *testing.T) { - want := map[string]string{ - "foo1": "bar1", - "foo2": "bar2", +func TestMulticolumnTables(t *testing.T) { + want := []report.Row{ + { + ID: "row1", + Entries: map[string]string{ + "col1": "r1c1", + "col2": "r1c2", + "col3": "r1c3", + }, + }, + { + ID: "row2", + Entries: map[string]string{ + "col1": "r2c1", + "col3": "r2c3", + }, + }, } + nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixMulticolumnTable("foo_", want) + + template := report.TableTemplate{ + Type: report.MulticolumnTableType, + Prefix: "foo_", + } - nmd = nmd.AddPrefixLabels("foo_", want) - have, truncationCount := nmd.ExtractTable(report.TableTemplate{Prefix: "foo_"}) + have, truncationCount := nmd.ExtractTable(template) if truncationCount != 0 { t.Error("Table shouldn't had been truncated") @@ -28,49 +47,258 @@ func TestPrefixTables(t *testing.T) { } } -func TestFixedTables(t *testing.T) { - want := map[string]string{ +func TestPrefixPropertyLists(t *testing.T) { + want := []report.Row{ + { + ID: "label_foo1", + Entries: map[string]string{ + "label": "foo1", + "value": "bar1", + }, + }, + { + ID: "label_foo3", + Entries: map[string]string{ + "label": "foo3", + "value": "bar3", + }, + }, + } + + nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixPropertyList("foo_", map[string]string{ + "foo3": "bar3", "foo1": "bar1", + }) + nmd = nmd.AddPrefixPropertyList("zzz_", map[string]string{ "foo2": "bar2", + }) + + template := report.TableTemplate{ + Type: report.PropertyListType, + Prefix: "foo_", + } + + have, truncationCount := nmd.ExtractTable(template) + + if truncationCount != 0 { + t.Error("Table shouldn't had been truncated") + } + + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +} + +func TestFixedPropertyLists(t *testing.T) { + want := []report.Row{ + { + ID: "label_foo1", + Entries: map[string]string{ + "label": "foo1", + "value": "bar1", + }, + }, + { + ID: "label_foo2", + Entries: map[string]string{ + "label": "foo2", + "value": "bar2", + }, + }, } + nmd := report.MakeNodeWith("foo1", map[string]string{ - "foo1key": "bar1", "foo2key": "bar2", + "foo1key": "bar1", }) - template := report.TableTemplate{FixedRows: map[string]string{ - "foo1key": "foo1", - "foo2key": "foo2", - }, + template := report.TableTemplate{ + Type: report.PropertyListType, + FixedRows: map[string]string{ + "foo2key": "foo2", + "foo1key": "foo1", + }, } - have, _ := nmd.ExtractTable(template) + have, truncationCount := nmd.ExtractTable(template) + + if truncationCount != 0 { + t.Error("Table shouldn't had been truncated") + } if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } } -func TestTruncation(t *testing.T) { +func TestPropertyListTruncation(t *testing.T) { wantTruncationCount := 1 - want := map[string]string{} + propertyList := map[string]string{} for i := 0; i < report.MaxTableRows+wantTruncationCount; i++ { key := fmt.Sprintf("key%d", i) value := fmt.Sprintf("value%d", i) - want[key] = value + propertyList[key] = value } nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixPropertyList("foo_", propertyList) + + template := report.TableTemplate{ + Type: report.PropertyListType, + Prefix: "foo_", + } - nmd = nmd.AddPrefixLabels("foo_", want) - _, truncationCount := nmd.ExtractTable(report.TableTemplate{Prefix: "foo_"}) + _, truncationCount := nmd.ExtractTable(template) if truncationCount != wantTruncationCount { t.Error( - "Table should had been truncated by", + "Property list should had been truncated by", wantTruncationCount, "and not", truncationCount, ) } } + +func TestMulticolumnTableTruncation(t *testing.T) { + wantTruncationCount := 1 + rows := []report.Row{} + for i := 0; i < report.MaxTableRows+wantTruncationCount; i++ { + rowID := fmt.Sprintf("row%d", i) + colID := fmt.Sprintf("col%d", i) + value := fmt.Sprintf("value%d", i) + rows = append(rows, report.Row{ + ID: rowID, + Entries: map[string]string{ + colID: value, + }, + }) + } + + nmd := report.MakeNode("foo1") + nmd = nmd.AddPrefixMulticolumnTable("foo_", rows) + + template := report.TableTemplate{ + Type: report.MulticolumnTableType, + Prefix: "foo_", + } + + _, truncationCount := nmd.ExtractTable(template) + + if truncationCount != wantTruncationCount { + t.Error( + "Property list should had been truncated by", + wantTruncationCount, + "and not", + truncationCount, + ) + } +} + +func TestTables(t *testing.T) { + want := []report.Table{ + { + ID: "AAA", + Label: "Aaa", + Type: report.PropertyListType, + Columns: nil, + Rows: []report.Row{ + { + ID: "label_foo1", + Entries: map[string]string{ + "label": "foo1", + "value": "bar1", + }, + }, + { + ID: "label_foo3", + Entries: map[string]string{ + "label": "foo3", + "value": "bar3", + }, + }, + }, + }, + { + ID: "BBB", + Label: "Bbb", + Type: report.MulticolumnTableType, + Columns: []report.Column{{ID: "col1", Label: "Column 1"}}, + Rows: []report.Row{ + { + ID: "row1", + Entries: map[string]string{ + "col1": "r1c1", + }, + }, + { + ID: "row2", + Entries: map[string]string{ + "col3": "r2c3", + }, + }, + }, + }, + { + ID: "CCC", + Label: "Ccc", + Type: report.PropertyListType, + Columns: nil, + Rows: []report.Row{ + { + ID: "label_foo3", + Entries: map[string]string{ + "label": "foo3", + "value": "bar3", + }, + }, + }, + }, + } + + nmd := report.MakeNodeWith("foo1", map[string]string{ + "foo3key": "bar3", + "foo1key": "bar1", + }) + nmd = nmd.AddPrefixMulticolumnTable("bbb_", []report.Row{ + {ID: "row1", Entries: map[string]string{"col1": "r1c1"}}, + {ID: "row2", Entries: map[string]string{"col3": "r2c3"}}, + }) + nmd = nmd.AddPrefixPropertyList("aaa_", map[string]string{ + "foo3": "bar3", + "foo1": "bar1", + }) + + aaaTemplate := report.TableTemplate{ + ID: "AAA", + Label: "Aaa", + Prefix: "aaa_", + Type: report.PropertyListType, + } + bbbTemplate := report.TableTemplate{ + ID: "BBB", + Label: "Bbb", + Prefix: "bbb_", + Type: report.MulticolumnTableType, + Columns: []report.Column{{ID: "col1", Label: "Column 1"}}, + } + cccTemplate := report.TableTemplate{ + ID: "CCC", + Label: "Ccc", + Prefix: "ccc_", + Type: report.PropertyListType, + FixedRows: map[string]string{"foo3key": "foo3"}, + } + templates := report.TableTemplates{ + aaaTemplate.ID: aaaTemplate, + bbbTemplate.ID: bbbTemplate, + cccTemplate.ID: cccTemplate, + } + + have := templates.Tables(nmd) + + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +}