From d809a440253b0aef33705149269437902c9aaa87 Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 11 Nov 2024 19:45:20 +0100 Subject: [PATCH 01/11] chore(sonar): Clean basic sonar lint issues for all affected Graph Repo Client classes --- .../heigit/ors/api/services/GraphService.java | 17 ++++-- .../listeners/ORSInitContextListener.java | 8 +-- .../heigit/ors/apitests/ORSStartupTest.java | 2 +- .../extensions/manage/GraphInfo.java | 33 ++-------- .../GraphManagementRuntimeProperties.java | 41 ++++++++----- .../extensions/manage/ORSGraphManager.java | 16 ++--- .../manage/local/ORSGraphFileManager.java | 60 ++++++++++++------- .../manage/remote/FileSystemRepoManager.java | 30 +++++----- .../manage/remote/HttpRepoManager.java | 50 +++++++++------- .../remote/NamedGraphsRepoStrategy.java | 6 +- .../manage/remote/NullRepoManager.java | 1 + .../GraphManagementRuntimePropertiesTest.java | 2 +- .../manage/ORSGraphManagerTest.java | 4 +- .../manage/RepoManagerTestHelper.java | 18 +++--- .../manage/local/ORSGraphFileManagerTest.java | 16 ++--- .../remote/FileSystemRepoManagerTest.java | 16 +++-- .../manage/remote/HttpRepoManagerTest.java | 16 ++--- .../remote/TestAbstractRepoManagerTest.java | 26 ++++---- 18 files changed, 182 insertions(+), 180 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java b/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java index a459fbf592..bd3f7d0f69 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java @@ -29,10 +29,15 @@ public class GraphService { private static final Logger LOGGER = Logger.getLogger(GraphService.class.getName()); - private List graphManagers = new ArrayList<>(); + private final List graphManagers = new ArrayList<>(); - private AtomicBoolean graphActivationAttemptWasBlocked = new AtomicBoolean(false); - private AtomicBoolean isActivatingGraphs = new AtomicBoolean(true); + private final AtomicBoolean graphActivationAttemptWasBlocked = new AtomicBoolean(false); + private final AtomicBoolean isActivatingGraphs = new AtomicBoolean(true); + + @Autowired + public GraphService(EngineProperties engineProperties) { + this.engineProperties = engineProperties; + } public void addGraphManagerInstance(ORSGraphManager orsGraphManager) { if (orsGraphManager.useGraphRepository()) { @@ -48,7 +53,7 @@ public void setIsActivatingGraphs(boolean value) { @Scheduled(cron = "${ors.engine.graph_management.download_schedule:0 0 0 31 2 *}")//Default is "never" public void checkForUpdatesInRepo() { - if (!enabled) { + if (Boolean.FALSE.equals(enabled)) { LOGGER.debug("Graph management is disabled, skipping scheduled repository check..."); return; } @@ -87,7 +92,7 @@ public void checkForDownloadedGraphsToActivate() { } public void checkForDownloadedGraphsToActivate(String trigger) { - if (!enabled) { + if (Boolean.FALSE.equals(enabled)) { LOGGER.debug("Graph management is disabled, skipping %s activation check...".formatted(trigger.toLowerCase())); return; } @@ -147,7 +152,7 @@ public void checkForDownloadedGraphsToActivate(String trigger) { @Async @Scheduled(fixedDelay = 1, timeUnit = TimeUnit.MINUTES) public void repeatedGraphActivationAttempt() { - if (!enabled) { + if (Boolean.FALSE.equals(enabled)) { LOGGER.debug("Graph management is disabled, skipping repeated attempt to activate graphs..."); return; } diff --git a/ors-api/src/main/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListener.java b/ors-api/src/main/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListener.java index 58395e4b61..73b2f851b1 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListener.java +++ b/ors-api/src/main/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListener.java @@ -22,6 +22,7 @@ import jakarta.servlet.ServletContextEvent; import jakarta.servlet.ServletContextListener; +import lombok.AllArgsConstructor; import org.apache.juli.logging.LogFactory; import org.apache.log4j.Logger; import org.heigit.ors.api.services.GraphService; @@ -40,17 +41,12 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +@AllArgsConstructor public class ORSInitContextListener implements ServletContextListener { private static final Logger LOGGER = Logger.getLogger(ORSInitContextListener.class); private final EngineProperties engineProperties; private final GraphService graphService; - public ORSInitContextListener(EngineProperties engineProperties, GraphService graphService) { - // Initialize properties object loaded by spring - this.engineProperties = engineProperties; - this.graphService = graphService; - } - @Override public void contextInitialized(ServletContextEvent contextEvent) { String outputTarget = configurationOutputTarget(engineProperties); diff --git a/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java b/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java index 4b4e208739..0f20abc74e 100644 --- a/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java +++ b/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java @@ -13,7 +13,7 @@ import static org.junit.jupiter.api.Assertions.*; -public class ORSStartupTest extends ServiceTest { +class ORSStartupTest extends ServiceTest { SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); @Test diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java index d2da45ac00..09d45ea399 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java @@ -1,37 +1,26 @@ package org.heigit.ors.routing.graphhopper.extensions.manage; import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.experimental.Accessors; import java.io.File; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; -import java.util.Objects; +@Accessors(chain = true) @Getter +@Setter +@NoArgsConstructor public class GraphInfo { - public GraphInfo() { - } - private URI remoteUri = null; private File localDirectory = null; private PersistedGraphInfo persistedGraphInfo; - public GraphInfo withRemoteUri(URI remoteUri) { - this.remoteUri = remoteUri; - return this; - } - - public boolean exists() { - return !Objects.isNull(persistedGraphInfo); - } - - public boolean isRemote() { - return remoteUri != null; - } - public GraphInfo withRemoteUrl(URL url) { try { this.remoteUri = url.toURI(); @@ -40,14 +29,4 @@ public GraphInfo withRemoteUrl(URL url) { } return this; } - - public GraphInfo withLocalDirectory(File directory) { - this.localDirectory = directory; - return this; - } - - public GraphInfo withPersistedInfo(PersistedGraphInfo persistedGraphInfo) { - this.persistedGraphInfo = persistedGraphInfo; - return this; - } } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java index 61c306d251..8622b742d9 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java @@ -12,6 +12,7 @@ import java.net.URL; import java.nio.file.Path; import java.util.Arrays; +import java.util.List; import java.util.Optional; import static java.util.Optional.ofNullable; @@ -64,30 +65,41 @@ public static Builder empty() { public static Builder from(EngineProperties engineProperties, ProfileProperties profileProperties, String graphVersion) { Builder builder = new Builder(); builder.enabled = ofNullable(engineProperties).map(EngineProperties::getGraphManagement).map(GraphManagementProperties::getEnabled).orElse(false); - builder.repoBaseUri = getFirstPresentString( + builder.repoBaseUri = getFirstPresentString(List.of( ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri)); - builder.repoName = getFirstPresentString( + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri)) + ); + builder.repoName = getFirstPresentString(List.of( ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName)); - builder.repoCoverage = getFirstPresentString( + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName)) + ); + builder.repoCoverage = getFirstPresentString(List.of( ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent)); - builder.repoProfileGroup = getFirstPresentString( + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent)) + ); + builder.repoProfileGroup = getFirstPresentString(List.of( ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup)); - builder.maxNumberOfGraphBackups = engineProperties.getGraphManagement().getMaxBackups(); + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup)) + ); + + builder.maxNumberOfGraphBackups = ofNullable(engineProperties).map(EngineProperties::getGraphManagement).map(GraphManagementProperties::getMaxBackups).orElse(0); builder.graphVersion = graphVersion; builder.localProfileName = ofNullable(profileProperties).map(ProfileProperties::getProfileName).map(String::valueOf).orElse(null); - builder.localGraphsRootAbsPath = getFirstPresentString( + builder.localGraphsRootAbsPath = getFirstPresentString(List.of( ofNullable(profileProperties).map(ProfileProperties::getGraphPath).map(Path::toString), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getGraphPath).map(Path::toString)); + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getGraphPath).map(Path::toString)) + ); + builder.encoderName = ofNullable(profileProperties).map(ProfileProperties::getEncoderName).map(String::valueOf).orElse(null); return builder; } - private static String getFirstPresentString(Optional... objects) { - return Arrays.stream(objects).filter(Optional::isPresent).findFirst().map(Optional::get).map(String::valueOf).orElse(null); + private static String getFirstPresentString(List> objects) { + return objects.stream() + .flatMap(Optional::stream) + .map(String::valueOf) + .findFirst() + .orElse(null); } public Builder withEnabled(boolean enabled) { @@ -191,8 +203,7 @@ private boolean isSupportedUrlScheme(URI uri) { } private boolean isSupportedFileScheme(URI uri) { - if (uri == null) return false; - return Arrays.asList("file").contains(uri.getScheme()); + return uri != null && "file".equals(uri.getScheme()); } private URI toUri(String string) { diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java index a0844f7069..6eaa722ae8 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java @@ -1,5 +1,7 @@ package org.heigit.ors.routing.graphhopper.extensions.manage; +import lombok.AllArgsConstructor; +import lombok.NoArgsConstructor; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.heigit.ors.config.EngineProperties; @@ -14,6 +16,8 @@ import static java.util.Optional.ofNullable; +@NoArgsConstructor +@AllArgsConstructor public class ORSGraphManager { private static final Logger LOGGER = Logger.getLogger(ORSGraphManager.class.getName()); @@ -24,19 +28,9 @@ public class ORSGraphManager { private ORSGraphFileManager orsGraphFileManager; private ORSGraphRepoManager orsGraphRepoManager; - public ORSGraphManager() { - } - - public ORSGraphManager(GraphManagementRuntimeProperties managementRuntimeProperties, ORSGraphFileManager orsGraphFileManager, ORSGraphRepoManager orsGraphRepoManager) { - this.managementRuntimeProperties = managementRuntimeProperties; - this.orsGraphFileManager = orsGraphFileManager; - this.orsGraphRepoManager = orsGraphRepoManager; - } - public static ORSGraphManager initializeGraphManagement(String graphVersion, EngineProperties engineProperties, ProfileProperties profileProperties) { GraphManagementRuntimeProperties managementProps = GraphManagementRuntimeProperties.Builder.from(engineProperties, profileProperties, graphVersion).build(); - ORSGraphManager orsGraphManager = initializeGraphManagement(managementProps); - return orsGraphManager; + return initializeGraphManagement(managementProps); } public static ORSGraphManager initializeGraphManagement(GraphManagementRuntimeProperties managementProps) { diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java index c85063ece6..f9b512524b 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java @@ -7,6 +7,7 @@ import com.graphhopper.GraphHopper; import com.graphhopper.util.Helper; import com.graphhopper.util.Unzipper; +import lombok.NoArgsConstructor; import org.apache.commons.io.FileUtils; import org.apache.commons.io.filefilter.RegexFileFilter; import org.apache.commons.lang3.StringUtils; @@ -20,6 +21,7 @@ import java.io.File; import java.io.FilenameFilter; import java.io.IOException; +import java.nio.file.Files; import java.text.DateFormat; import java.text.ParseException; import java.time.LocalDateTime; @@ -29,6 +31,7 @@ import static com.fasterxml.jackson.core.JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN; import static com.fasterxml.jackson.dataformat.yaml.YAMLGenerator.Feature.*; +@NoArgsConstructor public class ORSGraphFileManager implements ORSGraphFolderStrategy { private static final Logger LOGGER = Logger.getLogger(ORSGraphFileManager.class.getName()); @@ -36,9 +39,6 @@ public class ORSGraphFileManager implements ORSGraphFolderStrategy { GraphManagementRuntimeProperties graphManagementRuntimeProperties; private ORSGraphFolderStrategy orsGraphFolderStrategy; - public ORSGraphFileManager() { - } - public ORSGraphFileManager(GraphManagementRuntimeProperties graphManagementRuntimeProperties, ORSGraphFolderStrategy orsGraphFolderStrategy) { this.graphManagementRuntimeProperties = graphManagementRuntimeProperties; this.orsGraphFolderStrategy = orsGraphFolderStrategy; @@ -92,24 +92,30 @@ public boolean isBusy() { asIncompleteFile(getDownloadedExtractedGraphDirectory()).exists(); } + private void deleteFileWithLogging(File file, String successMessage, String errorMessage) { + try { + Files.deleteIfExists(file.toPath()); + LOGGER.debug(successMessage.formatted(getProfileDescriptiveName(), file.getAbsolutePath())); + } catch (IOException e) { + LOGGER.error(errorMessage.formatted(e.getMessage())); + } + } + public void cleanupIncompleteFiles() { File incompleteDownloadFile = asIncompleteFile(getDownloadedCompressedGraphFile()); - if (incompleteDownloadFile.exists()) { - LOGGER.debug("[%s] Deleted incomplete graph download file from previous application run: %s".formatted(getProfileDescriptiveName(), incompleteDownloadFile.getAbsolutePath())); - incompleteDownloadFile.delete(); - } + deleteFileWithLogging(incompleteDownloadFile, + "[%s] Deleted incomplete graph download file from previous application run: %s", + "Error deleting incomplete download file: %s"); File graphInfoDownloadFile = getDownloadedGraphInfoFile(); - if (graphInfoDownloadFile.exists()) { - LOGGER.debug("[%s] Deleted graph-info download file from previous application run: %s".formatted(getProfileDescriptiveName(), graphInfoDownloadFile.getAbsolutePath())); - graphInfoDownloadFile.delete(); - } + deleteFileWithLogging(graphInfoDownloadFile, + "[%s] Deleted graph-info download file from previous application run: %s", + "Error deleting graph-info download file: %s"); File incompleteGraphInfoDownloadFile = asIncompleteFile(getDownloadedGraphInfoFile()); - if (incompleteGraphInfoDownloadFile.exists()) { - LOGGER.debug("[%s] Deleted incomplete graph download file from previous application run: %s".formatted(getProfileDescriptiveName(), incompleteGraphInfoDownloadFile.getAbsolutePath())); - incompleteGraphInfoDownloadFile.delete(); - } + deleteFileWithLogging(incompleteGraphInfoDownloadFile, + "[%s] Deleted incomplete graph download file from previous application run: %s", + "Error deleting incomplete graph download file: %s"); File incompleteExtractionFolder = asIncompleteDirectory(getDownloadedExtractedGraphDirectory()); if (incompleteExtractionFolder.exists() && incompleteExtractionFolder.isDirectory()) { @@ -184,7 +190,7 @@ public GraphInfo getActiveGraphInfo() { if (!hasActiveGraph()) { LOGGER.trace("[%s] No active graph directory found.".formatted(getProfileDescriptiveName())); - return new GraphInfo().withLocalDirectory(activeGraphDirectory); + return new GraphInfo().setLocalDirectory(activeGraphDirectory); } return getGraphInfo(getActiveGraphInfoFile()); @@ -196,7 +202,7 @@ public GraphInfo getDownloadedExtractedGraphInfo() { if (!hasDownloadedExtractedGraph()) { LOGGER.trace("[%s] No downloaded graph directory found.".formatted(getProfileDescriptiveName())); - return new GraphInfo().withLocalDirectory(downloadedExtractedGraphDirectory); + return new GraphInfo().setLocalDirectory(downloadedExtractedGraphDirectory); } return getGraphInfo(getDownloadedExtractedGraphInfoFile()); @@ -206,12 +212,12 @@ private GraphInfo getGraphInfo(File graphInfoFile) { File graphDirectory = graphInfoFile.getParentFile(); if (!graphInfoFile.exists() || !graphInfoFile.isFile()) { LOGGER.trace("[%s] No graph info file %s found in %s".formatted(getProfileDescriptiveName(), graphInfoFile.getName(), graphInfoFile.getParentFile().getName())); - return new GraphInfo().withLocalDirectory(graphDirectory); + return new GraphInfo().setLocalDirectory(graphDirectory); } PersistedGraphInfo graphInfo = readOrsGraphInfo(graphInfoFile); LOGGER.trace("[%s] Found local graph info with importDate=%s".formatted(getProfileDescriptiveName(), graphInfo.getGraphBuildDate())); - return new GraphInfo().withLocalDirectory(graphDirectory).withPersistedInfo(graphInfo); + return new GraphInfo().setLocalDirectory(graphDirectory).setPersistedGraphInfo(graphInfo); } static ObjectMapper getYamlMapper() { @@ -260,7 +266,12 @@ public PersistedGraphInfo getDownloadedGraphInfo() { public void activateExtractedDownloadedGraph() { if (hasDownloadedExtractedGraph()) { LOGGER.info("[%s] Activating extracted downloaded graph.".formatted(getProfileDescriptiveName())); - getDownloadedExtractedGraphDirectory().renameTo(getActiveGraphDirectory()); + File downloadedExtractedGraphDirectory = getDownloadedExtractedGraphDirectory(); + if (downloadedExtractedGraphDirectory.renameTo(getActiveGraphDirectory())) { + LOGGER.debug("[%s] Successfully activated extracted downloaded graph.".formatted(getProfileDescriptiveName())); + } else { + LOGGER.error("[%s] Failed to activate extracted downloaded graph.".formatted(getProfileDescriptiveName())); + } } } @@ -292,7 +303,9 @@ public void extractDownloadedGraph() { getProfileDescriptiveName(), end - start, graphDownloadFileAbsPath)); - graphDownloadFile.delete(); + deleteFileWithLogging(graphDownloadFile, + "[%s] Deleted downloaded graph file %s".formatted(getProfileDescriptiveName(), graphDownloadFileAbsPath), + "Error deleting downloaded graph file: %s"); LOGGER.debug("[%s] Renaming extraction directory to %s".formatted( getProfileDescriptiveName(), @@ -312,7 +325,7 @@ public void extractDownloadedGraph() { targetDirectoryAbsPath)); throw new RuntimeException("Caught ", ioException); } - LOGGER.info("[%s] Downloaded graph was extracted and will be activated at next graph activation check or application start.".formatted(getProfileDescriptiveName(), extractionDirectoryAbsPath)); + LOGGER.info("[%s] Downloaded graph was extracted and will be activated at next graph activation check or application start.".formatted(getProfileDescriptiveName())); } public void writeOrsGraphInfoFileIfNotExists(ORSGraphHopper gh) { @@ -352,8 +365,9 @@ Date getDateFromGhProperty(GraphHopper gh, String ghProperty) { DateFormat f = Helper.createFormatter(); return f.parse(importDateString); } catch (ParseException e) { + LOGGER.error("Could not parse date from GraphHopper property %s".formatted(ghProperty)); + return null; } - return null; } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java index 16d580aa40..6e37a3ea64 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java @@ -18,13 +18,13 @@ public class FileSystemRepoManager extends AbstractRepoManager implements ORSGraphRepoManager { private static final Logger LOGGER = Logger.getLogger(FileSystemRepoManager.class.getName()); - private String graphsRepoPath; - private String graphsRepoName; - private String graphsProfileGroup; - private String graphsRepoCoverage; - private String graphsRepoGraphVersion; - private ORSGraphFileManager orsGraphFileManager; - private ORSGraphRepoStrategy orsGraphRepoStrategy; + private final String graphsRepoPath; + private final String graphsRepoName; + private final String graphsProfileGroup; + private final String graphsRepoCoverage; + private final String graphsRepoGraphVersion; + private final ORSGraphFileManager orsGraphFileManager; + private final ORSGraphRepoStrategy orsGraphRepoStrategy; public FileSystemRepoManager(GraphManagementRuntimeProperties graphManagementRuntimeProperties, ORSGraphRepoStrategy orsGraphRepoStrategy, ORSGraphFileManager orsGraphFileManager) { this.graphsRepoPath = graphManagementRuntimeProperties.getDerivedRepoPath().toAbsolutePath().toString(); @@ -104,10 +104,10 @@ GraphInfo downloadLatestGraphInfoFromRepository() { if (downloadedGraphInfoFile.exists()) { Path latestCompressedGraphInRepoPath = Path.of(graphsRepoPath, graphsRepoName, graphsProfileGroup, graphsRepoCoverage, graphsRepoGraphVersion, orsGraphRepoStrategy.getRepoCompressedGraphFileName()); URI uri = latestCompressedGraphInRepoPath.toUri(); - latestGraphInfoInRepo.withRemoteUri(uri); + latestGraphInfoInRepo.setRemoteUri(uri); PersistedGraphInfo persistedGraphInfo = orsGraphFileManager.readOrsGraphInfo(downloadedGraphInfoFile); - latestGraphInfoInRepo.withPersistedInfo(persistedGraphInfo); + latestGraphInfoInRepo.setPersistedGraphInfo(persistedGraphInfo); } else { LOGGER.error("[%s] Invalid download path for graphInfo file: %s".formatted(getProfileDescriptiveName(), latestGraphInfoInRepoPath)); } @@ -121,13 +121,11 @@ public void downloadFile(Path repoPath, File localPath) { } else { LOGGER.info("[%s] Downloading %s...".formatted(getProfileDescriptiveName(), repoPath.toFile().getName())); } - if (repoPath != null) { - try { - Files.copy(repoPath, localPath.toPath(), StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e) { - LOGGER.warn("[%s] Caught %s when trying to download %s".formatted(getProfileDescriptiveName(), repoPath.toFile().getAbsolutePath())); - throw new IllegalArgumentException(e); - } + try { + Files.copy(repoPath, localPath.toPath(), StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + LOGGER.warn("[%s] Caught %s when trying to download %s".formatted(getProfileDescriptiveName(), e, repoPath.toFile().getAbsolutePath())); + throw new IllegalArgumentException(e); } } } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java index de758d3637..5e6546142f 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java @@ -1,5 +1,6 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.remote; +import lombok.NoArgsConstructor; import lombok.Setter; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; @@ -13,12 +14,14 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.Files; import java.util.stream.Collectors; import java.util.stream.Stream; import static com.google.common.base.Strings.isNullOrEmpty; @Setter +@NoArgsConstructor public class HttpRepoManager extends AbstractRepoManager implements ORSGraphRepoManager { private static final Logger LOGGER = Logger.getLogger(HttpRepoManager.class.getName()); @@ -28,9 +31,6 @@ public class HttpRepoManager extends AbstractRepoManager implements ORSGraphRepo private ORSGraphFileManager orsGraphFileManager; private ORSGraphRepoStrategy orsGraphRepoStrategy; - public HttpRepoManager() { - } - public HttpRepoManager(GraphManagementRuntimeProperties managementProps, ORSGraphRepoStrategy orsGraphRepoStrategy, ORSGraphFileManager orsGraphFileManager) { this.managementProps = managementProps; this.orsGraphRepoStrategy = orsGraphRepoStrategy; @@ -41,6 +41,16 @@ String getProfileDescriptiveName() { return orsGraphFileManager.getProfileDescriptiveName(); } + public static String concatenateToUrlPath(String... values) { + return Stream.of(values) + .filter(StringUtils::isNotBlank) + .map(String::trim) + .map(s -> s.replaceAll("^/", "")) + .map(s -> s.replaceAll("/$", "")) + .filter(s -> !s.equals(".")) + .collect(Collectors.joining("/")); + } + public URL createDownloadUrl(String fileName) { String urlString = concatenateToUrlPath(this.managementProps.getDerivedRepoBaseUrl().toString(), this.managementProps.getRepoName(), @@ -57,15 +67,13 @@ public URL createDownloadUrl(String fileName) { } } - public static String concatenateToUrlPath(String... values) { - String urlString = Stream.of(values) - .filter(StringUtils::isNotBlank) - .map(String::trim) - .map(s -> s.replaceAll("^/", "")) - .map(s -> s.replaceAll("/$", "")) - .filter(s -> !s.equals(".")) - .collect(Collectors.joining("/")); - return urlString; + private void deleteFileWithLogging(File file, String successMessage, String errorMessage) { + try { + Files.deleteIfExists(file.toPath()); + LOGGER.debug(successMessage.formatted(getProfileDescriptiveName(), file.getAbsolutePath())); + } catch (IOException e) { + LOGGER.error(errorMessage.formatted(e.getMessage())); + } } @Override @@ -112,9 +120,7 @@ GraphInfo downloadGraphInfoFromRepository() { return graphInfoInRepo; } File downloadedGraphInfoFile = orsGraphFileManager.getDownloadedGraphInfoFile(); - if (downloadedGraphInfoFile.exists()) { - downloadedGraphInfoFile.delete(); - } + deleteFileWithLogging(downloadedGraphInfoFile, "[%s] Deleted old downloaded graphInfo file: %s", "[%s] Could not delete old downloaded graphInfo file: %s"); downloadFile(downloadUrl, downloadedGraphInfoFile); if (!downloadedGraphInfoFile.exists()) { LOGGER.info("[%s] No graphInfo found in remote repository.".formatted(getProfileDescriptiveName())); @@ -123,7 +129,7 @@ GraphInfo downloadGraphInfoFromRepository() { graphInfoInRepo.withRemoteUrl(downloadUrl); PersistedGraphInfo persistedGraphInfo = orsGraphFileManager.readOrsGraphInfo(downloadedGraphInfoFile); - graphInfoInRepo.withPersistedInfo(persistedGraphInfo); + graphInfoInRepo.setPersistedGraphInfo(persistedGraphInfo); return graphInfoInRepo; } @@ -133,9 +139,7 @@ void downloadCompressedGraphFromRepository() { return; } File downloadedFile = orsGraphFileManager.getDownloadedCompressedGraphFile(); - if (downloadedFile.exists()) { - downloadedFile.delete(); - } + deleteFileWithLogging(downloadedFile, "[%s] Deleted old downloaded compressed graph file: %s", "[%s] Could not delete old downloaded compressed graph file: %s"); long start = System.currentTimeMillis(); downloadFile(downloadUrl, downloadedFile);//mocked!!! @@ -161,11 +165,15 @@ public void downloadFile(URL downloadUrl, File outputFile) { tempDownloadFile, connectionTimeoutMillis, readTimeoutMillis); - tempDownloadFile.renameTo(outputFile); + if (tempDownloadFile.renameTo(outputFile)) { + LOGGER.debug("[%s] Renamed temp file to %s".formatted(getProfileDescriptiveName(), outputFile.getAbsolutePath())); + } else { + LOGGER.error("[%s] Could not rename temp file to %s".formatted(getProfileDescriptiveName(), outputFile.getAbsolutePath())); + } } catch (IOException e) { LOGGER.warn("[%s] Caught %s when trying to download %s".formatted(getProfileDescriptiveName(), e.getClass().getName(), downloadUrl)); } finally { - tempDownloadFile.delete(); + deleteFileWithLogging(tempDownloadFile, "[%s] Deleted temp download file: %s", "[%s] Could not delete temp download file: %s"); } } } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NamedGraphsRepoStrategy.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NamedGraphsRepoStrategy.java index 3f18ca26b4..0ce5af130d 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NamedGraphsRepoStrategy.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NamedGraphsRepoStrategy.java @@ -1,16 +1,14 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.remote; +import lombok.AllArgsConstructor; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; import org.heigit.ors.routing.graphhopper.extensions.manage.local.ORSGraphFolderStrategy; +@AllArgsConstructor public class NamedGraphsRepoStrategy implements ORSGraphRepoStrategy { private final GraphManagementRuntimeProperties managementProperties; - public NamedGraphsRepoStrategy(GraphManagementRuntimeProperties managementProperties) { - this.managementProperties = managementProperties; - } - private String getConcatenatedRepoFileName() { return String.join("_", managementProperties.getRepoProfileGroup(), diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NullRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NullRepoManager.java index fb71277127..ebd469bd69 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NullRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/NullRepoManager.java @@ -3,5 +3,6 @@ public class NullRepoManager implements ORSGraphRepoManager { @Override public void downloadGraphIfNecessary() { + // The NullRepoManager does nothing at this point. } } diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimePropertiesTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimePropertiesTest.java index 1a4552ca7b..d1006a2007 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimePropertiesTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimePropertiesTest.java @@ -22,7 +22,7 @@ class GraphManagementRuntimePropertiesTest { "~/absolute/path, FILESYSTEM", "~/absolute/path.txt, FILESYSTEM" }) - public void deriveRepoBaseUrl(String repoBaseUri, GraphManagementRuntimeProperties.GraphRepoType expectedType) { + void deriveRepoBaseUrl(String repoBaseUri, GraphManagementRuntimeProperties.GraphRepoType expectedType) { GraphManagementRuntimeProperties managementRuntimeProperties = GraphManagementRuntimeProperties.Builder.empty().withRepoBaseUri(repoBaseUri).build(); assertEquals(expectedType, managementRuntimeProperties.getDerivedRepoType()); switch (expectedType) { diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManagerTest.java index 8f625fa31d..263bfabd26 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManagerTest.java @@ -21,7 +21,7 @@ class ORSGraphManagerTest { "false, true, , http://my.domain.com", "false, false, repoName, http://my.domain.com", }) - public void useGraphRepository(boolean expectUseRepo, boolean enable, String repoName, String baseUri) { + void useGraphRepository(boolean expectUseRepo, boolean enable, String repoName, String baseUri) { GraphManagementRuntimeProperties managementProps = GraphManagementRuntimeProperties.Builder.empty() .withEnabled(enable) .withRepoName(repoName) @@ -48,7 +48,7 @@ public void useGraphRepository(boolean expectUseRepo, boolean enable, String rep "FileSystemRepoManager, ~/absolute/path", "FileSystemRepoManager, ~/absolute/path.txt" }) - public void getOrsGraphRepoManager(String className, String repoUri) { + void getOrsGraphRepoManager(String className, String repoUri) { GraphManagementRuntimeProperties managementProps = GraphManagementRuntimeProperties.Builder.empty() .withLocalGraphsRootAbsPath("graphs") .withRepoBaseUri(repoUri) diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java index 3bcff587e1..21f0e00cfe 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java @@ -30,17 +30,17 @@ public class RepoManagerTestHelper { public static final long MIDDLE_DATE = 1692373222000L; // Fr 18. Aug 17:40:22 CEST 2023 public static final long LATER_DATE = 1692373333000L; // Fr 18. Aug 17:42:13 CEST 2023 - public static long REPO_CAR_OSM_DATE; - public static long REPO_HGV_OSM_DATE; - public static long REPO_CAR_GRAPH_BUILD_DATE; - public static long REPO_HGV_GRAPH_BUILD_DATE; + public static long repoCarOsmDate; + public static long repoHgvOsmDate; + public static long repoCarGraphBuildDate; + public static long repoHgvGraphBuildDate; static { DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ"); - REPO_CAR_OSM_DATE = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); - REPO_HGV_OSM_DATE = ZonedDateTime.parse("2024-01-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); - REPO_CAR_GRAPH_BUILD_DATE = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); - REPO_HGV_GRAPH_BUILD_DATE = ZonedDateTime.parse("2024-06-26T10:23:39+0000", formatter).toInstant().toEpochMilli(); + repoCarOsmDate = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); + repoHgvOsmDate = ZonedDateTime.parse("2024-01-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); + repoCarGraphBuildDate = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); + repoHgvGraphBuildDate = ZonedDateTime.parse("2024-06-26T10:23:39+0000", formatter).toInstant().toEpochMilli(); } public static Path createLocalGraphsRootDirectory(Path tempDir) throws IOException { @@ -64,7 +64,7 @@ public static Path createLocalGraphDirectoryWithGraphInfoFile(Path localGraphsRo return localGraphDir; } - public static void saveActiveGraphInfoFile(File activeGraphInfoFile, Long graphBuildDate, Long osmDate) throws IOException { + public static void saveActiveGraphInfoFile(File activeGraphInfoFile, Long graphBuildDate, Long osmDate) { PersistedGraphInfo activeGraphInfoObject = new PersistedGraphInfo(); if (graphBuildDate != null) activeGraphInfoObject.setGraphBuildDate(new Date(graphBuildDate)); if (osmDate != null) activeGraphInfoObject.setOsmDate(new Date(osmDate)); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java index 4cac5b696d..d4cbf4e462 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java @@ -91,7 +91,7 @@ void writeOrsGraphInfo() throws IOException { PersistedGraphInfo persistedGraphInfo = createOrsGraphInfoV1(); assertFalse(testFile.exists()); - orsGraphFileManager.writeOrsGraphInfo(persistedGraphInfo, testFile); + ORSGraphFileManager.writeOrsGraphInfo(persistedGraphInfo, testFile); assertTrue(testFile.exists()); } @@ -120,7 +120,7 @@ void backupExistingGraph_noPreviousBackup() throws IOException { localGraphDir = localGraphPath.toFile(); assertFalse(localGraphDir.exists()); assertEquals(1, orsGraphFileManager.findGraphBackupsSortedByName().size()); - orsGraphFileManager.findGraphBackupsSortedByName().forEach(dir -> assertCorrectBackupDir(dir)); + orsGraphFileManager.findGraphBackupsSortedByName().forEach(this::assertCorrectBackupDir); } @Test @@ -134,7 +134,7 @@ void backupExistingGraph_withPreviousBackup() throws IOException { assertFalse(localGraphPath.toFile().exists()); assertEquals(2, orsGraphFileManager.findGraphBackupsSortedByName().size()); - orsGraphFileManager.findGraphBackupsSortedByName().forEach(dir -> assertCorrectBackupDir(dir)); + orsGraphFileManager.findGraphBackupsSortedByName().forEach(this::assertCorrectBackupDir); } @Test @@ -150,13 +150,13 @@ void backupExistingGraph_withMaxNumOfPreviousBackups() throws IOException { assertFalse(localGraphPath.toFile().exists()); List backups = orsGraphFileManager.findGraphBackupsSortedByName(); assertEquals(2, backups.size()); - backups.forEach(dir -> assertCorrectBackupDir(dir)); + backups.forEach(this::assertCorrectBackupDir); assertEquals("truck_2023-01-01_060000", backups.get(0).getName()); assertNotEquals("truck_2022-12-31_235959", backups.get(1).getName()); } @Test - public void deleteOldestBackups() throws IOException { + void deleteOldestBackups() throws IOException { setupORSGraphManager(managementPropsBuilder().withMaxNumberOfGraphBackups(3).build()); createBackupDirectory("2023-01-01_060000"); createBackupDirectory("2023-01-02_060000"); @@ -169,12 +169,12 @@ public void deleteOldestBackups() throws IOException { List backups = orsGraphFileManager.findGraphBackupsSortedByName(); assertEquals(3, backups.size()); - backups.forEach(dir -> assertCorrectBackupDir(dir)); + backups.forEach(this::assertCorrectBackupDir); assertEquals(Arrays.asList("truck_2023-01-03_060000", "truck_2023-01-04_060000", "truck_2023-01-05_060000"), backups.stream().map(File::getName).toList()); } @Test - public void deleteOldestBackups_maxNumberOfGraphBackupsIsZero() throws IOException { + void deleteOldestBackups_maxNumberOfGraphBackupsIsZero() throws IOException { setupORSGraphManager(managementPropsBuilder().withMaxNumberOfGraphBackups(0).build()); createBackupDirectory("2023-01-01_060000"); createBackupDirectory("2023-01-02_060000"); @@ -190,7 +190,7 @@ public void deleteOldestBackups_maxNumberOfGraphBackupsIsZero() throws IOExcepti } @Test - public void deleteOldestBackups_maxNumberOfGraphBackupsIsNegative() throws IOException { + void deleteOldestBackups_maxNumberOfGraphBackupsIsNegative() throws IOException { setupORSGraphManager(managementPropsBuilder().withMaxNumberOfGraphBackups(-5).build()); createBackupDirectory("2023-01-01_060000"); createBackupDirectory("2023-01-02_060000"); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java index 73f17762ad..c060822bc3 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java @@ -26,14 +26,12 @@ class FileSystemRepoManagerTest { private Path tempDir; private Path localGraphsRootPath; - private Path localGraphPath; private FileSystemRepoManager fileSystemRepoManager; private ORSGraphFileManager orsGraphFileManager; @BeforeEach public void setUp() throws IOException { localGraphsRootPath = createLocalGraphsRootDirectory(tempDir); - localGraphPath = createLocalGraphDirectory(localGraphsRootPath, PROFILE_NAME); } @AfterEach @@ -73,7 +71,7 @@ void downloadGraphIfNecessary_noLocalData_remoteDataExists() { } @Test - void downloadGraphIfNecessary_localDataExists_noRemoteData() throws IOException { + void downloadGraphIfNecessary_localDataExists_noRemoteData() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_NONEXISTING_GRAPHS_VERSION).build()); saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), LATER_DATE, EARLIER_DATE); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_0_driving-hgv.yml").toFile().exists()); @@ -84,9 +82,9 @@ void downloadGraphIfNecessary_localDataExists_noRemoteData() throws IOException } @Test - void downloadGraphIfNecessary_localDate_before_remoteDate() throws IOException { + void downloadGraphIfNecessary_localDate_before_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), REPO_HGV_OSM_DATE - 1000000, REPO_HGV_GRAPH_BUILD_DATE - 1000000); + saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), repoHgvOsmDate - 1000000, repoHgvGraphBuildDate - 1000000); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.ghz").toFile().exists()); fileSystemRepoManager.downloadGraphIfNecessary(); @@ -95,9 +93,9 @@ void downloadGraphIfNecessary_localDate_before_remoteDate() throws IOException { } @Test - void downloadGraphIfNecessary_localDate_equals_remoteDate() throws IOException { + void downloadGraphIfNecessary_localDate_equals_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), REPO_HGV_GRAPH_BUILD_DATE, REPO_HGV_OSM_DATE); + saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), repoHgvGraphBuildDate, repoHgvOsmDate); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.ghz").toFile().exists()); fileSystemRepoManager.downloadGraphIfNecessary(); @@ -106,9 +104,9 @@ void downloadGraphIfNecessary_localDate_equals_remoteDate() throws IOException { } @Test - void downloadGraphIfNecessary_localDate_after_remoteDate() throws IOException { + void downloadGraphIfNecessary_localDate_after_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), REPO_HGV_GRAPH_BUILD_DATE + 1000000, REPO_HGV_OSM_DATE + 1000000); + saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), repoHgvGraphBuildDate + 1000000, repoHgvOsmDate + 1000000); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.ghz").toFile().exists()); fileSystemRepoManager.downloadGraphIfNecessary(); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java index 548e21ded9..1c870f90f5 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java @@ -81,7 +81,7 @@ private static GraphManagementRuntimeProperties.Builder managementPropsBuilder() .withRepoBaseUri(nginxUrl); } - private void setupActiveGraphDirectory(Long osmDateLocal) throws IOException { + private void setupActiveGraphDirectory(Long osmDateLocal) { saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), osmDateLocal, null); } @@ -115,7 +115,7 @@ void createDownloadUrl() { .build(); FlatORSGraphFolderStrategy orsGraphFolderStrategy = new FlatORSGraphFolderStrategy(managementProps); - ORSGraphFileManager orsGraphFileManager = new ORSGraphFileManager(managementProps, orsGraphFolderStrategy); + orsGraphFileManager = new ORSGraphFileManager(managementProps, orsGraphFolderStrategy); ORSGraphRepoStrategy orsGraphRepoStrategy = new NamedGraphsRepoStrategy(managementProps); HttpRepoManager httpRepoManager = new HttpRepoManager(managementProps, orsGraphRepoStrategy, orsGraphFileManager); URL downloadUrl = httpRepoManager.createDownloadUrl("graph.ghz"); @@ -153,7 +153,7 @@ void checkSetupActiveGraphDirectory() throws IOException { } @Test - void downloadGraphIfNecessary_noDownloadWhen_localDataExists_noRemoteData() throws IOException { + void downloadGraphIfNecessary_noDownloadWhen_localDataExists_noRemoteData() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_NONEXISTING_GRAPHS_VERSION).build()); setupActiveGraphDirectory(EARLIER_DATE); @@ -178,7 +178,7 @@ void downloadGraphIfNecessary_downloadWhen_noLocalData_remoteDataExists() { } @Test - void downloadGraphIfNecessary_downloadWhen_localDate_before_remoteDate() throws IOException { + void downloadGraphIfNecessary_downloadWhen_localDate_before_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); setupActiveGraphDirectory(EARLIER_DATE); @@ -191,9 +191,9 @@ void downloadGraphIfNecessary_downloadWhen_localDate_before_remoteDate() throws } @Test - void downloadGraphIfNecessary_noDownloadWhen_localDate_equals_remoteDate() throws IOException { + void downloadGraphIfNecessary_noDownloadWhen_localDate_equals_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(REPO_CAR_GRAPH_BUILD_DATE); + setupActiveGraphDirectory(repoCarGraphBuildDate); orsGraphRepoManager.downloadGraphIfNecessary(); @@ -204,9 +204,9 @@ void downloadGraphIfNecessary_noDownloadWhen_localDate_equals_remoteDate() throw } @Test - void downloadGraphIfNecessary_noDownloadWhen_localDate_after_remoteDate() throws IOException { + void downloadGraphIfNecessary_noDownloadWhen_localDate_after_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(REPO_CAR_GRAPH_BUILD_DATE + 1000000); + setupActiveGraphDirectory(repoCarGraphBuildDate + 1000000); orsGraphRepoManager.downloadGraphIfNecessary(); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/TestAbstractRepoManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/TestAbstractRepoManagerTest.java index 90babf77b7..36eaf8fe3f 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/TestAbstractRepoManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/TestAbstractRepoManagerTest.java @@ -52,30 +52,30 @@ public static Stream shouldDownloadGraphMethodSource() { ); } - @ParameterizedTest - @MethodSource("comparisonDates") - public void getDateOrEpocStart(Date expectedDate, GraphInfo graphInfo) { - assertEquals(expectedDate, orsGraphRepoManager.getDateOrEpocStart(graphInfo)); - } - public static Stream comparisonDates() throws MalformedURLException { Date now = new Date(); Date epocStart = new Date(0); return Stream.of( Arguments.of(new Date(0), null), Arguments.of(epocStart, new GraphInfo()), - Arguments.of(epocStart, new GraphInfo().withLocalDirectory(tempDir.toFile())), + Arguments.of(epocStart, new GraphInfo().setLocalDirectory(tempDir.toFile())), Arguments.of(epocStart, new GraphInfo().withRemoteUrl(new URL("http://some.url.ors/"))), - Arguments.of(epocStart, new GraphInfo().withPersistedInfo(null)), - Arguments.of(epocStart, new GraphInfo().withPersistedInfo(new PersistedGraphInfo())), - Arguments.of(epocStart, new GraphInfo().withPersistedInfo(PersistedGraphInfo.withOsmDate(now))), - Arguments.of(now, new GraphInfo().withPersistedInfo(PersistedGraphInfo.withGraphBuildDate(now))) + Arguments.of(epocStart, new GraphInfo().setPersistedGraphInfo(null)), + Arguments.of(epocStart, new GraphInfo().setPersistedGraphInfo(new PersistedGraphInfo())), + Arguments.of(epocStart, new GraphInfo().setPersistedGraphInfo(PersistedGraphInfo.withOsmDate(now))), + Arguments.of(now, new GraphInfo().setPersistedGraphInfo(PersistedGraphInfo.withGraphBuildDate(now))) ); } + @ParameterizedTest + @MethodSource("comparisonDates") + void getDateOrEpocStart(Date expectedDate, GraphInfo graphInfo) { + assertEquals(expectedDate, orsGraphRepoManager.getDateOrEpocStart(graphInfo)); + } + @ParameterizedTest @MethodSource("comparisonDatesForDownloadFiles") - public void getDateOrEpocStart(Date expectedDate, File downloadFile, PersistedGraphInfo persistedGraphInfo) throws IOException { + void getDateOrEpocStart(Date expectedDate, File downloadFile, PersistedGraphInfo persistedGraphInfo) { assertEquals(expectedDate, orsGraphRepoManager.getDateOrEpocStart(downloadFile, persistedGraphInfo)); } @@ -105,7 +105,7 @@ public static Stream comparisonDatesForDownloadFiles() throws IOExcep } @Test - public void newestDate() { + void newestDate() { assertEquals(new Date(LATER_DATE), orsGraphRepoManager.newestDate( new Date(MIDDLE_DATE), From 0de60e48dfc621371034185089b8791a8c85637a Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 11 Nov 2024 19:46:01 +0100 Subject: [PATCH 02/11] chore(sonar): Reduce the complexity of checkForDownloadedGraphsToActivate(). java:S3776. --- .../heigit/ors/api/services/GraphService.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java b/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java index bd3f7d0f69..a8c041de88 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java @@ -109,21 +109,11 @@ public void checkForDownloadedGraphsToActivate(String trigger) { boolean graphActivationAllowed = true; for (ORSGraphManager orsGraphManager : graphManagers) { - if (orsGraphManager.isBusy() || orsGraphManager.hasGraphDownloadFile()) { - if (!graphActivationAttemptWasBlocked.get()) { - LOGGER.info("[%s] %s graph activation check: Download or extraction in progress".formatted( - orsGraphManager.getQualifiedProfileName(), - trigger - )); - } + if (isGraphManagerBusyOrHasDownloadFile(orsGraphManager, trigger)) { graphActivationAllowed = false; } if (orsGraphManager.hasDownloadedExtractedGraph()) { - if (!graphActivationAttemptWasBlocked.get()) { - LOGGER.info("[%s] %s graph activation check: Downloaded extracted graph available".formatted( - orsGraphManager.getQualifiedProfileName(), - trigger)); - } + logDownloadedExtractedGraphAvailable(orsGraphManager, trigger); graphActivationNeeded = true; } } @@ -149,6 +139,27 @@ public void checkForDownloadedGraphsToActivate(String trigger) { activateGraphs(); } + private boolean isGraphManagerBusyOrHasDownloadFile(ORSGraphManager orsGraphManager, String trigger) { + if (orsGraphManager.isBusy() || orsGraphManager.hasGraphDownloadFile()) { + if (!graphActivationAttemptWasBlocked.get()) { + LOGGER.info("[%s] %s graph activation check: Download or extraction in progress".formatted( + orsGraphManager.getQualifiedProfileName(), + trigger + )); + } + return true; + } + return false; + } + + private void logDownloadedExtractedGraphAvailable(ORSGraphManager orsGraphManager, String trigger) { + if (!graphActivationAttemptWasBlocked.get()) { + LOGGER.info("[%s] %s graph activation check: Downloaded extracted graph available".formatted( + orsGraphManager.getQualifiedProfileName(), + trigger)); + } + } + @Async @Scheduled(fixedDelay = 1, timeUnit = TimeUnit.MINUTES) public void repeatedGraphActivationAttempt() { From 013e7cfd7395ffd9ffcd15d33b7f9c83828b2c96 Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 14:38:45 +0100 Subject: [PATCH 03/11] chore(sonar): Address the wrong autowiring. java:S6813. Autowired injection should always happen on the constructor to ensure that the class cannot accidentally be called without injection. --- .../java/org/heigit/ors/api/services/GraphService.java | 3 +-- .../api/servlet/listeners/ORSInitContextListenerTest.java | 8 +++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java b/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java index a8c041de88..361b5bc085 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java @@ -24,8 +24,7 @@ public class GraphService { @Value("${ors.engine.graph_management.enabled:false}") private Boolean enabled = false; - @Autowired - private EngineProperties engineProperties; + private final EngineProperties engineProperties; private static final Logger LOGGER = Logger.getLogger(GraphService.class.getName()); diff --git a/ors-api/src/test/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListenerTest.java b/ors-api/src/test/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListenerTest.java index 54ecc9417e..ab6fd38daa 100644 --- a/ors-api/src/test/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListenerTest.java +++ b/ors-api/src/test/java/org/heigit/ors/api/servlet/listeners/ORSInitContextListenerTest.java @@ -3,14 +3,20 @@ import org.heigit.ors.api.services.GraphService; import org.heigit.ors.config.EngineProperties; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +@SpringBootTest class ORSInitContextListenerTest { + @Autowired + private GraphService graphService; + @Test void testConfigurationOutputTarget() { - ORSInitContextListener orsInitContextListener = new ORSInitContextListener(new EngineProperties(), new GraphService()); + ORSInitContextListener orsInitContextListener = new ORSInitContextListener(new EngineProperties(), graphService); EngineProperties engineProperties = new EngineProperties(); assertNull(orsInitContextListener.configurationOutputTarget(engineProperties), "default should return null"); From cdff3c4f41e1eb90d9890accd98b584e2eec57fc Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 14:51:08 +0100 Subject: [PATCH 04/11] chore(sonar): Introduce a custom RuntimeException ORSGraphFileManagerException.java. java:S112. --- .../heigit/ors/apitests/ORSStartupTest.java | 3 ++- .../ORSGraphFileManagerException.java | 7 +++++++ .../extensions/manage/ORSGraphManager.java | 7 ++++--- .../manage/local/ORSGraphFileManager.java | 18 ++++++++++-------- .../manage/remote/FileSystemRepoManager.java | 3 ++- .../manage/remote/HttpRepoManager.java | 3 ++- .../manage/local/ORSGraphFileManagerTest.java | 3 ++- .../remote/FileSystemRepoManagerTest.java | 3 ++- 8 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerException.java diff --git a/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java b/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java index 0f20abc74e..221416a001 100644 --- a/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java +++ b/ors-api/src/test/java/org/heigit/ors/apitests/ORSStartupTest.java @@ -3,6 +3,7 @@ import org.heigit.ors.apitests.common.ServiceTest; import org.heigit.ors.common.EncoderNameEnum; import org.heigit.ors.config.profile.ProfileProperties; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.RoutingProfile; import org.heigit.ors.routing.RoutingProfileManager; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphInfo; @@ -17,7 +18,7 @@ class ORSStartupTest extends ServiceTest { SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); @Test - void testGraphInfoFilesWrittenCorrectly() throws ParseException { + void testGraphInfoFilesWrittenCorrectly() throws ParseException, ORSGraphFileManagerException { RoutingProfileManager rpm = RoutingProfileManager.getInstance(); RoutingProfile profile = rpm.getRoutingProfile(EncoderNameEnum.DRIVING_CAR.getName()); GraphInfo graphInfo = profile.getGraphhopper().getOrsGraphManager().getActiveGraphInfo(); diff --git a/ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerException.java b/ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerException.java new file mode 100644 index 0000000000..ceb3bc4f91 --- /dev/null +++ b/ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerException.java @@ -0,0 +1,7 @@ +package org.heigit.ors.exceptions; + +public class ORSGraphFileManagerException extends RuntimeException { + public ORSGraphFileManagerException(String message, Throwable cause) { + super(message, cause); + } +} \ No newline at end of file diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java index 6eaa722ae8..330548545c 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java @@ -6,6 +6,7 @@ import org.apache.log4j.Logger; import org.heigit.ors.config.EngineProperties; import org.heigit.ors.config.profile.ProfileProperties; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.graphhopper.extensions.ORSGraphHopper; import org.heigit.ors.routing.graphhopper.extensions.manage.local.FlatORSGraphFolderStrategy; import org.heigit.ors.routing.graphhopper.extensions.manage.local.ORSGraphFileManager; @@ -67,7 +68,7 @@ public static ORSGraphRepoManager getOrsGraphRepoManager(GraphManagementRuntimeP return orsGraphRepoManager; } - public ProfileProperties loadProfilePropertiesFromActiveGraph(ORSGraphManager orsGraphManager, ProfileProperties profileProperties) { + public ProfileProperties loadProfilePropertiesFromActiveGraph(ORSGraphManager orsGraphManager, ProfileProperties profileProperties) throws ORSGraphFileManagerException { profileProperties.mergeLoaded(orsGraphManager.getActiveGraphProfileProperties()); return profileProperties; } @@ -155,11 +156,11 @@ public void writeOrsGraphInfoFileIfNotExists(ORSGraphHopper gh) { orsGraphFileManager.writeOrsGraphInfoFileIfNotExists(gh); } - public GraphInfo getActiveGraphInfo() { + public GraphInfo getActiveGraphInfo() throws ORSGraphFileManagerException { return orsGraphFileManager.getActiveGraphInfo(); } - public ProfileProperties getActiveGraphProfileProperties() { + public ProfileProperties getActiveGraphProfileProperties() throws ORSGraphFileManagerException { return ofNullable(getActiveGraphInfo()) .map(GraphInfo::getPersistedGraphInfo) .map(PersistedGraphInfo::getProfileProperties) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java index f9b512524b..74d909488c 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java @@ -13,6 +13,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.heigit.ors.config.profile.ProfileProperties; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.graphhopper.extensions.ORSGraphHopper; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphInfo; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; @@ -184,7 +185,7 @@ public List findGraphBackupsSortedByName() { return Arrays.asList(Objects.requireNonNull(obj)).stream().sorted(Comparator.comparing(File::getName)).toList(); } - public GraphInfo getActiveGraphInfo() { + public GraphInfo getActiveGraphInfo() throws ORSGraphFileManagerException { LOGGER.trace("[%s] Checking active graph info...".formatted(getProfileDescriptiveName())); File activeGraphDirectory = getActiveGraphDirectory(); @@ -196,7 +197,7 @@ public GraphInfo getActiveGraphInfo() { return getGraphInfo(getActiveGraphInfoFile()); } - public GraphInfo getDownloadedExtractedGraphInfo() { + public GraphInfo getDownloadedExtractedGraphInfo() throws ORSGraphFileManagerException { LOGGER.trace("[%s] Checking downloaded graph info...".formatted(getProfileDescriptiveName())); File downloadedExtractedGraphDirectory = getDownloadedExtractedGraphDirectory(); @@ -208,7 +209,7 @@ public GraphInfo getDownloadedExtractedGraphInfo() { return getGraphInfo(getDownloadedExtractedGraphInfoFile()); } - private GraphInfo getGraphInfo(File graphInfoFile) { + private GraphInfo getGraphInfo(File graphInfoFile) throws ORSGraphFileManagerException { File graphDirectory = graphInfoFile.getParentFile(); if (!graphInfoFile.exists() || !graphInfoFile.isFile()) { LOGGER.trace("[%s] No graph info file %s found in %s".formatted(getProfileDescriptiveName(), graphInfoFile.getName(), graphInfoFile.getParentFile().getName())); @@ -235,12 +236,13 @@ static ObjectMapper getYamlMapper() { return mapper; } - public PersistedGraphInfo readOrsGraphInfo(File graphInfoFile) { + public PersistedGraphInfo readOrsGraphInfo(File graphInfoFile) throws ORSGraphFileManagerException { try { return getYamlMapper() .readValue(graphInfoFile, PersistedGraphInfo.class); } catch (IOException e) { - throw new RuntimeException(e); + LOGGER.error("Error reading ORS graph info: %s".formatted(e.getMessage())); + throw new ORSGraphFileManagerException("Error reading ORS graph info: ", e); } } @@ -250,11 +252,11 @@ public static void writeOrsGraphInfo(PersistedGraphInfo persistedGraphInfo, File .writeValue(outputFile, persistedGraphInfo); } catch (IOException e) { LOGGER.error("Could not write file %s".formatted(outputFile.getAbsolutePath())); - throw new RuntimeException(e); + throw new ORSGraphFileManagerException("Could not write file: ", e); } } - public PersistedGraphInfo getDownloadedGraphInfo() { + public PersistedGraphInfo getDownloadedGraphInfo() throws ORSGraphFileManagerException { LOGGER.trace("[%s] Checking graph info of previous check...".formatted(getProfileDescriptiveName())); File downloadedGraphInfoFile = getDownloadedGraphInfoFile(); if (downloadedGraphInfoFile.exists()) { @@ -323,7 +325,7 @@ public void extractDownloadedGraph() { graphDownloadFileAbsPath, extractionDirectoryAbsPath, targetDirectoryAbsPath)); - throw new RuntimeException("Caught ", ioException); + throw new ORSGraphFileManagerException("Error during extraction of downloaded graph file: ", ioException); } LOGGER.info("[%s] Downloaded graph was extracted and will be activated at next graph activation check or application start.".formatted(getProfileDescriptiveName())); } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java index 6e37a3ea64..3f49d8534e 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java @@ -1,6 +1,7 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.remote; import org.apache.log4j.Logger; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphInfo; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; import org.heigit.ors.routing.graphhopper.extensions.manage.PersistedGraphInfo; @@ -88,7 +89,7 @@ public void downloadGraphIfNecessary() { } } - GraphInfo downloadLatestGraphInfoFromRepository() { + GraphInfo downloadLatestGraphInfoFromRepository() throws ORSGraphFileManagerException { GraphInfo latestGraphInfoInRepo = new GraphInfo(); LOGGER.debug("[%s] Checking latest graphInfo in remote repository...".formatted(getProfileDescriptiveName())); diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java index 5e6546142f..1bc2c8e1ae 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java @@ -5,6 +5,7 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphInfo; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; import org.heigit.ors.routing.graphhopper.extensions.manage.PersistedGraphInfo; @@ -111,7 +112,7 @@ public void downloadGraphIfNecessary() { } } - GraphInfo downloadGraphInfoFromRepository() { + GraphInfo downloadGraphInfoFromRepository() throws ORSGraphFileManagerException { GraphInfo graphInfoInRepo = new GraphInfo(); LOGGER.debug("[%s] Checking latest graphInfo in remote repository...".formatted(getProfileDescriptiveName())); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java index d4cbf4e462..ad61aa063f 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManagerTest.java @@ -1,6 +1,7 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.local; import org.heigit.ors.config.profile.ProfileProperties; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; import org.heigit.ors.routing.graphhopper.extensions.manage.PersistedGraphInfo; import org.heigit.ors.routing.graphhopper.extensions.manage.RepoManagerTestHelper; @@ -97,7 +98,7 @@ void writeOrsGraphInfo() throws IOException { } @Test - void readOrsGraphInfo() throws IOException { + void readOrsGraphInfo() throws IOException, ORSGraphFileManagerException { setupORSGraphManager(managementPropsBuilder().build()); File writtenTestFile = new File(localGraphPath.toFile(), "readOrsGraphInfoV1.yml"); PersistedGraphInfo writtenPersistedGraphInfo = createOrsGraphInfoV1(); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java index c060822bc3..e7884fd627 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java @@ -1,5 +1,6 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.remote; +import org.heigit.ors.exceptions.ORSGraphFileManagerException; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; import org.heigit.ors.routing.graphhopper.extensions.manage.local.FlatORSGraphFolderStrategy; import org.heigit.ors.routing.graphhopper.extensions.manage.local.ORSGraphFileManager; @@ -53,7 +54,7 @@ private GraphManagementRuntimeProperties.Builder managementPropsBuilder() { } @Test - void downloadLatestGraphInfoFromRepository() { + void downloadLatestGraphInfoFromRepository() throws ORSGraphFileManagerException { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); fileSystemRepoManager.downloadLatestGraphInfoFromRepository(); assertTrue(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); From 4df7fd5944ec7655435dadae990b8537877e6f2f Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 14:56:07 +0100 Subject: [PATCH 05/11] chore(sonar): Remove unnecessary Lombok annotations This removes unneeded Lombok annotations for purely internal access classes. --- .../extensions/manage/remote/HttpRepoManager.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java index 1bc2c8e1ae..721ef921a1 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java @@ -1,7 +1,6 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.remote; import lombok.NoArgsConstructor; -import lombok.Setter; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; @@ -21,13 +20,10 @@ import static com.google.common.base.Strings.isNullOrEmpty; -@Setter @NoArgsConstructor public class HttpRepoManager extends AbstractRepoManager implements ORSGraphRepoManager { private static final Logger LOGGER = Logger.getLogger(HttpRepoManager.class.getName()); - private int connectionTimeoutMillis = 2000; - private int readTimeoutMillis = 200000; private GraphManagementRuntimeProperties managementProps; private ORSGraphFileManager orsGraphFileManager; private ORSGraphRepoStrategy orsGraphRepoStrategy; @@ -161,6 +157,8 @@ public void downloadFile(URL downloadUrl, File outputFile) { LOGGER.info("[%s] Downloading %s...".formatted(getProfileDescriptiveName(), downloadUrl)); } try { + int connectionTimeoutMillis = 2000; + int readTimeoutMillis = 200000; FileUtils.copyURLToFile( downloadUrl, tempDownloadFile, From ca6307ce27978dfac391b46db243992e9d201503 Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 16:11:31 +0100 Subject: [PATCH 06/11] chore(sonar): Introduce a custom RuntimeException ORSGraphFileManagerExceptionException.java. java:S112. --- .../ORSGraphFileManagerExceptionException.java | 7 +++++++ .../extensions/manage/GraphInfo.java | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerExceptionException.java diff --git a/ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerExceptionException.java b/ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerExceptionException.java new file mode 100644 index 0000000000..2fc9d1fc30 --- /dev/null +++ b/ors-engine/src/main/java/org/heigit/ors/exceptions/ORSGraphFileManagerExceptionException.java @@ -0,0 +1,7 @@ +package org.heigit.ors.exceptions; + +public class ORSGraphFileManagerExceptionException extends RuntimeException { + public ORSGraphFileManagerExceptionException(String message, Throwable cause) { + super(message, cause); + } +} \ No newline at end of file diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java index 09d45ea399..febd5edfeb 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java @@ -1,31 +1,38 @@ package org.heigit.ors.routing.graphhopper.extensions.manage; +import lombok.AllArgsConstructor; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; import lombok.experimental.Accessors; +import org.apache.log4j.Logger; +import org.heigit.ors.exceptions.ORSGraphFileManagerExceptionException; import java.io.File; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; -@Accessors(chain = true) -@Getter -@Setter @NoArgsConstructor +@AllArgsConstructor +@Setter +@Accessors(chain = true) public class GraphInfo { - private URI remoteUri = null; + private URI remoteUri; + @Getter private File localDirectory = null; + Logger logger = Logger.getLogger(GraphInfo.class.getName()); + @Getter private PersistedGraphInfo persistedGraphInfo; public GraphInfo withRemoteUrl(URL url) { try { this.remoteUri = url.toURI(); } catch (URISyntaxException e) { - throw new RuntimeException(e); + logger.error("Error while parsing remote URL %s with message %s".formatted(url, e.getMessage())); + throw new ORSGraphFileManagerExceptionException("Error while parsing remote URL %s.".formatted(url), e); } return this; } From f9d9f994157a4996b947094a866954af465792a0 Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 16:52:34 +0100 Subject: [PATCH 07/11] fix: Only print if something was deleted --- .../extensions/manage/local/ORSGraphFileManager.java | 4 ++-- .../graphhopper/extensions/manage/remote/HttpRepoManager.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java index 74d909488c..b192ceabc0 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/local/ORSGraphFileManager.java @@ -95,8 +95,8 @@ public boolean isBusy() { private void deleteFileWithLogging(File file, String successMessage, String errorMessage) { try { - Files.deleteIfExists(file.toPath()); - LOGGER.debug(successMessage.formatted(getProfileDescriptiveName(), file.getAbsolutePath())); + if (Files.deleteIfExists(file.toPath())) + LOGGER.debug(successMessage.formatted(getProfileDescriptiveName(), file.getAbsolutePath())); } catch (IOException e) { LOGGER.error(errorMessage.formatted(e.getMessage())); } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java index 721ef921a1..d8fd033735 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManager.java @@ -66,8 +66,8 @@ public URL createDownloadUrl(String fileName) { private void deleteFileWithLogging(File file, String successMessage, String errorMessage) { try { - Files.deleteIfExists(file.toPath()); - LOGGER.debug(successMessage.formatted(getProfileDescriptiveName(), file.getAbsolutePath())); + if (Files.deleteIfExists(file.toPath())) + LOGGER.debug(successMessage.formatted(getProfileDescriptiveName(), file.getAbsolutePath())); } catch (IOException e) { LOGGER.error(errorMessage.formatted(e.getMessage())); } From 21c235e0451fde5760be838dd44b5afc2b822dba Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 17:00:24 +0100 Subject: [PATCH 08/11] fix: Catch the null variables and catch the missed download --- .../manage/remote/FileSystemRepoManager.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java index 3f49d8534e..ded7edc338 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManager.java @@ -83,7 +83,11 @@ public void downloadGraphIfNecessary() { downloadFile(latestCompressedGraphInRepoPath, downloadedCompressedGraphFile); long end = System.currentTimeMillis(); - LOGGER.info("[%s] Download of compressed graph file finished after %d ms".formatted(getProfileDescriptiveName(), end - start)); + if (downloadedCompressedGraphFile.exists()) { + LOGGER.info("[%s] Download of compressed graph file finished after %d ms".formatted(getProfileDescriptiveName(), end - start)); + } else { + LOGGER.error("[%s] Invalid download path for compressed graph file: %s".formatted(getProfileDescriptiveName(), latestCompressedGraphInRepoPath)); + } } catch (Exception e) { LOGGER.error("[%s] Caught an exception during graph download check or graph download:".formatted(getProfileDescriptiveName()), e); } @@ -117,6 +121,10 @@ GraphInfo downloadLatestGraphInfoFromRepository() throws ORSGraphFileManagerExce } public void downloadFile(Path repoPath, File localPath) { + if (repoPath == null || localPath == null) { + LOGGER.warn("[%s] Invalid download or local path: %s or %s".formatted(getProfileDescriptiveName(), repoPath, localPath)); + return; + } if (LOGGER.isTraceEnabled()) { LOGGER.trace("[%s] Downloading %s to local file %s...".formatted(getProfileDescriptiveName(), repoPath.toFile().getAbsolutePath(), localPath.getAbsolutePath())); } else { From d67b8898f67bd4a419eff62129578d6e6a4539f7 Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 17:36:33 +0100 Subject: [PATCH 09/11] refactor: Remove redundant function and use java native ofNullable --- .../GraphManagementRuntimeProperties.java | 44 +++++++------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java index 8622b742d9..9f2334d447 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java @@ -12,8 +12,6 @@ import java.net.URL; import java.nio.file.Path; import java.util.Arrays; -import java.util.List; -import java.util.Optional; import static java.util.Optional.ofNullable; @@ -65,42 +63,30 @@ public static Builder empty() { public static Builder from(EngineProperties engineProperties, ProfileProperties profileProperties, String graphVersion) { Builder builder = new Builder(); builder.enabled = ofNullable(engineProperties).map(EngineProperties::getGraphManagement).map(GraphManagementProperties::getEnabled).orElse(false); - builder.repoBaseUri = getFirstPresentString(List.of( - ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri)) - ); - builder.repoName = getFirstPresentString(List.of( - ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName)) - ); - builder.repoCoverage = getFirstPresentString(List.of( - ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent)) - ); - builder.repoProfileGroup = getFirstPresentString(List.of( - ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup)) - ); + + builder.repoBaseUri = ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri).orElseGet(() -> + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri).orElse(null)); + + builder.repoName = ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName).orElseGet(() -> + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName).orElse(null)); + + builder.repoCoverage = ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent).orElseGet(() -> + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent).orElse(null)); + + builder.repoProfileGroup = ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup).orElseGet(() -> + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup).orElse(null)); builder.maxNumberOfGraphBackups = ofNullable(engineProperties).map(EngineProperties::getGraphManagement).map(GraphManagementProperties::getMaxBackups).orElse(0); builder.graphVersion = graphVersion; builder.localProfileName = ofNullable(profileProperties).map(ProfileProperties::getProfileName).map(String::valueOf).orElse(null); - builder.localGraphsRootAbsPath = getFirstPresentString(List.of( - ofNullable(profileProperties).map(ProfileProperties::getGraphPath).map(Path::toString), - ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getGraphPath).map(Path::toString)) - ); + + builder.localGraphsRootAbsPath = ofNullable(profileProperties).map(ProfileProperties::getGraphPath).map(Path::toString).orElseGet(() -> + ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getGraphPath).map(Path::toString).orElse(null)); builder.encoderName = ofNullable(profileProperties).map(ProfileProperties::getEncoderName).map(String::valueOf).orElse(null); return builder; } - private static String getFirstPresentString(List> objects) { - return objects.stream() - .flatMap(Optional::stream) - .map(String::valueOf) - .findFirst() - .orElse(null); - } public Builder withEnabled(boolean enabled) { this.enabled = enabled; From e203e4e2293986ea2a247b839de08498f7f122a1 Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 17:43:38 +0100 Subject: [PATCH 10/11] refactor: Streamline the guard check --- .../extensions/manage/GraphManagementRuntimeProperties.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java index 9f2334d447..8944ec89c1 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java @@ -12,6 +12,7 @@ import java.net.URL; import java.nio.file.Path; import java.util.Arrays; +import java.util.Objects; import static java.util.Optional.ofNullable; @@ -189,7 +190,8 @@ private boolean isSupportedUrlScheme(URI uri) { } private boolean isSupportedFileScheme(URI uri) { - return uri != null && "file".equals(uri.getScheme()); + if (uri == null) return false; + return Objects.equals("file", uri.getScheme()); } private URI toUri(String string) { From 9aab61d6cc08c02ff7e10c14275033a9810f3c9e Mon Sep 17 00:00:00 2001 From: Julian Psotta Date: Mon, 25 Nov 2024 18:34:40 +0100 Subject: [PATCH 11/11] refactor(idempotence): Make sure the test methods don't interact Before the methods shared global resources and depending on the parallel or sequential execution overwrite them in an unexpected way. --- .../manage/RepoManagerTestHelper.java | 16 +-- .../remote/FileSystemRepoManagerTest.java | 6 +- .../manage/remote/HttpRepoManagerTest.java | 112 ++++++++++-------- 3 files changed, 75 insertions(+), 59 deletions(-) diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java index 21f0e00cfe..ebe56ff97f 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/RepoManagerTestHelper.java @@ -30,17 +30,17 @@ public class RepoManagerTestHelper { public static final long MIDDLE_DATE = 1692373222000L; // Fr 18. Aug 17:40:22 CEST 2023 public static final long LATER_DATE = 1692373333000L; // Fr 18. Aug 17:42:13 CEST 2023 - public static long repoCarOsmDate; - public static long repoHgvOsmDate; - public static long repoCarGraphBuildDate; - public static long repoHgvGraphBuildDate; + public static final long REPO_CAR_OSM_DATE; + public static final long REPO_HGV_OSM_DATE; + public static final long REPO_CAR_GRAPH_BUILD_DATE; + public static final long REPO_HGV_GRAPH_BUILD_DATE; static { DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ"); - repoCarOsmDate = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); - repoHgvOsmDate = ZonedDateTime.parse("2024-01-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); - repoCarGraphBuildDate = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); - repoHgvGraphBuildDate = ZonedDateTime.parse("2024-06-26T10:23:39+0000", formatter).toInstant().toEpochMilli(); + REPO_CAR_OSM_DATE = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); + REPO_HGV_OSM_DATE = ZonedDateTime.parse("2024-01-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); + REPO_CAR_GRAPH_BUILD_DATE = ZonedDateTime.parse("2024-06-26T10:23:31+0000", formatter).toInstant().toEpochMilli(); + REPO_HGV_GRAPH_BUILD_DATE = ZonedDateTime.parse("2024-06-26T10:23:39+0000", formatter).toInstant().toEpochMilli(); } public static Path createLocalGraphsRootDirectory(Path tempDir) throws IOException { diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java index e7884fd627..1f1eb02300 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/FileSystemRepoManagerTest.java @@ -85,7 +85,7 @@ void downloadGraphIfNecessary_localDataExists_noRemoteData() { @Test void downloadGraphIfNecessary_localDate_before_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), repoHgvOsmDate - 1000000, repoHgvGraphBuildDate - 1000000); + saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), REPO_HGV_OSM_DATE - 1000000, REPO_HGV_GRAPH_BUILD_DATE - 1000000); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.ghz").toFile().exists()); fileSystemRepoManager.downloadGraphIfNecessary(); @@ -96,7 +96,7 @@ void downloadGraphIfNecessary_localDate_before_remoteDate() { @Test void downloadGraphIfNecessary_localDate_equals_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), repoHgvGraphBuildDate, repoHgvOsmDate); + saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), REPO_HGV_GRAPH_BUILD_DATE, REPO_HGV_OSM_DATE); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.ghz").toFile().exists()); fileSystemRepoManager.downloadGraphIfNecessary(); @@ -107,7 +107,7 @@ void downloadGraphIfNecessary_localDate_equals_remoteDate() { @Test void downloadGraphIfNecessary_localDate_after_remoteDate() { setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), repoHgvGraphBuildDate + 1000000, repoHgvOsmDate + 1000000); + saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), REPO_HGV_GRAPH_BUILD_DATE + 1000000, REPO_HGV_OSM_DATE + 1000000); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.yml").toFile().exists()); assertFalse(localGraphsRootPath.resolve("vendor-xyz_fastisochrones_heidelberg_1_driving-hgv.ghz").toFile().exists()); fileSystemRepoManager.downloadGraphIfNecessary(); diff --git a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java index 1c870f90f5..8efa6d85d3 100644 --- a/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java +++ b/ors-engine/src/test/java/org/heigit/ors/routing/graphhopper/extensions/manage/remote/HttpRepoManagerTest.java @@ -1,5 +1,7 @@ package org.heigit.ors.routing.graphhopper.extensions.manage.remote; +import lombok.AllArgsConstructor; +import lombok.Getter; import org.apache.commons.io.FileUtils; import org.assertj.core.util.Files; import org.heigit.ors.routing.graphhopper.extensions.manage.GraphManagementRuntimeProperties; @@ -9,6 +11,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.CleanupMode; import org.junit.jupiter.api.io.TempDir; @@ -24,6 +27,7 @@ import java.io.IOException; import java.net.URL; import java.nio.file.Path; +import java.util.Objects; import static org.apache.commons.io.FileUtils.readFileToString; import static org.heigit.ors.routing.graphhopper.extensions.manage.RepoManagerTestHelper.*; @@ -31,33 +35,29 @@ @Testcontainers @ExtendWith(TestcontainersExtension.class) +@TestInstance(TestInstance.Lifecycle.PER_METHOD) class HttpRepoManagerTest { private static final String LOCAL_PROFILE_NAME = "driving-car"; private static final String ENCODER_NAME = "driving-car"; - @TempDir(cleanup = CleanupMode.ALWAYS) - private static Path tempDir; @Container - private static NginxContainer nginx; - private static String nginxUrl; + private static final NginxContainer NGINX; + private static final String NGINX_URL; private static Path localGraphsRootPath; - private HttpRepoManager orsGraphRepoManager; - private ORSGraphFileManager orsGraphFileManager; - static { DockerImageName dockerImageName = DockerImageName.parse("nginx:1.23.4-alpine"); MountableFile mountableFile = MountableFile.forHostPath("src/test/resources/test-filesystem-repos/"); - nginx = new NginxContainer<>(dockerImageName) + NGINX = new NginxContainer<>(dockerImageName) .withCopyFileToContainer(mountableFile, "/usr/share/nginx/html") .waitingFor(new HttpWaitStrategy()); - nginx.start(); - nginxUrl = "http://" + nginx.getHost() + ":" + nginx.getFirstMappedPort() + "/"; + NGINX.start(); + NGINX_URL = "http://" + NGINX.getHost() + ":" + NGINX.getFirstMappedPort() + "/"; } @BeforeEach - void setUp() throws IOException { + void setUp(@TempDir(cleanup = CleanupMode.ALWAYS) Path tempDir) throws IOException { localGraphsRootPath = createLocalGraphsRootDirectory(tempDir); createLocalGraphDirectory(localGraphsRootPath, LOCAL_PROFILE_NAME); } @@ -68,20 +68,41 @@ void deleteFiles() throws IOException { cleanupLocalGraphsRootDirectory(localGraphsRootPath); } - private void setupORSGraphManager(GraphManagementRuntimeProperties managementProps) { + + @AllArgsConstructor + @Getter + static class OrsGraphHelper { + ORSGraphFileManager orsGraphFileManager; + ORSGraphRepoManager orsGraphRepoManager; + + } + + private OrsGraphHelper setupOrsGraphHelper(GraphManagementRuntimeProperties graphManagementRuntimeProperties, Long timeVariable) { + ORSGraphFileManager orsGraphFileManager = setupORSGraphFileManager(graphManagementRuntimeProperties); + if (timeVariable != null) + setupActiveGraphDirectory(timeVariable, orsGraphFileManager); + ORSGraphRepoManager orsGraphRepoManager = setupOrsGraphRepoManager(graphManagementRuntimeProperties, orsGraphFileManager); + return new OrsGraphHelper(orsGraphFileManager, orsGraphRepoManager); + } + + private ORSGraphFileManager setupORSGraphFileManager(GraphManagementRuntimeProperties managementProps) { ORSGraphFolderStrategy orsGraphFolderStrategy = new FlatORSGraphFolderStrategy(managementProps); - orsGraphFileManager = new ORSGraphFileManager(managementProps, orsGraphFolderStrategy); + ORSGraphFileManager orsGraphFileManager = new ORSGraphFileManager(managementProps, orsGraphFolderStrategy); orsGraphFileManager.initialize(); + return orsGraphFileManager; + } + + private ORSGraphRepoManager setupOrsGraphRepoManager(GraphManagementRuntimeProperties managementProps, ORSGraphFileManager orsGraphFileManager) { ORSGraphRepoStrategy repoStrategy = new NamedGraphsRepoStrategy(managementProps); - orsGraphRepoManager = new HttpRepoManager(managementProps, repoStrategy, orsGraphFileManager); + return new HttpRepoManager(managementProps, repoStrategy, orsGraphFileManager); } private static GraphManagementRuntimeProperties.Builder managementPropsBuilder() { return createGraphManagementRuntimePropertiesBuilder(localGraphsRootPath, LOCAL_PROFILE_NAME, ENCODER_NAME) - .withRepoBaseUri(nginxUrl); + .withRepoBaseUri(NGINX_URL); } - private void setupActiveGraphDirectory(Long osmDateLocal) { + private void setupActiveGraphDirectory(Long osmDateLocal, ORSGraphFileManager orsGraphFileManager) { saveActiveGraphInfoFile(orsGraphFileManager.getActiveGraphInfoFile(), osmDateLocal, null); } @@ -115,7 +136,7 @@ void createDownloadUrl() { .build(); FlatORSGraphFolderStrategy orsGraphFolderStrategy = new FlatORSGraphFolderStrategy(managementProps); - orsGraphFileManager = new ORSGraphFileManager(managementProps, orsGraphFolderStrategy); + ORSGraphFileManager orsGraphFileManager = new ORSGraphFileManager(managementProps, orsGraphFolderStrategy); ORSGraphRepoStrategy orsGraphRepoStrategy = new NamedGraphsRepoStrategy(managementProps); HttpRepoManager httpRepoManager = new HttpRepoManager(managementProps, orsGraphRepoStrategy, orsGraphFileManager); URL downloadUrl = httpRepoManager.createDownloadUrl("graph.ghz"); @@ -126,8 +147,8 @@ void createDownloadUrl() { void checkNginx() throws IOException { Path path = Path.of("src/test/resources/test-filesystem-repos"); printValue("path to test-filesystem-repos", path.toAbsolutePath()); - assertNotNull(this.nginxUrl); - URL url = new URL(this.nginxUrl + "vendor-xyz/fastisochrones/heidelberg/1/fastisochrones_heidelberg_1_driving-car.yml"); + assertNotNull(NGINX_URL); + URL url = new URL(NGINX_URL + "vendor-xyz/fastisochrones/heidelberg/1/fastisochrones_heidelberg_1_driving-car.yml"); printValue("fileUrl", url); File file = Files.newTemporaryFile(); assertEquals(0, file.length()); @@ -138,14 +159,13 @@ void checkNginx() throws IOException { @Test void checkSetupActiveGraphDirectory() throws IOException { - setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(EARLIER_DATE); - File activeGraphDirectory = orsGraphFileManager.getActiveGraphDirectory(); + OrsGraphHelper orsGraphHelper = setupOrsGraphHelper(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build(), EARLIER_DATE); + File activeGraphDirectory = orsGraphHelper.getOrsGraphFileManager().getActiveGraphDirectory(); printValue("content of activeGraphDirectory:"); - for (File file : activeGraphDirectory.listFiles()) { + for (File file : Objects.requireNonNull(activeGraphDirectory.listFiles())) { printValue(file.getAbsolutePath()); } - File activeGraphInfoFile = orsGraphFileManager.getActiveGraphInfoFile(); + File activeGraphInfoFile = orsGraphHelper.getOrsGraphFileManager().getActiveGraphInfoFile(); assertTrue(activeGraphInfoFile.exists()); printFileContent("activeGraphInfoFile", activeGraphInfoFile); String content = readFileToString(activeGraphInfoFile, "UTF-8"); @@ -154,64 +174,60 @@ void checkSetupActiveGraphDirectory() throws IOException { @Test void downloadGraphIfNecessary_noDownloadWhen_localDataExists_noRemoteData() { - setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_NONEXISTING_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(EARLIER_DATE); + OrsGraphHelper orsGraphHelper = setupOrsGraphHelper(managementPropsBuilder().withGraphVersion(REPO_NONEXISTING_GRAPHS_VERSION).build(), EARLIER_DATE); - orsGraphRepoManager.downloadGraphIfNecessary(); + orsGraphHelper.getOrsGraphRepoManager().downloadGraphIfNecessary(); - File downloadedGraphInfoFile = orsGraphFileManager.getDownloadedGraphInfoFile(); - File downloadedCompressedGraphFile = orsGraphFileManager.getDownloadedCompressedGraphFile(); + File downloadedGraphInfoFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedGraphInfoFile(); + File downloadedCompressedGraphFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedCompressedGraphFile(); assertFalse(downloadedGraphInfoFile.exists()); assertFalse(downloadedCompressedGraphFile.exists()); } @Test void downloadGraphIfNecessary_downloadWhen_noLocalData_remoteDataExists() { - setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); + OrsGraphHelper orsGraphHelper = setupOrsGraphHelper(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build(), null); - orsGraphRepoManager.downloadGraphIfNecessary(); + orsGraphHelper.getOrsGraphRepoManager().downloadGraphIfNecessary(); - File downloadedGraphInfoFile = orsGraphFileManager.getDownloadedGraphInfoFile(); - File downloadedCompressedGraphFile = orsGraphFileManager.getDownloadedCompressedGraphFile(); + File downloadedGraphInfoFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedGraphInfoFile(); + File downloadedCompressedGraphFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedCompressedGraphFile(); assertTrue(downloadedGraphInfoFile.exists()); assertTrue(downloadedCompressedGraphFile.exists()); } @Test void downloadGraphIfNecessary_downloadWhen_localDate_before_remoteDate() { - setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(EARLIER_DATE); + OrsGraphHelper orsGraphHelper = setupOrsGraphHelper(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build(), EARLIER_DATE); - orsGraphRepoManager.downloadGraphIfNecessary(); + orsGraphHelper.getOrsGraphRepoManager().downloadGraphIfNecessary(); - File downloadedGraphInfoFile = orsGraphFileManager.getDownloadedGraphInfoFile(); - File downloadedCompressedGraphFile = orsGraphFileManager.getDownloadedCompressedGraphFile(); + File downloadedGraphInfoFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedGraphInfoFile(); + File downloadedCompressedGraphFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedCompressedGraphFile(); assertTrue(downloadedGraphInfoFile.exists()); assertTrue(downloadedCompressedGraphFile.exists()); } @Test void downloadGraphIfNecessary_noDownloadWhen_localDate_equals_remoteDate() { - setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(repoCarGraphBuildDate); + OrsGraphHelper orsGraphHelper = setupOrsGraphHelper(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build(), REPO_CAR_GRAPH_BUILD_DATE); - orsGraphRepoManager.downloadGraphIfNecessary(); + orsGraphHelper.getOrsGraphRepoManager().downloadGraphIfNecessary(); - File downloadedGraphInfoFile = orsGraphFileManager.getDownloadedGraphInfoFile(); - File downloadedCompressedGraphFile = orsGraphFileManager.getDownloadedCompressedGraphFile(); + File downloadedGraphInfoFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedGraphInfoFile(); + File downloadedCompressedGraphFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedCompressedGraphFile(); assertTrue(downloadedGraphInfoFile.exists()); assertFalse(downloadedCompressedGraphFile.exists()); } @Test void downloadGraphIfNecessary_noDownloadWhen_localDate_after_remoteDate() { - setupORSGraphManager(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build()); - setupActiveGraphDirectory(repoCarGraphBuildDate + 1000000); + OrsGraphHelper orsGraphHelper = setupOrsGraphHelper(managementPropsBuilder().withGraphVersion(REPO_GRAPHS_VERSION).build(), REPO_CAR_GRAPH_BUILD_DATE + 1000000); - orsGraphRepoManager.downloadGraphIfNecessary(); + orsGraphHelper.getOrsGraphRepoManager().downloadGraphIfNecessary(); - File downloadedGraphInfoFile = orsGraphFileManager.getDownloadedGraphInfoFile(); - File downloadedCompressedGraphFile = orsGraphFileManager.getDownloadedCompressedGraphFile(); + File downloadedGraphInfoFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedGraphInfoFile(); + File downloadedCompressedGraphFile = orsGraphHelper.getOrsGraphFileManager().getDownloadedCompressedGraphFile(); assertTrue(downloadedGraphInfoFile.exists()); assertFalse(downloadedCompressedGraphFile.exists()); }