Ejabberd: Shared roster group opts name changed to label - breaks when upgraded from 20.04 and older

[sarsonj]

Environment

• ejabberd version: 21.04
  

  Bug description

We are upgrading after year to new version of Ejabberd (21.04) and we realised, that shared roster group data structure changed (commit has that made this change - e197b25e820ff33390d73551e3e8fac0964d7e2c). To be specific, the attribute "name" in opts changed to "label". This is breaking change for us - we are encoding some information into this field and when there is none, our app don't work correctly. What worse - the API call srg_create still accepts name in its parameter list and also docs refers to name field, not label (https://docs.ejabberd.im/developer/ejabberd-api/admin-api/). The label is even not accepted by sgr_create, so that there is no way to create shared roster group using API (with label).

I checked to mod_shared_roster.erl and it looks like that nobody considered migration when made this change.

So it would be nice at least update docs and srg_create API to returns error when name is set to avoid confusion and find problem soon.

Now I am thinking how to fix this issue in our production environment. I probably don't want to change ejabberd source files, because I want to upgrade again in the future. But I can change database. I'd like to ask, if it is safe to just update opts fields in SQL database - change name to label or even better duplicate name data and have them in both name and label field? Don't it break anything?

[badlop]

the attribute "name" in opts changed to "label".

Right, but that commit maintained support for opts "name", precisely to handle not only new "label", but also old databases without requiring any changes by the system administrator:

https://github.com/processone/ejabberd/commit/e197b25e820ff33390d73551e3e8fac0964d7e2c#diff-0ff381597dbb443bc5fb765d0c78468cebe7a8061f9cb84991276f2de7f77735L1140

Now that you found a problem, I'll check again that code...

if it is safe to just update opts fields in SQL database - change name to label

Yes. Apparently there's some problem that breaks handling old opts->name, and you can update explicitely your database, even if, in theory, it shouldn't be needed.

the API call srg_create still accepts name in its parameter list

Oops, I completely forgot about that administration API. I'll check and update them.

[sarsonj]

Thanks for clarification, this happen when I create new group with API in new eJabberd and add user to this group with API. Then the roster in XMPP client don't get the old name label. Probably, this case is not handled.

I also experiencing cache related problem with shared roster groups - explicitly with shared_roster_user_groups_cache / shared_roster_group_explicit_cache. We are using shared roster groups to create "family" of devices, that see each other. Before we pair 2 devices, each device have its initial shared roster group. After paired, one of devices is set as "master" and the second device is added to its shared roster group and removed from its group (so that one group have 2 devices, second 0).

Initial state (2 devices, 2 groups):

iex([email protected])2> :ets.tab2list(:shared_roster_user_groups_cache)
[
  {{"ejabberd", {"5237d696-3f09-4fa3-a7a0-769141447843__1", "ejabberd"}},
   ["tAChPpBx7F"], 1618606889129},
  {{"ejabberd", {"0eae9596-f03f-4d1e-8cfa-07504fa143ad__1", "ejabberd"}},
   ["AdcKm3WBDS"], 1618606927373}
]

When I removed 0eae9596-f03f-4d1e-8cfa-07504fa143ad__1 from group AdcKm3WBDS and added 0eae9596-f03f-4d1e-8cfa-07504fa143ad__1 to group tAChPpBx7F - here is the bug - 0eae9596-f03f-4d1e-8cfa-07504fa143ad__1 has no group, but should have tAChPpBx7F:

iex([email protected])3> :ets.tab2list(:shared_roster_user_groups_cache)
[
  {{"ejabberd", {"5237d696-3f09-4fa3-a7a0-769141447843__1", "ejabberd"}},
   ["tAChPpBx7F"], 1618606889129},
  {{"ejabberd", {"0eae9596-f03f-4d1e-8cfa-07504fa143ad__1", "ejabberd"}}, [],
   1618606990975}
]

When I restart ejabberd, cache invalidates / reloads and now here is correct state:

iex([email protected])2> :ets.tab2list(:shared_roster_user_groups_cache)
[
  {{"ejabberd", {"5237d696-3f09-4fa3-a7a0-769141447843__1", "ejabberd"}},
   ["tAChPpBx7F"], 1618607171129},
  {{"ejabberd", {"0eae9596-f03f-4d1e-8cfa-07504fa143ad__1", "ejabberd"}},
   ["tAChPpBx7F"], 1618607178239}
]

Maybe this is also API related, because we manipulate users and groups using administrative API - sgr_user_add and sgr_user_del. When I disabled shared_roster_user_groups_cache and shared_roster_group_explicit_cache, it works as expected.

[sarsonj]

I fixed both issues in pull request #3578 .

[badlop]

Now that your fixes are committed, I've prepared the change in docs.ejabberd.im, and will be automatically published when the next release is published.