From fe9912cc4b6c17c1fb6436253919cb9253c56eef Mon Sep 17 00:00:00 2001 From: 2kai2kai2 <2kai2kai2@gmail.com> Date: Thu, 2 Nov 2023 13:58:36 -0400 Subject: [PATCH 1/6] Add tests for OSM data service --- apps/ios/GuideDogs.xcodeproj/project.pbxproj | 20 ++++ .../Code/App/ExternalNavigationApps.swift | 4 +- .../Services/OSM/OSMServiceModelTest.swift | 111 ++++++++++++++++++ 3 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift diff --git a/apps/ios/GuideDogs.xcodeproj/project.pbxproj b/apps/ios/GuideDogs.xcodeproj/project.pbxproj index eee43114..86651c4a 100644 --- a/apps/ios/GuideDogs.xcodeproj/project.pbxproj +++ b/apps/ios/GuideDogs.xcodeproj/project.pbxproj @@ -666,6 +666,7 @@ 914BAAF32AD745E400CB2171 /* DestinationManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914BAAF22AD745E400CB2171 /* DestinationManagerTest.swift */; }; 914BAAFD2AD7483300CB2171 /* AudioEngineTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914BAAFC2AD7483300CB2171 /* AudioEngineTest.swift */; }; 915FF9F62ADE4BAF002B3690 /* AuthoredActivityContentTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 915FF9F42ADE3F91002B3690 /* AuthoredActivityContentTest.swift */; }; + 91B6ADAA2AF19CB600FFE4E9 /* OSMServiceModelTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */; }; 91C82AAD2A5DCF040086D126 /* GeolocationManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91C82AAC2A5DCF040086D126 /* GeolocationManagerTest.swift */; }; 91C82ABE2A6B08500086D126 /* RouteGuidanceTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91C82ABD2A6B08500086D126 /* RouteGuidanceTest.swift */; }; 91DC0CF92A46134600244CC8 /* GeometryUtilsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914DEBDC2A3CE901007B161C /* GeometryUtilsTest.swift */; }; @@ -1584,6 +1585,7 @@ 914DEBCD2A3CE6B9007B161C /* UnitTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = UnitTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; 914DEBDC2A3CE901007B161C /* GeometryUtilsTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeometryUtilsTest.swift; sourceTree = ""; }; 915FF9F42ADE3F91002B3690 /* AuthoredActivityContentTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthoredActivityContentTest.swift; sourceTree = ""; }; + 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OSMServiceModelTest.swift; sourceTree = ""; }; 91C82AAC2A5DCF040086D126 /* GeolocationManagerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = " GeolocationManagerTest.swift"; path = "UnitTests/Sensors/Geolocation/Geolocation Manager/ GeolocationManagerTest.swift"; sourceTree = SOURCE_ROOT; }; 91C82ABD2A6B08500086D126 /* RouteGuidanceTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = RouteGuidanceTest.swift; path = "UnitTests/Behaviors/Route Guidance/RouteGuidanceTest.swift"; sourceTree = SOURCE_ROOT; }; B90C27D51EAF81D600007368 /* Sound.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = Sound.swift; path = Code/Audio/Protocols/Sound.swift; sourceTree = ""; }; @@ -4277,6 +4279,22 @@ path = "Interactive View"; sourceTree = ""; }; + 914BAAED2AD745BC00CB2171 /* Services */ = { + isa = PBXGroup; + children = ( + 914BAAEE2AD745BC00CB2171 /* OSM */, + ); + path = Services; + sourceTree = ""; + }; + 914BAAEE2AD745BC00CB2171 /* OSM */ = { + isa = PBXGroup; + children = ( + 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */, + ); + path = OSM; + sourceTree = ""; + }; 914BAAF12AD745E400CB2171 /* Destination Manager */ = { isa = PBXGroup; children = ( @@ -4358,6 +4376,7 @@ children = ( 915FF9F32ADE3F91002B3690 /* Authored Activities */, 914BAAF12AD745E400CB2171 /* Destination Manager */, + 914BAAED2AD745BC00CB2171 /* Services */, ); path = Data; sourceTree = ""; @@ -5543,6 +5562,7 @@ 91C82ABE2A6B08500086D126 /* RouteGuidanceTest.swift in Sources */, 91C82AAD2A5DCF040086D126 /* GeolocationManagerTest.swift in Sources */, 91DC0CF92A46134600244CC8 /* GeometryUtilsTest.swift in Sources */, + 91B6ADAA2AF19CB600FFE4E9 /* OSMServiceModelTest.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/apps/ios/GuideDogs/Code/App/ExternalNavigationApps.swift b/apps/ios/GuideDogs/Code/App/ExternalNavigationApps.swift index e81a3fab..71d64f5d 100644 --- a/apps/ios/GuideDogs/Code/App/ExternalNavigationApps.swift +++ b/apps/ios/GuideDogs/Code/App/ExternalNavigationApps.swift @@ -14,7 +14,7 @@ import CoreLocation /// then add to the enum and the switches below /// "deeplinks" are different than URL schemes; deeplinks can be arbitrary domains /// and will usually fall back to a web version if the corresponding app isn't installed, while URL schemes tend to be less cross-platform and need to be manually added in Info.plist. -enum ExternalNavigationApps: String, CaseIterable{ +enum ExternalNavigationApps: String, CaseIterable { case appleMaps case googleMaps case waze @@ -22,7 +22,7 @@ enum ExternalNavigationApps: String, CaseIterable{ /// Should return a localized title for each supported app var localizedTitle: String { switch self { - case .googleMaps: return "Google Maps" + case .googleMaps: return "Google Maps" case .waze: return "Waze" case .appleMaps: return "Apple Maps" } diff --git a/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift b/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift new file mode 100644 index 00000000..bc60a929 --- /dev/null +++ b/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift @@ -0,0 +1,111 @@ +// +// OSMServiceModelTest.swift +// +// +// Created by Kai on 9/29/23. +// + +import XCTest +import CoreLocation +@testable import Soundscape + +final class OSMServiceModelTest: XCTestCase { + let osm = OSMServiceModel() + let tile0_0 = VectorTile(latitude: 0, longitude: 0, zoom: 16) + let tileRPI = VectorTile(latitude: 42.73036, longitude: -73.67663, zoom: 16) + + /// Tests a point in the middle of the ocean, which should be empty + func testGetTileData_Empty() throws { + let expectation = XCTestExpectation() + + // I think [:] means all categories + osm.getTileData(tile: tile0_0, categories: [:]) {status,tiledata,err in + XCTAssertNil(err) + XCTAssertEqual(status, .success) + XCTAssertNotNil(tiledata) + + guard let tiledata = tiledata else { + XCTFail("tiledata was nil") + expectation.fulfill() + return + } + // (0, 0) is in the Atlantic Ocean + // There should be nothing here + XCTAssertTrue(tiledata.pois.isEmpty) + XCTAssertTrue(tiledata.roads.isEmpty) + XCTAssertTrue(tiledata.paths.isEmpty) + XCTAssertTrue(tiledata.intersections.isEmpty) + // But we should have generated metadata + XCTAssertFalse(tiledata.etag.isEmpty) + XCTAssertFalse(tiledata.quadkey.isEmpty) + XCTAssertGreaterThan(tiledata.ttl.addingTimeInterval(-10), Date(timeIntervalSinceNow: 0)) // it should live past now + expectation.fulfill() + } + + XCTAssertEqual(XCTWaiter.wait(for: [expectation], timeout: 10), .completed, "OSM getTileData timed out") + } + + /// Tests the tile containing Rensselaer Polytechnic Institute + func testGetTileData_RPI() throws { + let expectation = XCTestExpectation() + + // I think [:] means all categories + osm.getTileData(tile: tileRPI, categories: [:]) {status,tiledata,err in + XCTAssertNil(err) + XCTAssertEqual(status, .success) + + guard let tiledata = tiledata else { + XCTFail("tiledata was nil") + expectation.fulfill() + return + } + // RPI is a busy place with lots of stuff + // There should be a lot of data + XCTAssertFalse(tiledata.pois.isEmpty) + XCTAssertFalse(tiledata.roads.isEmpty) + XCTAssertFalse(tiledata.paths.isEmpty) + XCTAssertFalse(tiledata.intersections.isEmpty) + // We should have generated metadata + XCTAssertFalse(tiledata.etag.isEmpty) + XCTAssertFalse(tiledata.quadkey.isEmpty) + XCTAssertGreaterThan(tiledata.ttl.addingTimeInterval(-10), Date(timeIntervalSinceNow: 0)) // cache should live longer than just right now + + // RPI should be in here + guard let RPI = tiledata.pois.first(where: {$0.name == "Rensselaer Polytechnic Institute"}) else { + // assuming RPI will still exist + XCTFail("could not find RPI in its tile") + expectation.fulfill() + return + } + XCTAssertEqual(RPI.amenity, "university") + //XCTAssertEqual(RPI.geometryType, .multiPolygon) + //XCTAssertEqual(RPI.dynamicURL, "https://rpi.edu") + XCTAssertEqual(RPI.streetName, "8th Street") + XCTAssertEqual(RPI.addressLine, "110 8th Street") + XCTAssertFalse(RPI.coordinates?.isEmpty ?? true) + // Ensure RPI is roughly where it should be (with error since the exact location may shift as properties change over time) + XCTAssertEqual(RPI.centroidLatitude, 42.73036, accuracy: 0.05) + XCTAssertEqual(RPI.centroidLongitude, -73.67663, accuracy: 0.05) + + // get by id since there are multiple segments of Sage Avenue + guard let sage_ave = tiledata.roads.first(where: {$0.key == "ft-282843345"}) else { + // assuming Sage ave. will still exist + XCTFail("could not find Sage Avenue in its tile") + expectation.fulfill() + return + } + XCTAssertEqual(sage_ave.name, "Sage Avenue") + XCTAssertEqual(sage_ave.type, "road") + // XCTAssertEqual(sage_ave.geometryType, .lineString) + XCTAssertNil(sage_ave.streetName) // Streets are at themselves, so have no address + XCTAssertNil(sage_ave.addressLine) + XCTAssertNil(sage_ave.phone) // Streets don't have phone numbers + XCTAssertFalse(sage_ave.roundabout) // unless they've done work since now + + expectation.fulfill() + } + + XCTAssertEqual(XCTWaiter.wait(for: [expectation], timeout: 10), .completed, "OSM getTileData timed out") + } + +} From 4a90bf9456c1854866be81b39b57824f44c07e15 Mon Sep 17 00:00:00 2001 From: 2kai2kai2 <2kai2kai2@gmail.com> Date: Fri, 3 Nov 2023 15:55:26 -0400 Subject: [PATCH 2/6] Disabled AudioEngineTests Sometimes failing due to issues with voices. --- apps/ios/UnitTests.xctestplan | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/ios/UnitTests.xctestplan b/apps/ios/UnitTests.xctestplan index c4e2fa5b..a09a331a 100644 --- a/apps/ios/UnitTests.xctestplan +++ b/apps/ios/UnitTests.xctestplan @@ -32,6 +32,9 @@ }, "testTargets" : [ { + "skippedTests" : [ + "AudioEngineTest" + ], "target" : { "containerPath" : "container:GuideDogs.xcodeproj", "identifier" : "914DEBCC2A3CE6B9007B161C", From 90bbf1d7c00c88e46b014c5a0a16851142de1ef6 Mon Sep 17 00:00:00 2001 From: 2kai2kai2 <2kai2kai2@gmail.com> Date: Wed, 8 Nov 2023 21:09:55 -0500 Subject: [PATCH 3/6] Massive GeoJson parsing revamp The previous strategy used for parsing the OSM GeoJson data received from our services was based on old Objective-C compatibility, which is generally no longer needed. Replacing it provides significant simplification and type safety. First, we now use `JSONDecoder` and the `Decodable` protocol to parse GeoJson data, rather than the old (NS)JSONSerialization strategy. Second, `GeometryType` and `GeoJsonGeometry` have finally been merged into a single enum (which was previously not done due to Objective-C support) - This guarantees that invalid coordinate structure for a geometry type is unrepresentable - We also use CLLocationCoordinate2D instead of keeping Double arrays Since GDASpatialDataResultEntity objects (among others) are cached in the local Realm database, the schema is left unchanged, despite otherwise improving the usage - Eventually, a migration from the legacy Obj-C Realm mode to the new Swift mode should be done, which will also allow objects to be stored more naturally. I would also like to note that the various implementations of GeoJson objects do not entirely adhere to the spec, and it may be preferable to parse GeoJson objects while adhering to spec, and only then convert it into the desired forms. --- .../Code/App/Helpers/GeometryUtils.swift | 71 +-- .../GDASpatialDataResultEntity.swift | 74 ++- .../Models/Cache Models/Intersection.swift | 9 +- .../Data/Models/Cache Models/TileData.swift | 9 +- .../GDASpatialDataResult+Extensions.swift | 4 +- .../GDASpatialDataResultEntity+Distance.swift | 55 +- .../JSON Parsing/OSM/GeoJsonFeature.swift | 176 +++--- .../JSON Parsing/OSM/GeoJsonGeometry.swift | 517 +++++++++--------- .../Data/Services/Helpers/ServiceModel.swift | 6 +- .../Data/Services/OSM/OSMServiceModel.swift | 44 ++ .../Geocoding/ReverseGeocoderContext.swift | 12 +- 11 files changed, 451 insertions(+), 526 deletions(-) diff --git a/apps/ios/GuideDogs/Code/App/Helpers/GeometryUtils.swift b/apps/ios/GuideDogs/Code/App/Helpers/GeometryUtils.swift index c4ed974a..20e49b9d 100644 --- a/apps/ios/GuideDogs/Code/App/Helpers/GeometryUtils.swift +++ b/apps/ios/GuideDogs/Code/App/Helpers/GeometryUtils.swift @@ -27,30 +27,6 @@ class GeometryUtils { static let earthRadius = Double(6378137) - /// Parses a GeoJSON string and returns the coordinates and type values. - /// - /// See: - /// * https://geojson.org - /// * RFC 7946 - static func coordinates(geoJson: String) -> (type: GeometryType?, points: [Any]?) { - guard !geoJson.isEmpty, let jsonObject = GDAJSONObject(string: geoJson) else { - return (nil, nil) - } - - let geometryType: GeometryType? - if let typeString = jsonObject.string(atPath: "type") { - geometryType = GeometryType(rawValue: typeString) - } else { - geometryType = nil - } - - guard let geometry = jsonObject.array(atPath: "coordinates") else { - return (geometryType, nil) - } - - return (geometryType, geometry) - } - /// Returns whether a coordinate lies inside of the region contained within the coordinate path. /// The path is always considered closed, regardless of whether the last point equals the first or not. static func geometryContainsLocation(location: CLLocationCoordinate2D, coordinates: [CLLocationCoordinate2D]) -> Bool { @@ -307,24 +283,9 @@ class GeometryUtils { return ((cX - projX) * (cX - projX) + (cY - projY) * (cY - projY), lat, lon) } - /// Finds the closest point on an edge of the polygon (including intermediate points along edges) to the given coordinate. - /// - Returns: `nil` if the polygon is empty (i.e. no edges) - static func closestEdge(from coordinate: CLLocationCoordinate2D, on polygon: GAMultiLine) -> CLLocation? { - var coordinates: [CLLocationCoordinate2D] = [] - - // Transform to a continuous coordinates path - for line in polygon { - for point in line { - coordinates.append(point.toCoordinate()) - } - } - - return closestEdge(from: coordinate, on: coordinates) - } - /// Finds the closest point on the path (including intermediate points along edges) to the given coordinate. /// - Returns: `nil` if there are no edges (less than two points in `path`) - static func closestEdge(from coordinate: CLLocationCoordinate2D, on path: [CLLocationCoordinate2D]) -> CLLocation? { + static func closestEdge(from coordinate: CLLocationCoordinate2D, on path: [CLLocationCoordinate2D]) -> CLLocationCoordinate2D? { guard !path.isEmpty else { return nil; } @@ -333,7 +294,7 @@ class GeometryUtils { let zoomLevel: UInt = 23 let res: Double = VectorTile.groundResolution(latitude: coordinate.latitude, zoom: zoomLevel) - var closestLocation: CLLocation? + var closestLocation: CLLocationCoordinate2D? var minimumDistance = CLLocationDistanceMax for i in 0.. CLLocationCoordinate2D?` - static func centroid(geoJson: String) -> CLLocationCoordinate2D? { - guard let points = GeometryUtils.coordinates(geoJson: geoJson).points else { - return nil - } - - // Check if `points` contains one point (e.g. point) - if let point = points as? GAPoint { - return point.toCoordinate() - } - - // Check if `points` contains an array of points (e.g. line, polygon) - if let points = points as? GALine { - return GeometryUtils.centroid(coordinates: points.toCoordinates()) - } - - // Check if `points` contains a two dimensional array of points (e.g. lines, polygons) - if let points = points as? GAMultiLine { - let flattened = Array(points.toCoordinates().joined()) - return GeometryUtils.centroid(coordinates: flattened) - } - - return nil - } - /// Returns a generated coordinate representing the mean center of a given array of `CLLocation` objects. /// - Note: See `centroid(coordinates: [CLLocationCoordinate2D]) -> CLLocationCoordinate2D?` static func centroid(locations: [CLLocation]) -> CLLocationCoordinate2D? { diff --git a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift index fa6e9f01..b21d7fd3 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift @@ -60,29 +60,22 @@ class GDASpatialDataResultEntity: Object { // MARK: - Computed & Non-Realm Properties - var geometryType: GeometryType? - - private var _coordinates: [Any]? - var coordinates: [Any]? { - if _coordinates != nil { - return _coordinates - } - - // If there aren't coordinates, there is nothing to return - guard let coordinatesJson = coordinatesJson, !coordinatesJson.isEmpty else { + private var _geometry: GeoJsonGeometry? + var geometry: GeoJsonGeometry? { + if _geometry != nil { + return _geometry + } + // Otherwise, try to parse + // TODO: we might want to store that we tried before and failed + guard let geoJSON = coordinatesJson else { return nil } - - // Get the coordinates and the geometry type from the GeoJSON object - let parsedCoordinates = GeometryUtils.coordinates(geoJson: coordinatesJson) - - if let geometryType = parsedCoordinates.type { - self.geometryType = geometryType - } - - _coordinates = parsedCoordinates.points - - return _coordinates + _geometry = GeoJsonGeometry(geoJSON: geoJSON) + return _geometry + } + + var coordinates: Any? { + return geometry?.coordinates } private var _entrances: [POI]? @@ -92,6 +85,9 @@ class GDASpatialDataResultEntity: Object { } // Only POIs with non-point geometries can have entrances + if case .point = geometry { + return nil + } guard coordinates != nil, let jsonData = entrancesJson?.data(using: .utf8) else { return nil @@ -122,8 +118,8 @@ class GDASpatialDataResultEntity: Object { /// Returns the names of properties which Realm should ignore static override func ignoredProperties() -> [String] { - return ["geometryType", - "_coordinates", + return ["_geometry", + "geometry", "coordinates", "_entrances", "entrances"] @@ -183,20 +179,16 @@ class GDASpatialDataResultEntity: Object { // Set geolocation information if let geometry = feature.geometry { - if geometry.type == .point, let point = geometry.point, point.count > 1 { - latitude = point[1] - longitude = point[0] - } else { - coordinatesJson = geometry.coordinateJSON + if case .point(let point) = geometry { + latitude = point.latitude + longitude = point.longitude + } else if let json_data = try? JSONEncoder().encode(geometry) { + coordinatesJson = String(data: json_data, encoding: .utf8) } - if let centroid = geometry.centroid { - centroidLatitude = centroid[1] - centroidLongitude = centroid[0] - } else { - centroidLatitude = latitude - centroidLongitude = longitude - } + let centroid = geometry.centroid + centroidLatitude = centroid.latitude + centroidLongitude = centroid.longitude } // Road specific metadata @@ -225,14 +217,12 @@ class GDASpatialDataResultEntity: Object { // MARK: - Geometries /// Returns whether a coordinate lies inside the entity. - /// - note: This is only valid for entities that contain geometries, such as buildings. + /// - note: This is only valid for entities that contain geometries with an area (polygons and multiPolygons), such as buildings. func contains(location: CLLocationCoordinate2D) -> Bool { - guard let points = self.coordinates as? GAMultiLine else { return false } - let coordinates = points.toCoordinates() - - guard !coordinates.isEmpty else { return false } - - return GeometryUtils.geometryContainsLocation(location: location, coordinates: coordinates.first!) + guard let geometry = geometry else { + return false + } + return geometry.withinArea(location) } // MARK: `NSObject` diff --git a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/Intersection.swift b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/Intersection.swift index c4d3c3d7..26fc218c 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/Intersection.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/Intersection.swift @@ -231,12 +231,9 @@ class Intersection: Object, Locatable, Localizable { self.key += String(roadId) } - if let lat = feature.geometry?.point?[1] { - latitude = lat - } - - if let lon = feature.geometry?.point?[0] { - longitude = lon + if case .point(let coordinate) = feature.geometry { + latitude = coordinate.latitude + longitude = coordinate.longitude } self.key += String(latitude) + String(longitude) diff --git a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift index c6c1f1c7..7b6aeaa2 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift @@ -73,7 +73,7 @@ class TileData: Object { return VectorTile(quadKey: quadkey) } - convenience init(withParsedData json: [String: Any], quadkey: String, etag: String, superCategories: SuperCategories) { + convenience init(withParsedData json: OSMTileDataJson, quadkey: String, etag: String, superCategories: SuperCategories) { self.init() // Get the vector tile info @@ -85,12 +85,7 @@ class TileData: Object { // Store the etag for checking future updates self.etag = etag - guard let featuresJson = json["features"] as? [Any] else { return } - - for featureJson in featuresJson { - // Try to parse the feature - the GeoJsonFeature initializer is failable - guard let feature = GeoJsonFeature(json: featureJson as! [String: Any], superCategories: superCategories) else { continue } - + for feature in json.features { // Check if it is a road, intersection, etc. if feature.superCategory == .roads { roads.append(GDASpatialDataResultEntity(feature: feature)!) diff --git a/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResult+Extensions.swift b/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResult+Extensions.swift index 60b958a6..d12adfe3 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResult+Extensions.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResult+Extensions.swift @@ -30,8 +30,8 @@ extension GDASpatialDataResultEntity: SelectablePOI { func closestLocation(from location: CLLocation, useEntranceIfAvailable: Bool) -> CLLocation { if useEntranceIfAvailable, let entrance = closestEntrance(from: location) { return entrance.closestLocation(from: location) - } else if let edge = closestEdge(from: location) { - return edge + } else if let edge = closestEdge(from: location.coordinate) { + return CLLocation(edge) } return CLLocation(latitude: latitude, longitude: longitude) diff --git a/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Distance.swift b/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Distance.swift index b382832e..7fb6a68c 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Distance.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Distance.swift @@ -31,59 +31,8 @@ extension GDASpatialDataResultEntity { return closestEntrance } - func closestEdge(from location: CLLocation) -> CLLocation? { - guard let coordinates = coordinates else { - return nil - } - // If we have coordinates, use those to update the distance and bearing, - // otherwise, use the `latitude` and `longitude` properties - - var closestLocation: CLLocation? - var minimumDistance = CLLocationDistanceMax - - if geometryType == .lineString || geometryType == .multiPoint { - guard let coordinates = coordinates as? GALine else { - return nil - } - - for coordinate in coordinates { - let lat = coordinate[1] - let lon = coordinate[0] - - let newLocation = CLLocation(latitude: lat, longitude: lon) - let distance = newLocation.distance(from: location) - - if distance < minimumDistance { - closestLocation = newLocation - minimumDistance = distance - } - } - } else if geometryType == .multiLineString || geometryType == .polygon { - guard let polygon = coordinates as? GAMultiLine else { - return nil - } - - closestLocation = GeometryUtils.closestEdge(from: location.coordinate, on: polygon) - } else if geometryType == .multiPolygon { - guard let polygons = coordinates as? GAMultiLineCollection else { - return nil - } - - for polygon in polygons { - guard let newLocation = GeometryUtils.closestEdge(from: location.coordinate, on: polygon) else { - continue - } - - let distance = newLocation.distance(from: location) - - if distance < minimumDistance { - closestLocation = newLocation - minimumDistance = distance - } - } - } - - return closestLocation + func closestEdge(from location: CLLocationCoordinate2D) -> CLLocationCoordinate2D? { + return geometry?.closestEdge(to: location) } } diff --git a/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift b/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift index 1bc9e9eb..eeb65e1c 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift @@ -8,37 +8,40 @@ import Foundation -struct GeoJsonKeys { - /// Key for accessing the feature_type string of a GeoJson feature - static let featureType = "feature_type" - - /// Key for accessing the feature_value string of a GeoJson feature - static let featureValue = "feature_value" - - /// Key for accessing the geometry object of a GeoJson feature - static let geometry = "geometry" - - /// Key for accessing the OSM IDs array of a GeoJson feature - static let osmIds = "osm_ids" - - /// Key for accessing the priority integer of a GeoJson feature - static let priority = "priority" - - /// Key for accessing the properties dictionary of a GeoJson feature - static let properties = "properties" - - /// Key for accessing the name property from the properties dictionary of a GeoJson feature - static let name = "name" - - /// Key for accessing the ref property from the properties dictionary of a GeoJson feature - static let ref = "ref" - - /// Prefix string which all localized names have in the properties dictionary of a GeoJson feature - static let i18nNamePrefix = "name:" +enum GeoJsonFeatureError: Error { + /// Thrown when the json's `type` property is not `"Feature"` + case incorrectTypeField + /// When a feature does not have a name + /// + /// Optionally contains a string containing its `feature_type` and `feature_value`: `=`. + /// + /// While the [GeoJson spec](https://datatracker.ietf.org/doc/html/rfc7946#section-3.2) does not require a name, we are unable to handle features we can't find a name for (Note: I'm not sure why, so I may just be replicating a historical bug). + case nameless(propVal: String?) } -class GeoJsonFeature { - +final class GeoJsonFeature: Decodable { + enum CodingKeys: String, CodingKey { + /// Key for accessing the `feature_type` property of a GeoJson feature + case feature_type + /// Key for accessing the `feature_value` property of a GeoJson feature + case feature_value + /// Key for accessing the `geometry` (``GeoJsonGeometry``) object of a GeoJson feature + case geometry + /// Key for accessing the OSM IDs array of a GeoJson feature + case osm_ids + /// Key for accessing the `priority` integer of a GeoJson feature + case priority + /// Key for accessing the `properties` dictionary of a GeoJson feature + case properties + /// Key for accessing the `name` property from the properties dictionary of a GeoJson feature + case name + /// Key for accessing the `ref` property from the properties dictionary of a GeoJson feature + case ref + /// Prefix string which all localized names have in the properties dictionary of a GeoJson feature + case i18nNamePrefix = "name:" + /// Key for the json `type`, should always be `"Feature"` + case type + } // MARK: Properties /// Name of the feature @@ -77,6 +80,7 @@ class GeoJsonFeature { var superCategory: SuperCategory = .undefined /// Geometry of this feature + /// While Features are required to have a `geometry` member, it may be `null` for unlocated features, represented here as `nil` var geometry: GeoJsonGeometry? var isCrossing = false @@ -85,18 +89,27 @@ class GeoJsonFeature { // MARK: Initializers - init?(json: [String: Any], superCategories: SuperCategories) { - // Parse the OSM IDs - if let ids = GeoJsonFeature.extractIDs(from: json) { - osmIds = ids + /// ``JSONDecoder`` + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + + // Object's `type` property must be "Feature" + let parsed_type = try container.decode(String.self, forKey: .type) + guard parsed_type == "Feature" else { + throw GeoJsonFeatureError.incorrectTypeField } - // Parse the general feature information - type = json[GeoJsonKeys.featureType] as? String ?? "" - value = json[GeoJsonKeys.featureValue] as? String ?? "" + // Parse OSM IDs + let parsed_ids = try? container.decode([Int].self, forKey: .osm_ids) + osmIds = parsed_ids?.map({ "ft\($0)" }) ?? [] - let nameObjects = GeoJsonFeature.extractNames(from: json) + // Parse the general feature information + let parsed_feature_type = try? container.decode(String.self, forKey: .feature_type) + type = parsed_feature_type ?? "" + let parsed_feature_value = try? container.decode(String.self, forKey: .feature_value) + value = parsed_feature_value ?? "" + let nameObjects = GeoJsonFeature.extractNames(from: container) // Entities should have a name or a tag (which will be transformed to a localized name at runtime) guard nameObjects.name != nil || nameObjects.tag != nil else { // Collect information about this nameless feature for later analysis @@ -106,28 +119,21 @@ class GeoJsonFeature { GeoJsonFeature.unhandledNamelessFeatures.insert(propVal) } - return nil + throw GeoJsonFeatureError.nameless(propVal: propVal) } name = nameObjects.name names = nameObjects.names nameTag = nameObjects.tag - let featureProperties = json[GeoJsonKeys.properties] as? [String: String] - - if let properties = featureProperties { - self.properties = properties + // note: `properties` isn't required by spec to be [String: String], but appears to always be in data from OSM + if let featureProperties = try? container.decode([String: String].self, forKey: .properties) { + self.properties = featureProperties - if let ref = properties[GeoJsonKeys.ref] { + if let ref = properties[CodingKeys.ref.rawValue] { self.ref = ref } } - - // Parse the geometry information - features must have a geometry according to the GeoJSON spec - guard let geoData = json[GeoJsonKeys.geometry] as? [String: Any] else { - return nil - } - if GeoJsonFeature.hasTag("footway=crossing", props: properties) || nameObjects.tag == GeoJsonFeature.FeatureNameTag.crossing { isCrossing = true @@ -137,26 +143,18 @@ class GeoJsonFeature { isRoundabout = true } - // Ensure we have a valid geometry or return nil otherwise - guard let geo = GeoJsonGeometry(geoJSON: geoData) else { - return nil - } + // TODO: it is unclear if geometry not existing should cause the feature to be rejected + geometry = try? container.decode(GeoJsonGeometry.self, forKey: .geometry) // Fix geometries for crossings with LineString geometries - if isCrossing && geo.type == .lineString { - if let median = geo.getLineMedian() { - geometry = GeoJsonGeometry(point: median) - } else { - geometry = geo - } - } else { - geometry = geo + if isCrossing, case .lineString = geometry, + let median = geometry?.getLineMedian() { + geometry = GeoJsonGeometry(point: median) } // Parse the priority of the feature - if let parsedPriority = json[GeoJsonKeys.priority] as? UInt { - priority = parsedPriority - } + let parsedPriority = try? container.decode(UInt.self, forKey: .priority) + priority = parsedPriority ?? 0 // // Deal with super categories and missing names: @@ -224,6 +222,8 @@ class GeoJsonFeature { } } +#if false + // TODO: this is currently unused (we never passed in any supercategories, even before refactoring), but should probably be reviewed to determine if it should get implemented // General Case: look up the category in the list of categories we got from the server var applicableCategories: Set = [] for (key, value) in properties { @@ -238,6 +238,7 @@ class GeoJsonFeature { if let prioritizedCategory = GeoJsonFeature.prioritizedCategories.first(where: { applicableCategories.contains($0) }) { superCategory = prioritizedCategory } +#endif } init(copyFrom: GeoJsonFeature) { @@ -261,28 +262,27 @@ class GeoJsonFeature { self.properties[key] = value } + // enums are pass-by-value self.priority = copyFrom.priority self.superCategory = copyFrom.superCategory - self.geometry = GeoJsonGeometry(copyFrom: copyFrom.geometry) + self.geometry = copyFrom.geometry } func decomposePathToStartAndEndCrossings() -> (start: GeoJsonFeature, end: GeoJsonFeature)? { let startCopy = GeoJsonFeature(copyFrom: self) let endCopy = GeoJsonFeature(copyFrom: self) - guard let createdStart = startCopy.geometry?.clipToFirstPoint(), let createdEnd = endCopy.geometry?.clipToLastPoint() else { + guard let first = geometry?.first, let last = geometry?.last else { return nil } - if createdStart && createdEnd { - // Make crossing start and end points mobility POIs by default - startCopy.superCategory = SuperCategory.mobility - endCopy.superCategory = SuperCategory.mobility + startCopy.geometry = .point(coordinates: first) + startCopy.superCategory = SuperCategory.mobility + + endCopy.geometry = .point(coordinates: last) + endCopy.superCategory = SuperCategory.mobility - return (startCopy, endCopy) - } - - return nil + return (startCopy, endCopy) } /// Check if a set of properties contains a key-value pair matching the input tag. Tags are @@ -335,28 +335,28 @@ class GeoJsonFeature { extension GeoJsonFeature { fileprivate static func extractIDs(from json: [String: Any]) -> [String]? { - guard let ids = json[GeoJsonKeys.osmIds] as? [Int] else { return nil } + guard let ids = json[CodingKeys.osm_ids.rawValue] as? [Int] else { return nil } return ids.map({ (id) -> String in return "ft\(id)" }) } - fileprivate static func extractNames(from json: [String: Any]) -> (name: String?, names: [String: String]?, tag: String?) { - guard let properties = json[GeoJsonKeys.properties] as? [String: String] else { return (nil, nil, nil) } + fileprivate static func extractNames(from container: KeyedDecodingContainer) -> (name: String?, names: [String: String]?, tag: String?) { + guard let properties = try? container.decode([String: String].self, forKey: .properties) else { + return (nil, nil, nil) + } - if let value = json[GeoJsonKeys.featureValue] as? String { + if let value = try? container.decode(String.self, forKey: .feature_value), + value == "gd_intersection" { // Special case: intersections (calculated intersections have value "gd_intersection" by definition) - if value == "gd_intersection" { - return (name: GDLocalizedString("osm.tag.intersection"), nil, "intersection") - } + return (name: GDLocalizedString("osm.tag.intersection"), nil, "intersection") } - if let type = json[GeoJsonKeys.featureType] as? String { + if let type = try? container.decode(String.self, forKey: .feature_type), + type == "gd_entrance_list"{ // Special case: entrance list (calculated entrance lists have type "gd_entrance_list" by definition) - if type == "gd_entrance_list" { - return (name: GDLocalizedString("directions.amenity.entrance_list"), nil, nil) - } + return (name: GDLocalizedString("directions.amenity.entrance_list"), nil, nil) } var name: String? @@ -364,12 +364,12 @@ extension GeoJsonFeature { // Try to extract name and i18n names for (property, value) in properties { - if property == GeoJsonKeys.name { + if property == CodingKeys.name.rawValue { name = value continue } - if let range = property.range(of: GeoJsonKeys.i18nNamePrefix) { + if let range = property.range(of: CodingKeys.i18nNamePrefix.rawValue) { var languageCode = property languageCode.removeSubrange(range) @@ -382,7 +382,7 @@ extension GeoJsonFeature { } // Custom name extractions - if name == nil, let featureValue = json[GeoJsonKeys.featureValue] as? String { + if name == nil, let featureValue = try? container.decode(String.self, forKey: .feature_value) { if featureValue == "atm", let atm = properties["operator"], atm.count > 0 { name = atm } else if featureValue == "bank" { diff --git a/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonGeometry.swift b/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonGeometry.swift index a7f7d41d..e370ace9 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonGeometry.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonGeometry.swift @@ -7,36 +7,37 @@ // import Foundation +import CoreLocation -/// Geometry types from the GeoJSON spec ([link](https://tools.ietf.org/html/rfc7946)). -/// Note that this looks like a very odd/inefficient way to define an `Enum:String` type. That is -/// because it is. This was only defined this way so as to allow for Objective-C support. In -/// Objective-C, enums must have an integer type, hence the additional work to allow for strings. +/// GeoJsonGeometry is a Swift representation of the `geometry` property in a GeoJSON feature. +/// Geometry types from the GeoJSON [spec](https://tools.ietf.org/html/rfc7946). +/// +/// - `point`: The geometry consists of a single point +/// - `lineString`: The geometry consists of at least two points +/// - `multiPoint`: The geometry consists of any number of points +/// - `polygon`: The geometry consists of an array of linear ring coordinate arrays (see spec above) +/// - `multiLineString`: The geometry consists of an array of lineStrings +/// - `multiPolygon`: The geometry consists of an array of polygons /// -/// - point: The geometry consists of a single point -/// - lineString: The geometry consists of at least two points -/// - multiPoint: The geometry consists of any number of points -/// - polygon: The geometry consists of an array of linear ring coordinate arrays (see spec above) -/// - multiLineString: The geometry consists of an array of lineStrings -/// - multiPolygon: The geometry consists of an array of polygons -@objc public enum GeometryType: Int, RawRepresentable { +/// Note: every `coordinates` list and sub-list should have at least one point in it (or more depending on spec) +public enum GeoJsonGeometry: Equatable, Codable { /// The geometry consists of a single point - case point + case point(coordinates: CLLocationCoordinate2D) /// The geometry consists of at least two points - case lineString + case lineString(coordinates: [CLLocationCoordinate2D]) /// The geometry consists of any number of points - case multiPoint + case multiPoint(coordinates: [CLLocationCoordinate2D]) /// The geometry consists of an array of linear ring coordinate arrays (see spec above) - case polygon + case polygon(coordinates: [[CLLocationCoordinate2D]]) /// The geometry consists of an array of lineStrings - case multiLineString + case multiLineString(coordinates: [[CLLocationCoordinate2D]]) /// The geometry consists of an array of polygons - case multiPolygon + case multiPolygon(coordinates: [[[CLLocationCoordinate2D]]]) public typealias RawValue = String @@ -57,294 +58,312 @@ import Foundation } } - public init?(rawValue: RawValue) { - switch rawValue { + enum GeoJsonGeometryError: Error { + /// When the `coordinates` property is of the correct type, but is semantically incorrect (e.g. being empty, or missing longitudes) + case invalidCoordinates + /// A general error + case notParsable + } + + enum CodingKeys: String, CodingKey { + case type + case coordinates + } + + /// Decodes JSON or similarly structured decoders + public init(from decoder: Decoder) throws { + let values = try decoder.container(keyedBy: CodingKeys.self) + let typeString = try values.decode(String.self, forKey: .type) + switch typeString { case "Point": - self = .point + let coord = try values.decode([Double].self, forKey: .coordinates) + guard let x = GeoJsonGeometry.toCoordinate(coord) else { + throw GeoJsonGeometryError.invalidCoordinates + } + self = .point(coordinates: x) case "LineString": - self = .lineString + let coords = try values.decode([[Double]].self, forKey: .coordinates) + guard let x = GeoJsonGeometry.toCoordinates(coords), x.count >= 2 else { + throw GeoJsonGeometryError.invalidCoordinates + } + self = .lineString(coordinates: x) case "MultiPoint": - self = .multiPoint + let coords = try values.decode([[Double]].self, forKey: .coordinates) + guard let x = GeoJsonGeometry.toCoordinates(coords), x.count > 0 else { + throw GeoJsonGeometryError.invalidCoordinates + } + self = .multiPoint(coordinates: x) case "Polygon": - self = .polygon + // Ensuring correctness: https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.6 + let coords = try values.decode([[[Double]]].self, forKey: .coordinates) + guard let x = GeoJsonGeometry.toCoordinates(coords), + x.first?.first != nil, + x.allSatisfy({ $0.count >= 4 && $0.first == $0.last }) else { + throw GeoJsonGeometryError.invalidCoordinates + } + self = .polygon(coordinates: x) case "MultiLineString": - self = .multiLineString + let coords = try values.decode([[[Double]]].self, forKey: .coordinates) + guard let x = GeoJsonGeometry.toCoordinates(coords), + x.count > 0, x.allSatisfy({ $0.count >= 2 }) else { + throw GeoJsonGeometryError.invalidCoordinates + } + self = .multiLineString(coordinates: x) case "MultiPolygon": - self = .multiPolygon + let coords = try values.decode([[[[Double]]]].self, forKey: .coordinates) + guard let x = GeoJsonGeometry.toCoordinates(coords), + x.first?.first?.first != nil else { + throw GeoJsonGeometryError.invalidCoordinates + } + self = .multiPolygon(coordinates: x) default: - self = .multiPolygon + throw DecodingError.dataCorrupted(DecodingError.Context(codingPath: values.codingPath, debugDescription: "Invalid GeoJsonGeometry type \"\(typeString)\"")) } } -} - -/// GeoJsonGeometry is a Swift representation of the `geometry` property in a GeoJSON feature. -class GeoJsonGeometry { - - /// Type of this geometry from the parsed GeoJSON - private(set) var type: GeometryType - /// String encoding of the coordinates in this geometry - private(set) var coordinateJSON: String - - /// Coordinates from the parsed GeoJSON - private(set) var coordinates: [[[[Double]]]]? - - /// A single lat/lon coordinate in the case self.type is `Point` - var point: [Double]? { - guard type == .point else { + public init?(geoJSON: String) { + guard let data = geoJSON.data(using: .utf8), + let json = try? JSONDecoder().decode(GeoJsonGeometry.self, from: data) else { return nil } - - return coordinates?[0][0][0] + self = json } - /// An array of points in the case self.type is `LineString` or `MultiPoint` - var points: [[Double]]? { - guard type == .lineString || type == .multiPoint else { - return nil - } - - return coordinates?[0][0] + public init(point: CLLocationCoordinate2D) { + self = .point(coordinates: point) } - /// An array of lines in the case self.type is `Polygon` or `MultiLineString` - var polygon: [[[Double]]]? { - guard type == .polygon || type == .multiLineString else { - return nil - } - - return coordinates?[0] - } - - /// An array of polygons in the case self.type is `MultiPolygon` - var multipolygon: [[[[Double]]]]? { - guard type == .multiPolygon else { - return nil + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(self.rawValue, forKey: .type) + switch self { + case .point(coordinates: let coordinates): + try container.encode([coordinates.longitude, coordinates.latitude], forKey: .coordinates) + case .lineString(coordinates: let coordinates), .multiPoint(coordinates: let coordinates): + let expanded = coordinates.map(GeoJsonGeometry.into_coord_pair) + try container.encode(expanded, forKey: .coordinates) + case .polygon(coordinates: let coordinates), .multiLineString(coordinates: let coordinates): + let expanded = coordinates.map({$0.map(GeoJsonGeometry.into_coord_pair)}) + try container.encode(expanded, forKey: .coordinates) + case .multiPolygon(coordinates: let coordinates): + let expanded = coordinates.map({$0.map({$0.map(GeoJsonGeometry.into_coord_pair)})}) + try container.encode(expanded, forKey: .coordinates) } - - return coordinates } - /// The geometry's centroid, represented as `[longitude, latitude]` - var centroid: [Double]? { - guard let points = coordinates else { - return nil - } - - let flattenedCoordinates = Array(points.toCoordinates().joined().joined()) - guard let centroid = GeometryUtils.centroid(coordinates: flattenedCoordinates) else { - return nil + /// note: the centroid is based on the implementation in ``GeometryUtils`` and may have any of the same issues. + var centroid: CLLocationCoordinate2D { + switch self { + case .point(coordinates: let coordinates): + return coordinates + case .lineString(coordinates: let coordinates), .multiPoint(coordinates: let coordinates): + return GeometryUtils.centroid(coordinates: coordinates)! + case .polygon(coordinates: let coordinates), .multiLineString(coordinates: let coordinates): + return GeometryUtils.centroid(coordinates: coordinates.flatMap({$0}))! + case .multiPolygon(coordinates: let coordinates): + return GeometryUtils.centroid(coordinates: coordinates.flatMap({$0.flatMap({$0})}))! } - - return [centroid.longitude, centroid.latitude] } - init?(geoJSON: [String: Any]) { - // Parse the geometry type - guard let typeString = geoJSON["type"] as? String else { - return nil - } - - guard let parsedType = GeometryType(rawValue: typeString) else { + /// Finds the median point in the list of points, or if there are an even number of points, gets the mid point between the two median points. Returns nil if the geometry isn't a LineString + func getLineMedian() -> CLLocationCoordinate2D? { + guard case .lineString(coordinates: let coordinates) = self else { return nil } - type = parsedType - - // Save the coordinates JSON - do { - let data = try JSONSerialization.data(withJSONObject: geoJSON) - coordinateJSON = String(data: data, encoding: String.Encoding.utf8)! - } catch { + guard !coordinates.isEmpty else { return nil } - switch type { - case .point: - if let pt = geoJSON["coordinates"] as? [Double] { - coordinates = [[[pt]]] - } - case .lineString: - if let ln = geoJSON["coordinates"] as? [[Double]] { - coordinates = [[ln]] - } - case .multiPoint: - if let ln = geoJSON["coordinates"] as? [[Double]] { - coordinates = [[ln]] - } - case .polygon: - if let poly = geoJSON["coordinates"] as? [[[Double]]] { - coordinates = [poly] - } - case .multiLineString: - if let poly = geoJSON["coordinates"] as? [[[Double]]] { - coordinates = [poly] - } - case .multiPolygon: - if let poly = geoJSON["coordinates"] as? [[[[Double]]]] { - coordinates = poly - } + if coordinates.count % 2 == 1 { + return coordinates[coordinates.count / 2] } - // Validate that the provided coordinates are valid and not malformed - guard validate() else { - return nil + let first = coordinates[(coordinates.count / 2) - 1] + let last = coordinates[coordinates.count / 2] + return first.coordinateBetween(coordinate: last, distance: first.distance(from: last) / 2) + } + + /// Find the very first point - in the typical line case, this works just fine. In the multipolygon sort of case, this will be weird + var first: CLLocationCoordinate2D { + switch self { + case .point(coordinates: let coordinates): + return coordinates + case .lineString(coordinates: let coordinates): + return coordinates.first! + case .multiPoint(coordinates: let coordinates): + return coordinates.first! + case .polygon(coordinates: let coordinates): + return coordinates.first!.first! + case .multiLineString(coordinates: let coordinates): + return coordinates.first!.first! + case .multiPolygon(coordinates: let coordinates): + return coordinates.first!.first!.first! } } - init?(point: [Double]) { - guard point.count == 2 else { - return nil + /// Find the very last point - in the typical line case, this works just fine. In the multipolygon sort of case, this will be weird + var last: CLLocationCoordinate2D { + switch self { + case .point(coordinates: let coordinates): + return coordinates + case .lineString(coordinates: let coordinates): + return coordinates.last! + case .multiPoint(coordinates: let coordinates): + return coordinates.last! + case .polygon(coordinates: let coordinates): + return coordinates.last!.last! + case .multiLineString(coordinates: let coordinates): + return coordinates.last!.last! + case .multiPolygon(coordinates: let coordinates): + return coordinates.last!.last!.last! } - - type = .point - coordinates = [[[point]]] - coordinateJSON = "{\"coordinates\": [\(point[0]),\(point[1])], \"type\": \"Point\"}" } - - init?(copyFrom: GeoJsonGeometry?) { - guard let copy = copyFrom else { - return nil + + var coordinates: Any { + switch self { + case .point(coordinates: let coordinates): + return coordinates + case .lineString(coordinates: let coordinates): + return coordinates + case .multiPoint(coordinates: let coordinates): + return coordinates + case .polygon(coordinates: let coordinates): + return coordinates + case .multiLineString(coordinates: let coordinates): + return coordinates + case .multiPolygon(coordinates: let coordinates): + return coordinates } - self.type = copy.type - self.coordinateJSON = copy.coordinateJSON - self.coordinates = copy.coordinates } - func validate() -> Bool { - switch type { - case .point: - // A point must have 2 values (lat, lon) - if let point = point, point.count == 2 { - return true + /// If the geometry contains the point. + /// Since only `polygon` and `multiPolygon` geometries have any area (and thus contain stuff), all other geometry types will return false. + /// + /// true if within the region of the first (outer) ring, but none of the other rings (holes) + func withinArea(_ point: CLLocationCoordinate2D) -> Bool { + switch self { + case .polygon(coordinates: let coordinates): + guard GeometryUtils.geometryContainsLocation(location: point, coordinates: coordinates.first!) else { + return false } - - case .lineString, .multiPoint: - // A lineString or multiPoint geometry must have 1 or more points and those points must have 2 values (lat, lon) - if let points = points, points.count > 0 { - return !points.contains(where: { $0.count != 2 }) + for i in 1.. 0 { - return !polygon.contains(where: { (points) -> Bool in - return points.count == 0 || points.contains(where: { $0.count != 2}) - }) + return true + case .multiPolygon(coordinates: let polys): + return polys.contains(where: { poly in + guard GeometryUtils.geometryContainsLocation(location: point, coordinates: poly.first!) else { + return false + } + for i in 1.. CLLocationCoordinate2D? { + switch self { + case .point(coordinates: let coordinates): + return coordinates + case .lineString(coordinates: let coordinates), .multiPoint(coordinates: let coordinates): + return GeometryUtils.closestEdge(from: point, on: coordinates) + case .polygon(coordinates: let coordinates), .multiLineString(coordinates: let coordinates): + var closestLocation: CLLocationCoordinate2D? = nil + var minimumDistance = CLLocationDistanceMax + for line in coordinates { + guard let closest = GeometryUtils.closestEdge(from: point, on: line) else { + continue + } + let distance = closest.distance(from: point) + if distance < minimumDistance { + closestLocation = closest + minimumDistance = distance + } } - - case .multiPolygon: - // A multipolygon geometry must have 1 or more polygons, each with 1 or more linesStrings, each with 1 or more points, and those points must have 2 values (lat, lon) - if let multipolygon = multipolygon, multipolygon.count > 0 { - return !multipolygon.contains(where: { (polygon) -> Bool in - return polygon.count == 0 || polygon.contains(where: { (points) -> Bool in - return points.count == 0 || points.contains(where: { $0.count != 2 }) - }) - }) + return closestLocation + case .multiPolygon(coordinates: let coordinates): + var closestLocation: CLLocationCoordinate2D? = nil + var minimumDistance = CLLocationDistanceMax + for polygon in coordinates { + for line in polygon { + guard let closest = GeometryUtils.closestEdge(from: point, on: line) else { + continue + } + let distance = closest.distance(from: point) + if distance < minimumDistance { + closestLocation = closest + minimumDistance = distance + } + } } + return closestLocation } - - return false + } +} + + +// MARK: Conversion Functions + +extension GeoJsonGeometry { + private static func into_coord_pair(_ coord: CLLocationCoordinate2D) -> [CLLocationDegrees] { + return [coord.latitude, coord.longitude] } - /// Finds the median point in the list of points, or if there are an even number of points, gets the mid point between the two median points. Returns nil if the geometry isn't a LineString - func getLineMedian() -> [Double]? { - guard type == .lineString else { + + + /// Transform to a `CLLocationCoordinate2D` object. + private static func toCoordinate(_ arr: [Double]?) -> CLLocationCoordinate2D? { + guard let arr = arr, arr.count >= 2 else { return nil } - - guard let pts = coordinates?[0][0] else { + return CLLocationCoordinate2DMake(arr[1], arr[0]) + } + /// Transform to an array of `CLLocationCoordinate2D` objects. + private static func toCoordinates(_ arr: [[Double]]?) -> [CLLocationCoordinate2D]? { + guard let arr = arr else { return nil } - - guard pts.count > 1 else { - if pts.count == 1 { - return pts[0] + return try? arr.map({ (point) -> CLLocationCoordinate2D in + guard let coord = toCoordinate(point) else { + throw GeoJsonGeometryError.notParsable } - - return nil - } - - if pts.count % 2 == 1 { - return pts[pts.count / 2] - } - - let first = pts[(pts.count / 2) - 1] - let last = pts[pts.count / 2] - - let toRadians = .pi / 180.0 - let toDegrees = 180.0 / .pi - - let phi1 = first[1] * toRadians - let phi2 = last[1] * toRadians - let lambda1 = first[0] * toRadians - let lambda2 = last[0] * toRadians - - let bX = cos(phi2) * cos(lambda2 - lambda1) - let bY = cos(phi2) * sin(lambda2 - lambda1) - let lat = toDegrees * atan2(sin(phi1) + sin(phi2), sqrt((cos(phi1) + bX) * (cos(phi1) + bX) + bY * bY)) - let lon = fmod((lambda1 + atan2(bY, cos(phi1) + bX)) * toDegrees + 540, 360.0) - 180.0 - - return [lon, lat] + return coord + }) } - - func clipToFirstPoint() -> Bool { - guard let firstPoint = coordinates?[0][0][0] else { - return false + /// Transform to an array of `CLLocationCoordinate2D` objects. + private static func toCoordinates(_ arr: [[[Double]]]?) -> [[CLLocationCoordinate2D]]? { + guard let arr = arr else { + return nil } - - do { - let jsonObj = try JSONSerialization.data(withJSONObject: ["coordinates": firstPoint, "type": "Point"]) - - guard let jsonStr = String(data: jsonObj, encoding: String.Encoding.utf8) else { - return false + return try? arr.map({ (point) -> [CLLocationCoordinate2D] in + guard let coord = toCoordinates(point) else { + throw GeoJsonGeometryError.notParsable } - - // We were able to properly encode the new clipped geometry, so update everything - type = .point - coordinates = [[[firstPoint]]] - coordinateJSON = jsonStr - } catch { - return false - } - - return true + return coord + }) } - - func clipToLastPoint() -> Bool { - // Find the very last point - in the typical line case, this works just fine. In the multipolygon sort of case, this will be weird - guard let firstIdx = coordinates?.count else { - return false - } - - guard let secondIdx = coordinates?[firstIdx - 1].count else { - return false - } - - guard let thirdIdx = coordinates?[firstIdx - 1][secondIdx - 1].count else { - return false - } - - guard let lastPoint = coordinates?[firstIdx - 1][secondIdx - 1][thirdIdx - 1] else { - return false + /// Transform to an array of `CLLocationCoordinate2D` objects. + private static func toCoordinates(_ arr: [[[[Double]]]]?) -> [[[CLLocationCoordinate2D]]]? { + guard let arr = arr else { + return nil } - - do { - let jsonObj = try JSONSerialization.data(withJSONObject: ["coordinates": lastPoint, "type": "Point"]) - - guard let jsonStr = String(data: jsonObj, encoding: String.Encoding.utf8) else { - return false + return try? arr.map({ (point) -> [[CLLocationCoordinate2D]] in + guard let coord = toCoordinates(point) else { + throw GeoJsonGeometryError.notParsable } - - // We were able to properly encode the new clipped geometry, so update everything - type = .point - coordinates = [[[lastPoint]]] - coordinateJSON = jsonStr - } catch { - return false - } - - return true + return coord + }) } } diff --git a/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift b/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift index ad246a94..3bec6152 100644 --- a/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift +++ b/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift @@ -148,7 +148,7 @@ class ServiceModel { return json as? [String: Any] } - static func validateJsonResponse(request: URLRequest, response: URLResponse?, data: Data?, error: Error?, callback: @escaping (HTTPStatusCode, Error?) -> Void) -> (HTTPStatusCode, String, [String: Any])? { + static func validateJsonResponse(request: URLRequest, response: URLResponse?, data: Data?, error: Error?, callback: @escaping (HTTPStatusCode, Error?) -> Void) -> (HTTPStatusCode, String, OSMTileDataJson)? { // Some more housekeeping ServiceModel.logNetworkResponse(response, request: request, error: error) @@ -194,7 +194,7 @@ class ServiceModel { } // If we get this far, then the data property should not be nil, and it should be valid JSON - guard let data = data, let parsed = try? JSONSerialization.jsonObject(with: data), let json = parsed as? [String: Any] else { + guard let data = data, let parsed = try? JSONDecoder().decode(OSMTileDataJson.self, from: data) else { DispatchQueue.main.async { callback(status, ServiceError.jsonParseFailed) } @@ -202,7 +202,7 @@ class ServiceModel { return nil } - return (status, newEtag, json) + return (status, newEtag, parsed) } static func logNetworkRequest(_ request: URLRequest) { diff --git a/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift b/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift index 61d0090f..8dc32fb7 100644 --- a/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift +++ b/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift @@ -23,6 +23,50 @@ enum ServiceError: Error { case jsonParseFailed } +final class FailableDecode: Decodable { + var result: Result + + public init(from decoder: Decoder) throws { + result = Result(catching: { try T(from: decoder) }) + } +} + +/// Represents the parsed json response from the OSM tiles service +final class OSMTileDataJson: Decodable { + var features: [GeoJsonFeature] + + private enum CodingKeys: CodingKey { + /// Contains an array of ``GeoJsonFeature``s + case features + /// Should always be `"FeatureCollection"` + case type + } + + enum OSMTileDataParseError: Error { + /// The `type` property of an ``OSMTileDataJson`` should always be `"FeatureCollection"` + case incorrectTypeField + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let type = try container.decode(String.self, forKey: .type) + guard type == "FeatureCollection" else { + throw OSMTileDataParseError.incorrectTypeField + } + + /// Some parsed features may error, since our ``GeoJsonFeature`` implementation requires a name + /// As a result, we simply filter out the failing ones + let parsed_features = try container.decode([FailableDecode].self, forKey: .features) + features = parsed_features.compactMap({ + switch $0.result { + case .success(let feature): return feature + case .failure(_): return nil + } + }) + + } +} + class OSMServiceModel: OSMServiceModelProtocol { /// Path to the tile server private static let path = "/tiles" diff --git a/apps/ios/GuideDogs/Code/Generators/Geocoding/ReverseGeocoderContext.swift b/apps/ios/GuideDogs/Code/Generators/Geocoding/ReverseGeocoderContext.swift index c6dbb98d..b036378d 100644 --- a/apps/ios/GuideDogs/Code/Generators/Geocoding/ReverseGeocoderContext.swift +++ b/apps/ios/GuideDogs/Code/Generators/Geocoding/ReverseGeocoderContext.swift @@ -267,22 +267,18 @@ class ReverseGeocoderContext: ReverseGeocoder { continue } - guard let points = osmEntity.coordinates as? GALine else { continue } + guard let points = osmEntity.geometry?.coordinates as? [CLLocationCoordinate2D] else { continue } let isStickyRoad = stickyRoad?.localizedName == road.localizedName var previous: CLLocationCoordinate2D? for point in points { - guard previous != nil else { - previous = point.toCoordinate() - continue - } - - let current = point.toCoordinate() + let prev = previous ?? point + let current = point // Calculate the distance from the user's location to the road segment let (dist, lat, lon) = GeometryUtils.squaredDistance(location: usersLocation.coordinate, - start: previous!, + start: prev, end: current, zoom: zoomLevel) From d80d1b91ccbef56585b26d59d962e6cb518dbe07 Mon Sep 17 00:00:00 2001 From: 2kai2kai2 <2kai2kai2@gmail.com> Date: Wed, 8 Nov 2023 21:10:29 -0500 Subject: [PATCH 4/6] Tests for GeoJson parsers --- apps/ios/GuideDogs.xcodeproj/project.pbxproj | 36 ++++- .../App/Helpers/GeometryUtilsTest.swift | 144 +----------------- .../JSON Parsing/OSM/GeoJsonFeatureTest.swift | 130 ++++++++++++++++ .../OSM/GeoJsonGeometryTest.swift | 122 +++++++++++++++ .../Services/OSM/OSMServiceModelTest.swift | 11 +- 5 files changed, 302 insertions(+), 141 deletions(-) create mode 100644 apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift create mode 100644 apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonGeometryTest.swift diff --git a/apps/ios/GuideDogs.xcodeproj/project.pbxproj b/apps/ios/GuideDogs.xcodeproj/project.pbxproj index 86651c4a..5d3198cc 100644 --- a/apps/ios/GuideDogs.xcodeproj/project.pbxproj +++ b/apps/ios/GuideDogs.xcodeproj/project.pbxproj @@ -666,10 +666,12 @@ 914BAAF32AD745E400CB2171 /* DestinationManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914BAAF22AD745E400CB2171 /* DestinationManagerTest.swift */; }; 914BAAFD2AD7483300CB2171 /* AudioEngineTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914BAAFC2AD7483300CB2171 /* AudioEngineTest.swift */; }; 915FF9F62ADE4BAF002B3690 /* AuthoredActivityContentTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 915FF9F42ADE3F91002B3690 /* AuthoredActivityContentTest.swift */; }; + 91745DD52AFB0E6C003EC104 /* GeoJsonGeometryTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91745DD42AFB0E6C003EC104 /* GeoJsonGeometryTest.swift */; }; + 91745DD62AFB0F32003EC104 /* GeometryUtilsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914DEBDC2A3CE901007B161C /* GeometryUtilsTest.swift */; }; + 91745DD82AFC48E0003EC104 /* GeoJsonFeatureTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91745DD72AFC48E0003EC104 /* GeoJsonFeatureTest.swift */; }; 91B6ADAA2AF19CB600FFE4E9 /* OSMServiceModelTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */; }; 91C82AAD2A5DCF040086D126 /* GeolocationManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91C82AAC2A5DCF040086D126 /* GeolocationManagerTest.swift */; }; 91C82ABE2A6B08500086D126 /* RouteGuidanceTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91C82ABD2A6B08500086D126 /* RouteGuidanceTest.swift */; }; - 91DC0CF92A46134600244CC8 /* GeometryUtilsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914DEBDC2A3CE901007B161C /* GeometryUtilsTest.swift */; }; B90C27D61EAF81D600007368 /* Sound.swift in Sources */ = {isa = PBXBuildFile; fileRef = B90C27D51EAF81D600007368 /* Sound.swift */; }; B918EE9825100FFF00A5354A /* CalloutRangeContext.swift in Sources */ = {isa = PBXBuildFile; fileRef = B918EE9725100FFF00A5354A /* CalloutRangeContext.swift */; }; B91D3F6427AB5546004159A8 /* UserAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = B91D3F6327AB5546004159A8 /* UserAction.swift */; }; @@ -1585,6 +1587,8 @@ 914DEBCD2A3CE6B9007B161C /* UnitTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = UnitTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; 914DEBDC2A3CE901007B161C /* GeometryUtilsTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeometryUtilsTest.swift; sourceTree = ""; }; 915FF9F42ADE3F91002B3690 /* AuthoredActivityContentTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthoredActivityContentTest.swift; sourceTree = ""; }; + 91745DD42AFB0E6C003EC104 /* GeoJsonGeometryTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GeoJsonGeometryTest.swift; sourceTree = ""; }; + 91745DD72AFC48E0003EC104 /* GeoJsonFeatureTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeoJsonFeatureTest.swift; sourceTree = ""; }; 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OSMServiceModelTest.swift; sourceTree = ""; }; 91C82AAC2A5DCF040086D126 /* GeolocationManagerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = " GeolocationManagerTest.swift"; path = "UnitTests/Sensors/Geolocation/Geolocation Manager/ GeolocationManagerTest.swift"; sourceTree = SOURCE_ROOT; }; 91C82ABD2A6B08500086D126 /* RouteGuidanceTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = RouteGuidanceTest.swift; path = "UnitTests/Behaviors/Route Guidance/RouteGuidanceTest.swift"; sourceTree = SOURCE_ROOT; }; @@ -4347,6 +4351,31 @@ path = "Authored Activities"; sourceTree = ""; }; + 91745DD12AFB0E6C003EC104 /* Models */ = { + isa = PBXGroup; + children = ( + 91745DD22AFB0E6C003EC104 /* JSON Parsing */, + ); + path = Models; + sourceTree = ""; + }; + 91745DD22AFB0E6C003EC104 /* JSON Parsing */ = { + isa = PBXGroup; + children = ( + 91745DD32AFB0E6C003EC104 /* OSM */, + ); + path = "JSON Parsing"; + sourceTree = ""; + }; + 91745DD32AFB0E6C003EC104 /* OSM */ = { + isa = PBXGroup; + children = ( + 91745DD72AFC48E0003EC104 /* GeoJsonFeatureTest.swift */, + 91745DD42AFB0E6C003EC104 /* GeoJsonGeometryTest.swift */, + ); + path = OSM; + sourceTree = ""; + }; 91C82AA62A4F56A70086D126 /* Sensors */ = { isa = PBXGroup; children = ( @@ -4374,6 +4403,7 @@ 91C82AB52A67182E0086D126 /* Data */ = { isa = PBXGroup; children = ( + 91745DD12AFB0E6C003EC104 /* Models */, 915FF9F32ADE3F91002B3690 /* Authored Activities */, 914BAAF12AD745E400CB2171 /* Destination Manager */, 914BAAED2AD745BC00CB2171 /* Services */, @@ -5561,8 +5591,10 @@ 914BAAF32AD745E400CB2171 /* DestinationManagerTest.swift in Sources */, 91C82ABE2A6B08500086D126 /* RouteGuidanceTest.swift in Sources */, 91C82AAD2A5DCF040086D126 /* GeolocationManagerTest.swift in Sources */, - 91DC0CF92A46134600244CC8 /* GeometryUtilsTest.swift in Sources */, + 91745DD82AFC48E0003EC104 /* GeoJsonFeatureTest.swift in Sources */, + 91745DD52AFB0E6C003EC104 /* GeoJsonGeometryTest.swift in Sources */, 91B6ADAA2AF19CB600FFE4E9 /* OSMServiceModelTest.swift in Sources */, + 91745DD62AFB0F32003EC104 /* GeometryUtilsTest.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/apps/ios/UnitTests/App/Helpers/GeometryUtilsTest.swift b/apps/ios/UnitTests/App/Helpers/GeometryUtilsTest.swift index be1905ad..d46d9416 100644 --- a/apps/ios/UnitTests/App/Helpers/GeometryUtilsTest.swift +++ b/apps/ios/UnitTests/App/Helpers/GeometryUtilsTest.swift @@ -12,132 +12,6 @@ import CoreLocation class GeometryUtilsTest: XCTestCase { - // TODO: `GeometryUtils::coordinates(geoJson:)` would be better if the `GeometryType` enum used associated values (coordinates), which would let us avoid the fact that it currently returns a vague `[Any]?` and instead just return a `GeometryType`. According to comments in `GeometryUtils`, the reason for this is compatibility with Objective-C. However, if we can move away from that, we could have much better code. - - // GeoJSON strings taken/adapted from the GeoJSON spec, RFC-7946 - - /// normal test case for `GeometryUtils::coordinates(geoJson:)` - func testGeoJSONCoordinates_Point() throws { - /// `Point`-- coordinates are a `[Double]` - let point = GeometryUtils.coordinates(geoJson: """ -{ - "type": "Point", - "coordinates": [100.0, 0.0] -} -""") - XCTAssertEqual(point.type, GeometryType.point) - XCTAssertEqual(point.points as! [Double], [100.0, 0.0]) - } - - /// normal test case for `GeometryUtils::coordinates(geoJson:)` - func testGeoJSONCoordinates_LineString() throws { - /// `LineString`-- coordinates are a `[[Double]]` - let lineString = GeometryUtils.coordinates(geoJson: """ -{ - "type": "LineString", - "coordinates": [ - [100.0, 0.0], - [101.0, 1.0] - ] -} -""") - XCTAssertEqual(lineString.type, GeometryType.lineString) - XCTAssertEqual(lineString.points as! [[Double]], [[100.0, 0.0], [101.0, 1.0]]) - } - /// normal test case for `GeometryUtils::coordinates(geoJson:)` - func testGeoJSONCoordinates_Polygon() throws { - /// `Polygon`-- coordinates are a `[[[Double]]]` - let poly = GeometryUtils.coordinates(geoJson: """ -{ - "type": "Polygon", - "coordinates": [ - [ - [100.0, 0.0], - [101.0, 0.0], - [101.0, 1.0], - [100.0, 1.0], - [100.0, 0.0] - ], - [ - [100.8, 0.8], - [100.8, 0.2], - [100.2, 0.2], - [100.2, 0.8], - [100.8, 0.8] - ] - ] -} -""") - XCTAssertEqual(poly.type, GeometryType.polygon) - XCTAssertEqual(poly.points as! [[[Double]]], [ - [ - [100.0, 0.0], - [101.0, 0.0], - [101.0, 1.0], - [100.0, 1.0], - [100.0, 0.0] - ], - [ - [100.8, 0.8], - [100.8, 0.2], - [100.2, 0.2], - [100.2, 0.8], - [100.8, 0.8] - ] - ]) - } - - // Skipping type `MultiPoint` as equivalent - // Skipping type `MultiLineString` as equivalent - // Skipping type `MultiPolygon` as equivalent - - func testGeoJSONCoordinates_invalidType() throws { - let a = GeometryUtils.coordinates(geoJson: """ -{ - "type": "a", - "coordinates": [100.0, 0.0] -} -""") - // TODO: apparently invalid types become GeometryType.multiPolygon - should it? - XCTAssertEqual(a.type, .multiPolygon) - XCTAssertEqual(a.points as! [Double], [100.0, 0.0]) - } - - /// edge case for `GeometryUtils::coordinates(geoJson:)` with empty input - /// which should result in `(nil, nil)` - func testGeoJSONCoordinates_emptystring() throws { - let emptyString = GeometryUtils.coordinates(geoJson: "") - XCTAssertNil(emptyString.type) - XCTAssertNil(emptyString.points) - } - - /// edge cases for `GeometryUtils::coordinates(geoJson:)` with malformed json - /// which should result in `(nil, nil)` - func testGeoJSONCoordinates_malformed() throws { - let badKey = GeometryUtils.coordinates(geoJson: "{a: 1}"); - XCTAssertNil(badKey.type) - XCTAssertNil(badKey.points) - - let badValue = GeometryUtils.coordinates(geoJson: "{\"a\": asdf}") - XCTAssertNil(badValue.type) - XCTAssertNil(badValue.points) - } - - /// edge cases for `GeometryUtils::coordinates(geoJson:)` with missing keys - /// which should result in one or both return fields being `nil` - func testGeoJSONCoordinates_missing() throws { - let noType = GeometryUtils.coordinates(geoJson: """ -{"coordinates": [100.0, 0.0]} -""") - XCTAssertNil(noType.type) - XCTAssertEqual(noType.points as! [Double], [100.0, 0.0]) - - let noCoords = GeometryUtils.coordinates(geoJson: """ -{"type": "Point"} -""") - XCTAssertEqual(noCoords.type, GeometryType.point) - XCTAssertNil(noCoords.points) - } // TODO: test `geometryContainsLocation` func testExample() throws { XCTAssert(Soundscape.GeometryUtils.geometryContainsLocation(location: CLLocationCoordinate2D.init(latitude: 1, longitude: 1), coordinates: [CLLocationCoordinate2D.init(latitude: 1, longitude: 1), CLLocationCoordinate2D.init(latitude: 3, longitude: 3)])) @@ -334,25 +208,21 @@ class GeometryUtilsTest: XCTestCase { for lon in [0.0, 5.0, 10.0, 15.0, 20.0] { let on_path = CLLocationCoordinate2DMake(0, lon) let on_path_closest = GeometryUtils.closestEdge(from: on_path, on: path) - XCTAssertNotNil(on_path_closest) - XCTAssertEqual(on_path_closest!.coordinate, on_path) + XCTAssertEqual(on_path_closest, on_path) let parallel = CLLocationCoordinate2DMake(10, lon) let parallel_closest = GeometryUtils.closestEdge(from: parallel, on: path) - XCTAssertNotNil(parallel_closest) - XCTAssertEqual(parallel_closest!.coordinate, on_path) + XCTAssertEqual(parallel_closest, on_path) } for lat in [-10.0, -5.0, 0, 5.0, 10.0] { let before = CLLocationCoordinate2DMake(lat, -10) let before_closest = GeometryUtils.closestEdge(from: before, on: path) - XCTAssertNotNil(before_closest); - XCTAssertEqual(before_closest!.coordinate, path.first) + XCTAssertEqual(before_closest, path.first) let after = CLLocationCoordinate2DMake(lat, 30) let after_closest = GeometryUtils.closestEdge(from: after, on: path) - XCTAssertNotNil(after_closest) - XCTAssertEqual(after_closest!.coordinate, path.last) + XCTAssertEqual(after_closest, path.last) } } @@ -397,13 +267,11 @@ class GeometryUtilsTest: XCTestCase { let n_pole = CLLocationCoordinate2DMake(90, 0) let n_pole_closest = GeometryUtils.closestEdge(from: n_pole, on: path) - XCTAssertNotNil(n_pole_closest) - XCTAssertEqual(n_pole_closest!.coordinate, path.first) + XCTAssertEqual(n_pole_closest, path.first) let s_pole = CLLocationCoordinate2DMake(-90, 0) let s_pole_closest = GeometryUtils.closestEdge(from: s_pole, on: path) - XCTAssertNotNil(s_pole_closest) - XCTAssertEqual(s_pole_closest!.coordinate, path.first) + XCTAssertEqual(s_pole_closest, path.first) } diff --git a/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift b/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift new file mode 100644 index 00000000..f517eacc --- /dev/null +++ b/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift @@ -0,0 +1,130 @@ +// +// GeoJsonFeatureTest.swift +// UnitTests +// +// Created by Kai on 11/8/23. +// Copyright © 2023 Soundscape community. All rights reserved. +// + +import XCTest +import CoreLocation +@testable import Soundscape + + +/// Note: see ``GeoJsonGeometryTest`` if issues arise regarding parsing of the contained geometry objects. +final class GeoJsonFeatureTest: XCTestCase { + + func testParseRPI() throws { + let rpi_json = """ +{ + "feature_type": "amenity", + "feature_value": "university", + "geometry": { + "coordinates": [[[[-73.686467, 42.730566], [-73.686149, 42.732009], [-73.685002, 42.731883], [-73.683726, 42.731804], [-73.683392, 42.733057], [-73.682639, 42.732884], [-73.682458, 42.732802], [-73.681911, 42.732191], [-73.681714, 42.732282], [-73.681278, 42.733728], [-73.680441, 42.733561], [-73.679727, 42.733414], [-73.679364, 42.733336], [-73.678998, 42.733282], [-73.676712, 42.732955], [-73.676761, 42.732781], [-73.677151, 42.732838], [-73.677193, 42.732696], [-73.677726, 42.732782], [-73.677815, 42.732456], [-73.677903, 42.732135], [-73.677477, 42.731928], [-73.677807, 42.730778], [-73.677417, 42.730723], [-73.677159, 42.730717], [-73.676006, 42.73056], [-73.676017, 42.730672], [-73.675961, 42.73074], [-73.675475, 42.731016], [-73.674638, 42.730496], [-73.674799, 42.730409], [-73.67493, 42.730482], [-73.675212, 42.730647], [-73.675679, 42.730222], [-73.675501, 42.73017], [-73.675254, 42.730054], [-73.675167, 42.729998], [-73.674907, 42.729795], [-73.674852, 42.729732], [-73.674624, 42.729523], [-73.674487, 42.729452], [-73.674105, 42.729369], [-73.673404, 42.729424], [-73.672718, 42.729196], [-73.673533, 42.726224], [-73.675969, 42.726626], [-73.67629, 42.726823], [-73.67672, 42.727068], [-73.677428, 42.727328], [-73.677975, 42.727462], [-73.678137, 42.7275], [-73.678159, 42.727322], [-73.678187, 42.727021], [-73.678192, 42.727006], [-73.678198, 42.726995], [-73.678212, 42.726982], [-73.678226, 42.726972], [-73.678244, 42.726962], [-73.678264, 42.726956], [-73.678291, 42.726951], [-73.678314, 42.726951], [-73.678494, 42.726986], [-73.678777, 42.727041], [-73.679187, 42.727121], [-73.684755, 42.728203], [-73.684343, 42.729734], [-73.684201, 42.730261], [-73.686467, 42.730566]]], [[[-73.673503, 42.731682], [-73.673442, 42.731937], [-73.672991, 42.731879], [-73.672954, 42.732025], [-73.671747, 42.731875], [-73.671779, 42.731583], [-73.671382, 42.731524], [-73.671119, 42.731615], [-73.671037, 42.731946], [-73.671653, 42.732027], [-73.671589, 42.732252], [-73.67158, 42.732275], [-73.671566, 42.732285], [-73.67155, 42.73229], [-73.671534, 42.73229], [-73.671493, 42.732285], [-73.671478, 42.732296], [-73.67098, 42.732248], [-73.670851, 42.732399], [-73.670733, 42.732616], [-73.670164, 42.733215], [-73.669569, 42.733928], [-73.669268, 42.734996], [-73.669022, 42.734964], [-73.668904, 42.735413], [-73.666012, 42.734937], [-73.665894, 42.735311], [-73.66553, 42.735268], [-73.665396, 42.735792], [-73.66517, 42.735756], [-73.66414, 42.73626], [-73.664017, 42.736438], [-73.664017, 42.736627], [-73.664237, 42.737001], [-73.663373, 42.737222], [-73.66296, 42.736477], [-73.663352, 42.735524], [-73.663143, 42.735437], [-73.662665, 42.735126], [-73.662923, 42.734176], [-73.66347, 42.733928], [-73.664596, 42.73301], [-73.665857, 42.732037], [-73.666731, 42.730965], [-73.666458, 42.730792], [-73.666377, 42.73048], [-73.666517, 42.72959], [-73.667557, 42.729795], [-73.667311, 42.730445], [-73.667187, 42.730792], [-73.667729, 42.730977], [-73.668035, 42.730497], [-73.668169, 42.730287], [-73.668244, 42.73009], [-73.668233, 42.729846], [-73.668056, 42.729546], [-73.668196, 42.729341], [-73.667917, 42.7292], [-73.668078, 42.72901], [-73.667649, 42.728758], [-73.667455, 42.728411], [-73.66826, 42.728289], [-73.669166, 42.729026], [-73.669011, 42.729144], [-73.66885, 42.729286], [-73.668775, 42.72946], [-73.668775, 42.729605], [-73.668877, 42.729862], [-73.669295, 42.730476], [-73.670583, 42.730449], [-73.672042, 42.730641], [-73.671792, 42.731521], [-73.672944, 42.731702], [-73.672965, 42.731612], [-73.673503, 42.731682]]], [[[-73.663248, 42.732892], [-73.663118, 42.733223], [-73.662257, 42.733463], [-73.661554, 42.735411], [-73.66107, 42.735223], [-73.660714, 42.734099], [-73.660986, 42.733317], [-73.661919, 42.732265], [-73.663121, 42.732549], [-73.663248, 42.732892]]], [[[-73.673902, 42.73513], [-73.673492, 42.736589], [-73.67202, 42.736366], [-73.67243, 42.734907], [-73.673902, 42.73513]]], [[[-73.670324, 42.736868], [-73.670168, 42.737384], [-73.668723, 42.737149], [-73.668801, 42.73689], [-73.668548, 42.736848], [-73.668626, 42.736591], [-73.670324, 42.736868]]]], + "type": "MultiPolygon" + }, + "osm_ids": [-100000000008670722], + "properties": { + "addr:city": "Troy", + "addr:flats": "209;4213", + "addr:housenumber": "110", + "addr:postcode": "12180", + "addr:state": "NY", + "addr:street": "8th Street", + "amenity": "university", + "name": "Rensselaer Polytechnic Institute", + "nysgissam:nysaddresspointid": "RENS081205;RENS045006;RENS080924", + "smoking": "no", + "type": "multipolygon", + "website": "https://rpi.edu", + "wikidata": "Q49211", + "wikipedia": "en:Rensselaer Polytechnic Institute" + }, + "type": "Feature" +} +""".data(using: .utf8)! + + let rpi_feature = try JSONDecoder().decode(GeoJsonFeature.self, from: rpi_json) + // Since it's defined in a string, changes to OSM shouldn't affect this test + + XCTAssertEqual(rpi_feature.name, "Rensselaer Polytechnic Institute") + XCTAssertEqual(rpi_feature.type, "amenity") + XCTAssertEqual(rpi_feature.value, "university") + XCTAssertEqual(rpi_feature.osmIds, ["ft-100000000008670722"]) + XCTAssertEqual(rpi_feature.geometry?.rawValue, "MultiPolygon") + //XCTAssertEqual(rpi_feature.superCategory, .undefined) + + XCTAssertEqual(rpi_feature.properties, [ + "addr:city": "Troy", + "addr:flats": "209;4213", + "addr:housenumber": "110", + "addr:postcode": "12180", + "addr:state": "NY", + "addr:street": "8th Street", + "amenity": "university", + "name": "Rensselaer Polytechnic Institute", + "nysgissam:nysaddresspointid": "RENS081205;RENS045006;RENS080924", + "smoking": "no", + "type": "multipolygon", + "website": "https://rpi.edu", + "wikidata": "Q49211", + "wikipedia": "en:Rensselaer Polytechnic Institute" + ]) + + // Is not a road + XCTAssertFalse(rpi_feature.isCrossing) // that would make no sense + XCTAssertFalse(rpi_feature.isRoundabout) // this too + XCTAssertNil(rpi_feature.ref) + XCTAssertNil(rpi_feature.nameTag) + } + + func testParseSageAvenue() throws { + let sage_json = """ +{ + "feature_type": "highway", + "feature_value": "tertiary", + "geometry": { + "coordinates": [[-73.677224, 42.730786], [-73.677061, 42.730764], [-73.676573, 42.730701], [-73.676491, 42.73068], [-73.676317, 42.730619], [-73.676147, 42.730521], [-73.675929, 42.730405]], + "type": "LineString" + }, + "osm_ids": [-669453514], + "properties": { + "highway": "tertiary", + "maxspeed": "30 mph", + "name": "Sage Avenue", + "surface": "asphalt" + }, + "type": "Feature" +} +""".data(using: .utf8)! + + let sage_feature = try JSONDecoder().decode(GeoJsonFeature.self, from: sage_json) + // Since it's defined in a string, changes to OSM shouldn't affect this test + + XCTAssertEqual(sage_feature.name, "Sage Avenue") + XCTAssertEqual(sage_feature.type, "highway") + XCTAssertEqual(sage_feature.value, "tertiary") + XCTAssertEqual(sage_feature.osmIds, ["ft-669453514"]) + XCTAssertEqual(sage_feature.geometry?.rawValue, "LineString") + + XCTAssertEqual(sage_feature.properties, [ + "highway": "tertiary", + "maxspeed": "30 mph", + "name": "Sage Avenue", + "surface": "asphalt" + ]) + + // these are mostly determined by us, not a part of GeoJson spec + XCTAssertEqual(sage_feature.superCategory, .roads) + XCTAssertFalse(sage_feature.isCrossing) + XCTAssertFalse(sage_feature.isRoundabout) + XCTAssertNil(sage_feature.ref) + XCTAssertEqual(sage_feature.nameTag, "road") + } + + func testParseEmpty() throws { + let data_empty_string = "".data(using: .utf8)! + XCTAssertThrowsError(try JSONDecoder().decode(GeoJsonFeature.self, from: data_empty_string)) + } + +} diff --git a/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonGeometryTest.swift b/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonGeometryTest.swift new file mode 100644 index 00000000..17e48907 --- /dev/null +++ b/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonGeometryTest.swift @@ -0,0 +1,122 @@ +// +// GeoJsonGeometryTest.swift +// +// +// Created by Kai on 11/7/23. +// + +import XCTest +import CoreLocation +@testable import Soundscape + +final class GeoJsonGeometryTest: XCTestCase { + + // GeoJSON strings taken/adapted from the GeoJSON spec, RFC-7946 + + /// normal test case for `GeoJsonGeometry.init(geoJSON: String)` + func testParseGeoJsonGeometry_Point() throws { + /// `Point`-- coordinates are a `[Double]` + let point = GeoJsonGeometry(geoJSON: """ +{ + "type": "Point", + "coordinates": [100.0, 0.0] +} +""") + + XCTAssertEqual(point, .point(coordinates: CLLocationCoordinate2DMake(0, 100))) + } + + /// normal test case for `GeoJsonGeometry.init(geoJSON: String)` + func testParseGeoJsonGeometry_LineString() throws { + /// `LineString`-- coordinates are a `[[Double]]` + let lineString = GeoJsonGeometry(geoJSON: """ +{ + "type": "LineString", + "coordinates": [ + [100.0, 0.0], + [101.0, 1.0] + ] +} +""") + XCTAssertEqual(lineString, .lineString(coordinates: [CLLocationCoordinate2DMake(0, 100), + CLLocationCoordinate2DMake(1, 101)])) + } + /// normal test case for `GeoJsonGeometry.init(geoJSON: String)` + func testParseGeoJsonGeometry_Polygon() throws { + /// `Polygon`-- coordinates are a `[[[Double]]]` + let poly = GeoJsonGeometry(geoJSON: """ +{ + "type": "Polygon", + "coordinates": [ + [ + [100.0, 0.0], + [101.0, 0.0], + [101.0, 1.0], + [100.0, 1.0], + [100.0, 0.0] + ], + [ + [100.8, 0.8], + [100.8, 0.2], + [100.2, 0.2], + [100.2, 0.8], + [100.8, 0.8] + ] + ] +} +""") + XCTAssertEqual(poly, .polygon(coordinates: [ + [ + CLLocationCoordinate2DMake(0, 100), + CLLocationCoordinate2DMake(0, 101), + CLLocationCoordinate2DMake(1, 101), + CLLocationCoordinate2DMake(1, 100), + CLLocationCoordinate2DMake(0, 100) + ], + [ + CLLocationCoordinate2DMake(0.8, 100.8), + CLLocationCoordinate2DMake(0.2, 100.8), + CLLocationCoordinate2DMake(0.2, 100.2), + CLLocationCoordinate2DMake(0.8, 100.2), + CLLocationCoordinate2DMake(0.8, 100.8) + ] + ])) + } + + func testParseGeoJsonGeometry_invalidType() throws { + let a = GeoJsonGeometry(geoJSON: """ +{ + "type": "a", + "coordinates": [100.0, 0.0] +} +""") + XCTAssertNil(a) + } + + /// edge case for `GeoJsonGeometry.init(geoJSON: String)` with empty input + /// which should result in `(nil, nil)` + func testParseGeoJsonGeometry_emptystring() throws { + XCTAssertNil(GeoJsonGeometry(geoJSON: "")) + } + + /// edge cases for `GeoJsonGeometry.init(geoJSON: String)` with malformed json + func testParseGeoJsonGeometry_malformed() throws { + XCTAssertNil(GeoJsonGeometry(geoJSON: "{a: 1}")) + XCTAssertNil(GeoJsonGeometry(geoJSON: "{\"a\": asdf}")) + } + + /// edge cases for `GeoJsonGeometry.init(geoJSON: String)` with missing keys + /// which should result in `nil` + func testParseGeoJsonGeometry_missing() throws { + let noType = GeoJsonGeometry(geoJSON: """ +{"coordinates": [100.0, 0.0]} +""") + XCTAssertNil(noType) + + let noCoords = GeoJsonGeometry(geoJSON: """ +{"type": "Point"} +""") + XCTAssertNil(noCoords) + } + +} diff --git a/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift b/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift index bc60a929..4e4cacfa 100644 --- a/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift +++ b/apps/ios/UnitTests/Data/Services/OSM/OSMServiceModelTest.swift @@ -82,11 +82,20 @@ final class OSMServiceModelTest: XCTestCase { //XCTAssertEqual(RPI.dynamicURL, "https://rpi.edu") XCTAssertEqual(RPI.streetName, "8th Street") XCTAssertEqual(RPI.addressLine, "110 8th Street") - XCTAssertFalse(RPI.coordinates?.isEmpty ?? true) + let geometry = RPI.geometry + XCTAssertNotNil(geometry) + if case .multiPolygon(let coordinates) = geometry { + XCTAssertFalse(coordinates.isEmpty) + } else { + XCTFail("RPI geometry should be a multiPolygon") + } // Ensure RPI is roughly where it should be (with error since the exact location may shift as properties change over time) XCTAssertEqual(RPI.centroidLatitude, 42.73036, accuracy: 0.05) XCTAssertEqual(RPI.centroidLongitude, -73.67663, accuracy: 0.05) + + + // get by id since there are multiple segments of Sage Avenue guard let sage_ave = tiledata.roads.first(where: {$0.key == "ft-282843345"}) else { // assuming Sage ave. will still exist From d5fd4e27a3ce3e0711cc0e81df919bd61176cce7 Mon Sep 17 00:00:00 2001 From: 2kai2kai2 <2kai2kai2@gmail.com> Date: Tue, 14 Nov 2023 14:57:05 -0500 Subject: [PATCH 5/6] Move OSMTileDataJson out to GeoJsonFeatureCollection This improves consistency with naming of GeoJsonGeometry and GeoJsonFeature, as well as reducing clutter. --- apps/ios/GuideDogs.xcodeproj/project.pbxproj | 4 ++ .../Data/Models/Cache Models/TileData.swift | 2 +- .../Data/Services/Helpers/ServiceModel.swift | 4 +- .../Data/Services/OSM/OSMServiceModel.swift | 44 --------------- .../GuideDogs/GeoJsonFeatureCollection.swift | 53 +++++++++++++++++++ 5 files changed, 60 insertions(+), 47 deletions(-) create mode 100644 apps/ios/GuideDogs/GeoJsonFeatureCollection.swift diff --git a/apps/ios/GuideDogs.xcodeproj/project.pbxproj b/apps/ios/GuideDogs.xcodeproj/project.pbxproj index 5d3198cc..182f0ea8 100644 --- a/apps/ios/GuideDogs.xcodeproj/project.pbxproj +++ b/apps/ios/GuideDogs.xcodeproj/project.pbxproj @@ -669,6 +669,7 @@ 91745DD52AFB0E6C003EC104 /* GeoJsonGeometryTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91745DD42AFB0E6C003EC104 /* GeoJsonGeometryTest.swift */; }; 91745DD62AFB0F32003EC104 /* GeometryUtilsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 914DEBDC2A3CE901007B161C /* GeometryUtilsTest.swift */; }; 91745DD82AFC48E0003EC104 /* GeoJsonFeatureTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91745DD72AFC48E0003EC104 /* GeoJsonFeatureTest.swift */; }; + 91745DDA2AFED7FF003EC104 /* GeoJsonFeatureCollection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91745DD92AFED7FF003EC104 /* GeoJsonFeatureCollection.swift */; }; 91B6ADAA2AF19CB600FFE4E9 /* OSMServiceModelTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */; }; 91C82AAD2A5DCF040086D126 /* GeolocationManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91C82AAC2A5DCF040086D126 /* GeolocationManagerTest.swift */; }; 91C82ABE2A6B08500086D126 /* RouteGuidanceTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91C82ABD2A6B08500086D126 /* RouteGuidanceTest.swift */; }; @@ -1589,6 +1590,7 @@ 915FF9F42ADE3F91002B3690 /* AuthoredActivityContentTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthoredActivityContentTest.swift; sourceTree = ""; }; 91745DD42AFB0E6C003EC104 /* GeoJsonGeometryTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GeoJsonGeometryTest.swift; sourceTree = ""; }; 91745DD72AFC48E0003EC104 /* GeoJsonFeatureTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeoJsonFeatureTest.swift; sourceTree = ""; }; + 91745DD92AFED7FF003EC104 /* GeoJsonFeatureCollection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeoJsonFeatureCollection.swift; sourceTree = ""; }; 91B6ADA92AF19CB600FFE4E9 /* OSMServiceModelTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OSMServiceModelTest.swift; sourceTree = ""; }; 91C82AAC2A5DCF040086D126 /* GeolocationManagerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = " GeolocationManagerTest.swift"; path = "UnitTests/Sensors/Geolocation/Geolocation Manager/ GeolocationManagerTest.swift"; sourceTree = SOURCE_ROOT; }; 91C82ABD2A6B08500086D126 /* RouteGuidanceTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = RouteGuidanceTest.swift; path = "UnitTests/Behaviors/Route Guidance/RouteGuidanceTest.swift"; sourceTree = SOURCE_ROOT; }; @@ -4516,6 +4518,7 @@ children = ( D298328F1E4BF80600352A5A /* GeoJsonFeature.swift */, D29832951E4E249700352A5A /* GeoJsonGeometry.swift */, + 91745DD92AFED7FF003EC104 /* GeoJsonFeatureCollection.swift */, ); name = OSM; sourceTree = ""; @@ -6104,6 +6107,7 @@ 31D0D7301E031A0E0087C847 /* UINavigationItem+Extension.swift in Sources */, 2821F61B220A6D1600D15EFF /* AuthoredActivityLoader.swift in Sources */, 6220D2D027CD7D4B0063BEA6 /* BeaconPickerItemView.swift in Sources */, + 91745DDA2AFED7FF003EC104 /* GeoJsonFeatureCollection.swift in Sources */, 287D3E8D22DE50340084B92B /* StatusTableViewController.swift in Sources */, 6258B3872469DAFA0051F60B /* UniversalLinkPathComponents.swift in Sources */, 28A16122283C3AF00081CFA4 /* TourGenerator.swift in Sources */, diff --git a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift index 7b6aeaa2..bcb2839c 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/TileData.swift @@ -73,7 +73,7 @@ class TileData: Object { return VectorTile(quadKey: quadkey) } - convenience init(withParsedData json: OSMTileDataJson, quadkey: String, etag: String, superCategories: SuperCategories) { + convenience init(withParsedData json: GeoJsonFeatureCollection, quadkey: String, etag: String, superCategories: SuperCategories) { self.init() // Get the vector tile info diff --git a/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift b/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift index 3bec6152..14888b38 100644 --- a/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift +++ b/apps/ios/GuideDogs/Code/Data/Services/Helpers/ServiceModel.swift @@ -148,7 +148,7 @@ class ServiceModel { return json as? [String: Any] } - static func validateJsonResponse(request: URLRequest, response: URLResponse?, data: Data?, error: Error?, callback: @escaping (HTTPStatusCode, Error?) -> Void) -> (HTTPStatusCode, String, OSMTileDataJson)? { + static func validateJsonResponse(request: URLRequest, response: URLResponse?, data: Data?, error: Error?, callback: @escaping (HTTPStatusCode, Error?) -> Void) -> (HTTPStatusCode, String, GeoJsonFeatureCollection)? { // Some more housekeeping ServiceModel.logNetworkResponse(response, request: request, error: error) @@ -194,7 +194,7 @@ class ServiceModel { } // If we get this far, then the data property should not be nil, and it should be valid JSON - guard let data = data, let parsed = try? JSONDecoder().decode(OSMTileDataJson.self, from: data) else { + guard let data = data, let parsed = try? JSONDecoder().decode(GeoJsonFeatureCollection.self, from: data) else { DispatchQueue.main.async { callback(status, ServiceError.jsonParseFailed) } diff --git a/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift b/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift index 8dc32fb7..61d0090f 100644 --- a/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift +++ b/apps/ios/GuideDogs/Code/Data/Services/OSM/OSMServiceModel.swift @@ -23,50 +23,6 @@ enum ServiceError: Error { case jsonParseFailed } -final class FailableDecode: Decodable { - var result: Result - - public init(from decoder: Decoder) throws { - result = Result(catching: { try T(from: decoder) }) - } -} - -/// Represents the parsed json response from the OSM tiles service -final class OSMTileDataJson: Decodable { - var features: [GeoJsonFeature] - - private enum CodingKeys: CodingKey { - /// Contains an array of ``GeoJsonFeature``s - case features - /// Should always be `"FeatureCollection"` - case type - } - - enum OSMTileDataParseError: Error { - /// The `type` property of an ``OSMTileDataJson`` should always be `"FeatureCollection"` - case incorrectTypeField - } - - init(from decoder: Decoder) throws { - let container = try decoder.container(keyedBy: CodingKeys.self) - let type = try container.decode(String.self, forKey: .type) - guard type == "FeatureCollection" else { - throw OSMTileDataParseError.incorrectTypeField - } - - /// Some parsed features may error, since our ``GeoJsonFeature`` implementation requires a name - /// As a result, we simply filter out the failing ones - let parsed_features = try container.decode([FailableDecode].self, forKey: .features) - features = parsed_features.compactMap({ - switch $0.result { - case .success(let feature): return feature - case .failure(_): return nil - } - }) - - } -} - class OSMServiceModel: OSMServiceModelProtocol { /// Path to the tile server private static let path = "/tiles" diff --git a/apps/ios/GuideDogs/GeoJsonFeatureCollection.swift b/apps/ios/GuideDogs/GeoJsonFeatureCollection.swift new file mode 100644 index 00000000..334a6b02 --- /dev/null +++ b/apps/ios/GuideDogs/GeoJsonFeatureCollection.swift @@ -0,0 +1,53 @@ +// +// GeoJsonFeatureCollection.swift +// Soundscape +// +// Created by Kai on 11/10/23. +// Copyright © 2023 Soundscape community. All rights reserved. +// + +import Foundation + +final class FailableDecode: Decodable { + var result: Result + + public init(from decoder: Decoder) throws { + result = Result(catching: { try T(from: decoder) }) + } +} + +/// Represents the parsed json response from the OSM tiles service +final class GeoJsonFeatureCollection: Decodable { + var features: [GeoJsonFeature] + + private enum CodingKeys: CodingKey { + /// Contains an array of ``GeoJsonFeature``s + case features + /// Should always be `"FeatureCollection"` + case type + } + + enum GeoJsonFeatureCollectionParseError: Error { + /// The `type` property of an ``GeoJsonFeatureCollection`` should always be `"FeatureCollection"` + case incorrectTypeField + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let type = try container.decode(String.self, forKey: .type) + guard type == "FeatureCollection" else { + throw GeoJsonFeatureCollectionParseError.incorrectTypeField + } + + /// Some parsed features may error, since our ``GeoJsonFeature`` implementation requires a name + /// As a result, we simply filter out the failing ones + let parsed_features = try container.decode([FailableDecode].self, forKey: .features) + features = parsed_features.compactMap({ + switch $0.result { + case .success(let feature): return feature + case .failure(_): return nil + } + }) + + } +} From 792a98d30dd3a76a14eafa9a664a22fe2e17cb10 Mon Sep 17 00:00:00 2001 From: 2kai2kai2 <2kai2kai2@gmail.com> Date: Tue, 5 Dec 2023 13:35:07 -0500 Subject: [PATCH 6/6] Require `GeoJsonFeature`s to have geometry Despite spec allowing non-located features, we need a location, so will reject features that lack a geometry --- .../GDASpatialDataResultEntity.swift | 21 +++++++++---------- .../JSON Parsing/OSM/GeoJsonFeature.swift | 19 +++++++---------- .../JSON Parsing/OSM/GeoJsonFeatureTest.swift | 4 ++-- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift index b21d7fd3..84f91dca 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/Cache Models/GDASpatialDataResultEntity.swift @@ -178,19 +178,18 @@ class GDASpatialDataResultEntity: Object { } // Set geolocation information - if let geometry = feature.geometry { - if case .point(let point) = geometry { - latitude = point.latitude - longitude = point.longitude - } else if let json_data = try? JSONEncoder().encode(geometry) { - coordinatesJson = String(data: json_data, encoding: .utf8) - } - - let centroid = geometry.centroid - centroidLatitude = centroid.latitude - centroidLongitude = centroid.longitude + if case .point(let point) = feature.geometry { + latitude = point.latitude + longitude = point.longitude + } else if let json_data = try? JSONEncoder().encode(feature.geometry) { + coordinatesJson = String(data: json_data, encoding: .utf8) + _geometry = feature.geometry; } + let centroid = feature.geometry.centroid + centroidLatitude = centroid.latitude + centroidLongitude = centroid.longitude + // Road specific metadata roundabout = feature.isRoundabout diff --git a/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift b/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift index eeb65e1c..340c414a 100644 --- a/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift +++ b/apps/ios/GuideDogs/Code/Data/Models/JSON Parsing/OSM/GeoJsonFeature.swift @@ -80,8 +80,9 @@ final class GeoJsonFeature: Decodable { var superCategory: SuperCategory = .undefined /// Geometry of this feature - /// While Features are required to have a `geometry` member, it may be `null` for unlocated features, represented here as `nil` - var geometry: GeoJsonGeometry? + /// + /// GeoJSON spec allows null geometries, but we don't (we throw a parse failure if missing) + var geometry: GeoJsonGeometry var isCrossing = false @@ -143,12 +144,12 @@ final class GeoJsonFeature: Decodable { isRoundabout = true } - // TODO: it is unclear if geometry not existing should cause the feature to be rejected - geometry = try? container.decode(GeoJsonGeometry.self, forKey: .geometry) + // GeoJSON spec permits null geometries, but we don't + geometry = try container.decode(GeoJsonGeometry.self, forKey: .geometry) // Fix geometries for crossings with LineString geometries if isCrossing, case .lineString = geometry, - let median = geometry?.getLineMedian() { + let median = geometry.getLineMedian() { geometry = GeoJsonGeometry(point: median) } @@ -272,14 +273,10 @@ final class GeoJsonFeature: Decodable { let startCopy = GeoJsonFeature(copyFrom: self) let endCopy = GeoJsonFeature(copyFrom: self) - guard let first = geometry?.first, let last = geometry?.last else { - return nil - } - - startCopy.geometry = .point(coordinates: first) + startCopy.geometry = .point(coordinates: geometry.first) startCopy.superCategory = SuperCategory.mobility - endCopy.geometry = .point(coordinates: last) + endCopy.geometry = .point(coordinates: geometry.last) endCopy.superCategory = SuperCategory.mobility return (startCopy, endCopy) diff --git a/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift b/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift index f517eacc..c6cfa397 100644 --- a/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift +++ b/apps/ios/UnitTests/Data/Models/JSON Parsing/OSM/GeoJsonFeatureTest.swift @@ -51,7 +51,7 @@ final class GeoJsonFeatureTest: XCTestCase { XCTAssertEqual(rpi_feature.type, "amenity") XCTAssertEqual(rpi_feature.value, "university") XCTAssertEqual(rpi_feature.osmIds, ["ft-100000000008670722"]) - XCTAssertEqual(rpi_feature.geometry?.rawValue, "MultiPolygon") + XCTAssertEqual(rpi_feature.geometry.rawValue, "MultiPolygon") //XCTAssertEqual(rpi_feature.superCategory, .undefined) XCTAssertEqual(rpi_feature.properties, [ @@ -105,7 +105,7 @@ final class GeoJsonFeatureTest: XCTestCase { XCTAssertEqual(sage_feature.type, "highway") XCTAssertEqual(sage_feature.value, "tertiary") XCTAssertEqual(sage_feature.osmIds, ["ft-669453514"]) - XCTAssertEqual(sage_feature.geometry?.rawValue, "LineString") + XCTAssertEqual(sage_feature.geometry.rawValue, "LineString") XCTAssertEqual(sage_feature.properties, [ "highway": "tertiary",