[java-identity-provider COMMIT] in /trunk/idp-attribute-resolver-impl: doc/ src/main/java/net/shibboleth/idp/attribut...

[noreply at shibboleth.net]

Author: rdw
Date: Wed Aug 31 17:04:43 2011
New Revision: 4045

URL: http://svn.shibboleth.net/view/java-identity-provider?rev=4045&view=rev
Log:
Code review from Chad.

Added:
    trunk/idp-attribute-resolver-impl/doc/
Modified:
    trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AbstractPrincipalAttributeDefinition.java
    trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/PrincipalAuthenticationMethodAttributeDefinition.java
    trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/PrincipalNameAttributeDefinition.java

Modified: trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AbstractPrincipalAttributeDefinition.java
URL: http://svn.shibboleth.net/view/java-identity-provider/trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AbstractPrincipalAttributeDefinition.java?rev=4045&r1=4044&r2=4045&view=diff
==============================================================================
--- trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AbstractPrincipalAttributeDefinition.java (original)
+++ trunk/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AbstractPrincipalAttributeDefinition.java Wed Aug 31 17:04:43 2011
@@ -19,7 +19,6 @@
 
 import net.jcip.annotations.ThreadSafe;
 import net.shibboleth.idp.attribute.resolver.AttributeResolutionContext;
-import net.shibboleth.idp.attribute.resolver.AttributeResolutionException;
 import net.shibboleth.idp.attribute.resolver.BaseAttributeDefinition;
 import net.shibboleth.idp.session.AuthenticationEvent;
 import net.shibboleth.idp.session.IdPSession;
@@ -41,14 +40,14 @@
  * <li>Contain a {@link IdPSession}.
  * <li>Implement {@link InOutOperationContext} (usually it will be a BasicInOutOperationContext, but in tests it may
  * not).</li>
- * <li>Have associated with this, a valid {@link MessageContext} at getInboundMessageContext which in
- * turn <em>must</em>:
+ * <li>Have associated with this, a valid {@link MessageContext} at getInboundMessageContext which in turn
+ *  <em>must</em>:
  * <ul>
  * <li>Contain a {@link BasicMessageMetadataSubcontext} to provide the relying party name.</li>
  * </ul>
  * </ul>
  * 
- * */
+ */
 @ThreadSafe
 public abstract class AbstractPrincipalAttributeDefinition extends BaseAttributeDefinition {
 
@@ -57,75 +56,63 @@
 
     /**
      * Traverse the contexts as described above to get hold of the relying party (from the
-     * BasicMessageMetadataSubcontext). <bl />Emit errors and exceptions as we go.
+     * BasicMessageMetadataSubcontext). <bl />Emit errors and exceptions as we go. <br />
+     * <em>Note</em> If any of the context invariants fail we return null.
      * 
      * @param parent the parent of the attribute request.
      * @return the principal name, or null
-     * @throws AttributeResolutionException if we fail to traverse the tree.
      */
-    private String getRelyingParty(SubcontextContainer parent) throws AttributeResolutionException {
+    private String getRelyingParty(SubcontextContainer parent) {
         if (!(parent instanceof InOutOperationContext)) {
-            String errMsg =
-                    "Principal Attribute Definition " + getId()
-                            + ": Container does not implement InOutOperationContext";
-            log.error(errMsg);
-            throw new AttributeResolutionException(errMsg);
+            log.error("Attribute Defintion {}: Container does not implement InOutOperationContext.", getId());
+            return null;
         }
 
         final InOutOperationContext<?, ?> parentIoOperationContext = (InOutOperationContext) parent;
         final MessageContext<?> inboundContext = parentIoOperationContext.getInboundMessageContext();
 
         if (null == inboundContext) {
-            String errMsg = "Principal Attribute Definition " + getId() + ": No inbound context present";
-            log.error(errMsg);
-            throw new AttributeResolutionException(errMsg);
+            log.error("Attribute Defintion {}: No inbound context present.", getId());
+            return null;
         }
 
         final BasicMessageMetadataSubcontext messageMetadata =
                 inboundContext.getSubcontext(BasicMessageMetadataSubcontext.class, false);
         if (null == messageMetadata) {
-            String errMsg = "Principal Attribute Definition " + getId() + ": No Message Metadata context present";
-            log.error(errMsg);
-            throw new AttributeResolutionException(errMsg);
+            log.error("Attribute Defintion {}: No Message Metadata context presentt.", getId());
+            return null;
         }
-        final String relyingParty = StringSupport.trimOrNull(messageMetadata.getMessageIssuer());
-        if (null == relyingParty) {
-            String errMsg = "Principal Attribute Definition " + getId() + ": RelyingParty is null";
-            log.error(errMsg);

[... 149 lines stripped ...]