Refactor catalog refresh to make it more resilient

InseeFrLab · Nov 14, 2024 · 3afc422 · 3afc422
1 parent 6fe0f88
commit 3afc422
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 40 deletions.
diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java
@@ -14,9 +14,11 @@
 import org.springframework.core.env.MapPropertySource;
 import org.springframework.core.env.MutablePropertySources;
 import org.springframework.scheduling.annotation.EnableAsync;
+import org.springframework.scheduling.annotation.EnableScheduling;
 
 @SpringBootApplication(scanBasePackages = {"io.github.inseefrlab", "fr.insee.onyxia"})
 @EnableAsync(proxyTargetClass = true)
+@EnableScheduling
 public class Application {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(Application.class);

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogRefresher.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogRefresher.java
@@ -3,37 +3,30 @@
 import fr.insee.onyxia.api.configuration.Catalogs;
 import io.github.inseefrlab.helmwrapper.service.HelmRepoService;
 import java.io.IOException;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.concurrent.TimeoutException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
-import org.springframework.boot.ApplicationArguments;
-import org.springframework.boot.ApplicationRunner;
+import org.springframework.boot.context.event.ApplicationReadyEvent;
+import org.springframework.context.event.EventListener;
+import org.springframework.scheduling.annotation.Scheduled;
 import org.springframework.stereotype.Service;
 
 @Service
-public class CatalogRefresher implements ApplicationRunner {
+public class CatalogRefresher {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(CatalogRefresher.class);
 
     private final Catalogs catalogs;
     private final CatalogLoader catalogLoader;
-    private final long refreshTime;
     private final HelmRepoService helmRepoService;
 
     @Autowired
     public CatalogRefresher(
-            Catalogs catalogs,
-            CatalogLoader catalogLoader,
-            HelmRepoService helmRepoService,
-            @Value("${catalogs.refresh.ms}") long refreshTime) {
+            Catalogs catalogs, CatalogLoader catalogLoader, HelmRepoService helmRepoService) {
         this.catalogs = catalogs;
         this.catalogLoader = catalogLoader;
         this.helmRepoService = helmRepoService;
-        this.refreshTime = refreshTime;
     }
 
     private void refreshCatalogs() {
@@ -76,32 +69,18 @@ private void refresh() throws InterruptedException {
         refreshCatalogs();
     }
 
-    @Override
-    public void run(ApplicationArguments args) throws Exception {
-        LOGGER.info("Starting catalog refresher...");
+    @Scheduled(fixedDelayString = "${catalogs.refresh.ms}")
+    public synchronized void run() throws Exception {
+        LOGGER.info("Refreshing catalogs");
         try {
             refresh();
-        } catch (InterruptedException e) {
-            LOGGER.warn("Run method interrupted", e);
-            Thread.currentThread().interrupt();
+        } catch (Exception e) {
+            LOGGER.error("Catalog refreshing failed", e);
         }
+    }
 
-        if (refreshTime > 0L) {
-            Timer timer = new Timer();
-            TimerTask timerTask =
-                    new TimerTask() {
-                        @Override
-                        public void run() {
-                            LOGGER.info("Refreshing catalogs");
-                            try {
-                                refresh();
-                            } catch (InterruptedException e) {
-                                LOGGER.warn("Timer task interrupted", e);
-                                Thread.currentThread().interrupt();
-                            }
-                        }
-                    };
-            timer.scheduleAtFixedRate(timerTask, refreshTime, refreshTime);
-        }
+    @EventListener(ApplicationReadyEvent.class)
+    public void initialRefresh() throws Exception {
+        run();
     }
 }
diff --git a/onyxia-api/src/test/java/fr/insee/onyxia/api/dao/universe/CatalogRefresherTest.java b/onyxia-api/src/test/java/fr/insee/onyxia/api/dao/universe/CatalogRefresherTest.java
@@ -15,8 +15,10 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.boot.ApplicationArguments;
+import org.springframework.test.context.TestPropertySource;
 
 @ExtendWith(MockitoExtension.class)
+@TestPropertySource(properties = "catalogs.refresh.ms=1000")
 class CatalogRefresherTest {
 
     @Mock private Catalogs catalogs;
@@ -31,7 +33,7 @@ class CatalogRefresherTest {
 
     @BeforeEach
     void setUp() {
-        catalogRefresher = new CatalogRefresher(catalogs, catalogLoader, helmRepoService, 1000);
+        catalogRefresher = new CatalogRefresher(catalogs, catalogLoader, helmRepoService);
         Thread.interrupted(); // Clear any existing interrupted status
     }
 
@@ -42,7 +44,7 @@ void testInterruptedExceptionHandling() throws Exception {
                 .when(helmRepoService)
                 .repoUpdate();
 
-        catalogRefresher.run(applicationArguments);
+        catalogRefresher.run();
 
         verify(helmRepoService).repoUpdate();
         verify(catalogLoader, never()).updateCatalog(any(CatalogWrapper.class));
@@ -56,7 +58,7 @@ void testInterruptedExceptionHandling() throws Exception {
     void testTimeoutExceptionHandling() throws Exception {
         doThrow(new TimeoutException("Timeout")).when(helmRepoService).repoUpdate();
 
-        catalogRefresher.run(applicationArguments);
+        catalogRefresher.run();
 
         verify(helmRepoService).repoUpdate();
         verify(catalogLoader, never()).updateCatalog(any(CatalogWrapper.class));
@@ -67,7 +69,7 @@ void testTimeoutExceptionHandling() throws Exception {
     void testIOExceptionHandling() throws Exception {
         doThrow(new IOException("IO error")).when(helmRepoService).repoUpdate();
 
-        catalogRefresher.run(applicationArguments);
+        catalogRefresher.run();
 
         verify(helmRepoService).repoUpdate();
         verify(catalogLoader, never()).updateCatalog(any(CatalogWrapper.class));
@@ -85,7 +87,7 @@ void testSuccessfulRefresh() throws Exception {
         when(catalogs.getCatalogs()).thenReturn(List.of(catalogWrapper));
         when(helmRepoService.addHelmRepo("location", "id", false, null)).thenReturn("Repo added");
 
-        catalogRefresher.run(applicationArguments);
+        catalogRefresher.run();
 
         verify(helmRepoService).repoUpdate();
         verify(helmRepoService, times(1)).addHelmRepo("location", "id", false, null);