diff --git a/commons/data_access_layer/cosmos_db.py b/commons/data_access_layer/cosmos_db.py index 9cdf7f1c..3c8555d0 100644 --- a/commons/data_access_layer/cosmos_db.py +++ b/commons/data_access_layer/cosmos_db.py @@ -1,15 +1,15 @@ import dataclasses import logging -from typing import Callable, List +from typing import Callable import azure.cosmos.cosmos_client as cosmos_client import azure.cosmos.exceptions as exceptions -import flask from azure.cosmos import ContainerProxy, PartitionKey from flask import Flask from werkzeug.exceptions import HTTPException from commons.data_access_layer.database import CRUDDao, EventContext +from utils.query_builder import CosmosDBQueryBuilder class CosmosDBFacade: @@ -124,55 +124,6 @@ def from_definition( custom_cosmos_helper=custom_cosmos_helper, ) - @staticmethod - def create_sql_condition_for_visibility( - visible_only: bool, container_name='c' - ) -> str: - if visible_only: - # We are considering that `deleted == null` is not a choice - return 'AND NOT IS_DEFINED(%s.deleted)' % container_name - return '' - - @staticmethod - def create_sql_active_condition( - status_value: str, container_name='c' - ) -> str: - if status_value != None: - not_defined_condition = '' - condition_operand = ' AND ' - if status_value == 'active': - not_defined_condition = ( - 'AND NOT IS_DEFINED({container_name}.status)'.format( - container_name=container_name - ) - ) - condition_operand = ' OR ' - - defined_condition = '(IS_DEFINED({container_name}.status) \ - AND {container_name}.status = \'{status_value}\')'.format( - container_name=container_name, status_value=status_value - ) - return ( - not_defined_condition + condition_operand + defined_condition - ) - - return '' - - @staticmethod - def create_sql_where_conditions( - conditions: dict, container_name='c' - ) -> str: - where_conditions = [] - for k in conditions.keys(): - where_conditions.append(f'{container_name}.{k} = @{k}') - - if len(where_conditions) > 0: - return "AND {where_conditions_clause}".format( - where_conditions_clause=" AND ".join(where_conditions) - ) - else: - return "" - @staticmethod def generate_params(conditions: dict) -> list: result = [] @@ -206,16 +157,6 @@ def attach_context(data: dict, event_context: EventContext): "session_id": event_context.session_id, } - @staticmethod - def create_sql_date_range_filter(date_range: dict) -> str: - if 'start_date' in date_range and 'end_date' in date_range: - return """ - AND ((c.start_date BETWEEN @start_date AND @end_date) OR - (c.end_date BETWEEN @start_date AND @end_date)) - """ - else: - return '' - def create( self, data: dict, event_context: EventContext, mapper: Callable = None ): @@ -257,53 +198,38 @@ def find_all( mapper: Callable = None, ): conditions = conditions if conditions else {} - partition_key_value = self.find_partition_key_value(event_context) - max_count = self.get_page_size_or(max_count) - params = [ - {"name": "@partition_key_value", "value": partition_key_value}, - {"name": "@offset", "value": offset}, - {"name": "@max_count", "value": max_count}, - ] - - status_value = None - if conditions.get('status') != None: - status_value = conditions.get('status') + max_count: int = self.get_page_size_or(max_count) + + status_value = conditions.get('status') + if status_value: conditions.pop('status') date_range = date_range if date_range else {} - date_range_params = ( - self.generate_params(date_range) if date_range else [] - ) - params.extend(self.generate_params(conditions)) - params.extend(date_range_params) - - query_str = """ - SELECT * FROM c - WHERE c.{partition_key_attribute}=@partition_key_value - {conditions_clause} - {active_condition} - {date_range_sql_condition} - {visibility_condition} - {order_clause} - OFFSET @offset LIMIT @max_count - """.format( - partition_key_attribute=self.partition_key_attribute, - visibility_condition=self.create_sql_condition_for_visibility( - visible_only - ), - active_condition=self.create_sql_active_condition(status_value), - conditions_clause=self.create_sql_where_conditions(conditions), - date_range_sql_condition=self.create_sql_date_range_filter( - date_range - ), - order_clause=self.create_sql_order_clause(), + + query_builder = ( + CosmosDBQueryBuilder() + .add_sql_where_equal_condition(conditions) + .add_sql_active_condition(status_value) + .add_sql_date_range_condition(date_range) + .add_sql_visibility_condition(visible_only) + .add_sql_limit_condition(max_count) + .add_sql_offset_condition(offset) + .build() ) + if len(self.order_fields) > 1: + attribute = self.order_fields[0] + order = self.order_fields[1] + query_builder.add_sql_order_by_condition(attribute, order) + + query_str = query_builder.get_query() + params = query_builder.get_parameters() + partition_key_value = self.find_partition_key_value(event_context) + result = self.container.query_items( query=query_str, parameters=params, partition_key=partition_key_value, - max_item_count=max_count, ) function_mapper = self.get_mapper_or_dict(mapper) diff --git a/tests/commons/data_access_layer/cosmos_db_test.py b/tests/commons/data_access_layer/cosmos_db_test.py index c7a04eaf..07548988 100644 --- a/tests/commons/data_access_layer/cosmos_db_test.py +++ b/tests/commons/data_access_layer/cosmos_db_test.py @@ -660,28 +660,6 @@ def test_delete_permanently_with_valid_id_should_succeed( assert e.status_code == 404 -def test_repository_create_sql_where_conditions_with_multiple_values( - cosmos_db_repository: CosmosDBRepository, -): - result = cosmos_db_repository.create_sql_where_conditions( - {'owner_id': 'mark', 'customer_id': 'me'}, "c" - ) - - assert result is not None - assert ( - result == "AND c.owner_id = @owner_id AND c.customer_id = @customer_id" - ) - - -def test_repository_create_sql_where_conditions_with_no_values( - cosmos_db_repository: CosmosDBRepository, -): - result = cosmos_db_repository.create_sql_where_conditions({}, "c") - - assert result is not None - assert result == "" - - def test_repository_append_conditions_values( cosmos_db_repository: CosmosDBRepository, ): diff --git a/tests/time_tracker_api/activities/activities_model_test.py b/tests/time_tracker_api/activities/activities_model_test.py index e9ea54b3..fe84ce3a 100644 --- a/tests/time_tracker_api/activities/activities_model_test.py +++ b/tests/time_tracker_api/activities/activities_model_test.py @@ -1,5 +1,4 @@ from unittest.mock import Mock, patch -import pytest from commons.data_access_layer.database import EventContext from time_tracker_api.activities.activities_model import ( @@ -8,9 +7,6 @@ ) -@patch( - 'time_tracker_api.activities.activities_model.ActivityCosmosDBRepository.create_sql_condition_for_visibility' -) @patch( 'time_tracker_api.activities.activities_model.ActivityCosmosDBRepository.find_partition_key_value' ) diff --git a/tests/time_tracker_api/time_entries/time_entries_namespace_test.py b/tests/time_tracker_api/time_entries/time_entries_namespace_test.py index 8f22f45f..ce4a3a23 100644 --- a/tests/time_tracker_api/time_entries/time_entries_namespace_test.py +++ b/tests/time_tracker_api/time_entries/time_entries_namespace_test.py @@ -8,12 +8,8 @@ from pytest_mock import MockFixture, pytest from utils.time import ( - get_current_year, - get_current_month, current_datetime, current_datetime_str, - get_date_range_of_month, - datetime_str, ) from utils import worked_time from time_tracker_api.time_entries.time_entries_model import ( @@ -204,10 +200,6 @@ def test_get_time_entry_should_succeed_with_valid_id( 'time_tracker_api.time_entries.time_entries_dao.TimeEntriesCosmosDBDao.handle_date_filter_args', Mock(), ) -@patch( - 'time_tracker_api.time_entries.time_entries_repository.TimeEntryCosmosDBRepository.create_sql_date_range_filter', - Mock(), -) @patch( 'commons.data_access_layer.cosmos_db.CosmosDBRepository.generate_params', Mock(), @@ -232,7 +224,6 @@ def test_get_time_entries_by_type_of_user_when_is_user_tester( expected_user_ids, ): test_user_id = "id1" - non_test_user_id = "id2" te1 = TimeEntryCosmosDBModel( { "id": '1', @@ -285,10 +276,6 @@ def test_get_time_entries_by_type_of_user_when_is_user_tester( 'time_tracker_api.time_entries.time_entries_dao.TimeEntriesCosmosDBDao.handle_date_filter_args', Mock(), ) -@patch( - 'time_tracker_api.time_entries.time_entries_repository.TimeEntryCosmosDBRepository.create_sql_date_range_filter', - Mock(), -) @patch( 'commons.data_access_layer.cosmos_db.CosmosDBRepository.generate_params', Mock(), @@ -313,7 +300,6 @@ def test_get_time_entries_by_type_of_user_when_is_not_user_tester( expected_user_ids, ): test_user_id = "id1" - non_test_user_id = "id2" te1 = TimeEntryCosmosDBModel( { "id": '1', @@ -386,7 +372,6 @@ def test_get_time_entry_should_succeed_with_valid_id( ) def test_get_time_entry_raise_http_exception( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, http_exception: HTTPException, @@ -407,7 +392,6 @@ def test_get_time_entry_raise_http_exception( def test_update_time_entry_calls_partial_update_with_incoming_payload( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, owner_id: str, @@ -465,7 +449,6 @@ def test_update_time_entry_should_reject_bad_request( def test_update_time_entry_raise_not_found( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, owner_id: str, @@ -499,7 +482,6 @@ def test_update_time_entry_raise_not_found( def test_delete_time_entry_calls_delete( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, time_entries_dao, @@ -529,7 +511,6 @@ def test_delete_time_entry_calls_delete( ) def test_delete_time_entry_raise_http_exception( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, http_exception: HTTPException, @@ -554,7 +535,6 @@ def test_delete_time_entry_raise_http_exception( def test_stop_time_entry_calls_partial_update( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, time_entries_dao, @@ -581,7 +561,6 @@ def test_stop_time_entry_calls_partial_update( def test_stop_time_entry_raise_unprocessable_entity( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, time_entries_dao, @@ -611,7 +590,6 @@ def test_stop_time_entry_raise_unprocessable_entity( def test_restart_time_entry_calls_partial_update( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, time_entries_dao, @@ -638,7 +616,6 @@ def test_restart_time_entry_calls_partial_update( def test_restart_time_entry_raise_unprocessable_entity( client: FlaskClient, - mocker: MockFixture, valid_header: dict, valid_id: str, time_entries_dao, diff --git a/tests/time_tracker_api/time_entries/time_entries_query_builder_test.py b/tests/time_tracker_api/time_entries/time_entries_query_builder_test.py index fd23bd01..f3fa7efa 100644 --- a/tests/time_tracker_api/time_entries/time_entries_query_builder_test.py +++ b/tests/time_tracker_api/time_entries/time_entries_query_builder_test.py @@ -6,7 +6,7 @@ from utils.repository import remove_white_spaces -def test_TimeEntryQueryBuilder_is_subclass_CosmosDBQueryBuilder(): +def test_time_entry_query_builder_should_be_subclass_of_cosmos_query_builder(): query_builder = CosmosDBQueryBuilder() time_entries_query_builder = TimeEntryQueryBuilder() @@ -15,50 +15,6 @@ def test_TimeEntryQueryBuilder_is_subclass_CosmosDBQueryBuilder(): ) -def test_add_sql_date_range_condition_should_update_where_list(): - start_date = "2021-03-19T05:07:00.000Z" - end_date = "2021-03-25T10:00:00.000Z" - time_entry_query_builder = ( - TimeEntryQueryBuilder().add_sql_date_range_condition( - { - "start_date": start_date, - "end_date": end_date, - } - ) - ) - expected_params = [ - {"name": "@start_date", "value": start_date}, - {"name": "@end_date", "value": end_date}, - ] - assert len(time_entry_query_builder.where_conditions) == 1 - assert len(time_entry_query_builder.parameters) == len(expected_params) - assert time_entry_query_builder.get_parameters() == expected_params - - -def test_build_with_add_sql_date_range_condition(): - time_entry_query_builder = ( - TimeEntryQueryBuilder() - .add_sql_date_range_condition( - { - "start_date": "2021-04-19T05:00:00.000Z", - "end_date": "2021-04-20T10:00:00.000Z", - } - ) - .build() - ) - - expected_query = """ - SELECT * FROM c - WHERE ((c.start_date BETWEEN @start_date AND @end_date) OR - (c.end_date BETWEEN @start_date AND @end_date)) - """ - query = time_entry_query_builder.get_query() - - assert remove_white_spaces(query) == remove_white_spaces(expected_query) - assert len(time_entry_query_builder.where_conditions) == 1 - assert len(time_entry_query_builder.get_parameters()) == 2 - - def test_add_sql_interception_with_date_range_condition(): start_date = "2021-01-19T05:07:00.000Z" end_date = "2021-01-25T10:00:00.000Z" diff --git a/tests/utils/query_builder_test.py b/tests/utils/query_builder_test.py index 742730db..dc66b4f1 100644 --- a/tests/utils/query_builder_test.py +++ b/tests/utils/query_builder_test.py @@ -331,3 +331,93 @@ def test_add_sql_not_in_condition( ) assert len(query_builder.where_conditions) == len(expected_not_in_list) assert query_builder.where_conditions == expected_not_in_list + + +def test_add_sql_date_range_condition_should_update_where_list(): + start_date = "2021-03-19T05:07:00.000Z" + end_date = "2021-03-25T10:00:00.000Z" + query_builder = CosmosDBQueryBuilder().add_sql_date_range_condition( + { + "start_date": start_date, + "end_date": end_date, + } + ) + expected_params = [ + {"name": "@start_date", "value": start_date}, + {"name": "@end_date", "value": end_date}, + ] + assert len(query_builder.where_conditions) == 1 + assert len(query_builder.parameters) == len(expected_params) + assert query_builder.get_parameters() == expected_params + + +def test_build_with_add_sql_date_range_condition(): + query_builder = ( + CosmosDBQueryBuilder() + .add_sql_date_range_condition( + { + "start_date": "2021-04-19T05:00:00.000Z", + "end_date": "2021-04-20T10:00:00.000Z", + } + ) + .build() + ) + + expected_query = """ + SELECT * FROM c + WHERE ((c.start_date BETWEEN @start_date AND @end_date) OR + (c.end_date BETWEEN @start_date AND @end_date)) + """ + query = query_builder.get_query() + + assert remove_white_spaces(query) == remove_white_spaces(expected_query) + assert len(query_builder.where_conditions) == 1 + assert len(query_builder.get_parameters()) == 2 + + +def test_add_sql_active_condition_should_update_where_conditions(): + status_value = 'active' + expected_active_query = f""" + SELECT * FROM c + WHERE NOT IS_DEFINED(c.status) OR (IS_DEFINED(c.status) AND c.status = '{status_value}') + """ + expected_condition = f"NOT IS_DEFINED(c.status) OR (IS_DEFINED(c.status) AND c.status = '{status_value}')" + + query_builder = ( + CosmosDBQueryBuilder() + .add_sql_active_condition(status_value=status_value) + .build() + ) + + active_query = query_builder.get_query() + + assert remove_white_spaces(active_query) == remove_white_spaces( + expected_active_query + ) + assert len(query_builder.where_conditions) == 1 + assert query_builder.where_conditions[0] == expected_condition + + +def test_add_sql_inactive_condition_should_update_where_conditions(): + status_value = 'inactive' + expected_inactive_query = f""" + SELECT * FROM c + WHERE (IS_DEFINED(c.status) AND c.status = '{status_value}') + """ + expected_condition = ( + f"(IS_DEFINED(c.status) AND c.status = '{status_value}')" + ) + + query_builder = ( + CosmosDBQueryBuilder() + .add_sql_active_condition(status_value=status_value) + .build() + ) + + inactive_query = query_builder.get_query() + + assert remove_white_spaces(inactive_query) == remove_white_spaces( + expected_inactive_query + ) + assert len(query_builder.where_conditions) == 1 + assert query_builder.where_conditions[0] == expected_condition diff --git a/time_tracker_api/time_entries/time_entries_query_builder.py b/time_tracker_api/time_entries/time_entries_query_builder.py index 3147d43f..2417ac85 100644 --- a/time_tracker_api/time_entries/time_entries_query_builder.py +++ b/time_tracker_api/time_entries/time_entries_query_builder.py @@ -5,23 +5,6 @@ class TimeEntryQueryBuilder(CosmosDBQueryBuilder): def __init__(self): super(TimeEntryQueryBuilder, self).__init__() - def add_sql_date_range_condition(self, date_range: tuple = None): - if date_range and len(date_range) == 2: - start_date = date_range['start_date'] - end_date = date_range['end_date'] - condition = """ - ((c.start_date BETWEEN @start_date AND @end_date) OR - (c.end_date BETWEEN @start_date AND @end_date)) - """ - self.where_conditions.append(condition) - self.parameters.extend( - [ - {'name': '@start_date', 'value': start_date}, - {'name': '@end_date', 'value': end_date}, - ] - ) - return self - def add_sql_interception_with_date_range_condition( self, start_date, end_date ): diff --git a/utils/query_builder.py b/utils/query_builder.py index 2899aab4..b66f9ec1 100644 --- a/utils/query_builder.py +++ b/utils/query_builder.py @@ -34,6 +34,41 @@ def add_sql_in_condition( self.where_conditions.append(f"c.{attribute} IN {ids_values}") return self + def add_sql_active_condition(self, status_value: str): + if status_value: + not_defined_condition = '' + condition_operand = '' + if status_value == 'active': + not_defined_condition = 'NOT IS_DEFINED(c.status)' + condition_operand = ' OR ' + + defined_condition = ( + f"(IS_DEFINED(c.status) AND c.status = '{status_value}')" + ) + condition = ( + not_defined_condition + condition_operand + defined_condition + ) + self.where_conditions.append(condition) + return self + + def add_sql_date_range_condition(self, date_range: dict = None): + if date_range: + start_date = date_range.get('start_date') + end_date = date_range.get('end_date') + if start_date and end_date: + condition = """ + ((c.start_date BETWEEN @start_date AND @end_date) OR + (c.end_date BETWEEN @start_date AND @end_date)) + """ + self.where_conditions.append(condition) + self.parameters.extend( + [ + {'name': '@start_date', 'value': start_date}, + {'name': '@end_date', 'value': end_date}, + ] + ) + return self + def add_sql_where_equal_condition(self, data: dict = None): if data: for k, v in data.items():