[java-identity-provider] branch master updated: IDP-1366 - Remove <SourceAttribute> from "Template" attribute definition

Scott Cantor cantor.2 at osu.edu
Thu May 23 12:33:28 EDT 2019


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=4118615cda6f80e6c1eb8c5dc97ef6b14ec2b7ab

The following commit(s) were added to refs/heads/master by this push:
       new  4118615   IDP-1366 - Remove <SourceAttribute> from "Template" attribute definition
4118615 is described below

commit 4118615cda6f80e6c1eb8c5dc97ef6b14ec2b7ab
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu May 23 12:33:26 2019 -0400

    IDP-1366 - Remove <SourceAttribute> from "Template" attribute definition
    
    https://issues.shibboleth.net/jira/browse/IDP-1366
---
 .../ad/impl/TemplateAttributeDefinition.java       | 51 +---------------------
 .../resolver/ad/impl/TemplateAttributeTest.java    | 46 +------------------
 .../ad/impl/TemplateAttributeDefinitionParser.java | 22 ----------
 .../ad/TemplateAttributeDefinitionParserTest.java  | 12 +----
 .../spring/ad/resolver/templateAttributes.xml      |  2 -
 .../spring/ad/resolver/templateTwoTemplate.xml     |  2 -
 .../schema/shibboleth-attribute-resolver.xsd       |  8 ----
 7 files changed, 5 insertions(+), 138 deletions(-)

diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java
index db46d17..ded6959 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeDefinition.java
@@ -18,7 +18,6 @@
 package net.shibboleth.idp.attribute.resolver.ad.impl;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -33,8 +32,6 @@ import org.apache.velocity.exception.VelocityException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Predicates;
-
 import net.shibboleth.idp.attribute.EmptyAttributeValue;
 import net.shibboleth.idp.attribute.IdPAttribute;
 import net.shibboleth.idp.attribute.IdPAttributeValue;
@@ -47,10 +44,7 @@ import net.shibboleth.idp.attribute.resolver.context.AttributeResolutionContext;
 import net.shibboleth.idp.attribute.resolver.context.AttributeResolverWorkContext;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
-import net.shibboleth.utilities.java.support.annotation.constraint.NullableElements;
 import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
-import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
-import net.shibboleth.utilities.java.support.collection.CollectionSupport;
 import net.shibboleth.utilities.java.support.collection.LazyMap;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -82,38 +76,6 @@ public class TemplateAttributeDefinition extends AbstractAttributeDefinition {
     /** VelocityEngine. */
     @NonnullAfterInit private VelocityEngine engine;
 
-    /** The names of the attributes we need. */
-    @Deprecated @Nonnull @NonnullElements private List<String> sourceAttributes;
-    
-    /** Constructor. */
-    public TemplateAttributeDefinition() {
-        sourceAttributes = Collections.emptyList();
-    }
-
-    /**
-     * Get the source attribute IDs.
-     * @deprecated This should be inferred from the environment, but we keep this for V4
-     * @return the source attribute IDs
-     */
-    @Deprecated @Nonnull @Unmodifiable @NonnullElements public List<String> getSourceAttributes() {
-        return Collections.unmodifiableList(sourceAttributes);
-    }
-
-    /**
-     * Set the source attribute IDs.
-     * 
-     * @deprecated This should be inferred from the environment, but we keep this for V4
-     * @param newSourceAttributes the source attribute IDs
-     */
-    @Deprecated public void setSourceAttributes(@Nonnull @NullableElements final List<String> newSourceAttributes) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
-        Constraint.isNotNull(newSourceAttributes, "Source attribute list cannot be null");
-
-        sourceAttributes = new ArrayList<>(newSourceAttributes.size());
-        CollectionSupport.addIf(sourceAttributes, newSourceAttributes, Predicates.notNull());
-    }
-
     /**
      * Get the template text to be evaluated.
      * 
@@ -298,17 +260,8 @@ public class TemplateAttributeDefinition extends AbstractAttributeDefinition {
 
         int valueCount = 0;
 
-        if (getSourceAttributes().isEmpty()) {
-            for (final Entry<String, List<IdPAttributeValue>> entry : dependencyAttributes.entrySet() ) {
-                valueCount = addAttributeValues(entry.getKey(), entry.getValue(), sourceValues, valueCount);
-            }
-        } else {
-            for (final String attributeName:getSourceAttributes()) {
-                valueCount = addAttributeValues(attributeName,
-                        dependencyAttributes.get(attributeName),
-                        sourceValues,
-                        valueCount);
-            }
+        for (final Entry<String, List<IdPAttributeValue>> entry : dependencyAttributes.entrySet() ) {
+            valueCount = addAttributeValues(entry.getKey(), entry.getValue(), sourceValues, valueCount);
         }
 
         return valueCount;
diff --git a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java
index c7dba46..9fd81ff 100644
--- a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java
+++ b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/impl/TemplateAttributeTest.java
@@ -17,21 +17,14 @@
 
 package net.shibboleth.idp.attribute.resolver.ad.impl;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
-import static org.testng.Assert.assertTrue;
-import static org.testng.Assert.fail;
+import static org.testng.Assert.*;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
-import javax.annotation.concurrent.ThreadSafe;
-
 import org.apache.velocity.app.VelocityEngine;
 import org.testng.annotations.Test;
 
@@ -52,8 +45,6 @@ import net.shibboleth.utilities.java.support.collection.LazySet;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 
 /** test for {@link net.shibboleth.idp.attribute.resolver.impl.TemplateAttribute}. */
- at ThreadSafe
- at SuppressWarnings("deprecation")
 public class TemplateAttributeTest {
 
     /** The name. */
@@ -162,7 +153,6 @@ public class TemplateAttributeTest {
         attr.setTemplateText(TEST_ATTRIBUTES_TEMPLATE_ATTR);
         attr.setDataConnectorDependencies(Collections.singleton(TestSources.makeDataConnectorDependency("foo", "bar")));
         
-        attr.setSourceAttributes(Collections.singletonList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
         attr.initialize();
         assertNotNull(attr.getTemplate());
         try {
@@ -181,12 +171,9 @@ public class TemplateAttributeTest {
         } catch (final ComponentInitializationException ex) {
             // OK
         }
-        attr.setSourceAttributes(Collections.singletonList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
         attr.setTemplateText( "${" + TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR + "}");
         attr.initialize();
         assertEquals(attr.getTemplateText(), "${" + TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR + "}");
-        assertEquals(attr.getSourceAttributes().get(0), TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR);
-        assertEquals(attr.getSourceAttributes().size(), 1);
 
     }
 
@@ -236,29 +223,6 @@ public class TemplateAttributeTest {
      * @throws ComponentInitializationException if it goes wrong.
      */
     @Test public void templateWithValues() throws ResolutionException, ComponentInitializationException {
-        templateWithValues(false);
-    }
-
-    /**
-     * Test resolution of an template script with data generated from the attributes, but with
-     * explicit setting of source attributes.
-     *
-     * This is a canary for IDP-1386
-     *
-     * @throws ResolutionException if it goes wrong.
-     * @throws ComponentInitializationException if it goes wrong.
-     */
-    @Test public void templateWithValuesTestSources() throws ResolutionException, ComponentInitializationException {
-        templateWithValues(true);
-    }
-
-    /** Worker function for the templateWithValues and templateWithValuesTestSources tests.
-     *
-     * @param setSources whether to all {@link TemplateAttributeDefinition#setSourceAttributes(List)}
-     * @throws ResolutionException if it goes wrong.
-     * @throws ComponentInitializationException if it goes wrong.
-     */
-    private final void templateWithValues(boolean setSources) throws ResolutionException, ComponentInitializationException {
 
         final String name = TEST_ATTRIBUTE_BASE_NAME + "3";
 
@@ -270,17 +234,11 @@ public class TemplateAttributeTest {
         final Set<ResolverAttributeDefinitionDependency> ds = new LazySet<>();
         
         ds.add(TestSources.makeAttributeDefinitionDependency(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR));
-        if (setSources) {
-            ds.add(TestSources.makeAttributeDefinitionDependency(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR+"2"));
-        }
+        
         templateDef.setAttributeDependencies(ds);
         templateDef.setDataConnectorDependencies(Collections.singleton(
                 TestSources.makeDataConnectorDependency(TestSources.STATIC_CONNECTOR_NAME,
                             TestSources.DEPENDS_ON_SECOND_ATTRIBUTE_NAME)));
-        if (setSources) {
-            templateDef.setSourceAttributes(Arrays.asList(TestSources.DEPENDS_ON_ATTRIBUTE_NAME_ATTR,
-                    TestSources.DEPENDS_ON_SECOND_ATTRIBUTE_NAME));
-        }
         templateDef.initialize();
 
         final Set<AttributeDefinition> attrDefinitions = new LazySet<>();
diff --git a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java
index 8c1aa7c..26a0cce 100644
--- a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java
+++ b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/impl/TemplateAttributeDefinitionParser.java
@@ -26,15 +26,12 @@ import javax.xml.namespace.QName;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
-import org.springframework.beans.factory.support.ManagedList;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.w3c.dom.Element;
 
 import net.shibboleth.idp.attribute.resolver.ad.impl.TemplateAttributeDefinition;
 import net.shibboleth.idp.attribute.resolver.spring.ad.BaseAttributeDefinitionParser;
 import net.shibboleth.idp.attribute.resolver.spring.impl.AttributeResolverNamespaceHandler;
-import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
-import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 import net.shibboleth.utilities.java.support.xml.ElementSupport;
 
@@ -51,10 +48,6 @@ public class TemplateAttributeDefinitionParser extends BaseAttributeDefinitionPa
     @Nonnull public static final QName TEMPLATE_ELEMENT_NAME_RESOLVER =
             new QName(AttributeResolverNamespaceHandler.NAMESPACE, "Template");
 
-    /** SourceValue element name. */
-    @Nonnull public static final QName SOURCE_ATTRIBUTE_ELEMENT_NAME_RESOLVER =
-            new QName(AttributeResolverNamespaceHandler.NAMESPACE, "SourceAttribute");
-
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(TemplateAttributeDefinitionParser.class);
 
@@ -82,21 +75,6 @@ public class TemplateAttributeDefinitionParser extends BaseAttributeDefinitionPa
             builder.addPropertyValue("templateText", templateText);
         }
 
-        final List<Element> sourceAttributeElements =
-            ElementSupport.getChildElements(config, SOURCE_ATTRIBUTE_ELEMENT_NAME_RESOLVER);
-        if (null != sourceAttributeElements && !sourceAttributeElements.isEmpty()) {
-            // V4 deprecation
-            DeprecationSupport.warnOnce(ObjectType.ELEMENT, SOURCE_ATTRIBUTE_ELEMENT_NAME_RESOLVER.getLocalPart(),
-                    parserContext.getReaderContext().getResource().getDescription(),
-                    "by using <InputAttributeDefinition> and <InputDataConnector>");
-            final List<String> sourceAttributes = new ManagedList<>(sourceAttributeElements.size());
-            for (final Element element : sourceAttributeElements) {
-                sourceAttributes.add(StringSupport.trimOrNull(element.getTextContent()));
-            }
-            log.debug("{} Source attributes are '{}'.", getLogPrefix(), sourceAttributes);
-            builder.addPropertyValue("sourceAttributes", sourceAttributes);
-        }
-
         String velocityEngineRef = StringSupport.trimOrNull(config.getAttributeNS(null, "velocityEngine"));
         if (null == velocityEngineRef) {
             velocityEngineRef = "shibboleth.VelocityEngine";
diff --git a/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/TemplateAttributeDefinitionParserTest.java b/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/TemplateAttributeDefinitionParserTest.java
index 2e64299..f705ff5 100644
--- a/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/TemplateAttributeDefinitionParserTest.java
+++ b/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/TemplateAttributeDefinitionParserTest.java
@@ -17,9 +17,7 @@
 
 package net.shibboleth.idp.attribute.resolver.spring.ad;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
-import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.*;
 
 import org.testng.annotations.Test;
 
@@ -31,7 +29,6 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 /**
  * Test for {@link TemplateAttributeDefinitionParser}
  */
- at SuppressWarnings("deprecation")
 public class TemplateAttributeDefinitionParserTest extends BaseAttributeDefinitionParserTest {
 
     @Test(enabled = false) public void noAttr() throws ComponentInitializationException {
@@ -41,7 +38,6 @@ public class TemplateAttributeDefinitionParserTest extends BaseAttributeDefiniti
 
         assertEquals(defn.getId(), "templateId");
         assertNull(defn.getTemplateText());
-        assertTrue(defn.getSourceAttributes().isEmpty());
     }
 
     @Test public void withAttr() throws ComponentInitializationException {
@@ -51,9 +47,6 @@ public class TemplateAttributeDefinitionParserTest extends BaseAttributeDefiniti
 
         assertEquals(defn.getId(), "templateIdAttr");
         assertEquals(defn.getTemplateText(), "TheTemplate");
-        assertEquals(defn.getSourceAttributes().size(), 2);
-        assertTrue(defn.getSourceAttributes().contains("att1"));
-        assertTrue(defn.getSourceAttributes().contains("att2"));
     }
     
     @Test public void dupl() throws ComponentInitializationException {
@@ -63,9 +56,6 @@ public class TemplateAttributeDefinitionParserTest extends BaseAttributeDefiniti
 
         assertEquals(defn.getId(), "templateIdAttr");
         assertEquals(defn.getTemplateText(), "TheTemplate");
-        assertEquals(defn.getSourceAttributes().size(), 2);
-        assertTrue(defn.getSourceAttributes().contains("att1"));
-        assertTrue(defn.getSourceAttributes().contains("att2"));
     }
 
 }
diff --git a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateAttributes.xml b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateAttributes.xml
index 193a1fb..58692fc 100644
--- a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateAttributes.xml
+++ b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateAttributes.xml
@@ -5,8 +5,6 @@
     velocityEngine="otherVe"
     xsi:type="Template"
     xsi:schemaLocation="urn:mace:shibboleth:2.0:resolver http://shibboleth.net/schema/idp/shibboleth-attribute-resolver.xsd" >
-  <SourceAttribute>att1</SourceAttribute>
   <InputAttributeDefinition ref="TheOrphan"/>
-  <SourceAttribute>att2</SourceAttribute>
   <Template>TheTemplate</Template>
 </AttributeDefinition>
\ No newline at end of file
diff --git a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateTwoTemplate.xml b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateTwoTemplate.xml
index db8b184..5bfbcb7 100644
--- a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateTwoTemplate.xml
+++ b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/resolver/templateTwoTemplate.xml
@@ -8,8 +8,6 @@
     
   <InputAttributeDefinition ref="TheOrphan"/>
   <Template>TheTemplate</Template>
-  <SourceAttribute>att1</SourceAttribute>
   <Template>TheTemplate</Template>
-  <SourceAttribute>att2</SourceAttribute>
   <Template>TheTemplate</Template>
 </AttributeDefinition>
\ No newline at end of file
diff --git a/idp-schema/src/main/resources/schema/shibboleth-attribute-resolver.xsd b/idp-schema/src/main/resources/schema/shibboleth-attribute-resolver.xsd
index 5d2943a..dc2fde6 100644
--- a/idp-schema/src/main/resources/schema/shibboleth-attribute-resolver.xsd
+++ b/idp-schema/src/main/resources/schema/shibboleth-attribute-resolver.xsd
@@ -655,14 +655,6 @@
                             </documentation>
                         </annotation>
                     </element>
-                    <element name="SourceAttribute" type="string" maxOccurs="unbounded">
-                        <annotation>
-                            <documentation>
-                                Attribute IDs which should be used in this definition.
-                                It is preferred to provide these using the dependencies.
-                            </documentation>
-                        </annotation>
-                    </element>
                 </choice>
                 <attribute name="velocityEngine" type="string">
                     <annotation>

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


More information about the commits mailing list