[java-shib-metadata] branch main updated: Fix up null component handling.

Scott Cantor cantor.2 at osu.edu
Mon Nov 28 18:59:15 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-metadata.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-metadata.git;a=commit;h=22e4a1f7dbf3dfaf5a0050193b517eb91fc7174b

The following commit(s) were added to refs/heads/main by this push:
     new 22e4a1f7 Fix up null component handling.
22e4a1f7 is described below

commit 22e4a1f7dbf3dfaf5a0050193b517eb91fc7174b
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Nov 28 13:59:12 2022 -0500

    Fix up null component handling.
---
 .../impl/MetadataResolverServiceGaugeSet.java      | 38 +++++++--------
 .../metadata/impl/ReloadableMetadataResolver.java  | 55 +++++++++-------------
 .../spring/metadata/EmptyChainService.java         |  3 +-
 .../spring/metadata/InlineMetadataParserTest.java  |  1 -
 .../spring/metadata/MetadataFailFastTest.java      |  9 ++--
 5 files changed, 47 insertions(+), 59 deletions(-)

diff --git a/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java b/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java
index 78676562..9c02c420 100644
--- a/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java
+++ b/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java
@@ -45,6 +45,7 @@ import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.resolver.ResolverException;
 import net.shibboleth.shared.service.ReloadableServiceGaugeSet;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 
 /**
@@ -202,18 +203,18 @@ public class MetadataResolverServiceGaugeSet extends ReloadableServiceGaugeSet<M
     private <T> Map<String,T> valueGetter(final BiConsumer<Builder<String,T>, MetadataResolver> consume) {
         final Builder<String,T> mapBuilder = ImmutableMap.builder();
         try (final ServiceableComponent<?> component = getService().getServiceableComponent()) {
-            if (component != null) {
-                // Check type - just in case
-                if (!(component.getComponent() instanceof MetadataResolver)) {
-                    log.warn("{} : Injected Service was not for an Metadata Resolver : ({}) ",
-                            getLogPrefix(), component.getComponent().getClass());
-                } else {
-                    for (final MetadataResolver resolver : getMetadataResolvers(
-                            (MetadataResolver) component.getComponent())) {
-                        consume.accept(mapBuilder, resolver);
-                    }
+            // Check type - just in case
+            if (!(component.getComponent() instanceof MetadataResolver)) {
+                log.warn("{} : Injected Service was not for an Metadata Resolver : ({}) ",
+                        getLogPrefix(), component.getComponent().getClass());
+            } else {
+                for (final MetadataResolver resolver : getMetadataResolvers(
+                        (MetadataResolver) component.getComponent())) {
+                    consume.accept(mapBuilder, resolver);
                 }
             }
+        } catch (final ServiceException e) {
+            // Nothing to do.
         }
         return mapBuilder.build();
     }
@@ -225,17 +226,16 @@ public class MetadataResolverServiceGaugeSet extends ReloadableServiceGaugeSet<M
         super.doInitialize();
 
         try (final ServiceableComponent<?> component = getService().getServiceableComponent()) {
-            if (component != null) {
-                if (component.getComponent() instanceof MetadataResolver) {
-                    return;
-                }
-                log.error("{} : Injected service was not for a MetadataResolver ({}) ",
-                        getLogPrefix(), component.getClass());
-                throw new ComponentInitializationException("Injected service was not for a MetadataResolver");
+            if (component.getComponent() instanceof MetadataResolver) {
+                return;
             }
+            log.error("{} : Injected service was not for a MetadataResolver ({}) ",
+                    getLogPrefix(), component.getClass());
+            throw new ComponentInitializationException("Injected service was not for a MetadataResolver");
+        } catch (final ServiceException e) {
+            log.debug("{} : Injected service has not initialized sucessfully yet. Skipping type test",
+                    getLogPrefix(), e);
         }
-        log.debug("{} : Injected service has not initialized sucessfully yet. Skipping type test",
-                getLogPrefix());
     }
 
     /** Get all the resolvers rooted in the provider tree (including the root).
diff --git a/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ReloadableMetadataResolver.java b/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ReloadableMetadataResolver.java
index d458357c..b87aee7b 100644
--- a/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ReloadableMetadataResolver.java
+++ b/shib-metadata-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/ReloadableMetadataResolver.java
@@ -34,6 +34,7 @@ import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.resolver.CriteriaSet;
 import net.shibboleth.shared.resolver.ResolverException;
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 
 /**
@@ -63,15 +64,12 @@ public class ReloadableMetadataResolver extends AbstractIdentifiableInitializabl
     @Override @Nonnull public Iterable<EntityDescriptor> resolve(@Nullable final CriteriaSet criteria) throws ResolverException {
         checkComponentActive();
         try (final ServiceableComponent<MetadataResolver> component = service.getServiceableComponent()) {
-            if (null == component) {
-                log.error("RelyingPartyMetadataProvider '{}': Error accessing underlying metadata source: "
-                        + "Invalid configuration.", getId());
-            } else {
-                final MetadataResolver resolver = component.getComponent();
-                return resolver.resolve(criteria);
-            }
+            return component.getComponent().resolve(criteria);
         } catch (final ResolverException e) {
-            log.error("RelyingPartyMetadataProvider '{}': Error during resolution", getId(), e);
+            log.error("ReloadableMetadataResolver '{}': Error during resolution", getId(), e);
+        } catch (final ServiceException e) {
+            log.error("ReloadableMetadataResolver '{}': Error accessing underlying metadata source: "
+                    + "Invalid configuration.", getId(), e);
         }
 
         return Collections.emptySet();
@@ -81,15 +79,12 @@ public class ReloadableMetadataResolver extends AbstractIdentifiableInitializabl
     @Override @Nullable public EntityDescriptor resolveSingle(@Nullable final CriteriaSet criteria) throws ResolverException {
         checkComponentActive();
         try (final ServiceableComponent<MetadataResolver> component = service.getServiceableComponent()) {
-            if (null == component) {
-                log.error("RelyingPartyMetadataProvider '{}': Error accessing underlying metadata source: "
-                        + "Invalid configuration.", getId());
-            } else {
-                final MetadataResolver resolver = component.getComponent();
-                return resolver.resolveSingle(criteria);
-            }
+            return component.getComponent().resolveSingle(criteria);
         } catch (final ResolverException e) {
-            log.error("RelyingPartyResolver '{}': Error during resolution", getId(), e);
+            log.error("ReloadableMetadataResolver '{}': Error during resolution", getId(), e);
+        } catch (final ServiceException e) {
+            log.error("ReloadableMetadataResolver '{}': Error accessing underlying metadata source: "
+                    + "Invalid configuration.", getId(), e);
         }
         return null;
     }
@@ -98,15 +93,12 @@ public class ReloadableMetadataResolver extends AbstractIdentifiableInitializabl
     @Override public boolean isRequireValidMetadata() {
         checkComponentActive();
         try (final ServiceableComponent<MetadataResolver> component = service.getServiceableComponent()) {
-            if (null == component) {
-                log.error("RelyingPartyMetadataProvider '{}': Error accessing underlying metadata source: "
-                        + "Invalid configuration.", getId());
-            } else {
-                final MetadataResolver resolver = component.getComponent();
-                return resolver.isRequireValidMetadata();
-            }
+            return component.getComponent().isRequireValidMetadata();
+        } catch (final ServiceException e) {
+            log.error("ReloadableMetadataResolver '{}': Error accessing underlying metadata source: "
+                    + "Invalid configuration.", getId(), e);
+            throw e;
         }
-        throw new IllegalAccessError("Could not find a valid MetadataResolver");
     }
 
     /** {@inheritDoc} */
@@ -118,20 +110,17 @@ public class ReloadableMetadataResolver extends AbstractIdentifiableInitializabl
     @Override public MetadataFilter getMetadataFilter() {
         checkComponentActive();
         try (final ServiceableComponent<MetadataResolver> component = service.getServiceableComponent()) {
-            if (null == component) {
-                log.error("RelyingPartyMetadataProvider '{}': Error accessing underlying metadata source: "
-                        + "Invalid configuration.", getId());
-            } else {
-                final MetadataResolver resolver = component.getComponent();
-                return resolver.getMetadataFilter();
-            }
+            return component.getComponent().getMetadataFilter();
+        } catch (final ServiceException e) {
+            log.error("ReloadableMetadataResolver '{}': Error accessing underlying metadata source: "
+                    + "Invalid configuration.", getId(), e);
+            throw e;
         }
-        throw new IllegalAccessError("Could not find a valid MetadataResolver");
     }
 
     /** {@inheritDoc} */
     @Override public void setMetadataFilter(@Nullable final MetadataFilter newFilter) {
-        throw new IllegalAccessError("Cannot set Metadata filter");
+        throw new UnsupportedOperationException("Cannot set Metadata filter");
     }
     
 }
\ No newline at end of file
diff --git a/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/EmptyChainService.java b/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/EmptyChainService.java
index c4ee41aa..7a6f821f 100644
--- a/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/EmptyChainService.java
+++ b/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/EmptyChainService.java
@@ -33,10 +33,9 @@ public class EmptyChainService extends AbstractMetadataParserTest {
     @Test public void setup() throws IOException {
         final ReloadableService<RefreshableMetadataResolver> service = getBean(ReloadableService.class, "empty-chain-svc.xml");
         try (final ServiceableComponent<RefreshableMetadataResolver> comp = service.getServiceableComponent()) {
-            assert comp != null;
             final ChainingMetadataResolver chain = (ChainingMetadataResolver) comp.getComponent();
             Assert.assertTrue(chain.getResolvers().isEmpty());
         }
     }
 
-}
+}
\ No newline at end of file
diff --git a/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/InlineMetadataParserTest.java b/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/InlineMetadataParserTest.java
index d2cde888..0c00116d 100644
--- a/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/InlineMetadataParserTest.java
+++ b/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/InlineMetadataParserTest.java
@@ -119,7 +119,6 @@ public class InlineMetadataParserTest extends AbstractMetadataParserTest {
                 context.getBean("shibboleth.MetadataResolverService", ReloadableSpringService.class);
 
         try (final ServiceableComponent<MetadataResolver> msc = ms.getServiceableComponent()){
-            assert msc != null;
             final MetadataResolver resolver = msc.getComponent();
             Assert.assertNotNull(resolver.resolveSingle(criteriaFor(IDP_ID)));
             Assert.assertNotNull(resolver.resolveSingle(criteriaFor(SP_ID)));
diff --git a/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/MetadataFailFastTest.java b/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/MetadataFailFastTest.java
index 39bf5cc8..bb641650 100644
--- a/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/MetadataFailFastTest.java
+++ b/shib-metadata-spring/src/test/java/net/shibboleth/spring/metadata/MetadataFailFastTest.java
@@ -17,8 +17,7 @@
 
 package net.shibboleth.spring.metadata;
 
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
+import static org.testng.Assert.*;
 
 import java.io.IOException;
 import java.util.List;
@@ -29,6 +28,7 @@ import org.testng.annotations.Ignore;
 import org.testng.annotations.Test;
 
 import net.shibboleth.shared.service.ReloadableService;
+import net.shibboleth.shared.service.ServiceException;
 import net.shibboleth.shared.service.ServiceableComponent;
 import net.shibboleth.shared.testing.RepositorySupport;
 import net.shibboleth.spring.testing.AbstractFailFastTest;
@@ -84,7 +84,9 @@ public class MetadataFailFastTest extends AbstractFailFastTest {
         }
         assertNotNull(service);
         try (final ServiceableComponent<MetadataResolver> component = service.getServiceableComponent()) {
-            assertNull(component);
+            fail("Should not have worked");
+        } catch (final ServiceException e) {
+            // OK
         }
     }
 
@@ -108,7 +110,6 @@ public class MetadataFailFastTest extends AbstractFailFastTest {
         final ReloadableService<MetadataResolver > service = (ReloadableService<MetadataResolver>) bean;
         assertNotNull(service);
         try (final ServiceableComponent<MetadataResolver> srv = service.getServiceableComponent()) {
-            assert srv != null;
             final MetadataResolver resolver = srv.getComponent();
             assertNotNull(resolver);
         }

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


More information about the commits mailing list