From e3a945c79b523da6ec8d562603e2e04a1014ee1d Mon Sep 17 00:00:00 2001 From: Micah Allen Date: Fri, 3 Apr 2020 13:39:07 -0400 Subject: [PATCH 1/4] Add 'renderOrder' property to schema to decide in which order components render --- lib/routes/_components.js | 2 +- lib/routes/_layouts.js | 2 +- lib/routes/_pages.js | 2 +- lib/services/components.js | 2 +- lib/services/composer.js | 103 ++++++++++++++++++++++++-------- lib/services/composer.test.js | 52 ++++++++++++++-- lib/services/layouts.js | 2 +- lib/services/pages.js | 4 +- lib/services/references.js | 26 ++++++++ lib/services/references.test.js | 43 +++++++++++++ 10 files changed, 199 insertions(+), 39 deletions(-) diff --git a/lib/routes/_components.js b/lib/routes/_components.js index b8b53b51..b142b93a 100644 --- a/lib/routes/_components.js +++ b/lib/routes/_components.js @@ -20,7 +20,7 @@ function getComposed(uri, locals) { return controller.get(uri, locals).then(function (data) { // TODO: Check that we can't just reference the composer // require here because otherwise it's a circular dependency (via html-composer) - return require('../services/composer').resolveComponentReferences(data, locals); + return require('../services/composer').resolveComponentReferences(uri, data, locals); }); } diff --git a/lib/routes/_layouts.js b/lib/routes/_layouts.js index 898c77ba..5a21ab28 100644 --- a/lib/routes/_layouts.js +++ b/lib/routes/_layouts.js @@ -23,7 +23,7 @@ function getComposed(uri, locals) { return controller.get(uri, locals).then(data => { // TODO: Check that we can't just reference the composer // require here because otherwise it's a circular dependency (via html-composer) - return require('../services/composer').resolveComponentReferences(data, locals); + return require('../services/composer').resolveComponentReferences(uri, data, locals); }); } diff --git a/lib/routes/_pages.js b/lib/routes/_pages.js index 5467e5b5..2571a4f1 100644 --- a/lib/routes/_pages.js +++ b/lib/routes/_pages.js @@ -23,7 +23,7 @@ const _ = require('lodash'), */ function getComposed(uri, locals) { return db.get(uri) - .then(pageData => composer.composePage(pageData, locals)); + .then(pageData => composer.composePage(uri, pageData, locals)); } /** diff --git a/lib/services/components.js b/lib/services/components.js index 0485986c..cc64c19d 100644 --- a/lib/services/components.js +++ b/lib/services/components.js @@ -64,7 +64,7 @@ function publish(uri, data, locals) { } return get(replaceVersion(uri), locals) - .then(latestData => composer.resolveComponentReferences(latestData, locals, composer.filterBaseInstanceReferences)) + .then(latestData => composer.resolveComponentReferences(uri, latestData, locals, composer.filterBaseInstanceReferences)) .then(versionedData => dbOps.cascadingPut(put)(uri, versionedData, locals)); } diff --git a/lib/services/composer.js b/lib/services/composer.js index 20a811be..c5093c88 100644 --- a/lib/services/composer.js +++ b/lib/services/composer.js @@ -5,58 +5,108 @@ const _ = require('lodash'), components = require('./components'), references = require('./references'), mapLayoutToPageData = require('../utils/layout-to-page-data'), - referenceProperty = '_ref'; + { getSchema } = require('../utils/schema'), + { isLayout, isComponent } = require('clayutils'), + referenceProperty = '_ref', + renderOrderProperty = 'renderOrder', + defaultRenderOrder = 1; var log = require('./logger').setup({ file: __filename }); /** * Compose a component, recursively filling in all component references with * instance data. + * @param {string} uri * @param {object} data * @param {object} locals Extra data that some GETs use * @param {function|string} [filter='_ref'] * @returns {Promise} - Resolves with composed component data */ -function resolveComponentReferences(data, locals, filter = referenceProperty) { - const referenceObjects = references.listDeepObjects(data, filter); - - return bluebird.all(referenceObjects).each(referenceObject => { - return components.get(referenceObject[referenceProperty], locals) - .then(obj => { - // the thing we got back might have its own references - return resolveComponentReferences(obj, locals, filter).finally(() => { - _.assign(referenceObject, _.omit(obj, referenceProperty)); - }).catch(function (error) { - const logObj = { - stack: error.stack, - cmpt: referenceObject[referenceProperty] - }; - - if (error.status) { - logObj.status = error.status; - } - - log('error', `${error.message}`, logObj); - - return bluebird.reject(error); +function resolveComponentReferences(uri, data, locals, filter = referenceProperty) { + const referenceObjects = references.listDeepValuesByKey(data, filter), + schemaPromise = Object.keys(referenceObjects).length && (isComponent(uri) || isLayout(uri)) ? getSchema(uri) : Promise.resolve(); + + return schemaPromise.then(schema => { + const orderedRefs = orderReferences(referenceObjects, schema); + + return bluebird.each(orderedRefs, ([, referenceObject]) => { + const ref = referenceObject[referenceProperty]; + + return components.get(ref, locals) + .then(obj => { + // the thing we got back might have its own references + return resolveComponentReferences(ref, obj, locals, filter).finally(() => { + _.assign(referenceObject, _.omit(obj, referenceProperty)); + }).catch(function (error) { + const logObj = { + stack: error.stack, + cmpt: referenceObject[referenceProperty] + }; + + if (error.status) { + logObj.status = error.status; + } + + log('error', `${error.message}`, logObj); + + return bluebird.reject(error); + }); }); - }); + }); }).then(() => data); } +/** + * Orders deep references based on the parent's schema. + * @param {object} referenceObjects + * @param {object} schema + * @returns {array} + */ +function orderReferences(referenceObjects, schema) { + const pairedRefs = _.toPairs(referenceObjects); + + if (schema) { + // group and sort by orders from the schema + return _.chain(pairedRefs) + .groupBy(([path]) => { + const splitPath = path.split('.'); + + // remove array indexes + if (!isNaN(splitPath[splitPath.length - 1])) { + splitPath.pop(); + } + + return [ + _.get(schema, [...splitPath, '_componentList', renderOrderProperty]), + _.get(schema, [...splitPath, '_component', renderOrderProperty]), + defaultRenderOrder + ].find(order => _.isNumber(order)); + }) + .toPairs() + .sortBy(a => a[0]) + .map(a => a[1]) + .flatten() + .value(); + } else { + // if there's no schema, return original refs + return pairedRefs; + } +} + /** * Compose a page, recursively filling in all component references with * instance data. + * @param {string} uri * @param {object} pageData * @param {object} locals * @return {Promise} - Resolves with composed page data */ -function composePage(pageData, locals) { +function composePage(uri, pageData, locals) { const layoutReference = pageData.layout, pageDataNoConf = references.omitPageConfiguration(pageData); return components.get(layoutReference) .then(layoutData => mapLayoutToPageData(pageDataNoConf, layoutData)) - .then(fullData => resolveComponentReferences(fullData, locals)); + .then(fullData => resolveComponentReferences(uri, fullData, locals)); } /** @@ -74,3 +124,4 @@ module.exports.filterBaseInstanceReferences = filterBaseInstanceReferences; // For testing module.exports.setLog = (fakeLogger) => log = fakeLogger; +module.exports.orderReferences = orderReferences; diff --git a/lib/services/composer.test.js b/lib/services/composer.test.js index bf552fc0..7b7ae302 100644 --- a/lib/services/composer.test.js +++ b/lib/services/composer.test.js @@ -35,7 +35,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/b').returns(bluebird.resolve({g: 'h'})); components.get.withArgs('/c/e').returns(bluebird.resolve({i: 'j'})); - return fn(data).then(function (result) { + return fn('', data).then(function (result) { expect(result).to.deep.equal({ a: {_ref: '/c/b', g: 'h'}, c: {d: {_ref: '/c/e', i: 'j'}} @@ -53,7 +53,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/e').returns(bluebird.resolve({i: 'j', k: {_ref: '/c/m'}})); components.get.withArgs('/c/m').returns(bluebird.resolve({n: 'o'})); - return fn(data).then(function (result) { + return fn('', data).then(function (result) { expect(result).to.deep.equal({ a: {_ref: '/c/b', g: 'h'}, c: { @@ -83,7 +83,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/e').returns(bluebird.resolve({i: 'j', k: {_ref: '/c/m'}})); components.get.withArgs('/c/m').returns(bluebird.resolve({n: 'o'})); - return fn(data, undefined, filter).then(function (result) { + return fn('', data, undefined, filter).then(function (result) { expect(result).to.deep.equal({ a: {_ref: '/c/b', g: 'h'}, c: { @@ -107,7 +107,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/m').returns(bluebird.reject(myError)); // use done() rather than returning the promise, so we can catch and test errors - fn(data).then(done).catch((error) => { + fn('', data).then(done).catch((error) => { sinon.assert.calledOnce(logSpy); expect(error.message).to.equal('hello!'); done(); @@ -127,7 +127,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/m').returns(bluebird.reject(myError)); // use done() rather than returning the promise, so we can catch and test errors - fn(data).then(done).catch((error) => { + fn('', data).then(done).catch((error) => { sinon.assert.calledOnce(logSpy); expect(error.message).to.equal('hello!'); sinon.assert.calledWith(logSpy, 'error', 'hello!', { status: myError.status, cmpt: '/c/e', stack: myError.stack }); @@ -167,7 +167,7 @@ describe(_.startCase(filename), function () { layout: '/some/layout' }; - fn(pageData) + fn('', pageData) .then((result) => { expect(result).to.deep.equal({ head: [{ @@ -200,4 +200,44 @@ describe(_.startCase(filename), function () { expect(fn({ bar: 'foo' })).to.be.false; }); }); + + describe('orderReferences', function () { + const fn = lib[this.title], + referenceObjects = { + someComponent: { _ref: 'ref1' }, + anotherComponent: { _ref: 'ref2' }, + 'componentList.0': { _ref: 'ref3' }, + 'componentList.1': { _ref: 'ref4' } + }; + + it('maintains order without a schema', function () { + const result = fn(referenceObjects); + + expect(result).to.have.deep.property('0.0', 'someComponent'); + expect(result).to.have.deep.property('1.0', 'anotherComponent'); + expect(result).to.have.deep.property('2.0', 'componentList.0'); + expect(result).to.have.deep.property('3.0', 'componentList.1'); + }); + + it('orders based on schema', function () { + const schema = { + someComponent: { + _component: { + renderOrder: 3 + } + }, + componentList: { + _componentList: { + renderOrder: 2 + } + } + }, + result = fn(referenceObjects, schema); + + expect(result).to.have.deep.property('0.0', 'anotherComponent'); + expect(result).to.have.deep.property('1.0', 'componentList.0'); + expect(result).to.have.deep.property('2.0', 'componentList.1'); + expect(result).to.have.deep.property('3.0', 'someComponent'); + }); + }); }); diff --git a/lib/services/layouts.js b/lib/services/layouts.js index 9335c5aa..bf856357 100644 --- a/lib/services/layouts.js +++ b/lib/services/layouts.js @@ -72,7 +72,7 @@ function publish(uri, data, locals) { } return get(replaceVersion(uri), locals) - .then(latestData => composer.resolveComponentReferences(latestData, locals, composer.filterBaseInstanceReferences)) + .then(latestData => composer.resolveComponentReferences(uri, latestData, locals, composer.filterBaseInstanceReferences)) .then(versionedData => dbOps.cascadingPut(put)(uri, versionedData, locals)) .then(data => meta.publishLayout(uri, user).then(() => { bus.publish('publishLayout', { uri, data, user }); diff --git a/lib/services/pages.js b/lib/services/pages.js index 51e78a08..67a6300e 100644 --- a/lib/services/pages.js +++ b/lib/services/pages.js @@ -72,7 +72,7 @@ function getPageClonePutOperations(pageData, locals) { // for all strings that are component references promises.push(components.get(pageValue, locals) // only follow the paths of instance references. Don't clone default components - .then(refData => composer.resolveComponentReferences(refData, locals, isInstanceReferenceObject)) + .then(refData => composer.resolveComponentReferences(pageValue, refData, locals, isInstanceReferenceObject)) .then(resolvedData => { // for each instance reference within resolved data _.each(references.listDeepObjects(resolvedData, isInstanceReferenceObject), obj => { @@ -149,7 +149,7 @@ function getRecursivePublishedPutOperations(locals) { * 4) Get list of put operations needed */ return components.get(rootComponentRef, locals) - .then(data => composer.resolveComponentReferences(data, locals)) + .then(data => composer.resolveComponentReferences(rootComponentRef, data, locals)) .then(data => dbOps.getPutOperations(components.cmptPut, replaceVersion(rootComponentRef, 'published'), data, locals)); }; } diff --git a/lib/services/references.js b/lib/services/references.js index a699fe23..f9d130bf 100644 --- a/lib/services/references.js +++ b/lib/services/references.js @@ -39,6 +39,31 @@ function listDeepObjects(obj, filter) { return list; } +/** + * Search through an object and find all deep key-value pairs matching a filter. + * @param {object} obj + * @param {Function} [filter=_.identity] Optional filter + * @param {array} [path] Path of this value + * @returns {object} + */ +function listDeepValuesByKey(obj, filter = _.identity, path = []) { + let result = {}; + + if (_.isObject(obj)) { + // loop through any objects or arrays + result = _.reduce(obj, (cumm, curr, key) => _.assign(cumm, listDeepValuesByKey(curr, filter, [...path, key])), {}); + } + + if (_.iteratee(filter)(obj) && path.length) { + // add the key to results if it matches the filter + const key = path.join('.'); + + result[key] = obj; + } + + return result; +} + /** * @param {string} version * @returns {function} @@ -155,3 +180,4 @@ module.exports.urlToUri = urlToUri; module.exports.omitPageConfiguration = omitPageConfiguration; module.exports.getPageReferences = getPageReferences; module.exports.listDeepObjects = listDeepObjects; +module.exports.listDeepValuesByKey = listDeepValuesByKey; diff --git a/lib/services/references.test.js b/lib/services/references.test.js index c2cec967..37f2031a 100644 --- a/lib/services/references.test.js +++ b/lib/services/references.test.js @@ -170,4 +170,47 @@ describe(_.startCase(filename), function () { ]); }); }); + + describe('listDeepValuesByKey', function () { + const fn = lib[this.title]; + + it('listDeepValuesByKey gets all deep values by key', function () { + const result = fn({a:{b:{c:{d:'e'}}, f:{g:{h:'e'}}}, abc:[6,{p:2}]}); + + expect(result).to.deep.equal({ + a: {b:{c:{d:'e'}}, f:{g:{h:'e'}}}, + 'a.b': {c:{d:'e'}}, + 'a.b.c': {d:'e'}, + 'a.b.c.d': 'e', + 'a.f': {g:{h:'e'}}, + 'a.f.g': {h:'e'}, + 'a.f.g.h': 'e', + abc: [6,{p:2}], + 'abc.0': 6, + 'abc.1': {p:2}, + 'abc.1.p': 2 + }); + }); + + it('listDeepValuesByKey gets all deep objects using filter', function () { + const result = fn({a:{b:{c:{d:'e'}}, f:{g:{h:'e'}}}}, _.isObject); + + expect(Object.values(result)).to.have.length(5); + }); + + it('listDeepValuesByKey can filter by existence of properties', function () { + const result = fn({a:{b:{c:{d:'e'}}, f:{d:{g:'e'}}}}, 'd'); + + expect(Object.values(result)).to.have.length(2); + }); + + it('listDeepValuesByKey can filter by component', function () { + const result = fn({a: {type:'yarn'}, b: {c: {type:'sweater'}}}, function (obj) { return !!obj.type; }); + + expect(result).to.deep.equal({ + a: { type: 'yarn' }, + 'b.c': { type: 'sweater' } + }); + }); + }); }); From 27c5ccd17d8e76e759b18cf242e85aed2a3a4149 Mon Sep 17 00:00:00 2001 From: Micah Allen Date: Fri, 3 Apr 2020 15:06:58 -0400 Subject: [PATCH 2/4] Fix resolveComponentReferences to always return bluebird promise --- lib/services/composer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/services/composer.js b/lib/services/composer.js index c5093c88..2351873d 100644 --- a/lib/services/composer.js +++ b/lib/services/composer.js @@ -23,7 +23,7 @@ var log = require('./logger').setup({ file: __filename }); */ function resolveComponentReferences(uri, data, locals, filter = referenceProperty) { const referenceObjects = references.listDeepValuesByKey(data, filter), - schemaPromise = Object.keys(referenceObjects).length && (isComponent(uri) || isLayout(uri)) ? getSchema(uri) : Promise.resolve(); + schemaPromise = Object.keys(referenceObjects).length && (isComponent(uri) || isLayout(uri)) ? getSchema(uri) : bluebird.resolve(); return schemaPromise.then(schema => { const orderedRefs = orderReferences(referenceObjects, schema); From a3a55acc6e0e0e8ec4f105f2da9ae0ca6af8265e Mon Sep 17 00:00:00 2001 From: Micah Allen Date: Fri, 3 Apr 2020 15:29:01 -0400 Subject: [PATCH 3/4] Remove resolveComponentReferences/composePage breaking changes --- lib/render.js | 2 +- lib/routes/_components.js | 4 +++- lib/routes/_layouts.js | 4 +++- lib/routes/_pages.js | 2 +- lib/services/components.js | 2 +- lib/services/composer.js | 14 +++++++------- lib/services/composer.test.js | 12 ++++++------ lib/services/layouts.js | 2 +- lib/services/pages.js | 4 ++-- 9 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/render.js b/lib/render.js index 372301f8..289e94d5 100644 --- a/lib/render.js +++ b/lib/render.js @@ -177,7 +177,7 @@ function formDataFromLayout(locals, uri) { * @return {Promise} */ function formDataForRenderer(unresolvedData, { _layoutRef, _ref }, locals) { - return composer.resolveComponentReferences(unresolvedData, locals) + return composer.resolveComponentReferences(unresolvedData, locals, composer.referenceProperty, _ref) .then((data) => ({ data, options: { locals, _ref, _layoutRef } diff --git a/lib/routes/_components.js b/lib/routes/_components.js index b142b93a..8d7bf9bd 100644 --- a/lib/routes/_components.js +++ b/lib/routes/_components.js @@ -20,7 +20,9 @@ function getComposed(uri, locals) { return controller.get(uri, locals).then(function (data) { // TODO: Check that we can't just reference the composer // require here because otherwise it's a circular dependency (via html-composer) - return require('../services/composer').resolveComponentReferences(uri, data, locals); + const composer = require('../services/composer'); + + return composer.resolveComponentReferences(data, locals, composer.referenceProperty, uri); }); } diff --git a/lib/routes/_layouts.js b/lib/routes/_layouts.js index 5a21ab28..5de9d75b 100644 --- a/lib/routes/_layouts.js +++ b/lib/routes/_layouts.js @@ -23,7 +23,9 @@ function getComposed(uri, locals) { return controller.get(uri, locals).then(data => { // TODO: Check that we can't just reference the composer // require here because otherwise it's a circular dependency (via html-composer) - return require('../services/composer').resolveComponentReferences(uri, data, locals); + const composer = require('../services/composer'); + + return composer.resolveComponentReferences(data, locals, composer.referenceProperty, uri); }); } diff --git a/lib/routes/_pages.js b/lib/routes/_pages.js index 2571a4f1..5467e5b5 100644 --- a/lib/routes/_pages.js +++ b/lib/routes/_pages.js @@ -23,7 +23,7 @@ const _ = require('lodash'), */ function getComposed(uri, locals) { return db.get(uri) - .then(pageData => composer.composePage(uri, pageData, locals)); + .then(pageData => composer.composePage(pageData, locals)); } /** diff --git a/lib/services/components.js b/lib/services/components.js index cc64c19d..63a48224 100644 --- a/lib/services/components.js +++ b/lib/services/components.js @@ -64,7 +64,7 @@ function publish(uri, data, locals) { } return get(replaceVersion(uri), locals) - .then(latestData => composer.resolveComponentReferences(uri, latestData, locals, composer.filterBaseInstanceReferences)) + .then(latestData => composer.resolveComponentReferences(latestData, locals, composer.filterBaseInstanceReferences, uri)) .then(versionedData => dbOps.cascadingPut(put)(uri, versionedData, locals)); } diff --git a/lib/services/composer.js b/lib/services/composer.js index 2351873d..161f8b1a 100644 --- a/lib/services/composer.js +++ b/lib/services/composer.js @@ -15,15 +15,15 @@ var log = require('./logger').setup({ file: __filename }); /** * Compose a component, recursively filling in all component references with * instance data. - * @param {string} uri * @param {object} data * @param {object} locals Extra data that some GETs use * @param {function|string} [filter='_ref'] + * @param {string} [uri] * @returns {Promise} - Resolves with composed component data */ -function resolveComponentReferences(uri, data, locals, filter = referenceProperty) { +function resolveComponentReferences(data, locals, filter = referenceProperty, uri) { const referenceObjects = references.listDeepValuesByKey(data, filter), - schemaPromise = Object.keys(referenceObjects).length && (isComponent(uri) || isLayout(uri)) ? getSchema(uri) : bluebird.resolve(); + schemaPromise = uri && Object.keys(referenceObjects).length && (isComponent(uri) || isLayout(uri)) ? getSchema(uri) : bluebird.resolve(); return schemaPromise.then(schema => { const orderedRefs = orderReferences(referenceObjects, schema); @@ -34,7 +34,7 @@ function resolveComponentReferences(uri, data, locals, filter = referencePropert return components.get(ref, locals) .then(obj => { // the thing we got back might have its own references - return resolveComponentReferences(ref, obj, locals, filter).finally(() => { + return resolveComponentReferences(obj, locals, filter, ref).finally(() => { _.assign(referenceObject, _.omit(obj, referenceProperty)); }).catch(function (error) { const logObj = { @@ -95,18 +95,17 @@ function orderReferences(referenceObjects, schema) { /** * Compose a page, recursively filling in all component references with * instance data. - * @param {string} uri * @param {object} pageData * @param {object} locals * @return {Promise} - Resolves with composed page data */ -function composePage(uri, pageData, locals) { +function composePage(pageData, locals) { const layoutReference = pageData.layout, pageDataNoConf = references.omitPageConfiguration(pageData); return components.get(layoutReference) .then(layoutData => mapLayoutToPageData(pageDataNoConf, layoutData)) - .then(fullData => resolveComponentReferences(uri, fullData, locals)); + .then(fullData => resolveComponentReferences(fullData, locals)); } /** @@ -121,6 +120,7 @@ function filterBaseInstanceReferences(obj) { module.exports.resolveComponentReferences = resolveComponentReferences; module.exports.composePage = composePage; module.exports.filterBaseInstanceReferences = filterBaseInstanceReferences; +module.exports.referenceProperty = referenceProperty; // For testing module.exports.setLog = (fakeLogger) => log = fakeLogger; diff --git a/lib/services/composer.test.js b/lib/services/composer.test.js index 7b7ae302..3e724106 100644 --- a/lib/services/composer.test.js +++ b/lib/services/composer.test.js @@ -35,7 +35,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/b').returns(bluebird.resolve({g: 'h'})); components.get.withArgs('/c/e').returns(bluebird.resolve({i: 'j'})); - return fn('', data).then(function (result) { + return fn(data).then(function (result) { expect(result).to.deep.equal({ a: {_ref: '/c/b', g: 'h'}, c: {d: {_ref: '/c/e', i: 'j'}} @@ -53,7 +53,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/e').returns(bluebird.resolve({i: 'j', k: {_ref: '/c/m'}})); components.get.withArgs('/c/m').returns(bluebird.resolve({n: 'o'})); - return fn('', data).then(function (result) { + return fn(data).then(function (result) { expect(result).to.deep.equal({ a: {_ref: '/c/b', g: 'h'}, c: { @@ -83,7 +83,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/e').returns(bluebird.resolve({i: 'j', k: {_ref: '/c/m'}})); components.get.withArgs('/c/m').returns(bluebird.resolve({n: 'o'})); - return fn('', data, undefined, filter).then(function (result) { + return fn(data, undefined, filter).then(function (result) { expect(result).to.deep.equal({ a: {_ref: '/c/b', g: 'h'}, c: { @@ -107,7 +107,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/m').returns(bluebird.reject(myError)); // use done() rather than returning the promise, so we can catch and test errors - fn('', data).then(done).catch((error) => { + fn(data).then(done).catch((error) => { sinon.assert.calledOnce(logSpy); expect(error.message).to.equal('hello!'); done(); @@ -127,7 +127,7 @@ describe(_.startCase(filename), function () { components.get.withArgs('/c/m').returns(bluebird.reject(myError)); // use done() rather than returning the promise, so we can catch and test errors - fn('', data).then(done).catch((error) => { + fn(data).then(done).catch((error) => { sinon.assert.calledOnce(logSpy); expect(error.message).to.equal('hello!'); sinon.assert.calledWith(logSpy, 'error', 'hello!', { status: myError.status, cmpt: '/c/e', stack: myError.stack }); @@ -167,7 +167,7 @@ describe(_.startCase(filename), function () { layout: '/some/layout' }; - fn('', pageData) + fn(pageData) .then((result) => { expect(result).to.deep.equal({ head: [{ diff --git a/lib/services/layouts.js b/lib/services/layouts.js index bf856357..b07940cc 100644 --- a/lib/services/layouts.js +++ b/lib/services/layouts.js @@ -72,7 +72,7 @@ function publish(uri, data, locals) { } return get(replaceVersion(uri), locals) - .then(latestData => composer.resolveComponentReferences(uri, latestData, locals, composer.filterBaseInstanceReferences)) + .then(latestData => composer.resolveComponentReferences(latestData, locals, composer.filterBaseInstanceReferences, uri)) .then(versionedData => dbOps.cascadingPut(put)(uri, versionedData, locals)) .then(data => meta.publishLayout(uri, user).then(() => { bus.publish('publishLayout', { uri, data, user }); diff --git a/lib/services/pages.js b/lib/services/pages.js index 67a6300e..fcb60feb 100644 --- a/lib/services/pages.js +++ b/lib/services/pages.js @@ -72,7 +72,7 @@ function getPageClonePutOperations(pageData, locals) { // for all strings that are component references promises.push(components.get(pageValue, locals) // only follow the paths of instance references. Don't clone default components - .then(refData => composer.resolveComponentReferences(pageValue, refData, locals, isInstanceReferenceObject)) + .then(refData => composer.resolveComponentReferences(refData, locals, isInstanceReferenceObject, pageValue)) .then(resolvedData => { // for each instance reference within resolved data _.each(references.listDeepObjects(resolvedData, isInstanceReferenceObject), obj => { @@ -149,7 +149,7 @@ function getRecursivePublishedPutOperations(locals) { * 4) Get list of put operations needed */ return components.get(rootComponentRef, locals) - .then(data => composer.resolveComponentReferences(rootComponentRef, data, locals)) + .then(data => composer.resolveComponentReferences(data, locals, composer.referenceProperty, rootComponentRef)) .then(data => dbOps.getPutOperations(components.cmptPut, replaceVersion(rootComponentRef, 'published'), data, locals)); }; } From 08b09eba6acde790e20bbcd356ac0903c3d26903 Mon Sep 17 00:00:00 2001 From: Micah Allen Date: Mon, 6 Apr 2020 16:31:09 -0400 Subject: [PATCH 4/4] Make instance filters check for valid objects --- lib/services/composer.js | 2 +- lib/services/pages.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/services/composer.js b/lib/services/composer.js index 161f8b1a..452a4fb3 100644 --- a/lib/services/composer.js +++ b/lib/services/composer.js @@ -114,7 +114,7 @@ function composePage(pageData, locals) { * @returns {Boolean} */ function filterBaseInstanceReferences(obj) { - return _.isString(obj[referenceProperty]) && obj[referenceProperty].indexOf('/instances/') !== -1; + return _.isObject(obj) && _.isString(obj[referenceProperty]) && obj[referenceProperty].indexOf('/instances/') !== -1; } module.exports.resolveComponentReferences = resolveComponentReferences; diff --git a/lib/services/pages.js b/lib/services/pages.js index fcb60feb..4a0a8345 100644 --- a/lib/services/pages.js +++ b/lib/services/pages.js @@ -45,7 +45,7 @@ function addOp(uri, data, ops) { * @returns {boolean} */ function isInstanceReferenceObject(data) { - return _.isString(data._ref) && data._ref.indexOf('/instances/') > -1; + return _.isObject(data) && _.isString(data._ref) && data._ref.indexOf('/instances/') > -1; } /**