Skip to content

Commit

Permalink
GBFSMetadata : supprimer clé cors_header_value (#4313)
Browse files Browse the repository at this point in the history
  • Loading branch information
AntoineAugusti authored Nov 14, 2024
1 parent 893a790 commit c55166e
Show file tree
Hide file tree
Showing 11 changed files with 23 additions and 96 deletions.
17 changes: 6 additions & 11 deletions apps/shared/lib/gbfs_metadata.ex
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
defmodule Transport.Shared.GBFSMetadata.Wrapper do
@moduledoc """
Defines a behavior
Behavior for a module in charge of computing metadata for GBFS feeds.
"""

@callback compute_feed_metadata(binary(), binary()) :: map()
@callback compute_feed_metadata(binary()) :: map()
def compute_feed_metadata(url), do: impl().compute_feed_metadata(url)

def impl, do: Application.get_env(:transport, :gbfs_metadata_impl)
def compute_feed_metadata(url, cors_base_url), do: impl().compute_feed_metadata(url, cors_base_url)
end

defmodule Transport.Shared.GBFSMetadata do
Expand Down Expand Up @@ -39,16 +40,15 @@ defmodule Transport.Shared.GBFSMetadata do
It will do multiple HTTP requests (calling GBFS sub-feeds) to compute various statistics.
"""
@impl Transport.Shared.GBFSMetadata.Wrapper
def compute_feed_metadata(url, cors_base_url) do
{:ok, %{status_code: 200, body: body} = response} = http_client().get(url, [{"origin", cors_base_url}])
def compute_feed_metadata(url) do
{:ok, %HTTPoison.Response{status_code: 200, body: body}} = http_client().get(url)
{:ok, json} = Jason.decode(body)

# we compute the feed delay before the rest for accuracy
feed_timestamp_delay = feed_timestamp_delay(json)

%{
validation: validation(url),
cors_header_value: cors_header_value(response),
feeds: feeds(json),
versions: versions(json),
languages: languages(json),
Expand Down Expand Up @@ -92,11 +92,6 @@ defmodule Transport.Shared.GBFSMetadata do
end
end

defp cors_header_value(%HTTPoison.Response{headers: headers}) do
headers = headers |> Enum.into(%{}, fn {h, v} -> {String.downcase(h), v} end)
Map.get(headers, "access-control-allow-origin")
end

defp ttl(%{"data" => _data} = payload) do
feed_name = feed_to_use_for_ttl(types(payload))

Expand Down
44 changes: 10 additions & 34 deletions apps/shared/test/gbfs_metadata_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ defmodule Transport.Shared.GBFSMetadataTest do
validator_version: "31c5325",
validator: :validator_module
},
cors_header_value: "*",
feed_timestamp_delay: _,
vehicle_types: ["bicycle"]
} = compute_feed_metadata(@gbfs_url, "http://example.com")
} = compute_feed_metadata(@gbfs_url)
end

test "for a stations + free floating feed with a multiple versions" do
Expand Down Expand Up @@ -80,18 +79,17 @@ defmodule Transport.Shared.GBFSMetadataTest do
"geofencing_zones",
"gbfs_versions"
],
cors_header_value: "*",
feed_timestamp_delay: feed_timestamp_delay,
vehicle_types: ["bicycle", "scooter"]
} = compute_feed_metadata(@gbfs_url, "http://example.com")
} = compute_feed_metadata(@gbfs_url)

assert feed_timestamp_delay > 0
end

test "for feed with a 500 error on the root URL" do
setup_feeds([:gbfs_with_server_error])

{res, logs} = with_log(fn -> compute_feed_metadata(@gbfs_url, "http://example.com") end)
{res, logs} = with_log(fn -> compute_feed_metadata(@gbfs_url) end)

assert %{} == res
assert logs =~ "Could not compute GBFS feed metadata"
Expand All @@ -101,7 +99,7 @@ defmodule Transport.Shared.GBFSMetadataTest do
setup_feeds([:gbfs_with_invalid_gbfs_json])
setup_validation_result({:error, nil})

{res, logs} = with_log(fn -> compute_feed_metadata(@gbfs_url, "http://example.com") end)
{res, logs} = with_log(fn -> compute_feed_metadata(@gbfs_url) end)

assert %{} == res
assert logs =~ "Could not compute GBFS feed metadata"
Expand Down Expand Up @@ -600,62 +598,40 @@ defmodule Transport.Shared.GBFSMetadataTest do
defp setup_feed(:station_information), do: setup_station_information_response()
defp setup_feed(:vehicle_types), do: setup_vehicle_types_response()

defp setup_response_with_headers(expected_url, body) do
Transport.HTTPoison.Mock
|> expect(:get, fn url, headers ->
assert headers == [{"origin", "http://example.com"}]
assert url == expected_url

{:ok,
%HTTPoison.Response{
status_code: 200,
body: body,
headers: [{"Content-Type", "application/json"}, {"Access-Control-Allow-Origin", "*"}]
}}
end)
end

defp setup_response(expected_url, body) do
Transport.HTTPoison.Mock
|> expect(:get, fn url ->
assert url == expected_url

{:ok,
%HTTPoison.Response{
status_code: 200,
body: body,
headers: [{"Content-Type", "application/json"}]
}}
|> expect(:get, fn ^expected_url ->
{:ok, %HTTPoison.Response{status_code: 200, body: body, headers: [{"content-type", "application/json"}]}}
end)
end

defp setup_gbfs_with_server_error_response do
Transport.HTTPoison.Mock
|> expect(:get, fn _url, _headers -> {:ok, %HTTPoison.Response{status_code: 500}} end)
|> expect(:get, fn _url -> {:ok, %HTTPoison.Response{status_code: 500}} end)
end

defp setup_gbfs_response do
body = """
{"data":{"fr":{"feeds":[{"name":"system_information","url":"https://example.com/system_information.json"},{"name":"station_information","url":"https://example.com/station_information.json"},{"name":"station_status","url":"https://example.com/station_status.json"}]}},"last_updated":1636116464,"ttl":3600,"version":"1.1"}
"""

setup_response_with_headers(@gbfs_url, body)
setup_response(@gbfs_url, body)
end

defp setup_invalid_gbfs_response do
body = """
{"foo": "bar"}
"""

setup_response_with_headers(@gbfs_url, body)
setup_response(@gbfs_url, body)
end

defp setup_gbfs_with_versions_response do
body = """
{"last_updated":1636365542,"ttl":60,"version":"2.2","data":{"fr":{"feeds":[{"name":"system_information.json","url":"https://example.com/system_information.json"},{"name":"free_bike_status.json","url":"https://example.com/free_bike_status.json"},{"name":"vehicle_types.json","url":"https://example.com/vehicle_types.json"},{"name":"system_pricing_plans.json","url":"https://example.com/system_pricing_plans.json"},{"name":"station_information.json","url":"https://example.com/station_information.json"},{"name":"station_status.json","url":"https://example.com/station_status.json"},{"name":"geofencing_zones.json","url":"https://example.com/geofencing_zones.json"},{"name":"gbfs_versions.json","url":"https://example.com/gbfs_versions.json"}]}}}
"""

setup_response_with_headers(@gbfs_url, body)
setup_response(@gbfs_url, body)
end

defp setup_gbfs_versions_response do
Expand Down
2 changes: 1 addition & 1 deletion apps/transport/lib/transport/gbfs_metadata.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule Transport.GBFSMetadata do
def compute_feed_metadata(%DB.Resource{url: url}), do: compute_feed_metadata(url)

def compute_feed_metadata(url) when is_binary(url),
do: Transport.Shared.GBFSMetadata.Wrapper.compute_feed_metadata(url, TransportWeb.Endpoint.url())
do: Transport.Shared.GBFSMetadata.Wrapper.compute_feed_metadata(url)

def validator_name, do: "GBFS-Validator"
end
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,6 @@
</td>
</tr>
<% end %>

<% cors_header_value = Map.get(@metadata, :cors_header_value) %>
<tr>
<td>
<%= dgettext("gbfs_analyzer", "CORS headers") %>
</td>
<td>
<%= if cors_header_value do %>
Access-Control-Allow-Origin: <%= cors_header_value %>
<% else %>
<%= dgettext("gbfs_analyzer", "None") %>
<% end %>
</td>
</tr>
</table>

<h2><%= dgettext("gbfs_analyzer", "Visualization") %></h2>
Expand Down
2 changes: 1 addition & 1 deletion apps/transport/lib/validators/gbfs_validator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ defmodule Transport.Validators.GBFSValidator do

@impl Transport.Validators.Validator
def validate_and_save(%DB.Resource{url: url, format: "gbfs", id: resource_id}) do
result = GBFSMetadata.compute_feed_metadata(url, "https://#{Application.fetch_env!(:transport, :domain_name)}")
result = GBFSMetadata.compute_feed_metadata(url)

{validator_version, validation_result} = result |> Map.fetch!(:validation) |> Map.pop!(:validator_version)

Expand Down
8 changes: 0 additions & 8 deletions apps/transport/priv/gettext/en/LC_MESSAGES/gbfs_analyzer.po
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,3 @@ msgstr ""
#, elixir-autogen, elixir-format
msgid "validation was not performed"
msgstr "Validation was not performed. There may be a fatal error with your feed or the GBFS validator is not available."

#, elixir-autogen, elixir-format, fuzzy
msgid "CORS headers"
msgstr ""

#, elixir-autogen, elixir-format, fuzzy
msgid "None"
msgstr ""
8 changes: 0 additions & 8 deletions apps/transport/priv/gettext/fr/LC_MESSAGES/gbfs_analyzer.po
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,3 @@ msgstr "Lien"
#, elixir-autogen, elixir-format
msgid "validation was not performed"
msgstr "La validation n'a pas effectuée. Il peut y avoir une erreur fatale dans votre flux ou le validateur GBFS est temporairement indisponible."

#, elixir-autogen, elixir-format
msgid "CORS headers"
msgstr "Headers CORS"

#, elixir-autogen, elixir-format
msgid "None"
msgstr "Aucun"
8 changes: 0 additions & 8 deletions apps/transport/priv/gettext/gbfs_analyzer.pot
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,3 @@ msgstr ""
#, elixir-autogen, elixir-format
msgid "validation was not performed"
msgstr ""

#, elixir-autogen, elixir-format
msgid "CORS headers"
msgstr ""

#, elixir-autogen, elixir-format
msgid "None"
msgstr ""
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ defmodule Transport.Test.Transport.Jobs.GBFSMultiValidationDispatcherJobTest do
assert DB.Resource.gbfs?(resource)

Transport.Shared.GBFSMetadata.Mock
|> expect(:compute_feed_metadata, fn ^url, "https://www.example.com" ->
|> expect(:compute_feed_metadata, fn ^url ->
%{
languages: ["fr"],
system_details: %{"name" => "velhop", "timezone" => "Europe/Paris"},
Expand All @@ -60,8 +60,7 @@ defmodule Transport.Test.Transport.Jobs.GBFSMultiValidationDispatcherJobTest do
version_validated: "1.1",
validator_version: "31c5325",
validator: :validator_module
},
cors_header_value: "*"
}
}
end)

Expand All @@ -71,7 +70,6 @@ defmodule Transport.Test.Transport.Jobs.GBFSMultiValidationDispatcherJobTest do
metadata: %DB.ResourceMetadata{
metadata: %{
"feeds" => ["system_information", "station_information", "station_status"],
"cors_header_value" => "*",
"languages" => ["fr"],
"system_details" => %{"name" => "velhop", "timezone" => "Europe/Paris"},
"ttl" => 3600,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ defmodule Transport.Validators.GBFSValidatorTest do
assert DB.Resource.gbfs?(resource)

Transport.Shared.GBFSMetadata.Mock
|> expect(:compute_feed_metadata, fn ^url, "https://www.example.com" ->
|> expect(:compute_feed_metadata, fn ^url ->
%{
languages: ["fr"],
system_details: %{"name" => "velhop", "timezone" => "Europe/Paris"},
Expand All @@ -34,8 +34,7 @@ defmodule Transport.Validators.GBFSValidatorTest do
version_validated: "1.1",
validator_version: "31c5325",
validator: :validator_module
},
cors_header_value: "*"
}
}
end)

Expand All @@ -45,7 +44,6 @@ defmodule Transport.Validators.GBFSValidatorTest do
metadata: %DB.ResourceMetadata{
metadata: %{
"feeds" => ["system_information", "station_information", "station_status"],
"cors_header_value" => "*",
"languages" => ["fr"],
"system_details" => %{"name" => "velhop", "timezone" => "Europe/Paris"},
"ttl" => 3600,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ defmodule TransportWeb.GbfsAnalyzerControllerTest do
gbfs_url = "/gbfs.json"

Transport.Shared.GBFSMetadata.Mock
|> expect(:compute_feed_metadata, fn ^gbfs_url, cors_base_url ->
assert cors_base_url == TransportWeb.Endpoint.url()

|> expect(:compute_feed_metadata, fn ^gbfs_url ->
%{
system_details: %{"name" => "GBFS feed name"},
feeds: ["station_information"]
Expand Down

0 comments on commit c55166e

Please sign in to comment.