#374 - Extend API to save and load cohort and dataselection

- add max queries per user variable to DataquerySpringConfig - don't throw storage full exception on dataquery without result - fix wrong path on CodeableConceptRestController and TerminologyRestController - add more tests to check for handling of full storage
medizininformatik-initiative · Feb 27, 2025 · 81882f0 · 81882f0
1 parent 6f569c5
commit 81882f0
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 30 deletions.
diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/dataquery/DataqueryHandler.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/dataquery/DataqueryHandler.java
@@ -28,13 +28,13 @@ public class DataqueryHandler {
   @NonNull
   private DataqueryRepository dataqueryRepository;
 
-  @Value("${app.maxSavedQueriesPerUser}")
-  private int maxDataqueriesPerUser;
+  @NonNull
+  private Integer maxDataqueriesPerUser;
 
   public Long storeDataquery(@NonNull Dataquery dataquery, @NonNull String userId) throws DataqueryException, DataqueryStorageFullException {
-    var usedSlots = dataqueryRepository.countByCreatedByWhereResultIsNotNull(userId);
 
-    if (usedSlots >= maxDataqueriesPerUser) {
+    // By definition, a user can save an unlimited amount of queries without result
+    if (dataquery.resultSize() != null && dataqueryRepository.countByCreatedByWhereResultIsNotNull(userId) >= maxDataqueriesPerUser) {
       throw new DataqueryStorageFullException();
     }
 

diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/dataquery/DataquerySpringConfig.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/dataquery/DataquerySpringConfig.java
@@ -3,6 +3,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import de.numcodex.feasibility_gui_backend.query.persistence.DataqueryRepository;
 import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
@@ -12,8 +13,9 @@ public class DataquerySpringConfig {
   @Bean
   public DataqueryHandler createDataqueryHandler(
       @Qualifier("translation") ObjectMapper jsonUtil,
-      DataqueryRepository dataqueryRepository
+      DataqueryRepository dataqueryRepository,
+      @Value("${app.maxSavedQueriesPerUser}") Integer maxSavedQueriesPerUser
   ) {
-    return new DataqueryHandler(jsonUtil, dataqueryRepository);
+    return new DataqueryHandler(jsonUtil, dataqueryRepository, maxSavedQueriesPerUser);
   }
 }
diff --git a/...ava/de/numcodex/feasibility_gui_backend/terminology/v5/CodeableConceptRestController.java b/...ava/de/numcodex/feasibility_gui_backend/terminology/v5/CodeableConceptRestController.java
@@ -11,7 +11,7 @@
 import java.util.List;
 
 @RestController
-@RequestMapping("api/v4/codeable-concept")
+@RequestMapping("api/v5/codeable-concept")
 @ConditionalOnExpression("${app.elastic.enabled}")
 @CrossOrigin
 public class CodeableConceptRestController {

diff --git a/...in/java/de/numcodex/feasibility_gui_backend/terminology/v5/TerminologyRestController.java b/...in/java/de/numcodex/feasibility_gui_backend/terminology/v5/TerminologyRestController.java
@@ -18,7 +18,7 @@
  */
 
 
-@RequestMapping("api/v4/terminology")
+@RequestMapping("api/v5/terminology")
 @RestController
 @CrossOrigin
 @ConditionalOnExpression("${app.elastic.enabled}")

diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/QueryHandlerServiceTest.java
@@ -96,8 +96,8 @@ public void testRunQuery_failsWithMonoErrorOnQueryDispatchException() throws Que
     }
 
     @ParameterizedTest
-    @CsvSource({"true,true", "true,false", "false,true", "false,false"})
-    void convertQueriesToQueryListEntries(String withSavedQuery, String skipValidation) throws JsonProcessingException {
+    @CsvSource({"false,true", "false,false"})
+    void convertQueriesToQueryListEntries(String skipValidation) throws JsonProcessingException {
         var queryList = List.of(createQuery());
         if (!Boolean.parseBoolean(skipValidation)) {
             doReturn(
@@ -110,10 +110,6 @@ void convertQueriesToQueryListEntries(String withSavedQuery, String skipValidati
         assertThat(queryListEntries.size()).isEqualTo(1);
         assertThat(queryListEntries.get(0).id()).isEqualTo(QUERY_ID);
         assertThat(queryListEntries.get(0).createdAt()).isEqualTo(LAST_MODIFIED);
-        if (Boolean.parseBoolean(withSavedQuery)) {
-            assertThat(queryListEntries.get(0).label()).isEqualTo(LABEL);
-            assertThat(queryListEntries.get(0).totalNumberOfPatients()).isEqualTo(RESULT_SIZE);
-        }
         if (Boolean.parseBoolean(skipValidation)) {
             assertThat(queryListEntries.get(0).isValid()).isNull();
         } else {

diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/dataquery/DataqueryHandlerTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/dataquery/DataqueryHandlerTest.java
@@ -12,6 +12,8 @@
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.mockito.Mock;
 import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -25,8 +27,7 @@
 import static org.junit.Assert.assertThrows;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.*;
 
 @Tag("query")
 @Tag("peristence")
@@ -37,6 +38,7 @@ class DataqueryHandlerTest {
     public static final String LABEL = "some-label";
     public static final String COMMENT = "some-comment";
     public static final String TIME_STRING = "1969-07-20 20:17:40.0";
+    private static final int MAX_QUERIES_PER_USER = 5;
 
     @Spy
     private ObjectMapper jsonUtil = new ObjectMapper();
@@ -45,7 +47,7 @@ class DataqueryHandlerTest {
     private DataqueryRepository dataqueryRepository;
 
     private DataqueryHandler createDataqueryHandler() {
-        return new DataqueryHandler(jsonUtil, dataqueryRepository);
+        return new DataqueryHandler(jsonUtil, dataqueryRepository, MAX_QUERIES_PER_USER);
     }
 
     @Test
@@ -78,9 +80,43 @@ void storeDataquery_throwsOnNonEmptyQueryAndEmptyUser() {
         assertThrows(NullPointerException.class, () -> dataqueryHandler.storeDataquery(createDataquery(), null));
     }
 
+    @ParameterizedTest
+    @CsvSource({"true", "false"})
+    @DisplayName("storeDataquery() -> trying to store a dataquery when no slots are free throws")
+    void storeDataquery_throwsOnNoFreeSlots(boolean withResult) throws JsonProcessingException {
+        var dataqueryHandler = createDataqueryHandler();
+        lenient().doReturn(MAX_QUERIES_PER_USER + 1L).when(dataqueryRepository).countByCreatedByWhereResultIsNotNull(any(String.class));
+        lenient().doReturn(createDataqueryEntity()).when(dataqueryRepository).save(any(de.numcodex.feasibility_gui_backend.query.persistence.Dataquery.class));
+
+        if (withResult) {
+            assertThrows(DataqueryStorageFullException.class, () -> dataqueryHandler.storeDataquery(createDataquery(withResult), CREATOR));
+        } else {
+            assertDoesNotThrow(() -> dataqueryHandler.storeDataquery(createDataquery(withResult), CREATOR));
+        }
+    }
+
+    @ParameterizedTest
+    @CsvSource({"true,-1", "true,0", "true,1", "false,-1", "false,0", "false,1"})
+    @DisplayName("storeDataquery() -> checking around the query limit")
+    void storeDataquery_testFreeSlotOnEdgeCases(boolean withResult, long offset) throws JsonProcessingException {
+        var dataqueryHandler = createDataqueryHandler();
+        lenient().doReturn(MAX_QUERIES_PER_USER + offset).when(dataqueryRepository).countByCreatedByWhereResultIsNotNull(any(String.class));
+        lenient().doReturn(createDataqueryEntity()).when(dataqueryRepository).save(any(de.numcodex.feasibility_gui_backend.query.persistence.Dataquery.class));
+
+        if (withResult) {
+            if (offset < 0) {
+                assertDoesNotThrow(() -> dataqueryHandler.storeDataquery(createDataquery(withResult), CREATOR));
+            } else {
+                assertThrows(DataqueryStorageFullException.class, () -> dataqueryHandler.storeDataquery(createDataquery(withResult), CREATOR));
+            }
+        } else {
+            assertDoesNotThrow(() -> dataqueryHandler.storeDataquery(createDataquery(withResult), CREATOR));
+        }
+    }
+
     @Test
     @DisplayName("storeDataquery() -> error in json serialization throws an exception")
-    void storeNewQuery_throwsOnJsonSerializationError() throws JsonProcessingException {
+    void storeDataquery_throwsOnJsonSerializationError() throws JsonProcessingException {
         var dataquery = createDataquery();
         doThrow(JsonProcessingException.class).when(jsonUtil).writeValueAsString(any(Crtdl.class));
 
@@ -275,18 +311,22 @@ void convertPersistenceToApi() throws JsonProcessingException {
         assertEquals(convertedDataquery.content().cohortDefinition().exclusionCriteria(), originalCrtdl.cohortDefinition().exclusionCriteria());
     }
 
-    private Dataquery createDataquery() {
+    private Dataquery createDataquery(boolean withResult) {
         return Dataquery.builder()
             .id(1L)
             .label(LABEL)
             .comment(COMMENT)
             .content(createCrtdl())
-            .resultSize(123L)
+            .resultSize(withResult ? 123L : null)
             .createdBy(CREATOR)
             .lastModified(TIME_STRING)
             .build();
     }
 
+    private Dataquery createDataquery() {
+        return createDataquery(false);
+    }
+
     private Crtdl createCrtdl() {
         return Crtdl.builder()
             .cohortDefinition(createValidStructuredQuery())

diff --git a/...ava/de/numcodex/feasibility_gui_backend/query/ratelimiting/RateLimitingInterceptorIT.java b/...ava/de/numcodex/feasibility_gui_backend/query/ratelimiting/RateLimitingInterceptorIT.java
@@ -1,6 +1,6 @@
 package de.numcodex.feasibility_gui_backend.query.ratelimiting;
 
-import static de.numcodex.feasibility_gui_backend.config.WebSecurityConfig.PATH_API;
+import static de.numcodex.feasibility_gui_backend.config.WebSecurityConfig.*;
 import static de.numcodex.feasibility_gui_backend.query.persistence.ResultType.SUCCESS;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
@@ -89,7 +89,7 @@ void setupMockBehaviour() throws InvalidAuthenticationException {
   @EnumSource
   public void testGetResult_SucceedsOnFirstCall(ResultDetail resultDetail) throws Exception {
     var authorName = UUID.randomUUID().toString();
-    var requestUri = PATH_API + "/query/1";
+    var requestUri = PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1";
     boolean isAdmin = false;
 
     switch (resultDetail) {
@@ -118,7 +118,7 @@ public void testGetResult_SucceedsOnFirstCall(ResultDetail resultDetail) throws
   @EnumSource
   public void testGetResult_FailsOnImmediateSecondCall(ResultDetail resultDetail) throws Exception {
     var authorName = UUID.randomUUID().toString();
-    var requestUri = PATH_API + "/query/1";
+    var requestUri = PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1";
 
     switch (resultDetail) {
       case SUMMARY -> requestUri = requestUri + WebSecurityConfig.PATH_SUMMARY_RESULT;
@@ -152,7 +152,7 @@ public void testGetResult_FailsOnImmediateSecondCall(ResultDetail resultDetail)
   @EnumSource
   public void testGetResult_SucceedsOnDelayedSecondCall(ResultDetail resultDetail) throws Exception {
     var authorName = UUID.randomUUID().toString();
-    var requestUri = PATH_API + "/query/1";
+    var requestUri = PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1";
 
     switch (resultDetail) {
       case SUMMARY -> requestUri = requestUri + WebSecurityConfig.PATH_SUMMARY_RESULT;
@@ -178,7 +178,7 @@ public void testGetResult_SucceedsOnDelayedSecondCall(ResultDetail resultDetail)
 
     mockMvc
         .perform(
-            get(URI.create(PATH_API + "/query/1" + WebSecurityConfig.PATH_SUMMARY_RESULT)).with(csrf())
+            get(URI.create(PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1" + WebSecurityConfig.PATH_SUMMARY_RESULT)).with(csrf())
                 .with(user(authorName).password("pass").roles("DATAPORTAL_TEST_USER"))
         )
         .andExpect(status().isOk());
@@ -190,7 +190,7 @@ public void testGetResult_SucceedsOnImmediateMultipleCallsAsAdmin(ResultDetail r
       throws Exception {
 
     var authorName = UUID.randomUUID().toString();
-    var requestUri = PATH_API + "/query/1";
+    var requestUri = PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1";
 
     switch (resultDetail) {
       case SUMMARY -> requestUri = requestUri + WebSecurityConfig.PATH_SUMMARY_RESULT;
@@ -217,7 +217,7 @@ public void testGetResult_SucceedsOnImmediateMultipleCallsAsAdmin(ResultDetail r
   public void testGetResult_SucceedsOnImmediateSecondCallAsOtherUser(ResultDetail resultDetail)
       throws Exception {
     var authorName = UUID.randomUUID().toString();
-    var requestUri = PATH_API + "/query/1";
+    var requestUri = PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1";
 
     switch (resultDetail) {
       case SUMMARY -> requestUri = requestUri + WebSecurityConfig.PATH_SUMMARY_RESULT;
@@ -244,7 +244,7 @@ public void testGetResult_SucceedsOnImmediateSecondCallAsOtherUser(ResultDetail
 
     mockMvc
         .perform(
-            get(URI.create(PATH_API + "/query/1" + WebSecurityConfig.PATH_SUMMARY_RESULT)).with(csrf())
+            get(URI.create(PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1" + WebSecurityConfig.PATH_SUMMARY_RESULT)).with(csrf())
                 .with(user(authorName).password("pass").roles("DATAPORTAL_TEST_USER"))
         )
         .andExpect(status().isOk());
@@ -253,7 +253,7 @@ public void testGetResult_SucceedsOnImmediateSecondCallAsOtherUser(ResultDetail
   @Test
   public void testGetDetailedObfuscatedResult_FailsOnLimitExceedingCall() throws Exception {
     var authorName = UUID.randomUUID().toString();
-    var requestUri = PATH_API + "/query/1" + WebSecurityConfig.PATH_DETAILED_OBFUSCATED_RESULT;
+    var requestUri = PATH_API + PATH_QUERY + PATH_FEASIBILITY + "/1" + WebSecurityConfig.PATH_DETAILED_OBFUSCATED_RESULT;
 
     doReturn(false).when(authenticationHelper)
         .hasAuthority(any(Authentication.class), eq("DATAPORTAL_TEST_ADMIN"));

diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml
@@ -14,7 +14,7 @@ app:
   keycloakAllowedRole: "DATAPORTAL_TEST_USER"
   keycloakPowerRole: "DATAPORTAL_TEST_POWER"
   keycloakAdminRole: "DATAPORTAL_TEST_ADMIN"
-  maxSavedQueriesPerUser: 2
+  maxSavedQueriesPerUser: 10
   broker:
     aktin:
       enabled: false