APP-2908: Fluent API setup #205
Conversation
Create ServiceFactory class for providing services to SymphonyBdk entry point. Simplify code in SymphonyBdk Experimental: First implementing OBO call on StreamService. User can do an OBO authentication by calling SymphonyBdk#obo(Obo.Handle). This method will return an AuthSession which is an obo session. OboStreamService contains only OBO-enabled stream apis. User will do the OBO authentication out of scope of SymphonyBdk, then will pass the AuthSession which is returned by the previous step to the method to perform the call with oboSession.
symphony-hong
requested review from
thibauult,
symphony-elias and
symphony-youri
|
|
||
| AuthSession oboSession = bdk.obo(Obo.username("hong.le")); | ||
| StreamFilter streamFilter = new StreamFilter().addStreamTypesItem(new StreamType().type(StreamType.TypeEnum.IM)); | ||
| List<StreamAttributes> streamsList = bdk.streams().listStreams(streamFilter, oboSession); |
There was a problem hiding this comment.
how would we handle session expiration here?
There was a problem hiding this comment.
when the session expiration, the API call returns error 401, in this case, we will refresh the session by calling AuthSession#refresh
|
|
||
| class ServiceFactory { | ||
|
|
||
| private final UserService userService; |
There was a problem hiding this comment.
Why does this class should contain single instance for each service? Is it really a factory?
There was a problem hiding this comment.
I agree that it's not really a factory. I want to decouple the service stuff in SymphonyBdk to a dedicated class. The instance of each class is initialized at the constructor given an AuthSession. And I think for one AuthSession, we should only one instance of each service.
Maybe I should rename this class?
symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java
Outdated
Show resolved
Hide resolved
symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java
Outdated
Show resolved
Hide resolved
symphony-hong
force-pushed
the
APP-2908
branch
from
0694bf0 to
9f52481
Compare
It will be removed 👍 |
symphony-bdk-examples/bdk-core-examples/src/main/java/com/symphony/bdk/examples/AuthMain.java
Show resolved
Hide resolved
Create OboUserService for calling user service methods using OBO Session Remove methods wrapping the deprecated user service API Remove redundant OBO class
|
|
||
| AuthSession oboSession = bdk.obo("hong.le"); | ||
| StreamFilter streamFilter = new StreamFilter().addStreamTypesItem(new StreamType().type(StreamType.TypeEnum.IM)); | ||
| List<StreamAttributes> streamsList = bdk.streams().listStreams(oboSession, streamFilter); |
There was a problem hiding this comment.
According to https://perzoinc.atlassian.net/browse/APP-2908, we wanted to have something like :
bdk.messages(Obo.username("user.name")).send(STREAM, MESSAGE); or bdk.streams(Obo.username("user.name")).listStreams(streamFilter);
Why changing it to bdk.streams().listStreams(oboSession, streamFilter) ?
There was a problem hiding this comment.
Because we don't want to initialize a new service instance each time we pass an Obo to bdk#messages, it will cause an memory issue if we call it a lot of times. (For ex: if we want to call an api OBO for 1000 thousands user, we have to instantiate 1000 message service instances)
| * @return The information about the stream with the given id. | ||
| * @see <a href="https://developers.symphony.com/restapi/reference#stream-info-v2">Stream Info V2</a> | ||
| */ | ||
| public V2StreamAttributes getStreamInfo(AuthSession oboSession, String streamId) { |
There was a problem hiding this comment.
IMHO this is misleading as oboSession can actually be a bot session. authSession could be a better option along with a comment in the javadoc saying it can be a bot or obo session.
There was a problem hiding this comment.
Okay I will rename it.
(But IMHO, the bot session is actually the obo session that the bot do a call on behalf of itself 😄 )
|
|
||
| import java.util.List; | ||
|
|
||
| class OboStreamService { |
There was a problem hiding this comment.
IMHO name is misleading as theses services are not limited to OBO.
There was a problem hiding this comment.
The idea behind this class is that me and @symphony-thibault want two layers for one service, the first one is at the Obo level which contains only OBO methods and the second one contains all the methods.
symphony-hong
requested review from
thibauult,
symphony-elias and
symphony-youri
docs/fluent.md
Outdated
| @@ -0,0 +1,70 @@ | |||
| # Fluent API | |||
|
|
|||
| The Fluent API is the most basic component of the BDK. This component provides the developers very quickly an entry point | |||
There was a problem hiding this comment.
I don't think that the Fluent API is a "component"
There was a problem hiding this comment.
Should I call it "feature"?
docs/fluent.md
Outdated
|
|
||
| public static void main(String[] args) { | ||
| //Init the BDK entry point | ||
| final SymphonyBdk bdk = new SymphonyBdk(loadFromSymphonyDir("config.yaml")); |
There was a problem hiding this comment.
I'd prefer to haveloadFromClasspath("/config.yaml") in doc examples
There was a problem hiding this comment.
Do you want me to update this one as well?
symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java
Show resolved
Hide resolved
I will fully implement with all service after StreamService is reviewed. (unittest and doc as well)**
Create ServiceFactory class for providing services to SymphonyBdk entry point.
Simplify code in SymphonyBdk
Experimental: First implementing OBO call on StreamService.
User can do an OBO authentication by calling SymphonyBdk#obo(Obo.Handle). This method will return an AuthSession which is an obo session.
OboStreamService contains only OBO-enabled stream apis. User will do the OBO authentication out of scope of SymphonyBdk, then will pass the AuthSession which is returned by the previous step to the method to perform the call with oboSession.
Added unittest and documentation