Début de sortie des lectures d'env à compile time (credo check + 1er refactoring) (#5521)

Author: thbar

.credo.exs

@@ -21,14 +21,21 @@
         # You can give explicit globs or simply directories.
         # In the latter case `**/*.{ex,exs}` will be used.
         #
-        included: ["lib/", "src/", "web/", "apps/"],
-        excluded: [~r"/_build/", ~r"/deps/", ~r"/apps/transport/client/", "apps/transport/lib/transport/protobuf/gtfs-realtime.pb.ex"]
+        included: ["lib/", "src/", "web/", "apps/", "config/"],
+        excluded: [
+          ~r"/_build/",
+          ~r"/deps/",
+          ~r"/apps/transport/client/",
+          "apps/transport/lib/transport/protobuf/gtfs-realtime.pb.ex",
+          # Template only — copied by devs to dev.secret.exs (gitignored), never loaded at runtime.
+          "config/dev.secret.template.exs"
+        ]
       },
       #
       # If you create your own checks, you must specify the source files for
       # them here, so they can be loaded by Credo before running the analysis.
       #
-      requires: [],
+      requires: ["credo_checks/no_system_env_at_compile_time.ex"],
       #
       # If you want to enforce a style guide and need a more traditional linting
       # experience, you can change `strict` to `true` below:
@@ -117,6 +124,9 @@
         {Credo.Check.Warning.UnusedTupleOperation},
         {Credo.Check.Warning.RaiseInsideRescue},
         {Credo.Check.Warning.MixEnv},
+        {Credo.Check.Warning.ApplicationConfigInModuleAttribute},
+        # During migration, do not block the CI.
+        {Credo.Check.Custom.NoSystemEnvAtCompileTime, exit_status: 0},
 
         # Controversial and experimental checks (opt-in, just remove `, false`)
         #

config/config.exs

@@ -76,6 +76,8 @@ config :phoenix, :template_engines,
 
 config :phoenix_markdown, :server_tags, :all
 
+config :transport, gbfs_validator_impl: Shared.Validation.GBFSValidator.HTTPValidatorClient
+
 config :transport,
   cache_impl: Transport.Cache.Cachex,
   ex_aws_impl: ExAws,
@@ -231,12 +233,12 @@ config :appsignal, :config,
     "Unlock.Controller#fetch"
   ]
 
+config :transport, DB.Repo, types: DB.PostgrexTypes
+config :transport, ecto_repos: [DB.Repo]
+
 # Import environment specific config. This must remain at the bottom
 # of this file so it overrides the configuration defined above.
 import_config "datagouvfr.exs"
-import_config "database.exs"
-import_config "gtfs_validator.exs"
-import_config "gbfs_validator.exs"
 import_config "mail.exs"
 import_config "data_sharing_pilot.exs"
 import_config "#{config_env()}.exs"

config/database.exs

@@ -46,6 +46,19 @@ config :transport,
   dynamic_irve_tick_interval: :timer.seconds(if(config_env() == :dev, do: 10, else: 30)),
   dynamic_irve_initial_sync: config_env() != :test
 
+config :transport,
+  # This endpoint is not really public but we can use it for now
+  # See https://github.com/MobilityData/gbfs-validator/issues/53#issuecomment-957917240
+  gbfs_validator_url:
+    System.get_env("GBFS_VALIDATOR_URL", "https://gbfs-validator.netlify.app/.netlify/functions/validator"),
+  gbfs_validator_website: System.get_env("GBFS_VALIDATOR_WEBSITE", "https://gbfs-validator.netlify.app")
+
+# In :test, gtfs_validator_url is set in test.exs.
+if config_env() != :test do
+  config :transport,
+    gtfs_validator_url: System.get_env("GTFS_VALIDATOR_URL", "https://validation.transport.data.gouv.fr")
+end
+
 # Inside IEx, we do not want jobs to start processing, nor plugins working.
 # The jobs can be heavy and for instance in production, one person could
 # unknowningly create duplicate RAM heavy jobs. With this trick, we can still
@@ -236,6 +249,12 @@ if config_env() == :dev do
     watchers: if(webserver, do: [npm: ["run", "--prefix", "apps/transport/client", "watch"]], else: [])
 end
 
+if config_env() == :dev do
+  config :transport, DB.Repo,
+    url: System.get_env("PG_URL") || "ecto://postgres:postgres@localhost/transport_repo",
+    pool_size: (System.get_env("PG_POOL_SIZE") || "10") |> String.to_integer()
+end
+
 if config_env() == :prod do
   pool_size =
     case app_env do
@@ -262,8 +281,10 @@ if config_env() == :prod do
     ],
     socket_options: [keepalive: true],
     # See https://hexdocs.pm/db_connection/DBConnection.html#start_link/2-queue-config
-    # [Ecto.Repo] :pool_timeout is no longer supported in favor of a new queue system described in DBConnection.start_link/2
-    # under "Queue config". For most users, configuring :timeout is enough, as it now includes both queue and query time
+    # [Ecto.Repo] :pool_timeout is no longer supported in favor of a new queue system
+    # described in DBConnection.start_link/2
+    # under "Queue config". For most users, configuring :timeout is enough,
+    # as it now includes both queue and query time
     timeout: 15_000
 
   config :transport, TransportWeb.Endpoint,

config/gbfs_validator.exs

@@ -92,9 +92,6 @@ config :oauth2, Authentication,
   client_id: "my_client_id",
   client_secret: "my_client_secret"
 
-# Validator configuration
-config :transport, gtfs_validator_url: System.get_env("GTFS_VALIDATOR_URL") || "http://127.0.0.1:7878"
-
 config :exvcr,
   vcr_cassette_library_dir: "test/fixture/cassettes",
   filter_request_headers: ["authorization"]

config/gtfs_validator.exs

@@ -0,0 +1,40 @@
+defmodule Credo.Check.Custom.NoSystemEnvAtCompileTime do
+  @moduledoc "Make sure to keep an env-free compilation (for Elixir releases)."
+  use Credo.Check, base_priority: :normal, category: :refactor
+
+  @forbidden ~w(get_env fetch_env fetch_env!)a
+
+  @impl true
+  def run(%SourceFile{filename: filename} = source_file, params \\ []) do
+    if compile_time_config?(filename) do
+      issue_meta = IssueMeta.for(source_file, params)
+      Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
+    else
+      []
+    end
+  end
+
+  defp compile_time_config?(filename) do
+    rel = Path.relative_to_cwd(filename)
+    String.starts_with?(rel, "config/") and rel != "config/runtime.exs"
+  end
+
+  # AST shape of any dotted call `_.<fun>(...args)`:
+  #   {{:., _, [_module, fun]}, call_meta, args}
+  defp traverse(
+         {{:., _, [_module, fun]}, meta, _args} = ast,
+         issues,
+         issue_meta
+       )
+       when fun in @forbidden do
+    issue =
+      format_issue(issue_meta,
+        message: "#{fun} forbidden at compile time; move to config/runtime.exs",
+        line_no: meta[:line]
+      )
+
+    {ast, [issue | issues]}
+  end
+
+  defp traverse(ast, issues, _issue_meta), do: {ast, issues}
+end