From 5f9b01dd2c305e3e7c9b7acc3971788a7bd1b151 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 29 Mar 2025 11:07:50 +1100 Subject: [PATCH 1/3] Detect and prohibit new Guava classes --- build.gradle | 2 + .../groovy/graphql/GuavaLimitCheck.groovy | 78 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 src/test/groovy/graphql/GuavaLimitCheck.groovy diff --git a/build.gradle b/build.gradle index 1a379c317a..96242099a0 100644 --- a/build.gradle +++ b/build.gradle @@ -125,6 +125,8 @@ dependencies { testAnnotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess:1.37' jmh 'org.openjdk.jmh:jmh-core:1.37' jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.37' + + testImplementation "com.tngtech.archunit:archunit-junit5:1.2.0" } shadowJar { diff --git a/src/test/groovy/graphql/GuavaLimitCheck.groovy b/src/test/groovy/graphql/GuavaLimitCheck.groovy new file mode 100644 index 0000000000..815a97959e --- /dev/null +++ b/src/test/groovy/graphql/GuavaLimitCheck.groovy @@ -0,0 +1,78 @@ +package graphql + +import com.tngtech.archunit.core.domain.JavaClasses +import com.tngtech.archunit.core.importer.ClassFileImporter +import com.tngtech.archunit.core.importer.ImportOption +import spock.lang.Specification + +/* + * We selectively shade in a few classes of Guava, however we want to minimise dependencies so we want to keep this list small. + * This check ensures no new Guava classes are added + */ +class GuavaLimitCheck extends Specification { + + static final String GUAVA_PACKAGE_PREFIX = "com.google.common" + + static final Set ALLOWED_GUAVA_CLASSES = [ + "com.google.common.collect.ImmutableMap", + "com.google.common.collect.ImmutableSet", + "com.google.common.collect.ImmutableList", + "com.google.common.base.Strings", + "com.google.common.collect.BiMap", + "com.google.common.collect.HashBiMap", + "com.google.common.collect.ImmutableCollection", + "com.google.common.collect.LinkedHashMultimap", + "com.google.common.collect.Multimap", + "com.google.common.collect.Table", + "com.google.common.collect.Sets", + "com.google.common.collect.Multimaps", + "com.google.common.collect.Iterables", + "com.google.common.collect.HashBasedTable", + "com.google.common.collect.HashMultimap", + "com.google.common.collect.Interner", + "com.google.common.collect.Interners" + ] + + def "should identify which classes use prohibited Guava dependencies"() { + given: + JavaClasses importedClasses = new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages("graphql") + + when: + Map> violationsByClass = [:] + + importedClasses.each { javaClass -> + def className = javaClass.name + def guavaUsages = javaClass.getAccessesFromSelf() + .collect { it.targetOwner } + .findAll { it.packageName.startsWith(GUAVA_PACKAGE_PREFIX) && !ALLOWED_GUAVA_CLASSES.contains(it) } + .toSet() + + if (!guavaUsages.isEmpty()) { + violationsByClass[className] = guavaUsages + } + } + + then: + violationsByClass.isEmpty() + + cleanup: "if the test fails, provide detailed information about which classes have violations" + if (!violationsByClass.isEmpty()) { + def errorMessage = new StringBuilder("Prohibited Guava class usage found:\n") + + violationsByClass.each { className, guavaClasses -> + errorMessage.append("\nClass: ${className} uses these prohibited Guava classes:\n") + guavaClasses.each { guavaClass -> + errorMessage.append(" - ${guavaClass}\n") + } + } + + errorMessage.append("\nEither:\n") + errorMessage.append("1. Preferred option: Replace them with alternatives that don't depend on Guava\n") + errorMessage.append("2. If absolutely necessary: Add these Guava classes to the shade configuration in the build.gradle file\n") + + println errorMessage.toString() + } + } +} From 8507df14df1c2c7138342c6dd18787a0d84892a7 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 29 Mar 2025 11:42:06 +1100 Subject: [PATCH 2/3] Fix bug should be fully qualified name --- src/test/groovy/graphql/GuavaLimitCheck.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/graphql/GuavaLimitCheck.groovy b/src/test/groovy/graphql/GuavaLimitCheck.groovy index 815a97959e..19be5de631 100644 --- a/src/test/groovy/graphql/GuavaLimitCheck.groovy +++ b/src/test/groovy/graphql/GuavaLimitCheck.groovy @@ -46,7 +46,7 @@ class GuavaLimitCheck extends Specification { def className = javaClass.name def guavaUsages = javaClass.getAccessesFromSelf() .collect { it.targetOwner } - .findAll { it.packageName.startsWith(GUAVA_PACKAGE_PREFIX) && !ALLOWED_GUAVA_CLASSES.contains(it) } + .findAll { it.packageName.startsWith(GUAVA_PACKAGE_PREFIX) && !ALLOWED_GUAVA_CLASSES.contains(it.fullName) } .toSet() if (!guavaUsages.isEmpty()) { From 6fb0db222f2bf4f78fe874e574688442142c4db8 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 29 Mar 2025 11:47:02 +1100 Subject: [PATCH 3/3] Add full list of Guava exemptions --- .../groovy/graphql/GuavaLimitCheck.groovy | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/test/groovy/graphql/GuavaLimitCheck.groovy b/src/test/groovy/graphql/GuavaLimitCheck.groovy index 19be5de631..cc4fc7ff58 100644 --- a/src/test/groovy/graphql/GuavaLimitCheck.groovy +++ b/src/test/groovy/graphql/GuavaLimitCheck.groovy @@ -14,23 +14,34 @@ class GuavaLimitCheck extends Specification { static final String GUAVA_PACKAGE_PREFIX = "com.google.common" static final Set ALLOWED_GUAVA_CLASSES = [ - "com.google.common.collect.ImmutableMap", - "com.google.common.collect.ImmutableSet", - "com.google.common.collect.ImmutableList", "com.google.common.base.Strings", "com.google.common.collect.BiMap", + "com.google.common.collect.HashBasedTable", "com.google.common.collect.HashBiMap", + "com.google.common.collect.HashMultimap", + "com.google.common.collect.HashMultiset", + "com.google.common.collect.ImmutableBiMap", "com.google.common.collect.ImmutableCollection", + "com.google.common.collect.ImmutableList", + "com.google.common.collect.ImmutableList\$Builder", + "com.google.common.collect.ImmutableListMultimap", + "com.google.common.collect.ImmutableListMultimap\$Builder", + "com.google.common.collect.ImmutableMap", + "com.google.common.collect.ImmutableMap\$Builder", + "com.google.common.collect.ImmutableSet", + "com.google.common.collect.ImmutableSet\$Builder", + "com.google.common.collect.Interner", + "com.google.common.collect.Interners", + "com.google.common.collect.Iterables", "com.google.common.collect.LinkedHashMultimap", + "com.google.common.collect.Maps", "com.google.common.collect.Multimap", - "com.google.common.collect.Table", - "com.google.common.collect.Sets", "com.google.common.collect.Multimaps", - "com.google.common.collect.Iterables", - "com.google.common.collect.HashBasedTable", - "com.google.common.collect.HashMultimap", - "com.google.common.collect.Interner", - "com.google.common.collect.Interners" + "com.google.common.collect.Multiset", + "com.google.common.collect.Multisets", + "com.google.common.collect.Sets", + "com.google.common.collect.Sets\$SetView", + "com.google.common.collect.Table" ] def "should identify which classes use prohibited Guava dependencies"() {