expose ModelConstructor

[pocesar]

Can the ModelConstructor (inside AbstractClass) be exposed, so I can monkey patch it? I want to create a promise based approach to jugglingdb instead of using callbacks on all prototype functions

[1602]

When you define class, new ModelConstructor instance created (with no common ancestor class), so it's not possible to monkey patch it. Why do you need it? I would be happy to help you achieve your goal (promise based approach), and I don't think that monkey-patching is good way to do it as it adds some overhead. User should not pay in performance for syntactic sugar. I think better to implement it in core.

[pocesar]

well, right now there are two problems that I'm facing:

• I'm using Q library to create the promises, and changing all AbstractClass functions and prototypes to promises, which is, in part, working, but fails when stillConnecting is used (because of deprecated use of arguments.callee)
• I'm creating a "dual version" of the jugglingdb API, it can be called both ways, using callbacks OR using promises, with the same function (it's a wrapper), so I thought it would be compatible with both usages (some people would still prefer using callbacks). This is the wrapper it creates:

  wrapConditionalPromise: (context, funcName, isReturn = false) ->
    return if typeof context[funcName] isnt 'function'

    original = context[funcName]

    defined = original.length # number of arguments, last one is usually the callback
    Q = @$.Q

    if isReturn is true
      context[funcName] = ->
        d = Q.defer()

        try
          d.resolve(original.apply(context, arguments))
        catch e
          d.reject(e)

        d.promise

      return
    else if defined is 0
      return

    # eg: if theres 3 args, and the callback is passed, don't return a promise, but execute the callback
    context[funcName] = ->
      args = Array.prototype.slice.call(arguments)

      if args.length is defined
        # callback is included, act like the original call
        original.apply(context, args)
      else
        # return the promise this time
        d = Q.defer()
        args.push(d.makeNodeResolver())

        try
          original.apply(context, args)
        catch e
          d.reject(e)

        d.promise

so I'm looping through AbstractClass and overwriting the original functions with this wrapper (the performance hit is negligible)

the problem is, since most of the functions in jugglingdb are bound to this keyword, it fails in many situations, even though I'm trying to keep the context 'sane' (by currying the original functions). I'd love to see a Q promise version of Jugglingdb

[1602]

It's not possible to save context in AbstractClass, as it's only defined in particular class. If you do it for every model I think it will work.

[pocesar]

ok I'll try to modify the resulting class constructor that schema.define gives instead of pre-patching the AbstractClass, bbiab

[pocesar]

doesn't seem to work, it works for the 'non-prototype', like User.find or User.all, but when used User.create().save() (that is, create a instance that uses the prototype), the getters and setters stop working.

    @model = schema.define(name, definition)

    # monkey patch jugglingdb to use promises
    nproto = [
      'update','create','upsert','updateOrCreate',
      'findOrCreate','exists','find','all','findOne',
      'destroyAll','count'
    ]

    @$.Utils.wrapConditionalPromise(@model, i) for i in nproto

    proto = [
      'save','destroy','updateAttribute','updateAttributes','reload'
    ]

    @$.Utils.wrapConditionalPromise(@model::, i) for i in proto

User.create({name: 'Steph'}).save().done(function(){}) is giving me:

Uncaught TypeError: Cannot read property 'name' of undefined
    at ModelConstructor.Object.defineProperty.get [as name] (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\schema.js:290:39)
    at C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:748:17
    at Array.forEach (native)
    at Function.NewClass.forEachProperty (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\schema.js:263:33)
    at ModelConstructor.AbstractClass.toObject (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:747:22)
    at ModelConstructor.AbstractClass.save (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:670:21)
    at ModelConstructor.context.(anonymous function) [as save] (C:\Inetpub\socketexpress\lib\system\classes\utils.js:130:20)

[1602]

I guess that's because of using @model:: as a context, context here is an actual object and not prototype, and you calling instance methods on prototype rather than on object, it should be a different wrapper for instance methods.

[pocesar]

I tried both with no success, when I use @model as the context (in apply), the error:

    Uncaught TypeError: Cannot read property 'connected' of undefined
    at stillConnecting (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:337:15)
    at Function.AbstractClass.save (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:652:9)
    at ModelConstructor.context.(anonymous function) [as save] (C:\Inetpub\socketexpress\lib\system\classes\utils.js:130:20)

[1602]

@model is wrong as context too. context is object here, not constructor and not prototype of constructor.

[1602]

But i don't think it's possible in case of your implementation of wrapConditionalPromise

[pocesar]

I've changed it (forgot to tell):

  wrapConditionalPromise: (context, funcName, isReturn = false, baseContext = null) ->
    return if typeof context[funcName] isnt 'function'

    original = context[funcName]

    defined = original.length # number of arguments, last one is usually the callback
    Q = @$.Q

    if isReturn is true
      context[funcName] = ->
        d = Q.defer()

        try
          d.resolve(original.apply(context, arguments))
        catch e
          d.reject(e)

        d.promise

      return
    else if defined is 0
      return

    # eg: if theres 3 args, and the callback is passed, don't return a promise, but execute the callback
    context[funcName] = ->
      args = Array.prototype.slice.call(arguments)

      if args.length is defined
        # callback is included, act like the original call
        original.apply(baseContext || context, args)
      else
        # return the promise this time
        d = Q.defer()
        args.push(d.makeNodeResolver())

        try
          original.apply(baseContext || context, args)
        catch e
          d.reject(e)

        d.promise

then the prototype wrapper:

    proto = [
      'save','destroy','updateAttribute',
      'updateAttributes','reload'
    ]

    @$.Utils.wrapConditionalPromise(@model::, i, false, @model) for i in proto

should the baseContext be AbstractClass? or ModelConstructor?

[1602]

If you push your code with failing test case somewhere in github I will take a look at it in the evening.

[pocesar]

here is the failing tests branch https://github.com/pocesar/node-socketexpress/tree/jugglingdb

[pocesar]

@1602 the failing tests are: https://github.com/pocesar/node-socketexpress/blob/jugglingdb/test/model.spec.coffee#L93 https://github.com/pocesar/node-socketexpress/blob/jugglingdb/test/model.spec.coffee#L167

usually the problem happens after creating a model, and calling save(), the this context is lost (becomes undefined).

Monkey patch starts here https://github.com/pocesar/node-socketexpress/blob/jugglingdb/src/system/classes/model.coffee#L84

[anatoliychakkaev]

I mean something isolated :) just to get an issue related to jugglingdb. i think it's easy to extract, much easier than install this project.

On 11 February 2014 20:03, Paulo Cesar notifications@github.com wrote:

@1602 https://github.com/1602 the failing tests are https://github.com/pocesar/node-socketexpress/blob/jugglingdb/test/model.spec.coffee#L93and https://github.com/pocesar/node-socketexpress/blob/jugglingdb/test/model.spec.coffee#L167usually the problem happens after creating a model, and calling save(), the this context is lost (becomes undefined). Monkey patch starts here https://github.com/pocesar/node-socketexpress/blob/jugglingdb/src/system/classes/model.coffee#L84

Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/375#issuecomment-34799377 .

[pocesar]

Ok, basically, you can do this:

jugglingdb = require('jugglingdb')
Q = require('q')

wrapConditionalPromise = (context, funcName, isReturn = false, baseContext = null) ->
    return if typeof context[funcName] isnt 'function'

    original = context[funcName]

    defined = original.length # number of arguments, last one is usually the callback

    if isReturn is true
      context[funcName] = ->
        d = Q.defer()

        try
          d.resolve(original.apply(context, arguments))
        catch e
          d.reject(e)

        d.promise

      return
    else if defined is 0
      return

    # eg: if theres 3 args, and the callback is passed, don't return a promise, but execute the callback
    context[funcName] = ->
      args = Array.prototype.slice.call(arguments)

      if args.length is defined
        # callback is included, act like the original call
        original.apply(baseContext || context, args)
      else
        # return the promise this time
        d = Q.defer()
        args.push(d.makeNodeResolver())

        try
          original.apply(baseContext || context, args)
        catch e
          d.reject(e)

        d.promise

schema = new jugglingdb.Schema('memory')
User = schema.define('User', {
  name: String
  active: Boolean
})

nproto = [
      'update','create','upsert','updateOrCreate',
      'findOrCreate','exists','find','all','findOne',
      'destroyAll','count'
]

wrapConditionalPromise(User, i) for i in nproto

proto = [
  'save','destroy','updateAttribute',
  'updateAttributes','reload'
]

wrapConditionalPromise(User::, i) for i in proto

User.create({name: 'test', active: true}).done((user)->
  user.save() # should throw TypeError in here
)

gives

C:\Inetpub\socketexpress>coffee test.coffee

C:\Inetpub\socketexpress\node_modules\q\q.js:126
                    throw e;
                          ^
TypeError: Cannot read property 'name' of undefined
  at ModelConstructor.Object.defineProperty.get [as name] (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\schema.js:290:39)
  at C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:748:17
  at Array.forEach (native)
  at Function.NewClass.forEachProperty (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\schema.js:263:33)
  at ModelConstructor.AbstractClass.toObject (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:747:22)
  at ModelConstructor.AbstractClass.save (C:\Inetpub\socketexpress\node_modules\jugglingdb\lib\model.js:670:21)
  at ModelConstructor.context.(anonymous function) [as save] (C:\Inetpub\socketexpress\test.coffee:39:20)
  at C:\Inetpub\socketexpress\test.coffee:67:8
  at _fulfilled (C:\Inetpub\socketexpress\node_modules\q\q.js:797:54)
  at self.promiseDispatch.done (C:\Inetpub\socketexpress\node_modules\q\q.js:826:30)
  at Promise.promise.promiseDispatch (C:\Inetpub\socketexpress\node_modules\q\q.js:759:13)
  at C:\Inetpub\socketexpress\node_modules\q\q.js:573:44
  at flush (C:\Inetpub\socketexpress\node_modules\q\q.js:108:17)
  at process._tickCallback (node.js:415:13)
  at Function.Module.runMain (module.js:499:11)
  at startup (node.js:119:16)
  at node.js:901:3

[1602]

As I said, problem in wrapConditionalPromise(User::, i) for i in proto particularly in User::

I changed it to

User.afterInitialize = ->
  wrapConditionalPromise(@, i) for i in proto

to get instance methods working. Here's full listing:

jugglingdb = require('jugglingdb')
Q = require('q')

wrapConditionalPromise = (context, funcName, isReturn = false, baseContext = null) ->
    return if typeof context[funcName] isnt 'function'

    original = context[funcName]

    defined = original.length # number of arguments, last one is usually the callback

    if isReturn is true
      context[funcName] = ->
        d = Q.defer()

        try
          d.resolve(original.apply(context, arguments))
        catch e
          d.reject(e)

        d.promise

      return
    else if defined is 0
      return

    # eg: if theres 3 args, and the callback is passed, don't return a promise, but execute the callback
    context[funcName] = ->
      args = Array.prototype.slice.call(arguments)

      if args.length is defined
        # callback is included, act like the original call
        original.apply(baseContext || context, args)
      else
        # return the promise this time
        d = Q.defer()
        args.push(d.makeNodeResolver())

        try
          original.apply(baseContext || context, args)
        catch e
          d.reject(e)

        d.promise

schema = new jugglingdb.Schema('memory')
User = schema.define('User', {
  name: String
  active: Boolean
})

nproto = [
      'update','create','upsert','updateOrCreate',
      'findOrCreate','exists','find','all','findOne',
      'destroyAll','count'
]

wrapConditionalPromise(User, i) for i in nproto

proto = [
  'save','destroy','updateAttribute',
  'updateAttributes','reload'
]

User.afterInitialize = ->
  wrapConditionalPromise(@, i) for i in proto

User.create({name: 'test', active: true}).done((user)->
  user.save() # should throw TypeError in here
)

[pocesar]

it works, but now that's an overhead... because everytime I get a request, I need to wrap the same functions over and over again. that's the reason why I asked for the modelconstructor to be exposed, so I could patch the prototype, doing it ONCE when schema.define was called... isn't a better way?

[anatoliychakkaev]

model constructor is exposed, but you can not bind something to instance when this instance is not exists. what you want to do is to bind instance methods to it's instance so that you can do something like:

save = obj.save; save();

and it will be the same as

obj.save()

and this is not possible.

On 12 February 2014 14:08, Paulo Cesar notifications@github.com wrote:

it works, but now that's an overhead... because everytime I get a request, I need to wrap the same functions over and over again. that's the reason why I asked for the modelconstructor to be exposed, so I could patch the prototype, doing it ONCE when schema.define was called... isn't a better way?

Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/375#issuecomment-34871425 .

Thanks, Anatoliy Chakkaev

[pocesar]

would you accept a pull request to make jugglingdb work naturally with promises? if so, what would be preferred?

1. Create a global switch to use promises
2. Create a wrapper that allows you to use both promises and callbacks like i'm doing in my framework (so it's backward compatible)
3. Create a separate branch that doesn't use callbacks at all, and use promises everywhere

[anatoliychakkaev]

I think it should be done as an addon, as it adds some overhead, we are not using promises anyway, so better to split code and responsibility to support it.

On 12 February 2014 14:23, Paulo Cesar notifications@github.com wrote:

would you accept a pull request to make jugglingdb work naturally with promises? if so, what would be preferred?

1. Create a global switch to use promises
2. Create a wrapper that allows you to use both promises and callbacks like i'm doing in my framework (so it's backward compatible)
3. Create a separate branch that doesn't use callbacks at all, and use promises everywhere

Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/375#issuecomment-34872795 .

Thanks, Anatoliy Chakkaev

[pocesar]

ok, I'll create a promised-jugglingdb, and to counter the overhead, I'll remove all the function binds as well, sounds good?

[anatoliychakkaev]

Yep.

On Wed, Feb 12, 2014 at 5:30 PM, Paulo Cesar notifications@github.com wrote:

ok, I'll create a promised-jugglingdb, and to counter the overhead, I'll remove all the function binds as well, sounds good?

Reply to this email directly or view it on GitHub: https://github.com/1602/jugglingdb/issues/375#issuecomment-34893335