Cache raw patches

Author: ericmj

lib/diff/hex/hex.ex

@@ -101,9 +101,13 @@ defmodule Diff.Hex do
 
           with {_, true} <- {:file_size_old, file_size_check?(path_old)},
                {_, true} <- {:file_size_new, file_size_check?(path_new)},
-               {_, {:ok, output}} <- {:git_diff, git_diff(path_old, path_new)},
-               {_, {:ok, patches}} <- {:parse_patch, parse_patch(output, path_from, path_to)} do
-            Enum.map(patches, &{:ok, &1})
+               {_, {:ok, output}} <- {:git_diff, git_diff(path_old, path_new)} do
+            # Store raw git diff output with base paths for relative conversion
+            if output do
+              [{:ok, {output, path_from, path_to}}]
+            else
+              []
+            end
           else
             {:file_size_old, false} ->
               [{:too_large, Path.relative_to(path_old, path_from)}]
@@ -150,14 +154,6 @@ defmodule Diff.Hex do
     end
   end
 
-  defp parse_patch(_output = nil, _path_from, _path_to) do
-    {:ok, []}
-  end
-
-  defp parse_patch(output, path_from, path_to) do
-    GitDiff.parse_patch(output, relative_from: path_from, relative_to: path_to)
-  end
-
   defp file_size_check?(path) do
     File.stat!(path).size <= @max_file_size
   end

lib/diff_web/live/diff_live_view.ex

@@ -279,30 +279,41 @@ defmodule DiffWeb.DiffLiveView do
       Enum.reduce(stream, {metadata, [], patch_index}, fn element,
                                                           {acc_metadata, acc_patch_ids, index} ->
         case element do
-          {:ok, patch} ->
-            if patch.chunks != [] do
-              # Store raw patch data directly - convert struct to map and sanitize for JSON encoding
-              sanitized_patch = sanitize_patch_content(patch)
-              patch_data = Jason.encode!(struct_to_map(sanitized_patch))
-              patch_id = "patch-#{index}"
-
-              Diff.Storage.put_patch(package, from, to, patch_id, patch_data)
-
-              additions = count_lines(patch, "+")
-              deletions = count_lines(patch, "-")
-
-              updated_metadata = %{
-                acc_metadata
-                | total_patches: acc_metadata.total_patches + 1,
-                  total_additions: acc_metadata.total_additions + additions,
-                  total_deletions: acc_metadata.total_deletions + deletions,
-                  files_changed: acc_metadata.files_changed + 1
-              }
-
-              {updated_metadata, acc_patch_ids ++ [patch_id], index + 1}
-            else
-              {acc_metadata, acc_patch_ids, index}
-            end
+          {:ok, {raw_diff, path_from, path_to}} ->
+            # Store raw git diff output with base paths for relative conversion
+            patch_id = "patch-#{index}"
+
+            patch_data =
+              Jason.encode!(%{
+                "diff" => sanitize_utf8(raw_diff),
+                "path_from" => path_from,
+                "path_to" => path_to
+              })
+
+            Diff.Storage.put_patch(package, from, to, patch_id, patch_data)
+
+            # Count additions and deletions from raw diff (exclude +++ and --- headers)
+            lines = String.split(raw_diff, "\n")
+
+            additions =
+              Enum.count(lines, fn line ->
+                String.starts_with?(line, "+") and not String.starts_with?(line, "+++")
+              end)
+
+            deletions =
+              Enum.count(lines, fn line ->
+                String.starts_with?(line, "-") and not String.starts_with?(line, "---")
+              end)
+
+            updated_metadata = %{
+              acc_metadata
+              | total_patches: acc_metadata.total_patches + 1,
+                total_additions: acc_metadata.total_additions + additions,
+                total_deletions: acc_metadata.total_deletions + deletions,
+                files_changed: acc_metadata.files_changed + 1
+            }
+
+            {updated_metadata, acc_patch_ids ++ [patch_id], index + 1}
 
           {:too_large, file_path} ->
             # Store raw too_large data directly
@@ -341,17 +352,6 @@ defmodule DiffWeb.DiffLiveView do
       {:error, :invalid_diff}
   end
 
-  defp count_lines(patch, line_type) do
-    patch.chunks
-    |> Enum.flat_map(& &1.lines)
-    |> Enum.count(fn line ->
-      case line_type do
-        "+" -> line.type == :add
-        "-" -> line.type == :remove
-      end
-    end)
-  end
-
   defp parse_versions(input) do
     with {:ok, [from, to]} <- versions_from_input(input),
          {:ok, from} <- parse_version(from),
@@ -432,126 +432,56 @@ defmodule DiffWeb.DiffLiveView do
 
   defp load_patch_content(package, from, to, patch_id) do
     case Diff.Storage.get_patch(package, from, to, patch_id) do
-      {:ok, patch_json} ->
-        case Jason.decode(patch_json) do
+      {:ok, raw_content} ->
+        # Parse the stored content based on format
+        case Jason.decode(raw_content) do
           {:ok, %{"type" => "too_large", "file" => file_path}} ->
-            # Handle too_large patches
+            # Handle legacy too_large patches
             Phoenix.View.render_to_string(DiffWeb.RenderView, "too_large.html", file: file_path)
             |> sanitize_utf8()
 
-          {:ok, patch_data} when is_map(patch_data) ->
-            # Handle normal patches - raw patch data stored directly as map
-            # Check if this looks like a patch (has expected keys)
-            if Map.has_key?(patch_data, "chunks") or Map.has_key?(patch_data, :chunks) do
-              Phoenix.View.render_to_string(DiffWeb.RenderView, "patch.html",
-                patch: atomize_patch(patch_data)
-              )
-              |> sanitize_utf8()
-            else
-              "<div class='patch-error'>Invalid patch format</div>"
-            end
-
-          {:error, _} ->
-            "<div class='patch-error'>Invalid patch data</div>"
-        end
-
-      {:error, _reason} ->
-        "<div class='patch-error'>Failed to load patch</div>"
-    end
-  end
-
-  # Convert string keys back to atom keys for patch rendering
-  # Also handle converting lists back to tuples where needed
-  defp atomize_patch(patch) when is_map(patch) do
-    patch
-    |> Enum.map(fn {k, v} ->
-      atom_key = if is_binary(k), do: String.to_existing_atom(k), else: k
-
-      converted_value =
-        case {atom_key, v} do
-          # Convert index field back to tuple (was {nil, nil, nil} originally)
-          {:index, [a, b, c]} -> {a, b, c}
-          {"index", [a, b, c]} -> {a, b, c}
-          _ -> atomize_patch(v)
-        end
+          {:ok, %{"diff" => raw_diff, "path_from" => path_from, "path_to" => path_to}} ->
+            # Handle new format with raw diff and base paths for relative conversion
+            case GitDiff.parse_patch(raw_diff, relative_from: path_from, relative_to: path_to) do
+              {:ok, []} ->
+                "<div class='patch-info'>No changes in patch</div>"
 
-      {atom_key, converted_value}
-    end)
-    |> Map.new()
-  rescue
-    ArgumentError ->
-      # If atom doesn't exist, return original map with string keys
-      patch
-      |> Enum.map(fn {k, v} ->
-        converted_value =
-          case {k, v} do
-            {"index", [a, b, c]} -> {a, b, c}
-            _ -> atomize_patch(v)
-          end
-
-        {k, converted_value}
-      end)
-      |> Map.new()
-  end
+              {:ok, patches} ->
+                # Take the first patch (should only be one per file)
+                patch = List.first(patches)
 
-  defp atomize_patch(list) when is_list(list) do
-    Enum.map(list, &atomize_patch/1)
-  end
+                Phoenix.View.render_to_string(DiffWeb.RenderView, "patch.html", patch: patch)
+                |> sanitize_utf8()
 
-  defp atomize_patch(value), do: value
-
-  # Convert structs to maps recursively for JSON encoding
-  defp struct_to_map(%{__struct__: _} = struct) do
-    struct
-    |> Map.from_struct()
-    |> Enum.map(fn {k, v} -> {k, struct_to_map(v)} end)
-    |> Map.new()
-  end
-
-  defp struct_to_map(tuple) when is_tuple(tuple) do
-    # Convert tuples to lists for JSON compatibility
-    tuple
-    |> Tuple.to_list()
-    |> Enum.map(&struct_to_map/1)
-  end
-
-  defp struct_to_map(list) when is_list(list) do
-    Enum.map(list, &struct_to_map/1)
-  end
-
-  defp struct_to_map(map) when is_map(map) do
-    map
-    |> Enum.map(fn {k, v} -> {k, struct_to_map(v)} end)
-    |> Map.new()
-  end
+              {:error, reason} ->
+                Logger.error("Failed to parse patch #{patch_id}: #{inspect(reason)}")
+                "<div class='patch-error'>Failed to parse patch</div>"
+            end
 
-  defp struct_to_map(value), do: value
+          _ ->
+            # Fallback: try parsing as raw diff without relative paths (legacy raw format)
+            case GitDiff.parse_patch(raw_content) do
+              {:ok, []} ->
+                "<div class='patch-info'>No changes in patch</div>"
 
-  # Sanitize all binary content in patch data to prevent JSON encoding errors
-  defp sanitize_patch_content(%{__struct__: _} = struct) do
-    struct
-    |> Map.from_struct()
-    |> Enum.map(fn {k, v} -> {k, sanitize_patch_content(v)} end)
-    |> Map.new()
-    |> then(fn map -> struct(struct.__struct__, map) end)
-  end
+              {:ok, patches} ->
+                # Take the first patch (should only be one per file)
+                patch = List.first(patches)
 
-  defp sanitize_patch_content(list) when is_list(list) do
-    Enum.map(list, &sanitize_patch_content/1)
-  end
+                Phoenix.View.render_to_string(DiffWeb.RenderView, "patch.html", patch: patch)
+                |> sanitize_utf8()
 
-  defp sanitize_patch_content(map) when is_map(map) do
-    map
-    |> Enum.map(fn {k, v} -> {k, sanitize_patch_content(v)} end)
-    |> Map.new()
-  end
+              {:error, reason} ->
+                Logger.error("Failed to parse patch #{patch_id}: #{inspect(reason)}")
+                "<div class='patch-error'>Failed to parse patch</div>"
+            end
+        end
 
-  defp sanitize_patch_content(binary) when is_binary(binary) do
-    sanitize_utf8(binary)
+      {:error, _reason} ->
+        "<div class='patch-error'>Failed to load patch</div>"
+    end
   end
 
-  defp sanitize_patch_content(value), do: value
-
   defp parse_diff(diff) do
     case String.split(diff, ":", trim: true) do
       [app, from, to] -> {app, from, to, build_url(app, from, to)}