From 4dbcbb984b77ad5b4a96b5343eac1bb097735f60 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sat, 6 May 2017 21:01:04 +0900 Subject: [PATCH 01/18] Updated mix.lock format --- mix.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mix.lock b/mix.lock index d1313a4..b15d727 100644 --- a/mix.lock +++ b/mix.lock @@ -1,4 +1,4 @@ -%{"fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], []}, - "poison": {:hex, :poison, "1.5.0", "f2f4f460623a6f154683abae34352525e1d918380267cdbd949a07ba57503248", [:mix], []}, - "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], []}, - "porcelain": {:hex, :porcelain, "2.0.1", "9c3db2b47d8cf6879c0d9ac79db8657333974a88faff09e856569e00c1b5e119", [:mix], []}} +%{"fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], [], "hexpm"}, + "poison": {:hex, :poison, "1.5.0", "f2f4f460623a6f154683abae34352525e1d918380267cdbd949a07ba57503248", [:mix], [], "hexpm"}, + "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"}, + "porcelain": {:hex, :porcelain, "2.0.1", "9c3db2b47d8cf6879c0d9ac79db8657333974a88faff09e856569e00c1b5e119", [:mix], [], "hexpm"}} From 851e7880915bdd881cc33e1d92814b07a22edf71 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sat, 6 May 2017 21:41:12 +0900 Subject: [PATCH 02/18] Setup test environment --- test/fixtures/echo.py | 5 +++++ test/std_json_io_test.exs | 5 +++-- test/test_helper.exs | 4 ++++ 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100755 test/fixtures/echo.py diff --git a/test/fixtures/echo.py b/test/fixtures/echo.py new file mode 100755 index 0000000..6335461 --- /dev/null +++ b/test/fixtures/echo.py @@ -0,0 +1,5 @@ +#!/usr/bin/env python +import sys +for line in iter(sys.stdin.readline, ''): + line = line.rstrip('\n') + sys.stdout.write('{"response": '+ line + '}'), diff --git a/test/std_json_io_test.exs b/test/std_json_io_test.exs index fff7ed6..9a5b2e2 100644 --- a/test/std_json_io_test.exs +++ b/test/std_json_io_test.exs @@ -2,7 +2,8 @@ defmodule StdJsonIoTest do use ExUnit.Case doctest StdJsonIo - test "the truth" do - assert 1 + 1 == 2 + setup do + {:ok, _} = StdJsonIoMock.start_link([]) + {:ok, %{}} end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 869559e..9f0824c 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1 +1,5 @@ ExUnit.start() + +defmodule StdJsonIoMock do + use StdJsonIo, otp_app: :std_json_io, script: "python -u test/fixtures/echo.py" +end From 339f811c1ed6e8f70d212e30502816cabb7cfa01 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sat, 6 May 2017 22:01:00 +0900 Subject: [PATCH 03/18] Updated call to Supervisor.start_link by passing :atom instead of {:local, :atom} as a name --- lib/std_json_io.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std_json_io.ex b/lib/std_json_io.ex index 619b16d..93a176e 100644 --- a/lib/std_json_io.ex +++ b/lib/std_json_io.ex @@ -14,7 +14,7 @@ defmodule StdJsonIo do def start_link(opts \\ []) do - Supervisor.start_link(__MODULE__, :ok, name: {:local, __MODULE__}) + Supervisor.start_link(__MODULE__, :ok, name: __MODULE__) end def init(:ok) do From 1aacfe3489637e0980693390cd3dfa8e9d062aed Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sat, 6 May 2017 22:28:41 +0900 Subject: [PATCH 04/18] Updated StdJsonIo.Worker to append trailing newline to data sent to external program --- lib/std_json_io/worker.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std_json_io/worker.ex b/lib/std_json_io/worker.ex index 668ea53..72bf6f7 100644 --- a/lib/std_json_io/worker.ex +++ b/lib/std_json_io/worker.ex @@ -17,7 +17,7 @@ defmodule StdJsonIo.Worker do nil -> {:error, :json_error} {:error, reason} -> {:error, reason} {:ok, json} -> - Proc.send_input(state.js_proc, json) + Proc.send_input(state.js_proc, json <> "\n") receive do {_js_pid, :data, :out, msg} -> {:reply, {:ok, msg}, state} From 17f44d56f8ba42b8aeaa3b5ecebdfebf9aeffa72 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sat, 6 May 2017 22:29:21 +0900 Subject: [PATCH 05/18] Added test for StdJsonIo.json_call and StdJsonIo.json_call! --- test/std_json_io_test.exs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/std_json_io_test.exs b/test/std_json_io_test.exs index 9a5b2e2..03c8c9c 100644 --- a/test/std_json_io_test.exs +++ b/test/std_json_io_test.exs @@ -6,4 +6,16 @@ defmodule StdJsonIoTest do {:ok, _} = StdJsonIoMock.start_link([]) {:ok, %{}} end + + test "Call to json_call returns correct value" do + message = %{"hello" => "world"} + expected = {:ok, %{"response" => message}} + assert StdJsonIoMock.json_call(message) == expected + end + + test "Call to json_call! returns correct value" do + message = %{"hello" => "world"} + expected = %{"response" => message} + assert StdJsonIoMock.json_call!(message) == expected + end end From a968e06d66749ce035854e910885d2933900f46a Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sat, 6 May 2017 23:51:43 +0900 Subject: [PATCH 06/18] Fixed processing big response --- lib/std_json_io/worker.ex | 26 +++++++++++++++++++------- test/std_json_io_test.exs | 6 ++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/std_json_io/worker.ex b/lib/std_json_io/worker.ex index 72bf6f7..1181d1c 100644 --- a/lib/std_json_io/worker.ex +++ b/lib/std_json_io/worker.ex @@ -17,13 +17,25 @@ defmodule StdJsonIo.Worker do nil -> {:error, :json_error} {:error, reason} -> {:error, reason} {:ok, json} -> - Proc.send_input(state.js_proc, json <> "\n") - receive do - {_js_pid, :data, :out, msg} -> - {:reply, {:ok, msg}, state} - response -> - {:reply, {:error, response}, state} - end + receiver = fn f, data -> + receive do + {_pid, :data, :out, msg} -> + new_data = data <> msg + case Poison.decode(new_data) do + {:error, _} -> + # Couldn't decode JSON, there are more chunks + # to receive and concat with + f.(f, new_data) + {:ok, _} -> + # All chunks received + {:reply, {:ok, new_data}, state} + end + other -> + {:reply, {:error, other}, state} + end + end + Proc.send_input(state.js_proc, json <> "\n") + receiver.(receiver, "") end end diff --git a/test/std_json_io_test.exs b/test/std_json_io_test.exs index 03c8c9c..d2fe787 100644 --- a/test/std_json_io_test.exs +++ b/test/std_json_io_test.exs @@ -18,4 +18,10 @@ defmodule StdJsonIoTest do expected = %{"response" => message} assert StdJsonIoMock.json_call!(message) == expected end + + test "Can handle big response" do + message = %{"thisishuge" => String.duplicate("Lorem Ipsum Dolor Sit Amet", 10000)} + expected = {:ok, %{"response" => message}} + assert StdJsonIoMock.json_call(message) == expected + end end From c92cab01484c0dc9068c79bab79161b1f1dd61af Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 02:11:01 +0900 Subject: [PATCH 07/18] Removed Reloader module and fs dependency --- lib/std_json_io.ex | 24 ------------------------ lib/std_json_io/reloader.ex | 23 ----------------------- mix.exs | 6 ++---- mix.lock | 3 +-- 4 files changed, 3 insertions(+), 53 deletions(-) delete mode 100644 lib/std_json_io/reloader.ex diff --git a/lib/std_json_io.ex b/lib/std_json_io.ex index 93a176e..d286b56 100644 --- a/lib/std_json_io.ex +++ b/lib/std_json_io.ex @@ -29,33 +29,9 @@ defmodule StdJsonIo do children = [:poolboy.child_spec(@pool_name, pool_options, [script: script])] - files = Keyword.get(@options, :watch_files) - - if files && length(files) > 0 do - Application.ensure_started(:fs, :permanent) - - reloader_spec = worker( - StdJsonIo.Reloader, - [__MODULE__, Enum.map(files, &Path.expand/1)], - [] - ) - - children = [reloader_spec | children] - end - supervise(children, strategy: :one_for_one, name: __MODULE__) end - def restart_io_workers! do - case Process.whereis(@pool_name) do - nil -> - Supervisor.restart_child(__MODULE__, @pool_name) - _pid -> - Supervisor.terminate_child(__MODULE__, @pool_name) - Supervisor.restart_child(__MODULE__, @pool_name) - end - end - def json_call!(map, timeout \\ 10000) do case json_call(map, timeout) do {:ok, data} -> data diff --git a/lib/std_json_io/reloader.ex b/lib/std_json_io/reloader.ex deleted file mode 100644 index 639b1f5..0000000 --- a/lib/std_json_io/reloader.ex +++ /dev/null @@ -1,23 +0,0 @@ -defmodule StdJsonIo.Reloader do - use GenServer - - def start_link(mod, files) do - GenServer.start_link(__MODULE__, [mod, files], name: {:local, __MODULE__}) - end - - def init([mod, files]) do - :fs.subscribe() - {:ok, %{files: files, mod: mod}} - end - - def handle_info({_, {:fs, :file_event}, {path, _}}, %{files: files, mod: mod} = state) do - if Enum.member?(files, path |> to_string) do - mod.restart_io_workers! - end - {:noreply, state} - end - - def handle_info(_msg, state) do - {:noreply, state} - end -end diff --git a/mix.exs b/mix.exs index f617d84..80cdf01 100644 --- a/mix.exs +++ b/mix.exs @@ -23,8 +23,7 @@ defmodule StdJsonIo.Mixfile do def application do [ - applications: [:logger, :porcelain], - included_applications: [:fs] + applications: [:logger, :porcelain] ] end @@ -40,8 +39,7 @@ defmodule StdJsonIo.Mixfile do [ {:porcelain, "~> 2.0"}, {:poolboy, "~> 1.5.1"}, - {:poison, "~> 1.5.0"}, - {:fs, "~> 0.9.1"}, + {:poison, "~> 1.5.0"} ] end diff --git a/mix.lock b/mix.lock index b15d727..3c440f8 100644 --- a/mix.lock +++ b/mix.lock @@ -1,4 +1,3 @@ -%{"fs": {:hex, :fs, "0.9.2", "ed17036c26c3f70ac49781ed9220a50c36775c6ca2cf8182d123b6566e49ec59", [:rebar], [], "hexpm"}, - "poison": {:hex, :poison, "1.5.0", "f2f4f460623a6f154683abae34352525e1d918380267cdbd949a07ba57503248", [:mix], [], "hexpm"}, +%{"poison": {:hex, :poison, "1.5.0", "f2f4f460623a6f154683abae34352525e1d918380267cdbd949a07ba57503248", [:mix], [], "hexpm"}, "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"}, "porcelain": {:hex, :porcelain, "2.0.1", "9c3db2b47d8cf6879c0d9ac79db8657333974a88faff09e856569e00c1b5e119", [:mix], [], "hexpm"}} From eadb6f6ffc59d87314cbcfd0bbd6c90d7c6e1efa Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 02:25:17 +0900 Subject: [PATCH 08/18] Added OTP Application and Supervisor --- lib/std_json_io/application.ex | 10 ++++++++++ mix.exs | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 lib/std_json_io/application.ex diff --git a/lib/std_json_io/application.ex b/lib/std_json_io/application.ex new file mode 100644 index 0000000..208bfa2 --- /dev/null +++ b/lib/std_json_io/application.ex @@ -0,0 +1,10 @@ +defmodule StdJsonIo.Application do + use Application + + def start(_type, _args) do + import Supervisor.Spec, warn: false + children = [] + opts = [strategy: :one_for_one, name: StdJsonIo.Supervisor] + Supervisor.start_link(children, opts) + end +end diff --git a/mix.exs b/mix.exs index 80cdf01..81a24ba 100644 --- a/mix.exs +++ b/mix.exs @@ -23,7 +23,8 @@ defmodule StdJsonIo.Mixfile do def application do [ - applications: [:logger, :porcelain] + applications: [:logger, :porcelain], + mod: {StdJsonIo.Application, []} ] end From 65ad6b9e91646ae2a66dda35809f81e3e4f42115 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 11:44:54 +0900 Subject: [PATCH 09/18] Moved init functionality to OTP Application start\2 function --- config/config.exs | 4 ++ lib/std_json_io.ex | 70 +++++++++------------------------- lib/std_json_io/application.ex | 11 +++++- test/std_json_io_test.exs | 11 ++---- test/test_helper.exs | 4 -- 5 files changed, 34 insertions(+), 66 deletions(-) diff --git a/config/config.exs b/config/config.exs index f5dd697..073e25e 100644 --- a/config/config.exs +++ b/config/config.exs @@ -28,3 +28,7 @@ use Mix.Config # here (which is why it is important to import them last). # # import_config "#{Mix.env}.exs" +config :std_json_io, + pool_size: 5, + pool_max_overflow: 10, + script: "python -u test/fixtures/echo.py" diff --git a/lib/std_json_io.ex b/lib/std_json_io.ex index d286b56..09e1017 100644 --- a/lib/std_json_io.ex +++ b/lib/std_json_io.ex @@ -1,60 +1,24 @@ defmodule StdJsonIo do - - defmacro __using__(opts) do - otp_app = Keyword.get(opts, :otp_app) - - if !otp_app do - raise "StdJsonIo requires an otp_app" + def json_call!(map, timeout \\ 10000) do + case json_call(map, timeout) do + {:ok, data} -> data + {:error, reason } -> raise "Failed to call to json service #{__MODULE__} #{to_string(reason)}" end + end - quote do - use Supervisor - @pool_name Module.concat(__MODULE__, Pool) - @options Keyword.merge(unquote(opts), (Application.get_env(unquote(otp_app), __MODULE__) || [])) - - - def start_link(opts \\ []) do - Supervisor.start_link(__MODULE__, :ok, name: __MODULE__) - end - - def init(:ok) do - pool_options = [ - name: {:local, @pool_name}, - worker_module: StdJsonIo.Worker, - size: Keyword.get(@options, :pool_size, 5), - max_overflow: Keyword.get(@options, :max_overflow, 10) - ] - - script = Keyword.get(@options, :script) - - children = [:poolboy.child_spec(@pool_name, pool_options, [script: script])] - - supervise(children, strategy: :one_for_one, name: __MODULE__) - end - - def json_call!(map, timeout \\ 10000) do - case json_call(map, timeout) do - {:ok, data} -> data - {:error, reason } -> raise "Failed to call to json service #{__MODULE__} #{to_string(reason)}" - end - end - - def json_call(map, timeout \\ 10000) do - result = :poolboy.transaction(@pool_name, fn worker -> - GenServer.call(worker, {:json, map}, timeout) - end) - - case result do - {:ok, json} -> - {:ok, data} = Poison.decode(json) - if data["error"] do - {:error, Map.get(data, "error")} - else - {:ok, data} - end - other -> other + def json_call(map, timeout \\ 10000) do + result = :poolboy.transaction(StdJsonIo.Pool, fn worker -> + GenServer.call(worker, {:json, map}, timeout) + end) + case result do + {:ok, json} -> + {:ok, data} = Poison.decode(json) + if data["error"] do + {:error, Map.get(data, "error")} + else + {:ok, data} end - end + other -> other end end end diff --git a/lib/std_json_io/application.ex b/lib/std_json_io/application.ex index 208bfa2..d7efc5a 100644 --- a/lib/std_json_io/application.ex +++ b/lib/std_json_io/application.ex @@ -3,7 +3,16 @@ defmodule StdJsonIo.Application do def start(_type, _args) do import Supervisor.Spec, warn: false - children = [] + config = Application.get_all_env(:std_json_io) + pool_options = [ + name: {:local, StdJsonIo.Pool}, + worker_module: StdJsonIo.Worker, + size: Keyword.get(config, :pool_size, 15), + max_overflow: Keyword.get(config, :pool_max_overflow, 10) + ] + children = [ + :poolboy.child_spec(StdJsonIo.Pool, pool_options, [script: Keyword.fetch!(config, :script)]) + ] opts = [strategy: :one_for_one, name: StdJsonIo.Supervisor] Supervisor.start_link(children, opts) end diff --git a/test/std_json_io_test.exs b/test/std_json_io_test.exs index d2fe787..80807fa 100644 --- a/test/std_json_io_test.exs +++ b/test/std_json_io_test.exs @@ -2,26 +2,21 @@ defmodule StdJsonIoTest do use ExUnit.Case doctest StdJsonIo - setup do - {:ok, _} = StdJsonIoMock.start_link([]) - {:ok, %{}} - end - test "Call to json_call returns correct value" do message = %{"hello" => "world"} expected = {:ok, %{"response" => message}} - assert StdJsonIoMock.json_call(message) == expected + assert StdJsonIo.json_call(message) == expected end test "Call to json_call! returns correct value" do message = %{"hello" => "world"} expected = %{"response" => message} - assert StdJsonIoMock.json_call!(message) == expected + assert StdJsonIo.json_call!(message) == expected end test "Can handle big response" do message = %{"thisishuge" => String.duplicate("Lorem Ipsum Dolor Sit Amet", 10000)} expected = {:ok, %{"response" => message}} - assert StdJsonIoMock.json_call(message) == expected + assert StdJsonIo.json_call(message) == expected end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 9f0824c..869559e 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,5 +1 @@ ExUnit.start() - -defmodule StdJsonIoMock do - use StdJsonIo, otp_app: :std_json_io, script: "python -u test/fixtures/echo.py" -end From 3670bb90f65b970c2dcf4114f62e5f8aadb00613 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 12:03:11 +0900 Subject: [PATCH 10/18] Upgraded Poison and Porcelain dependencies --- lib/std_json_io/worker.ex | 8 ++++---- mix.exs | 4 ++-- mix.lock | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/std_json_io/worker.ex b/lib/std_json_io/worker.ex index 1181d1c..2f2cd1d 100644 --- a/lib/std_json_io/worker.ex +++ b/lib/std_json_io/worker.ex @@ -22,13 +22,13 @@ defmodule StdJsonIo.Worker do {_pid, :data, :out, msg} -> new_data = data <> msg case Poison.decode(new_data) do - {:error, _} -> - # Couldn't decode JSON, there are more chunks - # to receive and concat with - f.(f, new_data) {:ok, _} -> # All chunks received {:reply, {:ok, new_data}, state} + _ -> + # Couldn't decode JSON, there are more chunks + # to receive and concat with + f.(f, new_data) end other -> {:reply, {:error, other}, state} diff --git a/mix.exs b/mix.exs index 81a24ba..04ecb91 100644 --- a/mix.exs +++ b/mix.exs @@ -38,9 +38,9 @@ defmodule StdJsonIo.Mixfile do defp deps do [ - {:porcelain, "~> 2.0"}, + {:porcelain, "~> 2.0.3"}, {:poolboy, "~> 1.5.1"}, - {:poison, "~> 1.5.0"} + {:poison, "~> 3.1.0"} ] end diff --git a/mix.lock b/mix.lock index 3c440f8..9854352 100644 --- a/mix.lock +++ b/mix.lock @@ -1,3 +1,3 @@ -%{"poison": {:hex, :poison, "1.5.0", "f2f4f460623a6f154683abae34352525e1d918380267cdbd949a07ba57503248", [:mix], [], "hexpm"}, +%{"poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"}, "poolboy": {:hex, :poolboy, "1.5.1", "6b46163901cfd0a1b43d692657ed9d7e599853b3b21b95ae5ae0a777cf9b6ca8", [:rebar], [], "hexpm"}, - "porcelain": {:hex, :porcelain, "2.0.1", "9c3db2b47d8cf6879c0d9ac79db8657333974a88faff09e856569e00c1b5e119", [:mix], [], "hexpm"}} + "porcelain": {:hex, :porcelain, "2.0.3", "2d77b17d1f21fed875b8c5ecba72a01533db2013bd2e5e62c6d286c029150fdc", [:mix], [], "hexpm"}} From 15e57b1497edcb29f1a857dfdf1537b012fc692c Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 15:48:13 +0900 Subject: [PATCH 11/18] Fixed warnings --- config/config.exs | 2 ++ mix.exs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/config/config.exs b/config/config.exs index 073e25e..d9f50f3 100644 --- a/config/config.exs +++ b/config/config.exs @@ -32,3 +32,5 @@ config :std_json_io, pool_size: 5, pool_max_overflow: 10, script: "python -u test/fixtures/echo.py" + +config :porcelain, driver: Porcelain.Driver.Basic diff --git a/mix.exs b/mix.exs index 04ecb91..0ced0d8 100644 --- a/mix.exs +++ b/mix.exs @@ -17,8 +17,8 @@ defmodule StdJsonIo.Mixfile do maintainers: @maintainers, description: "Application for managing and communicating with IO servers via JSON", homepage_url: @url, - docs: docs, - deps: deps] + docs: docs(), + deps: deps()] end def application do From e5885a226bf00db1317892d939a84c4f8425b7df Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 21:47:50 +0900 Subject: [PATCH 12/18] Updated Worker design --- lib/std_json_io.ex | 24 ++++++++------- lib/std_json_io/worker.ex | 62 ++++++++++++++------------------------- 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/lib/std_json_io.ex b/lib/std_json_io.ex index 09e1017..5a398c0 100644 --- a/lib/std_json_io.ex +++ b/lib/std_json_io.ex @@ -6,19 +6,21 @@ defmodule StdJsonIo do end end - def json_call(map, timeout \\ 10000) do + def json_call(data, timeout \\ 10000) do result = :poolboy.transaction(StdJsonIo.Pool, fn worker -> - GenServer.call(worker, {:json, map}, timeout) + GenServer.cast(worker, {:json, data, self()}) + receive do + response -> + response + after + timeout -> + %{"error" => "timeout"} + end end) - case result do - {:ok, json} -> - {:ok, data} = Poison.decode(json) - if data["error"] do - {:error, Map.get(data, "error")} - else - {:ok, data} - end - other -> other + if result["error"] do + {:error, Map.get(result, "error")} + else + {:ok, result} end end end diff --git a/lib/std_json_io/worker.ex b/lib/std_json_io/worker.ex index 2f2cd1d..2f84970 100644 --- a/lib/std_json_io/worker.ex +++ b/lib/std_json_io/worker.ex @@ -1,6 +1,6 @@ defmodule StdJsonIo.Worker do use GenServer - alias Porcelain.Process, as: Proc + alias Porcelain.Process, as: PProc alias Porcelain.Result def start_link(opts \\ []) do @@ -8,53 +8,35 @@ defmodule StdJsonIo.Worker do end def init(script) do - :erlang.process_flag(:trap_exit, true) - {:ok, %{js_proc: start_io_server(script)}} + Process.flag(:trap_exit, true) + pproc = Porcelain.spawn_shell(script, in: :receive, out: {:send, self()}) + {:ok, %{pproc: pproc, buffer: "", reply_to: nil}} end - def handle_call({:json, blob}, _from, state) do - case Poison.encode(blob) do - nil -> {:error, :json_error} - {:error, reason} -> {:error, reason} - {:ok, json} -> - receiver = fn f, data -> - receive do - {_pid, :data, :out, msg} -> - new_data = data <> msg - case Poison.decode(new_data) do - {:ok, _} -> - # All chunks received - {:reply, {:ok, new_data}, state} - _ -> - # Couldn't decode JSON, there are more chunks - # to receive and concat with - f.(f, new_data) - end - other -> - {:reply, {:error, other}, state} - end - end - Proc.send_input(state.js_proc, json <> "\n") - receiver.(receiver, "") - end + def handle_cast({:json, data, reply_to}, %{pproc: pproc} = state) do + {:ok, json} = Poison.encode(data) + PProc.send_input(pproc, json <> "\n") + {:noreply, %{state | reply_to: reply_to}} end - def handle_call(:stop, _from, state), do: {:stop, :normal, :ok, state} - + def handle_info({pproc_pid, :data, :out, data}, %{pproc: %PProc{pid: pproc_pid}, buffer: buffer} = state) do + new_buffer = buffer <> data + case Poison.decode(new_buffer) do + {:ok, decoded} -> + Process.send(state[:reply_to], decoded, []) + {:noreply, %{state | buffer: ""}} + _ -> + {:noreply, %{state | buffer: new_buffer}} + end + end # The js server has stopped - def handle_info({_js_pid, :result, %Result{err: _, status: _status}}, state) do + def handle_info({pproc_pid, :result, %Result{err: _, status: _status}}, %{pproc: %PProc{pid: pproc_pid}} = state) do {:stop, :normal, state} end - def terminate(_reason, %{js_proc: server}) do - Proc.signal(server, :kill) - Proc.stop(server) + def terminate(_reason, %{pproc: pproc}) do + PProc.signal(pproc, :kill) + PProc.stop(pproc) :ok end - - def terminate(_reason, _state), do: :ok - - defp start_io_server(script) do - Porcelain.spawn_shell(script, in: :receive, out: {:send, self()}) - end end From 3a088deada8a2c3e5d0135beb1a670bfb156d286 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Sun, 7 May 2017 22:21:31 +0900 Subject: [PATCH 13/18] Updated README --- README.md | 64 +++++++++++++++++-------------------------------------- 1 file changed, 19 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index f78dec2..bd7293f 100644 --- a/README.md +++ b/README.md @@ -9,62 +9,36 @@ JSON on STDOUT. ## Installation -If [available in Hex](https://hex.pm/docs/publish), the package can be installed as: - - 1. Add `std_json_io` to your list of dependencies in `mix.exs`: - - def deps do - [{:std_json_io, "~> 0.1.0"}] - end - - 2. Ensure `std_json_io` is started before your application: - - def application do - [applications: [:std_json_io]] - end - -### Setup - -Define a module and use StdJsonIo. - +1. Add `std_json_io` to your list of dependencies in `mix.exs`: ```elixir -defmodule MyApp.ReactIo do - use StdJsonIo, otp_app: :my_app +def deps do + [{:std_json_io, "~> 0.1.0"}] end ``` - -When you use `StdJsonIo` your module becomes a supervisor. You'll need to add it -to your supervision tree. - +2. Ensure `std_json_io` is started before your application: ```elixir -children = [ - # snip - supervisor(MyApp.ReactIo, []) -] - -opts = [strategy: :one_for_one, name: MyApp] - -Supervisor.start_link(children, opts) +def application do + [applications: [:std_json_io]] +end ``` - - ### Configuration You can either configure as additional arguments of the use statement, or in your config file. ```elixir -config :my_app, MyApp.ReactIo, - pool_size: 20, # default 5 - max_overflow: 10, # default 10 - script: "path/to/script", # for react-io use "react-stdio" - watch_files: [ - Path.join([__DIR__, "../priv/server/js/component.js"]) # do not watch files in dev - ] +config :std_json_io, + pool_size: 5, + pool_max_overflow: 10, + script: "node_modules/.bin/react-stdio" ``` +* `pool_size` - see [Poolboy options](https://github.com/devinus/poolboy#options), option "size" +* `pool_max_overflow` - See [Poolboy options](https://github.com/devinus/poolboy#options), option "max_overflow" * `script` - the script to run for the IO server -* `watch_files` - A list of files to watch for changes. When the file changes, - kill the IO worker and restart, picking up any changes. Use only in dev. -* `pool_size` - The size for the pool of workers - See poolboy `size` -* `max_overflow` - The poolboy `max_overflow` +### Usage example +```elixir +{:ok, data} = StdJsonIo.json_call(%{"component" => "my/component.js"} +# or +data = StdJsonIo.json_call!(%{"component" => "my/component.js"} +``` From abe78207415d345ef85bf0521e6682d33ab7e9a5 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Mon, 15 May 2017 01:37:24 +0900 Subject: [PATCH 14/18] Relaxed Poison dependency to allow 2.0 --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 0ced0d8..3b6fcfc 100644 --- a/mix.exs +++ b/mix.exs @@ -40,7 +40,7 @@ defmodule StdJsonIo.Mixfile do [ {:porcelain, "~> 2.0.3"}, {:poolboy, "~> 1.5.1"}, - {:poison, "~> 3.1.0"} + {:poison, "~> 2.0 or ~> 3.1.0"} ] end From 271d882272c7902f0a7e28391401b727259fbb77 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Mon, 15 May 2017 15:22:09 +0900 Subject: [PATCH 15/18] Added check if incoming message in StdJsonIo is from Worker --- lib/std_json_io.ex | 2 +- lib/std_json_io/worker.ex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std_json_io.ex b/lib/std_json_io.ex index 5a398c0..67f9e02 100644 --- a/lib/std_json_io.ex +++ b/lib/std_json_io.ex @@ -10,7 +10,7 @@ defmodule StdJsonIo do result = :poolboy.transaction(StdJsonIo.Pool, fn worker -> GenServer.cast(worker, {:json, data, self()}) receive do - response -> + {:std_json_io_response, response} -> response after timeout -> diff --git a/lib/std_json_io/worker.ex b/lib/std_json_io/worker.ex index 2f84970..3ed3c0d 100644 --- a/lib/std_json_io/worker.ex +++ b/lib/std_json_io/worker.ex @@ -23,7 +23,7 @@ defmodule StdJsonIo.Worker do new_buffer = buffer <> data case Poison.decode(new_buffer) do {:ok, decoded} -> - Process.send(state[:reply_to], decoded, []) + Process.send(state[:reply_to], {:std_json_io_response, decoded}, []) {:noreply, %{state | buffer: ""}} _ -> {:noreply, %{state | buffer: new_buffer}} From 7045e256cf2269b546ceb1009cdca1db8a6537d2 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Tue, 16 May 2017 21:40:30 +0900 Subject: [PATCH 16/18] Refactored timeout/error handling --- README.md | 3 +++ config/config.exs | 2 +- lib/std_json_io.ex | 11 ++--------- lib/std_json_io/worker.ex | 40 +++++++++++++++++++++++++++++++-------- test/fixtures/echo.py | 14 +++++++++++++- test/std_json_io_test.exs | 32 +++++++++++++++++++++++++++++++ test/test_helper.exs | 1 + 7 files changed, 84 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index bd7293f..21f8be4 100644 --- a/README.md +++ b/README.md @@ -42,3 +42,6 @@ config :std_json_io, # or data = StdJsonIo.json_call!(%{"component" => "my/component.js"} ``` + +### Development +There are some tests taking long to run (testing timeouts, long replies, etc.) with tag `long: true` which are excluded by default. To run all the tests including long, you have to run `mix test --include long:true` diff --git a/config/config.exs b/config/config.exs index d9f50f3..7851172 100644 --- a/config/config.exs +++ b/config/config.exs @@ -31,6 +31,6 @@ use Mix.Config config :std_json_io, pool_size: 5, pool_max_overflow: 10, - script: "python -u test/fixtures/echo.py" + script: "python -u test/fixtures/echo.py 2>/dev/null" config :porcelain, driver: Porcelain.Driver.Basic diff --git a/lib/std_json_io.ex b/lib/std_json_io.ex index 67f9e02..1ac7ce2 100644 --- a/lib/std_json_io.ex +++ b/lib/std_json_io.ex @@ -2,20 +2,13 @@ defmodule StdJsonIo do def json_call!(map, timeout \\ 10000) do case json_call(map, timeout) do {:ok, data} -> data - {:error, reason } -> raise "Failed to call to json service #{__MODULE__} #{to_string(reason)}" + {:error, reason } -> raise "Failed to call to json service, reason: #{to_string(reason)}" end end def json_call(data, timeout \\ 10000) do result = :poolboy.transaction(StdJsonIo.Pool, fn worker -> - GenServer.cast(worker, {:json, data, self()}) - receive do - {:std_json_io_response, response} -> - response - after - timeout -> - %{"error" => "timeout"} - end + GenServer.call(worker, {:json, data, timeout}, :infinity) end) if result["error"] do {:error, Map.get(result, "error")} diff --git a/lib/std_json_io/worker.ex b/lib/std_json_io/worker.ex index 3ed3c0d..86664a9 100644 --- a/lib/std_json_io/worker.ex +++ b/lib/std_json_io/worker.ex @@ -3,6 +3,14 @@ defmodule StdJsonIo.Worker do alias Porcelain.Process, as: PProc alias Porcelain.Result + @initial_state %{ + pproc: nil, + buffer: "", + from: nil, + timer: false, + stop_reason: nil + } + def start_link(opts \\ []) do GenServer.start_link(__MODULE__, opts[:script], opts) end @@ -10,33 +18,49 @@ defmodule StdJsonIo.Worker do def init(script) do Process.flag(:trap_exit, true) pproc = Porcelain.spawn_shell(script, in: :receive, out: {:send, self()}) - {:ok, %{pproc: pproc, buffer: "", reply_to: nil}} + {:ok, %{@initial_state | pproc: pproc}} end - def handle_cast({:json, data, reply_to}, %{pproc: pproc} = state) do + def handle_call({:json, data, timeout}, from, %{pproc: pproc} = state) do {:ok, json} = Poison.encode(data) PProc.send_input(pproc, json <> "\n") - {:noreply, %{state | reply_to: reply_to}} + timer = Process.send_after(self(), :timeout, timeout) + {:noreply, %{state | from: from, timer: timer}} end def handle_info({pproc_pid, :data, :out, data}, %{pproc: %PProc{pid: pproc_pid}, buffer: buffer} = state) do new_buffer = buffer <> data case Poison.decode(new_buffer) do {:ok, decoded} -> - Process.send(state[:reply_to], {:std_json_io_response, decoded}, []) - {:noreply, %{state | buffer: ""}} + Process.cancel_timer(state[:timer]) + GenServer.reply(state[:from], decoded) + {:noreply, %{state | buffer: "", timer: false}} _ -> {:noreply, %{state | buffer: new_buffer}} end end # The js server has stopped def handle_info({pproc_pid, :result, %Result{err: _, status: _status}}, %{pproc: %PProc{pid: pproc_pid}} = state) do - {:stop, :normal, state} + {:stop, :normal, %{state | stop_reason: "Server have been terminated"}} + end + # Complete response was not received within given timeout + # Stop the server with appropriate reason + def handle_info(:timeout, state) do + {:stop, :normal, %{state | stop_reason: "timeout"}} end - def terminate(_reason, %{pproc: pproc}) do - PProc.signal(pproc, :kill) + def terminate(_reason, %{pproc: pproc, timer: timer, from: from, buffer: buffer, stop_reason: stop_reason}) do + unless timer == false do + # Process is being terminated while client is awaiting response + error = %{ + "message" => stop_reason, + "buffer" => buffer + } + GenServer.reply(from, %{"error" => error}) + Process.cancel_timer(timer) + end PProc.stop(pproc) + PProc.signal(pproc, :kill) :ok end end diff --git a/test/fixtures/echo.py b/test/fixtures/echo.py index 6335461..1ca8675 100755 --- a/test/fixtures/echo.py +++ b/test/fixtures/echo.py @@ -1,5 +1,17 @@ #!/usr/bin/env python import sys +import time + for line in iter(sys.stdin.readline, ''): line = line.rstrip('\n') - sys.stdout.write('{"response": '+ line + '}'), + if line == "{\"test\":\"sleep3s\"}": + time.sleep(3) + sys.stdout.write('{"response": '+ line + '}'), + elif line == "{\"test\":\"error\"}": + sys.stdout.write('{"error": '+ line + '}'), + elif line == "{\"test\":\"crash\"}": + raise Exception('some exception') + elif line == "{\"test\":\"not_json\"}": + sys.stdout.write('plaintext'), + else: + sys.stdout.write('{"response": '+ line + '}'), diff --git a/test/std_json_io_test.exs b/test/std_json_io_test.exs index 80807fa..21c222b 100644 --- a/test/std_json_io_test.exs +++ b/test/std_json_io_test.exs @@ -19,4 +19,36 @@ defmodule StdJsonIoTest do expected = {:ok, %{"response" => message}} assert StdJsonIo.json_call(message) == expected end + + @tag long: true + test "Can handle reply taking 3s" do + message = %{"test" => "sleep3s"} + expected = {:ok, %{"response" => message}} + assert StdJsonIo.json_call(message) == expected + end + + @tag long: true + test "Proper timeout error is returned in case of timeout" do + message = %{"test" => "sleep3s"} + expected = {:error, %{"message" => "timeout", "buffer" => ""}} + assert StdJsonIo.json_call(message, 1000) == expected + end + + test "Can handle error key in program response" do + message = %{"test" => "error"} + expected = {:error, message} + assert StdJsonIo.json_call(message) == expected + end + + test "Can handle program crash" do + message = %{"test" => "crash"} + expected = {:error, %{"message" => "Server have been terminated", "buffer" => ""}} + assert StdJsonIo.json_call(message) == expected + end + + test "Can handle incorrect response from program" do + message = %{"test" => "not_json"} + expected = {:error, %{"message" => "timeout", "buffer" => "plaintext"}} + assert StdJsonIo.json_call(message) == expected + end end diff --git a/test/test_helper.exs b/test/test_helper.exs index 869559e..f5da227 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1 +1,2 @@ ExUnit.start() +ExUnit.configure(exclude: [long: true]) From 9994afbb1751a3db6f90ad6b887f9ac2f936ad74 Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Tue, 16 May 2017 21:44:09 +0900 Subject: [PATCH 17/18] Updated test to not be unnecessarily long --- test/std_json_io_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/std_json_io_test.exs b/test/std_json_io_test.exs index 21c222b..04f429d 100644 --- a/test/std_json_io_test.exs +++ b/test/std_json_io_test.exs @@ -49,6 +49,6 @@ defmodule StdJsonIoTest do test "Can handle incorrect response from program" do message = %{"test" => "not_json"} expected = {:error, %{"message" => "timeout", "buffer" => "plaintext"}} - assert StdJsonIo.json_call(message) == expected + assert StdJsonIo.json_call(message, 500) == expected end end From 90c73f80a07fc0080eb8fba5ccc48911160ae80a Mon Sep 17 00:00:00 2001 From: Roman Chvanikov Date: Tue, 16 May 2017 22:15:01 +0900 Subject: [PATCH 18/18] Changed Poolboy strategy from lifo to fifo --- lib/std_json_io/application.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/std_json_io/application.ex b/lib/std_json_io/application.ex index d7efc5a..08224a4 100644 --- a/lib/std_json_io/application.ex +++ b/lib/std_json_io/application.ex @@ -8,7 +8,8 @@ defmodule StdJsonIo.Application do name: {:local, StdJsonIo.Pool}, worker_module: StdJsonIo.Worker, size: Keyword.get(config, :pool_size, 15), - max_overflow: Keyword.get(config, :pool_max_overflow, 10) + max_overflow: Keyword.get(config, :pool_max_overflow, 10), + strategy: :fifo ] children = [ :poolboy.child_spec(StdJsonIo.Pool, pool_options, [script: Keyword.fetch!(config, :script)])