From 8b37723ba70e0d51456cdeb78163b6995a2de948 Mon Sep 17 00:00:00 2001 From: rjohanek Date: Tue, 9 Jul 2024 10:08:32 -0400 Subject: [PATCH 01/11] update settings and snapshot access request types --- .../service/snapshot/SnapshotService.java | 10 +- .../api/data-repository-openapi.yaml | 25 ++--- .../service/snapshot/SnapshotServiceTest.java | 7 +- .../SnapshotBuilderTestData.java | 25 +---- src/test/resources/omop/settings.json | 103 +++++++++++++++--- .../omop/snapshot-access-request.json | 28 ++++- .../OMOPDataset/snapshot-access-request.json | 28 ++++- .../snapshot_builder_settings.json | 103 +++++++++++++++--- 8 files changed, 248 insertions(+), 81 deletions(-) diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index 6537c3636d..946e7aec41 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -34,7 +34,7 @@ import bio.terra.model.SamPolicyModel; import bio.terra.model.SnapshotAccessRequestResponse; import bio.terra.model.SnapshotAccessRequestStatus; -import bio.terra.model.SnapshotBuilderFeatureValueGroup; +import bio.terra.model.SnapshotBuilderOutputTable; import bio.terra.model.SnapshotBuilderSettings; import bio.terra.model.SnapshotBuilderTable; import bio.terra.model.SnapshotIdsAndRolesModel; @@ -722,18 +722,18 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( @VisibleForTesting List pullTables(SnapshotAccessRequestResponse snapshotRequestModel) { - var valueSets = snapshotRequestModel.getSnapshotSpecification().getValueSets(); - var valueSetNames = valueSets.stream().map(SnapshotBuilderFeatureValueGroup::getName).toList(); + var tables = snapshotRequestModel.getSnapshotSpecification().getOutputTables(); + var tableNames = tables.stream().map(SnapshotBuilderOutputTable::getName).toList(); Map tableMap = populateManualTableMap(); - Set missing = new HashSet<>(valueSetNames); + Set missing = new HashSet<>(tableNames); missing.removeAll(tableMap.keySet()); if (!missing.isEmpty()) { throw new IllegalArgumentException("Unknown value set names: " + missing); } - return valueSetNames.stream().map(tableMap::get).toList(); + return tableNames.stream().map(tableMap::get).toList(); } private Map populateManualTableMap() { diff --git a/src/main/resources/api/data-repository-openapi.yaml b/src/main/resources/api/data-repository-openapi.yaml index ca1af3905a..1db523de70 100644 --- a/src/main/resources/api/data-repository-openapi.yaml +++ b/src/main/resources/api/data-repository-openapi.yaml @@ -7216,10 +7216,6 @@ components: type: array items: $ref: '#/components/schemas/SnapshotBuilderProgramDataOption' - featureValueGroups: - type: array - items: - $ref: '#/components/schemas/SnapshotBuilderFeatureValueGroup' datasetConceptSets: type: array items: @@ -7308,14 +7304,11 @@ components: ⚠️ A concept set provided for the dataset, usually used to access tables that are not searchable as domains required: - name - - featureValueGroupName properties: name: type: string - featureValueGroupName: - type: string - concept: - $ref: '#/components/schemas/SnapshotBuilderConcept' + table: + $ref: '#/components/schemas/SnapshotBuilderTable' SnapshotBuilderDomainOption: allOf: @@ -7391,17 +7384,17 @@ components: items: $ref: '#/components/schemas/SnapshotBuilderParentConcept' - SnapshotBuilderFeatureValueGroup: + SnapshotBuilderOutputTable: type: object description: > ⚠️ Describes a set of columns for a table with given name required: - name - - values + - columns properties: name: type: string - values: + columns: type: array items: type: string @@ -7510,14 +7503,10 @@ components: type: array items: $ref: '#/components/schemas/SnapshotBuilderCohort' - conceptSets: - type: array - items: - $ref: '#/components/schemas/SnapshotBuilderDatasetConceptSet' - valueSets: + outputTables: type: array items: - $ref: '#/components/schemas/SnapshotBuilderFeatureValueGroup' + $ref: '#/components/schemas/SnapshotBuilderOutputTable' description: Specification of cohort and participant data for a requested snapshot. SnapshotBuilderCohort: diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java index 1ef9f58e84..430b7569f3 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java @@ -50,7 +50,7 @@ import bio.terra.model.SamPolicyModel; import bio.terra.model.SnapshotAccessRequestResponse; import bio.terra.model.SnapshotAccessRequestStatus; -import bio.terra.model.SnapshotBuilderFeatureValueGroup; +import bio.terra.model.SnapshotBuilderOutputTable; import bio.terra.model.SnapshotBuilderRequest; import bio.terra.model.SnapshotIdsAndRolesModel; import bio.terra.model.SnapshotLinkDuosDatasetResponse; @@ -1338,9 +1338,8 @@ void makeSnapshotFromSnapshotRequestByRequestId() { new SnapshotAccessRequestResponse() .snapshotSpecification( new SnapshotBuilderRequest() - .addValueSetsItem(new SnapshotBuilderFeatureValueGroup().name("Drug")) - .addValueSetsItem( - new SnapshotBuilderFeatureValueGroup().name("Condition")))); + .addOutputTablesItem(new SnapshotBuilderOutputTable().name("Drug")) + .addOutputTablesItem(new SnapshotBuilderOutputTable().name("Condition")))); Snapshot actual = service.makeSnapshotFromSnapshotRequest(snapshotRequestModel, dataset); SnapshotSource snapshotSource = new SnapshotSource().dataset(dataset); diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java index 2a873d781b..5543119c5c 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java @@ -13,8 +13,8 @@ import bio.terra.model.SnapshotBuilderDatasetConceptSet; import bio.terra.model.SnapshotBuilderDomainCriteria; import bio.terra.model.SnapshotBuilderDomainOption; -import bio.terra.model.SnapshotBuilderFeatureValueGroup; import bio.terra.model.SnapshotBuilderOption; +import bio.terra.model.SnapshotBuilderOutputTable; import bio.terra.model.SnapshotBuilderProgramDataListCriteria; import bio.terra.model.SnapshotBuilderProgramDataListItem; import bio.terra.model.SnapshotBuilderProgramDataListOption; @@ -168,21 +168,10 @@ private static SnapshotBuilderProgramDataListOption generateSnapshotBuilderProgr Person.RACE_CONCEPT_ID, "Race", List.of(new SnapshotBuilderProgramDataListItem().id(43).name("unused 3"))))) - .featureValueGroups( - List.of( - new SnapshotBuilderFeatureValueGroup().name("Condition"), - new SnapshotBuilderFeatureValueGroup().name("Observation"), - new SnapshotBuilderFeatureValueGroup().name("Procedure"), - new SnapshotBuilderFeatureValueGroup().name("Surveys"), - new SnapshotBuilderFeatureValueGroup().name("Person"))) .datasetConceptSets( List.of( - new SnapshotBuilderDatasetConceptSet() - .name("Demographics") - .featureValueGroupName("Person"), - new SnapshotBuilderDatasetConceptSet() - .name("All surveys") - .featureValueGroupName("Surveys"))); + new SnapshotBuilderDatasetConceptSet().name("Demographics"), + new SnapshotBuilderDatasetConceptSet().name("All surveys"))); public static final Column PERSON_ID_COLUMN = new Column().name("person_id").type(TableDataType.INTEGER); @@ -362,12 +351,8 @@ public static SnapshotBuilderCohort createCohort() { public static SnapshotBuilderRequest createSnapshotBuilderRequest() { return new SnapshotBuilderRequest() .addCohortsItem(createCohort()) - .addConceptSetsItem( - new SnapshotBuilderDatasetConceptSet() - .name("conceptSet") - .featureValueGroupName("featureValueGroupName")) - .addValueSetsItem(new SnapshotBuilderFeatureValueGroup().name("Drug")) - .addValueSetsItem(new SnapshotBuilderFeatureValueGroup().name("Condition")); + .addOutputTablesItem(new SnapshotBuilderOutputTable().name("Drug")) + .addOutputTablesItem(new SnapshotBuilderOutputTable().name("Condition")); } public static SnapshotAccessRequest createSnapshotAccessRequest(UUID sourceSnapshotId) { diff --git a/src/test/resources/omop/settings.json b/src/test/resources/omop/settings.json index 9b59aed8b2..0b52b4c990 100644 --- a/src/test/resources/omop/settings.json +++ b/src/test/resources/omop/settings.json @@ -170,37 +170,110 @@ ] } ], - "featureValueGroups": [ + "datasetConceptSets": [ { "name": "Drug", - "values": [] + "table": { + "datasetTableName": "drug_exposure", + "columns": [], + "primaryTableRelationship": "fpk_drug_person", + "secondaryTableRelationships": [ + "fpk_drug_type_concept", + "fpk_drug_concept", + "fpk_drug_route_concept", + "fpk_drug_concept_s"] + } },{ "name": "Measurement", - "values": [] + "table": { + "datasetTableName": "measurement", + "columns": [], + "primaryTableRelationship": "fpk_measurement_person", + "secondaryTableRelationships": [ + "fpk_measurement_concept", + "fpk_measurement_unit", + "fpk_measurement_concept_s", + "fpk_measurement_value", + "fpk_measurement_type_concept", + "fpk_measurement_operator"] + } },{ "name": "Visit", - "values": [] + "table": { + "datasetTableName": "visit_occurrence", + "columns": [], + "primaryTableRelationship": "fpk_visit_person", + "secondaryTableRelationships": [ + "fpk_visit_preceding", + "fpk_visit_concept_s", + "fpk_visit_type_concept", + "fpk_visit_concept", + "fpk_visit_discharge"] + } },{ "name": "Device", - "values": [] + "table": { + "datasetTableName": "device_exposure", + "columns": [], + "primaryTableRelationship": "fpk_device_person", + "secondaryTableRelationships": [ + "fpk_device_concept", + "fpk_device_concept_s", + "fpk_device_type_concept"] + } },{ "name": "Procedure", - "values": [] + "table": { + "datasetTableName": "procedure_occurrence", + "columns": [], + "primaryTableRelationship": "fpk_procedure_person", + "secondaryTableRelationships": [ + "fpk_procedure_concept", + "fpk_procedure_concept_s", + "fpk_procedure_type_concept", + "fpk_procedure_modifier"] + } },{ "name": "Condition", - "values": [] + "table": { + "datasetTableName": "condition_occurrence", + "columns": [], + "primaryTableRelationship": "fpk_condition_person", + "secondaryTableRelationships": [ + "fpk_condition_concept", + "fpk_condition_status_concept", + "fpk_condition_concept_s"] + } },{ "name": "Observation", - "values": [] + "table": { + "datasetTableName": "observation", + "columns": [], + "primaryTableRelationship": "fpk_observation_person", + "secondaryTableRelationships": [ + "fpk_observation_period_person", + "fpk_observation_concept", + "fpk_observation_concept_s", + "fpk_observation_unit", + "fpk_observation_qualifier", + "fpk_observation_type_concept", + "fpk_observation_period_concept", + "fpk_observation_value"] + } },{ - "name": "Person", - "values": [] - } - ], - "datasetConceptSets": [ - { "name": "Demographics", - "featureValueGroupName": "Person" + "table": { + "datasetTableName": "person", + "columns": [], + "secondaryTableRelationships": ["fpk_person_ethnicity_concept", "fpk_person_ethnicity_concept_s", "fpk_person_race_concept"] + } + },{ + "name": "Genomics", + "table": { + "datasetTableName": "sample", + "columns": [], + "primaryTableRelationship": "fpk_sample_person" + } } ] } diff --git a/src/test/resources/omop/snapshot-access-request.json b/src/test/resources/omop/snapshot-access-request.json index e41772995c..318c9dd30b 100644 --- a/src/test/resources/omop/snapshot-access-request.json +++ b/src/test/resources/omop/snapshot-access-request.json @@ -1,5 +1,29 @@ -{"sourceSnapshotId": "00000000-0000-0000-0000-000000000000", +{ + "sourceSnapshotId": "00000000-0000-0000-0000-000000000000", "name": "simple_snapshot_access_request", "researchPurposeStatement": "purpose", - "snapshotBuilderRequest": {"cohorts": [{"name": "test", "criteriaGroups": [{"name": "Group 1", "meetAll": false, "criteria": [], "mustMeet": true}]}], "valueSets": [{"name": "Condition", "values": []}], "conceptSets": [{"name": "Condition", "concept": {"id": 19, "code": null, "name": "Condition", "hasChildren": false}, "featureValueGroupName": "Condition"}]} + "snapshotBuilderRequest": { + "cohorts": [ + { + "name": "test", + "criteriaGroups": [ + { + "name": "Group 1", + "meetAll": false, + "criteria": [ ], + "mustMeet": true + } + ] + } + ], + "outputTables": [ + { + "name": "Condition", + "columns": [ + "Condition Column 1", + "Condition Column 2" + ] + } + ] + } } diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot-access-request.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot-access-request.json index e41772995c..318c9dd30b 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot-access-request.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot-access-request.json @@ -1,5 +1,29 @@ -{"sourceSnapshotId": "00000000-0000-0000-0000-000000000000", +{ + "sourceSnapshotId": "00000000-0000-0000-0000-000000000000", "name": "simple_snapshot_access_request", "researchPurposeStatement": "purpose", - "snapshotBuilderRequest": {"cohorts": [{"name": "test", "criteriaGroups": [{"name": "Group 1", "meetAll": false, "criteria": [], "mustMeet": true}]}], "valueSets": [{"name": "Condition", "values": []}], "conceptSets": [{"name": "Condition", "concept": {"id": 19, "code": null, "name": "Condition", "hasChildren": false}, "featureValueGroupName": "Condition"}]} + "snapshotBuilderRequest": { + "cohorts": [ + { + "name": "test", + "criteriaGroups": [ + { + "name": "Group 1", + "meetAll": false, + "criteria": [ ], + "mustMeet": true + } + ] + } + ], + "outputTables": [ + { + "name": "Condition", + "columns": [ + "Condition Column 1", + "Condition Column 2" + ] + } + ] + } } diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json index 9b59aed8b2..0b52b4c990 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json @@ -170,37 +170,110 @@ ] } ], - "featureValueGroups": [ + "datasetConceptSets": [ { "name": "Drug", - "values": [] + "table": { + "datasetTableName": "drug_exposure", + "columns": [], + "primaryTableRelationship": "fpk_drug_person", + "secondaryTableRelationships": [ + "fpk_drug_type_concept", + "fpk_drug_concept", + "fpk_drug_route_concept", + "fpk_drug_concept_s"] + } },{ "name": "Measurement", - "values": [] + "table": { + "datasetTableName": "measurement", + "columns": [], + "primaryTableRelationship": "fpk_measurement_person", + "secondaryTableRelationships": [ + "fpk_measurement_concept", + "fpk_measurement_unit", + "fpk_measurement_concept_s", + "fpk_measurement_value", + "fpk_measurement_type_concept", + "fpk_measurement_operator"] + } },{ "name": "Visit", - "values": [] + "table": { + "datasetTableName": "visit_occurrence", + "columns": [], + "primaryTableRelationship": "fpk_visit_person", + "secondaryTableRelationships": [ + "fpk_visit_preceding", + "fpk_visit_concept_s", + "fpk_visit_type_concept", + "fpk_visit_concept", + "fpk_visit_discharge"] + } },{ "name": "Device", - "values": [] + "table": { + "datasetTableName": "device_exposure", + "columns": [], + "primaryTableRelationship": "fpk_device_person", + "secondaryTableRelationships": [ + "fpk_device_concept", + "fpk_device_concept_s", + "fpk_device_type_concept"] + } },{ "name": "Procedure", - "values": [] + "table": { + "datasetTableName": "procedure_occurrence", + "columns": [], + "primaryTableRelationship": "fpk_procedure_person", + "secondaryTableRelationships": [ + "fpk_procedure_concept", + "fpk_procedure_concept_s", + "fpk_procedure_type_concept", + "fpk_procedure_modifier"] + } },{ "name": "Condition", - "values": [] + "table": { + "datasetTableName": "condition_occurrence", + "columns": [], + "primaryTableRelationship": "fpk_condition_person", + "secondaryTableRelationships": [ + "fpk_condition_concept", + "fpk_condition_status_concept", + "fpk_condition_concept_s"] + } },{ "name": "Observation", - "values": [] + "table": { + "datasetTableName": "observation", + "columns": [], + "primaryTableRelationship": "fpk_observation_person", + "secondaryTableRelationships": [ + "fpk_observation_period_person", + "fpk_observation_concept", + "fpk_observation_concept_s", + "fpk_observation_unit", + "fpk_observation_qualifier", + "fpk_observation_type_concept", + "fpk_observation_period_concept", + "fpk_observation_value"] + } },{ - "name": "Person", - "values": [] - } - ], - "datasetConceptSets": [ - { "name": "Demographics", - "featureValueGroupName": "Person" + "table": { + "datasetTableName": "person", + "columns": [], + "secondaryTableRelationships": ["fpk_person_ethnicity_concept", "fpk_person_ethnicity_concept_s", "fpk_person_race_concept"] + } + },{ + "name": "Genomics", + "table": { + "datasetTableName": "sample", + "columns": [], + "primaryTableRelationship": "fpk_sample_person" + } } ] } From 955ae07fc0954fd30e2ba5eacf79b92921af982e Mon Sep 17 00:00:00 2001 From: rjohanek Date: Tue, 9 Jul 2024 14:55:24 -0400 Subject: [PATCH 02/11] pull tables form settings and update test data --- .../service/snapshot/SnapshotService.java | 130 +++--------------- .../service/snapshot/SnapshotServiceTest.java | 14 +- .../SnapshotBuilderTestData.java | 57 +++++++- .../utils/constants/Measurement.java | 8 ++ .../utils/constants/Observation.java | 8 ++ .../constants/ObservationOccurrence.java | 8 -- src/test/resources/omop/settings.json | 1 + .../snapshot_builder_settings.json | 1 + 8 files changed, 103 insertions(+), 124 deletions(-) create mode 100644 src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java create mode 100644 src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Observation.java delete mode 100644 src/test/java/bio/terra/service/snapshotbuilder/utils/constants/ObservationOccurrence.java diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index 946e7aec41..3989759fe6 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -34,6 +34,7 @@ import bio.terra.model.SamPolicyModel; import bio.terra.model.SnapshotAccessRequestResponse; import bio.terra.model.SnapshotAccessRequestStatus; +import bio.terra.model.SnapshotBuilderDatasetConceptSet; import bio.terra.model.SnapshotBuilderOutputTable; import bio.terra.model.SnapshotBuilderSettings; import bio.terra.model.SnapshotBuilderTable; @@ -688,7 +689,7 @@ public SnapshotAccessRequestResponse getSnapshotAccessRequestById(UUID accessReq } public AssetSpecification buildAssetFromSnapshotAccessRequest( - Dataset dataset, SnapshotAccessRequestResponse snapshotRequestModel) { + Dataset dataset, SnapshotAccessRequestResponse snapshotAccessRequest) { // build asset model from snapshot request AssetModel assetModel = new AssetModel() @@ -699,7 +700,7 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( assetModel.addTablesItem(new AssetTableModel().name("person")); // Build asset tables and columns based on the concept sets included in the snapshot request - List tables = pullTables(snapshotRequestModel); + List tables = pullTables(snapshotAccessRequest); tables.forEach( table -> { assetModel.addTablesItem( @@ -721,116 +722,29 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( } @VisibleForTesting - List pullTables(SnapshotAccessRequestResponse snapshotRequestModel) { - var tables = snapshotRequestModel.getSnapshotSpecification().getOutputTables(); - var tableNames = tables.stream().map(SnapshotBuilderOutputTable::getName).toList(); - - Map tableMap = populateManualTableMap(); - - Set missing = new HashSet<>(tableNames); - missing.removeAll(tableMap.keySet()); + List pullTables(SnapshotAccessRequestResponse snapshotAccessRequest) { + List includedTableNames = + snapshotAccessRequest.getSnapshotSpecification().getOutputTables().stream() + .map(SnapshotBuilderOutputTable::getName) + .toList(); + List allTables = + snapshotBuilderSettingsDao + .getBySnapshotId(snapshotAccessRequest.getSourceSnapshotId()) + .getDatasetConceptSets(); + + Set missing = new HashSet<>(includedTableNames); + allTables.stream() + .map(SnapshotBuilderDatasetConceptSet::getName) + .toList() + .forEach(missing::remove); if (!missing.isEmpty()) { throw new IllegalArgumentException("Unknown value set names: " + missing); } - return tableNames.stream().map(tableMap::get).toList(); - } - - private Map populateManualTableMap() { - // manual definition of domain names -> dataset table - Map tableMap = new HashMap<>(); - tableMap.put( - "Demographics", - new SnapshotBuilderTable() - .datasetTableName("person") - .secondaryTableRelationships( - List.of( - "fpk_person_gender_concept", - "fpk_person_race_concept", - "fpk_person_ethnicity_concept", - "fpk_person_gender_concept_s", - "fpk_person_race_concept_s", - "fpk_person_ethnicity_concept_s"))); - tableMap.put( - "Drug", - new SnapshotBuilderTable() - .datasetTableName("drug_exposure") - .primaryTableRelationship("fpk_person_drug") - .secondaryTableRelationships( - List.of( - "fpk_drug_concept", - "fpk_drug_type_concept", - "fpk_drug_route_concept", - "fpk_drug_concept_s"))); - tableMap.put( - "Measurement", - new SnapshotBuilderTable() - .datasetTableName("measurement") - .primaryTableRelationship("fpk_person_measurement") - .secondaryTableRelationships( - List.of( - "fpk_measurement_concept", - "fpk_measurement_unit", - "fpk_measurement_concept_s", - "fpk_measurement_value", - "fpk_measurement_type_concept", - "fpk_measurement_operator"))); - tableMap.put( - "Visit", - new SnapshotBuilderTable() - .datasetTableName("visit_occurrence") - .primaryTableRelationship("fpk_person_visit") - .secondaryTableRelationships( - List.of( - "fpk_visit_preceding", - "fpk_visit_concept_s", - "fpk_visit_type_concept", - "fpk_visit_concept", - "fpk_visit_discharge"))); - tableMap.put( - "Device", - new SnapshotBuilderTable() - .datasetTableName("device_exposure") - .primaryTableRelationship("fpk_person_device") - .secondaryTableRelationships( - List.of("fpk_device_concept", "fpk_device_concept_s", "fpk_device_type_concept"))); - tableMap.put( - "Condition", - new SnapshotBuilderTable() - .datasetTableName("condition_occurrence") - .primaryTableRelationship("fpk_person_condition") - .secondaryTableRelationships( - List.of( - "fpk_condition_concept", - "fpk_condition_type_concept", - "fpk_condition_status_concept", - "fpk_condition_concept_s"))); - tableMap.put( - "Procedure", - new SnapshotBuilderTable() - .datasetTableName("procedure_occurrence") - .primaryTableRelationship("fpk_person_procedure") - .secondaryTableRelationships( - List.of( - "fpk_procedure_concept", - "fpk_procedure_concept_s", - "fpk_procedure_type_concept", - "fpk_procedure_modifier"))); - tableMap.put( - "Observation", - new SnapshotBuilderTable() - .datasetTableName("observation") - .primaryTableRelationship("fpk_person_observation") - .secondaryTableRelationships( - List.of( - "fpk_observation_concept", - "fpk_observation_concept_s", - "fpk_observation_unit", - "fpk_observation_qualifier", - "fpk_observation_type_concept", - "fpk_observation_value"))); - - return tableMap; + return allTables.stream() + .filter((table) -> includedTableNames.contains(table.getName())) + .map(SnapshotBuilderDatasetConceptSet::getTable) + .toList(); } public static AssetSpecification getAssetByNameFromDataset(Dataset dataset, String assetName) { diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java index 430b7569f3..d80d8ce1b4 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java @@ -160,6 +160,7 @@ class SnapshotServiceTest { @Mock private EcmService ecmService; @Mock private RawlsService rawlsService; @Mock private DuosClient duosClient; + @Mock private SnapshotBuilderSettingsDao settingsDao; private final UUID snapshotId = UUID.randomUUID(); private final UUID datasetId = UUID.randomUUID(); private final UUID snapshotTableId = UUID.randomUUID(); @@ -187,7 +188,7 @@ void beforeEach() { azureSynapsePdao, rawlsService, duosClient, - mock(SnapshotBuilderSettingsDao.class)); + settingsDao); } @Test @@ -1094,7 +1095,8 @@ void testCreateSnapshotWithByRequestId() { SnapshotAccessRequestResponse snapshotAccessRequestResponse = new SnapshotAccessRequestResponse() .status(SnapshotAccessRequestStatus.APPROVED) - .id(snapshotAccessRequestId); + .id(snapshotAccessRequestId) + .sourceSnapshotId(UUID.randomUUID()); when(snapshotRequestDao.getById(snapshotAccessRequestId)) .thenReturn(snapshotAccessRequestResponse); when(snapshotDao.retrieveSnapshot(snapshotAccessRequestResponse.getSourceSnapshotId())) @@ -1102,7 +1104,6 @@ void testCreateSnapshotWithByRequestId() { new Snapshot() .snapshotSources( List.of(new SnapshotSource().dataset(new Dataset().id(UUID.randomUUID()))))); - String result = service.createSnapshot( request, service.getSourceDatasetFromSnapshotRequest(request), TEST_USER); @@ -1333,13 +1334,17 @@ void makeSnapshotFromSnapshotRequestByRequestId() { Dataset dataset = SnapshotBuilderTestData.DATASET; dataset.name("datasetName"); + UUID sourceSnapshotId = UUID.randomUUID(); when(snapshotRequestDao.getById(snapshotAccessRequestId)) .thenReturn( new SnapshotAccessRequestResponse() + .sourceSnapshotId(sourceSnapshotId) .snapshotSpecification( new SnapshotBuilderRequest() .addOutputTablesItem(new SnapshotBuilderOutputTable().name("Drug")) .addOutputTablesItem(new SnapshotBuilderOutputTable().name("Condition")))); + when(settingsDao.getBySnapshotId(sourceSnapshotId)) + .thenReturn(SnapshotBuilderTestData.SETTINGS); Snapshot actual = service.makeSnapshotFromSnapshotRequest(snapshotRequestModel, dataset); SnapshotSource snapshotSource = new SnapshotSource().dataset(dataset); @@ -1453,6 +1458,8 @@ void pullTables() { var accessRequestResponse = SnapshotBuilderTestData.createSnapshotAccessRequestResponse(sourceSnapshotId); accessRequestResponse.id(snapshotAccessRequestId); + when(settingsDao.getBySnapshotId(sourceSnapshotId)) + .thenReturn(SnapshotBuilderTestData.SETTINGS); var firstTable = service.pullTables(accessRequestResponse).get(0); assertThat(firstTable.getDatasetTableName(), is("drug_exposure")); // Must preserve relationship order @@ -1477,6 +1484,7 @@ void buildAssetFromSnapshotAccessRequest() { var accessRequestResponse = SnapshotBuilderTestData.createSnapshotAccessRequestResponse(snapshotId); accessRequestResponse.id(snapshotAccessRequestId); + when(settingsDao.getBySnapshotId(snapshotId)).thenReturn(SnapshotBuilderTestData.SETTINGS); var actualAssetSpec = service.buildAssetFromSnapshotAccessRequest(sourceDataset, accessRequestResponse); diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java index 5543119c5c..a4ca9b2a2d 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java @@ -22,6 +22,7 @@ import bio.terra.model.SnapshotBuilderProgramDataRangeOption; import bio.terra.model.SnapshotBuilderRequest; import bio.terra.model.SnapshotBuilderSettings; +import bio.terra.model.SnapshotBuilderTable; import bio.terra.model.SnapshotRequestContentsModel; import bio.terra.model.SnapshotRequestIdModel; import bio.terra.model.SnapshotRequestModel; @@ -33,7 +34,8 @@ import bio.terra.service.snapshotbuilder.query.table.Person; import bio.terra.service.snapshotbuilder.utils.constants.ConditionOccurrence; import bio.terra.service.snapshotbuilder.utils.constants.DrugExposure; -import bio.terra.service.snapshotbuilder.utils.constants.ObservationOccurrence; +import bio.terra.service.snapshotbuilder.utils.constants.Measurement; +import bio.terra.service.snapshotbuilder.utils.constants.Observation; import bio.terra.service.snapshotbuilder.utils.constants.ProcedureOccurrence; import java.util.List; import java.util.UUID; @@ -122,8 +124,8 @@ private static SnapshotBuilderProgramDataListOption generateSnapshotBuilderProgr .hasChildren(true)), generateSnapshotBuilderDomainOption( OBSERVATION_DOMAIN_ID, - ObservationOccurrence.TABLE_NAME, - ObservationOccurrence.OBSERVATION_CONCEPT_ID, + Observation.TABLE_NAME, + Observation.OBSERVATION_CONCEPT_ID, "Observation", new SnapshotBuilderConcept() .id(300) @@ -170,8 +172,53 @@ private static SnapshotBuilderProgramDataListOption generateSnapshotBuilderProgr List.of(new SnapshotBuilderProgramDataListItem().id(43).name("unused 3"))))) .datasetConceptSets( List.of( - new SnapshotBuilderDatasetConceptSet().name("Demographics"), - new SnapshotBuilderDatasetConceptSet().name("All surveys"))); + new SnapshotBuilderDatasetConceptSet() + .name("Drug") + .table( + new SnapshotBuilderTable() + .datasetTableName(DrugExposure.TABLE_NAME) + .primaryTableRelationship("fpk_person_drug") + .secondaryTableRelationships( + List.of( + "fpk_drug_concept", + "fpk_drug_type_concept", + "fpk_drug_route_concept", + "fpk_drug_concept_s"))), + new SnapshotBuilderDatasetConceptSet() + .name("Condition") + .table( + new SnapshotBuilderTable() + .datasetTableName(ConditionOccurrence.TABLE_NAME) + .primaryTableRelationship("fpk_person_condition") + .secondaryTableRelationships( + List.of( + "fpk_condition_concept", + "fpk_condition_type_concept", + "fpk_condition_status_concept", + "fpk_condition_concept_s"))), + new SnapshotBuilderDatasetConceptSet() + .name("Procedure") + .table( + new SnapshotBuilderTable() + .datasetTableName(ProcedureOccurrence.TABLE_NAME)), + new SnapshotBuilderDatasetConceptSet() + .name("Observation") + .table(new SnapshotBuilderTable().datasetTableName(Observation.TABLE_NAME)), + new SnapshotBuilderDatasetConceptSet() + .name("Measurement") + .table(new SnapshotBuilderTable().datasetTableName(Measurement.TABLE_NAME)), + new SnapshotBuilderDatasetConceptSet() + .name("Visit") + .table(new SnapshotBuilderTable().datasetTableName("visit_occurrence")), + new SnapshotBuilderDatasetConceptSet() + .name("Device") + .table(new SnapshotBuilderTable().datasetTableName("device_exposure")), + new SnapshotBuilderDatasetConceptSet() + .name("Demographics") + .table(new SnapshotBuilderTable().datasetTableName(Person.TABLE_NAME)), + new SnapshotBuilderDatasetConceptSet() + .name("Genomics") + .table(new SnapshotBuilderTable().datasetTableName("sample")))); public static final Column PERSON_ID_COLUMN = new Column().name("person_id").type(TableDataType.INTEGER); diff --git a/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java b/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java new file mode 100644 index 0000000000..4784ce9ad3 --- /dev/null +++ b/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java @@ -0,0 +1,8 @@ +package bio.terra.service.snapshotbuilder.utils.constants; + +public class Measurement { + private Measurement() {} + + public static final String TABLE_NAME = "measurement"; + public static final String MEASUREMENT_CONCEPT_ID = "measurement_concept_id"; +} diff --git a/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Observation.java b/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Observation.java new file mode 100644 index 0000000000..1f638ae6fa --- /dev/null +++ b/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Observation.java @@ -0,0 +1,8 @@ +package bio.terra.service.snapshotbuilder.utils.constants; + +public final class Observation { + private Observation() {} + + public static final String TABLE_NAME = "observation"; + public static final String OBSERVATION_CONCEPT_ID = "observation_concept_id"; +} diff --git a/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/ObservationOccurrence.java b/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/ObservationOccurrence.java deleted file mode 100644 index 2a795c297b..0000000000 --- a/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/ObservationOccurrence.java +++ /dev/null @@ -1,8 +0,0 @@ -package bio.terra.service.snapshotbuilder.utils.constants; - -public final class ObservationOccurrence { - private ObservationOccurrence() {} - - public static final String TABLE_NAME = "observation_occurrence"; - public static final String OBSERVATION_CONCEPT_ID = "observation_concept_id"; -} diff --git a/src/test/resources/omop/settings.json b/src/test/resources/omop/settings.json index 0b52b4c990..eba4345259 100644 --- a/src/test/resources/omop/settings.json +++ b/src/test/resources/omop/settings.json @@ -241,6 +241,7 @@ "primaryTableRelationship": "fpk_condition_person", "secondaryTableRelationships": [ "fpk_condition_concept", + "fpk_condition_type_concept", "fpk_condition_status_concept", "fpk_condition_concept_s"] } diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json index 0b52b4c990..eba4345259 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json @@ -241,6 +241,7 @@ "primaryTableRelationship": "fpk_condition_person", "secondaryTableRelationships": [ "fpk_condition_concept", + "fpk_condition_type_concept", "fpk_condition_status_concept", "fpk_condition_concept_s"] } From 3699d671a5777e5bde2a931c84ae376512761753 Mon Sep 17 00:00:00 2001 From: rjohanek Date: Tue, 9 Jul 2024 16:05:41 -0400 Subject: [PATCH 03/11] update settings to match dev --- src/test/resources/omop/settings.json | 16 ++++++++-------- .../OMOPDataset/snapshot_builder_settings.json | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/test/resources/omop/settings.json b/src/test/resources/omop/settings.json index eba4345259..60b4162b6f 100644 --- a/src/test/resources/omop/settings.json +++ b/src/test/resources/omop/settings.json @@ -176,7 +176,7 @@ "table": { "datasetTableName": "drug_exposure", "columns": [], - "primaryTableRelationship": "fpk_drug_person", + "primaryTableRelationship": "fpk_person_drug", "secondaryTableRelationships": [ "fpk_drug_type_concept", "fpk_drug_concept", @@ -188,7 +188,7 @@ "table": { "datasetTableName": "measurement", "columns": [], - "primaryTableRelationship": "fpk_measurement_person", + "primaryTableRelationship": "fpk_person_measurement", "secondaryTableRelationships": [ "fpk_measurement_concept", "fpk_measurement_unit", @@ -202,7 +202,7 @@ "table": { "datasetTableName": "visit_occurrence", "columns": [], - "primaryTableRelationship": "fpk_visit_person", + "primaryTableRelationship": "fpk_person_visit", "secondaryTableRelationships": [ "fpk_visit_preceding", "fpk_visit_concept_s", @@ -215,7 +215,7 @@ "table": { "datasetTableName": "device_exposure", "columns": [], - "primaryTableRelationship": "fpk_device_person", + "primaryTableRelationship": "fpk_person_device", "secondaryTableRelationships": [ "fpk_device_concept", "fpk_device_concept_s", @@ -226,7 +226,7 @@ "table": { "datasetTableName": "procedure_occurrence", "columns": [], - "primaryTableRelationship": "fpk_procedure_person", + "primaryTableRelationship": "fpk_person_procedure", "secondaryTableRelationships": [ "fpk_procedure_concept", "fpk_procedure_concept_s", @@ -238,7 +238,7 @@ "table": { "datasetTableName": "condition_occurrence", "columns": [], - "primaryTableRelationship": "fpk_condition_person", + "primaryTableRelationship": "fpk_person_condition", "secondaryTableRelationships": [ "fpk_condition_concept", "fpk_condition_type_concept", @@ -250,7 +250,7 @@ "table": { "datasetTableName": "observation", "columns": [], - "primaryTableRelationship": "fpk_observation_person", + "primaryTableRelationship": "fpk_person_observation", "secondaryTableRelationships": [ "fpk_observation_period_person", "fpk_observation_concept", @@ -273,7 +273,7 @@ "table": { "datasetTableName": "sample", "columns": [], - "primaryTableRelationship": "fpk_sample_person" + "primaryTableRelationship": "fpk_person_sample" } } ] diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json index eba4345259..60b4162b6f 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json @@ -176,7 +176,7 @@ "table": { "datasetTableName": "drug_exposure", "columns": [], - "primaryTableRelationship": "fpk_drug_person", + "primaryTableRelationship": "fpk_person_drug", "secondaryTableRelationships": [ "fpk_drug_type_concept", "fpk_drug_concept", @@ -188,7 +188,7 @@ "table": { "datasetTableName": "measurement", "columns": [], - "primaryTableRelationship": "fpk_measurement_person", + "primaryTableRelationship": "fpk_person_measurement", "secondaryTableRelationships": [ "fpk_measurement_concept", "fpk_measurement_unit", @@ -202,7 +202,7 @@ "table": { "datasetTableName": "visit_occurrence", "columns": [], - "primaryTableRelationship": "fpk_visit_person", + "primaryTableRelationship": "fpk_person_visit", "secondaryTableRelationships": [ "fpk_visit_preceding", "fpk_visit_concept_s", @@ -215,7 +215,7 @@ "table": { "datasetTableName": "device_exposure", "columns": [], - "primaryTableRelationship": "fpk_device_person", + "primaryTableRelationship": "fpk_person_device", "secondaryTableRelationships": [ "fpk_device_concept", "fpk_device_concept_s", @@ -226,7 +226,7 @@ "table": { "datasetTableName": "procedure_occurrence", "columns": [], - "primaryTableRelationship": "fpk_procedure_person", + "primaryTableRelationship": "fpk_person_procedure", "secondaryTableRelationships": [ "fpk_procedure_concept", "fpk_procedure_concept_s", @@ -238,7 +238,7 @@ "table": { "datasetTableName": "condition_occurrence", "columns": [], - "primaryTableRelationship": "fpk_condition_person", + "primaryTableRelationship": "fpk_person_condition", "secondaryTableRelationships": [ "fpk_condition_concept", "fpk_condition_type_concept", @@ -250,7 +250,7 @@ "table": { "datasetTableName": "observation", "columns": [], - "primaryTableRelationship": "fpk_observation_person", + "primaryTableRelationship": "fpk_person_observation", "secondaryTableRelationships": [ "fpk_observation_period_person", "fpk_observation_concept", @@ -273,7 +273,7 @@ "table": { "datasetTableName": "sample", "columns": [], - "primaryTableRelationship": "fpk_sample_person" + "primaryTableRelationship": "fpk_person_sample" } } ] From 3026a9fc3aa382a3d35cd50d67f472a7e599e7a8 Mon Sep 17 00:00:00 2001 From: rjohanek Date: Fri, 12 Jul 2024 13:56:29 -0400 Subject: [PATCH 04/11] remove unnecessary toList call --- src/main/java/bio/terra/service/snapshot/SnapshotService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index 3989759fe6..45da7ed6e7 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -735,7 +735,6 @@ List pullTables(SnapshotAccessRequestResponse snapshotAcce Set missing = new HashSet<>(includedTableNames); allTables.stream() .map(SnapshotBuilderDatasetConceptSet::getName) - .toList() .forEach(missing::remove); if (!missing.isEmpty()) { throw new IllegalArgumentException("Unknown value set names: " + missing); From caa14d3095931de0c9d942be909ca128b133ce1a Mon Sep 17 00:00:00 2001 From: rjohanek Date: Fri, 12 Jul 2024 14:40:54 -0400 Subject: [PATCH 05/11] spotlessApply --- src/main/java/bio/terra/service/snapshot/SnapshotService.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index 45da7ed6e7..5e2da873e5 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -733,9 +733,7 @@ List pullTables(SnapshotAccessRequestResponse snapshotAcce .getDatasetConceptSets(); Set missing = new HashSet<>(includedTableNames); - allTables.stream() - .map(SnapshotBuilderDatasetConceptSet::getName) - .forEach(missing::remove); + allTables.stream().map(SnapshotBuilderDatasetConceptSet::getName).forEach(missing::remove); if (!missing.isEmpty()) { throw new IllegalArgumentException("Unknown value set names: " + missing); } From c25db6850f6b33ebbbf0b318f44ca4cf912c1a2d Mon Sep 17 00:00:00 2001 From: rjohanek Date: Mon, 15 Jul 2024 09:33:07 -0400 Subject: [PATCH 06/11] remove genomics from datasetConceptSets because sample table does not exist yet --- src/test/resources/omop/settings.json | 7 ------- .../files/OMOPDataset/snapshot_builder_settings.json | 7 ------- 2 files changed, 14 deletions(-) diff --git a/src/test/resources/omop/settings.json b/src/test/resources/omop/settings.json index 60b4162b6f..e1b928ee04 100644 --- a/src/test/resources/omop/settings.json +++ b/src/test/resources/omop/settings.json @@ -268,13 +268,6 @@ "columns": [], "secondaryTableRelationships": ["fpk_person_ethnicity_concept", "fpk_person_ethnicity_concept_s", "fpk_person_race_concept"] } - },{ - "name": "Genomics", - "table": { - "datasetTableName": "sample", - "columns": [], - "primaryTableRelationship": "fpk_person_sample" - } } ] } diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json index 60b4162b6f..e1b928ee04 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json @@ -268,13 +268,6 @@ "columns": [], "secondaryTableRelationships": ["fpk_person_ethnicity_concept", "fpk_person_ethnicity_concept_s", "fpk_person_race_concept"] } - },{ - "name": "Genomics", - "table": { - "datasetTableName": "sample", - "columns": [], - "primaryTableRelationship": "fpk_person_sample" - } } ] } From 2fb7a90084b6240c48a0de9825cb53564f237749 Mon Sep 17 00:00:00 2001 From: rjohanek Date: Mon, 15 Jul 2024 13:05:14 -0400 Subject: [PATCH 07/11] account for tables with primary or secondary relationships --- .../bio/terra/service/snapshot/SnapshotService.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index 58b1913c3c..ec0f41f200 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -712,11 +712,17 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( assetModel.addTablesItem( new AssetTableModel().name(table.getDatasetTableName()).columns(table.getColumns())); // First add all person <-> occurrence relationships - assetModel.addFollowItem(table.getPrimaryTableRelationship()); + if (table.getPrimaryTableRelationship() != null) { + assetModel.addFollowItem(table.getPrimaryTableRelationship()); + } }); // Second, add all occurrence <-> concept relationships tables.forEach( - table -> table.getSecondaryTableRelationships().forEach(assetModel::addFollowItem)); + table -> { + if (table.getSecondaryTableRelationships() != null) { + table.getSecondaryTableRelationships().forEach(assetModel::addFollowItem); + } + }); assetModel.addTablesItem(new AssetTableModel().name("concept")); From 889cfe2e3b5ca8e5e049c09c23d236f99b7a5e79 Mon Sep 17 00:00:00 2001 From: rjohanek Date: Mon, 15 Jul 2024 13:13:46 -0400 Subject: [PATCH 08/11] update settings to match what was hardcoded in asset creation --- src/test/resources/omop/settings.json | 9 ++++++--- .../files/OMOPDataset/snapshot_builder_settings.json | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/test/resources/omop/settings.json b/src/test/resources/omop/settings.json index e1b928ee04..507e496119 100644 --- a/src/test/resources/omop/settings.json +++ b/src/test/resources/omop/settings.json @@ -252,13 +252,11 @@ "columns": [], "primaryTableRelationship": "fpk_person_observation", "secondaryTableRelationships": [ - "fpk_observation_period_person", "fpk_observation_concept", "fpk_observation_concept_s", "fpk_observation_unit", "fpk_observation_qualifier", "fpk_observation_type_concept", - "fpk_observation_period_concept", "fpk_observation_value"] } },{ @@ -266,7 +264,12 @@ "table": { "datasetTableName": "person", "columns": [], - "secondaryTableRelationships": ["fpk_person_ethnicity_concept", "fpk_person_ethnicity_concept_s", "fpk_person_race_concept"] + "secondaryTableRelationships": ["fpk_person_gender_concept", + "fpk_person_race_concept", + "fpk_person_ethnicity_concept", + "fpk_person_gender_concept_s", + "fpk_person_race_concept_s", + "fpk_person_ethnicity_concept_s"] } } ] diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json index e1b928ee04..507e496119 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json @@ -252,13 +252,11 @@ "columns": [], "primaryTableRelationship": "fpk_person_observation", "secondaryTableRelationships": [ - "fpk_observation_period_person", "fpk_observation_concept", "fpk_observation_concept_s", "fpk_observation_unit", "fpk_observation_qualifier", "fpk_observation_type_concept", - "fpk_observation_period_concept", "fpk_observation_value"] } },{ @@ -266,7 +264,12 @@ "table": { "datasetTableName": "person", "columns": [], - "secondaryTableRelationships": ["fpk_person_ethnicity_concept", "fpk_person_ethnicity_concept_s", "fpk_person_race_concept"] + "secondaryTableRelationships": ["fpk_person_gender_concept", + "fpk_person_race_concept", + "fpk_person_ethnicity_concept", + "fpk_person_gender_concept_s", + "fpk_person_race_concept_s", + "fpk_person_ethnicity_concept_s"] } } ] From bd61314ba0ec75fe9d672cf9512cde26048dd83b Mon Sep 17 00:00:00 2001 From: rjohanek Date: Tue, 16 Jul 2024 11:56:20 -0400 Subject: [PATCH 09/11] add rootTable and dictionaryTable to settings --- .../service/snapshot/SnapshotService.java | 29 ++++++++++------- .../api/data-repository-openapi.yaml | 17 ++++++++++ .../service/snapshot/SnapshotServiceTest.java | 5 ++- .../SnapshotBuilderTestData.java | 9 +++++- src/test/resources/omop/settings.json | 32 +++++++++++-------- .../snapshot_builder_settings.json | 32 +++++++++++-------- 6 files changed, 83 insertions(+), 41 deletions(-) diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index ec0f41f200..5692175ec1 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -37,6 +37,7 @@ import bio.terra.model.SnapshotAccessRequestStatus; import bio.terra.model.SnapshotBuilderDatasetConceptSet; import bio.terra.model.SnapshotBuilderOutputTable; +import bio.terra.model.SnapshotBuilderRootTable; import bio.terra.model.SnapshotBuilderSettings; import bio.terra.model.SnapshotBuilderTable; import bio.terra.model.SnapshotIdsAndRolesModel; @@ -696,17 +697,21 @@ public SnapshotAccessRequestResponse getSnapshotAccessRequestById(UUID accessReq public AssetSpecification buildAssetFromSnapshotAccessRequest( Dataset dataset, SnapshotAccessRequestResponse snapshotAccessRequest) { + SnapshotBuilderSettings settings = + snapshotBuilderSettingsDao.getBySnapshotId(snapshotAccessRequest.getSourceSnapshotId()); // build asset model from snapshot request + SnapshotBuilderRootTable rootTable = settings.getRootTable(); AssetModel assetModel = new AssetModel() .name("snapshot-by-request-asset") - .rootTable("person") - .rootColumn("person_id"); - // Manually add dictionary tables, leave columns empty to return all columns - assetModel.addTablesItem(new AssetTableModel().name("person")); + .rootTable(rootTable.getDatasetTableName()) + .rootColumn(rootTable.getRootColumn()); + + // Add root table, leave columns empty to return all columns + assetModel.addTablesItem(new AssetTableModel().name(rootTable.getDatasetTableName())); // Build asset tables and columns based on the concept sets included in the snapshot request - List tables = pullTables(snapshotAccessRequest); + List tables = pullTables(snapshotAccessRequest, settings); tables.forEach( table -> { assetModel.addTablesItem( @@ -716,6 +721,7 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( assetModel.addFollowItem(table.getPrimaryTableRelationship()); } }); + // Second, add all occurrence <-> concept relationships tables.forEach( table -> { @@ -724,7 +730,9 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( } }); - assetModel.addTablesItem(new AssetTableModel().name("concept")); + // Add dictionary table, leave columns empty to return all columns + assetModel.addTablesItem( + new AssetTableModel().name(settings.getDictionaryTable().getDatasetTableName())); // Make sure we just built a valid asset model dataset.validateDatasetAssetSpecification(assetModel); @@ -734,15 +742,14 @@ public AssetSpecification buildAssetFromSnapshotAccessRequest( } @VisibleForTesting - List pullTables(SnapshotAccessRequestResponse snapshotAccessRequest) { + List pullTables( + SnapshotAccessRequestResponse snapshotAccessRequest, SnapshotBuilderSettings settings) { List includedTableNames = snapshotAccessRequest.getSnapshotSpecification().getOutputTables().stream() .map(SnapshotBuilderOutputTable::getName) .toList(); - List allTables = - snapshotBuilderSettingsDao - .getBySnapshotId(snapshotAccessRequest.getSourceSnapshotId()) - .getDatasetConceptSets(); + + List allTables = settings.getDatasetConceptSets(); Set missing = new HashSet<>(includedTableNames); allTables.stream().map(SnapshotBuilderDatasetConceptSet::getName).forEach(missing::remove); diff --git a/src/main/resources/api/data-repository-openapi.yaml b/src/main/resources/api/data-repository-openapi.yaml index f4f9588cfe..a867c4b02c 100644 --- a/src/main/resources/api/data-repository-openapi.yaml +++ b/src/main/resources/api/data-repository-openapi.yaml @@ -7207,6 +7207,7 @@ components: - programDataOptions - featureValueGroups - datasetConceptSets + - rootTable properties: domainOptions: type: array @@ -7221,6 +7222,10 @@ components: items: $ref: '#/components/schemas/SnapshotBuilderDatasetConceptSet' + rootTable: + $ref: '#/components/schemas/SnapshotBuilderRootTable' + dictionaryTable: + $ref: '#/components/schemas/SnapshotBuilderTable' SnapshotBuilderOption: type: object @@ -7420,6 +7425,18 @@ components: items: type: string + SnapshotBuilderRootTable: + allOf: + - $ref: '#/components/schemas/SnapshotBuilderTable' + - type: object + required: + - rootColumn + properties: + rootColumn: + type: string + description: > + The primary column in the root table that is used to join with other tables + EnumerateSnapshotAccessRequest: type: object description: The model for enumerated SnapshotAccessRequests. diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java index a59003d28d..3bf7a7a4b0 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java @@ -1478,9 +1478,8 @@ void pullTables() { var accessRequestResponse = SnapshotBuilderTestData.createSnapshotAccessRequestResponse(sourceSnapshotId); accessRequestResponse.id(snapshotAccessRequestId); - when(settingsDao.getBySnapshotId(sourceSnapshotId)) - .thenReturn(SnapshotBuilderTestData.SETTINGS); - var firstTable = service.pullTables(accessRequestResponse).get(0); + var firstTable = + service.pullTables(accessRequestResponse, SnapshotBuilderTestData.SETTINGS).get(0); assertThat(firstTable.getDatasetTableName(), is("drug_exposure")); // Must preserve relationship order assertThat(firstTable.getPrimaryTableRelationship(), equalTo("fpk_person_drug")); diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java index a4ca9b2a2d..3f6898272c 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java @@ -21,6 +21,7 @@ import bio.terra.model.SnapshotBuilderProgramDataRangeCriteria; import bio.terra.model.SnapshotBuilderProgramDataRangeOption; import bio.terra.model.SnapshotBuilderRequest; +import bio.terra.model.SnapshotBuilderRootTable; import bio.terra.model.SnapshotBuilderSettings; import bio.terra.model.SnapshotBuilderTable; import bio.terra.model.SnapshotRequestContentsModel; @@ -218,7 +219,13 @@ private static SnapshotBuilderProgramDataListOption generateSnapshotBuilderProgr .table(new SnapshotBuilderTable().datasetTableName(Person.TABLE_NAME)), new SnapshotBuilderDatasetConceptSet() .name("Genomics") - .table(new SnapshotBuilderTable().datasetTableName("sample")))); + .table(new SnapshotBuilderTable().datasetTableName("sample")))) + .rootTable( + (SnapshotBuilderRootTable) + new SnapshotBuilderRootTable() + .rootColumn(Person.PERSON_ID) + .datasetTableName(Person.TABLE_NAME)) + .dictionaryTable(new SnapshotBuilderTable().datasetTableName(Concept.TABLE_NAME)); public static final Column PERSON_ID_COLUMN = new Column().name("person_id").type(TableDataType.INTEGER); diff --git a/src/test/resources/omop/settings.json b/src/test/resources/omop/settings.json index 507e496119..8a3c187c95 100644 --- a/src/test/resources/omop/settings.json +++ b/src/test/resources/omop/settings.json @@ -259,18 +259,24 @@ "fpk_observation_type_concept", "fpk_observation_value"] } - },{ - "name": "Demographics", - "table": { - "datasetTableName": "person", - "columns": [], - "secondaryTableRelationships": ["fpk_person_gender_concept", - "fpk_person_race_concept", - "fpk_person_ethnicity_concept", - "fpk_person_gender_concept_s", - "fpk_person_race_concept_s", - "fpk_person_ethnicity_concept_s"] - } } - ] + ], + "rootTable": { + "datasetTableName": "person", + "rootColumn": "person_id", + "columns": [], + "secondaryTableRelationships": [ + "fpk_person_gender_concept", + "fpk_person_race_concept", + "fpk_person_ethnicity_concept", + "fpk_person_gender_concept_s", + "fpk_person_race_concept_s", + "fpk_person_ethnicity_concept_s" + ] + }, + "dictionaryTable": { + "datasetTableName": "concept", + "columns": [], + "secondaryTableRelationships": [] + } } diff --git a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json index 507e496119..8a3c187c95 100644 --- a/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json +++ b/tools/setupResourceScripts/files/OMOPDataset/snapshot_builder_settings.json @@ -259,18 +259,24 @@ "fpk_observation_type_concept", "fpk_observation_value"] } - },{ - "name": "Demographics", - "table": { - "datasetTableName": "person", - "columns": [], - "secondaryTableRelationships": ["fpk_person_gender_concept", - "fpk_person_race_concept", - "fpk_person_ethnicity_concept", - "fpk_person_gender_concept_s", - "fpk_person_race_concept_s", - "fpk_person_ethnicity_concept_s"] - } } - ] + ], + "rootTable": { + "datasetTableName": "person", + "rootColumn": "person_id", + "columns": [], + "secondaryTableRelationships": [ + "fpk_person_gender_concept", + "fpk_person_race_concept", + "fpk_person_ethnicity_concept", + "fpk_person_gender_concept_s", + "fpk_person_race_concept_s", + "fpk_person_ethnicity_concept_s" + ] + }, + "dictionaryTable": { + "datasetTableName": "concept", + "columns": [], + "secondaryTableRelationships": [] + } } From 9be742ee2e99a39d245ba6edf20bc441734f439d Mon Sep 17 00:00:00 2001 From: rjohanek Date: Tue, 16 Jul 2024 15:24:47 -0400 Subject: [PATCH 10/11] use test data for update settings tests --- .../bio/terra/app/controller/SnapshotsApiControllerTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/bio/terra/app/controller/SnapshotsApiControllerTest.java b/src/test/java/bio/terra/app/controller/SnapshotsApiControllerTest.java index ab6b609781..790d614b38 100644 --- a/src/test/java/bio/terra/app/controller/SnapshotsApiControllerTest.java +++ b/src/test/java/bio/terra/app/controller/SnapshotsApiControllerTest.java @@ -37,7 +37,6 @@ import bio.terra.model.SnapshotBuilderCountResponseResult; import bio.terra.model.SnapshotBuilderGetConceptHierarchyResponse; import bio.terra.model.SnapshotBuilderParentConcept; -import bio.terra.model.SnapshotBuilderSettings; import bio.terra.model.SnapshotModel; import bio.terra.model.SnapshotPreviewModel; import bio.terra.model.SnapshotRequestModel; @@ -497,7 +496,7 @@ void getSnapshotSnapshotBuilderSettingsNotFound() throws Exception { @Test void updateSnapshotSnapshotBuilderSettings() throws Exception { mockValidators(); - var snapshotBuilderSettings = new SnapshotBuilderSettings(); + var snapshotBuilderSettings = SnapshotBuilderTestData.SETTINGS; mvc.perform( put(SNAPSHOT_BUILDER_SETTINGS_ENDPOINT, SNAPSHOT_ID) .contentType(MediaType.APPLICATION_JSON) @@ -516,7 +515,7 @@ void updateSnapshotSnapshotBuilderSettingsForbidden() throws Exception { mvc.perform( put(SNAPSHOT_BUILDER_SETTINGS_ENDPOINT, SNAPSHOT_ID) .contentType(MediaType.APPLICATION_JSON) - .content("{}")) + .content(TestUtils.mapToJson(SnapshotBuilderTestData.SETTINGS))) .andExpect(status().isForbidden()); verifyAuthorizationCall(iamAction); From ef8bd5e4637a3f3f984a7f88a1b286c113c2f871 Mon Sep 17 00:00:00 2001 From: rjohanek Date: Wed, 17 Jul 2024 09:20:46 -0400 Subject: [PATCH 11/11] delete measurement table for test data --- .../service/snapshotbuilder/SnapshotBuilderTestData.java | 3 +-- .../snapshotbuilder/utils/constants/Measurement.java | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) delete mode 100644 src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java index 3f6898272c..ad56a9186c 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java @@ -35,7 +35,6 @@ import bio.terra.service.snapshotbuilder.query.table.Person; import bio.terra.service.snapshotbuilder.utils.constants.ConditionOccurrence; import bio.terra.service.snapshotbuilder.utils.constants.DrugExposure; -import bio.terra.service.snapshotbuilder.utils.constants.Measurement; import bio.terra.service.snapshotbuilder.utils.constants.Observation; import bio.terra.service.snapshotbuilder.utils.constants.ProcedureOccurrence; import java.util.List; @@ -207,7 +206,7 @@ private static SnapshotBuilderProgramDataListOption generateSnapshotBuilderProgr .table(new SnapshotBuilderTable().datasetTableName(Observation.TABLE_NAME)), new SnapshotBuilderDatasetConceptSet() .name("Measurement") - .table(new SnapshotBuilderTable().datasetTableName(Measurement.TABLE_NAME)), + .table(new SnapshotBuilderTable().datasetTableName("measurement")), new SnapshotBuilderDatasetConceptSet() .name("Visit") .table(new SnapshotBuilderTable().datasetTableName("visit_occurrence")), diff --git a/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java b/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java deleted file mode 100644 index 4784ce9ad3..0000000000 --- a/src/test/java/bio/terra/service/snapshotbuilder/utils/constants/Measurement.java +++ /dev/null @@ -1,8 +0,0 @@ -package bio.terra.service.snapshotbuilder.utils.constants; - -public class Measurement { - private Measurement() {} - - public static final String TABLE_NAME = "measurement"; - public static final String MEASUREMENT_CONCEPT_ID = "measurement_concept_id"; -}