nodegit · srajko · Mar 23, 2016 · Mar 23, 2016 · Mar 24, 2016 · Mar 23, 2016
diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json
@@ -1767,6 +1767,8 @@
       "ignore": true
     },
     "repository": {
+      "cacheResult": true,
+      "oneToOne": true,
       "functions": {
         "git_repository__cleanup": {
           "ignore": true

diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js
@@ -179,7 +179,9 @@ var Helpers = {
     typeDef.filename = typeDef.typeName;
     typeDef.isLibgitType = true;
     typeDef.dependencies = [];
+    typeDef.cacheResult = Boolean(typeDefOverrides.cacheResult);
     typeDef.selfFreeing = Boolean(typeDefOverrides.selfFreeing);
+    typeDef.oneToOne = Boolean(typeDefOverrides.oneToOne);
 
     if (typeDefOverrides.freeFunctionName) {
       typeDef.freeFunctionName = typeDefOverrides.freeFunctionName;

diff --git a/generate/templates/manual/include/functions/noop.h b/generate/templates/manual/include/functions/noop.h
@@ -0,0 +1,8 @@
+#ifndef NOOP_FUNCTIONS
+#define NOOP_FUNCTIONS
+
+#include <nan.h>
+
+void noop(const Nan::WeakCallbackInfo<void> &data);
+
+#endif
diff --git a/generate/templates/manual/src/functions/noop.cc b/generate/templates/manual/src/functions/noop.cc
@@ -0,0 +1,4 @@
+
+#include "../../include/functions/noop.h"
+
+void noop(const Nan::WeakCallbackInfo<void> &data) {}
diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc
@@ -2,6 +2,11 @@
 {%partial doc .%}
 NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
   Nan::EscapableHandleScope scope;
+
+{%if return.cacheResult %}
+  {{ cppClassName }} *thisObj = Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This());
+  return info.GetReturnValue().Set(scope.Escape(Nan::New(thisObj->{{ cppFunctionName }}_cachedResult)));
+{% else %}
   {%partial guardArguments .%}
 
   {%each .|returnsInfo 'true' as _return %}
@@ -35,7 +40,7 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL
 {% endif %}
 
   giterr_clear();
-  
+
   {
     LockMaster lockMaster(true{%each args|argsInfo as arg %}
       {%if arg.cType|isPointer%}{%if not arg.isReturn%}
@@ -125,4 +130,38 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL
     {%endif%}
   {%endif%}
   }
+{%endif%}
 }
+
+{%if return.cacheResult %}
+void {{ cppClassName }}::{{ cppFunctionName }}_cache() {
+  if (!raw) {
+    {{ cppFunctionName }}_cachedResult.Reset(Nan::Null());
+    return;
+  }
+
+  LockMaster lockMaster(true{%each args|argsInfo as arg %}
+    {%if arg.cType|isPointer%}{%if not arg.isReturn%}
+      ,{%if arg.isSelf %}
+        raw
+    {%endif%}
+    {%endif%}{%endif%}
+  {%endeach%});
+
+  {{ return.cType }} result = {{ cFunctionName }}(
+  {%each args|argsInfo as arg %}
+    {%if arg.isSelf %}
+      raw
+    {%endif%}
+  {%endeach%}
+  );
+
+  Local<v8::Value> to;
+
+  {%each .|returnsInfo as _return %}
+    {%partial convertToV8 _return %}
+  {%endeach%}
+
+  {{ cppFunctionName }}_cachedResult.Reset(to);
+}
+{% endif %}
diff --git a/generate/templates/templates/binding.gyp b/generate/templates/templates/binding.gyp
@@ -18,6 +18,7 @@
         "src/promise_completion.cc",
         "src/wrapper.cc",
         "src/functions/copy.cc",
+        "src/functions/noop.cc",
         "src/functions/sleep_for_ms.cc",
         "src/convenient_patch.cc",
         "src/convenient_hunk.cc",

diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc
@@ -10,6 +10,7 @@ extern "C" {
 
 #include "../include/lock_master.h"
 #include "../include/functions/copy.h"
+#include "../include/functions/noop.h"
 #include "../include/{{ filename }}.h"
 #include "../include/functions/sleep_for_ms.h"
 
@@ -25,6 +26,14 @@ using namespace node;
 
 {% if cType %}
   {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) {
+    {% if oneToOne %}
+    if(instances.find(raw) != instances.end()) {
+      Nan::ThrowError("{{ cppClassName }} constructor called on an already wrapped {{ cType }} object");
+      return;
+    }
+    selfFreeing = true;
+    {% endif %}
+
     if (shouldDuplicate) {
       {% if shouldAlloc %}
     this->raw = ({{ cType }} *)malloc(sizeof({{ cType }}));
@@ -44,6 +53,16 @@ using namespace node;
       NonSelfFreeingConstructedCount++;
     }
 
+
+	{%each functions as function%}
+      {%if not function.ignore %}
+        {%if not function.isAsync %}
+          {%if function.return.cacheResult %}
+            {{ function.cppFunctionName }}_cache(); // populate cached value
+          {%endif%}
+        {%endif%}
+      {%endif%}
+    {%endeach%}
   }
 
   {{ cppClassName }}::~{{ cppClassName }}() {
@@ -52,7 +71,7 @@ using namespace node;
         {{ freeFunctionName }}(this->raw);
         SelfFreeingInstanceCount--;
 
-        this->raw = NULL;
+        ClearValue();
       }
     {% endif %}
 
@@ -126,8 +145,24 @@ using namespace node;
 
   Local<v8::Value> {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) {
     Nan::EscapableHandleScope scope;
+
+    {% if oneToOne %}
+    auto mapElement = instances.find(const_cast<{{ cType }} *>(raw));
+    if(mapElement != instances.end()) {
+      return scope.Escape(Nan::New(*mapElement->second));
+    }
+    {% endif %}
+
     Local<v8::Value> argv[3] = { Nan::New<External>((void *)raw), Nan::New(selfFreeing), Nan::New(shouldDuplicate) };
-    return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 3, argv).ToLocalChecked());
+    Local<v8::Object> instance = Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 3, argv).ToLocalChecked();
+    {% if oneToOne %}
+    Nan::Persistent<v8::Object> *persistent = new Nan::Persistent<v8::Object>(instance);
+    persistent->SetWeak((void *)NULL, noop, Nan::WeakCallbackType::kParameter);
+    if (raw) {
+      instances[const_cast<git_repository *>(raw)] = persistent;
+    }
+    {% endif %}
+    return scope.Escape(instance);
   }
 
   NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) {
@@ -143,6 +178,14 @@ using namespace node;
   }
 
   void {{ cppClassName }}::ClearValue() {
+    {% if oneToOne %}
+    auto mapElement = instances.find(raw);
+    if(mapElement != instances.end()) {
+      delete mapElement->second;
+      instances.erase(mapElement);
+    }
+    {% endif %}
+
     this->raw = NULL;
   }
 
@@ -182,5 +225,10 @@ using namespace node;
   Nan::Persistent<Function> {{ cppClassName }}::constructor_template;
 {% endif %}
 
+{%if oneToOne %}
+// map to ensure one wrapper per libgit2 instance
+std::map<{{ cType }} *, Nan::Persistent<v8::Object> *> {{ cppClassName }}::instances;
+{%endif%}
+
 int {{ cppClassName }}::SelfFreeingInstanceCount;
 int {{ cppClassName }}::NonSelfFreeingConstructedCount;
diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h
@@ -1,6 +1,7 @@
 #ifndef {{ cppClassName|upper }}_H
 #define {{ cppClassName|upper }}_H
 #include <nan.h>
+#include <map>
 #include <string>
 #include <queue>
 #include <utility>
@@ -36,6 +37,10 @@ using namespace node;
 using namespace v8;
 
 class {{ cppClassName }} : public Nan::ObjectWrap {
+    {%if oneToOne %}
+    // map to ensure one wrapper per libgit2 instance
+    static std::map<{{ cType }} *, Nan::Persistent<v8::Object> *> instances;
+    {%endif%}
   public:
 
     static Nan::Persistent<Function> constructor_template;
@@ -141,6 +146,14 @@ class {{ cppClassName }} : public Nan::ObjectWrap {
       private:
         {{ function.cppFunctionName }}Baton *baton;
     };
+        {%else%}
+    {%if function.return.cacheResult %}
+    // For simple sync functions that return a wrapped object and pass `raw`
+    // as the the only parameter to libgit2, we cache the results.
+    // CopyablePersistentTraits are used to get the reset-on-destruct behavior.
+    void {{ function.cppFunctionName }}_cache();
+    Nan::Persistent<Value, Nan::CopyablePersistentTraits<Value> > {{ function.cppFunctionName }}_cachedResult;
+    {%endif%}
         {%endif%}
 
     static NAN_METHOD({{ function.cppFunctionName }});

diff --git a/test/tests/commit.js b/test/tests/commit.js
@@ -404,6 +404,38 @@ describe("Commit", function() {
     assert.ok(owner instanceof Repository);
   });
 
+  it("caches its owner", function(done) {
+    var objects = {};
+
+    reinitialize(objects)
+      .then(function() {
+        var owner = objects.commit.owner();
+        var ownerAgain = objects.commit.owner();
+        assert.ok(owner === ownerAgain);
+        assert.ok(owner === objects.repository);
+
+        // make sure the owner gets freed at the correct time
+        garbageCollect();
+        var Repository = NodeGit.Repository;
+        var startCount = Repository.getSelfFreeingInstanceCount();
+        owner = ownerAgain = objects.repository = null;
+
+        setTimeout(function() {
+          garbageCollect();
+          // the commit should still hold the repository
+          assert.equal(startCount, Repository.getSelfFreeingInstanceCount());
+          objects.commit = null;
+          // without the setTimeout, not seeing a free
+          setTimeout(function() {
+            garbageCollect();
+            assert.equal(startCount - 1,
+              Repository.getSelfFreeingInstanceCount());
+            done();
+          }, 10);
+        }, 10);
+      });
+  });
+
   it("can walk its repository's history", function(done) {
     var historyCount = 0;
     var expectedHistoryCount = 364;

diff --git a/test/tests/submodule.js b/test/tests/submodule.js
@@ -134,7 +134,7 @@ describe("Submodule", function() {
         return reference.peel(NodeGit.Object.TYPE.COMMIT);
       })
       .then(function(commit) {
-        return submoduleRepo.createBranch("master", commit);
+        return submoduleRepo.createBranch("master", commit.id());
       })
       .then(function() {
         return submodule.addFinalize();
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,4 @@

		#include "../../include/functions/noop.h"

		void noop(const Nan::WeakCallbackInfo<void> &data) {}