[
https://issues.apache.org/jira/browse/THRIFT-1827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14068705#comment-14068705
]
ASF GitHub Bot commented on THRIFT-1827:
----------------------------------------
GitHub user ruijiang opened a pull request:
https://github.com/apache/thrift/pull/161
Always use bit field or BitSet for isSetXXX
This is to address the issue:
https://issues.apache.org/jira/browse/THRIFT-1827
Due to the the inconsistent behavior of isSet for primitive field and
nullable field (e.g.., enum), enums with defaults will always have
isSetAnyEnumField() as true when the class is initialized. Example:
enum Gender {
UNSPECIFIED = 0,
FEMALE = 1,
MALE = 2,
}
struct User {
1: optional Gender gender = Gender.UNSPECIFIED,
2: optional i32 age = 0;
}
With this change, the generated code will look like this:
public User setGender(Gender gender) {
if (gender == null) {
setGenderIsSet(false);
return this;
}
this.gender = gender;
setGenderIsSet(true);
return this;
}
public void unsetGender() {
this.gender = null;
__isset_bitfield = EncodingUtils.clearBit(__isset_bitfield,
__GENDER_ISSET_ID);
}
/** Returns true if field gender is set (has been assigned a value) and
false otherwise */
public boolean isSetGender() {
return EncodingUtils.testBit(__isset_bitfield, __GENDER_ISSET_ID);
}
public void setGenderIsSet(boolean value) {
if (!value) {
this.gender = null;
}
__isset_bitfield = EncodingUtils.setBit(__isset_bitfield,
__GENDER_ISSET_ID, value);
}
I haven't tested this but just want to see if this the desired direction.
If so I will do more testing and make sure it works.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ruijiang/thrift THRIFT-1827
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/161.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #161
----
commit 1c4c3c0983c6f7e87973b3bef2e52838b26c7960
Author: Rui Jiang <ruiji...@pinterest.com>
Date: 2014-07-21T15:59:44Z
Always use bit field or BitSet for isSetXXX
----
> Inconsistent behavior in isSet mechanism
> ----------------------------------------
>
> Key: THRIFT-1827
> URL: https://issues.apache.org/jira/browse/THRIFT-1827
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
> Affects Versions: 0.9
> Reporter: Jason Clawson
> Fix For: 1.0
>
>
> This issue was mentioned previously in THRIFT-1394. The reporter suggested a
> fix at that time which would have resolved it. Instead, the fix that was
> actually applied did not fix the real issue.
> The issue is that code relies on 2 different mechanisms to determine "isSet"
> state. On the one hand, bit fields are kept for primitivies, on the other
> "isSet" is determined if the value is null or not. The latter part is
> incorrect considering one may wish to set a value to null.
> Consider the use case where a Thrift payload is used to send delta's of
> changed information. A client can determine which fields were "set" by using
> the "isSet" mechanism. This will not work for String / Struct types due to
> the inconsistent behavior. Client code will be unable to determine if a
> String type was: a) just not provided b) or was explicitly set to null.
> Also, it makes methods like set__IsSet and get__IsSet inconsistent. For
> example:
> user.setName(null);
> user.setNameIsSet(true);
> boolean isNameSet = user.getNameIsSet() //false!
> My proposal to fix is the same as the original proposal in THRIFT-1394:
> We already use a BitSet to track primitive types in Java. The compiler should
> extend the bit vector to also guard nullable types, to be consistent with C++.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
