Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Dec 7, 2017

this got lost during the performance changes (sic!), but now comes with a unit test 😊

Basically you see an additional request on LDAP after getting a user list (even on login) that checks for existence of the user. The data is fresh from LDAP however and can be cached right away to avoid this additional requests.

@nextcloud/ldap

@blizzz blizzz added this to the Nextcloud 13 milestone Dec 7, 2017
@blizzz blizzz added the 3. to review Waiting for reviews label Dec 7, 2017
@codecov
Copy link

codecov bot commented Dec 7, 2017

Codecov Report

Merging #7418 into master will decrease coverage by 23.65%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #7418       +/-   ##
=============================================
- Coverage     50.93%   27.27%   -23.66%     
- Complexity    24710    24712        +2     
=============================================
  Files          1586     1522       -64     
  Lines         94144    86415     -7729     
  Branches       1364        0     -1364     
=============================================
- Hits          47948    23568    -24380     
- Misses        46196    62847    +16651
Impacted Files Coverage Δ Complexity Δ
apps/user_ldap/lib/Helper.php 29% <ø> (-9%) 34 <0> (ø)
apps/user_ldap/lib/Access.php 0% <0%> (-26.29%) 315 <0> (+2)
lib/private/DB/OCSqlitePlatform.php 0% <0%> (-100%) 5% <0%> (ø)
lib/private/Files/Cache/Wrapper/JailPropagator.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/SystemTag/ManagerFactory.php 0% <0%> (-100%) 3% <0%> (ø)
lib/private/Share20/Hooks.php 0% <0%> (-100%) 3% <0%> (ø)
apps/files_trashbin/lib/Command/Expire.php 0% <0%> (-100%) 3% <0%> (ø)
apps/files_trashbin/lib/Hooks.php 0% <0%> (-100%) 3% <0%> (ø)
...yBuilder/FunctionBuilder/SqliteFunctionBuilder.php 0% <0%> (-100%) 1% <0%> (ø)
apps/user_ldap/lib/LDAPProviderFactory.php 0% <0%> (-100%) 2% <0%> (ø)
... and 443 more

@blizzz blizzz force-pushed the ldap-fix-cache-retrieved-user branch from 7310724 to 27f14ee Compare December 7, 2017 21:47
@blizzz blizzz mentioned this pull request Dec 8, 2017
28 tasks
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please!

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and still works 👍

@MorrisJobke MorrisJobke merged commit 7fdc4b5 into master Dec 11, 2017
@MorrisJobke MorrisJobke deleted the ldap-fix-cache-retrieved-user branch December 11, 2017 14:39
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.

4 participants