Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OReflectionHelper scanning ignores non-public classes #404

Open
manmedia opened this issue Sep 29, 2024 · 4 comments
Open

OReflectionHelper scanning ignores non-public classes #404

manmedia opened this issue Sep 29, 2024 · 4 comments

Comments

@manmedia
Copy link

manmedia commented Sep 29, 2024

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,

@xvik
Copy link
Owner

xvik commented Sep 30, 2024

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

@manmedia
Copy link
Author

@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
Copy link
Author

@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

@xvik
Copy link
Owner

xvik commented Sep 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants