pf4j-spring icon indicating copy to clipboard operation
pf4j-spring copied to clipboard

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

Open michael-becker opened this issue 4 years ago • 2 comments

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 avatar Nov 16 '20 23:11 michael-becker

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 avatar Nov 17 '20 11:11 michael-becker

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

michael-becker avatar Dec 08 '20 20:12 michael-becker

@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.

alegacy avatar Feb 01 '23 13:02 alegacy

@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 avatar Feb 02 '23 18:02 decebals

@michael-becker Thanks for investigation and solution!

decebals avatar Feb 02 '23 18:02 decebals

@decebals Thank you!

alegacy avatar Feb 03 '23 22:02 alegacy