Let’s refactor: std_json_io.

Roman Chvanikov
May 7, 2017 · 15 min read
git add mix.lock && git commit -m 'Updated mix.lock format'

SETTING UP TEST ENVIRONMENT

#!/usr/bin/env python
import sys
for line in iter(sys.stdin.readline, ''):
line = line.rstrip('\n')
sys.stdout.write('{"response": '+ line + '}'),
ExUnit.start()defmodule StdJsonIoMock do
use StdJsonIo, otp_app: :std_json_io, script: "python -u test/fixtures/echo.py"
end
setup do
{:ok, _} = StdJsonIoMock.start_link([])
{:ok, %{}}
end
git add test && git commit -m 'Setup test environment'

Writing first tests

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
git add lib/std_json_io.ex && git commit -m 'Updated call to Supervisor.start_link by passing :atom instead of {:local, :atom} as a name'
git add lib/std_json_io/worker.ex && git commit -m 'Updated StdJsonIo.Worker to append trailing newline to data sent to external program'git add test && git commit -m 'Added test for StdJsonIo.json_call and StdJsonIo.json_call!'

Keep on testing

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
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, "")
git add . && git commit -m 'Fixed processing big response'

Removing StdJsonIo.Reloader

-        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
-      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
-      applications: [:logger, :porcelain],
- included_applications: [:fs]
+ applications: [:logger, :porcelain]
...
- {:poison, "~> 1.5.0"},
- {:fs, "~> 0.9.1"},
+ {:poison, "~> 1.5.0"}
git add . && git commit -m 'Removed Reloader module and fs dependency'

Getting rid of unnecessary module

defmodule Myapp.StdJsonIo do
use StdJsonIo, otp_app: :myapp
end
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
def application do
[
applications: [:logger, :porcelain],
mod: {StdJsonIo.Application, []}
]
end
git add . && git commit -m 'Added OTP Application and Supervisor' 
config :std_json_io,
pool_size: 5,
pool_max_overflow: 10,
script: "python -u test/fixtures/echo.py"
def start(_type, _args) do
import Supervisor.Spec, warn: false
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
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)}"
end
end
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
other -> other
end
end
end
git add . && git commit -m 'Moved init functionality to OTP Application start\2 function'

Updating dependencies

Dependency  Current  Latest  Update possible
poison 1.5.0 3.1.0 No
poolboy 1.5.1 1.5.1
porcelain 2.0.1 2.0.3 Yes
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
git add . && git commit -m 'Upgraded Poison and Porcelain dependencies'
-     docs: docs,
- deps: deps]
+ docs: docs(),
+ deps: deps()]
git add . && git commit -m 'Fixed warnings'

Refactoring is never enough!

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}}
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
-  alias Porcelain.Process, as: Proc
+ alias Porcelain.Process, as: PProc
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
-  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

UPDATE WARNING: following is not the best approach for API <-> Worker communication and have disadvantages. You can check out the library fork repo for a better design approach.

def json_call(data, timeout \\ 10000) do
result = :poolboy.transaction(StdJsonIo.Pool, fn worker ->
GenServer.cast(worker, {:json, data, self()})
receive do
response ->
response
after
timeout ->
%{"error" => "timeout"}
end
end)
if result["error"] do
{:error, Map.get(result, "error")}
else
{:ok, result}
end
end
git add . && git commit -m 'Updated Worker design'

Roman Chvanikov

Written by

Keep calm and curry on

Welcome to a place where words matter. On Medium, smart voices and original ideas take center stage - with no ads in sight. Watch
Follow all the topics you care about, and we’ll deliver the best stories for you to your homepage and inbox. Explore
Get unlimited access to the best stories on Medium — and support writers while you’re at it. Just $5/month. Upgrade