Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] On being a poet (or about writing stanzas) #101

Open
erszcz opened this issue Mar 24, 2016 · 4 comments
Open

[RFC] On being a poet (or about writing stanzas) #101

erszcz opened this issue Mar 24, 2016 · 4 comments

Comments

@erszcz
Copy link
Member

erszcz commented Mar 24, 2016

Most, if not all, of stanzas in escalus_stanza and custom stanzas in tests use the following style of building up the Erlang representation - that is, records are composed directly:

-spec attach(binary(), binary()) -> #xmlel{}.
attach(User, Host) ->
    iq(<<"set">>,
       [#xmlel{name = <<"attach">>,
               attrs = [{<<"xmlns">>, <<"urn:ietf:params:xml:ns:xmpp-attach">>}],
               children = [#xmlel{name = <<"node">>,
                                  children = [exml:escape_cdata(User)]},
                           #xmlel{name = <<"domain">>,
                                  children = [exml:escape_cdata(Host)]}]}]).

While this approach is easily understandable for Erlang developers, it requires extra effort to get the XML as it really looks in text format, e.g. in cases where we want to provide snippets for tutorials or communication with client app devs.

However, for some time there's also some support for using Mustache for generating stanzas, which makes the constructors look like this:

-spec attach(binary(), binary()) -> #xmlel{}.
attach(User, Host) ->
    T = <<"<iq type='set' id='attach1'>
             <attach xmlns='urn:ietf:params:xml:ns:xmpp-attach'>
               <node>{{user}}</node>
               <domain>{{host}}</domain>
             </attach>
           </iq>">>,
    escalus_stanza:from_template(T, [{user, User}, {host, Host}]).

This approach is also composable, because of the following form:

-spec attach(binary()) -> #xmlel{}.
attach(Host) ->
    Node = escalus_stanza:from_xml(<<"<node>john</node>">>),
    T = <<"<iq type='set' id='attach1'>
             <attach xmlns='urn:ietf:params:xml:ns:xmpp-attach'>
               {{{node}}}
               <domain>{{host}}</domain>
             </attach>
           </iq>">>,
    escalus_stanza:from_template(T, [{node, Node}, {host, Host}]).

escalus_stanza:from_xml/1 and escalus_stanza:from_template/2 differ in the fact that the former doesn't take any parameters to substitute - effectively it's the same as exml:parse/1. Both return an Erlang/exml representation of XML. Triple braces {{{some_element}}} mean that the element passed will be treated as an exml style XML element to ensure composability.

The main benefit is regular represenation of XML in documentation, XEPs, test code and in communication with front-end / client developers. In this case it's pretty easy to just copy and paste the template and turn it into an example for non-Erlangers.

The main disadvantage of this solution is heavily decreased performance due to the templates being parsed first by mustache and then by exml, especially if we use rendered templates to parameterize subsequent templates. Therefore, this method probably should not be used in MongooseIM itself, only in the test code.

I wanted to ask about your opinion:

  • what pros / cons of both approaches do you see?
  • should we switch?

I'm asking these questions now, because I'm going to write support for a new extension and would like to avoid using code that will be frowned upon in the longer run.

@erszcz erszcz changed the title On being a poet (or about writing stanzas) [RFC] On being a poet (or about writing stanzas) Mar 24, 2016
@sstrigler
Copy link

I like it a lot. Makes code much more readable (and less error prone - the whole point of testing, right?). Is there no way to introduce some sort of clever caching?

@michalwski
Copy link
Contributor

I also like your idea @erszcz. I'd like to add one bit from me to the performance issue. We're also using escalus for load tests with amoc. And here we probably would like to better performance also and that's why I'm not sure weather we should switch to the more user-friendly format or not.

On the other hand, how difficult would it be to have a parse transform changing the XML at compile time to the record representation? Would it make sense at all?

@erszcz
Copy link
Member Author

erszcz commented Mar 29, 2016

On the other hand, how difficult would it be to have a parse transform changing the XML at compile time to the record representation?

For from_xml/1 it's trivial.
For from_template/2 it's not as easy, because the parameters are passed in at runtime when the call is made. In this case, at compile time the template could be rewritten from XML to exml terms, but the final substitution would have to be done by traversing the exml structure at runtime when we actually have the parameter values.
However, I'm not sure we should go this way simply due to growing complexity. Maybe runtime caching in an ETS is a better way to go when (if?) we really need it.

@michalwski
Copy link
Contributor

I agree, runtime caching sounds easier and for me personally it's the missing puzzle before switching to your approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants