Skip to content

Commit 20aefb8

Browse files
Changes
1 parent a7fe969 commit 20aefb8

4 files changed

Lines changed: 160 additions & 2 deletions

File tree

sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandler.java

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,16 @@ public void processBefore(CdsReadEventContext context) throws IOException {
127127
String upIdKey = SDMUtils.getUpIdKey(attachmentDraftEntity.get());
128128
CqnSelect select = (CqnSelect) context.get("cqn");
129129
String upID = SDMUtils.fetchUPIDFromCQN(select, attachmentDraftEntity.get());
130-
dbQuery.deleteDraftEntriesWithNullObjectIdAndFolderId(
131-
attachmentDraftEntity.get(), persistenceService, upID, upIdKey);
130+
131+
// Only delete attachments with null objectId and uploading status if this is NOT a
132+
// single entity read (i.e., doesn't have attachment ID in the keys)
133+
// This prevents 404 errors when fetching a specific attachment that's being deleted
134+
boolean isSingleEntityRead = isSingleAttachmentRead(context.getCqn());
135+
if (!isSingleEntityRead) {
136+
dbQuery.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
137+
attachmentDraftEntity.get(), persistenceService, upID, upIdKey);
138+
}
139+
132140
if (!repoValue.getIsAsyncVirusScanEnabled()) {
133141

134142
dbQuery.updateInProgressUploadStatusToSuccess(
@@ -342,4 +350,30 @@ private void processAttachmentVirusScanStatus(
342350
e.getMessage());
343351
}
344352
}
353+
354+
/**
355+
* Checks if the CQN select is for a single attachment entity (has ID in keys).
356+
*
357+
* @param select the CQN select to check
358+
* @return true if this is a single entity read with ID specified, false otherwise
359+
*/
360+
private boolean isSingleAttachmentRead(CqnSelect select) {
361+
try {
362+
// Check if the select has keys (filters by ID)
363+
// A single entity read will have ID specified in the where clause
364+
if (select.ref() != null && select.ref().segments() != null) {
365+
return select.ref().segments().stream()
366+
.anyMatch(
367+
segment ->
368+
segment.keys() != null
369+
&& !segment.keys().isEmpty()
370+
&& segment.keys().stream().anyMatch(key -> "ID".equals(key.ref())));
371+
}
372+
return false;
373+
} catch (Exception e) {
374+
// If we can't determine, assume it's not a single entity read to be safe
375+
logger.debug("Could not determine if single entity read: {}", e.getMessage());
376+
return false;
377+
}
378+
}
345379
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,38 @@ private List<CmisDocument> mapResultToCmisDocuments(Result result) {
579579
return documents;
580580
}
581581

582+
/**
583+
* Deletes draft entries from the attachment entity where objectId is null and uploadStatus is
584+
* 'uploading'. This is used to clean up incomplete upload entries when the application is
585+
* refreshed.
586+
*
587+
* @param attachmentEntity the draft attachment entity to delete from
588+
* @param persistenceService the persistence service to use for database operations
589+
* @param upID the up__ID to filter attachments
590+
* @param upIdKey the key name for up__ID field (e.g., "up__ID")
591+
*/
592+
public void deleteAttachmentsWithNullObjectIdAndUploadingStatus(
593+
CdsEntity attachmentEntity,
594+
PersistenceService persistenceService,
595+
String upID,
596+
String upIdKey) {
597+
var deleteQuery =
598+
Delete.from(attachmentEntity)
599+
.where(
600+
doc ->
601+
doc.get(upIdKey)
602+
.eq(upID)
603+
.and(doc.get("objectId").isNull())
604+
.and(doc.get("uploadStatus").eq(SDMConstants.UPLOAD_STATUS_IN_PROGRESS)));
605+
Result result = persistenceService.run(deleteQuery);
606+
if (result.rowCount() > 0) {
607+
logger.info(
608+
"Deleted {} attachment(s) with null objectId and uploading status for upID: {}",
609+
result.rowCount(),
610+
upID);
611+
}
612+
}
613+
582614
/**
583615
* Deletes draft entries from the attachment entity where both objectId and folderId are null.
584616
* This is used to clean up failed or incomplete upload entries.

sdm/src/test/java/unit/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandlerTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ void testModifyCqnForAttachmentsEntity_Success() throws IOException {
6666
sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(any())).thenReturn("mockUpIdKey");
6767
sdmUtilsMock.when(() -> SDMUtils.fetchUPIDFromCQN(any(), any())).thenReturn("mockUpID");
6868

69+
doNothing()
70+
.when(dbQuery)
71+
.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
72+
any(), any(), anyString(), anyString());
6973
doNothing()
7074
.when(dbQuery)
7175
.updateInProgressUploadStatusToSuccess(any(), any(), anyString(), anyString());
@@ -75,6 +79,9 @@ void testModifyCqnForAttachmentsEntity_Success() throws IOException {
7579

7680
// Assert
7781
verify(context).setCqn(any(CqnSelect.class));
82+
verify(dbQuery)
83+
.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
84+
any(), any(), eq("mockUpID"), eq("mockUpIdKey"));
7885
verify(dbQuery)
7986
.updateInProgressUploadStatusToSuccess(any(), any(), eq("mockUpID"), eq("mockUpIdKey"));
8087
}
@@ -249,4 +256,55 @@ void testProcessBefore_DeleteDraftEntriesWithNullObjectIdAndFolderId() throws IO
249256
verify(context).setCqn(any(CqnSelect.class));
250257
}
251258
}
259+
260+
@Test
261+
void testProcessBefore_SingleEntityRead_NoDelete() throws IOException {
262+
// Arrange - simulate a single entity read with ID in keys
263+
CqnSelect select = Mockito.mock(CqnSelect.class);
264+
when(context.getTarget()).thenReturn(cdsEntity);
265+
when(cdsEntity.getAnnotationValue(SDMConstants.ANNOTATION_IS_MEDIA_DATA, false))
266+
.thenReturn(true);
267+
when(context.getCqn()).thenReturn(select);
268+
when(context.get("cqn")).thenReturn(select);
269+
270+
// Mock select to appear as single entity read (has ID in keys)
271+
when(select.ref()).thenReturn(null); // Simplified - in reality would have segment with ID key
272+
273+
RepoValue repoValue = new RepoValue();
274+
repoValue.setIsAsyncVirusScanEnabled(false);
275+
when(sdmService.checkRepositoryType(any(), any())).thenReturn(repoValue);
276+
when(context.getUserInfo()).thenReturn(userInfo);
277+
when(userInfo.getTenant()).thenReturn("tenant1");
278+
279+
CdsEntity attachmentDraftEntity = Mockito.mock(CdsEntity.class);
280+
CdsModel model = Mockito.mock(CdsModel.class);
281+
when(context.getModel()).thenReturn(model);
282+
when(model.findEntity(anyString())).thenReturn(Optional.of(attachmentDraftEntity));
283+
when(cdsEntity.getQualifiedName()).thenReturn("TestEntity");
284+
285+
try (MockedStatic<SDMUtils> sdmUtilsMock = Mockito.mockStatic(SDMUtils.class)) {
286+
sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(any())).thenReturn("mockUpIdKey");
287+
sdmUtilsMock.when(() -> SDMUtils.fetchUPIDFromCQN(any(), any())).thenReturn("mockUpID");
288+
289+
doNothing()
290+
.when(dbQuery)
291+
.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
292+
any(), any(), anyString(), anyString());
293+
doNothing()
294+
.when(dbQuery)
295+
.updateInProgressUploadStatusToSuccess(any(), any(), anyString(), anyString());
296+
297+
// Act
298+
sdmReadAttachmentsHandler.processBefore(context);
299+
300+
// Assert - deleteAttachmentsWithNullObjectIdAndUploadingStatus should NOT be called for
301+
// single entity reads
302+
verify(dbQuery, never())
303+
.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
304+
any(), any(), anyString(), anyString());
305+
verify(dbQuery)
306+
.updateInProgressUploadStatusToSuccess(any(), any(), eq("mockUpID"), eq("mockUpIdKey"));
307+
verify(context).setCqn(any(CqnSelect.class));
308+
}
309+
}
252310
}

sdm/src/test/java/unit/com/sap/cds/sdm/persistence/DBQueryTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,40 @@ void setUp() {
3737
dbQuery = DBQuery.getDBQueryInstance();
3838
}
3939

40+
@Test
41+
void testDeleteAttachmentsWithNullObjectIdAndUploadingStatus_Success() {
42+
// Arrange
43+
String upID = "testUpID";
44+
String upIdKey = "up__ID";
45+
when(mockResult.rowCount()).thenReturn(3L);
46+
when(mockPersistenceService.run(any(CqnDelete.class))).thenReturn(mockResult);
47+
48+
// Act
49+
dbQuery.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
50+
mockDraftEntity, mockPersistenceService, upID, upIdKey);
51+
52+
// Assert
53+
verify(mockPersistenceService).run(any(CqnDelete.class));
54+
verify(mockResult).rowCount();
55+
}
56+
57+
@Test
58+
void testDeleteAttachmentsWithNullObjectIdAndUploadingStatus_NoRecordsDeleted() {
59+
// Arrange
60+
String upID = "testUpID";
61+
String upIdKey = "up__ID";
62+
when(mockResult.rowCount()).thenReturn(0L);
63+
when(mockPersistenceService.run(any(CqnDelete.class))).thenReturn(mockResult);
64+
65+
// Act
66+
dbQuery.deleteAttachmentsWithNullObjectIdAndUploadingStatus(
67+
mockDraftEntity, mockPersistenceService, upID, upIdKey);
68+
69+
// Assert
70+
verify(mockPersistenceService).run(any(CqnDelete.class));
71+
verify(mockResult).rowCount();
72+
}
73+
4074
@Test
4175
void testDeleteDraftEntriesWithNullObjectIdAndFolderId_Success() {
4276
// Arrange

0 commit comments

Comments
 (0)