You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Nathan Marz (JIRA)" <ji...@apache.org> on 2008/12/03 23:37:44 UTC
[jira] Created: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Should have a reflection feature to know if the field corresponding to a field id is set
----------------------------------------------------------------------------------------
Key: THRIFT-222
URL: https://issues.apache.org/jira/browse/THRIFT-222
Project: Thrift
Issue Type: Improvement
Components: Compiler (Java)
Reporter: Nathan Marz
The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski reassigned THRIFT-222:
---------------------------------------
Assignee: Piotr Kozikowski
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660905#action_12660905 ]
David Reiss commented on THRIFT-222:
------------------------------------
It was in patch v4.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659852#action_12659852 ]
Bryan Duxbury commented on THRIFT-222:
--------------------------------------
Awesome, I love it. I think it's ready for commit.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-222:
------------------------------------
Attachment: thrift-222-v4.patch
Here is a patch that includes a test case. I had to create a new test thrift file since the existing files must be compiled without the beans option, otherwise the other tests won't work. Both the new thrift file and the test case are called JavaBeansTest anticipating more beans-only stuff can be packed there in the future.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-222:
------------------------------------
Attachment: thrift-222-v2.patch
Here is a second patch with code cleaned up a bit. I removed entirely the part that checks for "nocamel_style_" since the thrift documentation defines nocamel as "Do not use CamelCase field accessors with beans", and I'm not producing accessors here. There are a number of other methods such as toString and setFieldValue that also ignore nocamel.
If you still would like to see the nocamel_check throughout the code refactored as a method I think it deserves its own issue, as it doesn't really have anything to do with this one.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660875#action_12660875 ]
David Reiss commented on THRIFT-222:
------------------------------------
I split this into two patches, and I moved the isSet generation into the generate_beans_boilerplate function. What do you think? http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=7eb052f;hb=82c9b14
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660913#action_12660913 ]
David Reiss commented on THRIFT-222:
------------------------------------
It looks like Chad or I accidentally added an extra one in r665562. I probably messed up a merge.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656893#action_12656893 ]
Bryan Duxbury commented on THRIFT-222:
--------------------------------------
The patch looks pretty good. Here's a few thoughts I had:
* A simple test to verify it works would be pretty nice.
* There's some commented code on the end of line 55 in the patch that doesn't seem intentional.
* Stick some javadoc that says what isSet does in the generated code.
* It might be nice to make this into a separate method, as it appears throughout the code now:
{{{
+ if (nocamel_style_) {
+ cap_name = "_" + cap_name;
+ } else {
+ cap_name[0] = toupper(cap_name[0]);
+ }
}}}
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (THRIFT-222) Should have a reflection
feature to know if the field corresponding to a field id is set
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656893#action_12656893 ]
bryanduxbury edited comment on THRIFT-222 at 12/15/08 9:55 PM:
----------------------------------------------------------------
The patch looks pretty good. Here's a few thoughts I had:
* A simple test to verify it works would be pretty nice.
* There's some commented code on the end of line 55 in the patch that doesn't seem intentional.
* Stick some javadoc that says what isSet does in the generated code.
* It might be nice to make this into a separate method, as it appears throughout the code now:
{code}
+ if (nocamel_style_) {
+ cap_name = "_" + cap_name;
+ } else {
+ cap_name[0] = toupper(cap_name[0]);
+ }
{code}
was (Author: bryanduxbury):
The patch looks pretty good. Here's a few thoughts I had:
* A simple test to verify it works would be pretty nice.
* There's some commented code on the end of line 55 in the patch that doesn't seem intentional.
* Stick some javadoc that says what isSet does in the generated code.
* It might be nice to make this into a separate method, as it appears throughout the code now:
{{{
+ if (nocamel_style_) {
+ cap_name = "_" + cap_name;
+ } else {
+ cap_name[0] = toupper(cap_name[0]);
+ }
}}}
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-222:
---------------------------------
Patch Info: [Patch Available]
I think this looks good. It would be nice to have a small test case, though.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657471#action_12657471 ]
Bryan Duxbury commented on THRIFT-222:
--------------------------------------
I think you need to take the nocamel stuff into account. Otherwise, a field like "my_field" will generate an isSet method that looks like "isSetMy_field".
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660914#action_12660914 ]
David Reiss commented on THRIFT-222:
------------------------------------
Missed a file. New link: http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=f14c3e1;hb=a867768
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-222:
------------------------------------
Attachment: thrift-222-v3.patch
Point taken, here.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-222:
------------------------------------
Attachment: thrift-222.patch
Here is a patch. It creates an isSetFieldName method for each field as well as a generic isSet(int fieldID) method. It generates the methods only for java beans. Let me know if you would change anything.
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-222.
--------------------------------
Resolution: Fixed
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-222) Should have a reflection feature to
know if the field corresponding to a field id is set
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660904#action_12660904 ]
Bryan Duxbury commented on THRIFT-222:
--------------------------------------
In general I'm happy with this division. One point of confusion - what's with the removal of the line including DebugProtoTest in the test suite?
> Should have a reflection feature to know if the field corresponding to a field id is set
> ----------------------------------------------------------------------------------------
>
> Key: THRIFT-222
> URL: https://issues.apache.org/jira/browse/THRIFT-222
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-222-v2.patch, thrift-222-v3.patch, thrift-222-v4.patch, thrift-222.patch
>
>
> The closest thing to this now is "getFieldValue(int field)", but this only provides the desired behavior for nullable fields since it delegates to the getters for each field.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.