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

Compliant, codable, equatable GeoJSON #154

Merged
merged 16 commits into from
Sep 28, 2021
Merged

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Sep 28, 2021

This PR revamps the public interface and implementation of GeoJSON support to take better advantage of the Swift compiler’s type checking capabilities. It includes a number of backwards-incompatible changes to the public API:

  • Replaced the GeoJSON class and GeoJSON protocol with a unified GeoJSONObject enumeration. Instead of calling GeoJSON.parse(_:) or GeoJSON.parse<T: GeoJSONObject>(_:from:), use JSONDecoder:
    // Before
    if let feature = try GeoJSON.parse(data)?.decodedFeature,
       case let .lineString(lineString) = feature.geometry {  }
    // Now
    if case let .feature(feature) = try JSONDecoder().decode(GeoJSONObject.self, from: data),
       case let .lineString(lineString) = feature.geometry {  }
  • Removed the FeatureCollection.identifier and FeatureCollection.properties properties with no replacement. These properties had been represented in GeoJSON by foreign members, which are not yet implemented. If you had been relying on the identifier or properties foreign members of FeatureCollection objects, move the data to each individual feature in the collection.
  • The Feature.properties property is now a JSONObject? (in other words, [String: JSONValue?]?). JSONObject is type-checked at compile time instead of runtime, but you can initialize it using a literal or full-width conversion from Any?. Code that builds a JSON object using literals will have to be modified to either specify a JSONValue case for each value or call the JSONObject(rawValue:) initializer.
  • The Feature.geometry property is now optional.
  • Removed the Geometry.type property. Use pattern matching (case let) instead.
  • Removed the Geometry.value property. This type erasure is unnecessary and can potentially become a source of bugs. Use pattern matching (case let) instead.
  • Removed the Number enumeration in favor of a Double-typed FeatureIdentifier.number(_:) case. JSON doesn’t distinguish between integers and double-precision floating point numbers. Any distinction in the type system or encoded JSON is purely cosmetic.
  • Feature and FeatureCollection now conform to the Equatable protocol.
  • Each geometric type, such as Point, now conforms to the Codable and Equatable protocols.
  • BoundingBox now conforms to the Hashable protocol.

The resulting GeoJSON interface is probably much closer to other Swift implementations of GeoJSON like GeoJSON and GEOSwift. (@macdrevx gets credit for, among other things, introducing me to the ExpressibleBy*Literal protocols as syntactic sugar for JSON.) That said, there’s still a pragmatic reliance on Core Location types (or workalikes on Linux), such as CLLocationCoordinate2D/LocationCoordinate2D in place of a dedicated Position struct. Also, null is still represented by Optional.none instead of a redundant JSONValue.null case. Perhaps a future refactor could migrate to or vendor one of the dedicated GeoJSON libraries for more strict conformance.

In practice, most of these changes appear to be less invasive than they sound. Some downstream projects are being updated:

I’ll be the first to admit that a refactor of this size is unfortunate so close to the final release of Turf v2.0.0, but it’s the only way we can get Equatable conformance and better GeoJSON compliance, which is important for the wider variety of use cases associated with the map SDK compared to the navigation SDK.

Test coverage increased from 84.0% to 90.3%.

Fixes #150 and fixes #153. See #45 #96 for the previous iterations of this part of the codebase.

/cc @mapbox/navigation-ios @mapbox/maps-ios

Geometry, FeatureIdentifier, and Number no longer have value properties to offer type erasure. The type property has also been removed in favor of proper pattern matching.
Replaced the GeoJSONObject protocol with an enumeration of the same name with associated values for the structs that previously conformed to the protocol. Removed GeoJSON in favor of GeoJSONObject, which now conforms to Codable. Renamed type coding keys to kind for consistency.
Pushed Codable conformance from Geometry to each individual geometric struct, so that Geometry is only a container for a geometric type with no additional smarts.
Removed support for the id and properties foreign members on FeatureCollection objects in GeoJSON.
Added a JSONValue enumeration (along with JSONArray and JSONObject type aliases) to represent JSON values with more compile-time type checking. JSONValue can be expressed by various types of literals, so developers do not need to explicitly create JSONValue when programmatically initializing a feature and its properties. When getting a property, developers now use the case let syntax instead of type casting. A rawValue property has been retained in case type casting is absolutely necessary.

Deleted the custom decoding and encoding container method implementations in favor of more straightforward JSONValue Codable conformance. Deleted the Number enumeration: JSON doesn’t distinguish between the two numeric types, so any additional precision is purely cosmetic.

Made the remaining GeoJSON types conform to Equatable, now that the properties object’s type is more specifically known.
@1ec5 1ec5 added bug improvement Improvement for an existing feature. op-ex Refactoring, Tech Debt or any other operational excellence work. backwards incompatible changes that break backwards compatibility of public API labels Sep 28, 2021
@1ec5 1ec5 added this to the v2.0.0 (GA) milestone Sep 28, 2021
@1ec5 1ec5 self-assigned this Sep 28, 2021
Comment on lines +18 to +40
extension Point: Codable {
enum CodingKeys: String, CodingKey {
case kind = "type"
case coordinates
}

enum Kind: String, Codable {
case Point
}

public init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
_ = try container.decode(Kind.self, forKey: .kind)
let coordinates = try container.decode(LocationCoordinate2DCodable.self, forKey: .coordinates).decodedCoordinates
self = .init(coordinates)
}

public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(Kind.Point, forKey: .kind)
try container.encode(coordinates.codableCoordinates, forKey: .coordinates)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each of the geometric types has a very similar Codable implementation. In the interest of time, I’m leaving this redundancy in place, but it would be nice to figure out a more generic implementation at some point. The sticking point is that everything is ultimately based on CLLocationCoordinate2D, which we can’t conform to Codable because it would potentially collide with other libraries that use that standard type in a non-GeoJSON context: #84.

Make it easier to convert JSONSerialization-style objects to JSONValue and back.
Sources/Turf/JSON.swift Outdated Show resolved Hide resolved
Comment on lines 81 to 85
public typealias RawValue = [Any?]

public init(rawValue values: RawValue) {
self = values.map(JSONValue.init(rawValue:))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is intended, but the way that JSONValue's init is failable means that I can pass an array of arbitrary values into this init and get a result that just has a nil for each unsupported input type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is consistent with RawRepresentable(rawValue:) semantics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or would it be better for this initializer to fail if any of the elements fail to initialize?

The GeoJSON specification allows a Feature object to have no geometry at all.
@1ec5 1ec5 force-pushed the 1ec5-geojson-collection-conform-150 branch from 3e30f46 to dd19d88 Compare September 28, 2021 16:55
@1ec5 1ec5 merged commit 3f3fb5b into main Sep 28, 2021
@1ec5 1ec5 deleted the 1ec5-geojson-collection-conform-150 branch September 28, 2021 22:23
@1ec5
Copy link
Contributor Author

1ec5 commented Sep 28, 2021

8aaabff updates the GeoJSON example in the readme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible changes that break backwards compatibility of public API bug improvement Improvement for an existing feature. op-ex Refactoring, Tech Debt or any other operational excellence work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FeatureCollection has nonstandard properties Conform all geojson related types to Equatable
3 participants