Merge pull request #7 from corka149/bugfix/#6-remove-sorting

Bugfix/#6 remove sorting

Author: corka149

lib/jsonpatch.ex

@@ -30,15 +30,16 @@ defmodule Jsonpatch do
 
   @doc """
   Apply a Jsonpatch or a list of Jsonpatches to a map or struct. The whole patch will not be applied
-  when any path is invalid or any other error occured.
+  when any path is invalid or any other error occured. When a list is provided, the operations are
+  applied in the order as they appear in the list.
 
   ## Examples
       iex> patch = [
       ...> %Jsonpatch.Operation.Add{path: "/age", value: 33},
       ...> %Jsonpatch.Operation.Replace{path: "/hobbies/0", value: "Elixir!"},
       ...> %Jsonpatch.Operation.Replace{path: "/married", value: true},
-      ...> %Jsonpatch.Operation.Remove{path: "/hobbies/1"},
       ...> %Jsonpatch.Operation.Remove{path: "/hobbies/2"},
+      ...> %Jsonpatch.Operation.Remove{path: "/hobbies/1"},
       ...> %Jsonpatch.Operation.Copy{from: "/name", path: "/surname"},
       ...> %Jsonpatch.Operation.Move{from: "/home", path: "/work"},
       ...> %Jsonpatch.Operation.Test{path: "/name", value: "Bob"}
@@ -61,14 +62,10 @@ defmodule Jsonpatch do
   def apply_patch(json_patch, target)
 
   def apply_patch(json_patch, %{} = target) when is_list(json_patch) do
-    # Operatons MUST be sorted before applying because a remove operation for path "/foo/2" must be done
-    # before the remove operation for path "/foo/1". Without order it could be possible that the wrong
-    # value will be removed or only one value instead of two.
+    # https://datatracker.ietf.org/doc/html/rfc6902#section-3
+    # > Operations are applied sequentially in the order they appear in the array.
     result =
       json_patch
-      |> Enum.map(&create_sort_value/1)
-      |> Enum.sort(fn {sort_value_1, _}, {sort_value_2, _} -> sort_value_1 >= sort_value_2 end)
-      |> Enum.map(fn {_, patch} -> patch end)
       |> Enum.reduce(target, &Jsonpatch.Operation.apply_op/2)
 
     case result do
@@ -88,7 +85,8 @@ defmodule Jsonpatch do
 
   @doc """
   Apply a Jsonpatch or a list of Jsonpatches to a map or struct. In case of an error
-  it will raise an exception.
+  it will raise an exception. When a list is provided, the operations are applied in
+  the order as they appear in the list.
 
   (See Jsonpatch.apply_patch/2 for more details)
   """
@@ -219,25 +217,4 @@ defmodule Jsonpatch do
   defp escape(subpath) do
     subpath
   end
-
-  # Create once a easy sortable value for a operation
-  defp create_sort_value(%{path: path} = operation) do
-    fragments = String.split(path, "/")
-
-    x = Jsonpatch.PathUtil.operation_sort_value?(operation) * 1_000_000 * 100_000_000
-    y = length(fragments) * 100_000_000
-
-    z =
-      case List.last(fragments) |> Integer.parse() do
-        :error -> 0
-        {int, _} -> int
-      end
-
-    # Structure of recorde sort value
-    # x = Kind of PathUtil
-    # y = Amount of fragments (how deep goes the path?)
-    # z = At which position in a list?
-    # xxxxyyyyyyzzzzzzzz
-    {x + y + z, operation}
-  end
 end

lib/jsonpatch/path_util.ex

@@ -4,11 +4,6 @@ defmodule Jsonpatch.PathUtil do
   # ===== Internal documentation =====
   # Helper module for handling JSON paths.
 
-  alias Jsonpatch.Operation.Add
-  alias Jsonpatch.Operation.Remove
-  alias Jsonpatch.Operation.Replace
-  alias Jsonpatch.Operation.Test
-
   @doc """
   Uses a JSON patch path to get the last map that this path references.
 
@@ -61,20 +56,6 @@ defmodule Jsonpatch.PathUtil do
     do_update_final_destination(target, new_destination, fragments)
   end
 
-  @doc """
-  Determines the sort value for the operation of a patch. This value
-  assure in which order patches are applied. (Example: shall remove
-  patches be applied before add patches?)
-  """
-  @spec operation_sort_value?(Jsonpatch.t()) :: integer()
-  def operation_sort_value?(patch)
-
-  def operation_sort_value?(%Test{}), do: 600
-  def operation_sort_value?(%Add{}), do: 500
-  def operation_sort_value?(%Replace{}), do: 400
-  def operation_sort_value?(%Remove{}), do: 300
-  def operation_sort_value?(_), do: 0
-
   @doc """
   Unescape `~1` to  `/` and `~0` to `~`.
   """

mix.exs

@@ -6,7 +6,7 @@ defmodule Jsonpatch.MixProject do
       app: :jsonpatch,
       name: "Jsonpatch",
       description: "Implementation of RFC 6902 in pure Elixir",
-      version: "0.11.2",
+      version: "0.12.0",
       elixir: "~> 1.10",
       start_permanent: Mix.env() == :prod,
       deps: deps(),

test/jsonpatch/operation/add_test.exs

@@ -73,4 +73,11 @@ defmodule Jsonpatch.Operation.AddTest do
 
     assert %{"a" => [0, 1, 2, 3]} = Operation.apply_op(patch, target)
   end
+
+  test "Return error when patch error was provided to add operation" do
+    patch = %Add{path: "/a", value: false}
+    error = {:error, :invalid_index, "4"}
+
+    assert ^error = Operation.apply_op(patch, error)
+  end
 end

test/jsonpatch/operation/remove_test.exs

@@ -83,4 +83,11 @@ defmodule RemoveTest do
     remove_patch = %Remove{path: "/hobbies/1/description"}
     assert {:error, :invalid_index, "1"} = Jsonpatch.apply_patch(remove_patch, target)
   end
+
+  test "Return error when patch error was provided to remove operation" do
+    patch = %Remove{path: "/a"}
+    error = {:error, :invalid_index, "4"}
+
+    assert ^error = Operation.apply_op(patch, error)
+  end
 end

test/jsonpatch/operation/replace_test.exs

@@ -81,4 +81,11 @@ defmodule Jsonpatch.Operation.ReplaceTest do
 
     assert {:error, :invalid_index, "c"} = patched_target
   end
+
+  test "Return error when patch error was provided to replace operation" do
+    patch = %Replace{path: "/a", value: true}
+    error = {:error, :invalid_index, "4"}
+
+    assert ^error = Operation.apply_op(patch, error)
+  end
 end

test/jsonpatch_test.exs

@@ -179,6 +179,19 @@ defmodule JsonpatchTest do
                )
     end
 
+    test "Apply patch with multilple operations" do
+      patch = [
+        %Jsonpatch.Operation.Remove{path: "/age"},
+        %Jsonpatch.Operation.Add{path: "/age", value: 34},
+        %Jsonpatch.Operation.Replace{path: "/age", value: 35}
+      ]
+
+      target = %{"age" => "33"}
+      patched = Jsonpatch.apply_patch!(patch, target)
+
+      assert %{"age" => 35} = patched
+    end
+
     test "Apply patch with invalid target source path and expect error" do
       target = %{
         "name" => "Bob",