Support inline single attachments via Attachment type - #768
Support inline single attachments via Attachment type#768busehalis-sap wants to merge 80 commits into
Conversation
SummaryThe following content is AI-generated and provides a summary of the pull request: Support Inline Single Attachments via
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR introduces inline single-attachment support via the Attachment type, which is a meaningful extension of the existing composition-based approach. However, there are several substantive correctness issues that need attention before merging: a recurring "first prefix wins" bug across CreateAttachmentEvent, MarkAsDeletedAttachmentEvent, and DefaultAttachmentsServiceHandler that will produce wrong behavior for entities with more than one inline attachment field; an inverted/dead branch in ModifyApplicationHandlerHelper for contentId resolution; missing root-entity key columns in AttachmentsReader when both inline and expand columns are present; and a conflation in ReadAttachmentsHandler.getAttachmentAssociations that incorrectly treats inline-only root entities as direct media association roots. These should be resolved before the feature is considered production-ready.
PR Bot Information
Version: 1.19.3 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback
- Event Trigger:
pull_request.opened - Correlation ID:
bbb28610-2bff-11f1-8fd9-1dd0dd2b50a5 - LLM:
anthropic--claude-4.6-sonnet
lisajulia
left a comment
lisajulia
left a comment
There was a problem hiding this comment.
Thanks! 👍
Please update the README file as well 📖 - then I'll approve ;)
8a97051 to
f041e27
Compare
…e-attachments # Conflicts: # cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java # cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/helper/ReadonlyDataContextEnhancerTest.java
…er mock The feature branch added an inlinePrefix parameter to EndTransactionMalwareScanProvider.getChangeSetListener. A test added on main was still using the 2-arg overload.
|
Blocked until bug resolved with cds-services! |
Adds extractFrom(Map<String, Object> values) to AttachmentContext, which extracts an Attachments view from raw data - either wrapping directly (composition) or stripping the prefix (inline). Eliminates the remaining if/else branching in: - DeleteAttachmentsHandler - ReadAttachmentsHandler - DraftCancelAttachmentsHandler Handlers now simply call attachmentCtx.extractFrom(path.target().values()) instead of branching on isInline() and manually calling extractInlineAttachment.
The parameter holds the attachment entity's own keys (for composition) or the parent entity's keys (for inline). Calling it 'parentKeys' was misleading for the composition case. 'keys' is neutral and accurate.
…to feature/support-single-attachments
…e entity is already deleted
… fix SpotBugs NCR_NOT_PROPERLY_CHECKED_READ
…AttachmentsHandler
… contentCanBeReadFromDraft to prevent flaky timeout
…nchanged in UpdateAttachmentsHandler
…1 pattern matching
…stInlineAttachmentMetadata
…O(n) contentId stream
| } | ||
|
|
||
| /** Context for composition-based attachments where the attachment is its own entity. */ | ||
| record Composition() implements AttachmentContext { |
There was a problem hiding this comment.
Records in java are used to represent value objects. Creating a record which does not hold any data is no idiomatic. A final class should be used instead.
| * Context for inline attachments where the structured type fields are flattened into the parent | ||
| * entity with a prefix (e.g. {@code "avatar_content"}, {@code "avatar_contentId"}). | ||
| */ | ||
| record Inline(String prefix) implements AttachmentContext { |
There was a problem hiding this comment.
Records in java are used to represent value objects. Creating a record which does not hold any data is no idiomatic. A final class should be used instead.
| * and write that attachment's fields in the data map, and how to match it against existing | ||
| * attachment data. | ||
| */ | ||
| public sealed interface AttachmentContext { |
There was a problem hiding this comment.
Naming: AttachmentContext is not a descriptive name to me.
Everything here is about attachments and "context" is a very generic term. There are also other things named context in this context...
From the subscription, it sounds as if it is a FieldAccessor.
MEDIA_CONTENT_FILTER only matches _content suffix elements, so each inline prefix triggers the validator exactly once — the seenPrefixes HashSet never had any effect.
persistInlineAttachmentMetadata was building a WHERE clause without IsActiveEntity, causing the UPDATE to match both draft and active shadow rows in the draft table. Added IsActiveEntity = false to ensure only the draft row is updated.
ae9e2aa to
930567a
Compare
…n media entity name collection
…ent fields Inline attachment fields (e.g. avatar_content) are flattened into the parent entity and are not reachable via findElement(content). Add fetchInlineAcceptableMediaTypes to scan _content-suffixed elements and register their annotations under entityName#prefix keys. Update AttachmentDataExtractor to collect inline file names under the same keys so per-field media type validation works for both composition and inline attachments.
4 participants
Summary
sap.attachments.Attachmentstructured type for single-file attachment fields directly on an entity (e.g.profileIcon : Attachment), complementing the existing composition-basedAttachmentsaspect for multi-file scenariosCDS Model Changes
type Attachment : MediaData {}for single-file use cases.managedaspect fromMediaDatatoAttachmentsso inline fields don't inheritcreatedBy/modifiedByon the parent entity.Core.ContentDisposition.FilenameandCore.ContentDisposition.Typeannotations.Core.MediaTypeannotation now uses path expression syntax(mimeType).Application Code Changes
Core detection logic (
ApplicationHandlerHelper) — New methods to discover inline attachment fields by scanning for flattenedprefix_contentelements with_is_media_dataannotations.MEDIA_CONTENT_FILTERnow matches both composition-based and inline attachment content fields.extractInlineAttachment()strips the prefix from flattened column names and tags results with an_inlinePrefixmarker for matching.condenseAttachments()extended to extract inline data from parent entity rows.Event processing (
ModifyAttachmentEventand implementations) — All event implementations (Create,Update,MarkAsDeleted,DoNothing) gain anOptional<String> inlinePrefixparameter. Create and delete events useresolveField()/ prefixed field names to read and write the correct columns.CreateAttachmentEventmanually extracts fileName/mimeType from HTTP headers for inline attachments since CDS flattening breaks path references.MarkAsDeletedAttachmentEventadditionally clears mimeType/fileName for inline types.Read path (
ReadAttachmentsHandler,BeforeReadItemsModifier) — Before-read now discovers inline prefixes and ensuresprefix_contentId/prefix_status/prefix_scannedAtcolumns are selected whenprefix_contentis explicitly queried. After-read extracts inline attachment data and feeds it through status verification and lazy proxy wrapping. Re-scan transitions use prefixed column names.Write path (
UpdateAttachmentsHandler,ModifyApplicationHandlerHelper) — Early-return optimization now also checks for inline attachment fields in the data. Size limit resolution falls back toprefix_contentelements. Existing attachment matching uses the prefix marker instead of entity keys for inline types.Draft handling (
DraftPatchAttachmentsHandler,DraftCancelAttachmentsHandler,ReadonlyDataContextEnhancer) — Draft patch extracts existing inline data from DB using prefixed columns and explicitly persists mimeType/fileName viaPersistenceService(the framework's DRAFT_PATCH handler only persists readonly fields). Draft cancel extended to detect and handle prefixedcontentIdelements. Readonly field backup/restore now supportsprefix_DRAFT_READONLY_CONTEXTkeys and preservesfileName.Composition tree traversal (
AssociationCascader,AttachmentsReader) — Entities with inline attachments are recorded as media entity paths but traversal continues into their compositions (unlike composition-based attachment entities which are leaf nodes). Attachment reader injects inline columns into both root selects and nested expand clauses.Malware scanning — All scanner interfaces and implementations (
AttachmentMalwareScanner,AsyncMalwareScanExecutor,EndTransactionMalwareScanRunner,DefaultAttachmentMalwareScanner) threadinlinePrefixthrough to build correct SELECT/UPDATE statements with prefixed column names.Minor changes —
MediaTypeService.resolveMimeType()returnsnullinstead ofapplication/octet-streamfor unknown types.MediaTypeResolverusesfindElement()to avoid NPE on entities without acontentelement.CreateAttachmentInputrecord gains aninlinePrefixfield.Unit Tests
ApplicationHandlerHelperTest,ReadAttachmentsHandlerTest,ModifyApplicationHandlerHelperTest,BeforeReadItemsModifierTest,DefaultAttachmentMalwareScannerTestfor inline detection, prefix resolution, size limits, read modifier, and scan with prefixed columnsCreateAttachmentEventTest(header extraction, prefixed writes, fallbacks) andMarkAsDeletedAttachmentEventTest(prefixed field clearing)ReadonlyDataContextEnhancerTestrewritten to cover both composition and inline preserve/restoreDraftCancelAttachmentsHandlerTestextended with inline cancel scenariosprofilePictureonRootsand a newInlineOnlyentityinlinePrefixparameterIntegration Tests
SingleAttachmentDraftTest(new) — Full draft lifecycle: create+activate, create+cancel, update+activate, delete+activate, delete+cancel, draft content reads, no-change re-activation, error rollback, Content-Disposition header persistence, update+cancel cleanupSingleAttachmentNonDraftTest(new) — Full non-draft lifecycle: CRUD on root-level inline attachments, multiple inline fields on same entity, inline on composition children, deep delete/update cascading, concurrent inline and composition-based attachments, header handling, size limit enforcement, error rollbackPipeline / General Changes
profileIcon/coverImageinline attachments and Fiori UI facets; bumped dependency versionsAttachmenttype, flattened column naming, UI annotations, and combining both patterns