From b03c6606cc9dbd28560687198722c2a8b3cd7120 Mon Sep 17 00:00:00 2001 From: Brett Lentz Date: Wed, 12 Feb 2020 15:40:02 -0500 Subject: [PATCH] add unit tests for ReportQueryHandler (#1723) * add unit tests for ReportQueryHandler * add comparisons for QueryFilter * DRY up a QueryHandler loop --- koku/api/query_filter.py | 10 + koku/api/query_handler.py | 14 +- koku/api/query_params.py | 1 - koku/api/report/test/tests_queries.py | 268 +++++++++++++++++++++ koku/api/report/test/tests_query_filter.py | 28 +++ 5 files changed, 312 insertions(+), 9 deletions(-) diff --git a/koku/api/query_filter.py b/koku/api/query_filter.py index c471a3ed06..243c55bf56 100644 --- a/koku/api/query_filter.py +++ b/koku/api/query_filter.py @@ -17,10 +17,12 @@ """QueryFilterfor Reports.""" from collections import defaultdict from collections import UserDict +from functools import total_ordering from django.db.models import Q +@total_ordering class QueryFilter(UserDict): """Dict-like object representing a single query filter.""" @@ -106,6 +108,10 @@ def __eq__(self, other): """Exact comparison.""" return self.data == other.data and self.logical_operator == other.logical_operator + def __lt__(self, other): + """Decide if self < other.""" + return str(self.data) < str(other.data) + def __repr__(self): """Return string representation.""" return str(self.composed_Q()) @@ -221,6 +227,10 @@ def __contains__(self, item): return True return False + def __eq__(self, other): + """Exact comparison.""" + return sorted(self._filters) == sorted(other._filters) + def __iter__(self): """Return an iterable of the collection.""" return self._filters.__iter__() diff --git a/koku/api/query_handler.py b/koku/api/query_handler.py index a5b356712d..1311a52c60 100644 --- a/koku/api/query_handler.py +++ b/koku/api/query_handler.py @@ -320,12 +320,10 @@ def filter_to_order_by(self, parameters): # noqa: C901 parameters (QueryParameters): The parameters object """ - for group_by_key in parameters.parameters.get("group_by", {}): - for group_by_value in parameters.parameters["group_by"][group_by_key]: - if group_by_value == "*": - # find if there is a filter[X]=Y that matches this group_by[X]=* - # get filter value for current group_by_key - filter_value = parameters.parameters.get("filter", {}).get(group_by_key) - if filter_value: - parameters.parameters["group_by"][group_by_key] = filter_value + # find if there is a filter[key]=value that matches this group_by[key]=value + for key, value in parameters.parameters.get("group_by", {}).items(): + if self.has_wildcard(value): + filter_value = parameters.parameters.get("filter", {}).get(key) + if filter_value: + parameters.parameters["group_by"][key] = filter_value return parameters diff --git a/koku/api/query_params.py b/koku/api/query_params.py index 037b35fadc..96b0aabb22 100644 --- a/koku/api/query_params.py +++ b/koku/api/query_params.py @@ -119,7 +119,6 @@ def _process_tag_query_params(self, query_params): param_tag_keys.add(value) if key in tag_key_set: param_tag_keys.add(key) - return param_tag_keys def _set_access(self, filter_key, access_key, raise_exception=True): diff --git a/koku/api/report/test/tests_queries.py b/koku/api/report/test/tests_queries.py index 23a204ffc5..0f23741226 100644 --- a/koku/api/report/test/tests_queries.py +++ b/koku/api/report/test/tests_queries.py @@ -15,13 +15,24 @@ # along with this program. If not, see . # """Test the Report Queries.""" +from unittest.mock import Mock + from django.test import TestCase +from faker import Faker +from api.iam.test.iam_test_case import IamTestCase +from api.query_filter import QueryFilter +from api.query_filter import QueryFilterCollection from api.report.aws.query_handler import AWSReportQueryHandler from api.report.azure.openshift.query_handler import OCPAzureReportQueryHandler from api.report.azure.query_handler import AzureReportQueryHandler from api.report.ocp.query_handler import OCPReportQueryHandler from api.report.ocp_aws.query_handler import OCPAWSReportQueryHandler +from api.report.provider_map import ProviderMap +from api.report.queries import ReportQueryHandler +from api.report.view import ReportView + +FAKE = Faker() class ReportQueryUtilsTest(TestCase): @@ -105,3 +116,260 @@ def test_group_data_by_list_missing_units(self): ], } self.assertEqual(expected, out_data) + + +def create_test_handler(params, mapper=None): + """Create a TestableReportQueryHandler using the supplied args. + + Args: + + params (QueryParameters) mocked query parameters + mapper (dict) mocked ProviderMap dictionary + """ + if not mapper: + mapper = {"filter": [{}], "filters": {}} + + class TestableReportQueryHandler(ReportQueryHandler): + """ A testable minimal implementation of ReportQueryHandler. + + ReportQueryHandler can't be instantiated directly without first setting + a few attributes that are required by QueryHandler.__init__(). + """ + + _mapper = Mock( + spec=ProviderMap, + _report_type_map=Mock(return_value=mapper, get=lambda x, y=None: mapper.get(x, y)), + _provider_map=Mock(return_value=mapper, get=lambda x, y=None: mapper.get(x, y)), + tag_column="tags", + ) + + return TestableReportQueryHandler(params) + + +def assertSameQ(one, two): + """Compare two Q-objects and decide if they're equivalent. + + Q objects don't have their own comparison methods defined. + + This function is intended to give an approximate comparison suitable + for our purposes. + + Args: + one, two (Q) Django Q Object + + Returns: + (boolean) whether the objects match. + """ + + for item_one in one.children: + if not is_child(item_one, two): + return False + + for item_two in two.children: + if not is_child(item_two, one): + return False + + if one.connector != two.connector: + return False + + if one.negated != two.negated: + return False + + # no mismatches found. we will assume the two + # Q objects are roughly equivalent. + return True + + +def is_child(item, obj): + """Test whether the given item is in the target Q object's children. + + Args: + item (dict | tuple) a dict or tuple + obj (Q) a Django Q object + """ + test_dict = item + if isinstance(item, tuple): + test_dict = {item[0]: item[1]} + + test_tuple = item + if isinstance(item, dict): + test_tuple = (item.keys()[0], item.values()[0]) + + # Q objects can have both tuples and dicts in their children. + # So, we are comparing equivalent forms in case our test object is + # formatted differently than the real version. + if test_dict not in obj.children and test_tuple not in obj.children: + return False + return True + + +class ReportQueryHandlerTest(IamTestCase): + """Test the report query handler functions.""" + + def setUp(self): + """Test setup.""" + self.mock_tag_key = FAKE.word() + self.mock_view = Mock( + spec=ReportView, + report="mock", + permission_classes=[Mock], + provider="mock", + serializer=Mock, + query_handler=Mock, + tag_handler=[ + Mock( + objects=Mock( + values=Mock(return_value=[{"key": self.mock_tag_key, "values": [FAKE.word(), FAKE.word()]}]) + ) + ) + ], + ) + + def test_init(self): + """Test that we can instantiate a minimal ReportQueryHandler.""" + params = self.mocked_query_params("", self.mock_view) + rqh = create_test_handler(params) + self.assertIsInstance(rqh, ReportQueryHandler) + + def test_set_operator_specified_filters_and(self): + """Test that AND/OR terms are correctly applied to param filters.""" + operator = "and" + + term = FAKE.word() + first = FAKE.word() + second = FAKE.word() + operation = FAKE.word() + + url = f"?filter[time_scope_value]=-1&group_by[and:{term}]={first}&group_by[and:{term}]={second}" + params = self.mocked_query_params(url, self.mock_view) + + mapper = {"filter": [{}], "filters": {term: {"field": term, "operation": operation}}} + rqh = create_test_handler(params, mapper=mapper) + output = rqh._set_operator_specified_filters(operator) + self.assertIsNotNone(output) + + expected = QueryFilterCollection( + filters=[ + QueryFilter(field=term, operation=operation, parameter=second, logical_operator=operator), + QueryFilter(field=term, operation=operation, parameter=first, logical_operator=operator), + ] + ) + assertSameQ(output, expected.compose()) + + def test_set_operator_specified_filters_or(self): + """Test that AND/OR terms are correctly applied to param filters.""" + operator = "or" + + term = FAKE.word() + first = FAKE.word() + second = FAKE.word() + operation = FAKE.word() + + url = f"?filter[time_scope_value]=-1&group_by[or:{term}]={first}&group_by[or:{term}]={second}" + params = self.mocked_query_params(url, self.mock_view) + + mapper = {"filter": [{}], "filters": {term: {"field": term, "operation": operation}}} + rqh = create_test_handler(params, mapper=mapper) + output = rqh._set_operator_specified_filters(operator) + self.assertIsNotNone(output) + + expected = QueryFilterCollection( + filters=[ + QueryFilter(field=term, operation=operation, parameter=second, logical_operator=operator), + QueryFilter(field=term, operation=operation, parameter=first, logical_operator=operator), + ] + ) + assertSameQ(output, expected.compose()) + + def test_set_operator_specified_tag_filters_and(self): + """Test that AND/OR terms are correctly applied to tag filters.""" + operator = "and" + + term = self.mock_tag_key + first = FAKE.word() + second = FAKE.word() + operation = "icontains" + + url = ( + f"?filter[time_scope_value]=-1&" + f"filter[and:tag:{term}]={first}&" + f"filter[and:tag:{term}]={second}&" + f"group_by[and:tag:{term}]={first}&" + f"group_by[and:tag:{term}]={second}" + ) + + params = self.mocked_query_params(url, self.mock_view) + mapper = {"filter": [{}], "filters": {term: {"field": term, "operation": operation}}} + rqh = create_test_handler(params, mapper=mapper) + output = rqh._set_operator_specified_tag_filters(QueryFilterCollection(), operator) + + self.assertIsNotNone(output) + + expected = QueryFilterCollection( + filters=[ + QueryFilter( + table="tags", field=term, operation=operation, parameter=second, logical_operator=operator + ), + QueryFilter(table="tags", field=term, operation=operation, parameter=first, logical_operator=operator), + ] + ) + self.assertIsInstance(output, QueryFilterCollection) + assertSameQ(output.compose(), expected.compose()) + + def test_set_operator_specified_tag_filters_or(self): + """Test that AND/OR terms are correctly applied to tag filters.""" + operator = "or" + + term = self.mock_tag_key + first = FAKE.word() + second = FAKE.word() + operation = "icontains" + + url = ( + f"?filter[time_scope_value]=-1&" + f"filter[or:tag:{term}]={first}&" + f"filter[or:tag:{term}]={second}&" + f"group_by[or:tag:{term}]={first}&" + f"group_by[or:tag:{term}]={second}" + ) + params = self.mocked_query_params(url, self.mock_view) + mapper = {"filter": [{}], "filters": {term: {"field": term, "operation": operation}}} + rqh = create_test_handler(params, mapper=mapper) + output = rqh._set_operator_specified_tag_filters(QueryFilterCollection(), operator) + self.assertIsNotNone(output) + + expected = QueryFilterCollection( + filters=[ + QueryFilter(field=term, operation=operation, parameter=second, logical_operator=operator), + QueryFilter(field=term, operation=operation, parameter=first, logical_operator=operator), + ] + ) + self.assertIsInstance(output, QueryFilterCollection) + assertSameQ(output.compose(), expected.compose()) + + # FIXME: need test for _apply_group_by + # FIXME: need test for _apply_group_null_label + # FIXME: need test for _build_custom_filter_list } + # FIXME: need test for _create_previous_totals + # FIXME: need test for _get_filter + # FIXME: need test for _get_group_by + # FIXME: need test for _get_previous_totals_filter + # FIXME: need test for _get_search_filter + # FIXME: need test for _get_tag_group_by + # FIXME: need test for _group_data_by_list + # FIXME: need test for _pack_data_object + # FIXME: need test for _percent_delta + # FIXME: need test for _perform_rank_summation + # FIXME: need test for _ranked_list + # FIXME: need test for _set_or_filters + # FIXME: need test for _set_tag_filters + # FIXME: need test for _transform_data + # FIXME: need test for add_deltas + # FIXME: need test for annotations + # FIXME: need test for date_group_data + # FIXME: need test for get_tag_filter_keys + # FIXME: need test for get_tag_group_by_keys + # FIXME: need test for get_tag_order_by + # FIXME: need test for initialize_totals + # FIXME: need test for order_by + # FIXME: need test for unpack_date_grouped_data diff --git a/koku/api/report/test/tests_query_filter.py b/koku/api/report/test/tests_query_filter.py index 6ee8e357b0..2eceb9c83d 100644 --- a/koku/api/report/test/tests_query_filter.py +++ b/koku/api/report/test/tests_query_filter.py @@ -110,6 +110,34 @@ def test_from_string_wrong_parts_more(self): with self.assertRaises(TypeError): QueryFilter().from_string(test_string) + def test_comparison_eq(self): + """Test the __eq__() method.""" + word = self.fake.word() + qf1 = QueryFilter(table=word) + qf2 = QueryFilter(table=word) + self.assertTrue(qf1 == qf2) + + def test_comparison_ne(self): + """Test the __ne__() method provided by @total_ordering.""" + word = self.fake.word() + qf1 = QueryFilter(table=word) + qf2 = QueryFilter(field=word) + self.assertTrue(qf1 != qf2) + + def test_comparison_lt(self): + """Test the __lt__() method.""" + word = self.fake.word() + qf1 = QueryFilter(table=word) + qf2 = QueryFilter(field=word) + self.assertTrue(qf1 < qf2) + + def test_comparison_gt(self): + """Test the __gt__() method provided by @total_ordering.""" + word = self.fake.word() + qf1 = QueryFilter(field=word) + qf2 = QueryFilter(table=word) + self.assertTrue(qf1 > qf2) + class QueryFilterCollectionTest(TestCase): """Test the QueryFilterCollection class."""