[weld-dev] superclass in WeldClassImpl

[Marko Lukša]

OK, after talking with Pete, I went ahead and changed 
Beans.getFieldInjectionPoints in such a way that it now only uses the 
fields returned by AnnotatedType.getFields() instead of traversing up 
the class hierarchy (through WeldClass.getWeldSuperclass) and using 
WeldClass.getDeclaredWeldFields.

But since fields declared higher up in the class hierarchy need to be 
injected before those declared lower in the hierarchy, I need to sort 
the fields according to where they are declared. I do this in 
Beans.injectFieldsAndInitializers, which could be problematic 
performance-wise. I could move the sorting to the initialization of 
AbstractClassBean, but the sorting would be much more involved (in 
injectFieldsAndInitializers, I have the object instance that is being 
injected into, and I can simply use its class hierarchy, whereas if I 
move the sorting to the init of AbstractClassBean, I will not have a 
complete picture of the whole hierarchy - I would need to deduce the 
class hierarchy from field.getJavaMember().getDeclaringClass() and 
initializerMethod.getJM().getDeclaringClass(). Plus, I would need to 
sort both fields and initializers at the same time, since they need to 
be injected/invoked together.

Anyway, I really don't feel comfortable issuing a pull request for this 
without a detailed review by at least some of you guys.

Also, I have a bad feeling that this still doesn't solve all the 
problems regarding WeldClass.superclass = Object.class. The superclass 
is also used in a number of other places and will probably still cause 
problems at some of them. I haven't looked into that yet, but I can do 
it after we review/discuss my commit.

The commit is at 
https://github.com/luksa/weld-core/commit/0549a041544689c2122f18d1691226a3a7f80458

Marko



On 19.7.2012 23:39, Marko Lukša wrote:
> Hmm, actually the example Extension in WELD-1144 does return an 
> AnnotatedType which returns *all* the fields (even the superclass' 
> fields) in getFields().
>
> The problem is that Beans.getFieldInjectionPoints() only looks at 
> *declared* fields in WeldClass and then calls 
> WeldClass.getSuperClass() and looks at its declared fields (in this 
> case, the superclass is Object, so Weld actually only sees the actual 
> class' fields).
>
> So if we can't change WeldClass.superclass, we should change 
> Beans.getFieldInjectionPoints so it takes into account all the fields 
> returned by WeldClass.getFields(). Of course these would need to be 
> partitioned (as getFieldInjectionPoints() actually returns a 
> List<Set<FieldInjectionPoint>> - a Set of field injection points for 
> every class in the hierarchy).
>
> WDYT?
>
>
> On 19.7.2012 23:18, Stuart Douglas wrote:
>> The assumption is that if an annotated type is registered via an 
>> extension it must also include all superclass fields as well.
>>
>> This gives much more flexibility to extension authors, otherwise it 
>> is basically impossible to modify superclass annotations.
>>
>> Stuart
>>
>>
>> On 20/07/2012, at 12:10 AM, Marko Lukša wrote:
>>
>>> Hey guys.
>>>
>>> Anyone know why WeldClassImpl sets superclass to Object.class for some
>>> classes? This causes weld not to inject fields, declared in
>>> superclasses, where the annotatedType is supplied by an extension. (OK,
>>> not in all cases, only in cases where the annotatedType is detected as
>>> being modified). See https://issues.jboss.org/browse/WELD-1144
>>>
>>> Take a look at
>>> https://github.com/weld/core/blob/master/impl/src/main/java/org/jboss/weld/introspector/jlr/WeldClassImpl.java#L137 
>>>
>>>
>>> I'm thinking of changing this to something along the lines of:
>>>
>>>      this.superclass =
>>> classTransformer.loadClass(annotatedType.getJavaClass().getSuperclass()); 
>>>
>>>
>>> This change does break two tests (SuperTypeTest and
>>> SuperclassModifiedTest), but IMO for these two tests to pass, we should
>>> change Beans.getFieldInjectionPoints() so it returns only the fields
>>> that were returned by AnnotatedType.getFields().
>>>
>>> Is there any other reason why WeldClassImpl would need to set
>>> this.superclass to Object.class?
>>>
>>> Marko
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> weld-dev mailing list
>>> weld-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>