NeTEx -> GeoJSON : Couverture de tests (#5474)

Author: ptitfred

apps/transport/lib/netex/archive_parser.ex

@@ -201,21 +201,25 @@ defmodule Transport.NeTEx.ArchiveParser do
   end
 
   defp read_all(zip_file_name, reader) do
-    with_zip_file_handle(zip_file_name, fn unzip ->
-      unzip
-      |> Unzip.list_entries()
-      |> Enum.map(fn metadata ->
-        Logger.debug("Processing #{metadata.file_name}")
-
-        {
-          metadata.file_name,
-          reader.(unzip, metadata.file_name)
-        }
-      end)
-    end)
-  end
-
-  def with_zip_file_handle(zip_file_name, cb) do
+    with_zip_file_handle(
+      zip_file_name,
+      fn unzip ->
+        unzip
+        |> Unzip.list_entries()
+        |> Enum.map(fn metadata ->
+          Logger.debug("Processing #{metadata.file_name}")
+
+          {
+            metadata.file_name,
+            reader.(unzip, metadata.file_name)
+          }
+        end)
+      end,
+      fn _error -> [] end
+    )
+  end
+
+  def with_zip_file_handle(zip_file_name, cb, on_error) do
     zip_file = Unzip.LocalFile.open(zip_file_name)
 
     try do
@@ -225,7 +229,7 @@ defmodule Transport.NeTEx.ArchiveParser do
 
         {:error, message} ->
           Logger.error("Error while reading #{zip_file_name}: #{message}")
-          []
+          on_error.(message)
       end
     after
       Unzip.LocalFile.close(zip_file)

apps/transport/lib/netex/to_geojson/to_geojson.ex

@@ -50,18 +50,20 @@ defmodule Transport.NeTEx.ToGeoJSON do
   def convert_archive(zip_path, opts \\ []) do
     types = Keyword.get(opts, :types, @all_types)
 
-    Transport.NeTEx.ArchiveParser.with_zip_file_handle(zip_path, fn unzip ->
-      features =
-        unzip
-        |> Unzip.list_entries()
-        |> Enum.flat_map(fn metadata ->
-          extract_features_from_entry(unzip, metadata.file_name, types)
-        end)
-
-      {:ok, GeoJSONBuilder.feature_collection(features)}
-    end)
-  rescue
-    e -> {:error, Exception.message(e)}
+    Transport.NeTEx.ArchiveParser.with_zip_file_handle(
+      zip_path,
+      fn unzip ->
+        features =
+          unzip
+          |> Unzip.list_entries()
+          |> Enum.flat_map(fn metadata ->
+            extract_features_from_entry(unzip, metadata.file_name, types)
+          end)
+
+        {:ok, GeoJSONBuilder.feature_collection(features)}
+      end,
+      fn e -> {:error, e} end
+    )
   end
 
   @doc """

apps/transport/test/netex/to_geojson/to_geojson_test.exs

@@ -3,18 +3,6 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
 
   alias Transport.NeTEx.ToGeoJSON
 
-  defmodule ZipCreator do
-    @spec create!(String.t(), [{String.t(), binary()}]) :: no_return()
-    def create!(zip_filename, file_data) do
-      {:ok, ^zip_filename} =
-        :zip.create(
-          zip_filename,
-          file_data
-          |> Enum.map(fn {name, content} -> {name |> to_charlist(), content} end)
-        )
-    end
-  end
-
   describe "convert_xml/1" do
     test "converts StopPlaces to Point features" do
       xml = """
@@ -305,19 +293,16 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
       </root>
       """
 
-      tmp_file = System.tmp_dir!() |> Path.join("netex-geojson-#{Ecto.UUID.generate()}.zip")
-      ZipCreator.create!(tmp_file, [{"stops.xml", xml1}, {"quays.xml", xml2}])
+      ZipCreator.with_tmp_zip([{"stops.xml", xml1}, {"quays.xml", xml2}], fn tmp_file ->
+        assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file)
 
-      assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file)
+        assert geojson["type"] == "FeatureCollection"
+        assert length(geojson["features"]) == 2
 
-      assert geojson["type"] == "FeatureCollection"
-      assert length(geojson["features"]) == 2
-
-      ids = Enum.map(geojson["features"], & &1["id"])
-      assert "stop_1" in ids
-      assert "quay_1" in ids
-
-      File.rm!(tmp_file)
+        ids = Enum.map(geojson["features"], & &1["id"])
+        assert "stop_1" in ids
+        assert "quay_1" in ids
+      end)
     end
 
     test "filters by types option" do
@@ -344,15 +329,12 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
       </root>
       """
 
-      tmp_file = System.tmp_dir!() |> Path.join("netex-geojson-#{Ecto.UUID.generate()}.zip")
-      ZipCreator.create!(tmp_file, [{"data.xml", xml}])
-
-      assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file, types: [:quays])
-
-      assert length(geojson["features"]) == 1
-      assert hd(geojson["features"])["id"] == "quay_1"
+      ZipCreator.with_tmp_zip([{"data.xml", xml}], fn tmp_file ->
+        assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file, types: [:quays])
 
-      File.rm!(tmp_file)
+        assert length(geojson["features"]) == 1
+        assert hd(geojson["features"])["id"] == "quay_1"
+      end)
     end
 
     test "skips non-XML files" do
@@ -370,14 +352,11 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
       </root>
       """
 
-      tmp_file = System.tmp_dir!() |> Path.join("netex-geojson-#{Ecto.UUID.generate()}.zip")
-      ZipCreator.create!(tmp_file, [{"data.xml", xml}, {"readme.txt", "Some text"}])
-
-      assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file)
-
-      assert length(geojson["features"]) == 1
+      ZipCreator.with_tmp_zip([{"data.xml", xml}, {"readme.txt", "Some text"}], fn tmp_file ->
+        assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file)
 
-      File.rm!(tmp_file)
+        assert length(geojson["features"]) == 1
+      end)
     end
 
     test "skips directories" do
@@ -395,15 +374,12 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
       </root>
       """
 
-      tmp_file = System.tmp_dir!() |> Path.join("netex-geojson-#{Ecto.UUID.generate()}.zip")
-      ZipCreator.create!(tmp_file, [{"subdir/data.xml", xml}])
+      ZipCreator.with_tmp_zip([{"subdir/data.xml", xml}], fn tmp_file ->
+        assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file)
 
-      assert {:ok, geojson} = ToGeoJSON.convert_archive(tmp_file)
-
-      # Should still find the XML in the subdirectory
-      assert length(geojson["features"]) == 1
-
-      File.rm!(tmp_file)
+        # Should still find the XML in the subdirectory
+        assert length(geojson["features"]) == 1
+      end)
     end
   end
 
@@ -423,15 +399,12 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
       </root>
       """
 
-      tmp_file = System.tmp_dir!() |> Path.join("netex-geojson-#{Ecto.UUID.generate()}.zip")
-      ZipCreator.create!(tmp_file, [{"data.xml", xml}])
-
-      assert {:ok, geojson} = Transport.NeTEx.ArchiveParser.to_geojson(tmp_file)
+      ZipCreator.with_tmp_zip([{"data.xml", xml}], fn tmp_file ->
+        assert {:ok, geojson} = Transport.NeTEx.ArchiveParser.to_geojson(tmp_file)
 
-      assert geojson["type"] == "FeatureCollection"
-      assert length(geojson["features"]) == 1
-
-      File.rm!(tmp_file)
+        assert geojson["type"] == "FeatureCollection"
+        assert length(geojson["features"]) == 1
+      end)
     end
 
     test "accepts types option" do
@@ -458,15 +431,32 @@ defmodule Transport.NeTEx.ToGeoJSONTest do
       </root>
       """
 
-      tmp_file = System.tmp_dir!() |> Path.join("netex-geojson-#{Ecto.UUID.generate()}.zip")
-      ZipCreator.create!(tmp_file, [{"data.xml", xml}])
+      ZipCreator.with_tmp_zip([{"data.xml", xml}], fn tmp_file ->
+        assert {:ok, geojson} = Transport.NeTEx.ArchiveParser.to_geojson(tmp_file, types: [:stop_places])
+
+        assert length(geojson["features"]) == 1
+        assert hd(geojson["features"])["id"] == "stop_1"
+      end)
+    end
 
-      assert {:ok, geojson} = Transport.NeTEx.ArchiveParser.to_geojson(tmp_file, types: [:stop_places])
+    test "empty archive" do
+      ZipCreator.with_tmp_zip([], fn tmp_file ->
+        assert {:ok, geojson} = Transport.NeTEx.ArchiveParser.to_geojson(tmp_file, types: [:stop_places])
 
-      assert length(geojson["features"]) == 1
-      assert hd(geojson["features"])["id"] == "stop_1"
+        assert geojson == %{"features" => [], "type" => "FeatureCollection"}
+      end)
+    end
+
+    test "bad NeTEx" do
+      xml = """
+      not an XML
+      """
+
+      ZipCreator.with_tmp_zip([{"data.xml", xml}], fn tmp_file ->
+        assert {:ok, geojson} = Transport.NeTEx.ArchiveParser.to_geojson(tmp_file, types: [:stop_places])
 
-      File.rm!(tmp_file)
+        assert geojson == %{"features" => [], "type" => "FeatureCollection"}
+      end)
     end
   end
 end

apps/transport/test/transport/jobs/conversions/netex_to_geojson_converter_job_test.exs

@@ -0,0 +1,54 @@
+defmodule Transport.Jobs.NeTExToGeoJSONConverterJobTest do
+  use ExUnit.Case, async: true
+  use Oban.Testing, repo: DB.Repo
+  import DB.Factory
+  import Mox
+  alias Transport.Jobs.NeTExToGeoJSONConverterJob
+
+  setup do
+    Ecto.Adapters.SQL.Sandbox.checkout(DB.Repo)
+  end
+
+  setup :verify_on_exit!
+
+  test "NeTEx to GeoJSON jobs are enqueued" do
+    %{id: resource_history_id} =
+      insert(:resource_history,
+        datagouv_id: "1",
+        payload: %{"format" => "NeTEx", "uuid" => Ecto.UUID.generate()}
+      )
+
+    insert(:resource_history,
+      datagouv_id: "2",
+      payload: %{"format" => "GTFS", "uuid" => Ecto.UUID.generate()}
+    )
+
+    insert(:resource_history, datagouv_id: "3", payload: %{})
+
+    # Ignored because it previously had a fatal conversion error
+    insert(:resource_history,
+      datagouv_id: "4",
+      payload: %{"format" => "NeTEx", "uuid" => Ecto.UUID.generate(), "conversion_GeoJSON_fatal_error" => true}
+    )
+
+    # This resource_history should not get enqueued for conversion,
+    # as a matching data_conversion already exists
+    insert(:resource_history,
+      datagouv_id: "4",
+      payload: %{"format" => "NeTEx", uuid: uuid = Ecto.UUID.generate()}
+    )
+
+    insert(:data_conversion,
+      convert_from: :NeTEx,
+      convert_to: :GeoJSON,
+      converter: DB.DataConversion.converter_to_use(:NeTEx, :GeoJSON),
+      resource_history_uuid: uuid,
+      payload: %{}
+    )
+
+    assert :ok = perform_job(NeTExToGeoJSONConverterJob, %{})
+
+    assert [%Oban.Job{args: %{"resource_history_id" => ^resource_history_id}}] =
+             all_enqueued()
+  end
+end