Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XStreamMarshaller - no way to set a MapperWrapper on XStream [SPR-10421] #15054

Closed
spring-issuemaster opened this issue Mar 27, 2013 · 8 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Mar 27, 2013

Robert Thaler opened SPR-10421 and commented

We need a HibernateMapper for XStream to support lazy/proxy entities.
Before Spring 3.2.2 we have subclassed XStreamMarshaller and overwritten getXStream to provide an appropriate instance of XStream.
Since 3.2.2 XStreamMarshaller.getXStream is declared as final. The only way to set a MapperWrapper is to force access on the attribute 'xstream' via reflection.

In my opinion XStreamMarshaller should provide an API method either to set MapperWrappers or to create a XStream instance.

kind regards
robert


Affects: 3.2.2

Issue Links:

  • #16258 XStreamMarshaller forces XPP dependency
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 23, 2013

Jason Klapste commented

We also have overridden getXStream() to create an XStreamDataHolder instead of an XStream and the introduction of final on the getXStream() method prevent this.

This is preventing us from staying current with Spring (which we've been doing since 3.0) and for us this is a high priority item.

Changelog: https://fisheye.springsource.org/changelog/spring-framework?cs=9e9cdf5f1312a1d80b2809e68c6ce7883c0a3d8d

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 5, 2013

Juergen Hoeller commented

For Spring Framework 4.0, I've reworked XStreamMarshaller's XStream setup: We're exposing all of XStream 1.4's configurable strategies as XStreamMarshaller bean properties now (including "mapper", "converterLookup" and "marshallingStrategy"). We also have a buildXStream template method that is being called after all bean properties have been set (i.e. we're not constructing the XStream instance early anymore, in order to influence its constructor arguments up until all bean properties have been set). The getXStream() method is still final since it just exposes the fully configured XStream instance now; overriding it to create a different XStream instance would be pointless in that context.

Since Spring Framework 3.2.x needs to remain compatible with XStream 1.3 and should also keep constructing the XStream instance early for the time being, I've simply restored the old getXStream() behavior of it being non-final and all of XStreamMarshaller's methods delegating to it. So you should be able to keep doing your pre-3.2.3 customizations against 3.2.4 now. However, note that you'll need to rework them for 4.0 - hopefully being able to apply any kind of customization in a cleaner way then.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 5, 2013

Juergen Hoeller commented

In addition to the above, I'm still considering a first-class way to specify MapperWrappers in 4.0...

One option would be to have a "Class[] mapperWrappers" property that we construct and apply one after the other, expecting a constructor with a Mapper or MapperWrapper argument on each. We would need our own internal XStream subclass for that purpose, overriding "wrapMapper" accordingly.

Another option would be to have a "constructXStream" method that is being called just for constructor invocation, with all other settings still getting applied in a regular fashion afterwards - in contrast to "buildXStream" which combines all of that stuff into one big template method.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 5, 2013

Juergen Hoeller commented

I've introduced a "mapperWrappers" bean property now, as suggested above.

If there are any remaining gaps in what you'd like to do in terms of XStream customizations, let me know. Ideally, Spring Framework 4.0's XStreamMarshaller should provide all relevant hooks and escape hatches out of the box.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 7, 2013

Jason Klapste commented

Thanks Juergen!

One comment looking at the 4.x branch. The new buildXStream() method won't work for us-- we're not using XStream() directly, we're subclassing it (hence my previous comment about "XStreamDataHolder"). Not sure the best way to fix this-- maybe a template method that just constructs and returns a XStream object and then buildXStream() finishes the setup? We'd still have do the MapperWrapper work on our override but it's a heck of a lot less then the whole buildXSteam().

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 7, 2013

Juergen Hoeller commented

Jason,

So what exactly are you doing on your custom XStream subclass? Is there something we may provide as an out-of-the-box feature?

Generally speaking, we can certainly introduce a constructXStream template method in addition to our existing options. I'd just like to minimize the actual need for that method.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 7, 2013

Jason Klapste commented

XStream's marshal and unmarshal methods have a variant that allows you to pass in a DataHolder object that can be used by the converters. We've added toXML() and fromXML() methods that accept a DataHolder as a parameter and convert the to/fromXML()'s source/dest parameter into the stream reader and writer as necessary. For example:

public Object fromXML(InputStream input, DataHolder holder) {
  return unmarshal(driver.createReader(input), null, holder);
}

public void toXML(Object obj, OutputStream out, DataHolder holder) {
  marshal(obj, driver.createWriter(out), holder);
}

Given that we can set the stream driver on XStreamMarshaller, the addition of a DataHolder parameter to methods such as marshalOutputStream() or unmarshalInputStream() would eliminate the need for us to subclass XStream.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 9, 2013

Juergen Hoeller commented

I've added public marshal/unmarshal methods with Input/OutputStreams and Reader/Writers, with and without a DataHolder parameter. Have a look and let me know whether those would meet your needs.

Additionally, buildXStream delegates to separate constructXStream and configureXStream methods now.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.