You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Jens Geyer (JIRA)" <ji...@apache.org> on 2017/11/14 23:33:00 UTC

[jira] [Comment Edited] (THRIFT-4381) Wrong isset bitfield value after transmission

    [ https://issues.apache.org/jira/browse/THRIFT-4381?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16252682#comment-16252682 ] 

Jens Geyer edited comment on THRIFT-4381 at 11/14/17 11:32 PM:
---------------------------------------------------------------

As I said, this is by design. 

Key to understanding and to get what you want is the term ["requiredness"|https://thrift.apache.org/docs/idl]. With Thrift, we have three, while only two of them actually have a keyword assigned, the third one is implicitly used if you don't say anything else. Here's an example:

{code}
struct custom {
	1: /*default*/	i32  default_field
	2: optional	i32  optional_field
	3: required	i32  required_field
}
{code}

The semantics w/regard to read and write are different for all three:

* {{optional}}  is only read and written when there is a value assigned. This is checked via the "isset" bit flags.
* {{required}} fields are, big surprise, required and thus are always written and read.
* If neither {{optional}} nor {{required}} is specified, the so-called "default" requiredness takes over. 

In that latter case of "default", values are only read when they are on the wire, but *always* written. So "default" is kind of a mix between {{optional}} and {{required}}: On write, values are always written like {{required}} (unlesss they are {{null}} pointers, which integers and most other primitives aren't) but {{optional}} on read.

*Consequently, the solution in your case is to specify {{optional}} before the field type.*

Here's the Java code of the {{write()}} method generated from the IDL example above:

{code}
    public void write(org.apache.thrift.protocol.TProtocol oprot, custom struct) throws org.apache.thrift.TException {
      struct.validate();

      oprot.writeStructBegin(STRUCT_DESC);
      oprot.writeFieldBegin(DEFAULT_FIELD_FIELD_DESC);
      oprot.writeI32(struct.default_field);
      oprot.writeFieldEnd();
      if (struct.isSetOptional_field()) {
        oprot.writeFieldBegin(OPTIONAL_FIELD_FIELD_DESC);
        oprot.writeI32(struct.optional_field);
        oprot.writeFieldEnd();
      }
      oprot.writeFieldBegin(REQUIRED_FIELD_FIELD_DESC);
      oprot.writeI32(struct.required_field);
      oprot.writeFieldEnd();
      oprot.writeFieldStop();
      oprot.writeStructEnd();
    }
  }
{code}






was (Author: jensg):
As I said, this is by design. 

Key to understanding and to get what you want is the term "requiredness". With Thrift, we have three, while only two of them actually have a keyword assigned, the third one is implicitly used if you don't say anything else. Here's an example:

{code}
struct custom {
	1: /*default*/	i32  default_field
	2: optional	i32  optional_field
	3: required	i32  required_field
}
{code}

The semantics w/regard to read and write are different for all three:

* {{optional}}  is only read and written when there is a value assigned. This is checked via the "isset" bit flags.
* {{required}} fields are, big surprise, required and thus are always written and read.
* If neither {{optional}} nor {{required}} is specified, the so-called "default" requiredness takes over. 

In that latter case of "default", values are only read when they are on the wire, but *always* written. So "default" is kind of a mix between {{optional}} and {{required}}: On write, values are always written like {{required}} (unlesss they are {{null}} pointers, which integers and most other primitives aren't) but {{optional}} on read.

*Consequently, the solution in your case is to specify {{optional}} before the field type.*

Here's the Java code of the {{write()}} method generated from the IDL example above:

{code}
    public void write(org.apache.thrift.protocol.TProtocol oprot, custom struct) throws org.apache.thrift.TException {
      struct.validate();

      oprot.writeStructBegin(STRUCT_DESC);
      oprot.writeFieldBegin(DEFAULT_FIELD_FIELD_DESC);
      oprot.writeI32(struct.default_field);
      oprot.writeFieldEnd();
      if (struct.isSetOptional_field()) {
        oprot.writeFieldBegin(OPTIONAL_FIELD_FIELD_DESC);
        oprot.writeI32(struct.optional_field);
        oprot.writeFieldEnd();
      }
      oprot.writeFieldBegin(REQUIRED_FIELD_FIELD_DESC);
      oprot.writeI32(struct.required_field);
      oprot.writeFieldEnd();
      oprot.writeFieldStop();
      oprot.writeStructEnd();
    }
  }
{code}





> Wrong isset bitfield value after transmission
> ---------------------------------------------
>
>                 Key: THRIFT-4381
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4381
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Library
>    Affects Versions: 0.10.0
>         Environment: Linux Arch / Oracle JDK v1.8.0u152
> and
> Windows 7 / Oracle JDK v1.8.0u151
>            Reporter: Nicolas V.
>            Assignee: Jens Geyer
>              Labels: newbie, usability
>             Fix For: 0.11.0
>
>
> The bitfield field is set to true for every field after deserialization of a Thrift message structure.
> Here is a simple test program : 
> {code:java}
> public class BitFieldTest
> {
> 	public static void main(String[] args) throws TException
> 	{
> 		final CBChannel chan = new CBChannel();
> 		chan.setId(42L);
> 		chan.setName("test");
> 		// should return true, true, false
> 		System.out.println("id is set ? " + chan.isSetId());
> 		System.out.println("name is set ? " + chan.isSetName());
> 		System.out.println("duration max is set ? " + chan.isSetDurationMax());
> 		final TProtocolFactory protoFactory = new TCompactProtocol.Factory();
> 		final byte[] buf = new TSerializer(protoFactory).serialize(chan);
> 		System.out.println("\n---- Hexdump serialized message : ----\n" + HexTools.toHexString(buf));
> 		final CBChannel chanDst = new CBChannel();
> 		new TDeserializer(protoFactory).deserialize(chanDst, buf);
> 		System.out.println("---- toString() of deserialized api msg : ----\n" + chanDst);
> 		// should return true, true, false
> 		System.out.println("\nid is set ? " + chanDst.isSetId());
> 		System.out.println("name is set ? " + chanDst.isSetName());
> 		System.out.println("duration max is set ? " + chanDst.isSetDurationMax());
> 	}
> }
> {code}
> The output is :
> {noformat}
> id is set ? true
> name is set ? true
> duration max is set ? false
> ---- Hexdump serialized message : ----
> 00	16 54 18 04 74 65 73 74 12 26 00 16 00 16 00 16 	.T..test.&......
> 10	00 16 00 16 00 16 00 00                         	........
> ---- toString() of deserialized api msg : ----
> CBChannel(id:42, name:test, enabled:false, type:null, durationMin:0, durationMax:0, inactivityAlarm:0, analogicConfiguration:0, digitalConfiguration:0, voipConfiguration:0, r17Index:0, group:null)
> id is set ? true
> name is set ? true
> duration max is set ? true
> {noformat}
> You can see that the last test for is set on the durationMax field return true, when it sould return false.
> Here is the idl file :
> {code:java}
> enum ChannelType
> {
> 	ANALOGIC
> 	DIGITAL
> 	VOIP
> }
> struct CBChannelGroup
> {
>   1: i64 id;
>   2: i64 parentId;
>   3: string name;
> }
> struct CBChannel
> {
>   1: i64 id;
>   2: string name;
>   3: bool enabled;
>   4: ChannelType type;
>   5: i64 durationMin;
>   6: i64 durationMax;
>   7: i64 inactivityAlarm;
>   8: i64 analogicConfiguration;
>   9: i64 digitalConfiguration;
>   10: i64 voipConfiguration;
>   11: i64 r17Index;
>   12: CBChannelGroup group;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)