Cleaned code

Author: corka149

CHANGELOG

@@ -2,5 +2,8 @@
 
 - Made jsonpatch more Elixir-API-like by adding Jsonpatch.apply_patch! (which raise an exception) and changed Jsonpatch.apply_patch to return a tuple.
 - Implemented escaping for '~' and '/'
-- Allow usage of '-' for Add operation
-- Fixed adding values to array
+- Allow usage of '-' for Add and Copy operation
+- Fixed adding and copying values to array
+- Improved error feedback of test operation
+- Fixed: Replace operation adds error to target
+- Cleaned code: Replaced strange constructs of Enum.with_index with Enum.fetch

README.md

@@ -20,7 +20,7 @@ Features:
     - test
 3. De/Encoding and mapping
 4. Escaping of "`/`" (by "`~1`") and "`~`" (by "`~0`")
-5. Allow usage of `-` for appending things to list
+5. Allow usage of `-` for appending things to list (Add and Copy operation)
 
 
 ## Getting started

lib/jsonpatch/operation/add.ex

@@ -36,8 +36,7 @@ defimpl Jsonpatch.Operation, for: Jsonpatch.Operation.Add do
   defp do_add({:error, _, _} = error, _target, _path, _value), do: error
 
   # Map
-  defp do_add({final_destination, last_fragment}, target, path, value)
-       when is_map(final_destination) do
+  defp do_add({%{} = final_destination, last_fragment}, target, path, value) do
     updated_final_destination = Map.put_new(final_destination, last_fragment, value)
     Jsonpatch.PathUtil.update_final_destination(target, updated_final_destination, path)
   end

lib/jsonpatch/operation/copy.ex

@@ -70,14 +70,9 @@ defimpl Jsonpatch.Operation, for: Jsonpatch.Operation.Copy do
         {:error, :invalid_index, fragment}
 
       {index, _} ->
-        result =
-          final_destination
-          |> Enum.with_index()
-          |> Enum.find(fn {_, other_index} -> index == other_index end)
-
-        case result do
-          nil -> {:error, :invalid_index, fragment}
-          {val, _} -> val
+        case Enum.fetch(final_destination, index) do
+          :error -> {:error, :invalid_index, fragment}
+          {:ok, val} -> val
         end
     end
   end
@@ -88,12 +83,20 @@ defimpl Jsonpatch.Operation, for: Jsonpatch.Operation.Copy do
 
   defp do_add({copy_target, _last_fragment}, copied_value, copy_path_end)
        when is_list(copy_target) do
-    case Integer.parse(copy_path_end) do
-      :error ->
-        {:error, :invalid_index, copy_path_end}
-
-      {index, _} ->
-        List.insert_at(copy_target, index, copied_value)
+    if copy_path_end == "-" do
+      List.insert_at(copy_target, length(copy_target), copied_value)
+    else
+      case Integer.parse(copy_path_end) do
+        :error ->
+          {:error, :invalid_index, copy_path_end}
+
+        {index, _} ->
+          if index < length(copy_target) do
+            List.update_at(copy_target, index, fn _old -> copied_value end)
+          else
+            {:error, :invalid_index, copy_path_end}
+          end
+      end
     end
   end
 

lib/jsonpatch/path_util.ex

@@ -108,9 +108,9 @@ defmodule Jsonpatch.PathUtil do
   defp find_final_destination(target, [fragment | tail]) when is_list(target) do
     {index, _} = Integer.parse(fragment)
 
-    case Enum.with_index(target) |> Enum.find(fn {_val, i} -> i == index end) do
-      nil -> {:error, :invalid_index, fragment}
-      {val, _} -> find_final_destination(val, tail)
+    case Enum.fetch(target, index) do
+      :error -> {:error, :invalid_index, fragment}
+      {:ok, val} -> find_final_destination(val, tail)
     end
   end
 

test/jsonpatch/operation/copy_test.exs

@@ -7,8 +7,9 @@ defmodule Jsonpatch.Operation.CopyTest do
   doctest Copy
 
   test "Copy element by path with multiple indices" do
-    path = "/a/b/1/c/3"
     from = "/a/b/1/c/2"
+    # Copy to  end
+    path = "/a/b/1/c/-"
 
     target = %{
       "a" => %{
@@ -114,4 +115,24 @@ defmodule Jsonpatch.Operation.CopyTest do
 
     assert {:error, :invalid_index, "b"} = patched_target
   end
+
+  test "Copy list element" do
+    patch = %Copy{from: "/a/0", path: "/a/1"}
+
+    target = %{"a" => [999, 888]}
+
+    patched = Operation.apply_op(patch, target)
+
+    assert %{"a" => [999, 999]} = patched
+  end
+
+  test "Copy list element with invalid index" do
+    patch = %Copy{from: "/a/0", path: "/a/5"}
+
+    target = %{"a" => [999, 888]}
+
+    patched_error = Operation.apply_op(patch, target)
+
+    assert {:error, :invalid_index, "5"} = patched_error
+  end
 end

test/jsonpatch/operation/move_test.exs

@@ -1,4 +1,6 @@
 defmodule Jsonpatch.Operation.MoveTest do
   use ExUnit.Case
   doctest Jsonpatch.Operation.Move
+
+  # Move is a combination of the copy and remove operation.
 end

test/jsonpatch/operation/remove_test.exs

@@ -1,4 +1,4 @@
-defmodule Jsonpatch.Operation.RemoveTest do
+defmodule RemoveTest do
   use ExUnit.Case
 
   alias Jsonpatch.Operation
@@ -53,7 +53,7 @@ defmodule Jsonpatch.Operation.RemoveTest do
       "home" => "Berlin"
     }
 
-    remove_patch = %Jsonpatch.Operation.Remove{path: "/nameX"}
+    remove_patch = %Remove{path: "/nameX"}
     assert {:error, :invalid_path, "nameX"} = Jsonpatch.apply_patch(remove_patch, target)
   end
 
@@ -65,7 +65,7 @@ defmodule Jsonpatch.Operation.RemoveTest do
       "home" => "Berlin"
     }
 
-    remove_patch = %Jsonpatch.Operation.Remove{path: "/hobbies/a"}
+    remove_patch = %Remove{path: "/hobbies/a"}
     assert {:error, :invalid_index, "a"} = Jsonpatch.apply_patch(remove_patch, target)
 
     # Longer path
@@ -76,18 +76,11 @@ defmodule Jsonpatch.Operation.RemoveTest do
       "home" => "Berlin"
     }
 
-    remove_patch = %Jsonpatch.Operation.Remove{path: "/hobbies/b/description"}
+    remove_patch = %Remove{path: "/hobbies/b/description"}
     assert {:error, :invalid_index, "b"} = Jsonpatch.apply_patch(remove_patch, target)
 
-    # Longer path, numeric
-    target = %{
-      "name" => "Bob",
-      "married" => false,
-      "hobbies" => [%{"description" => "Foo"}],
-      "home" => "Berlin"
-    }
-
-    remove_patch = %Jsonpatch.Operation.Remove{path: "/hobbies/1/description"}
+    # Longer path, numeric - out of
+    remove_patch = %Remove{path: "/hobbies/1/description"}
     assert {:error, :invalid_index, "1"} = Jsonpatch.apply_patch(remove_patch, target)
   end
 end

test/jsonpatch/operation/test_test.exs

@@ -51,4 +51,18 @@ defmodule Jsonpatch.Operation.TestTest do
 
     assert {:error, :test_failed, "Expected value '42' at '/a/b/1/c/1'"} = patched_target
   end
+
+  test "Test list with index out of range" do
+    test = %Test{path: "/m/2", value: "foo"}
+    target = %{"m" => [0, 1]}
+
+    assert {:error, :invalid_index, "2"} = Operation.apply_op(test, target)
+  end
+
+  test "Test list with invalid index" do
+    test = %Test{path: "/m/b", value: "foo"}
+    target = %{"m" => [0, 1]}
+
+    assert {:error, :invalid_index, "b"} = Operation.apply_op(test, target)
+  end
 end