From 7f643fda25ec1898c1c9d7e172a213e703d19e75 Mon Sep 17 00:00:00 2001 From: Vyacheslav Rusakov Date: Thu, 14 Nov 2024 17:47:53 +0700 Subject: [PATCH] fixed scope accuracy for linked bindings in guice bindings report --- CHANGELOG.md | 2 + .../report/guice/util/GuiceModelParser.java | 13 +- .../util/visitor/GuiceScopingVisitor.java | 43 +++++- .../eager/EagerSingletonInstaller.java | 4 +- .../jersey/AbstractJerseyInstaller.java | 22 +-- .../module/installer/util/BindingUtils.java | 28 ++++ ...GuiceRendererSingletonDetectionTest.groovy | 143 ++++++++++++++++++ .../guice/util/ScopingVisitorTest.groovy | 8 +- 8 files changed, 232 insertions(+), 31 deletions(-) create mode 100644 dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/GuiceRendererSingletonDetectionTest.groovy diff --git a/CHANGELOG.md b/CHANGELOG.md index b6971c0d2..319151dcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ * Update to dropwizard 4.0.9 +* Improve guice bindings report: + - fixed scope accuracy for linked bindings ### 7.1.4 (2024-09-14) * Update to dropwizard 4.0.8 diff --git a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/GuiceModelParser.java b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/GuiceModelParser.java index 84ea6102f..c4b9715e0 100644 --- a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/GuiceModelParser.java +++ b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/GuiceModelParser.java @@ -21,7 +21,15 @@ import ru.vyarus.dropwizard.guice.module.installer.util.BindingUtils; import java.lang.annotation.Annotation; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; /** * Guice introspection utils. Parse guice SPI model. Supports both elements from modules and injector bindings. @@ -223,8 +231,7 @@ private static void fillDeclaration(final BindingDeclaration dec, final Injector return; } } - Class scope = - (Class) existingBinding.acceptScopingVisitor(SCOPE_DETECTOR); + Class scope = SCOPE_DETECTOR.performDetection(existingBinding); if (scope != null && scope.equals(EagerSingleton.class)) { scope = jakarta.inject.Singleton.class; } diff --git a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/visitor/GuiceScopingVisitor.java b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/visitor/GuiceScopingVisitor.java index 0a66daa45..6de234433 100644 --- a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/visitor/GuiceScopingVisitor.java +++ b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/debug/report/guice/util/visitor/GuiceScopingVisitor.java @@ -1,5 +1,6 @@ package ru.vyarus.dropwizard.guice.debug.report.guice.util.visitor; +import com.google.inject.Binding; import com.google.inject.Scope; import com.google.inject.Scopes; import com.google.inject.Singleton; @@ -7,7 +8,9 @@ import com.google.inject.servlet.ServletScopes; import com.google.inject.servlet.SessionScoped; import com.google.inject.spi.DefaultBindingScopingVisitor; +import com.google.inject.spi.LinkedKeyBinding; import ru.vyarus.dropwizard.guice.module.installer.feature.eager.EagerSingleton; +import ru.vyarus.dropwizard.guice.module.installer.util.BindingUtils; import ru.vyarus.dropwizard.guice.module.support.scope.Prototype; import java.lang.annotation.Annotation; @@ -19,6 +22,18 @@ public class GuiceScopingVisitor extends DefaultBindingScopingVisitor> { + /** + * Method to call DIRECTLY on visitor instead of "normal" visitor appliance. Required for more accurate + * scope resolution. + * + * @param binding binding to analyze + * @return resolved scope (or proposed prototype scope when scope not detected) + */ + @SuppressWarnings("unchecked") + public Class performDetection(final Binding binding) { + return finalizeScopeDetection((Class) binding.acceptScopingVisitor(this), binding); + } + @Override public Class visitEagerSingleton() { return EagerSingleton.class; @@ -55,9 +70,31 @@ public Class visitScopeAnnotation(final Class scopeAnnotat @Override public Class visitNoScoping() { - // special case: when checking direct module elements guice know only directly configured scope info and - // ignore annotations.. so instead of correct scope from annotation no scope is returned + // no scope annotation declared OR linked binding with scope annotation on TARGET class (no way to know it) + + return null; + } - return Prototype.class; + /** + * Method should be called manually! Scoping visitor would not detect scoping annotation on linked binding. + * This method tries to fix this (to improve accuracy). + * + * @param scope resolved scope or null + * @param binding binding under analysis + * @return scoping annotation (exactly resolved or assumed prototype) + */ + private Class finalizeScopeDetection(final Class scope, + final Binding binding) { + if (scope != null) { + return scope; + } + Class res = null; + if (binding instanceof LinkedKeyBinding) { + // NOTE: the link may be a part of long chain, but this is ignored - consider only length of 1 + // (which may obviously produce false scope value) + final LinkedKeyBinding linkedBinding = (LinkedKeyBinding) binding; + res = BindingUtils.findScopingAnnotation(linkedBinding.getLinkedKey().getTypeLiteral().getRawType(), true); + } + return res == null ? Prototype.class : visitScopeAnnotation(res); } } diff --git a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/eager/EagerSingletonInstaller.java b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/eager/EagerSingletonInstaller.java index 7d46d5695..0dbc5a128 100644 --- a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/eager/EagerSingletonInstaller.java +++ b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/eager/EagerSingletonInstaller.java @@ -54,8 +54,8 @@ public void bind(final Binder binder, final Class type, final boolean lazy) { public void manualBinding(final Binder binder, final Class type, final Binding binding) { // we can only validate existing binding here (actually entire extension is pretty useless in case of manual // binding) - final Class scope = binding.acceptScopingVisitor(VISITOR); - // in production all services will work as eager singletons, for report (TOOL stage) consider also valid + final Class scope = VISITOR.performDetection(binding); + // in production, all services will work as eager singletons, for report (TOOL stage) consider also valid Preconditions.checkArgument(scope.equals(EagerSingleton.class) || (!binder.currentStage().equals(Stage.DEVELOPMENT) && scope.equals(Singleton.class)), diff --git a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/jersey/AbstractJerseyInstaller.java b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/jersey/AbstractJerseyInstaller.java index aa454abe1..21f00cae7 100644 --- a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/jersey/AbstractJerseyInstaller.java +++ b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/feature/jersey/AbstractJerseyInstaller.java @@ -1,18 +1,15 @@ package ru.vyarus.dropwizard.guice.module.installer.feature.jersey; import com.google.inject.Binder; -import com.google.inject.ScopeAnnotation; import com.google.inject.binder.AnnotatedBindingBuilder; +import jakarta.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import ru.vyarus.dropwizard.guice.module.installer.FeatureInstaller; import ru.vyarus.dropwizard.guice.module.installer.install.JerseyInstaller; import ru.vyarus.dropwizard.guice.module.installer.install.binding.LazyBinding; import ru.vyarus.dropwizard.guice.module.installer.option.InstallerOptionsSupport; - -import jakarta.inject.Scope; -import jakarta.inject.Singleton; -import java.lang.annotation.Annotation; +import ru.vyarus.dropwizard.guice.module.installer.util.BindingUtils; import static ru.vyarus.dropwizard.guice.module.installer.InstallersOptions.ForceSingletonForJerseyExtensions; import static ru.vyarus.dropwizard.guice.module.installer.InstallersOptions.JerseyExtensionsManagedByGuice; @@ -92,19 +89,6 @@ protected boolean isForceSingleton(final Class type, final boolean hkManaged) * @return true if scope annotation found, false otherwise */ private boolean hasScopeAnnotation(final Class type, final boolean hkManaged) { - boolean found = false; - for (Annotation ann : type.getAnnotations()) { - final Class annType = ann.annotationType(); - if (annType.isAnnotationPresent(Scope.class)) { - found = true; - break; - } - // guice has special marker annotation - if (!hkManaged && annType.isAnnotationPresent(ScopeAnnotation.class)) { - found = true; - break; - } - } - return found; + return BindingUtils.findScopingAnnotation(type, !hkManaged) != null; } } diff --git a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/util/BindingUtils.java b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/util/BindingUtils.java index d6ad152d7..a7df572b9 100644 --- a/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/util/BindingUtils.java +++ b/dropwizard-guicey/src/main/java/ru/vyarus/dropwizard/guice/module/installer/util/BindingUtils.java @@ -2,10 +2,13 @@ import com.google.inject.Binding; import com.google.inject.Module; +import com.google.inject.ScopeAnnotation; import com.google.inject.internal.util.StackTraceElements; import com.google.inject.spi.Element; import com.google.inject.spi.ElementSource; +import jakarta.inject.Scope; +import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; @@ -149,4 +152,29 @@ private static List replaceLambdaModuleClasses(final List module } return res; } + + /** + * Searches for scoping annotation on class. Base classes are not checked as scope is not inheritable. + * + * @param type class to search for + * @param countGuiceSpecific true to count guice-specific annotations (with {@link ScopeAnnotation}) + * @return detected annotation or null + */ + public static Class findScopingAnnotation(final Class type, + final boolean countGuiceSpecific) { + Class res = null; + for (Annotation ann : type.getAnnotations()) { + final Class annType = ann.annotationType(); + if (annType.isAnnotationPresent(Scope.class)) { + res = annType; + break; + } + // guice has special marker annotation + if (countGuiceSpecific && annType.isAnnotationPresent(ScopeAnnotation.class)) { + res = annType; + break; + } + } + return res; + } } diff --git a/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/GuiceRendererSingletonDetectionTest.groovy b/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/GuiceRendererSingletonDetectionTest.groovy new file mode 100644 index 000000000..23be3a04c --- /dev/null +++ b/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/GuiceRendererSingletonDetectionTest.groovy @@ -0,0 +1,143 @@ +package ru.vyarus.dropwizard.guice.debug.renderer.guice + +import com.google.inject.AbstractModule +import com.google.inject.Injector +import io.dropwizard.core.Application +import io.dropwizard.core.Configuration +import io.dropwizard.core.setup.Bootstrap +import io.dropwizard.core.setup.Environment +import jakarta.inject.Inject +import jakarta.inject.Singleton +import ru.vyarus.dropwizard.guice.GuiceBundle +import ru.vyarus.dropwizard.guice.bundle.lookup.PropertyBundleLookup +import ru.vyarus.dropwizard.guice.debug.report.guice.GuiceBindingsRenderer +import ru.vyarus.dropwizard.guice.debug.report.guice.GuiceConfig +import ru.vyarus.dropwizard.guice.module.support.scope.Prototype +import ru.vyarus.dropwizard.guice.test.jupiter.TestDropwizardApp +import spock.lang.Specification + +/** + * @author Vyacheslav Rusakov + * @since 14.11.2024 + */ +@TestDropwizardApp(App) +class GuiceRendererSingletonDetectionTest extends Specification { + + static { + System.clearProperty(PropertyBundleLookup.BUNDLES_PROPERTY) + } + + @Inject + Injector injector + GuiceBindingsRenderer renderer + + void setup() { + renderer = new GuiceBindingsRenderer(injector) + } + + def "Check singleton detection"() { + + expect: + render(new GuiceConfig() + .hideGuiceBindings() + .hideGuiceyBindings()) == """ + + 1 MODULES with 12 bindings + │ + └── Module (r.v.d.g.d.r.g.GuiceRendererSingletonDetectionTest) + ├── linkedkey [@Prototype] Bind4 --> Ext4 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── linkedkey [@Prototype] Bind5 --> Ext5 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── linkedkey [@Prototype] LongBind --> LongExt1 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── linkedkey [@Singleton] Bind1 --> Ext1 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── linkedkey [@Singleton] Bind2 --> Ext2 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── linkedkey [@Singleton] Bind3 --> Ext3 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── linkedkey [@Singleton] LongExt1 --> LongExt2 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── untargetted [@Prototype] Simple4 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── untargetted [@Prototype] Simple5 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── untargetted [@Singleton] Simple1 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + ├── untargetted [@Singleton] Simple2 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + └── untargetted [@Singleton] Simple3 at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321) + + + BINDING CHAINS + ├── Bind1 --[linked]--> Ext1 + ├── Bind2 --[linked]--> Ext2 + ├── Bind3 --[linked]--> Ext3 + ├── Bind4 --[linked]--> Ext4 + ├── Bind5 --[linked]--> Ext5 + └── LongBind --[linked]--> LongExt1 --[linked]--> LongExt2 +""" as String + } + + + static class App extends Application { + + @Override + void initialize(Bootstrap bootstrap) { + bootstrap.addBundle(GuiceBundle.builder() + .modules(new Module()) + .printGuiceBindings() + .build()) + } + + @Override + void run(Configuration configuration, Environment environment) throws Exception { + } + } + + static class Module extends AbstractModule { + @Override + protected void configure() { + bind(Simple1) + bind(Simple2).in(Singleton.class) + bind(Simple3).asEagerSingleton() + bind(Simple4) + bind(Simple5) + + // linked + bind(Bind1).to(Ext1) + bind(Bind2).to(Ext2).in(Singleton.class) + bind(Bind3).to(Ext3).asEagerSingleton() + bind(Bind4).to(Ext4) + bind(Bind5).to(Ext5) + + bind(LongBind).to(LongExt1) + bind(LongExt1).to(LongExt2) + } + } + + @Singleton + static class Simple1 {} + static class Simple2 {} + static class Simple3 {} + static class Simple4 {} + @Prototype + static class Simple5 {} + + static interface Bind1 {} + @Singleton + static class Ext1 implements Bind1 {} + + static interface Bind2 {} + static class Ext2 implements Bind2 {} + + static interface Bind3 {} + static class Ext3 implements Bind3 {} + + static interface Bind4 {} + static class Ext4 implements Bind4 {} + + static interface Bind5 {} + @Prototype + static class Ext5 implements Bind5 {} + + + static interface LongBind {} + static class LongExt1 implements LongBind {} + @Singleton + static class LongExt2 extends LongExt1 {} + + String render(GuiceConfig config) { + renderer.renderReport(config).replaceAll("\r", "").replaceAll(" +\n", "\n") + } +} diff --git a/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/util/ScopingVisitorTest.groovy b/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/util/ScopingVisitorTest.groovy index d6079a270..50adfaab0 100644 --- a/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/util/ScopingVisitorTest.groovy +++ b/dropwizard-guicey/src/test/groovy/ru/vyarus/dropwizard/guice/debug/renderer/guice/util/ScopingVisitorTest.groovy @@ -69,7 +69,7 @@ class ScopingVisitorTest extends Specification { def "Check direct visitor cases"() { expect: "correct scope annotations" - visitor.visitNoScoping() == Prototype + visitor.visitNoScoping() == null visitor.visitEagerSingleton() == EagerSingleton visitor.visitScope(Scopes.SINGLETON) == Singleton @@ -85,12 +85,12 @@ class ScopingVisitorTest extends Specification { } Class scope(Injector injector, Class type) { - injector.getExistingBinding(Key.get(type)).acceptScopingVisitor(visitor) + visitor.performDetection(injector.getExistingBinding(Key.get(type))) } Class scope(List elements, Class type) { - (elements.find { it instanceof Binding && (it as Binding).getKey().getTypeLiteral().getRawType() == type } as Binding) - .acceptScopingVisitor(visitor) + visitor.performDetection( + elements.find { it instanceof Binding && (it as Binding).getKey().getTypeLiteral().getRawType() == type } as Binding) } static class Module extends AbstractModule {