feat: parsed options (protobufjs#1256)

* Add option parsing tests to ensure no regressions are caused

* Properly parse option values into objects in addition to the regular option parsing. Repeated options are preserved as well as repeated fields within nested options.
Parsed options are kept in a parsedOptions field on every level (message, field etc.)

* fix: bad merge

* fix: lint

* fix: lint

* fix: lint

* fix: lint

* fix: lint

* fix: build types

Co-authored-by: Alexander Fenster <fenster@google.com>
Co-authored-by: Alexander Fenster <github@fenster.name>

Author: eyalpost

index.d.ts

@@ -859,6 +859,9 @@ export abstract class ReflectionObject {
     /** Options. */
     public options?: { [k: string]: any };
 
+    /** Options. */
+    public parsedOptions?: { [k: string]: any }[];
+
     /** Unique name within its namespace. */
     public name: string;
 
@@ -920,6 +923,15 @@ export abstract class ReflectionObject {
      */
     public setOption(name: string, value: any, ifNotSet?: boolean): ReflectionObject;
 
+    /**
+     * Sets a parsed option.
+     * @param name parsed Option name
+     * @param value Option value
+     * @param [propName] dot '.' delimited full path of property within the option to set. if undefined\empty, will add a new option with that value
+     * @returns `this`
+     */
+    public setParsedOption(name: string, value: any, propName?: string): ReflectionObject;
+
     /**
      * Sets multiple options.
      * @param options Options to set
@@ -2162,6 +2174,15 @@ export namespace util {
      */
     function decorateEnum(object: object): Enum;
 
+    /**
+     * Sets the value of a property by property path. If a value already exists, it is turned to an array
+     * @param dst Destination object
+     * @param path dot '.' delimited path of the property to set
+     * @param value the value to set
+     * @returns Destination object
+     */
+    function setProperty(dst: { [k: string]: any }, path: string, value: object);
+
     /** Decorator root (TypeScript). */
     let decorateRoot: Root;
 

src/object.js

@@ -29,6 +29,12 @@ function ReflectionObject(name, options) {
      */
     this.options = options; // toJSON
 
+    /**
+     * Parsed Options.
+     * @type {Array.<Object.<string,*>>|undefined}
+     */
+    this.parsedOptions = null;
+
     /**
      * Unique name within its namespace.
      * @type {string}
@@ -169,6 +175,43 @@ ReflectionObject.prototype.setOption = function setOption(name, value, ifNotSet)
     return this;
 };
 
+/**
+ * Sets a parsed option.
+ * @param {string} name parsed Option name
+ * @param {*} value Option value
+ * @param {string} propName dot '.' delimited full path of property within the option to set. if undefined\empty, will add a new option with that value
+ * @returns {ReflectionObject} `this`
+ */
+ReflectionObject.prototype.setParsedOption = function setParsedOption(name, value, propName) {
+    if (!this.parsedOptions) {
+        this.parsedOptions = [];
+    }
+    var parsedOptions = this.parsedOptions;
+    if (propName) {
+        // If setting a sub property of an option then try to merge it
+        // with an existing option
+        var opt = parsedOptions.find(function (opt) {
+            return Object.prototype.hasOwnProperty.call(opt, name);
+        });
+        if (opt) {
+            // If we found an existing option - just merge the property value
+            var newValue = opt[name];
+            util.setProperty(newValue, propName, value);
+        } else {
+            // otherwise, create a new option, set it's property and add it to the list
+            opt = {};
+            opt[name] = util.setProperty({}, propName, value);
+            parsedOptions.push(opt);
+        }
+    } else {
+        // Always create a new option when setting the value of the option itself
+        var newOpt = {};
+        newOpt[name] = value;
+        parsedOptions.push(newOpt);
+    }
+    return this;
+};
+
 /**
  * Sets multiple options.
  * @param {Object.<string,*>} options Options to set

src/parse.js

@@ -542,39 +542,58 @@ function parse(source, root, options) {
             throw illegal(token, "name");
 
         var name = token;
+        var option = name;
+        var propName;
+
         if (isCustom) {
             skip(")");
             name = "(" + name + ")";
+            option = name;
             token = peek();
             if (fqTypeRefRe.test(token)) {
+                propName = token.substr(1); //remove '.' before property name
                 name += token;
                 next();
             }
         }
         skip("=");
-        parseOptionValue(parent, name);
+        var optionValue = parseOptionValue(parent, name);
+        setParsedOption(parent, option, optionValue, propName);
     }
 
     function parseOptionValue(parent, name) {
         if (skip("{", true)) { // { a: "foo" b { c: "bar" } }
+            var result = {};
             while (!skip("}", true)) {
                 /* istanbul ignore if */
                 if (!nameRe.test(token = next()))
                     throw illegal(token, "name");
 
+                var value;
+                var propName = token;
                 if (peek() === "{")
-                    parseOptionValue(parent, name + "." + token);
+                    value = parseOptionValue(parent, name + "." + token);
                 else {
                     skip(":");
                     if (peek() === "{")
-                        parseOptionValue(parent, name + "." + token);
-                    else
-                        setOption(parent, name + "." + token, readValue(true));
+                        value = parseOptionValue(parent, name + "." + token);
+                    else {
+                        value = readValue(true);
+                        setOption(parent, name + "." + token, value);
+                    }
                 }
+                var prevValue = result[propName];
+                if (prevValue)
+                    value = [].concat(prevValue).concat(value);
+                result[propName] = value;
                 skip(",", true);
             }
-        } else
-            setOption(parent, name, readValue(true));
+            return result;
+        }
+
+        var simpleValue = readValue(true);
+        setOption(parent, name, simpleValue);
+        return simpleValue;
         // Does not enforce a delimiter to be universal
     }
 
@@ -583,6 +602,11 @@ function parse(source, root, options) {
             parent.setOption(name, value);
     }
 
+    function setParsedOption(parent, name, value, propName) {
+        if (parent.setParsedOption)
+            parent.setParsedOption(name, value, propName);
+    }
+
     function parseInlineOptions(parent) {
         if (skip("[", true)) {
             do {

src/util.js

@@ -165,6 +165,37 @@ util.decorateEnum = function decorateEnum(object) {
     return enm;
 };
 
+
+/**
+ * Sets the value of a property by property path. If a value already exists, it is turned to an array
+ * @param {Object.<string,*>} dst Destination object
+ * @param {string} path dot '.' delimited path of the property to set
+ * @param {Object} value the value to set
+ * @returns {Object.<string,*>} Destination object
+ */
+util.setProperty = function setProperty(dst, path, value) {
+    function setProp(dst, path, value) {
+        var part = path.shift();
+        if (path.length > 0) {
+            dst[part] = setProp(dst[part] || {}, path, value);
+        } else {
+            var prevValue = dst[part];
+            if (prevValue)
+                value = [].concat(prevValue).concat(value);
+            dst[part] = value;
+        }
+        return dst;
+    }
+
+    if (typeof dst !== "object")
+        throw TypeError("dst must be an object");
+    if (!path)
+        throw TypeError("path must be specified");
+
+    path = path.split(".");
+    return setProp(dst, path, value);
+};
+
 /**
  * Decorator root (TypeScript).
  * @name util.decorateRoot

tests/api_object.js

@@ -24,6 +24,18 @@ tape.test("reflection objects", function(test) {
     obj.setOption("c", 3);
     test.same(obj.options, { a: 1, b: 2, c: 3 }, "should set single options");
 
+    obj.setParsedOption("opt1", {a: 1, b: 2});
+    test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}], "should set single parsed option");
+    obj.setParsedOption("opt1", {a: 3, b: 4});
+    test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}], "should allow same option twice");
+    obj.setParsedOption("opt2", 1, "x");
+    test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}, {"opt2": {x: 1}}], "should create new option using property path");
+    obj.setParsedOption("opt2", 5, "a.b");
+    test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}, {"opt2": {x: 1, a: {b :5}}}], "should merge new property path in existing option");
+    obj.setParsedOption("opt2", 6, "x");
+    test.same(obj.parsedOptions, [{"opt1": {a: 1, b: 2}}, {"opt1": {a: 3, b: 4}}, {"opt2": {x: [1,6], a: {b :5}}}], "should convert property to array when set more than once");
+
+
     test.equal(obj.toString(), "ReflectionObject Test", "should convert to a string (even if not part of a root)");
     obj.name = "";
     test.equal(obj.toString(), "ReflectionObject", "should convert to a string even with no full name");

tests/api_util.js

@@ -70,5 +70,34 @@ tape.test("util", function(test) {
          test.end();
     });
 
+    test.test(test.name + " - setProperty", function(test) {
+        var o = {};
+
+        test.throws(function() {
+            util.setProperty(5, 'prop1', 5);
+        }, TypeError, "dst must be an object");
+
+        test.throws(function () {
+            util.setProperty(o, '', 5);
+        }, TypeError, "path must be specified");
+
+        util.setProperty(o, 'prop1', 5);
+        test.same(o, {prop1: 5}, "should set single property value");
+
+        util.setProperty(o, 'prop1', 6);
+        test.same(o, {prop1: [5, 6]}, "should convert to array if same property is set");
+
+        util.setProperty(o, 'prop.subprop', { subsub: 5});
+        test.same(o, {prop1: [5, 6], prop: {subprop: {subsub: 5}}}, "should handle nested properties properly");
+
+        util.setProperty(o, 'prop.subprop.subsub', 6);
+        test.same(o, {prop1: [5, 6], prop: {subprop: {subsub: [5, 6]}}}, "should convert to array nested property");
+
+        util.setProperty(o, 'prop.subprop', { subsub2: 7});
+        test.same(o, {prop1: [5, 6], prop: {subprop: [{subsub: [5,6]}, {subsub2: 7}]}}, "should convert nested properties to array");
+
+        test.end();
+    });
+
     test.end();
 });

tests/comp_options-parse.js

@@ -0,0 +1,121 @@
+var tape = require("tape");
+var protobuf = require("..");
+
+tape.test("Options", function (test) {
+    var root = protobuf.loadSync("tests/data/options_test.proto");
+
+    test.test(test.name + " - field options (Int)", function (test) {
+        var TestFieldOptionsInt = root.lookup("TestFieldOptionsInt");
+        test.equal(TestFieldOptionsInt.fields.field1.options["(fo_rep_int)"], 2, "should take second repeated int option");
+        test.same(TestFieldOptionsInt.fields.field1.parsedOptions, [{"(fo_rep_int)": 1}, {"(fo_rep_int)": 2}], "should take all repeated int option");
+
+        test.equal(TestFieldOptionsInt.fields.field2.options["(fo_single_int)"], 3, "should correctly parse single int option");
+        test.same(TestFieldOptionsInt.fields.field2.parsedOptions, [{"(fo_single_int)": 3}], "should correctly parse single int option");
+        test.end();
+    });
+
+    test.test(test.name + " - message options (Int)", function (test) {
+        var TestMessageOptionsInt = root.lookup("TestMessageOptionsInt");
+        test.equal(TestMessageOptionsInt.options["(mo_rep_int)"], 2, "should take second repeated int message option");
+        test.equal(TestMessageOptionsInt.options["(mo_single_int)"], 3, "should correctly parse single int message option");
+        test.same(TestMessageOptionsInt.parsedOptions, [{"(mo_rep_int)": 1}, {"(mo_rep_int)": 2}, {"(mo_single_int)": 3}], "should take all int message option");
+        test.end();
+    });
+
+    test.test(test.name + " - field options (Message)", function (test) {
+        var TestFieldOptionsMsg = root.lookup("TestFieldOptionsMsg");
+        test.equal(TestFieldOptionsMsg.fields.field1.options["(fo_rep_msg).value"], 4, "should take second repeated message option");
+        test.equal(TestFieldOptionsMsg.fields.field1.options["(fo_rep_msg).rep_value"], 6, "should take second repeated int in second repeated option");
+        test.same(TestFieldOptionsMsg.fields.field1.parsedOptions, [
+            {"(fo_rep_msg)": {value: 1, rep_value: [2, 3]}},
+            {"(fo_rep_msg)": {value: 4, rep_value: [5, 6]}}], "should take all repeated message option");
+        test.equal(TestFieldOptionsMsg.fields.field2.options["(fo_single_msg).value"], 7, "should correctly parse single msg option");
+        test.equal(TestFieldOptionsMsg.fields.field2.options["(fo_single_msg).rep_value"], 9, "should take second repeated int in single msg option");
+        test.same(TestFieldOptionsMsg.fields.field2.parsedOptions, [{"(fo_single_msg)": {value: 7, rep_value: [8,9]}}], "should take all repeated message option");
+        test.end();
+    });
+
+    test.test(test.name + " - message options (Message)", function (test) {
+        var TestMessageOptionsMsg = root.lookup("TestMessageOptionsMsg");
+        test.equal(TestMessageOptionsMsg.options["(mo_rep_msg).value"], 4, "should take second repeated message option");
+        test.equal(TestMessageOptionsMsg.options["(mo_rep_msg).rep_value"], 6, "should take second repeated int in second repeated option");
+        test.equal(TestMessageOptionsMsg.options["(mo_single_msg).value"], 7, "should correctly parse single msg option");
+        test.equal(TestMessageOptionsMsg.options["(mo_single_msg).rep_value"], 9, "should take second repeated int in single msg option");
+        test.same(TestMessageOptionsMsg.parsedOptions, [
+            {"(mo_rep_msg)": {value: 1, rep_value: [2, 3]}},
+            {"(mo_rep_msg)": {value: 4, rep_value: [5, 6]}},
+            {"(mo_single_msg)": {value: 7, rep_value: [8, 9]}},
+        ], "should take all message options");
+        test.end();
+    });
+
+    test.test(test.name + " - field options (Nested)", function (test) {
+        var TestFieldOptionsNested = root.lookup("TestFieldOptionsNested");
+        test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).value"], 1, "should merge repeated options messages");
+        test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).rep_value"], 3, "should parse in any order");
+        test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).nested.nested.value"], "x", "should correctly parse nested field options");
+        test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).rep_nested.value"], "z", "should take second repeated nested options");
+        test.equal(TestFieldOptionsNested.fields.field1.options["(fo_rep_msg).nested.value"], "w", "should merge nested options");
+        test.same(TestFieldOptionsNested.fields.field1.parsedOptions,[
+            {"(fo_rep_msg)": {value: 1, nested: { nested: { value: "x"}}, rep_nested: [{value: "y"},{value: "z"}], rep_value: 3}},
+            {"(fo_rep_msg)": { nested: { value: "w"}}},
+        ],"should parse all options including nested");
+
+        test.equal(TestFieldOptionsNested.fields.field2.options["(fo_single_msg).nested.value"], "x", "should correctly parse nested property name");
+        test.equal(TestFieldOptionsNested.fields.field2.options["(fo_single_msg).rep_nested.value"], "y", "should take second repeated nested options");
+        test.same(TestFieldOptionsNested.fields.field2.parsedOptions, [{
+            "(fo_single_msg)": {
+                nested: {value: "x"},
+                rep_nested: [{value: "x"}, {value: "y"}]
+            }
+        }
+        ], "should parse single nested option correctly");
+
+        test.equal(TestFieldOptionsNested.fields.field3.options["(fo_single_msg).nested.value"], "x", "should correctly parse nested field options");
+        test.equal(TestFieldOptionsNested.fields.field3.options["(fo_single_msg).nested.nested.nested.value"], "y", "should correctly parse several nesting levels");
+        test.same(TestFieldOptionsNested.fields.field3.parsedOptions, [{
+            "(fo_single_msg)": {
+                nested: {
+                    value: "x",
+                    nested: {nested: {value: "y"}}
+                }
+            }
+        }], "should correctly parse several nesting levels");
+
+        test.end();
+    });
+
+    test.test(test.name + " - message options (Nested)", function (test) {
+        var TestMessageOptionsNested = root.lookup("TestMessageOptionsNested");
+        test.equal(TestMessageOptionsNested.options["(mo_rep_msg).value"], 1, "should merge repeated options messages");
+        test.equal(TestMessageOptionsNested.options["(mo_rep_msg).rep_value"], 3, "should parse in any order");
+        test.equal(TestMessageOptionsNested.options["(mo_rep_msg).nested.nested.value"], "x", "should correctly parse nested field options");
+        test.equal(TestMessageOptionsNested.options["(mo_rep_msg).rep_nested.value"], "z", "should take second repeated nested options");
+        test.equal(TestMessageOptionsNested.options["(mo_rep_msg).nested.value"], "w", "should merge nested options");
+
+        test.equal(TestMessageOptionsNested.options["(mo_single_msg).nested.value"], "x", "should correctly parse nested property name");
+        test.equal(TestMessageOptionsNested.options["(mo_single_msg).rep_nested.value"], "y", "should take second repeated nested options");
+        test.equal(TestMessageOptionsNested.options["(mo_single_msg).rep_nested.nested.nested.value"], "y", "should correctly parse several nesting levels");
+
+        test.same(TestMessageOptionsNested.parsedOptions, [
+            {
+                "(mo_rep_msg)": {
+                    value: 1,
+                    nested: {nested: {value: "x"}},
+                    rep_nested: [{value: "y"}, {value: "z"}],
+                    rep_value: 3
+                }
+            },
+            {"(mo_rep_msg)": {nested: {value: "w"}}},
+            {
+                "(mo_single_msg)": {
+                    nested: {value: "x"},
+                    rep_nested: [{value: "x", nested: {nested: {value: "y"}}}, {value: "y"}]
+                }
+            }
+        ], "should correctly parse all nested message options");
+        test.end();
+    });
+
+    test.end();
+});