[jira] Commented: (THRIFT-562) Java is inconsistent checking for required fields

["Jonathan Ellis (JIRA)" <ji...@apache.org>]

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

Jonathan Ellis commented on THRIFT-562:
---------------------------------------

new patch series attached.

0001 cleans up the boilerplate struct creation by moving common test code into StructFactory.
0002 makes the change to the compiler and adds required fields to CompactProtoTestStruct

This reveals a bug in the generated equals() methods:

      // TODO equals is broken when List<array> is involved, since AbstractList.equals(Object o)
      // calls o.equals, but for arrays that is just == which will never work when you are
      // comparing pre- and post- serialized versions.  You need to use Arrays.equals instead.
      // So, here I have special-cased CPTS to avoid failing the test b/c of this bug.

(I'm fine with adding the required fields, but I submit that fixing this too is _really_ outside the scope of this ticket.)


> Java is inconsistent checking for required fields
> -------------------------------------------------
>
>                 Key: THRIFT-562
>                 URL: https://issues.apache.org/jira/browse/THRIFT-562
>             Project: Thrift
>          Issue Type: Bug
>            Reporter: Jonathan Ellis
>         Attachments: 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-null-checking.txt, 562.patch
>
>
> In several places, the java generator does things like
>     // Most existing Thrift code does not use isset or optional/required,
>     // so we treat "default" fields as required.
>     bool is_optional = (*m_iter)->get_req() == t_field::T_OPTIONAL;
> but it doesn't do so consistently.  This replaces the remaining == REQUIRED checks with != OPTIONAL to also include fields in the DEFAULT state.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.