From 97b5fd426e9ca2dda4906cf8e987fe1ed03ead96 Mon Sep 17 00:00:00 2001 From: Dustin Farris Date: Wed, 10 Mar 2021 07:01:48 -0800 Subject: [PATCH] fix type errors (#14) The child_spec was initializing the worker with a tuple instead of a propslist. Also, the Snowflex module was referencing two types: * `:odbc.odbc_data_type` * `:odbc.value` These types are defined in the odbc OTP documentation, but do not actually exist in the OTP source code. This change corrects the child_spec call and steals the rest of the pertinent odbc type documentation and implements it for ourselves. Dialyzer has been installed to verify these changes, however the child_spec change, which is part of the Connection __using__ callback, must be (and has been) verified in a consuming library. --- lib/snowflex.ex | 29 ++++++++++++++++++++++++++--- lib/snowflex/connection.ex | 6 +++++- lib/snowflex/worker.ex | 6 +++++- mix.exs | 1 + mix.lock | 2 ++ test/snowflex/worker_test.exs | 18 ++++++++++++++---- 6 files changed, 53 insertions(+), 9 deletions(-) diff --git a/lib/snowflex.ex b/lib/snowflex.ex index a37b756..ec9f552 100644 --- a/lib/snowflex.ex +++ b/lib/snowflex.ex @@ -9,11 +9,34 @@ defmodule Snowflex do alias Ecto.Changeset alias Snowflex.Worker - @type query_param :: {:odbc.odbc_data_type(), list(:odbc.value())} + # Shamelessly copied from http://erlang.org/doc/man/odbc.html#common-data-types- + @type precision :: integer() + @type scale :: integer() + @type size :: integer() + @type odbc_data_type :: + :sql_integer + | :sql_smallint + | :sql_tinyint + | {:sql_decimal, precision(), scale()} + | {:sql_numeric, precision(), scale()} + | {:sql_char, size()} + | {:sql_wchar, size()} + | {:sql_varchar, size()} + | {:sql_wvarchar, size()} + | {:sql_float, precision()} + | {:sql_wlongvarchar, size()} + | {:sql_float, precision()} + | :sql_real + | :sql_double + | :sql_bit + | atom() + @type value :: nil | term() + + @type query_param :: {odbc_data_type(), [value()]} @type sql_data :: list(%{optional(String.t()) => String.t()}) @spec sql_query(atom(), String.t(), timeout()) :: - sql_data() | {:error, term} + sql_data() | {:error, term()} def sql_query(pool_name, query, timeout) do case :poolboy.transaction( pool_name, @@ -26,7 +49,7 @@ defmodule Snowflex do end @spec param_query(atom(), String.t(), list(query_param()), timeout()) :: - sql_data() | {:error, term} + sql_data() | {:error, term()} def param_query(pool_name, query, params \\ [], timeout) do case :poolboy.transaction( pool_name, diff --git a/lib/snowflex/connection.ex b/lib/snowflex/connection.ex index fdadca3..76df85c 100644 --- a/lib/snowflex/connection.ex +++ b/lib/snowflex/connection.ex @@ -121,7 +121,11 @@ defmodule Snowflex.Connection do {:max_overflow, min_pool_size} ] - :poolboy.child_spec(@name, opts, {connection, @keep_alive?, @heartbeat_interval}) + :poolboy.child_spec(@name, opts, + connection_args: connection, + keep_alive?: @keep_alive?, + heartbeat_interval: @heartbeat_interval + ) end @impl Snowflex.Connection diff --git a/lib/snowflex/worker.ex b/lib/snowflex/worker.ex index d05a318..6c944d3 100644 --- a/lib/snowflex/worker.ex +++ b/lib/snowflex/worker.ex @@ -28,7 +28,11 @@ defmodule Snowflex.Worker do ## GENSERVER CALL BACKS @impl GenServer - def init({connection_args, keep_alive?, heartbeat_interval}) do + def init( + connection_args: connection_args, + keep_alive?: keep_alive?, + heartbeat_interval: heartbeat_interval + ) do send(self(), {:start, connection_args, keep_alive?, heartbeat_interval}) {:ok, %{backoff: :backoff.init(2, 60), state: :not_connected}} end diff --git a/mix.exs b/mix.exs index 52c1236..4b3803b 100644 --- a/mix.exs +++ b/mix.exs @@ -44,6 +44,7 @@ defmodule Snowflex.MixProject do {:poolboy, "~> 1.5.1"}, {:backoff, "~> 1.1.6"}, {:ecto, "~> 3.0"}, + {:dialyxir, "~> 1.0", only: :dev, runtime: false}, {:ex_doc, "~> 0.21", only: :dev, runtime: false}, {:meck, "~> 0.9", only: :test} ] diff --git a/mix.lock b/mix.lock index 01228e1..e5be934 100644 --- a/mix.lock +++ b/mix.lock @@ -1,9 +1,11 @@ %{ "backoff": {:hex, :backoff, "1.1.6", "83b72ed2108ba1ee8f7d1c22e0b4a00cfe3593a67dbc792799e8cce9f42f796b", [:rebar3], [], "hexpm", "cf0cfff8995fb20562f822e5cc47d8ccf664c5ecdc26a684cbe85c225f9d7c39"}, "decimal": {:hex, :decimal, "2.0.0", "a78296e617b0f5dd4c6caf57c714431347912ffb1d0842e998e9792b5642d697", [:mix], [], "hexpm", "34666e9c55dea81013e77d9d87370fe6cb6291d1ef32f46a1600230b1d44f577"}, + "dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"}, "earmark": {:hex, :earmark, "1.4.3", "364ca2e9710f6bff494117dbbd53880d84bebb692dafc3a78eb50aa3183f2bfd", [:mix], [], "hexpm", "8cf8a291ebf1c7b9539e3cddb19e9cef066c2441b1640f13c34c1d3cfc825fec"}, "earmark_parser": {:hex, :earmark_parser, "1.4.10", "6603d7a603b9c18d3d20db69921527f82ef09990885ed7525003c7fe7dc86c56", [:mix], [], "hexpm", "8e2d5370b732385db2c9b22215c3f59c84ac7dda7ed7e544d7c459496ae519c0"}, "ecto": {:hex, :ecto, "3.5.5", "48219a991bb86daba6e38a1e64f8cea540cded58950ff38fbc8163e062281a07", [:mix], [{:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "98dd0e5e1de7f45beca6130d13116eae675db59adfa055fb79612406acf6f6f1"}, + "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"}, "ex_doc": {:hex, :ex_doc, "0.23.0", "a069bc9b0bf8efe323ecde8c0d62afc13d308b1fa3d228b65bca5cf8703a529d", [:mix], [{:earmark_parser, "~> 1.4.0", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "f5e2c4702468b2fd11b10d39416ddadd2fcdd173ba2a0285ebd92c39827a5a16"}, "makeup": {:hex, :makeup, "1.0.5", "d5a830bc42c9800ce07dd97fa94669dfb93d3bf5fcf6ea7a0c67b2e0e4a7f26c", [:mix], [{:nimble_parsec, "~> 0.5 or ~> 1.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cfa158c02d3f5c0c665d0af11512fed3fba0144cf1aadee0f2ce17747fba2ca9"}, "makeup_elixir": {:hex, :makeup_elixir, "0.15.0", "98312c9f0d3730fde4049985a1105da5155bfe5c11e47bdc7406d88e01e4219b", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.1", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "75ffa34ab1056b7e24844c90bfc62aaf6f3a37a15faa76b07bc5eba27e4a8b4a"}, diff --git a/test/snowflex/worker_test.exs b/test/snowflex/worker_test.exs index 2b35a76..9236aab 100644 --- a/test/snowflex/worker_test.exs +++ b/test/snowflex/worker_test.exs @@ -8,6 +8,16 @@ defmodule Snowflex.WorkerTest do role: "DEV", warehouse: "CUSTOMER_DEV_WH" ] + @without_keep_alive [ + connection_args: @connection_args, + keep_alive?: false, + heartbeat_interval: 10 + ] + @with_keep_alive [ + connection_args: @connection_args, + keep_alive?: true, + heartbeat_interval: 10 + ] setup do :meck.new(:odbc, [:no_link]) @@ -22,7 +32,7 @@ defmodule Snowflex.WorkerTest do end test "does not send a heartbeat if `keep_alive?` is false" do - start_supervised!({Snowflex.Worker, {@connection_args, false, 10}}) + start_supervised!({Snowflex.Worker, @without_keep_alive}) Process.sleep(15) assert :meck.num_calls(:odbc, :sql_query, ["mock pid", 'SELECT 1']) == 0 @@ -30,7 +40,7 @@ defmodule Snowflex.WorkerTest do test "sends heartbeat every interval if `keep_alive?` is true" do assert capture_log(fn -> - start_supervised!({Snowflex.Worker, {@connection_args, true, 10}}) + start_supervised!({Snowflex.Worker, @with_keep_alive}) Process.sleep(30) end) =~ "sending heartbeat" @@ -44,7 +54,7 @@ defmodule Snowflex.WorkerTest do refute( capture_log(fn -> - worker = start_supervised!({Snowflex.Worker, {@connection_args, true, 10}}) + worker = start_supervised!({Snowflex.Worker, @with_keep_alive}) Process.sleep(7) Snowflex.Worker.sql_query(worker, "SELECT * FROM my_table") Process.sleep(7) @@ -63,7 +73,7 @@ defmodule Snowflex.WorkerTest do refute( capture_log(fn -> - worker = start_supervised!({Snowflex.Worker, {@connection_args, true, 10}}) + worker = start_supervised!({Snowflex.Worker, @with_keep_alive}) Process.sleep(7) Snowflex.Worker.param_query(worker, "SELECT * FROM my_table WHERE name=?", [