Documentation and test fixes.
parent
8309412f49
commit
336b21ef31
|
@ -47,8 +47,8 @@ defmodule FarmbotCeleryScript.CompilerGroupsTest do
|
||||||
canary_actual = :crypto.hash(:sha, Macro.to_string(result))
|
canary_actual = :crypto.hash(:sha, Macro.to_string(result))
|
||||||
|
|
||||||
canary_expected =
|
canary_expected =
|
||||||
<<157, 69, 5, 38, 188, 78, 10, 183, 154, 99, 151, 193, 214, 208, 187, 130,
|
<<136, 140, 48, 226, 216, 155, 178, 103, 244, 88, 225, 146, 130, 216, 125,
|
||||||
183, 73, 13, 48>>
|
72, 113, 195, 65, 1>>
|
||||||
|
|
||||||
# READ THE NOTE ABOVE IF THIS TEST FAILS!!!
|
# READ THE NOTE ABOVE IF THIS TEST FAILS!!!
|
||||||
assert canary_expected == canary_actual
|
assert canary_expected == canary_actual
|
||||||
|
|
|
@ -79,7 +79,7 @@ defmodule FarmbotCeleryScript.CompilerTest do
|
||||||
end
|
end
|
||||||
|
|
||||||
test "identifier sanitization" do
|
test "identifier sanitization" do
|
||||||
label = "System.cmd(\"rm\", [\"-rf /*\"])"
|
label = "System.cmd(\"echo\", [\"lol\"])"
|
||||||
value_ast = AST.Factory.new("coordinate", x: 1, y: 1, z: 1)
|
value_ast = AST.Factory.new("coordinate", x: 1, y: 1, z: 1)
|
||||||
identifier_ast = AST.Factory.new("identifier", label: label)
|
identifier_ast = AST.Factory.new("identifier", label: label)
|
||||||
|
|
||||||
|
@ -120,11 +120,19 @@ defmodule FarmbotCeleryScript.CompilerTest do
|
||||||
[
|
[
|
||||||
fn params ->
|
fn params ->
|
||||||
_ = inspect(params)
|
_ = inspect(params)
|
||||||
|
unsafe_U3lzdGVtLmNtZCgiZWNobyIsIFsibG9sIl0p = FarmbotCeleryScript.SysCalls.coordinate(1, 1, 1)
|
||||||
|
|
||||||
#{var_name} =
|
better_params = %{
|
||||||
FarmbotCeleryScript.SysCalls.coordinate(1, 1, 1)
|
"System.cmd(\\"echo\\", [\\"lol\\"])" => %FarmbotCeleryScript.AST{
|
||||||
|
args: %{x: 1, y: 1, z: 1},
|
||||||
|
body: [],
|
||||||
|
comment: nil,
|
||||||
|
kind: :coordinate,
|
||||||
|
meta: nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
[fn -> #{var_name} end]
|
[fn -> unsafe_U3lzdGVtLmNtZCgiZWNobyIsIFsibG9sIl0p end]
|
||||||
end
|
end
|
||||||
]
|
]
|
||||||
""")
|
""")
|
||||||
|
@ -372,19 +380,38 @@ defmodule FarmbotCeleryScript.CompilerTest do
|
||||||
unsafe_cGFyZW50 =
|
unsafe_cGFyZW50 =
|
||||||
Keyword.get(params, :unsafe_cGFyZW50, FarmbotCeleryScript.SysCalls.coordinate(1, 2, 3))
|
Keyword.get(params, :unsafe_cGFyZW50, FarmbotCeleryScript.SysCalls.coordinate(1, 2, 3))
|
||||||
|
|
||||||
|
better_params = %{}
|
||||||
|
|
||||||
[
|
[
|
||||||
fn ->
|
fn ->
|
||||||
FarmbotCeleryScript.Compiler.UpdateResource.do_update(
|
me = FarmbotCeleryScript.Compiler.UpdateResource
|
||||||
%FarmbotCeleryScript.AST{
|
|
||||||
args: %{label: "parent"},
|
variable = %FarmbotCeleryScript.AST{
|
||||||
body: [],
|
args: %{label: "parent"},
|
||||||
comment: nil,
|
body: [],
|
||||||
kind: :identifier,
|
comment: nil,
|
||||||
meta: nil
|
kind: :identifier,
|
||||||
},
|
meta: nil
|
||||||
%{"plant_stage" => "removed"},
|
}
|
||||||
[]
|
|
||||||
)
|
update = %{"plant_stage" => "removed"}
|
||||||
|
|
||||||
|
case(variable) do
|
||||||
|
%AST{kind: :identifier} ->
|
||||||
|
args = Map.fetch!(variable, :args)
|
||||||
|
label = Map.fetch!(args, :label)
|
||||||
|
resource = Map.fetch!(better_params, label)
|
||||||
|
me.do_update(resource, update)
|
||||||
|
|
||||||
|
%AST{kind: :point} ->
|
||||||
|
me.do_update(variable.args(), update)
|
||||||
|
|
||||||
|
%AST{kind: :resource} ->
|
||||||
|
me.do_update(variable.args(), update)
|
||||||
|
|
||||||
|
res ->
|
||||||
|
raise("Resource error. Please notfiy support: \#{inspect(res)}")
|
||||||
|
end
|
||||||
end
|
end
|
||||||
]
|
]
|
||||||
end
|
end
|
||||||
|
@ -405,20 +432,38 @@ defmodule FarmbotCeleryScript.CompilerTest do
|
||||||
[
|
[
|
||||||
fn params ->
|
fn params ->
|
||||||
_ = inspect(params)
|
_ = inspect(params)
|
||||||
|
better_params = %{}
|
||||||
|
|
||||||
[
|
[
|
||||||
fn ->
|
fn ->
|
||||||
FarmbotCeleryScript.Compiler.UpdateResource.do_update(
|
me = FarmbotCeleryScript.Compiler.UpdateResource
|
||||||
%FarmbotCeleryScript.AST{
|
|
||||||
args: %{resource_id: 23, resource_type: "Plant"},
|
variable = %FarmbotCeleryScript.AST{
|
||||||
body: [],
|
args: %{resource_id: 23, resource_type: "Plant"},
|
||||||
comment: nil,
|
body: [],
|
||||||
kind: :resource,
|
comment: nil,
|
||||||
meta: nil
|
kind: :resource,
|
||||||
},
|
meta: nil
|
||||||
%{"plant_stage" => "planted", "r" => 23},
|
}
|
||||||
[]
|
|
||||||
)
|
update = %{"plant_stage" => "planted", "r" => 23}
|
||||||
|
|
||||||
|
case(variable) do
|
||||||
|
%AST{kind: :identifier} ->
|
||||||
|
args = Map.fetch!(variable, :args)
|
||||||
|
label = Map.fetch!(args, :label)
|
||||||
|
resource = Map.fetch!(better_params, label)
|
||||||
|
me.do_update(resource, update)
|
||||||
|
|
||||||
|
%AST{kind: :point} ->
|
||||||
|
me.do_update(variable.args(), update)
|
||||||
|
|
||||||
|
%AST{kind: :resource} ->
|
||||||
|
me.do_update(variable.args(), update)
|
||||||
|
|
||||||
|
res ->
|
||||||
|
raise("Resource error. Please notfiy support: \#{inspect(res)}")
|
||||||
|
end
|
||||||
end
|
end
|
||||||
]
|
]
|
||||||
end
|
end
|
||||||
|
|
|
@ -8,7 +8,7 @@ defmodule FarmbotExt.API.DirtyWorker do
|
||||||
require Logger
|
require Logger
|
||||||
require FarmbotCore.Logger
|
require FarmbotCore.Logger
|
||||||
use GenServer
|
use GenServer
|
||||||
@timeout 1
|
@timeout 500
|
||||||
|
|
||||||
# these resources can't be accessed by `id`.
|
# these resources can't be accessed by `id`.
|
||||||
@singular [
|
@singular [
|
||||||
|
@ -42,33 +42,21 @@ defmodule FarmbotExt.API.DirtyWorker do
|
||||||
|
|
||||||
@impl GenServer
|
@impl GenServer
|
||||||
def handle_info(:do_work, %{module: module} = state) do
|
def handle_info(:do_work, %{module: module} = state) do
|
||||||
Process.sleep(1000)
|
Process.sleep(@timeout)
|
||||||
list = Enum.uniq((Private.list_dirty(module) ++ Private.list_local(module)))
|
list = Enum.uniq(Private.list_dirty(module) ++ Private.list_local(module))
|
||||||
|
|
||||||
stale = Enum.find(list, fn item ->
|
unless has_race_condition?(module, list) do
|
||||||
if (item.id) do
|
|
||||||
if item == Repo.get_by(module, id: item.id) do
|
|
||||||
false
|
|
||||||
else
|
|
||||||
IO.inspect(item, label: "=== STALE RECORD DETECTED!")
|
|
||||||
true
|
|
||||||
end
|
|
||||||
else
|
|
||||||
false
|
|
||||||
end
|
|
||||||
end)
|
|
||||||
|
|
||||||
if stale do
|
|
||||||
Process.send_after(self(), :do_work, @timeout)
|
|
||||||
{:noreply, state}
|
|
||||||
else
|
|
||||||
Enum.map(list, fn dirty -> work(dirty, module) end)
|
Enum.map(list, fn dirty -> work(dirty, module) end)
|
||||||
Process.send_after(self(), :do_work, @timeout)
|
|
||||||
{:noreply, state}
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
Process.send_after(self(), :do_work, @timeout)
|
||||||
|
{:noreply, state}
|
||||||
end
|
end
|
||||||
|
|
||||||
def work(dirty, module) do
|
def work(dirty, module) do
|
||||||
|
# Go easy on the API
|
||||||
|
Process.sleep(333)
|
||||||
|
|
||||||
case http_request(dirty, module) do
|
case http_request(dirty, module) do
|
||||||
# Valid data
|
# Valid data
|
||||||
{:ok, %{status: s, body: body}} when s > 199 and s < 300 ->
|
{:ok, %{status: s, body: body}} when s > 199 and s < 300 ->
|
||||||
|
@ -139,4 +127,42 @@ defmodule FarmbotExt.API.DirtyWorker do
|
||||||
data = render(module, dirty)
|
data = render(module, dirty)
|
||||||
API.patch(API.client(), path, data)
|
API.patch(API.client(), path, data)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# This is a fix for a race condtion. The root cause is unknown
|
||||||
|
# as of 18 May 2020. The problem is that records are marked
|
||||||
|
# diry _before_ the dirty data is saved. That means that FBOS
|
||||||
|
# knows a record has changed, but for a brief moment, it only
|
||||||
|
# has the old copy of the record (not the changes).
|
||||||
|
# Because of this race condtion,
|
||||||
|
# The race condition:
|
||||||
|
#
|
||||||
|
# * Is nondeterministic
|
||||||
|
# * Happens frequently when running many MARK AS steps in one go.
|
||||||
|
# * Happens frequently when Erlang VM only has one thread
|
||||||
|
# * Ie: `iex --erl '+S 1 +A 1' -S mix`
|
||||||
|
# * Happens frequently when @timeout is decreased to `1`.
|
||||||
|
#
|
||||||
|
# This function PREVENTS CORRUPTION OF API DATA. It can be
|
||||||
|
# removed once the root cause of the data race is determined.
|
||||||
|
# - RC 18 May 2020
|
||||||
|
def has_race_condition?(module, list) do
|
||||||
|
Enum.find(list, fn item ->
|
||||||
|
if item.id do
|
||||||
|
if item == Repo.get_by(module, id: item.id) do
|
||||||
|
# This item is OK - no race condition.
|
||||||
|
false
|
||||||
|
else
|
||||||
|
# There was a race condtion. We probably can't trust
|
||||||
|
# any of the data in this list. We need to wait and
|
||||||
|
# try again later.
|
||||||
|
Process.sleep(@timeout * 3)
|
||||||
|
true
|
||||||
|
end
|
||||||
|
else
|
||||||
|
# This item only exists on the FBOS side.
|
||||||
|
# It will never be affected by the data race condtion.
|
||||||
|
false
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue