Miscellaneous refactoring of code (#326)

InseeFrLab · Dec 18, 2023 · b1d1e39 · b1d1e39
1 parent b1078ee
commit b1d1e39
Show file tree

Hide file tree

Showing 23 changed files with 86 additions and 103 deletions.
diff --git a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmInstallService.java b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmInstallService.java
@@ -25,7 +25,7 @@
 /** HelmInstall */
 public class HelmInstallService {
 
-    private final Logger logger = LoggerFactory.getLogger(HelmInstallService.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(HelmInstallService.class);
 
     private final Pattern helmNamePattern =
             Pattern.compile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$");
@@ -154,12 +154,8 @@ private String getReleaseInfo(
             } else {
                 return Command.execute(configuration, command.toString()).getOutput().getString();
             }
-        } catch (IOException e) {
-            e.printStackTrace();
-        } catch (InterruptedException e) {
-            e.printStackTrace();
-        } catch (TimeoutException e) {
-            e.printStackTrace();
+        } catch (IOException | InterruptedException | TimeoutException e) {
+            LOGGER.warn("Exception occurred", e);
         }
         return "";
     }

diff --git a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/utils/Command.java b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/utils/Command.java
@@ -20,7 +20,7 @@ public class Command {
     private static final Pattern safeToConcatenate =
             Pattern.compile(
                     "^[ ]*[a-z0-9]([-a-z0-9 ]*[a-z0-9 ])?(\\.[a-z0-9 ]([-a-z0-9 ]*[a-z0-9 ])?)*[ ]*$");
-    private static Logger LOGGER = LoggerFactory.getLogger(Command.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(Command.class);
 
     private static ProcessExecutor getProcessExecutor() {
         ProcessExecutor processExecutor = new ProcessExecutor();

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogFilter.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogFilter.java
@@ -12,7 +12,7 @@ public class CatalogFilter {
 
     @Autowired private RegionsConfiguration regionsConfiguration;
 
-    private Logger logger = LoggerFactory.getLogger(CatalogFilter.class);
+    private Logger LOGGER = LoggerFactory.getLogger(CatalogFilter.class);
 
     public List<CatalogWrapper> filterCatalogs(List<CatalogWrapper> catalogs) {
         return catalogs;

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogsLoader.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogsLoader.java
@@ -22,15 +22,15 @@ public class CatalogsLoader {
 
     @Autowired private CatalogsConfiguration catalogsConfiguration;
 
-    private static Logger logger = LoggerFactory.getLogger(CatalogsLoader.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(CatalogsLoader.class);
 
     @Bean
     @Scope(ConfigurableBeanFactory.SCOPE_SINGLETON)
     public Catalogs catalogs() {
         Catalogs catalogs = new Catalogs();
         catalogs.setCatalogs(catalogsConfiguration.getResolvedCatalogs());
         catalogs.setCatalogs(catalogFilter.filterCatalogs(catalogs.getCatalogs()));
-        logger.info("Serving " + catalogs.getCatalogs().size() + " catalogs");
+        LOGGER.info("Serving {} catalogs", catalogs.getCatalogs().size());
         return catalogs;
     }
 }
diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/checks/CompatibilityChecks.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/checks/CompatibilityChecks.java
@@ -15,7 +15,7 @@
 @Configuration
 public class CompatibilityChecks {
 
-    private static Logger LOGGER = LoggerFactory.getLogger(RegionsConfiguration.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(CompatibilityChecks.class);
     @Autowired RegionsConfiguration regionsConfiguration;
     @Autowired KubernetesClientProvider kubernetesClientProvider;
 
@@ -24,7 +24,7 @@ public class CompatibilityChecks {
     @EventListener(ContextRefreshedEvent.class)
     public void checkHelm() {
         try {
-            LOGGER.info("Using helm " + helmVersionService.getVersion());
+            LOGGER.info("Using helm {}", helmVersionService.getVersion());
         } catch (Exception e) {
             LOGGER.error("Could not determine helm version", e);
             System.exit(0);
@@ -44,18 +44,15 @@ public void checkKubernetesVersion() {
                                         kubernetesClientProvider.getRootClient(region);
                                 try {
                                     LOGGER.info(
-                                            "Region "
-                                                    + region.getName()
-                                                    + " kubernetes v"
-                                                    + client.getKubernetesVersion().getMajor()
-                                                    + "."
-                                                    + client.getKubernetesVersion().getMinor());
+                                            "Region {} kubernetes v{}.{}",
+                                            region.getName(),
+                                            client.getKubernetesVersion().getMajor(),
+                                            client.getKubernetesVersion().getMinor());
                                 } catch (Exception e) {
                                     LOGGER.error(
-                                            "Could not contact Kubernetes APIServer for region "
-                                                    + region.getName()
-                                                    + " at "
-                                                    + client.getMasterUrl(),
+                                            "Could not contact Kubernetes APIServer for region {} at {}",
+                                            region.getName(),
+                                            client.getMasterUrl(),
                                             e);
                                 }
                             }

diff --git a/.../src/main/java/fr/insee/onyxia/api/configuration/kubernetes/KubernetesClientProvider.java b/.../src/main/java/fr/insee/onyxia/api/configuration/kubernetes/KubernetesClientProvider.java
@@ -16,7 +16,7 @@
 @Configuration
 public class KubernetesClientProvider {
 
-    private final Logger logger = LoggerFactory.getLogger(KubernetesClientProvider.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(KubernetesClientProvider.class);
 
     @Autowired private SecurityConfig securityConfig;
 

diff --git a/...-api/src/main/java/fr/insee/onyxia/api/configuration/properties/RegionsConfiguration.java b/...-api/src/main/java/fr/insee/onyxia/api/configuration/properties/RegionsConfiguration.java
@@ -29,7 +29,7 @@
 @ConfigurationProperties
 public class RegionsConfiguration {
 
-    private static Logger LOGGER = LoggerFactory.getLogger(RegionsConfiguration.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(RegionsConfiguration.class);
     private String regions;
     private List<Region> resolvedRegions;
     @Autowired private ObjectMapper mapper;

diff --git a/...api/src/main/java/fr/insee/onyxia/api/controller/api/cloudshell/CloudShellController.java b/...api/src/main/java/fr/insee/onyxia/api/controller/api/cloudshell/CloudShellController.java
@@ -52,9 +52,11 @@ public CloudShellStatus getCloudShellStatus(
         } catch (Exception e) {
             status.setStatus(CloudShellStatus.STATUS_DOWN);
             status.setPackageToDeploy(
-                    catalogService.getPackage(
-                            cloudshellConfiguration.getCatalogId(),
-                            cloudshellConfiguration.getPackageName()));
+                    catalogService
+                            .getPackage(
+                                    cloudshellConfiguration.getCatalogId(),
+                                    cloudshellConfiguration.getPackageName())
+                            .get());
             status.setCatalogId(cloudshellConfiguration.getCatalogId());
             status.setUrl(null);
         }

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java
@@ -1,7 +1,7 @@
 package fr.insee.onyxia.api.controller.api.mylab;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
 import fr.insee.onyxia.api.configuration.CatalogWrapper;
+import fr.insee.onyxia.api.configuration.NotFoundException;
 import fr.insee.onyxia.api.services.AppsService;
 import fr.insee.onyxia.api.services.CatalogService;
 import fr.insee.onyxia.api.services.UserProvider;
@@ -19,7 +19,6 @@
 import io.swagger.v3.oas.annotations.media.Schema;
 import io.swagger.v3.oas.annotations.security.SecurityRequirement;
 import io.swagger.v3.oas.annotations.tags.Tag;
-import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.CompletableFuture;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -223,18 +222,22 @@ public Object publishService(
             @Parameter(hidden = true) Region region,
             @Parameter(hidden = true) Project project,
             @RequestBody CreateServiceDTO requestDTO)
-            throws JsonProcessingException, IOException, Exception {
+            throws Exception {
         return publishApps(region, project, requestDTO);
     }
 
     private Collection<Object> publishApps(
             Region region, Project project, CreateServiceDTO requestDTO) throws Exception {
         String catalogId = "internal";
-        if (requestDTO.getCatalogId() != null && requestDTO.getCatalogId().length() > 0) {
+        if (requestDTO.getCatalogId() != null && !requestDTO.getCatalogId().isEmpty()) {
             catalogId = requestDTO.getCatalogId();
         }
         CatalogWrapper catalog = catalogService.getCatalogById(catalogId);
-        Pkg pkg = catalog.getCatalog().getPackageByName(requestDTO.getPackageName());
+        Pkg pkg =
+                catalog.getCatalog()
+                        .getPackageByName(requestDTO.getPackageName())
+                        .orElseThrow(NotFoundException::new);
+
         boolean skipTlsVerify = catalog.getSkipTlsVerify();
         String caFile = catalog.getCaFile();
         User user = userProvider.getUser(region);

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/QuotaController.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/QuotaController.java
@@ -33,7 +33,7 @@
 @SecurityRequirement(name = "auth")
 public class QuotaController {
 
-    private final Logger logger = LoggerFactory.getLogger(QuotaController.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(QuotaController.class);
 
     @Autowired private KubernetesService kubernetesService;
 
@@ -143,23 +143,23 @@ public void resetQuota(
         if (owner.getType() == Owner.OwnerType.USER) {
             checkUserQuotaIsEnabled(region);
             if (region.getServices().getQuotas().isEnabled()) {
-                logger.warn(
+                LOGGER.warn(
                         "resetting to old enabled style quota, this parameter will be deprecated");
                 kubernetesService.applyQuota(
                         region,
                         project,
                         userProvider.getUser(region),
                         region.getServices().getQuotas().getDefaultQuota());
             } else {
-                logger.info("resetting to user enabled style quota");
+                LOGGER.info("resetting to user enabled style quota");
                 kubernetesService.applyQuota(
                         region,
                         project,
                         userProvider.getUser(region),
                         region.getServices().getQuotas().getUserQuota());
             }
         } else if (owner.getType() == Owner.OwnerType.GROUP) {
-            logger.info("resetting to group enabled style quota");
+            LOGGER.info("resetting to group enabled style quota");
             checkGroupQuotaIsEnabled(region);
             kubernetesService.applyQuota(
                     region,

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/pub/CatalogController.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/pub/CatalogController.java
@@ -84,11 +84,11 @@ public CatalogWrapper getCatalogById(@PathVariable String catalogId) {
             })
     @GetMapping("{catalogId}/{packageName}")
     public Pkg getPackage(@PathVariable String catalogId, @PathVariable String packageName) {
-        Pkg pkg = catalogService.getPackage(catalogId, packageName);
+        Pkg pkg =
+                catalogService
+                        .getPackage(catalogId, packageName)
+                        .orElseThrow(NotFoundException::new);
         addCustomOnyxiaProperties(pkg);
-        if (pkg == null) {
-            throw new NotFoundException();
-        }
         return pkg;
     }
 

diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogLoader.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogLoader.java
@@ -1,5 +1,7 @@
 package fr.insee.onyxia.api.dao.universe;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -12,6 +14,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.util.List;
 import java.util.stream.Collectors;
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
@@ -27,7 +30,7 @@
 @Service
 public class CatalogLoader {
 
-    private final Logger logger = LoggerFactory.getLogger(CatalogLoader.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(CatalogLoader.class);
 
     @Autowired private ResourceLoader resourceLoader;
 
@@ -36,11 +39,11 @@ public class CatalogLoader {
     private ObjectMapper mapperHelm;
 
     public void updateCatalog(CatalogWrapper cw) {
-        logger.info("updating catalog with id :" + cw.getId() + " and type " + cw.getType());
-        switch (cw.getType()) {
-            case Repository.TYPE_HELM:
-                updateHelmRepository(cw);
-                break;
+        LOGGER.info("updating catalog with id :{} and type {}", cw.getId(), cw.getType());
+        if (cw.getType().equals(Repository.TYPE_HELM)) {
+            updateHelmRepository(cw);
+        } else {
+            LOGGER.warn("Unsupported catalog type: id: {}, type: {}", cw.getId(), cw.getType());
         }
     }
 
@@ -52,7 +55,7 @@ private void updateHelmRepository(CatalogWrapper cw) {
                             resourceLoader
                                     .getResource(cw.getLocation() + "/index.yaml")
                                     .getInputStream(),
-                            "UTF-8");
+                            UTF_8);
             Repository repository = mapperHelm.readValue(reader, Repository.class);
             repository
                     .getEntries()
@@ -74,19 +77,19 @@ private void updateHelmRepository(CatalogWrapper cw) {
                                                         refreshPackage(cw, pkg);
                                                     } catch (CatalogLoaderException
                                                             | IOException e) {
-                                                        e.printStackTrace();
+                                                        LOGGER.info("Exception occurred", e);
                                                     }
                                                 });
                             });
             repository.setPackages(
                     repository.getEntries().values().stream()
-                            .map(charts -> charts.get(0))
+                            .map(List::getFirst)
                             .filter(chart -> "application".equalsIgnoreCase(chart.getType()))
                             .collect(Collectors.toList()));
             cw.setCatalog(repository);
             cw.setLastUpdateTime(System.currentTimeMillis());
         } catch (Exception e) {
-            e.printStackTrace();
+            LOGGER.info("Exception occurred", e);
         }
     }
 
@@ -116,7 +119,7 @@ private void extractDataFromTgz(InputStream in, Chart chart) throws IOException
         try (TarArchiveInputStream tarIn = new TarArchiveInputStream(gzipIn)) {
             TarArchiveEntry entry;
 
-            while ((entry = tarIn.getNextTarEntry()) != null) {
+            while ((entry = tarIn.getNextEntry()) != null) {
                 if (entry.getName().endsWith(chart.getName() + "/values.schema.json")
                         && !entry.getName()
                                 .endsWith("charts/" + chart.getName() + "/values.schema.json")) {
@@ -127,12 +130,6 @@ private void extractDataFromTgz(InputStream in, Chart chart) throws IOException
                     Config config = mapper.readValue(tarIn, Config.class);
                     chart.setConfig(config);
                 }
-
-                if (entry.isDirectory()) {
-
-                } else {
-
-                }
             }
         }
     }

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
@@ -17,7 +17,7 @@
 @Service
 public class CatalogRefresher implements ApplicationRunner {
 
-    private final Logger logger = LoggerFactory.getLogger(CatalogRefresher.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(CatalogRefresher.class);
 
     @Autowired private Catalogs catalogs;
 
@@ -33,26 +33,22 @@ private void refresh() {
                 .forEach(
                         c -> {
                             try {
-                                logger.info(
+                                LOGGER.info(
                                         helmRepoService.addHelmRepo(
                                                 c.getLocation(),
                                                 c.getId(),
                                                 c.getSkipTlsVerify(),
                                                 c.getCaFile()));
                                 catalogLoader.updateCatalog(c);
                             } catch (Exception e) {
-                                e.printStackTrace();
+                                LOGGER.warn("Exception occurred", e);
                             }
                         });
 
         try {
             helmRepoService.repoUpdate();
-        } catch (InterruptedException e) {
-            e.printStackTrace();
-        } catch (TimeoutException e) {
-            e.printStackTrace();
-        } catch (IOException e) {
-            e.printStackTrace();
+        } catch (InterruptedException | TimeoutException | IOException e) {
+            LOGGER.warn("Exception occurred", e);
         }
     }
 
@@ -65,7 +61,7 @@ public void run(ApplicationArguments args) throws Exception {
                     new TimerTask() {
                         @Override
                         public void run() {
-                            logger.info("refreshing catalogs..");
+                            LOGGER.info("Refreshing catalogs");
                             refresh();
                         }
                     };