[java-identity-provider] branch master updated: IDP-1168 - Collapse duplicated scripted implementations

Scott Cantor cantor.2 at osu.edu
Thu May 4 21:31:07 EDT 2017


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

scantor pushed a commit to branch master
in repository java-identity-provider.

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

The following commit(s) were added to refs/heads/master by this push:
       new  4d2ab0a   IDP-1168 -  Collapse duplicated scripted implementations
4d2ab0a is described below

commit 4d2ab0a16b5853675a8ca50e6bfa3049b3f7ef9c
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu May 4 21:31:04 2017 -0400

    IDP-1168 -  Collapse duplicated scripted implementations
    
    https://issues.shibboleth.net/jira/browse/IDP-1168
    
    Re-base attribute resolver plugins, and some addtl. cleanup.
---
 .../filter/matcher/impl/ScriptedMatcher.java       |  10 +-
 .../filter/policyrule/impl/ScriptedPolicyRule.java |  10 +-
 .../ad/impl/ScriptedAttributeDefinition.java       | 192 ++++++++------
 .../resolver/dc/impl/ScriptedDataConnector.java    | 275 ++++++++++++---------
 .../dc/impl/ScriptedDataConnectorTest.java         |  15 ++
 .../idp/attribute/resolver/impl/dc/error.js        |   1 +
 .../idp/attribute/resolver/impl/dc/v8/error.js     |   1 +
 7 files changed, 294 insertions(+), 210 deletions(-)

diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java
index 02a8aa5..001eb13 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/ScriptedMatcher.java
@@ -107,6 +107,7 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
      */
     public void setCustomObject(@Nullable final Object object) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
         customObject = object;
     }
@@ -127,6 +128,7 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
      */
     public void setScript(@Nonnull final EvaluableScript matcherScript) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         script = Constraint.isNotNull(matcherScript, "Attribute value matching script cannot be null");
     }
@@ -141,6 +143,7 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
     public void setProfileRequestContextLookupStrategy(
             @Nonnull final Function<AttributeFilterContext, ProfileRequestContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         prcLookupStrategy = Constraint.isNotNull(strategy, "ProfileRequestContext lookup strategy cannot be null");
     }
@@ -155,6 +158,7 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
     public void setSubjectContextLookupStrategy(
             @Nonnull final Function<ProfileRequestContext, SubjectContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         scLookupStrategy = Constraint.isNotNull(strategy, "SubjectContext lookup strategy cannot be null");
     }
@@ -244,12 +248,6 @@ public class ScriptedMatcher extends AbstractIdentifiableInitializableComponent
             // Turn ScriptException result into default "error" result, which is left at null.
             setHideExceptions(true);
         }
-        
-        /** {@inheritDoc} */
-        @Override
-        @Nullable public Object getCustomObject() {
-            return super.getCustomObject();
-        }
 
         /**
          * Execution hook.
diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java
index d383d50..b52574f 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/impl/ScriptedPolicyRule.java
@@ -101,6 +101,7 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
      */
     public void setCustomObject(@Nullable final Object object) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
         customObject = object;
     }
@@ -121,6 +122,7 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
      */
     public void setScript(@Nonnull final EvaluableScript matcherScript) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         script = Constraint.isNotNull(matcherScript, "Attribute value matching script can not be null");
     }
@@ -135,6 +137,7 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
     public void setProfileRequestContextLookupStrategy(
             @Nonnull final Function<AttributeFilterContext, ProfileRequestContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         prcLookupStrategy = Constraint.isNotNull(strategy, "ProfileRequestContext lookup strategy cannot be null");
     }
@@ -149,6 +152,7 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
     public void
             setSubjectContextLookupStrategy(@Nonnull final Function<ProfileRequestContext, SubjectContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         scLookupStrategy = Constraint.isNotNull(strategy, "SubjectContext lookup strategy cannot be null");
     }
@@ -239,12 +243,6 @@ public class ScriptedPolicyRule extends AbstractIdentifiableInitializableCompone
             setHideExceptions(true);
         }
         
-        /** {@inheritDoc} */
-        @Override
-        @Nullable public Object getCustomObject() {
-            return super.getCustomObject();
-        }
-
         /**
          * Execution hook.
          * 
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/ScriptedAttributeDefinition.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/ScriptedAttributeDefinition.java
index bbfcbec..63b2204 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/ScriptedAttributeDefinition.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/ScriptedAttributeDefinition.java
@@ -26,7 +26,6 @@ import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 import javax.script.ScriptContext;
 import javax.script.ScriptException;
-import javax.script.SimpleScriptContext;
 import javax.security.auth.Subject;
 
 import net.shibboleth.idp.attribute.IdPAttribute;
@@ -41,6 +40,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterI
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.scripting.AbstractScriptEvaluator;
 import net.shibboleth.utilities.java.support.scripting.EvaluableScript;
 
 import org.opensaml.messaging.context.navigate.ChildContextLookup;
@@ -82,6 +82,9 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
 
     /** Script to be evaluated. */
     @NonnullAfterInit private EvaluableScript script;
+    
+    /** Evaluator. */
+    @NonnullAfterInit private AttributeDefinitionScriptEvaluator scriptEvaluator;
 
     /** Strategy used to locate the {@link ProfileRequestContext} to use. */
     @Nonnull private Function<AttributeResolutionContext, ProfileRequestContext> prcLookupStrategy;
@@ -96,7 +99,7 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
     public ScriptedAttributeDefinition() {
         // Defaults to ProfileRequestContext -> AttributeContext.
         prcLookupStrategy = new ParentContextLookup<>();
-        scLookupStrategy = new ChildContextLookup<ProfileRequestContext, SubjectContext>(SubjectContext.class);
+        scLookupStrategy = new ChildContextLookup<>(SubjectContext.class);
     }
 
     /**
@@ -114,6 +117,9 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
      * @param object the custom object
      */
     @Nullable public void setCustomObject(final Object object) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
         customObject = object;
     }
 
@@ -148,6 +154,7 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
     public void setProfileRequestContextLookupStrategy(
             @Nonnull final Function<AttributeResolutionContext, ProfileRequestContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         prcLookupStrategy = Constraint.isNotNull(strategy, "ProfileRequestContext lookup strategy cannot be null");
     }
@@ -162,6 +169,7 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
     public void
             setSubjectContextLookupStrategy(@Nonnull final Function<ProfileRequestContext, SubjectContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         scLookupStrategy = Constraint.isNotNull(strategy, "SubjectContext lookup strategy cannot be null");
     }
@@ -173,6 +181,10 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
         if (null == script) {
             throw new ComponentInitializationException(getLogPrefix() + " no script was configured");
         }
+        
+        scriptEvaluator = new AttributeDefinitionScriptEvaluator(script);
+        scriptEvaluator.setCustomObject(customObject);
+        scriptEvaluator.setLogPrefix(getLogPrefix());
     }
 
     /** {@inheritDoc} */
@@ -182,97 +194,123 @@ public class ScriptedAttributeDefinition extends AbstractAttributeDefinition {
         Constraint.isNotNull(resolutionContext, "AttributeResolutionContext cannot be null");
         Constraint.isNotNull(workContext, "AttributeResolverWorkContext cannot be null");
 
-        final ScriptContext context = getScriptContext(resolutionContext, workContext);
+        return scriptEvaluator.execute(resolutionContext, workContext);
+    }
 
-        try {
-            script.eval(context);
-        } catch (final ScriptException e) {
-            throw new ResolutionException(getLogPrefix() + " unable to execute script", e);
+    /**
+     * Evaluator bound to the AttributeDefinition semantic.
+     */
+    private class AttributeDefinitionScriptEvaluator extends AbstractScriptEvaluator {
+
+        /**
+         * Constructor.
+         * 
+         * @param theScript the script we will evaluate.
+         */
+        public AttributeDefinitionScriptEvaluator(@Nonnull final EvaluableScript theScript) {
+            super(theScript);
         }
-        final Object result = context.getAttribute(getId());
 
-        if (null == result) {
-            log.info("{} no value returned", getLogPrefix());
-            return null;
+        /**
+         * Execution hook.
+         * 
+         * @param resolutionContext resolution context
+         * @param workContext work context
+         * 
+         * @return script result
+         * @throws ResolutionException if the script fails
+         */
+        @Nullable protected IdPAttribute execute(@Nonnull final AttributeResolutionContext resolutionContext,
+                @Nonnull final AttributeResolverWorkContext workContext) throws ResolutionException {
+            try {
+                return (IdPAttribute) evaluate(resolutionContext, workContext);
+            } catch (final RuntimeException e) {
+                throw new ResolutionException(getLogPrefix() + "Script did not run successfully", e);
+            }
         }
 
-        if (result instanceof ScriptedIdPAttributeImpl) {
-
-            final ScriptedIdPAttributeImpl scriptedAttribute = (ScriptedIdPAttributeImpl) result;
-            return scriptedAttribute.getResultingAttribute();
+        /** {@inheritDoc} */
+        @Override
+        protected void prepareContext(@Nonnull final ScriptContext scriptContext, @Nullable final Object... input) {
 
-        } else {
+            final Map<String, List<IdPAttributeValue<?>>> dependencyAttributes =
+                    PluginDependencySupport.getAllAttributeValues(
+                            (AttributeResolverWorkContext) input[1], getDependencies());
 
-            throw new ResolutionException(getLogPrefix() + " returned variable was of wrong type ("
-                    + result.getClass().toString() + ")");
-        }
-
-    }
-
-    /**
-     * Constructs the {@link ScriptContext} used when evaluating the script.
-     * 
-     * @param resolutionContext current resolution context
-     * @param workContext current work context
-     * 
-     * @return constructed script context
-     * @throws ResolutionException thrown if dependent data connectors or attribute definitions can not be resolved
-     */
-    @SuppressWarnings("deprecation") @Nonnull private ScriptContext getScriptContext(
-            @Nonnull final AttributeResolutionContext resolutionContext,
-            @Nonnull final AttributeResolverWorkContext workContext) throws ResolutionException {
-
-        final SimpleScriptContext scriptContext = new SimpleScriptContext();
-        final Map<String, List<IdPAttributeValue<?>>> dependencyAttributes =
-                PluginDependencySupport.getAllAttributeValues(workContext, getDependencies());
+            if (dependencyAttributes.containsKey(getId())) {
+                log.debug("{} The attribute ID to be populated is a dependency, not created", getLogPrefix());
+            } else {
+                log.debug("{} Adding to-be-populated attribute to script context", getLogPrefix());
+                final IdPAttribute newAttribute = new IdPAttribute(getId());
+                scriptContext.setAttribute(getId(), new ScriptedIdPAttributeImpl(newAttribute, getLogPrefix()),
+                        ScriptContext.ENGINE_SCOPE);
+            }
 
-        if (dependencyAttributes.containsKey(getId())) {
-            log.debug("{} to-be-populated attribute is a dependency.  Not created", getLogPrefix());
-        } else {
-            log.debug("{} adding to-be-populated attribute to script context", getLogPrefix());
-            final IdPAttribute newAttribute = new IdPAttribute(getId());
-            scriptContext.setAttribute(getId(), new ScriptedIdPAttributeImpl(newAttribute, getLogPrefix()),
+            log.debug("{} Adding contexts to script context", getLogPrefix());
+            scriptContext.setAttribute("resolutionContext", input[0], ScriptContext.ENGINE_SCOPE);
+            scriptContext.setAttribute("workContext",
+                    new DelegatedWorkContext((AttributeResolverWorkContext) input[1], getLogPrefix()),
                     ScriptContext.ENGINE_SCOPE);
-        }
+            scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
+            
+            final ProfileRequestContext prc = prcLookupStrategy.apply((AttributeResolutionContext) input[0]);
+            if (null == prc) {
+                log.error("{} ProfileRequestContext could not be located", getLogPrefix());
+            }
+            scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
 
-        log.debug("{} adding contexts to script context", getLogPrefix());
-        scriptContext.setAttribute("resolutionContext", resolutionContext, ScriptContext.ENGINE_SCOPE);
-        scriptContext.setAttribute("workContext", new DelegatedWorkContext(workContext, getLogPrefix()),
-                ScriptContext.ENGINE_SCOPE);
-        scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
-        final ProfileRequestContext prc = prcLookupStrategy.apply(resolutionContext);
-        if (null == prc) {
-            log.error("{} ProfileRequestContext could not be located", getLogPrefix());
-        }
-        scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
-
-        final SubjectContext sc = scLookupStrategy.apply(prc);
-        if (null == sc) {
-            log.debug("{} Could not locate SubjectContext", getLogPrefix());
-        } else {
-            final List<Subject> subjects = sc.getSubjects();
-            if (null == subjects) {
-                log.debug("{} Could not locate Subjects", getLogPrefix());
+            final SubjectContext sc = scLookupStrategy.apply(prc);
+            if (null == sc) {
+                log.debug("{} Could not locate SubjectContext", getLogPrefix());
             } else {
-                scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
-                        ScriptContext.ENGINE_SCOPE);
+                final List<Subject> subjects = sc.getSubjects();
+                if (null == subjects) {
+                    log.debug("{} Could not locate Subjects", getLogPrefix());
+                } else {
+                    scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
+                            ScriptContext.ENGINE_SCOPE);
+                }
             }
-        }
 
-        log.debug("{} adding emulated V2 request context to script context", getLogPrefix());
-        scriptContext.setAttribute("requestContext", new V2SAMLProfileRequestContext(resolutionContext, getId()),
-                ScriptContext.ENGINE_SCOPE);
+            log.debug("{} Adding emulated V2 request context to script context", getLogPrefix());
+            scriptContext.setAttribute("requestContext",
+                    new V2SAMLProfileRequestContext((AttributeResolutionContext) input[0], getId()),
+                    ScriptContext.ENGINE_SCOPE);
 
-        for (final Entry<String, List<IdPAttributeValue<?>>> dependencyAttribute : dependencyAttributes.entrySet()) {
-            log.debug("{} adding dependent attribute '{}' with the following values to the script context: {}",
-                    new Object[] {getLogPrefix(), dependencyAttribute.getKey(), dependencyAttribute.getValue(),});
-            final IdPAttribute pseudoAttribute = new IdPAttribute(dependencyAttribute.getKey());
-            pseudoAttribute.setValues(dependencyAttribute.getValue());
+            for (final Entry<String,List<IdPAttributeValue<?>>> dependencyAttribute : dependencyAttributes.entrySet()) {
+                log.trace("{} Adding dependent attribute '{}' with the following values to the script context: {}",
+                        new Object[] {getLogPrefix(), dependencyAttribute.getKey(), dependencyAttribute.getValue(),});
+                final IdPAttribute pseudoAttribute = new IdPAttribute(dependencyAttribute.getKey());
+                pseudoAttribute.setValues(dependencyAttribute.getValue());
 
-            scriptContext.setAttribute(dependencyAttribute.getKey(), new ScriptedIdPAttributeImpl(pseudoAttribute,
-                    getLogPrefix()), ScriptContext.ENGINE_SCOPE);
+                scriptContext.setAttribute(dependencyAttribute.getKey(),
+                        new ScriptedIdPAttributeImpl(pseudoAttribute, getLogPrefix()), ScriptContext.ENGINE_SCOPE);
+            }
         }
 
-        return scriptContext;
+        /** {@inheritDoc} */
+        @Override
+        @Nullable protected Object finalizeContext(@Nonnull final ScriptContext scriptContext,
+                @Nullable final Object scriptResult) throws ScriptException {
+            
+            final Object result = scriptContext.getAttribute(getId());
+            if (null == result) {
+                log.info("{} No value returned", getLogPrefix());
+                return null;
+            }
+
+            if (result instanceof ScriptedIdPAttributeImpl) {
+                final ScriptedIdPAttributeImpl scriptedAttribute = (ScriptedIdPAttributeImpl) result;
+                try {
+                    return scriptedAttribute.getResultingAttribute();
+                } catch (final ResolutionException e) {
+                    throw new ScriptException(e);
+                }
+            } else {
+                throw new ScriptException(getLogPrefix() + " returned variable was of wrong type ("
+                        + result.getClass().toString() + ")");
+            }
+        }
     }
+
 }
\ No newline at end of file
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnector.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnector.java
index bd34bdc..cc02664 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnector.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnector.java
@@ -30,7 +30,6 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.script.ScriptContext;
 import javax.script.ScriptException;
-import javax.script.SimpleScriptContext;
 import javax.security.auth.Subject;
 
 import net.shibboleth.idp.attribute.IdPAttribute;
@@ -47,6 +46,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterI
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.scripting.AbstractScriptEvaluator;
 import net.shibboleth.utilities.java.support.scripting.EvaluableScript;
 
 import org.opensaml.messaging.context.navigate.ChildContextLookup;
@@ -72,11 +72,14 @@ public class ScriptedDataConnector extends AbstractDataConnector {
     /** Script to be evaluated. */
     @NonnullAfterInit private EvaluableScript script;
 
+    /** Evaluator. */
+    @NonnullAfterInit private DataConnectorScriptEvaluator scriptEvaluator;
+    
     /** Strategy used to locate the {@link ProfileRequestContext} to use. */
-    @Nonnull private Function<AttributeResolutionContext, ProfileRequestContext> prcLookupStrategy;
+    @Nonnull private Function<AttributeResolutionContext,ProfileRequestContext> prcLookupStrategy;
 
     /** Strategy used to locate the {@link SubjectContext} to use. */
-    @Nonnull private Function<ProfileRequestContext, SubjectContext> scLookupStrategy;
+    @Nonnull private Function<ProfileRequestContext,SubjectContext> scLookupStrategy;
 
     /** The custom object we inject into all scripts. */
     @Nullable private Object customObject;
@@ -85,7 +88,7 @@ public class ScriptedDataConnector extends AbstractDataConnector {
     public ScriptedDataConnector() {
         // Defaults to ProfileRequestContext -> RelyingPartyContext -> AttributeContext.
         prcLookupStrategy = new ParentContextLookup<>();
-        scLookupStrategy = new ChildContextLookup<ProfileRequestContext, SubjectContext>(SubjectContext.class);
+        scLookupStrategy = new ChildContextLookup<>(SubjectContext.class);
     }
 
     /**
@@ -103,6 +106,9 @@ public class ScriptedDataConnector extends AbstractDataConnector {
      * @param object the custom object
      */
     @Nullable public void setCustomObject(final Object object) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
         customObject = object;
     }
 
@@ -137,6 +143,7 @@ public class ScriptedDataConnector extends AbstractDataConnector {
     public void setProfileRequestContextLookupStrategy(
             @Nonnull final Function<AttributeResolutionContext, ProfileRequestContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         prcLookupStrategy = Constraint.isNotNull(strategy, "ProfileRequestContext lookup strategy cannot be null");
     }
@@ -151,6 +158,7 @@ public class ScriptedDataConnector extends AbstractDataConnector {
     public void
             setSubjectContextLookupStrategy(@Nonnull final Function<ProfileRequestContext, SubjectContext> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         scLookupStrategy = Constraint.isNotNull(strategy, "SubjectContext lookup strategy cannot be null");
     }
@@ -161,144 +169,169 @@ public class ScriptedDataConnector extends AbstractDataConnector {
         if (null == script) {
             throw new ComponentInitializationException(getLogPrefix() + ": No script supplied");
         }
+        
+        scriptEvaluator = new DataConnectorScriptEvaluator(script);
+        scriptEvaluator.setCustomObject(customObject);
+        scriptEvaluator.setLogPrefix(getLogPrefix());
     }
 
-    /**
-     * Constructs the {@link ScriptContext} used when evaluating the script.
-     * 
-     * @param resolutionContext current resolution context
-     * @param workContext current work context
-     * @return constructed script context
-     * @throws ResolutionException thrown if dependent data connectors or attribute definitions can not be resolved
-     */
-    @Nonnull private ScriptContext getScriptContext(@Nonnull final AttributeResolutionContext resolutionContext,
+    /** {@inheritDoc} */
+    @Override @Nullable protected Map<String, IdPAttribute> doDataConnectorResolve(
+            @Nonnull final AttributeResolutionContext resolutionContext,
             @Nonnull final AttributeResolverWorkContext workContext) throws ResolutionException {
+        Constraint.isNotNull(resolutionContext, "AttributeResolutionContext cannot be null");
+        Constraint.isNotNull(workContext, "AttributeResolverWorkContext cannot be null");
 
-        final SimpleScriptContext scriptContext = new SimpleScriptContext();
-
-        log.debug("{} adding to-be-populated attribute set {} to script context", getLogPrefix(), RESULTS_STRING);
-        scriptContext.setAttribute(RESULTS_STRING, new HashSet<>(), ScriptContext.ENGINE_SCOPE);
-
-        log.debug("{} adding current attribute resolution contexts to script context", getLogPrefix());
-        scriptContext.setAttribute("resolutionContext", resolutionContext, ScriptContext.ENGINE_SCOPE);
-        scriptContext.setAttribute("workContext", new DelegatedWorkContext(workContext, getLogPrefix()),
-                ScriptContext.ENGINE_SCOPE);
-        final ProfileRequestContext prc = prcLookupStrategy.apply(resolutionContext);
-        scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
-        scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
-
-        final SubjectContext sc = scLookupStrategy.apply(prc);
-        if (null == sc) {
-            log.warn("{} Could not locate SubjectContext", getLogPrefix());
-        } else {
-            final List<Subject> subjects = sc.getSubjects();
-            if (null == subjects) {
-                log.warn("{} Could not locate Subjects", getLogPrefix());
-            } else {
-                scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
-                        ScriptContext.ENGINE_SCOPE);
-            }
-        }
-
-        final Map<String, List<IdPAttributeValue<?>>> dependencyAttributes =
-                PluginDependencySupport.getAllAttributeValues(workContext, getDependencies());
-
-        for (final Entry<String, List<IdPAttributeValue<?>>> dependencyAttribute : dependencyAttributes.entrySet()) {
-            log.debug("{} adding dependent attribute '{}' with the following values to the script context: {}",
-                    new Object[] {getLogPrefix(), dependencyAttribute.getKey(), dependencyAttribute.getValue(),});
-            final IdPAttribute pseudoAttribute = new IdPAttribute(dependencyAttribute.getKey());
-            pseudoAttribute.setValues(dependencyAttribute.getValue());
-
-            scriptContext.setAttribute(dependencyAttribute.getKey(), new ScriptedIdPAttributeImpl(pseudoAttribute,
-                    getLogPrefix()), ScriptContext.ENGINE_SCOPE);
-        }
-
-        return scriptContext;
+        return scriptEvaluator.execute(resolutionContext, workContext);
     }
 
     /**
-     * Ensure that all the values in the attribute are of the correct type.
-     * 
-     * @param attribute the attribute to look at
+     * Evaluator bound to the DataConnector semantic.
      */
-    private void checkValues(final IdPAttribute attribute) {
-
-        if (null == attribute.getValues()) {
-            log.info("{} Attribute '{}' has no values provided.", getLogPrefix(), attribute.getId());
-            attribute.setValues(Collections.<IdPAttributeValue<?>> emptyList());
-            return;
+    private class DataConnectorScriptEvaluator extends AbstractScriptEvaluator {
+
+        /**
+         * Constructor.
+         * 
+         * @param theScript the script we will evaluate.
+         */
+        public DataConnectorScriptEvaluator(@Nonnull final EvaluableScript theScript) {
+            super(theScript);
         }
-        log.debug("{} Attribute '{}' has {} values.", getLogPrefix(), attribute.getId(), attribute.getValues().size());
-        final List<IdPAttributeValue<?>> inputValues = attribute.getValues();
-        final List<IdPAttributeValue<?>> outputValues = new ArrayList<>(inputValues.size());
 
-        for (final Object o : inputValues) {
-            if (o instanceof IdPAttributeValue<?>) {
-                outputValues.add((IdPAttributeValue<?>) o);
-            } else {
-                log.error("{} Attribute '{} has attribute value of type {}.  This will be ignored", getLogPrefix(),
-                        attribute.getId(), o.getClass().getName());
+        /**
+         * Execution hook.
+         * 
+         * @param resolutionContext resolution context
+         * @param workContext work context
+         * 
+         * @return script result
+         * @throws ResolutionException if the script fails
+         */
+        @Nullable protected Map<String,IdPAttribute> execute(
+                @Nonnull final AttributeResolutionContext resolutionContext,
+                @Nonnull final AttributeResolverWorkContext workContext) throws ResolutionException {
+            try {
+                return (Map<String,IdPAttribute>) evaluate(resolutionContext, workContext);
+            } catch (final RuntimeException e) {
+                throw new ResolutionException(getLogPrefix() + "Script did not run successfully", e);
             }
         }
-        attribute.setValues(outputValues);
-    }
 
-    /**
-     * Given the Script context we extract the attribute list and then (very carefully) construct the results map.
-     * 
-     * @param context the Scripting context. We know that there is a variable inside called {@literal RESULTS_STRING}.
-     * @return the map of attributesids to attributes.
-     * @throws ResolutionException if the output was "wrong".
-     */
-    private Map<String, IdPAttribute> buildResult(final ScriptContext context) throws ResolutionException {
-        final Object res = context.getAttribute(RESULTS_STRING);
+        /** {@inheritDoc} */
+        @Override
+        protected void prepareContext(@Nonnull final ScriptContext scriptContext, @Nullable final Object... input) {
 
-        if (null == res) {
-            log.error("{} Could not locate output '{}' from script", getLogPrefix(), RESULTS_STRING);
-            throw new ResolutionException("Could not locate output from script");
-        }
-        if (!(res instanceof Collection)) {
-            log.error("{} Output '{}' was of type '{}', expected '{}'", getLogPrefix(), res.getClass().getName(),
-                    Collection.class.getName());
-            throw new ResolutionException("Output was of the wrong type");
-        }
+            log.debug("{} Adding to-be-populated attribute set '{}' to script context", getLogPrefix(), RESULTS_STRING);
+            scriptContext.setAttribute(RESULTS_STRING, new HashSet<>(), ScriptContext.ENGINE_SCOPE);
+
+            log.debug("{} Adding current attribute resolution contexts to script context", getLogPrefix());
+            scriptContext.setAttribute("resolutionContext", input[0], ScriptContext.ENGINE_SCOPE);
+            scriptContext.setAttribute("workContext",
+                    new DelegatedWorkContext((AttributeResolverWorkContext) input[1], getLogPrefix()),
+                    ScriptContext.ENGINE_SCOPE);
+            
+            scriptContext.setAttribute("custom", getCustomObject(), ScriptContext.ENGINE_SCOPE);
 
-        final Collection outputCollection = (Collection) res;
-        final Map<String, IdPAttribute> outputMap = new HashMap<>(outputCollection.size());
-        for (final Object o : outputCollection) {
-            if (o instanceof IdPAttribute) {
-                final IdPAttribute attribute = (IdPAttribute) o;
-                if (null == attribute.getId()) {
-                    log.error("{} Anonymous Attribute encountered, ignored", getLogPrefix());
+            final ProfileRequestContext prc = prcLookupStrategy.apply((AttributeResolutionContext) input[0]);
+            if (null == prc) {
+                log.error("{} ProfileRequestContext could not be located", getLogPrefix());
+            }
+            scriptContext.setAttribute("profileContext", prc, ScriptContext.ENGINE_SCOPE);
+            
+            final SubjectContext sc = scLookupStrategy.apply(prc);
+            if (null == sc) {
+                log.warn("{} Could not locate SubjectContext", getLogPrefix());
+            } else {
+                final List<Subject> subjects = sc.getSubjects();
+                if (null == subjects) {
+                    log.warn("{} Could not locate Subjects", getLogPrefix());
                 } else {
-                    checkValues(attribute);
-                    outputMap.put(attribute.getId(), attribute);
+                    scriptContext.setAttribute("subjects", subjects.toArray(new Subject[subjects.size()]),
+                            ScriptContext.ENGINE_SCOPE);
                 }
-            } else {
-                log.error("{} Output collection contained an object of type '{}'.  This was ignored", getLogPrefix(), o
-                        .getClass().getName());
             }
-        }
 
-        return outputMap;
-    }
+            final Map<String, List<IdPAttributeValue<?>>> dependencyAttributes =
+                    PluginDependencySupport.getAllAttributeValues(
+                            (AttributeResolverWorkContext) input[1], getDependencies());
 
-    /** {@inheritDoc} */
-    @Override @Nullable protected Map<String, IdPAttribute> doDataConnectorResolve(
-            @Nonnull final AttributeResolutionContext resolutionContext,
-            @Nonnull final AttributeResolverWorkContext workContext) throws ResolutionException {
-        Constraint.isNotNull(resolutionContext, "AttributeResolutionContext cannot be null");
-        Constraint.isNotNull(workContext, "AttributeResolverWorkContext cannot be null");
-
-        final ScriptContext context = getScriptContext(resolutionContext, workContext);
+            for (final Entry<String,List<IdPAttributeValue<?>>> dependencyAttribute : dependencyAttributes.entrySet()) {
+                log.trace("{} Adding dependent attribute '{}' with the following values to the script context: {}",
+                        new Object[] {getLogPrefix(), dependencyAttribute.getKey(), dependencyAttribute.getValue(),});
+                final IdPAttribute pseudoAttribute = new IdPAttribute(dependencyAttribute.getKey());
+                pseudoAttribute.setValues(dependencyAttribute.getValue());
 
-        try {
-            script.eval(context);
-        } catch (final ScriptException e) {
-            throw new ResolutionException(getLogPrefix() + " unable to execute script", e);
+                scriptContext.setAttribute(dependencyAttribute.getKey(), new ScriptedIdPAttributeImpl(pseudoAttribute,
+                        getLogPrefix()), ScriptContext.ENGINE_SCOPE);
+            }
         }
 
-        return buildResult(context);
-    }
+        /** {@inheritDoc} */
+        @Override
+        @Nullable protected Object finalizeContext(@Nonnull final ScriptContext scriptContext,
+                @Nullable final Object scriptResult) throws ScriptException {
+            
+            // The real result in our case is a variable in the context.
+            final Object res = scriptContext.getAttribute(RESULTS_STRING);
+
+            if (null == res) {
+                log.error("{} Could not locate output variable '{}' from script", getLogPrefix(), RESULTS_STRING);
+                throw new ScriptException("Could not locate output from script");
+            }
+            if (!(res instanceof Collection)) {
+                log.error("{} Output '{}' was of type '{}', expected '{}'", getLogPrefix(), res.getClass().getName(),
+                        Collection.class.getName());
+                throw new ScriptException("Output was of the wrong type");
+            }
+
+            final Collection outputCollection = (Collection) res;
+            final Map<String, IdPAttribute> outputMap = new HashMap<>(outputCollection.size());
+            for (final Object o : outputCollection) {
+                if (o instanceof IdPAttribute) {
+                    final IdPAttribute attribute = (IdPAttribute) o;
+                    if (null == attribute.getId()) {
+                        log.warn("{} Anonymous Attribute encountered, ignored", getLogPrefix());
+                    } else {
+                        checkValues(attribute);
+                        outputMap.put(attribute.getId(), attribute);
+                    }
+                } else {
+                    log.warn("{} Output collection contained an object of type '{}', ignored", getLogPrefix(),
+                            o.getClass().getName());
+                }
+            }
 
-}
+            return outputMap;
+        }
+        
+        /**
+         * Ensure that all the values in the attribute are of the correct type.
+         * 
+         * @param attribute the attribute to look at
+         */
+        private void checkValues(final IdPAttribute attribute) {
+
+            if (null == attribute.getValues()) {
+                log.info("{} Attribute '{}' has no values provided.", getLogPrefix(), attribute.getId());
+                attribute.setValues(Collections.<IdPAttributeValue<?>> emptyList());
+                return;
+            }
+            log.debug("{} Attribute '{}' has {} value(s).", getLogPrefix(), attribute.getId(),
+                    attribute.getValues().size());
+            final List<IdPAttributeValue<?>> inputValues = attribute.getValues();
+            final List<IdPAttributeValue<?>> outputValues = new ArrayList<>(inputValues.size());
+
+            for (final Object o : inputValues) {
+                if (o instanceof IdPAttributeValue<?>) {
+                    outputValues.add((IdPAttributeValue<?>) o);
+                } else {
+                    log.error("{} Attribute '{} has attribute value of type {}.  This will be ignored", getLogPrefix(),
+                            attribute.getId(), o.getClass().getName());
+                }
+            }
+            attribute.setValues(outputValues);
+        }
+    }
+    
+}
\ No newline at end of file
diff --git a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnectorTest.java b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnectorTest.java
index 3e86c2a..e70f0dc 100644
--- a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnectorTest.java
+++ b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/dc/impl/ScriptedDataConnectorTest.java
@@ -64,6 +64,21 @@ public class ScriptedDataConnectorTest {
         return StringSupport.inputStreamToString(getClass().getResourceAsStream(name), null);
     }
 
+    @Test(expectedExceptions=ResolutionException.class)
+    public void error() throws ComponentInitializationException, ScriptException, IOException, ResolutionException {
+        final ScriptedDataConnector connector = new ScriptedDataConnector();
+        connector.setId("Scripted");
+        final EvaluableScript definitionScript = new EvaluableScript("javascript", getScript("error.js"));
+        connector.setScript(definitionScript);
+
+        connector.initialize();
+
+        final AttributeResolutionContext context = new ProfileRequestContext<>().getSubcontext(AttributeResolutionContext.class,  true);
+        context.getSubcontext(AttributeResolverWorkContext.class, true);
+        
+        connector.resolve(context);
+    }
+    
     @Test public void simple() throws ComponentInitializationException, ResolutionException, ScriptException, IOException {
 
         final ScriptedDataConnector connector = new ScriptedDataConnector();
diff --git a/idp-attribute-resolver-impl/src/test/resources/net/shibboleth/idp/attribute/resolver/impl/dc/error.js b/idp-attribute-resolver-impl/src/test/resources/net/shibboleth/idp/attribute/resolver/impl/dc/error.js
new file mode 100644
index 0000000..bff809a
--- /dev/null
+++ b/idp-attribute-resolver-impl/src/test/resources/net/shibboleth/idp/attribute/resolver/impl/dc/error.js
@@ -0,0 +1 @@
+connectorResult.add(custom);
diff --git a/idp-attribute-resolver-impl/src/test/resources/net/shibboleth/idp/attribute/resolver/impl/dc/v8/error.js b/idp-attribute-resolver-impl/src/test/resources/net/shibboleth/idp/attribute/resolver/impl/dc/v8/error.js
new file mode 100644
index 0000000..bff809a
--- /dev/null
+++ b/idp-attribute-resolver-impl/src/test/resources/net/shibboleth/idp/attribute/resolver/impl/dc/v8/error.js
@@ -0,0 +1 @@
+connectorResult.add(custom);

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


More information about the commits mailing list