Get rid of guava

[ptahchiev]

Do you really need the guava as runtime scope?

[Shredder121]

I mean, yes.
At compile time we compile against some APIs, that have to be then present at runtime, so for now, yes.

[Tibor17]

+1: for removing guava. We have compatibility issues between Dropwizard, JBoss and finally QueryDSL.
@Shredder121
Which runtime libraries are dependent on guava?
Maybe at least some QueryDSL modules could minimize using guava?

[ieugen]

I also support removing guava. It is a dependency of quite some projects. When using them together it's very easy to get errors because of incompatible versions. It happened to me so many times.
I had to spend a lot of times finding the right guava version compatible with all my dependencies.

[Tibor17]

Here is complete list of imports from querydsl-core. A lot of classes are using ImmutableList.
I cannot say for this project that they are not used but I can tell you how we solve this problem in Apache. We use the maven-shade-plugin and we reallocate the classes to org.apache.maven. In this case the resulting packages will be reallocated to com.querydsl.core.com.google.common.cache.

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.base.Objects;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
import com.google.common.base.Function;
import com.google.common.primitives.Primitives;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.*;

[Tibor17]

@Shredder121
How the proposal with maven-shade-plugin sounds to you?
I will try to propose the same in dropwizard project.

[Tibor17]

The Dropwizard project decided to follow more difficult way and removed Guava, see dropwizard/dropwizard#2486

[Shredder121]

I was also thinking of dropping Guava, although the only annoyance would be the template cache (which already causes some memory related issues).
Thanks for the link though.

[Tibor17]

@Shredder121 ok, understand, but what is your plan? You want to copy Dropwizard's utilities? This may help and speedup.
You say causes some memory related issues. You mean Java Memory Model JSR-133?
I may help you regarding caches if you show me what's going on.

[Shredder121]

template cache (which already causes some memory related issues).

I meant the current one already causes some issues #2218. The latest commit on that pull request (switch to Guava cache) is directly perpendicular to this issue.
So first have to depend on something like caffeine?

All other things can be replaced by JDK8, or thin wrappers/small methods. (creating maps, etc)

[ben-manes]

Could you instead depend on custom cache type, ConcurrentMap, or slimer interface with just computeIfAbsent call? It seems that's all you would need in AliasFactory (which is unbounded so could be a ConcurrentHashMap if acceptable) and TemplateFactory (which does not memoize atomically).

I don't know enough about the lifecycle / injection points, but this way you could carve out the cache to be optional or pluggable. You could default it to your preference (unbounded, caffeine) and let the user reconfigure and exclude the default's dependencies.

Another alternative if you just a concurrent LRU and a light weight dependency, is to use the much less feature rich ConcurrentLinkedHashMap. That was commonly shaded or copied into projects directly (e.g. SqlServer driver, Groovy). That was a JDK5 era project to work out a concurrent LRU algorithm, so well proven but minimalistic.

[hohwille]

• Please note that guava is affected by vulnerability: CVE-2018-10237.
• Instead of ImmutableList just use Collections.unmodifyableList(myMutableList)
• Using com.google.common.base.Objects is an anti-pattern. Please use java.util.Objects instead of this proprietary guava code.
• Alternatives for guavas cache are not so easy to find. So in case you need that, you might want to keep guava but should at least update to a most recent version (IMHO 27.1-jre)

querydsl/pom.xml

Line 60 in 8321eea

<guava.version>18.0</guava.version>

18.0 is old and has several vunerabilities that are meanwhile fixed.

[Shredder121]

For guavas cache I was thinking Caffeine, but maybe it's a bit much to pull in a new dependency.
Anyway, migrating away from guava entirely is not an easy feat unfortunately.

[entonio]

Is there any news on this? Being tied to guava 18 is a serious issue. What would be the work involved in at least getting into a modern version?

[Shredder121]

What would be the work involved in at least getting into a modern version

You could pull in your own version of Guava, but YMMV, since it's not really a backwards-compatible friendly library anymore. (which is more apparent in more recent versions unfortunately)

[Shredder121]

As for doing work for getting a more modern version, I'd much rather replace guava with java 8+ features or with libraries specifically suited for a task (to tackle the configurable caching issue).

[entonio]

I totally get that, but the impression I get is that it isn't being done any time soon. The problem here is that the functionalities in question are not trivial, and outsiders are likely to introduce subtle bugs f trying to migrate to something else. That's why I thought that someone savvy moving to a version that is reasonably modern but still compatible would be the safest quick win.

[Tibor17]

@entonio
That's all true and pragmatic, but there is one way to do it. I am also OSS developer and I am aware of the view where the outsider has always different experiences.
Certain Guava version has worked fine for QueryDSL, right? Roughly spoken, what functionality would be broken if you copy-paste the content of guava's JAR file into yours? An elegant solution in this way is to use Maven plugin maven-shade-plugin and configure the plugin properly with optimizations (only called Guava classes to shade).

[Tibor17]

After shading the libs, you will have a plenty of time to reduce Guava classes from simple to more complex ones. And you will have sustaining progress and next release to drop this problem once for all.

[bradfordChiang]

can the guava be removed? it causes lots incompatible issues. And we're in JDK 8 9 10 11 now, why still need it?

[OrangeDog]

N.B. if updating to a more recent guava, you'll want to exclude all these: google/guava#2824

24.1.1-jre (earliest version with known CVEs fixed) seems to be working fine.

[jwgmeligmeyling]

Personally I have used QueryDSL in both J2EE and non-J2EE projects for years without problems.... In containers that ship Guava, just exclude the Guava delivered transitively as Maven dependency, add the version added by your container in <provided> scope and make sure the module is exposed to your application and you're ready to go. Spring Boot applications that package all dependencies within Maven anyway should resolve to the right version anyway...

The version indicated in the pom is by the way the version that QueryDSL is compiled against. If your project references any newer version of Guava - Maven will automatically favour the version specified. Any "security issues" - if you're so worried about them, although CVE-2018-10237 can't be exploited through QueryDSL - can be easily resolved by bumping Guava in your own application. I am using QueryDSL flawlessly against Guava 25.0-jre from WildFly 18.0.1.Final.

There will always be folks promoting to minimise dependencies... But let's face it: for some of the constructs used from Guava, there are no constructs yet in the targeted JRE. Which will lead to new complexity in QueryDSL itself which needs to be maintained. For the same reason its very likely that you end up with a version of Guava nonetheless in your application... Most of the constructs that arrived in newer versions of Java that are good candidate replacements for Guava, only arrived after JRE9 or so. It's simply unfavourable for a library so widely used to demand Java 9+ from users. Keep in mind that as of to date QueryDSL targets Java 6 support! On the roadmap for 5.x it can be seen that it's targeted to bump the version support to Java 8 minimally.

I wouldn't mind seeing the requirement on Guava being dropped, but is it really time well spent? It also hasn't exactly received a lot of attention even.

As for the suggestion of shading: I am not a big fan of shading, as it pollutes the classpath and IDE autocomplete as a result. If a shaded artefact solution is pursued, please ship this under a separate classifier - so both shaded and unshaded artefact remain available. For example spotify-docker-client did this to shade their dependency to Jerseys implementation of HTTP client.

Not sure if the decision for removal already has been made and someones working on it. It seems fairly easy to do - we're not using that much Guava. Anyways, I want some of the false concerns related to Guava out of the way. Its still safe to use an old version of QueryDSL.

If Guava is not removed in the next release though, it would probably be nice to bump it to a more recent release.

[ptahchiev]

You are building a framework, you are not building a product. I am building a product and I am using your framework - the result is out of 450 dependencies, querydsl is the only one that brings guava and pollutes my classpath.

[kamko]

First of all dropping of guava is planned for 5.x release which also aims to finally drop java 6 support.

Requesting your users to explicitly care about dependency which comes from your library is not really friendly if it is not necessary. I personally had to resolve these conflicts before when other libraries required newer versions and byte code compatibility isn't always there (or even when it is you have to care about it and pollute your project pom/build.gradle with explicit exclusions).

In my opinion, if we won't find a good alternative for the tools currently used from guava we should definitely at least shade it under a different namespace.

In the end, you are right is (probably) not dangerous to use guava it is simply preferred to minimize dependencies in libraries as you're polluting someone's classpath with that.

[Tibor17]

Here Maven resolver is not a problem. The problem is broken compatibility of Guava between versions. We are not talking about new versions of Wildfly and another containers. We are talking about the way to prevent from the same troubles happenning in the future. Removing the guava library would remove future conflicts.

[idosal]

As @kamko mentioned, this is planned for a 5.X release. However, as there is a workaround, it is not the team's top priority. If anyone would like to contribute a fix we'd be happy to consider it. :)

[jfcabral]

Good job 👍