Skip to content

Commit

Permalink
Default decoded GeoJSON to SRID 4326 (WGS 84) per the spec (#219)
Browse files Browse the repository at this point in the history
* Default decoded GeoJSON to SRID 4326 (WGS 84) per the spec

The GeoJSON spec [indicates](https://tools.ietf.org/html/rfc7946#section-4) that all GeoJSON should be assumed to use the WGS 84 datum by default. We should be permissive and allow overriding that datum (as we did previously), but I think the correct behavior here is to make the datum explicit in our decoded `Geo.Geometry.t()` values.

This is a breaking change, but one which I expect to have quite little impact on users. (See the CHANGELOG.md for more.)

Resolves #129

* Prepare the rest of the v4.0.0 release notes

* Add final release date
  • Loading branch information
s3cur3 authored Sep 17, 2024
1 parent 9ad50c3 commit 44ee1de
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 19 deletions.
67 changes: 67 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,72 @@
# Changelog

## v4.0.0 — 2024-09-17

### Potentially breaking change: [Default decoded GeoJSON to SRID 4326 (WGS 84)](https://github.com/felt/geo/pull/219)

This aligns our GeoJSON decoding with [the GeoJSON spec](https://tools.ietf.org/html/rfc7946#section-4) by making all decoded GeoJSON infer the WGS 84 datum (SRID 4326) by default. Whereas previously when you called `Geo.JSON.decode/1` or `decode!/1`, we would return geometries with an `:srid` of `nil`, we now return `srid: 4326`. Likewise when encoding GeoJSON, we explicitly output a `crs` field indicating the datum.

This is unlikely to break real-world usage unless your implementation was assuming a different datum by default.

A couple examples of the changes:

**Before**:

```elixir
iex> Geo.JSON.decode!(%{"type" => "Point", "coordinates" => [1.0, 2.0]})
%Geo.Point{
coordinates: {1.0, 2.0},
# Note the old default nil SRID!
srid: nil
}
```

**After**

```elixir
iex> Geo.JSON.decode!(%{"type" => "Point", "coordinates" => [1.0, 2.0]})
%Geo.Point{
coordinates: {1.0, 2.0},
# New explicit default of WGS 84
srid: 4326
}
```

If you were to then encode this value again, you'd end up with a new `crs` field in the output GeoJSON:

```elixir
iex> %{"type" => "Point", "coordinates" => [1.0, 2.0]}
...> |> Geo.JSON.decode!()
...> |> GeoJSON.encode!()
%{
"type" => "Point",
"coordinates" => [1.0, 2.0],
# Note the new `crs` field which was not present in the input to Geo.JSON.decode!/1
"crs" => %{"properties" => %{"name" => "EPSG:4326"}, "type" => "name"}
}
```

This last behavior is the most potentially troublesome. However, we don't have a good way of distinguishing a case where you explicitly had the `crs` set in the input to the decoding function (in which case you would probably also like to have it present in the re-encoded version) compared to one in which it's been inferred.

Thanks to @gworkman for reporting this issue ([#129](https://github.com/felt/geo/issues/129)).

### Potentially breaking change: [Convert string coordinates to floats, or raise an error](https://github.com/felt/geo/pull/218)

This fixes an issue where we were silently accepting non-numeric coordinates in the GeoJSON decoder, such that you could wind up doing things like decoding a point like `%Geo.Point{coordinates: {"100.0", "-10.0"}}`. This would obviously not have gone well for you later in your processing pipeline, and it violates our typespecs.

The fix here, suggested by @LostKobrakai, is to convert those strings to numbers where we can do so unambiguously. While such inputs are clearly invalid, it's easy enough to handle them in the way that the user was hoping that we should probably just do it. In cases where there's any ambiguity at all, we raise an `ArgumentError`.

### Other bug fixes in v4.0.0

- [Support GeoJSON Feature object with nested GeometryCollection](https://github.com/felt/geo/pull/194) by new contributor @carstenpiepel (🎉)

### Other changes in v4.0.0

- [Fix typo in the README](https://github.com/felt/geo/pull/197) by @caspg
- [Fix typo](https://github.com/felt/geo/pull/216) by new contributor @preciz (🎉)
- [Optional dependency bump for `jason` to v1.4.4](https://github.com/felt/geo/pull/215)
- Dev dependency bumps for ex_doc, benchee, stream_data

## v3.6.0 — 2023-10-19

As of v3.6.0, `geo` (like [`geo_postgis`](https://github.com/felt/geo_postgis)) is being maintained by the Felt team. As a company building a geospatial product on Elixir, with a track record of [supporting open source software](https://felt.com/open-source), we're excited for the future of the project.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ _Note_: If you are looking to do geospatial calculations in memory with Geo's st
```elixir
defp deps do
[
{:geo, "~> 3.6"}
{:geo, "~> 4.0"}
]
end
```
Expand Down
5 changes: 4 additions & 1 deletion lib/geo/json.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ defmodule Geo.JSON do
so that you can use the resulting GeoJSON structure as a property
in larger JSON structures.
Note that, per [the GeoJSON spec](https://tools.ietf.org/html/rfc7946#section-4),
all geometries are assumed to use the WGS 84 datum (SRID 4326) by default.
## Examples
# Using Jason as the JSON parser for these examples
iex> json = "{ \\"type\\": \\"Point\\", \\"coordinates\\": [100.0, 0.0] }"
...> json |> Jason.decode!() |> Geo.JSON.decode!()
%Geo.Point{coordinates: {100.0, 0.0}, srid: nil}
%Geo.Point{coordinates: {100.0, 0.0}, srid: 4326}
iex> geom = %Geo.Point{coordinates: {100.0, 0.0}, srid: nil}
...> Jason.encode!(geom)
Expand Down
11 changes: 11 additions & 0 deletions lib/geo/json/decoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ defmodule Geo.JSON.Decoder do
true ->
raise DecodeError, value: geo_json
end
# Per #129, the GeoJSON spec says all GeoJSON coordinates default to SRID 4326 (WGS 84)
# https://tools.ietf.org/html/rfc7946#section-4
|> default_srid_4326()
end

@doc """
Expand Down Expand Up @@ -279,4 +282,12 @@ defmodule Geo.JSON.Decoder do
defp ensure_numeric(other) do
raise ArgumentError, "expected a numeric coordinate, got: #{inspect(other)}"
end

defp default_srid_4326(%{srid: nil} = geom), do: %{geom | srid: 4326}

defp default_srid_4326(%{geometries: geometries} = geom) when is_list(geometries) do
%{geom | geometries: Enum.map(geometries, &default_srid_4326/1)}
end

defp default_srid_4326(geom), do: geom
end
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ defmodule Geo.Mixfile do
use Mix.Project

@source_url "https://github.com/felt/geo"
@version "3.6.0"
@version "4.0.0"

def project do
[
Expand Down
58 changes: 42 additions & 16 deletions test/geo/json_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ defmodule Geo.JSON.Test do
json = "{\"type\":\"Point\",\"coordinates\":[100.0,0.0,70.0]}"
geom = Jason.decode!(json) |> Geo.JSON.decode!()

assert(geom == %Geo.PointZ{coordinates: {100.0, 0.0, 70.0}})
assert geom == %Geo.PointZ{coordinates: {100.0, 0.0, 70.0}, srid: 4326}
end

test "LineString to GeoJson" do
Expand All @@ -54,7 +54,7 @@ defmodule Geo.JSON.Test do
assert(geom.coordinates == {100.0, 0.0})

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson Point without coordinates" do
Expand All @@ -64,7 +64,7 @@ defmodule Geo.JSON.Test do
assert(is_nil(geom.coordinates))

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson with SRID to Point and back" do
Expand All @@ -78,7 +78,7 @@ defmodule Geo.JSON.Test do
assert(geom.srid == 4326)

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to LineString and back" do
Expand All @@ -88,7 +88,7 @@ defmodule Geo.JSON.Test do

assert(geom.coordinates == [{100.0, 0.0}, {101.0, 1.0}])
new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to LineStringZ and back" do
Expand All @@ -100,7 +100,7 @@ defmodule Geo.JSON.Test do

assert(geom.coordinates == [{100.0, 0.0, 50.0}, {101.0, 1.0, 20.0}])
new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "Drops M coordinate when decoding LineStringZM" do
Expand Down Expand Up @@ -145,7 +145,7 @@ defmodule Geo.JSON.Test do
)

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to MultiPoint and back" do
Expand All @@ -155,7 +155,7 @@ defmodule Geo.JSON.Test do

assert(geom.coordinates == [{100.0, 0.0}, {101.0, 1.0}])
new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to MultiLineString and back" do
Expand All @@ -167,7 +167,7 @@ defmodule Geo.JSON.Test do

assert(geom.coordinates == [[{100.0, 0.0}, {101.0, 1.0}], [{102.0, 2.0}, {103.0, 3.0}]])
new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to MultiLineStringZ and back" do
Expand All @@ -185,7 +185,7 @@ defmodule Geo.JSON.Test do
)

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to MultiPolygon and back" do
Expand All @@ -206,7 +206,7 @@ defmodule Geo.JSON.Test do
)

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJson to GeometryCollection and back" do
Expand All @@ -219,7 +219,7 @@ defmodule Geo.JSON.Test do
assert(Enum.count(geom.geometries) == 2)

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "Unable to encode non-geo type" do
Expand Down Expand Up @@ -281,7 +281,7 @@ defmodule Geo.JSON.Test do
assert(Enum.count(geom.geometries) == 2)

new_exjson = Geo.JSON.encode!(geom)
assert(exjson == new_exjson)
assert_geojson_equal(exjson, new_exjson)
end

test "GeoJSON to GeometryCollection" do
Expand Down Expand Up @@ -454,7 +454,16 @@ defmodule Geo.JSON.Test do
y <- float()
) do
geom = %Geo.Point{coordinates: {x, y}}
assert geom == Geo.JSON.encode!(geom) |> Geo.JSON.decode!()
assert %{geom | srid: 4326} == Geo.JSON.encode!(geom) |> Geo.JSON.decode!()

geom_with_srid_and_props = %Geo.Point{
coordinates: {x, y},
srid: 1234,
properties: %{"foo" => "bar"}
}

assert %{geom_with_srid_and_props | srid: 1234} ==
Geo.JSON.encode!(geom_with_srid_and_props) |> Geo.JSON.decode!()
end
end

Expand All @@ -463,13 +472,13 @@ defmodule Geo.JSON.Test do
json = Geo.JSON.encode!(geom) |> Jason.encode!()

assert(json == "{\"coordinates\":[],\"type\":\"Point\"}")
assert geom == Geo.JSON.encode!(geom) |> Geo.JSON.decode!()
assert %{geom | srid: 4326} == Geo.JSON.encode!(geom) |> Geo.JSON.decode!()
end

property "encodes and decodes back to the correct LineString struct" do
check all(list <- list_of({float(), float()}, min_length: 1)) do
geom = %Geo.LineString{coordinates: list}
assert geom == Geo.JSON.encode!(geom) |> Geo.JSON.decode!()
assert %{geom | srid: 4326} == Geo.JSON.encode!(geom) |> Geo.JSON.decode!()
end
end

Expand Down Expand Up @@ -588,4 +597,21 @@ defmodule Geo.JSON.Test do
assert Enum.all?(geom.geometries, &match?(%Geo.MultiPolygon{}, &1))
assert geom.properties["id"] == "FLC017"
end

defp assert_geojson_equal(%{} = json_1, %{} = json_2) do
# Per the GeoJSON spec, GeoJSON is assumed to have WGS 84 datum (SRID 4326) by default
assert drop_srid_4326(json_1) == drop_srid_4326(json_2),
"Inequivalent GeoJSON values:\n" <>
"Left:\n" <>
"#{inspect(json_1, pretty: true)}\n" <>
"Right:\n" <>
"#{inspect(json_2, pretty: true)}"
end

defp drop_srid_4326(%{"crs" => crs} = json)
when crs == %{"properties" => %{"name" => "EPSG:4326"}, "type" => "name"} do
Map.delete(json, "crs")
end

defp drop_srid_4326(%{} = json), do: json
end

0 comments on commit 44ee1de

Please sign in to comment.