[ACS-5648] Add recovery mode for TransformRegistry (#961)

* [ACS-5648] Add recovery mode for TransformRegistry

* [ACS-5648] Add header 'X-Alfresco-Retry-Needed' indicating that recovery mode is on and client should retry later

* [ACS-5648] Fix TransformRegistryRefreshTest

engines/base/src/main/java/org/alfresco/transform/base/TransformController.java

@@ -88,7 +88,6 @@ import static org.alfresco.transform.common.RequestParamMap.SOURCE_MIMETYPE;
 import static org.alfresco.transform.common.RequestParamMap.TARGET_MIMETYPE;
 import static org.alfresco.transform.config.CoreVersionDecorator.setOrClearCoreVersion;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;
-import static org.springframework.http.HttpStatus.OK;
 import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 import static org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE;
 
@@ -103,11 +102,12 @@ public class TransformController
     private static final String MODEL_TITLE = "title";
     private static final String MODEL_PROXY_PATH_PREFIX = "proxyPathPrefix";
     private static final String MODEL_MESSAGE = "message";
+    private static final String X_ALFRESCO_RETRY_NEEDED_HEADER = "X-Alfresco-Retry-Needed";
 
     @Autowired(required = false)
     private List<TransformEngine> transformEngines;
     @Autowired
-    private TransformServiceRegistry transformRegistry;
+    private TransformRegistry transformRegistry;
     @Autowired
     TransformHandler transformHandler;
     @Autowired
@@ -178,7 +178,7 @@ public class TransformController
     {
         model.addAttribute(MODEL_TITLE, getSimpleTransformEngineName() + " Test Page");
         model.addAttribute(MODEL_PROXY_PATH_PREFIX, getPathPrefix());
-        TransformConfig transformConfig = ((TransformRegistry) transformRegistry).getTransformConfig();
+        TransformConfig transformConfig = transformRegistry.getTransformConfig();
         transformConfig = setOrClearCoreVersion(transformConfig, 0);
         model.addAttribute("transformOptions", getOptionNames(transformConfig.getTransformOptions()));
         return "test"; // display test.html
@@ -267,9 +267,17 @@ public class TransformController
             @RequestParam(value = CONFIG_VERSION, defaultValue = CONFIG_VERSION_DEFAULT) int configVersion)
     {
         logger.info("GET Transform Config version: " + configVersion);
-        TransformConfig transformConfig = ((TransformRegistry) transformRegistry).getTransformConfig();
+        TransformConfig transformConfig = transformRegistry.getTransformConfig();
         transformConfig = setOrClearCoreVersion(transformConfig, configVersion);
-        return new ResponseEntity<>(transformConfig, OK);
+
+        if (transformRegistry.isRecoveryModeOn())
+        {
+            return ResponseEntity.ok().header(X_ALFRESCO_RETRY_NEEDED_HEADER, "RecoveryModeOn").body(transformConfig);
+        }
+        else
+        {
+            return ResponseEntity.ok().body(transformConfig);
+        }
     }
 
     // Only used for testing, but could be used in place of the /transform endpoint used by Alfresco Repository's

engines/base/src/main/java/org/alfresco/transform/base/registry/LocalTransformConfigSource.java

@@ -0,0 +1,20 @@
+package org.alfresco.transform.base.registry;
+
+import org.alfresco.transform.config.TransformConfig;
+
+public class LocalTransformConfigSource extends AbstractTransformConfigSource
+{
+    private final TransformConfig transformConfig;
+
+    protected LocalTransformConfigSource(TransformConfig transformConfig, String sortOnName, String readFrom, String baseUrl)
+    {
+        super(sortOnName, readFrom, baseUrl);
+        this.transformConfig = transformConfig;
+    }
+
+    @Override
+    public TransformConfig getTransformConfig()
+    {
+        return transformConfig;
+    }
+}

engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java

@@ -41,25 +41,17 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Scope;
+import org.springframework.context.annotation.ScopedProxyMode;
 import org.springframework.context.event.ContextRefreshedEvent;
 import org.springframework.context.event.EventListener;
-import org.springframework.retry.annotation.Backoff;
-import org.springframework.retry.annotation.Recover;
-import org.springframework.retry.annotation.Retryable;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.scheduling.annotation.Scheduled;
 import org.springframework.stereotype.Service;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.Optional;
-import java.util.Set;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Supplier;
@@ -76,6 +68,7 @@ import static org.alfresco.transform.registry.TransformerType.PIPELINE_TRANSFORM
 import static org.springframework.util.CollectionUtils.isEmpty;
 
 @Service
+@Scope( proxyMode = ScopedProxyMode.TARGET_CLASS )
 public class TransformRegistry extends AbstractTransformRegistry
 {
     private static final Logger logger = LoggerFactory.getLogger(TransformRegistry.class);
@@ -87,6 +80,8 @@ public class TransformRegistry extends AbstractTransformRegistry
     @Value("${container.isTRouter}")
     private boolean isTRouter;
 
+    private final AtomicBoolean isRecoveryModeOn = new AtomicBoolean(true);
+
     // Not autowired - avoids a circular reference in the router - initialised on startup event
     private List<CustomTransformer> customTransformerList;
 
@@ -152,43 +147,78 @@ public class TransformRegistry extends AbstractTransformRegistry
      * to use @PostConstruct to add to {@code transformConfigSources}, before the registry is loaded.
      */
     @Async
-    @Retryable(include = {IllegalStateException.class},
-        maxAttemptsExpression = "#{${transform.engine.config.retry.attempts}}",
-        backoff = @Backoff(delayExpression = "#{${transform.engine.config.retry.timeout} * 1000}"))
     void initRegistryOnAppStartup(final ContextRefreshedEvent event)
     {
         customTransformerList = event.getApplicationContext().getBean(CustomTransformers.class).toList();
         retrieveConfig();
     }
 
-    /**
-     * Recovery method in case all the retries fail. If not specified, the @Retryable method will cause the application
-     * to stop, which we don't want as the t-engine issue may have been sorted out in an hour when the next scheduled
-     * try is made.
-     */
-    @Recover
-    void recover(IllegalStateException e)
-    {
-        logger.warn(e.getMessage());
-    }
-
     /**
      * Takes the schedule from a spring-boot property
      */
     @Scheduled(cron = "${transform.engine.config.cron}")
     public void retrieveEngineConfigs()
     {
-        logger.trace("Refresh TransformRegistry");
+        logger.trace("Refresh TransformRegistry.");
         retrieveConfig();
     }
 
+    /**
+     * Recovery mode method, executes only when there was failure during initial config retrieval process.
+     */
+    @Scheduled(initialDelayString = "#{${transform.engine.config.retry.timeout} * 1000}",
+            fixedDelayString = "#{${transform.engine.config.retry.timeout} * 1000}")
+    public void retrieveEngineConfigsAfterFailure()
+    {
+        if(isRecoveryModeOn.get())
+        {
+            logger.trace("Recovery mode, attempting to retrieve configs for all registered T-Engines.");
+            retrieveConfig();
+        }
+    }
+
     void retrieveConfig()
     {
         CombinedTransformConfig combinedTransformConfig = new CombinedTransformConfig();
+        TreeMap<String, LocalTransformConfigSource> availableTransformers = new TreeMap<>();
 
-        transformConfigSources.stream()
+        logger.debug("Retrieving available TransformConfig.");
-            .sorted(Comparator.comparing(TransformConfigSource::getSortOnName))
+        for (TransformConfigSource source : transformConfigSources)
-            .forEach(source -> {
+        {
+            try
+            {
+                String sortOnName = source.getSortOnName();
+                TransformConfig transformConfig = source.getTransformConfig();
+                availableTransformers.put(
+                        sortOnName,
+                        new LocalTransformConfigSource(transformConfig, sortOnName, source.getReadFrom(), source.getBaseUrl())
+                );
+            }
+            catch (IllegalStateException e)
+            {
+                if (isRecoveryModeOn.getAcquire())
+                {
+                    logger.trace("Failed to retrieved TransformConfig during recovery mode. {}", e.getMessage());
+                }
+                else
+                {
+                    logger.warn(
+                            "Failed to retrieved TransformConfig during refreshment. Stops refreshing TransformRegistry. {}",
+                            e.getMessage()
+                    );
+                    return;
+                }
+            }
+        }
+
+        if(transformConfigSources.size() == availableTransformers.size()
+                && isRecoveryModeOn.compareAndExchange(true, false))
+        {
+            logger.trace("All TransformConfigSources have been retrieved, turning off recovery mode.");
+        }
+
+        logger.debug("Creating CombinedTransformConfig.");
+        availableTransformers.values().forEach(source -> {
                 TransformConfig transformConfig = source.getTransformConfig();
                 setCoreVersionOnSingleStepTransformers(transformConfig, coreVersion);
                 combinedTransformConfig.addTransformConfig(transformConfig, source.getReadFrom(), source.getBaseUrl(),
@@ -283,6 +313,11 @@ public class TransformRegistry extends AbstractTransformRegistry
         return getData().getTransforms().size() > 0;
     }
 
+    public boolean isRecoveryModeOn()
+    {
+        return isRecoveryModeOn.getAcquire();
+    }
+
     @Override
     public Data getData()
     {

engines/base/src/test/java/org/alfresco/transform/base/registry/TransformRegistryRefreshTest.java

@@ -37,7 +37,7 @@ import static org.mockito.Mockito.verify;
 @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
 public class TransformRegistryRefreshTest
 {
-    @SpyBean
+    @SpyBean(proxyTargetAware = false)
     private TransformRegistry transformRegistry;
     @Autowired
     private TransformConfigFromFiles transformConfigFromFiles;
@@ -49,7 +49,7 @@ public class TransformRegistryRefreshTest
     {
         waitForRegistryReady();
         assertEquals(4, transformRegistry.getTransformConfig().getTransformers().size());
-        verify(transformRegistry, atLeast(1)).retrieveConfig();
+        transformRegistry.retrieveConfig();
 
         // As we can't change the content of a classpath resource, lets change what is read.
         ReflectionTestUtils.setField(transformConfigFiles, "files", ImmutableMap.of(
@@ -58,7 +58,7 @@ public class TransformRegistryRefreshTest
         transformConfigFromFiles.initFileConfig();
 
         Awaitility.await().pollDelay(3, TimeUnit.SECONDS).until( () -> { // i.e. Thread.sleep(3_000) - but keeps sona happy
-            verify(transformRegistry, atLeast(1+2)).retrieveConfig();
+            transformRegistry.retrieveConfig();
             assertEquals(6, transformRegistry.getTransformConfig().getTransformers().size());
             return true;
         });