Skip to content

Commit

Permalink
fixed scope accuracy for linked bindings in guice bindings report
Browse files Browse the repository at this point in the history
  • Loading branch information
xvik committed Nov 20, 2024
1 parent 0c2c465 commit 7f643fd
Show file tree
Hide file tree
Showing 8 changed files with 232 additions and 31 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -223,8 +231,7 @@ private static void fillDeclaration(final BindingDeclaration dec, final Injector
return;
}
}
Class<? extends Annotation> scope =
(Class<? extends Annotation>) existingBinding.acceptScopingVisitor(SCOPE_DETECTOR);
Class<? extends Annotation> scope = SCOPE_DETECTOR.performDetection(existingBinding);
if (scope != null && scope.equals(EagerSingleton.class)) {
scope = jakarta.inject.Singleton.class;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
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;
import com.google.inject.servlet.RequestScoped;
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;
Expand All @@ -19,6 +22,18 @@
public class GuiceScopingVisitor
extends DefaultBindingScopingVisitor<Class<? extends Annotation>> {

/**
* 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<? extends Annotation> performDetection(final Binding binding) {
return finalizeScopeDetection((Class<? extends Annotation>) binding.acceptScopingVisitor(this), binding);
}

@Override
public Class<? extends Annotation> visitEagerSingleton() {
return EagerSingleton.class;
Expand Down Expand Up @@ -55,9 +70,31 @@ public Class<? extends Annotation> visitScopeAnnotation(final Class scopeAnnotat

@Override
public Class<? extends Annotation> 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<? extends Annotation> finalizeScopeDetection(final Class<? extends Annotation> scope,
final Binding binding) {
if (scope != null) {
return scope;
}
Class<? extends Annotation> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public void bind(final Binder binder, final Class<?> type, final boolean lazy) {
public <T> void manualBinding(final Binder binder, final Class<T> type, final Binding<T> binding) {
// we can only validate existing binding here (actually entire extension is pretty useless in case of manual
// binding)
final Class<? extends Annotation> scope = binding.acceptScopingVisitor(VISITOR);
// in production all services will work as eager singletons, for report (TOOL stage) consider also valid
final Class<? extends Annotation> 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)),
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<? extends Annotation> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -149,4 +152,29 @@ private static List<String> replaceLambdaModuleClasses(final List<String> 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<? extends Annotation> findScopingAnnotation(final Class<?> type,
final boolean countGuiceSpecific) {
Class<? extends Annotation> res = null;
for (Annotation ann : type.getAnnotations()) {
final Class<? extends Annotation> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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<Configuration> {

@Override
void initialize(Bootstrap<Configuration> 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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -85,12 +85,12 @@ class ScopingVisitorTest extends Specification {
}

Class<? extends Annotation> scope(Injector injector, Class type) {
injector.getExistingBinding(Key.get(type)).acceptScopingVisitor(visitor)
visitor.performDetection(injector.getExistingBinding(Key.get(type)))
}

Class<? extends Annotation> scope(List<Element> 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 {
Expand Down

0 comments on commit 7f643fd

Please # to comment.