feat: support versioned providers #35
Conversation
WalkthroughThe changes introduce support for Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ReclaimProofRequest
participant SessionUtils
participant Backend
Client->>ReclaimProofRequest: init(applicationId, appSecret, providerId, {providerVersion})
ReclaimProofRequest->>SessionUtils: initSession(providerId, appId, timestamp, signature, providerVersion)
SessionUtils->>Backend: POST /api/sdk/init/session/ (with versionNumber)
Backend-->>SessionUtils: { resolvedProviderVersion }
SessionUtils-->>ReclaimProofRequest: InitSessionResponse
ReclaimProofRequest-->>Client: ReclaimProofRequest instance (with resolvedProviderVersion)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/utils/sessionUtils.ts (1)
25-26: Consider adding validation for the new parameter.The optional
versionNumberparameter maintains backward compatibility, but consider adding validation within this function to ensure type safety.export async function initSession( providerId: string, appId: string, timestamp: string, signature: string, versionNumber?: string ): Promise<InitSessionResponse> { logger.info(`Initializing session for providerId: ${providerId}, appId: ${appId}`); + + // Validate versionNumber if provided + if (versionNumber !== undefined) { + validateFunctionParams([ + { input: versionNumber, paramName: 'versionNumber', isString: true } + ], 'initSession'); + } + try {src/Reclaim.ts (1)
512-513: Consider handling undefined resolvedProviderVersion more explicitly.Defaulting to empty string might mask cases where
resolvedProviderVersionshould be present but isn't set.- jsonProofResponse: this.jsonProofResponse, - resolvedProviderVersion: this.resolvedProviderVersion ?? '' + jsonProofResponse: this.jsonProofResponse, + resolvedProviderVersion: this.resolvedProviderVersion || ''Consider logging a warning when
resolvedProviderVersionis undefined:+ if (!this.resolvedProviderVersion) { + logger.info('Warning: resolvedProviderVersion is undefined during serialization'); + } resolvedProviderVersion: this.resolvedProviderVersion ?? ''
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/Reclaim.ts(11 hunks)src/utils/sessionUtils.ts(1 hunks)src/utils/types.ts(4 hunks)src/utils/validationUtils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/utils/sessionUtils.ts (2)
src/utils/types.ts (1)
InitSessionResponse(69-72)src/utils/constants.ts (1)
BACKEND_BASE_URL(2-2)
src/utils/validationUtils.ts (1)
src/utils/errors.ts (1)
InvalidParamError(28-28)
🔇 Additional comments (10)
src/utils/validationUtils.ts (1)
152-155: LGTM! Validation follows established patterns.The
providerVersionvalidation is correctly implemented using the same pattern as existing option validations, with proper type checking and error handling.src/utils/sessionUtils.ts (1)
33-33: JSON payload correctly includes version information.The
versionNumberis appropriately included in the request body and will be sent to the backend API.src/utils/types.ts (3)
34-34: Comprehensive type support for version options.The
ProofRequestOptionstype correctly includes bothversionNumberandproviderVersionas optional fields, supporting flexible version specification.Also applies to: 41-41
71-71: Appropriate addition of version tracking in session response.The
resolvedProviderVersionfield inInitSessionResponseenables the backend to communicate the resolved version information back to the client.
107-107: Consistent version field integration across data structures.Both
ProofPropertiesJSONandTemplateDatatypes are properly updated to include version fields, ensuring consistent serialization and template data handling.Also applies to: 123-124
src/Reclaim.ts (5)
142-143: Proper initialization of version fields in template data.The empty template data correctly includes the new version fields with appropriate default values.
156-156: Appropriate private property for resolved version.The
resolvedProviderVersionproperty is correctly defined as optional since it's set after session initialization.
225-229: Consistent validation pattern for provider version.The validation follows the established pattern used for other options in the constructor.
268-270: Proper integration with session initialization.The
providerVersionis correctly passed toinitSessionand theresolvedProviderVersionresponse is properly stored.
529-530: Consistent version field propagation in template data.Both
getRequestUrlandtriggerReclaimFlowmethods correctly include the version fields in their template data, ensuring consistency across different flow paths.Also applies to: 586-587
| if (options?.providerVersion) { | ||
| validateFunctionParams([ | ||
| { input: options.providerVersion, paramName: 'providerVersion', isString: true } | ||
| ], 'fromJsonString'); | ||
| } | ||
|
|
||
| if (resolvedProviderVersion) { | ||
| validateFunctionParams([ | ||
| { input: resolvedProviderVersion, paramName: 'resolvedProviderVersion', isString: true } | ||
| ], 'fromJsonString'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making resolvedProviderVersion validation mandatory.
The validation only occurs if resolvedProviderVersion exists, but according to the type definition in ProofPropertiesJSON, it's a required field.
- if (resolvedProviderVersion) {
- validateFunctionParams([
- { input: resolvedProviderVersion, paramName: 'resolvedProviderVersion', isString: true }
- ], 'fromJsonString');
- }
+ validateFunctionParams([
+ { input: resolvedProviderVersion, paramName: 'resolvedProviderVersion', isString: true }
+ ], 'fromJsonString');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (options?.providerVersion) { | |
| validateFunctionParams([ | |
| { input: options.providerVersion, paramName: 'providerVersion', isString: true } | |
| ], 'fromJsonString'); | |
| } | |
| if (resolvedProviderVersion) { | |
| validateFunctionParams([ | |
| { input: resolvedProviderVersion, paramName: 'resolvedProviderVersion', isString: true } | |
| ], 'fromJsonString'); | |
| } | |
| if (options?.providerVersion) { | |
| validateFunctionParams([ | |
| { input: options.providerVersion, paramName: 'providerVersion', isString: true } | |
| ], 'fromJsonString'); | |
| } | |
| validateFunctionParams([ | |
| { input: resolvedProviderVersion, paramName: 'resolvedProviderVersion', isString: true } | |
| ], 'fromJsonString'); |
🤖 Prompt for AI Agents
In src/Reclaim.ts around lines 337 to 347, the validation for
resolvedProviderVersion is conditional but it should be mandatory since the type
definition requires it. Remove the conditional check so that
validateFunctionParams is always called for resolvedProviderVersion, ensuring it
is validated regardless of its presence check.
Description
Testing (ignore for documentation update)
Type of change
Checklist:
Additional Notes:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation