[java-shib-shared] 01/01: JSSH-71 Remove the impact of the DestructableComponent Interface
Codeberg
noreply at shibboleth.net
Fri May 22 15:24:13 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch dev/JSSH-71
in repository java-shib-shared.
View the commit online:
https://codeberg.org/Shibboleth/java-shib-shared/commit/cee048c9dbcf44d55998c00825d13a486d64b5c1
commit cee048c9dbcf44d55998c00825d13a486d64b5c1
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri May 22 14:22:27 2026 +0100
JSSH-71 Remove the impact of the DestructableComponent Interface
https://shibboleth.atlassian.net/browse/JSSH-71
Convert doDestroy to annotated, per class @PreDestroy method.
Also update ReloadableSpringService.java to do the work (by
loading the appropriate BeanPostprocessor)
In the case of FileCachingHttpClientBuilder, it suffices to annotate
the destroy method of the appropriate subclass
---
shib-networking/pom.xml | 5 +++++
.../httpclient/FileCachingHttpClientBuilder.java | 3 ++-
shib-security/pom.xml | 5 +++++
.../security/impl/BasicKeystoreKeyStrategy.java | 8 ++++----
.../shared/security/impl/ScriptedKeyStrategy.java | 9 ++++-----
shib-service/pom.xml | 5 +++++
.../shared/service/AbstractReloadableService.java | 7 ++++---
.../service/AbstractServiceableComponent.java | 11 ++++++-----
.../spring/service/ReloadableSpringService.java | 22 ++++++++++++++++++----
.../spring/service/NonReloadableTestBean.java | 8 +++-----
.../service/ReloadableSpringServiceTest.java | 22 +++++++++++-----------
.../shared/spring/service/ReloadableTestBean.java | 7 +++----
.../spring/service/TestServiceableComponent.java | 7 ++++++-
13 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/shib-networking/pom.xml b/shib-networking/pom.xml
index db0cac62..8d13ed10 100644
--- a/shib-networking/pom.xml
+++ b/shib-networking/pom.xml
@@ -52,6 +52,11 @@
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>jakarta.annotation</groupId>
+ <artifactId>jakarta.annotation-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
<!-- Runtime Dependencies -->
diff --git a/shib-networking/src/main/java/net/shibboleth/shared/httpclient/FileCachingHttpClientBuilder.java b/shib-networking/src/main/java/net/shibboleth/shared/httpclient/FileCachingHttpClientBuilder.java
index db3c9485..ced7ceb3 100644
--- a/shib-networking/src/main/java/net/shibboleth/shared/httpclient/FileCachingHttpClientBuilder.java
+++ b/shib-networking/src/main/java/net/shibboleth/shared/httpclient/FileCachingHttpClientBuilder.java
@@ -37,6 +37,7 @@ import org.apache.hc.core5.io.CloseMode;
import org.apache.hc.core5.io.ModalCloseable;
import org.slf4j.Logger;
+import jakarta.annotation.PreDestroy;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.AbstractInitializableComponent;
@@ -365,7 +366,7 @@ public class FileCachingHttpClientBuilder extends HttpClientBuilder {
}
/** {@inheritDoc} */
- public void destroy() {
+ @PreDestroy public synchronized void destroy() {
maintenanceTask.cancel();
timer.cancel();
maintenanceTask = null;
diff --git a/shib-security/pom.xml b/shib-security/pom.xml
index 3cd366eb..604eef51 100644
--- a/shib-security/pom.xml
+++ b/shib-security/pom.xml
@@ -65,6 +65,11 @@
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>jakarta.annotation</groupId>
+ <artifactId>jakarta.annotation-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
<!-- Runtime Dependencies -->
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
index 7f8aea78..6475b5c4 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
@@ -36,6 +36,8 @@ import javax.crypto.SecretKey;
import org.slf4j.Logger;
+import jakarta.annotation.PreDestroy;
+
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.collection.Pair;
@@ -270,9 +272,8 @@ public class BasicKeystoreKeyStrategy extends AbstractInitializableComponent imp
}
}
- /** {@inheritDoc} */
- @Override
- protected void doDestroy() {
+ /** Bean-specific function to handle tear down. */
+ @PreDestroy synchronized public void teardown() {
if (updateTask != null) {
updateTask.cancel();
updateTask = null;
@@ -281,7 +282,6 @@ public class BasicKeystoreKeyStrategy extends AbstractInitializableComponent imp
}
internalTaskTimer = null;
}
- super.doDestroy();
}
/** {@inheritDoc} */
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
index 127dbfc7..9f828949 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
@@ -30,6 +30,8 @@ import javax.script.SimpleScriptContext;
import org.slf4j.Logger;
+import jakarta.annotation.PreDestroy;
+
import net.shibboleth.shared.annotation.constraint.NonNegative;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
@@ -193,9 +195,8 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
}
}
- /** {@inheritDoc} */
- @Override
- protected void doDestroy() {
+ /** Bean-specific function to handle tear down. */
+ @PreDestroy synchronized public void teardown() {
if (updateTask != null) {
updateTask.cancel();
updateTask = null;
@@ -204,10 +205,8 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
}
internalTaskTimer = null;
}
- super.doDestroy();
}
-
/** {@inheritDoc} */
@Nonnull public Pair<String,SecretKey> getDefaultKey() throws KeyException {
final NamedKey keyrec = getDefaultKeyRecord();
diff --git a/shib-service/pom.xml b/shib-service/pom.xml
index 1894b8a9..f7c795b2 100644
--- a/shib-service/pom.xml
+++ b/shib-service/pom.xml
@@ -79,6 +79,11 @@
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>jakarta.annotation</groupId>
+ <artifactId>jakarta.annotation-api</artifactId>
+ <scope>provided</scope>
+ </dependency>
<!-- Runtime Dependencies -->
diff --git a/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java b/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java
index 0fe1b2d0..d08ca701 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java
@@ -24,6 +24,8 @@ import javax.annotation.Nullable;
import org.slf4j.Logger;
+import jakarta.annotation.PreDestroy;
+
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
@@ -202,8 +204,8 @@ public abstract class AbstractReloadableService<T> extends AbstractIdentifiableI
}
}
- /** {@inheritDoc} */
- @Override protected void doDestroy() {
+ /** Bean-specific function to handle tear down. */
+ @PreDestroy public synchronized void teardown() {
log.info("{} Starting shutdown", getLogPrefix());
if (reloadTask != null) {
reloadTask.cancel();
@@ -214,7 +216,6 @@ public abstract class AbstractReloadableService<T> extends AbstractIdentifiableI
}
internalTaskTimer = null;
log.info("{} Completing shutdown", getLogPrefix());
- super.doDestroy();
}
/** {@inheritDoc} */
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java
index 68ad231c..8f4cd929 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java
@@ -25,6 +25,8 @@ import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ConfigurableApplicationContext;
+import jakarta.annotation.PreDestroy;
+
import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -139,14 +141,13 @@ public abstract class AbstractServiceableComponent<T> extends AbstractIdentifiab
}
}
-
- /**
- * {@inheritDoc}. Force unload; this will usually be a no-op since the component should have been explicitly
+ /** Bean-specific function to handle tear down.
+ *
+ * Force unload; this will usually be a no-op since the component should have been explicitly
* unloaded, but we do the unload here so that error cases also clean up.
*/
- @Override protected void doDestroy() {
+ @PreDestroy public synchronized void teardown() {
unloadComponent();
- super.doDestroy();
}
}
\ No newline at end of file
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java
index 88c934ec..1e809ccb 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java
@@ -32,10 +32,13 @@ import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
+import org.springframework.context.annotation.CommonAnnotationBeanPostProcessor;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.io.Resource;
+import jakarta.annotation.PreDestroy;
+
import net.shibboleth.shared.annotation.ParameterName;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.ComponentInitializationException;
@@ -126,7 +129,7 @@ public class ReloadableSpringService<T> extends AbstractReloadableService<T> imp
theClaz = Constraint.isNotNull(claz, "Class cannot be null");
serviceStrategy = Constraint.isNotNull(strategy, "Strategy cannot be null");
factoryPostProcessors = CollectionSupport.emptyList();
- postProcessors = CollectionSupport.emptyList();
+ postProcessors = CollectionSupport.singletonList(new CommonAnnotationBeanPostProcessor());
beanProfiles = CollectionSupport.emptyList();
serviceConfigurations = CollectionSupport.emptyList();
}
@@ -234,6 +237,18 @@ public class ReloadableSpringService<T> extends AbstractReloadableService<T> imp
checkSetterPreconditions();
Constraint.isNotNull(processors, "BeanPostProcessor collection cannot be null");
+ // Check that the Annotation processor is there
+ boolean foundProcessor = false;
+ for (BeanPostProcessor bean:processors) {
+ if (bean instanceof CommonAnnotationBeanPostProcessor) {
+ foundProcessor = true;
+ break;
+ }
+ }
+ if (!foundProcessor) {
+ log.warn("Injected collection of bean processors should contain a CommonAnnotationBeanPostProcessor");
+ }
+
postProcessors = CollectionSupport.copyToList(processors);
}
@@ -402,8 +417,8 @@ public class ReloadableSpringService<T> extends AbstractReloadableService<T> imp
log.info("{} Reload complete", getLogPrefix());
}
- /** {@inheritDoc} */
- @Override protected void doDestroy() {
+ /** Bean-specific function to handle tear down. */
+ @PreDestroy public synchronized void teardown() {
final AbstractServiceableComponent<T> oldComponent = cachedComponent;
cachedComponent = null;
// And tear down. Note that we are synchronized on this right now
@@ -412,7 +427,6 @@ public class ReloadableSpringService<T> extends AbstractReloadableService<T> imp
if (null != oldComponent) {
oldComponent.unloadComponent();
}
- super.doDestroy();
}
/**
diff --git a/shib-service/src/test/java/net/shibboleth/shared/spring/service/NonReloadableTestBean.java b/shib-service/src/test/java/net/shibboleth/shared/spring/service/NonReloadableTestBean.java
index 4b4ad70c..28907ccb 100644
--- a/shib-service/src/test/java/net/shibboleth/shared/spring/service/NonReloadableTestBean.java
+++ b/shib-service/src/test/java/net/shibboleth/shared/spring/service/NonReloadableTestBean.java
@@ -19,6 +19,7 @@ import javax.annotation.Nullable;
import org.slf4j.Logger;
+import jakarta.annotation.PreDestroy;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.AbstractInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
@@ -48,11 +49,8 @@ public class NonReloadableTestBean extends AbstractInitializableComponent {
log.debug("NonReloadableTestBean {} initialized", id);
}
- /** {@inheritDoc} */
- protected void doDestroy() {
- log.debug("NonReloadableTestBean {} destroyed", id);
-
- super.doDestroy();
+ @PreDestroy public synchronized void teardown() {
+ log.debug("NonReloadableTestBean {} torn down", id);
}
public ReloadableTestBean getChild() {
diff --git a/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableSpringServiceTest.java b/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableSpringServiceTest.java
index a5ef2faa..bba41854 100644
--- a/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableSpringServiceTest.java
+++ b/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableSpringServiceTest.java
@@ -95,22 +95,22 @@ public class ReloadableSpringServiceTest {
final TestServiceableComponent component = serviceableComponent.getComponent();
Assert.assertEquals("One", component.getTheValue());
- Assert.assertFalse(component.isDestroyed());
+ Assert.assertFalse(component.destroyed);
serviceableComponent.unpinComponent();
overwriteFileWith("net/shibboleth/shared/spring/service/ServiceableBean2.xml");
long count = 70;
- while (count > 0 && !component.isDestroyed()) {
+ while (count > 0 && !component.destroyed) {
Thread.sleep(RELOAD_DELAY.toMillis());
count--;
}
- Assert.assertTrue(component.isDestroyed(), "After 7 second initial component has still not be destroyed");
+ Assert.assertTrue(component.destroyed, "After 7 second initial component has still not be destroyed");
//
// The reload will have destroyed the old component
//
- Assert.assertTrue(serviceableComponent.getComponent().isDestroyed());
+ Assert.assertTrue(((TestServiceableComponent)(serviceableComponent.getComponent())).destroyed);
serviceableComponent = service.getServiceableComponent();
assert(serviceableComponent != null);
@@ -144,7 +144,7 @@ public class ReloadableSpringServiceTest {
Assert.assertEquals(x, service.getLastSuccessfulReloadInstant());
Assert.assertEquals(component.getTheValue(), "One");
- Assert.assertFalse(component.isDestroyed());
+ Assert.assertFalse(component.destroyed);
Thread.sleep(RELOAD_DELAY.toMillis() * 3);
Assert.assertEquals(x, service.getLastReloadAttemptInstant());
@@ -154,7 +154,7 @@ public class ReloadableSpringServiceTest {
//
// The reload will not have destroyed the old component yet
//
- Assert.assertFalse(component.isDestroyed());
+ Assert.assertFalse(component.destroyed);
long count = 70;
TestServiceableComponent component2 = null;
@@ -173,11 +173,11 @@ public class ReloadableSpringServiceTest {
Assert.assertNotNull(component2, "After 7 second initial component has still not got new value");
count = 70;
- while (count > 0 && !component.isDestroyed()) {
+ while (count > 0 && !component.destroyed) {
Thread.sleep(RELOAD_DELAY.toMillis());
count--;
}
- Assert.assertTrue(component.isDestroyed(), "After 7 second initial component has still not be destroyed");
+ Assert.assertTrue(component.destroyed, "After 7 second initial component has still not be destroyed");
service.destroy();
deleteFile();
}
@@ -258,16 +258,16 @@ public class ReloadableSpringServiceTest {
final TestServiceableComponent component = serviceableComponent.getComponent();
Assert.assertEquals(component.getTheValue(), "Two");
- Assert.assertFalse(component.isDestroyed());
+ Assert.assertFalse(component.destroyed);
serviceableComponent.unpinComponent();
service.destroy();
count = 70;
- while (count > 0 && !component.isDestroyed()) {
+ while (count > 0 && !component.destroyed) {
Thread.sleep(RELOAD_DELAY.toMillis());
count--;
}
- Assert.assertTrue(component.isDestroyed(), "After 7 seconds component has still not be destroyed");
+ Assert.assertTrue(component.destroyed, "After 7 seconds component has still not be destroyed");
deleteFile();
}
diff --git a/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableTestBean.java b/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableTestBean.java
index 6c016eba..e6f76804 100644
--- a/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableTestBean.java
+++ b/shib-service/src/test/java/net/shibboleth/shared/spring/service/ReloadableTestBean.java
@@ -18,6 +18,7 @@ import javax.annotation.Nonnull;
import org.slf4j.Logger;
+import jakarta.annotation.PreDestroy;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.AbstractInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
@@ -53,11 +54,9 @@ public class ReloadableTestBean extends AbstractInitializableComponent {
log.debug("ReloadableTestBean {} initialized", id);
}
- /** {@inheritDoc} */
- protected void doDestroy() {
- log.debug("ReloadableTestBean {} destroyed", id);
+ @PreDestroy public synchronized void teardown() {
+ log.debug("ReloadableTestBean {} torn down", id);
- super.doDestroy();
}
/**
diff --git a/shib-service/src/test/java/net/shibboleth/shared/spring/service/TestServiceableComponent.java b/shib-service/src/test/java/net/shibboleth/shared/spring/service/TestServiceableComponent.java
index 44f1c12b..2caeab2b 100644
--- a/shib-service/src/test/java/net/shibboleth/shared/spring/service/TestServiceableComponent.java
+++ b/shib-service/src/test/java/net/shibboleth/shared/spring/service/TestServiceableComponent.java
@@ -14,13 +14,14 @@
package net.shibboleth.shared.spring.service;
+import jakarta.annotation.PreDestroy;
import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
@SuppressWarnings("javadoc")
public class TestServiceableComponent extends AbstractIdentifiableInitializableComponent {
private String theValue;
-
+ public boolean destroyed = false;
/**
* @return Returns the theValue.
@@ -36,4 +37,8 @@ public class TestServiceableComponent extends AbstractIdentifiableInitializableC
theValue = value;
}
+ @PreDestroy
+ synchronized public void teardown() {
+ destroyed = true;
+ }
}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list