Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add topojson graph export rebase #1926

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f853345
feat(topojson-export): First object definition of the new TopoJson ex…
MichaelsJP Nov 20, 2024
f826d83
feat(topojson-export): Introduce proper lombok builder
MichaelsJP Nov 20, 2024
50c8a3d
feat(topojson-export): Introduce a separate class for Arcs
MichaelsJP Nov 21, 2024
316f243
refactor(topojson-export): rename Objects class to Layers
Chwiggy Nov 21, 2024
9f4a5f4
test(topojson-export): add basic endpoint ping test
Chwiggy Nov 25, 2024
bad5b68
feat(topojson-export): add basic endpoint
Chwiggy Nov 25, 2024
e5a32b4
test(topojson-export): add simple export api ping tests
Chwiggy Nov 25, 2024
34dca5c
test(topojson-export): add test for export/json format
Chwiggy Nov 25, 2024
3b681e4
feat(topojson-export): work in progress topojson response generation
Chwiggy Nov 27, 2024
9404b47
feat: add OSM id and geometry to ExportResult
takb Dec 3, 2024
a612fdc
feat: usage of OSM id and geometry from ExportResult
takb Dec 3, 2024
3e2980d
fix(topojson): Create broken setter
MichaelsJP Dec 4, 2024
fb9d54b
feat(topojson): Add topology as default type
MichaelsJP Dec 4, 2024
147f67b
fix(topojson): Repair wrong serialization
MichaelsJP Dec 4, 2024
cd6194f
refactor(topojson): Merge TopoJson and TopoJsonExportResponse
MichaelsJP Dec 4, 2024
dd208b4
test(topojson): Finish first endpoint test for topojson
MichaelsJP Dec 4, 2024
8fbab06
refactor(topojson): Remove intermediate structure Layers.java.
MichaelsJP Dec 4, 2024
aff6244
refactor(sonarlint): Inline the profile Path variable
MichaelsJP Dec 4, 2024
efeecf8
feat: TopoJSON copmbining arcs with same OSM id to single geometries,…
takb Dec 9, 2024
1b8e3af
chore: cleanup, fix typos and inline doc
takb Dec 10, 2024
5ba1d71
test: fix TopoJsonExportResponseTest
takb Dec 10, 2024
2d49044
chore: cosmetics
takb Dec 10, 2024
9b7530f
test: fix expectInvalidResponseFormat with correct path param
takb Dec 10, 2024
3f8ad0d
refactor: restructure ExportRequest.computeExport(), rename API param…
takb Dec 10, 2024
d63e48a
test: fix testWheelchairDebugExport
takb Dec 11, 2024
96ea04e
refactor: TopoJSON response schema classes naming, TopoJSONExportResp…
takb Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ RELEASING:
## [unreleased]

### Added
- TopoJSON graph export ([#1926](https://github.com/GIScience/openrouteservice/pull/1926))

### Changed

Expand Down
3 changes: 2 additions & 1 deletion ors-api/src/main/java/org/heigit/ors/api/APIEnums.java
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ public String toString() {

@Schema(name = "Export response type", description = "Format of the export response.")
public enum ExportResponseType {
JSON("json");
JSON("json"),
TOPOJSON("topojson");

private final String value;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.heigit.ors.api.errors.CommonResponseEntityExceptionHandler;
import org.heigit.ors.api.requests.export.ExportApiRequest;
import org.heigit.ors.api.responses.export.json.JsonExportResponse;
import org.heigit.ors.api.responses.export.topojson.TopoJsonExportResponse;
import org.heigit.ors.api.services.ExportService;
import org.heigit.ors.common.EncoderNameEnum;
import org.heigit.ors.exceptions.*;
Expand Down Expand Up @@ -76,8 +77,8 @@ public String getPostMapping(@RequestBody ExportApiRequest request) throws Missi
// Matches any response type that has not been defined
@PostMapping(value = "/{profile}/*")
@Operation(hidden = true)
public void getInvalidResponseType() throws StatusCodeException {
throw new StatusCodeException(HttpServletResponse.SC_NOT_ACCEPTABLE, ExportErrorCodes.UNSUPPORTED_EXPORT_FORMAT, "This response format is not supported");
public void getInvalidResponseType(@PathVariable String profile) throws StatusCodeException {
throw new StatusCodeException(HttpServletResponse.SC_NOT_ACCEPTABLE, ExportErrorCodes.UNSUPPORTED_EXPORT_FORMAT, "The response format %s is not supported".formatted(profile));
koebi marked this conversation as resolved.
Show resolved Hide resolved
}

// Functional request methods
Expand All @@ -104,7 +105,7 @@ public JsonExportResponse getDefault(@Parameter(description = "Specifies the rou

@PostMapping(value = "/{profile}/json", produces = {"application/json;charset=UTF-8"})
@Operation(
description = "Returns a list of points, edges and weights within a given bounding box for a selected profile JSON.",
description = "Returns a list of points, edges and weights within a given bounding box for a selected profile as JSON.",
summary = "Export Service JSON"
)
@ApiResponse(
Expand All @@ -127,6 +128,32 @@ public JsonExportResponse getJsonExport(
return new JsonExportResponse(result);
}

@PostMapping(value = "/{profile}/topojson", produces = {"application/json;charset=UTF-8"})
@Operation(
description = "Returns a list of edges, edge properties, and their topology within a given bounding box for a selected profile.",
summary = "Export Service TopoJSON"
)
@ApiResponse(
responseCode = "200",
description = "TopoJSON Response.",
content = {@Content(
mediaType = "application/json",
schema = @Schema(implementation = TopoJsonExportResponse.class)
)
})
public TopoJsonExportResponse getTopoJsonExport(
@Parameter(description = "Specifies the profile.", required = true, example = "driving-car") @PathVariable String profile,
@Parameter(description = "The request payload", required = true) @RequestBody ExportApiRequest request) throws StatusCodeException {
request.setProfile(getProfileEnum(profile));
request.setProfileName(profile);
request.setResponseType(APIEnums.ExportResponseType.TOPOJSON);

ExportResult result = exportService.generateExportFromRequest(request);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it feels a bit weird that this is using the same function as the JSON endpoint, as they are returning vastly different data?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in both cases that function gets data from the graph for a specific bbox, the function below does different things with that result, I don't think that's so confusing...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with both of you :)
The main issue is burried deeper in the software design that spreads the computation logic across several levels of abstraction and differently for each end-point. This will become better, once the refactorings of RoutingProfile and RoutingProfileManager are fully done.


return TopoJsonExportResponse.fromExportResult(result);
}


@ExceptionHandler(MissingServletRequestParameterException.class)
public ResponseEntity<Object> handleMissingParams(final MissingServletRequestParameterException e) {
return errorHandler.handleStatusCodeException(new MissingParameterException(ExportErrorCodes.MISSING_PARAMETER, e.getParameterName()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
public class APIRequest {
public static final String PARAM_ID = "id";
public static final String PARAM_PROFILE = "profile";
public static final String PARAM_PROFILE_NAME = "profile_name";

@Schema(name = PARAM_ID, description = "Arbitrary identification string of the request reflected in the meta information.",
example = "my_request")
Expand All @@ -19,7 +20,7 @@ public class APIRequest {
@Schema(name = PARAM_PROFILE, hidden = true)
protected APIEnums.Profile profile;


@Schema(name = PARAM_PROFILE_NAME, hidden = true)
protected String profileName;

public boolean hasId() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.swagger.v3.oas.annotations.media.Schema;
import org.heigit.ors.api.requests.common.APIRequest;
import org.heigit.ors.api.APIEnums;
import org.heigit.ors.api.requests.common.APIRequest;

import java.util.List;

Expand All @@ -17,6 +17,7 @@ public class ExportApiRequest extends APIRequest {
public static final String PARAM_BBOX = "bbox";
public static final String PARAM_PROFILE = "profile";
public static final String PARAM_FORMAT = "format";
public static final String PARAM_GEOMETRY = "geometry";

public static final String PARAM_DEBUG = "debug";

Expand All @@ -40,6 +41,10 @@ public class ExportApiRequest extends APIRequest {
@JsonProperty(PARAM_FORMAT)
private APIEnums.ExportResponseType responseType = APIEnums.ExportResponseType.JSON;

@Schema(name = PARAM_GEOMETRY, description = "Whether to return the exact geometry of the graph or a simplified (beeline) representation.", example = "true", defaultValue = "true")
@JsonProperty(PARAM_GEOMETRY)
private boolean geometry = true;

@Schema(name = PARAM_DEBUG, hidden = true)
@JsonProperty(PARAM_DEBUG)
private boolean debug;
Expand Down Expand Up @@ -86,4 +91,11 @@ public void setResponseType(APIEnums.ExportResponseType responseType) {
this.responseType = responseType;
}

public APIEnums.ExportResponseType getResponseType() {
return responseType;
}

public boolean getGeometry() {
return geometry;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public JsonExportResponse(ExportResult exportResult) {
nodesCount = nodes.stream().count();

edges = new ArrayList<>();
for (Map.Entry<Pair<Integer, Integer>, Double> edgeWeight : exportResult.getEdgeWeigths().entrySet()) {
for (Map.Entry<Pair<Integer, Integer>, Double> edgeWeight : exportResult.getEdgeWeights().entrySet()) {
edges.add(new JsonEdge(edgeWeight));
}
edgesCount = edges.stream().count();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.heigit.ors.api.responses.export.topojson;

import com.fasterxml.jackson.annotation.JsonValue;
import lombok.Builder;
import lombok.Getter;
import lombok.Setter;

import java.io.Serializable;
import java.util.List;

@Builder
@Getter
@Setter
public class Arc implements Serializable {

private List<List<Double>> coordinates;

public Arc(List<List<Double>> coordinates) {
this.coordinates = coordinates;
}

@JsonValue
public List<List<Double>> getCoordinates() {
return coordinates;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.heigit.ors.api.responses.export.topojson;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import lombok.Builder;
import lombok.Getter;

import java.io.Serializable;
import java.util.List;
import java.util.Map;

@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({"type", "properties", "arcs"})
@Getter
@Builder
public class Geometry implements Serializable {

@JsonProperty("type")
private String type;
@JsonProperty("properties")
private transient Properties properties;
@JsonProperty("arcs")
private List<Integer> arcs;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

package org.heigit.ors.api.responses.export.topojson;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import lombok.Builder;
import lombok.Getter;

import java.io.Serializable;
import java.util.List;

@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({
"type",
"geometries"
})
@Getter
@Builder
public class Layer implements Serializable {

@JsonProperty("type")
private String type;
@JsonProperty("geometries")
private List<Geometry> geometries;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

package org.heigit.ors.api.responses.export.topojson;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import lombok.Builder;
import lombok.Getter;

import java.io.Serializable;
import java.util.List;

@JsonInclude(JsonInclude.Include.NON_NULL)
@Getter
@Builder
public class Network implements Serializable {
@JsonProperty("network")
private Layer network;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it a bit confusing that the class and the field are both named network but the field is of type Layer.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.heigit.ors.api.responses.export.topojson;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import lombok.Builder;
import lombok.Getter;

import java.io.Serializable;
import java.util.List;
import java.util.Map;

@JsonInclude(JsonInclude.Include.NON_NULL)
@Getter
@Builder
public class Properties implements Serializable {
@JsonProperty("weight")
private Double weight;
@JsonProperty("osm_id")
private Long osmId;
@JsonProperty("both_directions")
private Boolean bothDirections;
@JsonProperty("speed")
private Double speed;
@JsonProperty("speed_reverse")
private Double speedReverse;
@JsonProperty("ors_ids")
private List<Integer> orsIds;
@JsonProperty("ors_nodes")
private List<Integer> orsNodes;
@JsonProperty("distances")
private List<Double> distances;
}
Loading
Loading