Support constructor injection in extensions (implementation of ExtensionPoint)

[m-schroeer]

Hi,

first of all: great framework, great possibilities to override all factory methods for custom needs.

However I encountered a "problem" using pf4j-spring. "Problem", because one could workaround this with ease, but this breaks with conventions.

So what am I talking about?

Background

Dependency injection can be achieved by

• constructor injection
• setter injection or
• field injection.

These are in fact not specific to spring, they're just all ways Java gives us to deal with dependent classes.

Although they seem to be equally qualified, they're not. The convention is to use them in certian situations:

1. Constructor injection, when dependencies are mandatory
2. Setter injection, when dependencies are optional
3. Field injection, not recommended at all (NPEs, testability, see: http://olivergierke.de/2013/11/why-field-injection-is-evil/)

Relation to pf4j-spring

And here we come back to pf4j-spring. Currently pf4j-spring only supports the latter two (setter and field injection) as it calls the default constructor of the extension class right before checking the autowire flag (see code below).

pf4j-spring/pf4j-spring/src/main/java/org/pf4j/spring/SpringExtensionFactory.java

Lines 50 to 52 in d89469f

	public <T> T create(Class<T> extensionClass) {
	T extension = createWithoutSpring(extensionClass);
	if (autowire && extension != null) {

pf4j-spring/pf4j-spring/src/main/java/org/pf4j/spring/SpringExtensionFactory.java

Lines 73 to 75 in d89469f

	protected <T> T createWithoutSpring(Class<?> extensionClass) {
	try {
	return (T) extensionClass.newInstance();

This missing support forces user of this framework to break with conventions and not use constructor injection for mandatory dependencies in classes that implement ExtensionPoint.

Workarounds for the users

The easy way

As a user of pf4j-spring one can avoid using constructor injection for mandatory dependencies. This will break with mentioned convention, but works.

The convention following way
As a user of pf4j-spring one can override <T> T create(Class<T> extensionClass) in anonymous or subclass to support the desired behaviour (did I mention the great extensibility possibilities?). As a down side of this approach each user has to come up with an own solution.

Anyways, it is my opinion that following the convention should be provided by this framework by default.

Anything to code for you?

No, I will provide a pull request.

Sneak peek
As a sneak peek the PR rewrites SpringExtensionFactory.java (but it retains the current overall structure and extensibility) and uses springs beanFactory.autowire(extensionClass, AUTOWIRE_CONSTRUCTOR, ...)to solve the mentioned problem.

It will also slightly change the HelloPlugin of demo plugin2 in order to demonstrate the new possibiliy.

---

Stay tuned for updates.

Best regards
M Schröer

[Mishin870]

Hello! Came across the same problem while studying this great framework
Awesome solution!
Just one question: are there any downsides to this solution besides potential conflicts with updates of the main repository?
I see it still not merged..

[decebals]

@m-schroeer First of all I'm sorry for delay and thanks for the beautiful description of issue.
I see an interest in the solution supplied by you.
In my projects I don't use Spring with PF4J and I wonder if you see in your solution a possible compatibility issue with the current behavior. The idea is that I want to merge PR as soon as possible, but without breaking something that already works.

[m-schroeer]

Sorry for my delay as well.
First of all I want to thank you for your positive feedback!

It is a while since I got into it in depth but if my memory is correct there should not be any compatibility issues with this.
My initial intention was to support all three ways of injection that Spring offers us (whether they are recommended or not). And because of this I had deeply in mind that my solution to support constructor injection should not break the existing injection possibilties and also does not change or minimize pf4j's extensibility.

For my part, I have tested all three injection options with the code from PR and I am convinced of it.