Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add feature resolution for protobuf editions #2029

Merged
merged 45 commits into from
Oct 16, 2024
Merged
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
b4b5ca4
feat: api_converters_editions tests added and run successfully"
sofisl Aug 23, 2024
d8eb1b4
remove logging
sofisl Aug 26, 2024
65d3ed1
feat: add feature resolution for protobuf editions
sofisl Sep 10, 2024
0a7bbd1
cleanup PR
sofisl Sep 10, 2024
68e4480
see diff
sofisl Sep 10, 2024
759cc45
fix broken tests
sofisl Sep 10, 2024
8dabd5e
add tests
sofisl Sep 10, 2024
68b5339
feat: add feature resolution and tests
sofisl Sep 14, 2024
dae1b3e
undo lockfile update
sofisl Sep 14, 2024
f204288
add comment
sofisl Sep 14, 2024
f0491be
cleanup
sofisl Sep 14, 2024
d9c2ec4
remove only
sofisl Sep 16, 2024
b802c42
save for now
sofisl Sep 17, 2024
73590cc
undo bundle changes
sofisl Sep 17, 2024
a70c6a4
revert package-lock changes
sofisl Sep 17, 2024
a9ffc8a
feat: add feature resolution
sofisl Sep 19, 2024
43a703a
revert package-lock updates/
sofisl Sep 19, 2024
2fabe04
cleanup
sofisl Sep 19, 2024
1bd72ff
chore: run lint
sofisl Sep 19, 2024
5d35287
Merge branch 'master' into protobufEditionsWork
sofisl Sep 19, 2024
225fddb
typo
sofisl Sep 19, 2024
840907e
Merge branch 'protobufEditionsWork' of github.com:protobufjs/protobuf…
sofisl Sep 19, 2024
43183bd
typo
sofisl Sep 19, 2024
c4c486d
Update api_enum.js
sofisl Sep 20, 2024
1ca7586
respond to comments
sofisl Sep 24, 2024
e73bb3c
chore: add grammar tests
sofisl Sep 24, 2024
e1abb31
run lint
sofisl Sep 24, 2024
10dae84
address comments
sofisl Sep 24, 2024
087a33e
remove comment
sofisl Sep 24, 2024
608009b
respond to comments
sofisl Sep 26, 2024
062caa5
fix test
sofisl Sep 26, 2024
c7cee06
add default feature resolution at the end
sofisl Oct 2, 2024
da2df8a
run lint
sofisl Oct 2, 2024
c11ddf4
Update object.js
sofisl Oct 2, 2024
d2d47d9
fix: change tree traversal order and feature resolution algorithm
sofisl Oct 4, 2024
2d81627
resolve conflicts
sofisl Oct 4, 2024
b57bde7
run lint
sofisl Oct 4, 2024
da2663a
Update feature_resolution_editions.js
sofisl Oct 4, 2024
caad500
tests: added and broke up tests
sofisl Oct 8, 2024
1f1d33c
chore: remove specific overwriting
sofisl Oct 8, 2024
4ca95c3
chore: respond to test comments
sofisl Oct 9, 2024
a9eebac
simplify tests
sofisl Oct 9, 2024
6f7d540
chore: respond to comments
sofisl Oct 14, 2024
530d59a
chore: respond to comments
sofisl Oct 15, 2024
23c8ab9
respond to comments
sofisl Oct 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: respond to comments
sofisl committed Oct 14, 2024
commit 6f7d5400cc9296e04ab46f463990b251e678f670
16 changes: 5 additions & 11 deletions src/enum.js
Original file line number Diff line number Diff line change
@@ -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;
};


7 changes: 7 additions & 0 deletions src/field.js
Original file line number Diff line number Diff line change
@@ -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);
};

10 changes: 8 additions & 2 deletions src/object.js
Original file line number Diff line number Diff line change
@@ -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 || {});
37 changes: 25 additions & 12 deletions src/root.js
Original file line number Diff line number Diff line change
@@ -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,24 +145,26 @@ 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
function fetch(filename, weak) {
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

6 changes: 3 additions & 3 deletions src/type.js
Original file line number Diff line number Diff line change
@@ -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;
};

62 changes: 62 additions & 0 deletions tests/data/feature-resolution.proto
Original file line number Diff line number Diff line change
@@ -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<string,int64> 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;
}
Loading