Skip to content

Commit

Permalink
fix type errors (#14)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dustinfarris authored Mar 10, 2021
1 parent f19044b commit 97b5fd4
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 9 deletions.
29 changes: 26 additions & 3 deletions lib/snowflex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion lib/snowflex/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion lib/snowflex/worker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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}
]
Expand Down
2 changes: 2 additions & 0 deletions mix.lock
Original file line number Diff line number Diff line change
@@ -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"},
Expand Down
18 changes: 14 additions & 4 deletions test/snowflex/worker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -22,15 +32,15 @@ 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
end

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"

Expand All @@ -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)
Expand All @@ -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=?", [
Expand Down

0 comments on commit 97b5fd4

Please sign in to comment.