This content has been marked as final.
Show 43 replies
-
15. Re: Field injection
alesj Mar 14, 2008 8:31 AM (in response to alesj)A first hurdle.
How to get past this usage in PropertyDispatchWrapper (kernel module):PropertyInfo propertyInfo = BeanInfoUtil.getPropertyInfo(beanInfo, target, name); ValueMetaData valueMetaData = property.getValue(); Object value = valueMetaData.getValue(propertyInfo.getType(), cl); beanInfo.setProperty(target, name, value);
Since I need the expected type of the value in order to convert it to the right type, e.g. string --> Date.
But what if my bean is in FIELD or ALL mode, and there is no such property?
Anything that comes to my mind in api extension to get the type by name from BeanInfo feels wrong. :-) -
16. Re: Field injection
adrian.brock Mar 14, 2008 8:42 AM (in response to alesj)"alesj" wrote:
A first hurdle.
How to get past this usage in PropertyDispatchWrapper (kernel module):PropertyInfo propertyInfo = BeanInfoUtil.getPropertyInfo(beanInfo, target, name); ValueMetaData valueMetaData = property.getValue(); Object value = valueMetaData.getValue(propertyInfo.getType(), cl); beanInfo.setProperty(target, name, value);
Since I need the expected type of the value in order to convert it to the right type, e.g. string --> Date.
But what if my bean is in FIELD or ALL mode, and there is no such property?
Anything that comes to my mind in api extension to get the type by name from BeanInfo feels wrong. :-)
I don't understand.
So either you don't undetrstand or you haven't explained it very well. ;-)
You can't set a property unless it exists in the BeanInfopublic class MyClass { private Date myDate; }
This should have a BeanInfo with one property which is a FieldPropertyInfo
i.e. instead of:
MethodInfo getter, setter;
it has
FieldInfo field; (there is no getter/setter)
propertyInfo.getType() -> field.getType(); // instead of the "return type"
propertyInfo.set() -> field.set(); // instead of a method invocation -
17. Re: Field injection
adrian.brock Mar 14, 2008 8:45 AM (in response to alesj)"adrian@jboss.org" wrote:
FieldInfo field; (there is no getter/setter)
Since you can no longer use// Readable? if (property.getGetter() != null)
We probably want to add isReadable() and isWritable() to the PropertyInfo interface. -
18. Re: Field injection
alesj Mar 14, 2008 8:51 AM (in response to alesj)"adrian@jboss.org" wrote:
This should have a BeanInfo with one property which is a FieldPropertyInfo
i.e. instead of:
MethodInfo getter, setter;
it has
FieldInfo field; (there is no getter/setter)
propertyInfo.getType() -> field.getType(); // instead of the "return type"
propertyInfo.set() -> field.set(); // instead of a method invocation
This is exactly what I was looking for ... and had similar hack in mind. :-)
So, it looks like it was explained well after all.
Or you could be psychic. :-) -
19. Re: Field injection
adrian.brock Mar 14, 2008 8:57 AM (in response to alesj)"alesj" wrote:
Or you could be psychic. :-)
Not really, I just explained how I would do it. I assumed you were heading up
some blind alley with some different (unexplained) implementation choice. ;-) -
20. Re: Field injection
alesj Mar 14, 2008 11:10 AM (in response to alesj)"adrian@jboss.org" wrote:
This should have a BeanInfo with one property which is a FieldPropertyInfo
What about if the bean is in ALL or FIELDS mode and the property has just setter or just getter?
OK, there should be some kind of CombinedPropertyInfo.
The real question is, should I also replace the existing AbstractPropertyInfo in AbstractBeanInfo.properties or just in AbstractBeanInfo.propertiesByName? -
21. Re: Field injection
adrian.brock Mar 14, 2008 11:25 AM (in response to alesj)"alesj" wrote:
"adrian@jboss.org" wrote:
This should have a BeanInfo with one property which is a FieldPropertyInfo
What about if the bean is in ALL or FIELDS mode and the property has just setter or just getter?
OK, there should be some kind of CombinedPropertyInfo.
I don't know, I'm not deep in the implementation details and don't want to be.
I have my own work to do. :-)
If you think there is a policy choice to be made then let
the user decide with a addtional enum value.
Don't mix policy and implementation! :-)
The real question is, should I also replace the existing AbstractPropertyInfo in AbstractBeanInfo.properties or just in AbstractBeanInfo.propertiesByName?
I know you're probably scared of me telling you did it wrong after the fact,
but now I'll tell you that you are asking a stupid question before the fact. :-)
Isn't it obvious that PropertyInfo is the contract and AbstractPropertyInfo
is an implementation detail?
There should be some refactoring like;public abstract class AbstractPropertyInfo implements PropertyInfo { String name } public class StandardPropertyInfo extends AbstractPropertyInfo { MethodInfo getter, MethodInfo setter } public class FieldPropertyInfo extends AbstractPropertyInfo { FieldInfo field }
I'll let you decide some better names :-) -
22. Re: Field injection
adrian.brock Mar 14, 2008 11:36 AM (in response to alesj)"adrian@jboss.org" wrote:
"alesj" wrote:
"adrian@jboss.org" wrote:
This should have a BeanInfo with one property which is a FieldPropertyInfo
What about if the bean is in ALL or FIELDS mode and the property has just setter or just getter?
OK, there should be some kind of CombinedPropertyInfo.
I don't know, I'm not deep in the implementation details and don't want to be.
I have my own work to do. :-)
If you think there is a policy choice to be made then let
the user decide with a addtional enum value.
Or post examples where you think there is a problem we don't have
to do full analysis ourselves.
Then we can decide whether use case makes sense.
To me the only ones that I'd personally use are the three I gave above,
but I could be wrong.
The more permutations and external policy you add, the more you've got to test and
the more chance of error due to complexity. -
23. Re: Field injection
alesj Mar 14, 2008 11:51 AM (in response to alesj)"adrian@jboss.org" wrote:
I know you're probably scared of me telling you did it wrong after the fact,
but now I'll tell you that you are asking a stupid question before the fact. :-)
Could be, but I think it's again the case of 'me explaining what I want badly'. :-)
See below for more details."adrian@jboss.org" wrote:
Isn't it obvious that PropertyInfo is the contract and AbstractPropertyInfo
is an implementation detail?
Really?
I thought I was missing some conclusion after 2 years of work on MC? :-)"adrian@jboss.org" wrote:
There should be some refactoring like;public abstract class AbstractPropertyInfo implements PropertyInfo { String name } public class StandardPropertyInfo extends AbstractPropertyInfo { MethodInfo getter, MethodInfo setter } public class FieldPropertyInfo extends AbstractPropertyInfo { FieldInfo field }
Already got exactly all this in place. ;-)
OK, let me try once more to explain what I see as an issue.
...
Eh, during thinking what I wanted to ask/write, I understood what I really needed to do.
So, yes, we can call my previous post again a bad explanation. Since I see I couldn't explain it even to myself at that time. :-) -
24. Re: Field injection
adrian.brock Mar 14, 2008 11:58 AM (in response to alesj)"alesj" wrote:
"adrian@jboss.org" wrote:
I know you're probably scared of me telling you did it wrong after the fact,
but now I'll tell you that you are asking a stupid question before the fact. :-)
Could be, but I think it's again the case of 'me explaining what I want badly'. :-)
See below for more details.
...
Eh, during thinking what I wanted to ask/write, I understood what I really needed to do.
So, yes, we can call my previous post again a bad explanation. Since I see I couldn't explain it even to myself at that time. :-)
LOL Cardboard programming at its best :-)
http://c2.com/cgi/wiki?CardboardProgrammer -
25. Re: Field injection
alesj Mar 14, 2008 12:05 PM (in response to alesj)"adrian@jboss.org" wrote:
LOL Cardboard programming at its best :-)
Yeah, I didn't even bother to delete previous writings.
Got annoyed with myself for stupidity, so I though public 'cardboard shame' is in place. :-) -
26. Re: Field injection
alesj Mar 15, 2008 5:27 PM (in response to alesj)I've commited the initial impl.
Also added JIRA issue to follow:
- http://jira.jboss.com/jira/browse/JBREFLECT-22 -
27. Re: Field injection
adrian.brock Mar 17, 2008 9:11 AM (in response to alesj)This is a security hole:
/** * Set the field * * @param field the field */ public void setField(Field field) { this.field = field; if (isPublic() == false && field != null) field.setAccessible(true); } /** * Get the field * * @return the field */ public Field getField() { return field; } =
e.g.java.lang.reflect.Field field = ...; // I can't do setAccesible ReflectFieldInfoImpl impl = new ReflectionFieldInfoImpl(); impl.setField(); // So I'll use this hole field = impl.getField(); // This should have an access check
-
28. Re: Field injection
adrian.brock Mar 17, 2008 9:15 AM (in response to alesj)Also I don't see a test in the kernel project that is validating
that you can't use the xml deployment (or programmatic deployment)
to bypass the private field access.
e.g. See the AccessControlContextTestCase that validates
that somebody can't use the MC to get access the system properties if they
don't have the right to do so. -
29. Re: Field injection
adrian.brock Mar 17, 2008 9:19 AM (in response to alesj)"adrian@jboss.org" wrote:
public void setField(Field field)
{
this.field = field;
if (isPublic() == false && field != null)
field.setAccessible(true);
}
Actually its not a security hole, because you have done it properly.
The setAccessible will fail depending upon the who the caller is.