Skip to content

Commit eb50898

Browse files
yashmeet29yashmeet29
committed
Fix issue in discarding newly created entities which has multi-level of nested children
1 parent f56e35b commit eb50898

5 files changed

Lines changed: 275 additions & 46 deletions

File tree

‎.gitignore‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
sdm/target/
2+
**/target/
23
.flattened-pom.xml
34
node_modules/
5+
**/package-lock.json
6+
**/src/gen/
47

58
# Compiled class file
69
*.class
@@ -22,6 +25,12 @@ node_modules/
2225
*.zip
2326
*.tar.gz
2427
*.rar
28+
*.hdbtable
29+
*.hdbview
30+
*.xml
31+
*.json
32+
*.sql
33+
*.mtar
2534

2635
# virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml
2736
hs_err_pid*

‎sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/helper/AttachmentsHandlerUtils.java‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,20 +62,30 @@ public static List<String> getAttachmentEntityPaths(
6262
}
6363

6464
/**
65-
* Creates a mapping of attachment entity paths to their corresponding actual paths within the CDS
66-
* model.
65+
* Creates a mapping of direct attachment entity paths for the given CDS entity.
6766
*
68-
* <p>This method analyzes both direct and nested attachment compositions within the given entity.
69-
* It processes direct attachments that are immediate compositions of the entity, and also
70-
* traverses nested compositions to find attachments in related entities. The resulting mapping
71-
* provides a translation between logical attachment paths and their actual implementation paths.
67+
* <p>This method processes only direct attachment compositions (immediate compositions of the
68+
* entity that target sap.attachments.Attachments). Nested compositions are not traversed.
7269
*
73-
* @param model the CDS model containing entity definitions and relationships
74-
* @param entity the target CDS entity to analyze for attachment path mappings
75-
* @param persistenceService the persistence service used for data access operations
76-
* @return a map where keys are attachment entity paths and values are the corresponding actual
77-
* paths, or an empty map if no attachments are found or if an error occurs during processing
70+
* @param entity the target CDS entity to analyze for direct attachment path mappings
71+
* @return a map where keys and values are the direct attachment entity paths, or an empty map if
72+
* no direct attachments are found
7873
*/
74+
public static Map<String, String> getDirectAttachmentPathMapping(CdsEntity entity) {
75+
logger.debug(
76+
"Getting direct attachment path mapping for entity: {}", entity.getQualifiedName());
77+
Map<String, String> pathMapping = new HashMap<>();
78+
entity
79+
.compositions()
80+
.forEach(
81+
composition -> processDirectAttachmentComposition(entity, pathMapping, composition));
82+
logger.debug(
83+
"Found {} direct attachment path mappings for entity: {}",
84+
pathMapping.size(),
85+
entity.getQualifiedName());
86+
return pathMapping;
87+
}
88+
7989
public static Map<String, String> getAttachmentPathMapping(
8090
CdsModel model, CdsEntity entity, PersistenceService persistenceService) {
8191
logger.debug("Getting attachment path mapping for entity: {}", entity.getQualifiedName());

‎sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -568,37 +568,41 @@ public CmisDocument getuploadStatusForAttachment(
568568
String objectId,
569569
AttachmentReadEventContext context) {
570570
logger.debug("Fetching uploadStatus for objectId: {} from entity: {}", objectId, entity);
571-
Optional<CdsEntity> attachmentEntity = context.getModel().findEntity(entity + "_drafts");
572-
CqnSelect q =
573-
Select.from(attachmentEntity.get())
574-
.columns("uploadStatus")
575-
.where(doc -> doc.get("objectId").eq(objectId));
576-
Result result = persistenceService.run(q);
577571
CmisDocument cmisDocument = new CmisDocument();
578572
boolean isAttachmentFound = false;
579-
for (Row row : result.list()) {
580-
cmisDocument.setUploadStatus(
581-
row.get("uploadStatus") != null
582-
? row.get("uploadStatus").toString()
583-
: SDMConstants.UPLOAD_STATUS_IN_PROGRESS);
584-
isAttachmentFound = true;
585-
}
586-
if (!isAttachmentFound) {
587-
logger.debug(
588-
"Attachment not found in draft table for objectId: {}, checking active entity: {}",
589-
objectId,
590-
entity);
591-
attachmentEntity = context.getModel().findEntity(entity);
592-
q =
593-
Select.from(attachmentEntity.get())
573+
Optional<CdsEntity> draftEntityOpt = context.getModel().findEntity(entity + "_drafts");
574+
if (draftEntityOpt.isPresent()) {
575+
CqnSelect q =
576+
Select.from(draftEntityOpt.get())
594577
.columns("uploadStatus")
595578
.where(doc -> doc.get("objectId").eq(objectId));
596-
result = persistenceService.run(q);
579+
Result result = persistenceService.run(q);
597580
for (Row row : result.list()) {
598581
cmisDocument.setUploadStatus(
599582
row.get("uploadStatus") != null
600583
? row.get("uploadStatus").toString()
601584
: SDMConstants.UPLOAD_STATUS_IN_PROGRESS);
585+
isAttachmentFound = true;
586+
}
587+
}
588+
if (!isAttachmentFound) {
589+
logger.debug(
590+
"Attachment not found in draft table for objectId: {}, checking active entity: {}",
591+
objectId,
592+
entity);
593+
Optional<CdsEntity> activeEntityOpt = context.getModel().findEntity(entity);
594+
if (activeEntityOpt.isPresent()) {
595+
CqnSelect q =
596+
Select.from(activeEntityOpt.get())
597+
.columns("uploadStatus")
598+
.where(doc -> doc.get("objectId").eq(objectId));
599+
Result result = persistenceService.run(q);
600+
for (Row row : result.list()) {
601+
cmisDocument.setUploadStatus(
602+
row.get("uploadStatus") != null
603+
? row.get("uploadStatus").toString()
604+
: SDMConstants.UPLOAD_STATUS_IN_PROGRESS);
605+
}
602606
}
603607
}
604608
logger.debug(

‎sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -187,18 +187,13 @@ public void handleDraftDiscardForLinks(DraftCancelEventContext context) throws I
187187
logger.debug("Processing draft cancel for entity: {}", parentEntityName);
188188

189189
Optional<CdsEntity> parentActiveEntityOpt = context.getModel().findEntity(parentEntityName);
190-
Map<String, String> compositionPathMapping =
191-
parentActiveEntityOpt
192-
.map(
193-
cdsEntity ->
194-
AttachmentsHandlerUtils.getAttachmentPathMapping(
195-
context.getModel(), cdsEntity, persistenceService))
196-
.orElse(new HashMap<>());
197-
198-
logger.debug("Found {} composition paths to process", compositionPathMapping.size());
199-
for (Map.Entry<String, String> entry : compositionPathMapping.entrySet()) {
200-
String attachmentCompositionDefinition = entry.getKey();
201-
revertLinksForComposition(context, parentKeys, attachmentCompositionDefinition);
190+
if (parentActiveEntityOpt.isPresent()) {
191+
Map<String, String> compositionPathMapping =
192+
AttachmentsHandlerUtils.getDirectAttachmentPathMapping(parentActiveEntityOpt.get());
193+
logger.debug("Found {} composition paths to process", compositionPathMapping.size());
194+
for (Map.Entry<String, String> entry : compositionPathMapping.entrySet()) {
195+
revertLinksForComposition(context, parentKeys, entry.getKey());
196+
}
202197
}
203198
revertNestedEntityLinks(context);
204199
logger.debug("END: Handle draft discard for links");
@@ -697,7 +692,13 @@ private void checkAttachmentConstraints(
697692
throws ServiceException {
698693
logger.debug("Checking attachment constraints for upID: {}", upID);
699694
CdsModel cdsModel = context.getModel();
700-
CdsEntity attachmentEntity = cdsModel.findEntity(context.getTarget().getQualifiedName()).get();
695+
CdsEntity attachmentEntity =
696+
cdsModel
697+
.findEntity(context.getTarget().getQualifiedName())
698+
.orElseThrow(
699+
() ->
700+
new ServiceException(
701+
"Entity not found in model: " + context.getTarget().getQualifiedName()));
701702

702703
// Fetch the row count for current repository
703704
Result result =

‎sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,211 @@ void testHandleDraftDiscardForLinks_CallsRevertNestedEntityLinks() throws IOExce
17971797
}
17981798
}
17991799

1800+
@Test
1801+
void testHandleDraftDiscardForLinks_OnlyDirectAttachmentsProcessedViaPathMapping()
1802+
throws IOException {
1803+
// Verifies that handleDraftDiscardForLinks uses getDirectAttachmentPathMapping (not
1804+
// getAttachmentPathMapping) on the root entity — i.e. only direct attachments on the root
1805+
// are processed via Path 1. Nested attachments are handled exclusively by
1806+
// revertNestedEntityLinks.
1807+
DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class);
1808+
CdsEntity parentDraftEntity = mock(CdsEntity.class);
1809+
CqnAnalyzer analyzer = mock(CqnAnalyzer.class);
1810+
AnalysisResult analysisResult = mock(AnalysisResult.class);
1811+
CqnDelete cqnDelete = mock(CqnDelete.class);
1812+
CdsEntity parentActiveEntity = mock(CdsEntity.class);
1813+
1814+
when(draftContext.getTarget()).thenReturn(parentDraftEntity);
1815+
when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts");
1816+
when(draftContext.getModel()).thenReturn(cdsModel);
1817+
when(draftContext.getCqn()).thenReturn(cqnDelete);
1818+
cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer);
1819+
when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult);
1820+
when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123"));
1821+
when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.of(parentActiveEntity));
1822+
when(parentActiveEntity.compositions()).thenReturn(Stream.empty());
1823+
1824+
try (var attachmentUtilsMock =
1825+
mockStatic(
1826+
com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) {
1827+
1828+
attachmentUtilsMock
1829+
.when(
1830+
() -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)))
1831+
.thenReturn(new HashMap<>());
1832+
1833+
sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext);
1834+
1835+
// getDirectAttachmentPathMapping must be called on the root entity
1836+
attachmentUtilsMock.verify(
1837+
() -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)),
1838+
times(1));
1839+
1840+
// getAttachmentPathMapping must NOT be called on the root entity
1841+
attachmentUtilsMock.verify(
1842+
() ->
1843+
AttachmentsHandlerUtils.getAttachmentPathMapping(
1844+
eq(cdsModel), eq(parentActiveEntity), any()),
1845+
never());
1846+
}
1847+
}
1848+
1849+
@Test
1850+
void testHandleDraftDiscardForLinks_DirectAttachmentOnRootIsReverted() throws IOException {
1851+
// Verifies that when the root entity has a direct attachment composition,
1852+
// revertLinksForComposition is called for it via Path 1 (getDirectAttachmentPathMapping).
1853+
DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class);
1854+
CdsEntity parentDraftEntity = mock(CdsEntity.class);
1855+
CqnAnalyzer analyzer = mock(CqnAnalyzer.class);
1856+
AnalysisResult analysisResult = mock(AnalysisResult.class);
1857+
CqnDelete cqnDelete = mock(CqnDelete.class);
1858+
CdsEntity parentActiveEntity = mock(CdsEntity.class);
1859+
CdsEntity attachmentDraftEntity = mock(CdsEntity.class);
1860+
CdsEntity attachmentActiveEntity = mock(CdsEntity.class);
1861+
1862+
when(draftContext.getTarget()).thenReturn(parentDraftEntity);
1863+
when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts");
1864+
when(draftContext.getModel()).thenReturn(cdsModel);
1865+
when(draftContext.getCqn()).thenReturn(cqnDelete);
1866+
cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer);
1867+
when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult);
1868+
when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123"));
1869+
when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.of(parentActiveEntity));
1870+
when(parentActiveEntity.compositions()).thenReturn(Stream.empty());
1871+
1872+
// Direct attachment on root
1873+
Map<String, String> directMapping = new HashMap<>();
1874+
directMapping.put("AdminService.Books.attachments", "AdminService.Books.attachments");
1875+
1876+
when(cdsModel.findEntity("AdminService.Books.attachments_drafts"))
1877+
.thenReturn(Optional.of(attachmentDraftEntity));
1878+
when(cdsModel.findEntity("AdminService.Books.attachments"))
1879+
.thenReturn(Optional.of(attachmentActiveEntity));
1880+
1881+
CdsElement upElement = mock(CdsElement.class);
1882+
when(attachmentDraftEntity.elements()).thenReturn(Stream.of(upElement));
1883+
when(upElement.getName()).thenReturn("up__ID");
1884+
sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(attachmentDraftEntity)).thenReturn("up__ID");
1885+
1886+
Result emptyResult = mock(Result.class);
1887+
when(emptyResult.iterator()).thenReturn(Collections.emptyIterator());
1888+
when(persistenceService.run(any(CqnSelect.class))).thenReturn(emptyResult);
1889+
1890+
SDMCredentials sdmCredentials = mock(SDMCredentials.class);
1891+
UserInfo userInfo = mock(UserInfo.class);
1892+
when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials);
1893+
when(draftContext.getUserInfo()).thenReturn(userInfo);
1894+
when(userInfo.isSystemUser()).thenReturn(false);
1895+
1896+
try (var attachmentUtilsMock =
1897+
mockStatic(
1898+
com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) {
1899+
attachmentUtilsMock
1900+
.when(
1901+
() -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)))
1902+
.thenReturn(directMapping);
1903+
1904+
assertDoesNotThrow(() -> sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext));
1905+
1906+
// persistence was called — confirming revertLinksForComposition was entered for the direct
1907+
// attachment
1908+
verify(persistenceService, atLeastOnce()).run(any(CqnSelect.class));
1909+
}
1910+
}
1911+
1912+
@Test
1913+
void testHandleDraftDiscardForLinks_GrandchildAttachmentDoesNotCrash() throws IOException {
1914+
// Regression test for the bug: root → Chapters (no attachments) → Sections (has attachments).
1915+
// With the old code, getAttachmentPathMapping on root would construct a wrong entity name
1916+
// "AdminService.Chapters.attachments" causing NoSuchElementException.
1917+
// With the fix, getDirectAttachmentPathMapping returns empty for root (no direct attachments),
1918+
// and revertNestedEntityLinks correctly handles Sections via Chapters.
1919+
DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class);
1920+
CdsEntity parentDraftEntity = mock(CdsEntity.class);
1921+
CqnAnalyzer analyzer = mock(CqnAnalyzer.class);
1922+
AnalysisResult analysisResult = mock(AnalysisResult.class);
1923+
CqnDelete cqnDelete = mock(CqnDelete.class);
1924+
CdsEntity parentActiveEntity = mock(CdsEntity.class);
1925+
CdsElement chaptersComposition = mock(CdsElement.class);
1926+
CdsAssociationType chaptersAssocType = mock(CdsAssociationType.class);
1927+
CdsEntity chaptersEntity = mock(CdsEntity.class);
1928+
1929+
when(draftContext.getTarget()).thenReturn(parentDraftEntity);
1930+
when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts");
1931+
when(draftContext.getModel()).thenReturn(cdsModel);
1932+
when(draftContext.getCqn()).thenReturn(cqnDelete);
1933+
cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer);
1934+
when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult);
1935+
when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123"));
1936+
when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.of(parentActiveEntity));
1937+
1938+
// Root has one composition: Chapters (no direct attachments)
1939+
when(parentActiveEntity.compositions()).thenReturn(Stream.of(chaptersComposition));
1940+
when(chaptersComposition.getType()).thenReturn(chaptersAssocType);
1941+
when(chaptersAssocType.getTarget()).thenReturn(chaptersEntity);
1942+
when(chaptersEntity.getQualifiedName()).thenReturn("AdminService.Chapters");
1943+
1944+
// Chapters_drafts does not exist (simulates non-draft-enabled or grandchild scenario)
1945+
when(cdsModel.findEntity("AdminService.Chapters_drafts")).thenReturn(Optional.empty());
1946+
1947+
try (var attachmentUtilsMock =
1948+
mockStatic(
1949+
com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) {
1950+
1951+
// Root has NO direct attachments
1952+
attachmentUtilsMock
1953+
.when(
1954+
() -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)))
1955+
.thenReturn(new HashMap<>());
1956+
1957+
// Must not throw NoSuchElementException
1958+
assertDoesNotThrow(() -> sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext));
1959+
1960+
// getDirectAttachmentPathMapping called on root — not getAttachmentPathMapping
1961+
attachmentUtilsMock.verify(
1962+
() -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(eq(parentActiveEntity)),
1963+
times(1));
1964+
attachmentUtilsMock.verify(
1965+
() ->
1966+
AttachmentsHandlerUtils.getAttachmentPathMapping(
1967+
eq(cdsModel), eq(parentActiveEntity), any()),
1968+
never());
1969+
}
1970+
}
1971+
1972+
@Test
1973+
void testHandleDraftDiscardForLinks_ActiveEntityNotFound_SkipsDirectAttachments()
1974+
throws IOException {
1975+
// When the active entity is not found in the model, no attachment processing should occur.
1976+
DraftCancelEventContext draftContext = mock(DraftCancelEventContext.class);
1977+
CdsEntity parentDraftEntity = mock(CdsEntity.class);
1978+
CqnAnalyzer analyzer = mock(CqnAnalyzer.class);
1979+
AnalysisResult analysisResult = mock(AnalysisResult.class);
1980+
CqnDelete cqnDelete = mock(CqnDelete.class);
1981+
1982+
when(draftContext.getTarget()).thenReturn(parentDraftEntity);
1983+
when(parentDraftEntity.getQualifiedName()).thenReturn("AdminService.Books_drafts");
1984+
when(draftContext.getModel()).thenReturn(cdsModel);
1985+
when(draftContext.getCqn()).thenReturn(cqnDelete);
1986+
cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(analyzer);
1987+
when(analyzer.analyze(cqnDelete)).thenReturn(analysisResult);
1988+
when(analysisResult.rootKeys()).thenReturn(Map.of("ID", "book123"));
1989+
when(cdsModel.findEntity("AdminService.Books")).thenReturn(Optional.empty());
1990+
1991+
try (var attachmentUtilsMock =
1992+
mockStatic(
1993+
com.sap.cds.sdm.handler.applicationservice.helper.AttachmentsHandlerUtils.class)) {
1994+
1995+
assertDoesNotThrow(() -> sdmServiceGenericHandler.handleDraftDiscardForLinks(draftContext));
1996+
1997+
// Neither method should be called since active entity is absent
1998+
attachmentUtilsMock.verify(
1999+
() -> AttachmentsHandlerUtils.getDirectAttachmentPathMapping(any()), never());
2000+
attachmentUtilsMock.verify(
2001+
() -> AttachmentsHandlerUtils.getAttachmentPathMapping(any(), any(), any()), never());
2002+
}
2003+
}
2004+
18002005
@Test
18012006
void testRevertNestedEntityLinks_WithNullParentId() throws IOException {
18022007

0 commit comments

Comments
 (0)