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

Add method to read multi-value attributes from ldap #27515

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Jun 15, 2021

  • Add a method to read multi-value attributes
  • Migrate getUserAttribute to use getMultiValueUserAttribute internally
  • Also cache a negative / non-existing result¹

¹ we need this feature in Mail and will call getMultiValueUserAttribute frequently. Yet I'm unsure about the caching because false from readAttribute is either no attribute found or something went wrong. I guess the current behavior to cache both is fine for our use case in Mail. A way to distinguish between "ldap is fine but not data" and "something went wrong" would be nice.

@kesselb kesselb self-assigned this Jun 15, 2021
@kesselb kesselb force-pushed the enh/noid/read-multi-value-user-attribute branch 2 times, most recently from 610a240 to e858b2b Compare June 15, 2021 19:31
@kesselb kesselb added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 15, 2021
@kesselb kesselb added this to the Nextcloud 22 milestone Jun 15, 2021
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

looks good, just some phpdoc things to address

apps/user_ldap/lib/LDAPProvider.php Outdated Show resolved Hide resolved
lib/public/LDAP/ILDAPProvider.php Outdated Show resolved Hide resolved
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb force-pushed the enh/noid/read-multi-value-user-attribute branch from e858b2b to 04411df Compare June 16, 2021 09:35
@blizzz blizzz mentioned this pull request Jun 16, 2021
45 tasks
@kesselb kesselb mentioned this pull request Jun 16, 2021
10 tasks
@ChristophWurst ChristophWurst merged commit 39f0aa5 into master Jun 16, 2021
@ChristophWurst ChristophWurst deleted the enh/noid/read-multi-value-user-attribute branch June 16, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants