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
[ProtoBuf] Should we check duplicating of proto id? #2653
Comments
Note: It is legal to have duplicating ids in wire data. |
IIRC duplicating ids in wire data are allowed for messages merging/updating purpose; they should represent the same type, and the last one should always win (i.e. being deserialized into actual class) |
@sandwwraith Semantically they are supposed to refer to the same value/attribute. The question is whether/how this is verified. Putting it in the plugin is challenging (esp. to make it format independent - maybe allow the property to be annotated with a |
And tried with duplicating SerialName: @Serializable
data class Data(
@SerialName("name") val name: String,
@SerialName("name") val name2: String
) And IDE ( or the plugin? ) will just show error message
and also fails in compile time. I believe that duplicating id should also fail, either in compile time or run time. |
@xiaozhikang0916 The difference is that |
Brainstorming: We can have a base type But it is still a challenge to check across classes as ProtoOneOf required. |
@xiaozhikang0916 Annotations can't have super interfaces. But we can do this with meta-annotation, e.g.
|
Oh, sorry I have few experiance in annotation developing. So then we can introduce an With this, we can collect necessary info across classes for ProtoOneOf with sealed interface, but for those with open classes, static check is still impossible. |
@xiaozhikang0916 What would happen is that the code that generates the serial descriptor (for a generated serializer) would verify that annotations with the given meta annotation are unique (in whatever way we define it, noting that some annotations have multiple parameters and may need to be unique across some of them).
|
Right now the
ProtoBuf
format allow duplicating proto ids in one message, e.g.is a valid definition in this lib, and can be serialized into wire.
But when decoding from the same wire data will just throw message complaining wrong wire type.
Intent
IMO we should check that all proto ids should be unique, as described in the Protocol Buffer Documentation, before any deserializing and serializing happens.
With this check, developers can find out what is wrong ( or what will be wrong ) more easily, especially in messages with one-of properties (in #2546), whose ids may be in different classes.
The text was updated successfully, but these errors were encountered: