Skip to content

Conversation

Andy2003
Copy link
Collaborator

@Andy2003 Andy2003 commented Jul 3, 2025

Refactors all geometry-addition methods to accept a Map<String, Object> of properties instead of parallel String[] and Object[] arrays, updating both the API surface and all callers (including tests and importers).

  • Changed EditableLayer API and all implementations to use a single Map<String, Object> parameter.
  • Updated calls in tests (GeoPipesDocTest, TestSpatialUtils, TestSimplePointLayer, OsmAnalysisTest) and in ShapefileImporter to build and pass a properties map.
  • Removed now-unused overloads and cleaned up duplicate methods in DefaultLayer and DynamicLayerConfig.

@Andy2003 Andy2003 requested a review from Copilot July 3, 2025 09:53
@Andy2003 Andy2003 self-assigned this Jul 3, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Refactors all geometry-addition methods to accept a Map<String, Object> of properties instead of parallel String[] and Object[] arrays, updating both the API surface and all callers (including tests and importers).

  • Changed EditableLayer API and all implementations to use a single Map<String, Object> parameter.
  • Updated calls in tests (GeoPipesDocTest, TestSpatialUtils, TestSimplePointLayer, OsmAnalysisTest) and in ShapefileImporter to build and pass a properties map.
  • Removed now-unused overloads and cleaned up duplicate methods in DefaultLayer and DynamicLayerConfig.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server-plugin/src/main/java/org/neo4j/gis/spatial/EditableLayer.java Replaced array-based add method with a map-based signature.
server-plugin/src/main/java/org/neo4j/gis/spatial/EditableLayerImpl.java Updated implementations to accept and apply the properties map.
server-plugin/src/main/java/org/neo4j/gis/spatial/SimplePointLayer.java Adjusted overloads to forward to the new map-based add.
server-plugin/src/main/java/org/neo4j/gis/spatial/ShapefileImporter.java Builds a TreeMap of field→value entries and uses the map API.
server-plugin/src/main/java/org/neo4j/gis/spatial/OrderedEditableLayer.java Adapted internal addGeomNode to the map signature.
server-plugin/src/main/java/org/neo4j/gis/spatial/Layer.java Removed deprecated array-based methods.
server-plugin/src/main/java/org/neo4j/gis/spatial/DefaultLayer.java Cleans up old overrides now handled in EditableLayerImpl.
server-plugin/src/main/java/org/neo4j/gis/spatial/DynamicLayerConfig.java Removed stubs for array-based adds (no longer applicable).
server-plugin/src/test/java/org/neo4j/gis/spatial/pipes/GeoPipesDocTest.java Switched to Map.of(...) in sample geometry additions.
server-plugin/src/test/java/org/neo4j/gis/spatial/TestSpatialUtils.java Changed test helper calls to use a properties map; minor cleanup.
server-plugin/src/test/java/org/neo4j/gis/spatial/TestSimplePointLayer.java Casts to EditableLayer; updated overload usage.
server-plugin/src/test/java/org/neo4j/gis/spatial/OsmAnalysisTest.java Replaced array adds with a Map.of(...) call.
Comments suppressed due to low confidence (3)

server-plugin/src/main/java/org/neo4j/gis/spatial/EditableLayer.java:65

  • Removing the old String[]/Object[] overload is a breaking change for any existing callers. Consider deprecating the array-based method first or bumping the major version to signal this API change.
	SpatialDatabaseRecord add(Transaction tx, Geometry geometry, Map<String, Object> properties);

server-plugin/src/main/java/org/neo4j/gis/spatial/EditableLayer.java:64

  • [nitpick] The JavaDoc for the new add(..., Map<String, Object> properties) method should be expanded to explain how the properties map is used (e.g., permitted key names, handling of null, and behavior if omitted).
	 */

server-plugin/src/main/java/org/neo4j/gis/spatial/ShapefileImporter.java:178

  • [nitpick] The loop variable k is ambiguous. Renaming it to something like index would make the intent clearer when mapping fieldsName[index] to values.
											for (int k = 0; k < fieldsName.length; k++) {

…roperties instead of `String[]` and `Object[]`.
@Andy2003 Andy2003 force-pushed the refactor/property-usage branch from cd99178 to a946395 Compare July 3, 2025 10:04
@Andy2003 Andy2003 merged commit e73f56b into master Jul 3, 2025
1 check passed
@Andy2003 Andy2003 deleted the refactor/property-usage branch July 3, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant