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

#162 add study id for runs #163

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ public AsyncDataFetcher getDataFetcher() {
}
if (ANALYSIS_ENTITY.equals(values.get("__typename"))) {
final Object analysisId = values.get("analysisId");
if (analysisId instanceof String) {
return Mono.just(new Analysis((String) analysisId));
final Object studyId = values.get("studyId");
if (analysisId instanceof String && studyId instanceof String) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to handle the cases with just analysisId or just studyId? I think they are both optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both are optional but for the Analysis object creation we need both the arguments..

return Mono.just(new Analysis((String) analysisId, (String) studyId));
}
}
if (WORKFLOW_ENTITY.equals(values.get("__typename"))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Strings.isNullOrEmpty;
import static java.util.stream.Collectors.toUnmodifiableList;
import static org.icgc_argo.workflow.search.model.SearchFields.ANALYSIS_ID;
import static org.icgc_argo.workflow.search.model.SearchFields.STUDY_ID;
import static org.icgc_argo.workflow.search.util.Converter.asImmutableMap;
import static org.icgc_argo.workflow.search.util.JacksonUtils.convertValue;

Expand Down Expand Up @@ -108,17 +109,21 @@ public AsyncDataFetcher<List<GqlRun>> getNestedRunInAnalysisDataFetcher() {
return environment -> {
val analysis = (Analysis) environment.getSource();
val analysisId = analysis.getAnalysisId();
val studyId = analysis.getStudyId();

ImmutableMap<String, Object> filter = asImmutableMap(environment.getArgument("filter"));
val filerAnalysisId = filter.getOrDefault(ANALYSIS_ID, analysisId);
val filterStudyId = filter.getOrDefault(STUDY_ID, studyId);

// short circuit here since can't find runs for invalid analysisId
if (isNullOrEmpty(analysisId) || !analysisId.equals(filerAnalysisId)) {
if ((isNullOrEmpty(analysisId) || !analysisId.equals(filerAnalysisId))
&& (isNullOrEmpty(studyId) || !analysisId.equals(filterStudyId))) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&& (isNullOrEmpty(studyId) || !analysisId.equals(filterStudyId))) {
&& (isNullOrEmpty(studyId) || !studyId.equals(filterStudyId))) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the code. Can you please review.

return Mono.empty();
}

Map<String, Object> mergedFilter = new HashMap<>(filter);
mergedFilter.put(ANALYSIS_ID, analysisId);
mergedFilter.put(STUDY_ID, studyId);

// Need to cast to get appropriate jackson annotation (camelCase property naming)
return runService.getRuns(mergedFilter, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,5 @@ public class SearchFields {
public static final String ANALYSIS_ID = "analysisId";
public static final String CPUS = "cpus";
public static final String MEMORY = "memory";
public static final String STUDY_ID = "studyId";
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public class Analysis {

private String analysisId;

private String studyId;

@SneakyThrows
public static Analysis parse(@NonNull Map<String, Object> sourceMap) {
return MAPPER.convertValue(sourceMap, Analysis.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ public class RunRepository {
"parameters.analysis_id",
"parameters.normal_aln_analysis_id",
"parameters.tumour_aln_analysis_id");

private static final List<String> STUDY_SEARCH_FIELDS =
List.of(
"parameters.study_id",
"parameters.normal_aln_study_id",
"parameters.tumour_aln_study_id");

private static final Map<String, Function<String, AbstractQueryBuilder<?>>> QUERY_RESOLVER =
argumentPathMap();

Expand Down Expand Up @@ -182,6 +189,13 @@ private static Map<String, Function<String, AbstractQueryBuilder<?>>> argumentPa
q.minimumShouldMatch("100%");
return q;
})
.put(
STUDY_ID,
value -> {
val q = new MultiMatchQueryBuilder(value, STUDY_SEARCH_FIELDS.toArray(String[]::new));
q.minimumShouldMatch("100%");
return q;
})
Copy link
Member

Choose a reason for hiding this comment

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

Should this be TermQueryBuilder instead of a MultiMatchQueryBuilder ? I think a run can have multiple analysis_ids but only a single Study ID ever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a TermQueryBuilder also I have made the changes accordingly.

.put(REPOSITORY, RunRepository::repositoryQueryFunc)
.build();
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ type Run @key(fields: "runId") {
commandLine: String
engineParameters: EngineParameters
tasks(taskId: String, state: String, tag: String): [Task]
producedAnalyses(filter: RunsFilter): [Analysis] @fetch(from: "producedAnalyses")
inputAnalyses(filter: RunsFilter): [Analysis] @requires(fields: "parameters") @fetch(from: "inputAnalyses")
}

type EngineParameters {
Expand Down Expand Up @@ -75,6 +77,7 @@ input RunsFilter {
state: String
repository: String
analysisId: String
studyId: String
}

input TasksFilter {
Expand All @@ -94,6 +97,7 @@ directive @fetch(from : String!) on FIELD_DEFINITION

type Analysis @key(fields: "analysisId") @extends {
analysisId: ID! @external
studyId: String! @external
inputForRuns(filter: RunsFilter): [Run]
}

Expand Down