Fix regression when binding dates with alternate JavaScript frameworks

Author: cjbj

lib/connection.js

@@ -23,6 +23,7 @@ const BaseDbObject = require('./dbObject.js');
 const EventEmitter = require('events');
 const QueryStream = require('./querystream.js');
 const nodbUtil = require('./util.js');
+const util = require('util');
 
 // fetchRowsToReturn is used to materialize the rows for an execute call using
 // the resultSet returned from the C layer.
@@ -425,6 +426,10 @@ class Connection extends EventEmitter {
     return cls;
   }
 
+  _isDate(val) {
+    return util.types.isDate(val);
+  }
+
   // To obtain a SodaDatabase object (high-level SODA object associated with
   // current connection)
   getSodaDatabase() {

src/njsBaton.c

@@ -799,7 +799,9 @@ bool njsBaton_isBindValue(njsBaton *baton, napi_env env, napi_value value)
         return true;
 
     // dates can be bound directly
-    if (njsBaton_isDate(baton, env, value))
+    if (!njsBaton_isDate(baton, env, value, &check))
+        return false;
+    if (check)
         return true;
 
     // LOBs can be bound directly
@@ -823,17 +825,18 @@ bool njsBaton_isBindValue(njsBaton *baton, napi_env env, napi_value value)
 
 //-----------------------------------------------------------------------------
 // njsBaton_isDate()
-//   Returns a boolean indicating if the value refers to a date or not.
+//   Returns a boolean indicating if the value refers to a date or not. This
+// can be replaced by napi_is_date() once it is available in all LTS releases.
 //-----------------------------------------------------------------------------
-bool njsBaton_isDate(njsBaton *baton, napi_env env, napi_value value)
+bool njsBaton_isDate(njsBaton *baton, napi_env env, napi_value value,
+        bool *isDate)
 {
-    napi_status status;
-    bool check;
+    napi_value isDateObj;
 
-    status = napi_instanceof(env, value, baton->jsDateConstructor, &check);
-    if (status != napi_ok)
-        return false;
-    return check;
+    NJS_CHECK_NAPI(env, napi_call_function(env, baton->jsConnection,
+            baton->jsIsDateMethod, 1, &value, &isDateObj))
+    NJS_CHECK_NAPI(env, napi_get_value_bool(env, isDateObj, isDate))
+    return true;
 }
 
 
@@ -912,10 +915,21 @@ void njsBaton_reportError(njsBaton *baton, napi_env env)
 // njsBaton_setConstructors()
 //   Sets the constructors on the baton.
 //-----------------------------------------------------------------------------
-bool njsBaton_setConstructors(njsBaton *baton, napi_env env)
+bool njsBaton_setConstructors(njsBaton *baton, napi_env env, bool canBind)
 {
     napi_value global;
 
+    // if binding is possible, get the connection (which is the calling object)
+    // and acquire the method used for determining if a value is a date; this
+    // is needed until such time as napi_is_date() is available for all LTS
+    // releases
+    if (canBind) {
+        NJS_CHECK_NAPI(env, napi_get_reference_value(env, baton->jsCallingObj,
+                &baton->jsConnection))
+        NJS_CHECK_NAPI(env, napi_get_named_property(env, baton->jsConnection,
+                "_isDate", &baton->jsIsDateMethod))
+    }
+
     // acquire the Date constructor
     NJS_CHECK_NAPI(env, napi_get_global(env, &global))
     NJS_CHECK_NAPI(env, napi_get_named_property(env, global, "Date",

src/njsConnection.c

@@ -592,7 +592,7 @@ static bool njsConnection_executePostAsync(njsBaton *baton, napi_env env,
     napi_value implicitResults;
 
     // create constructors used for various types that might be returned
-    if (!njsBaton_setConstructors(baton, env))
+    if (!njsBaton_setConstructors(baton, env, true))
         return false;
 
     // create result object
@@ -685,7 +685,7 @@ static bool njsConnection_executeProcessArgs(njsBaton *baton,
             (void**) &baton->fetchAsStringTypes,
             &baton->numFetchAsStringTypes))
         return false;
-    if (!njsBaton_setConstructors(baton, env))
+    if (!njsBaton_setConstructors(baton, env, true))
         return false;
 
     // get SQL from first argument
@@ -880,7 +880,7 @@ static bool njsConnection_executeManyProcessArgs(njsBaton *baton,
 {
     // setup defaults and define constructors for use in various checks
     baton->autoCommit = baton->oracleDb->autoCommit;
-    if (!njsBaton_setConstructors(baton, env))
+    if (!njsBaton_setConstructors(baton, env, true))
         return false;
 
     // get SQL from first argument
@@ -1069,7 +1069,9 @@ static bool njsConnection_getBindInfoFromValue(njsBaton *baton,
     if (valueType == napi_object) {
 
         // dates can be bound
-        if (njsBaton_isDate(baton, env, value)) {
+        if (!njsBaton_isDate(baton, env, value, &check))
+            return false;
+        if (check) {
             *bindType = NJS_DATATYPE_DATE;
             return true;
         }

src/njsModule.h

@@ -479,6 +479,13 @@ struct njsBaton {
     napi_ref jsCallback;
     napi_ref jsSubscription;
 
+    // values required to check if a value is a date; this is only used when
+    // binding data in connection.execute() and connection.executeMany();
+    // this can be replaced with calls to napi_is_date() when it is available
+    // for all LTS releases
+    napi_value jsConnection;
+    napi_value jsIsDateMethod;
+
     // constructors
     napi_value jsDateConstructor;
     napi_value jsLobConstructor;
@@ -795,13 +802,14 @@ bool njsBaton_getValueFromArg(njsBaton *baton, napi_env env, napi_value *args,
         int argIndex, const char *propertyName, napi_valuetype expectedType,
         napi_value *value, bool *found);
 bool njsBaton_isBindValue(njsBaton *baton, napi_env env, napi_value value);
-bool njsBaton_isDate(njsBaton *baton, napi_env env, napi_value value);
+bool njsBaton_isDate(njsBaton *baton, napi_env env, napi_value value,
+        bool *isDate);
 bool njsBaton_queueWork(njsBaton *baton, napi_env env, const char *methodName,
         bool (*workCallback)(njsBaton*),
         bool (*afterWorkCallback)(njsBaton*, napi_env, napi_value*),
         unsigned int numCallbackArgs);
 void njsBaton_reportError(njsBaton *baton, napi_env env);
-bool njsBaton_setConstructors(njsBaton *baton, napi_env env);
+bool njsBaton_setConstructors(njsBaton *baton, napi_env env, bool canBind);
 bool njsBaton_setError(njsBaton *baton, int errNum, ...);
 bool njsBaton_setErrorDPI(njsBaton *baton);
 

src/njsResultSet.c

@@ -245,7 +245,7 @@ static bool njsResultSet_getRowsPostAsync(njsBaton *baton, napi_env env,
     njsVariable *var;
 
     // create constructors used for various types that might be returned
-    if (!njsBaton_setConstructors(baton, env))
+    if (!njsBaton_setConstructors(baton, env, false))
         return false;
 
     // if outFormat is OBJECT, create names for each of the variables

src/njsVariable.c

@@ -941,8 +941,8 @@ bool njsVariable_setScalarValue(njsVariable *var, uint32_t pos, napi_env env,
     if (valueType == napi_object) {
 
         // handle binding dates
-        NJS_CHECK_NAPI(env, napi_instanceof(env, value,
-                baton->jsDateConstructor, &check))
+        if (!njsBaton_isDate(baton, env, value, &check))
+            return false;
         if (check) {
             if (var->varTypeNum != DPI_ORACLE_TYPE_TIMESTAMP_LTZ)
                 return njsVariable_setInvalidBind(var, pos, baton);