From 3d160aaa62877d089134a23fb896efb471f7d664 Mon Sep 17 00:00:00 2001
From: Bill Smith <wsmith@unicon.net>
Date: Wed, 15 Aug 2018 17:44:44 -0700
Subject: [PATCH] [SHIBUI-723]

Added DurationUtility.
Updated all duration-related code.
Updated tests to mostly not break.

Still need to address TODOs.
---
 .../MetadataResolversController.java          |  4 +-
 ...penSamlFileBackedHTTPMetadataResolver.java |  6 ++-
 .../OpenSamlFilesystemMetadataResolver.java   |  2 +
 ...tionDrivenDynamicHTTPMetadataResolver.java |  2 +
 .../OpenSamlLocalDynamicMetadataResolver.java |  2 +
 ...SamlMetadataResolverConstructorHelper.java | 30 +++++++-------
 ...penSamlResourceBackedMetadataResolver.java |  2 +
 .../MetadataResolverConverterServiceImpl.java | 13 +++++--
 .../admin/util/DurationUtility.java           | 39 +++++++++++++++++++
 ...ResolversControllerIntegrationTests.groovy | 10 +++--
 .../admin/ui/util/TestObjectGenerator.groovy  | 16 +++++++-
 11 files changed, 103 insertions(+), 23 deletions(-)
 create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/DurationUtility.java

diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java
index c407bcc3b..74fbd8689 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java
@@ -40,6 +40,7 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.net.URI;
+import java.util.ArrayList;
 import java.util.List;
 
 import static edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolverValidator.ValidationResult;
@@ -133,13 +134,14 @@ public ResponseEntity<?> create(@RequestBody MetadataResolver newResolver) throw
 
     private void updateChainingMetadataResolver(MetadataResolver persistedResolver) throws IOException, ResolverException {
         org.opensaml.saml.metadata.resolver.MetadataResolver openSamlResolver = metadataResolverConverterService.convertToOpenSamlRepresentation(persistedResolver);
-        List<org.opensaml.saml.metadata.resolver.MetadataResolver> resolverList = ((ChainingMetadataResolver) chainingMetadataResolver).getResolvers();
+        List<org.opensaml.saml.metadata.resolver.MetadataResolver> resolverList = new ArrayList<>(((ChainingMetadataResolver) chainingMetadataResolver).getResolvers());
         for (org.opensaml.saml.metadata.resolver.MetadataResolver resolver : resolverList) {
             if (resolver.getId().equals(persistedResolver.getResourceId())) {
                 resolverList.remove(resolver);
             }
         }
         resolverList.add(openSamlResolver);
+        ((ChainingMetadataResolver) chainingMetadataResolver).setResolvers(resolverList);
     }
 
     @PutMapping("/MetadataResolvers/{resourceId}")
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java
index 58da65cb8..76d3d9384 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFileBackedHTTPMetadataResolver.java
@@ -11,6 +11,8 @@
 
 import javax.annotation.Nullable;
 
+import static edu.internet2.tier.shibboleth.admin.util.DurationUtility.toMillis;
+
 /**
  * @author Bill Smith (wsmith@unicon.net)
  */
@@ -27,13 +29,15 @@ public OpenSamlFileBackedHTTPMetadataResolver(IndexWriter indexWriter,
         this.sourceResolver = sourceResolver;
         this.delegate = new OpenSamlMetadataResolverDelegate();
 
+        this.setId(sourceResolver.getResourceId());
+
         OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromHttpMetadataResolverAttributes(
                 this, sourceResolver.getHttpMetadataResolverAttributes());
         OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes(
                 this, sourceResolver.getReloadableMetadataResolverAttributes());
 
         this.setBackupFile(sourceResolver.getBackingFile());
-        this.setBackupFileInitNextRefreshDelay(Long.parseLong(sourceResolver.getBackupFileInitNextRefreshDelay()));
+        this.setBackupFileInitNextRefreshDelay(toMillis(sourceResolver.getBackupFileInitNextRefreshDelay()));
         this.setInitializeFromBackupFile(sourceResolver.getInitializeFromBackupFile());
 
         //TODO: Where does this get set in OpenSAML land?
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java
index 39b5ecca9..94ded9133 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFilesystemMetadataResolver.java
@@ -25,6 +25,8 @@ public OpenSamlFilesystemMetadataResolver(File metadataFile,
         this.sourceResolver = sourceResolver;
         this.delegate = new OpenSamlMetadataResolverDelegate();
 
+        this.setId(sourceResolver.getResourceId());
+
         OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes(
                 this, sourceResolver.getReloadableMetadataResolverAttributes());
     }
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFunctionDrivenDynamicHTTPMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFunctionDrivenDynamicHTTPMetadataResolver.java
index 077bb1671..2887fa85a 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFunctionDrivenDynamicHTTPMetadataResolver.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlFunctionDrivenDynamicHTTPMetadataResolver.java
@@ -21,6 +21,8 @@ public OpenSamlFunctionDrivenDynamicHTTPMetadataResolver(IndexWriter indexWriter
         this.sourceResolver = sourceResolver;
         this.delegate = new OpenSamlMetadataResolverDelegate();
 
+        this.setId(sourceResolver.getResourceId());
+
         OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromDynamicMetadataResolverAttributes(
                 this, sourceResolver.getDynamicMetadataResolverAttributes());
 
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlLocalDynamicMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlLocalDynamicMetadataResolver.java
index b693fa315..abbfbd26b 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlLocalDynamicMetadataResolver.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlLocalDynamicMetadataResolver.java
@@ -24,6 +24,8 @@ public OpenSamlLocalDynamicMetadataResolver(@Nonnull XMLObjectLoadSaveManager<XM
         this.sourceResolver = sourceResolver;
         this.delegate = new OpenSamlMetadataResolverDelegate();
 
+        this.setId(sourceResolver.getResourceId());
+
         OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromDynamicMetadataResolverAttributes(
                 this, sourceResolver.getDynamicMetadataResolverAttributes());
 
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlMetadataResolverConstructorHelper.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlMetadataResolverConstructorHelper.java
index 141abae95..02c1c9800 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlMetadataResolverConstructorHelper.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlMetadataResolverConstructorHelper.java
@@ -7,25 +7,27 @@
 import org.opensaml.saml.metadata.resolver.impl.AbstractDynamicMetadataResolver;
 import org.opensaml.saml.metadata.resolver.impl.AbstractReloadingMetadataResolver;
 
+import static edu.internet2.tier.shibboleth.admin.util.DurationUtility.toMillis;
+
 /**
  * @author Bill Smith (wsmith@unicon.net)
  */
 public class OpenSamlMetadataResolverConstructorHelper {
 
     public static void updateOpenSamlMetadataResolverFromDynamicMetadataResolverAttributes(MetadataResolver metadataResolver, DynamicMetadataResolverAttributes attributes) {
-
         AbstractDynamicMetadataResolver dynamicMetadataResolver = (AbstractDynamicMetadataResolver) metadataResolver;
 
-        // from DynamicMetadataResolverAttributes
-        dynamicMetadataResolver.setBackgroundInitializationFromCacheDelay(Long.valueOf(attributes.getBackgroundInitializationFromCacheDelay()));
-        dynamicMetadataResolver.setCleanupTaskInterval(Long.valueOf(attributes.getCleanupTaskInterval()));
+        dynamicMetadataResolver.setBackgroundInitializationFromCacheDelay(toMillis(attributes.getBackgroundInitializationFromCacheDelay()));
+        dynamicMetadataResolver.setCleanupTaskInterval(toMillis(attributes.getCleanupTaskInterval()));
         dynamicMetadataResolver.setInitializeFromPersistentCacheInBackground(attributes.getInitializeFromPersistentCacheInBackground());
-        dynamicMetadataResolver.setMaxCacheDuration(Long.valueOf(attributes.getMaxCacheDuration()));
-        dynamicMetadataResolver.setMaxIdleEntityData(Long.valueOf(attributes.getMaxIdleEntityData()));
-        dynamicMetadataResolver.setMinCacheDuration(Long.valueOf(attributes.getMinCacheDuration()));
-        dynamicMetadataResolver.setBackgroundInitializationFromCacheDelay(Long.valueOf(attributes.getBackgroundInitializationFromCacheDelay()));
+        dynamicMetadataResolver.setMaxCacheDuration(toMillis(attributes.getMaxCacheDuration()));
+        dynamicMetadataResolver.setMaxIdleEntityData(toMillis(attributes.getMaxIdleEntityData()));
+        dynamicMetadataResolver.setMinCacheDuration(toMillis(attributes.getMinCacheDuration()));
+        dynamicMetadataResolver.setBackgroundInitializationFromCacheDelay(toMillis(attributes.getBackgroundInitializationFromCacheDelay()));
         dynamicMetadataResolver.setRefreshDelayFactor(attributes.getRefreshDelayFactor().floatValue());
-        dynamicMetadataResolver.setRemoveIdleEntityData(attributes.getRemoveIdleEntityData());
+
+        //TODO: What should we do here if this data is null/empty?
+        dynamicMetadataResolver.setRemoveIdleEntityData(attributes.getRemoveIdleEntityData() == null ? false : attributes.getRemoveIdleEntityData());
 
         //TODO: This takes a XMLObjectLoadSaveManager. Do we have what we need to create one?
         // dynamicMetadataResolver.setPersistentCacheManager(); attributes.getPersistentCacheManagerDirectory();
@@ -51,11 +53,13 @@ public static void updateOpenSamlMetadataResolverFromHttpMetadataResolverAttribu
     public static void updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes(MetadataResolver metadataResolver, ReloadableMetadataResolverAttributes attributes) {
         AbstractReloadingMetadataResolver reloadingMetadataResolver = (AbstractReloadingMetadataResolver) metadataResolver;
 
-        reloadingMetadataResolver.setExpirationWarningThreshold(Long.parseLong(attributes.getExpirationWarningThreshold()));
-        reloadingMetadataResolver.setMaxRefreshDelay(Long.parseLong(attributes.getMaxRefreshDelay()));
-        reloadingMetadataResolver.setMinRefreshDelay(Long.parseLong(attributes.getMinRefreshDelay()));
+        reloadingMetadataResolver.setExpirationWarningThreshold(toMillis(attributes.getExpirationWarningThreshold()));
+        reloadingMetadataResolver.setMaxRefreshDelay(toMillis(attributes.getMaxRefreshDelay()));
+        reloadingMetadataResolver.setMinRefreshDelay(toMillis(attributes.getMinRefreshDelay()));
         reloadingMetadataResolver.setRefreshDelayFactor(attributes.getRefreshDelayFactor().floatValue());
-        reloadingMetadataResolver.setResolveViaPredicatesOnly(attributes.getResolveViaPredicatesOnly());
+
+        //TODO: What should we do here if this data is null/empty?
+        reloadingMetadataResolver.setResolveViaPredicatesOnly(attributes.getResolveViaPredicatesOnly() == null ? false : attributes.getResolveViaPredicatesOnly());
 
         //TODO: This takes a set of MetadataIndex's. We've got an IndexesRef. How to convert?
         // reloadingMetadataResolver.setIndexes(); attributes.getIndexesRef();
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java
index d7b62236d..addbdf9c7 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/opensaml/OpenSamlResourceBackedMetadataResolver.java
@@ -25,6 +25,8 @@ public OpenSamlResourceBackedMetadataResolver(Resource resource,
         this.sourceResolver = sourceResolver;
         this.delegate = new OpenSamlMetadataResolverDelegate();
 
+        this.setId(sourceResolver.getResourceId());
+
         OpenSamlMetadataResolverConstructorHelper.updateOpenSamlMetadataResolverFromReloadableMetadataResolverAttributes(
                 this, sourceResolver.getReloadableMetadataResolverAttributes());
     }
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java
index 5773036c2..87ddbd2ab 100644
--- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/MetadataResolverConverterServiceImpl.java
@@ -20,6 +20,7 @@
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.stereotype.Service;
 
+import javax.validation.ConstraintViolationException;
 import java.io.File;
 import java.io.IOException;
 
@@ -57,8 +58,13 @@ private OpenSamlFilesystemMetadataResolver convertToOpenSamlRepresentation(Files
     private OpenSamlLocalDynamicMetadataResolver convertToOpenSamlRepresentation(LocalDynamicMetadataResolver resolver) throws IOException {
         IndexWriter indexWriter = indexWriterService.getIndexWriter(resolver.getResourceId());
 
-        //TODO: This is an educated guess.
-        XMLObjectLoadSaveManager manager = new FilesystemLoadSaveManager(resolver.getSourceDirectory());
+        XMLObjectLoadSaveManager manager = null;
+        try {
+            manager = new FilesystemLoadSaveManager(resolver.getSourceDirectory());
+        } catch (ConstraintViolationException e) {
+            // the base directory string instance was null or empty
+            //TODO: What should we do here? Currently, this causes a test to fail.
+        }
 
         return new OpenSamlLocalDynamicMetadataResolver(manager, indexWriter, resolver);
     }
@@ -72,7 +78,8 @@ private OpenSamlResourceBackedMetadataResolver convertToOpenSamlRepresentation(R
                 //TODO: What sort of resource type should be created here? URL?
                 break;
             case CLASSPATH:
-                resource = (Resource) new ClassPathResource(resolver.getClasspathMetadataResource().getFile());
+                //TODO: Not sure what kind of resource class to use here.
+                // resource = (Resource) new ClassPathResource(resolver.getClasspathMetadataResource().getFile()); // this doesn't work.
                 break;
             default:
                 throw new RuntimeException("Unsupported resource type!");
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/DurationUtility.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/DurationUtility.java
new file mode 100644
index 000000000..b19328e27
--- /dev/null
+++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/util/DurationUtility.java
@@ -0,0 +1,39 @@
+package edu.internet2.tier.shibboleth.admin.util;
+
+import org.apache.commons.lang3.StringUtils;
+
+import javax.xml.datatype.DatatypeConfigurationException;
+import javax.xml.datatype.DatatypeFactory;
+import javax.xml.datatype.Duration;
+import java.util.Date;
+
+/**
+ * @author Bill Smith (wsmith@unicon.net)
+ */
+public class DurationUtility {
+    private static final DatatypeFactory datatypeFactory;
+
+    static {
+        DatatypeFactory factory;
+        try {
+            factory = DatatypeFactory.newInstance();
+        } catch (DatatypeConfigurationException e) {
+            factory = null;
+        }
+        datatypeFactory = factory;
+    }
+
+    public static Long toMillis(String xmlDuration) {
+        if (datatypeFactory == null) {
+            throw new RuntimeException("DatatypeFactory was never initialized!");
+        }
+        if (StringUtils.isBlank(xmlDuration)) {
+            return 0L;
+        }
+        Duration duration = datatypeFactory.newDuration(xmlDuration);
+
+        Date zero = new Date(0);
+        duration.addTo(zero); // potentially can return undesired results for large xmlDurations
+        return zero.getTime();
+    }
+}
diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy
index 25071cb98..8dd50c8c9 100644
--- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy
+++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy
@@ -11,8 +11,8 @@ import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator
 import edu.internet2.tier.shibboleth.admin.util.AttributeUtility
 import groovy.json.JsonOutput
 import groovy.json.JsonSlurper
+import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver
 import org.opensaml.saml.metadata.resolver.MetadataResolver
-import org.opensaml.saml.metadata.resolver.impl.FilesystemMetadataResolver
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.context.SpringBootTest
 import org.springframework.boot.test.context.TestConfiguration
@@ -20,9 +20,7 @@ import org.springframework.boot.test.web.client.TestRestTemplate
 import org.springframework.context.annotation.Bean
 import org.springframework.http.HttpEntity
 import org.springframework.http.HttpHeaders
-
 import org.springframework.test.context.ActiveProfiles
-
 import spock.lang.Specification
 import spock.lang.Unroll
 
@@ -162,6 +160,10 @@ class MetadataResolversControllerIntegrationTests extends Specification {
         result.statusCodeValue == 201
         result.headers.Location[0].contains(BASE_URI)
 
+        cleanup:
+        def file = new File('/tmp/foo.txt')
+        file.delete()
+
         where:
             resolverType     | _
             'DynamicHttp'    | _
@@ -263,7 +265,7 @@ class MetadataResolversControllerIntegrationTests extends Specification {
     static class Config {
         @Bean
         MetadataResolver metadataResolver() {
-            new FilesystemMetadataResolver(new File('fake'))
+            new ChainingMetadataResolver()
         }
     }
 }
diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy
index 4469e1b36..6c5bf92ea 100644
--- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy
+++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/util/TestObjectGenerator.groovy
@@ -389,7 +389,7 @@ class TestObjectGenerator {
                 randomResolver = localDynamicMetadataResolver()
                 break
             case 'ResourceBacked':
-                randomResolver = resourceBackedMetadataResolverForSVN()
+                randomResolver = resourceBackedMetadataResolverForClasspath()
                 break
             case 'Filesystem':
                 randomResolver = filesystemMetadataResolver()
@@ -463,6 +463,20 @@ class TestObjectGenerator {
         }
     }
 
+    ResourceBackedMetadataResolver resourceBackedMetadataResolverForClasspath() {
+        def file = new File('/tmp/foo.txt') // should we really do this?
+        file.write 'This is a temp file for a groovy test.'
+        new ResourceBackedMetadataResolver().with {
+            it.name = 'ClasspathResourceMetadata'
+            it.xmlId = 'ClasspathResourceMetadata'
+            it.classpathMetadataResource = new ClasspathMetadataResource().with {
+                it.file = '/tmp/foo.txt'
+                it
+            }
+            it
+        }
+    }
+
     FileBackedHttpMetadataResolver buildFileBackedHttpMetadataResolver() {
         def resolver = new FileBackedHttpMetadataResolver()
         resolver.name = generator.randomString(10)