Skip to content

Commit

Permalink
Fix bu reported by @jsimmonds2 (#1420)
Browse files Browse the repository at this point in the history
prevent encoding error when serializing local_id.
  • Loading branch information
RickCarlino authored Oct 28, 2021
1 parent 2c4b044 commit 2229500
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 29 deletions.
8 changes: 7 additions & 1 deletion COVERAGE.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
# Jan - Mar 2021
# Coverage Stats

2021/10/28 - 64.3%

# Historic

Code coverage prior to conversion of app to monolith:

| Project |1/1/20 |2/6/20 |3/4/20 |3/26/21|5/25/21|
|-----------------------|-------|-------|-------|-------|-------|
Expand Down
8 changes: 8 additions & 0 deletions lib/core/asset/repo.ex
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
defmodule FarmbotCore.Asset.Repo do
@moduledoc "Repo for storing Asset data."
use Ecto.Repo, otp_app: :farmbot, adapter: Ecto.Adapters.SQLite3

# Local IDs are binary now.
# This causes lots of issues when printing
# or sending logs to the API. This method exists for
# safety.
def encode_local_id(local_id) do
local_id |> inspect() |> Base.encode64() |> String.slice(0..10)
end
end
6 changes: 4 additions & 2 deletions lib/core/asset_workers/peripheral_worker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ defimpl FarmbotCore.AssetWorker, for: FarmbotCore.Asset.Peripheral do
use GenServer
require Logger

alias FarmbotCore.{Asset.Peripheral, BotState}
alias FarmbotCore.{Asset.Repo, Asset.Peripheral, BotState}
alias FarmbotCore.Celery.AST

@retry_ms 1_000
Expand Down Expand Up @@ -106,8 +106,10 @@ defimpl FarmbotCore.AssetWorker, for: FarmbotCore.Asset.Peripheral do
end

def peripheral_to_rpc(peripheral) do
uuid = Repo.encode_local_id(peripheral.local_id || "unknown")

AST.Factory.new()
|> AST.Factory.rpc_request(peripheral.local_id)
|> AST.Factory.rpc_request(uuid)
|> AST.Factory.set_pin_io_mode(peripheral.pin, "output")
|> AST.Factory.read_pin(peripheral.pin, peripheral.mode)
end
Expand Down
10 changes: 5 additions & 5 deletions lib/ext/dirty_worker.ex
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,11 @@ defmodule FarmbotExt.DirtyWorker do

# HTTP Error. (500, network error, timeout etc.)
def handle_http_response(dirty, module, error) do
FarmbotCore.Logger.error(
2,
"[#{module} #{dirty.local_id} #{inspect(self())}] HTTP Error: #{module} #{inspect(error)}"
)

m = inspect(module)
e = inspect(error)
id = Repo.encode_local_id(dirty.local_id)
msg = "[#{m} #{id} #{inspect(self())}] HTTP Error: #{e}"
FarmbotCore.Logger.error(2, msg)
error
end

Expand Down
45 changes: 45 additions & 0 deletions test/asset_workers/peripheral_worker_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
defmodule FarmbotCore.Asset.PeripheralWorkerTest do
use ExUnit.Case
use Mimic

alias FarmbotCore.Celery.AST
alias FarmbotCore.AssetWorker.FarmbotCore.Asset.Peripheral, as: Worker
alias FarmbotCore.Asset.Peripheral

setup :verify_on_exit!

test "peripheral_to_rpc/1" do
raw = <<154, 60, 54, 238, 159, 53, 21, 160, 176, 202, 22, 177, 28>>
encoded = "PDwxNTQsIDY"

peripheral = %Peripheral{
local_id: raw,
mode: "output",
pin: 123
}

expect(AST.Factory, :new, 1, fn -> :fake_ast1 end)

expect(AST.Factory, :rpc_request, 1, fn ast, uuid ->
assert ast == :fake_ast1
assert uuid == encoded
:fake_ast2
end)

expect(AST.Factory, :set_pin_io_mode, 1, fn ast, pin, mode ->
assert mode == "output"
assert pin == 123
assert ast == :fake_ast2
:fake_ast3
end)

expect(AST.Factory, :read_pin, 1, fn ast, pin, mode ->
assert ast == :fake_ast3
assert pin == 123
assert mode == "output"
:fake_ast4
end)

assert :fake_ast4 == Worker.peripheral_to_rpc(peripheral)
end
end
31 changes: 10 additions & 21 deletions test/farmbot_ext/api/dirty_worker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ defmodule FarmbotExt.DirtyWorkerTest do
DirtyWorker.maybe_resync(0)
end

test "handle_http_response - other response" do
Helpers.delete_all_points()
Repo.delete_all(LocalMeta)
Repo.delete_all(FbosConfig)
Helpers.expect_log("HTTP Error: {:error, :other}")

p = Helpers.create_point(%{id: 2, pointer_type: "Weed"})
DirtyWorker.handle_http_response(p, Point, {:error, :other})
end

test "handle_http_response - 409 response" do
Helpers.delete_all_points()
Repo.delete_all(LocalMeta)
Expand Down Expand Up @@ -106,25 +116,4 @@ defmodule FarmbotExt.DirtyWorkerTest do

assert :ok == DirtyWorker.finalize(stub_data, Point)
end

# This test blinks too much:
#
# test "init" do
# Helpers.delete_all_points()
# Helpers.use_fake_jwt()
# {:ok, pid} = DirtyWorker.start_link(module: Point, timeout: 0)
# state = :sys.get_state(pid)
# assert state == %{module: Point}
# Helpers.wait_for(pid)
# GenServer.stop(pid, :normal)
# end

# This test blinks too much:
#
# test "work/2 error" do
# Helpers.delete_all_points()
# Helpers.use_fake_jwt()
# %mod{} = p = Helpers.create_point(%{id: 0, pointer_type: "Plant"})
# {:error, _} = DirtyWorker.work(p, mod)
# end
end
1 change: 1 addition & 0 deletions test/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ ExUnit.configure(
FarmbotCore.Asset.Repo,
FarmbotCore.BotState,
FarmbotCore.Celery,
FarmbotCore.Celery.AST.Factory,
FarmbotCore.Celery.Compiler.Lua,
FarmbotCore.Celery.Scheduler,
FarmbotCore.Celery.SpecialValue,
Expand Down

0 comments on commit 2229500

Please sign in to comment.