protobuf-php / protobuf

PHP implementation of Google's Protocol Buffers
MIT License
236 stars 29 forks source link

Error In PHP when using extensions #6

Closed luisgon1 closed 8 years ago

luisgon1 commented 8 years ago

Proto code bellow:

message BookVersion {
    required uint32 major = 1;
    required uint32 minor = 2;
    required uint32 patch = 3;
}
extend google.protobuf.MessageOptions {
    optional BookVersion version = 50001;
}
message BookData {
    required fixed64 timestamp   = 1;
    required BookInfo version    = 2;
}
message BookAlive {
    option (version).major = 1;
    option (version).minor = 0;
    option (version).patch = 1;

    required BookData data = 1;
}

I am getting php error calling version method not found in class version on BookVersion class

FabioBatSilva commented 8 years ago

Options currently not supported,

I'll be adding support for it. I'll ping you once i get it merged..

FabioBatSilva commented 8 years ago

@luisgon1 This https://github.com/protobuf-php/protobuf-plugin/commit/2000dd408ee7dc60c3e3983f928f37062fd11d1f should fix the problem with option (only messages options are supported for now).

<?php

/** @var $version \YourNamespace\BookVersion(); */
$version = \YourNamespace\BookAlive::descriptor()
    ->getOptions()
    ->extensions()
    ->get(\YourNamespace\Extension::version());

Please let me know if it works for you.

luisgon1 commented 8 years ago

It is not working, bellow is the $options set in the class built:

$options = \google\protobuf\MessageOptions::fromArray([ ]);

    $options->extensions()->put(\Extension::version(), \BookVersion::__set_state(array(
       'unknownFieldSet' => NULL,
       'extensions' => NULL,
       'major' => NULL,
       'minor' => NULL,
       'patch' => 9,
    )));

It is not defining major or minor

in another code I created it is not even defining the extensions

luisgon1 commented 8 years ago
import "google/protobuf/descriptor.proto";

message BookVersion {
    required uint32 major = 1;
    required uint32 minor = 2;
    required uint32 patch = 3;
}
extend google.protobuf.MessageOptions {
    optional BookVersion version = 50001;
}
message BookData {
    required fixed64 timestamp   = 1;
    required BookVersion version    = 2;
}
message BookAlive {
    option (version).major = 9;
    option (version).minor = 5;
    option (version).patch = 9;

    required BookData data = 1;
}
FabioBatSilva commented 8 years ago

After https://github.com/protobuf-php/protobuf-plugin/commit/b9c91c2baa5b17076c954943f437a82eaa16c9fc it should generate the following :

$options->extensions()->add(\Extension::version(), \BookVersion::__set_state(array(
   'unknownFieldSet' => NULL,
   'extensions' => NULL,
   'major' => 9,
   'minor' => 5,
   'patch' => 9,
)));

please make sure you have the latest version of :

luisgon1 commented 8 years ago

So if I use 0 as an option value it gives NULL. image

image

If I give a number it works. image

luisgon1 commented 8 years ago

I have updated everything from the latest versions

luisgon1 commented 8 years ago

Also it only works if you compile everything from 1 proto file. If I use 2 proto files it does not work.

bookInfo.proto

import "google/protobuf/descriptor.proto";

message BookVersion {
    required uint32 major = 1;
    required uint32 minor = 2;
    required uint32 patch = 3;
}

extend google.protobuf.MessageOptions {
    optional BookVersion version = 50001;
}

message BookData {
    required fixed64 timestamp   = 1;
    required BookVersion version    = 2;
}

books.proto

import "google/protobuf/descriptor.proto";
import "bookInfo.proto";
message BookAlive {
    option (version).major = 4;
    option (version).minor = 4;
    option (version).patch = 5;

    required BookData data = 1;
}
FabioBatSilva commented 8 years ago

@luisgon1 The 0 value issue should be fixed by https://github.com/protobuf-php/protobuf-plugin/commit/5763220478ec2967edf934c054ead9921c614e58,

Please make sure you generate both files otherwise the plugin considers bookInfo.proto a import and wont generate the BookVersion extension.

Something like :

./vendor/bin/protobuf \
    -o ./path-to-php-src\
    -i ./path-to-protos \
    ./path-to-protos/bookInfo.proto \
    ./path-to-protos/books.proto
FabioBatSilva commented 8 years ago

Closing it for now

Please let me know if the problem continues.

luisgon1 commented 8 years ago

Its working perfect, thanks for the support On Jun 18, 2016 10:49 AM, "Fabio B. Silva" notifications@github.com wrote:

Closing it for now

Please let me know if the problem continues.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/protobuf-php/protobuf/issues/6#issuecomment-226949345, or mute the thread https://github.com/notifications/unsubscribe/AGIoqJaC9hL1bk65DTADRUCTiiqw-Cbvks5qNBN-gaJpZM4HHEu6 .

FabioBatSilva commented 8 years ago

Thanks @luisgon1