Skip to content

Commit

Permalink
Empty entity group criteria should match any occurrences
Browse files Browse the repository at this point in the history 
The current behavior of empty entity group criteria is to returna null
filter, which effetively ignores the criteria. Instead, empty entity
group criteria should check for the existence of any matching
occurrence (e.g. any conditionOccurence for a person in a condition
criteria).
  • Loading branch information
@tjennison-work
tjennison-work committed Sep 16, 2024
1 parent 9442c99 commit 26ccebf
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 25 deletions.
15 changes: 10 additions & 5 deletions ...lay/src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class EntityGroupFilterBuilder extends EntityGroupFilterBuilderBase {
public EntityGroupFilterBuilder(CriteriaSelector criteriaSelector) {
Expand All @@ -35,9 +35,12 @@ public DTEntityGroup.EntityGroup deserializeData(String serialized) {

@Override
protected List<String> entityGroupIds() {
return deserializeConfig().getClassificationEntityGroupsList().stream()
.map(EntityGroupConfig::getId)
.collect(Collectors.toList());
CFEntityGroup.EntityGroup config = deserializeConfig();
return Stream.concat(
config.getClassificationEntityGroupsList().stream(),
config.getGroupingEntityGroupsList().stream())
.map(CFEntityGroup.EntityGroup.EntityGroupConfig::getId)
.toList();
}

@Override
Expand All @@ -58,6 +61,8 @@ protected Map<Literal, String> selectedIdsAndEntityGroups(String serializedSelec
@Override
protected ValueDataOuterClass.ValueData valueData(String serializedSelectionData) {
DTEntityGroup.EntityGroup selectionData = deserializeData(serializedSelectionData);
return selectionData.hasValueData() ? selectionData.getValueData() : null;
return selectionData != null && selectionData.hasValueData()
? selectionData.getValueData()
: null;
}
}
7 changes: 1 addition & 6 deletions ...src/main/java/bio/terra/tanagra/filterbuilder/impl/core/EntityGroupFilterBuilderBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ public EntityFilter buildForCohort(Underlay underlay, List<SelectionData> select
// We want to build one filter per entity group, not one filter per selected id.
Map<EntityGroup, List<Literal>> selectedIdsPerEntityGroup =
selectedIdsPerEntityGroup(underlay, criteriaSelectionData);
if (selectedIdsPerEntityGroup.isEmpty() && modifiersSelectionData.isEmpty()) {
// Empty selection data = null filter for a cohort.
return null;
}

List<EntityGroup> selectedEntityGroups =
selectedEntityGroups(underlay, selectedIdsPerEntityGroup);

Expand Down Expand Up @@ -139,9 +134,9 @@ public List<EntityOutput> buildForDataFeature(
throw new InvalidQueryException("Group by modifiers are not supported for data features");
}

// We want to build filters per entity group, not per selected id.
ValueDataOuterClass.ValueData valueData = valueData(criteriaSelectionData);

// We want to build filters per entity group, not per selected id.
Map<Entity, List<EntityFilter>> filtersPerEntity = new HashMap<>();
selectedEntityGroups.forEach(
entityGroup -> {
Expand Down
27 changes: 22 additions & 5 deletions ...va/bio/terra/tanagra/filterbuilder/EntityGroupFilterBuilderForCriteriaOccurrenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static bio.terra.tanagra.utils.ProtobufUtils.serializeToJson;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import bio.terra.tanagra.api.filter.AttributeFilter;
Expand Down Expand Up @@ -629,7 +628,13 @@ void criteriaWithAttrAndInstanceLevelAndGroupByModifiersCohortFilter() {

@Test
void emptyCriteriaCohortFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup config =
CFEntityGroup.EntityGroup.newBuilder()
.addClassificationEntityGroups(
CFEntityGroup.EntityGroup.EntityGroupConfig.newBuilder()
.setId("conditionPerson")
.build())
.build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"condition",
Expand All @@ -641,22 +646,34 @@ void emptyCriteriaCohortFilter() {
serializeToJson(config),
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);
EntityFilter expectedCohortFilter =
new PrimaryWithCriteriaFilter(
underlay,
(CriteriaOccurrence) underlay.getEntityGroup("conditionPerson"),
null,
Map.of(),
null,
null,
null);

// Null selection data.
SelectionData selectionData = new SelectionData("condition", null);
EntityFilter cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);

// Empty string selection data.
selectionData = new SelectionData("condition", "");
cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);

// Empty list selection.
DTEntityGroup.EntityGroup data = DTEntityGroup.EntityGroup.newBuilder().build();
selectionData = new SelectionData("condition", serializeToJson(data));
cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);
}

@Test
Expand Down
26 changes: 21 additions & 5 deletions ...y/src/test/java/bio/terra/tanagra/filterbuilder/EntityGroupFilterBuilderForGroupTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static bio.terra.tanagra.utils.ProtobufUtils.serializeToJson;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

import bio.terra.tanagra.api.filter.AttributeFilter;
import bio.terra.tanagra.api.filter.BooleanAndOrFilter;
Expand Down Expand Up @@ -412,7 +411,13 @@ void criteriaWithAttrAndGroupByModifiersCohortFilter() {

@Test
void emptyCriteriaCohortFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup config =
CFEntityGroup.EntityGroup.newBuilder()
.addClassificationEntityGroups(
CFEntityGroup.EntityGroup.EntityGroupConfig.newBuilder()
.setId("genotypingPerson")
.build())
.build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"genotyping",
Expand All @@ -424,22 +429,33 @@ void emptyCriteriaCohortFilter() {
serializeToJson(config),
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);
EntityFilter expectedCohortFilter =
new ItemInGroupFilter(
underlay,
(GroupItems) underlay.getEntityGroup("genotypingPerson"),
null,
null,
null,
null);

// Null selection data.
SelectionData selectionData = new SelectionData("genotyping", null);
EntityFilter cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);

// Empty string selection data.
selectionData = new SelectionData("genotyping", "");
cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);

// Empty list selection.
DTEntityGroup.EntityGroup data = DTEntityGroup.EntityGroup.newBuilder().build();
selectionData = new SelectionData("genotyping", serializeToJson(data));
cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);
}

@Test
Expand Down
19 changes: 15 additions & 4 deletions ...y/src/test/java/bio/terra/tanagra/filterbuilder/EntityGroupFilterBuilderForItemsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static bio.terra.tanagra.utils.ProtobufUtils.serializeToJson;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

import bio.terra.tanagra.api.filter.AttributeFilter;
import bio.terra.tanagra.api.filter.EntityFilter;
Expand Down Expand Up @@ -266,21 +265,33 @@ void emptyCriteriaCohortFilter() {
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

EntityFilter expectedCohortFilter =
new GroupHasItemsFilter(
underlay,
(GroupItems) underlay.getEntityGroup("bloodPressurePerson"),
null,
null,
null,
null);

// Null selection data.
SelectionData selectionData = new SelectionData("bloodPressure", null);
EntityFilter cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);

// Empty string selection data.
selectionData = new SelectionData("bloodPressure", "");
cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);

// Empty list selection.
DTEntityGroup.EntityGroup data = DTEntityGroup.EntityGroup.newBuilder().build();
selectionData = new SelectionData("bloodPressure", serializeToJson(data));
cohortFilter = filterBuilder.buildForCohort(underlay, List.of(selectionData));
assertNull(cohortFilter);
assertNotNull(cohortFilter);
assertEquals(expectedCohortFilter, cohortFilter);
}

@Test
Expand Down

0 comments on commit 26ccebf

Please sign in to comment.