From a91d12518d915c0d5b8adb868776cc5b40c47538 Mon Sep 17 00:00:00 2001 From: Aaron Suarez Date: Sun, 9 Dec 2018 20:08:52 -0600 Subject: [PATCH 1/6] Add timestamps to resource table --- app/models.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models.py b/app/models.py index 9d5038af..79aca101 100644 --- a/app/models.py +++ b/app/models.py @@ -1,5 +1,7 @@ from app import db from sqlalchemy_utils import URLType +from sqlalchemy import DateTime +from sqlalchemy.sql import func language_identifier = db.Table('language_identifier', @@ -28,6 +30,8 @@ class Resource(db.Model): upvotes = db.Column(db.INTEGER, default=0) downvotes = db.Column(db.INTEGER, default=0) times_clicked = db.Column(db.INTEGER, default=0) + created_at = db.Column(DateTime(timezone=True), server_default=func.now()) + last_updated = db.Column(DateTime(timezone=True), onupdate=func.now()) @property def serialize(self): From d440fc9f8657fd23e632e1423222b912b75ce83a Mon Sep 17 00:00:00 2001 From: Aaron Suarez Date: Sun, 9 Dec 2018 21:56:38 -0600 Subject: [PATCH 2/6] Add updated_after param in get_resources route --- app/api/routes.py | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/app/api/routes.py b/app/api/routes.py index 6021bb88..b61b22e8 100644 --- a/app/api/routes.py +++ b/app/api/routes.py @@ -1,7 +1,7 @@ from traceback import print_tb from flask import request -from sqlalchemy import and_, func +from sqlalchemy import or_, func from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound from app.api import bp @@ -72,43 +72,42 @@ def get_resources(): # Fetch the filter params from the url, if they were provided. language = request.args.get('language') category = request.args.get('category') + updated_after = request.args.get('updated_after') + + q = Resource.query # Filter on language - if language and not category: - query = Resource.query.filter( + if language: + q = q.filter( Resource.languages.any( Language.name.ilike(language) ) ) # Filter on category - elif category and not language: - query = Resource.query.filter( + if category: + q = q.filter( Resource.category.has( func.lower(Category.name) == category.lower() ) ) - # Filter on both - elif category and language: - query = Resource.query.filter( - and_( - Resource.languages.any( - Language.name.ilike(language) - ), - Resource.category.has( - func.lower(Category.name) == category.lower() - ) + # Filter on updated_after + if updated_after: + q = q.filter( + or_( + Resource.created_at >= updated_after, + Resource.last_updated >= updated_after ) ) - # No filters - else: - query = Resource.query - - resource_list = [ - resource.serialize for resource in resource_paginator.items(query) - ] + try: + resource_list = [ + resource.serialize for resource in resource_paginator.items(q) + ] + except: + # TODO: Make this error response better + return standardize_response(None, [{"code": "bad-request"}], "bad-request") return standardize_response(resource_list, None, "ok") From d8ef4f6b92a92efd04b1d25d0d3640263c430d28 Mon Sep 17 00:00:00 2001 From: Aaron Suarez Date: Mon, 10 Dec 2018 21:47:51 -0600 Subject: [PATCH 3/6] Catch exceptions, logging, and add status code to response --- .env | 3 ++- app/api/routes.py | 26 +++++++++++++++++++++----- app/utils.py | 6 +++--- tests/conftest.py | 2 ++ tests/unit/test_routes.py | 6 ++++++ 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/.env b/.env index 9dbbfc1f..bd1adef2 100644 --- a/.env +++ b/.env @@ -1,3 +1,4 @@ FLASK_APP=run.py -SQLALCHEMY_DATABASE_URI=postgresql://user_name:change_password@127.0.0.1:5432/resources +SQLALCHEMY_DATABASE_URI=postgresql://aaron:getmein@127.0.0.1:5432/resources FLASK_SKIP_DOTENV=1 +FLASK_ENV=development diff --git a/app/api/routes.py b/app/api/routes.py index b61b22e8..4119fe0f 100644 --- a/app/api/routes.py +++ b/app/api/routes.py @@ -8,6 +8,11 @@ from app.models import Language, Resource, Category from app import Config, db from app.utils import Paginator, standardize_response +from dateutil import parser +from datetime import datetime +import logging + +logger = logging.getLogger() # Routes @@ -94,10 +99,21 @@ def get_resources(): # Filter on updated_after if updated_after: + try: + uaDate = parser.parse(updated_after) + if uaDate > datetime.now(): + raise Exception("updated_after greater than today's date") + uaDate = uaDate.strftime("%Y-%m-%d") + except Exception as e: + logger.error(e) + message = 'The value for "updated_after" is invalid' + error = [{"code": "bad-value", "message": message}] + return standardize_response(None, error, "unprocessable-entity", 422) + q = q.filter( or_( - Resource.created_at >= updated_after, - Resource.last_updated >= updated_after + Resource.created_at >= uaDate, + Resource.last_updated >= uaDate ) ) @@ -105,9 +121,9 @@ def get_resources(): resource_list = [ resource.serialize for resource in resource_paginator.items(q) ] - except: - # TODO: Make this error response better - return standardize_response(None, [{"code": "bad-request"}], "bad-request") + except Exception as e: + logger.error(e) + return standardize_response(None, [{"code": "bad-request"}], "bad request", 400) return standardize_response(resource_list, None, "ok") diff --git a/app/utils.py b/app/utils.py index ef752f34..5cec20fe 100644 --- a/app/utils.py +++ b/app/utils.py @@ -16,15 +16,15 @@ def items(self, query): return query.paginate(self.page, self.page_size, False).items -def standardize_response(data, errors, status): +def standardize_response(data, errors, status, status_code=200): resp = { "status": status, "apiVersion": API_VERSION } - if data: + if data is not None: resp["data"] = data elif errors: resp["errors"] = errors else: resp["errors"] = [{"code": "something-went-wrong"}] - return jsonify(resp) + return jsonify(resp), status_code diff --git a/tests/conftest.py b/tests/conftest.py index 6ace3dde..299097e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,5 @@ +import os +import tempfile import pytest from app import create_app diff --git a/tests/unit/test_routes.py b/tests/unit/test_routes.py index a4b793b5..08d5cb4a 100644 --- a/tests/unit/test_routes.py +++ b/tests/unit/test_routes.py @@ -9,3 +9,9 @@ def test_does_nothing(): THEN check the email, hashed_password, authenticated, and role fields are defined correctly """ assert(1 == 1) + +def test_empty_db(client): + """Start with a blank database.""" + + rv = client.get('/') + assert b'No entries here so far' in rv.data From cbc078cc4de73e0fefd4098b1fbc5795a10acf5c Mon Sep 17 00:00:00 2001 From: Aaron Suarez Date: Sat, 15 Dec 2018 10:58:00 -0600 Subject: [PATCH 4/6] Add client to the tests --- tests/conftest.py | 16 +++++++++++++++- tests/unit/test_routes.py | 9 ++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 299097e3..841320db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,21 @@ import pytest from app import create_app +TESTDB = 'test_project.db' +TESTDB_PATH = "tests/data/{}".format(TESTDB) +TEST_DATABASE_URI = 'sqlite:///' + TESTDB_PATH + + +ALEMBIC_CONFIG = 'migrations/alembic.ini' + + @pytest.fixture(scope='module') def app(): app = create_app() - return app \ No newline at end of file + app.testing = True + return app + +@pytest.fixture(scope='module') +def client(app): + client = app.test_client() + return client diff --git a/tests/unit/test_routes.py b/tests/unit/test_routes.py index 08d5cb4a..fe405781 100644 --- a/tests/unit/test_routes.py +++ b/tests/unit/test_routes.py @@ -1,4 +1,5 @@ import pytest +from tests import conftest from app.models import Resource, Language, Category @@ -10,8 +11,6 @@ def test_does_nothing(): """ assert(1 == 1) -def test_empty_db(client): - """Start with a blank database.""" - - rv = client.get('/') - assert b'No entries here so far' in rv.data +def test_get_resources(app, client): + resources = client.get('api/v1/resources') + assert(resources.status_code == 200) From fe2e881e0fe8b346d8a85a5f4164223ee6186a1a Mon Sep 17 00:00:00 2001 From: Aaron Suarez Date: Sat, 15 Dec 2018 12:28:07 -0600 Subject: [PATCH 5/6] Try and fail to add an SQLite db instance to the test rig --- tests/conftest.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 841320db..697ae827 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,8 @@ import os import tempfile import pytest +import tempfile +from app import db from app import create_app TESTDB = 'test_project.db' @@ -19,5 +21,16 @@ def app(): @pytest.fixture(scope='module') def client(app): + db_fd, app.config['DATABASE'] = tempfile.mkstemp() + + app.config['SQLALCHEMY_DATABASE_URI'] = TEST_DATABASE_URI + + with app.app_context(): + db.create_all() + client = app.test_client() - return client + + yield client + + os.close(db_fd) + os.unlink(app.config['DATABASE']) \ No newline at end of file From b6dd327c461345cd2a00b5417b3df45857bd6e96 Mon Sep 17 00:00:00 2001 From: Aaron Suarez Date: Mon, 24 Dec 2018 00:34:37 -0600 Subject: [PATCH 6/6] Add tests for every current route --- pytest.ini | 2 + tests/conftest.py | 68 ++++++++++----- tests/unit/test_routes.py | 175 +++++++++++++++++++++++++++++++++++--- 3 files changed, 212 insertions(+), 33 deletions(-) create mode 100644 pytest.ini diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 00000000..c1fa8785 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +addopts = -p no:warnings \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 697ae827..c6f80c96 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,36 +1,58 @@ -import os -import tempfile import pytest -import tempfile -from app import db -from app import create_app +from app import create_app, db as _db +from configs import Config -TESTDB = 'test_project.db' -TESTDB_PATH = "tests/data/{}".format(TESTDB) -TEST_DATABASE_URI = 'sqlite:///' + TESTDB_PATH +TEST_DATABASE_URI = 'sqlite:///:memory:' +counter = 0 -ALEMBIC_CONFIG = 'migrations/alembic.ini' +@pytest.fixture(scope='session') +def app(request): + Config.SQLALCHEMY_DATABASE_URI = TEST_DATABASE_URI + Config.TESTING = True + app = create_app(Config) -@pytest.fixture(scope='module') -def app(): - app = create_app() - app.testing = True + + # Establish an application context before running the tests. + ctx = app.app_context() + ctx.push() + + def teardown(): + ctx.pop() + + request.addfinalizer(teardown) return app -@pytest.fixture(scope='module') -def client(app): - db_fd, app.config['DATABASE'] = tempfile.mkstemp() - app.config['SQLALCHEMY_DATABASE_URI'] = TEST_DATABASE_URI +@pytest.fixture(scope='session') +def db(app, request): + """Session-wide test database.""" + def teardown(): + _db.drop_all() + + _db.app = app + _db.create_all() + + request.addfinalizer(teardown) + return _db + + +@pytest.fixture(scope='function') +def session(db, request): + """Creates a new database session for a test.""" + connection = db.engine.connect() + transaction = connection.begin() - with app.app_context(): - db.create_all() + options = dict(bind=connection, binds={}) + session = db.create_scoped_session(options=options) - client = app.test_client() + db.session = session - yield client + def teardown(): + transaction.rollback() + connection.close() + session.remove() - os.close(db_fd) - os.unlink(app.config['DATABASE']) \ No newline at end of file + request.addfinalizer(teardown) + return session diff --git a/tests/unit/test_routes.py b/tests/unit/test_routes.py index fe405781..bc1af341 100644 --- a/tests/unit/test_routes.py +++ b/tests/unit/test_routes.py @@ -1,16 +1,171 @@ import pytest from tests import conftest from app.models import Resource, Language, Category +from configs import PaginatorConfig +from app.cli import import_resources -def test_does_nothing(): - """ - GIVEN a User model - WHEN a new User is created - THEN check the email, hashed_password, authenticated, and role fields are defined correctly - """ - assert(1 == 1) +########################################## +## Tests +########################################## -def test_get_resources(app, client): - resources = client.get('api/v1/resources') - assert(resources.status_code == 200) +# TODO: We need negative unit tests (what happens when bad data is sent) + +def test_getters(app, session, db): + # Importing the data takes a rather long time, so import it once + # here for all the getters that require pages of resources + import_resources(db) + client = app.test_client() + + # Actually conduct all the tests in helper functions + get_resources_test(app, session, db, client) + paginator_test(app, session, db, client) + filters_test(app, session, db, client) + languages_test(app, session, db, client) + categories_test(app, session, db, client) + + +def test_create_resource(app, session, db): + client = app.test_client() + + response = create_resource(client) + assert(response.status_code == 200) + assert(isinstance(response.json['data'].get('id'), int)) + assert(response.json['data'].get('name') == "Some Name") + + +def test_update_resource(app, session, db): + client = app.test_client() + + response = create_resource(client) + + id = response.json['data'].get('id') + assert(isinstance(id, int)) + + response = client.put(f"/api/v1/resources/{id}", json={ + "name": "New name" + }) + + assert(response.status_code == 200) + assert(response.json['data'].get('name') == "New name") + + +########################################## +## Helpers +########################################## + +def get_resources_test(app, session, db, client): + response = client.get('api/v1/resources') + + # Status should be OK + assert(response.status_code == 200) + + resources = response.json + + # Default page size shouold be specified in PaginatorConfig + assert(len(resources['data']) == PaginatorConfig.per_page) + check_resources(resources['data']) + + +def get_single_resource_test(app, session, db, client): + response = client.get('api/v1/resources/5') + + # Status should be OK + assert(response.status_code == 200) + + resource = response.json + + check_resources([resources['data']]) + assert(resources['data'].get('id') == 5) + + + +def paginator_test(app, session, db, client): + # Test page size + response = client.get('api/v1/resources?page_size=1') + assert(len(response.json['data']) == 1) + response = client.get('api/v1/resources?page_size=5') + assert(len(response.json['data']) == 5) + response = client.get('api/v1/resources?page_size=10') + assert(len(response.json['data']) == 10) + response = client.get('api/v1/resources?page_size=100') + assert(len(response.json['data']) == 100) + + # Test pages different and sequential + first_page_resource = response.json['data'][0] + assert(first_page_resource.get('id') == 1) + response = client.get('api/v1/resources?page_size=100&page=2') + second_page_resource = response.json['data'][0] + assert(second_page_resource.get('id') == 101) + response = client.get('api/v1/resources?page_size=100&page=3') + third_page_resource = response.json['data'][0] + assert(third_page_resource.get('id') == 201) + + # Test bigger than max page size + too_long = PaginatorConfig.max_page_size + 1 + response = client.get(f"api/v1/resources?page_size={too_long}") + assert(len(response.json['data']) == PaginatorConfig.max_page_size) + + # Test farther than last page + too_far = 99999999 + response = client.get(f"api/v1/resources?page_size=100&page={too_far}") + assert(len(response.json['data']) == 0) + + +def filters_test(app, session, db, client): + # Filter by language + response = client.get('api/v1/resources?language=python') + + for resource in response.json['data']: + assert(type(resource.get('languages')) is list) + assert('Python' in resource.get('languages')) + + # Filter by category + response = client.get('api/v1/resources?category=Back%20End%20Dev') + + for resource in response.json['data']: + assert(resource.get('category') == "Back End Dev") + + # TODO: Filter by updated_after + # (Need to figure out how to manually set last_updated and created_at) + + +def languages_test(app, session, db, client): + response = client.get('api/v1/languages') + + for language in response.json['data']: + assert(isinstance(language.get('id'), int)) + assert(isinstance(language.get('name'), str)) + assert(len(language.get('name')) > 0) + + +def categories_test(app, session, db, client): + response = client.get('api/v1/categories') + + for category in response.json['data']: + assert(isinstance(category.get('id'), int)) + assert(isinstance(category.get('name'), str)) + assert(len(category.get('name')) > 0) + + +def check_resources(resources): + # Each resource should have a name, url, languages and category + for resource in resources: + assert(isinstance(resource.get('name'), str)) + assert(resource.get('name') != "") + assert(isinstance(resource.get('url'), str)) + assert(resource.get('url') != "") + assert(isinstance(resource.get('category'), str)) + assert(resource.get('category') != "") + assert(type(resource.get('languages')) is list) + + +def create_resource(client): + return client.post('/api/v1/resources', json={ + "name": "Some Name", + "url": "http://example.org/", + "category": "New Category", + "languages": ["Python", "New Language"], + "paid": False, + "notes": "Some notes" + })