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
drastic ldap speedup (update) #9104
drastic ldap speedup (update) #9104
Conversation
add caching to getUserGroupIds
added description and blank lines in getUserGroupIds
💣 Test Failed. 💣 |
cc @blizzz |
defined $uid in getUserGroupIds
$this->cachedUserGroups[$uid] should be an array, right? |
💣 Test Failed. 💣 |
💣 Test Failed. 💣 |
💣 Test Failed. 💣 |
i have no clue why the build suddenly fails. the added function is not getting called in any test, so why would it fail? @blizzz any idea? |
💣 Test Failed. 💣 |
The test fails, because the cache is not being updated e.g. when groups are deleted. Have a look in the debug output and into the test cases. |
{ | ||
$groupIds[] = $group->getGID(); | ||
} | ||
} |
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.
brackets and else in the same line please
@icewind1991 I'd like to have your opinion, also see #9022 (comment) |
@macjohnny and please don't use "Update manager.php" as commit message but briefly state what really happens there and why. |
clean up of function getUserGroupIds and improved caching mechanism of cachedUserGroupIds
I would prefer not to keep a separate cache of group ids but instead just reuse the group cache |
@icewind1991 is there any reason why the group cache is not indexed with the group ids? |
That would be a good way to solve it |
removed cachedUserGroupIds, instead changed indexing in getUserGroups to groupId
💣 Test Failed. 💣 |
💣 Test Failed. 💣 |
The inspection completed: 9 new issues, 1 updated code elements |
🚀 Test Passed. 🚀 |
@blizzz Your expertise is requested here :-) |
💣 Test FAILed. 💣 |
💣 Test FAILed. 💣 |
@blizzz any update on this one? |
urks. @Xenopathic and i looked at it during our Berlin hackathon, but missed to comment here. It was fine, however 👍 Test passed after the latest code change, alas this is some time ago. I'd like to have another positive result from Jenkins. |
@owncloud-bot retest this please |
🚀 Test PASSed. 🚀 |
@karlitschek backport? basically it runtime-caches a user's groups in the user management and is a follow up to #9022. If so, after 7.0.3? |
backport please |
Update manager.php add caching to getUserGroupIds Update manager.php added description and blank lines in getUserGroupIds Update manager.php defined $uid in getUserGroupIds Update manager.php Update manager.php Update manager.php clean up function getUserGroupIds clean up of function getUserGroupIds and improved caching mechanism of cachedUserGroupIds modified caching mechanism in getUserGroupIds removed cachedUserGroupIds, instead changed indexing in getUserGroups to groupId adapted tests for a groupId indexed group array
backport to stable7: 174ab07 |
done. @macjohnny thank you again and my apologies for the long delay this PR had. |
related to drastic speedup for nested ldap groups #9022