From 1a56915d4bf06f47bd2efddb3a593bd3da99e2ee Mon Sep 17 00:00:00 2001 From: Zaid Abdulla Date: Sat, 15 Sep 2018 00:09:58 -0700 Subject: [PATCH] Defer tail declarations declare() calls that aren't followed by commit() are applied at the end of the provision script. This change defers those until all provisions have been executed. This prevents certain cases of preset loop faults. --- lib/cwmp.js | 11 ++-- lib/default-provisions.js | 105 +++++++++++++++++++++++++++----------- lib/device.js | 10 +++- lib/sandbox.js | 39 ++++++++------ lib/session.js | 81 +++++++++++++++++------------ 5 files changed, 163 insertions(+), 83 deletions(-) diff --git a/lib/cwmp.js b/lib/cwmp.js index cb3a2ce..8ab4404 100644 --- a/lib/cwmp.js +++ b/lib/cwmp.js @@ -353,9 +353,14 @@ function applyPresets(sessionContext) { } } - const declarations = []; - for (const v of Object.values(parameters)) - declarations.push([v, 1, { value: 1 }]); + const declarations = Object.values(parameters).map(v => ({ + path: v, + pathGet: 1, + pathSet: null, + attrGet: null, + attrSet: null, + defer: true + })); session.rpcRequest( sessionContext, diff --git a/lib/default-provisions.js b/lib/default-provisions.js index 1b71de6..362be69 100644 --- a/lib/default-provisions.js +++ b/lib/default-provisions.js @@ -35,11 +35,14 @@ function refresh(sessionContext, provision, declarations) { const t = scheduling.interval(sessionContext.timestamp, every, offset); for (let i = l; i < path.length; ++i) { - declarations.push([ - path.slice(0, i), - t, - { object: 1, writable: 1, value: t } - ]); + declarations.push({ + path: path.slice(0, i), + pathGet: t, + pathSet: null, + attrGet: { object: 1, writable: 1, value: t }, + attrSet: null, + defer: true + }); } return true; @@ -47,38 +50,55 @@ function refresh(sessionContext, provision, declarations) { function value(sessionContext, provision, declarations) { const path = common.parsePath(provision[1]); - declarations.push([path, 1, { value: 1 }, null, { value: [provision[2]] }]); + declarations.push({ + path: path, + pathGet: 1, + pathSet: null, + attrGet: { value: 1 }, + attrSet: { value: [provision[2]] }, + defer: true + }); return true; } function tag(sessionContext, provision, declarations) { const path = ["Tags", provision[1]]; - declarations.push([path, 1, { value: 1 }, null, { value: [provision[2]] }]); + + declarations.push({ + path: path, + pathGet: 1, + pathSet: null, + attrGet: { value: 1 }, + attrSet: { value: [provision[2]] }, + defer: true + }); return true; } function reboot(sessionContext, provision, declarations) { - declarations.push([ - ["Reboot"], - 1, - { value: 1 }, - null, - { value: [sessionContext.timestamp] } - ]); + declarations.push({ + path: ["Reboot"], + pathGet: 1, + pathSet: null, + attrGet: { value: 1 }, + attrSet: { value: [sessionContext.timestamp] }, + defer: true + }); return true; } function reset(sessionContext, provision, declarations) { - declarations.push([ - ["FactoryReset"], - 1, - { value: 1 }, - null, - { value: [sessionContext.timestamp] } - ]); + declarations.push({ + path: ["FactoryReset"], + pathGet: 1, + pathSet: null, + attrGet: { value: 1 }, + attrSet: { value: [sessionContext.timestamp] }, + defer: true + }); return true; } @@ -93,14 +113,23 @@ function download(sessionContext, provision, declarations) { provision[3] || "" ]; - declarations.push([["Downloads", alias], 1, {}, 1]); - declarations.push([ - ["Downloads", alias, "Download"], - 1, - { value: 1 }, - null, - { value: [sessionContext.timestamp] } - ]); + declarations.push({ + path: ["Downloads", alias], + pathGet: 1, + pathSet: 1, + attrGet: null, + attrSet: null, + defer: true + }); + + declarations.push({ + path: ["Downloads", alias, "Download"], + pathGet: 1, + pathSet: null, + attrGet: { value: 1 }, + attrSet: { value: [sessionContext.timestamp] }, + defer: true + }); return true; } @@ -119,7 +148,14 @@ function instances( const path = common.parsePath(provision[1]); if (provision[2][0] === "+" || provision[2][0] === "-") { - declarations.push([path, 1]); + declarations.push({ + path: path, + pathGet: 1, + pathSet: null, + attrGet: null, + attrSet: null, + defer: true + }); if (endRevision === startRevision) return false; @@ -131,7 +167,14 @@ function instances( count = Math.max(0, unpacked.length + count); } - declarations.push([path, 1, null, count]); + declarations.push({ + path: path, + pathGet: 1, + pathSet: count, + attrGet: null, + attrSet: null, + defer: true + }); return true; } diff --git a/lib/device.js b/lib/device.js index 781a7bc..267bc1a 100644 --- a/lib/device.js +++ b/lib/device.js @@ -98,8 +98,14 @@ function getAliasDeclarations(path, timestamp) { } } - if (prefix.length) decs.push([pattern, timestamp, { value: timestamp }]); - else decs.push([pattern, timestamp]); + decs.push({ + path: pattern, + pathGet: timestamp, + pathSet: null, + attrGet: prefix.length ? { value: timestamp } : null, + attrSet: null, + defer: true + }); } recursive(path, []); diff --git a/lib/sandbox.js b/lib/sandbox.js index f2eb5b4..fe796c5 100644 --- a/lib/sandbox.js +++ b/lib/sandbox.js @@ -197,36 +197,44 @@ function declare(path, timestamps, values) { const parsedPath = common.parsePath(path); - const declaration = [parsedPath, 1]; - const attrs = {}; + const declaration = { + path: parsedPath, + pathGet: 1, + pathSet: null, + attrGet: null, + attrSet: null, + defer: true + }; - for (const attrName in values) { - if (!timestamps[attrName]) timestamps[attrName] = 1; + const attrs = new Set(); + for (const [attrName, attrValue] of Object.entries(values)) { if (attrName === "path") { - declaration[3] = values.path; + declaration.pathSet = attrValue; } else { - attrs[attrName] = 1; - declaration[4] = declaration[4] || {}; + attrs.add(attrName); + if (!declaration.attrGet) declaration.attrGet = {}; + if (!declaration.attrSet) declaration.attrSet = {}; + declaration.attrGet[attrName] = 1; if (attrName === "value" && !Array.isArray(values.value)) - declaration[4].value = [values.value]; - else declaration[4][attrName] = values[attrName]; + declaration.attrSet.value = [values.value]; + else declaration.attrSet[attrName] = values[attrName]; } } - for (const attrName in timestamps) { + for (const [attrName, attrTimestamp] of Object.entries(timestamps)) { if (attrName === "path") { - declaration[1] = timestamps.path; + declaration.pathGet = attrTimestamp; } else { - attrs[attrName] = 1; - declaration[2] = declaration[2] || {}; - declaration[2][attrName] = timestamps[attrName]; + attrs.add(attrName); + if (!declaration.attrGet) declaration.attrGet = {}; + declaration.attrGet[attrName] = attrTimestamp; } } state.declarations.push(declaration); - return new ParameterWrapper(parsedPath, Object.keys(attrs)); + return new ParameterWrapper(parsedPath, attrs); } function clear(path, timestamp, attributes) { @@ -241,6 +249,7 @@ function commit() { state.uncommitted = false; if (state.revision === state.maxRevision + 1) { + for (const d of state.declarations) d.defer = false; throw COMMIT; } else if (state.revision > state.maxRevision + 1) { throw new Error( diff --git a/lib/session.js b/lib/session.js index a67c48a..b4e2b12 100644 --- a/lib/session.js +++ b/lib/session.js @@ -588,6 +588,7 @@ function runProvisions( if ((counter -= 2) === 1) { allDeclarations = Array.prototype.concat.apply([], allDeclarations); + if (done) for (const d of allDeclarations) d.defer = false; allClear = Array.prototype.concat.apply([], allClear); callback(null, null, done, allDeclarations, allClear); } @@ -597,6 +598,7 @@ function runProvisions( if ((counter -= 2) === 1) { allDeclarations = Array.prototype.concat.apply([], allDeclarations); + if (done) for (const d of allDeclarations) d.defer = false; allClear = Array.prototype.concat.apply([], allClear); callback(null, null, done, allDeclarations, allClear); } @@ -720,6 +722,7 @@ function runVirtualParameters( if ((counter -= 2) === 1) { allDeclarations = Array.prototype.concat.apply([], allDeclarations); + if (done) for (const d of allDeclarations) d.defer = false; allClear = Array.prototype.concat.apply([], allClear); callback( null, @@ -735,6 +738,7 @@ function runVirtualParameters( if ((counter -= 2) === 1) { allDeclarations = Array.prototype.concat.apply([], allDeclarations); + if (done) for (const d of allDeclarations) d.defer = false; allClear = Array.prototype.concat.apply([], allClear); callback( null, @@ -787,10 +791,10 @@ function runDeclarations(sessionContext, declarations) { } } - function mergeAttributeValues(p, attrs) { + function mergeAttributeValues(p, attrs, defer) { let cur = allDeclareAttributeValues.get(p); if (!cur) { - allDeclareAttributeValues.set(p, attrs); + if (!defer) allDeclareAttributeValues.set(p, attrs); } else { cur = Object.assign({}, cur, attrs); allDeclareAttributeValues.set(p, cur); @@ -798,7 +802,7 @@ function runDeclarations(sessionContext, declarations) { } for (const declaration of declarations) { - let path = common.addPathMeta(declaration[0]); + let path = common.addPathMeta(declaration.path); let unpacked; if ((path.alias | path.wildcard) & 1 || path[0] === "VirtualParameters") { @@ -817,17 +821,20 @@ function runDeclarations(sessionContext, declarations) { sessionContext.deviceData.paths.add(["FactoryReset"]); if (path.alias) { - const aliasDecs = device.getAliasDeclarations(path, declaration[1] || 1); + const aliasDecs = device.getAliasDeclarations( + path, + declaration.pathGet || 1 + ); for (const ad of aliasDecs) { - const p = sessionContext.deviceData.paths.add(ad[0]); + const p = sessionContext.deviceData.paths.add(ad.path); allDeclareTimestamps.set( p, - Math.max(ad[1] || 1, allDeclareTimestamps.get(p) || 0) + Math.max(ad.pathGet || 1, allDeclareTimestamps.get(p) || 0) ); let attrTrackers; - if (ad[2]) { - attrTrackers = Object.keys(ad[2]); - mergeAttributeTimestamps(p, ad[2]); + if (ad.attrGet) { + attrTrackers = Object.keys(ad.attrGet); + mergeAttributeTimestamps(p, ad.attrGet); } device.track( @@ -842,38 +849,41 @@ function runDeclarations(sessionContext, declarations) { for (const u of unpacked) { allDeclareTimestamps.set( u, - Math.max(declaration[1] || 1, allDeclareTimestamps.get(u) || 0) + Math.max(declaration.pathGet || 1, allDeclareTimestamps.get(u) || 0) ); - if (declaration[2]) mergeAttributeTimestamps(u, declaration[2]); + if (declaration.attrGet) + mergeAttributeTimestamps(u, declaration.attrGet); } } else { path = sessionContext.deviceData.paths.add(path); allDeclareTimestamps.set( path, - Math.max(declaration[1] || 1, allDeclareTimestamps.get(path) || 0) + Math.max(declaration.pathGet || 1, allDeclareTimestamps.get(path) || 0) ); - if (declaration[2]) mergeAttributeTimestamps(path, declaration[2]); + if (declaration.attrGet) + mergeAttributeTimestamps(path, declaration.attrGet); device.track(sessionContext.deviceData, path, "prerequisite"); } - if (declaration[4]) { + if (declaration.attrSet) { if (path.alias | path.wildcard) { if (!unpacked) unpacked = device.unpack(sessionContext.deviceData, path); - for (const u of unpacked) mergeAttributeValues(u, declaration[4]); + for (const u of unpacked) + mergeAttributeValues(u, declaration.attrSet, declaration.defer); } else { - mergeAttributeValues(path, declaration[4]); + mergeAttributeValues(path, declaration.attrSet, declaration.defer); } } - if (declaration[3] != null) { + if (declaration.pathSet != null) { let minInstances, maxInstances; - if (Array.isArray(declaration[3])) { - minInstances = declaration[3][0]; - maxInstances = declaration[3][1]; + if (Array.isArray(declaration.pathSet)) { + minInstances = declaration.pathSet[0]; + maxInstances = declaration.pathSet[1]; } else { - minInstances = maxInstances = declaration[3]; + minInstances = maxInstances = declaration.pathSet; } let parent = common.addPathMeta(path.slice(0, -1)); @@ -903,7 +913,8 @@ function runDeclarations(sessionContext, declarations) { unpacked, keys, minInstances, - maxInstances + maxInstances, + declaration.defer ); } else { const parentsUnpacked = device.unpack( @@ -921,7 +932,8 @@ function runDeclarations(sessionContext, declarations) { ), keys, minInstances, - maxInstances + maxInstances, + declaration.defer ); } } @@ -1002,14 +1014,15 @@ function rpcRequest(sessionContext, _declarations, callback) { for (const d of decs) { // Enforce max timestamp - if (d[1] > sessionContext.timestamp) d[1] = sessionContext.timestamp; - for (const [k, v] of Object.entries(d[2])) { + if (d.pathGet > sessionContext.timestamp) + d.pathGet = sessionContext.timestamp; + for (const [k, v] of Object.entries(d.attrGet)) { if (v > sessionContext.timestamp) - d[2][k] = sessionContext.timestamp; + d.attrGet[k] = sessionContext.timestamp; } - for (const ad of device.getAliasDeclarations(d[0], 1)) - loadPath(sessionContext, ad[0]); + for (const ad of device.getAliasDeclarations(d.path, 1)) + loadPath(sessionContext, ad.path); } clear(sessionContext, toClear, err => { @@ -1032,8 +1045,8 @@ function rpcRequest(sessionContext, _declarations, callback) { ); for (const d of _declarations) { - for (const ad of device.getAliasDeclarations(d[0], 1)) - loadPath(sessionContext, ad[0]); + for (const ad of device.getAliasDeclarations(d.path, 1)) + loadPath(sessionContext, ad.path); } return void loadParameters(sessionContext, err => { @@ -2115,7 +2128,8 @@ function processInstances( parameters, keys, minInstances, - maxInstances + maxInstances, + defer ) { let instancesToCreate, instancesToDelete; if (parent[0] === "Downloads") { @@ -2136,6 +2150,9 @@ function processInstances( } } + if (defer && instancesToCreate.size === 0 && instancesToDelete.size === 0) + return; + let counter = 0; for (const p of parameters) { ++counter; @@ -2559,7 +2576,7 @@ function deserialize(sessionContextString, callback) { virtualParametersCache.set(sessionContext, virtualParameters); for (const decs of sessionContext.declarations) - for (const d of decs) common.addPathMeta(d[0]); + for (const d of decs) common.addPathMeta(d.path); const deviceData = initDeviceData(); for (const r of sessionContext.deviceData) {