[java-identity-provider] branch master updated: IDP-1535 - Reduce per-request audit formatter parsing

Scott Cantor cantor.2 at osu.edu
Thu Jan 23 13:49:37 EST 2020


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=ca5c976f65ed0ef31b4fa1b5e49980263b39e8ff

The following commit(s) were added to refs/heads/master by this push:
       new  ca5c976   IDP-1535 - Reduce per-request audit formatter parsing
ca5c976 is described below

commit ca5c976f65ed0ef31b4fa1b5e49980263b39e8ff
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Jan 23 13:49:27 2020 -0500

    IDP-1535 - Reduce per-request audit formatter parsing
    
    https://issues.shibboleth.net/jira/browse/IDP-1535
    
    Optimize out for audit population actions.
---
 .../main/resources/system/conf/global-system.xml   |  6 +-
 .../flows/intercept/attribute-release-beans.xml    |  8 +-
 .../system/flows/intercept/terms-of-use-beans.xml  |  8 +-
 .../profile/audit/impl/PopulateAuditContext.java   | 97 +++++++++++++++-------
 .../audit/impl/PopulateAuditContextTest.java       |  3 +-
 5 files changed, 86 insertions(+), 36 deletions(-)

diff --git a/idp-conf/src/main/resources/system/conf/global-system.xml b/idp-conf/src/main/resources/system/conf/global-system.xml
index 06e7cb4..e299e08 100644
--- a/idp-conf/src/main/resources/system/conf/global-system.xml
+++ b/idp-conf/src/main/resources/system/conf/global-system.xml
@@ -432,9 +432,13 @@
     
     <!-- Action bean templates. -->
     
+    <bean id="shibboleth.AuditFormattingMapParser"
+        class="net.shibboleth.idp.profile.audit.impl.PopulateAuditContext.FormattingMapParser"
+        c:_0-ref="shibboleth.AuditFormattingMap" />
+    
     <bean id="shibboleth.AbstractPopulateAuditContext" abstract="true"
         class="net.shibboleth.idp.profile.audit.impl.PopulateAuditContext" scope="prototype"
-        p:formattingMap-ref="shibboleth.AuditFormattingMap"
+        p:formattingMapParser-ref="shibboleth.AuditFormattingMapParser"
         p:dateTimeFormat="#{getObject('shibboleth.AuditDateTimeFormat')}"
         p:useDefaultTimeZone="#{getObject('shibboleth.AuditDefaultTimeZone') ?: false}"
         p:fieldReplacements="#{getObject('shibboleth.AuditFieldReplacementMap')}" />
diff --git a/idp-conf/src/main/resources/system/flows/intercept/attribute-release-beans.xml b/idp-conf/src/main/resources/system/flows/intercept/attribute-release-beans.xml
index 12be05c..555a39a 100644
--- a/idp-conf/src/main/resources/system/flows/intercept/attribute-release-beans.xml
+++ b/idp-conf/src/main/resources/system/flows/intercept/attribute-release-beans.xml
@@ -85,6 +85,10 @@
         class="net.shibboleth.idp.consent.logic.impl.PreferExplicitOrderComparator"
         c:order="#{getObject('shibboleth.consent.attribute-release.AttributeDisplayOrder')}" />
 
+    <bean id="AttributeReleaseAuditFormattingMapParser"
+        class="net.shibboleth.idp.profile.audit.impl.PopulateAuditContext.FormattingMapParser"
+        c:_0-ref="shibboleth.consent.attribute-release.AuditFormattingMap" />
+
     <!-- Action beans -->
 
     <bean id="InitializeConsentContext"
@@ -101,7 +105,7 @@
     <bean id="PopulatePreConsentAuditContext" parent="shibboleth.AbstractPopulateAuditContext"
         p:auditContextCreationStrategy-ref="shibboleth.consent.ChildLookup.ConsentAuditContext"
         p:fieldExtractors="#{getObject('shibboleth.consent.PreConsentAuditExtractors') ?: getObject('shibboleth.consent.DefaultPreConsentAuditExtractors')}"
-        p:formattingMap-ref="shibboleth.consent.attribute-release.AuditFormattingMap" />
+        p:formattingMapParser-ref="AttributeReleaseAuditFormattingMapParser" />
 
     <bean id="PopulateConsentContext"
         class="net.shibboleth.idp.consent.flow.impl.PopulateConsentContext" scope="prototype"
@@ -131,7 +135,7 @@
     <bean id="PopulateConsentAuditContext" parent="shibboleth.AbstractPopulateAuditContext"
         p:auditContextCreationStrategy-ref="shibboleth.consent.ChildLookup.ConsentAuditContext"
         p:fieldExtractors="#{getObject('shibboleth.consent.ConsentAuditExtractors') ?: getObject('shibboleth.consent.DefaultConsentAuditExtractors')}"
-        p:formattingMap-ref="shibboleth.consent.attribute-release.AuditFormattingMap" />
+        p:formattingMapParser-ref="AttributeReleaseAuditFormattingMapParser" />
 
     <bean id="ExtractConsent"
         class="net.shibboleth.idp.consent.flow.impl.ExtractConsent" scope="prototype"
diff --git a/idp-conf/src/main/resources/system/flows/intercept/terms-of-use-beans.xml b/idp-conf/src/main/resources/system/flows/intercept/terms-of-use-beans.xml
index 1cace50..6188b07 100644
--- a/idp-conf/src/main/resources/system/flows/intercept/terms-of-use-beans.xml
+++ b/idp-conf/src/main/resources/system/flows/intercept/terms-of-use-beans.xml
@@ -63,6 +63,10 @@
         </constructor-arg>
     </bean>
 
+    <bean id="TermsOfUseAuditFormattingMapParser"
+        class="net.shibboleth.idp.profile.audit.impl.PopulateAuditContext.FormattingMapParser"
+        c:_0-ref="shibboleth.consent.terms-of-use.AuditFormattingMap" />
+
     <!-- Action beans -->
     
     <bean id="InitializeConsentContext"
@@ -75,7 +79,7 @@
     <bean id="PopulatePreConsentAuditContext" parent="shibboleth.AbstractPopulateAuditContext"
         p:auditContextCreationStrategy-ref="shibboleth.consent.ChildLookup.ConsentAuditContext"
         p:fieldExtractors="#{getObject('shibboleth.consent.PreConsentAuditExtractors') ?: getObject('shibboleth.consent.DefaultPreConsentAuditExtractors')}"
-        p:formattingMap-ref="shibboleth.consent.terms-of-use.AuditFormattingMap" />
+        p:formattingMapParser-ref="TermsOfUseAuditFormattingMapParser" />
 
     <bean id="ReadConsentFromStorage"
         class="net.shibboleth.idp.consent.flow.storage.impl.ReadConsentFromStorage" scope="prototype"
@@ -93,7 +97,7 @@
     <bean id="PopulateConsentAuditContext" parent="shibboleth.AbstractPopulateAuditContext"
         p:auditContextCreationStrategy-ref="shibboleth.consent.ChildLookup.ConsentAuditContext"
         p:fieldExtractors="#{getObject('shibboleth.consent.ConsentAuditExtractors') ?: getObject('shibboleth.consent.DefaultConsentAuditExtractors')}"
-        p:formattingMap-ref="shibboleth.consent.terms-of-use.AuditFormattingMap" />
+        p:formattingMapParser-ref="TermsOfUseAuditFormattingMapParser" />
 
     <bean id="WriteConsentAuditLog" class="net.shibboleth.idp.profile.audit.impl.WriteAuditLog" scope="prototype"
         p:auditContextLookupStrategy-ref="shibboleth.consent.ChildLookup.ConsentAuditContext"
diff --git a/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContext.java b/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContext.java
index d0aa46f..62f1fd9 100644
--- a/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContext.java
+++ b/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContext.java
@@ -36,6 +36,8 @@ import net.shibboleth.idp.profile.AbstractProfileAction;
 import net.shibboleth.idp.profile.context.AuditContext;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotLive;
+import net.shibboleth.utilities.java.support.annotation.constraint.Unmodifiable;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -130,44 +132,21 @@ public class PopulateAuditContext extends AbstractProfileAction {
     }
     
     /**
-     * Set the map of logging category to formatting strings for log entries.
+     * Set the parsed map of logging category to formatting strings for log entries.
      * 
      * <p>A formatting string consists of tokens prefixed by '%' separated by any non-alphanumeric or whitespace.
      * Tokens can contain any letter or number or a hyphen. Anything other than a token, including whitespace, is
      * a literal.</p>
      * 
-     * @param map map of categories to formatting strings
+     * <p>The input class exposes the parsed field data for efficiency.</p>
+     * 
+     * @param parser the parsed map
      */
-    public void setFormattingMap(@Nonnull @NonnullElements final Map<String,String> map) {
+    public void setFormattingMapParser(@Nonnull final FormattingMapParser parser) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        Constraint.isNotNull(map, "Audit formatting map cannot be null");
-        
-        fieldsToExtract = new HashSet<>(10);
+        Constraint.isNotNull(parser, "Parsed audit formatting map cannot be null");
         
-        for (final String s : map.values()) {
-            final int len = s.length();
-            boolean inToken = false;
-            final StringBuilder field = new StringBuilder();
-            for (int pos = 0; pos < len; ++pos) {
-                final char ch = s.charAt(pos);
-                if (inToken) {
-                    if (!Character.isLetterOrDigit(ch) && ch != '-' && ch != '%') {
-                        fieldsToExtract.add(field.substring(1));
-                        field.setLength(0);
-                        inToken = false;
-                    }
-                } else if (ch == '%') {
-                    field.setLength(0);
-                    inToken = true;
-                }
-                
-                field.append(ch);
-            }
-            
-            if (field.length() > 0 && inToken) {
-                fieldsToExtract.add(field.substring(1));
-            }
-        }
+        fieldsToExtract = parser.getFieldsToExtract();
     }
 
     /**
@@ -290,4 +269,62 @@ public class PopulateAuditContext extends AbstractProfileAction {
             }
         }
     }
+
+    /**
+     * Parser for the formatting strings that exposes a final set of field labels that are
+     * present in any of the input formatters.
+     * 
+     * @since 4.0.0
+     */
+    public static class FormattingMapParser {
+        
+        /** Set of parsed fields. */
+        @Nonnull @NonnullElements final Set<String> fields;
+        
+        /**
+         * Constructor.
+         *
+         * @param map map of formatters to parse
+         */
+        public FormattingMapParser(@Nonnull @NonnullElements final Map<String,String> map) {
+            final Set<String> fieldsToExtract = new HashSet<>(10);
+            
+            for (final String s : map.values()) {
+                final int len = s.length();
+                boolean inToken = false;
+                final StringBuilder field = new StringBuilder();
+                for (int pos = 0; pos < len; ++pos) {
+                    final char ch = s.charAt(pos);
+                    if (inToken) {
+                        if (!Character.isLetterOrDigit(ch) && ch != '-' && ch != '%') {
+                            fieldsToExtract.add(field.substring(1));
+                            field.setLength(0);
+                            inToken = false;
+                        }
+                    } else if (ch == '%') {
+                        field.setLength(0);
+                        inToken = true;
+                    }
+                    
+                    field.append(ch);
+                }
+                
+                if (field.length() > 0 && inToken) {
+                    fieldsToExtract.add(field.substring(1));
+                }
+            }
+            
+            fields = Set.copyOf(fieldsToExtract);
+        }
+        
+        /**
+         * Get the parsed set of fields.
+         * 
+         * @return the parsed fields
+         */
+        @Nonnull @NonnullElements @NotLive @Unmodifiable public Set<String> getFieldsToExtract() {
+            return fields;
+        }
+    }
+    
 }
\ No newline at end of file
diff --git a/idp-profile-impl/src/test/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContextTest.java b/idp-profile-impl/src/test/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContextTest.java
index 62110d5..17a3f34 100644
--- a/idp-profile-impl/src/test/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContextTest.java
+++ b/idp-profile-impl/src/test/java/net/shibboleth/idp/profile/audit/impl/PopulateAuditContextTest.java
@@ -29,6 +29,7 @@ import net.shibboleth.idp.profile.ActionTestingSupport;
 import org.opensaml.profile.context.ProfileRequestContext;
 
 import net.shibboleth.idp.profile.RequestContextBuilder;
+import net.shibboleth.idp.profile.audit.impl.PopulateAuditContext.FormattingMapParser;
 import net.shibboleth.idp.profile.context.AuditContext;
 import net.shibboleth.idp.profile.context.navigate.WebflowRequestContextProfileRequestContextLookup;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
@@ -99,7 +100,7 @@ public class PopulateAuditContextTest {
         map.put("A", new MockFunction(Arrays.asList("bar", "baz")));
         
         action.setFieldExtractors(map);
-        action.setFormattingMap(Collections.singletonMap("foo", "%A - %b %%"));
+        action.setFormattingMapParser(new FormattingMapParser(Collections.singletonMap("foo", "%A - %b %%")));
         action.initialize();
         
         final Event event = action.execute(src);

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


More information about the commits mailing list