From 99907d19eac2fc3d9e8d97b65f2e97ecdc78fdad Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Wed, 11 Dec 2024 12:10:43 +0100 Subject: [PATCH 1/9] refactor: remove unnecessary parameter This commit removes the second parameter `travellers` from method `IsochronesService.validateAgainstConfig(...)` This parameter is unnecessary because at the the only place, where this method is called, `travellers` is extracted from `isochroneRequest` just before the call. Hence this extraction can happen inside `validateAgainstConfig`. --- .../java/org/heigit/ors/api/services/IsochronesService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java index 3e4269039a..bec7827986 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java @@ -52,7 +52,7 @@ public void generateIsochronesFromRequest(IsochronesRequest isochronesRequest) t List travellers = isochronesRequest.getIsochroneRequest().getTravellers(); // TODO REFACTORING where should we put the validation code? - validateAgainstConfig(isochronesRequest.getIsochroneRequest(), travellers); + validateAgainstConfig(isochronesRequest.getIsochroneRequest()); if (!travellers.isEmpty()) { isochronesRequest.setIsoMaps(new IsochroneMapCollection()); @@ -288,7 +288,8 @@ RouteSearchParameters processIsochronesRequestOptions(IsochronesRequest isochron return parameters; } - void validateAgainstConfig(IsochroneRequest isochroneRequest, List travellers) throws StatusCodeException { + void validateAgainstConfig(IsochroneRequest isochroneRequest) throws StatusCodeException { + List travellers = isochroneRequest.getTravellers(); if (!isochroneRequest.isAllowComputeArea() && isochroneRequest.hasAttribute("area")) throw new StatusCodeException(StatusCode.BAD_REQUEST, IsochronesErrorCodes.FEATURE_NOT_SUPPORTED, "Area computation is not enabled."); From 5c848bb6a48ca80b0da9e426d7e50a9acd278117 Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Wed, 11 Dec 2024 13:18:09 +0100 Subject: [PATCH 2/9] refactor: reduce unnecessary usage of getters Reduce the usage of `getTravellers()` where it is unnecessary. --- .../org/heigit/ors/api/services/IsochronesService.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java index bec7827986..80d785a513 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java @@ -47,18 +47,19 @@ public IsochronesService(EndpointsProperties endpointsProperties, ApiEnginePrope } public void generateIsochronesFromRequest(IsochronesRequest isochronesRequest) throws Exception { - isochronesRequest.setIsochroneRequest(convertIsochroneRequest(isochronesRequest)); + IsochroneRequest isochroneRequest = convertIsochroneRequest(isochronesRequest); + isochronesRequest.setIsochroneRequest(isochroneRequest); // request object is built, now check if ors config allows all settings - List travellers = isochronesRequest.getIsochroneRequest().getTravellers(); + List travellers = isochroneRequest.getTravellers(); // TODO REFACTORING where should we put the validation code? - validateAgainstConfig(isochronesRequest.getIsochroneRequest()); + validateAgainstConfig(isochroneRequest); if (!travellers.isEmpty()) { isochronesRequest.setIsoMaps(new IsochroneMapCollection()); for (int i = 0; i < travellers.size(); ++i) { - IsochroneSearchParameters searchParams = isochronesRequest.getIsochroneRequest().getSearchParameters(i); + IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); IsochroneMap isochroneMap = RoutingProfileManager.getInstance().buildIsochrone(searchParams); isochronesRequest.getIsoMaps().add(isochroneMap); } From 8dae0ce9b55051869b18cc0e62f17ae26b49c0c2 Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Wed, 11 Dec 2024 15:03:11 +0100 Subject: [PATCH 3/9] refactor: decouple isochrone computation from request Instead of injecting partial responses into the `isochronesRequest` on the fly, this commit first computes the whole response and injects it afterwards. This is a preparatory step to move the computation into `buildIsochrone`. --- .../org/heigit/ors/api/services/IsochronesService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java index 80d785a513..1e17ff5b8c 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java @@ -56,14 +56,14 @@ public void generateIsochronesFromRequest(IsochronesRequest isochronesRequest) t validateAgainstConfig(isochroneRequest); if (!travellers.isEmpty()) { - isochronesRequest.setIsoMaps(new IsochroneMapCollection()); - + IsochroneMapCollection isoMaps = new IsochroneMapCollection(); for (int i = 0; i < travellers.size(); ++i) { IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); IsochroneMap isochroneMap = RoutingProfileManager.getInstance().buildIsochrone(searchParams); - isochronesRequest.getIsoMaps().add(isochroneMap); + isoMaps.add(isochroneMap); } - + // TODO: is this necessary? It seems unusual to transport the response through the request object + isochronesRequest.setIsoMaps(isoMaps); } } From 1e6db5d0d8c323c61f70227e1500226ad668db0f Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Thu, 12 Dec 2024 09:18:37 +0100 Subject: [PATCH 4/9] refactor: extract isochrones computation as method --- .../ors/api/services/IsochronesService.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java index 1e17ff5b8c..d385b30e83 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java @@ -56,17 +56,22 @@ public void generateIsochronesFromRequest(IsochronesRequest isochronesRequest) t validateAgainstConfig(isochroneRequest); if (!travellers.isEmpty()) { - IsochroneMapCollection isoMaps = new IsochroneMapCollection(); - for (int i = 0; i < travellers.size(); ++i) { - IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); - IsochroneMap isochroneMap = RoutingProfileManager.getInstance().buildIsochrone(searchParams); - isoMaps.add(isochroneMap); - } + IsochroneMapCollection isoMaps = computeIsochrones(isochroneRequest); // TODO: is this necessary? It seems unusual to transport the response through the request object isochronesRequest.setIsoMaps(isoMaps); } } + private static IsochroneMapCollection computeIsochrones(IsochroneRequest isochroneRequest) throws Exception { + IsochroneMapCollection isoMaps = new IsochroneMapCollection(); + for (int i = 0; i < isochroneRequest.getTravellers().size(); ++i) { + IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); + IsochroneMap isochroneMap = RoutingProfileManager.getInstance().buildIsochrone(searchParams); + isoMaps.add(isochroneMap); + } + return isoMaps; + } + Float convertSmoothing(Double smoothingValue) throws ParameterValueException { float f = (float) smoothingValue.doubleValue(); From 225859b88baa149eca75d3bc0bd68b8456bbdb97 Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Thu, 12 Dec 2024 09:42:49 +0100 Subject: [PATCH 5/9] refactor: move method computeIsochrones This commit moves the method `computeIsochrones` from `IsochronesService` into `RoutingProfileManager`and merges it with `buildIsochrones` by inlining. This is a preparatory step to move the for-loop of this method into `RoutingProfile.buildIsochrone`, which should then be moved into `IsochroneRequest`. --- .../ors/api/services/IsochronesService.java | 12 +--------- .../ors/routing/RoutingProfileManager.java | 22 ++++++++++--------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java index d385b30e83..9948d2c1c4 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java @@ -56,22 +56,12 @@ public void generateIsochronesFromRequest(IsochronesRequest isochronesRequest) t validateAgainstConfig(isochroneRequest); if (!travellers.isEmpty()) { - IsochroneMapCollection isoMaps = computeIsochrones(isochroneRequest); + IsochroneMapCollection isoMaps = RoutingProfileManager.getInstance().computeIsochrones(isochroneRequest); // TODO: is this necessary? It seems unusual to transport the response through the request object isochronesRequest.setIsoMaps(isoMaps); } } - private static IsochroneMapCollection computeIsochrones(IsochroneRequest isochroneRequest) throws Exception { - IsochroneMapCollection isoMaps = new IsochroneMapCollection(); - for (int i = 0; i < isochroneRequest.getTravellers().size(); ++i) { - IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); - IsochroneMap isochroneMap = RoutingProfileManager.getInstance().buildIsochrone(searchParams); - isoMaps.add(isochroneMap); - } - return isoMaps; - } - Float convertSmoothing(Double smoothingValue) throws ParameterValueException { float f = (float) smoothingValue.doubleValue(); diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java index 5233fcf52f..97c6903241 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java @@ -25,6 +25,8 @@ import org.heigit.ors.config.profile.ProfileProperties; import org.heigit.ors.exceptions.*; import org.heigit.ors.isochrones.IsochroneMap; +import org.heigit.ors.isochrones.IsochroneMapCollection; +import org.heigit.ors.isochrones.IsochroneRequest; import org.heigit.ors.isochrones.IsochroneSearchParameters; import org.heigit.ors.routing.pathprocessors.ExtraInfoProcessor; import org.heigit.ors.util.FormatUtility; @@ -583,16 +585,16 @@ public RoutingProfile getRouteProfileForRequest(RoutingRequest req, boolean oneT return rp; } - /** - * This function sends the {@link IsochroneSearchParameters} together with the Attributes to the {@link RoutingProfile}. - * - * @param parameters The input is a {@link IsochroneSearchParameters} - * @return Return is a {@link IsochroneMap} holding the calculated data plus statistical data if the attributes where set. - * @throws Exception - */ - public IsochroneMap buildIsochrone(IsochroneSearchParameters parameters) throws Exception { - RoutingProfile rp = getRoutingProfile(parameters.getRouteParameters().getProfileName()); - return rp.buildIsochrone(parameters); + + public IsochroneMapCollection computeIsochrones(IsochroneRequest isochroneRequest) throws Exception { + IsochroneMapCollection isoMaps = new IsochroneMapCollection(); + for (int i = 0; i < isochroneRequest.getTravellers().size(); ++i) { + IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); + RoutingProfile rp = getRoutingProfile(searchParams.getRouteParameters().getProfileName()); + IsochroneMap isochroneMap = rp.buildIsochrone(searchParams); + isoMaps.add(isochroneMap); + } + return isoMaps; } } From 13a756d2aacd51472aff06a9fd71d2b5d1ccda87 Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Thu, 12 Dec 2024 09:49:50 +0100 Subject: [PATCH 6/9] refactor: move method computeIsochrones This commit moves the method `computeIsochrones` from `RoutingProfileManager` into `IsochroneRequest`. --- .../ors/api/services/IsochronesService.java | 2 +- .../heigit/ors/isochrones/IsochroneRequest.java | 13 +++++++++++++ .../heigit/ors/routing/RoutingProfileManager.java | 15 --------------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java index 9948d2c1c4..e36bbeda01 100644 --- a/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java +++ b/ors-api/src/main/java/org/heigit/ors/api/services/IsochronesService.java @@ -56,7 +56,7 @@ public void generateIsochronesFromRequest(IsochronesRequest isochronesRequest) t validateAgainstConfig(isochroneRequest); if (!travellers.isEmpty()) { - IsochroneMapCollection isoMaps = RoutingProfileManager.getInstance().computeIsochrones(isochroneRequest); + IsochroneMapCollection isoMaps = isochroneRequest.computeIsochrones(RoutingProfileManager.getInstance()); // TODO: is this necessary? It seems unusual to transport the response through the request object isochronesRequest.setIsoMaps(isoMaps); } diff --git a/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java b/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java index 7a0bca08d2..ef35270cdc 100644 --- a/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java +++ b/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java @@ -17,6 +17,8 @@ import org.heigit.ors.common.TravelRangeType; import org.heigit.ors.common.TravellerInfo; import org.heigit.ors.isochrones.statistics.StatisticsProviderConfiguration; +import org.heigit.ors.routing.RoutingProfile; +import org.heigit.ors.routing.RoutingProfileManager; import org.heigit.ors.routing.RoutingProfileType; import org.heigit.ors.routing.WeightingMethod; import org.locationtech.jts.geom.Coordinate; @@ -282,4 +284,15 @@ public Map getStatsProviders() { public void setStatsProviders(Map statsProviders) { this.statsProviders = statsProviders; } + + public IsochroneMapCollection computeIsochrones(RoutingProfileManager routingProfileManager) throws Exception { + IsochroneMapCollection isoMaps = new IsochroneMapCollection(); + for (int i = 0; i < getTravellers().size(); ++i) { + IsochroneSearchParameters searchParams = getSearchParameters(i); + RoutingProfile rp = routingProfileManager.getRoutingProfile(searchParams.getRouteParameters().getProfileName()); + IsochroneMap isochroneMap = rp.buildIsochrone(searchParams); + isoMaps.add(isochroneMap); + } + return isoMaps; + } } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java index 97c6903241..873348789c 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfileManager.java @@ -24,10 +24,6 @@ import org.heigit.ors.config.EngineProperties; import org.heigit.ors.config.profile.ProfileProperties; import org.heigit.ors.exceptions.*; -import org.heigit.ors.isochrones.IsochroneMap; -import org.heigit.ors.isochrones.IsochroneMapCollection; -import org.heigit.ors.isochrones.IsochroneRequest; -import org.heigit.ors.isochrones.IsochroneSearchParameters; import org.heigit.ors.routing.pathprocessors.ExtraInfoProcessor; import org.heigit.ors.util.FormatUtility; import org.heigit.ors.util.RuntimeUtility; @@ -586,15 +582,4 @@ public RoutingProfile getRouteProfileForRequest(RoutingRequest req, boolean oneT } - public IsochroneMapCollection computeIsochrones(IsochroneRequest isochroneRequest) throws Exception { - IsochroneMapCollection isoMaps = new IsochroneMapCollection(); - for (int i = 0; i < isochroneRequest.getTravellers().size(); ++i) { - IsochroneSearchParameters searchParams = isochroneRequest.getSearchParameters(i); - RoutingProfile rp = getRoutingProfile(searchParams.getRouteParameters().getProfileName()); - IsochroneMap isochroneMap = rp.buildIsochrone(searchParams); - isoMaps.add(isochroneMap); - } - return isoMaps; - } - } From 17403b5ec4bff752f4fa4188d4859dd422cff00a Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Thu, 12 Dec 2024 11:32:03 +0100 Subject: [PATCH 7/9] refactor: move buildIsochrone from RoutingProfile to IsochroneRequest --- .../ors/isochrones/IsochroneRequest.java | 88 +++++++++++++++++-- .../heigit/ors/routing/RoutingProfile.java | 76 ---------------- 2 files changed, 83 insertions(+), 81 deletions(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java b/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java index ef35270cdc..e64819f645 100644 --- a/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java +++ b/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java @@ -13,19 +13,23 @@ */ package org.heigit.ors.isochrones; +import org.apache.log4j.Logger; import org.heigit.ors.common.ServiceRequest; import org.heigit.ors.common.TravelRangeType; import org.heigit.ors.common.TravellerInfo; +import org.heigit.ors.exceptions.InternalServerException; +import org.heigit.ors.isochrones.statistics.StatisticsProvider; import org.heigit.ors.isochrones.statistics.StatisticsProviderConfiguration; -import org.heigit.ors.routing.RoutingProfile; -import org.heigit.ors.routing.RoutingProfileManager; -import org.heigit.ors.routing.RoutingProfileType; -import org.heigit.ors.routing.WeightingMethod; +import org.heigit.ors.isochrones.statistics.StatisticsProviderFactory; +import org.heigit.ors.routing.*; +import org.heigit.ors.util.DebugUtility; +import org.heigit.ors.util.ProfileTools; import org.locationtech.jts.geom.Coordinate; import java.util.*; public class IsochroneRequest extends ServiceRequest { + public static final Logger LOGGER = Logger.getLogger(IsochroneRequest.class); private String profileName; private final List travellers; private String calcMethod; @@ -290,9 +294,83 @@ public IsochroneMapCollection computeIsochrones(RoutingProfileManager routingPro for (int i = 0; i < getTravellers().size(); ++i) { IsochroneSearchParameters searchParams = getSearchParameters(i); RoutingProfile rp = routingProfileManager.getRoutingProfile(searchParams.getRouteParameters().getProfileName()); - IsochroneMap isochroneMap = rp.buildIsochrone(searchParams); + IsochroneMap isochroneMap = buildIsochrone(searchParams, rp); isoMaps.add(isochroneMap); } return isoMaps; } + + /** + * This function creates the actual {@link IsochroneMap}. + * So the first step in the function is a checkup on that. + * + * @param parameters The input are {@link IsochroneSearchParameters} + * @param routingProfile + * @return The return will be an {@link IsochroneMap} + * @throws Exception + */ + public IsochroneMap buildIsochrone(IsochroneSearchParameters parameters, RoutingProfile routingProfile) throws Exception { + // TODO: refactor buildIsochrone as to not need to pass the SearchParameters as they are already present + // in IsochroneRequest. maybe merge with computeIsochrones + IsochroneMap result; + + try { + RouteSearchContext searchCntx = routingProfile.createSearchContext(parameters.getRouteParameters()); + IsochroneMapBuilderFactory isochroneMapBuilderFactory = new IsochroneMapBuilderFactory(searchCntx); + result = isochroneMapBuilderFactory.buildMap(parameters); + } catch (Exception ex) { + if (DebugUtility.isDebug()) { + LOGGER.error(ex); + } + throw new InternalServerException(IsochronesErrorCodes.UNKNOWN, "Unable to build an isochrone map."); + } + + if (result.getIsochronesCount() > 0) { + if (parameters.hasAttribute(ProfileTools.KEY_TOTAL_POP)) { + try { + Map> mapProviderToAttrs = new HashMap<>(); + StatisticsProviderConfiguration provConfig = parameters.getStatsProviders().get(ProfileTools.KEY_TOTAL_POP); + if (provConfig != null) { + List attrList = new ArrayList<>(); + attrList.add(ProfileTools.KEY_TOTAL_POP); + mapProviderToAttrs.put(provConfig, attrList); + } + for (Map.Entry> entry : mapProviderToAttrs.entrySet()) { + provConfig = entry.getKey(); + StatisticsProvider provider = StatisticsProviderFactory.getProvider(provConfig.getName(), provConfig.getParameters()); + String[] provAttrs = provConfig.getMappedProperties(entry.getValue()); + + for (Isochrone isochrone : result.getIsochrones()) { + + double[] attrValues = provider.getStatistics(isochrone, provAttrs); + isochrone.setAttributes(entry.getValue(), attrValues, provConfig.getAttribution()); + + } + } + } catch (Exception ex) { + LOGGER.error(ex); + + throw new InternalServerException(IsochronesErrorCodes.UNKNOWN, "Unable to compute isochrone total_pop attribute."); + } + } + if (parameters.hasAttribute("reachfactor") || parameters.hasAttribute("area")) { + for (Isochrone isochrone : result.getIsochrones()) { + String units = parameters.getUnits(); + String areaUnits = parameters.getAreaUnits(); + if (areaUnits != null) units = areaUnits; + double area = isochrone.calcArea(units); + if (parameters.hasAttribute("area")) { + isochrone.setArea(area); + } + if (parameters.hasAttribute("reachfactor")) { + double reachfactor = isochrone.calcReachfactor(units); + // reach factor could be > 1, which would confuse people + reachfactor = (reachfactor > 1) ? 1 : reachfactor; + isochrone.setReachfactor(reachfactor); + } + } + } + } + return result; + } } diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java index e9c5aee894..87303f1103 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java @@ -35,10 +35,6 @@ import org.heigit.ors.config.profile.PreparationProperties; import org.heigit.ors.config.profile.ProfileProperties; import org.heigit.ors.exceptions.InternalServerException; -import org.heigit.ors.isochrones.*; -import org.heigit.ors.isochrones.statistics.StatisticsProvider; -import org.heigit.ors.isochrones.statistics.StatisticsProviderConfiguration; -import org.heigit.ors.isochrones.statistics.StatisticsProviderFactory; import org.heigit.ors.routing.graphhopper.extensions.*; import org.heigit.ors.routing.graphhopper.extensions.flagencoders.FlagEncoderNames; import org.heigit.ors.routing.graphhopper.extensions.manage.ORSGraphManager; @@ -47,7 +43,6 @@ import org.heigit.ors.routing.graphhopper.extensions.util.ORSParameters; import org.heigit.ors.routing.parameters.ProfileParameters; import org.heigit.ors.routing.pathprocessors.ORSPathProcessorFactory; -import org.heigit.ors.util.DebugUtility; import org.heigit.ors.util.ProfileTools; import org.heigit.ors.util.StringUtility; import org.heigit.ors.util.TimeUtility; @@ -545,77 +540,6 @@ boolean requiresTimeDependentAlgorithm(RouteSearchParameters searchParams, Route || mGraphHopper.isTrafficEnabled(); } - /** - * This function creates the actual {@link IsochroneMap}. - * So the first step in the function is a checkup on that. - * - * @param parameters The input are {@link IsochroneSearchParameters} - * @return The return will be an {@link IsochroneMap} - * @throws Exception - */ - public IsochroneMap buildIsochrone(IsochroneSearchParameters parameters) throws Exception { - IsochroneMap result; - - try { - RouteSearchContext searchCntx = createSearchContext(parameters.getRouteParameters()); - IsochroneMapBuilderFactory isochroneMapBuilderFactory = new IsochroneMapBuilderFactory(searchCntx); - result = isochroneMapBuilderFactory.buildMap(parameters); - } catch (Exception ex) { - if (DebugUtility.isDebug()) { - LOGGER.error(ex); - } - throw new InternalServerException(IsochronesErrorCodes.UNKNOWN, "Unable to build an isochrone map."); - } - - if (result.getIsochronesCount() > 0) { - if (parameters.hasAttribute(ProfileTools.KEY_TOTAL_POP)) { - try { - Map> mapProviderToAttrs = new HashMap<>(); - StatisticsProviderConfiguration provConfig = parameters.getStatsProviders().get(ProfileTools.KEY_TOTAL_POP); - if (provConfig != null) { - List attrList = new ArrayList<>(); - attrList.add(ProfileTools.KEY_TOTAL_POP); - mapProviderToAttrs.put(provConfig, attrList); - } - for (Map.Entry> entry : mapProviderToAttrs.entrySet()) { - provConfig = entry.getKey(); - StatisticsProvider provider = StatisticsProviderFactory.getProvider(provConfig.getName(), provConfig.getParameters()); - String[] provAttrs = provConfig.getMappedProperties(entry.getValue()); - - for (Isochrone isochrone : result.getIsochrones()) { - - double[] attrValues = provider.getStatistics(isochrone, provAttrs); - isochrone.setAttributes(entry.getValue(), attrValues, provConfig.getAttribution()); - - } - } - } catch (Exception ex) { - LOGGER.error(ex); - - throw new InternalServerException(IsochronesErrorCodes.UNKNOWN, "Unable to compute isochrone total_pop attribute."); - } - } - if (parameters.hasAttribute("reachfactor") || parameters.hasAttribute("area")) { - for (Isochrone isochrone : result.getIsochrones()) { - String units = parameters.getUnits(); - String areaUnits = parameters.getAreaUnits(); - if (areaUnits != null) units = areaUnits; - double area = isochrone.calcArea(units); - if (parameters.hasAttribute("area")) { - isochrone.setArea(area); - } - if (parameters.hasAttribute("reachfactor")) { - double reachfactor = isochrone.calcReachfactor(units); - // reach factor could be > 1, which would confuse people - reachfactor = (reachfactor > 1) ? 1 : reachfactor; - isochrone.setReachfactor(reachfactor); - } - } - } - } - return result; - } - public boolean equals(Object o) { return o != null && o.getClass().equals(RoutingProfile.class) && this.hashCode() == o.hashCode(); } From 7c0710eda4a3767ed2b99e34fc62b366ac265e5d Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Thu, 12 Dec 2024 11:57:34 +0100 Subject: [PATCH 8/9] refactor: move createSearchContext out of RoutingProfile --- .../ors/isochrones/IsochroneRequest.java | 3 +- .../org/heigit/ors/matrix/MatrixRequest.java | 3 +- .../heigit/ors/routing/RoutingProfile.java | 82 ----------------- .../heigit/ors/routing/RoutingRequest.java | 4 +- .../heigit/ors/util/TemporaryUtilShelter.java | 91 ++++++++++++++++++- 5 files changed, 94 insertions(+), 89 deletions(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java b/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java index e64819f645..ff884f0678 100644 --- a/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java +++ b/ors-engine/src/main/java/org/heigit/ors/isochrones/IsochroneRequest.java @@ -24,6 +24,7 @@ import org.heigit.ors.routing.*; import org.heigit.ors.util.DebugUtility; import org.heigit.ors.util.ProfileTools; +import org.heigit.ors.util.TemporaryUtilShelter; import org.locationtech.jts.geom.Coordinate; import java.util.*; @@ -315,7 +316,7 @@ public IsochroneMap buildIsochrone(IsochroneSearchParameters parameters, Routing IsochroneMap result; try { - RouteSearchContext searchCntx = routingProfile.createSearchContext(parameters.getRouteParameters()); + RouteSearchContext searchCntx = TemporaryUtilShelter.createSearchContext(parameters.getRouteParameters(), routingProfile); IsochroneMapBuilderFactory isochroneMapBuilderFactory = new IsochroneMapBuilderFactory(searchCntx); result = isochroneMapBuilderFactory.buildMap(parameters); } catch (Exception ex) { diff --git a/ors-engine/src/main/java/org/heigit/ors/matrix/MatrixRequest.java b/ors-engine/src/main/java/org/heigit/ors/matrix/MatrixRequest.java index e5cd0df12e..ac60f78b51 100644 --- a/ors-engine/src/main/java/org/heigit/ors/matrix/MatrixRequest.java +++ b/ors-engine/src/main/java/org/heigit/ors/matrix/MatrixRequest.java @@ -37,6 +37,7 @@ import org.heigit.ors.routing.graphhopper.extensions.ORSGraphHopperStorage; import org.heigit.ors.routing.graphhopper.extensions.ORSWeightingFactory; import org.heigit.ors.util.ProfileTools; +import org.heigit.ors.util.TemporaryUtilShelter; import org.locationtech.jts.geom.Coordinate; public class MatrixRequest extends ServiceRequest { @@ -282,7 +283,7 @@ private MatrixResult computeDijkstraMatrix(GraphHopper gh, FlagEncoder flagEncod private MatrixResult computeCoreMatrix(GraphHopper gh, FlagEncoder flagEncoder, PMap hintsMap, String profileName, RoutingProfile routingProfile) throws Exception { Weighting weighting = new ORSWeightingFactory(gh.getGraphHopperStorage(), gh.getEncodingManager()).createWeighting(gh.getProfile(profileName), hintsMap, false); RoutingCHGraph graph = ((ORSGraphHopperStorage) gh.getGraphHopperStorage()).getCoreGraph(profileName); - RouteSearchContext searchCntx = routingProfile.createSearchContext(getSearchParameters()); + RouteSearchContext searchCntx = TemporaryUtilShelter.createSearchContext(getSearchParameters(), routingProfile); PMap additionalHints = searchCntx.getProperties(); EdgeFilter edgeFilter = new ORSEdgeFilterFactory().createEdgeFilter(additionalHints, flagEncoder, gh.getGraphHopperStorage()); diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java index 87303f1103..6fc592c856 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java @@ -34,14 +34,11 @@ import org.heigit.ors.config.profile.ExecutionProperties; import org.heigit.ors.config.profile.PreparationProperties; import org.heigit.ors.config.profile.ProfileProperties; -import org.heigit.ors.exceptions.InternalServerException; import org.heigit.ors.routing.graphhopper.extensions.*; -import org.heigit.ors.routing.graphhopper.extensions.flagencoders.FlagEncoderNames; import org.heigit.ors.routing.graphhopper.extensions.manage.ORSGraphManager; import org.heigit.ors.routing.graphhopper.extensions.storages.builders.BordersGraphStorageBuilder; import org.heigit.ors.routing.graphhopper.extensions.storages.builders.GraphStorageBuilder; import org.heigit.ors.routing.graphhopper.extensions.util.ORSParameters; -import org.heigit.ors.routing.parameters.ProfileParameters; import org.heigit.ors.routing.pathprocessors.ORSPathProcessorFactory; import org.heigit.ors.util.ProfileTools; import org.heigit.ors.util.StringUtility; @@ -412,85 +409,6 @@ public Double getAstarEpsilon() { return astarEpsilon; } - public RouteSearchContext createSearchContext(RouteSearchParameters searchParams) throws InternalServerException { - PMap props = new PMap(); - - int profileType = searchParams.getProfileType(); - String encoderName = RoutingProfileType.getEncoderName(profileType); - - if (FlagEncoderNames.UNKNOWN.equals(encoderName)) - throw new InternalServerException(RoutingErrorCodes.UNKNOWN, "unknown vehicle profile."); - - if (!mGraphHopper.getEncodingManager().hasEncoder(encoderName)) { - throw new IllegalArgumentException("Vehicle " + encoderName + " unsupported. " + "Supported are: " - + mGraphHopper.getEncodingManager()); - } - - FlagEncoder flagEncoder = mGraphHopper.getEncodingManager().getEncoder(encoderName); - ProfileParameters profileParams = searchParams.getProfileParameters(); - - // PARAMETERS FOR PathProcessorFactory - - props.putObject("routing_extra_info", searchParams.getExtraInfo()); - props.putObject("routing_suppress_warnings", searchParams.getSuppressWarnings()); - - props.putObject("routing_profile_type", profileType); - props.putObject("routing_profile_params", profileParams); - - /* - * PARAMETERS FOR EdgeFilterFactory - * ====================================================================================================== - */ - - /* Avoid areas */ - if (searchParams.hasAvoidAreas()) { - props.putObject("avoid_areas", searchParams.getAvoidAreas()); - } - - /* Heavy vehicle filter */ - if (profileType == RoutingProfileType.DRIVING_HGV) { - props.putObject("edgefilter_hgv", searchParams.getVehicleType()); - } - - /* Wheelchair filter */ - else if (profileType == RoutingProfileType.WHEELCHAIR) { - props.putObject("edgefilter_wheelchair", "true"); - } - - /* Avoid features */ - if (searchParams.hasAvoidFeatures()) { - props.putObject("avoid_features", searchParams); - } - - /* Avoid borders of some form */ - if ((searchParams.hasAvoidBorders() || searchParams.hasAvoidCountries()) - && (RoutingProfileType.isDriving(profileType) || RoutingProfileType.isCycling(profileType))) { - props.putObject("avoid_borders", searchParams); - if (searchParams.hasAvoidCountries()) - props.putObject("avoid_countries", Arrays.toString(searchParams.getAvoidCountries())); - } - - if (profileParams != null && profileParams.hasWeightings()) { - props.putObject(ProfileTools.KEY_CUSTOM_WEIGHTINGS, true); - Iterator iterator = profileParams.getWeightings().getIterator(); - while (iterator.hasNext()) { - ProfileWeighting weighting = iterator.next(); - if (!weighting.getParameters().isEmpty()) { - String name = ProfileWeighting.encodeName(weighting.getName()); - for (Map.Entry kv : weighting.getParameters().toMap().entrySet()) - props.putObject(name + kv.getKey(), kv.getValue()); - } - } - } - - String localProfileName = ProfileTools.makeProfileName(encoderName, WeightingMethod.getName(searchParams.getWeightingMethod()), Boolean.TRUE.equals(profileProperties.getBuild().getEncoderOptions().getTurnCosts())); - String profileNameCH = ProfileTools.makeProfileName(encoderName, WeightingMethod.getName(searchParams.getWeightingMethod()), false); - RouteSearchContext searchCntx = new RouteSearchContext(mGraphHopper, flagEncoder, localProfileName, profileNameCH); - searchCntx.setProperties(props); - - return searchCntx; - } - /** * Set the speedup techniques used for calculating the route. * Reults in usage of CH, Core or ALT/AStar, if they are enabled. diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingRequest.java b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingRequest.java index 8d64c7f923..ec7d995b16 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingRequest.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingRequest.java @@ -361,7 +361,7 @@ public GHResponse computeRoute(double lat0, double lon0, double lat1, double lon return res; } int weightingMethod = searchParams.getWeightingMethod(); - RouteSearchContext searchCntx = routingProfile.createSearchContext(searchParams); + RouteSearchContext searchCntx = TemporaryUtilShelter.createSearchContext(searchParams, routingProfile); int flexibleMode = searchParams.hasFlexibleMode() || Boolean.TRUE.equals(routingProfile.getProfileConfiguration().getService().getForceTurnCosts()) ? ProfileTools.KEY_FLEX_PREPROCESSED : ProfileTools.KEY_FLEX_STATIC; boolean optimized = searchParams.getOptimized(); @@ -473,7 +473,7 @@ public GHResponse computeRoundTripRoute(double lat0, double lon0, WayPointBearin try { int profileType = searchParams.getProfileType(); int weightingMethod = searchParams.getWeightingMethod(); - RouteSearchContext searchCntx = routingProfile.createSearchContext(searchParams); + RouteSearchContext searchCntx = TemporaryUtilShelter.createSearchContext(searchParams, routingProfile); List points = new ArrayList<>(); points.add(new GHPoint(lat0, lon0)); diff --git a/ors-engine/src/main/java/org/heigit/ors/util/TemporaryUtilShelter.java b/ors-engine/src/main/java/org/heigit/ors/util/TemporaryUtilShelter.java index 599b2e6be5..c5b7a8e9d1 100644 --- a/ors-engine/src/main/java/org/heigit/ors/util/TemporaryUtilShelter.java +++ b/ors-engine/src/main/java/org/heigit/ors/util/TemporaryUtilShelter.java @@ -3,11 +3,16 @@ import com.graphhopper.routing.util.EncodingManager; import com.graphhopper.routing.util.FlagEncoder; import com.graphhopper.storage.ConditionalEdges; -import org.heigit.ors.routing.RouteSearchContext; -import org.heigit.ors.routing.RouteSearchParameters; -import org.heigit.ors.routing.RoutingProfileType; +import com.graphhopper.util.PMap; +import org.heigit.ors.exceptions.InternalServerException; +import org.heigit.ors.routing.*; +import org.heigit.ors.routing.graphhopper.extensions.flagencoders.FlagEncoderNames; import org.heigit.ors.routing.parameters.ProfileParameters; +import java.util.Arrays; +import java.util.Iterator; +import java.util.Map; + /** * This class is only a temporary shelter for helper methods that lack appropriate places * during refactoring. @@ -48,4 +53,84 @@ public static boolean hasTimeDependentSpeed(RouteSearchParameters searchParams, String key = EncodingManager.getKey(flagEncoder, ConditionalEdges.SPEED); return searchParams.isTimeDependent() && flagEncoder.hasEncodedValue(key); } + + public static RouteSearchContext createSearchContext(RouteSearchParameters searchParams, RoutingProfile routingProfile) throws InternalServerException { + PMap props = new PMap(); + + int profileType = searchParams.getProfileType(); + String encoderName = RoutingProfileType.getEncoderName(profileType); + + if (FlagEncoderNames.UNKNOWN.equals(encoderName)) + throw new InternalServerException(RoutingErrorCodes.UNKNOWN, "unknown vehicle profile."); + + if (!routingProfile.getGraphhopper().getEncodingManager().hasEncoder(encoderName)) { + throw new IllegalArgumentException("Vehicle " + encoderName + " unsupported. " + "Supported are: " + + routingProfile.getGraphhopper().getEncodingManager()); + } + + FlagEncoder flagEncoder = routingProfile.getGraphhopper().getEncodingManager().getEncoder(encoderName); + ProfileParameters profileParams = searchParams.getProfileParameters(); + + // PARAMETERS FOR PathProcessorFactory + + props.putObject("routing_extra_info", searchParams.getExtraInfo()); + props.putObject("routing_suppress_warnings", searchParams.getSuppressWarnings()); + + props.putObject("routing_profile_type", profileType); + props.putObject("routing_profile_params", profileParams); + + /* + * PARAMETERS FOR EdgeFilterFactory + * ====================================================================================================== + */ + + /* Avoid areas */ + if (searchParams.hasAvoidAreas()) { + props.putObject("avoid_areas", searchParams.getAvoidAreas()); + } + + /* Heavy vehicle filter */ + if (profileType == RoutingProfileType.DRIVING_HGV) { + props.putObject("edgefilter_hgv", searchParams.getVehicleType()); + } + + /* Wheelchair filter */ + else if (profileType == RoutingProfileType.WHEELCHAIR) { + props.putObject("edgefilter_wheelchair", "true"); + } + + /* Avoid features */ + if (searchParams.hasAvoidFeatures()) { + props.putObject("avoid_features", searchParams); + } + + /* Avoid borders of some form */ + if ((searchParams.hasAvoidBorders() || searchParams.hasAvoidCountries()) + && (RoutingProfileType.isDriving(profileType) || RoutingProfileType.isCycling(profileType))) { + props.putObject("avoid_borders", searchParams); + if (searchParams.hasAvoidCountries()) + props.putObject("avoid_countries", Arrays.toString(searchParams.getAvoidCountries())); + } + + if (profileParams != null && profileParams.hasWeightings()) { + props.putObject(ProfileTools.KEY_CUSTOM_WEIGHTINGS, true); + Iterator iterator = profileParams.getWeightings().getIterator(); + while (iterator.hasNext()) { + ProfileWeighting weighting = iterator.next(); + if (!weighting.getParameters().isEmpty()) { + String name = ProfileWeighting.encodeName(weighting.getName()); + for (Map.Entry kv : weighting.getParameters().toMap().entrySet()) + props.putObject(name + kv.getKey(), kv.getValue()); + } + } + } + + String localProfileName = ProfileTools.makeProfileName(encoderName, WeightingMethod.getName(searchParams.getWeightingMethod()), + Boolean.TRUE.equals(routingProfile.getProfileProperties().getBuild().getEncoderOptions().getTurnCosts())); + String profileNameCH = ProfileTools.makeProfileName(encoderName, WeightingMethod.getName(searchParams.getWeightingMethod()), false); + RouteSearchContext searchCntx = new RouteSearchContext(routingProfile.getGraphhopper(), flagEncoder, localProfileName, profileNameCH); + searchCntx.setProperties(props); + + return searchCntx; + } } From 9a749aaf5cf8bf150201975e7da18dab7b4ca25f Mon Sep 17 00:00:00 2001 From: Sascha Fendrich Date: Thu, 12 Dec 2024 12:07:40 +0100 Subject: [PATCH 9/9] refactor: remove unused methods --- .../java/org/heigit/ors/routing/RoutingProfile.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java index 6fc592c856..003c344107 100644 --- a/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java +++ b/ors-engine/src/main/java/org/heigit/ors/routing/RoutingProfile.java @@ -377,10 +377,6 @@ public ORSGraphHopper getGraphhopper() { return mGraphHopper; } - public BBox getBounds() { - return mGraphHopper.getGraphHopperStorage().getBounds(); - } - public StorableProperties getGraphProperties() { return mGraphHopper.getGraphHopperStorage().getProperties(); } @@ -389,14 +385,6 @@ public ProfileProperties getProfileConfiguration() { return profileProperties; } - public Integer[] getPreferences() { - return mRoutePrefs; - } - - public boolean isCHEnabled() { - return mGraphHopper != null && mGraphHopper.getCHPreparationHandler().isEnabled(); - } - public void close() { mGraphHopper.close(); }