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.

Sign up for GitHub

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
+926 −11,802
Merged

feat: add feature resolution for protobuf editions #2029

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
Loading
Loading status checks…
chore: respond to test comments
@sofisl
sofisl committed Oct 9, 2024
commit 4ca95c31bb56a28f273bf51ab02100707b5184b8
124 changes: 111 additions & 13 deletions tests/feature_resolution_editions.js
View file
Original file line number Diff line number Diff line change
@@ -117,12 +117,36 @@ tape.test("feature resolution inheritance file to message", function(test) {
test.end();
});

tape.test("feature resolution inheritance file to nested message", function(test) {
tape.test("feature resolution inheritance message to field", 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 {
option features.json_format = LEGACY_BEST_EFFORT;
option features.(abc).d_e = deeply_nested_false;
string string_val = 1;
string string_repeated = 2 [features.enum_type = CLOSED];
}`).root.resolveAll();

// Should flip enum_type from default setting, inherit from Message,
// and keep everything else
test.same(rootEditionsOverriden.lookup("Message").fields.stringRepeated._features, {
enum_type: 'CLOSED',
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 message to nested message", function(test) {
var rootEditionsOverriden = protobuf.parse(`edition = "2023";
message Message {
option features.json_format = LEGACY_BEST_EFFORT;
option features.(abc).d_e = deeply_nested_false;
string string_val = 1;
string string_repeated = 2 [features.enum_type = CLOSED];
@@ -180,21 +204,20 @@ tape.test("feature resolution inheritance file to enums and enum values", functi
test.end();
});

tape.test("feature resolution inheritance file to oneofs", function(test) {
tape.test("feature resolution inheritance message to oneofs", 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 {
option features.json_format = LEGACY_BEST_EFFORT;
oneof SomeOneOf {
option features.json_format = ALLOW;
int32 a = 13;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can put features in oneofs (same syntax as for message), do those work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the pattern of your other tests, I'd expect these two tests to be collapsed into one that tests both inheritance and overriding of message -> oneof

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't that tested in the test below? (feature resolution inheritance oneofs)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindof, it's not clear why this one is split into two when all of the other ones test both things in a single test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I merged it into one test!

string b = 14;
}
}`).root.resolveAll();

// console.log(rootEditionsOverriden.lookup("SomeOneOf")._features)
test.same(rootEditionsOverriden.lookup("SomeOneOf")._features, {
enum_type: 'OPEN',
field_presence: 'EXPLICIT',
@@ -208,20 +231,96 @@ tape.test("feature resolution inheritance file to oneofs", function(test) {
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;
int32 a = 13;
string b = 14;
}
}`).root.resolveAll();

test.same(rootEditionsOverriden.lookup("SomeOneOf").fieldsArray.find(x => x.name === 'b')._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 file to extensions", function(test) {

var rootEditionsOverriden = protobuf.parse(`
edition = "2023";
option features.json_format = LEGACY_BEST_EFFORT;
option features.(abc).d_e = deeply_nested_false;
extend Message {
int32 bar = 10 [features.utf8_validation = NONE];
}
message Message {}`).root.resolveAll();

test.same(rootEditionsOverriden.lookup(".bar")._features, {
enum_type: 'OPEN',
field_presence: 'EXPLICIT',
json_format: 'LEGACY_BEST_EFFORT',
message_encoding: 'LENGTH_PREFIXED',
repeated_field_encoding: 'PACKED',
utf8_validation: 'NONE',
'(abc)': { d_e: 'deeply_nested_false' }
})

test.end();
});

tape.test("feature resolution inheritance message to extensions", function(test) {

var rootEditionsOverriden = protobuf.parse(`
edition = "2023";
option features.json_format = LEGACY_BEST_EFFORT;
option features.(abc).d_e = deeply_nested_false;
extend Message {
int32 bar = 10 [features.utf8_validation = NONE];
}
message Message {}`).root.resolveAll();

test.same(rootEditionsOverriden.lookup(".bar")._features, {
enum_type: 'OPEN',
field_presence: 'EXPLICIT',
json_format: 'LEGACY_BEST_EFFORT',
message_encoding: 'LENGTH_PREFIXED',
repeated_field_encoding: 'PACKED',
utf8_validation: 'NONE',
'(abc)': { d_e: 'deeply_nested_false' }
})

test.end();
});

tape.test("feature resolution inheritance message to enum", 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 {
extensions 10 to 100;
extend Message {
int32 bar = 10 [features.utf8_validation = NONE];
option features.utf8_validation = NONE;
enum SomeEnum {
ONE = 1;
TWO = 2;
}
}`).root.resolveAll();

test.same(rootEditionsOverriden.lookup("Message").lookup(".Message.bar")._features, {
test.same(rootEditionsOverriden.lookup("Message").lookup("SomeEnum")._features, {
enum_type: 'OPEN',
field_presence: 'EXPLICIT',
json_format: 'LEGACY_BEST_EFFORT',
@@ -234,7 +333,7 @@ tape.test("feature resolution inheritance file to extensions", function(test) {
test.end();
});

tape.test("feature resolution inheritance file to top-level enum", function(test) {
tape.test("feature resolution inheritance message to enum", function(test) {

var rootEditionsOverriden = protobuf.parse(`edition = "2023";
option features.json_format = LEGACY_BEST_EFFORT;
@@ -245,7 +344,6 @@ tape.test("feature resolution inheritance file to top-level enum", function(test
TWO = 2;
}`).root.resolveAll();

console.log(rootEditionsOverriden.lookup("SomeEnum")._features)
test.same(rootEditionsOverriden.lookup("SomeEnum")._features, {
enum_type: 'OPEN',
field_presence: 'EXPLICIT',
@@ -259,7 +357,7 @@ tape.test("feature resolution inheritance file to top-level enum", function(test
test.end();
});

tape.test("feature resolution inheritance file to service and method", function(test) {
tape.test("feature resolution inheritance file to service and service to method", function(test) {
var rootEditionsOverriden = protobuf.parse(`edition = "2023";
option features.json_format = LEGACY_BEST_EFFORT;
option features.(abc).d_e = deeply_nested_false;