From 7a2539843055b6daecb9f369c67a6cf588dbb54c Mon Sep 17 00:00:00 2001 From: Eyal Post Date: Sat, 11 Jul 2020 01:43:30 +0300 Subject: [PATCH] feat: parsed options (#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 Co-authored-by: Alexander Fenster --- index.d.ts | 21 ++++++ src/object.js | 43 ++++++++++++ src/parse.js | 38 +++++++++-- src/util.js | 31 +++++++++ tests/api_object.js | 12 ++++ tests/api_util.js | 29 ++++++++ tests/comp_options-parse.js | 121 ++++++++++++++++++++++++++++++++++ tests/data/options_test.proto | 108 ++++++++++++++++++++++++++++++ 8 files changed, 396 insertions(+), 7 deletions(-) create mode 100644 tests/comp_options-parse.js create mode 100644 tests/data/options_test.proto diff --git a/index.d.ts b/index.d.ts index 40d9d2852..867cfdcaf 100644 --- a/index.d.ts +++ b/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; diff --git a/src/object.js b/src/object.js index b6a5e5635..bd04ceca8 100644 --- a/src/object.js +++ b/src/object.js @@ -29,6 +29,12 @@ function ReflectionObject(name, options) { */ this.options = options; // toJSON + /** + * Parsed Options. + * @type {Array.>|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.} options Options to set diff --git a/src/parse.js b/src/parse.js index f1ab07b93..918d7c2eb 100644 --- a/src/parse.js +++ b/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 { diff --git a/src/util.js b/src/util.js index a5a9a8354..5ae88cc7f 100644 --- a/src/util.js +++ b/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.} dst Destination object + * @param {string} path dot '.' delimited path of the property to set + * @param {Object} value the value to set + * @returns {Object.} 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 diff --git a/tests/api_object.js b/tests/api_object.js index b6b5f6982..9dda73351 100644 --- a/tests/api_object.js +++ b/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"); diff --git a/tests/api_util.js b/tests/api_util.js index ae2a3e15a..7b6f50ffe 100644 --- a/tests/api_util.js +++ b/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(); }); \ No newline at end of file diff --git a/tests/comp_options-parse.js b/tests/comp_options-parse.js new file mode 100644 index 000000000..90c4299c0 --- /dev/null +++ b/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(); +}); diff --git a/tests/data/options_test.proto b/tests/data/options_test.proto new file mode 100644 index 000000000..1bdc0e470 --- /dev/null +++ b/tests/data/options_test.proto @@ -0,0 +1,108 @@ +syntax = "proto3"; + +import "google/protobuf/descriptor.proto"; + +package test; + +//Simple int32 based options (both single and repeated) +//for fields and messages + +extend google.protobuf.FieldOptions { + repeated int32 fo_rep_int = 50000; + int32 fo_single_int = 50001; +} + +extend google.protobuf.MessageOptions { + repeated int32 mo_rep_int = 50000; + int32 mo_single_int = 50001; +} + +message TestFieldOptionsInt { + string field1 = 2 [(fo_rep_int) = 1, (fo_rep_int) = 2]; + string field2 = 1 [(fo_single_int) = 3]; +} + +message TestMessageOptionsInt { + option (mo_rep_int) = 1; + option (mo_rep_int) = 2; + option (mo_single_int) = 3; +} + +//Message based options including nested sub messages (both single and repeated) +//for fields and messages + +message Msg { + int32 value = 1; + repeated int32 rep_value = 2; + SubMsg nested = 3; + repeated SubMsg rep_nested = 4; +} + +message SubMsg { + string value = 1; + SubMsg nested = 2; +} + +extend google.protobuf.FieldOptions { + repeated Msg fo_rep_msg = 50002; + Msg fo_single_msg = 50003; +} + +extend google.protobuf.MessageOptions { + repeated Msg mo_rep_msg = 50002; + Msg mo_single_msg = 50003; +} + +message TestFieldOptionsMsg { + string field1 = 1 [(fo_rep_msg) = {value: 1 rep_value: 2 rep_value: 3}, (fo_rep_msg) = {value: 4 rep_value: 5 rep_value: 6}]; + string field2 = 2 [(fo_single_msg).value = 7, (fo_single_msg).rep_value = 8, (fo_single_msg).rep_value = 9]; +} + +message TestMessageOptionsMsg { + option (mo_rep_msg) = { + value: 1 + rep_value: 2 + rep_value: 3 + }; + option (mo_rep_msg) = { + value: 4 + rep_value: 5 + rep_value: 6 + }; + option (mo_single_msg).value = 7; + option (mo_single_msg).rep_value = 8; + option (mo_single_msg).rep_value = 9; +} + +message TestFieldOptionsNested { + string field1 = 1 [(fo_rep_msg) = {value: 1 nested { nested {value: "x"} } rep_nested { value: "y"} rep_nested { value: "z" } rep_value: 3}, (fo_rep_msg) = { nested { value: "w" }}]; + string field2 = 2 [(fo_single_msg).nested.value = "x", (fo_single_msg).rep_nested = {value : "x"}, (fo_single_msg).rep_nested = {value : "y"}]; + string field3 = 3 [(fo_single_msg).nested = {value: "x" nested {nested{value: "y"}}}]; +} + + +message TestMessageOptionsNested { + option (mo_rep_msg) = { + value: 1 + nested { + nested { + value: "x" + } + } + rep_nested { + value: "y" + } + rep_nested { + value: "z" + } + rep_value: 3 + }; + option (mo_rep_msg) = { + nested { + value: "w" + } + }; + option (mo_single_msg).nested.value = "x"; + option (mo_single_msg).rep_nested = {value : "x" nested {nested{value: "y"}}}; + option (mo_single_msg).rep_nested = {value : "y"}; +}