From eb50898b30d426bfb68d9cf6d3618dfabc742a38 Mon Sep 17 00:00:00 2001 From: "Yashmeet ." Date: Fri, 26 Jun 2026 16:36:44 +0530 Subject: [PATCH 1/2] Fix issue in discarding newly created entities which has multi-level of nested children --- .gitignore | 9 + .../helper/AttachmentsHandlerUtils.java | 32 ++- .../com/sap/cds/sdm/persistence/DBQuery.java | 48 ++-- .../handler/SDMServiceGenericHandler.java | 27 +-- .../handler/SDMServiceGenericHandlerTest.java | 205 ++++++++++++++++++ 5 files changed, 275 insertions(+), 46 deletions(-) diff --git a/.gitignore b/.gitignore index 178799205..6bd8a1ae1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,9 @@ sdm/target/ +**/target/ .flattened-pom.xml node_modules/ +**/package-lock.json +**/src/gen/ # Compiled class file *.class @@ -22,6 +25,12 @@ node_modules/ *.zip *.tar.gz *.rar +*.hdbtable +*.hdbview +*.xml +*.json +*.sql +*.mtar # virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml hs_err_pid* diff --git a/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/helper/AttachmentsHandlerUtils.java b/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/helper/AttachmentsHandlerUtils.java index 0e42434f2..f5bc8ee93 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/helper/AttachmentsHandlerUtils.java +++ b/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/helper/AttachmentsHandlerUtils.java @@ -62,20 +62,30 @@ public static List getAttachmentEntityPaths( } /** - * Creates a mapping of attachment entity paths to their corresponding actual paths within the CDS - * model. + * Creates a mapping of direct attachment entity paths for the given CDS entity. * - *

This method analyzes both direct and nested attachment compositions within the given entity. - * It processes direct attachments that are immediate compositions of the entity, and also - * traverses nested compositions to find attachments in related entities. The resulting mapping - * provides a translation between logical attachment paths and their actual implementation paths. + *

This method processes only direct attachment compositions (immediate compositions of the + * entity that target sap.attachments.Attachments). Nested compositions are not traversed. * - * @param model the CDS model containing entity definitions and relationships - * @param entity the target CDS entity to analyze for attachment path mappings - * @param persistenceService the persistence service used for data access operations - * @return a map where keys are attachment entity paths and values are the corresponding actual - * paths, or an empty map if no attachments are found or if an error occurs during processing + * @param entity the target CDS entity to analyze for direct attachment path mappings + * @return a map where keys and values are the direct attachment entity paths, or an empty map if + * no direct attachments are found */ + public static Map getDirectAttachmentPathMapping(CdsEntity entity) { + logger.debug( + "Getting direct attachment path mapping for entity: {}", entity.getQualifiedName()); + Map pathMapping = new HashMap<>(); + entity + .compositions() + .forEach( + composition -> processDirectAttachmentComposition(entity, pathMapping, composition)); + logger.debug( + "Found {} direct attachment path mappings for entity: {}", + pathMapping.size(), + entity.getQualifiedName()); + return pathMapping; + } + public static Map getAttachmentPathMapping( CdsModel model, CdsEntity entity, PersistenceService persistenceService) { logger.debug("Getting attachment path mapping for entity: {}", entity.getQualifiedName()); diff --git a/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java b/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java index e2b56432e..b7323ba76 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java +++ b/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java @@ -568,37 +568,41 @@ public CmisDocument getuploadStatusForAttachment( String objectId, AttachmentReadEventContext context) { logger.debug("Fetching uploadStatus for objectId: {} from entity: {}", objectId, entity); - Optional attachmentEntity = context.getModel().findEntity(entity + "_drafts"); - CqnSelect q = - Select.from(attachmentEntity.get()) - .columns("uploadStatus") - .where(doc -> doc.get("objectId").eq(objectId)); - Result result = persistenceService.run(q); CmisDocument cmisDocument = new CmisDocument(); boolean isAttachmentFound = false; - for (Row row : result.list()) { - cmisDocument.setUploadStatus( - row.get("uploadStatus") != null - ? row.get("uploadStatus").toString() - : SDMConstants.UPLOAD_STATUS_IN_PROGRESS); - isAttachmentFound = true; - } - if (!isAttachmentFound) { - logger.debug( - "Attachment not found in draft table for objectId: {}, checking active entity: {}", - objectId, - entity); - attachmentEntity = context.getModel().findEntity(entity); - q = - Select.from(attachmentEntity.get()) + Optional draftEntityOpt = context.getModel().findEntity(entity + "_drafts"); + if (draftEntityOpt.isPresent()) { + CqnSelect q = + Select.from(draftEntityOpt.get()) .columns("uploadStatus") .where(doc -> doc.get("objectId").eq(objectId)); - result = persistenceService.run(q); + Result result = persistenceService.run(q); for (Row row : result.list()) { cmisDocument.setUploadStatus( row.get("uploadStatus") != null ? row.get("uploadStatus").toString() : SDMConstants.UPLOAD_STATUS_IN_PROGRESS); + isAttachmentFound = true; + } + } + if (!isAttachmentFound) { + logger.debug( + "Attachment not found in draft table for objectId: {}, checking active entity: {}", + objectId, + entity); + Optional activeEntityOpt = context.getModel().findEntity(entity); + if (activeEntityOpt.isPresent()) { + CqnSelect q = + Select.from(activeEntityOpt.get()) + .columns("uploadStatus") + .where(doc -> doc.get("objectId").eq(objectId)); + Result result = persistenceService.run(q); + for (Row row : result.list()) { + cmisDocument.setUploadStatus( + row.get("uploadStatus") != null + ? row.get("uploadStatus").toString() + : SDMConstants.UPLOAD_STATUS_IN_PROGRESS); + } } } logger.debug( diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java index 7d1c79caa..60708b671 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java @@ -187,18 +187,13 @@ public void handleDraftDiscardForLinks(DraftCancelEventContext context) throws I logger.debug("Processing draft cancel for entity: {}", parentEntityName); Optional parentActiveEntityOpt = context.getModel().findEntity(parentEntityName); - Map compositionPathMapping = - parentActiveEntityOpt - .map( - cdsEntity -> - AttachmentsHandlerUtils.getAttachmentPathMapping( - context.getModel(), cdsEntity, persistenceService)) - .orElse(new HashMap<>()); - - logger.debug("Found {} composition paths to process", compositionPathMapping.size()); - for (Map.Entry entry : compositionPathMapping.entrySet()) { - String attachmentCompositionDefinition = entry.getKey(); - revertLinksForComposition(context, parentKeys, attachmentCompositionDefinition); + if (parentActiveEntityOpt.isPresent()) { + Map compositionPathMapping = + AttachmentsHandlerUtils.getDirectAttachmentPathMapping(parentActiveEntityOpt.get()); + logger.debug("Found {} composition paths to process", compositionPathMapping.size()); + for (Map.Entry entry : compositionPathMapping.entrySet()) { + revertLinksForComposition(context, parentKeys, entry.getKey()); + } } revertNestedEntityLinks(context); logger.debug("END: Handle draft discard for links"); @@ -697,7 +692,13 @@ private void checkAttachmentConstraints( throws ServiceException { logger.debug("Checking attachment constraints for upID: {}", upID); CdsModel cdsModel = context.getModel(); - CdsEntity attachmentEntity = cdsModel.findEntity(context.getTarget().getQualifiedName()).get(); + CdsEntity attachmentEntity = + cdsModel + .findEntity(context.getTarget().getQualifiedName()) + .orElseThrow( + () -> + new ServiceException( + "Entity not found in model: " + context.getTarget().getQualifiedName())); // Fetch the row count for current repository Result result = diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java index 7a0d9f962..21dcb10f9 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java @@ -1797,6 +1797,211 @@ void testHandleDraftDiscardForLinks_CallsRevertNestedEntityLinks() throws IOExce } } + @Test + void testHandleDraftDiscardForLinks_OnlyDirectAttachmentsProcessedViaPathMapping() + throws IOException { + // Verifies that handleDraftDiscardForLinks uses getDirectAttachmentPathMapping (not + // getAttachmentPathMapping) on the root entity — i.e. only direct attachments on the root + // are processed via Path 1. Nested attachments are handled exclusively by + // revertNestedEntityLinks. + DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class); + CdsEntity parentDraftEntity = mock(CdsEntity.class); + CqnAnalyzer analyzer = mock(CqnAnalyzer.class); + AnalysisResult analysisResult = mock(AnalysisResult.class); + CqnDelete cqnDelete = mock(CqnDelete.class); + CdsEntity parentActiveEntity = mock(CdsEntity.class); + + when(draftContext.getTarget()).thenReturn(parentDraftEntity); + when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts"); + when(draftContext.getModel()).thenReturn(cdsModel); + when(draftContext.getCqn()).thenReturn(cqnDelete); + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer); + when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult); + when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123")); + when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.of(parentActiveEntity)); + when(parentActiveEntity.compositions()).thenReturn(Stream.empty()); + + try (var attachmentUtilsMock = + mockStatic( + com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) { + + attachmentUtilsMock + .when( + () -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity))) + .thenReturn(new HashMap<>()); + + sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext); + + // getDirectAttachmentPathMapping must be called on the root entity + attachmentUtilsMock.verify( + () -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)), + times(1)); + + // getAttachmentPathMapping must NOT be called on the root entity + attachmentUtilsMock.verify( + () -> + AttachmentsHandlerUtils.getAttachmentPathMapping( + eq(cdsModel), eq(parentActiveEntity), any()), + never()); + } + } + + @Test + void testHandleDraftDiscardForLinks_DirectAttachmentOnRootIsReverted() throws IOException { + // Verifies that when the root entity has a direct attachment composition, + // revertLinksForComposition is called for it via Path 1 (getDirectAttachmentPathMapping). + DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class); + CdsEntity parentDraftEntity = mock(CdsEntity.class); + CqnAnalyzer analyzer = mock(CqnAnalyzer.class); + AnalysisResult analysisResult = mock(AnalysisResult.class); + CqnDelete cqnDelete = mock(CqnDelete.class); + CdsEntity parentActiveEntity = mock(CdsEntity.class); + CdsEntity attachmentDraftEntity = mock(CdsEntity.class); + CdsEntity attachmentActiveEntity = mock(CdsEntity.class); + + when(draftContext.getTarget()).thenReturn(parentDraftEntity); + when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts"); + when(draftContext.getModel()).thenReturn(cdsModel); + when(draftContext.getCqn()).thenReturn(cqnDelete); + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer); + when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult); + when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123")); + when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.of(parentActiveEntity)); + when(parentActiveEntity.compositions()).thenReturn(Stream.empty()); + + // Direct attachment on root + Map directMapping = new HashMap<>(); + directMapping.put("AdminService.Books.attachments", "AdminService.Books.attachments"); + + when(cdsModel.findEntity("AdminService.Books.attachments_drafts")) + .thenReturn(Optional.of(attachmentDraftEntity)); + when(cdsModel.findEntity("AdminService.Books.attachments")) + .thenReturn(Optional.of(attachmentActiveEntity)); + + CdsElement upElement = mock(CdsElement.class); + when(attachmentDraftEntity.elements()).thenReturn(Stream.of(upElement)); + when(upElement.getName()).thenReturn("up__ID"); + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(attachmentDraftEntity)).thenReturn("up__ID"); + + Result emptyResult = mock(Result.class); + when(emptyResult.iterator()).thenReturn(Collections.emptyIterator()); + when(persistenceService.run(any(CqnSelect.class))).thenReturn(emptyResult); + + SDMCredentials sdmCredentials = mock(SDMCredentials.class); + UserInfo userInfo = mock(UserInfo.class); + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + when(draftContext.getUserInfo()).thenReturn(userInfo); + when(userInfo.isSystemUser()).thenReturn(false); + + try (var attachmentUtilsMock = + mockStatic( + com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) { + attachmentUtilsMock + .when( + () -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity))) + .thenReturn(directMapping); + + assertDoesNotThrow(() -> sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext)); + + // persistence was called — confirming revertLinksForComposition was entered for the direct + // attachment + verify(persistenceService, atLeastOnce()).run(any(CqnSelect.class)); + } + } + + @Test + void testHandleDraftDiscardForLinks_GrandchildAttachmentDoesNotCrash() throws IOException { + // Regression test for the bug: root → Chapters (no attachments) → Sections (has attachments). + // With the old code, getAttachmentPathMapping on root would construct a wrong entity name + // "AdminService.Chapters.attachments" causing NoSuchElementException. + // With the fix, getDirectAttachmentPathMapping returns empty for root (no direct attachments), + // and revertNestedEntityLinks correctly handles Sections via Chapters. + DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class); + CdsEntity parentDraftEntity = mock(CdsEntity.class); + CqnAnalyzer analyzer = mock(CqnAnalyzer.class); + AnalysisResult analysisResult = mock(AnalysisResult.class); + CqnDelete cqnDelete = mock(CqnDelete.class); + CdsEntity parentActiveEntity = mock(CdsEntity.class); + CdsElement chaptersComposition = mock(CdsElement.class); + CdsAssociationType chaptersAssocType = mock(CdsAssociationType.class); + CdsEntity chaptersEntity = mock(CdsEntity.class); + + when(draftContext.getTarget()).thenReturn(parentDraftEntity); + when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts"); + when(draftContext.getModel()).thenReturn(cdsModel); + when(draftContext.getCqn()).thenReturn(cqnDelete); + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer); + when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult); + when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123")); + when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.of(parentActiveEntity)); + + // Root has one composition: Chapters (no direct attachments) + when(parentActiveEntity.compositions()).thenReturn(Stream.of(chaptersComposition)); + when(chaptersComposition.getType()).thenReturn(chaptersAssocType); + when(chaptersAssocType.getTarget()).thenReturn(chaptersEntity); + when(chaptersEntity.getQualifiedName()).thenReturn("AdminService.Chapters"); + + // Chapters_drafts does not exist (simulates non-draft-enabled or grandchild scenario) + when(cdsModel.findEntity("AdminService.Chapters_drafts")).thenReturn(Optional.empty()); + + try (var attachmentUtilsMock = + mockStatic( + com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) { + + // Root has NO direct attachments + attachmentUtilsMock + .when( + () -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity))) + .thenReturn(new HashMap<>()); + + // Must not throw NoSuchElementException + assertDoesNotThrow(() -> sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext)); + + // getDirectAttachmentPathMapping called on root — not getAttachmentPathMapping + attachmentUtilsMock.verify( + () -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)), + times(1)); + attachmentUtilsMock.verify( + () -> + AttachmentsHandlerUtils.getAttachmentPathMapping( + eq(cdsModel), eq(parentActiveEntity), any()), + never()); + } + } + + @Test + void testHandleDraftDiscardForLinks_ActiveEntityNotFound_SkipsDirectAttachments() + throws IOException { + // When the active entity is not found in the model, no attachment processing should occur. + DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class); + CdsEntity parentDraftEntity = mock(CdsEntity.class); + CqnAnalyzer analyzer = mock(CqnAnalyzer.class); + AnalysisResult analysisResult = mock(AnalysisResult.class); + CqnDelete cqnDelete = mock(CqnDelete.class); + + when(draftContext.getTarget()).thenReturn(parentDraftEntity); + when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts"); + when(draftContext.getModel()).thenReturn(cdsModel); + when(draftContext.getCqn()).thenReturn(cqnDelete); + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer); + when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult); + when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123")); + when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.empty()); + + try (var attachmentUtilsMock = + mockStatic( + com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) { + + assertDoesNotThrow(() -> sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext)); + + // Neither method should be called since active entity is absent + attachmentUtilsMock.verify( + () -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(any()), never()); + attachmentUtilsMock.verify( + () -> AttachmentsHandlerUtils.getAttachmentPathMapping(any(), any(), any()), never()); + } + } + @Test void testRevertNestedEntityLinks_WithNullParentId() throws IOException { From a4d23c4529ec34b8d13614c7d9bc8ff50718d712 Mon Sep 17 00:00:00 2001 From: "Yashmeet ." Date: Mon, 29 Jun 2026 16:19:49 +0530 Subject: [PATCH 2/2] update pom version for testing --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8db2566c0..993d1f24f 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ - 1.8.1-SNAPSHOT + 1.0.0-RC1 21 ${java.version} ${java.version}