Fixed cli filter to support nested-namespace packages [issue #1854]

cli/filter.js

@@ -0,0 +1,114 @@
+"use strict";
+var fs       = require("fs"),
+    path     = require("path"),
+    protobuf = require("protobufjs");
+
+function getFullTypeName(type) {
+    var seen = [];
+    while(type && type.name) {
+        seen.unshift(type.name);
+        type = type.parent;
+    }
+    return seen.join(".");
+}
+
+/**
+ * DFS to get all message dependencies, cache in filterResult and saved set.
+ * @param {Root} root  The protobuf root instance
+ * @param {object} filterParams
+ * @param {Map} filterResult The result of message you need and their dependencies.
+ * @param {Map} saved Set that records which messages are in the filterResults using fully qualified name.
+ * @returns {undefined}  Does not return a value
+ */
+function dfsFilterDependencies(root, message, filterResult, saved) {
+    if (message instanceof protobuf.Type) {
+        if (saved.has(`${message.fullName}`)) return;
+        saved.add(`${message.fullName}`);
+        for (var field of message.fieldsArray) {
+            if (field.resolvedType) {
+                // handle nested message type
+                if (field.resolvedType.parent.name === message.name) {
+                    var nestedMessage = message.nested[field.resolvedType.name];
+                    dfsFilterDependencies(root, nestedMessage, filterResult, saved);
+                    continue;
+                }
+                var fullTypeName = getFullTypeName(field.resolvedType);
+                doFilterMessage(root, { messageNames: [fullTypeName] }, filterResult, saved);
+            }
+        }
+    }
+}
+
+/**
+ * DFS to get all message you need and their dependencies, cache in filterMap.
+ * @param {Root} root  The protobuf root instance
+ * @param {object} filterParams
+ * @param {Map} filterResult The result of message you need and their dependencies.
+ * @param {Map} saved Set that records which messages are in the filterResults using fully qualified name.
+ * @returns {undefined}  Does not return a value
+ */
+function doFilterMessage(root, filterParams, filterResult, resolved) {
+    var messageNameFilter = filterParams.messageNames;
+    filterResult.set(root, new Set());
+
+    for (var messageFullName of messageNameFilter) {
+        var nameSplit = messageFullName.split(".");
+        var namespaces = nameSplit.slice(0, -1);
+        var messageName = nameSplit.slice(-1);
+
+        // traverse path to ensure namespace exists, add namespaces if first time
+        var messageNamespace = root;
+        if (nameSplit.length > 1) {
+            var seen = [];
+            for (var packageName of namespaces) {
+                seen.push(packageName);
+                messageNamespace = messageNamespace.nested[packageName];
+                if (!messageNamespace || !(messageNamespace instanceof protobuf.Namespace)) {
+                    throw new Error(`namespace not found ${seen.join(".")}}`);
+                }
+                if (!filterResult.has(messageNamespace)) {
+                    filterResult.set(messageNamespace, new Set());
+                }
+            }
+        }
+
+        // ensure message exists in namespace
+        var message = messageNamespace.nested[messageName];
+        if (!message) {
+            throw new Error(`message not found ${messageFullName}`);
+        }
+        filterResult.get(messageNamespace).add(message);
+
+        // dfs to find all dependencies
+        dfsFilterDependencies(root, message, filterResult, resolved);
+    }
+}
+
+/**
+ * Filter scanned protobuf types for specific types and their dependencies. All others will be dropped from Root and will not be in codegen.
+ * @param {Root} root the protobuf root instance
+ * @param {object} filterParams
+ * @param {string[]} filterParams.messageNames  The message names array in the root namespace you want gen for. example: [msg1, msg2]
+ */
+exports.filterMessage = function (root, filterParams) {
+    var filterResult = new Map();
+    var resolved = new Set();
+    doFilterMessage(root, filterParams, filterResult, resolved);
+
+    // mutate the passed in Root object, removing types not found in the filterResult
+    var filterChildren = function(parent, ns) {
+        if (ns instanceof protobuf.Type || ns instanceof protobuf.Enum) {
+            return filterResult.get(parent).has(ns);
+        } else if (ns instanceof protobuf.Namespace) {
+            var filteredChildren = filterResult.get(ns);
+            if (!filteredChildren) {
+                return false;
+            }
+            ns._nestedArray = ns._nestedArray.filter(child => filterChildren(ns, child));
+
+            return true;
+        }
+        return true;
+    };
+    root._nestedArray = root._nestedArray.filter(child => filterChildren(root, child));
+};

cli/pbjs.js

@@ -5,6 +5,7 @@ var path     = require("path"),
     chalk    = require("chalk"),
     pkg      = require("./package.json"),
     util     = require("./util"),
+    filter   = require("./filter"),
     glob     = require("glob"),
     protobuf = require("protobufjs");
 
@@ -315,8 +316,8 @@ exports.main = function main(args, callback) {
         if (argv.filter) {
             // This is a piece of degradable logic
             try {
-                const needMessage = JSON.parse(fs.readFileSync(argv.filter));
-                util.filterMessage(root, needMessage);
+                const filterParams = JSON.parse(fs.readFileSync(argv.filter));
+                filter.filterMessage(root, filterParams);
             } catch (error) {
                 process.stderr.write(`The filter not work, please check whether the file is correct: ${error.message}\n`);
             }

cli/util.js

@@ -124,120 +124,3 @@ exports.pad = function(str, len, l) {
         str = l ? str + " " : " " + str;
     return str;
 };
-
-
-/**
- * DFS to get all message dependencies, cache in filterMap.
- * @param {Root} root  The protobuf root instance
- * @param {Message} message  The message need to process.
- * @param {Map} filterMap  The result of message you need and their dependencies.
- * @param {Map} flatMap  A flag to record whether the message was searched.
- * @returns {undefined}  Does not return a value
- */
-function dfsFilterMessageDependencies(root, message, filterMap, flatMap) {
-    if (message instanceof protobuf.Type) {
-        if (flatMap.get(`${message.fullName}`)) return;
-        flatMap.set(`${message.fullName}`, true);
-        for (var field of message.fieldsArray) {
-            if (field.resolvedType) {
-                // a nested message
-                if (field.resolvedType.parent.name === message.name) {
-                    var nestedMessage = message.nested[field.resolvedType.name];
-                    dfsFilterMessageDependencies(root, nestedMessage, filterMap, flatMap);
-                    continue;
-                }
-                var packageName = field.resolvedType.parent.name;
-                var typeName = field.resolvedType.name;
-                var fullName = packageName ? `${packageName}.${typeName}` : typeName;
-                doFilterMessage(root, { messageNames: [fullName] }, filterMap, flatMap, packageName);
-            }
-        }
-    }
-}
-
-/**
- * DFS to get all message you need and their dependencies, cache in filterMap.
- * @param {Root} root  The protobuf root instance
- * @param {object} needMessageConfig  Need message config:
- * @param {string[]} needMessageConfig.messageNames  The message names array in the root namespace you need to gen. example: [msg1, msg2]
- * @param {Map} filterMap The result of message you need and their dependencies.
- * @param {Map} flatMap A flag to record whether the message was searched.
- * @param {string} currentPackageName  Current package name
- * @returns {undefined}  Does not return a value
- */
-function doFilterMessage(root, needMessageConfig, filterMap, flatMap, currentPackageName) {
-    var needMessageNames = needMessageConfig.messageNames;
-
-    for (var messageFullName of needMessageNames) {
-        var nameSplit = messageFullName.split(".");
-        var packageName = "";
-        var messageName = "";
-        if (nameSplit.length > 1) {
-            packageName = nameSplit[0];
-            messageName = nameSplit[1];
-        } else {
-            messageName = nameSplit[0];
-        }
-
-        // in Namespace
-        if (packageName) {
-            var ns = root.nested[packageName];
-            if (!ns || !(ns instanceof protobuf.Namespace)) {
-                throw new Error(`package not foud ${currentPackageName}.${messageName}`);
-            }
-
-            doFilterMessage(root, { messageNames: [messageName] }, filterMap, flatMap, packageName);
-        } else {
-            var message = root.nested[messageName];
-
-            if (currentPackageName) {
-                message = root.nested[currentPackageName].nested[messageName];
-            }
-
-            if (!message) {
-                throw new Error(`message not foud ${currentPackageName}.${messageName}`);
-            }
-
-            var set = filterMap.get(currentPackageName);
-            if (!filterMap.has(currentPackageName)) {
-                set = new Set();
-                filterMap.set(currentPackageName, set);
-            }
-
-            set.add(messageName);
-
-            // dfs to find all dependencies
-            dfsFilterMessageDependencies(root, message, filterMap, flatMap, currentPackageName);
-        }
-    }
-}
-
-/**
- * filter the message you need and their dependencies, all others will be delete from root.
- * @param {Root} root  Root the protobuf root instance
- * @param {object} needMessageConfig  Need message config:
- * @param {string[]} needMessageConfig.messageNames  Tthe message names array in the root namespace you need to gen. example: [msg1, msg2]
- * @returns {boolean} True if a message should present in the generated files
- */
-exports.filterMessage = function (root, needMessageConfig) {
-    var filterMap = new Map();
-    var flatMap = new Map();
-    doFilterMessage(root, needMessageConfig, filterMap, flatMap, "");
-    root._nestedArray = root._nestedArray.filter(ns => {
-        if (ns instanceof protobuf.Type || ns instanceof protobuf.Enum) {
-            return filterMap.get("").has(ns.name);
-        } else if (ns instanceof protobuf.Namespace) {
-            if (!filterMap.has(ns.name)) {
-                return false;
-            }
-            ns._nestedArray = ns._nestedArray.filter(nns => {
-                const nnsSet = filterMap.get(ns.name);
-                return nnsSet.has(nns.name);
-            });
-
-            return true;
-        }
-        return true;
-    });
-};
-

tests/cli.js

@@ -171,11 +171,11 @@ tape.test("pbjs generates static code with message filter", function (test) {
         root.resolveAll();
 
         var staticTarget = require("../cli/targets/static");
-        var util = require("../cli/util");
+        var filter = require("../cli/filter");
 
-        const needMessageConfig = JSON.parse(fs.readFileSync("tests/data/cli/filter.json"));
+        const filterParams = JSON.parse(fs.readFileSync("tests/data/cli/filter.json"));
 
-        util.filterMessage(root, needMessageConfig);
+        filter.filterMessage(root, filterParams);
 
         staticTarget(root, {
             create: true,
@@ -214,3 +214,54 @@ tape.test("pbjs generates static code with message filter", function (test) {
         });
     });
 });
+
+tape.test("pbjs generates static code with message filter with long package names", function (test) {
+    cliTest(test, function () {
+        var root = protobuf.loadSync("tests/data/cli/test-filter-long-package.proto");
+        root.resolveAll();
+
+        var staticTarget = require("../cli/targets/static");
+        var filter = require("../cli/filter");
+
+        const filterParams = JSON.parse(fs.readFileSync("tests/data/cli/filter-long-package.json"));
+
+        filter.filterMessage(root, filterParams);
+
+        staticTarget(root, {
+            create: true,
+            decode: true,
+            encode: true,
+            convert: true,
+            "null-defaults": true,
+        }, function (err, jsCode) {
+            test.error(err, 'static code generation worked');
+
+            // jsCode is the generated code; we'll eval it
+            // (since this is what we normally does with the code, right?)
+            // This is a test code. Do not use this in production.
+            var $protobuf = protobuf;
+            eval(jsCode);
+
+            console.log(protobuf.roots);
+            console.log("Next part", protobuf.roots.default.filtertest);
+
+            var NeedMessage1 = protobuf.roots.default.filtertest.more.parts.NeedMessage1;
+            var NeedMessage2 = protobuf.roots.default.filtertest.more.parts.NeedMessage2;
+            var DependentMessage1 = protobuf.roots.default.filtertest.more.parts.DependentMessage1;
+            var DependentMessageFromImport = protobuf.roots.default.DependentMessageFromImport;
+
+            var NotNeedMessageInRootFile = protobuf.roots.default.filtertest.more.parts.NotNeedMessageInRootFile;
+            var NotNeedMessageInImportFile = protobuf.roots.default.NotNeedMessageInImportFile;
+
+            test.ok(NeedMessage1, "NeedMessage1 is loaded");
+            test.ok(NeedMessage2, "NeedMessage2 is loaded");
+            test.ok(DependentMessage1, "DependentMessage1 is loaded");
+            test.ok(DependentMessageFromImport, "DependentMessageFromImport is loaded");
+
+            test.notOk(NotNeedMessageInImportFile, "NotNeedMessageInImportFile is not loaded");
+            test.notOk(NotNeedMessageInRootFile, "NotNeedMessageInRootFile is not loaded");
+
+            test.end();
+        });
+    });
+});

tests/data/cli/filter-long-package.json

@@ -0,0 +1,3 @@
+{
+  "messageNames": ["filtertest.more.parts.NeedMessage1", "filtertest.more.parts.NeedMessage2"]
+}

tests/data/cli/test-filter-long-package.proto

@@ -0,0 +1,21 @@
+package filtertest.more.parts;
+import "./test-filter-import.proto";
+
+message NeedMessage1 {
+  optional uint32 test1 = 1;
+  optional NeedMessage2 needMessage2 = 2;
+  optional DependentMessage1 dependentMessage1 = 3;
+  optional DependentMessageFromImport dependentMessage2 = 4;
+}
+
+message NeedMessage2 {
+  optional uint32 test1 = 1;
+}
+
+message DependentMessage1 {
+  optional uint32 test1 = 1;
+}
+
+message NotNeedMessageInRootFile {
+  optional uint32 test1 = 1;
+}