Create Product Consumption Tracking Module#689
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new product_consumption_tracking module (types, constants, OAuth2-configured http:Client, import function) and a customer-portal POST endpoint that validates ZIP content and forwards the file to the tracking service; updates Dependencies.toml to register the module. ChangesDeployment Usage Import Feature
Sequence DiagramsequenceDiagram
participant Service as CustomerPortalService
participant ImportFn as importDeploymentUsage
participant Client as productConsumptionTrackingClient
participant External as ExternalTrackingService
Service->>ImportFn: importDeploymentUsage(email, zip bytes)
ImportFn->>Client: POST /deployment-usage (application/zip)
Client->>External: HTTP request (OAuth2 client-credentials)
External-->>Client: HTTP response
Client-->>ImportFn: DeploymentUsageImportResponse|error
ImportFn-->>Service: DeploymentUsageImportResponse|error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/customer-portal/backend/modules/product_consumption_tracking/client.bal`:
- Around line 31-41: The productConsumptionTrackingClient's retryConfig is
causing unsafe automatic retries for non-idempotent importDeploymentUsage;
remove the retryConfig block (or disable retries) from the
productConsumptionTrackingClient configuration so POSTs won't be automatically
retried, and ensure no other client-level retry is applied to
productConsumptionTrackingClient; reference the retryConfig setting and the
productConsumptionTrackingClient and importDeploymentUsage symbols when locating
the code to change.
In
`@apps/customer-portal/backend/modules/product_consumption_tracking/product_consumption_tracking.bal`:
- Around line 28-29: The code currently appends the user's email into the query
string via the path variable (`string path = string
`/deployment-usage-import?email=${encodedEmail}``) and calls
`productConsumptionTrackingClient->post(path, zipFile, mediaType =
CONTENT_TYPE_APPLICATION_ZIP)` which can leak PII; instead remove the
`?email=...` query param and send the email in a request header (e.g.,
`X-User-Email`) or include it in the authenticated token/context; update the
call to `productConsumptionTrackingClient->post` to use the cleaned path
(`/deployment-usage-import`) and add the header with the `encodedEmail` value
via the client call or its request metadata so the server reads the email from
the header/auth context rather than the URL.
In `@apps/customer-portal/backend/service.bal`:
- Around line 5128-5139: The code calls req.getBinaryPayload() without checking
size—add an explicit max-ZIP-size check before reading the payload (either
inside validateDeploymentUsageImportRequest or immediately before calling
req.getBinaryPayload()) to reject oversized uploads; e.g., determine a
MAX_ZIP_BYTES constant and if the incoming Content-Length (or a
streaming/pre-read size check) exceeds MAX_ZIP_BYTES return the same
<http:BadRequest>{ body: { message: "Upload too large" } } path instead of
calling req.getBinaryPayload(), and ensure any new check uses the same
validationError flow (or sets validationError) so subsequent logic (the existing
log:printWarn(validationError) and return) remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a05011e-8d2d-4a6d-94e2-a5592843fff7
📒 Files selected for processing (7)
apps/customer-portal/backend/Dependencies.tomlapps/customer-portal/backend/modules/product_consumption_tracking/client.balapps/customer-portal/backend/modules/product_consumption_tracking/constants.balapps/customer-portal/backend/modules/product_consumption_tracking/product_consumption_tracking.balapps/customer-portal/backend/modules/product_consumption_tracking/types.balapps/customer-portal/backend/service.balapps/customer-portal/backend/utils.bal
…ing/product_consumption_tracking.bal Co-authored-by: Rashmika Silva <[email protected]>
…ing/product_consumption_tracking.bal Co-authored-by: Rashmika Silva <[email protected]>
…ing/types.bal Co-authored-by: Rashmika Silva <[email protected]>
Co-authored-by: Rashmika Silva <[email protected]>
…ing/product_consumption_tracking.bal Co-authored-by: Anuradha Basnayake <[email protected]>
Co-authored-by: Anuradha Basnayake <[email protected]>
…ing/product_consumption_tracking.bal Co-authored-by: Anuradha Basnayake <[email protected]>
Refactor importDeploymentUsage to send a JSON ImportRequest (email and base64-encoded zip) to the productConsumptionTracking service instead of building an http:Request with binary payload. Adds the ImportRequest record type and removes the now-unused ballerina/http import. The zip is converted via zipFile.toBase64() and posted to /deployment-usage.
Rashmika998
left a comment
There was a problem hiding this comment.
Summary
Clean integration of the product consumption tracking module with sensible retry and auth configuration. Two required items: the ZIP upload endpoint reads the binary payload into memory with no size cap, and ImportRequest is an open record that silently accepts extra fields. A few suggestions around error detail propagation, Content-Type coverage, and the hard-coded 300s timeout.
Comment added by Claude on behalf of @Rashmika998
Co-authored-by: Anuradha Basnayake <[email protected]>
Replace hardcoded retry values in productConsumptionTrackingClient with RETRY_COUNT and RETRY_INTERVAL constants and add those constants to constants.bal. This centralizes retry configuration (retains previous values: 3 and 2.0) for easier maintenance and future changes.
This pull request introduces a new
product_consumption_trackingmodule to the customer portal backend, enabling the import of deployment usage data via a zip file. It includes the necessary service endpoint, request validation, and supporting types and constants. Supporting dependencies and imports are also updated to integrate the new functionality.New Product Consumption Tracking Feature:
product_consumption_trackingmodule, including:client.bal: Configures an HTTP client with OAuth2 for secure communication.constants.bal: Defines constants for content type and charset.types.bal: Introduces types for OAuth2 config and the deployment usage import response.product_consumption_tracking.bal: Implements theimportDeploymentUsagefunction to handle the import logic.API and Service Integration:
/deployment-usage-importinservice.balto accept zip files, validate requests, and delegate to the new import function.utils.balto ensure only valid zip files are processed.Dependency and Module Configuration:
ballerina/url,ballerina/urlpackage) inDependencies.tomlto ensure proper resolution and build. [1] [2] [3]service.balfor the new module.Summary by CodeRabbit
New Features
Bug Fixes / Reliability