[
https://issues.apache.org/jira/browse/THRIFT-735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12845713#action_12845713
]
Bryan Duxbury commented on THRIFT-735:
--------------------------------------
I don't think that using a type in a required field limits the flexibility of
that type. It certainly limits the flexibility of the struct that uses that
type - but that's fundamentally intentional.
While you're right that adding a field in the RPC context is the same as
"deleting" a field, again, this is only an issue if you are using required
fields. If you say that a field is required, you are stating unequivocally that
without some well-understood contents in that field, you can't work with the
entire struct. If you plan to evolve your union fields or enum values, then you
*must not use required fields*. I don't see how a well-meaning IDL coder could
specify that a field is required and then be OK getting a null in some spot
because you want to evolve elegantly.
Let's look at an RPC example.
{code:title=client_side.thrift}
union MyResponse {
1: string response_type_1;
}
service MyService {
MyResponse someMethod(string blah);
}
{code}
{code:title=server_side.thrift}
union MyResponse {
1: string response_type_1;
2: string response_type_2;
}
service MyService {
MyResponse someMethod(string blah);
}
{code}
With these two thrift files, how does it help the client to get back an unset
union instead of a validation error? As far as the client is concerned, the
result is unintelligible either way. At least if the validation works like I
describe, they won't have to implement arbitrarily complex app-level validation
to determine that they can't interpret the message.
> Required field checking is broken when the field type is a Union struct
> -----------------------------------------------------------------------
>
> Key: THRIFT-735
> URL: https://issues.apache.org/jira/browse/THRIFT-735
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java), Library (Java)
> Affects Versions: 0.2
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.3
>
> Attachments: thrift-735.patch
>
>
> The validate() method on generated structs verifies that required fields are
> set after validation. However, if the type of the field is a Union struct,
> then just checking that the field isn't null is not a valid check. The value
> may be a non-null union, but have an unset field. (We encountered this when
> deserializing a type that had a union for a field, and the union's set value
> was an enum value that had been removed from the definition, making it a
> skip.)
> In order to perform the correct validation, if the value is a Union, then we
> must also check that the set field and value are non-null.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
