From 3afc42226c52a18eacb1b1efbfefd10a7f42b0d3 Mon Sep 17 00:00:00 2001 From: Olivier Levitt Date: Thu, 14 Nov 2024 10:49:49 +0100 Subject: [PATCH 1/2] Refactor catalog refresh to make it more resilient --- .../java/fr/insee/onyxia/api/Application.java | 2 + .../api/dao/universe/CatalogRefresher.java | 49 ++++++------------- .../dao/universe/CatalogRefresherTest.java | 12 +++-- 3 files changed, 23 insertions(+), 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 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..91059684 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() 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 index 43da730b..751b7f10 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 @@ -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); From 9fac9930788269213c8d2e8e731ab5a966a7b625 Mon Sep 17 00:00:00 2001 From: Olivier Levitt Date: Thu, 14 Nov 2024 10:54:04 +0100 Subject: [PATCH 2/2] Fix sonar --- .../fr/insee/onyxia/api/dao/universe/CatalogRefresher.java | 4 ++-- .../insee/onyxia/api/dao/universe/CatalogRefresherTest.java | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) 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 91059684..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 @@ -70,7 +70,7 @@ private void refresh() throws InterruptedException { } @Scheduled(fixedDelayString = "${catalogs.refresh.ms}") - public synchronized void run() throws Exception { + public synchronized void run() { LOGGER.info("Refreshing catalogs"); try { refresh(); @@ -80,7 +80,7 @@ public synchronized void run() throws Exception { } @EventListener(ApplicationReadyEvent.class) - public void initialRefresh() throws Exception { + 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 751b7f10..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,7 +14,6 @@ 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) @@ -27,8 +26,6 @@ class CatalogRefresherTest { @Mock private HelmRepoService helmRepoService; - @Mock private ApplicationArguments applicationArguments; - private CatalogRefresher catalogRefresher; @BeforeEach