From a327702ad011db87e2e26a6281177adfdb9d8185 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Thu, 7 Jan 2016 17:31:17 +0000 Subject: [PATCH 01/12] Initial code added for models and services not functional yet. Bootstrap and migrations added for db. --- app/__init__.py | 7 +++ app/main/dao/__init__.py | 0 app/main/dao/services_dao.py | 28 ++++++++++ app/main/dao/users_dao.py | 20 +++++++ app/main/views/index.py | 5 ++ app/main/views/services.py | 38 +++++++++++++ app/models.py | 43 +++++++++++++++ application.py | 6 +- config.py | 5 ++ migrations/README | 1 + migrations/alembic.ini | 45 +++++++++++++++ migrations/env.py | 73 +++++++++++++++++++++++++ migrations/script.py.mako | 22 ++++++++ requirements.txt | 5 ++ scripts/bootstrap.sh | 7 +++ scripts/run_tests.sh | 1 + tests/app/main/dao/__init__.py | 0 tests/app/main/dao/test_services_dao.py | 8 +++ tests/app/main/dao/test_users_dao.py | 0 tests/conftest.py | 29 +++++++++- 20 files changed, 341 insertions(+), 2 deletions(-) create mode 100644 app/main/dao/__init__.py create mode 100644 app/main/dao/services_dao.py create mode 100644 app/main/dao/users_dao.py create mode 100644 app/main/views/services.py create mode 100644 app/models.py create mode 100644 migrations/README create mode 100644 migrations/alembic.ini create mode 100644 migrations/env.py create mode 100644 migrations/script.py.mako create mode 100644 tests/app/main/dao/__init__.py create mode 100644 tests/app/main/dao/test_services_dao.py create mode 100644 tests/app/main/dao/test_users_dao.py diff --git a/app/__init__.py b/app/__init__.py index d0b090459..0e4bf5016 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -2,11 +2,15 @@ import os from flask._compat import string_types from flask import Flask, _request_ctx_stack +from flask.ext.sqlalchemy import SQLAlchemy from werkzeug.local import LocalProxy from config import configs from utils import logging +db = SQLAlchemy() + + api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user) @@ -16,6 +20,7 @@ def create_app(config_name): application.config['NOTIFY_API_ENVIRONMENT'] = config_name application.config.from_object(configs[config_name]) + db.init_app(application) init_app(application) logging.init_app(application) @@ -26,6 +31,8 @@ def create_app(config_name): from .status import status as status_blueprint application.register_blueprint(status_blueprint) + from app import models + return application diff --git a/app/main/dao/__init__.py b/app/main/dao/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py new file mode 100644 index 000000000..1e497ec7c --- /dev/null +++ b/app/main/dao/services_dao.py @@ -0,0 +1,28 @@ +from datetime import datetime + +from sqlalchemy.orm import load_only + +from app import db +from app.models import Service + + +def create_new_service(service_name, + user, + limit=1000, + active=False, + restricted=True): + service = Service(name=service_name, + created_at=datetime.now(), + limit=limit, + active=active, + restricted=restricted) + db.session.add(service) + service.users.append(user) + db.session.commit() + return service.id + + +def get_services(user, service_id=None): + if service_id: + return Service.query.filter_by(user=user, service_id=service_id).one() + return Service.query.filter_by(user=user).all() diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py new file mode 100644 index 000000000..3c2bdce00 --- /dev/null +++ b/app/main/dao/users_dao.py @@ -0,0 +1,20 @@ +from datetime import datetime + +from sqlalchemy.orm import load_only + +from app import db +from app.models import User + + +def create_new_user(email_address): + user = User(email_address=email_address, + created_at=datetime.now()) + db.session.add(user) + db.session.commit() + return user.id + + +def get_users(user_id=None): + if user_id: + return User.query.filter_by(user_id=user_id).one() + return User.query.filter_by().all() diff --git a/app/main/views/index.py b/app/main/views/index.py index 4c1b535e5..2c9b620d9 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -2,11 +2,16 @@ from flask import jsonify from .. import main +# TODO need for health check url + + +# TODO remove @main.route('/', methods=['GET']) def get_index(): return jsonify(result="hello world"), 200 +# TODO remove @main.route('/', methods=['POST']) def post_index(): return jsonify(result="hello world"), 200 diff --git a/app/main/views/services.py b/app/main/views/services.py new file mode 100644 index 000000000..6ced550bb --- /dev/null +++ b/app/main/views/services.py @@ -0,0 +1,38 @@ +from flask import jsonify +from app.main.dao.services_dao import (create_new_service, get_services) +from app.main.dao.users_dao import (get_users) +from .. import main + + +# TODO auth to be added. +@main.route('/service', methods=['POST']) +def create_service(): + return jsonify(result="created"), 201 + + +# TODO auth to be added +@main.route('/service/', method=['PUT']) +def update_service(service_id): + return jsonify(result="updated") + + +# TODO auth to be added. +# Should be restricted by user, user id +# is pulled from the token +@main.route('/service/', method=['GET']) +@main.route('/service', methods=['GET']) +def get_service(service_id=None): + services = get_services + return jsonify( + data=services + ) + + +# TODO auth to be added +# auth should be allow for admin tokens only +@main.route('/user//service', method=['GET']) +@main.route('/user//service/', method=['GET']) +def get_service_by_user_id(user_id, service_id=None): + user = get_users(user_id=user_id) + services = get_services(user, service_id=service_id) + return jsonify(data=services) diff --git a/app/models.py b/app/models.py new file mode 100644 index 000000000..cd370d21b --- /dev/null +++ b/app/models.py @@ -0,0 +1,43 @@ +from . import db + + +class User(db.Model): + __tablename__ = 'users' + + id = db.Column(db.Integer, primary_key=True) + email_address = db.Column(db.String(255), nullable=False, index=True, unique=True) + created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) + updated_at = db.Column(db.DateTime, index=False, unique=False, nullable=True) + + +user_to_service = db.Table( + 'user_to_service', + db.Model.metadata, + db.Column('user_id', db.Integer, db.ForeignKey('users.id')), + db.Column('service_id', db.Integer, db.ForeignKey('services.id')) +) + + +class Service(db.Model): + __tablename__ = 'services' + + id = db.Column(db.Integer, primary_key=True) + name = db.Column(db.String(255), nullable=False) + created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) + active = db.Column(db.Boolean, index=False, unique=False, nullable=False) + limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False) + users = db.relationship('User', secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) + restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) + + def serialize(self): + serialized = { + 'id': self.id, + 'name': self.name, + 'createdAt': self.created_at.strftime(DATETIME_FORMAT), + 'active': self.active, + 'restricted': self.restricted, + 'limit': self.limit, + 'user': self.users.serialize() + } + + return filter_null_value_fields(serialized) diff --git a/application.py b/application.py index 00d322355..92f120626 100644 --- a/application.py +++ b/application.py @@ -5,14 +5,18 @@ from __future__ import print_function import os from flask.ext.script import Manager, Server +from flask.ext.migrate import Migrate, MigrateCommand -from app import create_app +from app import create_app, db application = create_app(os.getenv('NOTIFY_API_ENVIRONMENT') or 'development') manager = Manager(application) port = int(os.environ.get('PORT', 6011)) manager.add_command("runserver", Server(host='0.0.0.0', port=port)) +migrate = Migrate(application, db) +manager.add_command('db', MigrateCommand) + @manager.command def list_routes(): diff --git a/config.py b/config.py index adf6d97ff..5d863ae6b 100644 --- a/config.py +++ b/config.py @@ -4,6 +4,9 @@ class Config(object): NOTIFY_LOG_LEVEL = 'DEBUG' NOTIFY_APP_NAME = 'api' NOTIFY_LOG_PATH = '/var/log/notify/application.log' + SQLALCHEMY_COMMIT_ON_TEARDOWN = False + SQLALCHEMY_RECORD_QUERIES = True + SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/notification_api' class Development(Config): @@ -12,11 +15,13 @@ class Development(Config): class Test(Config): DEBUG = True + SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/notification_api_test' class Live(Config): pass + configs = { 'development': Development, 'test': Test, diff --git a/migrations/README b/migrations/README new file mode 100644 index 000000000..98e4f9c44 --- /dev/null +++ b/migrations/README @@ -0,0 +1 @@ +Generic single-database configuration. \ No newline at end of file diff --git a/migrations/alembic.ini b/migrations/alembic.ini new file mode 100644 index 000000000..8c9d61375 --- /dev/null +++ b/migrations/alembic.ini @@ -0,0 +1,45 @@ +# A generic, single database configuration. + +[alembic] +# template used to generate migration files +# file_template = %%(rev)s_%%(slug)s + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S \ No newline at end of file diff --git a/migrations/env.py b/migrations/env.py new file mode 100644 index 000000000..70961ce2c --- /dev/null +++ b/migrations/env.py @@ -0,0 +1,73 @@ +from __future__ import with_statement +from alembic import context +from sqlalchemy import engine_from_config, pool +from logging.config import fileConfig + +# this is the Alembic Config object, which provides +# access to the values within the .ini file in use. +config = context.config + +# Interpret the config file for Python logging. +# This line sets up loggers basically. +fileConfig(config.config_file_name) + +# add your model's MetaData object here +# for 'autogenerate' support +# from myapp import mymodel +# target_metadata = mymodel.Base.metadata +from flask import current_app +config.set_main_option('sqlalchemy.url', current_app.config.get('SQLALCHEMY_DATABASE_URI')) +target_metadata = current_app.extensions['migrate'].db.metadata + +# other values from the config, defined by the needs of env.py, +# can be acquired: +# my_important_option = config.get_main_option("my_important_option") +# ... etc. + +def run_migrations_offline(): + """Run migrations in 'offline' mode. + + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. + + Calls to context.execute() here emit the given string to the + script output. + + """ + url = config.get_main_option("sqlalchemy.url") + context.configure(url=url) + + with context.begin_transaction(): + context.run_migrations() + +def run_migrations_online(): + """Run migrations in 'online' mode. + + In this scenario we need to create an Engine + and associate a connection with the context. + + """ + engine = engine_from_config( + config.get_section(config.config_ini_section), + prefix='sqlalchemy.', + poolclass=pool.NullPool) + + connection = engine.connect() + context.configure( + connection=connection, + target_metadata=target_metadata + ) + + try: + with context.begin_transaction(): + context.run_migrations() + finally: + connection.close() + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() + diff --git a/migrations/script.py.mako b/migrations/script.py.mako new file mode 100644 index 000000000..95702017e --- /dev/null +++ b/migrations/script.py.mako @@ -0,0 +1,22 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision} +Create Date: ${create_date} + +""" + +# revision identifiers, used by Alembic. +revision = ${repr(up_revision)} +down_revision = ${repr(down_revision)} + +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +def upgrade(): + ${upgrades if upgrades else "pass"} + + +def downgrade(): + ${downgrades if downgrades else "pass"} diff --git a/requirements.txt b/requirements.txt index 253fb420b..fa5036c5e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,10 @@ Flask==0.10.1 Flask-Script==2.0.5 +Flask-Migrate==1.3.1 +Flask-SQLAlchemy==2.0 +psycopg2==2.6.1 +SQLAlchemy==1.0.5 +SQLAlchemy-Utils==0.30.5 PyJWT==1.4.0 git+https://github.com/alphagov/notifications-python-client.git@0.1.5#egg=notifications-python-client==0.1.5 diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index 1b97244bd..9686abb00 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -28,3 +28,10 @@ fi # Install Python development dependencies pip3 install -r requirements_for_test.txt + +# Create Postgres databases +createdb notification_api +createdb test_notification_api + +# Upgrade databases +python application.py db upgrade diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index e0a5a06cc..c11e8e29a 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -29,5 +29,6 @@ display_result $? 1 "Code style check" #py.test --cov=app tests/ #display_result $? 2 "Code coverage" + py.test -v display_result $? 3 "Unit tests" \ No newline at end of file diff --git a/tests/app/main/dao/__init__.py b/tests/app/main/dao/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/main/dao/test_services_dao.py b/tests/app/main/dao/test_services_dao.py new file mode 100644 index 000000000..b623149e5 --- /dev/null +++ b/tests/app/main/dao/test_services_dao.py @@ -0,0 +1,8 @@ + + +def test_create_service(notify_api): + pass + + +def test_get_all_services(notify_api): + pass diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/conftest.py b/tests/conftest.py index 7d8cf6f23..2ccbd1593 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,13 @@ import pytest import mock -from app import create_app +import os from config import configs +from alembic.command import upgrade +from alembic.config import Config +from flask.ext.migrate import Migrate, MigrateCommand +from flask.ext.script import Manager + +from app import create_app, db @pytest.fixture(scope='session') @@ -17,6 +23,27 @@ def notify_api(request): return app +@pytest.fixture(scope='session') +def notify_db(notify_api, request): + Migrate(notify_api, db) + Manager(db, MigrateCommand) + BASE_DIR = os.path.dirname(os.path.dirname(__file__)) + ALEMBIC_CONFIG = os.path.join(BASE_DIR, 'migrations') + config = Config(ALEMBIC_CONFIG + '/alembic.ini') + config.set_main_option("script_location", ALEMBIC_CONFIG) + + with notify_api.app_context(): + upgrade(config, 'head') + + def teardown(): + db.session.remove() + db.drop_all() + db.engine.execute("drop table alembic_version") + db.get_engine(notify_api).dispose() + + request.addfinalizer(teardown) + + @pytest.fixture(scope='function') def notify_config(notify_api): notify_api.config['NOTIFY_API_ENVIRONMENT'] = 'test' From 5bcc6158254a92f99327bf57099f40d13fa9c47f Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Fri, 8 Jan 2016 12:18:12 +0000 Subject: [PATCH 02/12] Added dao, test framework and tests for dao. --- app/__init__.py | 1 - app/main/dao/services_dao.py | 19 +++++++------ app/main/dao/users_dao.py | 4 +-- config.py | 2 +- tests/app/conftest.py | 22 ++++++++++++++ tests/app/main/__init__.py | 0 tests/app/main/dao/test_services_dao.py | 38 ++++++++++++++++++++++--- tests/app/main/dao/test_users_dao.py | 30 +++++++++++++++++++ tests/conftest.py | 19 ++++++++++++- 9 files changed, 118 insertions(+), 17 deletions(-) create mode 100644 tests/app/conftest.py create mode 100644 tests/app/main/__init__.py diff --git a/app/__init__.py b/app/__init__.py index 0e4bf5016..a39772ebd 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -10,7 +10,6 @@ from utils import logging db = SQLAlchemy() - api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 1e497ec7c..8dec61537 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -6,11 +6,11 @@ from app import db from app.models import Service -def create_new_service(service_name, - user, - limit=1000, - active=False, - restricted=True): +def create_service(service_name, + user, + limit=1000, + active=False, + restricted=True): service = Service(name=service_name, created_at=datetime.now(), limit=limit, @@ -22,7 +22,10 @@ def create_new_service(service_name, return service.id -def get_services(user, service_id=None): +def get_services(service_id=None, user_id=None): + # TODO need better mapping from function params to sql query. if service_id: - return Service.query.filter_by(user=user, service_id=service_id).one() - return Service.query.filter_by(user=user).all() + return Service.query.filter_by(id=service_id).one() + elif user_id: + return Service.query.filter(Service.users.any(id=user_id)).all() + return Service.query.all() diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 3c2bdce00..a271856b3 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -6,7 +6,7 @@ from app import db from app.models import User -def create_new_user(email_address): +def create_user(email_address): user = User(email_address=email_address, created_at=datetime.now()) db.session.add(user) @@ -16,5 +16,5 @@ def create_new_user(email_address): def get_users(user_id=None): if user_id: - return User.query.filter_by(user_id=user_id).one() + return User.query.filter_by(id=user_id).one() return User.query.filter_by().all() diff --git a/config.py b/config.py index 5d863ae6b..23cb9ad05 100644 --- a/config.py +++ b/config.py @@ -15,7 +15,7 @@ class Development(Config): class Test(Config): DEBUG = True - SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/notification_api_test' + SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/test_notification_api' class Live(Config): diff --git a/tests/app/conftest.py b/tests/app/conftest.py new file mode 100644 index 000000000..e31e6ac18 --- /dev/null +++ b/tests/app/conftest.py @@ -0,0 +1,22 @@ +import pytest +from app.main.dao.users_dao import (create_user, get_users) +from app.main.dao.services_dao import (create_service, get_services) + + +@pytest.fixture(scope='function') +def sample_user(notify_db, + notify_db_session, + email="notify@digital.cabinet-office.gov.uk"): + user_id = create_user(email) + return get_users(user_id=user_id) + + +@pytest.fixture(scope='function') +def sample_service(notify_db, + notify_db_session, + service_name="Sample service", + user=None): + if user is None: + user = sample_user(notify_db, notify_db_session) + service_id = create_service(service_name, user) + return get_services(service_id=service_id) diff --git a/tests/app/main/__init__.py b/tests/app/main/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/main/dao/test_services_dao.py b/tests/app/main/dao/test_services_dao.py index b623149e5..66d700e27 100644 --- a/tests/app/main/dao/test_services_dao.py +++ b/tests/app/main/dao/test_services_dao.py @@ -1,8 +1,38 @@ +from app.main.dao.services_dao import (create_service, get_services) +from tests.app.conftest import sample_service as create_sample_service +from app.models import Service -def test_create_service(notify_api): - pass +def test_create_service(notify_api, notify_db, notify_db_session, sample_user): + assert Service.query.count() == 0 + service_name = 'Sample Service' + service_id = create_service(service_name, sample_user) + assert Service.query.count() == 1 + assert Service.query.first().name == service_name + assert Service.query.first().id == service_id -def test_get_all_services(notify_api): - pass +def test_get_services(notify_api, notify_db, notify_db_session, sample_user): + sample_service = create_sample_service(notify_db, + notify_db_session, + user=sample_user) + assert Service.query.count() == 1 + assert len(get_services()) == 1 + service_name = "Another service" + sample_service = create_sample_service(notify_db, + notify_db_session, + service_name=service_name, + user=sample_user) + assert Service.query.count() == 2 + assert len(get_services()) == 2 + + +def test_get_user_service(notify_api, notify_db, notify_db_session, sample_user): + assert Service.query.count() == 0 + service_name = "Random service" + sample_service = create_sample_service(notify_db, + notify_db_session, + service_name=service_name, + user=sample_user) + assert get_services(service_id=sample_service.id).name == service_name + assert Service.query.count() == 1 diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index e69de29bb..d382f4a19 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -0,0 +1,30 @@ +from app.main.dao.users_dao import (create_user, get_users) +from tests.app.conftest import sample_user as create_sample_user +from app.models import User + + +def test_create_user(notify_api, notify_db, notify_db_session): + email = 'notify@digital.cabinet-office.gov.uk' + user_id = create_user(email) + assert User.query.count() == 1 + assert User.query.first().email_address == email + assert User.query.filter_by(id=user_id).one() + + +def test_get_all_users(notify_api, notify_db, notify_db_session, sample_user): + assert User.query.count() == 1 + assert len(get_users()) == 1 + email = "another.notify@digital.cabinet-office.gov.uk" + another_user = create_sample_user(notify_db, + notify_db_session, + email=email) + assert User.query.count() == 2 + assert len(get_users()) == 2 + + +def test_get_user(notify_api, notify_db, notify_db_session): + email = "another.notify@digital.cabinet-office.gov.uk" + another_user = create_sample_user(notify_db, + notify_db_session, + email=email) + assert get_users(user_id=another_user.id).email_address == email diff --git a/tests/conftest.py b/tests/conftest.py index 2ccbd1593..aeddcc02e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,8 +6,9 @@ from alembic.command import upgrade from alembic.config import Config from flask.ext.migrate import Migrate, MigrateCommand from flask.ext.script import Manager - +from sqlalchemy.schema import MetaData from app import create_app, db +from app import models @pytest.fixture(scope='session') @@ -33,6 +34,10 @@ def notify_db(notify_api, request): config.set_main_option("script_location", ALEMBIC_CONFIG) with notify_api.app_context(): + # TODO this next line shouldn't be needed, + # but cannot work out the import order to + # remove it. + db.create_all() upgrade(config, 'head') def teardown(): @@ -44,6 +49,18 @@ def notify_db(notify_api, request): request.addfinalizer(teardown) +@pytest.fixture(scope='function') +def notify_db_session(request): + def teardown(): + db.session.remove() + for tbl in reversed(meta.sorted_tables): + if tbl.fullname not in ['roles']: + db.engine.execute(tbl.delete()) + + meta = MetaData(bind=db.engine, reflect=True) + request.addfinalizer(teardown) + + @pytest.fixture(scope='function') def notify_config(notify_api): notify_api.config['NOTIFY_API_ENVIRONMENT'] = 'test' From 0bc4d02713745a7c40457cb8783628594f66f4db Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Fri, 8 Jan 2016 17:51:46 +0000 Subject: [PATCH 03/12] Work in progress, skeleton of the api created and testing started. Need to fix authentication tests. --- .gitignore | 5 ++- app/__init__.py | 4 ++ app/{main => }/dao/__init__.py | 0 app/{main => }/dao/services_dao.py | 4 +- app/{main => }/dao/users_dao.py | 0 app/main/__init__.py | 12 +----- app/main/views/healthcheck.py | 7 ---- app/main/views/index.py | 15 ------- app/main/views/notifications.py | 7 ---- app/main/views/services.py | 38 ----------------- app/models.py | 42 ++++++++++++++----- app/schemas.py | 20 +++++++++ app/service/__init__.py | 5 +++ .../dao => app/service/views}/__init__.py | 0 app/service/views/rest.py | 36 ++++++++++++++++ app/user/__init__.py | 5 +++ app/user/views/__init__.py | 0 app/user/views/rest.py | 27 ++++++++++++ requirements.txt | 2 + tests/app/conftest.py | 4 +- tests/app/dao/__init__.py | 0 tests/app/{main => }/dao/test_services_dao.py | 2 +- tests/app/{main => }/dao/test_users_dao.py | 20 ++++++++- tests/app/service/__init__.py | 0 tests/app/service/views/__init__.py | 0 tests/app/service/views/test_rest.py | 33 +++++++++++++++ tests/app/user/__init__.py | 0 tests/app/user/views/__init__.py | 0 tests/app/user/views/test_rest.py | 0 29 files changed, 193 insertions(+), 95 deletions(-) rename app/{main => }/dao/__init__.py (100%) rename app/{main => }/dao/services_dao.py (86%) rename app/{main => }/dao/users_dao.py (100%) delete mode 100644 app/main/views/healthcheck.py delete mode 100644 app/main/views/notifications.py delete mode 100644 app/main/views/services.py create mode 100644 app/schemas.py create mode 100644 app/service/__init__.py rename {tests/app/main/dao => app/service/views}/__init__.py (100%) create mode 100644 app/service/views/rest.py create mode 100644 app/user/__init__.py create mode 100644 app/user/views/__init__.py create mode 100644 app/user/views/rest.py create mode 100644 tests/app/dao/__init__.py rename tests/app/{main => }/dao/test_services_dao.py (96%) rename tests/app/{main => }/dao/test_users_dao.py (68%) create mode 100644 tests/app/service/__init__.py create mode 100644 tests/app/service/views/__init__.py create mode 100644 tests/app/service/views/test_rest.py create mode 100644 tests/app/user/__init__.py create mode 100644 tests/app/user/views/__init__.py create mode 100644 tests/app/user/views/test_rest.py diff --git a/.gitignore b/.gitignore index 574e9809d..b2f1a66a4 100644 --- a/.gitignore +++ b/.gitignore @@ -55,4 +55,7 @@ docs/_build/ # PyBuilder target/ -.idea/ \ No newline at end of file +.idea/ + +# Mac +*.DS_Store \ No newline at end of file diff --git a/app/__init__.py b/app/__init__.py index a39772ebd..46af09999 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -25,7 +25,11 @@ def create_app(config_name): logging.init_app(application) from .main import main as main_blueprint + from .service import service as service_blueprint + from .user import user as user_blueprint application.register_blueprint(main_blueprint) + application.register_blueprint(service_blueprint, url_prefix='/service') + application.register_blueprint(user_blueprint, url_prefix='/user') from .status import status as status_blueprint application.register_blueprint(status_blueprint) diff --git a/app/main/dao/__init__.py b/app/dao/__init__.py similarity index 100% rename from app/main/dao/__init__.py rename to app/dao/__init__.py diff --git a/app/main/dao/services_dao.py b/app/dao/services_dao.py similarity index 86% rename from app/main/dao/services_dao.py rename to app/dao/services_dao.py index 8dec61537..3974d32f6 100644 --- a/app/main/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -24,7 +24,9 @@ def create_service(service_name, def get_services(service_id=None, user_id=None): # TODO need better mapping from function params to sql query. - if service_id: + if user_id and service_id: + return Service.query.filter(Service.users.any(id=user_id), id=service_id).one() + elif service_id: return Service.query.filter_by(id=service_id).one() elif user_id: return Service.query.filter(Service.users.any(id=user_id)).all() diff --git a/app/main/dao/users_dao.py b/app/dao/users_dao.py similarity index 100% rename from app/main/dao/users_dao.py rename to app/dao/users_dao.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 9631e7011..538216f43 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -1,15 +1,5 @@ from flask import Blueprint -from app.main.authentication.auth import requires_auth - -AUTHORIZATION_HEADER = 'Authorization' -AUTHORIZATION_SCHEME = 'Bearer' -WINDOW = 1 main = Blueprint('main', __name__) - -main.before_request(requires_auth) - - -from app.main.views import notifications, index -from app.main import errors +from app.main.views import index diff --git a/app/main/views/healthcheck.py b/app/main/views/healthcheck.py deleted file mode 100644 index d4ab49ca7..000000000 --- a/app/main/views/healthcheck.py +++ /dev/null @@ -1,7 +0,0 @@ -from flask import jsonify -from .. import main - - -@main.route('/', methods=['GET']) -def get_index(): - return jsonify(result="hello world"), 200 diff --git a/app/main/views/index.py b/app/main/views/index.py index 2c9b620d9..8037ce7e1 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -1,17 +1,2 @@ from flask import jsonify from .. import main - - -# TODO need for health check url - - -# TODO remove -@main.route('/', methods=['GET']) -def get_index(): - return jsonify(result="hello world"), 200 - - -# TODO remove -@main.route('/', methods=['POST']) -def post_index(): - return jsonify(result="hello world"), 200 diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py deleted file mode 100644 index ae1c5d43e..000000000 --- a/app/main/views/notifications.py +++ /dev/null @@ -1,7 +0,0 @@ -from flask import jsonify -from .. import main - - -@main.route('/notification', methods=['POST']) -def create_notification(): - return jsonify(result="created"), 201 diff --git a/app/main/views/services.py b/app/main/views/services.py deleted file mode 100644 index 6ced550bb..000000000 --- a/app/main/views/services.py +++ /dev/null @@ -1,38 +0,0 @@ -from flask import jsonify -from app.main.dao.services_dao import (create_new_service, get_services) -from app.main.dao.users_dao import (get_users) -from .. import main - - -# TODO auth to be added. -@main.route('/service', methods=['POST']) -def create_service(): - return jsonify(result="created"), 201 - - -# TODO auth to be added -@main.route('/service/', method=['PUT']) -def update_service(service_id): - return jsonify(result="updated") - - -# TODO auth to be added. -# Should be restricted by user, user id -# is pulled from the token -@main.route('/service/', method=['GET']) -@main.route('/service', methods=['GET']) -def get_service(service_id=None): - services = get_services - return jsonify( - data=services - ) - - -# TODO auth to be added -# auth should be allow for admin tokens only -@main.route('/user//service', method=['GET']) -@main.route('/user//service/', method=['GET']) -def get_service_by_user_id(user_id, service_id=None): - user = get_users(user_id=user_id) - services = get_services(user, service_id=service_id) - return jsonify(data=services) diff --git a/app/models.py b/app/models.py index cd370d21b..1de8a3d5e 100644 --- a/app/models.py +++ b/app/models.py @@ -1,6 +1,12 @@ from . import db +def filter_null_value_fields(obj): + return dict( + filter(lambda x: x[1] is not None, obj.items()) + ) + + class User(db.Model): __tablename__ = 'users' @@ -9,6 +15,20 @@ class User(db.Model): created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) updated_at = db.Column(db.DateTime, index=False, unique=False, nullable=True) + # def serialize(self): + # serialized = { + # 'id': self.id, + # 'name': self.name, + # 'emailAddress': self.email_address, + # 'locked': self.failed_login_count > current_app.config['MAX_FAILED_LOGIN_COUNT'], + # 'createdAt': self.created_at.strftime(DATETIME_FORMAT), + # 'updatedAt': self.updated_at.strftime(DATETIME_FORMAT), + # 'role': self.role, + # 'passwordChangedAt': self.password_changed_at.strftime(DATETIME_FORMAT), + # 'failedLoginCount': self.failed_login_count + # } + # return filter_null_value_fields(serialized) + user_to_service = db.Table( 'user_to_service', @@ -29,15 +49,15 @@ class Service(db.Model): users = db.relationship('User', secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) - def serialize(self): - serialized = { - 'id': self.id, - 'name': self.name, - 'createdAt': self.created_at.strftime(DATETIME_FORMAT), - 'active': self.active, - 'restricted': self.restricted, - 'limit': self.limit, - 'user': self.users.serialize() - } + # def serialize(self): + # serialized = { + # 'id': self.id, + # 'name': self.name, + # 'createdAt': self.created_at.strftime(DATETIME_FORMAT), + # 'active': self.active, + # 'restricted': self.restricted, + # 'limit': self.limit, + # 'user': self.users.serialize() + # } - return filter_null_value_fields(serialized) + # return filter_null_value_fields(serialized) diff --git a/app/schemas.py b/app/schemas.py new file mode 100644 index 000000000..230e1f0f6 --- /dev/null +++ b/app/schemas.py @@ -0,0 +1,20 @@ +from marshmallow_sqlalchemy import ModelSchema +from . import models + + +class UserSchema(ModelSchema): + class Meta: + model = models.User + + +# TODO process users list, to return a list of user.id +# Should that list be restricted?? +class ServiceSchema(ModelSchema): + class Meta: + model = models.Service + + +user_schema = ServiceSchema() +users_schema = UserSchema(many=True) +service_schema = ServiceSchema() +services_schema = ServiceSchema(many=True) diff --git a/app/service/__init__.py b/app/service/__init__.py new file mode 100644 index 000000000..82be0231c --- /dev/null +++ b/app/service/__init__.py @@ -0,0 +1,5 @@ +from flask import Blueprint + +service = Blueprint('service', __name__) + +from app.service.views import rest diff --git a/tests/app/main/dao/__init__.py b/app/service/views/__init__.py similarity index 100% rename from tests/app/main/dao/__init__.py rename to app/service/views/__init__.py diff --git a/app/service/views/rest.py b/app/service/views/rest.py new file mode 100644 index 000000000..c2ca6a511 --- /dev/null +++ b/app/service/views/rest.py @@ -0,0 +1,36 @@ +from flask import jsonify +from sqlalchemy.exc import DataError +from sqlalchemy.orm.exc import NoResultFound +from app.dao.services_dao import (create_service, get_services) +from app.dao.users_dao import (get_users) +from .. import service +from app.schemas import (services_schema, service_schema) + + +# TODO auth to be added. +@service.route('/', methods=['POST']) +def create_service(): + # Be lenient with args passed in + parsed_data = service_schema(request.args) + return jsonify(result="created"), 201 + + +# TODO auth to be added +@service.route('/', methods=['PUT']) +def update_service(service_id): + service = get_services(service_id=service_id) + return jsonify(data=service_schema.dump(service)) + + +# TODO auth to be added. +@service.route('/', methods=['GET']) +@service.route('/', methods=['GET']) +def get_service(service_id=None): + try: + services = get_services(service_id=service_id) + except DataError: + return jsonify(result="error", message="Invalid service id"), 400 + except NoResultFound: + return jsonify(result="error", message="Service doesn't exist"), 404 + result = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) + return jsonify(data=result.data) diff --git a/app/user/__init__.py b/app/user/__init__.py new file mode 100644 index 000000000..07cb211c6 --- /dev/null +++ b/app/user/__init__.py @@ -0,0 +1,5 @@ +from flask import Blueprint + +user = Blueprint('user', __name__) + +from app.user.views import rest diff --git a/app/user/views/__init__.py b/app/user/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/user/views/rest.py b/app/user/views/rest.py new file mode 100644 index 000000000..dbb3406bf --- /dev/null +++ b/app/user/views/rest.py @@ -0,0 +1,27 @@ +from flask import jsonify +from sqlalchemy.exc import DataError +from sqlalchemy.orm.exc import NoResultFound +from app.dao.services_dao import get_services +from app.dao.users_dao import get_users +from .. import user + + +# TODO auth to be added +@user.route('//service', methods=['GET']) +@user.route('//service/', methods=['GET']) +def get_service_by_user_id(user_id, service_id=None): + try: + user = get_users(user_id=user_id) + except DataError: + return jsonify(result="error", message="Invalid user id"), 400 + except NoResultFound: + return jsonify(result="error", message="User doesn't exist"), 400 + + try: + services = get_services(user_id=user.id, service_id=service_id) + except DataError: + return jsonify(result="error", message="Invalid service id"), 400 + except NoResultFound: + return jsonify(result="error", message="Service doesn't exist"), 404 + + return jsonify(data=services) diff --git a/requirements.txt b/requirements.txt index fa5036c5e..91ee73899 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,6 +6,8 @@ psycopg2==2.6.1 SQLAlchemy==1.0.5 SQLAlchemy-Utils==0.30.5 PyJWT==1.4.0 +marshmallow==2.4.2 +marshmallow-sqlalchemy==0.8.0 git+https://github.com/alphagov/notifications-python-client.git@0.1.5#egg=notifications-python-client==0.1.5 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index e31e6ac18..8a23247d4 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,6 +1,6 @@ import pytest -from app.main.dao.users_dao import (create_user, get_users) -from app.main.dao.services_dao import (create_service, get_services) +from app.dao.users_dao import (create_user, get_users) +from app.dao.services_dao import (create_service, get_services) @pytest.fixture(scope='function') diff --git a/tests/app/dao/__init__.py b/tests/app/dao/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/main/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py similarity index 96% rename from tests/app/main/dao/test_services_dao.py rename to tests/app/dao/test_services_dao.py index 66d700e27..e297177fe 100644 --- a/tests/app/main/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,4 +1,4 @@ -from app.main.dao.services_dao import (create_service, get_services) +from app.dao.services_dao import (create_service, get_services) from tests.app.conftest import sample_service as create_sample_service from app.models import Service diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py similarity index 68% rename from tests/app/main/dao/test_users_dao.py rename to tests/app/dao/test_users_dao.py index d382f4a19..54bcacf75 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -1,4 +1,6 @@ -from app.main.dao.users_dao import (create_user, get_users) +from sqlalchemy.exc import DataError +from sqlalchemy.orm.exc import NoResultFound +from app.dao.users_dao import (create_user, get_users) from tests.app.conftest import sample_user as create_sample_user from app.models import User @@ -28,3 +30,19 @@ def test_get_user(notify_api, notify_db, notify_db_session): notify_db_session, email=email) assert get_users(user_id=another_user.id).email_address == email + + +def test_get_user_not_exists(notify_api, notify_db, notify_db_session): + try: + get_users(user_id="12345") + pytest.fail("NoResultFound exception not thrown.") + except: + pass + + +def test_get_user_invalid_id(notify_api, notify_db, notify_db_session): + try: + get_users(user_id="blah") + pytest.fail("DataError exception not thrown.") + except DataError: + pass diff --git a/tests/app/service/__init__.py b/tests/app/service/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/service/views/__init__.py b/tests/app/service/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py new file mode 100644 index 000000000..f774fc453 --- /dev/null +++ b/tests/app/service/views/test_rest.py @@ -0,0 +1,33 @@ +import json +from flask import url_for + + +def test_get_service_list(notify_api, notify_db, notify_db_session, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + response = client.get(url_for('service.get_service')) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + # TODO assert correct json returned + assert len(json_resp['data']) == 1 + assert json_resp['data'][0]['name'] == sample_service.name + assert json_resp['data'][0]['id'] == sample_service.id + + +def test_get_service(notify_api, notify_db, notify_db_session, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + resp = client.get(url_for('service.get_service', + service_id=sample_service.id)) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['name'] == sample_service.name + assert json_resp['data']['id'] == sample_service.id + + +def test_post_service(notify_api, notify_db, notify_db_session, sample_service): + pass + + +def test_put_service(notify_api, notify_db, notify_db_session, sample_service): + pass diff --git a/tests/app/user/__init__.py b/tests/app/user/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/user/views/__init__.py b/tests/app/user/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/user/views/test_rest.py b/tests/app/user/views/test_rest.py new file mode 100644 index 000000000..e69de29bb From 49e98c21e74e4bdcb8ad63223a46170d05f9182d Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Mon, 11 Jan 2016 15:07:13 +0000 Subject: [PATCH 04/12] Service and User API added, working with tests. Still need to polish the edges and add more tests. --- app/__init__.py | 3 ++ app/dao/services_dao.py | 29 +++++----- app/dao/users_dao.py | 9 ++-- app/models.py | 60 ++++++++++----------- app/schemas.py | 33 ++++++++++-- app/service/views/rest.py | 19 ++++--- app/user/views/rest.py | 45 +++++++++++++--- requirements.txt | 1 + tests/app/conftest.py | 21 +++++--- tests/app/dao/test_services_dao.py | 38 ++++++++++--- tests/app/dao/test_users_dao.py | 17 +++--- tests/app/main/views/test_authentication.py | 10 ++++ tests/app/service/views/test_rest.py | 34 ++++++++++-- tests/app/user/views/test_rest.py | 57 ++++++++++++++++++++ 14 files changed, 282 insertions(+), 94 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 46af09999..f15ba6ac0 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -3,12 +3,14 @@ import os from flask._compat import string_types from flask import Flask, _request_ctx_stack from flask.ext.sqlalchemy import SQLAlchemy +from flask_marshmallow import Marshmallow from werkzeug.local import LocalProxy from config import configs from utils import logging db = SQLAlchemy() +ma = Marshmallow() api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user) @@ -20,6 +22,7 @@ def create_app(config_name): application.config.from_object(configs[config_name]) db.init_app(application) + ma.init_app(application) init_app(application) logging.init_app(application) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 3974d32f6..d9bc4e5d7 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,31 +1,32 @@ +import json from datetime import datetime from sqlalchemy.orm import load_only +from sqlalchemy.exc import SQLAlchemyError from app import db from app.models import Service -def create_service(service_name, - user, - limit=1000, - active=False, - restricted=True): - service = Service(name=service_name, - created_at=datetime.now(), - limit=limit, - active=active, - restricted=restricted) +# Should I use SQLAlchemyError? +class DAOException(SQLAlchemyError): + pass + + +def create_model_service(service): + users_list = getattr(service, 'users', []) + if not users_list: + error_msg = {'users': 'Missing data for required attribute'} + raise DAOException(json.dumps(error_msg)) db.session.add(service) - service.users.append(user) db.session.commit() - return service.id -def get_services(service_id=None, user_id=None): +def get_model_services(service_id=None, user_id=None): # TODO need better mapping from function params to sql query. if user_id and service_id: - return Service.query.filter(Service.users.any(id=user_id), id=service_id).one() + return Service.query.filter( + Service.users.any(id=user_id), id=service_id).one() elif service_id: return Service.query.filter_by(id=service_id).one() elif user_id: diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index a271856b3..da4577fd3 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -6,15 +6,12 @@ from app import db from app.models import User -def create_user(email_address): - user = User(email_address=email_address, - created_at=datetime.now()) - db.session.add(user) +def create_model_user(usr): + db.session.add(usr) db.session.commit() - return user.id -def get_users(user_id=None): +def get_model_users(user_id=None): if user_id: return User.query.filter_by(id=user_id).one() return User.query.filter_by().all() diff --git a/app/models.py b/app/models.py index 1de8a3d5e..f14f95cf2 100644 --- a/app/models.py +++ b/app/models.py @@ -1,4 +1,5 @@ from . import db +import datetime def filter_null_value_fields(obj): @@ -12,22 +13,18 @@ class User(db.Model): id = db.Column(db.Integer, primary_key=True) email_address = db.Column(db.String(255), nullable=False, index=True, unique=True) - created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) - updated_at = db.Column(db.DateTime, index=False, unique=False, nullable=True) - - # def serialize(self): - # serialized = { - # 'id': self.id, - # 'name': self.name, - # 'emailAddress': self.email_address, - # 'locked': self.failed_login_count > current_app.config['MAX_FAILED_LOGIN_COUNT'], - # 'createdAt': self.created_at.strftime(DATETIME_FORMAT), - # 'updatedAt': self.updated_at.strftime(DATETIME_FORMAT), - # 'role': self.role, - # 'passwordChangedAt': self.password_changed_at.strftime(DATETIME_FORMAT), - # 'failedLoginCount': self.failed_login_count - # } - # return filter_null_value_fields(serialized) + created_at = db.Column( + db.DateTime, + index=False, + unique=False, + nullable=False, + default=datetime.datetime.now) + updated_at = db.Column( + db.DateTime, + index=False, + unique=False, + nullable=True, + onupdate=datetime.datetime.now) user_to_service = db.Table( @@ -43,21 +40,22 @@ class Service(db.Model): id = db.Column(db.Integer, primary_key=True) name = db.Column(db.String(255), nullable=False) - created_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) + created_at = db.Column( + db.DateTime, + index=False, + unique=False, + nullable=False, + default=datetime.datetime.now) + updated_at = db.Column( + db.DateTime, + index=False, + unique=False, + nullable=True, + onupdate=datetime.datetime.now) active = db.Column(db.Boolean, index=False, unique=False, nullable=False) limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False) - users = db.relationship('User', secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) + users = db.relationship( + 'User', + secondary=user_to_service, + backref=db.backref('user_to_service', lazy='dynamic')) restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) - - # def serialize(self): - # serialized = { - # 'id': self.id, - # 'name': self.name, - # 'createdAt': self.created_at.strftime(DATETIME_FORMAT), - # 'active': self.active, - # 'restricted': self.restricted, - # 'limit': self.limit, - # 'user': self.users.serialize() - # } - - # return filter_null_value_fields(serialized) diff --git a/app/schemas.py b/app/schemas.py index 230e1f0f6..912259a90 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -1,20 +1,43 @@ -from marshmallow_sqlalchemy import ModelSchema +from . import ma from . import models +from marshmallow import post_load + +# TODO I think marshmallow provides a better integration and error handling. +# Would be better to replace functionality in dao with the marshmallow supported +# functionality. +# http://marshmallow.readthedocs.org/en/latest/api_reference.html -class UserSchema(ModelSchema): +class UserSchema(ma.ModelSchema): class Meta: model = models.User + exclude = ("updated_at", "created_at", "user_to_service") + + def make_object(self, data): + # TODO possibly override to handle instance creation + return super(UserSchema, self).make_object(data) + + # def dump(self, obj, many=None, update_fields=True, **kwargs): + # retval = super(UserSchema, self).dump( + # obj, many=many, update_fields=update_fields, **kwargs) + # if not many and 'email_address' not in retval.data: + # retval.data['email_address'] = obj.email_address + # return retval # TODO process users list, to return a list of user.id -# Should that list be restricted?? -class ServiceSchema(ModelSchema): +# Should that list be restricted by the auth parsed?? +class ServiceSchema(ma.ModelSchema): class Meta: model = models.Service + exclude = ("updated_at", "created_at") + + def make_object(self, data): + # TODO possibly override to handle instance creation + return super(ServiceSchema, self).make_object(data) -user_schema = ServiceSchema() +user_schema = UserSchema() users_schema = UserSchema(many=True) service_schema = ServiceSchema() services_schema = ServiceSchema(many=True) diff --git a/app/service/views/rest.py b/app/service/views/rest.py index c2ca6a511..75054a336 100644 --- a/app/service/views/rest.py +++ b/app/service/views/rest.py @@ -1,8 +1,8 @@ -from flask import jsonify +from flask import (jsonify, request) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.services_dao import (create_service, get_services) -from app.dao.users_dao import (get_users) +from app.dao.services_dao import (create_model_service, get_model_services) +from app.dao.users_dao import get_model_users from .. import service from app.schemas import (services_schema, service_schema) @@ -10,16 +10,19 @@ from app.schemas import (services_schema, service_schema) # TODO auth to be added. @service.route('/', methods=['POST']) def create_service(): - # Be lenient with args passed in - parsed_data = service_schema(request.args) - return jsonify(result="created"), 201 + # TODO what exceptions get passed from schema parsing? + service = service_schema.load(request.get_json()).data + print(service_schema.dump(service).data) + # Some magic here, it automatically creates the service object. + # Cool but need to understand how this works. + return jsonify(data=service_schema.dump(service).data), 201 # TODO auth to be added @service.route('/', methods=['PUT']) def update_service(service_id): service = get_services(service_id=service_id) - return jsonify(data=service_schema.dump(service)) + return jsonify(data=service_schema.dump(service).data) # TODO auth to be added. @@ -27,7 +30,7 @@ def update_service(service_id): @service.route('/', methods=['GET']) def get_service(service_id=None): try: - services = get_services(service_id=service_id) + services = get_model_services(service_id=service_id) except DataError: return jsonify(result="error", message="Invalid service id"), 400 except NoResultFound: diff --git a/app/user/views/rest.py b/app/user/views/rest.py index dbb3406bf..615f6b7f6 100644 --- a/app/user/views/rest.py +++ b/app/user/views/rest.py @@ -1,27 +1,58 @@ -from flask import jsonify +from flask import (jsonify, request) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.services_dao import get_services -from app.dao.users_dao import get_users +from app.dao.services_dao import get_model_services +from app.dao.users_dao import (get_model_users, create_model_user) +from app.schemas import ( + user_schema, users_schema, service_schema, services_schema) from .. import user +# TODO auth to be added +@user.route('/', methods=['POST']) +def create_user(): + user = user_schema.load(request.get_json()).data + create_model_user(user) + return jsonify(data=user_schema.dump(user).data), 201 + + +# TODO auth to be added +@user.route('/', methods=['PUT']) +def update_user(user_id): + user = get_model_users(user_id=user_id) + return jsonify(data=user_schema.dump(user).data) + + +# TODO auth to be added. +@user.route('/', methods=['GET']) +@user.route('/', methods=['GET']) +def get_user(user_id=None): + try: + users = get_model_users(user_id=user_id) + except DataError: + return jsonify(result="error", message="Invalid user id"), 400 + except NoResultFound: + return jsonify(result="error", message="User doesn't exist"), 404 + result = users_schema.dump(users) if isinstance(users, list) else user_schema.dump(users) + return jsonify(data=result.data) + + # TODO auth to be added @user.route('//service', methods=['GET']) @user.route('//service/', methods=['GET']) def get_service_by_user_id(user_id, service_id=None): try: - user = get_users(user_id=user_id) + user = get_model_users(user_id=user_id) except DataError: return jsonify(result="error", message="Invalid user id"), 400 except NoResultFound: return jsonify(result="error", message="User doesn't exist"), 400 try: - services = get_services(user_id=user.id, service_id=service_id) + services = get_model_services(user_id=user.id, service_id=service_id) except DataError: return jsonify(result="error", message="Invalid service id"), 400 except NoResultFound: return jsonify(result="error", message="Service doesn't exist"), 404 - - return jsonify(data=services) + result = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) + return jsonify(data=result.data) diff --git a/requirements.txt b/requirements.txt index 91ee73899..c66faae28 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ SQLAlchemy-Utils==0.30.5 PyJWT==1.4.0 marshmallow==2.4.2 marshmallow-sqlalchemy==0.8.0 +flask-marshmallow==0.6.2 git+https://github.com/alphagov/notifications-python-client.git@0.1.5#egg=notifications-python-client==0.1.5 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 8a23247d4..19d706573 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,14 +1,16 @@ import pytest -from app.dao.users_dao import (create_user, get_users) -from app.dao.services_dao import (create_service, get_services) +from app.models import (User, Service) +from app.dao.users_dao import (create_model_user, get_model_users) +from app.dao.services_dao import create_model_service @pytest.fixture(scope='function') def sample_user(notify_db, notify_db_session, email="notify@digital.cabinet-office.gov.uk"): - user_id = create_user(email) - return get_users(user_id=user_id) + user = User(**{'email_address': email}) + create_model_user(user) + return user @pytest.fixture(scope='function') @@ -18,5 +20,12 @@ def sample_service(notify_db, user=None): if user is None: user = sample_user(notify_db, notify_db_session) - service_id = create_service(service_name, user) - return get_services(service_id=service_id) + data = { + 'name': service_name, + 'users': [user], + 'limit': 1000, + 'active': False, + 'restricted': False} + service = Service(**data) + create_model_service(service) + return service diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index e297177fe..146a2cd44 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,4 +1,6 @@ -from app.dao.services_dao import (create_service, get_services) +import pytest +from app.dao.services_dao import ( + create_model_service, get_model_services, DAOException) from tests.app.conftest import sample_service as create_sample_service from app.models import Service @@ -6,10 +8,17 @@ from app.models import Service def test_create_service(notify_api, notify_db, notify_db_session, sample_user): assert Service.query.count() == 0 service_name = 'Sample Service' - service_id = create_service(service_name, sample_user) + data = { + 'name': service_name, + 'users': [sample_user], + 'limit': 1000, + 'active': False, + 'restricted': False} + service = Service(**data) + create_model_service(service) assert Service.query.count() == 1 assert Service.query.first().name == service_name - assert Service.query.first().id == service_id + assert Service.query.first().id == service.id def test_get_services(notify_api, notify_db, notify_db_session, sample_user): @@ -17,14 +26,14 @@ def test_get_services(notify_api, notify_db, notify_db_session, sample_user): notify_db_session, user=sample_user) assert Service.query.count() == 1 - assert len(get_services()) == 1 + assert len(get_model_services()) == 1 service_name = "Another service" sample_service = create_sample_service(notify_db, notify_db_session, service_name=service_name, user=sample_user) assert Service.query.count() == 2 - assert len(get_services()) == 2 + assert len(get_model_services()) == 2 def test_get_user_service(notify_api, notify_db, notify_db_session, sample_user): @@ -34,5 +43,22 @@ def test_get_user_service(notify_api, notify_db, notify_db_session, sample_user) notify_db_session, service_name=service_name, user=sample_user) - assert get_services(service_id=sample_service.id).name == service_name + assert get_model_services(service_id=sample_service.id).name == service_name assert Service.query.count() == 1 + + +def test_missing_user_attribute(notify_api, notify_db, notify_db_session): + assert Service.query.count() == 0 + try: + service_name = 'Sample Service' + data = { + 'name': service_name, + 'limit': 1000, + 'active': False, + 'restricted': False} + + service = Service(**data) + create_model_service(service) + pytest.fail("DAOException not thrown") + except DAOException as e: + assert "Missing data for required attribute" in str(e) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 54bcacf75..dd342e668 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -1,27 +1,28 @@ from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.users_dao import (create_user, get_users) +from app.dao.users_dao import (create_model_user, get_model_users) from tests.app.conftest import sample_user as create_sample_user from app.models import User def test_create_user(notify_api, notify_db, notify_db_session): email = 'notify@digital.cabinet-office.gov.uk' - user_id = create_user(email) + user = User(**{'email_address': email}) + create_model_user(user) assert User.query.count() == 1 assert User.query.first().email_address == email - assert User.query.filter_by(id=user_id).one() + assert User.query.first().id == user.id def test_get_all_users(notify_api, notify_db, notify_db_session, sample_user): assert User.query.count() == 1 - assert len(get_users()) == 1 + assert len(get_model_users()) == 1 email = "another.notify@digital.cabinet-office.gov.uk" another_user = create_sample_user(notify_db, notify_db_session, email=email) assert User.query.count() == 2 - assert len(get_users()) == 2 + assert len(get_model_users()) == 2 def test_get_user(notify_api, notify_db, notify_db_session): @@ -29,12 +30,12 @@ def test_get_user(notify_api, notify_db, notify_db_session): another_user = create_sample_user(notify_db, notify_db_session, email=email) - assert get_users(user_id=another_user.id).email_address == email + assert get_model_users(user_id=another_user.id).email_address == email def test_get_user_not_exists(notify_api, notify_db, notify_db_session): try: - get_users(user_id="12345") + get_model_users(user_id="12345") pytest.fail("NoResultFound exception not thrown.") except: pass @@ -42,7 +43,7 @@ def test_get_user_not_exists(notify_api, notify_db, notify_db_session): def test_get_user_invalid_id(notify_api, notify_db, notify_db_session): try: - get_users(user_id="blah") + get_model_users(user_id="blah") pytest.fail("DataError exception not thrown.") except DataError: pass diff --git a/tests/app/main/views/test_authentication.py b/tests/app/main/views/test_authentication.py index dd7d482cd..cad52d6a6 100644 --- a/tests/app/main/views/test_authentication.py +++ b/tests/app/main/views/test_authentication.py @@ -1,7 +1,9 @@ +import pytest from flask import json from client.authentication import create_jwt_token +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_request_with_no_token(notify_api): response = notify_api.test_client().get("/") assert response.status_code == 401 @@ -9,6 +11,7 @@ def test_should_not_allow_request_with_no_token(notify_api): assert data['error'] == 'Unauthorized, authentication token must be provided' +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_request_with_incorrect_header(notify_api): response = notify_api.test_client().get( "/", @@ -21,6 +24,7 @@ def test_should_not_allow_request_with_incorrect_header(notify_api): assert data['error'] == 'Unauthorized, authentication bearer scheme must be used' +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_request_with_incorrect_token(notify_api): response = notify_api.test_client().get( "/", @@ -33,6 +37,7 @@ def test_should_not_allow_request_with_incorrect_token(notify_api): assert data['error'] == 'Invalid token: signature' +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_incorrect_path(notify_api): token = create_jwt_token(request_method="GET", request_path="/bad", secret="secret", client_id="client_id") response = notify_api.test_client().get( @@ -46,6 +51,7 @@ def test_should_not_allow_incorrect_path(notify_api): assert data['error'] == 'Invalid token: request' +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_incorrect_method(notify_api): token = create_jwt_token(request_method="POST", request_path="/", secret="secret", client_id="client_id") response = notify_api.test_client().get( @@ -59,6 +65,7 @@ def test_should_not_allow_incorrect_method(notify_api): assert data['error'] == 'Invalid token: request' +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_invalid_secret(notify_api): token = create_jwt_token(request_method="POST", request_path="/", secret="not-so-secret", client_id="client_id") response = notify_api.test_client().get( @@ -72,6 +79,7 @@ def test_should_not_allow_invalid_secret(notify_api): assert data['error'] == 'Invalid token: signature' +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_allow_valid_token(notify_api): token = create_jwt_token(request_method="GET", request_path="/", secret="secret", client_id="client_id") response = notify_api.test_client().get( @@ -83,6 +91,7 @@ def test_should_allow_valid_token(notify_api): assert response.status_code == 200 +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_allow_valid_token_with_post_body(notify_api): json_body = json.dumps({ "key1": "value1", @@ -106,6 +115,7 @@ def test_should_allow_valid_token_with_post_body(notify_api): assert response.status_code == 200 +@pytest.mark.xfail(reason="Authentication to be added.") def test_should_not_allow_valid_token_with_invalid_post_body(notify_api): json_body = json.dumps({ "key1": "value1", diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index f774fc453..01e687284 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -1,20 +1,27 @@ import json +from app.models import Service from flask import url_for def test_get_service_list(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests GET endpoint '/' to retrieve entire service list. + """ with notify_api.test_request_context(): with notify_api.test_client() as client: response = client.get(url_for('service.get_service')) assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) # TODO assert correct json returned - assert len(json_resp['data']) == 1 + assert len(json_resp) == 1 assert json_resp['data'][0]['name'] == sample_service.name assert json_resp['data'][0]['id'] == sample_service.id def test_get_service(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests GET endpoint '/' to retrieve a single service. + """ with notify_api.test_request_context(): with notify_api.test_client() as client: resp = client.get(url_for('service.get_service', @@ -25,8 +32,29 @@ def test_get_service(notify_api, notify_db, notify_db_session, sample_service): assert json_resp['data']['id'] == sample_service.id -def test_post_service(notify_api, notify_db, notify_db_session, sample_service): - pass +def test_post_service(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests POST endpoint '/' to create a service. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert Service.query.count() == 0 + data = { + 'name': 'created service', + 'users': [sample_user.id], + 'limit': 1000, + 'restricted': False, + 'active': False} + headers = [('Content-Type', 'application/json')] + resp = client.post( + url_for('service.create_service'), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 201 + service = Service.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['name'] == service.name + assert json_resp['data']['limit'] == service.limit def test_put_service(notify_api, notify_db, notify_db_session, sample_service): diff --git a/tests/app/user/views/test_rest.py b/tests/app/user/views/test_rest.py index e69de29bb..e4edb31c3 100644 --- a/tests/app/user/views/test_rest.py +++ b/tests/app/user/views/test_rest.py @@ -0,0 +1,57 @@ +import json +from app.models import User +from flask import url_for + + +def test_get_user_list(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests GET endpoint '/' to retrieve entire user list. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + response = client.get(url_for('user.get_user')) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + # TODO assert correct json returned + assert len(json_resp['data']) == 1 + assert json_resp['data'][0]['email_address'] == sample_user.email_address + assert json_resp['data'][0]['id'] == sample_user.id + + +def test_get_user(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests GET endpoint '/' to retrieve a single service. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + resp = client.get(url_for('user.get_user', + user_id=sample_user.id)) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['email_address'] == sample_user.email_address + assert json_resp['data']['id'] == sample_user.id + + +def test_post_user(notify_api, notify_db, notify_db_session): + """ + Tests POST endpoint '/' to create a user. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert User.query.count() == 0 + data = { + 'email_address': 'user@digital.cabinet-office.gov.uk'} + headers = [('Content-Type', 'application/json')] + resp = client.post( + url_for('user.create_user'), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 201 + user = User.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['email_address'] == user.email_address + assert json_resp['data']['id'] == user.id + + +def test_put_user(notify_api, notify_db, notify_db_session, sample_user): + pass From 74547013ba4f99c6390ce325d718e77e90169466 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Mon, 11 Jan 2016 17:19:06 +0000 Subject: [PATCH 05/12] All four http methods working now for user and service restful apis. --- app/dao/__init__.py | 6 ++++++ app/dao/services_dao.py | 20 ++++++++---------- app/dao/users_dao.py | 10 ++++++--- app/schemas.py | 15 -------------- app/service/views/rest.py | 31 ++++++++++++++++++++-------- app/user/views/rest.py | 19 ++++++++++++++--- tests/app/conftest.py | 8 +++---- tests/app/dao/test_services_dao.py | 6 +++--- tests/app/dao/test_users_dao.py | 4 ++-- tests/app/service/views/test_rest.py | 30 +++++++++++++++++++++++++-- tests/app/user/views/test_rest.py | 18 +++++++++++++++- 11 files changed, 114 insertions(+), 53 deletions(-) diff --git a/app/dao/__init__.py b/app/dao/__init__.py index e69de29bb..ce0df4421 100644 --- a/app/dao/__init__.py +++ b/app/dao/__init__.py @@ -0,0 +1,6 @@ +from sqlalchemy.exc import SQLAlchemyError + + +# Should I use SQLAlchemyError? +class DAOException(SQLAlchemyError): + pass diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index d9bc4e5d7..403b28579 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,24 +1,22 @@ import json from datetime import datetime - from sqlalchemy.orm import load_only -from sqlalchemy.exc import SQLAlchemyError - +from . import DAOException from app import db from app.models import Service -# Should I use SQLAlchemyError? -class DAOException(SQLAlchemyError): - pass - - -def create_model_service(service): - users_list = getattr(service, 'users', []) +def save_model_service(service, update_dict=None): + users_list = update_dict.get('users', []) if update_dict else getattr(service, 'users', []) if not users_list: error_msg = {'users': 'Missing data for required attribute'} raise DAOException(json.dumps(error_msg)) - db.session.add(service) + if update_dict: + del update_dict['id'] + del update_dict['users'] + db.session.query(Service).filter_by(id=service.id).update(update_dict) + else: + db.session.add(service) db.session.commit() diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index da4577fd3..ca4264cca 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -1,13 +1,17 @@ from datetime import datetime - +from . import DAOException from sqlalchemy.orm import load_only from app import db from app.models import User -def create_model_user(usr): - db.session.add(usr) +def save_model_user(usr, update_dict={}): + if update_dict: + del update_dict['id'] + db.session.query(User).filter_by(id=usr.id).update(update_dict) + else: + db.session.add(usr) db.session.commit() diff --git a/app/schemas.py b/app/schemas.py index 912259a90..0d2e14d06 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -13,17 +13,6 @@ class UserSchema(ma.ModelSchema): model = models.User exclude = ("updated_at", "created_at", "user_to_service") - def make_object(self, data): - # TODO possibly override to handle instance creation - return super(UserSchema, self).make_object(data) - - # def dump(self, obj, many=None, update_fields=True, **kwargs): - # retval = super(UserSchema, self).dump( - # obj, many=many, update_fields=update_fields, **kwargs) - # if not many and 'email_address' not in retval.data: - # retval.data['email_address'] = obj.email_address - # return retval - # TODO process users list, to return a list of user.id # Should that list be restricted by the auth parsed?? @@ -32,10 +21,6 @@ class ServiceSchema(ma.ModelSchema): model = models.Service exclude = ("updated_at", "created_at") - def make_object(self, data): - # TODO possibly override to handle instance creation - return super(ServiceSchema, self).make_object(data) - user_schema = UserSchema() users_schema = UserSchema(many=True) diff --git a/app/service/views/rest.py b/app/service/views/rest.py index 75054a336..18a14ba3f 100644 --- a/app/service/views/rest.py +++ b/app/service/views/rest.py @@ -1,9 +1,10 @@ from flask import (jsonify, request) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.services_dao import (create_model_service, get_model_services) +from app.dao.services_dao import (save_model_service, get_model_services) from app.dao.users_dao import get_model_users from .. import service +from app import db from app.schemas import (services_schema, service_schema) @@ -11,17 +12,29 @@ from app.schemas import (services_schema, service_schema) @service.route('/', methods=['POST']) def create_service(): # TODO what exceptions get passed from schema parsing? - service = service_schema.load(request.get_json()).data - print(service_schema.dump(service).data) - # Some magic here, it automatically creates the service object. - # Cool but need to understand how this works. + service, errors = service_schema.load(request.get_json()) + # I believe service is already added to the session but just needs a + # db.session.commit + save_model_service(service) return jsonify(data=service_schema.dump(service).data), 201 # TODO auth to be added @service.route('/', methods=['PUT']) def update_service(service_id): - service = get_services(service_id=service_id) + try: + service = get_model_services(service_id=service_id) + except DataError: + return jsonify(result="error", message="Invalid service id"), 400 + except NoResultFound: + return jsonify(result="error", message="Service not found"), 404 + # TODO there has got to be a better way to do the next three lines + update_service, errors = service_schema.load(request.get_json()) + update_dict, errors = service_schema.dump(update_service) + # TODO FIX ME + # Remove update_service model which is added to db.session + db.session.rollback() + save_model_service(service, update_dict=update_dict) return jsonify(data=service_schema.dump(service).data) @@ -34,6 +47,6 @@ def get_service(service_id=None): except DataError: return jsonify(result="error", message="Invalid service id"), 400 except NoResultFound: - return jsonify(result="error", message="Service doesn't exist"), 404 - result = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) - return jsonify(data=result.data) + return jsonify(result="error", message="Service not found"), 404 + data, errors = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) + return jsonify(data=data) diff --git a/app/user/views/rest.py b/app/user/views/rest.py index 615f6b7f6..dc0dcfd1f 100644 --- a/app/user/views/rest.py +++ b/app/user/views/rest.py @@ -2,24 +2,37 @@ from flask import (jsonify, request) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from app.dao.services_dao import get_model_services -from app.dao.users_dao import (get_model_users, create_model_user) +from app.dao.users_dao import (get_model_users, save_model_user) from app.schemas import ( user_schema, users_schema, service_schema, services_schema) from .. import user +from app import db # TODO auth to be added @user.route('/', methods=['POST']) def create_user(): user = user_schema.load(request.get_json()).data - create_model_user(user) + save_model_user(user) return jsonify(data=user_schema.dump(user).data), 201 # TODO auth to be added @user.route('/', methods=['PUT']) def update_user(user_id): - user = get_model_users(user_id=user_id) + try: + user = get_model_users(user_id=user_id) + except DataError: + return jsonify(result="error", message="Invalid user id"), 400 + except NoResultFound: + return jsonify(result="error", message="User not found"), 404 + # TODO there has got to be a better way to do the next three lines + update_user, errors = user_schema.load(request.get_json()) + update_dict, errors = user_schema.dump(update_user) + # TODO FIX ME + # Remove update_service model which is added to db.session + db.session.rollback() + save_model_user(user, update_dict=update_dict) return jsonify(data=user_schema.dump(user).data) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 19d706573..5f3b5ead4 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,7 +1,7 @@ import pytest from app.models import (User, Service) -from app.dao.users_dao import (create_model_user, get_model_users) -from app.dao.services_dao import create_model_service +from app.dao.users_dao import (save_model_user, get_model_users) +from app.dao.services_dao import save_model_service @pytest.fixture(scope='function') @@ -9,7 +9,7 @@ def sample_user(notify_db, notify_db_session, email="notify@digital.cabinet-office.gov.uk"): user = User(**{'email_address': email}) - create_model_user(user) + save_model_user(user) return user @@ -27,5 +27,5 @@ def sample_service(notify_db, 'active': False, 'restricted': False} service = Service(**data) - create_model_service(service) + save_model_service(service) return service diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 146a2cd44..a6b4abe68 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,6 +1,6 @@ import pytest from app.dao.services_dao import ( - create_model_service, get_model_services, DAOException) + save_model_service, get_model_services, DAOException) from tests.app.conftest import sample_service as create_sample_service from app.models import Service @@ -15,7 +15,7 @@ def test_create_service(notify_api, notify_db, notify_db_session, sample_user): 'active': False, 'restricted': False} service = Service(**data) - create_model_service(service) + save_model_service(service) assert Service.query.count() == 1 assert Service.query.first().name == service_name assert Service.query.first().id == service.id @@ -58,7 +58,7 @@ def test_missing_user_attribute(notify_api, notify_db, notify_db_session): 'restricted': False} service = Service(**data) - create_model_service(service) + save_model_service(service) pytest.fail("DAOException not thrown") except DAOException as e: assert "Missing data for required attribute" in str(e) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index dd342e668..443a1cf22 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -1,6 +1,6 @@ from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.users_dao import (create_model_user, get_model_users) +from app.dao.users_dao import (save_model_user, get_model_users) from tests.app.conftest import sample_user as create_sample_user from app.models import User @@ -8,7 +8,7 @@ from app.models import User def test_create_user(notify_api, notify_db, notify_db_session): email = 'notify@digital.cabinet-office.gov.uk' user = User(**{'email_address': email}) - create_model_user(user) + save_model_user(user) assert User.query.count() == 1 assert User.query.first().email_address == email assert User.query.first().id == user.id diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index 01e687284..4251e0508 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -1,5 +1,5 @@ import json -from app.models import Service +from app.models import (Service, User) from flask import url_for @@ -58,4 +58,30 @@ def test_post_service(notify_api, notify_db, notify_db_session, sample_user): def test_put_service(notify_api, notify_db, notify_db_session, sample_service): - pass + """ + Tests Put endpoint '/ Date: Mon, 11 Jan 2016 18:09:10 +0000 Subject: [PATCH 06/12] Add more tests. --- app/dao/services_dao.py | 2 +- app/service/views/rest.py | 4 + app/user/views/rest.py | 16 ++-- tests/app/user/views/test_rest.py | 126 +++++++++++++++++++++++++++++- 4 files changed, 140 insertions(+), 8 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 403b28579..9c94174a6 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -24,7 +24,7 @@ def get_model_services(service_id=None, user_id=None): # TODO need better mapping from function params to sql query. if user_id and service_id: return Service.query.filter( - Service.users.any(id=user_id), id=service_id).one() + Service.users.any(id=user_id)).filter_by(id=service_id).one() elif service_id: return Service.query.filter_by(id=service_id).one() elif user_id: diff --git a/app/service/views/rest.py b/app/service/views/rest.py index 18a14ba3f..41a411753 100644 --- a/app/service/views/rest.py +++ b/app/service/views/rest.py @@ -13,6 +13,8 @@ from app.schemas import (services_schema, service_schema) def create_service(): # TODO what exceptions get passed from schema parsing? service, errors = service_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 # I believe service is already added to the session but just needs a # db.session.commit save_model_service(service) @@ -30,6 +32,8 @@ def update_service(service_id): return jsonify(result="error", message="Service not found"), 404 # TODO there has got to be a better way to do the next three lines update_service, errors = service_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 update_dict, errors = service_schema.dump(update_service) # TODO FIX ME # Remove update_service model which is added to db.session diff --git a/app/user/views/rest.py b/app/user/views/rest.py index dc0dcfd1f..c0eb533ab 100644 --- a/app/user/views/rest.py +++ b/app/user/views/rest.py @@ -12,7 +12,9 @@ from app import db # TODO auth to be added @user.route('/', methods=['POST']) def create_user(): - user = user_schema.load(request.get_json()).data + user, errors = user_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 save_model_user(user) return jsonify(data=user_schema.dump(user).data), 201 @@ -28,6 +30,8 @@ def update_user(user_id): return jsonify(result="error", message="User not found"), 404 # TODO there has got to be a better way to do the next three lines update_user, errors = user_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 update_dict, errors = user_schema.dump(update_user) # TODO FIX ME # Remove update_service model which is added to db.session @@ -45,7 +49,7 @@ def get_user(user_id=None): except DataError: return jsonify(result="error", message="Invalid user id"), 400 except NoResultFound: - return jsonify(result="error", message="User doesn't exist"), 404 + return jsonify(result="error", message="User not found"), 404 result = users_schema.dump(users) if isinstance(users, list) else user_schema.dump(users) return jsonify(data=result.data) @@ -59,13 +63,13 @@ def get_service_by_user_id(user_id, service_id=None): except DataError: return jsonify(result="error", message="Invalid user id"), 400 except NoResultFound: - return jsonify(result="error", message="User doesn't exist"), 400 + return jsonify(result="error", message="User not found"), 404 try: services = get_model_services(user_id=user.id, service_id=service_id) except DataError: return jsonify(result="error", message="Invalid service id"), 400 except NoResultFound: - return jsonify(result="error", message="Service doesn't exist"), 404 - result = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) - return jsonify(data=result.data) + return jsonify(result="error", message="Service not found"), 404 + services, errors = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) + return jsonify(data=services) diff --git a/tests/app/user/views/test_rest.py b/tests/app/user/views/test_rest.py index d9d86fb56..9b6e7ac6f 100644 --- a/tests/app/user/views/test_rest.py +++ b/tests/app/user/views/test_rest.py @@ -1,5 +1,6 @@ import json -from app.models import User +from app.models import (User, Service) +from tests.app.conftest import sample_service as create_sample_service from flask import url_for @@ -53,7 +54,30 @@ def test_post_user(notify_api, notify_db, notify_db_session): assert json_resp['data']['id'] == user.id +def test_post_user_missing_attribute_email(notify_api, notify_db, notify_db_session): + """ + Tests POST endpoint '/' missing attribute email. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert User.query.count() == 0 + data = { + 'blah': 'blah.blah'} + headers = [('Content-Type', 'application/json')] + resp = client.post( + url_for('user.create_user'), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 400 + assert User.query.count() == 0 + json_resp = json.loads(resp.get_data(as_text=True)) + assert {'email_address': ['Missing data for required field.']} == json_resp['message'] + + def test_put_user(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests PUT endpoint '/' to update a user. + """ with notify_api.test_request_context(): with notify_api.test_client() as client: assert User.query.count() == 1 @@ -71,3 +95,103 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_user): json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['data']['email_address'] == new_email assert json_resp['data']['id'] == user.id + + +def test_put_user_missing_email(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests PUT endpoint '/' missing attribute email. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert User.query.count() == 1 + new_email = 'new@digital.cabinet-office.gov.uk' + data = { + 'blah': new_email} + headers = [('Content-Type', 'application/json')] + resp = client.put( + url_for('user.update_user', user_id=sample_user.id), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 400 + assert User.query.count() == 1 + user = User.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert user.email_address == sample_user.email_address + assert {'email_address': ['Missing data for required field.']} == json_resp['message'] + + +def test_get_user_services(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests GET endpoint "//service/" to retrieve services for a user. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + user = User.query.first() + another_name = "another name" + another_service = create_sample_service( + notify_db, + notify_db_session, + service_name=another_name, + user=user) + assert Service.query.count() == 2 + resp = client.get( + url_for('user.get_service_by_user_id', user_id=user.id), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + + +def test_get_user_service(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests GET endpoint "//service/" to retrieve a service for a user. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + user = User.query.first() + another_name = "another name" + another_service = create_sample_service( + notify_db, + notify_db_session, + service_name=another_name, + user=user) + assert Service.query.count() == 2 + resp = client.get( + url_for('user.get_service_by_user_id', user_id=user.id, service_id=another_service.id), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['name'] == another_name + assert json_resp['data']['id'] == another_service.id + + +def test_get_user_service_user_not_exists(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests GET endpoint "//service/" 404 is returned for invalid user. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + user = User.query.first() + assert Service.query.count() == 1 + resp = client.get( + url_for('user.get_service_by_user_id', user_id="123", service_id=sample_service.id), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 404 + json_resp = json.loads(resp.get_data(as_text=True)) + assert "User not found" in json_resp['message'] + + +def test_get_user_service_service_not_exists(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests GET endpoint "//service/" 404 is returned for invalid service. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + user = User.query.first() + assert Service.query.count() == 1 + resp = client.get( + url_for('user.get_service_by_user_id', user_id=user.id, service_id="123"), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 404 + json_resp = json.loads(resp.get_data(as_text=True)) + assert "Service not found" in json_resp['message'] From 308e2cb7de8c9c2e15fbe0934e8010afb1b5160d Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 12 Jan 2016 09:28:01 +0000 Subject: [PATCH 07/12] More tests. --- app/dao/services_dao.py | 20 +++-- app/service/views/rest.py | 15 +++- tests/app/service/views/test_rest.py | 126 ++++++++++++++++++++++++++- 3 files changed, 151 insertions(+), 10 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 9c94174a6..fafe68c8f 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -3,18 +3,28 @@ from datetime import datetime from sqlalchemy.orm import load_only from . import DAOException from app import db -from app.models import Service +from app.models import (Service, User) def save_model_service(service, update_dict=None): users_list = update_dict.get('users', []) if update_dict else getattr(service, 'users', []) if not users_list: - error_msg = {'users': 'Missing data for required attribute'} + error_msg = {'users': ['Missing data for required attribute']} raise DAOException(json.dumps(error_msg)) if update_dict: - del update_dict['id'] - del update_dict['users'] - db.session.query(Service).filter_by(id=service.id).update(update_dict) + # Make sure the update_dict doesn't contain conflicting + update_dict.pop('id', None) + update_dict.pop('users', None) + # TODO optimize this algorithm + new_users = User.query.filter(User.id.in_(users_list)).all() + for x in service.users: + if x in new_users: + new_users.remove(x) + else: + service.users.remove(x) + for x in new_users: + service.users.append(x) + Service.query.filter_by(id=service.id).update(update_dict) else: db.session.add(service) db.session.commit() diff --git a/app/service/views/rest.py b/app/service/views/rest.py index 41a411753..4f5567f56 100644 --- a/app/service/views/rest.py +++ b/app/service/views/rest.py @@ -3,6 +3,7 @@ from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from app.dao.services_dao import (save_model_service, get_model_services) from app.dao.users_dao import get_model_users +from app.dao import DAOException from .. import service from app import db from app.schemas import (services_schema, service_schema) @@ -17,7 +18,10 @@ def create_service(): return jsonify(result="error", message=errors), 400 # I believe service is already added to the session but just needs a # db.session.commit - save_model_service(service) + try: + save_model_service(service) + except DAOException as e: + return jsonify(result="error", message=str(e)), 400 return jsonify(data=service_schema.dump(service).data), 201 @@ -31,14 +35,17 @@ def update_service(service_id): except NoResultFound: return jsonify(result="error", message="Service not found"), 404 # TODO there has got to be a better way to do the next three lines - update_service, errors = service_schema.load(request.get_json()) + upd_serv, errors = service_schema.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 - update_dict, errors = service_schema.dump(update_service) + update_dict, errors = service_schema.dump(upd_serv) # TODO FIX ME # Remove update_service model which is added to db.session db.session.rollback() - save_model_service(service, update_dict=update_dict) + try: + save_model_service(service, update_dict=update_dict) + except DAOException as e: + return jsonify(result="error", message=str(e)), 400 return jsonify(data=service_schema.dump(service).data) diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index 4251e0508..e65c5cdc2 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -1,5 +1,7 @@ import json from app.models import (Service, User) +from app.dao.services_dao import save_model_service +from tests.app.conftest import sample_user as create_sample_user from flask import url_for @@ -57,9 +59,61 @@ def test_post_service(notify_api, notify_db, notify_db_session, sample_user): assert json_resp['data']['limit'] == service.limit +def test_post_service_multiple_users(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests POST endpoint '/' to create a service with multiple users. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + another_user = create_sample_user( + notify_db, + notify_db_session, + "new@digital.cabinet-office.gov.uk") + assert Service.query.count() == 0 + data = { + 'name': 'created service', + 'users': [sample_user.id, another_user.id], + 'limit': 1000, + 'restricted': False, + 'active': False} + headers = [('Content-Type', 'application/json')] + resp = client.post( + url_for('service.create_service'), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 201 + service = Service.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['name'] == service.name + assert json_resp['data']['limit'] == service.limit + assert len(service.users) == 2 + +def test_post_service_without_users_attribute(notify_api, notify_db, notify_db_session): + """ + Tests POST endpoint '/' to create a service without 'users' attribute. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert Service.query.count() == 0 + data = { + 'name': 'created service', + 'limit': 1000, + 'restricted': False, + 'active': False} + headers = [('Content-Type', 'application/json')] + resp = client.post( + url_for('service.create_service'), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 400 + assert Service.query.count() == 0 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['message'] == '{"users": ["Missing data for required attribute"]}' + + def test_put_service(notify_api, notify_db, notify_db_session, sample_service): """ - Tests Put endpoint '/' to edit a service. """ with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -85,3 +139,73 @@ def test_put_service(notify_api, notify_db, notify_db_session, sample_service): assert json_resp['data']['name'] == updated_service.name assert json_resp['data']['limit'] == updated_service.limit assert updated_service.name == new_name + + +def test_put_service_add_user(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests PUT endpoint '/' add user to the service. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert Service.query.count() == 1 + another_user = create_sample_user( + notify_db, + notify_db_session, + "new@digital.cabinet-office.gov.uk") + sample_user = User.query.first() + old_service = Service.query.first() + new_name = 'updated service' + data = { + 'name': new_name, + 'users': [sample_user.id, another_user.id], + 'limit': 1000, + 'restricted': False, + 'active': False} + headers = [('Content-Type', 'application/json')] + resp = client.put( + url_for('service.update_service', service_id=old_service.id), + data=json.dumps(data), + headers=headers) + assert Service.query.count() == 1 + assert resp.status_code == 200 + updated_service = Service.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert len(json_resp['data']['users']) == 2 + assert sample_user.id in json_resp['data']['users'] + assert another_user.id in json_resp['data']['users'] + + +def test_put_service_remove_user(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests PUT endpoint '/' add user to the service. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + sample_user = User.query.first() + another_user = create_sample_user( + notify_db, + notify_db_session, + "new@digital.cabinet-office.gov.uk") + data = { + 'name': sample_service.name, + 'users': [sample_user.id, another_user.id], + 'limit': sample_service.limit, + 'restricted': sample_service.restricted, + 'active': sample_service.active} + save_model_service(sample_service, update_dict=data) + assert Service.query.count() == 1 + sample_user = User.query.first() + data['users'] = [another_user.id] + headers = [('Content-Type', 'application/json')] + resp = client.put( + url_for('service.update_service', service_id=sample_service.id), + data=json.dumps(data), + headers=headers) + assert Service.query.count() == 1 + assert resp.status_code == 200 + updated_service = Service.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert len(json_resp['data']['users']) == 1 + assert sample_user.id not in json_resp['data']['users'] + assert another_user.id in json_resp['data']['users'] + From 3397274b83129808589ae78942e828abda0a70fb Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 12 Jan 2016 09:31:52 +0000 Subject: [PATCH 08/12] Test add. --- tests/app/service/views/test_rest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index e65c5cdc2..ff89f5d8b 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -209,3 +209,5 @@ def test_put_service_remove_user(notify_api, notify_db, notify_db_session, sampl assert sample_user.id not in json_resp['data']['users'] assert another_user.id in json_resp['data']['users'] + + From 752a359d3ddec826e7b692d89d4af27a5e467580 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 12 Jan 2016 09:56:42 +0000 Subject: [PATCH 09/12] Added versions file for initial db and fixed tests. --- migrations/versions/0001_initialise_data.py | 52 +++++++++++++++++++++ tests/app/service/views/test_rest.py | 4 +- tests/conftest.py | 4 -- 3 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 migrations/versions/0001_initialise_data.py diff --git a/migrations/versions/0001_initialise_data.py b/migrations/versions/0001_initialise_data.py new file mode 100644 index 000000000..a1f5ab250 --- /dev/null +++ b/migrations/versions/0001_initialise_data.py @@ -0,0 +1,52 @@ +"""empty message + +Revision ID: 0001_initialise_data +Revises: None +Create Date: 2016-01-12 09:33:29.249042 + +""" + +# revision identifiers, used by Alembic. +revision = '0001_initialise_data' +down_revision = None + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_table('services', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('name', sa.String(length=255), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.Column('active', sa.Boolean(), nullable=False), + sa.Column('limit', sa.BigInteger(), nullable=False), + sa.Column('restricted', sa.Boolean(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + op.create_table('users', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('email_address', sa.String(length=255), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_users_email_address'), 'users', ['email_address'], unique=True) + op.create_table('user_to_service', + sa.Column('user_id', sa.Integer(), nullable=True), + sa.Column('service_id', sa.Integer(), nullable=True), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ) + ) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_table('user_to_service') + op.drop_index(op.f('ix_users_email_address'), table_name='users') + op.drop_table('users') + op.drop_table('services') + ### end Alembic commands ### diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index ff89f5d8b..2e0cd3e8a 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -88,6 +88,7 @@ def test_post_service_multiple_users(notify_api, notify_db, notify_db_session, s assert json_resp['data']['limit'] == service.limit assert len(service.users) == 2 + def test_post_service_without_users_attribute(notify_api, notify_db, notify_db_session): """ Tests POST endpoint '/' to create a service without 'users' attribute. @@ -208,6 +209,3 @@ def test_put_service_remove_user(notify_api, notify_db, notify_db_session, sampl assert len(json_resp['data']['users']) == 1 assert sample_user.id not in json_resp['data']['users'] assert another_user.id in json_resp['data']['users'] - - - diff --git a/tests/conftest.py b/tests/conftest.py index aeddcc02e..a61f1e724 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -34,10 +34,6 @@ def notify_db(notify_api, request): config.set_main_option("script_location", ALEMBIC_CONFIG) with notify_api.app_context(): - # TODO this next line shouldn't be needed, - # but cannot work out the import order to - # remove it. - db.create_all() upgrade(config, 'head') def teardown(): From a893a41cfe369687c5f133f43391f1fadefea50c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 12 Jan 2016 10:39:09 +0000 Subject: [PATCH 10/12] Add db creation for travis --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 0e1b69fee..7e2e159c5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,8 +2,12 @@ sudo: false language: python python: - '3.4' +addons: + postgresql: '9.3' install: - pip install -r requirements_for_test.txt +before_script: +- psql -c 'create database test_notification_api;' -U postgres script: - ./scripts/run_tests.sh env: From 31f10c74573d3d68a68fe7d29f52a740459b25c3 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 12 Jan 2016 10:39:49 +0000 Subject: [PATCH 11/12] Added delete endpoint and tests. --- app/dao/services_dao.py | 5 ++++ app/dao/users_dao.py | 5 ++++ app/service/views/rest.py | 36 ++++++++++++++++------------ app/user/views/rest.py | 30 +++++++++++++---------- tests/app/dao/test_services_dao.py | 8 ++++++- tests/app/dao/test_users_dao.py | 9 ++++++- tests/app/service/views/test_rest.py | 12 ++++++++++ tests/app/user/views/test_rest.py | 12 ++++++++++ 8 files changed, 88 insertions(+), 29 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index fafe68c8f..54fb5dd01 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -30,6 +30,11 @@ def save_model_service(service, update_dict=None): db.session.commit() +def delete_model_service(service): + db.session.delete(service) + db.session.commit() + + def get_model_services(service_id=None, user_id=None): # TODO need better mapping from function params to sql query. if user_id and service_id: diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index ca4264cca..5b389e6b7 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -15,6 +15,11 @@ def save_model_user(usr, update_dict={}): db.session.commit() +def delete_model_user(user): + db.session.delete(user) + db.session.commit() + + def get_model_users(user_id=None): if user_id: return User.query.filter_by(id=user_id).one() diff --git a/app/service/views/rest.py b/app/service/views/rest.py index 4f5567f56..c31d7146c 100644 --- a/app/service/views/rest.py +++ b/app/service/views/rest.py @@ -1,7 +1,8 @@ from flask import (jsonify, request) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.services_dao import (save_model_service, get_model_services) +from app.dao.services_dao import ( + save_model_service, get_model_services, delete_model_service) from app.dao.users_dao import get_model_users from app.dao import DAOException from .. import service @@ -26,7 +27,7 @@ def create_service(): # TODO auth to be added -@service.route('/', methods=['PUT']) +@service.route('/', methods=['PUT', 'DELETE']) def update_service(service_id): try: service = get_model_services(service_id=service_id) @@ -34,19 +35,24 @@ def update_service(service_id): return jsonify(result="error", message="Invalid service id"), 400 except NoResultFound: return jsonify(result="error", message="Service not found"), 404 - # TODO there has got to be a better way to do the next three lines - upd_serv, errors = service_schema.load(request.get_json()) - if errors: - return jsonify(result="error", message=errors), 400 - update_dict, errors = service_schema.dump(upd_serv) - # TODO FIX ME - # Remove update_service model which is added to db.session - db.session.rollback() - try: - save_model_service(service, update_dict=update_dict) - except DAOException as e: - return jsonify(result="error", message=str(e)), 400 - return jsonify(data=service_schema.dump(service).data) + if request.method == 'DELETE': + status_code = 202 + delete_model_service(service) + else: + status_code = 200 + # TODO there has got to be a better way to do the next three lines + upd_serv, errors = service_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 + update_dict, errors = service_schema.dump(upd_serv) + # TODO FIX ME + # Remove update_service model which is added to db.session + db.session.rollback() + try: + save_model_service(service, update_dict=update_dict) + except DAOException as e: + return jsonify(result="error", message=str(e)), 400 + return jsonify(data=service_schema.dump(service).data), status_code # TODO auth to be added. diff --git a/app/user/views/rest.py b/app/user/views/rest.py index c0eb533ab..a2633d9f7 100644 --- a/app/user/views/rest.py +++ b/app/user/views/rest.py @@ -2,7 +2,8 @@ from flask import (jsonify, request) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from app.dao.services_dao import get_model_services -from app.dao.users_dao import (get_model_users, save_model_user) +from app.dao.users_dao import ( + get_model_users, save_model_user, delete_model_user) from app.schemas import ( user_schema, users_schema, service_schema, services_schema) from .. import user @@ -20,7 +21,7 @@ def create_user(): # TODO auth to be added -@user.route('/', methods=['PUT']) +@user.route('/', methods=['PUT', 'DELETE']) def update_user(user_id): try: user = get_model_users(user_id=user_id) @@ -28,16 +29,21 @@ def update_user(user_id): return jsonify(result="error", message="Invalid user id"), 400 except NoResultFound: return jsonify(result="error", message="User not found"), 404 - # TODO there has got to be a better way to do the next three lines - update_user, errors = user_schema.load(request.get_json()) - if errors: - return jsonify(result="error", message=errors), 400 - update_dict, errors = user_schema.dump(update_user) - # TODO FIX ME - # Remove update_service model which is added to db.session - db.session.rollback() - save_model_user(user, update_dict=update_dict) - return jsonify(data=user_schema.dump(user).data) + if request.method == 'DELETE': + status_code = 202 + delete_model_user(user) + else: + status_code = 200 + # TODO there has got to be a better way to do the next three lines + update_user, errors = user_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 + update_dict, errors = user_schema.dump(update_user) + # TODO FIX ME + # Remove update_service model which is added to db.session + db.session.rollback() + save_model_user(user, update_dict=update_dict) + return jsonify(data=user_schema.dump(user).data), status_code # TODO auth to be added. diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index a6b4abe68..66c377e4d 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1,6 +1,6 @@ import pytest from app.dao.services_dao import ( - save_model_service, get_model_services, DAOException) + save_model_service, get_model_services, DAOException, delete_model_service) from tests.app.conftest import sample_service as create_sample_service from app.models import Service @@ -62,3 +62,9 @@ def test_missing_user_attribute(notify_api, notify_db, notify_db_session): pytest.fail("DAOException not thrown") except DAOException as e: assert "Missing data for required attribute" in str(e) + + +def test_delete_service(notify_api, notify_db, notify_db_session, sample_service): + assert Service.query.count() == 1 + delete_model_service(sample_service) + assert Service.query.count() == 0 diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 443a1cf22..6190ab20b 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -1,6 +1,7 @@ from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.users_dao import (save_model_user, get_model_users) +from app.dao.users_dao import ( + save_model_user, get_model_users, delete_model_user) from tests.app.conftest import sample_user as create_sample_user from app.models import User @@ -47,3 +48,9 @@ def test_get_user_invalid_id(notify_api, notify_db, notify_db_session): pytest.fail("DataError exception not thrown.") except DataError: pass + + +def test_delete_users(notify_api, notify_db, notify_db_session, sample_user): + assert User.query.count() == 1 + delete_model_user(sample_user) + assert User.query.count() == 0 diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index 2e0cd3e8a..450b7979a 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -209,3 +209,15 @@ def test_put_service_remove_user(notify_api, notify_db, notify_db_session, sampl assert len(json_resp['data']['users']) == 1 assert sample_user.id not in json_resp['data']['users'] assert another_user.id in json_resp['data']['users'] + + +def test_delete_user(notify_api, notify_db, notify_db_session, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + service = Service.query.first() + resp = client.delete( + url_for('service.update_service', service_id=service.id), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 202 + json_resp = json.loads(resp.get_data(as_text=True)) + assert Service.query.count() == 0 diff --git a/tests/app/user/views/test_rest.py b/tests/app/user/views/test_rest.py index 9b6e7ac6f..f59804c96 100644 --- a/tests/app/user/views/test_rest.py +++ b/tests/app/user/views/test_rest.py @@ -195,3 +195,15 @@ def test_get_user_service_service_not_exists(notify_api, notify_db, notify_db_se assert resp.status_code == 404 json_resp = json.loads(resp.get_data(as_text=True)) assert "Service not found" in json_resp['message'] + + +def test_delete_user(notify_api, notify_db, notify_db_session, sample_user): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + user = User.query.first() + resp = client.delete( + url_for('user.update_user', user_id=user.id), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 202 + json_resp = json.loads(resp.get_data(as_text=True)) + assert User.query.count() == 0 From 81cd230a79696ec4d6045a310170d9de09ea2801 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 12 Jan 2016 10:59:27 +0000 Subject: [PATCH 12/12] More tests added. --- tests/app/service/views/test_rest.py | 42 +++++++++++++++++++++++++++- tests/app/user/views/test_rest.py | 39 ++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/tests/app/service/views/test_rest.py b/tests/app/service/views/test_rest.py index 450b7979a..3519e9f7d 100644 --- a/tests/app/service/views/test_rest.py +++ b/tests/app/service/views/test_rest.py @@ -142,6 +142,29 @@ def test_put_service(notify_api, notify_db, notify_db_session, sample_service): assert updated_service.name == new_name +def test_put_service_not_exists(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests PUT endpoint '/' service doesn't exist. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + sample_user = sample_service.users[0] + new_name = 'updated service' + data = { + 'name': new_name, + 'users': [sample_user.id], + 'limit': 1000, + 'restricted': False, + 'active': False} + resp = client.put( + url_for('service.update_service', service_id="123"), + data=data, + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 404 + assert Service.query.first().name == sample_service.name + assert Service.query.first().name != new_name + + def test_put_service_add_user(notify_api, notify_db, notify_db_session, sample_service): """ Tests PUT endpoint '/' add user to the service. @@ -211,7 +234,10 @@ def test_put_service_remove_user(notify_api, notify_db, notify_db_session, sampl assert another_user.id in json_resp['data']['users'] -def test_delete_user(notify_api, notify_db, notify_db_session, sample_service): +def test_delete_service(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests DELETE endpoint '/' delete service. + """ with notify_api.test_request_context(): with notify_api.test_client() as client: service = Service.query.first() @@ -220,4 +246,18 @@ def test_delete_user(notify_api, notify_db, notify_db_session, sample_service): headers=[('Content-Type', 'application/json')]) assert resp.status_code == 202 json_resp = json.loads(resp.get_data(as_text=True)) + json_resp['data']['name'] == sample_service.name assert Service.query.count() == 0 + + +def test_delete_service_not_exists(notify_api, notify_db, notify_db_session, sample_service): + """ + Tests DELETE endpoint '/' delete service doesn't exist. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + resp = client.delete( + url_for('service.update_service', service_id="123"), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 404 + assert Service.query.count() == 1 diff --git a/tests/app/user/views/test_rest.py b/tests/app/user/views/test_rest.py index f59804c96..43cf08949 100644 --- a/tests/app/user/views/test_rest.py +++ b/tests/app/user/views/test_rest.py @@ -97,6 +97,28 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_user): assert json_resp['data']['id'] == user.id +def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests PUT endpoint '/' to update a user doesn't exist. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert User.query.count() == 1 + new_email = 'new@digital.cabinet-office.gov.uk' + data = { + 'email_address': new_email} + headers = [('Content-Type', 'application/json')] + resp = client.put( + url_for('user.update_user', user_id="123"), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 404 + assert User.query.count() == 1 + user = User.query.first() + json_resp = json.loads(resp.get_data(as_text=True)) + assert user.email_address != new_email + + def test_put_user_missing_email(notify_api, notify_db, notify_db_session, sample_user): """ Tests PUT endpoint '/' missing attribute email. @@ -198,6 +220,9 @@ def test_get_user_service_service_not_exists(notify_api, notify_db, notify_db_se def test_delete_user(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests DELETE endpoint '/' delete user. + """ with notify_api.test_request_context(): with notify_api.test_client() as client: user = User.query.first() @@ -207,3 +232,17 @@ def test_delete_user(notify_api, notify_db, notify_db_session, sample_user): assert resp.status_code == 202 json_resp = json.loads(resp.get_data(as_text=True)) assert User.query.count() == 0 + + +def test_delete_user_not_exists(notify_api, notify_db, notify_db_session, sample_user): + """ + Tests DELETE endpoint '/' delete user. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + user = User.query.first() + resp = client.delete( + url_for('user.update_user', user_id="123"), + headers=[('Content-Type', 'application/json')]) + assert resp.status_code == 404 + assert User.query.count() == 1