From aedf05b41fad75c85ef846ef82d2ee03270efcf1 Mon Sep 17 00:00:00 2001 From: alandavis Date: Thu, 15 Aug 2019 14:33:15 +0100 Subject: [PATCH] REPO-4589 Configure Mimetypes for custom renditions (#562) - ConfigFileFinder and ConfigScheduler moved to data-model for use in MimetypeMap - Needed to force mimetypeMap to load config straight away to get tests to pass with: mimetype.config.cronExpression=0 0 0 ? JAN * 1970 --- pom.xml | 2 +- .../LocalTransformServiceRegistry.java | 3 +- .../RenditionDefinitionRegistry2Impl.java | 6 +- .../config/TransformServiceRegistryImpl.java | 3 +- .../org/alfresco/util/ConfigFileFinder.java | 219 ----------------- .../org/alfresco/util/ConfigScheduler.java | 229 ------------------ .../alfresco/content-services-context.xml | 10 +- .../resources/alfresco/repository.properties | 2 +- ...calTransformServiceRegistryConfigTest.java | 8 +- src/test/resources/alfresco-global.properties | 3 + 10 files changed, 22 insertions(+), 463 deletions(-) delete mode 100644 src/main/java/org/alfresco/util/ConfigFileFinder.java delete mode 100644 src/main/java/org/alfresco/util/ConfigScheduler.java diff --git a/pom.xml b/pom.xml index 442e94b57f..6c0dded5ba 100644 --- a/pom.xml +++ b/pom.xml @@ -40,7 +40,7 @@ 6.2 7.21 6.1 - 8.47 + 8.48 7.1 1.1 1.0.11 diff --git a/src/main/java/org/alfresco/repo/content/transform/LocalTransformServiceRegistry.java b/src/main/java/org/alfresco/repo/content/transform/LocalTransformServiceRegistry.java index bb83774095..e0fd49a52e 100644 --- a/src/main/java/org/alfresco/repo/content/transform/LocalTransformServiceRegistry.java +++ b/src/main/java/org/alfresco/repo/content/transform/LocalTransformServiceRegistry.java @@ -122,7 +122,6 @@ public class LocalTransformServiceRegistry extends TransformServiceRegistryImpl public void afterPropertiesSet() throws Exception { PropertyCheck.mandatory(this, "mimetypeService", mimetypeService); - PropertyCheck.mandatory(this, "pipelineConfigDir", pipelineConfigDir); PropertyCheck.mandatory(this, "properties", properties); PropertyCheck.mandatory(this, "transformerDebug", transformerDebug); strictMimetypeExceptions = getStrictMimetypeExceptions(); @@ -145,7 +144,7 @@ public class LocalTransformServiceRegistry extends TransformServiceRegistryImpl } @Override - public synchronized LocalData getData() + public LocalData getData() { return (LocalData)super.getData(); } diff --git a/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java b/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java index 193fe97526..ddebd5a716 100644 --- a/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java +++ b/src/main/java/org/alfresco/repo/rendition2/RenditionDefinitionRegistry2Impl.java @@ -191,9 +191,9 @@ public class RenditionDefinitionRegistry2Impl implements RenditionDefinitionRegi public void afterPropertiesSet() throws Exception { PropertyCheck.mandatory(this, "transformServiceRegistry", transformServiceRegistry); - PropertyCheck.mandatory(this, "renditionConfigDir", renditionConfigDir); PropertyCheck.mandatory(this, "timeoutDefault", timeoutDefault); PropertyCheck.mandatory(this, "jsonObjectMapper", jsonObjectMapper); + // If we have a cronExpression it indicates that we will schedule reading. if (cronExpression != null) { PropertyCheck.mandatory(this, "initialAndOnErrorCronExpression", initialAndOnErrorCronExpression); @@ -239,12 +239,12 @@ public class RenditionDefinitionRegistry2Impl implements RenditionDefinitionRegi return new Data(); } - public synchronized Data getData() + public Data getData() { return configScheduler.getData(); } - public boolean readConfig() throws IOException + public boolean readConfig() { boolean successReadingConfig = configFileFinder.readFiles("alfresco/renditions", log); if (renditionConfigDir != null && !renditionConfigDir.isBlank()) diff --git a/src/main/java/org/alfresco/transform/client/model/config/TransformServiceRegistryImpl.java b/src/main/java/org/alfresco/transform/client/model/config/TransformServiceRegistryImpl.java index 0ef7565adf..56d0fc142c 100644 --- a/src/main/java/org/alfresco/transform/client/model/config/TransformServiceRegistryImpl.java +++ b/src/main/java/org/alfresco/transform/client/model/config/TransformServiceRegistryImpl.java @@ -148,6 +148,7 @@ public abstract class TransformServiceRegistryImpl implements TransformServiceRe public void afterPropertiesSet() throws Exception { PropertyCheck.mandatory(this, "jsonObjectMapper", jsonObjectMapper); + // If we have a cronExpression it indicates that we will schedule reading. if (cronExpression != null) { PropertyCheck.mandatory(this, "initialAndOnErrorCronExpression", initialAndOnErrorCronExpression); @@ -162,7 +163,7 @@ public abstract class TransformServiceRegistryImpl implements TransformServiceRe return new Data(); } - public synchronized Data getData() + public Data getData() { return configScheduler.getData(); } diff --git a/src/main/java/org/alfresco/util/ConfigFileFinder.java b/src/main/java/org/alfresco/util/ConfigFileFinder.java deleted file mode 100644 index 6b796c788b..0000000000 --- a/src/main/java/org/alfresco/util/ConfigFileFinder.java +++ /dev/null @@ -1,219 +0,0 @@ -/* - * #%L - * Alfresco Repository - * %% - * Copyright (C) 2005 - 2019 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ -package org.alfresco.util; - -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.commons.logging.Log; - -import java.io.File; -import java.io.FileNotFoundException; -import java.io.FileReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.Reader; -import java.net.URL; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; - -/** - * Used to find configuration files as resources from the jar file or from some external location. The path supplied - * to {@link #readFiles(String, Log)} may be a directory name. Normally used by ConfigScheduler. - * - * @author adavis - */ -public abstract class ConfigFileFinder -{ - private final ObjectMapper jsonObjectMapper; - private int fileCount; - - public ConfigFileFinder(ObjectMapper jsonObjectMapper) - { - this.jsonObjectMapper = jsonObjectMapper; - } - - public int getFileCount() - { - return fileCount; - } - - public void setFileCount(int fileCount) - { - this.fileCount = fileCount; - } - - public boolean readFiles(String path, Log log) - { - AtomicBoolean successReadingConfig = new AtomicBoolean(true); - try - { - AtomicBoolean somethingRead = new AtomicBoolean(false); - - // Try reading resources in a jar - final File jarFile = new File(getClass().getProtectionDomain().getCodeSource().getLocation().getPath()); - if (jarFile.isFile()) - { - readFromJar(jarFile, path, log, successReadingConfig, somethingRead); - } - else - { - // Try reading resources from disk - URL url = getClass().getClassLoader().getResource(path); - if (url != null) - { - String urlPath = url.getPath(); - readFromDisk(urlPath, log, successReadingConfig, somethingRead); - } - } - - if (!somethingRead.get() && new File(path).exists()) - { - // Try reading files from disk - readFromDisk(path, log, successReadingConfig, somethingRead); - } - - if (!somethingRead.get()) - { - log.warn("No config read from "+path); - } - } - catch (IOException e) - { - log.error("Error reading from "+path); - successReadingConfig.set(false); - } - return successReadingConfig.get(); - } - - private void readFromJar(File jarFile, String path, Log log, AtomicBoolean successReadingConfig, AtomicBoolean somethingRead) throws IOException - { - JarFile jar = new JarFile(jarFile); - try - { - Enumeration entries = jar.entries(); // gives ALL entries in jar - String prefix = path + "/"; - List names = new ArrayList<>(); - while (entries.hasMoreElements()) - { - final String name = entries.nextElement().getName(); - if ((name.startsWith(prefix) && name.length() > prefix.length()) || - (name.equals(path))) - { - names.add(name); - } - } - Collections.sort(names); - for (String name : names) - { - InputStreamReader reader = new InputStreamReader(getResourceAsStream(name)); - readFromReader(successReadingConfig, somethingRead, reader, "resource", name, null, log); - } - } - finally - { - jar.close(); - } - } - - private void readFromDisk(String path, Log log, AtomicBoolean successReadingConfig, AtomicBoolean somethingRead) throws FileNotFoundException - { - File root = new File(path); - if (root.isDirectory()) - { - File[] files = root.listFiles(); - Arrays.sort(files, (file1, file2) -> file1.getName().compareTo(file2.getName())); - for (File file : files) - { - FileReader reader = new FileReader(file); - String filePath = file.getPath(); - readFromReader(successReadingConfig, somethingRead, reader, "file", filePath, null, log); - } - } - else - { - FileReader reader = new FileReader(root); - String filePath = root.getPath(); - readFromReader(successReadingConfig, somethingRead, reader, "file", filePath, null, log); - } - } - - private InputStream getResourceAsStream(String resource) - { - final InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream(resource); - return in == null ? getClass().getResourceAsStream(resource) : in; - } - - private void readFromReader(AtomicBoolean successReadingConfig, AtomicBoolean somethingRead, - Reader reader, String readFrom, String path, String baseUrl, Log log) - { - somethingRead.set(true); - boolean success = readFile(reader, readFrom, path, null, log); - if (success) - { - fileCount++; - } - boolean newSuccessReadingConfig = successReadingConfig.get(); - newSuccessReadingConfig &= success; - successReadingConfig.set(newSuccessReadingConfig); - } - - public boolean readFile(Reader reader, String readFrom, String path, String baseUrl, Log log) - { - // At the moment it is assumed the file is Json, but that does not need to be the case. - // We have the path including extension. - boolean successReadingConfig = true; - try - { - JsonNode jsonNode = jsonObjectMapper.readValue(reader, new TypeReference() {}); - String readFromMessage = readFrom + ' ' + path; - if (log.isTraceEnabled()) - { - log.trace(readFromMessage + " config is: " + jsonNode); - } - else - { - log.debug(readFromMessage + " config read"); - } - readJson(jsonNode, readFromMessage, baseUrl); - } - catch (Exception e) - { - log.error("Error reading "+path); - successReadingConfig = false; - } - return successReadingConfig; - } - - protected abstract void readJson(JsonNode jsonNode, String readFromMessage, String baseUrl) throws IOException; -} diff --git a/src/main/java/org/alfresco/util/ConfigScheduler.java b/src/main/java/org/alfresco/util/ConfigScheduler.java deleted file mode 100644 index aa982b880b..0000000000 --- a/src/main/java/org/alfresco/util/ConfigScheduler.java +++ /dev/null @@ -1,229 +0,0 @@ -/* - * #%L - * Alfresco Repository - * %% - * Copyright (C) 2005 - 2019 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ -package org.alfresco.util; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.quartz.CronExpression; -import org.quartz.CronScheduleBuilder; -import org.quartz.CronTrigger; -import org.quartz.Job; -import org.quartz.JobBuilder; -import org.quartz.JobDataMap; -import org.quartz.JobDetail; -import org.quartz.JobExecutionContext; -import org.quartz.JobExecutionException; -import org.quartz.JobKey; -import org.quartz.Scheduler; -import org.quartz.TriggerBuilder; -import org.quartz.impl.StdSchedulerFactory; - -import java.io.IOException; -import java.util.Date; - -/** - * Used to schedule reading of config. The config is assumed to change from time to time. - * Initially or on error the reading frequency is high but slower once no problems are reported. - * If the normal cron schedule is not set or is in the past, the config is read only once when - * {@link #run(boolean, Log, CronExpression, CronExpression)} is called. - * - * @author adavis - */ -public abstract class ConfigScheduler -{ - public static class ConfigSchedulerJob implements Job - { - @Override - public void execute(JobExecutionContext context) throws JobExecutionException - { - JobDataMap dataMap = context.getJobDetail().getJobDataMap(); - ConfigScheduler configScheduler = (ConfigScheduler)dataMap.get(CONFIG_SCHEDULER); - boolean successReadingConfig = configScheduler.readConfigAndReplace(true); - configScheduler.changeScheduleOnStateChange(successReadingConfig); - } - } - - public static final String CONFIG_SCHEDULER = "configScheduler"; - - private static final Log defaultLog = LogFactory.getLog(ConfigScheduler.class); - private static StdSchedulerFactory schedulerFactory = new StdSchedulerFactory(); - - private final String jobName; - private Log log; - private CronExpression cronExpression; - private CronExpression initialAndOnErrorCronExpression; - - private Scheduler scheduler; - private JobKey jobKey; - private boolean normalCronSchedule; - - protected Data data; - private ThreadLocal threadData = ThreadLocal.withInitial(() -> data); - - public ConfigScheduler(Object client) - { - jobName = client.getClass().getName()+"Job"; - } - - public abstract boolean readConfig() throws IOException; - - public abstract Data createData(); - - public synchronized Data getData() - { - Data data = threadData.get(); - if (data == null) - { - data = createData(); - setData(data); - } - return data; - } - - private synchronized void setData(Data data) - { - this.data = data; - threadData.set(data); - } - - private synchronized void clearData() - { - this.data = null; // as run() should only be called multiple times in testing, it is okay to discard the - // previous data, as there should be no other Threads trying to read it, unless they are - // left over from previous tests. - threadData.remove(); // we need to pick up the initial value next time (whatever the data value is at that point) - } - - /** - * This method should only be called once in production on startup generally from Spring afterPropertiesSet methods. - * In testing it is allowed to call this method multiple times, but in that case it is recommended to pass in a - * null cronExpression (or a cronExpression such as a date in the past) so the scheduler is not started. If this is - * done, the config is still read, but before the method returns. - */ - public void run(boolean enabled, Log log, CronExpression cronExpression, CronExpression initialAndOnErrorCronExpression) - { - clearPreviousSchedule(); - clearData(); - if (enabled) - { - this.log = log == null ? ConfigScheduler.defaultLog : log; - Date now = new Date(); - if (cronExpression != null && - initialAndOnErrorCronExpression != null && - cronExpression.getNextValidTimeAfter(now) != null && - initialAndOnErrorCronExpression.getNextValidTimeAfter(now) != null) - { - this.cronExpression = cronExpression; - this.initialAndOnErrorCronExpression = initialAndOnErrorCronExpression; - schedule(); - } - else - { - readConfigAndReplace(false); - } - } - } - - private synchronized void schedule() - { - try - { - scheduler = schedulerFactory.getScheduler(); - - JobDetail job = JobBuilder.newJob() - .withIdentity(jobName) - .ofType(ConfigSchedulerJob.class) - .build(); - jobKey = job.getKey(); - job.getJobDataMap().put(CONFIG_SCHEDULER, this); - CronExpression cronExpression = normalCronSchedule ? this.cronExpression : initialAndOnErrorCronExpression; - CronTrigger trigger = TriggerBuilder.newTrigger() - .withIdentity(jobName+"Trigger", Scheduler.DEFAULT_GROUP) - .withSchedule(CronScheduleBuilder.cronSchedule(cronExpression)) - .build(); - scheduler.startDelayed(0); - scheduler.scheduleJob(job, trigger); - log.debug("Schedule set "+cronExpression); - } - catch (Exception e) - { - log.error("Error scheduling "+e.getMessage()); - } - } - - private void clearPreviousSchedule() - { - if (scheduler != null) - { - try - { - scheduler.deleteJob(jobKey); - scheduler = null; - jobKey = null; - } - catch (Exception e) - { - log.error("Error clearing previous schedule " + e.getMessage()); - } - } - } - - private boolean readConfigAndReplace(boolean scheduledRead) - { - boolean successReadingConfig; - log.debug((scheduledRead ? "Scheduled" : "Unscheduled")+" config read started"); - Data data = getData(); - try - { - Data newData = createData(); - threadData.set(newData); - successReadingConfig = readConfig(); - data = newData; - log.debug("Config read finished "+data+ - (successReadingConfig ? "" : ". Config replaced but there were problems")+"\n"); - } - catch (Exception e) - { - successReadingConfig = false; - log.error("Config read failed. "+e.getMessage(), e); - } - setData(data); - return successReadingConfig; - } - - private void changeScheduleOnStateChange(boolean successReadingConfig) - { - // Switch schedule sequence if we were on the normal schedule and we now have problems or if - // we are on the initial/error schedule and there were no errors. - if ( normalCronSchedule && !successReadingConfig || - !normalCronSchedule && successReadingConfig) - { - normalCronSchedule = !normalCronSchedule; - clearPreviousSchedule(); - schedule(); - } - } -} diff --git a/src/main/resources/alfresco/content-services-context.xml b/src/main/resources/alfresco/content-services-context.xml index e71092ee8f..f0e69ac5b8 100644 --- a/src/main/resources/alfresco/content-services-context.xml +++ b/src/main/resources/alfresco/content-services-context.xml @@ -226,9 +226,15 @@ + + + + - - + + + + diff --git a/src/main/resources/alfresco/repository.properties b/src/main/resources/alfresco/repository.properties index 74aaa37242..5ab3dd8f85 100644 --- a/src/main/resources/alfresco/repository.properties +++ b/src/main/resources/alfresco/repository.properties @@ -1087,7 +1087,7 @@ mimetype.config.initialAndOnError.cronExpression=0/10 * * * * ? # Optional property to specify an external file or directory that will be read for mimetype definitions from YAML # files (possibly added to a volume via k8 ConfigMaps). -mimetype.config.dir=shared/classes/alfresco/extension/transform/mimetypes +mimetype.config.dir=shared/classes/alfresco/extension/mimetypes # Schedule for reading rendition config definitions dynamically. Initially checks every 10 seconds and then switches to # every hour after the configuration is read successfully. If there is a error later reading the config, the diff --git a/src/test/java/org/alfresco/transform/client/model/config/LocalTransformServiceRegistryConfigTest.java b/src/test/java/org/alfresco/transform/client/model/config/LocalTransformServiceRegistryConfigTest.java index 4a9bc1aa9f..b9805c58b3 100644 --- a/src/test/java/org/alfresco/transform/client/model/config/LocalTransformServiceRegistryConfigTest.java +++ b/src/test/java/org/alfresco/transform/client/model/config/LocalTransformServiceRegistryConfigTest.java @@ -36,7 +36,6 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.quartz.CronExpression; -import org.quartz.Scheduler; import java.io.IOException; import java.util.ArrayList; @@ -48,7 +47,6 @@ import java.util.Properties; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -94,7 +92,7 @@ public class LocalTransformServiceRegistryConfigTest extends TransformServiceReg } @Override - public synchronized LocalData getData() + public LocalData getData() { return dummyData; } @@ -140,7 +138,7 @@ public class LocalTransformServiceRegistryConfigTest extends TransformServiceReg private Map> officeToImageViaPdfSupportedTransformation; private int readConfigCount; - private long startMs; + private long startMs = System.currentTimeMillis(); @Before public void setUp() throws Exception @@ -427,7 +425,7 @@ public class LocalTransformServiceRegistryConfigTest extends TransformServiceReg readConfigCount = 0; registry.setInitialAndOnErrorCronExpression(new CronExpression(("0/2 * * ? * * *"))); // every 2 seconds rather than 10 seconds - registry.setCronExpression(new CronExpression(("0/4 * * ? * * *"))); // every 4 seconds rather than 10 mins + registry.setCronExpression(new CronExpression(("0/4 * * ? * * *"))); // every 4 seconds rather than every hour // Sleep until a 6 second boundary, in order to make testing clearer. // It avoids having to work out schedule offsets and extra quick runs that can otherwise take place. diff --git a/src/test/resources/alfresco-global.properties b/src/test/resources/alfresco-global.properties index c13dfb9355..4cd14982c1 100644 --- a/src/test/resources/alfresco-global.properties +++ b/src/test/resources/alfresco-global.properties @@ -46,3 +46,6 @@ identity-service.credentials.provider=secret identity-service.client-socket-timeout=1000 identity-service.client-connection-timeout=3000 identity-service.authentication.enable-username-password-authentication=false + +# In the past so it data is read straight away rather than being scheduled for the first time in a few milliseconds +mimetype.config.cronExpression=0 0 0 ? JAN * 1970