[java-identity-provider] branch master updated: IDP-1516 Reduce guava dependence idp-attribute

Rod Widdowson rdw at steadingsoftware.com
Tue Oct 29 06:49:52 EDT 2019


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

rdw 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=420a2417230377255ef5173d8cac912f5d2d4332

The following commit(s) were added to refs/heads/master by this push:
       new  420a241   IDP-1516 Reduce guava dependence idp-attribute
420a241 is described below

commit 420a2417230377255ef5173d8cac912f5d2d4332
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue Oct 29 10:35:39 2019 +0000

    IDP-1516 Reduce guava dependence idp-attribute
    
    https://issues.shibboleth.net/jira/browse/IDP-1516
---
 .../idp/attribute/AttributesMapContainer.java      |  4 +-
 .../net/shibboleth/idp/attribute/IdPAttribute.java | 62 +++++++++++++++-------
 .../idp/attribute/context/AttributeContext.java    | 30 +++++------
 .../idp/attribute/AttributeContextTest.java        |  7 +--
 .../shibboleth/idp/attribute/AttributeTest.java    | 20 +------
 5 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/AttributesMapContainer.java b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/AttributesMapContainer.java
index 12aed41..cc0b798 100644
--- a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/AttributesMapContainer.java
+++ b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/AttributesMapContainer.java
@@ -17,7 +17,7 @@
 
 package net.shibboleth.idp.attribute;
 
-import com.google.common.base.Supplier;
+import java.util.function.Supplier;
 import com.google.common.collect.Multimap;
 
 /**
@@ -43,4 +43,4 @@ public class AttributesMapContainer implements Supplier<Multimap<String,IdPAttri
         return providedValue;
     }
 
-}
\ No newline at end of file
+}
diff --git a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttribute.java b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttribute.java
index 13d4a88..76474d5 100644
--- a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttribute.java
+++ b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/IdPAttribute.java
@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -36,7 +37,6 @@ import org.slf4j.LoggerFactory;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
-import com.google.common.collect.ImmutableMap;
 
 import net.shibboleth.idp.attribute.EmptyAttributeValue.EmptyType;
 import net.shibboleth.utilities.java.support.annotation.ParameterName;
@@ -46,6 +46,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotLive;
 import net.shibboleth.utilities.java.support.annotation.constraint.NullableElements;
 import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
 import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 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;
@@ -66,6 +67,29 @@ public class IdPAttribute implements Comparable<IdPAttribute>, Cloneable {
     /** Logger - static. */
     @Nonnull private static final Logger LOG = LoggerFactory.getLogger(IdPAttribute.class);
     
+    /** Helper Function for map manipulation. */
+    @Nonnull private static Function<Entry<Locale, String>, Entry<Locale, String>> filterEntry =
+            new Function<>() {
+        public Entry<Locale, String> apply(final Entry<Locale, String> t) {
+            return new Entry<>() {
+                private String val = Constraint.isNotNull(StringSupport.trimOrNull(t.getValue()),
+                        "Values must not be null");
+                private Locale key = Constraint.isNotNull(t.getKey(), "Key must not be null");
+                public Locale getKey() {
+                    return key;
+                }
+
+                public String getValue() {
+                    return val;
+                }
+
+                public String setValue(final String value) {
+                    throw new ConstraintViolationException("Unmodifable map modified");
+                }
+            };
+        };
+    };
+
     /** ID of this attribute. */
     @Nonnull private final String id;
 
@@ -77,7 +101,7 @@ public class IdPAttribute implements Comparable<IdPAttribute>, Cloneable {
 
     /** Values for this attribute. */
     @Nonnull private List<IdPAttributeValue> values;
-    
+
     /**
      * Constructor.
      * 
@@ -146,22 +170,18 @@ public class IdPAttribute implements Comparable<IdPAttribute>, Cloneable {
      * @param inputMap the input map.
      * @return the unmodifiable, non null-containing output.
      */
-    @Nonnull @NonnullElements @Unmodifiable private Map<Locale, String> checkedNamesFrom(
-            @Nullable @NullableElements final Map<Locale, String> inputMap) {
+    @SuppressWarnings("unchecked")
+    @Nonnull @Unmodifiable private Map<Locale, String> checkedNamesFrom(@Nonnull final Map<Locale, String> inputMap) {
         
-        final ImmutableMap.Builder<Locale,String> builder = ImmutableMap.builder();
-
-        if (inputMap != null) {
-            for (final Entry<Locale,String> entry : inputMap.entrySet()) {
-                if (entry.getKey() != null) {
-                    final String trimmedName = StringSupport.trimOrNull(entry.getValue());
-                    if (trimmedName != null) {
-                        builder.put(entry.getKey(), trimmedName);
-                    }
-                }
-            }
+        if (inputMap.isEmpty()) {
+            return Collections.emptyMap();
         }
-        return builder.build();
+        return Map.ofEntries((Entry<Locale, String>[])
+                inputMap.
+                entrySet().
+                stream().
+                map(filterEntry).
+                toArray(Map.Entry[]::new));
     }
 
     /**
@@ -169,8 +189,9 @@ public class IdPAttribute implements Comparable<IdPAttribute>, Cloneable {
      * 
      * @param newNames the new names for this attribute
      */
-    public void setDisplayNames(@Nullable @NullableElements final Map<Locale, String> newNames) {
-        displayNames = checkedNamesFrom(newNames);
+    public void setDisplayNames(@Nonnull final Map<Locale, String> newNames) {
+        displayNames = checkedNamesFrom(
+                Constraint.isNotNull(newNames, "Display Names should not be null"));
     }
 
     /**
@@ -187,8 +208,9 @@ public class IdPAttribute implements Comparable<IdPAttribute>, Cloneable {
      * 
      * @param newDescriptions the new descriptions for this attribute
      */
-    public void setDisplayDescriptions(@Nullable @NullableElements final Map<Locale, String> newDescriptions) {
-        displayDescriptions = checkedNamesFrom(newDescriptions);
+    public void setDisplayDescriptions(@Nonnull final Map<Locale, String> newDescriptions) {
+        displayDescriptions = checkedNamesFrom(
+                Constraint.isNotNull(newDescriptions, "Display Descriptions should not be null"));
     }
 
     /**
diff --git a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java
index 4849e6a..3c31bdc 100644
--- a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java
+++ b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/context/AttributeContext.java
@@ -26,15 +26,15 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 
-import net.shibboleth.idp.attribute.IdPAttribute;
-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.Unmodifiable;
-
 import org.opensaml.messaging.context.BaseContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
+import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+
 /**
  * A {@link BaseContext} that tracks a set of attributes. Usually the tracked attributes are about a particular user and
  * associated with a particular service request.
@@ -71,17 +71,15 @@ public final class AttributeContext extends BaseContext {
      * 
      * @param newAttributes the attributes
      */
-    public void setIdPAttributes(@Nullable @NullableElements final Collection<IdPAttribute> newAttributes) {
-        if (newAttributes == null) {
+    public void setIdPAttributes(@Nullable final Collection<IdPAttribute> newAttributes) {
+        if (newAttributes == null || newAttributes.isEmpty()) {
             attributes = Collections.emptyMap();
             return;
         }
 
-        final HashMap<String,IdPAttribute> checkedAttributes = new HashMap<>();
+        final HashMap<String,IdPAttribute> checkedAttributes = new HashMap<>(newAttributes.size());
         for (final IdPAttribute attribute : newAttributes) {
-            if (attribute != null) {
-                checkedAttributes.put(attribute.getId(), attribute);
-            }
+            checkedAttributes.put(attribute.getId(), Constraint.isNotNull(attribute, "Cannot set null attributes"));
         }
 
         attributes = Map.copyOf(checkedAttributes);
@@ -106,20 +104,18 @@ public final class AttributeContext extends BaseContext {
      * 
      * @param newAttributes the attributes
      */
-    public void setUnfilteredIdPAttributes(@Nullable @NullableElements final Collection<IdPAttribute> newAttributes) {
+    public void setUnfilteredIdPAttributes(@Nullable final Collection<IdPAttribute> newAttributes) {
         if (null != unfilteredAttributes) {
             log.error("Unfiltered attributes have already been set in this flow.");
         }
-        if (newAttributes == null) {
+        if (newAttributes == null || newAttributes.isEmpty()) {
             unfilteredAttributes = Collections.emptyMap();
             return;
         }
 
-        final HashMap<String,IdPAttribute> checkedAttributes = new HashMap<>();
+        final HashMap<String,IdPAttribute> checkedAttributes = new HashMap<>(newAttributes.size());
         for (final IdPAttribute attribute : newAttributes) {
-            if (attribute != null) {
-                checkedAttributes.put(attribute.getId(), attribute);
-            }
+            checkedAttributes.put(attribute.getId(), Constraint.isNotNull(attribute, "non null Attribute"));
         }
 
         unfilteredAttributes = Map.copyOf(checkedAttributes);
diff --git a/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java b/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java
index 324651d..24e50c6 100644
--- a/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java
+++ b/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeContextTest.java
@@ -46,11 +46,8 @@ public class AttributeContextTest {
     @Test public void attributeContext() {
         AttributeContext context = new AttributeContext();
         
-        context.setIdPAttributes(Arrays.asList((IdPAttribute)null, null));
-        contextAttributes(context, 0);
-
-        context.setIdPAttributes(Arrays.asList(new IdPAttribute("foo"), null));
-        contextAttributes(context, 1);
+        context.setIdPAttributes(Arrays.asList(new IdPAttribute("foo"), new IdPAttribute("bar")));
+        contextAttributes(context, 2);
         
         context.setIdPAttributes(null);
         contextAttributes(context, 0);
diff --git a/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java b/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java
index 8d73d04..58adf35 100644
--- a/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java
+++ b/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java
@@ -95,23 +95,13 @@ public class AttributeTest {
         Locale enbr = new Locale("en", "br");
 
         IdPAttribute attrib = new IdPAttribute("foo");
-        attrib.setDisplayNames(null);
+        attrib.setDisplayNames(Collections.EMPTY_MAP);
         Assert.assertTrue(attrib.getDisplayNames().isEmpty());
 
         attrib.setDisplayNames(Collections.emptyMap());
         Assert.assertTrue(attrib.getDisplayNames().isEmpty());
 
         Map<Locale, String> displayNames = new HashMap<>();
-        displayNames.put(null, "wibble");
-        attrib.setDisplayNames(displayNames);
-        Assert.assertTrue(attrib.getDisplayNames().isEmpty());
-
-        displayNames.clear();
-        displayNames.put(en, null);
-        attrib.setDisplayNames(displayNames);
-        Assert.assertTrue(attrib.getDisplayNames().isEmpty());
-
-        displayNames.clear();
         // test adding one entry
         displayNames.put(en, " english ");
         attrib.setDisplayNames(displayNames);
@@ -161,24 +151,18 @@ public class AttributeTest {
         Locale enbr = new Locale("en", "br");
 
         IdPAttribute attrib = new IdPAttribute("foo");
-        attrib.setDisplayDescriptions(null);
+        attrib.setDisplayDescriptions(Collections.EMPTY_MAP);
         Assert.assertTrue(attrib.getDisplayNames().isEmpty());
 
         attrib.setDisplayNames(Collections.emptyMap());
         Assert.assertTrue(attrib.getDisplayDescriptions().isEmpty());
 
         Map<Locale, String> displayDescriptions = new HashMap<>();
-        displayDescriptions.put(null, "wibble");
-        attrib.setDisplayDescriptions(displayDescriptions);
-        Assert.assertTrue(attrib.getDisplayDescriptions().isEmpty());
-
         displayDescriptions.clear();
-        displayDescriptions.put(en, null);
         attrib.setDisplayDescriptions(displayDescriptions);
         Assert.assertTrue(attrib.getDisplayDescriptions().isEmpty());
 
         displayDescriptions.clear();
-        // test adding one entry
         displayDescriptions.put(en, " english ");
         attrib.setDisplayDescriptions(displayDescriptions);
         

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


More information about the commits mailing list