diff --git a/controllers/cohortdata.go b/controllers/cohortdata.go index 0f00368..f7bd545 100644 --- a/controllers/cohortdata.go +++ b/controllers/cohortdata.go @@ -29,7 +29,7 @@ func NewCohortDataController(cohortDataModel models.CohortDataI, dataDictionaryM } func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Context) { - sourceId, cohortId, conceptIdsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() @@ -37,12 +37,8 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co } // parse cohortPairs separately as well, so we can validate permissions - _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error parsing request body for prefixed concept ids", "error": err.Error()}) - c.Abort() - return - } + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) + validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { log.Printf("Error: invalid request") @@ -51,7 +47,7 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co return } - cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs) + cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving histogram data", "error": err.Error()}) c.Abort() @@ -69,22 +65,15 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co } func (u CohortDataController) RetrieveStatsForCohortIdAndConceptId(c *gin.Context) { - sourceIdStr := c.Param("sourceid") - log.Printf("Querying source: %s", sourceIdStr) - cohortIdStr := c.Param("cohortid") - log.Printf("Querying cohort for cohort definition id: %s", cohortIdStr) - conceptIdStr := c.Param("conceptid") - if sourceIdStr == "" || cohortIdStr == "" || conceptIdStr == "" { - c.JSON(http.StatusBadRequest, gin.H{"message": "bad request"}) + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() return } - filterConceptIdsAndValues, cohortPairs, _ := utils.ParseConceptDefsAndDichotomousDefs(c) - - sourceId, _ := strconv.Atoi(sourceIdStr) - cohortId, _ := strconv.Atoi(cohortIdStr) - conceptId, _ := strconv.ParseInt(conceptIdStr, 10, 64) + // parse cohortPairs separately as well, so we can validate permissions + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { @@ -94,7 +83,7 @@ func (u CohortDataController) RetrieveStatsForCohortIdAndConceptId(c *gin.Contex return } - cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(sourceId, cohortId, conceptId, filterConceptIdsAndValues, cohortPairs) + cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()}) c.Abort() @@ -105,37 +94,30 @@ func (u CohortDataController) RetrieveStatsForCohortIdAndConceptId(c *gin.Contex for _, personData := range cohortData { conceptValues = append(conceptValues, float64(*personData.ConceptValueAsNumber)) } + conceptToStat, errGetLast := utils.CheckAndGetLastCustomConceptVariableDef(conceptDefsAndCohortPairs) + if errGetLast != nil { + c.JSON(http.StatusInternalServerError, gin.H{"message": "Error: last variable should be of numeric type", "error": errGetLast.Error()}) + c.Abort() + return + } - statsData := utils.GenerateStatsData(cohortId, conceptId, conceptValues) + statsData := utils.GenerateStatsData(cohortId, conceptToStat.ConceptId, conceptValues) c.JSON(http.StatusOK, gin.H{"statsData": statsData}) } func (u CohortDataController) RetrieveDataBySourceIdAndCohortIdAndVariables(c *gin.Context) { // TODO - add some validation to ensure that only calls from Argo are allowed through since it outputs FULL data? - - // parse and validate all parameters: - sourceIdStr := c.Param("sourceid") - log.Printf("Querying source: %s", sourceIdStr) - cohortIdStr := c.Param("cohortid") - log.Printf("Querying cohort for cohort definition id: %s", cohortIdStr) - if sourceIdStr == "" || cohortIdStr == "" { - c.JSON(http.StatusBadRequest, gin.H{"message": "bad request"}) - c.Abort() - return - } - - conceptIdsAndValues, cohortPairs, err := utils.ParseConceptDefsAndDichotomousDefs(c) - conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues) - + // -> this concern is considered to be addressed by https://github.com/uc-cdis/cloud-automation/pull/1884 + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error parsing request body for prefixed concept ids and dichotomous Ids", "error": err.Error()}) + c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() return } - sourceId, _ := strconv.Atoi(sourceIdStr) - cohortId, _ := strconv.Atoi(cohortIdStr) + // parse cohortPairs separately as well, so we can validate permissions + conceptDefs, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { @@ -145,17 +127,32 @@ func (u CohortDataController) RetrieveDataBySourceIdAndCohortIdAndVariables(c *g return } - // call model method: - cohortData, err := u.cohortDataModel.RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId, cohortId, conceptIds) - if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()}) - c.Abort() - return + // Iterate over conceptDefsAndCohortPairs and collect the concept values for each person: + // {PersonId:1, ConceptId:1, ConceptValue: "A value with, comma!"}, + // {PersonId:1, ConceptId:2, ConceptValue: B}, + // {PersonId:2, ConceptId:1, ConceptValue: C}, + var variablesToQuery []interface{} + var finalConceptDataset []*models.PersonConceptAndValue + for _, item := range conceptDefsAndCohortPairs { + variablesToQuery = append(variablesToQuery, item) + // if item is of type CustomConceptVariableDef, get the data: + if _, ok := item.(utils.CustomConceptVariableDef); ok { + // use variablesToQuery to query an increasingly tight set (simulating the attrition table that generated this query) + cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, variablesToQuery) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()}) + c.Abort() + return + } + // add to final concept data set: + finalConceptDataset = append(finalConceptDataset, cohortData...) + } } - partialCSV := GeneratePartialCSV(sourceId, cohortData, conceptIds) + conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptDefs) + partialCSV := GeneratePartialCSV(sourceId, finalConceptDataset, conceptIds) // use conceptdefs to improve column description? nah...no person is reading this table....just needs to be unique - personIdToCSVValues, err := u.RetrievePeopleIdAndCohort(sourceId, cohortId, cohortPairs, cohortData) + personIdToCSVValues, err := u.RetrievePeopleIdAndCohort(sourceId, cohortId, cohortPairs, finalConceptDataset) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving people ID to csv value map", "error": err.Error()}) c.Abort() @@ -287,13 +284,12 @@ func populateConceptValue(row []string, cohortItem models.PersonConceptAndValue, func (u CohortDataController) RetrieveCohortOverlapStats(c *gin.Context) { errors := make([]error, 4) var sourceId, caseCohortId, controlCohortId int - var conceptIdsAndValues []utils.CustomConceptVariableDef - var cohortPairs []utils.CustomDichotomousVariableDef + var conceptDefsAndCohortPairs []interface{} sourceId, errors[0] = utils.ParseNumericArg(c, "sourceid") caseCohortId, errors[1] = utils.ParseNumericArg(c, "casecohortid") controlCohortId, errors[2] = utils.ParseNumericArg(c, "controlcohortid") - conceptIdsAndValues, cohortPairs, errors[3] = utils.ParseConceptDefsAndDichotomousDefs(c) - conceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues) + conceptDefsAndCohortPairs, errors[3] = utils.ParseConceptDefsAndDichotomousDefsAsSingleList(c) + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{caseCohortId, controlCohortId}, cohortPairs) if !validAccessRequest { @@ -309,7 +305,7 @@ func (u CohortDataController) RetrieveCohortOverlapStats(c *gin.Context) { return } overlapStats, err := u.cohortDataModel.RetrieveCohortOverlapStats(sourceId, caseCohortId, - controlCohortId, conceptIds, cohortPairs) + controlCohortId, conceptDefsAndCohortPairs) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving stats", "error": err.Error()}) c.Abort() diff --git a/controllers/concept.go b/controllers/concept.go index 7e3e149..7a14798 100644 --- a/controllers/concept.go +++ b/controllers/concept.go @@ -125,13 +125,16 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortId(c *gin.Co } func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariables(c *gin.Context) { - sourceId, cohortId, conceptIds, cohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesList(c) + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { - log.Printf("Error: %s", err.Error()) c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() return } + + // parse cohortPairs separately as well, so we can validate permissions + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) + validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { log.Printf("Error: invalid request") @@ -147,7 +150,7 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariabl c.Abort() return } - breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, conceptIds, cohortPairs, breakdownConceptId) + breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId) if err != nil { log.Printf("Error: %s", err.Error()) c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving stats", "error": err.Error()}) @@ -190,14 +193,14 @@ func generateRowForVariable(variableName string, breakdownConceptValuesToPeopleC } func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { - sourceId, cohortId, conceptIdsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) + sourceId, cohortId, conceptDefsAndCohortPairs, err := utils.ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { log.Printf("Error: %s", err.Error()) c.JSON(http.StatusBadRequest, gin.H{"message": "bad request", "error": err.Error()}) c.Abort() return } - _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) + _, cohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs) if !validAccessRequest { log.Printf("Error: invalid request") @@ -238,7 +241,7 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { c.Abort() return } - otherAttritionRows, err := u.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) + otherAttritionRows, err := u.GetAttritionRowForConceptDefsAndCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues) if err != nil { log.Printf("Error: %s", err.Error()) c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept breakdown rows for filter conceptIds and cohortPairs", "error": err.Error()}) @@ -249,13 +252,13 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) { c.String(http.StatusOK, b.String()) } -func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId int, cohortId int, conceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) { +func (u ConceptController) GetAttritionRowForConceptDefsAndCohortPairs(sourceId int, cohortId int, conceptDefsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([][]string, error) { var otherAttritionRows [][]string - for idx, conceptIdOrCohortPair := range conceptIdsAndCohortPairs { - // attrition filter: run each query with an increasingly longer list of filterConceptIdsAndCohortPairs, until the last query is run with them all: - filterConceptIdsAndCohortPairs := conceptIdsAndCohortPairs[0 : idx+1] + for idx, conceptIdOrCohortPair := range conceptDefsAndCohortPairs { + // attrition filter: run each query with an increasingly longer list of filterConceptDefsAndCohortPairs, until the last query is run with them all: + filterConceptDefsAndCohortPairs := conceptDefsAndCohortPairs[0 : idx+1] - attritionRow, err := u.GetAttritionRowForConceptIdOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) + attritionRow, err := u.GetAttritionRowForConceptDefOrCohortPair(sourceId, cohortId, conceptIdOrCohortPair, filterConceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues) if err != nil { log.Printf("Error: %s", err.Error()) return nil, err @@ -265,10 +268,10 @@ func (u ConceptController) GetAttritionRowForConceptIdsAndCohortPairs(sourceId i return otherAttritionRows, nil } -func (u ConceptController) GetAttritionRowForConceptIdOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptIdsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) { - filterConceptDefsAndValues, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptIdsAndCohortPairs) +func (u ConceptController) GetAttritionRowForConceptDefOrCohortPair(sourceId int, cohortId int, conceptIdOrCohortPair interface{}, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64, sortedConceptValues []string) ([]string, error) { + filterConceptDefsAndValues, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptDefsAndCohortPairs) filterConceptIds := utils.ExtractConceptIdsFromCustomConceptVariablesDef(filterConceptDefsAndValues) - breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, filterConceptIds, filterCohortPairs, breakdownConceptId) + breakdownStats, err := u.conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortId, filterConceptDefsAndCohortPairs, breakdownConceptId) if err != nil { return nil, fmt.Errorf("could not retrieve concept Breakdown for concepts %v dichotomous variables %v due to error: %s", filterConceptIds, filterCohortPairs, err.Error()) } diff --git a/models/cohortdata.go b/models/cohortdata.go index 1f0195f..9e49997 100644 --- a/models/cohortdata.go +++ b/models/cohortdata.go @@ -9,10 +9,8 @@ import ( ) type CohortDataI interface { - RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId int, cohortDefinitionId int, conceptIds []int64) ([]*PersonConceptAndValue, error) - RetrieveCohortOverlapStats(sourceId int, caseCohortId int, controlCohortId int, otherFilterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef) (CohortOverlapStats, error) + RetrieveCohortOverlapStats(sourceId int, caseCohortId int, controlCohortId int, filterConceptDefsAndCohortPairs []interface{}) (CohortOverlapStats, error) RetrieveDataByOriginalCohortAndNewCohort(sourceId int, originalCohortDefinitionId int, cohortDefinitionId int) ([]*PersonIdAndCohort, error) - RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptIdsAndValues []utils.CustomConceptVariableDef, filterCohortPairs []utils.CustomDichotomousVariableDef) ([]*PersonConceptAndValue, error) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*PersonConceptAndValue, error) RetrieveBarGraphDataBySourceIdAndCohortIdAndConceptIds(sourceId int, conceptId int64) ([]*NominalGroupData, error) RetrieveHistogramDataBySourceIdAndConceptId(sourceId int, histogramConceptId int64) ([]*PersonConceptAndValue, error) @@ -73,56 +71,29 @@ func (h CohortData) RetrieveDataByOriginalCohortAndNewCohort(sourceId int, origi return personData, meta_result.Error } -// Retrieves observation data. -// Assumption is that both OMOP and RESULTS schemas -// are on same DB. -func (h CohortData) RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId int, cohortDefinitionId int, conceptIds []int64) ([]*PersonConceptAndValue, error) { - log.Printf(">> Using inner join impl. for large cohorts") - var dataSourceModel = new(Source) - omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) - - resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) - - // get the observations for the subjects and the concepts, to build up the data rows to return: - var cohortData []*PersonConceptAndValue - query := omopDataSource.Db.Table(omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()). - Select("observation.person_id, observation.observation_concept_id as concept_id, concept.concept_class_id, value_as_concept.concept_name as observation_value_as_concept_name, observation.value_as_number as concept_value_as_number, observation.value_as_concept_id as concept_value_as_concept_id"). - Joins("INNER JOIN "+resultsDataSource.Schema+".cohort as cohort ON cohort.subject_id = observation.person_id"). - Joins("INNER JOIN "+omopDataSource.Schema+".concept as concept ON concept.concept_id = observation.observation_concept_id"). - Joins("LEFT JOIN "+omopDataSource.Schema+".concept as value_as_concept ON value_as_concept.concept_id = observation.value_as_concept_id"). - Where("cohort.cohort_definition_id = ?", cohortDefinitionId). - Where("observation.observation_concept_id in (?)", conceptIds). - Order("observation.person_id asc") // this order is important! - query, cancel := utils.AddTimeoutToQuery(query) - defer cancel() - meta_result := query.Scan(&cohortData) - return cohortData, meta_result.Error -} - func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*PersonConceptAndValue, error) { var dataSourceModel = new(Source) omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) finalCohortAlias := "final_cohort_alias" - histogramConcept, err := utils.CheckAndGetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs) - if err != nil { - log.Fatalf("failed: %v", err) - return nil, err - } // get the observations for the subjects and the concepts, to build up the data rows to return: var cohortData []*PersonConceptAndValue session := resultsDataSource.Db.Session(&gorm.Session{}) - err = session.Transaction(func(query *gorm.DB) error { // TODO - rename query? + err := session.Transaction(func(query *gorm.DB) error { // TODO - rename query? query, tmpTableName := QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalCohortAlias) if tmpTableName != "" { query = query.Select("distinct(" + tmpTableName + ".person_id), " + tmpTableName + ".observation_concept_id as concept_id, " + tmpTableName + ".value_as_number as concept_value_as_number") } else { + histogramConcept, errGetLast := utils.CheckAndGetLastCustomConceptVariableDef(filterConceptDefsAndCohortPairs) + if errGetLast != nil { + log.Fatalf("failed: %v", errGetLast) + return errGetLast + } query = query.Select("distinct(observation.person_id), observation.observation_concept_id as concept_id, observation.value_as_number as concept_value_as_number"). Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON "+finalCohortAlias+".subject_id = observation.person_id"). Where("observation.observation_concept_id = ?", histogramConcept.ConceptId). Where("observation.value_as_number is not null") } - query, cancel := utils.AddTimeoutToQuery(query) defer cancel() meta_result := query.Scan(&cohortData) @@ -135,27 +106,6 @@ func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlus return cohortData, err } -// DEPRECATED - USE RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs instead. -func (h CohortData) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptDefs []utils.CustomConceptVariableDef, filterCohortPairs []utils.CustomDichotomousVariableDef) ([]*PersonConceptAndValue, error) { - var dataSourceModel = new(Source) - omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) - resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) - - // get the observations for the subjects and the concepts, to build up the data rows to return: - var cohortData []*PersonConceptAndValue - query := QueryFilterByCohortPairsHelper(filterCohortPairs, resultsDataSource, cohortDefinitionId, "unionAndIntersect"). - Select("distinct(observation.person_id), observation.observation_concept_id as concept_id, observation.value_as_number as concept_value_as_number"). - Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON unionAndIntersect.subject_id = observation.person_id"). - Where("observation.observation_concept_id = ?", histogramConceptId). - Where("observation.value_as_number is not null") - - query = QueryFilterByConceptDefsHelper(query, sourceId, filterConceptDefs, omopDataSource, resultsDataSource.Schema, "unionAndIntersect") - query, cancel := utils.AddTimeoutToQuery(query) - defer cancel() - meta_result := query.Scan(&cohortData) - return cohortData, meta_result.Error -} - func (h CohortData) RetrieveHistogramDataBySourceIdAndConceptId(sourceId int, histogramConceptId int64) ([]*PersonConceptAndValue, error) { var dataSourceModel = new(Source) omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) @@ -193,27 +143,28 @@ func (h CohortData) RetrieveBarGraphDataBySourceIdAndCohortIdAndConceptIds(sourc return cohortData, meta_result.Error } -// Basically the same as the method above, but without the extra filtering on filterConceptId and filterConceptValue: func (h CohortData) RetrieveCohortOverlapStats(sourceId int, caseCohortId int, controlCohortId int, - filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef) (CohortOverlapStats, error) { + filterConceptDefsAndCohortPairs []interface{}) (CohortOverlapStats, error) { var dataSourceModel = new(Source) omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) var cohortOverlapStats CohortOverlapStats - query := QueryFilterByCohortPairsHelper(filterCohortPairs, resultsDataSource, caseCohortId, "case_cohort_unionedAndIntersectedWithFilters"). - Select("count(distinct(case_cohort_unionedAndIntersectedWithFilters.subject_id)) as case_control_overlap"). - Joins("INNER JOIN " + resultsDataSource.Schema + ".cohort as control_cohort ON control_cohort.subject_id = case_cohort_unionedAndIntersectedWithFilters.subject_id") // this one allows for the intersection between case and control and the assessment of the overlap + session := resultsDataSource.Db.Session(&gorm.Session{}) + err := session.Transaction(func(query *gorm.DB) error { + finalSetAlias := "case_cohort_unionedAndIntersectedWithFilters" + query, _ = QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, caseCohortId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalSetAlias) - if len(filterConceptIds) > 0 { - query = QueryFilterByConceptIdsHelper(query, sourceId, filterConceptIds, omopDataSource, resultsDataSource.Schema, "control_cohort.subject_id") - } - query = query.Where("control_cohort.cohort_definition_id = ?", controlCohortId) - query, cancel := utils.AddTimeoutToQuery(query) - defer cancel() - meta_result := query.Scan(&cohortOverlapStats) - return cohortOverlapStats, meta_result.Error + query = query.Select("count(distinct("+finalSetAlias+".subject_id)) as case_control_overlap"). + Joins("INNER JOIN "+resultsDataSource.Schema+".cohort as control_cohort ON control_cohort.subject_id = "+finalSetAlias+".subject_id"). // this one allows for the intersection between case and control and the assessment of the overlap + Where("control_cohort.cohort_definition_id = ?", controlCohortId) + query, cancel := utils.AddTimeoutToQuery(query) + defer cancel() + meta_result := query.Scan(&cohortOverlapStats) + return meta_result.Error + }) + return cohortOverlapStats, err } func (p *PersonConceptAndCount) String() string { diff --git a/models/concept.go b/models/concept.go index 33f870d..6724942 100644 --- a/models/concept.go +++ b/models/concept.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/uc-cdis/cohort-middleware/utils" + "gorm.io/gorm" ) type ConceptI interface { @@ -12,7 +13,7 @@ type ConceptI interface { RetrieveInfoBySourceIdAndConceptIds(sourceId int, conceptIds []int64) ([]*ConceptSimple, error) RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptTypes []string) ([]*ConceptSimple, error) RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error) - RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*ConceptBreakdown, error) + RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*ConceptBreakdown, error) } type Concept struct { ConceptId int64 `json:"concept_id"` @@ -127,39 +128,40 @@ func (h Concept) RetrieveInfoBySourceIdAndConceptTypes(sourceId int, conceptType // {ConceptValue: "B", NPersonsInCohortWithValue: N-M}, func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortId(sourceId int, cohortDefinitionId int, breakdownConceptId int64) ([]*ConceptBreakdown, error) { // this is identical to the result of the function below if called with empty filterConceptIds[] and empty filterCohortPairs... so call that: - filterConceptIds := []int64{} - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - return h.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortDefinitionId, filterConceptIds, filterCohortPairs, breakdownConceptId) + var filterConceptDefsAndCohortPairs []interface{} + return h.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, breakdownConceptId) } // Basically same goal as described in function above, but only count persons that have a non-null value for each -// of the ids in the given filterConceptIds. So, using the example documented in the function above, it will +// of the given filters. So, using the example documented in the function above, it will // return something like: // // {ConceptValue: "A", NPersonsInCohortWithValue: M-X}, // {ConceptValue: "B", NPersonsInCohortWithValue: N-M-X}, // -// where X is the number of persons that have NO value or just a "null" value for one or more of the ids in the given filterConceptIds. -func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*ConceptBreakdown, error) { - +// where X is the number of persons that have NO value or just a "null" value for one or more of the concepts in the given filters. +// Also supports cohort pairs in filters, filtering out persons that don't pass the cohort pair filter (drill down to that method for detais). +// Furthermore, this method also applies transformations and filtering, if these items are specified for the given concept definitions. +func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*ConceptBreakdown, error) { var dataSourceModel = new(Source) omopDataSource := dataSourceModel.GetDataSource(sourceId, Omop) resultsDataSource := dataSourceModel.GetDataSource(sourceId, Results) - - // count persons, grouping by concept value: + finalSetAlias := "final_set_alias" var conceptBreakdownList []*ConceptBreakdown - query := QueryFilterByCohortPairsHelper(filterCohortPairs, resultsDataSource, cohortDefinitionId, "unionAndIntersect"). - Select("observation.value_as_concept_id, count(distinct(observation.person_id)) as npersons_in_cohort_with_value"). - Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON unionAndIntersect.subject_id = observation.person_id"). - Where("observation.observation_concept_id = ?", breakdownConceptId). - Where(GetConceptValueNotNullCheckBasedOnConceptType("observation", sourceId, breakdownConceptId)) - - query = QueryFilterByConceptIdsHelper(query, sourceId, filterConceptIds, omopDataSource, resultsDataSource.Schema, "unionAndIntersect.subject_id") - - query, cancel := utils.AddTimeoutToQuery(query) - defer cancel() - meta_result := query.Group("observation.value_as_concept_id"). - Scan(&conceptBreakdownList) + session := resultsDataSource.Db.Session(&gorm.Session{}) + err := session.Transaction(func(query *gorm.DB) error { + query, _ = QueryFilterByConceptDefsPlusCohortPairsHelper(query, sourceId, cohortDefinitionId, filterConceptDefsAndCohortPairs, omopDataSource, resultsDataSource, finalSetAlias) + // count persons, grouping by concept value: + query = query.Select("observation.value_as_concept_id, count(distinct(observation.person_id)) as npersons_in_cohort_with_value"). + Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as observation"+omopDataSource.GetViewDirective()+" ON "+finalSetAlias+".subject_id = observation.person_id"). + Where("observation.observation_concept_id = ?", breakdownConceptId). + Where(GetConceptValueNotNullCheckBasedOnConceptType("observation", sourceId, breakdownConceptId)). + Group("observation.value_as_concept_id") + query, cancel := utils.AddTimeoutToQuery(query) + defer cancel() + meta_result := query.Scan(&conceptBreakdownList) + return meta_result.Error + }) // Add concept value (coded value) and concept name for each of the value_as_concept_id values: for _, conceptBreakdownItem := range conceptBreakdownList { @@ -170,5 +172,5 @@ func (h Concept) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCoho conceptBreakdownItem.ConceptValue = conceptInfo.ConceptCode conceptBreakdownItem.ValueName = conceptInfo.ConceptName } - return conceptBreakdownList, meta_result.Error + return conceptBreakdownList, err } diff --git a/models/helper.go b/models/helper.go index 70744ce..a8c5ef6 100644 --- a/models/helper.go +++ b/models/helper.go @@ -21,47 +21,36 @@ func QueryFilterByConceptDefsPlusCohortPairsHelper(query *gorm.DB, sourceId int, // Caching of temporary tables: for optimal performance, a temporary table dictionary / cache is updated, keeping a mapping // of existing temporary table names vs underlying subsets of items in filterConceptDefsAndCohortPairs that gave rise to these // tables. + // Returns: the final query, + // and, if the last variable in filterConceptDefsAndCohortPairs is a concept/numeric one, + // this method also returns the final observation table alias representing the filter + // added for this variable to the final query. query = query.Table(resultsDataSource.Schema+".cohort as "+finalCohortAlias). Select("*"). - Where("cohort_definition_id=?", mainCohortDefinitionId) + Where(finalCohortAlias+".cohort_definition_id=?", mainCohortDefinitionId) - tmpTableName := "" + finalObservationTableAlias := "" var err error for i, item := range filterConceptDefsAndCohortPairs { observationTableAlias := fmt.Sprintf("filter_%d", i) switch convertedItem := item.(type) { case utils.CustomConceptVariableDef: - query, tmpTableName, err = QueryFilterByConceptDefHelper(query, sourceId, convertedItem, omopDataSource, finalCohortAlias, observationTableAlias) + query, finalObservationTableAlias, err = QueryFilterByConceptDefHelper(query, sourceId, convertedItem, omopDataSource, finalCohortAlias, observationTableAlias) case utils.CustomDichotomousVariableDef: query = QueryFilterByCohortPairHelper(query, convertedItem, resultsDataSource, mainCohortDefinitionId, finalCohortAlias+".subject_id", observationTableAlias) - tmpTableName = "" + finalObservationTableAlias = "" } } if err != nil { log.Fatalf("Error: %s", err.Error()) panic("error") } - return query, tmpTableName + return query, finalObservationTableAlias } -// DEPRECATED - USE QueryFilterByConceptDefsHelper // Helper function that adds extra filter clauses to the query, joining on the right set of tables. // - It was added here to make it reusable, given these filters need to be added to many of the queries that take in -// a list of filters in the form of concept ids. -func QueryFilterByConceptIdsHelper(query *gorm.DB, sourceId int, filterConceptIds []int64, - omopDataSource *utils.DbAndSchema, resultSchemaName string, personIdFieldForObservationJoin string) *gorm.DB { - // iterate over the filterConceptIds, adding a new INNER JOIN and filters for each, so that the resulting set is the - // set of persons that have a non-null value for each and every one of the concepts: - for i, filterConceptId := range filterConceptIds { - observationTableAlias := fmt.Sprintf("observation_filter_%d", i) - log.Printf("Adding extra INNER JOIN with alias %s", observationTableAlias) - query = query.Joins("INNER JOIN "+omopDataSource.Schema+".observation_continuous as "+observationTableAlias+omopDataSource.GetViewDirective()+" ON "+observationTableAlias+".person_id = "+personIdFieldForObservationJoin). - Where(observationTableAlias+".observation_concept_id = ?", filterConceptId). - Where(GetConceptValueNotNullCheckBasedOnConceptType(observationTableAlias, sourceId, filterConceptId)) - } - return query -} - +// a list of filters in the form of concept definitions. func QueryFilterByConceptDefHelper(query *gorm.DB, sourceId int, filterConceptDef utils.CustomConceptVariableDef, omopDataSource *utils.DbAndSchema, finalCohortAlias string, observationTableAlias string) (*gorm.DB, string, error) { // 1 - check if filterConceptDef has a transformation @@ -82,6 +71,7 @@ func QueryFilterByConceptDefHelper(query *gorm.DB, sourceId int, filterConceptDe omopDataSource, personIdFieldForObservationJoin, "observation_continuous", observationAliasSimpleQuery) tmpTransformedTable, err := TransformDataIntoTempTable(omopDataSource, query, filterConceptDef) // TODO - the resulting query should actually be Select * from temptable.... as this collapses all underlying queries. + // Apply any filtering AFTER the transformation: query = queryJoinAndFilterByConceptDef(query, sourceId, filterConceptDef, omopDataSource, personIdFieldForObservationJoin, tmpTransformedTable, observationAliasFullQuery) return query, observationAliasFullQuery, err @@ -90,7 +80,7 @@ func QueryFilterByConceptDefHelper(query *gorm.DB, sourceId int, filterConceptDe // simple filterConceptDef with no transformation query := queryJoinAndFilterByConceptDef(query, sourceId, filterConceptDef, omopDataSource, personIdFieldForObservationJoin, "observation_continuous", observationTableAlias) - return query, "", nil + return query, observationTableAlias, nil } } diff --git a/server/router.go b/server/router.go index 5eda8f0..5fce42a 100644 --- a/server/router.go +++ b/server/router.go @@ -54,7 +54,7 @@ func NewRouter() *gin.Engine { authorized.POST("/cohort-data/by-source-id/:sourceid/by-cohort-definition-id/:cohortid", cohortData.RetrieveDataBySourceIdAndCohortIdAndVariables) // cohort data statistics - authorized.POST("/cohort-stats/by-source-id/:sourceid/by-cohort-definition-id/:cohortid/by-concept-id/:conceptid", cohortData.RetrieveStatsForCohortIdAndConceptId) + authorized.POST("/cohort-stats/by-source-id/:sourceid/by-cohort-definition-id/:cohortid", cohortData.RetrieveStatsForCohortIdAndConceptId) // histogram endpoint authorized.POST("/histogram/by-source-id/:sourceid/by-cohort-definition-id/:cohortid", cohortData.RetrieveHistogramForCohortIdAndConceptId) diff --git a/tests/controllers_tests/controllers_test.go b/tests/controllers_tests/controllers_test.go index 0d07577..782aa0a 100644 --- a/tests/controllers_tests/controllers_test.go +++ b/tests/controllers_tests/controllers_test.go @@ -67,25 +67,14 @@ var cohortDefinitionControllerWithFailingTeamProjectAuthz = controllers.NewCohor type dummyCohortDataModel struct{} -func (h dummyCohortDataModel) RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId int, cohortDefinitionId int, conceptIds []int64) ([]*models.PersonConceptAndValue, error) { - value := float32(0.0) +func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*models.PersonConceptAndValue, error) { + value := float32(5.1) cohortData := []*models.PersonConceptAndValue{ - {PersonId: 1, ConceptId: 10, ConceptClassId: "something", ObservationValueAsConceptName: "abc", ConceptValueAsNumber: &value}, + {PersonId: 1, ConceptId: 2000000324, ConceptClassId: "something", ObservationValueAsConceptName: "abc", ConceptValueAsNumber: &value}, } return cohortData, nil } -func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(sourceId int, cohortDefinitionId int, histogramConceptId int64, filterConceptIds []utils.CustomConceptVariableDef, filterCohortPairs []utils.CustomDichotomousVariableDef) ([]*models.PersonConceptAndValue, error) { - - cohortData := []*models.PersonConceptAndValue{} - return cohortData, nil -} - -func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}) ([]*models.PersonConceptAndValue, error) { - cohortData := []*models.PersonConceptAndValue{} - return cohortData, nil -} - func (h dummyCohortDataModel) RetrieveHistogramDataBySourceIdAndConceptId(sourceId int, histogramConceptId int64) ([]*models.PersonConceptAndValue, error) { cohortData := []*models.PersonConceptAndValue{} @@ -98,7 +87,7 @@ func (h dummyCohortDataModel) RetrieveBarGraphDataBySourceIdAndCohortIdAndConcep } func (h dummyCohortDataModel) RetrieveCohortOverlapStats(sourceId int, caseCohortId int, controlCohortId int, - otherFilterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef) (models.CohortOverlapStats, error) { + filterConceptDefsAndCohortPairs []interface{}) (models.CohortOverlapStats, error) { var zeroOverlap models.CohortOverlapStats return zeroOverlap, nil } @@ -274,10 +263,12 @@ func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortId(sourc } return conceptBreakdown, nil } -func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId int, cohortDefinitionId int, filterConceptIds []int64, filterCohortPairs []utils.CustomDichotomousVariableDef, breakdownConceptId int64) ([]*models.ConceptBreakdown, error) { +func (h dummyConceptDataModel) RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(sourceId int, cohortDefinitionId int, filterConceptDefsAndCohortPairs []interface{}, breakdownConceptId int64) ([]*models.ConceptBreakdown, error) { + filterConceptDefs, filterCohortPairs := utils.GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(filterConceptDefsAndCohortPairs) + conceptBreakdown := []*models.ConceptBreakdown{ {ConceptValue: "value1", NpersonsInCohortWithValue: 4 - len(filterCohortPairs)}, // simulate decreasing numbers as filter increases - the use of filterCohortPairs instead of filterConceptIds is otherwise meaningless here... - {ConceptValue: "value2", NpersonsInCohortWithValue: 7 - len(filterConceptIds)}, // simulate decreasing numbers as filter increases- the use of filterConceptIds instead of filterCohortPairs is otherwise meaningless here... + {ConceptValue: "value2", NpersonsInCohortWithValue: 7 - len(filterConceptDefs)}, // simulate decreasing numbers as filter increases- the use of filterConceptIds instead of filterCohortPairs is otherwise meaningless here... } if dummyModelReturnError { return nil, fmt.Errorf("error!") @@ -382,7 +373,7 @@ func TestRetrieveDataBySourceIdAndCohortIdAndVariablesCorrectParams(t *testing.T requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortid", Value: "1"}) requestContext.Writer = new(tests.CustomResponseWriter) requestContext.Request = new(http.Request) - requestBody := "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 2000000324},{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [1, 3]}]}" + requestBody := "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 2000000324},{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [2, 3]}]}" requestContext.Request.Body = io.NopCloser(strings.NewReader(requestBody)) cohortDataController.RetrieveDataBySourceIdAndCohortIdAndVariables(requestContext) // Params above are correct, so request should NOT abort: @@ -390,8 +381,22 @@ func TestRetrieveDataBySourceIdAndCohortIdAndVariablesCorrectParams(t *testing.T t.Errorf("Did not expect this request to abort") } result := requestContext.Writer.(*tests.CustomResponseWriter) - if !strings.Contains(result.CustomResponseWriterOut, "sample.id,") { - t.Errorf("Expected output starting with 'sample.id,...'") + expectedOut := "sample.id,ID_2000000324,ID_2_3\n1,abc,0\n" + if !strings.Contains(result.CustomResponseWriterOut, expectedOut) { + t.Errorf("Expected output to contain %s", expectedOut) + } + // test with different data + requestBody = "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 1111111},{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [1, 3]}]}" + requestContext.Request.Body = io.NopCloser(strings.NewReader(requestBody)) + cohortDataController.RetrieveDataBySourceIdAndCohortIdAndVariables(requestContext) + // Params above are correct, so request should NOT abort: + if requestContext.IsAborted() { + t.Errorf("Did not expect this request to abort") + } + result = requestContext.Writer.(*tests.CustomResponseWriter) + expectedOut = "sample.id,ID_1111111,ID_1_3\n1,NA,NA\n" + if !strings.Contains(result.CustomResponseWriterOut, expectedOut) { + t.Errorf("Expected output to contain %s", expectedOut) } // the same request should fail if the teamProject authorization fails: @@ -742,11 +747,17 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndVariablesModelError(t *te requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortid", Value: "1"}) requestContext.Params = append(requestContext.Params, gin.Param{Key: "breakdownconceptid", Value: "1"}) requestContext.Request = new(http.Request) - requestContext.Request.Body = io.NopCloser(strings.NewReader("{\"ConceptIds\":[1234,5678]}")) + requestContext.Request.Body = io.NopCloser(strings.NewReader("{\"variables\":[]}")) requestContext.Writer = new(tests.CustomResponseWriter) // set flag to let mock model layer return error instead of mock data: dummyModelReturnError = true conceptController.RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariables(requestContext) + + // Expected response status is "500": + if requestContext.Writer.Status() != 500 { + t.Errorf("Expected status to start with '500', got '%d'", requestContext.Writer.Status()) + } + if !requestContext.IsAborted() { t.Errorf("Expected aborted request") } @@ -903,7 +914,7 @@ func TestGenerateHeaderAndNonFilterRow(t *testing.T) { } } -func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { +func TestGetAttritionRowForConceptDefsAndCohortPairs(t *testing.T) { setUp(t) sourceId := 1 cohortId := 1 @@ -911,7 +922,7 @@ func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { sortedConceptValues := []string{"value1", "value2", "value3"} // mix of concept ids and CustomDichotomousVariableDef items: - conceptIdsAndCohortPairs := []interface{}{ + conceptDefsAndCohortPairs := []interface{}{ utils.CustomConceptVariableDef{ConceptId: int64(1234)}, utils.CustomConceptVariableDef{ConceptId: int64(5678)}, utils.CustomDichotomousVariableDef{ @@ -925,10 +936,10 @@ func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) { ProvidedName: "testB34"}, } - result, _ := conceptController.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues) - if len(result) != len(conceptIdsAndCohortPairs) { + result, _ := conceptController.GetAttritionRowForConceptDefsAndCohortPairs(sourceId, cohortId, conceptDefsAndCohortPairs, breakdownConceptId, sortedConceptValues) + if len(result) != len(conceptDefsAndCohortPairs) { t.Errorf("Expected %d data lines, found %d lines in total", - len(conceptIdsAndCohortPairs), + len(conceptDefsAndCohortPairs), len(result)) t.Errorf("Lines: %s", result) } @@ -1228,13 +1239,12 @@ func TestRetrieveStatsForCohortIdAndConceptIdWithCorrectParams(t *testing.T) { requestContext := new(gin.Context) requestContext.Params = append(requestContext.Params, gin.Param{Key: "sourceid", Value: strconv.Itoa(tests.GetTestSourceId())}) requestContext.Params = append(requestContext.Params, gin.Param{Key: "cohortid", Value: "4"}) - requestContext.Params = append(requestContext.Params, gin.Param{Key: "conceptid", Value: "2000006885"}) requestContext.Writer = new(tests.CustomResponseWriter) requestContext.Request = new(http.Request) - requestBody := "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 2000000324},{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [1, 3]}]}" + requestBody := "{\"variables\":[{\"variable_type\": \"concept\", \"concept_id\": 2000000324},{\"variable_type\": \"custom_dichotomous\", \"cohort_ids\": [1, 3]},{\"variable_type\": \"concept\", \"concept_id\": 2000006885}]}" requestContext.Request.Body = io.NopCloser(strings.NewReader(requestBody)) cohortDataController.RetrieveStatsForCohortIdAndConceptId(requestContext) - // Params above are correct, so request should NOT abort: + // Params above are correct, with the concept of interest as last variable, so request should NOT abort: if requestContext.IsAborted() { t.Errorf("Did not expect this request to abort") } diff --git a/tests/models_tests/models_test.go b/tests/models_tests/models_test.go index ec3ab91..fdf2a1d 100644 --- a/tests/models_tests/models_test.go +++ b/tests/models_tests/models_test.go @@ -222,13 +222,13 @@ func TestRetrieveInfoBySourceIdAndConceptTypesWrongType(t *testing.T) { } } -func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsNoResults(t *testing.T) { +func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairsNoResults(t *testing.T) { setUp(t) // empty: - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, + var filterConceptDefsAndCohortPairs []interface{} + stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, smallestCohort.Id, - allConceptIds, filterCohortPairs, allConceptIds[0]) + filterConceptDefsAndCohortPairs, allConceptIds[0]) // none of the subjects has a value in all the concepts, so we expect len==0 here: if len(stats) != 0 { t.Errorf("Expected no results, found %d", len(stats)) @@ -414,18 +414,21 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) { func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPairsWithResults(t *testing.T) { setUp(t) - filterIds := []int64{hareConceptId} populationCohort := largestCohort // setting the largest and smallest cohorts here as a pair: - filterCohortPairs := []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs := []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: largestCohort.Id, ProvidedName: "test"}, } + breakdownConceptId := hareConceptId // not normally the case...but we'll use the same here just for the test... - stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - populationCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + populationCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) // we expect results, and we expect the total of persons to be 6, since only 6 of the persons // in largestCohort have a HARE value (and smallestCohort does not overlap with largest): countPersons := 0 @@ -439,18 +442,18 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPai // then we should expect a reduction in the number of persons found. The reduction in this case // will take place because of a smaller intersection of the new cohorts with the population cohort, // and because of an overlaping person found in the two cohorts of the new pair. - filterCohortPairs = []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: largestCohort.Id, ProvidedName: "test"}, - { + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: secondLargestCohort.Id, CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, ProvidedName: "test2"}, } - stats, _ = conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - populationCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats, _ = conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + populationCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) countPersons = 0 for _, stat := range stats { countPersons += stat.NpersonsInCohortWithValue @@ -460,22 +463,24 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPai } } -func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsWithResults(t *testing.T) { +func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairsWithResults(t *testing.T) { setUp(t) - filterIds := []int64{hareConceptId} - // setting the same cohort id here (artificial...but just to check if that returns the same value as when this filter is not there): - filterCohortPairs := []utils.CustomDichotomousVariableDef{ - { + // setting the same extendedCopyOfSecondLargestCohort.Id here (artificial...but just to check if that returns the same value as when this filter is not there): + filterConceptDefsAndCohortPairs := []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: secondLargestCohort.Id, CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, ProvidedName: "test"}, } breakdownConceptId := hareConceptId // not normally the case...but we'll use the same here just for the test... - stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - extendedCopyOfSecondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats, err := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + extendedCopyOfSecondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) // we expect values since secondLargestCohort has multiple subjects with hare info: if len(stats) < 4 { - t.Errorf("Expected at least 4 results, found %d", len(stats)) + t.Errorf("Expected at least 4 results, found %d. Error: %v", len(stats), err) } prevName := "" for _, stat := range stats { @@ -489,26 +494,33 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsW } prevName = stat.ValueName } - // test without the filterCohortPairs, should return the same result: - filterCohortPairs = []utils.CustomDichotomousVariableDef{} - stats2, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - extendedCopyOfSecondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + // test without the CustomDichotomousVariableDefs, should return the same result: + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + } + stats2, err := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + extendedCopyOfSecondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) // very rough check (ideally we would check the individual stats as well...TODO?): if len(stats) > len(stats2) { - t.Errorf("First query is more restrictive, so its stats should not be larger than stats2 of second query. Got %d and %d", len(stats), len(stats2)) + t.Errorf("First query is more restrictive, so its stats should not be larger than stats2 of second query. Got %d and %d. Error: %v", len(stats), len(stats2), err) } // test filtering with secondLargestCohort, smallest and largestCohort. // Lenght of result set should be 2 persons (one HIS, one ASN), since there is a overlap of 1 between secondLargestCohort and smallest cohort, // and overlap of 2 between secondLargestCohort and largestCohort, BUT only 1 has a HARE value: - filterCohortPairs = []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: hareConceptId, + }, + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: largestCohort.Id, ProvidedName: "test"}, } - stats3, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, - secondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId) + stats3, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + secondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) if len(stats3) != 2 { t.Errorf("Expected only two items in resultset, found %d", len(stats3)) } @@ -519,6 +531,62 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsW if countPersons != 2 { t.Errorf("Expected only two persons in resultset, found %d", countPersons) } + + // test with a different concept: + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: tests.GetTestHistogramConceptId(), + }, + } + stats4, err := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + extendedCopyOfSecondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) + // expecting counts on all 4 HARE codes: + if len(stats4) != 4 { + t.Errorf("Expected 4 stats. Got %d, Error: %v", len(stats4), err) + } + // check counts for each HARE code: + for _, stat := range stats4 { + // some very basic checks, making sure fields are not empty, repeated in next row, etc: + if len(stat.ConceptValue) == len(stat.ValueName) || + len(stat.ConceptValue) == 0 || + len(stat.ValueName) == 0 || + stat.ValueAsConceptId == 0 || + stat.ValueName == prevName { + t.Errorf("Invalid results") + } + if stat.ValueAsConceptId == 2000007028 && stat.NpersonsInCohortWithValue != 2 { + t.Errorf("Invalid results. Expected count of persons == 2, got: %d", stat.NpersonsInCohortWithValue) + } + if stat.ValueAsConceptId == 2000007029 && stat.NpersonsInCohortWithValue != 3 { + t.Errorf("Invalid results. Expected count of persons == 3, got: %d", stat.NpersonsInCohortWithValue) + } + if stat.ValueAsConceptId == 2000007030 && stat.NpersonsInCohortWithValue != 2 { + t.Errorf("Invalid results. Expected count of persons == 2, got: %d", stat.NpersonsInCohortWithValue) + } + if stat.ValueAsConceptId == 2000007031 && stat.NpersonsInCohortWithValue != 1 { + t.Errorf("Invalid results. Expected count of persons == 1, got: %d", stat.NpersonsInCohortWithValue) + } + prevName = stat.ValueName + } + // test below is identical to above, but with a transformation and a filter: + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: tests.GetTestHistogramConceptId(), + Filters: []utils.Filter{ + { + Type: ">=", + Value: utils.Float64Ptr(0.871), // we have 2 records (each one on a different HARE) in extendedCopyOfSecondLargestCohort where the value is >= than this + }, + }, + Transformation: "log", + }, + } + stats5, err := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(testSourceId, + extendedCopyOfSecondLargestCohort.Id, filterConceptDefsAndCohortPairs, breakdownConceptId) + // expecting 2 records as commented above: + if len(stats5) != 2 { + t.Errorf("Expected 2 stats. Got %d, Error: %v", len(stats5), err) + } } func TestRetrieveBreakdownStatsBySourceIdAndCohortIdWithResults(t *testing.T) { @@ -757,31 +825,6 @@ func TestGetCohortDefinitionByName(t *testing.T) { } } -func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(t *testing.T) { - setUp(t) - filterConceptIdsAndValues := []utils.CustomConceptVariableDef{} - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - data, _ := cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptIdsAndValues, filterCohortPairs) - // everyone in the largestCohort has the histogramConceptId, but one person has NULL in the value_as_number: - if len(data) != largestCohort.CohortSize-1 { - t.Errorf("expected %d histogram data but got %d", largestCohort.CohortSize, len(data)) - } - - // now filter on the extendedCopyOfSecondLargestCohort - filterCohortPairs = []utils.CustomDichotomousVariableDef{ - { - CohortDefinitionId1: smallestCohort.Id, - CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, - ProvidedName: "test"}, - } - // then we expect histogram data for the overlapping population only (which is 5 for extendedCopyOfSecondLargestCohort and largestCohort): - data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsAndCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptIdsAndValues, filterCohortPairs) - if len(data) != 5 { - t.Errorf("expected 5 histogram data but got %d", len(data)) - } - -} - func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptDefsPlusCohortPairs(t *testing.T) { setUp(t) filterConceptDefsPlusCohortPairs := []interface{}{ @@ -1167,39 +1210,9 @@ func TestToSQL(t *testing.T) { } } -func TestQueryFilterByConceptIdsHelper(t *testing.T) { - // This test checks whether the query succeeds when the mainObservationTableAlias - // argument passed to QueryFilterByConceptIdsHelper (last argument) - // matches the alias used in the main query, and whether it fails otherwise. - - setUp(t) - omopDataSource := tests.GetOmopDataSource() - filterConceptIds := []int64{allConceptIds[0], allConceptIds[1], allConceptIds[2]} - var personIds []struct { - PersonId int64 - } - - // Subtest1: correct alias "observation": - query := omopDataSource.Db.Table(omopDataSource.Schema + ".observation_continuous as observation" + omopDataSource.GetViewDirective()). - Select("observation.person_id") - query = models.QueryFilterByConceptIdsHelper(query, testSourceId, filterConceptIds, omopDataSource, "", "observation.person_id") - meta_result := query.Scan(&personIds) - if meta_result.Error != nil { - t.Errorf("Did NOT expect an error") - } - // Subtest2: incorrect alias "observation"...should fail: - query = omopDataSource.Db.Table(omopDataSource.Schema + ".observation_continuous as observationWRONG"). - Select("*") - query = models.QueryFilterByConceptIdsHelper(query, testSourceId, filterConceptIds, omopDataSource, "", "observation.person_id") - meta_result = query.Scan(&personIds) - if meta_result.Error == nil { - t.Errorf("Expected an error") - } -} - func TestQueryFilterByConceptDefsHelper(t *testing.T) { // This test checks whether the query succeeds when the mainObservationTableAlias - // argument passed to QueryFilterByConceptIdsHelper (last argument) + // argument passed to QueryFilterByConceptDefsHelper (last argument) // matches the alias used in the main query, and whether it fails otherwise. setUp(t) @@ -1252,157 +1265,89 @@ func TestQueryFilterByConceptDefsHelper(t *testing.T) { } } -func TestRetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(t *testing.T) { - setUp(t) - cohortDefinitions, _ := cohortDefinitionModel.GetAllCohortDefinitionsAndStatsOrderBySizeDesc(testSourceId, defaultTeamProject) - var sumNumeric float32 = 0 - textConcat := "" - classIdConcat := "" - foundConceptValueAsNumberAsNil := false - for _, cohortDefinition := range cohortDefinitions { - - cohortData, _ := cohortDataModel.RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId( - testSourceId, cohortDefinition.Id, allConceptIds) - - // count nr observation records for cohort through an independent simpler query: - totalObservationsCohort := tests.GetCountWhere(tests.GetOmopDataSourceForSourceId(tests.GetTestSourceId()), "observation", - fmt.Sprintf("observation_concept_id > 0 and person_id in (Select b.subject_id from %s.cohort b where b.cohort_definition_id = %d)", - tests.GetResultsDataSource().Schema, cohortDefinition.Id)) - - // 1- the number of items in cohortsData and totalObservationsCohort should match: - if int64(len(cohortData)) != totalObservationsCohort { - t.Errorf("Expected %d observations, found %d", totalObservationsCohort, len(cohortData)) - } - - // 2- cohortData items > 0, assuming each cohort has a person wit at least one observation - if len(cohortData) <= 0 { - t.Errorf("Expected some cohort data") - } - // 3- check data size. Max size, if all persons have data for each concept, is cohort size x len(allConceptIds): - if len(cohortData) > cohortDefinition.CohortSize*len(allConceptIds) { - t.Errorf("Cohort data size larger than expected. Expected < %d, got %d", - cohortDefinition.CohortSize*len(allConceptIds), len(cohortData)) - } - - var previousPersonId int64 = -1 - defaultHareValue := map[string]bool{"non-Hispanic Asian": true, "non-Hispanic Black": true, "non-Hispanic White": true, "Hispanic": true} - emptyDataCounter := 0 - for _, cohortDatum := range cohortData { - // check for order: person_id is not smaller than previous person_id - if cohortDatum.PersonId < previousPersonId { - t.Errorf("Data not ordered by person_id!") - } - - if cohortDatum.ConceptId == 2000007027 { - //Only one row has empty data - if !defaultHareValue[cohortDatum.ObservationValueAsConceptName] { - if emptyDataCounter == 0 && cohortDatum.ObservationValueAsConceptName == "" { - emptyDataCounter++ - } else { - t.Errorf("Did not get concept value name correctly!") - } - } - } - - previousPersonId = cohortDatum.PersonId - if cohortDatum.ConceptValueAsNumber != nil { - sumNumeric += *cohortDatum.ConceptValueAsNumber - } else { - foundConceptValueAsNumberAsNil = true - } - textConcat += cohortDatum.ObservationValueAsConceptName - classIdConcat += cohortDatum.ConceptClassId - } - } - // check for data: sum of all numeric values > 0 - if sumNumeric == 0 { - t.Errorf("Expected some numeric cohort data") - } - // check for data: concat of all string values != "" - if textConcat == "" { - t.Errorf("Expected some string cohort data") - } - // check for data: some concepts have class id, so this should not be empty - if classIdConcat == "" { - t.Errorf("Expected query to return concept class id information") - } - // check if some numeric values were nil as expected: - if foundConceptValueAsNumberAsNil == false { - t.Errorf("Expected query to return some nil values for ConceptValueAsNumber") - } - -} - -func TestErrorForRetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(t *testing.T) { - // Tests if the method returns an error when query fails. - - cohortDefinitions, _ := cohortDefinitionModel.GetAllCohortDefinitionsAndStatsOrderBySizeDesc(testSourceId, defaultTeamProject) - - // break something in the Results schema to cause a query failure in the next method: - tests.BreakSomething(models.Results, "cohort", "cohort_definition_id") - // set last action to restore back: - // run test: - _, error := cohortDataModel.RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId( - testSourceId, cohortDefinitions[0].Id, allConceptIds) - if error == nil { - t.Errorf("Expected error") - } - // revert the broken part: - tests.FixSomething(models.Results, "cohort", "cohort_definition_id") -} - func TestRetrieveCohortOverlapStats(t *testing.T) { // Tests if we get the expected overlap setUp(t) caseCohortId := secondLargestCohort.Id controlCohortId := secondLargestCohort.Id // to ensure we get some overlap, just repeat the same here... - otherFilterConceptIds := []int64{} - filterCohortPairs := []utils.CustomDichotomousVariableDef{} - stats, _ := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, - otherFilterConceptIds, filterCohortPairs) + filterConceptDefsAndCohortPairs := []interface{}{} + stats, err := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, + filterConceptDefsAndCohortPairs) // basic test: if stats.CaseControlOverlap != int64(secondLargestCohort.CohortSize) { - t.Errorf("Expected nr persons to be %d, found %d", secondLargestCohort.CohortSize, stats.CaseControlOverlap) + t.Errorf("Expected nr persons to be %d, found %d. Error: %v", secondLargestCohort.CohortSize, stats.CaseControlOverlap, err) } // now use largestCohort as background and filter on the extendedCopyOfSecondLargestCohort caseCohortId = largestCohort.Id controlCohortId = largestCohort.Id // to ensure we get largestCohort as initial overlap, just repeat the same here... - filterCohortPairs = []utils.CustomDichotomousVariableDef{ - { + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomDichotomousVariableDef{ CohortDefinitionId1: smallestCohort.Id, CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id, - ProvidedName: "test"}, + ProvidedName: "test", + }, } // then we expect overlap of 6 for extendedCopyOfSecondLargestCohort and largestCohort: - stats, _ = cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, - otherFilterConceptIds, filterCohortPairs) + stats, err = cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, + filterConceptDefsAndCohortPairs) if stats.CaseControlOverlap != 6 { - t.Errorf("Expected nr persons to be %d, found %d", 6, stats.CaseControlOverlap) + t.Errorf("Expected nr persons to be %d, found %d. Error: %v", 6, stats.CaseControlOverlap, err) } // extra test: different parameters that should return the same as above ^: caseCohortId = largestCohort.Id controlCohortId = extendedCopyOfSecondLargestCohort.Id - filterCohortPairs = []utils.CustomDichotomousVariableDef{} - otherFilterConceptIds = []int64{histogramConceptId} // extra filter, to cover this part of the code... + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: histogramConceptId, + }, + } // extra filter, to cover this part of the code... // then we expect overlap of 5 for extendedCopyOfSecondLargestCohort and largestCohort (the filter on histogramConceptId should not matter // since all in largestCohort have an observation for this concept id except one person who has it but has value_as_number as NULL): - stats2, _ := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, - otherFilterConceptIds, filterCohortPairs) + stats2, err := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, + filterConceptDefsAndCohortPairs) if stats2.CaseControlOverlap != stats.CaseControlOverlap-1 { - t.Errorf("Expected nr persons to be %d, found %d", stats.CaseControlOverlap, stats2.CaseControlOverlap) + t.Errorf("Expected nr persons to be %d, found. %d Error: %v", stats.CaseControlOverlap, stats2.CaseControlOverlap, err) } // test for otherFilterConceptIds by filtering above on dummyContinuousConceptId, which is NOT // found in any observations of the largestCohort: - otherFilterConceptIds = []int64{histogramConceptId, dummyContinuousConceptId} + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: histogramConceptId, + }, + utils.CustomConceptVariableDef{ + ConceptId: dummyContinuousConceptId, + }, + } // all other arguments are the same as test above, and we expect overlap of 0, showing the otherFilterConceptIds // had the expected effect: - stats3, _ := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, - otherFilterConceptIds, filterCohortPairs) + stats3, err := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, + filterConceptDefsAndCohortPairs) if stats3.CaseControlOverlap != 0 { - t.Errorf("Expected nr persons to be 0, found %d", stats3.CaseControlOverlap) + t.Errorf("Expected nr persons to be 0, found %d. Error: %v", stats3.CaseControlOverlap, err) + } + + // test with filter and transformation. These 2 cohorts will have 6 overlapping persons: + caseCohortId = largestCohort.Id + controlCohortId = extendedCopyOfSecondLargestCohort.Id + filterConceptDefsAndCohortPairs = []interface{}{ + utils.CustomConceptVariableDef{ + ConceptId: tests.GetTestHistogramConceptId(), + Filters: []utils.Filter{ + { + Type: ">=", + Value: utils.Float64Ptr(0.871), // we have 2 records in the overlapping population where the value is >= than this + }, + }, + Transformation: "log", + }, + } + stats4, err := cohortDataModel.RetrieveCohortOverlapStats(testSourceId, caseCohortId, controlCohortId, + filterConceptDefsAndCohortPairs) + if stats4.CaseControlOverlap != 2 { + t.Errorf("Expected nr persons to be 2, found %d. Error: %v", stats4.CaseControlOverlap, err) } } diff --git a/utils/parsing.go b/utils/parsing.go index 8059034..1273684 100644 --- a/utils/parsing.go +++ b/utils/parsing.go @@ -175,7 +175,11 @@ func ParseConceptDefsAndDichotomousDefsAsSingleList(c *gin.Context) ([]interface log.Printf("Error decoding request body: %v", err) return nil, errors.New("failed to parse JSON request body") } - + // Check if the "variables" field is absent (nil) + if requestBody.Variables == nil { + log.Println("Missing 'variables' field in the request body") + return nil, errors.New("'variables' field must be present in the request body") + } conceptDefsAndDichotomousDefs := make([]interface{}, 0) for _, variable := range requestBody.Variables { @@ -196,7 +200,7 @@ func ParseConceptDefsAndDichotomousDefsAsSingleList(c *gin.Context) ([]interface conceptDefsAndDichotomousDefs = append(conceptDefsAndDichotomousDefs, dichotomousDef) default: log.Printf("Unsupported variable type: %s", variable.VariableType) - // return nil, errors.New("unsupported variable type in request body") - TODO - first fix frontend to stop sending empty variables {} + return nil, errors.New("unsupported variable type in request body") } } @@ -323,11 +327,11 @@ func ParseSourceAndCohortId(c *gin.Context) (int, int, error) { return sourceId, cohortId, nil } -// separates a conceptIdsAndCohortPairs into a conceptIds list and a cohortPairs list -func GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs []interface{}) ([]CustomConceptVariableDef, []CustomDichotomousVariableDef) { +// separates a conceptDefsAndCohortPairs into a conceptDefs list and a cohortPairs list +func GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs []interface{}) ([]CustomConceptVariableDef, []CustomDichotomousVariableDef) { conceptDefsAndValues := []CustomConceptVariableDef{} cohortPairs := []CustomDichotomousVariableDef{} - for _, item := range conceptIdsAndCohortPairs { + for _, item := range conceptDefsAndCohortPairs { switch convertedItem := item.(type) { case CustomConceptVariableDef: conceptDefsAndValues = append(conceptDefsAndValues, convertedItem) @@ -340,16 +344,16 @@ func GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPai // deprecated: returns the conceptIds and cohortPairs as separate lists (for backwards compatibility) func ParseSourceIdAndCohortIdAndVariablesList(c *gin.Context) (int, int, []int64, []CustomDichotomousVariableDef, error) { - sourceId, cohortId, conceptIdsAndCohortPairs, err := ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) + sourceId, cohortId, conceptDefsAndCohortPairs, err := ParseSourceIdAndCohortIdAndVariablesAsSingleList(c) if err != nil { return -1, -1, nil, nil, err } - conceptIdsAndValues, cohortPairs := GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs) + conceptIdsAndValues, cohortPairs := GetConceptDefsAndValuesAndCohortPairsAsSeparateLists(conceptDefsAndCohortPairs) conceptIds := ExtractConceptIdsFromCustomConceptVariablesDef(conceptIdsAndValues) return sourceId, cohortId, conceptIds, cohortPairs, nil } -// returns sourceid, cohortid, list of variables (formed by concept ids and/or of cohort tuples which are also known as custom dichotomous variables) +// returns sourceid, cohortid, list of variables (formed by concept defs and/or of cohort tuples which are also known as custom dichotomous variables) func ParseSourceIdAndCohortIdAndVariablesAsSingleList(c *gin.Context) (int, int, []interface{}, error) { // parse and validate all parameters: sourceId, cohortId, err := ParseSourceAndCohortId(c)