ensure code proposal contains a valid appup file with the right versi…

…ons (#899)

* ensure code proposal contains a valid appup file with the right versions

* review feedbacks

* refactor tests and get_changes function

* fixed appup validation to also handle up and down instructions and enhanced tests

lib/archethic/governance/code.ex

@@ -67,8 +67,11 @@ defmodule Archethic.Governance.Code do
   - Git diff/patch must be valid.
   """
   @spec valid_proposal?(Proposal.t()) :: boolean()
-  def valid_proposal?(prop = %Proposal{version: version}) do
-    with true <- succeessor_version?(current_version(), version),
+  def valid_proposal?(prop = %Proposal{version: version, changes: changes}) do
+    current_version = current_version()
+
+    with true <- successor_version?(current_version, version),
+         true <- valid_appup?(changes, version, current_version),
          true <- applicable_proposal?(prop) do
       true
     else
@@ -77,6 +80,74 @@ defmodule Archethic.Governance.Code do
     end
   end
 
+  @doc """
+    Ensure the code proposal contains a valid appup file
+  """
+  @spec valid_appup?(binary(), binary(), binary()) :: boolean()
+  def valid_appup?(changes, version, current_version) do
+    with {:ok, patches} <- GitDiff.parse_patch(changes),
+         %GitDiff.Patch{chunks: chunks} <-
+           Enum.find(
+             patches,
+             &(String.starts_with?(&1.to, "rel/appups/archethic") and
+                 String.ends_with?(&1.to, ".appup"))
+           ),
+         %GitDiff.Chunk{lines: lines} <- Enum.at(chunks, 0),
+         code_txt <-
+           Enum.reduce(lines, "", fn
+             %GitDiff.Line{type: :add, text: "+" <> text}, acc ->
+               acc <> text
+
+             _, acc ->
+               acc
+           end),
+         {:ok, {version_char, up_instructions, down_instructions}} <- eval_str(code_txt <> "\n"),
+         true <- version == to_string(version_char),
+         current_version_char_up <-
+           Enum.map(up_instructions, &elem(&1, 0))
+           |> Enum.uniq(),
+         current_version_char_down <-
+           Enum.map(down_instructions, &elem(&1, 0))
+           |> Enum.uniq(),
+         true <- current_version == to_string(current_version_char_up),
+         true <- current_version == to_string(current_version_char_down),
+         :ok <-
+           up_instructions
+           |> Enum.map(&elem(&1, 1))
+           |> List.flatten()
+           |> Distillery.Releases.Appup.Utils.validate_instructions(),
+         :ok <-
+           down_instructions
+           |> Enum.map(&elem(&1, 1))
+           |> List.flatten()
+           |> Distillery.Releases.Appup.Utils.validate_instructions() do
+      true
+    else
+      _ -> false
+    end
+  end
+
+  # working around a bug in typespecs for :erl_eval.eval_str
+  defp eval_str(str) do
+    bin = :erlang.binary_to_list(str)
+
+    {:done, {:ok, token, _}, []} = :erl_scan.tokens([], bin, 0)
+    {:ok, expr} = :erl_parse.parse_exprs(token)
+
+    {:value, val, _} =
+      :erl_eval.exprs(
+        expr,
+        :erl_eval.new_bindings(),
+        {:value, &catch_function_calls/2},
+        {:value, &catch_function_calls/2}
+      )
+
+    {:ok, val}
+  end
+
+  defp catch_function_calls(_func_name, _args),
+    do: throw("Appup file contained calls to a function which is not permitted")
+
   @doc """
   Ensure the code proposal is an applicable on the current branch.
   """
@@ -138,49 +209,49 @@ defmodule Archethic.Governance.Code do
 
   ## Examples
 
-    iex> Code.succeessor_version?("1.1.1", "1.1.2")
+    iex> Code.successor_version?("1.1.1", "1.1.2")
     true
 
-    iex> Code.succeessor_version?("1.1.1", "1.2.0")
+    iex> Code.successor_version?("1.1.1", "1.2.0")
     true
 
-    iex> Code.succeessor_version?("1.1.1", "2.0.0")
+    iex> Code.successor_version?("1.1.1", "2.0.0")
     true
 
-    iex> Code.succeessor_version?("1.1.1", "1.2.2")
+    iex> Code.successor_version?("1.1.1", "1.2.2")
     false
 
-    iex> Code.succeessor_version?("1.1.1", "1.2.1")
+    iex> Code.successor_version?("1.1.1", "1.2.1")
     false
 
-    iex> Code.succeessor_version?("1.1.1", "1.1.2-pre0")
+    iex> Code.successor_version?("1.1.1", "1.1.2-pre0")
     false
   """
-  @spec succeessor_version?(binary | Version.t(), binary | Version.t()) :: boolean
-  def succeessor_version?(version1, version2)
+  @spec successor_version?(binary | Version.t(), binary | Version.t()) :: boolean
+  def successor_version?(version1, version2)
       when is_binary(version1) and is_binary(version2) do
-    succeessor_version?(Version.parse!(version1), Version.parse!(version2))
+    successor_version?(Version.parse!(version1), Version.parse!(version2))
   end
 
-  def succeessor_version?(
+  def successor_version?(
         %Version{major: ma, minor: mi, patch: pa1, pre: [], build: nil},
         %Version{major: ma, minor: mi, patch: pa2, pre: [], build: nil}
       ),
       do: pa1 + 1 == pa2
 
-  def succeessor_version?(
+  def successor_version?(
         %Version{major: ma, minor: mi1, patch: _, pre: [], build: nil},
         %Version{major: ma, minor: mi2, patch: 0, pre: [], build: nil}
       ),
       do: mi1 + 1 == mi2
 
-  def succeessor_version?(
+  def successor_version?(
         %Version{major: ma1, minor: _, patch: _, pre: [], build: nil},
         %Version{major: ma2, minor: 0, patch: 0, pre: [], build: nil}
       ),
       do: ma1 + 1 == ma2
 
-  def succeessor_version?(%Version{}, %Version{}), do: false
+  def successor_version?(%Version{}, %Version{}), do: false
 
   defp current_version do
     :archethic |> Application.spec(:vsn) |> List.to_string()

lib/archethic/governance/code/proposal/parser.ex

@@ -51,7 +51,7 @@ defmodule Archethic.Governance.Code.Proposal.Parser do
        index 30787f1..6535f52 100644
        --- a/lib/archethic/governance.ex
        +++ b/lib/archethic/governance.ex
-       @@ -1,107 +1,107 @@" }
+       @@ -1,107 +1,107 @@\n" }
   """
   @spec get_changes(binary()) :: {:ok, binary()} | {:error, :missing_changes}
   def get_changes(content) when is_binary(content) do
@@ -61,6 +61,7 @@ defmodule Archethic.Governance.Code.Proposal.Parser do
           changes_match
           |> List.first()
           |> String.trim()
+          |> Kernel.<>("\n")
 
         {:ok, changes}
 

mix.exs

@@ -120,7 +120,8 @@ defmodule Archethic.MixProject do
       {:castore, "~> 1.0", override: true},
       {:floki, "~> 0.33"},
       {:ex_cldr, "~> 2.7"},
-      {:ex_cldr_numbers, "~> 2.29"}
+      {:ex_cldr_numbers, "~> 2.29"},
+      {:git_diff, "~> 0.6.4"}
     ]
   end
 

mix.lock

@@ -41,6 +41,7 @@
   "flow": {:hex, :flow, "1.2.3", "1d9239be6a6ec8ef33f22200c8d8b3a756a28145476c7e096fbb3fda04e12f5c", [:mix], [{:gen_stage, "~> 1.0", [hex: :gen_stage, repo: "hexpm", optional: false]}], "hexpm", "77eeb976cb5152a1168290ce9015b8b2d41b70eb5265a36d7538a817237891d9"},
   "gen_stage": {:hex, :gen_stage, "1.2.0", "ee49244b57803f54bdab08a60a927e1b4e5bb5d635c52eca0f376a0673af3f8c", [:mix], [], "hexpm", "c3e40992c72e74d9c4eda16d7515bf32c9e7b634e827ab11091fff3290f7b503"},
   "gen_state_machine": {:hex, :gen_state_machine, "3.0.0", "1e57f86a494e5c6b14137ebef26a7eb342b3b0070c7135f2d6768ed3f6b6cdff", [:mix], [], "hexpm", "0a59652574bebceb7309f6b749d2a41b45fdeda8dbb4da0791e355dd19f0ed15"},
+  "git_diff": {:hex, :git_diff, "0.6.4", "ec53ebf8bf83b4527d938d6433e3686b47a3e2a23135a21038f76736c16bb6e0", [:mix], [], "hexpm", "9e05563c136c91e960a306fd296156b2e8d74e294ae60961e69a36e118023a5f"},
   "git_hooks": {:hex, :git_hooks, "0.7.3", "09489e94d88dfc767662e22aff2b6208bd7cf555a19dd0e1477cca4683ce0701", [:mix], [{:blankable, "~> 1.0.0", [hex: :blankable, repo: "hexpm", optional: false]}, {:recase, "~> 0.7.0", [hex: :recase, repo: "hexpm", optional: false]}], "hexpm", "d6ddedeb4d3a8602bc3f84e087a38f6150a86d9e790628ed8bc70e6d90681659"},
   "hpax": {:hex, :hpax, "0.1.2", "09a75600d9d8bbd064cdd741f21fc06fc1f4cf3d0fcc335e5aa19be1a7235c84", [:mix], [], "hexpm", "2c87843d5a23f5f16748ebe77969880e29809580efdaccd615cd3bed628a8c13"},
   "html_entities": {:hex, :html_entities, "0.5.2", "9e47e70598da7de2a9ff6af8758399251db6dbb7eebe2b013f2bbd2515895c3c", [:mix], [], "hexpm", "c53ba390403485615623b9531e97696f076ed415e8d8058b1dbaa28181f4fdcc"},

test/archethic/governance/code_test.exs

@@ -30,5 +30,132 @@ defmodule Archethic.Governance.CodeTest do
     assert status in [:clean, :dirty]
   end
 
+  test "valid_appup? should pass" do
+    current_version = "1.0.7"
+    appup_version = version = "1.0.8"
+
+    instruction = "load_module"
+
+    changes = generate_diff(version, current_version, appup_version, instruction)
+
+    assert Code.valid_appup?(changes, version, current_version)
+  end
+
+  test "valid_appup? should fail because appup contains illegal instruction" do
+    current_version = "1.0.7"
+    appup_version = version = "1.0.8"
+
+    instruction = "invalid"
+
+    changes = generate_diff(version, current_version, appup_version, instruction)
+
+    refute Code.valid_appup?(changes, version, current_version)
+  end
+
+  test "valid_appup? should throw an error because appup contains illegal function calls" do
+    current_version = "1.0.7"
+    appup_version = version = "1.0.8"
+
+    changes = generate_diff_with_function_call(version, current_version, appup_version)
+
+    assert catch_throw(Code.valid_appup?(changes, version, current_version)) ==
+             "Appup file contained calls to a function which is not permitted"
+  end
+
+  test "valid_appup? should fail because appup contains wrong version" do
+    current_version = "1.0.7"
+    version = "1.0.8"
+    appup_version = "1.0.9"
+    instruction = "load_module"
+
+    changes = generate_diff(version, current_version, appup_version, instruction)
+
+    refute Code.valid_appup?(changes, version, current_version)
+  end
+
+  test "valid_appup? should fail because it doens't contain appup" do
+    current_version = "1.0.7"
+    appup_version = version = "1.0.8"
+    instruction = "load_module"
+
+    changes = generate_diff(version, current_version, appup_version, instruction, false)
+
+    refute Code.valid_appup?(changes, version, current_version)
+  end
+
+  defp generate_diff(
+         version,
+         current_version,
+         appup_version,
+         instruction,
+         should_add_appup? \\ true
+       ) do
+    base_appup(current_version, version) <>
+      maybe_add_appup(current_version, appup_version, instruction, should_add_appup?) <>
+      end_base_appup()
+  end
+
+  defp generate_diff_with_function_call(
+         version,
+         current_version,
+         appup_version
+       ) do
+    base_appup(current_version, version) <>
+      appup_with_function_call(current_version, appup_version) <>
+      end_base_appup()
+  end
+
+  defp base_appup(current_version, version) do
+    "diff --git a/mix.exs b/mix.exs\n" <>
+      "index c615ed94..d16e2d6f 100644\n" <>
+      "--- a/mix.exs\n" <>
+      "+++ b/mix.exs\n" <>
+      "@@ -4,7 +4,7 @@ defmodule Archethic.MixProject do\n" <>
+      "   def project do\n" <>
+      "     [\n" <>
+      "       app: :archethic,\n" <>
+      "-      version: \"#{current_version}\",\n" <>
+      "+      version: \"#{version}\",\n" <>
+      "       build_path: \"_build\",\n" <>
+      "       config_path: \"config/config.exs\",\n" <>
+      "       deps_path: \"deps\",\n"
+  end
+
+  defp end_base_appup() do
+    "\\ No newline at end of file"
+  end
+
+  defp appup_with_function_call(current_version, appup_version) do
+    "diff --git a/rel/appups/archethic/1.0.7_to_1.0.8.appup b/rel/appups/archethic/1.0.7_to_1.0.8.appup\n" <>
+      "new file mode 100644\n" <>
+      "index 00000000..0f18b8e1\n" <>
+      "--- /dev/null\n" <>
+      "+++ b/rel/appups/archethic/1.0.7_to_1.0.8.appup\n" <>
+      "@@ -0,0 +1,4 @@\n" <>
+      "+{\"#{appup_version}\",\n" <>
+      "+ [{\"#{current_version}\",\n" <>
+      "+   [{load_module, io:format(\"execute some code\"), []}]}],\n" <>
+      "+ [{\"#{current_version}\",\n" <>
+      "+   [{load_module, 'TOTO', []}]}]\n" <>
+      "+}.\n"
+  end
+
+  defp maybe_add_appup(_, _, _, false), do: ""
+
+  defp maybe_add_appup(current_version, appup_version, instruction, true) do
+    "diff --git a/rel/appups/archethic/1.0.7_to_1.0.8.appup b/rel/appups/archethic/1.0.7_to_1.0.8.appup\n" <>
+      "new file mode 100644\n" <>
+      "index 00000000..0f18b8e1\n" <>
+      "--- /dev/null\n" <>
+      "+++ b/rel/appups/archethic/1.0.7_to_1.0.8.appup\n" <>
+      "@@ -0,0 +1,4 @@\n" <>
+      "+{\"#{appup_version}\",\n" <>
+      "+ [{\"#{current_version}\",\n" <>
+      "+   [{#{instruction},'TOTO', []}]}],\n" <>
+      "+ [{\"#{current_version}\",\n" <>
+      "+   [{#{instruction},'TOTO', []}]}]\n" <>
+      "+}.\n"
+  end
+
   doctest Code
 end