From 808646a5ca61c6d668bdd610f235817f4cad8d53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Thu, 19 Jun 2014 08:12:30 +0200
Subject: [PATCH] Work towards elixir 14

While here, make the object be an elixir struct instead of delegating to
the erlang code.

As part of this, geef_nif:commit_tree() has been fixed to return the id,
insted of the type, as we know it's going to be a tree, but not then id.
---
 c_src/commit.c          |  5 +++-
 lib/geef/blob.ex        | 19 ++++++--------
 lib/geef/commit.ex      | 27 +++++++++-----------
 lib/geef/index.ex       | 36 +++++++++++++++------------
 lib/geef/iterator.ex    | 31 +++++++++++++----------
 lib/geef/object.ex      | 27 +++++++++-----------
 lib/geef/pkt.ex         |  4 ++-
 lib/geef/reference.ex   | 26 +++++++++++++------
 lib/geef/signature.ex   | 14 +++++------
 lib/geef/tag.ex         | 15 ++++++-----
 lib/geef/tree.ex        | 55 +++++++++++++++++++++++------------------
 src/geef_commit.erl     |  4 +--
 test/object_test.exs    | 14 ++++-------
 test/reference_test.exs | 17 +++++--------
 test/test_helper.exs    |  3 ++-
 15 files changed, 155 insertions(+), 142 deletions(-)

diff --git a/c_src/commit.c b/c_src/commit.c
index e9c3c14..3a5cf49 100644
--- a/c_src/commit.c
+++ b/c_src/commit.c
@@ -28,6 +28,7 @@ geef_commit_tree_id(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
 ERL_NIF_TERM
 geef_commit_tree(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
 {
+	ErlNifBinary bin;
 	geef_object *obj, *tree;
 	ERL_NIF_TERM term_obj;
 
@@ -42,8 +43,10 @@ geef_commit_tree(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
 	term_obj = enif_make_resource(env, obj);
 	enif_release_resource(obj);
 
-	return enif_make_tuple3(env, atoms.ok, atoms.tree, term_obj);
+	if (geef_oid_bin(&bin, git_object_id(tree->obj)) < 0)
+		return geef_oom(env);
 
+	return enif_make_tuple3(env, atoms.ok, enif_make_binary(env, &bin), term_obj);
 }
 
 ERL_NIF_TERM
diff --git a/lib/geef/blob.ex b/lib/geef/blob.ex
index 4050205..f97c87b 100644
--- a/lib/geef/blob.ex
+++ b/lib/geef/blob.ex
@@ -1,22 +1,17 @@
 defmodule Geef.Blob do
-  alias Geef.Object
-  import Object, only: :macros
+  use Geef
 
   def lookup(repo, id) do
-    case :geef_obj.lookup(repo, id) do
-      {:ok, obj} ->
-        {:ok, Object.from_erl obj}
-      error ->
-        error
-    end
+    Object.lookup(repo, id, :blob)
   end
 
-  def size(obj = Object[type: :blob]) do
-    :geef_blob.size(rebind(obj))
+
+  def size(obj = %Object{type: :blob, handle: handle}) do
+    :geef_nif.blob_size(handle)
   end
 
-  def content(obj = Object[type: :blob]) do
-    :geef_blob.content(rebind(obj))
+  def content(obj = %Object{type: :blob, handle: handle}) do
+    :geef_nif.blob_content(handle)
   end
 
 end
diff --git a/lib/geef/commit.ex b/lib/geef/commit.ex
index f8ca38f..f956beb 100644
--- a/lib/geef/commit.ex
+++ b/lib/geef/commit.ex
@@ -1,39 +1,34 @@
 defmodule Geef.Commit do
   use Geef
-  import Object, only: :macros
 
   @type t :: Object[type: :commit]
 
+  @spec lookup(pid, Oid.t) :: Commit.t
   def lookup(repo, id) do
-    case :geef_commit.lookup(repo, id) do
-      {:ok, commit} ->
-        {:ok, Object.from_erl commit}
-      error ->
-        error
-    end
+    Object.lookup(repo, id, :commit)
   end
 
   @spec tree_id(Object.t) :: Oid.t
-  def tree_id(commit = Object[type: :commit]) do
-    :geef_commit.tree_id(rebind(commit))
+  def tree_id(commit = %Object{type: :commit, handle: handle}) do
+    :geef_nif.commit_tree_id(handle)
   end
 
   @spec tree(t) :: {:ok, Tree.t} | {:error, any}
-  def tree(commit = Object[type: :commit]) do
-    case :geef_commit.tree(rebind(commit)) do
-      {:ok, tree} ->
-        {:ok, Object.from_erl(tree)}
+  def tree(commit = %Object{type: :commit, handle: handle}) do
+    case :geef_nif.commit_tree(handle) do
+      {:ok, id, handle} ->
+        {:ok, %Object{type: :tree, id: id, handle: handle}}
       error = {:error, _} ->
         error
     end
   end
 
   @spec tree!(t) :: Tree.t
-  def tree!(commit = Object[type: :commit]), do: tree(commit) |> Geef.assert_ok
+  def tree!(commit = %Object{type: :commit}), do: tree(commit) |> Geef.assert_ok
 
   @spec create(pid, Signature.t, Signature.t, iolist, Oid.t, [Oid.t], [:proplists.property()]) :: {:ok, Oid.t} | {:error, term}
-  def create(repo, author = Signature[], committer = Signature[], message, tree, parents, opts \\ []) do
-    :geef_commit.create(repo, Signature.to_erl(author), Signature.to_erl(committer), message, tree, parents, opts)
+  def create(repo, author = %Signature{}, committer = %Signature{}, message, tree, parents, opts \\ []) do
+    :geef_commit.create(repo, Signature.to_record(author), Signature.to_record(committer), message, tree, parents, opts)
   end
 
 end
diff --git a/lib/geef/index.ex b/lib/geef/index.ex
index 53e8342..140229c 100644
--- a/lib/geef/index.ex
+++ b/lib/geef/index.ex
@@ -1,22 +1,28 @@
 require Record
 
+defmodule Geef.Index.Entry do
+  record = Record.extract(:geef_index_entry, from: "src/geef_records.hrl")
+  keys   = :lists.map(&elem(&1, 0), record)
+  vals   = :lists.map(&{&1, [], nil}, keys)
+  pairs  = :lists.zip(keys, vals)
+
+  defstruct keys
+
+  def from_record({:geef_index_entry, unquote_splicing(vals)}) do
+    %Geef.Index.Entry{unquote_splicing(pairs)}
+  end
+
+  def to_record(%Geef.Index.Entry{unquote_splicing(pairs)}) do
+    {:geef_index_entry, unquote_splicing(vals)}
+  end
+
+end
+
 defmodule Geef.Index do
   use Geef
   alias Geef.Index.Entry
 
-  defmacrop to_erl(entry) do
-    quote do
-      set_elem(unquote(entry), 0, :geef_index_entry)
-    end
-  end
-
-  defmacrop from_erl(entry) do
-    quote do
-      set_elem(unquote(entry), 0, Geef.Index.Entry)
-    end
-  end
-
-  defp maybe_entry({:ok, entry}), do: {:ok, from_erl(entry)}
+  defp maybe_entry({:ok, entry}), do: {:ok, Entry.from_record(entry)}
   defp maybe_entry(error = {:error, _}), do: error
 
   @spec new :: {:ok, pid()} | :ignore | {:error, term()}
@@ -26,7 +32,7 @@ defmodule Geef.Index do
 
   @spec add(pid(), Entry.t()) :: :ok | {:error, term()}
   def add(pid, entry) do
-    :geef_index.add(pid, to_erl(entry))
+    :geef_index.add(pid, Entry.to_record(entry))
   end
 
   @spec clear(pid()) :: :ok
@@ -68,5 +74,3 @@ defmodule Geef.Index do
   end
 
 end
-
-defrecord Geef.Index.Entry, Record.extract(:geef_index_entry, from: "src/geef_records.hrl")
diff --git a/lib/geef/iterator.ex b/lib/geef/iterator.ex
index c988d55..4dd1424 100644
--- a/lib/geef/iterator.ex
+++ b/lib/geef/iterator.ex
@@ -1,22 +1,25 @@
 require Record
 
-defrecord Geef.Iterator, Record.extract(:geef_iterator, from: "src/geef_records.hrl") do
+defmodule Geef.Iterator do
   alias Geef.Iterator
   alias Geef.Reference
 
-  @doc false
-  defmacro rebind(obj) do
-    quote do
-      set_elem(unquote(obj), 0, :geef_iterator)
-    end
+  record = Record.extract(:geef_iterator, from: "src/geef_records.hrl")
+  keys   = :lists.map(&elem(&1, 0), record)
+  vals   = :lists.map(&{&1, [], nil}, keys)
+  pairs  = :lists.zip(keys, vals)
+
+  defstruct keys
+
+  def from_record({:geef_iterator, unquote_splicing(vals)}) do
+    %Geef.Iterator{unquote_splicing(pairs)}
   end
 
-  @spec from_erl(term()) :: t
-  def from_erl(iterator) do
-    set_elem(iterator, 0, Geef.Iterator)
+  def to_record(%Geef.Iterator{unquote_splicing(pairs)}) do
+    {:geef_iterator, unquote_splicing(vals)}
   end
 
-  def stream!(Iterator[type: :ref, repo: repo, regexp: regexp]) do
+  def stream!(%Iterator{type: :ref, repo: repo, regexp: regexp}) do
     iter =
       case :geef_ref.iterator(repo, regexp) do
         {:ok, iter} ->
@@ -27,8 +30,8 @@ defrecord Geef.Iterator, Record.extract(:geef_iterator, from: "src/geef_records.
     &do_stream(iter, &1, &2)
   end
 
-  defp do_stream(iter = Iterator[type: :ref], acc, fun) do
-    case :geef_ref.next(rebind(iter)) do
+  defp do_stream(iter = %Iterator{type: :ref}, acc, fun) do
+    case :geef_ref.next(to_record(iter)) do
       {:ok, ref} ->
         do_stream(iter, fun.(Reference.from_erl(ref), acc), fun)
       {:error, :iterover} ->
@@ -39,4 +42,6 @@ defrecord Geef.Iterator, Record.extract(:geef_iterator, from: "src/geef_records.
   end
 end
 
-defexception Geef.IteratorError, [message: nil]
+defmodule Geef.IteratorError do
+  defexception [message: nil]
+end
diff --git a/lib/geef/object.ex b/lib/geef/object.ex
index 8df010a..3e59ca5 100644
--- a/lib/geef/object.ex
+++ b/lib/geef/object.ex
@@ -1,26 +1,21 @@
 require Record
 
-defrecord Geef.Object, Record.extract(:geef_object, from: "src/geef_records.hrl") do
+defmodule Geef.Object do
+  defstruct type: nil, id: nil, handle: nil
 
-  @doc false
-  defmacro rebind(obj) do
-    quote do
-      set_elem(unquote(obj), 0, :geef_object)
+  def lookup(repo, id) do
+    case :geef_repo.lookup_object(repo, id) do
+      {:ok, type, handle} ->
+        {:ok, %Geef.Object{type: type, id: id, handle: handle}}
+      error ->
+        error
     end
   end
 
-  def from_erl(obj) when elem(obj, 1) == :tree do
-    set_elem(obj, 0, Geef.Tree)
-  end
-
-  def from_erl(obj) do
-    set_elem(obj, 0, Geef.Object)
-  end
-
-  def lookup(repo, id) do
-    case :geef_obj.lookup(repo, id) do
+  def lookup(repo, id, type) do
+    case lookup(repo, id) do
       {:ok, obj} ->
-        {:ok, Geef.Object.from_erl obj}
+        {:ok, obj = %Geef.Object{type: type}}
       error ->
         error
     end
diff --git a/lib/geef/pkt.ex b/lib/geef/pkt.ex
index 77b1d15..bdfc02a 100644
--- a/lib/geef/pkt.ex
+++ b/lib/geef/pkt.ex
@@ -1,6 +1,8 @@
 require Record
 
-defrecord Geef.Request, Record.extract(:geef_request, from: "src/geef_records.hrl")
+defmodule Geef.Request do
+  defstruct Record.extract(:geef_request, from: "src/geef_records.hrl")
+end
 
 defmodule Geef.Pkt do
   
diff --git a/lib/geef/reference.ex b/lib/geef/reference.ex
index ce00eb5..1a05bcf 100644
--- a/lib/geef/reference.ex
+++ b/lib/geef/reference.ex
@@ -1,13 +1,25 @@
 require Record
 
-defrecord Geef.Reference, Record.extract(:geef_reference, from: "src/geef_records.hrl") do
+defmodule Geef.Reference do
   import Geef
   alias Geef.Reference
 
-  def from_erl(ref), do: set_elem(ref, 0, Geef.Reference)
-  def to_erl(ref), do: set_elem(ref, 0, :geef_reference)
+  record = Record.extract(:geef_reference, from: "src/geef_records.hrl")
+  keys   = :lists.map(&elem(&1, 0), record)
+  vals   = :lists.map(&{&1, [], nil}, keys)
+  pairs  = :lists.zip(keys, vals)
 
-  defp maybe_ref({:ok, ref}), do: {:ok, Reference.from_erl ref}
+  defstruct keys
+
+  def to_record(%Geef.Reference{unquote_splicing(pairs)}) do
+    {:geef_reference, unquote_splicing(vals)}
+  end
+
+  def from_record({:geef_reference, unquote_splicing(vals)}) do
+    %Geef.Reference{unquote_splicing(pairs)}
+  end
+
+  defp maybe_ref({:ok, ref}), do: {:ok, Reference.from_record(ref)}
   defp maybe_ref(err = {:error, _}), do: err
 
   def create(repo, name, target, force \\ :false) do
@@ -26,13 +38,13 @@ defrecord Geef.Reference, Record.extract(:geef_reference, from: "src/geef_record
   def lookup(repo, name), do: :geef_ref.lookup(repo, name) |> maybe_ref
   def lookup!(repo, name), do: lookup(repo, name) |> assert_ok
 
-  def resolve(ref = Reference[]), do: :geef_ref.resolve(to_erl(ref)) |> maybe_ref
-  def resolve!(ref =Reference[]), do: resolve(ref) |> assert_ok
+  def resolve(ref = %Reference{}), do: :geef_ref.resolve(to_record(ref)) |> maybe_ref
+  def resolve!(ref = %Reference{}), do: resolve(ref) |> assert_ok
 
   def dwim(repo, name), do: :geef_ref.dwim(repo, name) |> maybe_ref
   def dwim!(repo, name), do: dwim(repo, name) |> assert_ok
 
-  def shorthand(Reference[name: name]) do
+  def shorthand(%Reference{name: name}) do
     :geef_ref.shorthand(name)
   end
   def shorthand(name) do
diff --git a/lib/geef/signature.ex b/lib/geef/signature.ex
index 01da095..20e470a 100644
--- a/lib/geef/signature.ex
+++ b/lib/geef/signature.ex
@@ -1,17 +1,17 @@
 require Record
 
-defrecord Geef.Signature, Record.extract(:geef_signature, from: "src/geef_records.hrl") do
+defmodule Geef.Signature do
+  defstruct Record.extract(:geef_signature, from: "src/geef_records.hrl")
 
-  def now(name, email), do: :geef_sig.now(name, email) |> from_erl
+  def now(name, email), do: :geef_sig.now(name, email) |> from_record
 
   def default(repo), do: :geef_sig.default(repo) |> maybe_sig
 
-  defp maybe_sig({:ok, sig}), do: from_erl(sig)
+  defp maybe_sig({:ok, sig}), do: from_record(sig)
   defp maybe_sig(error = {:error, _}), do: error
 
-  defp from_erl(sig), do: set_elem(sig, 0, Geef.Signature)
-
-  @spec to_erl(t) :: :geef_sig.signature
-  def to_erl(sig), do: set_elem(sig, 0, :geef_signature)
+  def from_record({:geef_signature}) do
+    %Geef.Signature{}
+  end
 
 end
diff --git a/lib/geef/tag.ex b/lib/geef/tag.ex
index 4ad406a..132ac84 100644
--- a/lib/geef/tag.ex
+++ b/lib/geef/tag.ex
@@ -1,17 +1,20 @@
 defmodule Geef.Tag do
   alias Geef.Object
-  import Object, only: :macros
   import Geef
 
-  def peel(tag = Object[type: :tag]) do
-    case :geef_tag.peel(rebind(tag)) do
-      {:ok, peeled} ->
-        {:ok, Object.from_erl peeled}
+  def lookup(repo, id) do
+    Object.lookup(repo, id, :tag)
+  end
+
+  def peel(tag = %Object{type: :tag, handle: handle}) do
+    case :geef_nif.tag_peel(handle) do
+      {:ok, type, id, peeled_handle} ->
+        {:ok, %Object{type: type, id: id, handle: peeled_handle}}
       error ->
         error
     end
   end
 
-  def peel!(tag = Object[type: :tag]), do: peel(tag) |> assert_ok
+  def peel!(tag = %Object{type: :tag}), do: peel(tag) |> assert_ok
 
 end
\ No newline at end of file
diff --git a/lib/geef/tree.ex b/lib/geef/tree.ex
index 41df45e..ac0bb8c 100644
--- a/lib/geef/tree.ex
+++ b/lib/geef/tree.ex
@@ -1,60 +1,67 @@
 require Record
 
-defrecord Geef.TreeEntry, Record.extract(:geef_tree_entry, from: "src/geef_records.hrl") do
-  @spec from_erl(term()) :: t
-  def from_erl(obj) do
-    set_elem(obj, 0, Geef.TreeEntry)
-  end
+defmodule Geef.TreeEntry do
+  defstruct mode: nil, type: nil, id: nil, name: nil
 end
 
-defrecord Geef.Tree, Record.extract(:geef_object, from: "src/geef_records.hrl") do
+defmodule Geef.Tree do
   alias Geef.Object
   alias Geef.TreeEntry
 
-  import Object, only: :macros
-
-  @type t :: Object[type: :commit]
+  @type t :: Object[type: :tree]
 
   def lookup(repo, id) do
-    case :geef_tree.lookup(repo, id) do
-      {:ok, obj} ->
-        {:ok, Object.from_erl obj}
-      error ->
-        error
-    end
+    Object.lookup(repo, id, :tree)
   end
 
-  defp maybe_entry({:ok, entry}), do: {:ok, TreeEntry.from_erl entry}
+  defp maybe_entry({:ok, mode, type, id, name}) do
+    {:ok, %TreeEntry{mode: mode, type: type, id: id, name: name}}
+  end
   defp maybe_entry(error = {:error, _}), do: error
 
-  def get(tree, path), do: :geef_tree.get(rebind(tree), path) |> maybe_entry
-  def nth(tree, pos), do: :geef_tree.nth(rebind(tree), pos) |> maybe_entry
+  def get(%Object{type: :tree, handle: handle}, path) do
+    :geef_nif.tree_bypath(handle, path) |> maybe_entry
+  end
 
-  def count(tree), do: :geef_tree.count(rebind(tree))
+  def nth(%Object{type: :tree, handle: handle}, nth) do
+    :geef_nif.tree_nth(handle, nth) |> maybe_entry
+  end
+
+  def count(%Object{type: :tree, handle: handle}) do
+    :geef_nif.tree_count(handle)
+  end
 
 end
 
-defimpl Access, for: Geef.Tree do
+defimpl Access, for: Geef.Object do
+  alias Geef.Object
   alias Geef.Tree
 
-  def access(tree, key) when is_number(key) do
+  def get(tree = %Object{type: :tree}, key) when is_number(key) do
     case Tree.nth(tree, key) do
       {:ok, entry} -> entry
       {:error, _} -> nil
     end
   end
 
-  def access(tree, key) do
+  def get(tree = %Object{type: :tree}, key) do
     case Tree.get(tree, key) do
       {:ok, entry} -> entry
       {:error, _} -> nil
     end
   end
 
+  # Git data is immutable
+  def get_and_update(_tree, _key, _fun) do
+    raise ArgumentError
+  end
+
 end
 
-defexception Geef.TreeError, reason: nil do
-  def message(Geef.TreeError[reason: reason]) do
+defmodule Geef.TreeError do
+  defexception [reason: nil]
+
+  def message(%Geef.TreeError{reason: reason}) do
     "tree error #{inspect reason}"
   end
 end
diff --git a/src/geef_commit.erl b/src/geef_commit.erl
index be463ef..8bf65a3 100644
--- a/src/geef_commit.erl
+++ b/src/geef_commit.erl
@@ -14,8 +14,8 @@ tree_id(#geef_object{type=commit,handle=Handle}) ->
 -spec tree(commit()) -> {ok, geef_tree:tree()} | {error, term()}.
 tree(#geef_object{type=commit,handle=Handle}) ->
     case geef_nif:commit_tree(Handle) of
-	{ok, Type, Handle} ->
-	    {ok, #geef_object{type=Type, handle=Handle}};
+	{ok, Id, Handle} ->
+	    {ok, #geef_object{id=Id, handle=Handle}};
 	Other ->
 	    Other
     end.
diff --git a/test/object_test.exs b/test/object_test.exs
index cc6aea8..d79c177 100644
--- a/test/object_test.exs
+++ b/test/object_test.exs
@@ -1,21 +1,15 @@
-Code.require_file "../test_helper.exs", __FILE__
-
 defmodule ObjectTest do
   use ExUnit.Case
   use Geef
   import RepoHelpers
 
   setup do
-    {{:ok, repo}, path} = tmp_bare()
+    {repo, path} = tmp_bare()
+    Process.link(repo)
+    on_exit(fn -> File.rm_rf!(path) end)
     {:ok, [repo: repo, path: path]}
   end
 
-  teardown meta do
-    Repository.stop(meta[:repo])
-    File.rm_rf!(meta[:path])
-    :ok
-  end
-
   test "object OO helpers", meta do
     repo = meta[:repo]
     { :ok, odb } = Repository.odb(repo)
@@ -23,5 +17,7 @@ defmodule ObjectTest do
 
     content = "I'm some content"
     Odb.write(odb, content, :blob)
+
+    Repository.stop(repo)
   end
 end
diff --git a/test/reference_test.exs b/test/reference_test.exs
index 3eab212..d6a4658 100644
--- a/test/reference_test.exs
+++ b/test/reference_test.exs
@@ -1,21 +1,14 @@
-Code.require_file "../test_helper.exs", __FILE__
-
 defmodule ReferenceTest do
   use ExUnit.Case
   use Geef
   import RepoHelpers
 
   setup do
-    {{:ok, repo}, path} = tmp_bare()
+    {repo, path} = tmp_bare()
+    on_exit(fn -> File.rm_rf!(path) end)
     {:ok, [repo: repo, path: path]}
   end
 
-  teardown meta do
-    Repository.stop(meta[:repo])
-    File.rm_rf!(meta[:path])
-    :ok
-  end
-
   test "creating and looking up", meta do
     repo = meta[:repo]
     { :ok, odb } = Repository.odb(repo)
@@ -25,12 +18,14 @@ defmodule ReferenceTest do
     refname = "refs/tags/foo"
     {:ok, ref} = Reference.create(repo, refname, id)
     {:ok, looked_up} = Reference.lookup(repo, refname)
-    ref == looked_up
+    assert ref == looked_up
 
     refname = "refs/tags/foo2"
     {:ok, ref} = Reference.create_symbolic(repo, refname, id)
     {:ok, looked_up} = Reference.lookup(repo, refname)
-    ref == looked_up
+    assert ref == looked_up
+
+    Repository.stop(repo)
   end
 
 end
diff --git a/test/test_helper.exs b/test/test_helper.exs
index 57c7766..19b1813 100644
--- a/test/test_helper.exs
+++ b/test/test_helper.exs
@@ -6,6 +6,7 @@ defmodule RepoHelpers do
     n = node()
     dir = :io_lib.format("geef-~p~p~p~p.git", [n, a, b, c])
     path = Path.join(System.tmp_dir!, dir)
-    {Geef.Repository.init(path, true), path}
+    {:ok, repo} = Geef.Repository.init(path, true)
+    {repo, path}
   end
 end