[java-shib-shared] branch main updated: JSSH-5 - ServiceableComponent should implement AutoClose
Scott Cantor
cantor.2 at osu.edu
Mon Nov 28 18:11:05 UTC 2022
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-shib-shared.
View the commit online:
http://git.shibboleth.net/view/?p=java-shib-shared.git;a=commit;h=d26652a6facfce25b685e2b6df29d4c870c364ca
The following commit(s) were added to refs/heads/main by this push:
new d26652a6 JSSH-5 - ServiceableComponent should implement AutoClose
d26652a6 is described below
commit d26652a6facfce25b685e2b6df29d4c870c364ca
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Nov 28 13:11:01 2022 -0500
JSSH-5 - ServiceableComponent should implement AutoClose
https://shibboleth.atlassian.net/browse/JSSH-5
Fix some bugs and convert getServiceableComponent to non-null.
---
.../shared/service/ReloadableService.java | 12 +++++---
.../shared/service/impl/LogbackLoggingService.java | 5 ++-
.../impl/DelegatingAccessControlService.java | 27 ++--------------
.../service/AbstractServiceableComponent.java | 24 +++++++--------
.../service/ApplicationContextServiceStrategy.java | 14 ++++-----
.../spring/service/ClassBasedServiceStrategy.java | 9 +++++-
.../spring/service/ReloadableSpringService.java | 8 +++--
.../service/impl/SpringServiceableComponent.java | 16 ++--------
.../service/ReloadableSpringServiceTest.java | 36 +++++++++++++++++-----
9 files changed, 76 insertions(+), 75 deletions(-)
diff --git a/shib-service/src/main/java/net/shibboleth/shared/service/ReloadableService.java b/shib-service/src/main/java/net/shibboleth/shared/service/ReloadableService.java
index 331324f6..4e6df708 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/service/ReloadableService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/service/ReloadableService.java
@@ -19,6 +19,7 @@ package net.shibboleth.shared.service;
import java.time.Instant;
+import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import net.shibboleth.shared.component.InitializableComponent;
@@ -63,12 +64,15 @@ public interface ReloadableService<T> extends InitializableComponent {
/**
* Get the serviceable component that this service supports. If the component hasn't been successfully
- * loaded yet or if this service does not support a ServiceableComponent, null is returned. On a non-null
- * value, the returned component will be pinned and <em>MUST</em> be closed. This can be done
- * by exploiting the fact that a ServiceableComponent implements {@link AutoCloseable}
+ * loaded yet or if this service does not support a ServiceableComponent, a {@link ServiceException} is raised.
+ *
+ * <p>On a non-null value, the returned component will be pinned and <em>MUST</em> be closed. This can be done
+ * by exploiting the fact that a ServiceableComponent implements {@link AutoCloseable}</p>
*
* @return the component, if appropriate.
+ *
+ * @throws ServiceException if the component is not available
*/
- @Nullable ServiceableComponent<T> getServiceableComponent();
+ @Nonnull ServiceableComponent<T> getServiceableComponent() throws ServiceException;
}
\ No newline at end of file
diff --git a/shib-service/src/main/java/net/shibboleth/shared/service/impl/LogbackLoggingService.java b/shib-service/src/main/java/net/shibboleth/shared/service/impl/LogbackLoggingService.java
index 24e0cd49..e7391115 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/service/impl/LogbackLoggingService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/service/impl/LogbackLoggingService.java
@@ -110,9 +110,8 @@ public class LogbackLoggingService extends AbstractReloadableService<Object>
*
* This service does not support a ServiceableComponent, so return null.
*/
- @Override
- @Nullable public ServiceableComponent<Object> getServiceableComponent() {
- return null;
+ @Nonnull public ServiceableComponent<Object> getServiceableComponent() {
+ throw new ServiceException("LoggingService does not expose a serviceable component");
}
/** {@inheritDoc} */
diff --git a/shib-service/src/main/java/net/shibboleth/shared/service/security/impl/DelegatingAccessControlService.java b/shib-service/src/main/java/net/shibboleth/shared/service/security/impl/DelegatingAccessControlService.java
index fd446ff3..b4561888 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/service/security/impl/DelegatingAccessControlService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/service/security/impl/DelegatingAccessControlService.java
@@ -18,12 +18,7 @@
package net.shibboleth.shared.service.security.impl;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import jakarta.servlet.ServletRequest;
import net.shibboleth.shared.annotation.ParameterName;
import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
import net.shibboleth.shared.logic.Constraint;
@@ -39,11 +34,8 @@ import net.shibboleth.shared.service.ServiceableComponent;
public class DelegatingAccessControlService extends AbstractIdentifiableInitializableComponent
implements AccessControlService {
- /** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(DelegatingAccessControlService.class);
-
/** The service which manages the reloading. */
- private final ReloadableService<AccessControlService> service;
+ @Nonnull private final ReloadableService<AccessControlService> service;
/**
* Constructor.
@@ -61,21 +53,8 @@ public class DelegatingAccessControlService extends AbstractIdentifiableInitiali
checkComponentActive();
try (final ServiceableComponent<AccessControlService> component = service.getServiceableComponent()){
- if (null == component) {
- log.error("AccessControlService '{}': Error accessing underlying component: Invalid configuration.",
- getId());
- } else {
- final AccessControlService svc = component.getComponent();
- return svc.getInstance(name);
- }
+ return component.getComponent().getInstance(name);
}
-
- return new AccessControl() {
- public boolean checkAccess(@Nonnull final ServletRequest request, @Nullable final String operation,
- @Nullable final String resource) {
- return false;
- }
- };
}
-}
+}
\ No newline at end of file
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 e93ca29a..01586e93 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
@@ -76,7 +76,7 @@ public abstract class AbstractServiceableComponent<T> extends AbstractIdentifiab
/**
* {@inheritDoc}.
*/
- @Override @Nonnull public abstract T getComponent();
+ @Nonnull public abstract T getComponent();
/**
* Grab the service lock shared. This will block unloads until {@link #unpinComponent()} is called.
@@ -90,7 +90,6 @@ public abstract class AbstractServiceableComponent<T> extends AbstractIdentifiab
serviceLock.readLock().unlock();
}
- @Override
public void close() {
unpinComponent();
}
@@ -123,15 +122,6 @@ public abstract class AbstractServiceableComponent<T> extends AbstractIdentifiab
destroy();
}
- /**
- * {@inheritDoc}. 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() {
- unloadComponent();
- super.doDestroy();
- }
-
/** {@inheritDoc} */
@Override protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
@@ -145,4 +135,14 @@ public abstract class AbstractServiceableComponent<T> extends AbstractIdentifiab
}
}
-}
+
+ /**
+ * {@inheritDoc}. 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() {
+ unloadComponent();
+ super.doDestroy();
+ }
+
+}
\ No newline at end of file
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ApplicationContextServiceStrategy.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ApplicationContextServiceStrategy.java
index cf1700b2..1ebb0276 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ApplicationContextServiceStrategy.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ApplicationContextServiceStrategy.java
@@ -22,11 +22,10 @@ import java.util.function.Function;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.springframework.context.ApplicationContext;
import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.service.ServiceException;
/**
* Strategy for summoning up an {@link ApplicationContextServiceableComponent} wrapper
@@ -36,12 +35,9 @@ import net.shibboleth.shared.component.ComponentInitializationException;
*/
public class ApplicationContextServiceStrategy implements
Function<ApplicationContext, AbstractServiceableComponent<ApplicationContext>> {
-
- /** Logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(ApplicationContextServiceStrategy.class);
/** {@inheritDoc} */
- @Nullable public AbstractServiceableComponent<ApplicationContext> apply(@Nullable final ApplicationContext appContext) {
+ @Nonnull public AbstractServiceableComponent<ApplicationContext> apply(@Nullable final ApplicationContext appContext) {
if (appContext != null) {
final ApplicationContextServiceableComponent wrapper = new ApplicationContextServiceableComponent();
@@ -56,10 +52,12 @@ public class ApplicationContextServiceStrategy implements
wrapper.initialize();
return wrapper;
} catch (final ComponentInitializationException e) {
- log.error("Unable to initialize component wrapper for ApplicationContext", e);
+ throw new ServiceException("Unable to initialize component wraper for ApplicationContext "
+ + appContext.getDisplayName(), e);
}
+ } else {
+ throw new ServiceException("Unable to initialize component wraper for absent ApplicationContext");
}
- return null;
}
}
\ No newline at end of file
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ClassBasedServiceStrategy.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ClassBasedServiceStrategy.java
index 32c04d91..7e9a2858 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ClassBasedServiceStrategy.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ClassBasedServiceStrategy.java
@@ -26,6 +26,7 @@ import javax.annotation.Nullable;
import org.springframework.context.ApplicationContext;
import net.shibboleth.shared.annotation.ParameterName;
+import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.service.ServiceException;
import net.shibboleth.shared.spring.service.impl.SpringServiceableComponent;
@@ -67,8 +68,14 @@ public class ClassBasedServiceStrategy<T> implements Function<ApplicationContext
}
final T value = components.iterator().next();
assert value != null;
- final SpringServiceableComponent<T> result = new SpringServiceableComponent<>(value);
+ final SpringServiceableComponent<T> result = new SpringServiceableComponent<>(value);
result.setApplicationContext(appContext);
+ try {
+ result.initialize();
+ } catch (final ComponentInitializationException e) {
+ throw new ServiceException("Unable to initialize service type " + serviceClaz.getName() + " from "
+ + appContext.getDisplayName(), e);
+ }
return result;
}
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 3c044fb5..b136b0ff 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
@@ -442,18 +442,22 @@ public class ReloadableSpringService<T> extends AbstractReloadableService<T> imp
}
/**
+ * {@inheritDoc}
+ *
* Get the serviceable component. We do this under interlock and grab the lock on the component.
*
* @return the <em>pinned</em> component.
*/
@Override
- @Nullable public synchronized AbstractServiceableComponent<T> getServiceableComponent() {
+ @Nonnull public synchronized AbstractServiceableComponent<T> getServiceableComponent() throws ServiceException {
if (null != cachedComponent) {
cachedComponent.pinComponent();
+ assert cachedComponent != null;
+ return cachedComponent;
}
- return cachedComponent;
+ throw new ServiceException("Service component " + getId() + " is unavailable");
}
/** {@inheritDoc} */
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/impl/SpringServiceableComponent.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/impl/SpringServiceableComponent.java
index 6f2e3931..c3ed2dc1 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/impl/SpringServiceableComponent.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/impl/SpringServiceableComponent.java
@@ -20,9 +20,7 @@ package net.shibboleth.shared.spring.service.impl;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
-import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.component.IdentifiableComponent;
-import net.shibboleth.shared.component.InitializableComponent;
import net.shibboleth.shared.service.ServiceableComponent;
import net.shibboleth.shared.spring.service.AbstractServiceableComponent;
@@ -55,18 +53,8 @@ public class SpringServiceableComponent<T> extends AbstractServiceableComponent<
/** {@inheritDoc} */
@Override
- @Nonnull
- public T getComponent() {
+ @Nonnull public T getComponent() {
return theComponent;
}
- /** {@inheritDoc} */
- @Override
- protected void doInitialize() throws ComponentInitializationException {
- pinComponent();
- if (theComponent instanceof InitializableComponent) {
- ((InitializableComponent)theComponent).initialize();
- }
- unpinComponent();
- }
-}
+}
\ No newline at end of file
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 daa58856..d2d7bf05 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
@@ -28,6 +28,7 @@ import java.util.Collections;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import net.shibboleth.shared.service.ServiceException;
import net.shibboleth.shared.service.ServiceableComponent;
import net.shibboleth.shared.spring.util.ApplicationContextBuilder;
@@ -201,12 +202,24 @@ public class ReloadableSpringServiceTest {
} catch (final BeanInitializationException e) {
// OK
}
- Assert.assertNull(service.getServiceableComponent());
+
+ try {
+ service.getServiceableComponent();
+ Assert.fail("Expected to fail");
+ } catch (final ServiceException e) {
+ // OK
+ }
overwriteFileWith("net/shibboleth/shared/spring/service/ServiceableBean2.xml");
Thread.sleep(RELOAD_DELAY.toMillis() * 2);
- Assert.assertNull(service.getServiceableComponent());
+
+ try {
+ service.getServiceableComponent();
+ Assert.fail("Expected to fail");
+ } catch (final ServiceException e) {
+ // OK
+ }
service.stop();
deleteFile();
@@ -223,18 +236,27 @@ public class ReloadableSpringServiceTest {
service.setServiceConfigurations(Collections.singletonList(testFileResource()));
service.start();
- Assert.assertNull(service.getServiceableComponent());
+
+ try {
+ service.getServiceableComponent();
+ Assert.fail("Expected to fail");
+ } catch (final ServiceException e) {
+ // OK
+ }
overwriteFileWith("net/shibboleth/shared/spring/service/ServiceableBean2.xml");
long count = 700;
- AbstractServiceableComponent<TestServiceableComponent> serviceableComponent = service.getServiceableComponent();
+ AbstractServiceableComponent<TestServiceableComponent> serviceableComponent = null;
while (count > 0 && null == serviceableComponent) {
- Thread.sleep(RELOAD_DELAY.toMillis());
count--;
- serviceableComponent = service.getServiceableComponent();
+ try {
+ serviceableComponent = service.getServiceableComponent();
+ } catch (final ServiceException e) {
+ Thread.sleep(RELOAD_DELAY.toMillis());
+ }
}
- Assert.assertNotNull(serviceableComponent, "After 7 second component has still no initialized");
+ Assert.assertNotNull(serviceableComponent, "After 7 second component has still not initialized");
assert(serviceableComponent != null);
final TestServiceableComponent component = serviceableComponent.getComponent();
Assert.assertEquals(component.getTheValue(), "Two");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list