From 28635d3c85c7914617466061cbc69bfce2457af9 Mon Sep 17 00:00:00 2001 From: Olivier Levitt Date: Thu, 14 Nov 2024 12:04:11 +0100 Subject: [PATCH] Refactor catalog refresh to make it more resilient (#516) --- .../java/fr/insee/onyxia/api/Application.java | 2 + .../api/dao/universe/CatalogRefresher.java | 49 ++++++------------- .../dao/universe/CatalogRefresherTest.java | 15 +++--- 3 files changed, 23 insertions(+), 43 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 index 6f8db1d3..fa74039c 100644 --- 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 index 284cee56..689cd3c4 100644 --- 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() { + 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() { + 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 index 43da730b..39ee1695 100644 --- 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 @@ -14,9 +14,10 @@ import org.junit.jupiter.api.extension.ExtendWith; 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; @@ -25,13 +26,11 @@ class CatalogRefresherTest { @Mock private HelmRepoService helmRepoService; - @Mock private ApplicationArguments applicationArguments; - private CatalogRefresher catalogRefresher; @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 +41,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 +55,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 +66,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 +84,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);