Replace HTTPoison with Tesla on AdaptorRegistry (#2861)

* Replace HTTPoison with Tesla on AdaptorRegistry We want to eventually drop any HTTP code that uses hackney or httpc in favour of Finch/Mint based clients. This replaces the use of HTTPoison (hackney under the hood), for the AdaptorRegistry. Other places that still need refactoring include the WellKnown module and the `install_schemas` mix task.
OpenFn · Jan 24, 2025 · 29ee3e1 · 29ee3e1
1 parent 625219d
commit 29ee3e1
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 99 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,12 +23,13 @@ and this project adheres to
 ### Changed
 
 - Remove snapshot creation when performing the Github sync - no longer needed
-  post-migration.
-  [#2703](https://github.com/OpenFn/lightning/issues/2703)
+  post-migration. [#2703](https://github.com/OpenFn/lightning/issues/2703)
 - Remove some redundant code related to `WorkOrders.create_for`.
   [#2703](https://github.com/OpenFn/lightning/issues/2703)
 - Remove use of Snapshot.get_or_create_latest_for from tests.
   [#2703](https://github.com/OpenFn/lightning/issues/2703)
+- Replaced HTTPoison with Tesla in the AdaptorRegistry.
+  [#2861](https://github.com/OpenFn/lightning/pull/2861)
 
 ### Fixed
 

diff --git a/lib/lightning/adaptor_registry.ex b/lib/lightning/adaptor_registry.ex
@@ -48,17 +48,12 @@ defmodule Lightning.AdaptorRegistry do
     @moduledoc """
     NPM API functions
     """
-    use HTTPoison.Base
 
-    @impl true
-    def process_request_url(url) do
-      "https://registry.npmjs.org" <> url
-    end
-
-    @impl true
-    def process_response_body(body) do
-      body
-      |> Jason.decode!()
+    def client do
+      Tesla.client([
+        {Tesla.Middleware.BaseUrl, "https://registry.npmjs.org"},
+        Tesla.Middleware.JSON
+      ])
     end
 
     @doc """
@@ -67,12 +62,9 @@ defmodule Lightning.AdaptorRegistry do
     """
     @spec user_packages(user :: String.t()) :: [map()]
     def user_packages(user) do
-      get("/-/user/#{user}/package", [],
-        hackney: [pool: :default],
-        recv_timeout: 15_000
-      )
+      Tesla.get(client(), "/-/user/#{user}/package")
       |> case do
-        {:error, %HTTPoison.Error{reason: :nxdomain, id: nil}} ->
+        {:error, :nxdomain} ->
           Logger.info("Unable to connect to NPM; no adaptors fetched.")
           []
 
@@ -86,10 +78,7 @@ defmodule Lightning.AdaptorRegistry do
     """
     @spec package_detail(package_name :: String.t()) :: map()
     def package_detail(package_name) do
-      get!("/#{package_name}", [],
-        hackney: [pool: :default],
-        recv_timeout: 15_000
-      ).body
+      Tesla.get!(client(), "/#{package_name}").body
     end
   end
 

diff --git a/lib/mix/tasks/download_adaptor_registry_cache.ex b/lib/mix/tasks/download_adaptor_registry_cache.ex
@@ -11,7 +11,8 @@ defmodule Mix.Tasks.Lightning.DownloadAdaptorRegistryCache do
   alias Lightning.AdaptorRegistry
 
   def run(args) do
-    HTTPoison.start()
+    Application.ensure_started(:telemetry)
+    Finch.start_link(name: Lightning.Finch)
 
     case AdaptorRegistry.fetch() do
       [] ->

diff --git a/mix.lock b/mix.lock
@@ -152,7 +152,7 @@
   "telemetry_metrics_prometheus_core": {:hex, :telemetry_metrics_prometheus_core, "1.2.1", "c9755987d7b959b557084e6990990cb96a50d6482c683fb9622a63837f3cd3d8", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:telemetry_metrics, "~> 0.6 or ~> 1.0", [hex: :telemetry_metrics, repo: "hexpm", optional: false]}], "hexpm", "5e2c599da4983c4f88a33e9571f1458bf98b0cf6ba930f1dc3a6e8cf45d5afb6"},
   "telemetry_poller": {:hex, :telemetry_poller, "1.1.0", "58fa7c216257291caaf8d05678c8d01bd45f4bdbc1286838a28c4bb62ef32999", [:rebar3], [{:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "9eb9d9cbfd81cbd7cdd24682f8711b6e2b691289a0de6826e58452f28c103c8f"},
   "telemetry_registry": {:hex, :telemetry_registry, "0.3.1", "14a3319a7d9027bdbff7ebcacf1a438f5f5c903057b93aee484cca26f05bdcba", [:mix, :rebar3], [{:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "6d0ca77b691cf854ed074b459a93b87f4c7f5512f8f7743c635ca83da81f939e"},
-  "tesla": {:hex, :tesla, "1.13.0", "24a068a48d107080dd7c943a593997eee265977a38020eb2ab657cca78a12502", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: true]}, {:finch, "~> 0.13", [hex: :finch, repo: "hexpm", optional: true]}, {:fuse, "~> 2.4", [hex: :fuse, repo: "hexpm", optional: true]}, {:gun, ">= 1.0.0", [hex: :gun, repo: "hexpm", optional: true]}, {:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: true]}, {:ibrowse, "4.4.2", [hex: :ibrowse, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: true]}, {:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.0", [hex: :mint, repo: "hexpm", optional: true]}, {:mox, "~> 1.0", [hex: :mox, repo: "hexpm", optional: true]}, {:msgpax, "~> 2.3", [hex: :msgpax, repo: "hexpm", optional: true]}, {:poison, ">= 1.0.0", [hex: :poison, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: true]}], "hexpm", "7b8fc8f6b0640fa0d090af7889d12eb396460e044b6f8688a8e55e30406a2200"},
+  "tesla": {:hex, :tesla, "1.13.2", "85afa342eb2ac0fee830cf649dbd19179b6b359bec4710d02a3d5d587f016910", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: true]}, {:finch, "~> 0.13", [hex: :finch, repo: "hexpm", optional: true]}, {:fuse, "~> 2.4", [hex: :fuse, repo: "hexpm", optional: true]}, {:gun, ">= 1.0.0", [hex: :gun, repo: "hexpm", optional: true]}, {:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: true]}, {:ibrowse, "4.4.2", [hex: :ibrowse, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: true]}, {:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.0", [hex: :mint, repo: "hexpm", optional: true]}, {:mox, "~> 1.0", [hex: :mox, repo: "hexpm", optional: true]}, {:msgpax, "~> 2.3", [hex: :msgpax, repo: "hexpm", optional: true]}, {:poison, ">= 1.0.0", [hex: :poison, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: true]}], "hexpm", "960609848f1ef654c3cdfad68453cd84a5febecb6ed9fed9416e36cd9cd724f9"},
   "timex": {:hex, :timex, "3.7.11", "bb95cb4eb1d06e27346325de506bcc6c30f9c6dea40d1ebe390b262fad1862d1", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:gettext, "~> 0.20", [hex: :gettext, repo: "hexpm", optional: false]}, {:tzdata, "~> 1.1", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm", "8b9024f7efbabaf9bd7aa04f65cf8dcd7c9818ca5737677c7b76acbc6a94d1aa"},
   "tls_certificate_check": {:hex, :tls_certificate_check, "1.20.0", "1ac0c53f95e201feb8d398ef9d764ae74175231289d89f166ba88a7f50cd8e73", [:rebar3], [{:ssl_verify_fun, "~> 1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm", "ab57b74b1a63dc5775650699a3ec032ec0065005eff1f020818742b7312a8426"},
   "tzdata": {:hex, :tzdata, "1.1.2", "45e5f1fcf8729525ec27c65e163be5b3d247ab1702581a94674e008413eef50b", [:mix], [{:hackney, "~> 1.17", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "cec7b286e608371602318c414f344941d5eb0375e14cfdab605cca2fe66cba8b"},

diff --git a/test/lightning/adaptor_registry_test.exs b/test/lightning/adaptor_registry_test.exs
@@ -1,20 +1,15 @@
 defmodule Lightning.AdaptorRegistryTest do
   use ExUnit.Case, async: false
-  use Mimic
 
-  alias Lightning.AdaptorRegistry
-
-  describe "start_link/1" do
-    # AdaptorRegistry is a GenServer, and so stubbed (external) functions must
-    # be mocked globally. See: https://github.com/edgurgel/mimic#private-and-global-mode
-    setup :set_mimic_from_context
+  import Mox
+  import Tesla.Test
 
-    setup do
-      stub(:hackney)
+  setup :set_mox_from_context
+  setup :verify_on_exit!
 
-      :ok
-    end
+  alias Lightning.AdaptorRegistry
 
+  describe "start_link/1" do
     test "uses cache from a specific location" do
       file_path =
         Briefly.create!(extname: ".json")
@@ -38,19 +33,26 @@ defmodule Lightning.AdaptorRegistryTest do
     end
 
     test "retrieves a list of adaptors when caching is disabled" do
-      # :hackney.request(request.method, request.url, request.headers, request.body, hn_options)
-      expect(:hackney, :request, fn
-        :get,
-        "https://registry.npmjs.org/-/user/openfn/package",
-        [],
-        "",
-        [recv_timeout: 15_000, pool: :default] ->
-          {:ok, 200, "headers", :client}
-      end)
-
-      expect(:hackney, :body, fn :client, _timeout ->
-        {:ok, File.read!("test/fixtures/openfn-packages-npm.json")}
-      end)
+      default_npm_response =
+        File.read!("test/fixtures/language-common-npm.json") |> Jason.decode!()
+
+      expect_tesla_call(
+        times: 7,
+        returns: fn env, [] ->
+          case env.url do
+            "https://registry.npmjs.org/-/user/openfn/package" ->
+              {:ok,
+               json(
+                 %Tesla.Env{status: 200},
+                 File.read!("test/fixtures/openfn-packages-npm.json")
+                 |> Jason.decode!()
+               )}
+
+            "https://registry.npmjs.org/@openfn/" <> _adaptor ->
+              {:ok, json(%Tesla.Env{status: 200}, default_npm_response)}
+          end
+        end
+      )
 
       expected_adaptors = [
         "@openfn/language-asana",
@@ -61,26 +63,31 @@ defmodule Lightning.AdaptorRegistryTest do
         "@openfn/language-salesforce"
       ]
 
-      stub(:hackney, :body, fn :adaptor, _timeout ->
-        {:ok, File.read!("test/fixtures/language-common-npm.json")}
-      end)
-
-      stub(:hackney, :request, fn
-        :get,
-        "https://registry.npmjs.org/" <> adaptor,
-        [],
-        "",
-        [recv_timeout: 15_000, pool: :default] ->
-          assert adaptor in expected_adaptors
-          {:ok, 200, "headers", :adaptor}
-      end)
-
       start_supervised!(
         {AdaptorRegistry, [name: :test_adaptor_registry, use_cache: false]}
       )
 
       results = AdaptorRegistry.all(:test_adaptor_registry)
 
+      assert_received_tesla_call(env, [])
+
+      assert_tesla_env(env, %Tesla.Env{
+        method: :get,
+        url: "https://registry.npmjs.org/-/user/openfn/package"
+      })
+
+      1..length(expected_adaptors)
+      |> Enum.each(fn _ ->
+        assert_received_tesla_call(env, [])
+
+        assert %Tesla.Env{
+                 method: :get,
+                 url: "https://registry.npmjs.org/" <> adaptor
+               } = env
+
+        assert adaptor in expected_adaptors
+      end)
+
       assert length(results) == 6
 
       versions = [

diff --git a/test/lightning/download_adaptor_registry_test.exs b/test/lightning/download_adaptor_registry_test.exs
@@ -1,26 +1,29 @@
 defmodule Lightning.DownloadAdaptorRegistryCacheTest do
   use ExUnit.Case, async: false
-  use Mimic
+
+  import Mox
+  import Tesla.Test
+
+  setup :set_mox_from_context
+  setup :verify_on_exit!
 
   alias Mix.Tasks.Lightning.DownloadAdaptorRegistryCache
 
   describe "download_adaptor_registry_cache mix task" do
     @describetag :tmp_dir
-    setup do
-      stub(:hackney)
-
-      :ok
-    end
-
     test "does not write file when no adaptors are found", %{tmp_dir: tmp_dir} do
-      expect(:hackney, :request, fn
-        :get,
-        "https://registry.npmjs.org/-/user/openfn/package",
-        [],
-        "",
-        [recv_timeout: 15_000, pool: :default] ->
-          {:error, :nxdomain}
-      end)
+      expect_tesla_call(
+        times: 1,
+        returns: fn env, [] ->
+          case env.url do
+            "https://registry.npmjs.org/-/user/openfn/package" ->
+              {:ok, json(%Tesla.Env{status: 200}, [])}
+
+            "https://registry.npmjs.org/@openfn/language-asana" ->
+              {:ok, json(%Tesla.Env{status: 200}, [])}
+          end
+        end
+      )
 
       file_path = Path.join([tmp_dir, "cache.json"])
       refute File.exists?(file_path)
@@ -31,31 +34,26 @@ defmodule Lightning.DownloadAdaptorRegistryCacheTest do
     end
 
     test "writes to specified file", %{tmp_dir: tmp_dir} do
-      expect(:hackney, :request, fn
-        :get,
-        "https://registry.npmjs.org/-/user/openfn/package",
-        [],
-        "",
-        [recv_timeout: 15_000, pool: :default] ->
-          {:ok, 200, "headers", :client}
-      end)
-
-      expect(:hackney, :body, fn :client, _timeout ->
-        {:ok, File.read!("test/fixtures/openfn-packages-npm.json")}
-      end)
+      language_common_response =
+        File.read!("test/fixtures/language-common-npm.json") |> Jason.decode!()
 
-      stub(:hackney, :body, fn :adaptor, _timeout ->
-        {:ok, File.read!("test/fixtures/language-common-npm.json")}
-      end)
+      expect_tesla_call(
+        times: 7,
+        returns: fn env, [] ->
+          case env.url do
+            "https://registry.npmjs.org/-/user/openfn/package" ->
+              {:ok,
+               json(
+                 %Tesla.Env{status: 200},
+                 File.read!("test/fixtures/openfn-packages-npm.json")
+                 |> Jason.decode!()
+               )}
 
-      stub(:hackney, :request, fn
-        :get,
-        "https://registry.npmjs.org/" <> _adaptor,
-        [],
-        "",
-        [recv_timeout: 15_000, pool: :default] ->
-          {:ok, 200, "headers", :adaptor}
-      end)
+            "https://registry.npmjs.org/@openfn/" <> _adaptor ->
+              {:ok, json(%Tesla.Env{status: 200}, language_common_response)}
+          end
+        end
+      )
 
       file_path = Path.join([tmp_dir, "cache.json"])
       refute File.exists?(file_path)