Fixing uploadProfilePicture to use presignedURLs #13
Conversation
WalkthroughThis PR refactors the UploadProfilePicture API: the input becomes empty, and the output now returns a required PresignedPostData object (url_info). Corresponding Python and TypeScript models/types are updated, and the operation’s requestBody is removed from the TS OpenAPI models. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant ObjectStorage
Client->>API: POST /profiles/upload-picture (no body)
API-->>Client: 200 { url_info: PresignedPostData }
Client->>ObjectStorage: Upload using PresignedPostData
ObjectStorage-->>Client: 204/201 Upload success
Estimated code review effort🎯 2 (Simple) | ⏱️ ~9 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (1)**/*.smithy📄 CodeRabbit Inference Engine (.rules/001_prefermap)
Files:
🧠 Learnings (1)📚 Learning: 2025-07-19T02:03:53.681ZApplied to files:
🧬 Code Graph Analysis (3)typescript/src/index.ts (1)
typescript/src/models.ts (1)
python/api_model/types/models.py (2)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
model/profiles.smithy(1 hunks)python/api_model/types/models.py(1 hunks)typescript/src/index.ts(1 hunks)typescript/src/models.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.smithy
📄 CodeRabbit Inference Engine (.rules/001_prefermap)
When creating a set of uniquely named/id objects in the smithy model, prefer to use Map over List.
Files:
model/profiles.smithy
🧬 Code Graph Analysis (2)
typescript/src/index.ts (1)
python/api_model/types/models.py (1)
PresignedPostData(198-203)
python/api_model/types/models.py (2)
typescript/src/index.ts (1)
PresignedPostData(365-368)python/build/lib/api_model/types/models.py (1)
PresignedPostData(79-81)
🔇 Additional comments (5)
python/api_model/types/models.py (1)
283-285: Presigned URL response model aligns with presigned-post pattern (good for security and scalability).Returning
url_info: PresignedPostDatais consistent with the rest of the API and reduces server load and data handling risk. Type matches the sharedPresignedPostData.model/profiles.smithy (1)
82-84: UploadProfilePictureOutput returning PresignedPostData is the right design move.The
@required url_info: PresignedPostDataoutput cleanly supports client-side uploads and minimizes sensitive data handling server-side.typescript/src/index.ts (1)
434-436: TS type updated tourl_info: PresignedPostData— consistent and correct.Matches the Smithy and Python models;
unknownfor fields is acceptable given provider variability.typescript/src/models.ts (2)
591-592: Schema updated tourl_info: PresignedPostData— consistent across stacks.This keeps parity with
GetUploadURLResponseContentand the new Smithy definition.
1115-1115: ✅ UploadProfilePicture empty-input verified; no lingering references foundAll checks are green for removing the request body on the
UploadProfilePictureoperation:
requestBody?: neveris correctly applied intypescript/src/models.ts:1115.- No occurrences of the old
UploadProfilePictureRequestContentremain.- No client code still posts a body to
/uploadProfilePicture.PresignedPostDatausage is consistent across TypeScript and Python models.Approving these changes.
… patterns. might remove it again after debugging is done
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
model/profiles.smithy (1)
83-85: Expose presigned URL metadata (expiry and constraints) to harden clientsAdding optional metadata (expiresAt, max size, allowed content types) improves client-side security and UX (Security: enforce size/type constraints; Design: clearer contract). This was requested previously; repeating here for visibility.
structure UploadProfilePictureOutput { @required url_info: PresignedPostData + // Optional metadata for client guidance & validation + expiresAt: Timestamp + maxSizeBytes: Long + allowedContentTypes: StringList }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
model/equaliq.smithy(1 hunks)model/profiles.smithy(1 hunks)python/api_model/types/models.py(2 hunks)typescript/src/index.ts(2 hunks)typescript/src/models.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.smithy
📄 CodeRabbit Inference Engine (.rules/001_prefermap)
When creating a set of uniquely named/id objects in the smithy model, prefer to use Map over List.
Files:
model/equaliq.smithymodel/profiles.smithy
🧬 Code Graph Analysis (3)
python/api_model/types/models.py (2)
typescript/src/index.ts (3)
UploadProfilePictureInput(435-435)UploadProfilePictureResponseContent(437-439)PresignedPostData(366-369)python/build/lib/api_model/types/models.py (1)
PresignedPostData(79-81)
typescript/src/index.ts (1)
python/api_model/types/models.py (3)
UploadProfilePictureInput(283-284)UploadProfilePictureResponseContent(287-288)PresignedPostData(198-203)
typescript/src/models.ts (1)
typescript/src/index.ts (1)
components(4-4)
🔇 Additional comments (8)
model/equaliq.smithy (1)
39-40: LGTM: Exposing UploadProfilePictureInput is consistent and usefulGood addition to ExposedTypes; this keeps SDKs consistent and scalable when evolving public surface types.
typescript/src/index.ts (2)
223-226: LGTM: ExposeTypes includes uploadProfilePictureInputMatches Smithy and models.ts. This keeps the unwrapped types in sync.
437-439: Verify clients handle requiredurl_infofor uploadProfilePictureThe
UploadProfilePictureResponseContenttype now only includes theurl_infofield (see typescript/src/index.ts:438). Please confirm that:
- All client implementations have been updated to consume
url_infoinstead of the removedmessageandpictureIdfields.- Any legacy references or parsing logic for
messageorpictureId(e.g. in tests or shared utility functions) have been removed.- Consumers are using the presigned POST data to submit multipart/form-data, not sending a JSON body to
/uploadProfilePicture.- Integration tests or downstream services no longer expect the old response shape.
python/api_model/types/models.py (2)
283-285: LGTM: Empty UploadProfilePictureInput matches Smithy and exposed typesThis keeps the surface stable without requiring a request body.
535-538: LGTM: ExposeTypes includes uploadProfilePictureInputConsistent with Smithy and TS models; helps SDK consumers discover the type.
typescript/src/models.ts (3)
406-409: LGTM: ExposeTypes schema extended with uploadProfilePictureInputIn sync with the service model; no issues.
593-594: LGTM: Response shape returns required url_info (PresignedPostData)This is consistent with GetUploadURL and improves scalability by delegating file transfer to storage.
1117-1117: No breaking changes detected—UploadProfilePicture’s requestBody removal is safe
- Searches in TypeScript and Python client code found no invocations of uploadProfilePicture (no
.uploadProfilePicture(orupload_profile_picture(calls).- This repository is a client SDK; there are no server handler implementations here to update.
- Since no clients send a JSON body, removing
requestBodyfrom the model is backward-compatible.
…existing patterns. might remove it again after debugging is done" This reverts commit 957c571.
Summary by CodeRabbit