dropwizard-guicey icon indicating copy to clipboard operation
dropwizard-guicey copied to clipboard

OReflectionHelper scanning ignores non-public classes

Open manmedia opened this issue 1 year ago • 4 comments

Hello,

Between Guicey 5.1.0 and 7.0.0, I can see you have introduced isAcceptableClass() in OReflectionHelper scanning logic.

What is the reason behind this please? Becuase this is breaking a lot of our projects due to some of the classes being protected. Could you please share any bug/feature request ticket associated to it?

An example - if you had a Guice PrivateModule extension working with a protected class e.g. below

public class FooModule extends PrivateModule {

  @Override

  protected void configure() {
    bind(Bar.class)
        .to(BarImpl.class)
        .asEagerSingleton();
    expose(Bar.class);
  }
...
...
}

The above doesn't work, and our tests can't find any binding information after we upgrade to dropwizard-guicey 7.0.0 from 5.10.0.

Regards,

manmedia avatar Sep 29 '24 20:09 manmedia

Hello! The change (in OReflectionHandler) was done in 5.6.1. Initially there was issue 231 (which is not directly affect this case) and, as I remember, a few direct messages from users about too broad classpath scan. So It could be either preventive action (allow only publuc classes to avoid problems) or a consequence of some request.

But extensions are searched not only with classpath scan - they could also be resolved from guice bindings directly. Not sure why it's not working in your case.

I'm not sure it would be a good idea to allow searching for extensions in protected classes for classpath scanner (need to check, but not sure guice would be able to instantiate it in case of direct binding (after classpath scan).

I think such extensions should be detected from bindings, so I will try to reproduce your case in order to find out why exposed bindings from protected modules are not recognized.


our tests can't find any binding information

Could you please clarify what does it mean? For manual binding (described in module) there should be binding information. Maybe you mean guicey extension registration info.

And please describe a protected extension example (Bar.class). Just to be sure my local case would be the same

xvik avatar Sep 30 '24 07:09 xvik

@xvik I am uploading this zip file - which you can import in intelliJ and do a full gradle build. When you run it, you will see that TheProblemClass does not get registered/started. As soon as you make it public it runs.

In the past with guicey 5.10.0 we never had this issue with accessor modification. Reproducible_Hung_Server.zip

manmedia avatar Sep 30 '24 08:09 manmedia

@xvik I am uploading this zip file - which you can import in intelliJ and do a full gradle build. When you run it, you will see that TheProblemClass does not get registered/started. As soon as you make it public it runs.

In the past with guicey 5.10.0 we never had this issue with accessor modification. Reproducible_Hung_Server.zip

@xvik also - if you find it too complex - here is a more lightweight, leaner project which you can examine faster. But the same issue. The startup for my desired class fails (until I make the class public, which wasn't required before). Archive.zip

manmedia avatar Sep 30 '24 09:09 manmedia

Thank you! Reproduced. Indeed in 5.1.0 class was detected with classpath scan:

    └── CLASSPATH SCAN
        ├── extension  SampleResource               (org.example.resource)     
        └── extension  TheProblemClass              (org.example)      

I will think how to better handle this (maybe add an option to allow protected classes detection and not change the default behaviour). And, also, will look why bindings detection not work for private modules.

xvik avatar Sep 30 '24 09:09 xvik

7.2.0 released. It now supports private modules, so in your case it should detect extensions from exposed bindings.

Also, there is now an option to detect extensions in protected and package-private classes

GuiceBundle.builder()
    .option(GuiceyOptions.ScanProtectedClasses, true)

xvik avatar May 11 '25 18:05 xvik