feat(orm,web): add image field upload and dimension limits (PR-P2-F3) - #224
feat(orm,web): add image field upload and dimension limits (PR-P2-F3)#224buke wants to merge 6 commits into
Conversation
- Allow @field image/binary maxUploadBytes/maxWidth/maxHeight wired through FieldsGet/codegen. - Reject oversized uploads in OImageField and Bind against document.attachment.maxUploadBytes default. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds per-field upload-byte and image-dimension limits. It validates and stores these limits, propagates them through parser and metadata APIs, enforces them during attachment binding, and validates image uploads in the web component. ChangesPer-field upload limits
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant OImageField
participant FieldsGet
participant imageFieldLimits
participant AttachmentBinding
OImageField->>FieldsGet: request field limit metadata
FieldsGet-->>OImageField: return maxUploadBytes, maxWidth, maxHeight
OImageField->>imageFieldLimits: validate selected image
imageFieldLimits-->>OImageField: return validation result
OImageField->>AttachmentBinding: submit valid attachment
AttachmentBinding-->>OImageField: accept or reject field limits
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
- Keep a single DEFAULT_GLOBAL_MAX_UPLOAD_BYTES import via _upload re-export. Co-authored-by: Cursor <cursoragent@cursor.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
- Extract Bind validation messages into document.pot. - Add zh_CN translations for maxUploadBytes/maxWidth/maxHeight errors. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Cover webapistore MaxUploadBytes/Width/Height metadata emission. - Expand imageFieldLimits and OImageField gate unit tests to full branches. - Broaden attachment binding field-limit validation cases and simplify effective byte cap. Co-authored-by: Cursor <cursoragent@cursor.com>
…e gaps - Keep OImageField msgid literals extractable via imageFieldLimits _t calls. - Move limit source resolution out of the Vue SFC to drop the remaining partial. - Cover missing model metadata and non-numeric dimension probes on Bind validation. Co-authored-by: Cursor <cursoragent@cursor.com>
![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 (3)
modules/web/web/components/field/OImageField.test.ts (1)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport the shared upload cap constant instead of duplicating its value.
Line 20 hardcodes
20 * 1024 * 1024as a localDEFAULT_GLOBAL_MAX_UPLOAD_BYTES. The sibling testattachment_binding_limits.test.tsimports the same constant from@/core/service/orm/upload_limits. If the shared constant changes, this duplicated value goes stale and the assertions at lines 110-112 silently validate the wrong cap.♻️ Proposed refactor to import the shared constant
-const DEFAULT_GLOBAL_MAX_UPLOAD_BYTES = 20 * 1024 * 1024; +import { DEFAULT_GLOBAL_MAX_UPLOAD_BYTES } from '`@/core/service/orm/upload_limits`';🤖 Prompt for 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. In `@modules/web/web/components/field/OImageField.test.ts` at line 20, Replace the locally duplicated DEFAULT_GLOBAL_MAX_UPLOAD_BYTES value in OImageField.test.ts with an import of the shared constant from `@/core/service/orm/upload_limits`, and keep the existing assertions using that imported symbol.modules/document/service/models/_attachment_binding_ops.ts (2)
556-557: 🧹 Nitpick | 🔵 TrivialLGTM!
For future work: since the field-specific check only runs at bind time, content that fits the global cap but exceeds a stricter field limit is fully uploaded and stored before rejection. If this becomes a storage-cost concern, consider surfacing field-specific limits earlier in the upload/finalize flow as well.
🤖 Prompt for 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. In `@modules/document/service/models/_attachment_binding_ops.ts` around lines 556 - 557, For future work, consider moving the field-specific validation currently performed by validateAttachmentContentFieldLimits in the attachment binding flow earlier into the upload or finalize path, so content exceeding the ownerModel/fieldName limit is rejected before full storage while preserving the existing global cap validation.
85-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove unnecessary
as anycasts on typedAttachmentContentfields.
attachmentContentis typed asAttachmentContent, and that model already declaresSizeBytes,ImageWidth, andImageHeightas typed fields. Casting toanyat lines 87, 104, and 105 bypasses type checking for no functional reason. Access the fields directly to keep type safety on this validation path.♻️ Proposed refactor to drop the `as any` casts
- const sizeBytes = Number((attachmentContent as any).SizeBytes ?? 0); + const sizeBytes = Number(attachmentContent.SizeBytes ?? 0);- const imageWidth = (attachmentContent as any).ImageWidth; - const imageHeight = (attachmentContent as any).ImageHeight; + const imageWidth = attachmentContent.ImageWidth; + const imageHeight = attachmentContent.ImageHeight;Please confirm that the
AttachmentContenttype used in this file's imports actually exposesSizeBytes,ImageWidth, andImageHeightwithout further casting, since the type is not fully shown in the reviewed excerpt.🤖 Prompt for 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. In `@modules/document/service/models/_attachment_binding_ops.ts` around lines 85 - 134, In the attachment validation logic, remove the unnecessary any casts from accesses to attachmentContent.SizeBytes, attachmentContent.ImageWidth, and attachmentContent.ImageHeight. Confirm the imported AttachmentContent type exposes these fields directly, then preserve the existing fallback and dimension-validation behavior while relying on compile-time type checking.
🤖 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 `@internal/parser/backendtsparser/field_resolved.go`:
- Around line 779-787: Clear the MaxUploadBytes, MaxWidth, and MaxHeight fields
to 0 before the conditional checks that reapply them from spec.Structural. This
ensures that when a developer removes these constraints from the source, the
previously persisted values do not stick around on the next parse. Apply the
same clear-before-reapply pattern already established for the FieldHelp and
HelpText fields (visible just below in the same function) to prevent stale
upload-limit values from flowing into convertFieldToMetadata and affecting
generated Web API metadata.
---
Nitpick comments:
In `@modules/document/service/models/_attachment_binding_ops.ts`:
- Around line 556-557: For future work, consider moving the field-specific
validation currently performed by validateAttachmentContentFieldLimits in the
attachment binding flow earlier into the upload or finalize path, so content
exceeding the ownerModel/fieldName limit is rejected before full storage while
preserving the existing global cap validation.
- Around line 85-134: In the attachment validation logic, remove the unnecessary
any casts from accesses to attachmentContent.SizeBytes,
attachmentContent.ImageWidth, and attachmentContent.ImageHeight. Confirm the
imported AttachmentContent type exposes these fields directly, then preserve the
existing fallback and dimension-validation behavior while relying on
compile-time type checking.
In `@modules/web/web/components/field/OImageField.test.ts`:
- Line 20: Replace the locally duplicated DEFAULT_GLOBAL_MAX_UPLOAD_BYTES value
in OImageField.test.ts with an import of the shared constant from
`@/core/service/orm/upload_limits`, and keep the existing assertions using that
imported symbol.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 509d7a4b-af57-4a4a-b491-e8b5be5c9a86
📒 Files selected for processing (25)
internal/module/artifact/generate/webapistore.gointernal/module/artifact/generate/webapistore.ts.tplinternal/module/artifact/generate/webapistore_test.gointernal/parser/backendtsparser/field_resolved.gointernal/parser/backendtsparser/parser_migration_test.gomodules/core/service/orm/decorator/field.test.tsmodules/core/service/orm/decorator/field.tsmodules/core/service/orm/metadata/field.tsmodules/core/service/orm/model/model_fields_get.test.tsmodules/core/service/orm/model/model_fields_get_facade.tsmodules/core/service/orm/upload_limits.tsmodules/document/i18n/document.potmodules/document/i18n/zh_CN.pomodules/document/service/models/_attachment_binding_ops.tsmodules/document/service/models/_upload.tsmodules/document/service/tests/attachment_binding_limits.test.tsmodules/web/i18n/web.potmodules/web/i18n/zh_CN.pomodules/web/web/components/field/OImageField.test.tsmodules/web/web/components/field/OImageField.vuemodules/web/web/components/field/imageFieldLimits.tsmodules/web/web/stores/fieldsGet.test.tsmodules/web/web/stores/fieldsGet.tsmodules/web/web/stores/modelStore.tspkg/meta/meta_field.go
- Reset MaxUploadBytes/MaxWidth/MaxHeight before reapply so removed options do not stick. - Drop unnecessary AttachmentContent any casts and import the shared upload cap in FE tests. - Cover empty leaf resolution and missing URL when probing image dimensions. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
User description
Summary
@Field({ type: 'image'|'binary', maxUploadBytes?, maxWidth?, maxHeight? })through FieldMetadata, IR/codegen, and FieldsGet (wire keys only; no silent resize).maxUploadBytesagainstdocument.attachment.maxUploadBytesdefault (DEFAULT_GLOBAL_MAX_UPLOAD_BYTES= 20 MiB); FEOImageFieldand BEbindAttachmentreject oversize bytes/dimensions..dev/(gitignored) to name the config key.Test plan
go test ./internal/parser/backendtsparser/ ./internal/module/artifact/generate/ -count=1./choysum test unit core --be(decorator / FieldsGet image limits)./choysum test unit web --fe(OImageField / imageFieldLimits)./choysum test unit document --be(bind field limits)./choysum test typecheck core/web/documentMade with Cursor
PR Type
Enhancement, Tests
Description
Go Core: Add
maxUploadBytes,maxWidth, andmaxHeightto field IR metadata, TS parser, and web API store generator.TypeScript Modules: Enforce upload and dimension limits in
@Fielddecorators,FieldsGetfacade, document attachment bindings, and webOImageField.SPDX & Licensing: Added 4 new TypeScript source/test files with SPDX Apache-2.0 headers, maintaining clean module boundaries.
Test Coverage: Expanded Go parser unit tests, TS ORM decorator/FieldsGet tests, document binding limit tests, and web Vue component tests.
File Walkthrough
14 files
Add upload limit fields to generated metadataEmit upload limit fields in web API store templateParse upload and dimension limits from field optionsAdd upload limit fields to field IR structValidate upload and dimension limits in Field decoratorAdd upload limit properties to FieldMetadata typesExpose upload limit metadata in FieldsGet payloadDefine global default max upload byte constantValidate attachment content against field upload limitsRe-export global max upload byte limit constantAdd frontend image dimension and byte limit validatorsValidate image upload dimensions and size before selectionInclude upload limit attributes in FieldsGet requestsAdd upload limit fields to WebFieldMetadata interface4 files
Add Go parser tests for image limit metadataAdd unit tests for Field upload limit optionsAdd unit tests for attachment binding limit validationAdd unit tests for frontend image validation helpers2 files
Add i18n translation keys for image upload limit errorsAdd Chinese translations for image limit error messages2 files
Summary by CodeRabbit
New Features
Bug Fixes