search

pf4j / pf4j-spring

Plugin Framework for Spring (PF4J - Spring Framework integration)
Apache License 2.0
338 stars 105 forks source link

Fix for #58: SpringPlugin.stop nulls the application context #59

Closed michael-becker closed 1 year ago

michael-becker commented 3 years ago

I have added the line applicationContext=null to SpringPlugin to fix the exception thron in #58.

In addition, I added a test case in demo/app/src/test/java/org/pf4j/test/demo/SpringPluginTest.java. I hope, the test case is at the right location. If you remove the added line from SpringPlugin, the test case fails.

michael-becker commented 3 years ago

You are absolutely right with the location of the tests. I will move them to pf4j-spring. For this, I would create a simple mock plugin for testing purposes.

michael-becker commented 3 years ago

I see here a possible problem (it's not your fault). The perfect location for test(s) is outside of demo, in pf4j-spring. We have tests for pf4j but we don't have tests for pf4j-spring. What do you think?

Sorry for the late reply. I now finally managed to move the test to the pf4j-spring module. I struggled a little bit to create a runnable demo plugin on the fly. To overcome this and to not add another dependency, I copied the demo-plugin2 jar to the test resources folder and to load the plugin from there. I hope this approach works for you

alegacy commented 1 year ago

@decebals Any progress on this issue? This issue is preventing spring plugins from supporting start -> stop -> start -> etc... lifecycle operations. Since applicationContext is private it cannot (easily) be subclassed locally to address the issue.

decebals commented 1 year ago

@alegacy Sorry for delay. The fix is OK, I don't like the test (jar file in resources folder, ..). For testing I recommend what it's described in https://pf4j.org/doc/testing.html.

For the moment I will supply a quick fix and I will come with the test soon.

decebals commented 1 year ago

@michael-becker Thanks for investigation and solution!

alegacy commented 1 year ago

@decebals Thank you!