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

Fix bugs in mod_mam_odbc_prefs #654

Merged
merged 16 commits into from
Jan 29, 2016
Merged

Fix bugs in mod_mam_odbc_prefs #654

merged 16 commits into from
Jan 29, 2016

Conversation

arcusfelis
Copy link
Contributor

Add tests for MAM preferences
Bug 1: JIDs are not normalized before stored in preferences
Bug 2: Result from SQL query never matches in get_behaviour

Add tests for MAM preferences
Bug 1: JIDs are not normalized before stored in preferences
Bug 2: Result from SQL query never matches in get_behaviour
@arcusfelis
Copy link
Contributor Author

Progress so far:

  • More tests for MAM prefs.
  • Fixed mam_*_prefs modules. None of them worked.
  • Reduced mam_SUITE execution time.

@arcusfelis
Copy link
Contributor Author

Ready for review.

@erszcz
Copy link
Member

erszcz commented Jan 29, 2016

Let's not forget to remove apps/ejabberd/src/mod_mam_mnesia_prefs.erl and the relevant tests once this PR is merged. To make things simpler, let it be another PR.

The reason is that it duplicates functionality of mod_mam_mnesia_dirty_prefs while also probably being less efficient. All in all, MAM seems complex enough right now, we don't need more modules doing the same thing (as it was with mod_mam_user and mod_mam_server_user) and hyper-configurability to the degree of madness ;)

erszcz added a commit that referenced this pull request Jan 29, 2016
Fix bugs in mod_mam_odbc_prefs
@erszcz erszcz merged commit 85119a9 into master Jan 29, 2016
@erszcz erszcz deleted the arc-mam-prefs-fixes branch January 29, 2016 17:03
@michalwski michalwski mentioned this pull request Feb 12, 2016
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.

2 participants