diff --git a/generate/input/callbacks.json b/generate/input/callbacks.json index d18919768..eb6b128a3 100644 --- a/generate/input/callbacks.json +++ b/generate/input/callbacks.json @@ -96,7 +96,8 @@ "type": "int", "noResults": 1, "success": 0, - "error": -1 + "error": -1, + "throttle": 100 } }, "git_checkout_perfdata_cb": { @@ -207,7 +208,8 @@ "type": "int", "noResults": 1, "success": 0, - "error": -1 + "error": -1, + "throttle": 100 } }, "git_diff_hunk_cb": { @@ -560,7 +562,8 @@ "type": "int", "noResults":0, "success": 0, - "error": -1 + "error": -1, + "throttle": 100 } }, "git_stash_cb": { @@ -670,7 +673,8 @@ "type": "int", "noResults": 0, "success": 0, - "error": -1 + "error": -1, + "throttle": 100 } }, "git_transport_cb": { diff --git a/generate/templates/manual/include/async_baton.h b/generate/templates/manual/include/async_baton.h index a1ce5c380..fee87c4c1 100644 --- a/generate/templates/manual/include/async_baton.h +++ b/generate/templates/manual/include/async_baton.h @@ -4,6 +4,9 @@ #include #include +#include "lock_master.h" +#include "functions/sleep_for_ms.h" + // Base class for Batons used for callbacks (for example, // JS functions passed as callback parameters, // or field properties of configuration objects whose values are callbacks) @@ -13,4 +16,33 @@ struct AsyncBaton { bool done; }; +template +struct AsyncBatonWithResult : public AsyncBaton { + ResultT result; + ResultT defaultResult; // result returned if the callback doesn't return anything valid + + AsyncBatonWithResult(const ResultT &defaultResult) + : defaultResult(defaultResult) { + } + + ResultT ExecuteAsync(uv_async_cb asyncCallback) { + result = 0; + req.data = this; + done = false; + + uv_async_init(uv_default_loop(), &req, asyncCallback); + { + LockMaster::TemporaryUnlock temporaryUnlock; + + uv_async_send(&req); + + while(!done) { + sleep_for_ms(1); + } + } + + return result; + } +}; + #endif diff --git a/generate/templates/manual/include/callback_wrapper.h b/generate/templates/manual/include/callback_wrapper.h index 41552de3e..b23a7bb36 100644 --- a/generate/templates/manual/include/callback_wrapper.h +++ b/generate/templates/manual/include/callback_wrapper.h @@ -1,17 +1,60 @@ #ifndef CALLBACK_WRAPPER_H #define CALLBACK_WRAPPER_H -#include -#include - -#include "nan.h" +#include +#include using namespace v8; using namespace node; -struct CallbackWrapper { +class CallbackWrapper { Nan::Callback* jsCallback; - void * payload; + + // throttling data, used for callbacks that need to be throttled + int throttle; // in milliseconds - if > 0, calls to the JS callback will be throttled + uint64_t lastCallTime; + +public: + CallbackWrapper() { + jsCallback = NULL; + lastCallTime = 0; + throttle = 0; + } + + ~CallbackWrapper() { + SetCallback(NULL); + } + + bool HasCallback() { + return jsCallback != NULL; + } + + Nan::Callback* GetCallback() { + return jsCallback; + } + + void SetCallback(Nan::Callback* callback, int throttle = 0) { + if(jsCallback) { + delete jsCallback; + } + jsCallback = callback; + this->throttle = throttle; + } + + bool WillBeThrottled() { + if(!throttle) { + return false; + } + // throttle if needed + uint64_t now = uv_hrtime(); + if(lastCallTime > 0 && now < lastCallTime + throttle * 1000000) { + // throttled + return true; + } else { + lastCallTime = now; + return false; + } + } }; #endif diff --git a/generate/templates/manual/include/lock_master.h b/generate/templates/manual/include/lock_master.h index fd1a09b6f..7fd4ee8dc 100644 --- a/generate/templates/manual/include/lock_master.h +++ b/generate/templates/manual/include/lock_master.h @@ -1,6 +1,8 @@ #ifndef LOCK_MASTER_H #define LOCK_MASTER_H +#include + class LockMasterImpl; class LockMaster { diff --git a/generate/templates/partials/callback_helpers.cc b/generate/templates/partials/callback_helpers.cc index 0acd215ae..b776c0097 100644 --- a/generate/templates/partials/callback_helpers.cc +++ b/generate/templates/partials/callback_helpers.cc @@ -6,28 +6,14 @@ {{ arg.cType }} {{ arg.name}}{% if not arg.lastArg %},{% endif %} {% endeach %} ) { - {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton* baton = new {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton(); + {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton* baton = + new {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton({{ cbFunction.return.noResults }}); {% each cbFunction.args|argsInfo as arg %} baton->{{ arg.name }} = {{ arg.name }}; {% endeach %} - baton->result = 0; - baton->req.data = baton; - baton->done = false; - - uv_async_init(uv_default_loop(), &baton->req, (uv_async_cb) {{ cppFunctionName }}_{{ cbFunction.name }}_async); - { - LockMaster::TemporaryUnlock temporaryUnlock; - - uv_async_send(&baton->req); - - while(!baton->done) { - sleep_for_ms(1); - } - } - - return baton->result; + return baton->ExecuteAsync((uv_async_cb) {{ cppFunctionName }}_{{ cbFunction.name }}_async); } void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(uv_async_t* req, int status) { @@ -93,12 +79,12 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(uv_as baton->result = (int)result->ToNumber()->Value(); } else { - baton->result = {{ cbFunction.return.noResults }}; + baton->result = baton->defaultResult; } {% endif %} } else { - baton->result = {{ cbFunction.return.noResults }}; + baton->result = baton->defaultResult; } {% endeach %} @@ -127,12 +113,12 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_promiseComp baton->result = (int)result->ToNumber()->Value(); } else { - baton->result = {{ cbFunction.return.noResults }}; + baton->result = baton->defaultResult; } {% endif %} } else { - baton->result = {{ cbFunction.return.noResults }}; + baton->result = baton->defaultResult; } {% endeach %} } diff --git a/generate/templates/partials/field_accessors.cc b/generate/templates/partials/field_accessors.cc index 17b338dc3..6a3123095 100644 --- a/generate/templates/partials/field_accessors.cc +++ b/generate/templates/partials/field_accessors.cc @@ -11,8 +11,8 @@ info.GetReturnValue().Set(Nan::New(wrapper->{{ field.name }})); {% elsif field.isCallbackFunction %} - if (wrapper->{{field.name}} != NULL) { - info.GetReturnValue().Set(wrapper->{{ field.name }}->GetFunction()); + if (wrapper->{{field.name}}.HasCallback()) { + info.GetReturnValue().Set(wrapper->{{ field.name }}.GetCallback()->GetFunction()); } else { info.GetReturnValue().SetUndefined(); } @@ -31,6 +31,7 @@ } NAN_SETTER({{ cppClassName }}::Set{{ field.cppFunctionName }}) { + Nan::HandleScope scope; {{ cppClassName }} *wrapper = Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This()); @@ -47,16 +48,35 @@ wrapper->raw->{{ field.name }} = {% if not field.cType | isPointer %}*{% endif %}{% if field.cppClassName == 'GitStrarray' %}StrArrayConverter::Convert({{ field.name }}->ToObject()){% else %}Nan::ObjectWrap::Unwrap<{{ field.cppClassName }}>({{ field.name }}->ToObject())->GetValue(){% endif %}; {% elsif field.isCallbackFunction %} - if (wrapper->{{ field.name }} != NULL) { - delete wrapper->{{ field.name }}; - } + Nan::Callback *callback = NULL; + int throttle = {%if field.return.throttle %}{{ field.return.throttle }}{%else%}0{%endif%}; if (value->IsFunction()) { + callback = new Nan::Callback(value.As()); + } else if (value->IsObject()) { + Local object = value.As(); + Local callbackKey; + Nan::MaybeLocal maybeObjectCallback = Nan::Get(object, Nan::New("callback").ToLocalChecked()); + if (!maybeObjectCallback.IsEmpty()) { + Local objectCallback = maybeObjectCallback.ToLocalChecked(); + if (objectCallback->IsFunction()) { + callback = new Nan::Callback(objectCallback.As()); + Nan::MaybeLocal maybeObjectThrottle = Nan::Get(object, Nan::New("throttle").ToLocalChecked()); + if(!maybeObjectThrottle.IsEmpty()) { + Local objectThrottle = maybeObjectThrottle.ToLocalChecked(); + if (objectThrottle->IsNumber()) { + throttle = (int)objectThrottle.As()->Value(); + } + } + } + } + } + if (callback) { if (!wrapper->raw->{{ field.name }}) { wrapper->raw->{{ field.name }} = ({{ field.cType }}){{ field.name }}_cppCallback; } - wrapper->{{ field.name }} = new Nan::Callback(value.As()); + wrapper->{{ field.name }}.SetCallback(callback, throttle); } {% elsif field.payloadFor %} @@ -82,46 +102,42 @@ } {% if field.isCallbackFunction %} + {{ cppClassName }}* {{ cppClassName }}::{{ field.name }}_getInstanceFromBaton({{ field.name|titleCase }}Baton* baton) { + return static_cast<{{ cppClassName }}*>(baton->{% each field.args|argsInfo as arg %} + {% if arg.payload == true %}{{arg.name}}{% elsif arg.lastArg %}{{arg.name}}{% endif %} + {% endeach %}); + } + {{ field.return.type }} {{ cppClassName }}::{{ field.name }}_cppCallback ( {% each field.args|argsInfo as arg %} {{ arg.cType }} {{ arg.name}}{% if not arg.lastArg %},{% endif %} {% endeach %} ) { - {{ field.name|titleCase }}Baton* baton = new {{ field.name|titleCase }}Baton(); + {{ field.name|titleCase }}Baton* baton = + new {{ field.name|titleCase }}Baton({{ field.return.noResults }}); {% each field.args|argsInfo as arg %} baton->{{ arg.name }} = {{ arg.name }}; {% endeach %} - baton->result = 0; - baton->req.data = baton; - baton->done = false; - - uv_async_init(uv_default_loop(), &baton->req, (uv_async_cb) {{ field.name }}_async); - { - LockMaster::TemporaryUnlock temporaryUnlock; - - uv_async_send(&baton->req); + {{ cppClassName }}* instance = {{ field.name }}_getInstanceFromBaton(baton); - while(!baton->done) { - sleep_for_ms(1); - } + if (instance->{{ field.name }}.WillBeThrottled()) { + return baton->defaultResult; } - return baton->result; + return baton->ExecuteAsync((uv_async_cb) {{ field.name }}_async); } void {{ cppClassName }}::{{ field.name }}_async(uv_async_t* req, int status) { Nan::HandleScope scope; {{ field.name|titleCase }}Baton* baton = static_cast<{{ field.name|titleCase }}Baton*>(req->data); - {{ cppClassName }}* instance = static_cast<{{ cppClassName }}*>(baton->{% each field.args|argsInfo as arg %} - {% if arg.payload == true %}{{arg.name}}{% elsif arg.lastArg %}{{arg.name}}{% endif %} - {% endeach %}); + {{ cppClassName }}* instance = {{ field.name }}_getInstanceFromBaton(baton); - if (instance->{{ field.name }}->IsEmpty()) { + if (instance->{{ field.name }}.GetCallback()->IsEmpty()) { {% if field.return.type == "int" %} - baton->result = {{ field.return.noResults }}; // no results acquired + baton->result = baton->defaultResult; // no results acquired {% endif %} baton->done = true; @@ -163,7 +179,7 @@ }; Nan::TryCatch tryCatch; - Local result = instance->{{ field.name }}->Call({{ field.args|jsArgsCount }}, argv); + Local result = instance->{{ field.name }}.GetCallback()->Call({{ field.args|jsArgsCount }}, argv); uv_close((uv_handle_t*) &baton->req, NULL); @@ -187,12 +203,12 @@ baton->result = (int)result->ToNumber()->Value(); } else { - baton->result = {{ field.return.noResults }}; + baton->result = baton->defaultResult; } {% endif %} } else { - baton->result = {{ field.return.noResults }}; + baton->result = baton->defaultResult; } {% endeach %} baton->done = true; @@ -220,12 +236,12 @@ baton->result = (int)result->ToNumber()->Value(); } else{ - baton->result = {{ field.return.noResults }}; + baton->result = baton->defaultResult; } {% endif %} } else { - baton->result = {{ field.return.noResults }}; + baton->result = baton->defaultResult; } {% endeach %} } diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index c172e1fa5..8fead5aa8 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -11,7 +11,6 @@ extern "C" { #include "../include/lock_master.h" #include "../include/functions/copy.h" #include "../include/{{ filename }}.h" -#include "../include/functions/sleep_for_ms.h" {% each dependencies as dependency %} #include "{{ dependency }}" diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index 84c9b5c4f..b18defd0e 100644 --- a/generate/templates/templates/class_header.h +++ b/generate/templates/templates/class_header.h @@ -64,12 +64,14 @@ class {{ cppClassName }} : public Nan::ObjectWrap { static void {{ function.cppFunctionName }}_{{ arg.name }}_async(uv_async_t* req, int status); static void {{ function.cppFunctionName }}_{{ arg.name }}_promiseCompleted(bool isFulfilled, AsyncBaton *_baton, v8::Local result); - struct {{ function.cppFunctionName }}_{{ arg.name|titleCase }}Baton : AsyncBaton { + struct {{ function.cppFunctionName }}_{{ arg.name|titleCase }}Baton : public AsyncBatonWithResult<{{ arg.return.type }}> { {% each arg.args|argsInfo as cbArg %} {{ cbArg.cType }} {{ cbArg.name }}; {% endeach %} - {{ arg.return.type }} result; + {{ function.cppFunctionName }}_{{ arg.name|titleCase }}Baton(const {{ arg.return.type }} &defaultResult) + : AsyncBatonWithResult<{{ arg.return.type }}>(defaultResult) { + } }; {% endif %} {% endeach %} diff --git a/generate/templates/templates/struct_content.cc b/generate/templates/templates/struct_content.cc index cab524a80..b54af5510 100644 --- a/generate/templates/templates/struct_content.cc +++ b/generate/templates/templates/struct_content.cc @@ -17,7 +17,6 @@ extern "C" { #include "../include/lock_master.h" #include "../include/functions/copy.h" #include "../include/{{ filename }}.h" -#include "../include/functions/sleep_for_ms.h" {% each dependencies as dependency %} #include "{{ dependency }}" @@ -53,10 +52,7 @@ using namespace std; {% if not field.ignore %} {% if not field.isEnum %} {% if field.isCallbackFunction %} - if (this->{{ field.name }} != NULL) { - delete this->{{ field.name }}; this->raw->{{ fields|payloadFor field.name }} = NULL; - } {% endif %} {% endif %} {% endif %} @@ -84,7 +80,6 @@ void {{ cppClassName }}::ConstructFields() { // the current instance this->raw->{{ field.name }} = NULL; this->raw->{{ fields|payloadFor field.name }} = (void *)this; - this->{{ field.name }} = NULL; {% elsif field.payloadFor %} Local {{ field.name }} = Nan::Undefined(); diff --git a/generate/templates/templates/struct_header.h b/generate/templates/templates/struct_header.h index 0320fbbf6..87720aa4a 100644 --- a/generate/templates/templates/struct_header.h +++ b/generate/templates/templates/struct_header.h @@ -6,6 +6,7 @@ #include #include "async_baton.h" +#include "callback_wrapper.h" extern "C" { #include @@ -48,13 +49,17 @@ class {{ cppClassName }} : public Nan::ObjectWrap { static void {{ field.name }}_async(uv_async_t* req, int status); static void {{ field.name }}_promiseCompleted(bool isFulfilled, AsyncBaton *_baton, v8::Local result); - struct {{ field.name|titleCase }}Baton : public AsyncBaton { + struct {{ field.name|titleCase }}Baton : public AsyncBatonWithResult<{{ field.return.type }}> { {% each field.args|argsInfo as arg %} {{ arg.cType }} {{ arg.name}}; {% endeach %} - {{ field.return.type }} result; + {{ field.name|titleCase }}Baton(const {{ field.return.type }} &defaultResult) + : AsyncBatonWithResult<{{ field.return.type }}>(defaultResult) { + } }; + static {{ cppClassName }} * {{ field.name }}_getInstanceFromBaton ( + {{ field.name|titleCase }}Baton *baton); {% endif %} {% endif %} {% endeach %} @@ -73,7 +78,7 @@ class {{ cppClassName }} : public Nan::ObjectWrap { {% if field.isLibgitType %} Nan::Persistent {{ field.name }}; {% elsif field.isCallbackFunction %} - Nan::Callback* {{ field.name }}; + CallbackWrapper {{ field.name }}; {% elsif field.payloadFor %} Nan::Persistent {{ field.name }}; {% endif %} diff --git a/test/tests/clone.js b/test/tests/clone.js index 46830a889..3e2a86446 100644 --- a/test/tests/clone.js +++ b/test/tests/clone.js @@ -3,6 +3,7 @@ var assert = require("assert"); var promisify = require("promisify-node"); var fse = promisify(require("fs-extra")); var local = path.join.bind(path, __dirname); +var _ = require("lodash"); describe("Clone", function() { var NodeGit = require("../../"); @@ -37,6 +38,114 @@ describe("Clone", function() { }); }); + it("can clone twice with http using same config object", function() { + var test = this; + var url = "http://git.tbranyen.com/smart/site-content"; + var progressCount = 0; + var opts = { + fetchOpts: { + callbacks: { + transferProgress: function(progress) { + progressCount++; + } + } + } + }; + + return Clone(url, clonePath, opts) + .then(function(repo) { + assert.ok(repo instanceof Repository); + assert.notEqual(progressCount, 0); + return fse.remove(clonePath); + }) + .then(function() { + progressCount = 0; + return Clone(url, clonePath, opts); + }) + .then(function(repo) { + assert.ok(repo instanceof Repository); + assert.notEqual(progressCount, 0); + test.repository = repo; + }); + }); + + function updateProgressIntervals(progressIntervals, lastInvocation) { + var now = new Date(); + if (lastInvocation) { + progressIntervals.push(now - lastInvocation); + } + return now; + } + + it("can clone with http and default throttled progress", function() { + var test = this; + var url = "http://git.tbranyen.com/smart/site-content"; + var progressCount = 0; + var lastInvocation; + var progressIntervals = []; + var opts = { + fetchOpts: { + callbacks: { + transferProgress: function(progress) { + lastInvocation = updateProgressIntervals(progressIntervals); + progressCount++; + } + } + } + }; + + return Clone(url, clonePath, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + assert.notEqual(progressCount, 0); + var averageProgressInterval = _.sum(progressIntervals) / + progressIntervals.length; + // even though we are specifying a throttle period of 100, + // the throttle is applied on the scheduling side, + // and actual execution is at the mercy of the main js thread + // so the actual throttle intervals could be less than the specified + // throttle period + if (averageProgressInterval < 75) { + assert.fail(averageProgressInterval, 75, + "unexpected average time between callbacks", "<"); + } + test.repository = repo; + }); + }); + + it("can clone with http and explicitly throttled progress", function() { + var test = this; + var url = "http://git.tbranyen.com/smart/site-content"; + var progressCount = 0; + var lastInvocation; + var progressIntervals = []; + var opts = { + fetchOpts: { + callbacks: { + transferProgress: { + throttle: 50, + callback: function(progress) { + lastInvocation = updateProgressIntervals(progressIntervals, + lastInvocation); + progressCount++; + } + } + } + } + }; + + return Clone(url, clonePath, opts).then(function(repo) { + assert.ok(repo instanceof Repository); + assert.notEqual(progressCount, 0); + var averageProgressInterval = _.sum(progressIntervals) / + progressIntervals.length; + if (averageProgressInterval < 35) { + assert.fail(averageProgressInterval, 35, + "unexpected average time between callbacks", "<"); + } + test.repository = repo; + }); + }); + it("can clone with https", function() { var test = this; var url = "https://github.com/nodegit/test.git";