Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beacon search endpoint #402

Merged
merged 14 commits into from
May 17, 2023
69 changes: 69 additions & 0 deletions chord_metadata_service/patients/api_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,72 @@ def get(self, request, *args, **kwargs):
"experiment_type": experiment_types
}
})


class BeaconListIndividuals(APIView):
"""
View to return lists of individuals filtered using search terms from katsu's config.json.
Uncensored equivalent of PublicListIndividuals.
"""
def filter_queryset(self, queryset):
# Check query parameters validity
qp = self.request.query_params
if len(qp) > settings.CONFIG_PUBLIC["rules"]["max_query_parameters"]:
raise ValidationError(f"Wrong number of fields: {len(qp)}")

search_conf = settings.CONFIG_PUBLIC["search"]
field_conf = settings.CONFIG_PUBLIC["fields"]
queryable_fields = {
f"{f}": field_conf[f] for section in search_conf for f in section["fields"]
Copy link
Member

Choose a reason for hiding this comment

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

i think this is a needless format string - it can just be f:

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh noes! You want me to pay attention to the code I'm copypasting? 😭

(fixed)

}

for field, value in qp.items():
if field not in queryable_fields:
raise ValidationError(f"Unsupported field used in query: {field}")

field_props = queryable_fields[field]
options = get_field_options(field_props)
if value not in options \
and not (
# case-insensitive search on categories
field_props["datatype"] == "string"
and value.lower() in [o.lower() for o in options]
) \
and not (
# no restriction when enum is not set for categories
field_props["datatype"] == "string"
and field_props["config"]["enum"] is None
):
raise ValidationError(f"Invalid value used in query: {value}")

# recursion
queryset = filter_queryset_field_value(queryset, field_props, value)

return queryset

def get(self, request, *args, **kwargs):
if not settings.CONFIG_PUBLIC:
return Response(settings.NO_PUBLIC_DATA_AVAILABLE)
Copy link
Member

Choose a reason for hiding this comment

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

should this not be a 404 instead of a 200? fine to leave as-is for consistency with bento public responses but generally i think in API design it's good to put as much meaning in the status code as possible

Copy link
Member Author

@gsfk gsfk May 17, 2023

Choose a reason for hiding this comment

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

Yeah, this was a straight copypaste of the bento_public code, and it struck me as odd also. I won't break the old code, but I'll change the beacon responses to 400 / bad request. (even for missing config, arguably still 400 rather than 404)

Copy link
Member

Choose a reason for hiding this comment

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

hmm i think of 400s more meaning something is wrong with the request... if the client request is good, I still think this should be a 404 (or maybe a 500 if we treat it as a configuration error)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, okay, I actually re-read the documentation for 404 and this sounds reasonable ("missing resource"). 204 (No content) is also possible but probably a little obscure. I'll change it to 404.


base_qs = Individual.objects.all()
try:
filtered_qs = self.filter_queryset(base_qs)
except ValidationError as e:
return Response(errors.bad_request_error(
*(e.error_list if hasattr(e, "error_list") else e.error_dict.items()),
))

tissues_count, sampled_tissues = biosample_tissue_stats(filtered_qs)
experiments_count, experiment_types = experiment_type_stats(filtered_qs)

return Response({
"matches": filtered_qs.values_list("id", flat=True),
"biosamples": {
"count": tissues_count,
"sampled_tissue": sampled_tissues
},
"experiments": {
"count": experiments_count,
"experiment_type": experiment_types
}
})
49 changes: 49 additions & 0 deletions chord_metadata_service/patients/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -650,3 +650,52 @@ def test_public_filtering_age_range_min_and_max_no_config(self):
self.assertIsInstance(response_obj, dict)
self.assertIsInstance(response_obj, dict)
self.assertEqual(response_obj, settings.NO_PUBLIC_DATA_AVAILABLE)


class BeaconSearchTest(APITestCase):

random_range = 20

def setUp(self):
individuals = [c.generate_valid_individual() for _ in range(self.random_range)]
for individual in individuals:
Individual.objects.create(**individual)

# test beacon formatted response
@override_settings(CONFIG_PUBLIC=CONFIG_PUBLIC_TEST)
def test_beacon_search_response(self):
response = self.client.get('/api/beacon_search?sex=MALE')
male_count = Individual.objects.filter(sex="MALE").count()
self.assertEqual(response.status_code, status.HTTP_200_OK)
response_obj = response.json()
self.assertEqual(len(response_obj["matches"]), male_count)

@override_settings(CONFIG_PUBLIC={})
def test_beacon_search_response_no_config(self):
# test when config is not provided, returns NO_PUBLIC_DATA_AVAILABLE
response = self.client.get('/api/beacon_search?sex=MALE')
self.assertEqual(response.status_code, status.HTTP_200_OK)
response_obj = response.json()
self.assertIsInstance(response_obj, dict)
self.assertEqual(response_obj, settings.NO_PUBLIC_DATA_AVAILABLE)

@override_settings(CONFIG_PUBLIC=CONFIG_PUBLIC_TEST)
def test_beacon_search_response_invalid_search_key(self):
response = self.client.get('/api/beacon_search?birdwatcher=yes')
self.assertEqual(response.status_code, status.HTTP_200_OK)
response_obj = response.json()
self.assertEqual(response_obj["code"], 400)
Copy link
Member

Choose a reason for hiding this comment

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

assert equal to status.HTTP_400_BAD_REQUEST instead (for these 3 tests)

Copy link
Member Author

@gsfk gsfk May 17, 2023

Choose a reason for hiding this comment

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

These are just copies of the old tests, which, yeah, are a bit weird. Changed the response to 400 and updated the tests.


@override_settings(CONFIG_PUBLIC=CONFIG_PUBLIC_TEST)
def test_beacon_search_response_invalid_search_value(self):
response = self.client.get('/api/beacon_search?smoking=on_Sundays')
self.assertEqual(response.status_code, status.HTTP_200_OK)
response_obj = response.json()
self.assertEqual(response_obj["code"], 400)

@override_settings(CONFIG_PUBLIC=CONFIG_PUBLIC_TEST)
def test_beacon_search_too_many_params(self):
response = self.client.get('/api/beacon_search?sex=MALE&smoking=Non-smoker&death_dc=Deceased')
self.assertEqual(response.status_code, status.HTTP_200_OK)
response_obj = response.json()
self.assertEqual(response_obj["code"], 400)
3 changes: 3 additions & 0 deletions chord_metadata_service/restapi/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,7 @@
path('public_search_fields', public_search_fields, name='public-search-fields',),
path('public_overview', public_overview, name='public-overview',),
path('public_dataset', public_dataset, name='public-dataset'),

# uncensored endpoint for beacon search using fields from config.json
path('beacon_search', individual_views.BeaconListIndividuals.as_view(), name='beacon-search'),
]