[asterisk-dev] PJSIP CLI and AMI organization thoughts

[George Joseph]

On Fri, Dec 6, 2013 at 4:52 PM, Kevin Harwell <kharwell at digium.com> wrote:

> On Fri, 2013-12-06 at 14:21 -0700, George Joseph wrote:
> > This continues a conversation kharwell, mjordan and I were having on
> > irc this morning concerning the implementation of the cli output
> > formatting.
> >
> >
> > Basically, I'm thinking that while we should absolutely re-use the
> > underlying plumbing ( we don't need 2 versions of
> > ast_sip_for_each_contact, for instance), the CLI and AMI are different
> > enough that we shouldn't try to re-use the formatter registration and
> > execution plumbing.  The same pattern maybe, but not share the
> > structure definitions, list of formatters, etc.
> >
> >
> > The pjsip/sorcery implementation is obviously a relational/oo model.
> >  Endpoint, Aor, Auth, etc are all first class objects with foreign
> > keys/references to each other.  Typically an API in this environment
> > doesn't de-normalize/de-reference/re-hydrate the references to other
> > objects.  In REST terms, if you do a GET on an endpoint, the response
> > may contain a collection of aor ids but you shouldn't be getting the
> > detail for each aor in an endpoint response.  If you want the details,
> > you do a GET on each aor.   A CLI implementation is much different.
> >  If a human, particularly this human, has to do a 'pjsip show
> > endpoint', write down the list of aors, then do a 'pjsip show aor' on
> > each, the response from the human would be quite vivid.  Also, aors
> > being first-class objects in their own right can be listed and details
> > shown independently of any other object.
> >
> >
> > So, here's what I'm suggesting...
> > Two formatter registries, one for CLI and one for AMI.  I haven't been
> > following the ARI discussions at all so maybe there's a third?
> > Anyway, this way the CLI or AMI command processors don't have to
> > filter out the ones they don't need and we don't unnecessarily lock
> > the CLI and AMI implementations together.  Each object implementation
> > would register a formatter for each mechanism which would take care of
> > anything specific to that mechanism.  For the CLI for instance, the
> > formatter would provide a human consumable summary or detail depending
> > on a flag passed to it.
>
> I think the models (summary/detail/some other view) are the same in all
> scenarios, but we are just approaching it from two different
> implementation perspectives.  The way the code is currently structured
> removes the need for the flag, so there is no need for an if/switch
> statement.
>

For the CLI, the only difference between summary and detail is that the
detail flag causes the sorcery name/value pairs to be dumped in a tabular
format.

>
> > I already have a context object that maintains formatting flags,
> > summary statistics, etc. that would be much different for AMI or ARI.
>
> Yes, the output between all could be quite different.  For each object
> type there are a set of formatters, serializers, printers, or whatever
> we want to call them (for instance, to_string, to_json, to_xml, etc...).
>
> Typically when you want to print out a non-primitive property then it
> makes sense to call its formatter.  However, in our case we have several
> different views depending on the output type, so we have to have custom
> formatters for each view.
>
> Currently we can register these custom formatters with the master object
> and when it gets formatted the associated object's custom formatter gets
> called (no need for a flag specifying the formatting view).
>
> Although now that I think about it having that flag would allow you to
> register multiple custom views for a [sub]type.  That may actually be
> needed in some cases some I would be fine with adding that in.
>
> As far as breaking it out into separate lists (endpoints_to_json list,
> endpoints_to_cli list, aors_to_json, etc...) I don't see a need for it.
> It will add multiple/extra register functions and more looping when
> formatting/outputting.  For instance, currently you register the
> [sub]type format function with a particular object type.  If I am
> thinking about it correctly with separate lists you would essentially
> register the list with an object type and loop over the lists of lists
> to output the [sub]type custom view.
>
> Not suggesting separate lists for each object type.  What I'm really
suggesting is a hashtable for each of the output mechanisms.  One for CLI,
one for AMI, etc.  Each object (endpoint, aor, auth, etc) registers a
single cli formatter, single ami formatter, etc.  The key for the hastables
is the sorcery object type.  So, for example, if the CLI command processor
had an aor object it would do a simple table lookup in the CLI formatter
registry for "aor" to get the aor formatter.  In fact, it doesn't even have
to know it's an aor because it can just lookup by details->object->type.

>
> > As a side note...  most of the cli stuff is in
> > pjsip_configuration..c.  I'm thinking of moving the common cli stuff
> > into a new pjsip_cli.c and the formatters into their respective
> > objects and since pjsip_configuration.c really only manages endpoint,
> > renaming it to config_endpoint.c.  A pjsip_ami.c might be in order as
> > well for common pjsip cli stuff.
> >
> When it comes to file layout I am more of a fan of grouping
> functionality along with the object type.  Personally I would rather see
> the ami commands for endpoints in with the endpoints and not group with
> ami commands for other types.
>
> That being said I'd be fine with for instance config_auth.c,
> config_auth_ami.c, config_auth_cli.c if one was so inclined.
>
> I'd also be fine with renaming pjsip_configuration to config_endpoint.
>
> I wouldn't go down to config_auth_cli level.  config_auth should have all
auth related code whether CLI or AMI but there's CLI code at the pjsip
level that's not object specific that should probably come out of
pjsip_configuration and go in a pjsip_cli.c file.


> >
> > Am I overthinking all of this? It's all perfectly clear in MY mind. :)
> >
> I am quite sure I am :-)
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131206/73b49bc5/attachment.html>