Skip to content

Commit

Permalink
Implement support for proto3 optional
Browse files Browse the repository at this point in the history
proto3 optional requires opting into the
experimental feature in our code generator.

proto3 optional is implemented as a one-off
synthetic field so we must treat those as normal
objects in our codegenerator. You can find the
instructions on how to handle those in the upstream
protobuf documentation:
https://github.com/protocolbuffers/protobuf/blob/main/docs/implementing_proto3_presence.md

The PR adds tests for the logic for both binary
and JSON decoding. We are missing reflection
support but that seemed reasonable for now.
  • Loading branch information
jchaffraix-slack committed Apr 23, 2024
1 parent eaf0ef0 commit 9cd1824
Show file tree
Hide file tree
Showing 28 changed files with 1,305 additions and 243 deletions.
2 changes: 1 addition & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ hh_test(
hh_args = "lib_test/test.php",
)

INTEGRATION_PHP = glob(["test/**/*.php"])
INTEGRATION_PHP = glob(["test/**/*.php", "test/**/*.pb.json"])

ALL_PHP += INTEGRATION_PHP

Expand Down
5 changes: 4 additions & 1 deletion gen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ for SRC in $PBS
do
echo source: $SRC
ARGS="-I external/com_google_protobuf/src -I ./"
$PROTOC $ARGS --plugin=$GENHACK --hack_out="plugin=grpc,allow_proto2_dangerous:$TMP" $SRC
$PROTOC $ARGS --plugin=$GENHACK --hack_out="plugin=grpc,allow_proto2_dangerous:$TMP" --experimental_allow_proto3_optional $SRC
echo
done

ARGS="-I external/com_google_protobuf/src -I ./"
$PROTOC $ARGS --encode=foo.bar.example1 ./test/example1.proto < ./test/example1.pb.txt > $TMP/test/example1.pb.bin
$PROTOC $ARGS --encode=baz.optional_proto3 ./test/optional_proto3.proto < ./test/empty_optional_proto3.pb.txt > $TMP/test/empty_optional_proto3.pb.bin
$PROTOC $ARGS --encode=baz.optional_proto3 ./test/optional_proto3.proto < ./test/default_optional_proto3.pb.txt > $TMP/test/default_optional_proto3.pb.bin
$PROTOC $ARGS --encode=baz.optional_proto3 ./test/optional_proto3.proto < ./test/custom_optional_proto3.pb.txt > $TMP/test/custom_optional_proto3.pb.bin

if [ $# -gt 0 ]; then
# Comparison mode; see if there are diffs, if none, exit 0.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
$this->test_category = \conformance\TestCategory::FromInt($d->readVarint());
break;
case 6:
if ($this->jspb_encoding_options == null) $this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
if ($this->jspb_encoding_options is null) {
$this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
}
$this->jspb_encoding_options->MergeFrom($d->readDecoder());
break;
case 7:
Expand Down Expand Up @@ -397,8 +399,10 @@ public function MergeJsonFrom(mixed $m): void {
$this->test_category = \conformance\TestCategory::FromMixed($v);
break;
case 'jspb_encoding_options': case 'jspbEncodingOptions':
if ($v === null) break;
if ($this->jspb_encoding_options == null) $this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
if ($v is null) break;
if ($this->jspb_encoding_options is null) {
$this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
}
$this->jspb_encoding_options->MergeJsonFrom($v);
break;
case 'jspb_payload': case 'jspbPayload':
Expand All @@ -424,7 +428,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
$this->message_type = $o->message_type;
$this->test_category = $o->test_category;
$tmp = $o->jspb_encoding_options;
if ($tmp !== null) {
if ($tmp is nonnull) {
$nv = new \conformance\JspbEncodingConfig();
$nv->CopyFrom($tmp);
$this->jspb_encoding_options = $nv;
Expand Down
12 changes: 8 additions & 4 deletions generated/google/protobuf/api_proto.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
$this->version = $d->readString();
break;
case 5:
if ($this->source_context == null) $this->source_context = new \google\protobuf\SourceContext();
if ($this->source_context is null) {
$this->source_context = new \google\protobuf\SourceContext();
}
$this->source_context->MergeFrom($d->readDecoder());
break;
case 6:
Expand Down Expand Up @@ -158,8 +160,10 @@ public function MergeJsonFrom(mixed $m): void {
$this->version = \Protobuf\Internal\JsonDecoder::readString($v);
break;
case 'source_context': case 'sourceContext':
if ($v === null) break;
if ($this->source_context == null) $this->source_context = new \google\protobuf\SourceContext();
if ($v is null) break;
if ($this->source_context is null) {
$this->source_context = new \google\protobuf\SourceContext();
}
$this->source_context->MergeJsonFrom($v);
break;
case 'mixins':
Expand Down Expand Up @@ -195,7 +199,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
}
$this->version = $o->version;
$tmp = $o->source_context;
if ($tmp !== null) {
if ($tmp is nonnull) {
$nv = new \google\protobuf\SourceContext();
$nv->CopyFrom($tmp);
$this->source_context = $nv;
Expand Down
24 changes: 16 additions & 8 deletions generated/google/protobuf/compiler/plugin_proto.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
$this->parameter = $d->readString();
break;
case 3:
if ($this->compiler_version == null) $this->compiler_version = new \google\protobuf\compiler\Version();
if ($this->compiler_version is null) {
$this->compiler_version = new \google\protobuf\compiler\Version();
}
$this->compiler_version->MergeFrom($d->readDecoder());
break;
case 15:
Expand Down Expand Up @@ -226,8 +228,10 @@ public function MergeJsonFrom(mixed $m): void {
$this->parameter = \Protobuf\Internal\JsonDecoder::readString($v);
break;
case 'compiler_version': case 'compilerVersion':
if ($v === null) break;
if ($this->compiler_version == null) $this->compiler_version = new \google\protobuf\compiler\Version();
if ($v is null) break;
if ($this->compiler_version is null) {
$this->compiler_version = new \google\protobuf\compiler\Version();
}
$this->compiler_version->MergeJsonFrom($v);
break;
case 'proto_file': case 'protoFile':
Expand All @@ -250,7 +254,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
$this->file_to_generate = $o->file_to_generate;
$this->parameter = $o->parameter;
$tmp = $o->compiler_version;
if ($tmp !== null) {
if ($tmp is nonnull) {
$nv = new \google\protobuf\compiler\Version();
$nv->CopyFrom($tmp);
$this->compiler_version = $nv;
Expand Down Expand Up @@ -337,7 +341,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
$this->content = $d->readString();
break;
case 16:
if ($this->generated_code_info == null) $this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
if ($this->generated_code_info is null) {
$this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
}
$this->generated_code_info->MergeFrom($d->readDecoder());
break;
default:
Expand Down Expand Up @@ -391,8 +397,10 @@ public function MergeJsonFrom(mixed $m): void {
$this->content = \Protobuf\Internal\JsonDecoder::readString($v);
break;
case 'generated_code_info': case 'generatedCodeInfo':
if ($v === null) break;
if ($this->generated_code_info == null) $this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
if ($v is null) break;
if ($this->generated_code_info is null) {
$this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
}
$this->generated_code_info->MergeJsonFrom($v);
break;
default:
Expand All @@ -409,7 +417,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
$this->insertion_point = $o->insertion_point;
$this->content = $o->content;
$tmp = $o->generated_code_info;
if ($tmp !== null) {
if ($tmp is nonnull) {
$nv = new \google\protobuf\GeneratedCodeInfo();
$nv->CopyFrom($tmp);
$this->generated_code_info = $nv;
Expand Down
Loading

0 comments on commit 9cd1824

Please sign in to comment.