[asterisk-app-dev] Prototype voicemail application using ARI and Node.js

[Samuel Galarneau]

>
> I like the general approach, as I think it is more than flexible
> enough for our needs (and less bulky than a full ORM as well). I do
> think the SQLGenerator function could be broken up into separate
> functions. Having all of the tables in a single SQLGenerator feels
> like it mixes purposes slightly, particular when the prototype ends up
> having specific functions that only apply to a single table/data
> structure.
>
> Maybe something like:
>
> function VMContextGenerator(dialect) {
>     this.generator = new sql.Sql(dialect);
>     this.context = this.generator.define({
>         ...
>     });
> }
>
> I'm also curious why you went with the approach of having all of the
> functions defined on the prototype, instead of in the generator
> function (this may be just a general style question, as this technique
> is generally used throughout the library).
>

Having a function defined on the prototype makes it so all instances share
that function. Defining a function within a constructor creates a new copy
of the function for each instance. Since this application will be long
running with many copies of these objects, it's better to define functions
on the prototype rather than inside constructors.

I do agree that the SQLGenerator object does too much currently. It should
be broken up into generators for each table. We could then rename functions
such as getMailbox to get within a MailboxSqlGenerator object. Note that
this would involve duplicating some of the table definitions to make joins
work with the node-sql library responsible for generating the final SQL
text but we could look at defining the tables in a separate module to
remove this problem.


>
> On that same note, have you thought about using constructor
> functions/closures to build up objects with private data? I noticed
> that a lot of the data models use a ._ notation, which, unlike with
> Python, doesn't guarantee much (that I'm aware of).
>

Using _ as a prefix before a member is a convention only. It's typical of
many libraries. Having said that, some of the members I currently have
defined this way ended up being needed outside of the objects themselves
and should be redefined to show this. For members that are truly internal
to an object only, we could easily hide them using a closure, but then they
would only be accessible within a constructor. Since we are using prototype
functions, this is less than ideal. We could however add read only
functions in the constructor for these members and at least prohibit
writing to them (we could also define them as properties with a read only
accessor).


> EventEmitters: works for me. I think state transitions will occur
> beyond just DTMF key presses: for example, you could have someone
> press '#' to terminate recording a message. They can also just hang
> up.
>
> On that same note, some events that cause transitions in your finite
> state machine should have 'precedence'. For example, if I mash the
> keypad while traversing through VoiceMailMain menus and then hangup,
> you probably don't want to try and process all of those key presses.
>
> Having things be not just DTMF specific also lets you tie in other
> mechanisms to 'traverse' the VoiceMail applications, such as messages
> from some external source.
>

The idea is to define a state machine with known states and pre-defined
"events" that will cause a transition between those states. An event in
this case could be a DTMF event, another WebSocket event, or any external
application/library. The state machine exposes a handle function for
handling an event, it does not pre define where those events come from.


> The only problem I foresee with this approach is the explosion of
> 'ancillary' applications that can occur:
> * Authentication
> * Recording greetings
> * Changing which greeting is the default greeting
> * Putting your voicemail mailbox into 'away' mode
>
> Some of these are silly, but some aren't. Having a single WS
> connection for all of that has some drawbacks:
> * Complications dealing with state transitions (making sure we don't
> transition incorrectly due to thinking we are in the wrong
> application)
> * Potential limitations with scaling. For example, I may want one
> node.js instance to handle all VoiceMailMain apps, but Authentication
> to be handled by something other node.js instance (or even another
> server).
>

Currently the application spins up a Voicemail and VoicemailMain handler. I
like the idea of that being the default option, with CLI arguments for
spinning up a given one if an end user of the application would like to
split the handlers to 2 processes/servers/. Events will be tied to a given
channel that is either interacting with a Voicemail or VoicemailMain
handler, so thinking we are in the wrong application will not be a problem.
As for ancillary applications, my thought was to have those be Node.js
modules packaged with the application.


> A few other questions:
>
> - There's a fair amount of inheritance in use in the data models. As
> much of an OO-guy as I happen to be, I'm not sure it's warranted for
> the config objects (or that it's really even needed with a weakly
> (non?) typed language like JS). Why not go with a more duck-typed
> approach?
>

I agree with you here. Most of the data abstraction layer is overly
abstracted using base/derived objects and could be simplified to make the
application more easily digestible. I may have went overboard here and
there to try and reduce duplication. :)


> - The VoiceMailMain function is a little ... long. I'd break up that
> function into separate functions as soon as possible, and give some
> thought to how you want to be managed for the long run. (Yes,
> prototype - but in the 'real' one, this is where things will start to
> break down). Generally, I have a feeling that app_voicemail's
> vm_execmain wasn't originally the Stygian horror it has become either,
> but as Mark wrote:
>     /* XXX This is, admittedly, some pretty horrendous code.  For some
>        reason it just seemed a lot easier to do with GOTO's.  I feel
>        like I'm back in my GWBASIC days. XXX */
> This is probably the easiest place to fall back into that trap. What
> thoughts do you have on managing the complexity growth there?
>

Both the Voicemail and VoicemailMain menus were written quickly to be able
to test the message handling capabilities. They are not meant to reflect
where that portion of the application will go in the future.

I think the main thing we can do to reduce complexity is to work hard to
make use of small, reusable Node.js modules. These modules can be reused by
Voicemail and VoicemailMain as well as potentially taken out and used by
other applications. Not that a Node.js module does not necessarily mean an
NPM published module is this case.

>
> --

Samuel Fortier-Galarneau
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at:  www.digium.com  & www.asterisk.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-app-dev/attachments/20140728/bd696740/attachment.html>