Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rfc85 dynamic virtual study #11040

Merged
merged 9 commits into from
Nov 6, 2024
63 changes: 59 additions & 4 deletions src/main/java/org/cbioportal/web/SessionServiceController.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,18 @@
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;
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;
Expand All @@ -57,6 +59,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;

Expand All @@ -70,12 +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;

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;

Expand Down Expand Up @@ -211,7 +222,12 @@ public ResponseEntity<Session> 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);
Expand All @@ -232,6 +248,45 @@ public ResponseEntity<Session> 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
*/
Comment on lines +251 to +256
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Quick question: will this be cached? If yes, how?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pieterlukasse this method is not cached, but studyViewFilterApplier.apply(virtualStudyData.getStudyViewFilter()); is. I don't think we should cache this method as it'd be cheap transformation.

private void populateVirtualStudySamples(VirtualStudyData virtualStudyData) {
List<SampleIdentifier> sampleIdentifiers = studyViewFilterApplier.apply(virtualStudyData.getStudyViewFilter());
Set<VirtualStudySamples> virtualStudySamples = extractVirtualStudySamples(sampleIdentifiers);
virtualStudyData.setStudies(virtualStudySamples);
}

/**
* Transforms list of sample identifiers to set of virtual study samples
* @param sampleIdentifiers
*/
private Set<VirtualStudySamples> extractVirtualStudySamples(List<SampleIdentifier> sampleIdentifiers) {
Map<String, Set<String>> 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<String, Set<String>> groupSampleIdsByStudyId(List<SampleIdentifier> sampleIdentifiers) {
return sampleIdentifiers
.stream()
.collect(
Collectors.groupingBy(
SampleIdentifier::getStudyId,
Collectors.mapping(SampleIdentifier::getSampleId, Collectors.toSet())));
}

@RequestMapping(value = "/virtual_study", method = RequestMethod.GET)
@ApiResponse(responseCode = "200", description = "OK",
content = @Content(array = @ArraySchema(schema = @Schema(implementation = VirtualStudy.class))))
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/cbioportal/web/parameter/VirtualStudyData.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public class VirtualStudyData implements Serializable {
private String typeOfCancerId;
private String pmid;

private Boolean dynamic;

public String getOwner() {
return owner;
}
Expand Down Expand Up @@ -122,4 +124,12 @@ public String getPmid() {
public void setPmid(String pmid) {
this.pmid = pmid;
}

public Boolean getDynamic() {
return dynamic;
}

public void setDynamic(Boolean dynamic) {
this.dynamic = dynamic;
}
}
92 changes: 92 additions & 0 deletions src/test/java/org/cbioportal/web/SessionServiceControllerTest.java
Original file line number Diff line number Diff line change
@@ -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")));
}
}
Loading