Skip to content

Commit

Permalink
[#19] Fix update and get_and_update to avoid the :ttl be overridden
Browse files Browse the repository at this point in the history
  • Loading branch information
Carlos Bolanos committed Apr 18, 2018
1 parent 7320a58 commit ad3a721
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 83 deletions.
22 changes: 13 additions & 9 deletions lib/nebulex/adapters/local.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,11 @@ defmodule Nebulex.Adapters.Local do
do: nil
defp do_set(object, gen, cache, opts) do
version = cache.generate_vsn(object)
ttl = seconds_since_epoch(opts[:ttl])

ttl =
if ttl_opt = opts[:ttl],
do: seconds_since_epoch(ttl_opt),
else: object.ttl

%{object | version: version, ttl: ttl}
|> set_object(gen, cache)
Expand Down Expand Up @@ -279,7 +283,6 @@ defmodule Nebulex.Adapters.Local do
return =
%Object{key: key, value: val, version: vsn, ttl: ttl}
|> validate_return(opts)

fun.({key, return}, fold_acc)
end, acc, gen, cache.__state__)
end)
Expand Down Expand Up @@ -322,14 +325,15 @@ defmodule Nebulex.Adapters.Local do
@doc false
def get_and_update(cache, key, fun, opts) when is_function(fun, 1) do
generations = cache.__metadata__.generations
{gen, current} = get(generations, cache, key, Keyword.delete(opts, :return))
{gen, current} = get(generations, cache, key, Keyword.put(opts, :return, :object))
current = current || %Object{}

case fun.(current) do
case fun.(current.value) do
{get, update} ->
{get, do_set(%Object{key: key, value: update}, gen, cache, opts)}
{get, do_set(%{current | key: key, value: update}, gen, cache, opts)}
:pop ->
_ = do_delete(%Object{key: key}, generations, cache.__state__)
{current, nil}
{current.value, nil}
other ->
raise ArgumentError,
"the given function must return a two-element tuple or :pop, " <>
Expand All @@ -341,11 +345,11 @@ defmodule Nebulex.Adapters.Local do
def update(cache, key, initial, fun, opts) do
generations = cache.__metadata__.generations

case get(generations, cache, key, Keyword.delete(opts, :return)) do
case get(generations, cache, key, Keyword.put(opts, :return, :object)) do
{gen, nil} ->
do_set(%Object{key: key, value: initial}, gen, cache, opts)
{gen, val} ->
do_set(%Object{key: key, value: fun.(val)}, gen, cache, opts)
{gen, obj} ->
do_set(%{obj | key: key, value: fun.(obj.value)}, gen, cache, opts)
end
end

Expand Down
1 change: 0 additions & 1 deletion lib/nebulex/adapters/multilevel.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,6 @@ defmodule Nebulex.Adapters.Multilevel do
Enum.reduce(cache.__levels__, [], fn(level, acc) ->
[level.in_transaction?() | acc]
end)

true in results
end

Expand Down
4 changes: 3 additions & 1 deletion lib/nebulex/object.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ defmodule Nebulex.Object do
This is the struct used by the caches to store and retrieve data.
"""

defstruct [:key, :value, :version, ttl: :infinity]
defstruct [:key, :value, :version, :ttl]

@type t :: %__MODULE__{key: any, value: any, version: any, ttl: timeout}

Expand All @@ -18,6 +18,8 @@ defmodule Nebulex.Object do
Object.ttl(obj)
"""
@spec ttl(Nebulex.Object.t) :: timeout
def ttl(%Nebulex.Object{ttl: nil}),
do: :infinity
def ttl(%Nebulex.Object{ttl: :infinity}),
do: :infinity
def ttl(%Nebulex.Object{ttl: ttl}) when is_integer(ttl) do
Expand Down
15 changes: 8 additions & 7 deletions mix.lock
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
%{"benchfella": {:hex, :benchfella, "0.3.5", "b2122c234117b3f91ed7b43b6e915e19e1ab216971154acd0a80ce0e9b8c05f5", [], [], "hexpm"},
"bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [], [], "hexpm"},
"certifi": {:hex, :certifi, "2.0.0", "a0c0e475107135f76b8c1d5bc7efb33cd3815cb3cf3dea7aefdd174dabead064", [], [], "hexpm"},
"credo": {:hex, :credo, "0.8.10", "261862bb7363247762e1063713bb85df2bbd84af8d8610d1272cd9c1943bba63", [], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}], "hexpm"},
"certifi": {:hex, :certifi, "2.3.1", "d0f424232390bf47d82da8478022301c561cf6445b5b5fb6a84d49a9e76d2639", [], [{:parse_trans, "3.2.0", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"},
"credo": {:hex, :credo, "0.9.1", "f021affa11b32a94dc2e807a6472ce0914289c9132f99644a97fc84432b202a1", [], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:poison, ">= 0.0.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},
"dialyxir": {:hex, :dialyxir, "0.5.1", "b331b091720fd93e878137add264bac4f644e1ddae07a70bf7062c7862c4b952", [], [], "hexpm"},
"earmark": {:hex, :earmark, "1.2.4", "99b637c62a4d65a20a9fb674b8cffb8baa771c04605a80c911c4418c69b75439", [], [], "hexpm"},
"earmark": {:hex, :earmark, "1.2.5", "4d21980d5d2862a2e13ec3c49ad9ad783ffc7ca5769cf6ff891a4553fbaae761", [], [], "hexpm"},
"ex2ms": {:hex, :ex2ms, "1.5.0", "19e27f9212be9a96093fed8cdfbef0a2b56c21237196d26760f11dfcfae58e97", [], [], "hexpm"},
"ex_doc": {:hex, :ex_doc, "0.18.1", "37c69d2ef62f24928c1f4fdc7c724ea04aecfdf500c4329185f8e3649c915baf", [], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm"},
"ex_doc": {:hex, :ex_doc, "0.18.3", "f4b0e4a2ec6f333dccf761838a4b253d75e11f714b85ae271c9ae361367897b7", [], [{:earmark, "~> 1.1", [hex: :earmark, repo: "hexpm", optional: false]}], "hexpm"},
"ex_shards": {:hex, :ex_shards, "0.2.1", "65c14d5ae71d6c4b617531f29923f8e57007e5b9874c100a11163c55ec0c7b21", [], [{:ex2ms, "~> 1.4", [hex: :ex2ms, repo: "hexpm", optional: false]}, {:shards, "~> 0.5", [hex: :shards, repo: "hexpm", optional: false]}], "hexpm"},
"excoveralls": {:hex, :excoveralls, "0.8.0", "99d2691d3edf8612f128be3f9869c4d44b91c67cec92186ce49470ae7a7404cf", [], [{:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: false]}, {:hackney, ">= 0.12.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
"excoveralls": {:hex, :excoveralls, "0.8.1", "0bbf67f22c7dbf7503981d21a5eef5db8bbc3cb86e70d3798e8c802c74fa5e27", [], [{:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: false]}, {:hackney, ">= 0.12.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
"exjsx": {:hex, :exjsx, "4.0.0", "60548841e0212df401e38e63c0078ec57b33e7ea49b032c796ccad8cde794b5c", [], [{:jsx, "~> 2.8.0", [hex: :jsx, repo: "hexpm", optional: false]}], "hexpm"},
"hackney": {:hex, :hackney, "1.11.0", "4951ee019df102492dabba66a09e305f61919a8a183a7860236c0fde586134b6", [], [{:certifi, "2.0.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "5.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
"idna": {:hex, :idna, "5.1.0", "d72b4effeb324ad5da3cab1767cb16b17939004e789d8c0ad5b70f3cea20c89a", [], [{:unicode_util_compat, "0.3.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"},
"hackney": {:hex, :hackney, "1.12.1", "8bf2d0e11e722e533903fe126e14d6e7e94d9b7983ced595b75f532e04b7fdc7", [], [{:certifi, "2.3.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "5.1.1", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "1.0.2", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.1", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"},
"idna": {:hex, :idna, "5.1.1", "cbc3b2fa1645113267cc59c760bafa64b2ea0334635ef06dbac8801e42f7279c", [], [{:unicode_util_compat, "0.3.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"},
"inch_ex": {:hex, :inch_ex, "0.5.6", "418357418a553baa6d04eccd1b44171936817db61f4c0840112b420b8e378e67", [], [{:poison, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},
"jsx": {:hex, :jsx, "2.8.3", "a05252d381885240744d955fbe3cf810504eb2567164824e19303ea59eef62cf", [], [], "hexpm"},
"meck": {:hex, :meck, "0.8.9", "64c5c0bd8bcca3a180b44196265c8ed7594e16bcc845d0698ec6b4e577f48188", [], [], "hexpm"},
"metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [], [], "hexpm"},
"mimerl": {:hex, :mimerl, "1.0.2", "993f9b0e084083405ed8252b99460c4f0563e41729ab42d9074fd5e52439be88", [], [], "hexpm"},
"mock": {:hex, :mock, "0.3.1", "994f00150f79a0ea50dc9d86134cd9ebd0d177ad60bd04d1e46336cdfdb98ff9", [], [{:meck, "~> 0.8.8", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm"},
"parse_trans": {:hex, :parse_trans, "3.2.0", "2adfa4daf80c14dc36f522cf190eb5c4ee3e28008fc6394397c16f62a26258c2", [], [], "hexpm"},
"poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [], [], "hexpm"},
"shards": {:hex, :shards, "0.5.0", "2780002307ccf250368a067d59b364e7cf1af1c788a9f60b490d199a6028b6de", [], [], "hexpm"},
"ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.1", "28a4d65b7f59893bc2c7de786dec1e1555bd742d336043fe644ae956c3497fbe", [], [], "hexpm"},
Expand Down
29 changes: 6 additions & 23 deletions test/nebulex/adapters/dist_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ defmodule Nebulex.Adapters.DistTest do
end

test "check cluster nodes" do
assert node() == @primary
assert :lists.usort(Node.list()) == @cluster -- [node()]
assert Dist.nodes() == @cluster
assert @primary == node()
assert @cluster -- [node()] == :lists.usort(Node.list())
assert @cluster == Dist.nodes()
end

test "get_and_update" do
assert Dist.get_and_update(1, &Dist.get_and_update_fun/1) == {nil, 1}
assert Dist.get_and_update(1, &Dist.get_and_update_fun/1) == {1, 2}
assert Dist.get_and_update(1, &Dist.get_and_update_fun/1) == {2, 4}
assert {nil, 1} == Dist.get_and_update(1, &Dist.get_and_update_fun/1)
assert {1, 2} == Dist.get_and_update(1, &Dist.get_and_update_fun/1)
assert {2, 4} == Dist.get_and_update(1, &Dist.get_and_update_fun/1)

{4, %Object{key: 1, value: 8, ttl: _, version: _}} =
Dist.get_and_update(1, &Dist.get_and_update_fun/1, return: :object)
Expand All @@ -54,21 +54,4 @@ defmodule Nebulex.Adapters.DistTest do
|> Dist.get_and_update(&Dist.get_and_update_fun/1, version: -1)
end
end

test "update" do
for x <- 1..2, do: Dist.set x, x

assert Dist.update(1, 1, &Dist.update_fun/1) == 2
assert Dist.update(2, 1, &Dist.update_fun/1) == 4
assert Dist.update(3, 1, &Dist.update_fun/1) == 1

%Object{key: 11, value: 1, ttl: _, version: _} =
Dist.update(11, 1, &Dist.update_fun/1, return: :object)

assert_raise Nebulex.VersionConflictError, fn ->
:a
|> Dist.set(1, return: :key)
|> Dist.update(0, &Dist.update_fun/1, version: -1)
end
end
end
60 changes: 18 additions & 42 deletions test/nebulex/adapters/local_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ defmodule Nebulex.Adapters.LocalTest do
if v, do: {v, v * 2}, else: {v, 1}
end)

assert TestCache.get_and_update(1, &({&1, &1 * 2})) == {1, 2}
assert {1, 2} == TestCache.get_and_update(1, &({&1, &1 * 2}))
{2, %Object{key: 1, value: 6, ttl: _, version: _}} =
TestCache.get_and_update(1, &({&1, &1 * 3}), return: :object)
assert TestCache.get_and_update(1, &({&1, nil})) == {6, nil}
assert TestCache.get(1) == 6
assert TestCache.get_and_update(1, fn _ -> :pop end) == {6, nil}
assert TestCache.get_and_update(3, &({&1, 3})) == {nil, 3}
assert {6, nil} == TestCache.get_and_update(1, &({&1, nil}))
assert 6 == TestCache.get(1)
assert {6, nil} == TestCache.get_and_update(1, fn _ -> :pop end)
assert {nil, 3} == TestCache.get_and_update(3, &({&1, 3}))

assert {nil, :error} == TestCache.get_and_update(:a, fn v ->
if v, do: {v, :ok}, else: {v, :error}
Expand All @@ -60,41 +60,17 @@ defmodule Nebulex.Adapters.LocalTest do
end
end

test "update" do
TestCache.new_generation()

for x <- 1..2, do: TestCache.set x, x

assert TestCache.update(1, 1, &(&1 * 2)) == 2
assert TestCache.update(2, 1, &(&1 * 2)) == 4
assert TestCache.update(3, 1, &(&1 * 2)) == 1
refute TestCache.update(4, nil, &(&1 * 2))
refute TestCache.get(4)

%Object{key: 11, value: 1, ttl: _, version: _} =
TestCache.update(11, 1, &(&1 * 2), return: :object)

assert TestCache.update(3, 3, &(&1 * 2), version: -1, on_conflict: :nothing) == 2
assert TestCache.update(3, 3, &(&1 * 2), version: -1, on_conflict: nil) == 3

assert_raise Nebulex.VersionConflictError, fn ->
:a
|> TestCache.set(1, return: :key)
|> TestCache.update(0, &(&1 * 2), version: -1)
end
end

test "incr with update" do
TestCache.new_generation()

assert TestCache.update_counter(:counter) == 1
assert TestCache.update_counter(:counter) == 2
assert 1 == TestCache.update_counter(:counter)
assert 2 == TestCache.update_counter(:counter)

assert TestCache.get_and_update(:counter, &({&1, &1 * 2})) == {2, 4}
assert TestCache.update_counter(:counter) == 5
assert {2, 4} == TestCache.get_and_update(:counter, &({&1, &1 * 2}))
assert 5 == TestCache.update_counter(:counter)

assert TestCache.update(:counter, 1, &(&1 * 2)) == 10
assert TestCache.update_counter(:counter, -10) == 0
assert 10 == TestCache.update(:counter, 1, &(&1 * 2))
assert 0 == TestCache.update_counter(:counter, -10)

TestCache.set("foo", "bar")
assert_raise ArgumentError, fn ->
Expand All @@ -113,7 +89,7 @@ defmodule Nebulex.Adapters.LocalTest do
for x <- 1..2, do: TestCache.set x, x

# fetch one entry from new generation
assert TestCache.get(1) == 1
assert 1 == TestCache.get(1)

# fetch non-existent entries
refute TestCache.get(3)
Expand All @@ -125,23 +101,23 @@ defmodule Nebulex.Adapters.LocalTest do
# both entries should be in the old generation
refute get_from_new(1)
refute get_from_new(2)
assert (get_from_old(1)).value == 1
assert (get_from_old(2)).value == 2
assert 1 == (get_from_old(1)).value
assert 2 == (get_from_old(2)).value

# fetch entry 1 to set it into the new generation
assert TestCache.get(1) == 1
assert (get_from_new(1)).value == 1
assert 1 == TestCache.get(1)
assert 1 == (get_from_new(1)).value
refute get_from_new(2)
refute get_from_old(1)
assert (get_from_old(2)).value == 2
assert 2 == (get_from_old(2)).value

# create a new generation, the old generation should be deleted
TestCache.new_generation()

# entry 1 should be into the old generation and entry 2 deleted
refute get_from_new(1)
refute get_from_new(2)
assert (get_from_old(1)).value == 1
assert 1 == (get_from_old(1)).value
refute get_from_old(2)
end

Expand Down
54 changes: 54 additions & 0 deletions test/shared/cache_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,30 @@ defmodule Nebulex.CacheTest do
end
end

test "update" do
@cache.new_generation()

for x <- 1..2, do: @cache.set x, x

assert "1" == @cache.update(1, 1, &Integer.to_string/1)
assert "2" == @cache.update(2, 1, &Integer.to_string/1)
assert 1 == @cache.update(3, 1, &Integer.to_string/1)
refute @cache.update(4, nil, &Integer.to_string/1)
refute @cache.get(4)

%Object{key: 11, value: 1, ttl: _, version: _} =
@cache.update(11, 1, &Integer.to_string/1, return: :object)

assert "1" == @cache.update(3, 3, &Integer.to_string/1, version: -1, on_conflict: :nothing)
assert 3 == @cache.update(3, 3, &Integer.to_string/1, version: -1, on_conflict: nil)

assert_raise Nebulex.VersionConflictError, fn ->
:a
|> @cache.set(1, return: :key)
|> @cache.update(0, &Integer.to_string/1, version: -1)
end
end

test "update_counter" do
@cache.new_generation()

Expand Down Expand Up @@ -300,6 +324,36 @@ defmodule Nebulex.CacheTest do
1
|> @cache.update(nil, &:erlang.phash2/1, ttl: 3, return: :object)
|> Object.ttl()

assert :infinity == Object.ttl(%Object{})
end

test "get_and_update an existing object with ttl" do
@cache.new_generation()

assert ttl = @cache.set(1, 1, ttl: 2, return: :object).ttl
assert 1 == @cache.get(1)

_ = :timer.sleep(1000)
assert {1, 2} == @cache.get_and_update(1, &Nebulex.TestCache.Dist.get_and_update_fun/1)
assert ttl == @cache.get(1, return: :object).ttl

_ = :timer.sleep(1100)
refute @cache.get(1)
end

test "update an existing object with ttl" do
@cache.new_generation()

assert ttl = @cache.set(1, 1, ttl: 2, return: :object).ttl
assert 1 == @cache.get(1)

_ = :timer.sleep(1000)
assert "1" == @cache.update(1, 10, &Integer.to_string/1)
assert ttl == @cache.get(1, return: :object).ttl

_ = :timer.sleep(1100)
refute @cache.get(1)
end

test "transaction" do
Expand Down

0 comments on commit ad3a721

Please sign in to comment.