Issues with composite components

[Curtiss Howard <cu...@gmail.com>]

Hi,

I'm working on getting Sun's basic-ezcomp demo working, and it makes
extensive use of composite components.  Most things are working, but I
noticed two issues, one of which I fixed:

First, CompositeComponentResourceTagHandler.createComponent() was
incorrectly checking for required attributes.  Basically, it was only
seeing if the BeanInfo's PropertyDescriptors contained a "required"
attribute and threw an error if so.  I fixed this to check for the
"required" attribute AND to see if the tag attribute exists or not,
which makes more sense.  I have not yet committed this change because
of the next problem.

The second issue I saw, and much more pressing, is that the population
of the BeanInfo and its PropertyDescriptors is "flat" and does not
take into account that composite component attributes can contain
other attributes.  For instance, look at the following example from
basic-ezcomp:

<cc:interface name="double" ...>
   <cc:attribute name="controller" required="true">
        <cc:attribute name="number1" required="true"/>
        <cc:attribute name="number2" required="true"/>
   </cc:attribute>
</cc:interface>

In this case, the "controller" attribute is supposed to reference a
bean with "number1" and "number2" properties.  However, like I
mentioned, when the list of PropertyDescriptors is populated for the
composite component it is "flat", so when validating the tag it's
expected to contain "controller", "number1", and "number2" attributes.
 This is obviously incorrect, as only the "controller" attribute is
truly required from the perspective of the person using the composite
component.

Now, I suppose there are two ways to address this.  The first would
involve creating a non-flat BeanInfo that would look something like
this:

BeanInfo(double):
  PropertyDescriptor(controller) => BeanInfo:
    PropertyDescriptor(number1)
    PropertyDescriptor(number2)

This would be quite complicated to do because of the way Facelets tags
are visited (at least, that's what I gather from a cursory glance).
Also, this would introduce complexity in
CompositeComponentResourceTagHandler.createComponent().

The second approach would be to simply ignore the nested <attribute>
tags.  I think this is the correct thing to do.  After all, the user
of the composite component is only concerned with the top-level
attributes and in this case, if the bean listed for "controller"
doesn't have "number1" or "number2" properties, the composite
component implementation won't work anyway (we'd throw an EL
exception).

Please let me know your thoughts.

Thanks,


Curtiss Howard

Re: Issues with composite components

[Leonardo Uribe <lu...@gmail.com>]

2009/8/27 Curtiss Howard <cu...@gmail.com>

> Leonardo,
>
> Do you want me to contribute these changes or would you rather address
> them yourself?
>

Hi

If you can, contribute with these changes. I read again the code and it is a
misunderstanding.

regards

Leonardo Uribe

>
> Thanks,
>
>
> Curtiss Howard
>

Re: Issues with composite components

[Curtiss Howard <cu...@gmail.com>]

Leonardo,

Do you want me to contribute these changes or would you rather address
them yourself?

Thanks,


Curtiss Howard

Re: Issues with composite components

[Curtiss Howard <cu...@gmail.com>]

> I believe in facelets is valid assign an attribute to a component that "does
> not exist" or in other words "not declared on the class". I'm not very sure
> about do that, since the default behavior for normal components should be
> preserved for composite components.
>

I guess I'm not sure what you mean by this.  Basically the code was
only checking to see if the composite component interface defined an
attribute as "required" and throwing an exception no matter what.  I
simply modified it to see if the attribute is "required" and throw an
exception if the tag doesn't declare it.  I think that's the right
thing to do.

At any rate, without that change in place the sample fails with
MyFaces but runs in Mojarra.

Thanks,


Curtiss Howard

Re: Issues with composite components

[Leonardo Uribe <lu...@gmail.com>]

Hi

2009/8/27 Curtiss Howard <cu...@gmail.com>

> Hi,
>
> I'm working on getting Sun's basic-ezcomp demo working, and it makes
> extensive use of composite components.  Most things are working, but I
> noticed two issues, one of which I fixed:
>
> First, CompositeComponentResourceTagHandler.createComponent() was
> incorrectly checking for required attributes.  Basically, it was only
> seeing if the BeanInfo's PropertyDescriptors contained a "required"
> attribute and threw an error if so.  I fixed this to check for the
> "required" attribute AND to see if the tag attribute exists or not,
> which makes more sense.  I have not yet committed this change because
> of the next problem.


I believe in facelets is valid assign an attribute to a component that "does
not exist" or in other words "not declared on the class". I'm not very sure
about do that, since the default behavior for normal components should be
preserved for composite components.


>
>
> The second issue I saw, and much more pressing, is that the population
> of the BeanInfo and its PropertyDescriptors is "flat" and does not
> take into account that composite component attributes can contain
> other attributes.  For instance, look at the following example from
> basic-ezcomp:
>
> <cc:interface name="double" ...>
>   <cc:attribute name="controller" required="true">
>        <cc:attribute name="number1" required="true"/>
>        <cc:attribute name="number2" required="true"/>
>   </cc:attribute>
> </cc:interface>
>
> In this case, the "controller" attribute is supposed to reference a
> bean with "number1" and "number2" properties.  However, like I
> mentioned, when the list of PropertyDescriptors is populated for the
> composite component it is "flat", so when validating the tag it's
> expected to contain "controller", "number1", and "number2" attributes.
>  This is obviously incorrect, as only the "controller" attribute is
> truly required from the perspective of the person using the composite
> component.
>
> Now, I suppose there are two ways to address this.  The first would
> involve creating a non-flat BeanInfo that would look something like
> this:
>
> BeanInfo(double):
>  PropertyDescriptor(controller) => BeanInfo:
>    PropertyDescriptor(number1)
>    PropertyDescriptor(number2)
>
> This would be quite complicated to do because of the way Facelets tags
> are visited (at least, that's what I gather from a cursory glance).
> Also, this would introduce complexity in
> CompositeComponentResourceTagHandler.createComponent().
>
> The second approach would be to simply ignore the nested <attribute>
> tags.  I think this is the correct thing to do.  After all, the user
> of the composite component is only concerned with the top-level
> attributes and in this case, if the bean listed for "controller"
> doesn't have "number1" or "number2" properties, the composite
> component implementation won't work anyway (we'd throw an EL
> exception).
>

+1 to ignore inner attribute definitions. It seems to be declarative only
(no effects on any programming). For now the code takes them into account
but looking in deep it should not.

I'm working right now on finish composite component stuff, but I'm still
some pending issues so I'll commit all that code at the end of the week.

regards

Leonardo Uribe


>
> Please let me know your thoughts.
>
> Thanks,
>
>
> Curtiss Howard
>