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

Using service loader mediator annotations (see #630). #632

Closed
wants to merge 1 commit into from

Conversation

mnlipp
Copy link
Contributor

@mnlipp mnlipp commented Oct 23, 2022

Shows the usage of the service locator mediator with annotations that generate the "Require-Capability" header in MANIFEST.MF. (Alternatively the Header can be specified directly in pom.xml.)

The Providers (in angus-mail) need a corresponding @ServiceProvider annotation such as:

@aQute.bnd.annotation.spi.ServiceProvider(value = StreamProvider.class)
public class MailStreamProvider implements StreamProvider {

(Sorry, can't provide a PR for that, maven compilation for angus-mail doesn't work.)

@jbescos
Copy link
Member

jbescos commented Oct 25, 2022

Thank you for the PR.

The solution looks nice, as it very easily deal with OSGi.

I am curious to know how adding some depenencies it is able to modify the MANIFEST.MF. I guess maven-bundle-plugin:5.1.1:manifest is detecting these dependencies and it is delegating on them.

The only thing that I don't like is that if you decompile Session or StreamProvider, you are going to find the ServiceConsumer annotation, and also in the module-info. As this is only needed to generate the manfest, it looks too intrusive to find it in the generated jar.

Ideally there should be a plugin to deal with the manifest. Something like this:

<plugin>
	<groupId>org.apache.maven.plugins</groupId>
	<artifactId>maven-osgi-plugin</artifactId>
	<configuration>
		<jakarta.mail.util.StreamProvider>jakarta.mail.Provider</jakarta.mail.util.StreamProvider>
		<jakarta.mail.util.StreamProvider>jakarta.mail.util.StreamProvider</jakarta.mail.util.StreamProvider>
	</configuration>
</plugin>

I bring the attention of @lukasj. He has been dealing with OSGi more time than me, in other words, he knows more than me :).

@mnlipp
Copy link
Contributor Author

mnlipp commented Oct 25, 2022

Ideally there should be a plugin to deal with the manifest.

You don't need an extra plugin. You can simply add this information using the Felix maven bundle plugin which is already in pom.xml with instructions <Provide-Capability> and <Require-Capability> (that's doing it "manually", see #630 (comment)). In general, the manual approach is considered a bit more difficult to maintain, but once it's there, it rarely changes (only if you add another loaded service).

If it helps, I can provide another PR with the pom.xml based solution.

@@ -198,6 +198,7 @@
* @author Max Spivak
*/

@aQute.bnd.annotation.spi.ServiceConsumer(value = Provider.class)
Copy link
Contributor

@lukasj lukasj Nov 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is currently a restriction on using 3rd party libraries in the APIs and only Java SE/Jakarta EE packages can be used.

Copy link
Contributor

@lukasj lukasj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is currently not allowed to depend on 3rd party libraries from APIs

@mnlipp
Copy link
Contributor Author

mnlipp commented Nov 2, 2022

It is currently not allowed to depend on 3rd party libraries from APIs

This is why this has actually been superseded by #633.

@lukasj
Copy link
Contributor

lukasj commented Nov 2, 2022

so this should have been closed then

@lukasj lukasj closed this Nov 2, 2022
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

Successfully merging this pull request may close these issues.

3 participants