Incorrect JiBX binding for custom marshaller / unmarshallers

I'm not able to investigate the problem with the supplied Maven build. If it's possible for you to supply an Ant build to illustrate the problem I can try it out. Alternatively, try it out with the 1.2.2 release (out very soon) and see if that works - I have fixed a couple of issues around the handling of custom marshaller/unmarshallers.

I think I'm seeing the same or a similar issue. I'll try to put together a simple ant build project to demonstrate this here in a bit. But basically, from what I'm seeing, the first structure that declares its unmarshaller as a custom unmarshalling class forever has its element name tied to that unmarshaller. It doesn't matter what structure after the first one in the binding file also declares that class as its unmarshaller, that first name is all that's ever passed to the unmarshaller's constructer. For example:

<mapping class="custom.Request" name="Request" >
<structure name="Interest" unmarshaller="custom.Marshalling" field="interest" usage="optional" />
<value name="InterestAssumptionRate" field="interestAssumptionRate" usage="optional" />
<structure name="Mortality" unmarshaller="custom.Marshalling" field="mortality" usage="optional" />
</mapping>

The first structure is unmarshalled correctly. The next value is unmarshalled correctly, but the third structure, when the custom marshaller is instantiated, it's m_name is "Interest" instead of "Mortality". So the isAt() method fails, JiBX thinks it has moved on, but it hasn't. It looks for the next element, but it's still at <Mortality> so it keeps failing down the rest of the elements until it reaches the end of the mapping. Then it just fails with an error of the form "Expected </Request> end tag, but found <Mortality>".

If this isn't the same issue, I can create a separate one, but it seems to fit here without delving too much into the attached maven project.

Or it could just be that the same instance of the custom unmarshaller/marshaller is just being reused instead of a new instance being created each time?

I've attached a sample project with an ant build that I believe is the same issue. If it isn't, I'd happily create a separate one.

I've been looking at this for the better part of the day since it has my project stuck at the moment, so I figured I'd share what I've found so far.

The following method:

org.jibx.binding.def.BindingDefinition.getMarshallerUnmarshallerName(String)

properly handles the situation noted in its javadoc:

"The same class may have more than one marshaller/unmarshaller pair defined, so this uses the supplied class name as a base and appends a numeric suffix as necessary to generate a unique name."

However, the reverse can also be true. The same marshaller/unmarshaller pair can be used by more than one class.

In:

org.jibx.binding.def.DirectObject.createSubclass(boolean)

the lines:

String suffix = m_fixedName ? "" : m_mappingName.substring(m_mappingName.lastIndexOf('-')+1);
String name = pack + def.getPrefix() + tname + suffix;

define the name of the subclass generated to instantiate the custom marshaller/unmarshaller. But it uses the index provided previously by the call to getMarshallerUnmarshallerName which doesn't take into account marshallers/unmarshallers. So you end up with two different classes both building exactly the same subclass name. For the example given in the attached project, you get:

m_extraClasses =

m_extraMarshallers = [flexible.JiBX_bindingMarshalling0,flexible.JiBX_bindingMarshalling0]
m_extraUnmarshallers = [flexible.JiBX_bindingMarshalling0,flexible.JiBX_bindingMarshalling0]

But flexible.JiBX_bindingMarshalling0 only specifies a static element name. In this case you only get "These" as it doesn't write out another class of the same name.

Somehow the name of the generated class needs to take into account whether or not a subclass of that name already exists, but this information isn't shared across these two objects. A hack can make it work, but surely a more elegant method exists. For what it's worth, this only seemed to show up in the 1.2 code because it looks like this code previously didn't use names, just indexes.

I had considered adding the field name to the generated marshaller name, and this works ... sometimes. But it seems to introduce other issues as well. 8 \

Adam,

That very much sounds like the same issue I have been experiencing. Thank you for providing more information.

I have a fix for this that I'll be uploading a patch for people to use if they're interested. It's not a terribly clever or clean solution, but without input from the developers on this; I've done what I can. I'm going to clean this up today and hopefully have a patch file to attach to this issue before I go home.

If there is a simpler solution or anyone has a better design idea for this, please hop in. I'm not afforded the time to really delve into it to find the correct architectural solution.

Patch files are coming shortly.

Patch files updated. As stated, they aren't the most architecturally sound fixes, but if it helps point someone in the right direction or provides a "good enough" fix for anyone facing this issue now, I'm happy to share.

If someone does have a better fix for this or better ideas, toss 'em in.

FYI, there's still a bug in those patch files. I'm still working on getting a correct solution though until I hear otherwise.

Patch files update. Still not bullet-proof, but better at least.

Fixed by change to DirectObject to use unique class names for generated custom marshaller/unmarshaller subclasses. Thanks for your work in helping isolate the problem, Adam - it was actually due to DirectObject reusing the subclass name, but you were on the right track in finding it was a marshaller/unmarshaller class name issue. Thanks, too, for supplying a test case that allowed me to see the problem in action.