fix: Improve JSON functions and tests

ocean · ocean · commit 721b53a1efe7 · 2026-01-05T18:27:13.000+11:00
diff --git a/AGENTS.md b/AGENTS.md
@@ -1359,9 +1359,14 @@ fragment = JSON.arrow_fragment("settings", "theme")
 fragment = JSON.arrow_fragment("settings", "theme", :double_arrow)
 # Returns: "settings ->> 'theme'"
 
-# Use in Ecto queries
+# Use in Ecto queries - Option 1: Using the helper function
+arrow_sql = JSON.arrow_fragment("data", "active", :double_arrow)
 from u in User,
-  where: fragment(JSON.arrow_fragment("data", "active", :double_arrow), "=", true)
+  where: fragment(arrow_sql <> " = ?", true)
+
+# Option 2: Direct inline SQL (simpler approach)
+from u in User,
+  where: fragment("data ->> 'active' = ?", true)
 ```
 
 #### Ecto Integration
@@ -1388,9 +1393,14 @@ from u in User,
   where: fragment("json_extract(?, ?) = ?", u.settings, "$.theme", "dark"),
   select: u.name
 
-# Or using the helpers
+# Or using the helpers - Option 1: Arrow fragment helper
+arrow_sql = JSON.arrow_fragment("settings", "theme", :double_arrow)
 from u in User,
-  where: fragment(JSON.arrow_fragment("settings", "theme", :double_arrow), "=", "dark")
+  where: fragment(arrow_sql <> " = ?", "dark")
+
+# Option 2: Direct inline SQL (simpler for static fields)
+from u in User,
+  where: fragment("settings ->> 'theme' = ?", "dark")
 
 # Update JSON fields
 from u in User,
@@ -1435,10 +1445,15 @@ Create, update, and manipulate JSON structures:
 {:ok, json} = JSON.remove(state, ~s({"a":1,"b":2,"c":3}), ["$.a", "$.c"])
 # Returns: {:ok, "{\"b\":2}"}
 
-# Apply a JSON patch
+# Apply a JSON Merge Patch (RFC 7396)
+# Keys in patch are object keys, not JSON paths
 {:ok, json} = JSON.patch(state, ~s({"a":1,"b":2}), ~s({"a":10,"c":3}))
 # Returns: {:ok, "{\"a\":10,\"b\":2,\"c\":3}"}
 
+# Remove a key by patching with null
+{:ok, json} = JSON.patch(state, ~s({"a":1,"b":2,"c":3}), ~s({"b":null}))
+# Returns: {:ok, "{\"a\":1,\"c\":3}"}
+
 # Get all keys from a JSON object (SQLite 3.9.0+)
 {:ok, keys} = JSON.keys(state, ~s({"name":"Alice","age":30}))
 # Returns: {:ok, "[\"age\",\"name\"]"}  (sorted)
@@ -1535,35 +1550,47 @@ settings = ~s({"theme":"dark","notifications":true,"language":"es"})
 # Returns: {:ok, %{valid: true, type: "object", depth: 2}}
 ```
 
-#### Comparison: Set vs Replace vs Insert
+#### Comparison: Set vs Replace vs Insert vs Patch
 
-The three modification functions have different behaviors:
+The modification functions have different behaviors:
 
 ```elixir
 json = ~s({"a":1,"b":2})
 
-# SET: Creates or replaces any path
+# SET: Creates or replaces any path (uses JSON paths like "$.key")
 {:ok, result} = JSON.set(state, json, "$.c", 3)
 # Result: {"a":1,"b":2,"c":3}
 
 {:ok, result} = JSON.set(state, json, "$.a", 100)
 # Result: {"a":100,"b":2}
 
-# REPLACE: Only updates existing paths, ignores new paths
+# REPLACE: Only updates existing paths, ignores new paths (uses JSON paths)
 {:ok, result} = JSON.replace(state, json, "$.c", 3)
 # Result: {"a":1,"b":2}  (c not added)
 
 {:ok, result} = JSON.replace(state, json, "$.a", 100)
 # Result: {"a":100,"b":2}  (existing path updated)
 
-# INSERT: Adds new values without replacing existing ones
+# INSERT: Adds new values without replacing existing ones (uses JSON paths)
 {:ok, result} = JSON.insert(state, json, "$.c", 3)
 # Result: {"a":1,"b":2,"c":3}
 
 {:ok, result} = JSON.insert(state, json, "$.a", 100)
 # Result: {"a":1,"b":2}  (existing path unchanged)
+
+# PATCH: Applies JSON Merge Patch (RFC 7396) - keys are object keys, not paths
+{:ok, result} = JSON.patch(state, json, ~s({"a":10,"c":3}))
+# Result: {"a":10,"b":2,"c":3}
+
+# Use null to remove keys
+{:ok, result} = JSON.patch(state, json, ~s({"b":null}))
+# Result: {"a":1}
 ```
 
+**When to use each function:**
+- **SET/REPLACE/INSERT**: For path-based updates using JSON paths (e.g., "$.user.name")
+- **PATCH**: For bulk top-level key updates (implements RFC 7396 JSON Merge Patch)
+
 #### Performance Notes
 
 - JSONB format reduces storage by 5-10% vs text JSON
diff --git a/lib/ecto_libsql/json.ex b/lib/ecto_libsql/json.ex
@@ -89,8 +89,13 @@ defmodule EctoLibSql.JSON do
 
   ## Returns
 
-    - `{:ok, value}` - Extracted value, or nil if path doesn't exist
-    - `{:error, reason}` on failure
+  The return type depends on the extracted JSON value:
+    - `{:ok, string}` - For JSON text values (e.g., "dark")
+    - `{:ok, integer}` - For JSON integer values (e.g., 30)
+    - `{:ok, float}` - For JSON real/float values (e.g., 99.99)
+    - `{:ok, nil}` - For JSON null values or non-existent paths
+    - `{:ok, json_text}` - For JSON objects/arrays, returned as JSON text string
+    - `{:error, reason}` - On query failure
 
   ## Examples
 
@@ -100,14 +105,22 @@ defmodule EctoLibSql.JSON do
       {:ok, age} = EctoLibSql.JSON.extract(state, ~s({"user":{"age":30}}), "$.user.age")
       # Returns: {:ok, 30}
 
+      {:ok, items} = EctoLibSql.JSON.extract(state, ~s({"items":[1,2,3]}), "$.items")
+      # Returns: {:ok, "[1,2,3]"}  (JSON array as text)
+
+      {:ok, nil} = EctoLibSql.JSON.extract(state, ~s({"a":1}), "$.missing")
+      # Returns: {:ok, nil}  (path doesn't exist)
+
   ## Notes
 
-  - Returns JSON types as-is (objects and arrays returned as JSON text)
-  - Use json_extract to preserve JSON structure, or ->> operator to convert to SQL types
-  - Works with both text JSON and JSONB binary format
+  - JSON objects and arrays are returned as JSON text strings
+  - Use `-> operator in SQL queries to preserve JSON structure, or `->> operator to convert to SQL types
+  - Works with both text JSON and JSONB binary format (format conversion is automatic)
+  - For nested JSON structures, you can chain extractions or use JSON paths like "$.user.address.city"
 
   """
-  @spec extract(State.t(), String.t() | binary, String.t()) :: {:ok, term()} | {:error, term()}
+  @spec extract(State.t(), String.t() | binary, String.t()) ::
+          {:ok, String.t() | integer() | float() | nil} | {:error, term()}
   def extract(%State{} = state, json, path) when is_binary(json) and is_binary(path) do
     Native.query_args(
       state.conn_id,
@@ -509,8 +522,19 @@ defmodule EctoLibSql.JSON do
   end
 
   # Private helpers: Result handling patterns
-  # All Native.query_args/5 calls return a response map that needs handling.
+  # All Native.query_args/5 calls return a response map from Rust that needs handling.
   # These helpers reduce duplication and provide consistent error handling.
+  #
+  # Expected response shape from Native.query_args/5:
+  #   %{
+  #     "columns" => [list of column names],
+  #     "rows" => [list of result rows, where each row is a list of values],
+  #     "num_rows" => total number of rows
+  #   }
+  #
+  # Error responses are:
+  #   %{"error" => reason_string}  (from Rust)
+  #   {:error, reason}             (from Elixir error handling)
 
   @doc false
   defp handle_single_result(response) do
@@ -702,7 +726,8 @@ defmodule EctoLibSql.JSON do
       # Returns: {:ok, "{\"a\":1,\"c\":3}"}
 
       {:ok, json} = EctoLibSql.JSON.remove(state, ~s([1,2,3,4,5]), ["$[0]", "$[2]"])
-      # Returns: {:ok, "[2,4,5]"}
+      # Returns: {:ok, "[2,3,5]"}
+      # Note: Paths are removed in order; after removing $[0], the original $[2] is now at $[1]
 
   """
   @spec remove(State.t(), String.t() | binary, String.t() | [String.t()]) ::
@@ -845,26 +870,52 @@ defmodule EctoLibSql.JSON do
   end
 
   @doc """
-  Apply a JSON patch to modify JSON.
+  Apply a JSON Merge Patch to modify JSON (RFC 7396).
 
-  The patch is itself a JSON object where keys are paths and values are the updates to apply.
-  Effectively performs multiple set/replace operations in one call.
+  Implements RFC 7396 JSON Merge Patch semantics. The patch is a JSON object where:
+  - **Top-level keys** are object keys in the target, not JSON paths
+  - **Values** replace the corresponding object values in the target
+  - **Nested objects** are merged recursively
+  - **null values** remove the key from the target object
+
+  To update nested structures, the patch object must reflect the nesting level.
 
   ## Parameters
 
     - state: Connection state
-    - json: JSON text or JSONB binary data
-    - patch: JSON patch object (keys are paths, values are replacements)
+    - json: JSON text or JSONB binary data (must be an object)
+    - patch: JSON object with merge patch semantics (keys are object keys, not paths)
 
   ## Returns
 
-    - `{:ok, modified_json}` - JSON after applying patch
+    - `{:ok, modified_json}` - JSON after applying merge patch
     - `{:error, reason}` on failure
 
   ## Examples
 
-      {:ok, json} = EctoLibSql.JSON.patch(state, ~s({"a":1,"b":2}), ~s({"$.a":10,"$.c":3}))
-      # Returns: {:ok, "{\"a\":10,\"b\":2,\"c\":3}"}
+      # Top-level key replacement
+      {:ok, json} = EctoLibSql.JSON.patch(state, ~s({"a":1,"b":2}), ~s({"a":10}))
+      # Returns: {:ok, "{\"a\":10,\"b\":2}"}
+
+      # Add new top-level key
+      {:ok, json} = EctoLibSql.JSON.patch(state, ~s({"a":1,"b":2}), ~s({"c":3}))
+      # Returns: {:ok, "{\"a\":1,\"b\":2,\"c\":3}"}
+
+      # Remove key with null
+      {:ok, json} = EctoLibSql.JSON.patch(state, ~s({"a":1,"b":2,"c":3}), ~s({"b":null}))
+      # Returns: {:ok, "{\"a\":1,\"c\":3}"}
+
+      # Nested object merge (replaces entire nested object)
+      {:ok, json} = EctoLibSql.JSON.patch(state, ~s({"user":{"name":"Alice","age":30}}), ~s({"user":{"age":31}}))
+      # Returns: {:ok, "{\"user\":{\"age\":31}}"}  (replaces entire user object, not a deep merge)
+
+  ## Notes
+
+  - This implements RFC 7396 JSON Merge Patch, NOT RFC 6902 JSON Patch
+  - Object keys in the patch are literal keys, not JSON paths (use "a" not "$.a")
+  - For nested structures, the patch replaces the entire value at that key (not a deep recursive merge)
+  - To perform deep merges or path-based updates, use `json_set/4` or `json_replace/4` instead
+  - Works with both text JSON and JSONB binary format
 
   """
   @spec patch(State.t(), String.t() | binary, String.t() | binary) ::
diff --git a/test/json_helpers_test.exs b/test/json_helpers_test.exs
@@ -66,7 +66,9 @@ defmodule EctoLibSql.JSONHelpersTest do
       {:ok, result} = JSON.extract(state, json, "$.items")
       # Arrays are returned as JSON text
       assert is_binary(result)
-      assert String.contains?(result, ["1", "2", "3"])
+      # Parse the JSON array to verify exact content
+      {:ok, decoded} = Jason.decode(result)
+      assert decoded == [1, 2, 3]
     end
   end
 
@@ -180,9 +182,11 @@ defmodule EctoLibSql.JSONHelpersTest do
 
     test "arrays with strings containing special chars", %{state: state} do
       {:ok, json} = JSON.array(state, ["hello \"world\"", "tab\there"])
-      # Should be escaped properly - look for the escaped version
-      assert String.contains?(json, "hello")
-      assert String.contains?(json, "world")
+      # Parse and validate the structure with proper escape sequences
+      {:ok, decoded} = Jason.decode(json)
+      assert length(decoded) == 2
+      assert Enum.at(decoded, 0) == "hello \"world\""
+      assert Enum.at(decoded, 1) == "tab\there"
     end
   end
 
@@ -430,10 +434,10 @@ defmodule EctoLibSql.JSONHelpersTest do
       values = Enum.to_list(1..100)
       {:ok, json} = JSON.array(state, values)
       assert is_binary(json)
-      # Should contain all values
-      Enum.each(values, fn v ->
-        assert String.contains?(json, Integer.to_string(v))
-      end)
+      # Parse and verify exact array content
+      {:ok, decoded} = Jason.decode(json)
+      assert decoded == values
+      assert length(decoded) == 100
     end
 
     test "handles JSON with reserved characters", %{state: state} do
@@ -552,8 +556,9 @@ defmodule EctoLibSql.JSONHelpersTest do
 
     test "removes single index from array", %{state: state} do
       {:ok, result} = JSON.remove(state, ~s([1,2,3,4,5]), "$[2]")
-      # Should remove the 3 (index 2)
-      assert String.contains?(result, "[1,2,4,5]")
+      # Should remove the 3 (index 2), resulting in [1,2,4,5]
+      {:ok, decoded} = Jason.decode(result)
+      assert decoded == [1, 2, 4, 5]
     end
 
     test "removes multiple paths from object", %{state: state} do
