APP-2983: Implementation of user service in BDK 2.0 #195

symphony-hong · 2020-09-01T11:24:05Z

Create UserService class to wrap all the user api endpoints provided
by the pod to retrieve user, list of users, perform action to get and
update role, delegate, entitlements, avatar, disclaimer, status of user.

Create an entry to user service in SymphonyBdk by calling method SymphonyBdk#user

Create an example for user service

Update javadoc

Unittest added with code coverage >90%

Create UserService class to wrap all the user api endpoints provided by the pod to retrieve user, list of users, perform action to get and update role, delegate, entitlements, avatar, disclaimer, status of user. Create an entry to user service in SymphonyBdk by calling method SymphonyBdk#user Create an example for user service Update javadoc

symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserDetailMapper.java

symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java

thibauult · 2020-09-01T17:20:30Z

@symphony-hong How do the services will handle OBO authentication? At the moment, it is only possible to call those services using the regular bot authentication (i.e. bot's AuthSession). I think that we should also provide prototypes that accept an AuthSession as parameter. WDYT?

symphony-hong · 2020-09-01T17:28:20Z

How do the services will handle OBO authentication?

I thought, for handling the OBO authentication, we just need pass the obo's AuthSession to the constructor like what is being implemented in MessagaeService, isn't it?

thibauult · 2020-09-01T17:32:37Z

I thought, for handling the OBO authentication, we just need pass the obo's AuthSession to the constructor like what is being implemented in MessageService, isn't it?

MessageService was more a POC than a definitive implementation. The problem with this design is that we need to create a new instance each time we want to call in OBO mode. Which can be very costly in terms of memory if an application has to manage a lot of OBO sessions.

symphony-hong · 2020-09-01T17:58:26Z

I have several ideas for it:

Take the authentication out of the SymphonyBdk, accept AuthSession as parameter
Create service for both normal and obo mode, store in hashmap (Flyweight Pattern)

thibauult · 2020-09-01T18:36:37Z

@symphony-hong could you make a small demo for both of each?

symphony-hong · 2020-09-02T06:47:27Z

could you make a small demo for both of each?

Okay I will do it when we're working with the ticket about Fluent API

Using MapperStruct to create a mapper for user detail to make it more generic Refactor user service entry Update a more complicated example for user service

Define UserService#authSession for setting up the authSession using by UserService instance. In SymphonyBDK, passing the authSession or oboSession to UserService instance to make it run with normal or obo session respectively.

symphony-hong · 2020-09-02T07:27:01Z

@symphony-thibault or we can do like what I did in last update. Is this your point at the beginning?

thibauult · 2020-09-02T07:46:28Z

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java

+
+  private final UserApi userApi;
+  private final UsersApi usersApi;
+  private AuthSession authSession;


In a multi-threaded environment (Spring for instance): this will cause big issues for sure :) Also, we should also keep in mind that OBO is not supported for all API endpoints (cf. https://developers.symphony.com/restapi/reference#obo-enabled-endpoints).

We should enforce developers to use OBO authentication on OBO-Enabled endpoints only.

symphony-bdk-core/src/test/java/com/symphony/bdk/core/test/BdkMockServer.java

symphony-elias

LGTM

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserDetailMapper.java

pom.xml

symphony-bdk-bom/pom.xml

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java

thibauult · 2020-09-03T17:23:19Z

Last minute idea: we could create a custom annotation @OBOEnabled that we would place on top of methods that actually accept OBO authentication. Not very useful at the moment since we don't support OBO for services yet, but that could be really interesting in the future, I think...

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/OboEnabled.java

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/constant/RoleId.java

thibauult · 2020-09-04T12:47:15Z

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java

+
+  /**
+   * Retrieve user details of a particular user.
+   * @see <a href="https://developers.symphony.com/restapi/reference#get-user-v2">Get User v2</a>


@see is usually placed at the end of the Javadoc comment block (cf. https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html)

thibauult

LGTM 👍

symphony-youri · 2020-09-07T10:24:11Z

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java

+   * @param uid User Id
+   * @return    User found by uid
+   */
+  public UserV2 getUserById(@NonNull Long uid) {


We could have a getUser(UserId id) method to cover both cases (long id and string email)

And then UserId.id(123) and UserId.email("bob@mail.com")
This way we don't mess up with primitive types and avoid having methods suffixed with byId/byEmail

But maybe the API does not always support id and email?

the API for getting user accept only one of the params: username, uid or email. If we pass more than one param, the error 400 is returned. That's why I splitted it to 3 different methods.

It does not prevent the UserService to have the logic to set the username/uid/email based on the UserId passed as a parameter

but do all the APIs related to user support passing username, uid or email? (otherwise we would not have a consistent API where UserId can be used everywhere)

Nope, only the API for getting user:
https://github.com/symphonyoss/symphony-api-spec/blob/master/pod/pod-api-public.yaml#L1784
and the API for seaching users:
https://github.com/symphonyoss/symphony-api-spec/blob/master/pod/pod-api-public.yaml#L1426

symphony-youri · 2020-09-07T10:26:21Z

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java

+   * @param image The avatar image for the user profile picture.The image must be a base64-encoded.
+   * @see         <a href="https://developers.symphony.com/restapi/reference#update-user-avatar">Update User Avatar</a>
+   */
+  public void updateAvatarOfUser(@NonNull Long uid, @NonNull String image) {


As a API user in Java I would perhaps prefer to send an inputstream here (and then have the SDK do the encoding for me)

symphony-youri · 2020-09-07T10:26:43Z

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java

+   * @param image The avatar image in bytes array for the user profile picture.
+   * @see         <a href="https://developers.symphony.com/restapi/reference#update-user-avatar">Update User Avatar</a>
+   */
+  public void updateAvatarOfUser(@NonNull Long uid, @NonNull byte[] image) {


ah that one is better :)

symphony-hong requested review from thibauult, symphony-youri and symphony-elias September 1, 2020 11:24

thibauult reviewed Sep 1, 2020

View reviewed changes

symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java Outdated Show resolved Hide resolved

thibauult reviewed Sep 1, 2020

View reviewed changes

symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserDetailMapper.java Outdated Show resolved Hide resolved

thibauult reviewed Sep 1, 2020

View reviewed changes

symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java Outdated Show resolved Hide resolved

symphony-hong force-pushed the APP-2983 branch from 97b9a41 to 1d3ef0b Compare September 1, 2020 16:40

symphony-hong requested a review from thibauult September 1, 2020 16:50

Update user detail mapper and user service entry

1ea37a5

Using MapperStruct to create a mapper for user detail to make it more generic Refactor user service entry Update a more complicated example for user service

symphony-hong force-pushed the APP-2983 branch from 1d3ef0b to 1ea37a5 Compare September 2, 2020 07:13

UserService run with Obo session

f7e74f6

Define UserService#authSession for setting up the authSession using by UserService instance. In SymphonyBDK, passing the authSession or oboSession to UserService instance to make it run with normal or obo session respectively.

thibauult reviewed Sep 2, 2020

View reviewed changes

Update unittest

4fdca64

symphony-hong force-pushed the APP-2983 branch from 4df3dca to 4fdca64 Compare September 3, 2020 09:45

symphony-hong requested a review from thibauult September 3, 2020 10:18

symphony-hong marked this pull request as ready for review September 3, 2020 12:16

symphony-hong requested a review from a team September 3, 2020 12:16

symphony-elias reviewed Sep 3, 2020

View reviewed changes

symphony-bdk-core/src/test/java/com/symphony/bdk/core/test/BdkMockServer.java Show resolved Hide resolved

symphony-elias approved these changes Sep 3, 2020

View reviewed changes