You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Jonathan Ellis (JIRA)" <ji...@apache.org> on 2009/08/14 17:35:16 UTC
[jira] Created: (THRIFT-562) Java is inconsistent checking for
required fields
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: 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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12748554#action_12748554 ]
David Reiss commented on THRIFT-562:
------------------------------------
Reverted. Java tests still pass. Feel free to reopen if you want to discuss this further.
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12748850#action_12748850 ]
Hudson commented on THRIFT-562:
-------------------------------
Integrated in Cassandra #180 (See [http://hudson.zones.apache.org/hudson/job/Cassandra/180/])
bump thrift version to 808609 to fix regression from . patch by jbellis for CASSANDRA-387
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743458#action_12743458 ]
Jonathan Ellis commented on THRIFT-562:
---------------------------------------
new patches attached, using existing Fixtures class.
> 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-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12747062#action_12747062 ]
David Reiss commented on THRIFT-562:
------------------------------------
Sorry I'm a little late to the party, but I'm really confused by this patch. "default" fields are not meant to be required. They are always sent when writing, but not required when reading. The idea here is that we give as much information to the remote peer as we can, but if they do not know about a certain field, we don't complain that they don't send it to us. As far as I can tell, this is what the old behavior was, so I think this patch should be reverted. If you want Thrift to verify that a field is present, you should mark it required. Thoughts?
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12750132#action_12750132 ]
David Reiss commented on THRIFT-562:
------------------------------------
It is *probably* save to revert the part of r665255 that blocks the required keyword. We were more concerned with people misinterpreting "optional". I just haven't thought through all of the implications yet.
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743450#action_12743450 ]
Jonathan Ellis commented on THRIFT-562:
---------------------------------------
Hmm, I suppose it doesn't. I will re-patch.
> 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.
[jira] Resolved: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury resolved THRIFT-562.
----------------------------------
Resolution: Fixed
Fix Version/s: 0.2
Assignee: Jonathan Ellis
I just committed the second version of this patch. Thanks Jonathan!
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12750112#action_12750112 ]
Jonathan Ellis commented on THRIFT-562:
---------------------------------------
what about members of structs autogenerated for method params? can we make those "required" rather than "default"?
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743436#action_12743436 ]
Bryan Duxbury commented on THRIFT-562:
--------------------------------------
How does StructFactory differ in purpose from Fixtures?
> 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.
[jira] Updated: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated THRIFT-562:
----------------------------------
Attachment: 0002-assume-optional-required-for-java-null-checking.txt
0001-THRIFT-562-make-tests-actually-use-Fixtures.txt
> 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-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743405#action_12743405 ]
David Reiss commented on THRIFT-562:
------------------------------------
That would be THRIFT-226, I believe.
> 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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12748524#action_12748524 ]
Jonathan Ellis commented on THRIFT-562:
---------------------------------------
(leaving 01 should be ok though.)
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12750119#action_12750119 ]
David Reiss commented on THRIFT-562:
------------------------------------
No. One of the key design goals of Thrift is that you can add new parameters without doing a synchronized rollout. They just show up as default values on the server if the client doesn't send them. You can set the default in the IDL, if you want.
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Updated: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated THRIFT-562:
----------------------------------
Attachment: 562.patch
> 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: 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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12750124#action_12750124 ]
Jonathan Ellis commented on THRIFT-562:
---------------------------------------
No. Most thrift compilers don't do anything useful with default values, and most of my parameters don't have a sensible default value anyway.
Manually adding "required" in method params yields [WARNING:/home/jonathan/projects/cassandra/git-trunk/interface/cassandra.thrift:113] required keyword is ignored in argument lists.
This is broken.
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Commented: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12748523#action_12748523 ]
Jonathan Ellis commented on THRIFT-562:
---------------------------------------
this patch has the side effect of making validate() mandate the presence of exceptions which breaks all kinds of stuff when java is the client language. we should probably revert it.
> 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
> Assignee: Jonathan Ellis
> Fix For: 0.2
>
> Attachments: 0001-THRIFT-562-make-tests-actually-use-Fixtures.txt, 0001-THRIFT-562-move-boilerplate-into-StructFactory.txt, 0002-assume-optional-required-for-java-null-checking.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.
[jira] Updated: (THRIFT-562) Java is inconsistent checking for
required fields
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-562?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated THRIFT-562:
----------------------------------
Attachment: 0002-assume-optional-required-for-null-checking.txt
0001-THRIFT-562-move-boilerplate-into-StructFactory.txt
> 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.