Fix queries to conditionally include IsActiveEntity for non-draft entities

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
+## Version 1.9.4
+
+### Fixed
+- Fix `element does not exist` error when querying attachments on non-draft entities. Queries in `getAttachmentsForUPID`, `getAttachmentsForUPIDAndRepository`, and `getAttachmentsForFolder` now conditionally include the `IsActiveEntity` column only when the field is present on the attachment entity.
+
 ## Version 1.9.3
 
 ### Fixed

pom.xml

@@ -23,7 +23,7 @@
     </developers>
 
     <properties>
-        <revision>1.9.3</revision>
+        <revision>1.9.4</revision>
         <java.version>17</java.version>
         <maven.compiler.source>${java.version}</maven.compiler.source>
         <maven.compiler.target>${java.version}</maven.compiler.target>

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

@@ -50,9 +50,15 @@ public Result getAttachmentsForUPID(
         upID,
         upIdKey,
         attachmentEntity.getQualifiedName());
+    List<String> columns =
+        new ArrayList<>(
+            java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId", "mimeType"));
+    if (attachmentEntity.findElement("IsActiveEntity").isPresent()) {
+      columns.add("IsActiveEntity");
+    }
     CqnSelect q =
         Select.from(attachmentEntity)
-            .columns("fileName", "ID", "IsActiveEntity", "folderId", "repositoryId", "mimeType")
+            .columns(columns.toArray(new String[0]))
             .where(doc -> doc.get(upIdKey).eq(upID));
     Result result = persistenceService.run(q);
     logger.debug("Found {} attachment(s) for upID: {}", result.rowCount(), upID);
@@ -376,9 +382,14 @@ public Result getAttachmentsForUPIDAndRepository(
         upID,
         SDMConstants.REPOSITORY_ID,
         attachmentEntity.getQualifiedName());
+    List<String> columns =
+        new ArrayList<>(java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId"));
+    if (attachmentEntity.findElement("IsActiveEntity").isPresent()) {
+      columns.add("IsActiveEntity");
+    }
     CqnSelect q =
         Select.from(attachmentEntity)
-            .columns("fileName", "ID", "IsActiveEntity", "folderId", "repositoryId")
+            .columns(columns.toArray(new String[0]))
             .where(
                 doc ->
                     doc.get(upIdKey)
@@ -477,16 +488,17 @@ public List<CmisDocument> getAttachmentsForFolder(
     logger.debug("Fetching attachments for folderId: {} from entity: {}", folderId, entity);
     Optional<CdsEntity> attachmentEntity = context.getModel().findEntity(entity + "_drafts");
     List<CmisDocument> cmisDocuments = new ArrayList<>();
+    List<String> draftColumns =
+        new ArrayList<>(
+            java.util.Arrays.asList(
+                "fileName", "ID", "folderId", "repositoryId", "objectId", "uploadStatus"));
+    if (attachmentEntity.isPresent()
+        && attachmentEntity.get().findElement("IsActiveEntity").isPresent()) {
+      draftColumns.add("IsActiveEntity");
+    }
     CqnSelect q =
         Select.from(attachmentEntity.get())
-            .columns(
-                "fileName",
-                "IsActiveEntity",
-                "ID",
-                "folderId",
-                "repositoryId",
-                "objectId",
-                "uploadStatus")
+            .columns(draftColumns.toArray(new String[0]))
             .where(doc -> doc.get("folderId").eq(folderId));
     Result result = persistenceService.run(q);
     for (Row row : result.list()) {
@@ -508,16 +520,17 @@ public List<CmisDocument> getAttachmentsForFolder(
           folderId,
           entity);
       attachmentEntity = context.getModel().findEntity(entity);
+      List<String> activeColumns =
+          new ArrayList<>(
+              java.util.Arrays.asList(
+                  "fileName", "ID", "folderId", "repositoryId", "objectId", "uploadStatus"));
+      if (attachmentEntity.isPresent()
+          && attachmentEntity.get().findElement("IsActiveEntity").isPresent()) {
+        activeColumns.add("IsActiveEntity");
+      }
       q =
           Select.from(attachmentEntity.get())
-              .columns(
-                  "fileName",
-                  "IsActiveEntity",
-                  "ID",
-                  "folderId",
-                  "repositoryId",
-                  "objectId",
-                  "uploadStatus")
+              .columns(activeColumns.toArray(new String[0]))
               .where(doc -> doc.get("folderId").eq(folderId));
       result = persistenceService.run(q);
       for (Row row : result.list()) {

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

@@ -6,14 +6,18 @@
 
 import com.sap.cds.Result;
 import com.sap.cds.Row;
+import com.sap.cds.feature.attachments.service.model.servicehandler.AttachmentMarkAsDeletedEventContext;
 import com.sap.cds.ql.cqn.CqnSelect;
 import com.sap.cds.ql.cqn.CqnUpdate;
+import com.sap.cds.reflect.CdsElement;
 import com.sap.cds.reflect.CdsEntity;
+import com.sap.cds.reflect.CdsModel;
 import com.sap.cds.sdm.constants.SDMConstants;
 import com.sap.cds.sdm.model.CmisDocument;
 import com.sap.cds.sdm.persistence.DBQuery;
 import com.sap.cds.services.persistence.PersistenceService;
 import java.util.List;
+import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -25,6 +29,9 @@ class DBQueryTest {
 
   @Mock private CdsEntity mockDraftEntity;
   @Mock private CdsEntity mockActiveEntity;
+  @Mock private CdsElement mockCdsElement;
+  @Mock private CdsModel mockCdsModel;
+  @Mock private AttachmentMarkAsDeletedEventContext mockDeleteContext;
   @Mock private PersistenceService mockPersistenceService;
   @Mock private Result mockResult;
   @Mock private Row mockRow;
@@ -341,4 +348,228 @@ void testUpdateUploadStatusByScanStatus_AllScanStatuses() {
     // Verify all updates were called
     verify(mockPersistenceService, times(6)).run(any(CqnUpdate.class));
   }
+
+  @Test
+  void testGetAttachmentsForUPID_WithIsActiveEntity() {
+    String upID = "testUpID";
+    String upIdKey = "up__ID";
+
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.of(mockCdsElement));
+
+    Result result = mock(Result.class);
+    when(result.rowCount()).thenReturn(1L);
+    when(mockPersistenceService.run(any(CqnSelect.class))).thenReturn(result);
+
+    Result actual =
+        dbQuery.getAttachmentsForUPID(mockDraftEntity, mockPersistenceService, upID, upIdKey);
+
+    assertNotNull(actual);
+    verify(mockPersistenceService, times(1)).run(any(CqnSelect.class));
+    verify(mockDraftEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForUPID_WithoutIsActiveEntity() {
+    String upID = "testUpID";
+    String upIdKey = "up__ID";
+
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.empty());
+
+    Result result = mock(Result.class);
+    when(result.rowCount()).thenReturn(1L);
+    when(mockPersistenceService.run(any(CqnSelect.class))).thenReturn(result);
+
+    Result actual =
+        dbQuery.getAttachmentsForUPID(mockDraftEntity, mockPersistenceService, upID, upIdKey);
+
+    assertNotNull(actual);
+    verify(mockPersistenceService, times(1)).run(any(CqnSelect.class));
+    verify(mockDraftEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForUPIDAndRepository_WithIsActiveEntity() {
+    String upID = "testUpID";
+    String upIdKey = "up__ID";
+
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.of(mockCdsElement));
+
+    Result result = mock(Result.class);
+    when(result.rowCount()).thenReturn(1L);
+    when(mockPersistenceService.run(any(CqnSelect.class))).thenReturn(result);
+
+    Result actual =
+        dbQuery.getAttachmentsForUPIDAndRepository(
+            mockDraftEntity, mockPersistenceService, upID, upIdKey);
+
+    assertNotNull(actual);
+    verify(mockPersistenceService, times(1)).run(any(CqnSelect.class));
+    verify(mockDraftEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForUPIDAndRepository_WithoutIsActiveEntity() {
+    String upID = "testUpID";
+    String upIdKey = "up__ID";
+
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.empty());
+
+    Result result = mock(Result.class);
+    when(result.rowCount()).thenReturn(1L);
+    when(mockPersistenceService.run(any(CqnSelect.class))).thenReturn(result);
+
+    Result actual =
+        dbQuery.getAttachmentsForUPIDAndRepository(
+            mockDraftEntity, mockPersistenceService, upID, upIdKey);
+
+    assertNotNull(actual);
+    verify(mockPersistenceService, times(1)).run(any(CqnSelect.class));
+    verify(mockDraftEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForFolder_DraftEntity_WithIsActiveEntity() {
+    String entity = "TestEntity";
+    String folderId = "folder-1";
+
+    when(mockDeleteContext.getModel()).thenReturn(mockCdsModel);
+    when(mockCdsModel.findEntity(entity + "_drafts")).thenReturn(Optional.of(mockDraftEntity));
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.of(mockCdsElement));
+
+    Row row = mock(Row.class);
+    when(row.get("folderId")).thenReturn("folder-1");
+    when(row.get("repositoryId")).thenReturn("repo-1");
+    when(row.get("fileName")).thenReturn("file.pdf");
+    when(row.get("ID")).thenReturn("id-1");
+    when(row.get("objectId")).thenReturn("obj-1");
+    when(row.get("uploadStatus")).thenReturn("Clean");
+
+    Result draftResult = mock(Result.class);
+    when(draftResult.list()).thenReturn(List.of(row));
+    when(mockPersistenceService.run(any(CqnSelect.class))).thenReturn(draftResult);
+
+    List<CmisDocument> docs =
+        dbQuery.getAttachmentsForFolder(
+            entity, mockPersistenceService, folderId, mockDeleteContext);
+
+    assertNotNull(docs);
+    assertEquals(1, docs.size());
+    assertEquals("file.pdf", docs.get(0).getFileName());
+    verify(mockPersistenceService, times(1)).run(any(CqnSelect.class));
+    verify(mockDraftEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForFolder_DraftEntity_WithoutIsActiveEntity() {
+    String entity = "TestEntity";
+    String folderId = "folder-1";
+
+    when(mockDeleteContext.getModel()).thenReturn(mockCdsModel);
+    when(mockCdsModel.findEntity(entity + "_drafts")).thenReturn(Optional.of(mockDraftEntity));
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.empty());
+
+    Row row = mock(Row.class);
+    when(row.get("folderId")).thenReturn("folder-1");
+    when(row.get("repositoryId")).thenReturn("repo-1");
+    when(row.get("fileName")).thenReturn("file.pdf");
+    when(row.get("ID")).thenReturn("id-1");
+    when(row.get("objectId")).thenReturn("obj-1");
+    when(row.get("uploadStatus")).thenReturn("Clean");
+
+    Result draftResult = mock(Result.class);
+    when(draftResult.list()).thenReturn(List.of(row));
+    when(mockPersistenceService.run(any(CqnSelect.class))).thenReturn(draftResult);
+
+    List<CmisDocument> docs =
+        dbQuery.getAttachmentsForFolder(
+            entity, mockPersistenceService, folderId, mockDeleteContext);
+
+    assertNotNull(docs);
+    assertEquals(1, docs.size());
+    assertEquals("file.pdf", docs.get(0).getFileName());
+    verify(mockPersistenceService, times(1)).run(any(CqnSelect.class));
+    verify(mockDraftEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForFolder_FallsBackToActiveEntity_WithIsActiveEntity() {
+    String entity = "TestEntity";
+    String folderId = "folder-1";
+
+    when(mockDeleteContext.getModel()).thenReturn(mockCdsModel);
+    when(mockCdsModel.findEntity(entity + "_drafts")).thenReturn(Optional.of(mockDraftEntity));
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.of(mockCdsElement));
+
+    Result emptyDraftResult = mock(Result.class);
+    when(emptyDraftResult.list()).thenReturn(List.of());
+
+    when(mockCdsModel.findEntity(entity)).thenReturn(Optional.of(mockActiveEntity));
+    when(mockActiveEntity.findElement("IsActiveEntity")).thenReturn(Optional.of(mockCdsElement));
+
+    Row row = mock(Row.class);
+    when(row.get("folderId")).thenReturn("folder-1");
+    when(row.get("repositoryId")).thenReturn("repo-1");
+    when(row.get("fileName")).thenReturn("active-file.pdf");
+    when(row.get("ID")).thenReturn("id-2");
+    when(row.get("objectId")).thenReturn("obj-2");
+    when(row.get("uploadStatus")).thenReturn("Clean");
+
+    Result activeResult = mock(Result.class);
+    when(activeResult.list()).thenReturn(List.of(row));
+
+    when(mockPersistenceService.run(any(CqnSelect.class)))
+        .thenReturn(emptyDraftResult)
+        .thenReturn(activeResult);
+
+    List<CmisDocument> docs =
+        dbQuery.getAttachmentsForFolder(
+            entity, mockPersistenceService, folderId, mockDeleteContext);
+
+    assertNotNull(docs);
+    assertEquals(1, docs.size());
+    assertEquals("active-file.pdf", docs.get(0).getFileName());
+    verify(mockPersistenceService, times(2)).run(any(CqnSelect.class));
+    verify(mockActiveEntity).findElement("IsActiveEntity");
+  }
+
+  @Test
+  void testGetAttachmentsForFolder_FallsBackToActiveEntity_WithoutIsActiveEntity() {
+    String entity = "TestEntity";
+    String folderId = "folder-1";
+
+    when(mockDeleteContext.getModel()).thenReturn(mockCdsModel);
+    when(mockCdsModel.findEntity(entity + "_drafts")).thenReturn(Optional.of(mockDraftEntity));
+    when(mockDraftEntity.findElement("IsActiveEntity")).thenReturn(Optional.empty());
+
+    Result emptyDraftResult = mock(Result.class);
+    when(emptyDraftResult.list()).thenReturn(List.of());
+
+    when(mockCdsModel.findEntity(entity)).thenReturn(Optional.of(mockActiveEntity));
+    when(mockActiveEntity.findElement("IsActiveEntity")).thenReturn(Optional.empty());
+
+    Row row = mock(Row.class);
+    when(row.get("folderId")).thenReturn("folder-1");
+    when(row.get("repositoryId")).thenReturn("repo-1");
+    when(row.get("fileName")).thenReturn("active-file.pdf");
+    when(row.get("ID")).thenReturn("id-2");
+    when(row.get("objectId")).thenReturn("obj-2");
+    when(row.get("uploadStatus")).thenReturn("Clean");
+
+    Result activeResult = mock(Result.class);
+    when(activeResult.list()).thenReturn(List.of(row));
+
+    when(mockPersistenceService.run(any(CqnSelect.class)))
+        .thenReturn(emptyDraftResult)
+        .thenReturn(activeResult);
+
+    List<CmisDocument> docs =
+        dbQuery.getAttachmentsForFolder(
+            entity, mockPersistenceService, folderId, mockDeleteContext);
+
+    assertNotNull(docs);
+    assertEquals(1, docs.size());
+    assertEquals("active-file.pdf", docs.get(0).getFileName());
+    verify(mockPersistenceService, times(2)).run(any(CqnSelect.class));
+    verify(mockActiveEntity).findElement("IsActiveEntity");
+  }
 }