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

return the users username instead of the uuid in share responses #1370

Closed
wants to merge 1 commit into from

Conversation

C0rby
Copy link
Contributor

@C0rby C0rby commented Dec 14, 2020

A while ago we decided to use the users uuid only internally and to only display or return the users username. The ocs share api did still return the uuid.

Closes owncloud/ocis#990

@C0rby C0rby requested a review from labkode as a code owner December 14, 2020 09:46
Copy link
Contributor

@butonic butonic left a comment

Choose a reason for hiding this comment

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

this must be cached, otherwise we will make an rpc to the gateway and then to the user provider, probably to ldap ... and back, several times per file list entry ...

// UserServiceUserIDResolver returns a userId resolver using the UserService
func UserServiceUserIDResolver(ctx context.Context, client gatewayv1beta1.GatewayAPIClient) func(*userpb.UserId) (string, error) {
return func(id *userpb.UserId) (string, error) {
resp, err := client.GetUser(ctx, &userpb.GetUserRequest{UserId: id})
Copy link
Contributor

Choose a reason for hiding this comment

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

this must be cached, otherwise we will make an rpc to the gateway and then to the user provider, probably to ldap ... and back, several times per file list entry ...

Copy link
Contributor Author

@C0rby C0rby Dec 14, 2020

Choose a reason for hiding this comment

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

Ugh... I think this is also causing the tests to fail... The ldap server gets overwhelmed and then crashes...
Locally the pipelines work... 😞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented the username lookup similar to the display name lookup which you implemented. Including the caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants