Attributes problem

[adrian.brock]

I just submitted a test for attributes on Map Entry's key/value elements
because this isn't working.

See MapTestCase::testMapWithKeyValueOverrides()

Alex, can you look at it and see why its not setting the attributes?

From a bit of debugging, I can see that the problem is caused by the handler
being a BuilderParticleHandler, but I fail to see where in the builder we set this on
on a typeBinding with the exception of JBossXmlChild handlers which isn't in play here?

[aloubyansky]

Ok, I'll dig in.

[adrian.brock]

From a bit of debugging, I can see that the problem is caused by the handler
being a BuilderParticleHandler, but I fail to see where in the builder we set this on
on a typeBinding with the exception of JBossXmlChild handlers which isn't in play here?

It is this place but for a different reason.

The issue is that it is an @JBossXmlGroupText

@JBossXmlGroupText(wrapper=StringValueMetaData.class, property="value")

This correctly wraps the value (CDATA) in the StringValueMetaData but it doesn't
process the attributes because it uses a BuilderParticleHandler (BPH)
setup just before. Unlike the @JBossXMLClient there are no subelements for this case.

The thing I don't understand though is how to fix it? :-)

When I looked at when the attributes are passed to the BPH
the only information I have is that the parent is a MapEntry.

I'm obviously missing something or we are not setting up the parsing model
correctly for this case?

What it needs to do is process the attributes against the object returned by
the ValueHandler, i.e. the wrapped value.

[adrian.brock]

I'm obviously missing something or we are not setting up the parsing model
correctly for this case?

What it needs to do is process the attributes against the object returned by
the ValueHandler, i.e. the wrapped value.

Perhaps this just needs implementing in a different way?

e.g. maybe some kind of WrapperBeanAdapter
that will used the wrapper type by default, but will allow it to be replaced
by one the @JBossXmlChild types?

[aloubyansky]

The attributes don't belong to the CDATA, so it's logical that they are not set in this case. And it would even look confusing to me if we "fixed" it so that they are set.

Instead, the key should be unmarshalled to an object on which we would set its children that the attributes and the CDATA are.

[aloubyansky]

"adrian" wrote:
e.g. maybe some kind of WrapperBeanAdapter
that will used the wrapper type by default, but will allow it to be replaced
by one the @JBossXmlChild types?

I don't see a better way. The attributes are (and should be) visible only on the level of the element which declared them.

[adrian.brock]

The attributes don't belong to the CDATA, so it's logical that they are not set in this case. And it would even look confusing to me if we "fixed" it so that they are set.

Instead, the key should be unmarshalled to an object on which we would set its children that the attributes and the CDATA are.

Yes, but it is more complicated than that.
Its polymorphic and a mixed type.

The way I've done the wrapper for the CDATA is obviosly wrong. It doesn't
work if the wrapper has attributes like this case.

We should do like the old parsing did, which is to assume the wrapper
(StringValueMetaData) but then replace if we see a nested element
(e.g. ListValueMetaData).

I don't think its just a case of creating a new ParticleHandler to create the wrapper
because we don't want the ListValueMetaData injected onto the StringValueMetaData
we want to replace it if we see a subelement.

I'll look at it once I've resolved the other issues I've found.

[aloubyansky]

I thought that setting wrapper's attributes on other child elements (defined by @JBossXmlChild) would also be desired. Isn't it?
In addition it may be even simpler to implement it this way.

[adrian.brock]

I thought that setting wrapper's attributes on other child elements (defined by @JBossXmlChild) would also be desired. Isn't it?
In addition it may be even simpler to implement it this way.

Yes it is simpler, but its not what I want ;-)

I don't want people to have to write

<key><value class="x">cdata</value></key>

when they can do

<key class="x">cdata</key>

[aloubyansky]

No, that's not what I meant. Something like the following.

<key class='x'><array/></key>

Although, it may not be useful in this example.

[adrian.brock]

No, that's not what I meant. Something like the following.

<key class='x'><array/></key>

Although, it may not be useful in this example.

It doesn't do anything. The class attribute on the key is only used
to determine the type of the CDATA. The array element has its own class attribute.

[aloubyansky]

I used the example to show the idea of what can be possible/useful. Since, there is no difference in the current impl what the child is, i.e. whether it is CDATA, declared element or wildcard content. So, they can be treated the same way.

[adrian.brock]

I've fixed this by creating a WrapperBeanAdapter.

It's not very clean though. I think it shows we need better support
for being able to bind to a group and know how the group is processed.

The strategy I've used is to "steal" the BeanAdapter
(and other information) from the xml type
of the wrapping class then delegate to it from a WrapperBeanAdapter

 if (groupText != null && groupText.wrapper() != Object.class)
 {
 BeanInfo wrapperInfo = JBossXBBuilder.configuration.getBeanInfo(groupText.wrapper());
 TypeBinding wrapperTypeBinding = resolveTypeBinding(wrapperInfo.getClassInfo());
 // Steal the attributes
 Collection<AttributeBinding> otherAttributes = wrapperTypeBinding.getAttributes();
 if (otherAttributes != null)
 {
 for (AttributeBinding other : otherAttributes)
 elementTypeBinding.addAttribute(other);
 }
 ParticleHandler particleHandler = wrapperTypeBinding.getHandler();
 if (particleHandler instanceof BeanHandler == false)
 throw new IllegalStateException("Cannot wrap " + wrapperInfo.getName() + " not a bean type " + particleHandler);
 BeanHandler beanHandler = (BeanHandler) particleHandler;
 WrapperBeanAdapterFactory wrapperFactory = new WrapperBeanAdapterFactory(beanHandler.getBeanAdapterFactory(), propertyType.getType());
 elementTypeBinding.setHandler(new BeanHandler(wrapperInfo.getName(), wrapperFactory));
 elementTypeBinding.setSimpleType(wrapperTypeBinding.getSimpleType());
 }

The idea of the wrapper is that it will delegate requests to the wrapped BeanAdapter
until it spots an element from some other part of the group
then it will just return that element's object.

 @Override
 public Object getValue()
 {
 if (notWrapped != null)
 return notWrapped;
 else
 return wrapped.getValue();
 }

 @Override
 public void set(PropertyInfo propertyInfo, Object child) throws Throwable
 {
 Class<?> stopWrapping = getBeanAdapterFactory().getStopWrapping();
 if (child != null && stopWrapping != null && stopWrapping.isInstance(child))
 notWrapped = child;
 else
 wrapped.set(propertyInfo, child);
 }

Of course, it isn't really doing that. It's just using the property type
(the stopWrapping) to detect when it is another element from the group
which is obviously a kludge that could be fixed by knowing more
about the group.

[adrian.brock]

I used the example to show the idea of what can be possible/useful. Since, there is no difference in the current impl what the child is, i.e. whether it is CDATA, declared element or wildcard content. So, they can be treated the same way.

No they can't, its not a wildcard.
The @XmlChild from some other part of the group
needs to replace the CDATA. The CDATA and subelements are mutually
exclusive. You can't have

<key class="java.lang.Integer">12
 <list ...>
</key>

It doesn't make sense. :-)

[adrian.brock]

obviously a kludge that could be fixed by knowing more
about the group.

This would be a lot easier if there was someway to bind and reference model groups
and add some kind of ModelGroupAdapter to it.
But I guess that is quite complicated in general ;-)