[java-identity-provider] branch main updated: IDP-1676 DataConnectors: RetryDelay does not respect noPropagate

[Rod Widdowson]

This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=009710ca90c3d0d75704652caa5066f0dc90687e

The following commit(s) were added to refs/heads/main by this push:
       new  009710ca9 IDP-1676 DataConnectors: RetryDelay does not respect noPropagate
009710ca9 is described below

commit 009710ca90c3d0d75704652caa5066f0dc90687e
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed Sep 16 10:27:35 2020 +0100

    IDP-1676 DataConnectors: RetryDelay does not respect noPropagate
    
    https://issues.shibboleth.net/jira/browse/IDP-1676
    
    Add tests to reproduce.  Fix by consulting the DC's propagate setting before
    either throwing or whining and then returning nothing.
---
 .../attribute/resolver/AbstractDataConnector.java  |  1 -
 .../resolver/impl/AttributeResolverImpl.java       |  6 ++-
 .../resolver/impl/AttributeResolverImplTest.java   | 48 ++++++++++++++++++----
 3 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/idp-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/AbstractDataConnector.java b/idp-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/AbstractDataConnector.java
index abb69a80d..a8c6425be 100644
--- a/idp-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/AbstractDataConnector.java
+++ b/idp-attribute-resolver-api/src/main/java/net/shibboleth/idp/attribute/resolver/AbstractDataConnector.java
@@ -167,7 +167,6 @@ public abstract class AbstractDataConnector extends AbstractResolverPlugin<Map<S
         return exportAttributes;
     }
 
-
     /**
      * {@inheritDoc}
      * 
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java
index 26c47437f..d1259a81c 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java
@@ -384,7 +384,11 @@ public class AttributeResolverImpl extends AbstractServiceableComponent<Attribut
                 workContext.recordFailoverResolution(connector, dataConnectors.get(failoverDataConnectorId));
                 return;
             }
-            throw new ResolutionException("Previous resolve failed");
+            if (connector.isPropagateResolutionExceptions()) {
+                throw new ResolutionException("Previous resolve failed");
+            }
+            log.error("Data connector '{}' previously failed but was configured not to propagate");
+            return;
         }
 
         resolveDependencies(connector, resolutionContext);
diff --git a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java
index cb3e91422..b759ce58e 100644
--- a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java
+++ b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java
@@ -336,7 +336,8 @@ public class AttributeResolverImplTest {
         resolver.resolveAttributes(context);
 
         assertTrue(context.getResolvedIdPAttributes().isEmpty());
-        log.debug("Logged Resolve fails");
+        context = new AttributeResolutionContext();
+        resolver.resolveAttributes(context);
     }
 
     /**
@@ -494,13 +495,21 @@ public class AttributeResolverImplTest {
 
     /**
      * Test that resolve w/ dependencies returns the expected results.
-     * 
+     * @param propagate does the data connector propagate
+     * @param addNoRetryDelay Do we defer retry
+     * @param expectException do we?
      * @throws ComponentInitializationException if badness happens
      * @throws ResolutionException if badness happens in attribute resolution
      */
-    @Test public void resolveDataConnectorFail() throws ComponentInitializationException, ResolutionException {
+    private void resolveDataConnectorFail(final boolean propagate,
+            final boolean addNoRetryDelay,
+            final boolean expectException) throws ComponentInitializationException, ResolutionException {
         final MockDataConnector dc1 = new MockDataConnector("dc1", new HashMap<String, IdPAttribute>());
         dc1.setFailure(true);
+        dc1.setPropagateResolutionExceptions(propagate);
+        if (addNoRetryDelay) {
+            dc1.setNoRetryDelay(Duration.ofHours(1));
+        }
         dc1.initialize();
 
         final ResolverDataConnectorDependency dep1 = TestSources.makeDataConnectorDependency("dc1", null);
@@ -518,16 +527,41 @@ public class AttributeResolverImplTest {
         ad1.initialize();
         resolver.initialize();
 
-        final AttributeResolutionContext context = new AttributeResolutionContext();
+        AttributeResolutionContext context = new AttributeResolutionContext();
         try {
             resolver.resolveAttributes(context);
-            fail();
+            assertFalse(expectException, "First Resolve fails");
+            assertTrue(context.getResolvedIdPAttributes().isEmpty());
         } catch (final ResolutionException e) {
-            //
-            // OK
+            assertTrue(expectException, "First Resolve fails");
+        }
+        context = new AttributeResolutionContext();
+        try {
+            resolver.resolveAttributes(context);
+            assertFalse(expectException, "Second Resolve fails");
+            assertTrue(context.getResolvedIdPAttributes().isEmpty());
+        } catch (final ResolutionException e) {
+            assertTrue(expectException, "Second Resolve fails");
         }
     }
 
+    @Test public void resolveDataConnectorFailDefault() throws ComponentInitializationException, ResolutionException {
+        resolveDataConnectorFail(true, false, true);
+    }
+
+    @Test public void resolveDataConnectorFailRetry() throws ComponentInitializationException, ResolutionException {
+        resolveDataConnectorFail(true, true, true);
+    }
+
+    @Test public void resolveDataConnectorFailPropagate() throws ComponentInitializationException, ResolutionException {
+        resolveDataConnectorFail(false, true, false);
+    }
+
+    @Test public void resolveDataConnectorFailPropagateRetry() throws ComponentInitializationException, ResolutionException {
+        resolveDataConnectorFail(false, true, false);
+    }
+
+
     @Test public void cachedDataConnectorDependency() throws ComponentInitializationException, ResolutionException {
         final MockDataConnector dc1 = new MockDataConnector("dc1", (Map<String, IdPAttribute>) null);
         dc1.initialize();

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