citrusframework / citrus

Framework for automated integration tests with focus on messaging integration
https://citrusframework.org
Apache License 2.0
456 stars 134 forks source link

Consider shading on third party dependencies #683

Open svettwer opened 5 years ago

svettwer commented 5 years ago

We're currently observing issues using Citrus in projects with very old versions of common libraries. As Citrus ships newer versions as the SUT classpath provides, the version clash influences the behavior of Citrus.

E.g. Citrus allows base64 encoding using a citrus function in string literals. This for example allows on the fly creation of base64 encoded basic auth headers. As Citrus uses the Base64 implementation of apache commons, a older version of that library shipped with the SUT classpath might affect the result of that citrus functionality.

Therefore one should fixate the Citrus libraries to a specific version which cannot clash with other dependencies in the SUT classpath. One way of archiving this would be a shading of all Citrus third party dependencies. The downside would be, that Citrus has to be shipped as fat jars. So all third party dependencies (and we have a lot of those) have to be packaged with Citrus which would increase the jar size dramatically. On the other hand, even if we don't ship Citrus with dependencies, a download of the dependencies would be required anyways. So it does not make a big difference excepting that dependencies already available in the local maven repository cannot be reused.

At the moment I think the benefits of shading the Citrus dependencies would prevail the downside of bigger jar artifacts.

christophd commented 5 years ago

Citrus is supposed to be used as test scoped dependency and the SUT should be a previously built artifact deployed somewhere. I do not see how the classpath should be affected as you describe.

When you do the test module setup right you would never run into these problems as Citrus will always use the well designed software interfaces of the SUT. When the Citrus test project depends on the SUT and its classpath dependencies this would be a bad setup and needs to be fixed there IMO.

So I do not see the need for shading at the moment.

Wouldn't you have the very same problem with any other framework or library bringing in newer versions as dependencies?

svettwer commented 5 years ago

In general, I agree.

When you do the test module setup right

Unfortunately sometimes "doing it right" or "cleaning it up" is not achievable in the given amount of time or simply not part of the order.

Wouldn't you have the very same problem with any other framework or library bringing in newer versions as dependencies?

Yes, this issue exists from the moment where the dependency management is not clean. In the most cases this might not have any noticeable effect but it is there for sure.

christophd commented 5 years ago

So are we talking about something like a fat jar version of Citrus that is added to Maven Central in addition to the traditional jar artifacts?

Or is this somewhat meant replacing the traditional style artifacts published?

I would like to see this in addition to the traditional artifacts if necessary.

Use case still seems to me to be a quite rare and special.

svettwer commented 5 years ago

At first, this issue is just a note to remind me that there was a problem. I've no explicit plans. Just a brain dump. The fat jar shading thingy was just something that came to my mind to solve this. A replacement of the traditional artifacts would not have any downsides from my perspective but it would add some robustness to the framework functionality as we take care that a messed up maven project of a user will not affect the Citrus functionality.

christophd commented 5 years ago

A replacement of the traditional artifacts would not have any downsides...

I think it would have downsides. Especially for artifact size and duplicate downloads of bits from Maven Central. Also shading and handling of conventional configuration files in META- INF such as spring.handlers and spring.schemas is not an easy task and gives runtime errors if not taken care of by the library introducing the shading.

Traditional style artifacts should not be replaced in my opinion.

Maven and Gradle are doing a great job when resolving the dependency tree with transitive versioning and in almost all cases it will be sufficient out of the box. You would loose all of that when offering a shaded Citrus fat jar exclusively.

svettwer commented 5 years ago

I've found a way to work around such setups with maven functionality only. Therefore a shading is not required anymore. :tada:

svettwer commented 5 years ago

And here we are again... If we have a parent pom reference to share some properties, there is no way I know to pass the properties to the child module without sharing the dependency management configuration.

christophd commented 5 years ago

How is that related to shading? I do not understand

hmmlopez commented 4 years ago

Please do not go for fat jars. If I want to use newer dependencies with Citrus I'm now free to upgrade them as I see fit, without the downsite of other classes of the same on the classpath. 1+ vote for christophd