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

Add annotation for module-path InjectPlugin auto-detection #722

Merged
merged 11 commits into from
Nov 5, 2024

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Oct 31, 2024

  • adds a @PluginProvides annotation that the processor can autodetect via getAllModuleElements (not sure I like the name though, feel free to change it if you got a better one)
  • writes plugin/module text files for reuse during test compilation

resolves #724

@SentryMan SentryMan added the enhancement New feature or request label Oct 31, 2024
@SentryMan SentryMan added this to the 10.5 milestone Oct 31, 2024
@SentryMan SentryMan self-assigned this Oct 31, 2024
@SentryMan SentryMan requested a review from rbygrave October 31, 2024 16:41
@SentryMan SentryMan enabled auto-merge October 31, 2024 16:46
@SentryMan
Copy link
Collaborator Author

SentryMan commented Nov 1, 2024

@rbygrave yeah I'm good with this. (unless you have a better name for the annotation)

@rbygrave
Copy link
Contributor

rbygrave commented Nov 4, 2024

adds a @PluginProvides annotation

Why do you want to add this? Why do you think we need this?

@SentryMan
Copy link
Collaborator Author

SentryMan commented Nov 4, 2024

Why do you want to add this? Why do you think we need this?

With this, I can create plugin libraries that will be auto-detected by avaje inject without needing a consumer to have a maven/gradle plugin. I've got a couple of internal libraries that this would be convenient to have on.

Essentially, it is the natural extension of #716.

@SentryMan
Copy link
Collaborator Author

SentryMan commented Nov 4, 2024

Currently, we need to use the inject-maven-plugin if one of these statements are true:

  • the application is using the maven compiler plugin annotationProcessorPaths
  • The application has a module-info

This PR makes it so that the inject-maven-plugin is required if and only if:

  • the application doesn't have a module-info and the application is using the maven compiler plugin annotationProcessorPaths

As I like to make use of JPMS, this PR is very convenient for me

@rbygrave
Copy link
Contributor

rbygrave commented Nov 4, 2024

I can create internal plugin libraries

Like what? Why are you creating plugins rather than using "normal dependencies"? That is, I'm not really expecting many people to create plugins at all.

without needing a maven/gradle plugin

People just need to register a service loader right, so they don't need a maven/gradle plugin for that? I don't get it?

natural extension of #716

I don't see that no, in that I'm really expecting people to use "normal dependencies" and only very few expert users will create plugins. So no I don't see that point.

@SentryMan
Copy link
Collaborator Author

SentryMan commented Nov 4, 2024

Like what? Why are you creating plugins rather than using "normal dependencies"?

A plugin I have is for one of our encryption libraries. It only exposes a single class so a plugin that provides a single instance is a natural choice. I have a similar structure to the jsonb/validator plugins to configure it using the ConfigPropertyPlugin.

That is, I'm not really expecting many people to create plugins at all.

Yeah but for the people who do, this would be pretty convenient for their consumers.

@SentryMan
Copy link
Collaborator Author

SentryMan commented Nov 4, 2024

People just need to register a service loader right, so they don't need a maven/gradle plugin for that? I don't get it?

Ah wait I think I might see the disconnect. I'm talking about the consumer of custom InjectPlugins when I say they need a maven plugin. Either way a ServiceLoader Entry is required, but this about the generator detecting it.

Suppose I create a plugin EncryptPlugin and register it with the ServiceLoader

@ServiceProvider
public class EncryptPlugin implements InjectPlugin {

  @Override
  public Class<?>[] provides() {

    return new Class<?>[] {Encryptor.class};
  }

  @Override
  public void apply(BeanScopeBuilder builder) {
   Encryptor e = ...
    builder.beans(e);
  }
}

When someone pulls in the maven dependency for this library and tries to use it like this:

@Singleton
public class WireOther {
  Encryptor e;

  public WireOther(Encryptor e) {
    this.e = e;
  }
}

Compilation will fail if any of these are true

  • the application is using the maven compiler plugin annotationProcessorPaths
  • The application has a module-info

In these situations the ServiceLoader inside an apt tool is inoperable, so the inject generator cannot use the ServiceLoader to see what classes the EncryptPlugin provides without assistance and so fails with a dependency missing error.

The maven plugin solves this by using the ServiceLoader before compilation and writing text files. The generator can read these files to understand what plugin provided classes are available.

The obvious problem with this approach is that users must not forget to add the maven plugin or else things won't compile.

The point of this PR is to make it easier for users to consume custom plugins if they happen to be using JPMS.

@SentryMan SentryMan merged commit ff857f1 into avaje:master Nov 5, 2024
7 checks passed
@SentryMan SentryMan deleted the plugin-detection branch November 5, 2024 02:06
@SentryMan
Copy link
Collaborator Author

@rbygrave perchance can you deploy an RC when you find the time?

@rbygrave
Copy link
Contributor

rbygrave commented Nov 5, 2024

Hmmm. https://s01.oss.sonatype.org is currently returning status code 503 ... but yeah will do it shortly hopefully.

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

Successfully merging this pull request may close these issues.

Module-Path Detection doesn't work for test compile
3 participants