-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: new SDK core integration #114
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: mdwairi <[email protected]>
Co-authored-by: mohnoor94 <[email protected]>
# Conflicts: # README.md # code/build.gradle # code/src/main/kotlin/com/expediagroup/sdk/graphql/common/DefaultGraphQLExecutor.kt # code/src/main/kotlin/com/expediagroup/sdk/graphql/common/GraphQLClient.kt # code/src/main/kotlin/com/expediagroup/sdk/graphql/common/GraphQLExecutor.kt # code/src/main/kotlin/com/expediagroup/sdk/graphql/model/response/Error.kt # code/src/main/kotlin/com/expediagroup/sdk/lodgingconnectivity/configuration/ClientConfiguration.kt # code/src/main/kotlin/com/expediagroup/sdk/lodgingconnectivity/payment/PaymentClient.kt # code/src/main/kotlin/com/expediagroup/sdk/lodgingconnectivity/sandbox/SandboxDataManagementClient.kt # code/src/main/kotlin/com/expediagroup/sdk/lodgingconnectivity/supply/reservation/ReservationClient.kt # gradle.properties
# Conflicts: # code/build.gradle
mohnoor94
previously approved these changes
Dec 3, 2024
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.
Approving as I reviewed and/or updated all the sub PRs led to this.
...rc/main/kotlin/com/expediagroup/sdk/lodgingconnectivity/configuration/ClientConfiguration.kt
Show resolved
Hide resolved
code/src/main/kotlin/com/expediagroup/sdk/core/logging/common/LoggerDecorator.kt
Show resolved
Hide resolved
code/src/main/kotlin/com/expediagroup/sdk/core/authentication/bearer/BearerTokenStorage.kt
Show resolved
Hide resolved
code/src/main/kotlin/com/expediagroup/sdk/core/interceptor/Interceptor.kt
Show resolved
Hide resolved
jordan-n-schmidt
previously approved these changes
Dec 3, 2024
anssari1
reviewed
Dec 3, 2024
anssari1
previously approved these changes
Dec 3, 2024
code/src/main/kotlin/com/expediagroup/sdk/core/interceptor/Interceptor.kt
Show resolved
Hide resolved
code/src/main/kotlin/com/expediagroup/sdk/core/logging/LoggingInterceptor.kt
Show resolved
Hide resolved
anssari1
reviewed
Dec 3, 2024
code/src/main/kotlin/com/expediagroup/sdk/core/logging/masking/MaskLogsUtils.kt
Show resolved
Hide resolved
Mohammad-Dwairi
dismissed stale reviews from anssari1 and jordan-n-schmidt
via
December 3, 2024 14:46
06c1f50
anssari1
approved these changes
Dec 3, 2024
mohnoor94
approved these changes
Dec 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
This is the final feature branch we've been working on to build the new SDK core module with injectable HTTP client interfaces and utilities. Changes here were reviewed in previous PRs.
#104 | #105 | #106 | #107 | #108 | #109 | #110
Overview
Working towards offering richer and more flexible SDK interfaces, this PR brings a completely new SDK core module implementation with key enhancements/features like:
No direct dependency on any specific HTTP client.
Providing out-of-the-box, fully configurable
OkHttpClient
as a default client.Getting rid of all Kotlin coroutines dependencies for better Java compatibility.
Significantly reduced the number of dependencies and size
Implemented SDK-specific HTTP models (e.g.
Headers
,Request
,Response
, etc...) to reach full independence of any 3rd parties implementations.Designed a new SDK-level interceptor chain
Implemented a new bearer authentication package that can be used as an SDK interceptor.
Cleaned up the old core package
The New SDK Core
The new core package can be viewed as a tool-box that product SDKs use to build the final experience. It's completely product-agnostic and it doesn't contain any product SDKs concerns.
We've defined a set of requirements this new core package should satisfy in order to achieve the final experience.
The SDK should accept custom HTTP clients with minimal overhead on users.
The SDK should be able to maintain its standard request-response flow even with the custom HTTP clients, this includes authentication, logging, serialization/deserialization.
The SDK should provide a default HTTP client out-of-the-box. This default client should be configurable with options we see it's safe to expose to users.
The SDK accepts either:
SDK HTTP Models
Problem?
The old core was coupled with 3rd party libraries (e.g. Ktor, OkHTTP, etc..). This made designing a unified interface that enables users injecting HTTP clients harder and forced us to follow the library's standards and conventions, limiting our control on the SDK behaviour.
Solution
Implementing HTTP models specific for the SDK like
Request
,Response
,RequestBody
By having such models, we're decoupling the SDK from any 3rd party implementation and makes it easier to design the adapter interface for HTTP clients injection.Transport Interface
Problem?
To achieve complete independence between the SDK and the underlying HTTP client; the SDK shouldn't be aware of anything other than abstract interfaces. Therefore, we need one simple interface the SDK knows how to deal with, delegating the concrete implementations to users where they can use their custom HTTP client with the SDK by following the contract declared in the interface.
Solution
The
Transport
interface is introduced and expected to be implemented by users who wish to use their own HTTP clients with the SDK.execute(request: Request): Response
method that must be implemented.RequestExecutor Abstract Class
Problem?
The way we've been building the SDK is tightly coupled to the underlying HTTP client, so when it comes to custom HTTP clients injection, we need to abstract some aspects in the SDK like logging, authentication, serialization/deserialization.
Solution
The
RequestExecutor
is an abstract class that - mainly - wraps theTransport
instance and executes the needed interceptors before/after the request execution, regardless of the underlying HTTP client.Since we need a way to do these "interceptions" independently, we cannot rely on the HTTP client's native interceptors, therefore, a custom
Interceptor
interface is added to achieve similar experience to what we have at the moment, but completely managed by the SDK.Product SDK Configurations
Problem?
Different configuration classes related to concrete product SDKs were maintained in the core package, which introduced a lot of complexity in the core module to differentiate between configuration classes needed by each product SDK.
Solution
Each product SDK should maintain its own configurations, interfaces, executors, and any other product-specific concerns. Keeping the core module only aware of lower-level HTTP configurations.
This design enables us to get rid of the added complexity in SDK configs (Providers, Collectors, Traits, etc..) The request is managed in the core module that only cares about the API endpoint and the HTTP client configurations (timeouts, connections).
The product SDK is expected to have:
RequestExecutor
mentioned above. Where each SDK define its own suitable interceptors, including authentication, logging, etc...New Core - High-Level Package View
The following visualisation describes the set of tools the core module offers.