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

MessageType.verify should accept all values that MessageType.toObject can produce #748

Closed
murgatroid99 opened this issue Apr 4, 2017 · 10 comments
Labels

Comments

@murgatroid99
Copy link
Contributor

protobuf.js version: 6.7.0

For some conversion options, verify appears to reject some values that toObject can produce. For example, longs: String is a valid conversion option, but if a string is provided, verify outputs uint_64: integer|Long expected. Similarly, enums: String is a valid conversion option, but if a string is provided, verify outputs enum_value: enum value expected.

@dcodeIO
Copy link
Member

dcodeIO commented Apr 5, 2017

This is intended and that's why .fromObject exists. Basically, .toObject produces arbitrary plain objects for interoperability with other libraries, storage and whatnot while .fromObject converts any object to something understood by protobuf.js.

See: Valid Message

Runtime message instances, as produced by .fromObject, always are valid messages by design. The ability to encode plain objects without prior conversion is merely a sideproduct of using a dynamically typed language / JavaScript, where, technically, a plain object can also be a valid message. .verify is there to test this. In an ideal world, everyone would just use ideal JS types making this distinction unnecessary.

Reasoning is that, if .encode was able to handle every single JS type possible, it would be super slow because it'd require a lot of redundant additional assertions and implicit conversions. Hence, this functionality is split up into .encodeing of what's fast and .fromObject-converting what's not.

Btw., had to edit my answer multiple times to get this right. This stuff has clearly become too complex for a single README. I also start to worry that all those optimizations just make v6 too hard to use. Without it, it wouldn't be possible to encode half a million messages per second (referring to the benchmark, v5 was 10 times slower or something), though.

@dcodeIO
Copy link
Member

dcodeIO commented Apr 5, 2017

More or less confusing diagram of what I just said:

@dcodeIO
Copy link
Member

dcodeIO commented Apr 5, 2017

I now also rephrased the Usage section in this regard. Hope this helps.

@murgatroid99
Copy link
Contributor Author

OK, I think I understand now. verify indicates that calling create or encode directly on the plain object will succeed. fromObject, on the other hand, does conversion from a broader range of plain objects to create valid messages.

@LarsOL
Copy link

LarsOL commented Dec 19, 2017

We just encountered this discrepancy, in our situation we are transmitting json over the wire (from native GoLang/Java protobuf serializers ). We expected to call verify on the json followed by create, but it seems that verify expects a non standard internal format?

Specifically

message ReleaseRequest {
...
    google.protobuf.BoolValue test = 9;
    google.protobuf.Int32Value next = 10;

}

gets serialized into

{
  "test": true,
  "next": 50
}

By both java and Golang reference implementations. But the verify expects test to be a object. Something like (??) :

{
  "test": {
     value: true
  },
  "next": {
      value:50
   }
}

which is non standard?
From my understanding the only way for us to ingest the json is fromObject, though this seems to be a lot less strict in its checking?

@dcodeIO
Copy link
Member

dcodeIO commented Dec 19, 2017

It's not non-standard, see: https://github.com/google/protobuf/blob/fd046f6263fb17383cafdbb25c361e3451c31105/src/google/protobuf/wrappers.proto#L99

There simply isn't a wrapper yet that deals with these types specifically, so the underlying .proto representation is used.

@LarsOL
Copy link

LarsOL commented Dec 19, 2017

From my understanding the verify expects a raw javascript object formed 'as is' protobuf message format, rather than the JSON version?
The difference being that the protobuf JSON serialization/deserialization standard differs from the message format. As mentioned in that file

The JSON representation for BoolValue is JSON true and false.

I guess the confusion is then, which method is meant to deserialize JSON ,fromObject, or is verify? (assuming the wrapper get implemented)

Thanks for helping clarify

@dcodeIO
Copy link
Member

dcodeIO commented Dec 19, 2017

verify is used to test whether an object meets the criteria of being a valid message. It doesn't deserialize. The different methods are described in detail here.

@dinesh-nadar
Copy link

dinesh-nadar commented May 13, 2018

@dcodeIO I am using the version 6.8.6 and facing the issue described below (which seems to be related to this thread)

I would like to ensure the validation around missing required fields happens when using decode as described at https://github.com/dcodeIO/protobuf.js#toolset
If required fields are missing, it throws a util.ProtocolError with an instance property set to the so far decoded message.

For a type described below

type Person {
    required name = 1;
    enum Health { 
       GOOD = 0; 
       BAD=1;
       UNKNOWN=2;
    }
    required Health health = 2;   
}

If given a plain Javascript object such as const person = {name: 'John', health: 'BAD'}

  • message.verify(person) results in an error message as expected, given that the expected enum value is not a number
  • message.fromObject(person) creates a valid message instance which could then be passed to encode. Decoding this using decode and then using message.toObject({enums: String}) returns {name:'John', health:'BAD'}.

But if I do decide to use message.fromObject(person) and the plain Javascript object is missing required fields such as const person = {firstName: Rob} and encode the resultant message instance, decode does not throw exceptions on missing required fields (and the resultant Javascript object is {name:'', health: 'GOOD'}) since message.fromObject() created a valid message instance

Would there be a way whereby I could use message.fromObject() for the enum translation and still somehow ensure decode throws an error when missing required fields (or for verify to return an error message)?

@jlisee
Copy link
Contributor

jlisee commented Oct 26, 2018

This is an awesome library. It's such a joy to work dynamically with protobuf types like this. Which is why it's a bummer that I have the same issue as @dinesh-nadar.

I want to be able to use the default JSON mapping of enums to protobuf which is strings (as documented here), and at the same time I still want to be able to verify the validity of the object I am working with.

Right now I am using this code to pre-process the JSON objects calling protoEnumsToInts(message, object) before message.verify(object), it's an OK workaround, but was a little involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants