Skip to content

Fix Max Size Validation on First Upload#743

Merged
Schmarvinius merged 5 commits intomainfrom
bugfix/maxValNotFiringOnFirstUpload
Mar 9, 2026
Merged

Fix Max Size Validation on First Upload#743
Schmarvinius merged 5 commits intomainfrom
bugfix/maxValNotFiringOnFirstUpload

Conversation

@Schmarvinius
Copy link
Collaborator

@Schmarvinius Schmarvinius commented Mar 9, 2026

Fix Max Size Validation on First Upload

Bug Fix

🐛 Fixed an issue where the Validation.Maximum size constraint was not being applied correctly on the first upload of an attachment. The previous implementation relied on processing existing attachment data to extract the size annotation, which caused validation to be skipped when no prior attachments existed (e.g., on a brand-new draft).

Changes

  • ModifyApplicationHandlerHelper.java: Refactored getValMaxValue to read the Validation.Maximum annotation directly from the CdsEntity element definition instead of iterating over existing attachment data. This ensures the size limit is always enforced regardless of whether prior attachments exist. Also removed the now-unused VALMAX_FILTER static field and the AtomicReference/CdsDataProcessor imports.

  • ModifyApplicationHandlerHelperTest.java: Updated unit test setup to use an empty existing attachments list (List.<Attachments>of()) in the size-validation test cases, reflecting the fixed behavior where validation works even without pre-existing data.

  • SizeLimitedAttachmentsSizeValidationDraftTest.java: Added two new integration tests:

    • uploadContentWithinLimitAndActivateDraftSucceeds — verifies that a 3MB upload (within a 5MB limit) on a new draft succeeds and the draft can be prepared and activated.
    • uploadContentExceedingLimitOnFirstDraftRejects — verifies that a 6MB upload on a brand-new draft is correctly rejected with HTTP 413.
  • pom.xml: Bumped latest test versions of CAP Java (4.7.0) and cds-dk (9.7.2) used in integration tests.

  • 🔄 Regenerate and Update Summary

📬 Subscribe to the Hyperspace PR Bot DL to get the latest announcements and pilot features!

PR Bot Information

Version: 1.17.99 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Event Trigger: pull_request.opened
  • Summary Prompt: Default Prompt
  • Output Template: Default Template
  • Correlation ID: 974cd160-1b9f-11f1-8bf2-ad3c1b2f22ca
  • LLM: anthropic--claude-4.6-sonnet

Copy link
Contributor

@hyperspace-insights hyperspace-insights bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request is a clean refactor that correctly fixes the root bug (max-size annotation was previously read from instance data via CdsDataProcessor, which failed on first upload when no existing data existed) by reading it directly from the CdsEntity model metadata instead. The new getValMaxValue implementation is simpler, stateless, and correct.

The only issue posted is a minor dead-code redundancy in the Optional filter chain (v != null is unreachable after .map()), which should be cleaned up for clarity.

PR Bot Information

Version: 1.17.99 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Correlation ID: 974cd160-1b9f-11f1-8bf2-ad3c1b2f22ca
  • Event Trigger: pull_request.opened
  • LLM: anthropic--claude-4.6-sonnet

Schmarvinius and others added 2 commits March 9, 2026 11:12
…chments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java

Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
@Schmarvinius
Copy link
Collaborator Author

Schmarvinius commented Mar 9, 2026

For Context:
Why getValMaxValue no longer uses CdsDataProcessor
handleAttachmentForEntity is always called from inside an outer CdsDataProcessor converter, which already handles the traversal of deeply nested structures and resolves path.target().entity() to the specific attachment composition entity. By the time we need to read @Validation.Maximum, we already have the exact entity, so there's no need for a second CdsDataProcessor to walk data just to find a schema-level annotation. A direct entity.findElement("content").flatMap(e -> e.findAnnotation("Validation.Maximum")) is simpler and avoids the previous bug where the annotation wasn't found when the data list was empty.

@Schmarvinius Schmarvinius self-assigned this Mar 9, 2026
@Schmarvinius Schmarvinius merged commit c31590e into main Mar 9, 2026
12 checks passed
@Schmarvinius Schmarvinius deleted the bugfix/maxValNotFiringOnFirstUpload branch March 9, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants