From 439423f34dfe1b359a4ac15af274653f2c387b4f Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 29 Oct 2026 15:47:09 +0530 Subject: [PATCH 01/10] initial commit --- .../com/sap/cds/sdm/persistence/DBQuery.java | 43 ++++++++++++ .../handler/SDMServiceGenericHandler.java | 48 ++++++++++--- .../handler/SDMServiceGenericHandlerTest.java | 69 ++++++++++++++----- 3 files changed, 134 insertions(+), 26 deletions(-) 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 219c4f2e..546c46be 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 @@ -105,6 +105,49 @@ public CmisDocument getObjectIdForAttachmentID( return cmisDocument; } + public CmisDocument getObjectIdForAttachmentID( + CdsEntity attachmentEntity, + PersistenceService persistenceService, + String id, + String upIdKey, + String upId) { + logger.debug( + "Fetching objectId for attachment ID: {} upId: {} from entity: {}", + id, + upId, + attachmentEntity.getQualifiedName()); + CqnSelect q = + Select.from(attachmentEntity) + .columns( + "objectId", + "folderId", + "fileName", + "mimeType", + "contentId", + "linkUrl", + "uploadStatus") + .where(doc -> doc.get("ID").eq(id).and(doc.get(upIdKey).eq(upId))); + Result result = persistenceService.run(q); + Optional res = result.first(); + CmisDocument cmisDocument = new CmisDocument(); + if (res.isPresent()) { + logger.debug("Attachment found for ID: {} within parent scope", id); + Row row = res.get(); + cmisDocument.setObjectId(row.get("objectId").toString()); + cmisDocument.setFileName(row.get("fileName").toString()); + cmisDocument.setFolderId(row.get("folderId").toString()); + cmisDocument.setMimeType(row.get("mimeType").toString()); + cmisDocument.setContentId( + row.get("contentId") != null ? row.get("contentId").toString() : null); + cmisDocument.setUrl(row.get("linkUrl") != null ? row.get("linkUrl").toString() : null); + cmisDocument.setUploadStatus( + row.get("uploadStatus") != null ? row.get("uploadStatus").toString() : null); + } else { + logger.debug("No attachment found for ID: {} within parent scope (upId: {})", id, upId); + } + return cmisDocument; + } + public CmisDocument getAttachmentForObjectID( PersistenceService persistenceService, String id, AttachmentCopyEventContext context) { logger.debug("Fetching attachment for objectId: {}", id); 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 2ae1af56..7f05f082 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 @@ -79,26 +79,49 @@ public void changelog(AttachmentLogContext context) throws IOException { CqnAnalyzer cqnAnalyzer = CqnAnalyzer.create(cdsModel); - Optional attachmentEntity = + Optional attachmentDraftEntity = cdsModel.findEntity(context.getTarget().getQualifiedName() + "_drafts"); Map targetKeys = cqnAnalyzer.analyze((CqnSelect) context.get("cqn")).targetKeyValues(); - // get the objectId against the Id String id = targetKeys.get("ID").toString(); logger.debug("Fetching changelog for attachment ID: {}", id); - CmisDocument cmisDocument = - dbQuery.getObjectIdForAttachmentID(attachmentEntity.get(), persistenceService, id); + // Derive parent entity name and upIdKey to scope the DB lookup by parent record + String targetQualifiedName = context.getTarget().getQualifiedName(); + int lastDot = targetQualifiedName.lastIndexOf('.'); + String parentEntityName = lastDot > 0 ? targetQualifiedName.substring(0, lastDot) : null; + Optional parentEntityOpt = + parentEntityName != null ? cdsModel.findEntity(parentEntityName) : Optional.empty(); + String upIdKey = + attachmentDraftEntity.isPresent() + ? SDMUtils.getUpIdKey(attachmentDraftEntity.get()) + : "up__ID"; + + // fetchUPIDFromCQN extracts the parent record key from the navigation path in the CQN + String upId = + parentEntityOpt.isPresent() + ? SDMUtils.fetchUPIDFromCQN((CqnSelect) context.get("cqn"), parentEntityOpt.get()) + : null; + + if (upId == null || upId.isEmpty()) { + logger.warn("Cannot determine parent scope for changelog request on attachment ID: {}", id); + throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); + } + + CmisDocument cmisDocument = lookupAttachmentScoped(attachmentDraftEntity, id, upIdKey, upId); if (cmisDocument.getFileName() == null || cmisDocument.getFileName().isEmpty()) { - // open attachment is triggered on non-draft entity logger.debug("Draft entity returned empty fileName, fetching from active entity"); - attachmentEntity = cdsModel.findEntity(context.getTarget().getQualifiedName()); + Optional attachmentActiveEntity = + cdsModel.findEntity(context.getTarget().getQualifiedName()); + cmisDocument = lookupAttachmentScoped(attachmentActiveEntity, id, upIdKey, upId); + } - cmisDocument = - dbQuery.getObjectIdForAttachmentID(attachmentEntity.get(), persistenceService, id); + if (cmisDocument.getObjectId() == null || cmisDocument.getObjectId().isEmpty()) { + logger.warn("Attachment ID {} not found within authorized parent scope (upId: {})", id, upId); + throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); } SDMCredentials sdmCredentials = tokenHandler.getSDMCredentials(); @@ -114,6 +137,15 @@ public void changelog(AttachmentLogContext context) throws IOException { logger.debug("END: Changelog event"); } + private CmisDocument lookupAttachmentScoped( + Optional entityOpt, String id, String upIdKey, String upId) { + if (entityOpt.isEmpty()) { + return new CmisDocument(); + } + return dbQuery.getObjectIdForAttachmentID( + entityOpt.get(), persistenceService, id, upIdKey, upId); + } + @On(event = "copyAttachments") public void copyAttachments(EventContext context) throws IOException { logger.debug("START: Copy attachments event"); 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 4a0aa163..09ce3194 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 @@ -136,16 +136,25 @@ void testChangelogSuccess() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(false); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("MyService.MyEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("MyService.MyEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "test-id-123")) + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-123"); + + // Mock DB query (scoped by parent) + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "test-id-123", "up__ID", "parent-id-123")) .thenReturn(cmisDocument); // Mock token handler @@ -170,7 +179,9 @@ void testChangelogSuccess() throws IOException { && jsonResult.has("version"); })); - verify(dbQuery).getObjectIdForAttachmentID(draftEntity, persistenceService, "test-id-123"); + verify(dbQuery) + .getObjectIdForAttachmentID( + draftEntity, persistenceService, "test-id-123", "up__ID", "parent-id-123"); verify(tokenHandler).getSDMCredentials(); verify(sdmService).getChangeLog("object-123", sdmCredentials, false); } @@ -202,16 +213,25 @@ void testChangelogWithSystemUser() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(true); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("SystemService.SystemEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("SystemService.SystemEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "system-id-456")) + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-456"); + + // Mock DB query (scoped by parent) + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "system-id-456", "up__ID", "parent-id-456")) .thenReturn(cmisDocument); // Mock token handler @@ -283,16 +303,25 @@ void testChangelogServiceException() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(false); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("ErrorService.ErrorEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("ErrorService.ErrorEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "error-id-789")) + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-789"); + + // Mock DB query (scoped by parent) + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "error-id-789", "up__ID", "parent-id-789")) .thenReturn(cmisDocument); // Mock token handler @@ -337,31 +366,35 @@ void testChangelogWithNullObjectId() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(false); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("NullService.NullEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("NullService.NullEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query to return document with null objectId - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "null-object-id")) - .thenReturn(nullObjectIdDocument); - - // Mock token handler - when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("null-parent-id"); - // Mock SDM service - when(sdmService.getChangeLog(null, sdmCredentials, false)) - .thenThrow(new IllegalArgumentException("ObjectId cannot be null")); + // Mock DB query (scoped) to return document with null objectId + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "null-object-id", "up__ID", "null-parent-id")) + .thenReturn(nullObjectIdDocument); - // Act & Assert + // Act & Assert — handler throws ServiceException (FILE_NOT_FOUND) when objectId is null assertThrows( - IllegalArgumentException.class, + ServiceException.class, () -> { sdmServiceGenericHandler.changelog(mockLogContext); }); + + verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); } @Test From 4a0e7db3abe7bf9210b71ce6ade0b93c2fdd6e4f Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 29 Oct 2026 15:48:15 +0530 Subject: [PATCH 02/10] Update pom.xml --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2687ff68..0dcce049 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ - 1.10.1-SNAPSHOT + 1.0.0-RC1 21 ${java.version} ${java.version} From ed973c5b4b17982d771d0f12de9e66253d4e8a9a Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 29 Oct 2026 15:51:22 +0530 Subject: [PATCH 03/10] spotless --- .../handler/SDMServiceGenericHandler.java | 3 +- .../handler/SDMServiceGenericHandlerTest.java | 33 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) 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 7f05f082..8441d605 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 @@ -280,7 +280,8 @@ private void processNestedEntityComposition( Result nestedRecords = persistenceService.run( - Select.from(nestedDraftEntity.get()).where(e -> e.get("IsActiveEntity").eq(false))); + Select.from(nestedDraftEntity.get()) + .where(e -> e.get(upIdKey).eq(parentId).and(e.get("IsActiveEntity").eq(false)))); logger.debug("Found {} nested records to process", nestedRecords.rowCount()); for (Row nestedRecord : nestedRecords) { 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 09ce3194..885d2c1d 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 @@ -216,7 +216,8 @@ void testChangelogWithSystemUser() throws IOException { CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("SystemService.SystemEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); - when(cdsModel.findEntity("SystemService.SystemEntity")).thenReturn(Optional.of(mockParentEntity)); + when(cdsModel.findEntity("SystemService.SystemEntity")) + .thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); @@ -3349,7 +3350,10 @@ void testProcessNestedEntityComposition() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test @@ -3394,7 +3398,10 @@ void testProcessNestedEntityComposition_NoDraftEntity() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test @@ -3449,7 +3456,10 @@ void testProcessNestedEntityComposition_EmptyAttachmentMapping() throws Exceptio // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test @@ -3518,7 +3528,10 @@ void testProcessNestedEntityComposition_NoNestedRecords() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test @@ -3565,7 +3578,10 @@ void testProcessNestedEntityComposition_ExceptionHandling() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test and expect exception @@ -3715,7 +3731,10 @@ void testProcessNestedEntityComposition_MultipleAttachmentPaths() throws Excepti // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test From 7d2327c3d1851727917ed99d860a9ad3fce4f24d Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Mon, 2 Nov 2026 12:08:16 +0530 Subject: [PATCH 04/10] sdm acl check --- .../handler/SDMServiceGenericHandler.java | 35 +++- .../handler/SDMServiceGenericHandlerTest.java | 161 ++++++++++++++++-- 2 files changed, 175 insertions(+), 21 deletions(-) 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 8441d605..d8a809d8 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 @@ -126,6 +126,23 @@ public void changelog(AttachmentLogContext context) throws IOException { SDMCredentials sdmCredentials = tokenHandler.getSDMCredentials(); + try { + JSONObject objectResponse = + sdmService.getObject( + cmisDocument.getObjectId(), sdmCredentials, context.getUserInfo().isSystemUser()); + if (objectResponse == null) { + logger.warn("File not found in SDM for objectId: {}", cmisDocument.getObjectId()); + throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); + } + } catch (ServiceException e) { + if (e.getMessage() != null && e.getMessage().contains("User does not have required scope")) { + logger.warn( + "User not authorized to view changelog for objectId: {}", cmisDocument.getObjectId()); + throw new ServiceException(SDMUtils.getErrorMessage("USER_NOT_AUTHORISED_ERROR")); + } + throw e; + } + JSONObject jsonObject = sdmService.getChangeLog( cmisDocument.getObjectId(), sdmCredentials, context.getUserInfo().isSystemUser()); @@ -227,11 +244,12 @@ public void handleDraftDiscardForLinks(DraftCancelEventContext context) throws I revertLinksForComposition(context, parentKeys, entry.getKey()); } } - revertNestedEntityLinks(context); + revertNestedEntityLinks(context, parentKeys); logger.debug("END: Handle draft discard for links"); } - private void revertNestedEntityLinks(DraftCancelEventContext context) throws IOException { + private void revertNestedEntityLinks( + DraftCancelEventContext context, Map parentKeys) throws IOException { logger.debug("START: Revert nested entity links"); CdsEntity parentDraftEntity = context.getTarget(); @@ -246,7 +264,7 @@ private void revertNestedEntityLinks(DraftCancelEventContext context) throws IOE .forEach( composition -> { try { - processNestedEntityComposition(context, composition); + processNestedEntityComposition(context, composition, parentKeys); } catch (IOException e) { throw new RuntimeException(e); } @@ -256,7 +274,8 @@ private void revertNestedEntityLinks(DraftCancelEventContext context) throws IOE } private void processNestedEntityComposition( - DraftCancelEventContext context, CdsElement composition) throws IOException { + DraftCancelEventContext context, CdsElement composition, Map parentKeys) + throws IOException { logger.debug("Processing nested entity composition: {}", composition.getName()); CdsAssociationType associationType = (CdsAssociationType) composition.getType(); @@ -275,6 +294,14 @@ private void processNestedEntityComposition( return; } + String upIdKey = SDMUtils.getUpIdKey(nestedDraftEntity.get()); + if (upIdKey == null || upIdKey.isEmpty()) { + logger.debug("No upIdKey found for nested entity: {}, skipping", targetEntityName); + return; + } + String parentKeyName = upIdKey.replaceFirst("^up__", ""); + Object parentId = parentKeys.get(parentKeyName); + // Get the actual key field names from the entity instead of hardcoding "ID" List keyElementNames = getKeyElementNames(nestedDraftEntity.get()); 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 885d2c1d..97570f4b 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 @@ -160,6 +160,9 @@ void testChangelogSuccess() throws IOException { // Mock token handler when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock SDM ACL pre-check + when(sdmService.getObject("object-123", sdmCredentials, false)).thenReturn(new JSONObject()); + // Mock SDM service when(sdmService.getChangeLog("object-123", sdmCredentials, false)) .thenReturn(mockChangeLogResult); @@ -182,7 +185,7 @@ void testChangelogSuccess() throws IOException { verify(dbQuery) .getObjectIdForAttachmentID( draftEntity, persistenceService, "test-id-123", "up__ID", "parent-id-123"); - verify(tokenHandler).getSDMCredentials(); + verify(sdmService).getObject("object-123", sdmCredentials, false); verify(sdmService).getChangeLog("object-123", sdmCredentials, false); } @@ -238,6 +241,9 @@ void testChangelogWithSystemUser() throws IOException { // Mock token handler when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock SDM ACL pre-check + when(sdmService.getObject("system-object-456", sdmCredentials, true)).thenReturn(new JSONObject()); + // Mock SDM service when(sdmService.getChangeLog("system-object-456", sdmCredentials, true)) .thenReturn(mockChangeLogResult); @@ -328,6 +334,9 @@ void testChangelogServiceException() throws IOException { // Mock token handler when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock SDM ACL pre-check + when(sdmService.getObject("error-object-789", sdmCredentials, false)).thenReturn(new JSONObject()); + // Mock SDM service to throw ServiceException (runtime exception) when(sdmService.getChangeLog("error-object-789", sdmCredentials, false)) .thenThrow(new ServiceException("Network error")); @@ -342,6 +351,117 @@ void testChangelogServiceException() throws IOException { verify(sdmService).getChangeLog("error-object-789", sdmCredentials, false); } + @Test + void testChangelogSDMAccessDenied() throws IOException { + AttachmentLogContext mockLogContext = mock(AttachmentLogContext.class); + CqnAnalyzer mockCqnAnalyzer = mock(CqnAnalyzer.class); + AnalysisResult mockAnalysisResult = mock(AnalysisResult.class); + UserInfo mockUserInfo = mock(UserInfo.class); + CdsEntity mockTarget = mock(CdsEntity.class); + + Map targetKeys = new HashMap<>(); + targetKeys.put("ID", "denied-id-001"); + + cmisDocument.setFileName("denied-doc.pdf"); + cmisDocument.setObjectId("denied-object-001"); + + when(mockLogContext.getModel()).thenReturn(cdsModel); + when(mockLogContext.getTarget()).thenReturn(mockTarget); + when(mockTarget.getQualifiedName()).thenReturn("DeniedService.DeniedEntity.attachments"); + when(mockLogContext.get("cqn")).thenReturn(cqnSelect); + when(mockLogContext.getUserInfo()).thenReturn(mockUserInfo); + when(mockUserInfo.isSystemUser()).thenReturn(false); + + CdsEntity mockParentEntity = mock(CdsEntity.class); + when(cdsModel.findEntity("DeniedService.DeniedEntity.attachments_drafts")) + .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("DeniedService.DeniedEntity")) + .thenReturn(Optional.of(mockParentEntity)); + + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); + when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); + when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); + + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-001"); + + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "denied-id-001", "up__ID", "parent-id-001")) + .thenReturn(cmisDocument); + + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // getObject throws — user does not have required scope in SDM + when(sdmService.getObject("denied-object-001", sdmCredentials, false)) + .thenThrow(new ServiceException("User does not have required scope")); + + ServiceException ex = + assertThrows( + ServiceException.class, + () -> sdmServiceGenericHandler.changelog(mockLogContext)); + + assertTrue(ex.getMessage().contains("USER_NOT_AUTHORISED_ERROR") || + ex.getMessage().contains("not authorized") || + ex.getClass().getSimpleName().equals("ServiceException")); + verify(sdmService).getObject("denied-object-001", sdmCredentials, false); + verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); + } + + @Test + void testChangelogSDMObjectNotFound() throws IOException { + AttachmentLogContext mockLogContext = mock(AttachmentLogContext.class); + CqnAnalyzer mockCqnAnalyzer = mock(CqnAnalyzer.class); + AnalysisResult mockAnalysisResult = mock(AnalysisResult.class); + UserInfo mockUserInfo = mock(UserInfo.class); + CdsEntity mockTarget = mock(CdsEntity.class); + + Map targetKeys = new HashMap<>(); + targetKeys.put("ID", "missing-id-002"); + + cmisDocument.setFileName("missing-doc.pdf"); + cmisDocument.setObjectId("missing-object-002"); + + when(mockLogContext.getModel()).thenReturn(cdsModel); + when(mockLogContext.getTarget()).thenReturn(mockTarget); + when(mockTarget.getQualifiedName()).thenReturn("MissingService.MissingEntity.attachments"); + when(mockLogContext.get("cqn")).thenReturn(cqnSelect); + when(mockLogContext.getUserInfo()).thenReturn(mockUserInfo); + when(mockUserInfo.isSystemUser()).thenReturn(false); + + CdsEntity mockParentEntity = mock(CdsEntity.class); + when(cdsModel.findEntity("MissingService.MissingEntity.attachments_drafts")) + .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("MissingService.MissingEntity")) + .thenReturn(Optional.of(mockParentEntity)); + + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); + when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); + when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); + + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-002"); + + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "missing-id-002", "up__ID", "parent-id-002")) + .thenReturn(cmisDocument); + + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // getObject returns null — object does not exist in SDM + when(sdmService.getObject("missing-object-002", sdmCredentials, false)).thenReturn(null); + + assertThrows( + ServiceException.class, + () -> sdmServiceGenericHandler.changelog(mockLogContext)); + + verify(sdmService).getObject("missing-object-002", sdmCredentials, false); + verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); + } + @Test void testChangelogWithNullObjectId() throws IOException { // Arrange @@ -2409,13 +2529,13 @@ void testRevertNestedEntityLinks_MainFlow() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2453,13 +2573,13 @@ void testRevertNestedEntityLinks_MissingActiveEntity() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2487,13 +2607,13 @@ void testRevertNestedEntityLinks_EmptyCompositions() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2570,6 +2690,7 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { // Mock SDMUtils.getUpIdKey to return non-null value sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(attachmentDraftEntity)).thenReturn("up__ID"); + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(eq(nestedDraftEntity))).thenReturn("up__ID"); try (var attachmentUtilsMock = mockStatic( @@ -2588,13 +2709,13 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2637,7 +2758,7 @@ void testRevertNestedEntityLinks_ExceptionInProcessing() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); // Execute the test and expect RuntimeException to be thrown @@ -2645,7 +2766,7 @@ void testRevertNestedEntityLinks_ExceptionInProcessing() throws Exception { RuntimeException.class, () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3318,6 +3439,9 @@ void testProcessNestedEntityComposition() throws Exception { when(context.getUserInfo()).thenReturn(userInfo); when(userInfo.isSystemUser()).thenReturn(false); + // Mock upIdKey for nested draft entity + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(eq(nestedDraftEntity))).thenReturn("up__ID"); + // Mock the static method call try (var attachmentUtilsMock = mockStatic( @@ -3360,7 +3484,7 @@ void testProcessNestedEntityComposition() throws Exception { assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3408,7 +3532,7 @@ void testProcessNestedEntityComposition_NoDraftEntity() throws Exception { assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3466,7 +3590,7 @@ void testProcessNestedEntityComposition_EmptyAttachmentMapping() throws Exceptio assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3512,6 +3636,9 @@ void testProcessNestedEntityComposition_NoNestedRecords() throws Exception { Map attachmentMapping = new HashMap<>(); attachmentMapping.put("AdminService.Attachments", "path1"); + // Mock upIdKey for nested draft entity + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(eq(nestedDraftEntity))).thenReturn("up__ID"); + // Mock the static method call try (var attachmentUtilsMock = mockStatic( @@ -3538,7 +3665,7 @@ void testProcessNestedEntityComposition_NoNestedRecords() throws Exception { assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3589,7 +3716,7 @@ void testProcessNestedEntityComposition_ExceptionHandling() throws Exception { RuntimeException.class, () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3741,7 +3868,7 @@ void testProcessNestedEntityComposition_MultipleAttachmentPaths() throws Excepti assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); From 5a50df265c20c64189b78a951139ecc78b4df9fd Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Mon, 2 Nov 2026 13:34:38 +0530 Subject: [PATCH 05/10] spotless --- .../handler/SDMServiceGenericHandlerTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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 97570f4b..ca5ea2da 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 @@ -242,7 +242,8 @@ void testChangelogWithSystemUser() throws IOException { when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); // Mock SDM ACL pre-check - when(sdmService.getObject("system-object-456", sdmCredentials, true)).thenReturn(new JSONObject()); + when(sdmService.getObject("system-object-456", sdmCredentials, true)) + .thenReturn(new JSONObject()); // Mock SDM service when(sdmService.getChangeLog("system-object-456", sdmCredentials, true)) @@ -335,7 +336,8 @@ void testChangelogServiceException() throws IOException { when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); // Mock SDM ACL pre-check - when(sdmService.getObject("error-object-789", sdmCredentials, false)).thenReturn(new JSONObject()); + when(sdmService.getObject("error-object-789", sdmCredentials, false)) + .thenReturn(new JSONObject()); // Mock SDM service to throw ServiceException (runtime exception) when(sdmService.getChangeLog("error-object-789", sdmCredentials, false)) @@ -399,12 +401,12 @@ void testChangelogSDMAccessDenied() throws IOException { ServiceException ex = assertThrows( - ServiceException.class, - () -> sdmServiceGenericHandler.changelog(mockLogContext)); + ServiceException.class, () -> sdmServiceGenericHandler.changelog(mockLogContext)); - assertTrue(ex.getMessage().contains("USER_NOT_AUTHORISED_ERROR") || - ex.getMessage().contains("not authorized") || - ex.getClass().getSimpleName().equals("ServiceException")); + assertTrue( + ex.getMessage().contains("USER_NOT_AUTHORISED_ERROR") + || ex.getMessage().contains("not authorized") + || ex.getClass().getSimpleName().equals("ServiceException")); verify(sdmService).getObject("denied-object-001", sdmCredentials, false); verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); } @@ -454,9 +456,7 @@ void testChangelogSDMObjectNotFound() throws IOException { // getObject returns null — object does not exist in SDM when(sdmService.getObject("missing-object-002", sdmCredentials, false)).thenReturn(null); - assertThrows( - ServiceException.class, - () -> sdmServiceGenericHandler.changelog(mockLogContext)); + assertThrows(ServiceException.class, () -> sdmServiceGenericHandler.changelog(mockLogContext)); verify(sdmService).getObject("missing-object-002", sdmCredentials, false); verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); From 2d2183b1fab9f64f0a5e1efb9bec6b9f50a7edb7 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Mon, 2 Nov 2026 13:55:46 +0530 Subject: [PATCH 06/10] error message change --- .../java/com/sap/cds/sdm/constants/SDMErrorMessages.java | 2 ++ .../cds/sdm/service/handler/SDMServiceGenericHandler.java | 2 +- .../sdm/service/handler/SDMServiceGenericHandlerTest.java | 5 +---- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index 8ff266c2..4a47c36d 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -137,6 +137,8 @@ private SDMErrorMessages() { "\nPlease contact your administrator for assistance with any necessary adjustments."; public static final String MAX_COUNT_ERROR_MESSAGE = "Cannot upload more than %s attachments."; public static final String FETCH_CHANGELOG_ERROR = "Could not fetch the changelog"; + public static final String CHANGELOG_NOT_AUTHORISED_ERROR = + "You do not have the required permissions to view the changelog. Please contact your administrator for access."; public static final String FAILED_TO_MOVE_ATTACHMENT = "Failed to move attachment"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_PREFIX = "Invalid secondary properties detected: "; 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 d8a809d8..0b7e3a73 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 @@ -138,7 +138,7 @@ public void changelog(AttachmentLogContext context) throws IOException { if (e.getMessage() != null && e.getMessage().contains("User does not have required scope")) { logger.warn( "User not authorized to view changelog for objectId: {}", cmisDocument.getObjectId()); - throw new ServiceException(SDMUtils.getErrorMessage("USER_NOT_AUTHORISED_ERROR")); + throw new ServiceException(SDMUtils.getErrorMessage("CHANGELOG_NOT_AUTHORISED_ERROR")); } throw e; } 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 ca5ea2da..e59617af 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 @@ -403,10 +403,7 @@ void testChangelogSDMAccessDenied() throws IOException { assertThrows( ServiceException.class, () -> sdmServiceGenericHandler.changelog(mockLogContext)); - assertTrue( - ex.getMessage().contains("USER_NOT_AUTHORISED_ERROR") - || ex.getMessage().contains("not authorized") - || ex.getClass().getSimpleName().equals("ServiceException")); + assertTrue(ex.getMessage().contains("view the changelog")); verify(sdmService).getObject("denied-object-001", sdmCredentials, false); verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); } From e5153fc266f99d1bef05f4de278e88c25cbe4b2f Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Mon, 2 Nov 2026 14:13:01 +0530 Subject: [PATCH 07/10] fix message --- .../cds/sdm/service/handler/SDMServiceGenericHandler.java | 3 ++- .../sdm/service/handler/SDMServiceGenericHandlerTest.java | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) 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 0b7e3a73..48bb2901 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 @@ -135,7 +135,8 @@ public void changelog(AttachmentLogContext context) throws IOException { throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); } } catch (ServiceException e) { - if (e.getMessage() != null && e.getMessage().contains("User does not have required scope")) { + if (e.getMessage() != null + && e.getMessage().equals(SDMUtils.getErrorMessage("USER_NOT_AUTHORISED_ERROR"))) { logger.warn( "User not authorized to view changelog for objectId: {}", cmisDocument.getObjectId()); throw new ServiceException(SDMUtils.getErrorMessage("CHANGELOG_NOT_AUTHORISED_ERROR")); 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 e59617af..890e57b3 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 @@ -395,9 +395,10 @@ void testChangelogSDMAccessDenied() throws IOException { when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); - // getObject throws — user does not have required scope in SDM + // getObject throws USER_NOT_AUTHORISED_ERROR — same as SDMServiceImpl does on HTTP 403 + String unauthorisedMsg = SDMErrorMessages.USER_NOT_AUTHORISED_ERROR; when(sdmService.getObject("denied-object-001", sdmCredentials, false)) - .thenThrow(new ServiceException("User does not have required scope")); + .thenThrow(new ServiceException(unauthorisedMsg)); ServiceException ex = assertThrows( From 705ab8afec1cccdf013d6aa46ff6ac9de611742d Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Mon, 2 Nov 2026 17:19:15 +0530 Subject: [PATCH 08/10] integration test cases --- .../java/integration/com/sap/cds/sdm/Api.java | 35 +++++++ .../com/sap/cds/sdm/ApiInterface.java | 4 + .../integration/com/sap/cds/sdm/ApiMT.java | 31 +++++++ .../cds/sdm/IntegrationTest_SingleFacet.java | 93 +++++++++++++++++++ 4 files changed, 163 insertions(+) diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java b/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java index 212f67e4..502805d4 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java @@ -1354,6 +1354,41 @@ public Map fetchChangelog( } } + @Override + public String fetchChangelogRaw( + String appUrl, String entityName, String facetName, String entityID, String ID) + throws IOException { + String url = + "https://" + + appUrl + + "/odata/v4/" + + serviceName + + "/" + + entityName + + "(ID=" + + entityID + + ",IsActiveEntity=false)/" + + facetName + + "(up__ID=" + + entityID + + ",ID=" + + ID + + ",IsActiveEntity=false)/" + + serviceName + + ".changelog"; + + RequestBody body = RequestBody.create("{}", MediaType.parse("application/json")); + Request request = + new Request.Builder().url(url).addHeader("Authorization", token).post(body).build(); + + try (Response response = executeWithRetry(request)) { + String responseBody = response.body() != null ? response.body().string() : ""; + return response.code() + "|" + responseBody; + } catch (IOException e) { + throw new IOException("Error fetching changelog raw: " + e.getMessage(), e); + } + } + @Override public String createAttachmentInActive( String appUrl, String entityName, String facetName, String entityID) throws IOException { diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java index 0cbe54f1..49512bc2 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java @@ -139,6 +139,10 @@ public Map fetchChangelog( String appUrl, String entityName, String facetName, String entityID, String ID) throws IOException; + public String fetchChangelogRaw( + String appUrl, String entityName, String facetName, String entityID, String ID) + throws IOException; + public String downloadSelectedAttachments( String appUrl, String entityName, String facetName, String entityID, List ids) throws IOException; diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java index e0805f29..46804e11 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java @@ -1280,6 +1280,37 @@ public Map fetchChangelog( } } + @Override + public String fetchChangelogRaw( + String appUrl, String entityName, String facetName, String entityID, String ID) + throws IOException { + String url = + "https://" + + appUrl + + "/api/admin/" + + entityName + + "(ID=" + + entityID + + ",IsActiveEntity=false)/" + + facetName + + "(up__ID=" + + entityID + + ",ID=" + + ID + + ",IsActiveEntity=false)/AdminService.changelog"; + + RequestBody body = RequestBody.create("{}", MediaType.parse("application/json")); + Request request = + new Request.Builder().url(url).addHeader("Authorization", token).post(body).build(); + + try (Response response = executeWithRetry(request)) { + String responseBody = response.body() != null ? response.body().string() : ""; + return response.code() + "|" + responseBody; + } catch (IOException e) { + throw new IOException("Error fetching changelog raw: " + e.getMessage(), e); + } + } + @Override public String createAttachmentInActive( String appUrl, String entityName, String facetName, String entityID) throws IOException { diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java b/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java index 67286c04..8f302399 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java @@ -8705,4 +8705,97 @@ void testMoveAttachments_FromSdmFolder_WithSecondaryProperties_Preserved() throw api.deleteEntity(appUrl, entityName, targetEntity); api.deleteEntity(appUrl, entityName, sourceEntity); } + + @Test + @Order(104) + void testChangelogWithoutSDMRole() throws IOException { + System.out.println("Test (104): Changelog action denied for user without SDM role"); + + // Create entity and upload attachment using the privileged user + String entityId = api.createEntityDraft(appUrl, entityName, entityName2, srvpath); + assertNotNull(entityId, "Failed to create entity for changelog role test"); + + ClassLoader classLoader = getClass().getClassLoader(); + File file = new File(classLoader.getResource("sample.txt").getFile()); + assertTrue(file.exists(), "sample.txt should exist"); + + Map postData = new HashMap<>(); + postData.put("up__ID", entityId); + postData.put("mimeType", "text/plain"); + postData.put("createdAt", new Date().toString()); + postData.put("createdBy", "test@test.com"); + postData.put("modifiedBy", "test@test.com"); + + List createResponse = + api.createAttachment(appUrl, entityName, facetName, entityId, srvpath, postData, file); + assertEquals(2, createResponse.size()); + assertEquals("Attachment created", createResponse.get(0), "Attachment should be created"); + String attachmentId = createResponse.get(1); + + // Attempt changelog fetch using the no-role user + String raw = + apiNoRoles.fetchChangelogRaw(appUrl, entityName, facetName, entityId, attachmentId); + String[] parts = raw.split("\\|", 2); + int statusCode = Integer.parseInt(parts[0]); + String body = parts.length > 1 ? parts[1] : ""; + + assertTrue( + statusCode == 403 || statusCode == 500, + "Expected 403 or 500 for unauthorized changelog, got: " + statusCode); + assertTrue( + body.contains("view the changelog"), + "Error message should mention changelog access, got: " + body); + + // Clean up + api.deleteEntityDraft(appUrl, entityName, entityId); + } + + @Test + @Order(105) + void testChangelogForAttachmentOfDifferentEntity() throws IOException { + System.out.println( + "Test (105): Changelog action returns FILE_NOT_FOUND for attachment belonging to a different entity (IDOR guard)"); + + // Create two separate entities + String entityIdA = api.createEntityDraft(appUrl, entityName, entityName2, srvpath); + String entityIdB = api.createEntityDraft(appUrl, entityName, entityName2, srvpath); + assertNotNull(entityIdA, "Failed to create entity A"); + assertNotNull(entityIdB, "Failed to create entity B"); + + ClassLoader classLoader = getClass().getClassLoader(); + File file = new File(classLoader.getResource("sample.txt").getFile()); + assertTrue(file.exists(), "sample.txt should exist"); + + // Upload attachment to entity A + Map postData = new HashMap<>(); + postData.put("up__ID", entityIdA); + postData.put("mimeType", "text/plain"); + postData.put("createdAt", new Date().toString()); + postData.put("createdBy", "test@test.com"); + postData.put("modifiedBy", "test@test.com"); + + List createResponse = + api.createAttachment(appUrl, entityName, facetName, entityIdA, srvpath, postData, file); + assertEquals(2, createResponse.size()); + assertEquals("Attachment created", createResponse.get(0)); + String attachmentIdOfA = createResponse.get(1); + + // Request changelog for attachment of entity A, but scoped under entity B's navigation path + String raw = api.fetchChangelogRaw(appUrl, entityName, facetName, entityIdB, attachmentIdOfA); + String[] parts = raw.split("\\|", 2); + int statusCode = Integer.parseInt(parts[0]); + String body = parts.length > 1 ? parts[1] : ""; + + assertTrue( + statusCode == 404 || statusCode == 500, + "Expected 404 or 500 when attachment does not belong to the requested entity, got: " + + statusCode); + assertTrue( + body.contains("not found") || body.contains("Object not found"), + "Error message should indicate the attachment was not found, got: " + body); + + // Clean up + api.deleteEntityDraft(appUrl, entityName, entityIdA); + api.deleteEntityDraft(appUrl, entityName, entityIdB); + } } From c093c191df0f14516ff65948367e23da861892eb Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Tue, 3 Nov 2026 11:38:59 +0530 Subject: [PATCH 09/10] review fix --- .../handler/SDMServiceGenericHandler.java | 16 +++++++++-- .../cds/sdm/IntegrationTest_SingleFacet.java | 16 +++++++++-- .../handler/SDMServiceGenericHandlerTest.java | 28 ++++++++++++++++--- 3 files changed, 52 insertions(+), 8 deletions(-) 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 48bb2901..bfee568b 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 @@ -302,8 +302,13 @@ private void processNestedEntityComposition( } String parentKeyName = upIdKey.replaceFirst("^up__", ""); Object parentId = parentKeys.get(parentKeyName); - - // Get the actual key field names from the entity instead of hardcoding "ID" + if (parentId == null) { + logger.warn( + "parentId is null for upIdKey '{}' in entity '{}', skipping revert", + upIdKey, + targetEntityName); + return; + } List keyElementNames = getKeyElementNames(nestedDraftEntity.get()); Result nestedRecords = @@ -355,6 +360,13 @@ private void revertLinksForComposition( } String parentKeyName = upIdKey.replaceFirst("^up__", ""); Object parentId = parentKeys.get(parentKeyName); + if (parentId == null) { + logger.warn( + "parentId is null for upIdKey '{}', skipping link revert for: {}", + upIdKey, + attachmentCompositionDefinition); + return; + } CqnSelect selectDraftLinks = Select.from(draftEntity) diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java b/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java index 8f302399..0eb1729c 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java @@ -8736,7 +8736,13 @@ void testChangelogWithoutSDMRole() throws IOException { String raw = apiNoRoles.fetchChangelogRaw(appUrl, entityName, facetName, entityId, attachmentId); String[] parts = raw.split("\\|", 2); - int statusCode = Integer.parseInt(parts[0]); + int statusCode; + try { + statusCode = Integer.parseInt(parts[0]); + } catch (NumberFormatException e) { + fail("fetchChangelogRaw returned unexpected format (expected '|'): " + raw); + return; + } String body = parts.length > 1 ? parts[1] : ""; assertTrue( @@ -8783,7 +8789,13 @@ void testChangelogForAttachmentOfDifferentEntity() throws IOException { // Request changelog for attachment of entity A, but scoped under entity B's navigation path String raw = api.fetchChangelogRaw(appUrl, entityName, facetName, entityIdB, attachmentIdOfA); String[] parts = raw.split("\\|", 2); - int statusCode = Integer.parseInt(parts[0]); + int statusCode; + try { + statusCode = Integer.parseInt(parts[0]); + } catch (NumberFormatException e) { + fail("fetchChangelogRaw returned unexpected format (expected '|'): " + raw); + return; + } String body = parts.length > 1 ? parts[1] : ""; assertTrue( 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 890e57b3..cc36bfd5 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 @@ -2655,6 +2655,12 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { when(nestedRecordsResult.iterator()).thenReturn(Arrays.asList(nestedRecord).iterator()); when(nestedRecord.get("ID")).thenReturn("chapter1"); + // Mock nestedDraftEntity key element so nestedEntityKeys gets "ID" populated + CdsElement nestedKeyElement = mock(CdsElement.class); + when(nestedKeyElement.isKey()).thenReturn(true); + when(nestedKeyElement.getName()).thenReturn("ID"); + when(nestedDraftEntity.elements()).thenReturn(Stream.of(nestedKeyElement)); + Map attachmentMapping = new HashMap<>(); attachmentMapping.put("AdminService.Attachments", "path1"); @@ -2713,7 +2719,9 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-book-id"); + method.invoke(sdmServiceGenericHandler, context, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3482,7 +3490,9 @@ void testProcessNestedEntityComposition() throws Exception { assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-entity-id"); + method.invoke(sdmServiceGenericHandler, context, composition, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3663,7 +3673,9 @@ void testProcessNestedEntityComposition_NoNestedRecords() throws Exception { assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-entity-id"); + method.invoke(sdmServiceGenericHandler, context, composition, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3748,6 +3760,12 @@ void testProcessNestedEntityComposition_MultipleAttachmentPaths() throws Excepti when(nestedRecordsResult.iterator()).thenReturn(Arrays.asList(nestedRecord).iterator()); when(nestedRecord.get("ID")).thenReturn("chapter1"); + // Mock nestedDraftEntity key element so nestedEntityKeys gets "ID" populated + CdsElement nestedKeyElement = mock(CdsElement.class); + when(nestedKeyElement.isKey()).thenReturn(true); + when(nestedKeyElement.getName()).thenReturn("ID"); + when(nestedDraftEntity.elements()).thenReturn(Stream.of(nestedKeyElement)); + // Mock multiple attachment paths Map attachmentMapping = new HashMap<>(); attachmentMapping.put("AdminService.ChapterAttachments", "path1"); @@ -3866,7 +3884,9 @@ void testProcessNestedEntityComposition_MultipleAttachmentPaths() throws Excepti assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-entity-id"); + method.invoke(sdmServiceGenericHandler, context, composition, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); From b202d202e4e03cc182d523c3df158e5cc5cb5766 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Tue, 3 Nov 2026 11:41:31 +0530 Subject: [PATCH 10/10] Update pom.xml --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0dcce049..2687ff68 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ - 1.0.0-RC1 + 1.10.1-SNAPSHOT 21 ${java.version} ${java.version}