[java-shib-attribute] branch main updated: JSSH-5 - ServiceableComponent should implement AutoClose

Scott Cantor cantor.2 at osu.edu
Mon Nov 28 18:11:32 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-attribute.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-attribute.git;a=commit;h=3dc396916e2002ba7ec7a5fef272427d64e678ac

The following commit(s) were added to refs/heads/main by this push:
     new 3dc396916 JSSH-5 - ServiceableComponent should implement AutoClose
3dc396916 is described below

commit 3dc396916e2002ba7ec7a5fef272427d64e678ac
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Nov 28 13:11:29 2022 -0500

    JSSH-5 - ServiceableComponent should implement AutoClose
    
    https://shibboleth.atlassian.net/browse/JSSH-5
    
    Fix some bugs and convert getServiceableComponent to non-null.
---
 .../impl/AttributeFilterServiceStrategy.java       |  6 ++--
 .../impl/AttributeRegistryServiceStrategy.java     | 13 +++-----
 .../context/AttributeResolutionContext.java        | 12 +++----
 .../impl/AttributeResolverServiceStrategy.java     |  4 +--
 .../idp/attribute/resolver/spring/IdP1676Test.java | 38 +++++++++++++++++++---
 .../impl/AttributeMappingNodeProcessor.java        |  4 ---
 6 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/shib-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterServiceStrategy.java b/shib-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterServiceStrategy.java
index f4b127c59..194ae3f09 100644
--- a/shib-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterServiceStrategy.java
+++ b/shib-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterServiceStrategy.java
@@ -20,6 +20,7 @@ package net.shibboleth.idp.attribute.filter.spring.impl;
 import java.util.Collection;
 import java.util.function.Function;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.slf4j.Logger;
@@ -48,7 +49,7 @@ public class AttributeFilterServiceStrategy extends AbstractIdentifiableInitiali
     private final Logger log = LoggerFactory.getLogger(AttributeFilterServiceStrategy.class);
 
     /** {@inheritDoc} */
-    @Nullable
+    @Nonnull
     public ServiceableComponent<AttributeFilter> apply(@Nullable final ApplicationContext appContext) {
 
         if (appContext == null) {
@@ -67,9 +68,10 @@ public class AttributeFilterServiceStrategy extends AbstractIdentifiableInitiali
             result.setApplicationContext(appContext);
             result.setId(getId());
             result.initialize();
+            return result;
         } catch (final ComponentInitializationException e) {
             throw new ServiceException("Unable to initialize attribute filter for " + appContext.getDisplayName(), e);
         }
-        return result;
     }
+
 }
\ No newline at end of file
diff --git a/shib-attribute-impl/src/main/java/net/shibboleth/idp/attribute/transcoding/impl/AttributeRegistryServiceStrategy.java b/shib-attribute-impl/src/main/java/net/shibboleth/idp/attribute/transcoding/impl/AttributeRegistryServiceStrategy.java
index 0251c24e2..993a378fe 100644
--- a/shib-attribute-impl/src/main/java/net/shibboleth/idp/attribute/transcoding/impl/AttributeRegistryServiceStrategy.java
+++ b/shib-attribute-impl/src/main/java/net/shibboleth/idp/attribute/transcoding/impl/AttributeRegistryServiceStrategy.java
@@ -29,8 +29,6 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.opensaml.profile.context.ProfileRequestContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationContext;
 
@@ -50,9 +48,6 @@ import net.shibboleth.shared.spring.service.impl.SpringServiceableComponent;
 public class AttributeRegistryServiceStrategy extends AbstractIdentifiableInitializableComponent implements
         Function<ApplicationContext,AbstractServiceableComponent<AttributeTranscoderRegistry>> {
 
-    /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(AttributeRegistryServiceStrategy.class);
-
     /** Name of bean to supply naming function registry property. */
     @Nullable @NonnullElements private Collection<NamingFunction<?>> namingRegistry;
     
@@ -95,8 +90,9 @@ public class AttributeRegistryServiceStrategy extends AbstractIdentifiableInitia
     }
 
     /** {@inheritDoc} */
-    @Nullable public AbstractServiceableComponent<AttributeTranscoderRegistry> apply(
+    @Nonnull public AbstractServiceableComponent<AttributeTranscoderRegistry> apply(
             @Nullable final ApplicationContext appContext) {
+        checkComponentActive();
         
         if (appContext == null) {
             throw new ServiceException("ApplicationContext was null");
@@ -126,12 +122,13 @@ public class AttributeRegistryServiceStrategy extends AbstractIdentifiableInitia
         try {
             registry.initialize();
             result = new SpringServiceableComponent<>(registry);
+            result.setApplicationContext(appContext);
             result.initialize();
+            return result;
         } catch (final ComponentInitializationException e) {
             throw new ServiceException("Unable to initialize attribute transcoder registry for "
                     + appContext.getDisplayName(), e);
         }
-        return result;
     }
     
-}
+}
\ No newline at end of file
diff --git a/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java b/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java
index 3e03cd50a..1b3c25a5a 100644
--- a/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java
+++ b/shib-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/context/AttributeResolutionContext.java
@@ -46,6 +46,7 @@ import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 
 /** A context supplying input to the {@link net.shibboleth.idp.attribute.resolver.AttributeResolver} interface. */
@@ -334,14 +335,11 @@ public final class AttributeResolutionContext extends BaseContext {
         final Logger log = LoggerFactory.getLogger(AttributeResolutionContext.class);
         try (final ServiceableComponent<AttributeResolver> component
                 = attributeResolverService.getServiceableComponent()) {
-            if (null == component) {
-                log.error("Error resolving attributes: Invalid Attribute resolver configuration");
-            } else {
-                final AttributeResolver attributeResolver = component.getComponent();
-                attributeResolver.resolveAttributes(this);
-            }
-        } catch (final ResolutionException e) {
+            final AttributeResolver attributeResolver = component.getComponent();
+            attributeResolver.resolveAttributes(this);
+        } catch (final ResolutionException|ServiceException e) {
             log.error("Error resolving attributes", e);
         }
     }
+    
 }
\ No newline at end of file
diff --git a/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/impl/AttributeResolverServiceStrategy.java b/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/impl/AttributeResolverServiceStrategy.java
index 0861cb1b6..dabf99b55 100644
--- a/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/impl/AttributeResolverServiceStrategy.java
+++ b/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/impl/AttributeResolverServiceStrategy.java
@@ -67,7 +67,7 @@ public class AttributeResolverServiceStrategy extends AbstractIdentifiableInitia
     }
 
     /** {@inheritDoc} */
-    @Nullable public AbstractServiceableComponent<AttributeResolver> apply(@Nullable final ApplicationContext appContext) {
+    @Nonnull public AbstractServiceableComponent<AttributeResolver> apply(@Nullable final ApplicationContext appContext) {
 
         if (appContext == null) {
             throw new ServiceException("ApplicationContext was null");
@@ -93,10 +93,10 @@ public class AttributeResolverServiceStrategy extends AbstractIdentifiableInitia
             result.setApplicationContext(appContext);
             result.setId(getId());
             result.initialize();
+            return result;
         } catch (final ComponentInitializationException e) {
             throw new ServiceException("Unable to initialize attribute resolver for " + appContext.getDisplayName(), e);
         }
-        return result;
     }
     
 }
\ No newline at end of file
diff --git a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/IdP1676Test.java b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/IdP1676Test.java
index 8e10c948a..be86ebc21 100644
--- a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/IdP1676Test.java
+++ b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/IdP1676Test.java
@@ -18,7 +18,6 @@
 package net.shibboleth.idp.attribute.resolver.spring;
 
 import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
 import static org.testng.Assert.fail;
 
 import java.util.Arrays;
@@ -46,6 +45,7 @@ import net.shibboleth.idp.attribute.resolver.testing.TestSources;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 import net.shibboleth.shared.spring.config.IdentifiableBeanPostProcessor;
 import net.shibboleth.shared.spring.config.StringToDurationConverter;
@@ -153,9 +153,23 @@ public class IdP1676Test extends OpenSAMLInitBaseTestCase {
     @Test public void failFast() throws ComponentInitializationException, ResolutionException {
         connectorOff();
         ReloadableService<AttributeResolver> resolverService = getResolver(true, true);
-        assertNull(resolverService.getServiceableComponent());
+        
+        try {
+            resolverService.getServiceableComponent();
+            fail("Service should not have been available");
+        } catch (final ServiceException e) {
+            // OK
+        }
+        
         connectorOn();
-        assertNull(resolverService.getServiceableComponent());
+        
+        try {
+            resolverService.getServiceableComponent();
+            fail("Service should not have been available");
+        } catch (final ServiceException e) {
+            // OK
+        }
+
         resolverService = getResolver(true, true);
         testResolve(resolverService, 7);
         connectorOff();
@@ -170,9 +184,23 @@ public class IdP1676Test extends OpenSAMLInitBaseTestCase {
     @Test public void failFastNoPE() throws ComponentInitializationException, ResolutionException {
         connectorOff();
         ReloadableService<AttributeResolver> resolverService = getResolver(true, false);
-        assertNull(resolverService.getServiceableComponent());
+        
+        try {
+            resolverService.getServiceableComponent();
+            fail("Service should not have been available");
+        } catch (final ServiceException e) {
+            // OK
+        }
+
         connectorOn();
-        assertNull(resolverService.getServiceableComponent());
+
+        try {
+            resolverService.getServiceableComponent();
+            fail("Service should not have been available");
+        } catch (final ServiceException e) {
+            // OK
+        }
+
         resolverService = getResolver(true, false);
         testResolve(resolverService, 7);
         connectorOff();
diff --git a/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java b/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java
index 6e9ba19e9..63dc853ad 100644
--- a/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java
+++ b/shib-saml-attribute-impl/src/main/java/net/shibboleth/idp/saml/attribute/impl/AttributeMappingNodeProcessor.java
@@ -99,10 +99,6 @@ public class AttributeMappingNodeProcessor implements MetadataNodeProcessor {
         if (metadataNode instanceof AttributeConsumingService || metadataNode instanceof EntityDescriptor) {
             try (final ServiceableComponent<AttributeTranscoderRegistry> 
                        component = transcoderRegistry.getServiceableComponent()) {
-                if (component == null) {
-                    log.error("Attribute transcoding service unavailable");
-                    return;
-                } 
                 if (metadataNode instanceof AttributeConsumingService) {
                     handleAttributeConsumingService(component.getComponent(), (AttributeConsumingService) metadataNode);
                 } else if (metadataNode instanceof EntityDescriptor) {

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list