From 8dd3dcac571a32b1953301bdbc86d37b718eda36 Mon Sep 17 00:00:00 2001 From: "Yashmeet ." Date: Mon, 6 Jul 2026 10:14:43 +0530 Subject: [PATCH 1/3] Leading app changes to reproduce the issue --- .../personal-space/demoapp/db/pom.xml | 2 +- .../personal-space/demoapp/srv/pom.xml | 8 +- .../srv/src/main/resources/application.yaml | 12 ++- .../CatalogServiceNonDraftReadIT.java | 78 +++++++++++++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 app/single-tenant/personal-space/demoapp/srv/src/test/java/customer/demoapp/handlers/CatalogServiceNonDraftReadIT.java diff --git a/app/single-tenant/personal-space/demoapp/db/pom.xml b/app/single-tenant/personal-space/demoapp/db/pom.xml index 7a74659e..780b52cd 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 1a18bebd..d6d14644 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 a95dc604..1a3e366f 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 00000000..7862e23f --- /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()); + } +} From b7fb3a34cad15e3413e3af21d8594e1b89bfaf4b Mon Sep 17 00:00:00 2001 From: "Yashmeet ." Date: Mon, 6 Jul 2026 10:20:38 +0530 Subject: [PATCH 2/3] bug-fix --- .../SDMReadAttachmentsHandler.java | 7 ++++- .../com/sap/cds/sdm/persistence/DBQuery.java | 29 ++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) 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 776b12f9..90fa3f89 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,14 @@ 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 e2b56432..d01b7c61 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,14 @@ 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 +383,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 +521,15 @@ 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()) { From 875e40ca50631441e2658f7e5f365a9a7d3483b9 Mon Sep 17 00:00:00 2001 From: "Yashmeet ." Date: Mon, 6 Jul 2026 10:22:52 +0530 Subject: [PATCH 3/3] spotless fix --- .../SDMReadAttachmentsHandler.java | 3 ++- .../java/com/sap/cds/sdm/persistence/DBQuery.java | 15 +++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) 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 90fa3f89..241e2853 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 @@ -467,7 +467,8 @@ public void populateUploadableFlags(CdsReadEventContext context, List d for (CdsData row : data) { // Determine draft state per row — a single result set can mix active and draft records. // On a non-draft entity IsActiveEntity is absent; default to false (active). - boolean rowIsDraft = entityHasDraftSupport && Boolean.FALSE.equals(row.get("IsActiveEntity")); + 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 d01b7c61..fbe58af4 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,8 +52,9 @@ public Result getAttachmentsForUPID( upID, upIdKey, attachmentEntity.getQualifiedName()); - List columns = new ArrayList<>( - java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId", "mimeType")); + List columns = + new ArrayList<>( + java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId", "mimeType")); if (attachmentEntity.findElement("IsActiveEntity").isPresent()) { columns.add("IsActiveEntity"); } @@ -383,8 +384,8 @@ public Result getAttachmentsForUPIDAndRepository( upID, SDMConstants.REPOSITORY_ID, attachmentEntity.getQualifiedName()); - List columns = new ArrayList<>( - java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId")); + List columns = + new ArrayList<>(java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId")); if (attachmentEntity.findElement("IsActiveEntity").isPresent()) { columns.add("IsActiveEntity"); } @@ -521,8 +522,10 @@ public List getAttachmentsForFolder( folderId, entity); attachmentEntity = context.getModel().findEntity(entity); - List activeColumns = new ArrayList<>( - java.util.Arrays.asList("fileName", "ID", "folderId", "repositoryId", "objectId", "uploadStatus")); + 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");