From 32a80b4c44eea9dda8803b46d84813f680242ae0 Mon Sep 17 00:00:00 2001 From: EliuX Date: Fri, 24 Apr 2020 17:24:34 -0500 Subject: [PATCH 1/2] fix:bug: Close #107 Allow removing id by specifying null --- .../project_types_namespace_test.py | 4 ++-- .../time_entries_namespace_test.py | 23 +++++++++---------- .../activities/activities_namespace.py | 6 ++--- time_tracker_api/api.py | 21 +++++++++++------ .../customers/customers_namespace.py | 6 ++--- .../project_types/project_types_namespace.py | 13 ++++------- .../projects/projects_namespace.py | 13 ++++------- .../time_entries/time_entries_namespace.py | 15 +++++------- 8 files changed, 49 insertions(+), 52 deletions(-) diff --git a/tests/time_tracker_api/project_types/project_types_namespace_test.py b/tests/time_tracker_api/project_types/project_types_namespace_test.py index 3d0b3135..c14122fe 100644 --- a/tests/time_tracker_api/project_types/project_types_namespace_test.py +++ b/tests/time_tracker_api/project_types/project_types_namespace_test.py @@ -42,7 +42,7 @@ def test_create_project_type_should_reject_bad_request(client: FlaskClient, from time_tracker_api.project_types.project_types_namespace import project_type_dao invalid_project_type_data = valid_project_type_data.copy() invalid_project_type_data.update({ - "parent_id": None, + "name": None, }) repository_create_mock = mocker.patch.object(project_type_dao.repository, 'create', @@ -166,7 +166,7 @@ def test_update_project_should_reject_bad_request(client: FlaskClient, from time_tracker_api.project_types.project_types_namespace import project_type_dao invalid_project_type_data = valid_project_type_data.copy() invalid_project_type_data.update({ - "parent_id": None, + "name": None, }) repository_update_mock = mocker.patch.object(project_type_dao.repository, 'partial_update', 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 5ddc9741..65639cfe 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 @@ -7,7 +7,7 @@ from flask_restplus._http import HTTPStatus from pytest_mock import MockFixture -from commons.data_access_layer.cosmos_db import current_datetime +from commons.data_access_layer.cosmos_db import current_datetime, current_datetime_str fake = Faker() @@ -16,14 +16,14 @@ "project_id": fake.uuid4(), "activity_id": fake.uuid4(), "description": fake.paragraph(nb_sentences=2), - "start_date": str(yesterday.isoformat()), - "owner_id": fake.uuid4(), - "tenant_id": fake.uuid4() + "start_date": current_datetime_str(), } fake_time_entry = ({ "id": fake.random_int(1, 9999), "running": True, + "owner_id": fake.uuid4(), + "tenant_id": fake.uuid4(), }) fake_time_entry.update(valid_time_entry_input) @@ -86,21 +86,20 @@ def test_create_time_entry_should_succeed_with_valid_request(client: FlaskClient repository_create_mock.assert_called_once() -def test_create_time_entry_should_reject_bad_request(client: FlaskClient, - mocker: MockFixture, - valid_header: dict): +def test_create_time_entry_with_missing_req_field_should_return_bad_request(client: FlaskClient, + mocker: MockFixture, + valid_header: dict): from time_tracker_api.time_entries.time_entries_namespace import time_entries_dao - invalid_time_entry_input = valid_time_entry_input.copy() - invalid_time_entry_input.update({ - "project_id": None, - }) repository_create_mock = mocker.patch.object(time_entries_dao.repository, 'create', return_value=fake_time_entry) response = client.post("/time-entries", headers=valid_header, - json=invalid_time_entry_input, + json={ + "activity_id": fake.uuid4(), + "start_date": current_datetime_str(), + }, follow_redirects=True) assert HTTPStatus.BAD_REQUEST == response.status_code diff --git a/time_tracker_api/activities/activities_namespace.py b/time_tracker_api/activities/activities_namespace.py index d4f42f20..ba783bc2 100644 --- a/time_tracker_api/activities/activities_namespace.py +++ b/time_tracker_api/activities/activities_namespace.py @@ -1,13 +1,13 @@ from faker import Faker -from flask_restplus import fields, Resource, Namespace +from flask_restplus import fields, Resource from flask_restplus._http import HTTPStatus from time_tracker_api.activities.activities_model import create_dao -from time_tracker_api.api import common_fields +from time_tracker_api.api import common_fields, api faker = Faker() -ns = Namespace('activities', description='API for activities') +ns = api.namespace('activities', description='Namespace of the API for activities') # Activity Model activity_input = ns.model('ActivityInput', { diff --git a/time_tracker_api/api.py b/time_tracker_api/api.py index 3b686d22..a1618a69 100644 --- a/time_tracker_api/api.py +++ b/time_tracker_api/api.py @@ -38,30 +38,37 @@ def create_attributes_filter(ns: namespace, model: Model, filter_attrib_names: l return attribs_parser -# Common models structure +# Custom fields +class NullableString(fields.String): + __schema_type__ = ['string', 'null'] + + +class UUID(NullableString): + def __init__(self, *args, **kwargs): + super(UUID, self).__init__(*args, **kwargs) + self.pattern = UUID_REGEX + + common_fields = { - 'id': fields.String( + 'id': UUID( title='Identifier', readOnly=True, required=True, description='The unique identifier', - pattern=UUID_REGEX, example=faker.uuid4(), ), - 'tenant_id': fields.String( + 'tenant_id': UUID( title='Identifier of Tenant', readOnly=True, required=True, description='Tenant it belongs to', - # pattern=UUID_REGEX, This must be confirmed example=faker.uuid4(), ), - 'deleted': fields.String( + 'deleted': UUID( readOnly=True, required=True, title='Last event Identifier', description='Last event over this resource', - pattern=UUID_REGEX, ), } diff --git a/time_tracker_api/customers/customers_namespace.py b/time_tracker_api/customers/customers_namespace.py index f3b01fcc..f455cb22 100644 --- a/time_tracker_api/customers/customers_namespace.py +++ b/time_tracker_api/customers/customers_namespace.py @@ -1,13 +1,13 @@ from faker import Faker -from flask_restplus import Namespace, Resource, fields +from flask_restplus import Resource, fields from flask_restplus._http import HTTPStatus -from time_tracker_api.api import common_fields +from time_tracker_api.api import common_fields, api from time_tracker_api.customers.customers_model import create_dao faker = Faker() -ns = Namespace('customers', description='API for customers') +ns = api.namespace('customers', description='Namespace of the API for customers') # Customer Model customer_input = ns.model('CustomerInput', { diff --git a/time_tracker_api/project_types/project_types_namespace.py b/time_tracker_api/project_types/project_types_namespace.py index 116aa901..8e1e759e 100644 --- a/time_tracker_api/project_types/project_types_namespace.py +++ b/time_tracker_api/project_types/project_types_namespace.py @@ -1,14 +1,13 @@ from faker import Faker -from flask_restplus import Namespace, Resource, fields +from flask_restplus import Resource, fields from flask_restplus._http import HTTPStatus -from time_tracker_api.api import common_fields, create_attributes_filter +from time_tracker_api.api import common_fields, create_attributes_filter, api, UUID from time_tracker_api.project_types.project_types_model import create_dao -from time_tracker_api.security import UUID_REGEX faker = Faker() -ns = Namespace('project-types', description='API for project types') +ns = api.namespace('project-types', description='Namespace of the API for project types') # ProjectType Model project_type_input = ns.model('ProjectTypeInput', { @@ -26,19 +25,17 @@ description='Comments about the project type', example=faker.paragraph(), ), - 'customer_id': fields.String( + 'customer_id': UUID( title='Identifier of the Customer', required=False, description='Customer this project type belongs to. ' 'If not specified, it will be considered an internal project of the tenant.', - pattern=UUID_REGEX, example=faker.uuid4(), ), - 'parent_id': fields.String( + 'parent_id': UUID( title='Identifier of the parent project type', required=False, description='This parent node allows to created a tree-like structure for project types', - pattern=UUID_REGEX, example=faker.uuid4(), ), }) diff --git a/time_tracker_api/projects/projects_namespace.py b/time_tracker_api/projects/projects_namespace.py index 15613f92..634af17e 100644 --- a/time_tracker_api/projects/projects_namespace.py +++ b/time_tracker_api/projects/projects_namespace.py @@ -1,14 +1,13 @@ from faker import Faker -from flask_restplus import Namespace, Resource, fields +from flask_restplus import Resource, fields from flask_restplus._http import HTTPStatus -from time_tracker_api.api import common_fields, create_attributes_filter +from time_tracker_api.api import common_fields, create_attributes_filter, UUID, api from time_tracker_api.projects.projects_model import create_dao -from time_tracker_api.security import UUID_REGEX faker = Faker() -ns = Namespace('projects', description='API for projects (clients)') +ns = api.namespace('projects', description='Namespace of the API for projects') # Project Model project_input = ns.model('ProjectInput', { @@ -26,19 +25,17 @@ description='Description about the project', example=faker.paragraph(), ), - 'customer_id': fields.String( + 'customer_id': UUID( title='Identifier of the Customer', required=False, description='Customer this project type belongs to. ' 'If not specified, it will be considered an internal project of the tenant.', - pattern=UUID_REGEX, example=faker.uuid4(), ), - 'project_type_id': fields.String( + 'project_type_id': UUID( title='Identifier of the project type', required=False, description='Id of the project type it belongs. This allows grouping the projects.', - pattern=UUID_REGEX, example=faker.uuid4(), ), }) diff --git a/time_tracker_api/time_entries/time_entries_namespace.py b/time_tracker_api/time_entries/time_entries_namespace.py index 66a7eedf..87ba7f46 100644 --- a/time_tracker_api/time_entries/time_entries_namespace.py +++ b/time_tracker_api/time_entries/time_entries_namespace.py @@ -1,26 +1,24 @@ from datetime import timedelta from faker import Faker -from flask_restplus import fields, Resource, Namespace +from flask_restplus import fields, Resource from flask_restplus._http import HTTPStatus from commons.data_access_layer.cosmos_db import current_datetime, datetime_str, current_datetime_str from commons.data_access_layer.database import COMMENTS_MAX_LENGTH -from time_tracker_api.api import common_fields, create_attributes_filter -from time_tracker_api.security import UUID_REGEX +from time_tracker_api.api import common_fields, create_attributes_filter, api, UUID from time_tracker_api.time_entries.time_entries_model import create_dao faker = Faker() -ns = Namespace('time-entries', description='API for time entries') +ns = api.namespace('time-entries', description='Namespace of the API for time entries') # TimeEntry Model time_entry_input = ns.model('TimeEntryInput', { - 'project_id': fields.String( + 'project_id': UUID( title='Project', required=True, description='The id of the selected project', - pattern=UUID_REGEX, example=faker.uuid4(), ), 'start_date': fields.DateTime( @@ -30,11 +28,10 @@ description='When the user started doing this activity', example=datetime_str(current_datetime() - timedelta(days=1)), ), - 'activity_id': fields.String( + 'activity_id': UUID( title='Activity', required=False, description='The id of the selected activity', - pattern=UUID_REGEX, example=faker.uuid4(), ), 'description': fields.String( @@ -86,7 +83,7 @@ description='Whether this time entry is currently running or not', example=faker.boolean(), ), - 'owner_id': fields.String( + 'owner_id': UUID( required=True, readOnly=True, title='Owner of time entry', From 7dc262f3a61f541a3cb6ba3293ee7c57a5aa96f2 Mon Sep 17 00:00:00 2001 From: EliuX Date: Fri, 24 Apr 2020 19:09:55 -0500 Subject: [PATCH 2/2] feat: Close #107 Allow empty char values to be converted to null --- commons/data_access_layer/cosmos_db.py | 9 +++- .../data_access_layer/cosmos_db_test.py | 25 ++++++++++ .../time_entries_namespace_test.py | 49 ++++++++++++++++++- time_tracker_api/api.py | 2 +- .../time_entries/time_entries_model.py | 1 + 5 files changed, 83 insertions(+), 3 deletions(-) diff --git a/commons/data_access_layer/cosmos_db.py b/commons/data_access_layer/cosmos_db.py index cb2ab0c1..eb104a2c 100644 --- a/commons/data_access_layer/cosmos_db.py +++ b/commons/data_access_layer/cosmos_db.py @@ -132,9 +132,14 @@ def check_visibility(item, throw_not_found_if_deleted): if throw_not_found_if_deleted and item.get('deleted') is not None: raise exceptions.CosmosResourceNotFoundError(message='Deleted item', status_code=404) - return item + @staticmethod + def replace_empty_value_per_none(item_data: dict) -> dict: + for k, v in item_data.items(): + if isinstance(v, str) and len(v) == 0: + item_data[k] = None + def create(self, data: dict, mapper: Callable = None): self.on_create(data) function_mapper = self.get_mapper_or_dict(mapper) @@ -207,6 +212,8 @@ def on_create(self, new_item_data: dict): if new_item_data.get('id') is None: new_item_data['id'] = generate_uuid4() + self.replace_empty_value_per_none(new_item_data) + def on_update(self, update_item_data: dict): pass diff --git a/tests/commons/data_access_layer/cosmos_db_test.py b/tests/commons/data_access_layer/cosmos_db_test.py index 571311bc..0059370d 100644 --- a/tests/commons/data_access_layer/cosmos_db_test.py +++ b/tests/commons/data_access_layer/cosmos_db_test.py @@ -602,3 +602,28 @@ def test_datetime_str_comparison(): assert now_str > datetime_str(now - timedelta(days=1)) assert now_str < datetime_str(now + timedelta(days=1)) + + +def test_replace_empty_value_per_none(tenant_id: str): + initial_value = dict(id=fake.uuid4(), + name=fake.name(), + empty_str_attrib="", + array_attrib=[1, 2, 3], + empty_array_attrib=[], + description=" ", + age=fake.pyint(min_value=10, max_value=80), + size=0, + tenant_id=tenant_id) + + input = initial_value.copy() + + CosmosDBRepository.replace_empty_value_per_none(input) + + assert input["name"] == initial_value["name"] + assert input["empty_str_attrib"] is None + assert input["array_attrib"] == initial_value["array_attrib"] + assert input["empty_array_attrib"] == initial_value["empty_array_attrib"] + assert input["description"] == initial_value["description"] + assert input["age"] == initial_value["age"] + assert input["size"] == initial_value["size"] + assert input["tenant_id"] == initial_value["tenant_id"] 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 65639cfe..dc001333 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 @@ -5,7 +5,7 @@ from flask import json from flask.testing import FlaskClient from flask_restplus._http import HTTPStatus -from pytest_mock import MockFixture +from pytest_mock import MockFixture, pytest from commons.data_access_layer.cosmos_db import current_datetime, current_datetime_str @@ -423,3 +423,50 @@ def test_get_running_should_return_not_found_if_find_running_throws_StopIteratio assert HTTPStatus.NOT_FOUND == response.status_code repository_update_mock.assert_called_once_with(partition_key_value=tenant_id, owner_id=owner_id) + +@pytest.mark.parametrize( + 'invalid_uuid', ["zxy", "zxy%s" % fake.uuid4(), "%szxy" % fake.uuid4(), " "] +) +def test_create_with_invalid_uuid_format_should_return_bad_request(client: FlaskClient, + mocker: MockFixture, + valid_header: dict, + invalid_uuid: str): + from time_tracker_api.time_entries.time_entries_namespace import time_entries_dao + repository_container_create_item_mock = mocker.patch.object(time_entries_dao.repository.container, + 'create_item', + return_value=fake_time_entry) + invalid_time_entry_input = { + "project_id": fake.uuid4(), + "activity_id": invalid_uuid, + } + response = client.post("/time-entries", + headers=valid_header, + json=invalid_time_entry_input, + follow_redirects=True) + + assert HTTPStatus.BAD_REQUEST == response.status_code + repository_container_create_item_mock.assert_not_called() + +@pytest.mark.parametrize( + 'valid_uuid', ["", fake.uuid4()] +) +def test_create_with_valid_uuid_format_should_return_created(client: FlaskClient, + mocker: MockFixture, + valid_header: dict, + valid_uuid: str): + from time_tracker_api.time_entries.time_entries_namespace import time_entries_dao + repository_container_create_item_mock = mocker.patch.object(time_entries_dao.repository.container, + 'create_item', + return_value=fake_time_entry) + invalid_time_entry_input = { + "project_id": fake.uuid4(), + "activity_id": valid_uuid, + } + response = client.post("/time-entries", + headers=valid_header, + json=invalid_time_entry_input, + follow_redirects=True) + + assert HTTPStatus.CREATED == response.status_code + repository_container_create_item_mock.assert_called() + diff --git a/time_tracker_api/api.py b/time_tracker_api/api.py index a1618a69..089a04d6 100644 --- a/time_tracker_api/api.py +++ b/time_tracker_api/api.py @@ -46,7 +46,7 @@ class NullableString(fields.String): class UUID(NullableString): def __init__(self, *args, **kwargs): super(UUID, self).__init__(*args, **kwargs) - self.pattern = UUID_REGEX + self.pattern = r"^(|%s)$" % UUID_REGEX common_fields = { diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index 16bb557e..43f8aacf 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -85,6 +85,7 @@ def on_create(self, new_item_data: dict): def on_update(self, updated_item_data: dict): CosmosDBRepository.on_update(self, updated_item_data) self.validate_data(updated_item_data) + self.replace_empty_value_per_none(updated_item_data) def find_interception_with_date_range(self, start_date, end_date, owner_id, partition_key_value, ignore_id=None, visible_only=True, mapper: Callable = None):