Compatibility with 3rd party tools

[arittner]

Hello!

I noticed today that the IntelliJ IDEA plugin for PlantUML no longer works with the latest snapshot.

Regardless that this needs to be clarified with the plugin developers, the question is how to deal with such changes? I guess many tools around PlantUML are developed and modifications on public accessors (this includes class names) will have an impact.

Here is the detailed issue from PlantUML plugin: https://github.com/esteinberg/plantuml4idea/

java.lang.NoSuchMethodError: 'net.sourceforge.plantuml.cucadiagram.DisplayPositionned net.sourceforge.plantuml.error.PSystemError.getTitle()'
	at org.plantuml.idea.adapter.rendering.Titles.initTitles(Titles.java:54)
	at org.plantuml.idea.adapter.rendering.Titles.<init>(Titles.java:26)
	at org.plantuml.idea.adapter.rendering.MyBlock.<init>(MyBlock.java:28)
	at org.plantuml.idea.adapter.rendering.DiagramFactory.create(DiagramFactory.java:60)
	at org.plantuml.idea.adapter.rendering.DiagramFactory.create(DiagramFactory.java:48)
	at org.plantuml.idea.adapter.rendering.PlantUmlNormalRenderer.doRender(PlantUmlNormalRenderer.java:20)
	at org.plantuml.idea.adapter.rendering.PlantUmlRendererUtil.render(PlantUmlRendererUtil.java:52)
	at org.plantuml.idea.adapter.FacadeImpl.render(FacadeImpl.java:83)
	at org.plantuml.idea.rendering.RenderCommand.render(RenderCommand.java:143)
	at org.plantuml.idea.rendering.LazyApplicationPoolExecutor$MyRunnable.run(LazyApplicationPoolExecutor.java:130)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

This line of code:

https://github.com/esteinberg/plantuml4idea/blob/master/adapter/src/org/plantuml/idea/adapter/rendering/Titles.java#L54

But I also faced an issue on this line:

https://github.com/esteinberg/plantuml4idea/blob/master/adapter/src/org/plantuml/idea/adapter/rendering/Titles.java#L51

I guess this was a typo change to the new name DisplayPositioned (from DisplayPositionned).

[arittner]

FYI: I created an issue to inform the plugin owners: esteinberg/plantuml4idea#337

[arnaudroques]

Regardless that this needs to be clarified with the plugin developers, the question is how to deal with such changes? I guess many tools around PlantUML are developed and modifications on public accessors (this includes class names) will have an impact.

Yes, that's a real issue.
The main problem is due to the fact that our API for Java 3rd party tools is not clear.
We should probably propose an official and stable Java API for PlantUML but we need help here.

In the meantime, I propose the following change to make IntelliJ IDEA plugin working with latest snapshot. What do you think about it ?

[arittner]

In the meantime, I propose the following change to make IntelliJ IDEA plugin working with latest snapshot. What do you think about it ?

IMHO this will fix only source code compatibility, but not binary compatibility. This means, the source will build, but an already compiled Pluging with a configured plant UML JAR to render will still fail.

I checked it now (I created a JAR from your branch and configured it for the plugin), but the exception is not gone. Double-checked it with an older JAR (then it's working). I hope I didn't make a mistake now with my tests, but I think this only improves it if you make a new build as a user of the API. And then you can correct it right away.

[arnaudroques]

...but the exception is not gone

You're right, it's not that simple.
We must also change some method definitions from:

public DisplayPositioned getTitle();

to

public DisplayPositionned getTitle();

to ensure binary compatibility.

Let me do this carefully so that you can test again: I think that we can achieve binary compatibility (but not 100% sure now :-) )

[matthew16550]

Just a guess, an emptyDisplayPositionned class that extends DisplayPositioned might solve it.

[arnaudroques]

@matthew16550 Yes, it would also work. But we still have to keep the old definition public DisplayPositionned getTitle(); because that's what the IntelliJ IDEA plugin is actually expecting.

@arittner Could you try with last snapshot? If it does not work, could you post the full stacktrace of the error. Thanks!

[arittner]

Hi @arnaudroques

If it does not work, could you post the full stacktrace of the error. Thanks!

No, didn't work. sorry :-/

The stacktrace:

java.lang.IncompatibleClassChangeError: Found interface net.sourceforge.plantuml.cucadiagram.DisplayPositionned, but class was expected
	at org.plantuml.idea.adapter.rendering.Titles.initTitles(Titles.java:58)
	at org.plantuml.idea.adapter.rendering.Titles.<init>(Titles.java:26)
	at org.plantuml.idea.adapter.rendering.MyBlock.<init>(MyBlock.java:28)
	at org.plantuml.idea.adapter.rendering.DiagramFactory.create(DiagramFactory.java:60)
	at org.plantuml.idea.adapter.rendering.DiagramFactory.create(DiagramFactory.java:48)
	at org.plantuml.idea.adapter.rendering.PlantUmlNormalRenderer.doRender(PlantUmlNormalRenderer.java:20)
	at org.plantuml.idea.adapter.rendering.PlantUmlRendererUtil.render(PlantUmlRendererUtil.java:52)
	at org.plantuml.idea.adapter.FacadeImpl.render(FacadeImpl.java:83)
	at org.plantuml.idea.rendering.RenderCommand.render(RenderCommand.java:143)
	at org.plantuml.idea.rendering.LazyApplicationPoolExecutor$MyRunnable.run(LazyApplicationPoolExecutor.java:130)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

Still a binary compatibility issue.

I guess, we can manage this with the plugin owners. But I believe we'll have a lot of other projects with dependency on the PlantUML artifact.

The question is, how to handle it? Announce an incompatible change? Revert and announce it for a future version (and maybe add more refactorings with incompatible changes)? Or should we live with the typo?

[arnaudroques]

Or should we live with the typo?

Maybe...

I've just commit a new change. Could you try again ? Thanks!

[arittner]

Hi @arnaudroques

It's working :-) - Your last commit did it.