-
Notifications
You must be signed in to change notification settings - Fork 2
feature: support simple filtering operation #209
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
Open
Abolfazlrezaei93
wants to merge
7
commits into
develop
Choose a base branch
from
feature/support-simple-filtering-operation
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
67a61b4
feat(operations): add GeoJsonFilterOperation for centroid radius filtβ¦
Abolfazlrezaei93 c2094a3
test(operations): add GeoJsonFilterOperation tests and discovery asseβ¦
Abolfazlrezaei93 23f6cda
docs(queries): add DHN point-radius GeoJSON filter examples
Abolfazlrezaei93 efa22e7
fix(operations): avoid deep copy of features in GeoJsonFilterOperation
Abolfazlrezaei93 4778ca7
test(operations): convert GeoJsonFilterOperation unit test to integraβ¦
Abolfazlrezaei93 2a43bba
refactor(operations): replace null returns with NullObject in GeoJsonβ¦
Abolfazlrezaei93 4be83d2
test(operations): fix GeoJsonFilterOperationTest CI failures
Abolfazlrezaei93 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
32 changes: 32 additions & 0 deletions
32
Middleware/docs/examples/queries/dhnBuildingsGeoJsonFilterByPointRadius.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| { | ||
| "use": "ca.concordia.encs.citydata.producers.CKANProducer", | ||
| "withParams": [ | ||
| { | ||
| "name": "url", | ||
| "value": "https://ngci.encs.concordia.ca/ckan/api/3" | ||
| }, | ||
| { | ||
| "name": "resourceId", | ||
| "value": "1823209d-e66e-4a8e-b12a-2aced638da4c" | ||
| } | ||
| ], | ||
| "apply": [ | ||
| { | ||
| "name": "ca.concordia.encs.citydata.operations.GeoJsonFilterOperation", | ||
| "withParams": [ | ||
| { | ||
| "name": "centerLongitude", | ||
| "value": -73.74085 | ||
| }, | ||
| { | ||
| "name": "centerLatitude", | ||
| "value": 45.51895 | ||
| }, | ||
| { | ||
| "name": "radiusMeters", | ||
| "value": 200.0 | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
32 changes: 32 additions & 0 deletions
32
Middleware/docs/examples/queries/dhnRoadsGeoJsonFilterByPointRadius.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| { | ||
| "use": "ca.concordia.encs.citydata.producers.CKANProducer", | ||
| "withParams": [ | ||
| { | ||
| "name": "url", | ||
| "value": "https://ngci.encs.concordia.ca/ckan/api/3" | ||
| }, | ||
| { | ||
| "name": "resourceId", | ||
| "value": "a98f2d84-aa71-4adc-a12e-480ce133877e" | ||
| } | ||
| ], | ||
| "apply": [ | ||
| { | ||
| "name": "ca.concordia.encs.citydata.operations.GeoJsonFilterOperation", | ||
| "withParams": [ | ||
| { | ||
| "name": "centerLongitude", | ||
| "value": -73.74085 | ||
| }, | ||
| { | ||
| "name": "centerLatitude", | ||
| "value": 45.51895 | ||
| }, | ||
| { | ||
| "name": "radiusMeters", | ||
| "value": 200.0 | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
216 changes: 216 additions & 0 deletions
216
Middleware/src/main/java/ca/concordia/encs/citydata/operations/GeoJsonFilterOperation.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,216 @@ | ||
| package ca.concordia.encs.citydata.operations; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import com.google.gson.JsonArray; | ||
| import com.google.gson.JsonElement; | ||
| import com.google.gson.JsonObject; | ||
| import com.google.gson.JsonParser; | ||
|
|
||
| import ca.concordia.encs.citydata.core.contracts.IOperation; | ||
| import ca.concordia.encs.citydata.core.implementations.AbstractOperation; | ||
|
|
||
| /** | ||
| * Filters GeoJSON FeatureCollection features by distance to a given point. | ||
| * A feature is kept when its geometry centroid is within radiusMeters. | ||
| * | ||
| * @author Aboolfazl Rezaei | ||
| * @since 2026-02-25 | ||
| */ | ||
| public class GeoJsonFilterOperation extends AbstractOperation<String> implements IOperation<String> { | ||
|
|
||
| private static final double EARTH_RADIUS_METERS = 6371000.0; | ||
|
|
||
| private Double centerLongitude; | ||
| private Double centerLatitude; | ||
| private Double radiusMeters; | ||
|
|
||
| public void setCenterLongitude(Double centerLongitude) { | ||
| this.centerLongitude = centerLongitude; | ||
| } | ||
|
|
||
| public void setCenterLatitude(Double centerLatitude) { | ||
| this.centerLatitude = centerLatitude; | ||
| } | ||
|
|
||
| public void setRadiusMeters(Double radiusMeters) { | ||
| this.radiusMeters = radiusMeters; | ||
| } | ||
|
|
||
| @Override | ||
| public ArrayList<String> apply(ArrayList<String> inputs) { | ||
| this.validateParameters(); | ||
| final ArrayList<String> filteredOutputs = new ArrayList<>(); | ||
|
|
||
| for (String input : inputs) { | ||
| if (input == null || input.isBlank()) { | ||
| continue; | ||
| } | ||
|
|
||
| final JsonElement element = JsonParser.parseString(input); | ||
| if (element.isJsonObject()) { | ||
| final JsonObject object = element.getAsJsonObject(); | ||
| filteredOutputs.add(filterElementIfFeatureCollection(object).toString()); | ||
| } else if (element.isJsonArray()) { | ||
| final JsonArray resultArray = new JsonArray(); | ||
| for (JsonElement child : element.getAsJsonArray()) { | ||
| if (child.isJsonObject()) { | ||
| resultArray.add(filterElementIfFeatureCollection(child.getAsJsonObject())); | ||
| } else { | ||
| resultArray.add(child); | ||
| } | ||
| } | ||
| filteredOutputs.add(resultArray.toString()); | ||
| } else { | ||
| filteredOutputs.add(input); | ||
| } | ||
| } | ||
| return filteredOutputs; | ||
| } | ||
|
|
||
| private JsonObject filterElementIfFeatureCollection(JsonObject inputObject) { | ||
| if (!inputObject.has("type") || !"FeatureCollection".equalsIgnoreCase(inputObject.get("type").getAsString()) | ||
| || !inputObject.has("features") || !inputObject.get("features").isJsonArray()) { | ||
| return inputObject; | ||
| } | ||
|
|
||
| final JsonArray inputFeatures = inputObject.getAsJsonArray("features"); | ||
| final JsonArray filteredFeatures = new JsonArray(); | ||
|
|
||
| for (JsonElement featureElement : inputFeatures) { | ||
| if (!featureElement.isJsonObject()) { | ||
| continue; | ||
| } | ||
| final JsonObject feature = featureElement.getAsJsonObject(); | ||
| final Coordinate centroid = getFeatureCentroid(feature); | ||
| if (centroid == Coordinate.NONE) { | ||
| continue; | ||
| } | ||
|
|
||
| final double distance = haversineMeters(this.centerLatitude, this.centerLongitude, centroid.latitude, | ||
| centroid.longitude); | ||
| if (distance <= this.radiusMeters) { | ||
| filteredFeatures.add(feature); | ||
| } | ||
| } | ||
|
|
||
| final JsonObject output = new JsonObject(); | ||
| for (Map.Entry<String, JsonElement> entry : inputObject.entrySet()) { | ||
| if (!"features".equals(entry.getKey())) { | ||
| output.add(entry.getKey(), entry.getValue()); | ||
| } | ||
| } | ||
| output.add("features", filteredFeatures); | ||
| return output; | ||
| } | ||
|
|
||
| private Coordinate getFeatureCentroid(JsonObject feature) { | ||
| if (feature == null || !feature.has("geometry") || !feature.get("geometry").isJsonObject()) { | ||
| return Coordinate.NONE; | ||
| } | ||
| return getGeometryCentroid(feature.getAsJsonObject("geometry")); | ||
| } | ||
|
|
||
| private Coordinate getGeometryCentroid(JsonObject geometry) { | ||
| if (geometry == null || !geometry.has("type") || !geometry.get("type").isJsonPrimitive()) { | ||
| return Coordinate.NONE; | ||
| } | ||
| final List<Coordinate> coordinates = new ArrayList<>(); | ||
| collectCoordinatesFromGeometry(geometry, coordinates); | ||
|
|
||
| if (coordinates.isEmpty()) { | ||
| return Coordinate.NONE; | ||
| } | ||
| return averageCoordinates(coordinates); | ||
| } | ||
|
|
||
| private void collectCoordinatesFromGeometry(JsonObject geometry, List<Coordinate> collector) { | ||
| if (geometry == null || !geometry.has("type") || !geometry.get("type").isJsonPrimitive()) { | ||
| return; | ||
| } | ||
| final String geometryType = geometry.get("type").getAsString(); | ||
|
|
||
| if ("GeometryCollection".equalsIgnoreCase(geometryType) && geometry.has("geometries") | ||
| && geometry.get("geometries").isJsonArray()) { | ||
| for (JsonElement child : geometry.getAsJsonArray("geometries")) { | ||
| if (child.isJsonObject()) { | ||
| collectCoordinatesFromGeometry(child.getAsJsonObject(), collector); | ||
| } | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (geometry.has("coordinates")) { | ||
| collectCoordinatesRecursive(geometry.get("coordinates"), collector); | ||
| } | ||
| } | ||
|
|
||
| private void collectCoordinatesRecursive(JsonElement coordinatesElement, List<Coordinate> collector) { | ||
| if (coordinatesElement == null || !coordinatesElement.isJsonArray()) { | ||
| return; | ||
| } | ||
| final JsonArray array = coordinatesElement.getAsJsonArray(); | ||
| if (isCoordinatePair(array)) { | ||
| collector.add(new Coordinate(array.get(1).getAsDouble(), array.get(0).getAsDouble())); | ||
| return; | ||
| } | ||
|
|
||
| for (JsonElement child : array) { | ||
| collectCoordinatesRecursive(child, collector); | ||
| } | ||
| } | ||
|
|
||
| private boolean isCoordinatePair(JsonArray array) { | ||
| return array.size() >= 2 && array.get(0).isJsonPrimitive() && array.get(1).isJsonPrimitive() | ||
| && array.get(0).getAsJsonPrimitive().isNumber() && array.get(1).getAsJsonPrimitive().isNumber(); | ||
| } | ||
|
|
||
| private Coordinate averageCoordinates(List<Coordinate> coordinates) { | ||
| double lat = 0.0; | ||
| double lon = 0.0; | ||
| for (Coordinate coordinate : coordinates) { | ||
| lat += coordinate.latitude; | ||
| lon += coordinate.longitude; | ||
| } | ||
| return new Coordinate(lat / coordinates.size(), lon / coordinates.size()); | ||
| } | ||
|
|
||
| private double haversineMeters(double latitudeA, double longitudeA, double latitudeB, double longitudeB) { | ||
| final double latitudeDelta = Math.toRadians(latitudeB - latitudeA); | ||
| final double longitudeDelta = Math.toRadians(longitudeB - longitudeA); | ||
| final double base = Math.pow(Math.sin(latitudeDelta / 2.0), 2) | ||
| + Math.cos(Math.toRadians(latitudeA)) * Math.cos(Math.toRadians(latitudeB)) | ||
| * Math.pow(Math.sin(longitudeDelta / 2.0), 2); | ||
| return 2.0 * EARTH_RADIUS_METERS * Math.asin(Math.sqrt(base)); | ||
| } | ||
|
|
||
| private void validateParameters() { | ||
| if (this.centerLatitude == null || this.centerLongitude == null || this.radiusMeters == null) { | ||
| throw new IllegalArgumentException( | ||
| "GeoJsonFilterOperation requires centerLatitude, centerLongitude and radiusMeters."); | ||
| } | ||
| if (this.centerLatitude < -90.0 || this.centerLatitude > 90.0) { | ||
| throw new IllegalArgumentException("centerLatitude must be in range [-90, 90]."); | ||
| } | ||
| if (this.centerLongitude < -180.0 || this.centerLongitude > 180.0) { | ||
| throw new IllegalArgumentException("centerLongitude must be in range [-180, 180]."); | ||
| } | ||
| if (this.radiusMeters <= 0.0) { | ||
| throw new IllegalArgumentException("radiusMeters must be > 0."); | ||
| } | ||
| } | ||
|
|
||
| private static final class Coordinate { | ||
| static final Coordinate NONE = new Coordinate(Double.NaN, Double.NaN); | ||
|
|
||
| private final double latitude; | ||
| private final double longitude; | ||
|
|
||
| private Coordinate(double latitude, double longitude) { | ||
| this.latitude = latitude; | ||
| this.longitude = longitude; | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71 changes: 71 additions & 0 deletions
71
.../src/test/java/ca/concordia/encs/citydata/test/operations/GeoJsonFilterOperationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package ca.concordia.encs.citydata.operations; | ||
|
|
||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
|
|
||
| import org.junit.jupiter.api.Disabled; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; | ||
| import org.springframework.boot.test.context.SpringBootTest; | ||
| import org.springframework.context.annotation.ComponentScan; | ||
| import org.springframework.http.MediaType; | ||
|
|
||
| import com.google.gson.JsonArray; | ||
| import com.google.gson.JsonObject; | ||
|
|
||
| import ca.concordia.encs.citydata.PayloadFactory; | ||
| import ca.concordia.encs.citydata.config.TestConfig; | ||
| import ca.concordia.encs.citydata.core.BaseIntegrationTest; | ||
| import ca.concordia.encs.citydata.core.configs.AppConfig; | ||
|
|
||
| /** | ||
| * Integration tests for GeoJsonFilterOperation. | ||
| * | ||
| * @author Aboolfazl Rezaei | ||
| * @since 2026-02-25 | ||
| */ | ||
| @SpringBootTest(classes = { AppConfig.class, TestConfig.class }) | ||
| @AutoConfigureMockMvc | ||
| @ComponentScan(basePackages = "ca.concordia.encs.citydata.core") | ||
| public class GeoJsonFilterOperationTest extends BaseIntegrationTest { | ||
|
|
||
| @Test | ||
| @Disabled("DHN buildings dataset is too large for heap-constrained CI β known CKANProducer limitation") | ||
| public void testFilterDhnBuildingsByPointRadius() throws Exception { | ||
| String jsonPayload = PayloadFactory.getExampleQuery("dhnBuildingsGeoJsonFilterByPointRadius"); | ||
| mockMvc.perform(post("/apply/sync").header("Authorization", "Bearer " + getToken()) | ||
| .contentType(MediaType.APPLICATION_JSON).content(jsonPayload)).andExpect(status().isOk()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testFilterDhnRoadsByPointRadius() throws Exception { | ||
| String jsonPayload = PayloadFactory.getExampleQuery("dhnRoadsGeoJsonFilterByPointRadius"); | ||
| mockMvc.perform(post("/apply/sync").header("Authorization", "Bearer " + getToken()) | ||
| .contentType(MediaType.APPLICATION_JSON).content(jsonPayload)).andExpect(status().isOk()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testMissingRadiusParameterReturnsError() throws Exception { | ||
| String jsonPayload = PayloadFactory.getExampleQuery("dhnRoadsGeoJsonFilterByPointRadius"); | ||
| JsonObject jsonObject = com.google.gson.JsonParser.parseString(jsonPayload).getAsJsonObject(); | ||
| JsonArray withParams = jsonObject.getAsJsonArray("apply").get(0).getAsJsonObject() | ||
| .getAsJsonArray("withParams"); | ||
| for (int i = 0; i < withParams.size(); i++) { | ||
| if (withParams.get(i).getAsJsonObject().get("name").getAsString().equals("radiusMeters")) { | ||
| withParams.remove(i); | ||
| break; | ||
| } | ||
| } | ||
| mockMvc.perform(post("/apply/sync").header("Authorization", "Bearer " + getToken()) | ||
| .contentType(MediaType.APPLICATION_JSON).content(jsonObject.toString())) | ||
| .andExpect(status().is5xxServerError()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testInvalidJsonReturnsClientError() throws Exception { | ||
| String brokenJson = PayloadFactory.getInvalidJson(); | ||
| mockMvc.perform(post("/apply/sync").header("Authorization", "Bearer " + getToken()) | ||
| .contentType(MediaType.APPLICATION_JSON).content(brokenJson)) | ||
| .andExpect(status().is4xxClientError()); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you provided a Unit Test. Unless you don't want any user to run it through an HTTP request, it's ok. But since you'll finally merge it into the project, I suggest you update the test into an integration test.
The issue: you instantiate and call the operation directly
final GeoJsonFilterOperation operation = new GeoJsonFilterOperation(); final ArrayList<String> output = operation.apply(input);insteal of wiring the producer's (CKANProducer here, according to your operation and queries) output through the
SequentialRunnerinto the operation input as it is done in all the integration tests. This will provide a real query payload with CKANProducer as the data source and GeoJsonOperation as the operation to apply, and this is consistent with how the other operation is tested.You can get inspired by
MergeOperationTest.javaThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment and your review. I have changed this to an integration test with AI's help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I disabled
testFilterDhnBuildingsByPointRadius, the buildings CKAN dataset is too large for the CI heap.I switched
testMissingRadiusParameterReturnsErrorto use the roads payload instead of buildings: the roads dataset fetches successfully within the heap limits, then the operation fails fast withIllegalArgumentExceptionwhenradiusMetersis missing.