Relocates OAuth token management #12
Conversation
Introduces a new OCAPI client with specialized modules for system objects, site preferences, and catalog operations. This change implements a facade pattern, delegating calls to specialized clients and handling authentication. It also adds validation and query building utilities to ensure correct API usage.
Moves the OAuth token management functionality from a generic `auth` directory to a `base` directory within the `clients` directory. This change improves the project's structure by aligning the token management more closely with the HTTP client infrastructure, as it's integral to the base client authentication process. Updates relevant imports and references to reflect the new location.
Refactors code to directly assign the baseUrl property, instead of using a type assertion and assignment to 'any'. This improves code clarity and avoids potential type-related issues.
There was a problem hiding this comment.
Pull Request Overview
This PR relocates OAuth token management logic from the auth/ directory to the clients/base/ directory, improving the project's architectural cohesion by consolidating authentication-related code within the base client infrastructure. The refactoring transforms the monolithic OCAPI client into a facade pattern that orchestrates specialized client modules for different domain areas.
Key Changes:
- Moved OAuth token management from
auth/toclients/base/directory - Refactored OCAPI client into a modular architecture with specialized clients
- Added comprehensive validation and query building utilities
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/clients/base/oauth-token.ts | Relocated OAuth token management with updated import paths |
| src/clients/base/ocapi-auth-client.ts | New OCAPI-specific authentication client extending base HTTP functionality |
| src/clients/base/http-client.ts | New abstract base HTTP client providing common request handling and retry logic |
| src/clients/ocapi-client.ts | Refactored into facade pattern orchestrating specialized clients |
| src/clients/ocapi/system-objects-client.ts | New specialized client for system object operations |
| src/clients/ocapi/site-preferences-client.ts | New specialized client for site preference management |
| src/clients/ocapi/catalog-client.ts | New specialized client for catalog operations |
| src/utils/validator.ts | New validation utilities for API parameters |
| src/utils/query-builder.ts | New query string building utilities |
| src/index.ts | Updated import path for relocated TokenManager |
| tests/ | Comprehensive test suite for all new modules |
| .github/copilot-instructions.md | Updated documentation reflecting new architecture |
|
|
||
| super(config); | ||
| // Override the baseUrl for this specialized client | ||
| this.baseUrl = baseUrl; |
There was a problem hiding this comment.
Overriding the baseUrl property after calling super() breaks encapsulation. Consider passing the baseUrl to the parent constructor or providing a protected setter method in the base class.
| this.baseUrl = baseUrl; | |
| config.baseUrl = baseUrl; | |
| super(config); |
|
|
||
| super(config); | ||
| // Override the baseUrl for this specialized client | ||
| this.baseUrl = baseUrl; |
There was a problem hiding this comment.
Overriding the baseUrl property after calling super() breaks encapsulation. Consider passing the baseUrl to the parent constructor or providing a protected setter method in the base class.
| this.baseUrl = baseUrl; | |
| // Set baseUrl in config before calling super | |
| (config as any).baseUrl = baseUrl; | |
| super(config); |
|
|
||
| super(config); | ||
| // Override the baseUrl for this specialized client | ||
| this.baseUrl = baseUrl; |
There was a problem hiding this comment.
Overriding the baseUrl property after calling super() breaks encapsulation. Consider passing the baseUrl to the parent constructor or providing a protected setter method in the base class.
| this.baseUrl = baseUrl; | |
| config.baseUrl = baseUrl; | |
| super(config); |
| private tokenManager: TokenManager; | ||
|
|
||
| constructor(config: OCAPIConfig) { | ||
| super('', 'OCAPIAuthClient'); // Initialize BaseHttpClient with logger |
There was a problem hiding this comment.
Passing an empty string as baseUrl and then overriding it in subclasses is confusing. Consider redesigning the base class to accept optional baseUrl or provide a proper initialization pattern.
| super('', 'OCAPIAuthClient'); // Initialize BaseHttpClient with logger | |
| super(OCAPI_AUTH_CONSTANTS.AUTH_URL, 'OCAPIAuthClient'); // Initialize BaseHttpClient with logger and baseUrl |
Moves the OAuth token management logic from the
auth/directory to theclients/base/directory.This change improves the project's structure by consolidating authentication-related code within the base client infrastructure, making it more maintainable and easier to understand. It also prepares the codebase for potential future enhancements to the authentication mechanisms.