Make `withProviderClassesFrom` work with Gradle

[objecttrouve]

This PR addresses a little shortcoming I encountered when trying to use the Keycloak Testcontainer in a Gradle project:

The withProviderClassesFrom-method takes a relative path as an argument, usually targets/classes. The Gradle counterpart of that path is build/classes/java/main and it isn't resolved correctly for Gradle projects. Moreover, the logic assumes that the META-INF directory is located inside the extension classes folder. This is also not true for Gradle projects.

Maven target directory structure:

Gradle build directory structure:

The issue is demonstrated in this example test.

This PR contains a possible fix, in which the classes directory is resolved relative to the current user directory and the META-INF directory is sought in the location we suspect it with Gradle, and added, if found.

⚠️ By default, I expect the user working directory (as in the user.dir system property) to be the project or module directory, both with Maven and with Gradle. I'm not sure though, if there can be custom project setups where this assumption isn't true or where the user.dir property is set in an unexpected way. So, this might be a breaking change, after all. But I'm confident that it's going to work for most projects.

Followed Baeldung and plenty of Google hits for how to get hold of the current user working directory: System property user.dir.

Added a few unit tests for the path resolution functionality. The tests use the old logic to show that, under default conditions, the outcome didn't change for Maven layouts.

The example test that uses the right path succeeds if you use a KC testcontainers library with the fix.

If you have concerns about breaking the API or anything else, I can rewrite the PR according to your preferences.

Thanks!

---

Update: Changed the original suggestion to an API extension that makes the configuration agnostic to the build tool: Make multiple paths configurable for classes and resources that should be added to the on-the-fly .jar-file.

[dasniko]

Hi @objecttrouve, thank you for your PR, it's appreciated.

As I don't have any knowledge with Gradle, I'm wondering if it is anyhow possible to do some automated Gradle tests, either during the Maven tests, or separately with some Gradle commands!? Hopefully using a Gradle wrapper and no need to maintain a local environment. This should be as easy as possible to run, maintain, etc.

[objecttrouve]

Hi @objecttrouve, thank you for your PR, it's appreciated.

As I don't have any knowledge with Gradle, I'm wondering if it is anyhow possible to do some automated Gradle tests, either during the Maven tests, or separately with some Gradle commands!? Hopefully using a Gradle wrapper and no need to maintain a local environment. This should be as easy as possible to run, maintain, etc.

@dasniko, it's tricky. I can imagine several ways to accomplish this within the Maven project and within the same build, however none which is easy to maintain in the long run.

Perhaps the most intuitive thing to do would be to include an independent example project as this one. However, one would still have to maintain dependencies in it, especially the reference to the Keycloak testcontainer library. And it would require the library to be released first instead of running tests within the library's build pipeline.

Of course, I can try to do some magic and make the Gradle build deduce its dependencies from this project's POM and invoke the Gradle build from the Maven build. But then again I expect things to become quite cumbersome and maintainers to hate me.

Maybe an independent project that's built within the same GitHub CI job using a local testcontainer lib dependency?

What would you find acceptable?

[dasniko]

I'm a bit ambivalent about this. One one hand side, it would be great to support also Gradle environments, no question. But on the other hand side, as already mentioned, I don't have any experience with Gradle and with that (or without that), it'll be very hard for me to maintain anything in the future.

I still will have to think about it, if I'm willing to merge this feature to the codebase under these circumstances.

[carstenSpraener]

Would it be an option to give the problem back to the user? If the Testcontainer provides a list of "ImportModifiers" that are Consumers of ExplodedImporter and called during collecting the data it would be possible to write test cases and a gradle user needs to add the correct paths.

This way it would also be possible to add any directory structure to the providers jar if needed.

[objecttrouve]

I'm a bit ambivalent about this. One one hand side, it would be great to support also Gradle environments, no question. But on the other hand side, as already mentioned, I don't have any experience with Gradle and with that (or without that), it'll be very hard for me to maintain anything in the future.

@dasniko, I understand. It's crucial to have the project properly tested to be sure there are no bad surprises. Also, neither the Maven target directory nor the Gradle build dir structures are granted APIs, I suppose.

Would it be an option to give the problem back to the user? If the Testcontainer provides a list of "ImportModifiers" that are Consumers of ExplodedImporter and called during collecting the data it would be possible to write test cases and a gradle user needs to add the correct paths.

This extension suggested by @carstenSpraener would make the API definition independent of the build tools but require a deeper understanding on the user side. Also, you'd need still need this change. I didn't want to change or bloat your API, but if you approve the introduction of the suggested extension, I can give it a shot.

One could also think about a configurable list of additional directories for the .jar-archive. Might be a little easier for users than handing out the ExplodedImporter.

Also, I can try to come up with some way to test with Gradle, if that's desirable.

In any case, thanks for your feedback so far!

[objecttrouve]

Changed the code once more, going for the idea of an API extension, which seems cleaner to me, though, for a while, it's going to bloat the API. Also, the new (and old) functionality can be and is tested within this project.

The main idea is simply to make multiple source directories for the jar contents configurable:

// Maven
 keycloak = keycloakContainer()
                .withProviderClassesAndResourcesFrom("target/classes");

// Gradle
 keycloak = keycloakContainer()
                .withProviderClassesAndResourcesFrom("build/classes/java/main", "build/resources/main");

This change makes the configuration agnostic to the build tool.

At the same time, I suggest to deprecate the methods that only accepted a single String argument, as they're now redundant.

(See also here.)

[dasniko]

First of all, sorry for my lack of presence here.

I don't want to harm anyone, but I'm not very happy with this PR and the amount of changes it introduces, "just" to support the Gradle developers. Also, "just" to be able to support multiple resource locations instead of only one, I don't see a need to rename methods and deprecating existing ones. Perhaps having methods to accept var args (like .someMethod(String... args)) instead of a Set<String> would be better and keeping the existing names.

There was a new comment in the related issue two weeks ago from @fkowal: https://github.com/dasniko/testcontainers-keycloak/issues/115#issuecomment-1751821019 If this works, it would solve all the problems, right? So, no need to do anything!? Yes, one would have to build the Jars first... for me, as an individual maintainer of this project, without Gradle knowledge, the best approach after all. 🤷‍♂️

[dasniko]

For the idea in the first paragraph in my previous comment, see here: https://github.com/dasniko/testcontainers-keycloak/commit/7165e1d496f08c4fd36fbcba892c849a355de52f It's not yet testet, just changed the internals, existing tests are still green. API is stable, functionality is enhanced.

[objecttrouve]

@dasniko, thanks for the feedback.

Having varargs, as in 7165e1d was my first intuition, too. It's true, it doesn't break code on the calling side. However, I inferred from the "extendable" in the name of the ExtendableKeycloakContainer that the class was explicitly intended to be a superclass of something else. Changing the signatures of the public and protected methods will break extending classes if they override those methods. That's why I suggested to add new methods and deprecate the old, rather than to change the existing signatures.

But OK, I see the point. It's not worth bloating the API for a marginal build tool when there's a workaround.

So, I'm closing this PR now. Thank you for looking into it!