From aed496a06941c0bae8eb3cf997013d5bbe6638cf Mon Sep 17 00:00:00 2001 From: Joe Fiorini Date: Thu, 11 Jun 2015 12:22:58 -0400 Subject: [PATCH 1/5] Add tests for flat fact comparison --- .../js/system-tracking/compare-facts/flat.js | 4 +- .../compare-facts/flat-test.js | 127 ++++++++++++++++++ 2 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js diff --git a/awx/ui/static/js/system-tracking/compare-facts/flat.js b/awx/ui/static/js/system-tracking/compare-facts/flat.js index 1995caa33f..00ae634340 100644 --- a/awx/ui/static/js/system-tracking/compare-facts/flat.js +++ b/awx/ui/static/js/system-tracking/compare-facts/flat.js @@ -75,7 +75,7 @@ export default value1: slottedValues.left, value1IsAbsent: slottedValues.left === 'absent', value2: slottedValues.right, - value2IsAbsent: slotFactValues.right === 'absent' + value2IsAbsent: slottedValues.right === 'absent' }; }); } @@ -113,7 +113,7 @@ export default if (slottedValues.left !== slottedValues.right) { return { - keyName: key, + keyName: basisFact[nameKey], value1: slottedValues.left, value2: slottedValues.right }; diff --git a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js new file mode 100644 index 0000000000..52b0933a63 --- /dev/null +++ b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js @@ -0,0 +1,127 @@ +/* jshint node: true */ +/* globals -expect, -_ */ + +import compareFacts from 'tower/system-tracking/compare-facts/flat'; + +var expect = require('chai').expect; +var _ = require('lodash'); + +global._ = _; + +describe('CompareFacts.Flat', function() { + + it('returns empty array with empty basis facts', function() { + var result = compareFacts({ facts: [] }, { facts: [] }); + + expect(result).to.deep.equal([]); + }); + + it('returns empty array when no differences', function() { + var result = compareFacts( + { facts: + [{ 'name': 'foo', + 'value': 'bar' + }] + }, + { facts: + [{ 'name': 'foo', + 'value': 'bar' + }] + }, 'name', ['value']); + + expect(result).to.deep.equal([]); + }); + + context('when both collections contain facts', function() { + it('includes each fact value when a compareKey differs', function() { + var result = compareFacts( + { position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'baz' + }] + }, 'name', ['value']); + + expect(result).to.deep.equal( + [{ displayKeyPath: 'foo', + nestingLevel: 0, + facts: + [{ keyName: 'foo', + value1: 'bar', + value2: 'baz' + }] + }]); + }); + }); + + context('when value for nameKey is present in one collection but not the other', function() { + + function factData(leftFacts) { + var facts = [{ position: 'left', + facts: leftFacts + }, + { position: 'right', + facts: [] + }]; + + return facts; + } + + beforeEach(function () { + + }); + + it('uses "absent" for the missing value', function() { + + var facts = factData([{ 'name': 'foo' + }]); + + var result = compareFacts(facts[0], facts[1], 'name', ['value']); + + expect(result).to.deep.equal( + [{ displayKeyPath: 'foo', + nestingLevel: 0, + facts: + [{ keyName: 'name', + value1: 'foo', + value1IsAbsent: false, + value2: 'absent', + value2IsAbsent: true + }] + }]); + }); + + it('includes all keys from basisFacts', function() { + var facts = factData([{ 'name': 'foo', + 'value': 'bar' + }]); + + var result = compareFacts(facts[0], facts[1], 'name', ['value']); + + expect(result).to.deep.equal( + [{ displayKeyPath: 'foo', + nestingLevel: 0, + facts: + [{ keyName: 'name', + value1: 'foo', + value1IsAbsent: false, + value2: 'absent', + value2IsAbsent: true + }, + { keyName: 'value', + value1: 'bar', + value1IsAbsent: false, + value2: 'absent', + value2IsAbsent: true + }] + }]); + + }); + }); +}); From 06da1e16c2e61687e55b79369b23b5e1349df06a Mon Sep 17 00:00:00 2001 From: Joe Fiorini Date: Thu, 11 Jun 2015 16:14:04 -0400 Subject: [PATCH 2/5] Add tests for fact template rendering --- .../js/system-tracking/compare-facts.js | 7 ++- .../js/system-tracking/compare-facts/flat.js | 32 ++-------- .../compare-facts/flat-test.js | 61 +++++++++++++++++-- 3 files changed, 68 insertions(+), 32 deletions(-) diff --git a/awx/ui/static/js/system-tracking/compare-facts.js b/awx/ui/static/js/system-tracking/compare-facts.js index 04d1ee3a6d..fc64465624 100644 --- a/awx/ui/static/js/system-tracking/compare-facts.js +++ b/awx/ui/static/js/system-tracking/compare-facts.js @@ -6,6 +6,7 @@ import compareNestedFacts from './compare-facts/nested'; import compareFlatFacts from './compare-facts/flat'; +import FactTemplate from './compare-facts/fact-template'; export function compareFacts(module, facts) { if (module.displayType === 'nested') { @@ -15,7 +16,11 @@ export function compareFacts(module, facts) { } else { // For flat structures we compare left-to-right, then right-to-left to // make sure we get a good comparison between both hosts - var compare = _.partialRight(compareFlatFacts, module.nameKey, module.compareKey, module.factTemplate); + var compare = _.partialRight(compareFlatFacts, + module.nameKey, + module.compareKey, + new FactTemplate(module.factTemplate)); + var leftToRight = compare(facts[0], facts[1]); var rightToLeft = compare(facts[1], facts[0]); diff --git a/awx/ui/static/js/system-tracking/compare-facts/flat.js b/awx/ui/static/js/system-tracking/compare-facts/flat.js index 00ae634340..887b8fd2ad 100644 --- a/awx/ui/static/js/system-tracking/compare-facts/flat.js +++ b/awx/ui/static/js/system-tracking/compare-facts/flat.js @@ -4,23 +4,6 @@ * All Rights Reserved *************************************************/ -import stringFilters from 'tower/shared/string-filters/main'; - -var $injector = angular.injector(['ng', stringFilters.name]); -var $interpolate = $injector.get('$interpolate'); - -function getFactTemplate(factTemplate, fact) { - if (_.isFunction(factTemplate)) { - return factTemplate(fact); - } else { - return factTemplate; - } -} - -function renderFactTemplate(template, fact) { - return $interpolate(template)(fact); -} - function slotFactValues(basisPosition, basisValue, comparatorValue) { var leftValue, rightValue; @@ -45,7 +28,7 @@ export default var searcher = {}; searcher[nameKey] = basisFact[nameKey]; - var basisTemplate, comparatorTemplate, slottedValues, basisValue, comparatorValue; + var slottedValues, basisValue, comparatorValue; var matchingFact = _.where(comparatorFacts.facts, searcher); var diffs; @@ -54,9 +37,7 @@ export default if (!_.isUndefined(factTemplate)) { - basisTemplate = getFactTemplate(factTemplate, basisFact); - - basisValue = renderFactTemplate(basisTemplate, basisFact); + basisValue = factTemplate.render(basisFact); slottedValues = slotFactValues(basisFacts.position, basisValue, 'absent'); diffs = @@ -76,7 +57,7 @@ export default value1IsAbsent: slottedValues.left === 'absent', value2: slottedValues.right, value2IsAbsent: slottedValues.right === 'absent' - }; + }; }); } } else { @@ -85,11 +66,8 @@ export default if (!_.isUndefined(factTemplate)) { - basisTemplate = getFactTemplate(factTemplate, basisFact); - comparatorTemplate = getFactTemplate(factTemplate, matchingFact); - - basisValue = renderFactTemplate(basisTemplate, basisFact); - comparatorValue = renderFactTemplate(comparatorTemplate, matchingFact); + basisValue = factTemplate.render(basisFact); + comparatorValue = factTemplate.render(matchingFact); slottedValues = slotFactValues(basisFacts.position, basisValue, comparatorValue); diff --git a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js index 52b0933a63..6c829d33c1 100644 --- a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js +++ b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js @@ -73,10 +73,6 @@ describe('CompareFacts.Flat', function() { return facts; } - beforeEach(function () { - - }); - it('uses "absent" for the missing value', function() { var facts = factData([{ 'name': 'foo' @@ -123,5 +119,62 @@ describe('CompareFacts.Flat', function() { }]); }); + + context('with factTemplate', function() { + it('does not attempt to render the absent fact', function() { + var facts = factData([{ 'name': 'foo' + }]); + + var renderCallCount = 0; + var factTemplate = + { render: function() { + renderCallCount++; + }, + template: "" + }; + + compareFacts(facts[0], facts[1], 'name', ['value'], factTemplate); + + expect(renderCallCount).to.equal(1); + + }); + }); + }); + + context('with factTemplate', function() { + var factData; + + beforeEach(function() { + factData = [{ position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'baz' + }] + }]; + }); + + it('renders the template with each provided fact', function() { + + var renderCalledWith = []; + var factTemplate = + { render: function(fact) { + renderCalledWith.push(fact); + }, + template: "" + }; + + compareFacts(factData[0], factData[1], 'name', ['value'], factTemplate); + + expect(renderCalledWith).to.include(factData[0].facts[0]); + expect(renderCalledWith).to.include(factData[1].facts[0]); + }); + + }); }); From b0d773325b79afbe48a8622ea9320f86cef9161a Mon Sep 17 00:00:00 2001 From: Joe Fiorini Date: Fri, 12 Jun 2015 09:38:53 -0400 Subject: [PATCH 3/5] [system_tracking] Refactor flat comparison logic --- .../js/system-tracking/compare-facts/flat.js | 79 ++++++++----------- .../compare-facts/flat-test.js | 4 +- 2 files changed, 35 insertions(+), 48 deletions(-) diff --git a/awx/ui/static/js/system-tracking/compare-facts/flat.js b/awx/ui/static/js/system-tracking/compare-facts/flat.js index 887b8fd2ad..197abc4540 100644 --- a/awx/ui/static/js/system-tracking/compare-facts/flat.js +++ b/awx/ui/static/js/system-tracking/compare-facts/flat.js @@ -33,12 +33,17 @@ export default var matchingFact = _.where(comparatorFacts.facts, searcher); var diffs; - if (_.isEmpty(matchingFact)) { - if (!_.isUndefined(factTemplate)) { basisValue = factTemplate.render(basisFact); - slottedValues = slotFactValues(basisFacts.position, basisValue, 'absent'); + + if (_.isEmpty(matchingFact)) { + comparatorValue = 'absent'; + } else { + comparatorValue = factTemplate.render(matchingFact[0]); + } + + slottedValues = slotFactValues(basisFacts.position, basisValue, comparatorValue); diffs = { keyName: basisFact[nameKey], @@ -48,60 +53,40 @@ export default } else { - diffs = - _.map(basisFact, function(value, key) { - var slottedValues = slotFactValues(basisFacts.position, value, 'absent'); + if (_.isEmpty(matchingFact)) { + matchingFact = {}; + } else { + matchingFact = matchingFact[0]; + } - return { keyName: key, + diffs = + _(basisFact).map(function(value, key) { + var slottedValues = slotFactValues(basisFacts.position, value, matchingFact[key] || 'absent'); + var keyName; + + if (slottedValues.right !== 'absent') { + if(slottedValues.left === slottedValues.right) { + return; + } + + if (!_.include(compareKeys, key)) { + return; + } + keyName = basisFact[nameKey]; + } else { + keyName = key; + } + + return { keyName: keyName, value1: slottedValues.left, value1IsAbsent: slottedValues.left === 'absent', value2: slottedValues.right, value2IsAbsent: slottedValues.right === 'absent' }; - }); - } - } else { - - matchingFact = matchingFact[0]; - - if (!_.isUndefined(factTemplate)) { - - basisValue = factTemplate.render(basisFact); - comparatorValue = factTemplate.render(matchingFact); - - slottedValues = slotFactValues(basisFacts.position, basisValue, comparatorValue); - - if (basisValue !== comparatorValue) { - - diffs = - { keyName: basisFact[nameKey], - value1: slottedValues.left, - value2: slottedValues.right - }; - - } - - } else { - - diffs = _(compareKeys) - .map(function(key) { - var slottedValues = slotFactValues(basisFacts.position, - basisFact[key], - matchingFact[key]); - - if (slottedValues.left !== slottedValues.right) { - return { - keyName: basisFact[nameKey], - value1: slottedValues.left, - value2: slottedValues.right - }; - } }).compact() .value(); } - } - var descriptor = { displayKeyPath: basisFact[nameKey], nestingLevel: 0, diff --git a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js index 6c829d33c1..1e5f47822b 100644 --- a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js +++ b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js @@ -54,7 +54,9 @@ describe('CompareFacts.Flat', function() { facts: [{ keyName: 'foo', value1: 'bar', - value2: 'baz' + value1IsAbsent: false, + value2: 'baz', + value2IsAbsent: false }] }]); }); From ad0f26743570b9b840ed175a9186336bacd35e2a Mon Sep 17 00:00:00 2001 From: Joe Fiorini Date: Fri, 12 Jun 2015 13:50:28 -0400 Subject: [PATCH 4/5] [system_tracking] Extract parameters into options object --- .../js/system-tracking/compare-facts.js | 13 +- .../js/system-tracking/compare-facts/flat.js | 89 ++++-- .../get-module-options.factory.js | 9 +- .../compare-facts/flat-test.js | 254 +++++++++++++++--- 4 files changed, 285 insertions(+), 80 deletions(-) diff --git a/awx/ui/static/js/system-tracking/compare-facts.js b/awx/ui/static/js/system-tracking/compare-facts.js index fc64465624..092c613361 100644 --- a/awx/ui/static/js/system-tracking/compare-facts.js +++ b/awx/ui/static/js/system-tracking/compare-facts.js @@ -9,11 +9,10 @@ import compareFlatFacts from './compare-facts/flat'; import FactTemplate from './compare-facts/fact-template'; export function compareFacts(module, facts) { - if (module.displayType === 'nested') { - return { factData: compareNestedFacts(facts), - isNestedDisplay: true - }; - } else { + // If the module has a template or includes a list of keys to display, + // then perform a flat comparison, otherwise assume nested + // + if (module.factTemplate || module.nameKey) { // For flat structures we compare left-to-right, then right-to-left to // make sure we get a good comparison between both hosts var compare = _.partialRight(compareFlatFacts, @@ -33,5 +32,9 @@ export function compareFacts(module, facts) { }; }) .value(); + } else { + return { factData: compareNestedFacts(facts), + isNestedDisplay: true + }; } } diff --git a/awx/ui/static/js/system-tracking/compare-facts/flat.js b/awx/ui/static/js/system-tracking/compare-facts/flat.js index 197abc4540..b9a7ffd7df 100644 --- a/awx/ui/static/js/system-tracking/compare-facts/flat.js +++ b/awx/ui/static/js/system-tracking/compare-facts/flat.js @@ -21,7 +21,14 @@ function slotFactValues(basisPosition, basisValue, comparatorValue) { } export default - function flatCompare(basisFacts, comparatorFacts, nameKey, compareKeys, factTemplate) { + function flatCompare(basisFacts, + comparatorFacts, renderOptions) { + + var nameKey = renderOptions.nameKey; + var compareKeys = renderOptions.compareKey; + var keyNameMap = renderOptions.keyNameMap; + var valueFormatter = renderOptions.valueFormatter; + var factTemplate = renderOptions.factTemplate; return basisFacts.facts.reduce(function(arr, basisFact) { @@ -33,7 +40,44 @@ export default var matchingFact = _.where(comparatorFacts.facts, searcher); var diffs; - if (!_.isUndefined(factTemplate)) { + // Perform comparison and get back comparisonResults; like: + // { 'value': + // { leftValue: 'blah', + // rightValue: 'doo' + // } + // }; + // + var comparisonResults = + _.reduce(compareKeys, function(result, compareKey) { + + if (_.isEmpty(matchingFact)) { + comparatorValue = 'absent'; + } else { + comparatorValue = matchingFact[0][compareKey]; + } + + var slottedValues = slotFactValues(basisFacts.position, + basisFact[compareKey], + comparatorValue); + + if (_.isUndefined(slottedValues.left) && _.isUndefined(slottedValues.right)) { + return result; + } + + if (slottedValues.left !== slottedValues.right) { + slottedValues.isDivergent = true; + } else { + slottedValues.isDivergent = false; + } + + result[compareKey] = slottedValues; + + return result; + }, {}); + + var hasDiffs = _.any(comparisonResults, { isDivergent: true }); + + if (hasDiffs && factTemplate.hasTemplate()) { basisValue = factTemplate.render(basisFact); @@ -43,45 +87,34 @@ export default comparatorValue = factTemplate.render(matchingFact[0]); } - slottedValues = slotFactValues(basisFacts.position, basisValue, comparatorValue); + if (!_.isEmpty(comparisonResults)) { - diffs = - { keyName: basisFact[nameKey], - value1: slottedValues.left, - value2: slottedValues.right - }; + slottedValues = slotFactValues(basisFact.position, basisValue, comparatorValue); - } else { - - if (_.isEmpty(matchingFact)) { - matchingFact = {}; - } else { - matchingFact = matchingFact[0]; + diffs = + { keyName: basisFact[nameKey], + value1: slottedValues.left, + value2: slottedValues.right + }; } + } else if (hasDiffs) { + diffs = - _(basisFact).map(function(value, key) { - var slottedValues = slotFactValues(basisFacts.position, value, matchingFact[key] || 'absent'); - var keyName; + _(comparisonResults).map(function(slottedValues, key) { - if (slottedValues.right !== 'absent') { - if(slottedValues.left === slottedValues.right) { - return; - } + var keyName = key; - if (!_.include(compareKeys, key)) { - return; - } - keyName = basisFact[nameKey]; - } else { - keyName = key; + if (keyNameMap && keyNameMap[key]) { + keyName = keyNameMap[key]; } return { keyName: keyName, value1: slottedValues.left, value1IsAbsent: slottedValues.left === 'absent', value2: slottedValues.right, - value2IsAbsent: slottedValues.right === 'absent' + value2IsAbsent: slottedValues.right === 'absent', + isDivergent: slottedValues.isDivergent }; }).compact() .value(); diff --git a/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js b/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js index c82848015d..b55c262d51 100644 --- a/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js +++ b/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js @@ -2,29 +2,26 @@ var moduleConfig = { 'packages': { compareKey: ['release', 'version'], nameKey: 'name', - displayType: 'flat', sortKey: 1, factTemplate: "{{epoch|append:':'}}{{version}}-{{release}}{{arch|prepend:'.'}}" }, 'services': { compareKey: ['state', 'source'], nameKey: 'name', - displayType: 'flat', factTemplate: '{{state}} ({{source}})', sortKey: 2 }, 'files': { compareKey: ['size', 'mode', 'md5', 'mtime', 'gid', 'uid'], nameKey: 'path', - displayType: 'flat', + displayKeys: ['size', 'mode', 'mtime', 'uid', 'gid', 'md5'], sortKey: 3 }, 'ansible': - { displayType: 'nested', - sortKey: 4 + { sortKey: 4 }, 'custom': - { displayType: 'nested' + { } }; diff --git a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js index 1e5f47822b..076e527f66 100644 --- a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js +++ b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js @@ -1,17 +1,34 @@ +import compareFacts from 'tower/system-tracking/compare-facts/flat'; + /* jshint node: true */ /* globals -expect, -_ */ -import compareFacts from 'tower/system-tracking/compare-facts/flat'; - -var expect = require('chai').expect; +var chai = require('chai'); var _ = require('lodash'); +var chaiThings = require('chai-things'); +chai.use(chaiThings); + +global.expect = chai.expect; global._ = _; describe('CompareFacts.Flat', function() { + function options(overrides) { + return _.merge({}, defaultOptions, overrides); + } + + var defaultTemplate = + { hasTemplate: function() { return false; } + }; + + var defaultOptions = + { factTemplate: defaultTemplate, + nameKey: 'name' + }; + it('returns empty array with empty basis facts', function() { - var result = compareFacts({ facts: [] }, { facts: [] }); + var result = compareFacts({ facts: [] }, { facts: [] }, defaultOptions); expect(result).to.deep.equal([]); }); @@ -27,39 +44,177 @@ describe('CompareFacts.Flat', function() { [{ 'name': 'foo', 'value': 'bar' }] - }, 'name', ['value']); + }, options({ nameKey: 'name', + compareKey: ['value'], + })); + + expect(result).to.deep.equal([]); + }); + + it('returns empty array with multiple compare keys and no differences', function() { + var result = compareFacts( + { facts: + [{ 'name': 'foo', + 'value': 'bar' + }] + }, + { facts: + [{ 'name': 'foo', + 'value': 'bar' + }] + }, options({ compareKey: ['name', 'value'] + })); expect(result).to.deep.equal([]); }); context('when both collections contain facts', function() { - it('includes each fact value when a compareKey differs', function() { + it('includes each compare key value when a compareKey differs', function() { var result = compareFacts( { position: 'left', facts: [{ 'name': 'foo', - 'value': 'bar' + 'value': 'bar', + 'extra': 'doo' }] }, { position: 'right', facts: [{ 'name': 'foo', - 'value': 'baz' + 'value': 'baz', + 'extra': 'doo' }] - }, 'name', ['value']); + }, options({ compareKey: ['value', 'extra'] })); expect(result).to.deep.equal( [{ displayKeyPath: 'foo', nestingLevel: 0, facts: - [{ keyName: 'foo', + [{ keyName: 'value', value1: 'bar', value1IsAbsent: false, value2: 'baz', - value2IsAbsent: false + value2IsAbsent: false, + isDivergent: true + }, + { keyName: 'extra', + value1: 'doo', + value1IsAbsent: false, + value2: 'doo', + value2IsAbsent: false, + isDivergent: false }] }]); }); + + it('ignores compare keys with no values in fact', function() { + var result = compareFacts( + { position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'baz', + 'extra': 'doo' + }] + }, options({ compareKey: ['value', 'extra', 'blah'] })); + + expect(result).to.deep.equal( + [{ displayKeyPath: 'foo', + nestingLevel: 0, + facts: + [{ keyName: 'value', + value1: 'bar', + value1IsAbsent: false, + value2: 'baz', + value2IsAbsent: false, + isDivergent: true + }, + { keyName: 'extra', + value1: 'doo', + value1IsAbsent: false, + value2: 'doo', + value2IsAbsent: false, + isDivergent: false + }] + }]); + + }); + + it('allows mapping key names with keyNameMap parameter', function() { + var keyNameMap = + { 'extra': 'blah' + }; + + var result = compareFacts( + { position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'baz', + 'extra': 'doo' + }] + }, options({ compareKey: ['value', 'extra', 'blah'], + keyNameMap: keyNameMap + })); + + expect(result[0].facts).to.include.something.that.deep.equals( + { keyName: 'blah', + value1: 'doo', + value1IsAbsent: false, + value2: 'doo', + value2IsAbsent: false, + isDivergent: false + }); + + }); + + // it('allows formatting values with valueFormat parameter', function() { + // var valueFormat = + // function(key, values) { + // if (key === 'extra') { + // return 'formatted'; + // } + // } + + // var result = compareFacts( + // { position: 'left', + // facts: + // [{ 'name': 'foo', + // 'value': 'bar', + // 'extra': 'doo' + // }] + // }, + // { position: 'right', + // facts: + // [{ 'name': 'foo', + // 'value': 'baz', + // 'extra': 'doo' + // }] + // }, 'name', ['value', 'extra', 'blah'], keyNameMap, defaultTemplate, ); + + // expect(result[0].facts).to.include.something.that.deep.equals( + // { keyName: 'extra', + // value1: 'formatted', + // value1IsAbsent: false, + // value2: 'formatted', + // value2IsAbsent: false, + // isDivergent: false + // }); + + // }); + }); context('when value for nameKey is present in one collection but not the other', function() { @@ -77,46 +232,55 @@ describe('CompareFacts.Flat', function() { it('uses "absent" for the missing value', function() { - var facts = factData([{ 'name': 'foo' - }]); - - var result = compareFacts(facts[0], facts[1], 'name', ['value']); - - expect(result).to.deep.equal( - [{ displayKeyPath: 'foo', - nestingLevel: 0, - facts: - [{ keyName: 'name', - value1: 'foo', - value1IsAbsent: false, - value2: 'absent', - value2IsAbsent: true - }] - }]); - }); - - it('includes all keys from basisFacts', function() { var facts = factData([{ 'name': 'foo', 'value': 'bar' }]); - var result = compareFacts(facts[0], facts[1], 'name', ['value']); + var result = compareFacts(facts[0], facts[1], + options({ compareKey: ['value'] + })); expect(result).to.deep.equal( [{ displayKeyPath: 'foo', nestingLevel: 0, facts: - [{ keyName: 'name', - value1: 'foo', - value1IsAbsent: false, - value2: 'absent', - value2IsAbsent: true - }, - { keyName: 'value', + [{ keyName: 'value', value1: 'bar', value1IsAbsent: false, value2: 'absent', - value2IsAbsent: true + value2IsAbsent: true, + isDivergent: true + }] + }]); + }); + + it('includes given compare keys from basisFacts', function() { + var facts = factData([{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }]); + + var result = compareFacts(facts[0], facts[1], + options({ compareKey: ['value', 'extra'] + })); + + expect(result).to.deep.equal( + [{ displayKeyPath: 'foo', + nestingLevel: 0, + facts: + [{ keyName: 'value', + value1: 'bar', + value1IsAbsent: false, + value2: 'absent', + value2IsAbsent: true, + isDivergent: true + }, + { keyName: 'extra', + value1: 'doo', + value1IsAbsent: false, + value2: 'absent', + value2IsAbsent: true, + isDivergent: true }] }]); @@ -132,10 +296,14 @@ describe('CompareFacts.Flat', function() { { render: function() { renderCallCount++; }, + hasTemplate: function() { return true; }, template: "" }; - compareFacts(facts[0], facts[1], 'name', ['value'], factTemplate); + compareFacts(facts[0], facts[1], + options({ compareKey: ['value'], + factTemplate: factTemplate + })); expect(renderCallCount).to.equal(1); @@ -168,10 +336,14 @@ describe('CompareFacts.Flat', function() { { render: function(fact) { renderCalledWith.push(fact); }, + hasTemplate: function() { return true; }, template: "" }; - compareFacts(factData[0], factData[1], 'name', ['value'], factTemplate); + compareFacts(factData[0], factData[1], + options({ compareKey: ['value'], + factTemplate: factTemplate + })); expect(renderCalledWith).to.include(factData[0].facts[0]); expect(renderCalledWith).to.include(factData[1].facts[0]); From 61807efb9b5b235a68accce79172384f917fce07 Mon Sep 17 00:00:00 2001 From: Joe Fiorini Date: Mon, 15 Jun 2015 16:23:17 -0400 Subject: [PATCH 5/5] Allow fact template to flatten or keep key/value structure --- .../format-epoch/format-epoch.filter.js | 16 ++ awx/ui/static/js/shared/format-epoch/main.js | 9 + .../js/system-tracking/compare-facts.js | 29 +- .../compare-facts/fact-template.js | 33 +++ .../js/system-tracking/compare-facts/flat.js | 81 +++--- .../get-module-options.factory.js | 10 +- .../compare-facts/flat-test.js | 256 ++++++++++++------ 7 files changed, 313 insertions(+), 121 deletions(-) create mode 100644 awx/ui/static/js/shared/format-epoch/format-epoch.filter.js create mode 100644 awx/ui/static/js/shared/format-epoch/main.js create mode 100644 awx/ui/static/js/system-tracking/compare-facts/fact-template.js diff --git a/awx/ui/static/js/shared/format-epoch/format-epoch.filter.js b/awx/ui/static/js/shared/format-epoch/format-epoch.filter.js new file mode 100644 index 0000000000..2216249732 --- /dev/null +++ b/awx/ui/static/js/shared/format-epoch/format-epoch.filter.js @@ -0,0 +1,16 @@ +export default +[ 'moment', + function(moment) { + return function(seconds, formatStr) { + if (!formatStr) { + formatStr = 'll LT'; + } + + var millis = seconds * 1000; + + return moment(millis).format(formatStr); + }; + } +]; + + diff --git a/awx/ui/static/js/shared/format-epoch/main.js b/awx/ui/static/js/shared/format-epoch/main.js new file mode 100644 index 0000000000..916a81c7ab --- /dev/null +++ b/awx/ui/static/js/shared/format-epoch/main.js @@ -0,0 +1,9 @@ +import formatEpoch from './format-epoch.filter'; +import moment from 'tower/shared/moment/main'; + +export default + angular.module('formatEpoch', + [ moment.name + ]) + .filter('formatEpoch', formatEpoch); + diff --git a/awx/ui/static/js/system-tracking/compare-facts.js b/awx/ui/static/js/system-tracking/compare-facts.js index 092c613361..b8e466f00a 100644 --- a/awx/ui/static/js/system-tracking/compare-facts.js +++ b/awx/ui/static/js/system-tracking/compare-facts.js @@ -9,26 +9,39 @@ import compareFlatFacts from './compare-facts/flat'; import FactTemplate from './compare-facts/fact-template'; export function compareFacts(module, facts) { + + + var renderOptions = _.merge({}, module); + // If the module has a template or includes a list of keys to display, // then perform a flat comparison, otherwise assume nested // - if (module.factTemplate || module.nameKey) { + if (renderOptions.factTemplate || renderOptions.nameKey) { // For flat structures we compare left-to-right, then right-to-left to // make sure we get a good comparison between both hosts - var compare = _.partialRight(compareFlatFacts, - module.nameKey, - module.compareKey, - new FactTemplate(module.factTemplate)); - var leftToRight = compare(facts[0], facts[1]); - var rightToLeft = compare(facts[1], facts[0]); + if (_.isPlainObject(renderOptions.factTemplate)) { + renderOptions.factTemplate = + _.mapValues(renderOptions.factTemplate, function(template) { + if (typeof template === 'string' || typeof template === 'function') { + return new FactTemplate(template); + } else { + return template; + } + }); + } else { + renderOptions.factTemplate = new FactTemplate(renderOptions.factTemplate); + } + + var leftToRight = compareFlatFacts(facts[0], facts[1], renderOptions); + var rightToLeft = compareFlatFacts(facts[1], facts[0], renderOptions); return _(leftToRight) .concat(rightToLeft) .unique('displayKeyPath') .thru(function(result) { return { factData: result, - isNestedDisplay: _.isUndefined(module.factTemplate) + isNestedDisplay: _.isPlainObject(renderOptions.factTemplate) }; }) .value(); diff --git a/awx/ui/static/js/system-tracking/compare-facts/fact-template.js b/awx/ui/static/js/system-tracking/compare-facts/fact-template.js new file mode 100644 index 0000000000..d4c0e2d5d8 --- /dev/null +++ b/awx/ui/static/js/system-tracking/compare-facts/fact-template.js @@ -0,0 +1,33 @@ +import stringFilters from 'tower/shared/string-filters/main'; +import formatEpoch from 'tower/shared/format-epoch/main'; + +var $injector = angular.injector(['ng', stringFilters.name, formatEpoch.name]); +var $interpolate = $injector.get('$interpolate'); + +function FactTemplate(templateString) { + this.templateString = templateString; +} + +function loadFactTemplate(factTemplate, fact) { + if (_.isFunction(factTemplate)) { + return factTemplate(fact); + } else { + return factTemplate; + } +} + +FactTemplate.prototype.render = function(factData) { + + if (_.isUndefined(factData) || _.isEmpty(factData)) { + return 'absent'; + } + + var template = loadFactTemplate(this.templateString, factData); + return $interpolate(template)(factData); +}; + +FactTemplate.prototype.hasTemplate = function() { + return !_.isUndefined(this.templateString); +}; + +export default FactTemplate; diff --git a/awx/ui/static/js/system-tracking/compare-facts/flat.js b/awx/ui/static/js/system-tracking/compare-facts/flat.js index b9a7ffd7df..2c2615158f 100644 --- a/awx/ui/static/js/system-tracking/compare-facts/flat.js +++ b/awx/ui/static/js/system-tracking/compare-facts/flat.js @@ -27,7 +27,6 @@ export default var nameKey = renderOptions.nameKey; var compareKeys = renderOptions.compareKey; var keyNameMap = renderOptions.keyNameMap; - var valueFormatter = renderOptions.valueFormatter; var factTemplate = renderOptions.factTemplate; @@ -35,8 +34,6 @@ export default var searcher = {}; searcher[nameKey] = basisFact[nameKey]; - var slottedValues, basisValue, comparatorValue; - var matchingFact = _.where(comparatorFacts.facts, searcher); var diffs; @@ -50,53 +47,75 @@ export default var comparisonResults = _.reduce(compareKeys, function(result, compareKey) { - if (_.isEmpty(matchingFact)) { - comparatorValue = 'absent'; - } else { - comparatorValue = matchingFact[0][compareKey]; - } + var comparatorFact = matchingFact[0] || {}; + var isNestedDisplay = false; var slottedValues = slotFactValues(basisFacts.position, basisFact[compareKey], - comparatorValue); + comparatorFact[compareKey]); if (_.isUndefined(slottedValues.left) && _.isUndefined(slottedValues.right)) { return result; } + var template = factTemplate; + + if (_.isObject(template) && template.hasOwnProperty(compareKey)) { + template = template[compareKey]; + + // 'true' means render the key without formatting + if (template === true) { + template = + { render: function(fact) { return fact[compareKey]; } + }; + } + + isNestedDisplay = true; + } else if (typeof template.hasTemplate === 'function' && !template.hasTemplate()) { + template = + { render: function(fact) { return fact[compareKey]; } + }; + isNestedDisplay = true; + } else if (typeof factTemplate.render === 'function') { + template = factTemplate; + } else if (!template.hasOwnProperty(compareKey)) { + return result; + } + + if (basisFacts.position === 'left') { + slottedValues.left = template.render(basisFact); + slottedValues.right = template.render(comparatorFact); + } else { + slottedValues.left = template.render(comparatorFact); + slottedValues.right = template.render(basisFact); + } + if (slottedValues.left !== slottedValues.right) { slottedValues.isDivergent = true; } else { slottedValues.isDivergent = false; } - result[compareKey] = slottedValues; + if (isNestedDisplay) { + result[compareKey] = slottedValues; + } else { + result = slottedValues; + } return result; }, {}); - var hasDiffs = _.any(comparisonResults, { isDivergent: true }); + var hasDiffs = + _.any(comparisonResults, { isDivergent: true }) || + comparisonResults.isDivergent === true; - if (hasDiffs && factTemplate.hasTemplate()) { + if (hasDiffs && typeof factTemplate.render === 'function') { - basisValue = factTemplate.render(basisFact); - - if (_.isEmpty(matchingFact)) { - comparatorValue = 'absent'; - } else { - comparatorValue = factTemplate.render(matchingFact[0]); - } - - if (!_.isEmpty(comparisonResults)) { - - slottedValues = slotFactValues(basisFact.position, basisValue, comparatorValue); - - diffs = - { keyName: basisFact[nameKey], - value1: slottedValues.left, - value2: slottedValues.right - }; - } + diffs = + { keyName: basisFact[nameKey], + value1: comparisonResults.left, + value2: comparisonResults.right + }; } else if (hasDiffs) { @@ -111,9 +130,7 @@ export default return { keyName: keyName, value1: slottedValues.left, - value1IsAbsent: slottedValues.left === 'absent', value2: slottedValues.right, - value2IsAbsent: slottedValues.right === 'absent', isDivergent: slottedValues.isDivergent }; }).compact() diff --git a/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js b/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js index b55c262d51..f780cb12f7 100644 --- a/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js +++ b/awx/ui/static/js/system-tracking/data-services/get-module-options.factory.js @@ -13,8 +13,16 @@ var moduleConfig = }, 'files': { compareKey: ['size', 'mode', 'md5', 'mtime', 'gid', 'uid'], + keyNameMap: + { 'uid': 'ownership' + }, + factTemplate: + { 'uid': 'user id: {{uid}}, group id: {{gid}}', + 'mode': true, + 'md5': true, + 'mtime': '{{mtime|formatEpoch}}' + }, nameKey: 'path', - displayKeys: ['size', 'mode', 'mtime', 'uid', 'gid', 'md5'], sortKey: 3 }, 'ansible': diff --git a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js index 076e527f66..2995be4caa 100644 --- a/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js +++ b/awx/ui/tests/unit/system-tracking/compare-facts/flat-test.js @@ -3,14 +3,31 @@ import compareFacts from 'tower/system-tracking/compare-facts/flat'; /* jshint node: true */ /* globals -expect, -_ */ -var chai = require('chai'); -var _ = require('lodash'); -var chaiThings = require('chai-things'); +var _, expect; -chai.use(chaiThings); +// This makes this test runnable in node OR karma. The sheer +// number of times I had to run this test made the karma +// workflow just too dang slow for me. Maybe this can +// be a pattern going forward? Not sure... +// +(function(global) { + var chai = global.chai || require('chai'); -global.expect = chai.expect; -global._ = _; + if (typeof window === 'undefined') { + var chaiThings = global.chaiThings || require('chai-things'); + chai.use(chaiThings); + } + + _ = global._ || require('lodash'); + expect = global.expect || chai.expect; + + global.expect = expect; + + + + global._ = _; + +})(typeof window === 'undefined' ? global : window); describe('CompareFacts.Flat', function() { @@ -92,16 +109,12 @@ describe('CompareFacts.Flat', function() { facts: [{ keyName: 'value', value1: 'bar', - value1IsAbsent: false, value2: 'baz', - value2IsAbsent: false, isDivergent: true }, { keyName: 'extra', value1: 'doo', - value1IsAbsent: false, value2: 'doo', - value2IsAbsent: false, isDivergent: false }] }]); @@ -130,16 +143,12 @@ describe('CompareFacts.Flat', function() { facts: [{ keyName: 'value', value1: 'bar', - value1IsAbsent: false, value2: 'baz', - value2IsAbsent: false, isDivergent: true }, { keyName: 'extra', value1: 'doo', - value1IsAbsent: false, value2: 'doo', - value2IsAbsent: false, isDivergent: false }] }]); @@ -172,48 +181,131 @@ describe('CompareFacts.Flat', function() { expect(result[0].facts).to.include.something.that.deep.equals( { keyName: 'blah', value1: 'doo', - value1IsAbsent: false, value2: 'doo', - value2IsAbsent: false, isDivergent: false }); }); - // it('allows formatting values with valueFormat parameter', function() { - // var valueFormat = - // function(key, values) { - // if (key === 'extra') { - // return 'formatted'; - // } - // } + it('allows flattening values with factTemplate parameter', function() { + var factTemplate = + { hasTemplate: + function() { + return true; + }, + render: function(fact) { + return 'value: ' + fact.value; + } + }; - // var result = compareFacts( - // { position: 'left', - // facts: - // [{ 'name': 'foo', - // 'value': 'bar', - // 'extra': 'doo' - // }] - // }, - // { position: 'right', - // facts: - // [{ 'name': 'foo', - // 'value': 'baz', - // 'extra': 'doo' - // }] - // }, 'name', ['value', 'extra', 'blah'], keyNameMap, defaultTemplate, ); + var result = compareFacts( + { position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'baz', + 'extra': 'doo' + }] + }, options({ compareKey: ['value'], + factTemplate: factTemplate + })); - // expect(result[0].facts).to.include.something.that.deep.equals( - // { keyName: 'extra', - // value1: 'formatted', - // value1IsAbsent: false, - // value2: 'formatted', - // value2IsAbsent: false, - // isDivergent: false - // }); + expect(result[0].facts).to.deep.equal( + { keyName: 'foo', + value1: 'value: bar', + value2: 'value: baz' + }); + }); - // }); + it('allows formatting values with factTemplate parameter', function() { + var values = ['value1', 'value2']; + var factTemplate = + { 'value': + { hasTemplate: function() { + return true; + }, + render: function() { + return values.shift(); + } + }, + 'extra': true + }; + + var result = compareFacts( + { position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'baz', + 'extra': 'doo' + }] + }, options({ compareKey: ['value'], + factTemplate: factTemplate + })); + + expect(result[0].facts).to.include.something.that.deep.equals( + { keyName: 'value', + value1: 'value1', + value2: 'value2', + isDivergent: true + }, + { keyName: 'extra', + value1: 'doo', + value2: 'doo', + isDivergent: false + }); + + }); + + it('compares values using the formatted values, not the raw ones', function() { + var values = ['value1', 'value2']; + var factTemplate = + { 'extra': + { render: function() { + return values.shift(); + } + } + }; + + var result = compareFacts( + { position: 'left', + facts: + [{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }] + }, + { position: 'right', + facts: + [{ 'name': 'foo', + 'value': 'bar', + 'extra': 'doo' + }] + }, options({ factTemplate: factTemplate, + compareKey: ['extra'] + })); + + expect(result.length).to.be.greaterThan(0); + expect(result[0].facts).to.include.something.that.deep.equals( + { keyName: 'extra', + value1: 'value1', + value2: 'value2', + isDivergent: true + }); + + }); }); @@ -230,7 +322,7 @@ describe('CompareFacts.Flat', function() { return facts; } - it('uses "absent" for the missing value', function() { + it('keeps missing values as undefined', function() { var facts = factData([{ 'name': 'foo', 'value': 'bar' @@ -246,14 +338,45 @@ describe('CompareFacts.Flat', function() { facts: [{ keyName: 'value', value1: 'bar', - value1IsAbsent: false, - value2: 'absent', - value2IsAbsent: true, + value2: undefined, isDivergent: true }] }]); }); + it('still keeps missing values as undefined when using a template', function() { + + var factTemplate = + { hasTemplate: + function() { + return true; + }, + render: + function(fact) { + return fact.value; + } + }; + + var facts = factData([{ 'name': 'foo', + 'value': 'bar' + }]); + + var result = compareFacts(facts[0], facts[1], + options({ compareKey: ['value'], + factTemplate: factTemplate + })); + + expect(result).to.deep.equal( + [{ displayKeyPath: 'foo', + nestingLevel: 0, + facts: + { keyName: 'foo', + value1: 'bar', + value2: undefined + } + }]); + }); + it('includes given compare keys from basisFacts', function() { var facts = factData([{ 'name': 'foo', 'value': 'bar', @@ -270,45 +393,18 @@ describe('CompareFacts.Flat', function() { facts: [{ keyName: 'value', value1: 'bar', - value1IsAbsent: false, - value2: 'absent', - value2IsAbsent: true, + value2: undefined, isDivergent: true }, { keyName: 'extra', value1: 'doo', - value1IsAbsent: false, - value2: 'absent', - value2IsAbsent: true, + value2: undefined, isDivergent: true }] }]); }); - context('with factTemplate', function() { - it('does not attempt to render the absent fact', function() { - var facts = factData([{ 'name': 'foo' - }]); - - var renderCallCount = 0; - var factTemplate = - { render: function() { - renderCallCount++; - }, - hasTemplate: function() { return true; }, - template: "" - }; - - compareFacts(facts[0], facts[1], - options({ compareKey: ['value'], - factTemplate: factTemplate - })); - - expect(renderCallCount).to.equal(1); - - }); - }); }); context('with factTemplate', function() {