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 2009/01/08 23:24:59 UTC
[jira] Created: (THRIFT-253) Enhance FieldMetaData
Enhance FieldMetaData
---------------------
Key: THRIFT-253
URL: https://issues.apache.org/jira/browse/THRIFT-253
Project: Thrift
Issue Type: Improvement
Components: Compiler (Java)
Reporter: Nathan Marz
Priority: Minor
Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
// need a better name than "Necessity"
enum Necessity {
REQUIRED, OPTIONAL, DEFAULT
}
interface FieldValueMetaData {
}
class FieldMetaData {
String fieldName;
Necessity necessity;
FieldValueMetaData valueMetaData;
}
class ListFieldMetaData extends FieldValueMetaData {
FieldValueMetaData type;
}
class SetFieldMetaData extends FieldValueMetaData {
FieldValueMetaData type;
}
class MapFieldMetaData extends FieldValueMetaData {
FieldValueMetaData keyType;
FieldValueMetaData valueType;
}
class ObjectFieldMetaData implements FieldValueMetaData {
Class type;
}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-253:
------------------------------------
Attachment: thrift-253-v4.patch
Ok, here.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668332#action_12668332 ]
Bryan Duxbury commented on THRIFT-253:
--------------------------------------
Hm, probably not a bad idea.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch, thrift-253-v5.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski reassigned THRIFT-253:
---------------------------------------
Assignee: Piotr Kozikowski
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-253:
------------------------------------
Attachment: thrift-253-v5.patch
Here is a patch with the additional comments/javadoc and coding style corrections. As for the rest:
* FieldMetaData
** Is it really required to instantiate a class for it to be loaded? Once you have a class reference to its class, won't it be loaded already? Yes, it is. I tried it in the test.
** Do you need a separate static {} block to instantiate the structMap? Can't you just do it inline with the declaration? Again, yes. I tried to do it without the static {} block first and it doesn't work as intended.
* TFieldRequirementType
** Would it make sense for this to be a Java enum instead of a class? I created a class so that it's consistent with the way TType is implemented
* t_java_generator
** Is the final else in get_type_string necessary? if it should never happen, perhaps just skip the else, or let the base type branch be the else. If new thrift types were added (e.g. binary not as string), that else would trigger, giving you info about what happened.
is there a reason that it's better for the generated code to register itself with the map rather than have FieldMetaData.getStructMetadataMap to use Java reflection to get the metadata?
Well, it's just that this way we are sticking as much as possible to the idea that metadata is static information created as the class is loaded. I don't see any reason to change it.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch, thrift-253-v5.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667357#action_12667357 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
I think the declared exceptions IllegalAccessException and InstantiationException should be caught within getStructMetaDataMap and rethrown as RuntimeExceptions instead of having the "throws Exception" clause. Since the object instantiated is a TBase there's no reason for those exceptions to happen.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666647#action_12666647 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
To alleviate "the global map in FieldMetaData will only contain entries for loaded classes (e.g. an object of the class has been instantiated)", I suggest that we call sClass,newInstance() in getStructMetaDataMap if the class isn't in the map yet. This reflection cost would only be paid once and I think it's acceptable.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665248#action_12665248 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
Overall this is really good. A couple minor suggestions:
1. Instead of having variables isStruct and isContainer, can we have convenience methods instead? Such as:
public boolean isStruct() { return type==TType.STRUCT; }
2. Instead of "default : return NULL; break; // This should never happen!" is it possible to throw an exception of some sort?
Also, to make the meta data stuff complete, I think we should have a global map from the struct name to the field meta data map. This will make it possible to "search" the meta data without ever resorting to java reflection. Don't really know where this meta data map should go... maybe generate a new class called "GlobalMetaData". This class can go into a "meta_data" subpackage of the generated code to prevent collision with user chosen struct names. Or maybe have a "GlobalMetaData" class in the thrift library, and the static initializers of every thrift struct can add their meta data map to the global class.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665547#action_12665547 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
That sounds good to me.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665531#action_12665531 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
It could contain the Class. The same problem would still exist though - just given the Class, you can't get the field meta data map without doing java reflection, so some sort of global lookup would still be necessary (in this case, from Class to Map).
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-253:
------------------------------------
Attachment: thrift-253-v1.patch
All right, here is my first attempt. It follows more or less your suggestion. The class FieldMetaData contains properties only a field can have (i.e. fieldName and requirementType) and a FieldValueMetaData object. The FieldValueMetaData class contains the type, and the booleans isStruct and isContainer for convenience. If a value is a container, its metadata will be one of FieldValueMetaData's subclasses, and have additional members. For example, ListMetaData has an elemMetaData variable that contains a FieldValueMetaData object, which can also be a container, having its own FieldValueMetaData object, and so on.
If you like this approach, I'll write a test case to make sure all nesting possibilities are covered.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668306#action_12668306 ]
David Reiss commented on THRIFT-253:
------------------------------------
Should the functions that access the global map be synchronized?
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch, thrift-253-v5.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665541#action_12665541 ]
Bryan Duxbury commented on THRIFT-253:
--------------------------------------
If you wanted to add a helper to TBase or FieldMetaData or something that gives you the meta data map from a Class<? extends TBase>, I'd be fine with that. Struct names aren't reliable enough to use as a global identifier.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667746#action_12667746 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
+1
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665280#action_12665280 ]
Bryan Duxbury commented on THRIFT-253:
--------------------------------------
Do we really need a global metadata map? That seems like duplication of work. When will you have a struct name and not the type of its fields? Is there some use case you're thinking of that needs this?
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-253:
------------------------------------
Attachment: thrift-253-v3.patch
Ok, here is an updated patch that does that. I also added a small test case to check for this in the test.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
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-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12668091#action_12668091 ]
pkozikow edited comment on THRIFT-253 at 1/28/09 9:55 AM:
------------------------------------------------------------------
Here is a patch with the additional comments/javadoc and coding style corrections. As for the rest:
* FieldMetaData
** Is it really required to instantiate a class for it to be loaded? Once you have a class reference to its class, won't it be loaded already? Yes, it is. I tried it in the test.
** Do you need a separate static {} block to instantiate the structMap? Can't you just do it inline with the declaration? Again, yes. I tried to do it without the static {} block first and it doesn't work as intended.
* TFieldRequirementType
** Would it make sense for this to be a Java enum instead of a class? I created a class so that it's consistent with the way TType is implemented
* t_java_generator
** Is the final else in get_type_string necessary? if it should never happen, perhaps just skip the else, or let the base type branch be the else. If new thrift types were added (e.g. binary not as string), that else could be triggered, giving you a hint that you should update the method with the new type.
is there a reason that it's better for the generated code to register itself with the map rather than have FieldMetaData.getStructMetadataMap to use Java reflection to get the metadata?
Well, it's just that this way we are sticking as much as possible to the idea that metadata is static information created as the class is loaded. I don't see any reason to change it.
was (Author: pkozikow):
Here is a patch with the additional comments/javadoc and coding style corrections. As for the rest:
* FieldMetaData
** Is it really required to instantiate a class for it to be loaded? Once you have a class reference to its class, won't it be loaded already? Yes, it is. I tried it in the test.
** Do you need a separate static {} block to instantiate the structMap? Can't you just do it inline with the declaration? Again, yes. I tried to do it without the static {} block first and it doesn't work as intended.
* TFieldRequirementType
** Would it make sense for this to be a Java enum instead of a class? I created a class so that it's consistent with the way TType is implemented
* t_java_generator
** Is the final else in get_type_string necessary? if it should never happen, perhaps just skip the else, or let the base type branch be the else. If new thrift types were added (e.g. binary not as string), that else would trigger, giving you info about what happened.
is there a reason that it's better for the generated code to register itself with the map rather than have FieldMetaData.getStructMetadataMap to use Java reflection to get the metadata?
Well, it's just that this way we are sticking as much as possible to the idea that metadata is static information created as the class is loaded. I don't see any reason to change it.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch, thrift-253-v5.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-253:
------------------------------------
Attachment: thrift-253-v2.patch
Here is a new patch.
Main changes:
-isContainer and isStruct refactored as methods
-StructMetaData now contains a "structClass" member instead of "structType"
-FieldMetaData has a private map with metadata maps for struct classes, with methods for adding/getting maps
-The metadata map declaration in the generated code is more readable for deeply nested structures
Notes:
-exceptions are stored as type struct in the metadata map (no exception type in TType.java)
-typedefs are stored as the types they represent
-the global map in FieldMetaData will only contain entries for loaded classes (e.g. an object of the class has been instantiated)
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
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-253) Enhance FieldMetaData
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666544#action_12666544 ]
pkozikow edited comment on THRIFT-253 at 1/23/09 6:21 AM:
------------------------------------------------------------------
Here is a new patch.
Main changes:
-isContainer and isStruct refactored as methods
-StructMetaData now contains a "structClass" member instead of "structType"
-FieldMetaData has a private map with metadata maps for struct classes, with methods for adding/getting maps
-The metadata map declaration in the generated code is more readable for deeply nested structures
-Added a test and a new struct to ThriftTest.thrift
Notes:
-exceptions are stored as type struct in the metadata map (no exception type in TType.java)
-typedefs are stored as the types they represent
-the global map in FieldMetaData will only contain entries for loaded classes (e.g. an object of the class has been instantiated)
was (Author: pkozikow):
Here is a new patch.
Main changes:
-isContainer and isStruct refactored as methods
-StructMetaData now contains a "structClass" member instead of "structType"
-FieldMetaData has a private map with metadata maps for struct classes, with methods for adding/getting maps
-The metadata map declaration in the generated code is more readable for deeply nested structures
Notes:
-exceptions are stored as type struct in the metadata map (no exception type in TType.java)
-typedefs are stored as the types they represent
-the global map in FieldMetaData will only contain entries for loaded classes (e.g. an object of the class has been instantiated)
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667761#action_12667761 ]
Bryan Duxbury commented on THRIFT-253:
--------------------------------------
* FieldMetaData
** Could use a class comment explaining its purpose
** getStructMetaDataMap could use some javadoc
** Is it really required to instantiate a class for it to be loaded? Once you have a class reference to its class, won't it be loaded already?
** Do you need a separate static {} block to instantiate the structMap? Can't you just do it inline with the declaration?
* StructMetaData
** super(type) is mis-indented.
* TFieldRequirementType
** Would it make sense for this to be a Java enum instead of a class?
* t_java_generator
** get_java_type_string could use a method comment describing its purpose
** Is the final else in get_type_string necessary? if it should never happen, perhaps just skip the else, or let the base type branch be the else.
** Ending } and else should be on the same line
** Avoid using no-bracket else and ifs
Finally, just for the sake of argument, is there a reason that it's better for the generated code to register itself with the map rather than have FieldMetaData.getStructMetadataMap to use Java reflection to get the metadata? You could clearly use the same caching strategy to avoid repeated reflection. I'm not really sure it's better, just thinking out loud.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nathan Marz updated THRIFT-253:
-------------------------------
Description:
Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
// need a better name than "Necessity"
enum Necessity {
REQUIRED, OPTIONAL, DEFAULT
}
interface FieldValueMetaData {
}
class FieldMetaData {
String fieldName;
Necessity necessity;
FieldValueMetaData valueMetaData;
}
class ListFieldMetaData implements FieldValueMetaData {
FieldValueMetaData type;
}
class SetFieldMetaData implements FieldValueMetaData {
FieldValueMetaData type;
}
class MapFieldMetaData implements FieldValueMetaData {
FieldValueMetaData keyType;
FieldValueMetaData valueType;
}
class ObjectFieldMetaData implements FieldValueMetaData {
Class type;
}
was:
Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
// need a better name than "Necessity"
enum Necessity {
REQUIRED, OPTIONAL, DEFAULT
}
interface FieldValueMetaData {
}
class FieldMetaData {
String fieldName;
Necessity necessity;
FieldValueMetaData valueMetaData;
}
class ListFieldMetaData extends FieldValueMetaData {
FieldValueMetaData type;
}
class SetFieldMetaData extends FieldValueMetaData {
FieldValueMetaData type;
}
class MapFieldMetaData extends FieldValueMetaData {
FieldValueMetaData keyType;
FieldValueMetaData valueType;
}
class ObjectFieldMetaData implements FieldValueMetaData {
Class type;
}
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Priority: Minor
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665317#action_12665317 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
The use case is when searching a struct for a value of a particular type. For example, if I have the following structs:
struct C {
}
struct B {
C c;
}
struct A {
B b;
}
Suppose I am "searching" the struct tree for objects of type C. If I have a struct of type A, from the field meta data I can find that it has a struct named "B" inside of it. Just given that string, there's no way to get "B"'s meta data map without doing java reflection. I would like to be able to say GlobalMetaData.getFieldMetaMap("B") and get the mapping of field id's to meta data.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665526#action_12665526 ]
Bryan Duxbury commented on THRIFT-253:
--------------------------------------
Why would the FieldMetaData entry for struct A contain the string name of the B struct and not the Java class for B?
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-253) Enhance FieldMetaData
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury resolved THRIFT-253.
----------------------------------
Resolution: Fixed
OK, I made a few small edits (style), and moved the new test and build.xml changes to the right spots to play well with THRIFT-166. Committed.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch, thrift-253-v5.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-253) Enhance FieldMetaData
Posted by "Nathan Marz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667772#action_12667772 ]
Nathan Marz commented on THRIFT-253:
------------------------------------
Is it really required to instantiate a class for it to be loaded? Once you have a class reference to its class, won't it be loaded already?
Yes, this is indeed the case. Just having a class reference doesn't load the class.
> Enhance FieldMetaData
> ---------------------
>
> Key: THRIFT-253
> URL: https://issues.apache.org/jira/browse/THRIFT-253
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-253-v1.patch, thrift-253-v2.patch, thrift-253-v3.patch, thrift-253-v4.patch
>
>
> Having more meta data would be useful in the FieldMetaData structure. Specifically, it should be enhanced to contain information about whether the field is a list, set, map, base type, or another thrift structure. If it's a container type, it should contain information about what types are in the container. For base types and other thrift structures, it should specify the type. There's many ways we could encode this information - here's a first proposal and I'm open to suggestions.The idea is to have a "FieldMetaData", which includes data about the field like field name, "required", "optional", etc. It also includes a "FieldValueMetaData" which would have a subclass for regular objects or for containers, which contain further meta data about the types within.
> // need a better name than "Necessity"
> enum Necessity {
> REQUIRED, OPTIONAL, DEFAULT
> }
> interface FieldValueMetaData {
> }
> class FieldMetaData {
> String fieldName;
> Necessity necessity;
> FieldValueMetaData valueMetaData;
> }
> class ListFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class SetFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData type;
> }
> class MapFieldMetaData implements FieldValueMetaData {
> FieldValueMetaData keyType;
> FieldValueMetaData valueType;
> }
> class ObjectFieldMetaData implements FieldValueMetaData {
> Class type;
> }
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.