I haven't looked too closely at this, but had a couple of comments having
done a bunch of the work on mod_lcr.
1) Using %q is fragile for custom sql. If the user puts too many in there
mprintf is gonna have fits. You might want to look at using a (temp) event,
setting vars on it, and then using the var expansion support instead. So,
instead of %q, you would use ${varname}. This also makes things more
flexible as you go forward in your development. You can see how it is in
done in mod_lcr.
2) On the sql, I hope the final won't allow NULL in all fields. Some of
those look like auto-increment primary keys which can't be null. Others
shouldn't be null (would it make sense to have a null name?).
I haven't had a chance to look at the patch. Any chance you can put it in
the jira as a diff with a .txt extension? Easier to review that way rather
than a tgz.
On Thu, Jun 18, 2009 at 4:48 AM, Leon de Rooij <l...@scarlet-internet.nl>wrote:
> Hi all,
>
> I'd like to generate user directory xml directly in FS to bypass http
> +php (or other scripting language) entirely. So I started writing a
> module to do just that. It's still rough - I don't code c very often -
> but it works.
>
> http://jira.freeswitch.org/browse/XML-3
>
> Currently all queries are statically defined in the module so users
> cannot deviate from the sql schema. Bkw suggested to have a look at
> mod_lcr which look really cool, though I'm not really sure yet how to
> get the same behavior in this module.
>
> The sql schema (without indexes for readability) currently looks like
> this:
>
> http://pastebin.freeswitch.org/9427
>
> In the module, I'm doing 7 queries:
>
> Q1: Get domain_id and user_id
> Q2: Get domain_params
> Q3: Get domain_variables
> Q4: Get user_attrs
> Q5: Get user_params
> Q6: Get user_variables
> Q7: Get groups
>
> Where Q2-Q6 should return a list of name/value pairs.
>
> To make things more configurable by the user of the module, I could
> place all queries in the configuration file, like this:
>
> http://pastebin.freeswitch.org/9426
>
> Do you think that would be acceptable ?
>
> Then I'd remove all code that automatically generates tables (which
> would make the code a lot cleaner and I won't need to find out how to
> set auto_increment as it's different for each database).
>
> Are there any other things I should think about ?
>
> Do you think this module could be accepted in FS ? :-)
>
> Thanks for the input !
>
> Kind regards,
>
> Leon
>
> _______________________________________________
> Freeswitch-dev mailing list
> Freeswitch-dev@lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org
>
--
-Rupa
_______________________________________________
Freeswitch-dev mailing list
Freeswitch-dev@lists.freeswitch.org
http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
http://www.freeswitch.org
