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 17, 2024
1 parent cb5d4a0 commit 13df2e8
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 41 deletions.
16 changes: 10 additions & 6 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 @@ -5,13 +5,12 @@
import bio.terra.tanagra.api.shared.Literal;
import bio.terra.tanagra.proto.criteriaselector.ValueDataOuterClass;
import bio.terra.tanagra.proto.criteriaselector.configschema.CFEntityGroup;
import bio.terra.tanagra.proto.criteriaselector.configschema.CFEntityGroup.EntityGroup.EntityGroupConfig;
import bio.terra.tanagra.proto.criteriaselector.dataschema.DTEntityGroup;
import bio.terra.tanagra.underlay.uiplugin.CriteriaSelector;
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<CFEntityGroup.EntityGroup, DTEntityGroup.EntityGroup> {
Expand All @@ -36,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 @@ -59,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
57 changes: 37 additions & 20 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 @@ -56,7 +55,7 @@ void setup() {

@Test
void criteriaOnlyCohortFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup = CFEntityGroup.EntityGroup.newBuilder().build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"condition",
Expand All @@ -65,7 +64,7 @@ void criteriaOnlyCohortFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

Expand Down Expand Up @@ -629,7 +628,13 @@ void criteriaWithAttrAndInstanceLevelAndGroupByModifiersCohortFilter() {

@Test
void emptyCriteriaCohortFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup =
CFEntityGroup.EntityGroup.newBuilder()
.addClassificationEntityGroups(
CFEntityGroup.EntityGroup.EntityGroupConfig.newBuilder()
.setId("conditionPerson")
.build())
.build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"condition",
Expand All @@ -638,25 +643,37 @@ void emptyCriteriaCohortFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
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 All @@ -669,7 +686,7 @@ void emptyAttrModifierCohortFilter() {
true,
SZCorePlugin.ATTRIBUTE.getIdInConfig(),
serializeToJson(ageAtOccurrenceConfig));
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup = CFEntityGroup.EntityGroup.newBuilder().build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"condition",
Expand All @@ -678,7 +695,7 @@ void emptyAttrModifierCohortFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of(ageAtOccurrenceModifier));
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

Expand Down Expand Up @@ -743,7 +760,7 @@ void emptyGroupByModifierCohortFilter() {
false,
SZCorePlugin.UNHINTED_VALUE.getIdInConfig(),
serializeToJson(groupByConfig));
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup = CFEntityGroup.EntityGroup.newBuilder().build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"condition",
Expand All @@ -752,7 +769,7 @@ void emptyGroupByModifierCohortFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of(groupByModifier));
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

Expand Down Expand Up @@ -803,7 +820,7 @@ void emptyGroupByModifierCohortFilter() {

@Test
void criteriaOnlySingleOccurrenceDataFeatureFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup = CFEntityGroup.EntityGroup.newBuilder().build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"condition",
Expand All @@ -812,7 +829,7 @@ void criteriaOnlySingleOccurrenceDataFeatureFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

Expand Down Expand Up @@ -1015,7 +1032,7 @@ void criteriaWithAttrModifiersSingleOccurrenceDataFeatureFilter() {

@Test
void criteriaOnlyMultipleOccurrencesDataFeatureFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup = CFEntityGroup.EntityGroup.newBuilder().build();
CriteriaSelector criteriaSelector =
new CriteriaSelector(
"icd9cm",
Expand All @@ -1024,7 +1041,7 @@ void criteriaOnlyMultipleOccurrencesDataFeatureFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

Expand Down Expand Up @@ -1153,7 +1170,7 @@ void criteriaOnlyMultipleOccurrencesDataFeatureFilter() {

@Test
void criteriaWithAttrModifiersMultipleOccurrencesDataFeatureFilter() {
CFEntityGroup.EntityGroup config = CFEntityGroup.EntityGroup.newBuilder().build();
CFEntityGroup.EntityGroup entityGroup = CFEntityGroup.EntityGroup.newBuilder().build();
CFAttribute.Attribute ageAtOccurrenceConfig =
CFAttribute.Attribute.newBuilder().setAttribute("age_at_occurrence").build();
CriteriaSelector.Modifier ageAtOccurrenceModifier =
Expand All @@ -1170,7 +1187,7 @@ void criteriaWithAttrModifiersMultipleOccurrencesDataFeatureFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of(ageAtOccurrenceModifier));
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);

Expand Down Expand Up @@ -1231,7 +1248,7 @@ void criteriaWithAttrModifiersMultipleOccurrencesDataFeatureFilter() {

@Test
void emptyCriteriaDataFeatureFilter() {
CFEntityGroup.EntityGroup config =
CFEntityGroup.EntityGroup entityGroup =
CFEntityGroup.EntityGroup.newBuilder()
.addClassificationEntityGroups(
CFEntityGroup.EntityGroup.EntityGroupConfig.newBuilder()
Expand All @@ -1246,7 +1263,7 @@ void emptyCriteriaDataFeatureFilter() {
true,
"core.EntityGroupFilterBuilder",
SZCorePlugin.ENTITY_GROUP.getIdInConfig(),
serializeToJson(config),
serializeToJson(entityGroup),
List.of());
EntityGroupFilterBuilder filterBuilder = new EntityGroupFilterBuilder(criteriaSelector);
EntityOutput expectedEntityOutput1 =
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 13df2e8

Please sign in to comment.