mod_roster backends rewrite

[mpmiszczyk]

DO NOT MERGE YET

This is one of the modules mentioned in issue #123. While extracting common backend I've found few issues that should/need to be addressed before finishing

short introduction and file description

Original code started with two roster modules; mod_roster using Mnesia as it's backend and mod_roster_odbc which used odbc as its library. I've started with extracting all backend specific logic from mod_roster to new module mod_roster_mnesia; and repeated same process for mod_roster_odbc and new module mod_roster_odbc_back; after which I was left with four roster files. Two backend implementations (which share same behaviour) and two logic modules, which should be identical, and merger into one. Now, the whole problem is fact that they are not.

Since most of those differences should be considered bugs, and will list them all, and we should decide how to solve them (mostly it is question of choosing mnesia way or odbc way).

A missing hook handler

Edit: as suggested, removed hook from odbc

Odbc module subscribes and implements hook that Mnesia one does not.

While I could not find any code that would run this hook in Mongoose, the p1 roster implementation does have it, and it is run there from ejabber_c2s module.

Odbc module additional exports

Edit [2014-02-17]: removed unused functions

both item_to_xml/1 and roster_version/2 are exported, just like in p1 code, but no one seems to use them. And mnesia module, while still having those functions (and using them internally) does not export them.

- get_versioning_feature xmlel childrens

Edit [2014-02-17]: roster versioning without optional attribute

• mnesia does nothing
• odbc has specific inicialization
• [p1] uses empty list, which is default value (like in our mnesia)

Both versions ware introduced together with #xmlel record, which makes it hard to pin-point the newer one.

- process_item_attrs -

Edit [2014-02-17]: changed all to LJID

For given roster we can update i's jid; which mnesia module does with JID and odbc module does with LJID, (created from different fields of jid record). In p1 version LJID is used.

This seems like simple bug; and I would suggest sticking to LJID

We can see similar situation in process_item_attrs_ws function, with JID in mnesia and LJID in odbc.

- remove_user -

Edit: as suggested, removed string handling.

In mnesia this function can handle strings (as lists) while odbc and p1 does not. Such code causes no additional failures, which I would consider a bug, and remove this list handling guard.

- push_item not_found version handling

While implementation in both modules does not look to similar, the only difference is fact that in mnesia in case of missing version, we will not add <<"version">> at all while in odbc it is possible to write as versin atom not_found. Mnesia version seem older, but it is the one p1 uses

- process_item_els handling of #roster.xs'es

Edit: [2014-02-17] mysterious xs are not handled anywhere, not described by standard child elements of item (contact), that could be send by client. They should be send to support non-default, or future behaviours.

Not sure what those are. Mnesia save all of those (if there are no groups) with code from 2002; while odbc cares only about groups, by logic added with whole module in 2012. p1 acts like mnesia but it is hard to say how old code is (updated in 2013, but before ?? )

get_roster_of

This was originally part of process_item_set; and there are two main issues.

jid vs ljid

Edit [2014-02-17]: use LJID where I can

Again mnesia uses JID for new roster and if record found it changes from saved LJID (since that what should be saved) to JID.

Odbc uses LJID for new roster, and without any sense updates USJ, US and LJID (with values by which it was found.

resetting name and groups

Edit [2014-02-17]: Because a roster item is atomic, the item MUST be updated exactly as provided in the roster set. Hence resetting is necessary. Still, it could be little cleaner.

In same place, if roster is found in database both module reset name and groups

odbc in [original] code just doesn't read groups from table; which I do now; so this will have to be fixed one way or other

Roster than goes to process_item_attrs which can set new name (?so resetting it before might be obsolete). And to process_item_els which adds (not repleace) groups; which makes this addition necessary, or removing them before a bug.

- process_subscription jid vs ljid again

Edit [2014-02-17]: use LJID

mnesia creates new roster [with JID](https://github.com/mpmiszczyk/MongooseIM/blob/feature/mod_roster_backend_rewrite/apps/ejabberd/src/mod_roster.erl#L

Edit [2014-02-17]: 502) while odbc uses LJID.

And yes, this new_if_no_found behaviour will factored; but for now I wanted per-case reference for such differences.

- out_subscription -

Edit [2014-02-17]: changed list to binaries

Calls function with [] ( which is "") when it should be <<>> or even <<"">> like in p1.

- foreach vs flatmap -

Edit [2014-02-17]: decided to use flatmap since it is logic needed by commit b255dc2

This is tricky one, and it is possible, that I just don't understand something.

Mnesia uses foreach just like p1 while odbc uses flatmap. So while first one will return list of as many ok as elements was given; the second will return list with ok only for those elements that we ware able to transalte to jid. This does not make much difference.

What I don't understand is use of return of this function. Since we always return some kind of list (empty | one_element | ... ), how only use of this function could try to pattern match on single ok

odbc error handling

Edit [2014-02-17]: decided to log warning, and leave entry as it is

Data saved in odbc have to be translated from raw to binary. It is a process that can fail in some why. Wright now I handle such failure silently here and here; and we should decide on something more sane. Either

• propagate error outside (and slightly change backend interface) or
• remove found (bad) record and log such event

optimize mnesia

Edit [2014-02-17]: created new issue #153

Odbc stores roster in two tables; roster in one, and it's groups in second. This allows some optimization in calls like roster_without_groups and write_roster_subscription. Since we introduce level of abstraction over storing #roster record, we might introduce similar optimizations and keep groups list in separate mnesia table (without changing record itself).

[mpmiszczyk]

Updated code and commit to version that is ready to be merged in;

[erszcz]

Is this on its way to a title change (I mean the DO NOT MERGE YET)? ;)

[michalwski]

@mpmiszczyk ping :) Are you going to finish this? Or if this is finished, could you rebase it so travis can test it.

[michalwski]

Any news?

[arcusfelis]

lists:map(fun item_to_xml/1, ejabberd_hooks:run_fold(roster_get, To#jid.lserver, [], [US]))

is generic. can be extracted into a separate function.

[ppikula]

It has been done in #359