From d42a586d027deee81ced09f689d448dbb336d286 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 2 Oct 2024 17:47:43 +0200 Subject: [PATCH 1/6] Add isDynamic field to Virtual Study in Session Service --- .../org/cbioportal/web/parameter/VirtualStudyData.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java b/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java index 54a5a88a1d6..eb47ae067bc 100644 --- a/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java +++ b/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java @@ -24,6 +24,8 @@ public class VirtualStudyData implements Serializable { private String typeOfCancerId; private String pmid; + private Boolean isDynamic; + public String getOwner() { return owner; } @@ -122,4 +124,12 @@ public String getPmid() { public void setPmid(String pmid) { this.pmid = pmid; } + + public Boolean getDynamic() { + return isDynamic; + } + + public void setDynamic(Boolean dynamic) { + isDynamic = dynamic; + } } From ca767009d34ab5735101b8cd44a9fefddac8c7c2 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Wed, 2 Oct 2024 21:15:52 +0200 Subject: [PATCH 2/6] Query sample IDs using Virtual Study View Filters is it's of dynamic type --- .../web/SessionServiceController.java | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 83cf95c932a..286f8d9aa8b 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -23,10 +23,13 @@ import org.cbioportal.web.parameter.PageSettingsIdentifier; import org.cbioportal.web.parameter.PagingConstants; import org.cbioportal.web.parameter.ResultsPageSettings; +import org.cbioportal.web.parameter.SampleIdentifier; import org.cbioportal.web.parameter.SessionPage; import org.cbioportal.web.parameter.StudyPageSettings; import org.cbioportal.web.parameter.VirtualStudy; import org.cbioportal.web.parameter.VirtualStudyData; +import org.cbioportal.web.parameter.VirtualStudySamples; +import org.cbioportal.web.util.StudyViewFilterApplier; import org.json.simple.JSONObject; import org.json.simple.parser.JSONParser; import org.json.simple.parser.ParseException; @@ -57,6 +60,7 @@ import java.util.Optional; import java.util.Set; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static org.cbioportal.web.PublicVirtualStudiesController.ALL_USERS; @@ -76,6 +80,9 @@ public class SessionServiceController { @Autowired private ObjectMapper sessionServiceObjectMapper; + @Autowired + private StudyViewFilterApplier studyViewFilterApplier; + @Value("${session.service.url:}") private String sessionServiceURL; @@ -211,7 +218,12 @@ public ResponseEntity getSession(@PathVariable Session.SessionType type Session session; switch (type) { case virtual_study: - session = sessionServiceObjectMapper.readValue(sessionDataJson, VirtualStudy.class); + VirtualStudy virtualStudy = sessionServiceObjectMapper.readValue(sessionDataJson, VirtualStudy.class); + VirtualStudyData virtualStudyData = virtualStudy.getData(); + if (Boolean.TRUE.equals(virtualStudyData.getDynamic())) { + populateVirtualStudySamples(virtualStudyData); + } + session = virtualStudy; break; case settings: session = sessionServiceObjectMapper.readValue(sessionDataJson, PageSettings.class); @@ -232,6 +244,29 @@ public ResponseEntity getSession(@PathVariable Session.SessionType type } } + /** + * This method populates the `virtualStudyData` object with a new set of sample IDs retrieved as the result of executing a query based on virtual study view filters. + * It first applies the filters defined within the study view, runs the query to fetch the relevant sample IDs, and then updates the virtualStudyData to reflect these fresh results. + * This ensures that the virtual study contains the latest sample IDs. + * @param virtualStudyData + */ + private void populateVirtualStudySamples(VirtualStudyData virtualStudyData) { + List sampleIdentifiers = studyViewFilterApplier.apply(virtualStudyData.getStudyViewFilter()); + Map> sampleIdsByStudyId = sampleIdentifiers + .stream() + .collect( + Collectors.groupingBy( + SampleIdentifier::getStudyId, + Collectors.mapping(SampleIdentifier::getSampleId, Collectors.toSet()))); + Set virtualStudySamples = sampleIdsByStudyId.entrySet().stream().map(entry -> { + VirtualStudySamples vss = new VirtualStudySamples(); + vss.setId(entry.getKey()); + vss.setSamples(entry.getValue()); + return vss; + }).collect(Collectors.toSet()); + virtualStudyData.setStudies(virtualStudySamples); + } + @RequestMapping(value = "/virtual_study", method = RequestMethod.GET) @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = VirtualStudy.class)))) From cbab49aa10ab799cf599b1c8a312f55e60b39010 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Sat, 12 Oct 2024 11:45:39 +0200 Subject: [PATCH 3/6] Rename boolean varialbe name isDynamic -> dynamic --- .../java/org/cbioportal/web/parameter/VirtualStudyData.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java b/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java index eb47ae067bc..f3f60dfb360 100644 --- a/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java +++ b/src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java @@ -24,7 +24,7 @@ public class VirtualStudyData implements Serializable { private String typeOfCancerId; private String pmid; - private Boolean isDynamic; + private Boolean dynamic; public String getOwner() { return owner; @@ -126,10 +126,10 @@ public void setPmid(String pmid) { } public Boolean getDynamic() { - return isDynamic; + return dynamic; } public void setDynamic(Boolean dynamic) { - isDynamic = dynamic; + this.dynamic = dynamic; } } From 1adca8c6b67f20870d2366d6c0d9b0116136bb59 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Mon, 21 Oct 2024 09:53:26 +0200 Subject: [PATCH 4/6] Refactor dynamic virtual study population Extract smaller functions --- .../web/SessionServiceController.java | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 286f8d9aa8b..6f5182b122d 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -252,19 +252,36 @@ public ResponseEntity getSession(@PathVariable Session.SessionType type */ private void populateVirtualStudySamples(VirtualStudyData virtualStudyData) { List sampleIdentifiers = studyViewFilterApplier.apply(virtualStudyData.getStudyViewFilter()); + Set virtualStudySamples = extractVirtualStudySamples(sampleIdentifiers); + virtualStudyData.setStudies(virtualStudySamples); + } + + /** + * Transforms list of sample identifiers to set of virtual study samples + * @param sampleIdentifiers + */ + private Set extractVirtualStudySamples(List sampleIdentifiers) { + Map> sampleIdsByStudyId = groupSampleIdsByStudyId(sampleIdentifiers); + return sampleIdsByStudyId.entrySet().stream().map(entry -> { + VirtualStudySamples vss = new VirtualStudySamples(); + vss.setId(entry.getKey()); + vss.setSamples(entry.getValue()); + return vss; + }).collect(Collectors.toSet()); + } + + /** + * Groups sample IDs by their study ID + * @param sampleIdentifiers + */ + private Map> groupSampleIdsByStudyId(List sampleIdentifiers) { Map> sampleIdsByStudyId = sampleIdentifiers .stream() .collect( Collectors.groupingBy( SampleIdentifier::getStudyId, Collectors.mapping(SampleIdentifier::getSampleId, Collectors.toSet()))); - Set virtualStudySamples = sampleIdsByStudyId.entrySet().stream().map(entry -> { - VirtualStudySamples vss = new VirtualStudySamples(); - vss.setId(entry.getKey()); - vss.setSamples(entry.getValue()); - return vss; - }).collect(Collectors.toSet()); - virtualStudyData.setStudies(virtualStudySamples); + return sampleIdsByStudyId; } @RequestMapping(value = "/virtual_study", method = RequestMethod.GET) From 7c394bb6eb82f2c10370d65ecb67058e03a4d260 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Mon, 28 Oct 2024 15:37:52 +0100 Subject: [PATCH 5/6] Fix code smells discovered by sonarlint - Use the constructor dep. injection - return value withou assigning it to a variable --- .../cbioportal/web/SessionServiceController.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cbioportal/web/SessionServiceController.java b/src/main/java/org/cbioportal/web/SessionServiceController.java index 6f5182b122d..d2ad7499c1f 100644 --- a/src/main/java/org/cbioportal/web/SessionServiceController.java +++ b/src/main/java/org/cbioportal/web/SessionServiceController.java @@ -35,7 +35,6 @@ import org.json.simple.parser.ParseException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; @@ -74,15 +73,20 @@ public class SessionServiceController { private static final String QUERY_OPERATOR_SIZE = "$size"; private static final String QUERY_OPERATOR_AND = "$and"; - @Autowired SessionServiceRequestHandler sessionServiceRequestHandler; - @Autowired private ObjectMapper sessionServiceObjectMapper; - @Autowired private StudyViewFilterApplier studyViewFilterApplier; + public SessionServiceController(SessionServiceRequestHandler sessionServiceRequestHandler, + ObjectMapper sessionServiceObjectMapper, + StudyViewFilterApplier studyViewFilterApplier) { + this.sessionServiceRequestHandler = sessionServiceRequestHandler; + this.sessionServiceObjectMapper = sessionServiceObjectMapper; + this.studyViewFilterApplier = studyViewFilterApplier; + } + @Value("${session.service.url:}") private String sessionServiceURL; @@ -275,13 +279,12 @@ private Set extractVirtualStudySamples(List> groupSampleIdsByStudyId(List sampleIdentifiers) { - Map> sampleIdsByStudyId = sampleIdentifiers + return sampleIdentifiers .stream() .collect( Collectors.groupingBy( SampleIdentifier::getStudyId, Collectors.mapping(SampleIdentifier::getSampleId, Collectors.toSet()))); - return sampleIdsByStudyId; } @RequestMapping(value = "/virtual_study", method = RequestMethod.GET) From eaf55466521b9fb02a6da9ea5440a88ba9c61a90 Mon Sep 17 00:00:00 2001 From: Ruslan Forostianov Date: Tue, 5 Nov 2024 21:38:26 +0100 Subject: [PATCH 6/6] Add web mvc test for fetching static and dynamic virtual studies --- .../web/SessionServiceControllerTest.java | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 src/test/java/org/cbioportal/web/SessionServiceControllerTest.java diff --git a/src/test/java/org/cbioportal/web/SessionServiceControllerTest.java b/src/test/java/org/cbioportal/web/SessionServiceControllerTest.java new file mode 100644 index 00000000000..0f9e751d826 --- /dev/null +++ b/src/test/java/org/cbioportal/web/SessionServiceControllerTest.java @@ -0,0 +1,92 @@ +package org.cbioportal.web; + +import org.cbioportal.service.util.SessionServiceRequestHandler; +import org.cbioportal.utils.removeme.Session; +import org.cbioportal.web.config.TestConfig; +import org.cbioportal.web.parameter.SampleIdentifier; +import org.cbioportal.web.util.StudyViewFilterApplier; +import org.hamcrest.Matchers; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; +import org.springframework.test.web.servlet.result.MockMvcResultMatchers; + +import java.util.List; + +@RunWith(SpringJUnit4ClassRunner.class) +@WebMvcTest +@ContextConfiguration(classes = {SessionServiceController.class, TestConfig.class}) +public class SessionServiceControllerTest { + + @MockBean + SessionServiceRequestHandler sessionServiceRequestHandler; + + @MockBean + StudyViewFilterApplier studyViewFilterApplier; + + SampleIdentifier sampleIdentifier1 = new SampleIdentifier(); + { sampleIdentifier1.setStudyId("STUDY_1"); } + SampleIdentifier sampleIdentifier2 = new SampleIdentifier(); + { sampleIdentifier2.setStudyId("STUDY_2"); } + + @Autowired + private MockMvc mockMvc; + @Test + @WithMockUser + public void testStaticVirtualStudy() throws Exception { + Mockito.when(sessionServiceRequestHandler.getSessionDataJson(Session.SessionType.virtual_study, "123")) + .thenReturn(""" + { + "id": "123", + "data": { + "name": "Test", + "studies": [ + { "id": "STUDY_N", "samples": [ "S1", "S2" ] } + ] + } + } + """); + + // Should not be used + Mockito.when(studyViewFilterApplier.apply(Mockito.any())).thenReturn(List.of(sampleIdentifier1, sampleIdentifier2)); + + mockMvc.perform(MockMvcRequestBuilders.get("/api/session/virtual_study/123") + .accept(MediaType.APPLICATION_JSON)) + .andExpect(MockMvcResultMatchers.status().isOk()) + .andExpect(MockMvcResultMatchers.jsonPath("$.data.studies", Matchers.hasSize(1))) + .andExpect(MockMvcResultMatchers.jsonPath("$.data.studies[0].id").value("STUDY_N")); + Mockito.verify(studyViewFilterApplier, Mockito.never()).apply(Mockito.any()); + } + + @Test + @WithMockUser + public void testDynamicVirtualStudy() throws Exception { + Mockito.when(sessionServiceRequestHandler.getSessionDataJson(Session.SessionType.virtual_study, "123")) + .thenReturn(""" + { + "id": "123", + "data": { + "name": "Test", + "dynamic": true + } + } + """); + + Mockito.when(studyViewFilterApplier.apply(Mockito.any())).thenReturn(List.of(sampleIdentifier1, sampleIdentifier2)); + + mockMvc.perform(MockMvcRequestBuilders.get("/api/session/virtual_study/123") + .accept(MediaType.APPLICATION_JSON)) + .andExpect(MockMvcResultMatchers.status().isOk()) + .andExpect(MockMvcResultMatchers.jsonPath("$.data.studies", Matchers.hasSize(2))) + .andExpect(MockMvcResultMatchers.jsonPath("$.data.studies[*].id").value(Matchers.containsInAnyOrder("STUDY_1", "STUDY_2"))); + } +} \ No newline at end of file