Open
Conversation
Contributor
There was a problem hiding this comment.
The changes look good. The new minMaxAttachments field is properly configured with validation annotations for minimum and maximum item counts, following the existing pattern in the file for other attachment fields.
PR Bot Information
Version: 1.17.37 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback
- Correlation ID:
4b033b70-fc58-11f0-87bd-be00e489c088 - Event Trigger:
pull_request.opened - LLM:
anthropic--claude-4.5-sonnet
…ers and integrating validation directly into event handlers
Contributor
There was a problem hiding this comment.
This pull request adds comprehensive attachment count validation functionality. The code quality is generally good with thorough testing. I identified a few minor issues around missing newlines and a potential logic improvement to help detect configuration errors.
PR Bot Information
Version: 1.17.37 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback
- Correlation ID:
0988cdf0-fd10-11f0-8de8-e0a44d83f4a8 - LLM:
anthropic--claude-4.5-sonnet - Event Trigger:
issue_comment.created
...s/src/main/java/com/sap/cds/feature/attachments/handler/common/AttachmentCountValidator.java
Show resolved
Hide resolved
...s/src/main/java/com/sap/cds/feature/attachments/handler/common/AttachmentCountValidator.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Validation Constraints for Book Attachments
✨ New Features
Added min/max item validation constraints (
@Validation.MinItemsand@Validation.MaxItems) to attachment compositions, enabling control over the number of attachments per entity. This feature validates attachment counts during create, update, and draft save operations to ensure data consistency.📝 Changes
Core Validation Logic
AttachmentCountValidator.java: New validator class implementing comprehensive attachment count validation:validateForCreate(): Validates both min and max constraints for non-draft create operationsvalidateForUpdate(): Validates constraints based on the new attachment count in the requestvalidateForDraftSave(): Validates constraints during draft activation by querying the draft service@Validation.MinItems,@Validation.MaxItems, or both annotationsMinimumAmountNotFulfilled,MaximumAmountExceeded)Handler Integration
Registration.java: Wired the newAttachmentCountValidatorinto create, update, and draft activation handlersCreateAttachmentsHandler.java: Integrated count validation before processing create operationsUpdateAttachmentsHandler.java: Added validation for update operations when compositions are modifiedDraftActiveAttachmentsHandler.java: Added validation during draft activation with late handler orderingError Messages
messages.properties: Added localized error messages for min/max validation failuresTest Coverage
AttachmentCountValidatorTest.java: Comprehensive unit tests covering all validation scenarios (150+ test cases)AttachmentCountValidationDraftTest.java: Integration tests for draft-enabled entitiesAttachmentCountValidationNonDraftTest.java: Integration tests for non-draft entitiesCountValidatedRootEntityBuilder.java: Test helper for creating test entities with count-validated attachmentsSample Application
attachments.cds: Demonstrated usage in bookshop sample with:attachmentscomposition with@Validation.MinItems: 2and@Validation.MaxItems: 3minMaxAttachmentscomposition for testingadmin-service.cds: AddedNonDraftAdminServicefor testing non-draft scenariosapplication.yaml: Enabled debug logging for the validatorCI/CD & Versioning
.github/workflows/pipeline.yml: Removed obsolete dry-run parameter for pull request buildspom.xml: Updated version to1.4.0-SNAPSHOTCode Quality Improvements
CountingInputStreamTest.java: Fixed resource management with try-with-resources patternThis enhancement extends the existing attachments functionality by demonstrating validation capabilities for collections, ensuring users provide an appropriate number of attachments when required. The implementation supports both draft and non-draft entities, with validation triggered at appropriate lifecycle events to maintain data consistency.