diff --git a/src/enum.js b/src/enum.js index 36070d615..004ef55c7 100644 --- a/src/enum.js +++ b/src/enum.js @@ -89,20 +89,14 @@ function Enum(name, values, options, comment, comments, valuesOptions) { * @returns {Enum} `this` */ Enum.prototype.resolve = function resolve() { - - if (this.resolved) - return this; + ReflectionObject.prototype.resolve.call(this); for (var key of Object.keys(this._valuesProtoFeatures)) { - - if (this.parent) { - var parentFeaturesCopy = Object.assign({}, this.parent._features); - this._valuesFeatures[key] = Object.assign(parentFeaturesCopy, this._valuesProtoFeatures[key] || {}); - } else { - this._valuesFeatures[key] = Object.assign({}, this._valuesProtoFeatures[key]); - } + var parentFeaturesCopy = Object.assign({}, this._features); + this._valuesFeatures[key] = Object.assign(parentFeaturesCopy, this._valuesProtoFeatures[key] || {}); } - return ReflectionObject.prototype.resolve.call(this); + + return this; }; diff --git a/src/field.js b/src/field.js index e0feb8b43..d164cefa6 100644 --- a/src/field.js +++ b/src/field.js @@ -319,6 +319,13 @@ Field.prototype.resolve = function resolve() { if (this.parent instanceof Type) this.parent.ctor.prototype[this.name] = this.defaultValue; + + if (this.parent) { + var parentFeaturesCopy = Object.assign({}, this.parent._features); + this._features = Object.assign(parentFeaturesCopy, this._protoFeatures || {}); + } else { + this._features = Object.assign({}, this._protoFeatures); + } return ReflectionObject.prototype.resolve.call(this); }; diff --git a/src/object.js b/src/object.js index fb10e4815..a61af0deb 100644 --- a/src/object.js +++ b/src/object.js @@ -3,6 +3,7 @@ module.exports = ReflectionObject; ReflectionObject.className = "ReflectionObject"; +const OneOf = require("./oneof"); var util = require("./util"); var Root; // cyclic @@ -162,10 +163,10 @@ ReflectionObject.prototype.onRemove = function onRemove(parent) { ReflectionObject.prototype.resolve = function resolve() { if (this.resolved) return this; - if (this instanceof Root || this.parent && this.parent.resolved) + if (this instanceof Root || this.parent && this.parent.resolved) { this._resolveFeatures(); - if (this.root instanceof Root) this.resolved = true; + } return this; }; @@ -188,6 +189,11 @@ ReflectionObject.prototype._resolveFeatures = function _resolveFeatures() { if (this instanceof Root) { this._features = Object.assign(defaults, this._protoFeatures || {}); + // fields in Oneofs aren't actually children of them, so we have to + // special-case it + } else if (this.partOf instanceof OneOf) { + var lexicalParentFeaturesCopy = Object.assign({}, this.partOf._features); + this._features = Object.assign(lexicalParentFeaturesCopy, this._protoFeatures || {}); } else if (this.parent) { var parentFeaturesCopy = Object.assign({}, this.parent._features); this._features = Object.assign(parentFeaturesCopy, this._protoFeatures || {}); diff --git a/src/root.js b/src/root.js index 1a00cf3fb..9198e748f 100644 --- a/src/root.js +++ b/src/root.js @@ -88,20 +88,26 @@ Root.prototype.load = function load(filename, options, callback) { options = undefined; } var self = this; - if (!callback) + if (!callback) { return util.asPromise(load, self, filename, options); + } var sync = callback === SYNC; // undocumented // Finishes loading by calling the callback (exactly once) function finish(err, root) { /* istanbul ignore if */ - if (!callback) + if (!callback) { return; - if (sync) + } + if (sync) { throw err; + } var cb = callback; callback = null; + if (root) { + root.resolveAll(); + } cb(err, root); } @@ -139,8 +145,9 @@ Root.prototype.load = function load(filename, options, callback) { } catch (err) { finish(err); } - if (!sync && !queued) + if (!sync && !queued) { finish(null, self); // only once anyway + } } // Fetches a single file @@ -148,15 +155,16 @@ Root.prototype.load = function load(filename, options, callback) { filename = getBundledFileName(filename) || filename; // Skip if already loaded / attempted - if (self.files.indexOf(filename) > -1) + if (self.files.indexOf(filename) > -1) { return; + } self.files.push(filename); // Shortcut bundled definitions if (filename in common) { - if (sync) + if (sync) { process(filename, common[filename]); - else { + } else { ++queued; setTimeout(function() { --queued; @@ -182,8 +190,9 @@ Root.prototype.load = function load(filename, options, callback) { self.fetch(filename, function(err, source) { --queued; /* istanbul ignore if */ - if (!callback) + if (!callback) { return; // terminated meanwhile + } if (err) { /* istanbul ignore else */ if (!weak) @@ -200,17 +209,21 @@ Root.prototype.load = function load(filename, options, callback) { // Assembling the root namespace doesn't require working type // references anymore, so we can load everything in parallel - if (util.isString(filename)) + if (util.isString(filename)) { filename = [ filename ]; + } for (var i = 0, resolved; i < filename.length; ++i) if (resolved = self.resolvePath("", filename[i])) fetch(resolved); self.resolveAll(); - if (sync) + if (sync) { return self; - if (!queued) + } + if (!queued) { finish(null, self); - return undefined; + } + + return self; }; // function load(filename:string, options:IParseOptions, callback:LoadCallback):undefined diff --git a/src/type.js b/src/type.js index 30f2b31a2..600fad963 100644 --- a/src/type.js +++ b/src/type.js @@ -300,12 +300,12 @@ Type.prototype.toJSON = function toJSON(toJSONOptions) { */ Type.prototype.resolveAll = function resolveAll() { Namespace.prototype.resolveAll.call(this); - var fields = this.fieldsArray, i = 0; - while (i < fields.length) - fields[i++].resolve(); var oneofs = this.oneofsArray; i = 0; while (i < oneofs.length) oneofs[i++].resolve(); + var fields = this.fieldsArray, i = 0; + while (i < fields.length) + fields[i++].resolve(); return this; }; diff --git a/tests/data/feature-resolution.proto b/tests/data/feature-resolution.proto new file mode 100644 index 000000000..a8fd26766 --- /dev/null +++ b/tests/data/feature-resolution.proto @@ -0,0 +1,62 @@ +edition = "2023"; + +option features.amazing_feature = A; +option (mo_single_msg).nested.value = "x"; +service MyService { + option features.amazing_feature = E; + message MyRequest {}; + message MyResponse {}; + rpc MyMethod (MyRequest) returns (MyResponse) { + option features.amazing_feature = L; + }; +} + +message Message { + option features.amazing_feature = B; + + string string_val = 1; + string string_repeated = 2 [features.amazing_feature = F]; + + uint64 uint64_val = 3; + uint64 uint64_repeated = 4; + + bytes bytes_val = 5; + bytes bytes_repeated = 6; + + SomeEnum enum_val = 7; + SomeEnum enum_repeated = 8; + + extensions 10 to 100; + extend Message { + int32 bar = 10 [features.amazing_feature = I]; + } + + message Nested { + option features.amazing_feature = H; + int64 count = 9; + } + + enum SomeEnumInMessage { + option features.amazing_feature = G; + ONE = 11; + TWO = 12; + } + + oneof SomeOneOf { + option features.amazing_feature = J; + int32 a = 13; + string b = 14; + } + + map int64_map = 15; +} + +extend Message { + int32 bar = 16 [features.amazing_feature = D]; +} + +enum SomeEnum { + option features.amazing_feature = C; + ONE = 1 [features.amazing_feature = K]; + TWO = 2; +} \ No newline at end of file diff --git a/tests/feature_resolution_editions.js b/tests/feature_resolution_editions.js index 02b0a04e8..a197e817e 100644 --- a/tests/feature_resolution_editions.js +++ b/tests/feature_resolution_editions.js @@ -169,13 +169,14 @@ tape.test("feature resolution inheritance message to nested message", function(t test.end(); }); -tape.test("feature resolution inheritance file to enums and enum values", function(test) { +tape.test("feature resolution inheritance enum to enum value", function(test) { var rootEditionsOverriden = protobuf.parse(`edition = "2023"; option features.json_format = LEGACY_BEST_EFFORT; option features.(abc).d_e = deeply_nested_false; message Message { enum SomeEnum { + option features.field_presence = IMPLICIT; ONE = 1 [features.repeated_field_encoding = EXPANDED]; TWO = 2; } @@ -183,7 +184,7 @@ tape.test("feature resolution inheritance file to enums and enum values", functi test.same(rootEditionsOverriden.lookupEnum("SomeEnum")._valuesFeatures["ONE"], { enum_type: 'OPEN', - field_presence: 'EXPLICIT', + field_presence: 'IMPLICIT', json_format: 'LEGACY_BEST_EFFORT', message_encoding: 'LENGTH_PREFIXED', repeated_field_encoding: 'EXPANDED', @@ -193,7 +194,7 @@ tape.test("feature resolution inheritance file to enums and enum values", functi test.same(rootEditionsOverriden.lookupEnum("SomeEnum")._valuesFeatures["TWO"], { enum_type: 'OPEN', - field_presence: 'EXPLICIT', + field_presence: 'IMPLICIT', json_format: 'LEGACY_BEST_EFFORT', message_encoding: 'LENGTH_PREFIXED', repeated_field_encoding: 'PACKED', @@ -230,14 +231,41 @@ tape.test("feature resolution inheritance message to oneofs", function(test) { test.end(); }); + +tape.test("feature resolution inheritance oneofs", function(test) { + + var rootEditionsOverriden = protobuf.parse(` + edition = "2023"; + option features.(abc).d_e = deeply_nested_false; + message Message { + + oneof SomeOneOf { + option features.json_format = LEGACY_BEST_EFFORT; + int32 a = 13; + string b = 14; + } + }`).root.resolveAll(); + + test.same(rootEditionsOverriden.lookup("SomeOneOf")._features, { + enum_type: 'OPEN', + field_presence: 'EXPLICIT', + json_format: 'LEGACY_BEST_EFFORT', + message_encoding: 'LENGTH_PREFIXED', + repeated_field_encoding: 'PACKED', + utf8_validation: 'VERIFY', + '(abc)': { d_e: 'deeply_nested_false' } + }) + + test.end(); +}); + tape.test("feature resolution inheritance oneofs to field", function(test) { var rootEditionsOverriden = protobuf.parse(` edition = "2023"; option features.(abc).d_e = deeply_nested_false; message Message { - option features.json_format = LEGACY_BEST_EFFORT; oneof SomeOneOf { - option features.json_format = ALLOW; + option features.json_format = LEGACY_BEST_EFFORT; int32 a = 13; string b = 14; } @@ -291,17 +319,17 @@ tape.test("feature resolution inheritance message to extensions", function(test) message Message { option features.utf8_validation = NONE; extend Message { - int32 bar = 10 [features.utf8_validation = VERIFY]; + int32 bar = 10 [features.field_presence = IMPLICIT]; } }`).root.resolveAll(); test.same(rootEditionsOverriden.lookup(".bar")._features, { enum_type: 'OPEN', - field_presence: 'EXPLICIT', + field_presence: 'IMPLICIT', json_format: 'LEGACY_BEST_EFFORT', message_encoding: 'LENGTH_PREFIXED', repeated_field_encoding: 'PACKED', - utf8_validation: 'VERIFY', + utf8_validation: 'NONE', '(abc)': { d_e: 'deeply_nested_false' } }) @@ -316,11 +344,21 @@ tape.test("feature resolution inheritance message to enum", function(test) { message Message { option features.utf8_validation = NONE; enum SomeEnum { - ONE = 1; + ONE = 1 [features.field_presence = IMPLICIT]; TWO = 2; } }`).root.resolveAll(); + test.same(rootEditionsOverriden.lookup("Message").lookup("SomeEnum")._valuesFeatures["ONE"], { + enum_type: 'OPEN', + field_presence: 'IMPLICIT', + json_format: 'LEGACY_BEST_EFFORT', + message_encoding: 'LENGTH_PREFIXED', + repeated_field_encoding: 'PACKED', + utf8_validation: 'NONE', + '(abc)': { d_e: 'deeply_nested_false' } + }) + test.same(rootEditionsOverriden.lookup("Message").lookup("SomeEnum")._features, { enum_type: 'OPEN', field_presence: 'EXPLICIT', @@ -363,7 +401,7 @@ tape.test("feature resolution inheritance file to service and service to method" option features.json_format = LEGACY_BEST_EFFORT; option features.(abc).d_e = deeply_nested_false; service MyService { - option features.json_format = ALLOW; + option features.field_presence = IMPLICIT; message MyRequest {}; message MyResponse {}; rpc MyMethod (MyRequest) returns (MyResponse) { @@ -373,8 +411,8 @@ tape.test("feature resolution inheritance file to service and service to method" test.same(rootEditionsOverriden.lookup("MyService")._features, { enum_type: 'OPEN', - field_presence: 'EXPLICIT', - json_format: 'ALLOW', + field_presence: 'IMPLICIT', + json_format: 'LEGACY_BEST_EFFORT', message_encoding: 'LENGTH_PREFIXED', repeated_field_encoding: 'PACKED', utf8_validation: 'VERIFY', @@ -383,8 +421,8 @@ tape.test("feature resolution inheritance file to service and service to method" test.same(rootEditionsOverriden.lookup("MyService").lookup("MyMethod")._features, { enum_type: 'OPEN', - field_presence: 'EXPLICIT', - json_format: 'ALLOW', + field_presence: 'IMPLICIT', + json_format: 'LEGACY_BEST_EFFORT', message_encoding: 'LENGTH_PREFIXED', repeated_field_encoding: 'PACKED', utf8_validation: 'NONE', @@ -394,131 +432,21 @@ tape.test("feature resolution inheritance file to service and service to method" test.end(); }); -// Tests precedence for different levels of feature resolution tape.test("feature resolution editions precedence", function(test) { - var root1 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A;`).root.resolveAll() - - test.same(root1._features.amazing_feature, 'A'); - - var root2 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - - message Message { - option features.amazing_feature = B; - }`).root.resolveAll(); - - test.same(root2.lookup("Message")._features.amazing_feature, 'B') - - var root3 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - enum SomeEnum { - option features.amazing_feature = C; - ONE = 1; - TWO = 2; - }`).root.resolveAll(); - - test.same(root3.lookupEnum("SomeEnum")._features.amazing_feature, 'C') - - var root4 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - - message Message { - option features.amazing_feature = B; - } - - extend Message { - int32 bar = 16 [features.amazing_feature = D]; - } - `).root.resolveAll(); - - test.same(root4.lookup("Message").fields[".bar"].declaringField._features.amazing_feature, 'D') - - var root5 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - service MyService { - option features.amazing_feature = E; - message MyRequest {}; - message MyResponse {}; - } - `).root.resolveAll(); - - test.same(root5.lookupService("MyService")._features.amazing_feature, 'E'); - - var root6 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - message Message { - string string_val = 1; - string string_repeated = 2 [features.amazing_feature = F]; - }`).root.resolveAll(); - - test.same(root6.lookup("Message").fields.stringRepeated._features.amazing_feature, 'F') - - var root7 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - message Message { - enum SomeEnumInMessage { - option features.amazing_feature = G; - ONE = 11; - TWO = 12; - } - }`).root.resolveAll(); - - test.same(root7.lookup("Message").lookupEnum("SomeEnumInMessage")._features.amazing_feature, 'G') - - var root8 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - message Message { - message Nested { - option features.amazing_feature = H; - int64 count = 9; - } - }`).root.resolveAll(); - - test.same(root8.lookup("Message").lookup("Nested")._features.amazing_feature, 'H') - - var root9 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - message Message { - extend Message { - int32 bar = 10 [features.amazing_feature = I]; - } - }`).root.resolveAll(); - - test.same(root9.lookup("Message").lookup(".Message.bar")._features.amazing_feature, 'I') - - var root10 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - message Message { - oneof SomeOneOf { - option features.amazing_feature = J; - int32 a = 13; - string b = 14; - } - }`).root.resolveAll(); - test.same(root10.lookup("Message").lookup("SomeOneOf")._features.amazing_feature, 'J') - - var root11 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - enum SomeEnum { - option features.amazing_feature = C; - ONE = 1 [features.amazing_feature = K]; - TWO = 2; - }`).root.resolveAll(); - test.same(root11.lookupEnum("SomeEnum")._valuesFeatures["ONE"].amazing_feature, 'K') - - var root12 = protobuf.parse(`edition = "2023"; - option features.amazing_feature = A; - service MyService { - option features.amazing_feature = E; - message MyRequest {}; - message MyResponse {}; - rpc MyMethod (MyRequest) returns (MyResponse) { - option features.amazing_feature = L; - }; - }`).root.resolveAll(); - - test.same(root12.lookupService("MyService").lookup("MyMethod")._features.amazing_feature, 'L') + protobuf.load("tests/data/feature-resolution.proto", function(err, root) { + if (err) + throw test.fail(err.message); + test.same(root.lookup("Message").lookupEnum("SomeEnumInMessage")._features, + { + enum_type: 'CLOSED', + field_presence: 'EXPLICIT', + json_format: 'LEGACY_BEST_EFFORT', + message_encoding: 'LENGTH_PREFIXED', + repeated_field_encoding: 'EXPANDED', + utf8_validation: 'NONE', + amazing_feature: 'G' + }) test.end(); + }); })