-
Notifications
You must be signed in to change notification settings - Fork 29
GraphClient and GraphRequestAdapter #555
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
Changes from all commits
726b7f6
e05c636
1765be9
215dcae
e03c48b
069cb8e
ad3226d
d01363d
ef2b4bd
d7425c8
d163ed0
c84ac74
2a0549a
1115722
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| package com.microsoft.graph.Requests; | ||
|
|
||
| import com.microsoft.graph.content.BatchRequestBuilder; | ||
| import com.microsoft.kiota.RequestAdapter; | ||
| import com.microsoft.kiota.authentication.AuthenticationProvider; | ||
| import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
| import okhttp3.OkHttpClient; | ||
|
|
||
| import javax.annotation.Nonnull; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * Default client implementation. | ||
| */ | ||
| public class BaseClient implements IBaseClient{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe we can have a base client (class) in core anymore.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey Vincent, it seems that in C# there is this baseClient class as a means to create a lightweight client that doesn't rely on the service library. I believe we can keep this class here as GraphServiceClient will extend from BaseGraphServiceClient and implement IBaseClient. This is the same pattern that we have in C#. This means I will have to create the GraphServiceClient following the same pattern that is already in C#, but I believe we can keep both.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alright, but just to be clear, this one won't be in the inheritance structure of a client for somebody using the graph service client from the service lib, correct?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, the the GraphServiceClient will extend the BaseGraphServiceClient and implement IBaseClient, GraphServiceClient will not use any BaseClient implementations.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so should this class be removed from the PR? |
||
|
|
||
| private RequestAdapter requestAdapter; | ||
| /** RequestBuilder for completing Batch Requests */ | ||
| public BatchRequestBuilder batchRequestBuilder; | ||
|
|
||
| /** | ||
| * Constructor requiring only a RequestAdapter. | ||
| * | ||
| * @param requestAdapter the specified RequestAdapter used to complete requests. | ||
| */ | ||
| public BaseClient(@Nonnull RequestAdapter requestAdapter) { | ||
| setRequestAdapter(requestAdapter); | ||
| } | ||
|
|
||
| /** | ||
| * Constructor requiring only an AuthenticationProvider. | ||
| * | ||
| * @param authenticationProvider the specified AuthenticationProvider for use in requests. | ||
| */ | ||
| public BaseClient(@Nonnull AuthenticationProvider authenticationProvider) { | ||
| this(new BaseGraphRequestAdapter(authenticationProvider)); | ||
| } | ||
|
|
||
| /** | ||
| * Constructor requiring an AuthenticationProvider and Base URL. | ||
| * | ||
| * @param authenticationProvider the specified AuthenticationProvider for use in requests. | ||
| * @param baseUrl the specified base URL for use in requests. | ||
| */ | ||
| public BaseClient(@Nonnull AuthenticationProvider authenticationProvider, @Nonnull String baseUrl) { | ||
| this(new BaseGraphRequestAdapter(authenticationProvider, baseUrl)); | ||
| } | ||
|
|
||
| @Override | ||
| public void setRequestAdapter(RequestAdapter requestAdapter) { | ||
| this.requestAdapter = requestAdapter; | ||
| } | ||
|
|
||
| @Override | ||
| public RequestAdapter getRequestAdapter() { | ||
| return this.requestAdapter; | ||
| } | ||
|
|
||
| @Override | ||
| public BatchRequestBuilder getBatchRequestBuilder() { | ||
| //TODO: Refactor BatchRequestBuilder so that it accepts a request adapter as the param | ||
| //return this.batchRequestBuilder != null ? this.batchRequestBuilder : new BatchRequestBuilder(this.requestAdapter) | ||
| return this.batchRequestBuilder; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor issue: @ramsessanchez @baywet We can consider configuring a deterministic code formatter to avoid diff due to indenting and prevent merging code that's not formatted al together.
I've worked with spotless which can be configured to check PR's and can be used a maven / gradle plugin. For the formatting rules we can use a derivative of google-formatter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go for it! I usually rely on editor config, but as long as it works on vs code I'm happy. Don't hesitate to pr microsoft/kiota-java (no need to PR the service libs)