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

Fixed srg_create params and shared roster groups cache issues #3578

Merged
merged 2 commits into from
Apr 20, 2021

Conversation

sarsonj
Copy link
Contributor

@sarsonj sarsonj commented Apr 17, 2021

This pull request fixes issues with shared roster groups:

  1. fixed srg_create administrative api function - it accepted old name parameter and stores it as name into OPTS. The parameter should be now named and stored as label
  2. it fixes cache issues. The cache was cleaned before item was added / removed from shared roster group. So that it can happen (and it happened to us), that user was added to shared roster group, but cache item was reloaded before item was updated in database - so there were old data in cache. I changed the order (first update in DB, then delete cache) and it fixed this inconsistency issue.

This pull request is fixing issue #3576.

@p1bot
Copy link
Collaborator

p1bot commented Apr 17, 2021

Hi @sarsonj, many thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

@p1bot p1bot added the cla-missing Contributor needs to sign Contribution License Agreement label Apr 17, 2021
@coveralls
Copy link

Coverage Status

Coverage remained the same at 34.984% when pulling dcc5d87 on tappytaps:srg-cache-fix into e462f0a on processone:master.

@p1bot
Copy link
Collaborator

p1bot commented Apr 17, 2021

You did it @sarsonj!

Thank you for signing the ProcessOne Contribution License Agreement.

We will have a look at your contribution!

@p1bot p1bot removed the cla-missing Contributor needs to sign Contribution License Agreement label Apr 17, 2021
@processone processone deleted a comment from Neustradamus Apr 18, 2021
@badlop badlop self-assigned this Apr 20, 2021
@badlop
Copy link
Member

badlop commented Apr 20, 2021

Thanks!

I was finally able to get the testing evironment, and I've checked this:

  1. installed ejabberd 20.03
  2. created mysql & pgsql databases
  3. created several shared roster groups
  4. upgraded ejabberd to 21.04

Then i confirm:

  • A) opts->name are accepted correctly, no need to update the database
  • B) srg_create API uses the old "name" argument, which is confusing
  • C) newly created groups work correctly, but name is only displayed after a server restart

After applying your PR, B) and C) get fixed :)

@badlop badlop merged commit 2d38d48 into processone:master Apr 20, 2021
@badlop badlop added this to the ejabberd 21.xx milestone Apr 20, 2021
@Neustradamus
Copy link
Contributor

Thanks @badlop for the test and merging.

Thanks a lot to @sarson for your PR :)

@badlop
Copy link
Member

badlop commented Apr 20, 2021

Thanks @badlop for the test and merging.

Thanks a lot to @sarson for your PR :)

That is a useless comment, it adds nothing. It's annoying. You did nothing at all. You just added several useless comments.

Please, can you remove that comment immediately?

And please, can you NOT add more useless comments?

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants