-
Notifications
You must be signed in to change notification settings - Fork 494
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
7344 mailgroup regex #7351
7344 mailgroup regex #7351
Conversation
… of CDI. Otherwise it does not start and cannot be used as a facade. IQSS#7344
@scolapasta as promised, here you go. 🛩️ |
… happened, as we rely on the database... IQSS#7344
Some hints: | ||
|
||
- Due to their excessive CPU usage, regular expressions should be used rarely. | ||
- Also due to their CPU usage, all domain groups are cached. Cache updates when groups change via API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain a little more what you mean by this? what is cached and when?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The singleton has a copy of the groups in memory (instance variables).
Everytime authorization happens, the regular expressions aren't recompiled, which would be a huge waste of CPU resources. In other words: the singleton is a facade for the rarely changing group data.
This "cache" is updated (compiling the regex) on startup and when domain groups are changed via CRUD API actions (not during read, obviously).
This might not be necessary to mention in the guides, as an admin should never notice anything of it. I left it there for FWIW.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, I think Admins won't care about that detail - I think it's safe to remove.
Documentation Updates
@djbrooke I just merged your PR. Thx for polishing my bad style and writing! @scolapasta would you pass this along to @kcondon for QA? Thank you guys! |
Thx for merging @kcondon 🥳 |
What this PR does / why we need it:
Per #7344, @scolapasta asked for regex support in mail domain groups. Sounded like this would be a use case for Harvard Dataverse.
Which issue(s) this PR closes:
Closes #7344
Special notes for your reviewer:
None.
Suggestions on how to test this:
Create a group with a regex, create users with mail addresses matching the regex and some with non-matching. Assign perms to the group and test with different users.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
None.
Is there a release notes update needed for this change?:
This might be worth a short note.
Additional documentation:
None.