chore: cleanup the graph repo classes

ors-api/src/main/java/org/heigit/ors/api/services/GraphService.java

@@ -24,15 +24,19 @@ 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());
 
-    private List<ORSGraphManager> graphManagers = new ArrayList<>();
+    private final List<ORSGraphManager> graphManagers = new ArrayList<>();
+
+    private final AtomicBoolean graphActivationAttemptWasBlocked = new AtomicBoolean(false);
+    private final AtomicBoolean isActivatingGraphs = new AtomicBoolean(true);
 
-    private AtomicBoolean graphActivationAttemptWasBlocked = new AtomicBoolean(false);
-    private AtomicBoolean isActivatingGraphs = new AtomicBoolean(true);
+    @Autowired
+    public GraphService(EngineProperties engineProperties) {
+        this.engineProperties = engineProperties;
+    }
 
     public void addGraphManagerInstance(ORSGraphManager orsGraphManager) {
         if (orsGraphManager.useGraphRepository()) {
@@ -48,7 +52,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 +91,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;
         }
@@ -104,21 +108,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;
             }
         }
@@ -144,10 +138,31 @@ 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() {
-        if (!enabled) {
+        if (Boolean.FALSE.equals(enabled)) {
             LOGGER.debug("Graph management is disabled, skipping repeated attempt to activate graphs...");
             return;
         }

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);

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");

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;
@@ -13,11 +14,11 @@
 
 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
-    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();

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);
+    }
+}

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);
+    }
+}

ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphInfo.java

@@ -1,53 +1,39 @@
 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;
-import java.util.Objects;
 
-@Getter
+@NoArgsConstructor
+@AllArgsConstructor
+@Setter
+@Accessors(chain = true)
 public class GraphInfo {
 
-    public 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 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();
         } 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;
     }
-
-    public GraphInfo withLocalDirectory(File directory) {
-        this.localDirectory = directory;
-        return this;
-    }
-
-    public GraphInfo withPersistedInfo(PersistedGraphInfo persistedGraphInfo) {
-        this.persistedGraphInfo = persistedGraphInfo;
-        return this;
-    }
 }

ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/GraphManagementRuntimeProperties.java

@@ -12,7 +12,7 @@
 import java.net.URL;
 import java.nio.file.Path;
 import java.util.Arrays;
-import java.util.Optional;
+import java.util.Objects;
 
 import static java.util.Optional.ofNullable;
 
@@ -64,31 +64,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(
-                    ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri),
-                    ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryUri));
-            builder.repoName = getFirstPresentString(
-                    ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName),
-                    ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryName));
-            builder.repoCoverage = getFirstPresentString(
-                    ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent),
-                    ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getGraphExtent));
-            builder.repoProfileGroup = getFirstPresentString(
-                    ofNullable(profileProperties).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup),
-                    ofNullable(engineProperties).map(EngineProperties::getProfileDefault).map(ProfileProperties::getRepo).map(RepoProperties::getRepositoryProfileGroup));
-            builder.maxNumberOfGraphBackups = engineProperties.getGraphManagement().getMaxBackups();
+
+            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(
-                    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(Optional... objects) {
-            return Arrays.stream(objects).filter(Optional::isPresent).findFirst().map(Optional::get).map(String::valueOf).orElse(null);
-        }
 
         public Builder withEnabled(boolean enabled) {
             this.enabled = enabled;
@@ -192,7 +191,7 @@ private boolean isSupportedUrlScheme(URI uri) {
 
     private boolean isSupportedFileScheme(URI uri) {
         if (uri == null) return false;
-        return Arrays.asList("file").contains(uri.getScheme());
+        return Objects.equals("file", uri.getScheme());
     }
 
     private URI toUri(String string) {

ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/manage/ORSGraphManager.java

@@ -1,9 +1,12 @@
 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;
 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;
@@ -14,6 +17,8 @@
 
 import static java.util.Optional.ofNullable;
 
+@NoArgsConstructor
+@AllArgsConstructor
 public class ORSGraphManager {
 
     private static final Logger LOGGER = Logger.getLogger(ORSGraphManager.class.getName());
@@ -24,19 +29,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) {
@@ -73,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;
     }
@@ -161,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)