SHIBUI-2334: Scheduler continually tries to overwrite unchanged metadata

Adjust entity descriptor file writer to only write the those EDs that have no file or have been updated recently. Additionally added a locking mechanism so that multiple instances will not attempt to write while another process is doing so.
TIER · Mar 1, 2023 · 407cdd3 · 407cdd3
1 parent e1f8909
commit 407cdd3
Show file tree

Hide file tree

Showing 10 changed files with 121 additions and 38 deletions.
diff --git a/backend/build.gradle b/backend/build.gradle
@@ -165,6 +165,9 @@ dependencies {
     implementation "org.apache.logging.log4j:log4j-to-slf4j:${project.'log4JVersion'}"
     implementation "org.apache.logging.log4j:log4j-api:${project.'log4JVersion'}"
 
+    implementation "net.javacrumbs.shedlock:shedlock-spring:${project.'shedlockVersion'}"
+    implementation "net.javacrumbs.shedlock:shedlock-provider-jdbc-template:${project.'shedlockVersion'}"
+
     // TODO: figure out what this should really be
     runtimeOnly "org.springframework.boot:spring-boot-starter-tomcat:${project.'springbootVersion'}"
 

diff --git a/backend/src/main/app-resources/default.yml b/backend/src/main/app-resources/default.yml
@@ -12,6 +12,13 @@
 #  default-rootuser:root
 ## need to include the encoding for the password - be sure to quote the entire value as shown
 #  default-password: "{noop}foopassword"
+## Maximum amount of time to lock the scheduled task - the default is 30m, but should be increased if there are a large (1500+) entities
+## YMMV depending on the speed of connection to the DB, server processing capabilities etc.
+#  maxTask:
+#    lockTime: 30m
+## The first time the scheduler executes on this node, write out the entity descriptors to file [true|false] (default is true)
+#  entityDescriptor:
+#    writeOnStartup: true
 #  pac4j-enabled: true
 #  pac4j:
 #    keystorePath: "/etc/shibui/samlKeystore.jks"

diff --git a/backend/src/main/app-resources/release.yml b/backend/src/main/app-resources/release.yml
@@ -12,6 +12,12 @@
 #  default-rootuser:root
 ## need to include the encoding for the password - be sure to quote the entire value as shown
 #  default-password: "{noop}foopassword"
+## Maximum amount of time to lock the scheduled task - the default is 30m, but should be increased if there are a large (1500+) entities
+## YMMV depending on the speed of connection to the DB, server processing capabilities etc.
+#  maxTask:
+#    lockTime: 30m
+## The first time the scheduler executes on this node, write out the entity descriptors to file [true|false] (default is true)
+#  writeOnStartup: true
 #  pac4j-enabled: true
 #  pac4j:
 #    keystorePath: "/etc/shibui/samlKeystore.jks"

diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java b/...in/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java
@@ -39,6 +39,8 @@
 import edu.internet2.tier.shibboleth.admin.util.EntityDescriptorConversionUtils;
 import edu.internet2.tier.shibboleth.admin.util.LuceneUtility;
 import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions;
+import net.javacrumbs.shedlock.core.LockProvider;
+import net.javacrumbs.shedlock.provider.jdbctemplate.JdbcTemplateLockProvider;
 import org.apache.lucene.analysis.Analyzer;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -52,7 +54,7 @@
 import org.springframework.context.annotation.Import;
 import org.springframework.context.support.ResourceBundleMessageSource;
 import org.springframework.core.io.Resource;
-import org.springframework.web.client.RestTemplate;
+import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.web.servlet.LocaleResolver;
 import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
 import org.springframework.web.servlet.config.annotation.PathMatchConfigurer;
@@ -61,7 +63,7 @@
 import org.springframework.web.util.UrlPathHelper;
 
 import javax.servlet.http.HttpServletRequest;
-import java.net.URL;
+import javax.sql.DataSource;
 
 @Configuration
 @Import(SearchConfiguration.class)
@@ -96,26 +98,19 @@ public AttributeUtility attributeUtility() {
 
     @Bean
     @ConditionalOnProperty(name = "shibui.metadata-dir")
-    public EntityDescriptorFilesScheduledTasks entityDescriptorFilesScheduledTasks(
-                    EntityDescriptorRepository entityDescriptorRepository,
-                    @Value("${shibui.metadata-dir}") final String metadataDir) {
-        return new EntityDescriptorFilesScheduledTasks(metadataDir, entityDescriptorRepository, openSamlObjects(),
-                        fileWritingService());
+    public EntityDescriptorFilesScheduledTasks entityDescriptorFilesScheduledTasks(EntityDescriptorRepository entityDescriptorRepository, @Value("${shibui.metadata-dir}") final String metadataDir) {
+        return new EntityDescriptorFilesScheduledTasks(metadataDir, entityDescriptorRepository, openSamlObjects(), fileWritingService());
     }
 
     @Bean
     @ConditionalOnProperty(name = "shibui.metadataProviders.target")
-    public MetadataProvidersScheduledTasks metadataProvidersScheduledTasks(
-                    @Value("${shibui.metadataProviders.target}") final Resource resource,
-                    final MetadataResolverService metadataResolverService) {
+    public MetadataProvidersScheduledTasks metadataProvidersScheduledTasks(@Value("${shibui.metadataProviders.target}") final Resource resource, final MetadataResolverService metadataResolverService) {
         return new MetadataProvidersScheduledTasks(resource, metadataResolverService, fileWritingService());
     }
 
     @Bean
     @ConditionalOnProperty(name = "shibui.external.metadataProviders.target")
-    public ExternalMetadataProvidersScheduledTasks externalMetadataProvidersScheduledTasks(
-                    @Value("${shibui.external.metadataProviders.target}") final Resource resource,
-                    final MetadataResolverService metadataResolverService) {
+    public ExternalMetadataProvidersScheduledTasks externalMetadataProvidersScheduledTasks(@Value("${shibui.external.metadataProviders.target}") final Resource resource, final MetadataResolverService metadataResolverService) {
         return new ExternalMetadataProvidersScheduledTasks(resource, metadataResolverService, fileWritingService());
     }
 
@@ -254,4 +249,9 @@ public DynamicRegistrationService dynamicRegistrationService(DynamicRegistration
         ShibRestTemplateDelegate delegate = new ShibRestTemplateDelegate(config);
         return new JPADynamicRegistrationServiceImpl(groupService, driRepo, ownershipRepo, delegate, permissionEvaluator, userService);
     }
+
+    @Bean
+    public LockProvider lockProvider(DataSource dataSource) {
+        return new JdbcTemplateLockProvider(JdbcTemplateLockProvider.Configuration.builder().withJdbcTemplate(new JdbcTemplate(dataSource)).usingDbTime().build());
+    }
 }
diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/util/Shedlock.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/util/Shedlock.java
@@ -0,0 +1,20 @@
+package edu.internet2.tier.shibboleth.admin.ui.domain.util;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import java.util.Date;
+
+/**
+ * Generally speaking, this isn't used, but is here to ensure that the table gets created cleanly regardless of DB type
+ */
+@Entity
+public class Shedlock {
+    @Id
+    String name;
+
+    Date lockUntil;
+
+    Date lockedAt;
+
+    String locked_by;
+}
diff --git a/...edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java b/...edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java
@@ -5,10 +5,13 @@
 import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects;
 import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository;
 import edu.internet2.tier.shibboleth.admin.ui.service.FileWritingService;
+import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock;
+import net.javacrumbs.shedlock.spring.annotation.SchedulerLock;
 import org.bouncycastle.util.encoders.Hex;
 import org.opensaml.core.xml.io.MarshallingException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.scheduling.annotation.Scheduled;
@@ -21,12 +24,13 @@
 import java.nio.file.Paths;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.time.LocalDateTime;
+import java.util.HashMap;
 import java.util.Set;
 import java.util.stream.Stream;
 
 import static java.util.stream.Collectors.toSet;
 
-
 /**
  * This class is wrapped with Spring's scheduling facility to produce background scheduled periodic tasks pertaining to
  * generating, cleaning, etc. of entity descriptor files conforming to Shibboleth's <i>LocalDynamicMetadataProvider</i> naming
@@ -36,21 +40,22 @@
  */
 @Configuration
 @ConditionalOnProperty(name = "shibui.metadata-dir")
+@EnableSchedulerLock(defaultLockAtMostFor = "${shibui.maxTask.lockTime:30m}")
 public class EntityDescriptorFilesScheduledTasks {
-
     private static final Logger LOGGER = LoggerFactory.getLogger(EntityDescriptorFilesScheduledTasks.class);
+    private static final String SHA1HEX_FILENAME_TEMPLATE = "%s.xml";
+    private static final String TARGET_FILE_TEMPLATE = "%s/%s";
 
-    private String metadataDirName;
+    /** cache by entity id and the last time we wrote out (or started the system) */
+    private static final HashMap<String, LocalDateTime> LAST_WRITTEN_CACHE = new HashMap();
 
     private EntityDescriptorRepository entityDescriptorRepository;
-
+    private final FileWritingService fileWritingService;
+    private String metadataDirName;
     private OpenSamlObjects openSamlObjects;
 
-    private static final String SHA1HEX_FILENAME_TEMPLATE = "%s.xml";
-
-    private static final String TARGET_FILE_TEMPLATE = "%s/%s";
-
-    private final FileWritingService fileWritingService;
+    @Value("${shibui.entityDescriptor.writeOnStartup:true}")
+    private boolean writeOnStartup;
 
     public EntityDescriptorFilesScheduledTasks(String metadataDirName,
                                                EntityDescriptorRepository entityDescriptorRepository,
@@ -63,28 +68,60 @@ public EntityDescriptorFilesScheduledTasks(String metadataDirName,
     }
 
     @Scheduled(fixedRateString = "${shibui.taskRunRate:30000}")
+    @SchedulerLock(name = "generateEntityDescriptorFiles")
     @Transactional(readOnly = true)
-    public void generateEntityDescriptorFiles() throws MarshallingException {
+    public void generateEntityDescriptorFiles() {
         this.entityDescriptorRepository.findAllStreamByServiceEnabled(true)
                 .forEach(ed -> {
                     Path targetFilePath = targetFilePathFor(toSha1HexString(ed.getEntityID()));
-                    if (Files.exists(targetFilePath)) {
-                        LOGGER.info("Overwriting entity descriptor file [{}] for entity id [{}]", targetFilePath, ed.getEntityID());
-                    } else {
-                        LOGGER.info("Generating entity descriptor file [{}] for entity id [{}]", targetFilePath, ed.getEntityID());
-                    }
-
-                    try {
-                        String xmlContent = this.openSamlObjects.marshalToXmlString(ed);
-                        fileWritingService.write(targetFilePath, xmlContent);
-                    } catch (MarshallingException | IOException e) {
-                        //TODO: any other better way to handle it?
-                        LOGGER.error("Error marshalling entity descriptor into a file {} - {}", ed.getEntityID(), e.getMessage());
+                    if (needToWriteUpdate(ed)) {
+                        LOGGER.info("Generating/Overwriting entity descriptor file [{}] for entity id [{}]", targetFilePath, ed.getEntityID());
+                        writeFile(ed, targetFilePath);
                     }
                 });
     }
 
+    /**
+     * @return boolean true if any of the conditions for writing a file are true
+     * ** The file does not exist
+     * OR
+     * ** Write on startup (ie if we don't have a record of the file being written (there are a couple of scenarios that fit this)) IF configured to do so
+     * OR
+     * ** The ed last update/change time is after the last written cache time
+     */
+    private boolean needToWriteUpdate(EntityDescriptor ed) {
+        Path targetFilePath = targetFilePathFor(toSha1HexString(ed.getEntityID()));
+        // If the file has never been written, write it out
+        if (!Files.exists(targetFilePath)) {
+            return true;
+        }
+        // If we don't have an entry in the cache, add the entry and write it out only if configured to do so on startup
+        // Regardless, if we don't have a cached entry, cache one
+        if (!LAST_WRITTEN_CACHE.containsKey(ed.getEntityID())) {
+            LAST_WRITTEN_CACHE.put(ed.getEntityID(), LocalDateTime.now());
+            return writeOnStartup;
+        }
+        // If the entity is in the cache, write out only if the entity's last modified time is after the last cache time.
+        if (LAST_WRITTEN_CACHE.get(ed.getEntityID()).isBefore(ed.getModifiedDate())) {
+            LAST_WRITTEN_CACHE.put(ed.getEntityID(), LocalDateTime.now());
+            return true;
+        }
+        return false;
+    }
+
+    private void writeFile(EntityDescriptor ed, Path targetFilePath) {
+        try {
+            String xmlContent = this.openSamlObjects.marshalToXmlString(ed);
+            fileWritingService.write(targetFilePath, xmlContent);
+        } catch (MarshallingException | IOException e) {
+            //TODO: any other better way to handle it?
+            LOGGER.error("Error marshalling entity descriptor into a file {} - {}", ed.getEntityID(), e.getMessage());
+        }
+        LAST_WRITTEN_CACHE.put(ed.getEntityID(), LocalDateTime.now());
+    }
+
     @Scheduled(fixedDelayString = "${shibui.taskDelayRate:30000}")
+    @SchedulerLock(name = "removeDanglingEntityDescriptorFiles")
     @Transactional(readOnly = true)
     public void removeDanglingEntityDescriptorFiles() {
         Path targetDirPath = Paths.get(this.metadataDirName);
@@ -135,4 +172,4 @@ private Path targetFilePathFor(String sha1HexBaseFilename) {
         String filenamePath = String.format(TARGET_FILE_TEMPLATE, this.metadataDirName, filename);
         return Paths.get(filenamePath);
     }
-}
+}
diff --git a/...internet2/tier/shibboleth/admin/ui/scheduled/ExternalMetadataProvidersScheduledTasks.java b/...internet2/tier/shibboleth/admin/ui/scheduled/ExternalMetadataProvidersScheduledTasks.java
@@ -2,6 +2,8 @@
 
 import edu.internet2.tier.shibboleth.admin.ui.service.FileWritingService;
 import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService;
+import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock;
+import net.javacrumbs.shedlock.spring.annotation.SchedulerLock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -22,6 +24,7 @@
 
 @Configuration
 @ConditionalOnProperty("shibui.external.metadataProviders.target")
+@EnableSchedulerLock(defaultLockAtMostFor = "30m")
 public class ExternalMetadataProvidersScheduledTasks {
     private static final Logger logger = LoggerFactory.getLogger(ExternalMetadataProvidersScheduledTasks.class);
 
@@ -36,6 +39,7 @@ public ExternalMetadataProvidersScheduledTasks(Resource target, MetadataResolver
     }
 
     @Scheduled(fixedRateString = "${shibui.external.metadataProviders.taskRunRate:30000}")
+    @SchedulerLock(name = "generateExternalMetadataProvidersFile")
     @Transactional(readOnly = true)
     public void generateMetadataProvidersFile() {
         try (StringWriter os = new StringWriter()) {

diff --git a/...ava/edu/internet2/tier/shibboleth/admin/ui/scheduled/MetadataProvidersScheduledTasks.java b/...ava/edu/internet2/tier/shibboleth/admin/ui/scheduled/MetadataProvidersScheduledTasks.java
@@ -2,6 +2,8 @@
 
 import edu.internet2.tier.shibboleth.admin.ui.service.FileWritingService;
 import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService;
+import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock;
+import net.javacrumbs.shedlock.spring.annotation.SchedulerLock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -22,6 +24,7 @@
 
 @Configuration
 @ConditionalOnProperty("shibui.metadataProviders.target")
+@EnableSchedulerLock(defaultLockAtMostFor = "30m")
 public class MetadataProvidersScheduledTasks {
     private static final Logger logger = LoggerFactory.getLogger(MetadataProvidersScheduledTasks.class);
 
@@ -36,6 +39,7 @@ public MetadataProvidersScheduledTasks(Resource target, MetadataResolverService
     }
 
     @Scheduled(fixedRateString = "${shibui.metadataProviders.taskRunRate:30000}")
+    @SchedulerLock(name = "generateMetadataProvidersFile")
     @Transactional(readOnly = true)
     public void generateMetadataProvidersFile() {
         try (StringWriter os = new StringWriter()) {

diff --git a/...ernet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy b/...ernet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy
@@ -113,7 +113,7 @@ class EntityDescriptorFilesScheduledTasksTests extends AbstractBaseDataJpaTest {
         entityDescriptorFilesScheduledTasks.removeDanglingEntityDescriptorFiles()
 
         then:
-        def files = new File(tempPath, file)
-        files.size() == 0
+        def file2 = new File(tempPath, file)
+        !file2.exists()
     }
 }
diff --git a/gradle.properties b/gradle.properties
@@ -12,6 +12,8 @@ lombokVersion=5.3.3.3
 opensamlVersion=4.2.0
 pac4JVersion=5.4.3
 pac4jSpringSecurityVersion=7.0.3
+# update shedlock to 5.x when updating to java 17+Spring 6 etc
+shedlockVersion=4.42.0
 shibbolethVersion=4.2.1
 shibOIDCVersion=2.1.0
 springbootVersion=2.7.0