Hi @Ganesh Gautam,

Thank you for your reply. I have created a separate suggestion as per your recommendation CONFSERVER-60867.

Regards,

Ahmed

Hi ahmed.alaghbari

Thanks for using the new API and giving this amazing feedback. I am happy to see your app already taking advantage of our backward compatibility layer on new XStream.

Lastly, I would recommend you to raise a separate suggestion, should you need API for mapper in future.

Regards,

Ganesh

7.10.0 release has shipped with this change.

David, we are happy to convey that confluence-compat-lib 1.4.1 has been released with things discussed above. You can find full documentation for the same at the original XStream 1.4 upgrade doc.

Thanks,

Ganesh

Ganesh, really thanks again. It's been a pleasure to discuss with you about this .

I see the new version of the compat lib is out, so we will start applying the changes to our apps .

Regards,

David

16fa4887a4db

Right, we don't have XStream 1.1.1 dependency anymore in Confluence 7.10 rather just XStream 1.4.x. And it looks you have answered your question in next comment:

calling to the marshal/unmarshal methods from both XStreamManagerCompat or directly ConfluenceXStreamManager will be safe against those errors.

This is possible because we have introduced a layer in Confluence to handle the serialised formats from 1.1.1 through XStream 1.4.x using our custom wrappers. It doesn't mean we have XStream 1.1.1, rather we can handle 1.1.1 format  

Thanks,

Ganesh

And one more question, sorry. Just to be sure and avoid possible future errors with our users. As the jump to 1.4.x version introduced some errors while unmarshalling data generated with 1.1.1, we understand the implementation you use in Confluence 7.10 has some mechanisms to avoid those errors. And therefore calling to the marshal/unmarshal methods from both XStreamManagerCompat or directly ConfluenceXStreamManager will be safe against those errors.

And also, when 7.9 comes to EOL, is it planned to also deprecate the compat lib?

Kind regards!

Thanks again! We would go that way then . Once we see the new compat version is ready, we will implement the changes.

One more question just to solve my technical/implementation curiosity

I thought the internal dependencies of Confluence would change in 7.10 for using 1.4.x version of XStream. So then, the getXStream() method would retrieve an instance version of the XStream corresponding to the Confluence version. I mean, prior 7.10 it would always be an instance from 1.1.1. And in Confluence 7.10 onwards, the instance would be from 1.4.x (although the real object would not be the pure thoughtworks one but your implementation/wrapper).

However, after your last comment and looking a bit to your internal interface it seems I was wrong and you're doing something different. Because if the getXStream() will always return a 1.1.1 version, that means Confluence 7.10 is still keeping 1.1.1 as internal dependency or retrieved with other OSGI bundle loader to not clash with the new 1.4.x lib version. 

You don't have to answer me  because I'm entering into your implementation details and I know that it's "not my business". But as a tech guy, I couldn't resist my willing to know .

Kind regards!!

David,

The 4 changes you have mentioned are right. Let me also clarify the comment on Namespace converter. 
As per me, you can just do the following: 

public void doSomething() {
    confluenceXStreamCompat.getXStream().registerConverter(nameSpaceCoverter)
}

Since the confluenceXStreamCompat.getXStream() returns XStream type and that too for only 1.1.1, it will register your converter only on 1.1.1 XStream and not on 1.4.x XStream. You can then remove this Namespace converter tech-debt once 7.9 is out of life. Feel free to put a deprecation notice in your plugin's codebase.  
For all other converters which are not workaround of something caused by XStream bugs, you can directly register with two new methods we are introducing in ConfluenceXStreamCompat. These two methods will register your converter on both versions of XStream in underlying layer.

Thanks for highlighting the typo , it was indeed 7.10, I have corrected my previous comment.

Many thanks,

Ganesh

Hi again Ganesh!

Really good news!!! This is a sample of how we're using the alias and converters:

xStream.alias("Manifest", PublishManifest.class);
xStream.registerConverter(new CustomXstreamDateConverter());

We're happy with the two new methods you will provide in the XStreamManagerCompat library. For register the converter we will call the method passing 0 as the second argument (because this is what is doing internally the single argument method).

So, for making sure we're both aligned, these are the only changes we would have to make in our apps:

1. Use the confluence-compat-lib version 1.4.1 (or the version that includes the new methods)
2. Inject/Instantiate the XStreamManagerCompat where we need to marshal/unmarshal xml
3. if required, register specific converters and aliases using the new methods. 
4. Then use fromXml//toXml methods as usual.

But I would like to clarify my comment about checking which Confluence version the app is running on. I was referring to the app where we are using the converter for the Namespace. I guess defining a Namespace converter from Confluence 7.10 and onwards is not a desired thing. Because in those versions, the ConfluenceXStream already know how to deal with it.

Unless you tell me there is no worries to keep using in Confluence 7.10 the Namespace converter I wrote in the Jira issue description, I fear we will have to add the Confluence version checking just to avoid register the converter.

Apart from that, I would also like to confirm the following. You said:

o also register converters on latest XStream in 7.11 and do things efficiently, please see my next point

I hope the 7.11 mention is a typo and you were referring to 7.10 .

Really thanks again. Regards,

David Herrera

Hi David,
Thanks for quick analysis and reply.

I would like to highlight few things:

1. You don't need to check for confluence version, compat library will take care of wiring for you. You just need to call getXStream() and then wire your converter/alias. But yes there is an issue here, that XStream returned from this method is meant for 1.1.1 XStream, and in Confluence 7.10 it is our version of XStream only for backward compatibility.
   To also register converters on latest XStream in 7.10 and do things efficiently, please see my next point.
2. Since you have mentioned that you already have converter/aliases defined and there is no going-back. So, to ease out plugin development, we can look at providing some part of our internal interface in compat lib, please see the two methods mentioned in below code block. This is possible because each plugin calling compat lib or ConfluenceXStreamManager.getPluginXStream() gets its own ConfluenceXStream instance and hence converters/aliases won't conflict across plugins:
   XStreamManagerCompat.java

   void registerConverter(Converter converter, Integer integer);
   void alias(String alias, Class aliasClass);
   

Can you please give an example of your alias registration and confirm that above interface suffices you? Gladly the above things already are part of our new Confluence XStream infra in 7.10 and we can happily wire it up for backward compatibility(Confluence 7.9/prior and XStream data 1.1.1 format both) in Compatibility lib.

Coming to your questions, please find answers inline:

Is there any doco, changelog, etc of the compatibility library? 

     GG- We currently have all XStream related docs for compat lib mentioned in XStream 1.4 upgrade. We also plan to bring a changelog document for compat-lib soon on DAC.

How we would be informed when the new library version is released?

    GG - We have the PR ready and would cut a compat lib patch version tomorrow if the above interface suffices you. You can track this issue for combat-lib version or XStream 1.4 upgrade itself.

Regarding the final release of Confluence 7.10. Do you have an estimation date?

     GG - Release candidate(RC) was out on 8 Dec as per  Preparing for Confluence 7.10 document, the final release is generally release ~1 week post RC.

Thanks,
Ganesh

Thanks very much Ganesh.

With your detailed answer we know how to proceed with the Namespace issue. We would have to add specific code in our app checking the version of Confluence:

1. Versions prior to 7.10 will use the upcoming getXStream() so we can use required converters/aliases.
2. Versions from 7.10 onwards, will just use the from/to XML methods from the compatibility library

However, we have other scenarios where we are using aliases and converters for other classes (not Confluence ones). And in those cases, previous approach will not work. It's true that for versions prior to 7.10 we will be safe, but how about from Confluence 7.10 and onwards? As we cannot (or shouldn't) use alias nor converters, the marshal/unmarshal will fail in these cases. Do you have any solution/idea/suggestion?

Apart from this, I had some more questions:

• Is there any doco, changelog, etc of the compatibility library? 
• How we would be informed when the new library version is released?
• Regarding the final release of Confluence 7.10. Do you have an estimation date?

Thanks again Ganesh. Kind regards,

David Herrera,

Comalatech

Hi David, 

Many thanks for reporting your feedback and providing a detailed description. We have few points to talk about:

1. We have written a test(see code block below) on Confluence 7.10 with Namespace and would like to confirm that this issue does not exist in XStream 1.4.14. We have created a unit test which works fine on Confluence 7.10 binaries without any custom converter:
   The above test works fine on Confluence 7.10+. The SerializableConverter in XStream 1.4.14 has got a lot of fixes since 1.1.1 and can handle Serializable types properly now. Since Namespace is a Serializable  java type, it shouldn't pose any issue while serialization ideally. You can see the code diff for SerializableConverter here.
   XStreamManagerCompatTest.java

   @Test
   public void testNameSpaceMarshalling() throws Exception{
       Namespace personalNameSpace = Namespace.PERSONAL;
       String marshalledText = xStream.toXML(personalNameSpace);
       String expectedXML = "<com.atlassian.confluence.labels.Namespace>\n" +
           " <namespacePrefix>my</namespacePrefix>\n" +
           " <visibility>owner</visibility>\n" +
           "</com.atlassian.confluence.labels.Namespace>";
       assertXMLEqual(marshalledText, expectedXML);
    }
   

1. Coming to the reported issue, we were able to reproduce the mentioned problem with Confluence 7.9 and prior using the above test & compat library. The stack trace obtained is same as reported in this issue. 
   The stack trace mentioned causes ObjectAccessException for Namespace during serialization and it is caused while using XStream111Compat. This XStream111Compat class is responsible only for 1.1.1 compatibility internally in compat library.
   Effectively a feature is needed from XStream111Compat to either allow converter registration or expose XStream 1.1.1 equivalent implementation. We are planning to do the latter. We plan to release the lib early next week. You can expect following method in XStreamManagerCompat:
   XStreamManagerCompat.java

   /**
    * Provides way to access 1.1.1 XStream object
    * Supposed to be removed when Confluence 7.9 is EOL.
    *
    * @return XStream 1.1.1 equivalent object.
    *
    */
   public XStream getXStream() {
       return delegate.getXStream();
   }
   

We have looked at your questions and would like to answer them too:

 Are you plan to resolve this adding some way to customize the internal XStream? 

     GG - Our view is to not allow custom converters and aliases because of following reasons:

• Converters/aliases can interfere between plugins if they register converter for same thing, it can cause conflict.
• Due to security reason, we would want XStream to be tightly managed.
  Rather, we would want to improve our ConfluenceXStream API to empower plugins/vendors. XStream 1.4.x upgrade helps to solve a lot of shortcomings in previous versions already.

Or in this case as the error is related to your internal Label model, maybe you can provide XStreamManagerCompat implementation already being compatible to marshall/unmarshall Confluence model.

     GG - This is indeed a good idea, and Confluence 7.10 achieves a lot for the same. But, we would not be providing this converter out of the box as other plugins might be registering their converter in past and things could conflict. Since XStream111Compat initializes a new XStream in 7.9 or prior, please feel free to register your own for backward compatibility. An example of variations could be:

• XML generated from converter mentioned in this issue:

  <com.atlassian.confluence.labels.Namespace>
       <namespacePrefix>my</namespacePrefix>
   </com.atlassian.confluence.labels.Namespace>
  

• XML generated from XStream 1.4.14

  <com.atlassian.confluence.labels.Namespace>
      <namespacePrefix>my</namespacePrefix>
      <visibility>owner</visibility>
  </com.atlassian.confluence.labels.Namespace>
  

  Note: <visibility>owner</visibility> line#2 in second XML which was missing in first XML, though it doesn't break deserialization in either versions, but gives a fair example on why we need to be cautious with XStream.

Please feel to let us know if you have any further questions.

Thanks,
Ganesh Gautam
Confluence

Please could you change the type of the issue to Bug? The link to create issue from 7.10 EAP filled it with "Suggestion" but it isn't.

And additional note to the ticket description. This is not only a problem about the Exception throw trying to marshall/unmarshall the Label object, it's also about how to deal now in those cases where we're using converters or even aliases over the XStream object. 

Sorry I created this issue without a proper title... It should have been: Confluence 7.10 XStream error while saving Label