diff --git a/app/single-tenant/personal-space/demoapp/db/pom.xml b/app/single-tenant/personal-space/demoapp/db/pom.xml index 7a74659e1..780b52cdc 100644 --- a/app/single-tenant/personal-space/demoapp/db/pom.xml +++ b/app/single-tenant/personal-space/demoapp/db/pom.xml @@ -16,7 +16,7 @@ com.sap.cds sdm - 1.0.0-RC1 + 1.10.0 diff --git a/app/single-tenant/personal-space/demoapp/srv/pom.xml b/app/single-tenant/personal-space/demoapp/srv/pom.xml index 1a18bebd8..d6d146449 100644 --- a/app/single-tenant/personal-space/demoapp/srv/pom.xml +++ b/app/single-tenant/personal-space/demoapp/srv/pom.xml @@ -18,7 +18,7 @@ com.sap.cds sdm - 1.0.0-RC1 + 1.10.0 @@ -47,6 +47,12 @@ test + + org.springframework.security + spring-security-test + test + + com.h2database h2 diff --git a/app/single-tenant/personal-space/demoapp/srv/src/main/resources/application.yaml b/app/single-tenant/personal-space/demoapp/srv/src/main/resources/application.yaml index a95dc604c..1a3e366f1 100644 --- a/app/single-tenant/personal-space/demoapp/srv/src/main/resources/application.yaml +++ b/app/single-tenant/personal-space/demoapp/srv/src/main/resources/application.yaml @@ -8,4 +8,14 @@ spring: schema-locations: classpath:schema-h2.sql data-source: auto-config: - enabled: false \ No newline at end of file + enabled: false +cds: + security: + mock.users: + admin: + password: admin + roles: + - admin + - system-user + user: + password: user \ No newline at end of file diff --git a/app/single-tenant/personal-space/demoapp/srv/src/test/java/customer/demoapp/handlers/CatalogServiceNonDraftReadIT.java b/app/single-tenant/personal-space/demoapp/srv/src/test/java/customer/demoapp/handlers/CatalogServiceNonDraftReadIT.java new file mode 100644 index 000000000..7862e23f2 --- /dev/null +++ b/app/single-tenant/personal-space/demoapp/srv/src/test/java/customer/demoapp/handlers/CatalogServiceNonDraftReadIT.java @@ -0,0 +1,78 @@ +package customer.demoapp.handlers; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.web.servlet.MockMvc; + +/** + * Reproduces the SDM 1.10.0 regression: + * + * SDMReadAttachmentsHandler.populateUploadableFlags (new @After handler in 1.10.0) fires after + * every successful read on any entity whose composition has @SDM.Attachments. It then calls + * DBQuery.getAttachmentsForUPID, which unconditionally SELECTs "IsActiveEntity" — a column that + * only exists on draft-enabled entities. CatalogService.Books is a non-draft, @readonly + * projection, so its Books.attachments entity has no IsActiveEntity element. The CQN + * resolution throws CdsElementNotFoundException, which surfaces as HTTP 500. + * + * Expected (broken on SDM 1.10.0 without the fix): GET /odata/v4/CatalogService/Books → 500 + * Expected (after the fix): GET /odata/v4/CatalogService/Books → 200 + * + * The seed CSV ships 5 books, so the response always has rows — exactly the condition that + * triggers populateUploadableFlags (it returns early on empty data). + */ +@SpringBootTest +@AutoConfigureMockMvc +class CatalogServiceNonDraftReadIT { + + // CatalogService is bound at the default CDS OData endpoint path. + // The CAP Spring Boot starter mounts OData services under /odata/v4/. + private static final String BOOKS_URI = "/odata/v4/CatalogService/Books"; + + @Autowired + private MockMvc mockMvc; + + /** + * A plain, unauthenticated GET on the non-draft CatalogService.Books must return 200 + * and at least one book row. On SDM 1.10.0 (unpatched) this returns 500 because + * populateUploadableFlags tries to SELECT IsActiveEntity from a non-draft attachment entity. + */ + @Test + void nonDraftReadOfBooksWithAttachmentsMustReturn200() throws Exception { + mockMvc.perform(get(BOOKS_URI)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.value").isArray()) + .andExpect(jsonPath("$.value[0].ID").exists()); + } + + /** + * Same call with $top=1 — verifies it is not the 0-row early-exit case that happens to pass. + * populateUploadableFlags does `if (data == null || data.isEmpty()) return;`, so an empty + * result would never hit the crash. This test ensures at least one row is returned AND + * processed without error. + */ + @Test + void nonDraftReadWithTopOneStillReturns200() throws Exception { + mockMvc.perform(get(BOOKS_URI + "?$top=1")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.value").isArray()) + .andExpect(jsonPath("$.value[0].ID").exists()); + } + + /** + * Filtering to a row that is known to exist (Wuthering Heights is in the seed CSV). + * Exercises the exact code path: a query that returns exactly 1 row goes through + * populateUploadableFlags fully, hitting getAttachmentsForUPID once per facet. + */ + @Test + void nonDraftReadWithFilterStillReturns200() throws Exception { + mockMvc.perform(get(BOOKS_URI + "?$filter=stock gt 0")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.value").isArray()); + } +} diff --git a/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandler.java b/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandler.java index 776b12f92..241e28531 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/handler/applicationservice/SDMReadAttachmentsHandler.java @@ -460,9 +460,15 @@ public void populateUploadableFlags(CdsReadEventContext context, List d CdsModel model = context.getModel(); // Cache keyed by "facetName|parentId|isDraft" to avoid one DB query per row per facet. Map uploadableCache = new HashMap<>(); + + // Non-draft entities have no IsActiveEntity; treat every row as active (isDraft=false). + boolean entityHasDraftSupport = target.findElement("IsActiveEntity").isPresent(); + for (CdsData row : data) { // Determine draft state per row — a single result set can mix active and draft records. - boolean rowIsDraft = Boolean.FALSE.equals(row.get("IsActiveEntity")); + // On a non-draft entity IsActiveEntity is absent; default to false (active). + boolean rowIsDraft = + entityHasDraftSupport && Boolean.FALSE.equals(row.get("IsActiveEntity")); Object keyVal = row.get(keyField); if (keyVal == null) { logger.debug("populateUploadableFlags Path1: skipping row with null keyVal"); diff --git a/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java b/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java index e2b56432e..fbe58af44 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 @@ -52,9 +52,15 @@ public Result getAttachmentsForUPID( upID, upIdKey, attachmentEntity.getQualifiedName()); + List 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); @@ -378,9 +384,14 @@ public Result getAttachmentsForUPIDAndRepository( upID, SDMConstants.REPOSITORY_ID, attachmentEntity.getQualifiedName()); + List 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) @@ -511,16 +522,17 @@ public List getAttachmentsForFolder( folderId, entity); attachmentEntity = context.getModel().findEntity(entity); + List activeColumns = + new ArrayList<>( + java.util.Arrays.asList( + "fileName", "ID", "folderId", "repositoryId", "objectId", "uploadStatus")); + if (attachmentEntity.isPresent() + && attachmentEntity.get().findElement("IsActiveEntity").isPresent()) { + activeColumns.add(1, "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()) {