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

@@ -353,9 +353,14 @@ function applyPresets(sessionContext) {
       }
     }
 
-    const declarations = [];
+    const declarations = Object.values(parameters).map(v => ({
-    for (const v of Object.values(parameters))
+      path: v,
-      declarations.push([v, 1, { value: 1 }]);
+      pathGet: 1,
+      pathSet: null,
+      attrGet: null,
+      attrSet: null,
+      defer: true
+    }));
 
     session.rpcRequest(
       sessionContext,

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([
+    declarations.push({
-      path.slice(0, i),
+      path: path.slice(0, i),
-      t,
+      pathGet: t,
-      { object: 1, writable: 1, value: 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([
+  declarations.push({
-    ["Reboot"],
+    path: ["Reboot"],
-    1,
+    pathGet: 1,
-    { value: 1 },
+    pathSet: null,
-    null,
+    attrGet: { value: 1 },
-    { value: [sessionContext.timestamp] }
+    attrSet: { value: [sessionContext.timestamp] },
-  ]);
+    defer: true
+  });
 
   return true;
 }
 
 function reset(sessionContext, provision, declarations) {
-  declarations.push([
+  declarations.push({
-    ["FactoryReset"],
+    path: ["FactoryReset"],
-    1,
+    pathGet: 1,
-    { value: 1 },
+    pathSet: null,
-    null,
+    attrGet: { value: 1 },
-    { value: [sessionContext.timestamp] }
+    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({
-  declarations.push([
+    path: ["Downloads", alias],
-    ["Downloads", alias, "Download"],
+    pathGet: 1,
-    1,
+    pathSet: 1,
-    { value: 1 },
+    attrGet: null,
-    null,
+    attrSet: null,
-    { value: [sessionContext.timestamp] }
+    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;
 }

lib/device.js

@@ -98,8 +98,14 @@ function getAliasDeclarations(path, timestamp) {
       }
     }
 
-    if (prefix.length) decs.push([pattern, timestamp, { value: timestamp }]);
+    decs.push({
-    else decs.push([pattern, timestamp]);
+      path: pattern,
+      pathGet: timestamp,
+      pathSet: null,
+      attrGet: prefix.length ? { value: timestamp } : null,
+      attrSet: null,
+      defer: true
+    });
   }
 
   recursive(path, []);

lib/sandbox.js

@@ -197,36 +197,44 @@ function declare(path, timestamps, values) {
 
   const parsedPath = common.parsePath(path);
 
-  const declaration = [parsedPath, 1];
+  const declaration = {
-  const attrs = {};
+    path: parsedPath,
+    pathGet: 1,
+    pathSet: null,
+    attrGet: null,
+    attrSet: null,
+    defer: true
+  };
 
-  for (const attrName in values) {
+  const attrs = new Set();
-    if (!timestamps[attrName]) timestamps[attrName] = 1;
 
+  for (const [attrName, attrValue] of Object.entries(values)) {
     if (attrName === "path") {
-      declaration[3] = values.path;
+      declaration.pathSet = attrValue;
     } else {
-      attrs[attrName] = 1;
+      attrs.add(attrName);
-      declaration[4] = declaration[4] || {};
+      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];
+        declaration.attrSet.value = [values.value];
-      else declaration[4][attrName] = values[attrName];
+      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;
+      attrs.add(attrName);
-      declaration[2] = declaration[2] || {};
+      if (!declaration.attrGet) declaration.attrGet = {};
-      declaration[2][attrName] = timestamps[attrName];
+      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(

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]) {
+        if (ad.attrGet) {
-          attrTrackers = Object.keys(ad[2]);
+          attrTrackers = Object.keys(ad.attrGet);
-          mergeAttributeTimestamps(p, ad[2]);
+          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])) {
+      if (Array.isArray(declaration.pathSet)) {
-        minInstances = declaration[3][0];
+        minInstances = declaration.pathSet[0];
-        maxInstances = declaration[3][1];
+        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;
+          if (d.pathGet > sessionContext.timestamp)
-          for (const [k, v] of Object.entries(d[2])) {
+            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))
+          for (const ad of device.getAliasDeclarations(d.path, 1))
-            loadPath(sessionContext, ad[0]);
+            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))
+      for (const ad of device.getAliasDeclarations(d.path, 1))
-        loadPath(sessionContext, ad[0]);
+        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) {