PP-2338 feat: Add HTTP client provider pattern for custom networking#884
PP-2338 feat: Add HTTP client provider pattern for custom networking#884abolfazlimahdi wants to merge 58 commits into
Conversation
Implement provider pattern to allow consumers to inject custom OkHttpClient configurations while maintaining backward compatibility and keeping business logic isolated in the SDK. Key changes: - Created GiniHttpClientProvider interface for HTTP client abstraction - Implemented DefaultGiniHttpClientProvider with SDK defaults - Integrated provider into GiniCoreAPIBuilder, GiniBankAPIBuilder, and GiniHealthAPIBuilder - Exposed provider in GiniCaptureDefaultNetworkService.Builder - SDK automatically adds required configuration (TLS, certificates, User-Agent) on top of custom providers - Created CustomHttpClientProvider example demonstrating custom interceptors and logging This enables advanced consumers (e.g., Atruvia) to configure TLS, proxies, interceptors, and logging without reimplementing business logic. No breaking changes - existing integrators continue working as-is.
|
Add automatic version detection to bank-sdk.release.yml workflow that conditionally publishes to either Sonatype Snapshots or Maven Central Staging based on the version string. Changes: - Add version extraction step that reads from bank-sdk/sdk/gradle.properties - Detect SNAPSHOT versions (case-insensitive: -SNAPSHOT or -snapshot) - Add conditional publish to Sonatype Snapshots repository (https://oss.sonatype.org/content/repositories/snapshots/) - Make Maven Central Staging publish conditional (only for releases) - Make Central Portal upload conditional (only for releases) - Add SNAPSHOT tag patterns to workflow triggers Usage: - For snapshots: Set version=X.Y.Z-SNAPSHOT → publishes to Sonatype Snapshots - For releases: Set version=X.Y.Z → publishes to Maven Central Staging This allows testing fixes in snapshot builds without going through full release process, while maintaining existing release workflow unchanged.
Changes: - Updated Fastfile to accept optional build_number parameter - Defaults to 1 if not provided (backward compatible) - Updated bank-sdk.release.yml to pass github.run_number - Now consistent with regular release lane - Provides better traceability for snapshot builds This ensures every snapshot build has a unique versionCode for better tracking and avoids the Gradle error requiring versionCode >= 1.
…l staging - Changed repo URL from https://oss.sonatype.org/content/repositories/snapshots/ to https://ossrh-staging-api.central.sonatype.com/service/local/staging/deploy/maven2/ - Uses same Maven Central token credentials (fixes 401 authentication error) - Maven Central automatically routes SNAPSHOT versions to snapshots repository - Simplifies credential management (single system for both snapshots and releases)
- Changed from old staging URL to new Maven Central Portal snapshots repo - Old: https://ossrh-staging-api.central.sonatype.com/service/local/staging/deploy/maven2/ - New: https://central.sonatype.com/repository/maven-snapshots/ - Snapshots must be enabled in Portal for namespace: net.gini.android - See: https://central.sonatype.org/publish/publish-portal-snapshots/
- Added condition to skip 'check' job for SNAPSHOT versions - Release job now runs with always() to proceed even if check is skipped - Enables fast verification of snapshot publishing without waiting for tests - Will be reverted after confirming snapshot publishing works
Revert "PP-2338: Skip tests for SNAPSHOT releases (quick verification)" Revert "test: Bump Bank SDK version to 4.0.1-SNAPSHOT for testing snapshot publishing" These commits were for testing snapshot publishing mechanism only. Snapshot publishing is now verified and working correctly. Reverting to keep the branch clean.
…library, and capture-sdk - Added SNAPSHOT tag patterns to workflow triggers - Added version detection to identify SNAPSHOT vs release versions - Added conditional snapshot publishing to Maven Central Portal - Repository URL: https://central.sonatype.com/repository/maven-snapshots/ - Conditional Central Portal upload (only for releases) - Capture SDK workflow handles both sdk and default-network via matrix
- core-api-library: 3.0.0 → 3.1.0-SNAPSHOT - bank-api-library: 4.0.0 → 4.1.0-SNAPSHOT - capture-sdk: 4.0.0 → 4.1.0-SNAPSHOT - default-network: 4.0.0 → 4.1.0-SNAPSHOT - bank-sdk: 4.0.0 → 4.1.0-SNAPSHOT Updated RELEASE-ORDER.md with new versions.
…dling - Created GiniAuthenticationInterceptor to automatically add Bearer tokens - Created GiniAuthenticator for token refresh on 401 responses - Integrated both into GiniCoreAPIBuilder.createOkHttpClient() - Moved accessTokenMutex from DocumentRepository to interceptor - Handles special cases (skips auth for /oauth/token and user creation) This is phase 1 of refactoring to move authentication from business logic layer to HTTP layer, eliminating need for manual withAccessToken calls and allowing clients to avoid implementing SessionManager.
…l token passing Removed all withAccessToken wrappers from DocumentRepository: - Removed sessionManager field from constructor - Removed accessTokenMutex (now in interceptor) - Removed withAccessToken() helper method - Updated all 17 methods to directly call documentRemoteSource without access tokens - Removed unused imports (Mutex, withLock, SessionManager) Authentication is now handled automatically by GiniAuthenticationInterceptor. No access tokens are passed through business logic layer. Files modified: - DocumentRepository.kt (core-api-library) Testing: ./gradlew core-api-library:library:compileDebugKotlin - SUCCESS
… token passing Updated Bank API Library: - BankApiDocumentRepository: Removed sessionManager from constructor, removed withAccessToken wrappers from 4 methods - BankApiDocumentRemoteSource: Removed accessToken parameters from 3 methods, changed bearerHeaderMap to buildHeaderMap - BankApiDocumentService: Removed @HeaderMap bearer from sendFeedback override - TrackingAnalysisService: Removed @Header Authorization parameter - TrackingAnalysisRemoteSource: Removed accessToken parameter, removed BearerAuthorizatonHeader import - GiniBankAPIBuilder: Removed sessionManager from repository constructor call Updated Health API Library: - HealthApiDocumentRepository: Removed sessionManager from constructor, removed withAccessToken wrappers from 10 methods - HealthApiDocumentRemoteSource: Removed accessToken parameters from 10 methods, changed bearerHeaderMap to buildHeaderMap - HealthApiDocumentService: Removed @HeaderMap bearer from sendFeedback override - GiniHealthAPIBuilder: Removed sessionManager from repository constructor call All authentication is now handled automatically by GiniAuthenticationInterceptor. No access tokens are passed through business logic layer. Testing: Successfully compiled all three API libraries (core, bank, health).
Problem: - getUserApiRetrofit() called createOkHttpClient() - createOkHttpClient() called getSessionManager() - getSessionManager() called getUserRepository() - getUserRepository() needed getUserService() - getUserService() needed getUserApiRetrofit() → Infinite loop causing StackOverflowError Solution: Created two separate OkHttpClient creation methods: 1. createUserApiOkHttpClient() - WITHOUT authentication interceptor - Used for user login/registration endpoints - Prevents circular dependency 2. createOkHttpClient() - WITH authentication interceptor - Used for authenticated document API endpoints - Depends on SessionManager (which is now safe) Changes: - Added createUserApiOkHttpClient() method - Updated getUserApiRetrofit() to use createUserApiOkHttpClient() - Kept createOkHttpClient() for authenticated APIs This fixes the crash when launching the example app.
… architecture Changes: - Removed tests that checked for manual Bearer token headers (obsolete - now handled by interceptor) - Updated tests to verify methods work without manual authentication - Updated MockDocumentService to match new DocumentService interface signatures: - No @HeaderMap bearer parameters - uploadDocument signature changed (headers param not nullable) - getDocumentLayout returns valid DocumentLayoutResponse instead of null - All 21 unit tests now passing These tests now verify that DocumentRemoteSource methods can be called without manually passing access tokens, since authentication is handled automatically by GiniAuthenticationInterceptor.
…endpoints Problem: - sendFeedback API calls returned 415 Unsupported Media Type - Server expected 'application/vnd.gini.v1+json' but received 'application/json;charset=utf-8' - Old code passed correct Content-Type via bearerHeaderMap parameter - New code lost this after removing manual auth headers Solution: - Added @headers annotation with correct Content-Type to sendFeedback overrides: - BankApiDocumentService: 'application/vnd.gini.v1+json' - HealthApiDocumentService: 'application/vnd.gini.v4+json' - Added missing retrofit2.http.Headers import Files modified: - bank-api-library/.../BankApiDocumentService.kt - health-api-library/.../HealthApiDocumentService.kt This ensures feedback API calls use the correct Gini-specific media type.
…anual token passing - Remove @HeaderMap bearer parameters from BankApiDocumentService (3 methods) - Remove @HeaderMap bearer parameters from HealthApiDocumentService (10 methods) - Remove buildHeaderMap() calls from BankApiDocumentRemoteSource (3 methods) - Remove buildHeaderMap() calls from HealthApiDocumentRemoteSource (10 methods) - Update BankApiDocumentRemoteSourceTest (3 tests now verify business logic, not auth) - Update HealthApiDocumentRemoteSourceTest (4 tests now verify business logic, not auth) Authentication now handled automatically by GiniAuthenticationInterceptor. All tests passing. Completes missing refactoring from previous commits.
…erceptor - Add MockWebServer to version catalog (using squareup-okhttp3 version 4.12.0) - Add mockk to core-api-library test dependencies for mocking SessionManager - Create GiniAuthenticationInterceptorTest with 8 test cases: 1. Adds Authorization Bearer header to authenticated requests 2. Skips auth for /oauth/token endpoint 3. Skips auth for POST /users endpoint (user creation) 4. Skips auth when Authorization header already present 5. Throws IOException when SessionManager returns error 6. Throws IOException when SessionManager returns cancelled 7. Fetches token from SessionManager on first request 8. Uses mutex to prevent parallel token fetches All tests passing (8/8) ✅ Part of Phase 2: Add auth tests for the interceptor/authenticator refactoring
**Why GitHub Actions Passes Without This Fix:** GitHub Actions runs: `./gradlew <module>:testDebugUnitTest` (debug only) Local 'test' runs: BOTH testDebugUnitTest AND testReleaseUnitTest Example from .github/workflows/capture-sdk.check.yml: ```yaml - run: ./gradlew capture-sdk:sdk:testDebugUnitTest # Only debug ``` **Problem:** When running `./gradlew test` locally, it tries to compile release unit tests: - Fragment testing was debugImplementation (unavailable for release) - capture-sdk test resources only available in debug variant **Root Causes:** 1. Fragment testing dependency scope (4 modules): - Was: `debugImplementation(libs.androidx.fragment.testing)` - Issue: Release tests can't resolve FragmentScenario imports - Fix: Changed to `testImplementation` (available for all test variants) 2. capture-sdk test resources in src/test/res: - Only added to debug source set (build.gradle.kts:106-108) - Release tests can't access R.string.custom_help_screen_title - Fix: Explicitly disable release test tasks via afterEvaluate **Why afterEvaluate Block is Necessary:** - `testBuildType = "debug"` alone doesn't prevent compilation - It only sets the default variant, tasks still exist - Need to explicitly disable: compileReleaseUnitTestKotlin, compileReleaseUnitTestJavaWithJavac, testReleaseUnitTest **Changes:** 1. capture-sdk/sdk/build.gradle.kts: - Changed: debugImplementation → testImplementation - Added: testBuildType = "debug" - Added: afterEvaluate block to disable release test tasks 2. health-sdk/sdk/build.gradle.kts: - Changed: debugImplementation → testImplementation 3. internal-payment-sdk/sdk/build.gradle.kts: - Changed: debugImplementation → testImplementation 4. merchant-sdk/sdk/build.gradle.kts: - Changed: debugImplementation → testImplementation **Result:** ✅ `./gradlew test` works locally (runs only debug tests) ✅ GitHub Actions unchanged (already uses testDebugUnitTest) ✅ No impact on CI/CD **Testing:** ```bash # Local (previously failed, now works) ./gradlew test --continue # ✅ BUILD SUCCESSFUL # GitHub Actions (always worked) ./gradlew capture-sdk:sdk:testDebugUnitTest # ✅ BUILD SUCCESSFUL ``` Related to PP-2338 test infrastructure improvements.
**New Test File:** GiniAuthenticatorTest.kt **Test Coverage (6 tests - ALL PASSING ✅):** 1. ✅ authenticate refreshes token on 401 response - Verifies authenticator fetches new token when request fails with 401 - Confirms old token is replaced with new token on retry - Tests complete authentication flow: initial request → 401 → refresh → retry 2. ✅ authenticate retries request with new token - Verifies retry request includes the new token in Authorization header - Confirms second request succeeds with fresh credentials - Tests token replacement works correctly 3. ✅ authenticate gives up after 2 retry attempts - Prevents infinite retry loops by limiting to 2 attempts - Uses responseCount() to track retry chain - Returns 401 without further retries after limit reached 4. ✅ authenticate returns null if token refresh fails - Handles SessionManager errors gracefully - Returns null when Resource.Error received - Prevents retry when token refresh is impossible 5. ✅ authenticate returns null for non-401 responses - Only handles 401 Unauthorized responses - Ignores 403, 404, 500, etc (not auth issues) - Allows other error handlers to process non-auth errors 6. ✅ authenticate uses mutex to prevent parallel refresh - Prevents race conditions when multiple requests fail simultaneously - Ensures only one token refresh happens at a time - Uses refreshMutex to serialize refresh operations **Test Results:** ``` GiniAuthenticatorTest: 6 tests, 0 failures (1.518s) GiniAuthenticationInterceptorTest: 8 tests, 0 failures (1.753s) --- Total: 14 authentication tests passing ✅ ``` **Test Pattern:** - Uses MockWebServer for real HTTP request/response testing - MockK for SessionManager mocking - Tests integration between interceptor and authenticator - Verifies complete auth flow: initial → 401 → refresh → retry **Phase 2 Status:** - ✅ Task 1: Dependencies (MockWebServer + mockk) - ✅ Task 2: GiniAuthenticationInterceptorTest (8 tests) - ✅ Task 3: GiniAuthenticatorTest (6 tests) - ⏳ Task 4: Final verification Part of PP-2338 authentication infrastructure improvements.
There was a problem hiding this comment.
Pull request overview
This PR implements an HTTP client provider pattern (GiniHttpClientProvider / DefaultGiniHttpClientProvider) that lets SDK consumers inject custom OkHttpClient configurations (proxies, TLS, interceptors, etc.). As part of this change, the PR also refactors the entire authentication mechanism from manual @HeaderMap bearer token passing to an interceptor-based approach using GiniAuthenticationInterceptor and GiniAuthenticator, and adds SNAPSHOT release support to several CI workflows.
Changes:
- Introduced
GiniHttpClientProviderinterface andDefaultGiniHttpClientProviderimplementation, integrated into all three builder classes (GiniBankAPIBuilder,GiniHealthAPIBuilder,GiniCaptureDefaultNetworkService.Builder) - Refactored authentication: removed
bearerparameters from all service/repository/remote-source methods; moved auth logic toGiniAuthenticationInterceptor(adds token) andGiniAuthenticator(handles 401 refresh) - Added SNAPSHOT version publishing support to CI release workflows for core-api, bank-api, capture-sdk, and bank-sdk
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
core-api-library/.../GiniHttpClientProvider.kt |
New interface for the HTTP client provider pattern |
core-api-library/.../DefaultGiniHttpClientProvider.kt |
Default SDK implementation, moved SSL/timeout/logging config from builder |
core-api-library/.../GiniAuthenticationInterceptor.kt |
New interceptor that automatically adds Bearer tokens to authenticated requests |
core-api-library/.../GiniAuthenticator.kt |
New OkHttp Authenticator handling 401 token refresh |
core-api-library/.../GiniCoreAPIBuilder.kt |
Refactored HTTP client creation; removed old SSL/logging logic; added setHttpClientProvider() |
core-api-library/.../DocumentService.kt |
Removed @HeaderMap bearer from all service methods |
core-api-library/.../DocumentRemoteSource.kt |
Removed accessToken parameter from all methods |
core-api-library/.../DocumentRepository.kt |
Removed SessionManager dependency and withAccessToken helper |
bank-api-library/.../BankApiDocumentService.kt |
Removed @HeaderMap bearer from all service methods |
bank-api-library/.../GiniBankAPIBuilder.kt |
Added setHttpClientProvider() override for chaining |
bank-api-library/.../TrackingAnalysisService.kt |
Removed @Header("Authorization") from sendEvents |
health-api-library/.../HealthApiDocumentService.kt |
Removed @HeaderMap bearer from all service methods |
health-api-library/.../HealthApiDocumentRemoteSource.kt |
Removed accessToken parameters; lost Accept headers for PDF/PNG differentiation |
health-api-library/.../GiniHealthAPIBuilder.kt |
Added setHttpClientProvider() override for chaining |
capture-sdk/.../GiniCaptureDefaultNetworkService.kt |
Exposed setHttpClientProvider() in the builder |
bank-sdk/example-app/... |
Added example CustomHttpClientProvider and toggle UI |
.github/workflows/*.release.yml |
Added SNAPSHOT tag patterns and conditional publish steps |
fastlane/Fastfile |
Added optional build_number parameter to snapshot lane |
*/gradle.properties |
Version bumps to x.x.x-SNAPSHOT for changed modules |
Comments suppressed due to low confidence (1)
core-api-library/library/src/main/java/net/gini/android/core/api/internal/GiniCoreAPIBuilder.kt:507
- The
FALLBACK_USER_AGENTvalue is now duplicated in bothGiniCoreAPIBuilder.kt(line 505) andDefaultGiniHttpClientProvider.kt(line 272), and the User-Agent interceptor logic is also duplicated inGiniCoreAPIBuilder.createOkHttpClient()andcreateUserApiOkHttpClient(). This duplication could lead to inconsistencies if the fallback string or logic needs updating. Consider extractingFALLBACK_USER_AGENTto a shared constants location, or always routing throughDefaultGiniHttpClientProviderfor User-Agent handling.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
**Issue:** Comments on lines 409 and 417 contradicted each other: - Line 409 said: "Add interceptor FIRST (before consumer's)" - Line 417 said: "placed at end so consumer's run first" **Root Cause:** OkHttpClient.newBuilder() copies the base client's configuration INCLUDING all interceptors. Then addInterceptor() APPENDS to that list. **Actual Execution Order:** 1. Consumer's interceptors (from baseClient) 2. GiniAuthenticationInterceptor (we added) 3. User-Agent interceptor (we added) **Why This Order is Correct:** ✅ SDK always adds authentication (can't be removed by consumer) ✅ SDK intercepts AFTER consumer's modifications ✅ Prevents consumer from accidentally breaking authentication⚠️ Consumer's logging won't see Authorization header (security trade-off) **Fix:** Updated comments to accurately reflect the actual execution order and explain why this order is intentional for security reasons. **No Code Changes** - only comment clarifications. Resolves GitHub Copilot code review feedback.
Fixed contradictory comments about header management in GiniApiHeaderInterceptor and related code. The interceptor only manages Content-Type headers, not Accept. Changes: - GiniApiHeaderInterceptor: Clarified that it does NOT manage Accept headers for the main Gini API (pay-api.gini.net), but other APIs like User Center may still require explicit Accept headers via @HeaderMap or @headers annotation - GiniCoreAPIBuilder: Changed "headers" (plural) to "Content-Type" (singular) in comments to accurately reflect what the interceptor replaces - UserRemoteSource: Added comments explaining why User Center API explicitly passes Accept headers, unlike the main API This resolves confusion between the interceptor's behavior (Content-Type only) and the User Center API's requirements (explicit Accept headers). PP-2338
…d error handling Enhanced documentation and error messages in authentication interceptors to address code review feedback: Changes: - Added comprehensive KDoc to accessTokenMutex and refreshMutex explaining their purpose, why they are internal (not private), and why separate mutexes are needed for different auth scenarios - Documented runBlocking performance implications in both GiniAuthenticationInterceptor and GiniAuthenticator, explaining the necessary trade-off between sync OkHttp API and async SessionManager - Added Dispatchers.IO to runBlocking calls to avoid blocking main thread pool - Enhanced error messages to include request method and URL for better debugging of production authentication failures These changes improve code maintainability and debuggability without altering functional behavior. All existing tests pass. PP-2338
…nd enhance docs Added test validating network interceptor behavior and updated documentation to address code review feedback: Changes: - GiniApiHeaderInterceptorTest: Added test validating network interceptor runs AFTER application interceptors, which is critical for replacing Retrofit's default Content-Type headers with versioned media types - Updated test setup comments to reference the ordering validation test - GiniHttpClientProvider: Enhanced documentation to list ALL interceptors and configuration the SDK adds (User-Agent, GiniApiHeaderInterceptor, GiniAuthenticationInterceptor, GiniAuthenticator) - Documented interceptor ordering: Consumer → Auth → Headers → Authenticator - Clarified that consumer's interceptors run before SDK interceptors These changes improve understanding of the HTTP client provider pattern and validate correct interceptor ordering behavior. All tests pass. PP-2338
…d improve docs - Created configureSDKInterceptors() method to eliminate duplicate interceptor setup code in both custom provider and default provider paths - Enhanced mutex separation explanation in GiniAuthenticator with concurrency benefits - Fixed incorrect documentation in GiniHttpClientProvider about header handling (SDK does replace application/json Content-Type, not just add if missing) PP-2338
…n test Added test to verify the refreshMutex properly serializes concurrent token refresh attempts when multiple requests fail with 401 simultaneously. The test uses timing analysis to prove refreshes happen sequentially (not in parallel), preventing redundant API calls and potential race conditions. Also added visibility test to verify mutex is accessible for integration tests. PP-2338
Clarified why GiniAuthenticationInterceptor throws IOException on cancellation (to fail the entire request chain) while GiniAuthenticator returns null (to gracefully stop retry attempts after 401). This difference reflects their different roles: interceptor adds initial auth, authenticator handles retry. PP-2338
…annotations - Extracted MAX_AUTH_RETRIES constant (value: 2) to make retry limit configurable - Added @SInCE 3.1.0 annotations to new public APIs (GiniHttpClientProvider, setHttpClientProvider) - Added @SInCE 4.1.0 annotations to bank-api-library override methods - Enhanced logging example documentation with production best practices These improvements increase code maintainability and provide clear version tracking for new features. PP-2338
The Health API requires explicit Accept headers, unlike the main Gini API which supports content negotiation. Added HealthApiAcceptHeaderInterceptor that is automatically added to all Health API requests via a wrapper HTTP client provider. This fixes bank-api-library integration tests (testGetPayment, testResolvePayment) that were failing because the health API getPaymentProviders() call was returning an empty list due to missing Accept headers. The solution keeps the Accept header logic in the health-api-library module where it belongs, rather than adding it to the core interceptor. PP-2338
…nd Accept headers The wrapper provider pattern was incorrectly bypassing parent class HTTP client configuration when no custom provider was set. This commit restores the proper pattern where: - Wrapper provides base client (custom or empty OkHttpClient) - Parent class enhances base via newBuilder() adding SDK configs - Health API specific Accept header interceptor runs before parent interceptors Changes: - Re-add HealthApiHttpClientProviderWrapper inner class with correct pattern - Re-add HealthApiAcceptHeaderInterceptor for Health API specific headers - Add init block in GiniHealthAPIBuilder to set default wrapper - Modify setHttpClientProvider() to wrap custom providers This ensures custom TrustManager, cache, timeouts, and other builder settings are properly applied while maintaining Health API specific Accept headers. PP-2338
…se direct header passing The HTTP client provider pattern is not needed for Health API library as it: - Is not used by the client (Atruvia) who requested this feature - Can handle Accept headers directly via @HeaderMap parameters in service methods - Simplifies the codebase and avoids unnecessary wrapper complexity Changes: - Add @HeaderMap headers parameter to Health API service methods - Pass Accept headers directly from HealthApiDocumentRemoteSource - Remove HTTP client provider wrapper from GiniHealthAPIBuilder - Delete HealthApiAcceptHeaderInterceptor (no longer needed) - Update unit test mocks to match new method signatures This keeps Health API simple while maintaining the HTTP client provider pattern in Core API, Bank API, Bank SDK, and Capture SDK where it's actually needed. All unit tests passing. All integration tests passing (28/28). PP-2338
87d1c43 to
558cad0
Compare
|
|
|
|
|
|
|







Implement provider pattern to allow consumers to inject custom OkHttpClient configurations while maintaining backward compatibility and keeping business logic isolated in the SDK.
Key changes:
This enables advanced consumers (e.g., Atruvia) to configure TLS, proxies, interceptors, and logging without reimplementing business logic.
No breaking changes - existing integrators continue working as-is.