From 8e2e8d7f2d4926f9b85fc13ff20e48c5b0560c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Sat, 11 Nov 2017 15:22:22 +0100 Subject: [PATCH 01/11] Resources: Assistants are now a list in lectures, added selections and payments --- Backend/security.py | 20 ++++-- Backend/settings.py | 108 ++++++++++++++++++++++------- Backend/tests/conftest.py | 40 +++++++++++ Backend/tests/test_resources.py | 53 ++++++--------- Backend/tests/test_security.py | 116 +++++++++++++++++--------------- 5 files changed, 217 insertions(+), 120 deletions(-) diff --git a/Backend/security.py b/Backend/security.py index bbfc6f5..326b9d7 100644 --- a/Backend/security.py +++ b/Backend/security.py @@ -27,7 +27,7 @@ import json import requests from eve.auth import TokenAuth from eve.io.mongo import Validator -from flask import request, g, current_app +from flask import request, g, current_app, abort # Requests to AMIVAPI @@ -112,16 +112,26 @@ class APIAuth(TokenAuth): Furthermore, grant admin rights if the user is member of the admin group. + + By default, Eve only returns 401, we refine this a little: + - 401 if token missing (eve default already) or not found in AMIVAPI + - 403 if not permitted + """ g.token = token # Safe in g such that other methods can use it - # Valid Login always required + # Return 401 if token not recognized by AMIVAPI if get_user() is None: return False - # Read always allowed - # Write any resource but 'signups': you need to be an admin - return method == 'GET' or (resource == 'signups') or is_admin() + # Check Permissions, return 403 if not permitted + # Users: Read always allowed, write only on specific resources + # Admins: Can do all + user_writable = ['signups', 'selections', 'payments'] + if (method == 'GET' or (resource in user_writable) or is_admin()): + return True + else: + abort(403) # Dynamic Filter diff --git a/Backend/settings.py b/Backend/settings.py index f1e6764..0d2326d 100644 --- a/Backend/settings.py +++ b/Backend/settings.py @@ -2,6 +2,9 @@ Check out [the Eve docs for configuration](http://python-eve.org/config.html) if you are unsure about some of the settings. + +Several validation rules are still missing, they are marked with TODO in the +schema directly. """ from os import environ @@ -28,7 +31,7 @@ DATE_FORMAT = "%Y-%m-%dT%H:%M:%SZ" # A schema for required start/end time tuple -TIMESPAN = { +TIMESPAN_SCHEMA = { 'type': 'dict', 'schema': { 'start': { @@ -45,6 +48,10 @@ TIMESPAN = { } +# Same as Eve, but include 403 +STANDARD_ERRORS = [400, 401, 403, 404, 405, 406, 409, 410, 412, 422, 428] + + # Resources DOMAIN = { 'lectures': { @@ -69,25 +76,21 @@ DOMAIN = { 'max': 3, 'required': True }, + 'assistants': { + # List of nethz of assistants + 'type': 'list', + 'schema': { + 'type': 'string', + 'maxlength': 10, + 'empty': False, + 'nullable': False, + } + # TODO: Not the same nethz twice + # TODO: nethz is enough? + } }, }, - 'assistants': { - 'schema': { - 'nethz': { - 'type': 'string', - 'maxlength': 10, - 'unique': True, - 'empty': False, - 'nullable': False, - 'required': True, - }, - 'name': { - 'type': 'string', - 'readonly': True, - }, - }, - }, 'courses': { 'schema': { 'lecture': { @@ -100,19 +103,16 @@ DOMAIN = { 'not_patchable': True, # Course is tied to lecture }, 'assistant': { - 'type': 'objectid', - 'data_relation': { - 'resource': 'assistants', - 'field': '_id', - 'embeddable': True - }, + 'type': 'string' + # TODO: Assistant needs to exist for lecture }, - 'signup': TIMESPAN, + 'signup': TIMESPAN_SCHEMA, 'datetimes': { 'type': 'list', - 'schema': TIMESPAN, + 'schema': TIMESPAN_SCHEMA, + # TODO: Timeslots must not overlap }, 'room': { 'type': 'string', @@ -121,6 +121,7 @@ DOMAIN = { 'required': True, 'nullable': False, 'empty': False, + # TODO: Room must be empty for time slot }, 'spots': { 'type': 'integer', @@ -132,6 +133,8 @@ DOMAIN = { }, 'signups': { + # Signup for a user to a course + 'schema': { 'nethz': { 'type': 'string', @@ -150,12 +153,65 @@ DOMAIN = { 'embeddable': True }, 'unique_combination': ['nethz'], + # TODO: No overlapping courses }, 'status': { 'type': 'string', - 'allowed': ['waiting', 'accepted', 'accepted+paid'], + 'allowed': ['waiting', 'reserved', 'accepted'], 'readonly': True, }, }, + }, + + 'selections': { + # Easy way for users to safe their selections before signup is open + # List of selected courses per user + + 'schema': { + 'nethz': { + 'type': 'string', + 'maxlength': 10, + 'empty': False, + 'nullable': False, + 'required': True, + 'only_own_nethz': True, + 'unique': True, + }, + 'courses': { + 'type': 'list', + 'schema': { + 'type': 'objectid', + 'data_relation': { + 'resource': 'courses', + 'field': '_id', + 'embeddable': True + }, + # TODO: No duplicate entries + # TODO: No entries that are already reserved + }, + }, + }, + }, + + 'payments': { + # Dummy endpoint for payments. + # TODO: Implement as soon as PSP is known. + + 'schema': { + 'signups': { + 'type': 'list', + 'schema': { + 'type': 'objectid', + 'data_relation': { + 'resource': 'signups', + 'field': '_id', + 'embeddable': True + }, + # TODO: No duplicate entries + }, + 'required': True, + 'nullable': False, + } + } } } diff --git a/Backend/tests/conftest.py b/Backend/tests/conftest.py index 5e6b089..830ec40 100644 --- a/Backend/tests/conftest.py +++ b/Backend/tests/conftest.py @@ -10,9 +10,11 @@ to allow easier assertion of status_codes and make json handling easier. """ import json +from contextlib import contextmanager import pytest +from flask import g from flask.testing import FlaskClient from app import create_app @@ -37,6 +39,15 @@ class TestClient(FlaskClient): # Set header kwargs['content_type'] = "application/json" + # Add Fake Header if specified in context + try: + auth = g.fake_token + kwargs.setdefault('headers', {})['Authorization'] = auth + except (RuntimeError, AttributeError): + # RuntimeError: No g, AttributeError: key in g is missing + pass # No fake token to set + + # Send the actual request response = super(TestClient, self).open(*args, **kwargs) if assert_status is not None: @@ -54,6 +65,30 @@ def drop_database(application): database.drop_collection(collection) +@contextmanager +def user(self, **kwargs): + """Additional context to fake a user.""" + with self.test_request_context(): + g.user = 'Not None :)' + g.admin = False + + # The test requests will use this header + g.fake_token = 'Token Trolololo' + + # Allow to overwrite settings + for key, value in kwargs.items(): + setattr(g, key, value) + + yield + + +@contextmanager +def admin(self, **kwargs): + """Additional context to fake a user.""" + with self.user(**kwargs): + g.admin = True + yield + @pytest.fixture def app(): @@ -61,5 +96,10 @@ def app(): application = create_app(settings=TEST_SETTINGS) application.test_client_class = TestClient application.client = application.test_client() + + # Using __get__ binds the function to the application instance + application.user = user.__get__(application) + application.admin = admin.__get__(application) + yield application drop_database(application) diff --git a/Backend/tests/test_resources.py b/Backend/tests/test_resources.py index 2fd926a..ebc795f 100644 --- a/Backend/tests/test_resources.py +++ b/Backend/tests/test_resources.py @@ -1,35 +1,22 @@ """Tests for basic requests to all resources as admin.""" -from flask import g - def test_create(app): """Test creating everything as an admin user.""" - with app.test_request_context(): - # Fake a admin user - g.user = 'Not None :)' - g.admin = True - faketoken = {'Authorization': 'Token Trolololo'} - + with app.admin(): lecture = { 'title': "Awesome Lecture", 'department': "itet", 'year': 3, + 'assistants': ['pablo', 'pablone'], } lecture_response = app.client.post('lectures', data=lecture, - headers=faketoken, assert_status=201) - assistant = {'nethz': "Pablo"} - assistant_response = app.client.post('assistants', - data=assistant, - headers=faketoken, - assert_status=201) - course = { 'lecture': lecture_response['_id'], - 'assistant': assistant_response['_id'], + 'assistant': 'pablo', 'room': 'ETZ E 6', 'spots': 30, 'signup': { @@ -46,26 +33,29 @@ def test_create(app): } course_response = app.client.post('courses', data=course, - headers=faketoken, assert_status=201) + selection = { + 'nethz': 'Pablito', + 'courses': [course_response['_id']] + } + app.client.post('selections', data=selection, assert_status=201) + signup = { 'nethz': "Pablito", 'course': course_response['_id'] } - app.client.post('signups', - data=signup, - headers=faketoken, - assert_status=201) + signup_response = app.client.post('signups', + data=signup, + assert_status=201) + + payment = {'signups': [signup_response['_id']]} + app.client.post('payments', data=payment, assert_status=201) + def test_no_double_signup(app): """Users can signup for several courses, but not for any course twice.""" - with app.test_request_context(): - # Fake a admin user - g.user = 'Not None :)' - g.admin = True - faketoken = {'Authorization': 'Token Trolololo'} - + with app.admin(): # Create two fake courses to sign up to first = str(app.data.driver.db['courses'].insert({})) second = str(app.data.driver.db['courses'].insert({})) @@ -77,7 +67,6 @@ def test_no_double_signup(app): } app.client.post('signups', data=signup, - headers=faketoken, assert_status=assert_status) # No Double signup to same course @@ -87,18 +76,14 @@ def test_no_double_signup(app): _signup(second, 201) - def test_no_patch(app): """Test that certain fields cannot be changed. These are: Course->lecture and signup->nethz """ no_patch_error = "this field can not be changed with PATCH" - with app.test_request_context(): - # Fake a admin user - g.user = 'Not None :)' - g.admin = True - headers = {'Authorization': 'Token Trolololo', 'If-Match': 'tag'} + with app.admin(): + headers = {'If-Match': 'tag'} # Create fake resources, make sure to set _etag so we can patch course = str(app.data.driver.db['courses'].insert({'_etag': 'tag'})) diff --git a/Backend/tests/test_security.py b/Backend/tests/test_security.py index e60871e..810b649 100644 --- a/Backend/tests/test_security.py +++ b/Backend/tests/test_security.py @@ -5,16 +5,18 @@ import pytest from flask import g -@pytest.mark.parametrize('resource', - ['lectures', 'assistants', 'courses', 'signups']) +ALL_RESOURCES = ['lectures', 'courses', 'signups', 'selections', 'payments'] +ADMIN_RESOURCES = ['lectures', 'courses'] # only admin can write + + +@pytest.mark.parametrize('resource', ALL_RESOURCES) @pytest.mark.parametrize('method', ['get', 'post']) def test_auth_required_for_resource(app, resource, method): """Without auth header, we get get 401 for all methods.""" getattr(app.client, method)('/' + resource, data={}, assert_status=401) -@pytest.mark.parametrize('resource', - ['lectures', 'assistants', 'courses', 'signups']) +@pytest.mark.parametrize('resource', ALL_RESOURCES) @pytest.mark.parametrize('method', ['get', 'patch', 'delete']) def test_auth_required_for_item(app, resource, method): """Without auth provided, we can access any item either.""" @@ -27,7 +29,7 @@ def test_auth_required_for_item(app, resource, method): assert_status=401) -@pytest.mark.parametrize('resource', ['lectures', 'assistants', 'courses']) +@pytest.mark.parametrize('resource', ADMIN_RESOURCES) def test_user_can_read(app, resource): """Users should be able to to GET requests on resource and item. @@ -49,43 +51,30 @@ def test_user_can_read(app, resource): assert_status=200) -@pytest.mark.parametrize('resource', ['lectures', 'assistants', 'courses']) +@pytest.mark.parametrize('resource', ADMIN_RESOURCES) def test_user_cannot_write(app, resource): """Users cannot create, modify or delete.""" - with app.test_request_context(): - # Fake a user, specify non-admin - g.user = 'Not None :)' - g.admin = False - faketoken = {'Authorization': 'Token Trolololo'} + with app.user(): data = {} - # Post something + # Try to post something app.client.post('/' + resource, data=data, - headers=faketoken, - assert_status=401) + assert_status=403) # Create fake item, try to patch/delete it _id = app.data.driver.db[resource].insert({}) app.client.patch('/%s/%s' % (resource, _id), data=data, - headers=faketoken, - assert_status=401) + assert_status=403) app.client.delete('/%s/%s' % (resource, _id), - headers=faketoken, - assert_status=401) + assert_status=403) def test_signup_with_own_nethz_only(app): """Users can only post singups with their own nethz.""" - with app.test_request_context(): - nethz = 'Something' - # Fake a user, specify non-admin - g.user = 'Not None :)' - g.nethz = nethz - g.admin = False - faketoken = {'Authorization': 'Token Trolololo'} - + nethz = 'Something' + with app.user(nethz=nethz): # Create fake course to sign up to course = str(app.data.driver.db['courses'].insert({})) @@ -96,7 +85,6 @@ def test_signup_with_own_nethz_only(app): } app.client.post('/signups', data=bad_signup, - headers=faketoken, assert_status=422) # Try with own nethz @@ -106,69 +94,87 @@ def test_signup_with_own_nethz_only(app): } app.client.post('/signups', data=good_signup, - headers=faketoken, + assert_status=201) + + +def test_selection_for_own_nethz_only(app): + """Users can only select courses for themselves.""" + nethz = 'Something' + with app.user(nethz=nethz): + # Create fake course to select + course = str(app.data.driver.db['courses'].insert({})) + + # Try with other nethz + bad_selection = { + 'nethz': 'Notthenethz', + 'courses': [course], + } + app.client.post('/selections', + data=bad_selection, + assert_status=422) + + # Try with own nethz + good_selection = { + 'nethz': nethz, + 'courses': [course], + } + app.client.post('/selections', + data=good_selection, assert_status=201) def test_user_signup_visibility(app): """Test that we a user cannot see others' signups.""" - with app.test_request_context(): - # Fake a user, specify non-admin - g.user = 'Not None :)' - g.nethz = 'Something' - g.admin = False - token = {'Authorization': 'Token FakeeTrolololo', 'If-Match': 'Wrong'} - + nethz = 'Something' + with app.user(nethz=nethz): # Create fake signup with different nethz - own = str(app.data.driver.db['signups'].insert({'nethz': g.nethz})) + own = str(app.data.driver.db['signups'].insert({'nethz': nethz})) other = str(app.data.driver.db['signups'].insert({'nethz': 'trolo'})) # Resource: Can only see own, not both signups - response = app.client.get('/signups', headers=token, assert_status=200) + response = app.client.get('/signups', assert_status=200) assert len(response['_items']) == 1 - assert response['_items'][0]['nethz'] == g.nethz + assert response['_items'][0]['nethz'] == nethz # Items own_url = '/signups/' + own other_url = '/signups/' + other # Get - app.client.get(own_url, headers=token, assert_status=200) - app.client.get(other_url, headers=token, assert_status=404) + app.client.get(own_url, assert_status=200) + app.client.get(other_url, assert_status=404) - # Patch (if we can see item, we get 412 since etag is wrong) - app.client.patch(own_url, headers=token, data={}, assert_status=412) - app.client.patch(other_url, headers=token, data={}, assert_status=404) + # Patch (if we can see item, we get 428 since etag is missing) + app.client.patch(own_url, data={}, assert_status=428) + app.client.patch(other_url, data={}, assert_status=404) # Delete (etag missing again) - app.client.delete(own_url, headers=token, assert_status=412) - app.client.delete(other_url, headers=token, assert_status=404) + app.client.delete(own_url, assert_status=428) + app.client.delete(other_url, assert_status=404) def test_admin_signup_visibility(app): """Test that we an admin can see others' signups.""" - with app.test_request_context(): - # Fake a user, specify admin - g.user = 'Not None :)' - g.nethz = 'Nothing special really' - g.admin = True - token = {'Authorization': 'Token FakeeTrolololo', 'If-Match': 'Wrong'} + with app.admin(): + headers = {'If-Match': 'Wrong'} # Create fake signup with different nethz other = str(app.data.driver.db['signups'].insert({'nethz': 'trolo'})) # Resource: Can see signups - response = app.client.get('/signups', headers=token, assert_status=200) + response = app.client.get('/signups', + headers=headers, + assert_status=200) assert len(response['_items']) == 1 # Items url = '/signups/' + other # Get - app.client.get(url, headers=token, assert_status=200) + app.client.get(url, headers=headers, assert_status=200) # Patch (if we can see item, we get 412 since etag is wrong) - app.client.patch(url, headers=token, data={}, assert_status=412) + app.client.patch(url, headers=headers, data={}, assert_status=412) # Delete (etag missing again) - app.client.delete(url, headers=token, assert_status=412) + app.client.delete(url, headers=headers, assert_status=412) -- GitLab From 0950a6ce048720bfd8772fb7c0a371af4811b4fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Sat, 11 Nov 2017 15:46:04 +0100 Subject: [PATCH 02/11] Validation: Put rules in extra file validation.py --- Backend/app.py | 15 +++++----- Backend/security.py | 52 ++------------------------------- Backend/settings.py | 4 ++- Backend/tests/test_security.py | 29 ++++++++++--------- Backend/validation.py | 53 ++++++++++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 70 deletions(-) create mode 100644 Backend/validation.py diff --git a/Backend/app.py b/Backend/app.py index affcc3b..24bf96f 100644 --- a/Backend/app.py +++ b/Backend/app.py @@ -10,16 +10,15 @@ Next, you should check out the following files: The data model and `Eve` configuration. - `security.py`: - Authentication and Data Validation functions that are used in the model are - defined here. In particular, the interaction with AMIVAPI is handled here. - + Authentication is defined here, in particular the interaction with AMIVAPI. """ from os import getcwd from eve import Eve from flask import Config -from security import APIAuth, APIValidator, only_own_signups +from security import APIAuth, only_own_nethz +from validation import APIValidator def create_app(settings=None): @@ -38,9 +37,11 @@ def create_app(settings=None): # Eve provides hooks at several points of the request, # we use this do add dynamic filtering - for method in ['GET', 'PATCH', 'DELETE']: - event = getattr(application, 'on_pre_%s_signups' % method) - event += only_own_signups + for resource in ['signups', 'selections']: + for method in ['GET', 'PATCH', 'DELETE']: + event = getattr(application, + 'on_pre_%s_%s' % (method, resource)) + event += only_own_nethz return application diff --git a/Backend/security.py b/Backend/security.py index 326b9d7..c8e74f4 100644 --- a/Backend/security.py +++ b/Backend/security.py @@ -15,19 +15,13 @@ The important bits are: Take a look at the [Eve docs](http://python-eve.org/authentication.html) for more info. - -- Additional Validation rules - - More info [here](http://python-eve.org/validation.html). - """ from functools import wraps import json import requests from eve.auth import TokenAuth -from eve.io.mongo import Validator -from flask import request, g, current_app, abort +from flask import g, current_app, abort # Requests to AMIVAPI @@ -134,51 +128,11 @@ class APIAuth(TokenAuth): abort(403) -# Dynamic Filter +# Dynamic Visibility Filter -def only_own_signups(_, lookup): +def only_own_nethz(_, lookup): """Users can only see signups if their ID matches.""" if not is_admin(): # Add the additional lookup with an `$and` condition # or extend existing `$and`s lookup.setdefault('$and', []).append({'nethz': get_nethz()}) - - -# Validation - -class APIValidator(Validator): - """Provide a rule to check nethz of current user.""" - - def _validate_only_own_nethz(self, enabled, field, value): - """If the user is no admin, only own nethz is allowed for singup.""" - if enabled and not is_admin(): - if value != get_nethz(): - self._error(field, - "You can only use your own nethz to sign up.") - - def _validate_unique_combination(self, unique_combination, field, value): - """Validate that a combination of fields is unique. - - Code is copy-pasted from amivapi, see there for more explanation. - https://github.com/amiv-eth/amivapi/blob/master/amivapi/utils.py - """ - lookup = {field: value} # self - for other_field in unique_combination: - lookup[other_field] = self.document.get(other_field) - - if request.method == 'PATCH': - original = self._original_document - for key in unique_combination: - if key not in self.document.keys(): - lookup[key] = original[key] - - resource = self.resource - if current_app.data.find_one(resource, None, **lookup) is not None: - self._error(field, "value already exists in the database in " + - "combination with values for: %s" % - unique_combination) - - def _validate_not_patchable(self, enabled, field, _): - """Inhibit patching of the field, also copied from AMIVAPI.""" - if enabled and (request.method == 'PATCH'): - self._error(field, "this field can not be changed with PATCH") diff --git a/Backend/settings.py b/Backend/settings.py index 0d2326d..aefd00b 100644 --- a/Backend/settings.py +++ b/Backend/settings.py @@ -3,7 +3,9 @@ Check out [the Eve docs for configuration](http://python-eve.org/config.html) if you are unsure about some of the settings. -Several validation rules are still missing, they are marked with TODO in the +Our schema requires customized data validation. These validation rules are +implemented in `validation.py`. +Some validation rules are still missing, they are marked with TODO in the schema directly. """ diff --git a/Backend/tests/test_security.py b/Backend/tests/test_security.py index 810b649..16a6684 100644 --- a/Backend/tests/test_security.py +++ b/Backend/tests/test_security.py @@ -7,6 +7,7 @@ from flask import g ALL_RESOURCES = ['lectures', 'courses', 'signups', 'selections', 'payments'] ADMIN_RESOURCES = ['lectures', 'courses'] # only admin can write +PERSONAL_RESOURCES = ['signups', 'selections'] # users can only see their own @pytest.mark.parametrize('resource', ALL_RESOURCES) @@ -123,22 +124,23 @@ def test_selection_for_own_nethz_only(app): assert_status=201) -def test_user_signup_visibility(app): - """Test that we a user cannot see others' signups.""" +@pytest.mark.parametrize('resource', PERSONAL_RESOURCES) +def test_user_visibility(app, resource): + """Test that a user cannot see others' signups or selections.""" nethz = 'Something' with app.user(nethz=nethz): # Create fake signup with different nethz - own = str(app.data.driver.db['signups'].insert({'nethz': nethz})) - other = str(app.data.driver.db['signups'].insert({'nethz': 'trolo'})) + own = str(app.data.driver.db[resource].insert({'nethz': nethz})) + other = str(app.data.driver.db[resource].insert({'nethz': 'trolo'})) # Resource: Can only see own, not both signups - response = app.client.get('/signups', assert_status=200) + response = app.client.get('/' + resource, assert_status=200) assert len(response['_items']) == 1 assert response['_items'][0]['nethz'] == nethz # Items - own_url = '/signups/' + own - other_url = '/signups/' + other + own_url = '/%s/%s' % (resource, own) + other_url = '/%s/%s' % (resource, other) # Get app.client.get(own_url, assert_status=200) @@ -153,22 +155,23 @@ def test_user_signup_visibility(app): app.client.delete(other_url, assert_status=404) -def test_admin_signup_visibility(app): - """Test that we an admin can see others' signups.""" - with app.admin(): +@pytest.mark.parametrize('resource', PERSONAL_RESOURCES) +def test_admin_signup_visibility(app, resource): + """Test that we an admin can see others' signups and selections.""" + with app.admin(nethz='somethingsomething'): headers = {'If-Match': 'Wrong'} # Create fake signup with different nethz - other = str(app.data.driver.db['signups'].insert({'nethz': 'trolo'})) + other = str(app.data.driver.db[resource].insert({'nethz': 'trolo'})) # Resource: Can see signups - response = app.client.get('/signups', + response = app.client.get('/' + resource, headers=headers, assert_status=200) assert len(response['_items']) == 1 # Items - url = '/signups/' + other + url = '/%s/%s' % (resource, other) # Get app.client.get(url, headers=headers, assert_status=200) diff --git a/Backend/validation.py b/Backend/validation.py new file mode 100644 index 0000000..2174dcf --- /dev/null +++ b/Backend/validation.py @@ -0,0 +1,53 @@ +"""Custom Validators + +We require several custom validation rules, e.g. that user can only use their +own nethz. + +Learn how validation works [here](http://python-eve.org/validation.html). + +TODO: Several validation rules still need to be implemented. See `settings.py`. + +""" + +from flask import request, current_app +from eve.io.mongo import Validator + +from security import is_admin, get_nethz + + +class APIValidator(Validator): + """Provide a rule to check nethz of current user.""" + + def _validate_only_own_nethz(self, enabled, field, value): + """If the user is no admin, only own nethz is allowed for singup.""" + if enabled and not is_admin(): + if value != get_nethz(): + self._error(field, + "You can only use your own nethz to sign up.") + + def _validate_unique_combination(self, unique_combination, field, value): + """Validate that a combination of fields is unique. + + Code is copy-pasted from amivapi, see there for more explanation. + https://github.com/amiv-eth/amivapi/blob/master/amivapi/utils.py + """ + lookup = {field: value} # self + for other_field in unique_combination: + lookup[other_field] = self.document.get(other_field) + + if request.method == 'PATCH': + original = self._original_document + for key in unique_combination: + if key not in self.document.keys(): + lookup[key] = original[key] + + resource = self.resource + if current_app.data.find_one(resource, None, **lookup) is not None: + self._error(field, "value already exists in the database in " + + "combination with values for: %s" % + unique_combination) + + def _validate_not_patchable(self, enabled, field, value): + """Inhibit patching of the field, also copied from AMIVAPI.""" + if enabled and (request.method == 'PATCH'): + self._error(field, "this field can not be changed with PATCH") -- GitLab From e0b14f257cce17de1a7c989ad6e2ccb485f3e1f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Wed, 15 Nov 2017 14:14:47 +0100 Subject: [PATCH 03/11] Signups: add additional checks and payments --- Backend/app.py | 22 +++ Backend/security.py | 13 +- Backend/settings.py | 23 +++ Backend/signups.py | 132 +++++++++++++++++ Backend/tests/conftest.py | 3 +- Backend/tests/test_signups.py | 259 ++++++++++++++++++++++++++++++++++ 6 files changed, 445 insertions(+), 7 deletions(-) create mode 100644 Backend/signups.py create mode 100644 Backend/tests/test_signups.py diff --git a/Backend/app.py b/Backend/app.py index 24bf96f..0dcdb8d 100644 --- a/Backend/app.py +++ b/Backend/app.py @@ -11,6 +11,10 @@ Next, you should check out the following files: - `security.py`: Authentication is defined here, in particular the interaction with AMIVAPI. + +- `signups.py`: + Processing of signups: waiting list, payments, etc. + """ from os import getcwd @@ -19,6 +23,14 @@ from flask import Config from security import APIAuth, only_own_nethz from validation import APIValidator +from signups import ( + new_signups, + deleted_signup, + patched_signup, + patched_course, + block_course_deletion, + mark_as_paid, +) def create_app(settings=None): @@ -43,6 +55,16 @@ def create_app(settings=None): 'on_pre_%s_%s' % (method, resource)) event += only_own_nethz + # Also use hooks to add pre- and postprocessing to resources + application.on_post_POST_signups += new_signups + application.on_deleted_item_signups += deleted_signup + application.on_updated_signups += patched_signup + + application.on_updated_courses += patched_course + application.on_delete_item_courses += block_course_deletion + + application.on_inserted_payments += mark_as_paid + return application diff --git a/Backend/security.py b/Backend/security.py index c8e74f4..de820ad 100644 --- a/Backend/security.py +++ b/Backend/security.py @@ -47,7 +47,7 @@ def request_cache(key): try: # If the value is already in g, don't call function return getattr(g, key) - except KeyError: + except AttributeError: setattr(g, key, function(*args, **kwargs)) return getattr(g, key) return _wrapper @@ -118,11 +118,12 @@ class APIAuth(TokenAuth): if get_user() is None: return False - # Check Permissions, return 403 if not permitted - # Users: Read always allowed, write only on specific resources - # Admins: Can do all - user_writable = ['signups', 'selections', 'payments'] - if (method == 'GET' or (resource in user_writable) or is_admin()): + # Check permitted methods for users, return 403 if not permitted + # Admins can do everything + domain = current_app.config['DOMAIN'] + allowed_methods = domain[resource].get('user_methods', []) + + if (method in allowed_methods or is_admin()): return True else: abort(403) diff --git a/Backend/settings.py b/Backend/settings.py index aefd00b..0a53abe 100644 --- a/Backend/settings.py +++ b/Backend/settings.py @@ -23,6 +23,9 @@ MONGO_USERNAME = 'pvkuser' MONGO_PASSWORD = 'pvkpass' MONGO_DBNAME = 'pvk' +# Only JSON, simplifies hooks +XML = False + RESOURCE_METHODS = ['GET', 'POST'] ITEM_METHODS = ['GET', 'PATCH', 'DELETE'] @@ -32,6 +35,10 @@ ITEM_METHODS = ['GET', 'PATCH', 'DELETE'] DATE_FORMAT = "%Y-%m-%dT%H:%M:%SZ" +# More Feedback when creating something: Return all fields +BANDWIDTH_SAVER = False + + # A schema for required start/end time tuple TIMESPAN_SCHEMA = { 'type': 'dict', @@ -57,6 +64,9 @@ STANDARD_ERRORS = [400, 401, 403, 404, 405, 406, 409, 410, 412, 422, 428] # Resources DOMAIN = { 'lectures': { + + 'user_methods': ['GET'], + 'schema': { 'title': { 'type': 'string', @@ -94,6 +104,9 @@ DOMAIN = { }, 'courses': { + + 'user_methods': ['GET'], + 'schema': { 'lecture': { 'type': 'objectid', @@ -137,6 +150,8 @@ DOMAIN = { 'signups': { # Signup for a user to a course + 'user_methods': ['GET', 'POST', 'PATCH', 'DELETE'], + 'schema': { 'nethz': { 'type': 'string', @@ -155,12 +170,14 @@ DOMAIN = { 'embeddable': True }, 'unique_combination': ['nethz'], + 'required': True, # TODO: No overlapping courses }, 'status': { 'type': 'string', 'allowed': ['waiting', 'reserved', 'accepted'], 'readonly': True, + 'default': 'waiting', }, }, }, @@ -169,6 +186,8 @@ DOMAIN = { # Easy way for users to safe their selections before signup is open # List of selected courses per user + 'user_methods': ['GET', 'POST', 'PATCH', 'DELETE'], + 'schema': { 'nethz': { 'type': 'string', @@ -199,6 +218,9 @@ DOMAIN = { # Dummy endpoint for payments. # TODO: Implement as soon as PSP is known. + # Only admins can delete payments + 'user_methods': ['GET', 'POST', 'PATCH'], + 'schema': { 'signups': { 'type': 'list', @@ -210,6 +232,7 @@ DOMAIN = { 'embeddable': True }, # TODO: No duplicate entries + # TODO: No courses on waiting list }, 'required': True, 'nullable': False, diff --git a/Backend/signups.py b/Backend/signups.py new file mode 100644 index 0000000..cf2d177 --- /dev/null +++ b/Backend/signups.py @@ -0,0 +1,132 @@ +"""Signup processing (Waiting list and payments). + +So far only dummy functionality, i.e. if a payment is posted, all courses +are set to accepted. + +As soon as we have payment service provider, the definite functionality needs +to be implemented. + +TODO: Send notification mails +""" + +import json +from functools import wraps +from itertools import chain + +from flask import current_app, abort +from eve.methods.get import getitem_internal +from eve.methods.patch import patch_internal + + +def wrap_response(f): + """Wrapper to modify payload for successful requests (status 2xx).""" + @wraps(f) + def wrapped(request, response): + if response.status_code // 100 == 2: + payload = json.loads(response.get_data(as_text=True)) + + if '_items' in payload: + f(payload['_items']) + else: + f([payload]) + + response.set_data(json.dumps(payload)) + return wrapped + + +@wrap_response +def new_signups(signups): + """Update the status for all signups to a course.""" + # Remove duplicates by using a set + courses = set(item['course'] for item in signups) + # Re-format signups into a dict so we can update them easier later + signups_by_id = {str(item['_id']): item for item in signups} + + modified = chain.from_iterable(update_signups(course) + for course in courses) + + for _id in modified: + # Update response payload if needed + try: + signups_by_id[_id]['status'] = 'reserved' + except AttributeError: + pass # Not in response, nothing to do + + +def deleted_signup(signup): + """Update status of course the signup belonged to.""" + update_signups(signup['course']) + + +def patched_signup(update, original): + """Update status of new course.""" + if 'course' in update: + update_signups(str(update['course'])) + + +def patched_course(update, original): + """If the number of spots changed, update signups of course.""" + if 'spots' in update: + update_signups(str(original['_id'])) + + +def block_course_deletion(course): + """If a course has signups, it can't be deleted.""" + count = current_app.data.driver.db['signups'].count({ + 'course': str(course['_id']) + }) + + if count > 0: + abort(409, "Course cannot be deleted as long as it has signups.") + + +def update_signups(course): + """Update waiting list for all provided courses. + + Return list of ids of all signups with modified status. + """ + # If the course is embedded, we already have the data we need + course_data = getitem_internal('courses', _id=str(course))[0] + course_id = course_data['_id'] + total_spots = course_data.get('spots', 0) + + # Next, count current signups not on waiting list + collection = current_app.data.driver.db['signups'] + taken_spots = collection.count({'status': {'$ne': 'waiting'}}) + + available_spots = total_spots - taken_spots + + if available_spots <= 0: + return [] + + # Finally, get as many signups on the waiting list as spots available + # sort by _updated, use nethz as tie breaker + signups = collection.find({'course': course_id, 'status': 'waiting'}, + projection=['_id', 'status'], + sort=[('_updated', 1), ('nethz', 1)], + limit=available_spots) + + signups_to_update = [str(item['_id']) for item in signups + if item['status'] == 'waiting'] + + for signup_id in signups_to_update: + patch_internal('signups', + _id=signup_id, + payload={'status': 'reserved'}, + concurrency_check=False, + skip_validation=True) + + return signups_to_update + + +def mark_as_paid(payments): + """After successful payment, set status to `accepted`.""" + for payment in payments: + + for signup in payment['signups']: + data = {'status': 'accepted'} + patch_internal('signups', + _id=str(signup), + payload=data, + concurrency_check=False, + skip_validation=True) diff --git a/Backend/tests/conftest.py b/Backend/tests/conftest.py index 830ec40..9375a7e 100644 --- a/Backend/tests/conftest.py +++ b/Backend/tests/conftest.py @@ -54,7 +54,7 @@ class TestClient(FlaskClient): assert response.status_code == assert_status, \ response.get_data(as_text=True) - return json.loads(response.get_data(as_text=True)) + return json.loads(response.get_data(as_text=True) or '{}') def drop_database(application): @@ -70,6 +70,7 @@ def user(self, **kwargs): """Additional context to fake a user.""" with self.test_request_context(): g.user = 'Not None :)' + g.nethz = 'Something' g.admin = False # The test requests will use this header diff --git a/Backend/tests/test_signups.py b/Backend/tests/test_signups.py new file mode 100644 index 0000000..5ad00b0 --- /dev/null +++ b/Backend/tests/test_signups.py @@ -0,0 +1,259 @@ +"""Test for signup processing, in particular waiting list.""" + +from unittest.mock import patch, call +import pytest + +from datetime import datetime as dt + +from signups import update_signups + + +def test_success(app): + """If there are enough spots, the status will be 'reserved'.""" + with app.admin(): # Admin so we don't need to care about nethz + # Create fake courses to sign up to + course_id = str(app.data.driver.db['courses'].insert({'spots': 10})) + + signup = { + 'nethz': 'Something', + 'course': course_id, + } + + signup_response = app.client.post('/signups', + data=signup, + assert_status=201) + + assert signup_response['status'] == 'reserved' + + # Now pay + payment = { + 'signups': [signup_response['_id']] + } + app.client.post('/payments', + data=payment, + assert_status=201) + + # Check signup + updated_signup = app.client.get('/signups/' + signup_response['_id'], + assert_status=200) + + assert updated_signup['status'] == 'accepted' + + +def test_zero_spots(app): + """Settings spots to zero will just put everyone on the waiting list.""" + with app.admin(): + # Create fake courses to sign up to + course_id = str(app.data.driver.db['courses'].insert({'spots': 0})) + + signup = { + 'nethz': 'Something', + 'course': course_id, + } + + signup_response = app.client.post('/signups', + data=signup, + assert_status=201) + + assert signup_response['status'] == 'waiting' + + +def test_not_enough_spots(app): + """If there are not enough spots, signups go to waiting list.""" + with app.admin(): + # Create fake courses to sign up to + course_id = str(app.data.driver.db['courses'].insert({'spots': 1})) + + first = { + 'nethz': 'Something', + 'course': course_id, + } + + first_response = app.client.post('/signups', + data=first, + assert_status=201) + + print(first_response['_updated']) + + assert first_response['status'] == 'reserved' + + second = { + 'nethz': 'Otherthing', + 'course': course_id, + } + + second_response = app.client.post('/signups', + data=second, + assert_status=201) + + print(second_response['_updated']) + + assert second_response['status'] == 'waiting' + + +def test_update_spots(app): + """Test the main update function. + + As a key for sorting the _updated timestamp will be used, with + nethz as a tie breaker + """ + with app.admin(): + # Create a course with two spots + course = app.data.driver.db['courses'].insert({'spots': 2}) + + # Create four signups on waiting list + # 1. Oldest _created timestamp, but recently modified (recent _updated) + first_data = { + 'course': course, + 'status': 'waiting', + '_created': dt(2020, 10, 10), + '_updated': dt(2020, 10, 20), + } + first_id = str(app.data.driver.db['signups'].insert(first_data)) + + # 3. oldest _updated + second_data = { + 'course': course, + 'status': 'waiting', + '_created': dt(2020, 10, 11), + '_updated': dt(2020, 10, 11), + } + second_id = str(app.data.driver.db['signups'].insert(second_data)) + + # 3. earlier _created, second oldest _updated + third_data = { + 'course': course, + 'status': 'waiting', + '_created': dt(2020, 10, 12), + '_updated': dt(2020, 10, 15), + 'nethz': 'abc' + } + third_id = str(app.data.driver.db['signups'].insert(third_data)) + + # 4. Same updated time as 3, but different id that will loose tie + fourth_data = { + 'course': course, + 'status': 'waiting', + '_created': dt(2020, 10, 13), + '_updated': dt(2020, 10, 15), + 'nethz': 'bcd' + } + fourth_id = str(app.data.driver.db['signups'].insert(fourth_data)) + + # Do the update! + # We except 2 and 3 to get spots + update_signups(str(course)) + + def status(_id): + return app.client.get('/signups/' + _id, + assert_status=200)['status'] + + assert status(first_id) == 'waiting' + assert status(second_id) == 'reserved' + assert status(third_id) == 'reserved' + assert status(fourth_id) == 'waiting' + + +@pytest.fixture +def course(app): + """Create a fake course without any data for a test.""" + with app.admin(): + # Create a few courses to sign up to + db = app.data.driver.db['courses'] + yield str(db.insert({'_etag': 'tag'})) + + +@pytest.fixture +def mock_update(): + """Mock the actual updating of spots for a test.""" + with patch('signups.update_signups', return_value=[]) as update: + yield update + + +def test_post_signups_triggers_update(app, course, mock_update): + """Test the the update of spots gets triggered correctly.""" + data = { + 'course': course, + 'nethz': 'bli' + } + app.client.post('signups', data=data, assert_status=201) + mock_update.assert_called_with(course) + + +def test_batch_post_signups_triggers_update(app, course, mock_update): + """Test the the update of spots gets triggered correctly.""" + # We need a second course to test everything + other_course = str(app.data.driver.db['courses'].insert({})) + + batch = [{ + 'course': course, + 'nethz': 'bla' + }, { + 'course': course, + 'nethz': 'bli' + }, { + 'course': other_course, + 'nethz': 'bli' + }] + app.client.post('/signups', data=batch, assert_status=201) + # Same course doesn't get updated twice per request + mock_update.assert_has_calls([call(course), call(other_course)], + any_order=True) + + +def test_patch_signup_triggers_update(app, course, mock_update): + """Test the the update of spots gets triggered correctly.""" + fake = str(app.data.driver.db['signups'].insert({ + '_etag': 'tag', + 'nethz': 'lala', + })) + app.client.patch('/signups/' + fake, + data={'course': course}, + headers={'If-Match': 'tag'}, + assert_status=200) + mock_update.assert_called_with(course) + + +def test_delete_signup_triggers_update(app, course, mock_update): + """Test the the update of spots gets triggered correctly.""" + fake = str(app.data.driver.db['signups'].insert({ + 'course': course, + '_etag': 'tag' + })) + app.client.delete('/signups/' + fake, + headers={'If-Match': 'tag'}, + assert_status=204) + mock_update.assert_called_with(course) + + +def test_patch_course_without_update(app, course, mock_update): + """Update of spots gets only triggered if number of spots changes.""" + app.client.patch('/courses/' + course, + data={'room': 'Something'}, + headers={'If-Match': 'tag'}, + assert_status=200) + mock_update.assert_not_called() + + +def test_patch_course_with_update(app, course, mock_update): + """Update of spots gets only triggered if number of spots changes.""" + app.client.patch('/courses/' + course, + data={'spots': '10'}, + headers={'If-Match': 'tag'}, + assert_status=200) + mock_update.assert_called_with(course) + + +def test_block_delete_ok(app, course): + """If a course has no signups, it can be deleted.""" + app.client.delete('/courses/' + course, + headers={'If-Match': 'tag'}, + assert_status=204) + + +def test_block_delete_blocked(app, course): + """If a course has signups, it cannot be deleted.""" + str(app.data.driver.db['signups'].insert({'course': course})) + app.client.delete('/courses/' + course, + headers={'If-Match': 'tag'}, + assert_status=409) -- GitLab From 93d5adcc9620980708b9c48bd98531cc21260575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Wed, 15 Nov 2017 14:19:13 +0100 Subject: [PATCH 04/11] Settings: allow overwriting mongo connection info with env vars --- Backend/settings.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Backend/settings.py b/Backend/settings.py index 0a53abe..7fc583e 100644 --- a/Backend/settings.py +++ b/Backend/settings.py @@ -18,10 +18,10 @@ ADMIN_GROUP_NAME = 'PVK Admins' # DB MONGO_HOST = environ.get('MONGO_HOST', 'localhost') -MONGO_PORT = 27017 -MONGO_USERNAME = 'pvkuser' -MONGO_PASSWORD = 'pvkpass' -MONGO_DBNAME = 'pvk' +MONGO_PORT = environ.get('MONGO_PORT', 27017) +MONGO_DBNAME = environ.get('MONGO_DBNAME', 'pvk') +MONGO_USERNAME = environ.get('MONGO_USERNAME', 'pvkuser') +MONGO_PASSWORD = environ.get('MONGO_PASSWORD', 'pvkpass') # Only JSON, simplifies hooks XML = False @@ -98,7 +98,7 @@ DOMAIN = { 'nullable': False, } # TODO: Not the same nethz twice - # TODO: nethz is enough? + # TODO: Is nethz enough here? } }, }, -- GitLab From c77c2277bd2bc809fbc308d5e4a4d89f9c317f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 16 Nov 2017 02:33:16 +0100 Subject: [PATCH 05/11] Add a demo dev server for easier frontend development. --- Backend/Dockerfile | 22 ++++++ Backend/demo_server.py | 164 +++++++++++++++++++++++++++++++++++++++++ README.md | 50 ++++++++++++- 3 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 Backend/Dockerfile create mode 100644 Backend/demo_server.py diff --git a/Backend/Dockerfile b/Backend/Dockerfile new file mode 100644 index 0000000..823e566 --- /dev/null +++ b/Backend/Dockerfile @@ -0,0 +1,22 @@ +from python:3.6-alpine + +ENV FLASK_APP app.py +ENV FLASK_DEBUG true +EXPOSE 80 + +COPY ./requirements.txt /requirements.txt +RUN pip install -r requirements.txt + +COPY ./app.py /app.py +COPY ./settings.py /settings.py +COPY ./security.py /security.py +COPY ./signups.py /signups.py +COPY ./validation.py /validation.py + +COPY ./demo_server.py /demo_server.py + +WORKDIR / + +# Run database init before starting dev server +ENTRYPOINT ["python"] +CMD ["demo_server.py"] diff --git a/Backend/demo_server.py b/Backend/demo_server.py new file mode 100644 index 0000000..16a08e0 --- /dev/null +++ b/Backend/demo_server.py @@ -0,0 +1,164 @@ +"""Demo Server. + +Add some demo data to the database and run the app. +""" + +from os import getenv +from random import randint +from datetime import datetime as dt, timedelta +import json + +from flask import g + +from app import create_app + + +HOST = getenv('PVK_HOST', '0.0.0.0') +PORT = getenv('PVK_PORT', '80') + +APP = create_app() +CLIENT = APP.test_client() +DATE_FORMAT = APP.config['DATE_FORMAT'] +ASSISTANTS = ['pablo', 'assi', 'anon', 'mongo'] +MIN_SPOTS = 20 +MAX_SPOTS = 40 + +""" +# Clear everything +from pymongo import MongoClient + +connection = MongoClient(APP.config['MONGO_HOST'], + APP.config['MONGO_PORT']) +connection.drop_database(APP.config['MONGO_DBNAME']) +connection.close() +""" + + +def post(resource, data): + """Create something, ignoring auth.""" + with APP.test_request_context(): + g.user = 'Not None :)' + g.nethz = 'Something' + g.admin = True + + response = CLIENT.post(resource, + data=json.dumps(data), + content_type="application/json", + headers={'Authorization': 'Token Lala'}) + + if response.status_code != 201: + """ + error = json.loads(response.get_data(as_text=True)) + status = error['_error']['code'] + issues = str(error['_issues']) + print('Warning(%s):' % status, issues) + """ + return {} + + return json.loads(response.get_data(as_text=True)) + + +def create_lectures(department): + """Create a few lectures, return names.""" + lectures = [] + for lecture in ['Some %s Lecture', 'Cool %s Lecture', 'Boring %s Lecture']: + for year in range(1, 3): + special_name = '%s Year %s' % (department.upper(), year) + + data = { + 'title': lecture % special_name, + 'year': year, + 'department': department, + 'assistants': ASSISTANTS, + } + response = post('lectures', data) + if response: + lectures.append(response['_id']) + + return lectures + + +def room_gen(): + """Produce rooms.""" + i = 0 + while True: + yield "ETZ E %d" % i + i += 1 + + +def time_gen(starting_day): + """Produce a stream of non-overlapping time ranges.""" + while True: + year = starting_day.year + month = starting_day.month + day = starting_day.day + + # Random start and end time + start = dt(year, month, day, randint(7, 10)).strftime(DATE_FORMAT) + end = dt(year, month, day, randint(11, 14)).strftime(DATE_FORMAT) + yield {'start': start, 'end': end} + + # Only one slot per day to avoid overlap + starting_day += timedelta(days=1) + + +ROOM = room_gen() +TIME = time_gen(dt.now() + timedelta(days=90)) # Sometime in the future + + +def create_course(lecture, assistant, open_signup=True): + """Create several course for each lecture.""" + signup_diff = timedelta(days=-5) if open_signup else timedelta(15) + start = dt.utcnow() + signup_diff + end = start + timedelta(days=15) + + data = { + 'lecture': lecture, + 'assistant': assistant, + + 'signup': { + 'start': start.strftime(DATE_FORMAT), + 'end': end.strftime(DATE_FORMAT), + }, + + 'datetimes': [next(TIME) for _ in range(randint(1, 3))], + 'room': next(ROOM), + 'spots': randint(MIN_SPOTS, MAX_SPOTS), + } + return post('courses', data)['_id'] + + +def create_signups(course): + """Create random number of signups to a course.""" + for n in range(randint(0, 2*MAX_SPOTS)): + data = { + 'nethz': 'student%d' % n, + 'course': course + } + post('signups', data=data) + + +if __name__ == '__main__': + # Create courses + # (Repeat for itet and mavt) + for department in ['itet', 'mavt']: + # Create a few lectures + lectures = create_lectures(department) + + # Create a few courses with closed signup + for lecture in lectures: + for n in range(randint(0, 2)): + create_course(lecture, ASSISTANTS[n], open_signup=False) + + # Create a few courses with open signup + courses = [] + for lecture in lectures: + for n in range(randint(2, len(ASSISTANTS))): + courses.append(create_course(lecture, ASSISTANTS[n])) + + # Create signups for courses + for course in courses: + create_signups(course) + + # Start the app + APP.run(host=HOST, port=int(PORT)) diff --git a/README.md b/README.md index dd71840..72b3daf 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,48 @@ A new AMIV PVK tool using Eve and authenticating users with AMIVAPI. WIP. +### Starting A Local Backend for Development + +Using Docker, you can quickly set up a backend with demo data to focus on +frontend development. Here's how: + +1. [Install Docker](https://www.docker.com/community-edition#/download) + +2. You need MongoDB. Luckily, you can just use Docker for that, too. + (It's one long command, make sure to copy everything to your command line) + + ```bash + docker run --name mongodb -d -p 27017 \ + -e MONGODB_DATABASE="pvk" \ + -e MONGODB_USERNAME="pvkuser" \ + -e MONGODB_PASSWORD="pvkpass" \ + bitnami/mongodb:latest + ``` + + The environment variables specified with `-e` make sure that the db and user + are created + +3. Build and start the dev-container. (Two commands this time) + Adjust the path `./Backend` at the end as needed so it points to the + `backend` directory. + + ```bash + docker build -t pvk ./Backend/ + ``` + + ```bash + docker run --name pvk -d -p 80:80 \ + --link mongodb \ + -e MONGO_HOST="mongodb" \ + pvk + ``` + The `--link` and `-e` make sure the backend can access the db container. + +And now the backend is available at port 80, ready to use! + +Use the commands `docker start pvk` / `docker stop pvk` and +`docker start mongodb` / `docker stop mongodb` to control your containers. + ## Backend @@ -35,23 +77,23 @@ But don't worry, it's very easy to set them up -- take a look at the and `readWrite` permission to the `pvk_test` database. On Linux or similar, you can use this one-liner: - ``` + + ```bash mongo pvk_test --eval 'db.createUser({user:"pvk_user",pwd:"pvk_pass",roles:["readWrite"]});' ``` - Secondly, set up your virtual environment, install requirements as well as [py.test](https://docs.pytest.org/en/latest/). - ``` + ```bash python -m venv env source env/bin/activate - pip install -r requirements.txt pip install pytest ``` - Finally, you can run the tests from your virtual environment with: - ``` + ```bash py.test ``` -- GitLab From 57f3cd942bb9e280f1113b238bf4e35b70916577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 23 Nov 2017 22:36:45 +0100 Subject: [PATCH 06/11] Backend: Add Pylintrc to exclude some specific checks --- Backend/pylintrc | 427 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 427 insertions(+) create mode 100644 Backend/pylintrc diff --git a/Backend/pylintrc b/Backend/pylintrc new file mode 100644 index 0000000..4e6599f --- /dev/null +++ b/Backend/pylintrc @@ -0,0 +1,427 @@ +[MASTER] + +# A comma-separated list of package or module names from where C extensions may +# be loaded. Extensions are loading into the active Python interpreter and may +# run arbitrary code +extension-pkg-whitelist= + +# Add files or directories to the blacklist. They should be base names, not +# paths. +ignore=CVS + +# Add files or directories matching the regex patterns to the blacklist. The +# regex matches against base names, not paths. +ignore-patterns= + +# Python code to execute, usually for sys.path manipulation such as +# pygtk.require(). +#init-hook= + +# Use multiple processes to speed up Pylint. +jobs=1 + +# List of plugins (as comma separated values of python modules names) to load, +# usually to register additional checkers. +load-plugins= + +# Pickle collected data for later comparisons. +persistent=yes + +# Specify a configuration file. +#rcfile= + +# Allow loading of arbitrary C extensions. Extensions are imported into the +# active Python interpreter and may run arbitrary code. +unsafe-load-any-extension=no + + +[MESSAGES CONTROL] + +# Only show warnings with the listed confidence levels. Leave empty to show +# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED +confidence= + +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifiers separated by comma (,) or put this +# option multiple times (only on the command line, not in the configuration +# file where it should appear only once).You can also use "--disable=all" to +# disable everything first and then reenable specific checks. For example, if +# you want to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use"--disable=all --enable=classes +# --disable=W" +disable=print-statement,parameter-unpacking,unpacking-in-except,old-raise-syntax,backtick,long-suffix,old-ne-operator,old-octal-literal,import-star-module-level,raw-checker-failed,bad-inline-option,locally-disabled,locally-enabled,file-ignored,suppressed-message,useless-suppression,deprecated-pragma,apply-builtin,basestring-builtin,buffer-builtin,cmp-builtin,coerce-builtin,execfile-builtin,file-builtin,long-builtin,raw_input-builtin,reduce-builtin,standarderror-builtin,unicode-builtin,xrange-builtin,coerce-method,delslice-method,getslice-method,setslice-method,no-absolute-import,old-division,dict-iter-method,dict-view-method,next-method-called,metaclass-assignment,indexing-exception,raising-string,reload-builtin,oct-method,hex-method,nonzero-method,cmp-method,input-builtin,round-builtin,intern-builtin,unichr-builtin,map-builtin-not-iterating,zip-builtin-not-iterating,range-builtin-not-iterating,filter-builtin-not-iterating,using-cmp-argument,eq-without-hash,div-method,idiv-method,rdiv-method,exception-message-attribute,invalid-str-codec,sys-max-int,bad-python3-import,deprecated-string-function,deprecated-str-translate-call + +# Enable the message, report, category or checker with the given id(s). You can +# either give multiple identifier separated by comma (,) or put this option +# multiple time (only on the command line, not in the configuration file where +# it should appear only once). See also the "--disable" option for examples. +enable= + + +[REPORTS] + +# Python expression which should return a note less than 10 (10 is the highest +# note). You have access to the variables errors warning, statement which +# respectively contain the number of errors / warnings messages and the total +# number of statements analyzed. This is used by the global evaluation report +# (RP0004). +evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) + +# Template used to display messages. This is a python new-style format string +# used to format the message information. See doc for all details +#msg-template= + +# Set the output format. Available formats are text, parseable, colorized, json +# and msvs (visual studio).You can also give a reporter class, eg +# mypackage.mymodule.MyReporterClass. +output-format=text + +# Tells whether to display a full report or only the messages +reports=no + +# Activate the evaluation score. +score=yes + + +[REFACTORING] + +# Maximum number of nested blocks for function / method body +max-nested-blocks=5 + + +[BASIC] + +# Naming hint for argument names +argument-name-hint=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Regular expression matching correct argument names +argument-rgx=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Naming hint for attribute names +attr-name-hint=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Regular expression matching correct attribute names +attr-rgx=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo,bar,baz,toto,tutu,tata + +# Naming hint for class attribute names +class-attribute-name-hint=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Regular expression matching correct class attribute names +class-attribute-rgx=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Naming hint for class names +class-name-hint=[A-Z_][a-zA-Z0-9]+$ + +# Regular expression matching correct class names +class-rgx=[A-Z_][a-zA-Z0-9]+$ + +# Naming hint for constant names +const-name-hint=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Regular expression matching correct constant names +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Minimum line length for functions/classes that require docstrings, shorter +# ones are exempt. +docstring-min-length=-1 + +# Naming hint for function names +function-name-hint=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Regular expression matching correct function names +function-rgx=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Good variable names which should always be accepted, separated by a comma +good-names=i,j,k,ex,Run,_ + +# Include a hint for the correct naming format with invalid-name +include-naming-hint=no + +# Naming hint for inline iteration names +inlinevar-name-hint=[A-Za-z_][A-Za-z0-9_]*$ + +# Regular expression matching correct inline iteration names +inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ + +# Naming hint for method names +method-name-hint=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Regular expression matching correct method names +method-rgx=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Naming hint for module names +module-name-hint=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Regular expression matching correct module names +module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Colon-delimited sets of names that determine each other's naming style when +# the name regexes allow several styles. +name-group= + +# Regular expression which should only match function or class names that do +# not require a docstring. +no-docstring-rgx=^_ + +# List of decorators that produce properties, such as abc.abstractproperty. Add +# to this list to register other decorators that produce valid properties. +property-classes=abc.abstractproperty + +# Naming hint for variable names +variable-name-hint=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + +# Regular expression matching correct variable names +variable-rgx=(([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$ + + +[FORMAT] + +# Expected format of line ending, e.g. empty (any line ending), LF or CRLF. +expected-line-ending-format= + +# Regexp for a line that is allowed to be longer than the limit. +ignore-long-lines=^\s*(# )??$ + +# Number of spaces of indent required inside a hanging or continued line. +indent-after-paren=4 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +indent-string=' ' + +# Maximum number of characters on a single line. +max-line-length=100 + +# Maximum number of lines in a module +max-module-lines=1000 + +# List of optional constructs for which whitespace checking is disabled. `dict- +# separator` is used to allow tabulation in dicts, etc.: {1 : 1,\n222: 2}. +# `trailing-comma` allows a space between comma and closing bracket: (a, ). +# `empty-line` allows space-only lines. +no-space-check=trailing-comma,dict-separator + +# Allow the body of a class to be on the same line as the declaration if body +# contains single statement. +single-line-class-stmt=no + +# Allow the body of an if to be on the same line as the test if there is no +# else. +single-line-if-stmt=no + + +[SPELLING] + +# Spelling dictionary name. Available dictionaries: none. To make it working +# install python-enchant package. +spelling-dict= + +# List of comma separated words that should not be checked. +spelling-ignore-words= + +# A path to a file that contains private dictionary; one word per line. +spelling-private-dict-file= + +# Tells whether to store unknown words to indicated private dictionary in +# --spelling-private-dict-file option instead of raising a message. +spelling-store-unknown-words=no + + +[LOGGING] + +# Logging modules to check that the string format arguments are in logging +# function parameter format +logging-modules=logging + + +[TYPECHECK] + +# List of decorators that produce context managers, such as +# contextlib.contextmanager. Add to this list to register other decorators that +# produce valid context managers. +contextmanager-decorators=contextlib.contextmanager + +# List of members which are set dynamically and missed by pylint inference +# system, and so shouldn't trigger E1101 when accessed. Python regular +# expressions are accepted. +generated-members= + +# Tells whether missing members accessed in mixin class should be ignored. A +# mixin class is detected if its name ends with "mixin" (case insensitive). +ignore-mixin-members=yes + +# This flag controls whether pylint should warn about no-member and similar +# checks whenever an opaque object is returned when inferring. The inference +# can return multiple potential results while evaluating a Python object, but +# some branches might not be evaluated, which results in partial inference. In +# that case, it might be useful to still emit no-member and other checks for +# the rest of the inferred objects. +ignore-on-opaque-inference=yes + +# List of class names for which member attributes should not be checked (useful +# for classes with dynamically set attributes). This supports the use of +# qualified names. +# MODIFIED: Added Eve since it doesn't explicitly define event hooks +ignored-classes=optparse.Values,thread._local,_thread._local,Eve + +# List of module names for which member attributes should not be checked +# (useful for modules/projects where namespaces are manipulated during runtime +# and thus existing member attributes cannot be deduced by static analysis. It +# supports qualified module names, as well as Unix pattern matching. +ignored-modules= + +# Show a hint with possible names when a member name was not found. The aspect +# of finding the hint is based on edit distance. +missing-member-hint=yes + +# The minimum edit distance a name should have in order to be considered a +# similar match for a missing member name. +missing-member-hint-distance=1 + +# The total number of similar names that should be taken in consideration when +# showing a hint for a missing member. +missing-member-max-choices=1 + + +[VARIABLES] + +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= + +# Tells whether unused global variables should be treated as a violation. +allow-global-unused-variables=yes + +# List of strings which can identify a callback function by name. A callback +# name must start or end with one of those strings. +callbacks=cb_,_cb + +# A regular expression matching the name of dummy variables (i.e. expectedly +# not used). +dummy-variables-rgx=_+$|(_[a-zA-Z0-9_]*[a-zA-Z0-9]+?$)|dummy|^ignored_|^unused_ + +# Argument names that match this expression will be ignored. Default to name +# with leading underscore +ignored-argument-names=_.*|^ignored_|^unused_ + +# Tells whether we should check for unused import in __init__ files. +init-import=no + +# List of qualified module names which can have objects that can redefine +# builtins. +redefining-builtins-modules=six.moves,future.builtins + + +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +# MODIFIED: Removed TODO +notes=FIXME,XXX + + +[SIMILARITIES] + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + +# Ignore imports when computing similarities. +ignore-imports=no + +# Minimum lines number of a similarity. +min-similarity-lines=4 + + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=5 + +# Maximum number of attributes for a class (see R0902). +max-attributes=7 + +# Maximum number of boolean expressions in a if statement +max-bool-expr=5 + +# Maximum number of branch for function / method body +max-branches=12 + +# Maximum number of locals for function / method body +max-locals=15 + +# Maximum number of parents for a class (see R0901). +max-parents=7 + +# Maximum number of public methods for a class (see R0904). +max-public-methods=20 + +# Maximum number of return / yield for function / method body +max-returns=6 + +# Maximum number of statements in function / method body +max-statements=50 + +# Minimum number of public methods for a class (see R0903). +min-public-methods=2 + + +[IMPORTS] + +# Allow wildcard imports from modules that define __all__. +allow-wildcard-with-all=no + +# Analyse import fallback blocks. This can be used to support both Python 2 and +# 3 compatible code, which means that the block might have code that exists +# only in one or another interpreter, leading to false positives when analysed. +analyse-fallback-blocks=no + +# Deprecated modules which should not be used, separated by a comma +deprecated-modules=optparse,tkinter.tix + +# Create a graph of external dependencies in the given file (report RP0402 must +# not be disabled) +ext-import-graph= + +# Create a graph of every (i.e. internal and external) dependencies in the +# given file (report RP0402 must not be disabled) +import-graph= + +# Create a graph of internal dependencies in the given file (report RP0402 must +# not be disabled) +int-import-graph= + +# Force import order to recognize a module as part of the standard +# compatibility libraries. +known-standard-library= + +# Force import order to recognize a module as part of a third party library. +known-third-party=enchant + + +[CLASSES] + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__,__new__,setUp + +# List of member names, which should be excluded from the protected access +# warning. +exclude-protected=_asdict,_fields,_replace,_source,_make + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + +# List of valid names for the first argument in a metaclass class method. +valid-metaclass-classmethod-first-arg=mcs + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception -- GitLab From d19a8781b8d9faf0dd4b008f2d15a919b0b3266c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 23 Nov 2017 22:37:23 +0100 Subject: [PATCH 07/11] Backend: Improve code style --- Backend/security.py | 2 +- Backend/settings.py | 4 ++-- Backend/signups.py | 14 +++++++------- Backend/validation.py | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Backend/security.py b/Backend/security.py index de820ad..b6f9417 100644 --- a/Backend/security.py +++ b/Backend/security.py @@ -123,7 +123,7 @@ class APIAuth(TokenAuth): domain = current_app.config['DOMAIN'] allowed_methods = domain[resource].get('user_methods', []) - if (method in allowed_methods or is_admin()): + if method in allowed_methods or is_admin(): return True else: abort(403) diff --git a/Backend/settings.py b/Backend/settings.py index 7fc583e..74eb643 100644 --- a/Backend/settings.py +++ b/Backend/settings.py @@ -96,7 +96,7 @@ DOMAIN = { 'maxlength': 10, 'empty': False, 'nullable': False, - } + }, # TODO: Not the same nethz twice # TODO: Is nethz enough here? } @@ -118,7 +118,7 @@ DOMAIN = { 'not_patchable': True, # Course is tied to lecture }, 'assistant': { - 'type': 'string' + 'type': 'string', # TODO: Assistant needs to exist for lecture }, diff --git a/Backend/signups.py b/Backend/signups.py index cf2d177..a1f1b1f 100644 --- a/Backend/signups.py +++ b/Backend/signups.py @@ -18,20 +18,20 @@ from eve.methods.get import getitem_internal from eve.methods.patch import patch_internal -def wrap_response(f): +def wrap_response(function): """Wrapper to modify payload for successful requests (status 2xx).""" - @wraps(f) - def wrapped(request, response): + @wraps(function) + def _wrapped(_, response): if response.status_code // 100 == 2: payload = json.loads(response.get_data(as_text=True)) if '_items' in payload: - f(payload['_items']) + function(payload['_items']) else: - f([payload]) + function([payload]) response.set_data(json.dumps(payload)) - return wrapped + return _wrapped @wrap_response @@ -58,7 +58,7 @@ def deleted_signup(signup): update_signups(signup['course']) -def patched_signup(update, original): +def patched_signup(update, _): """Update status of new course.""" if 'course' in update: update_signups(str(update['course'])) diff --git a/Backend/validation.py b/Backend/validation.py index 2174dcf..169cc3d 100644 --- a/Backend/validation.py +++ b/Backend/validation.py @@ -47,7 +47,7 @@ class APIValidator(Validator): "combination with values for: %s" % unique_combination) - def _validate_not_patchable(self, enabled, field, value): + def _validate_not_patchable(self, enabled, field, _): """Inhibit patching of the field, also copied from AMIVAPI.""" if enabled and (request.method == 'PATCH'): self._error(field, "this field can not be changed with PATCH") -- GitLab From 22da06eff59f889b8e9c4d882f85002f3b963c9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 23 Nov 2017 22:37:55 +0100 Subject: [PATCH 08/11] Backend/demoserver: Move into down directory --- Backend/{ => demoserver}/Dockerfile | 12 ++++++------ Backend/{ => demoserver}/demo_server.py | 0 2 files changed, 6 insertions(+), 6 deletions(-) rename Backend/{ => demoserver}/Dockerfile (56%) rename Backend/{ => demoserver}/demo_server.py (100%) diff --git a/Backend/Dockerfile b/Backend/demoserver/Dockerfile similarity index 56% rename from Backend/Dockerfile rename to Backend/demoserver/Dockerfile index 823e566..43d7244 100644 --- a/Backend/Dockerfile +++ b/Backend/demoserver/Dockerfile @@ -4,14 +4,14 @@ ENV FLASK_APP app.py ENV FLASK_DEBUG true EXPOSE 80 -COPY ./requirements.txt /requirements.txt +COPY ../requirements.txt /requirements.txt RUN pip install -r requirements.txt -COPY ./app.py /app.py -COPY ./settings.py /settings.py -COPY ./security.py /security.py -COPY ./signups.py /signups.py -COPY ./validation.py /validation.py +COPY ../app.py /app.py +COPY ../settings.py /settings.py +COPY ../security.py /security.py +COPY ../signups.py /signups.py +COPY ../validation.py /validation.py COPY ./demo_server.py /demo_server.py diff --git a/Backend/demo_server.py b/Backend/demoserver/demo_server.py similarity index 100% rename from Backend/demo_server.py rename to Backend/demoserver/demo_server.py -- GitLab From a768744f362a2ec0afb60ece47904cfda5175309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 23 Nov 2017 22:52:41 +0100 Subject: [PATCH 09/11] Backend/tests: Improve coding style. --- Backend/tests/conftest.py | 4 ++-- Backend/tests/test_security.py | 2 +- Backend/tests/test_signups.py | 35 +++++++++++++++++++--------------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Backend/tests/conftest.py b/Backend/tests/conftest.py index 9375a7e..a537137 100644 --- a/Backend/tests/conftest.py +++ b/Backend/tests/conftest.py @@ -99,8 +99,8 @@ def app(): application.client = application.test_client() # Using __get__ binds the function to the application instance - application.user = user.__get__(application) - application.admin = admin.__get__(application) + application.user = user.__get__(application) # pylint: disable=E1121 + application.admin = admin.__get__(application) # pylint: disable=E1121 yield application drop_database(application) diff --git a/Backend/tests/test_security.py b/Backend/tests/test_security.py index 16a6684..2335ae7 100644 --- a/Backend/tests/test_security.py +++ b/Backend/tests/test_security.py @@ -98,7 +98,7 @@ def test_signup_with_own_nethz_only(app): assert_status=201) -def test_selection_for_own_nethz_only(app): +def test_selection_own_nethz(app): """Users can only select courses for themselves.""" nethz = 'Something' with app.user(nethz=nethz): diff --git a/Backend/tests/test_signups.py b/Backend/tests/test_signups.py index 5ad00b0..9e45665 100644 --- a/Backend/tests/test_signups.py +++ b/Backend/tests/test_signups.py @@ -1,10 +1,15 @@ """Test for signup processing, in particular waiting list.""" -from unittest.mock import patch, call -import pytest +# Some pylint config: +# Because of fixtures we re-use the same name often +# And some tests need long names to be descriptive +# pylint: disable=redefined-outer-name, invalid-name from datetime import datetime as dt +from unittest.mock import patch, call +import pytest + from signups import update_signups @@ -99,12 +104,12 @@ def test_update_spots(app): """ with app.admin(): # Create a course with two spots - course = app.data.driver.db['courses'].insert({'spots': 2}) + test_course = app.data.driver.db['courses'].insert({'spots': 2}) # Create four signups on waiting list # 1. Oldest _created timestamp, but recently modified (recent _updated) first_data = { - 'course': course, + 'course': test_course, 'status': 'waiting', '_created': dt(2020, 10, 10), '_updated': dt(2020, 10, 20), @@ -113,7 +118,7 @@ def test_update_spots(app): # 3. oldest _updated second_data = { - 'course': course, + 'course': test_course, 'status': 'waiting', '_created': dt(2020, 10, 11), '_updated': dt(2020, 10, 11), @@ -122,7 +127,7 @@ def test_update_spots(app): # 3. earlier _created, second oldest _updated third_data = { - 'course': course, + 'course': test_course, 'status': 'waiting', '_created': dt(2020, 10, 12), '_updated': dt(2020, 10, 15), @@ -132,7 +137,7 @@ def test_update_spots(app): # 4. Same updated time as 3, but different id that will loose tie fourth_data = { - 'course': course, + 'course': test_course, 'status': 'waiting', '_created': dt(2020, 10, 13), '_updated': dt(2020, 10, 15), @@ -142,16 +147,16 @@ def test_update_spots(app): # Do the update! # We except 2 and 3 to get spots - update_signups(str(course)) + update_signups(str(test_course)) - def status(_id): + def _status(_id): return app.client.get('/signups/' + _id, assert_status=200)['status'] - assert status(first_id) == 'waiting' - assert status(second_id) == 'reserved' - assert status(third_id) == 'reserved' - assert status(fourth_id) == 'waiting' + assert _status(first_id) == 'waiting' + assert _status(second_id) == 'reserved' + assert _status(third_id) == 'reserved' + assert _status(fourth_id) == 'waiting' @pytest.fixture @@ -159,8 +164,8 @@ def course(app): """Create a fake course without any data for a test.""" with app.admin(): # Create a few courses to sign up to - db = app.data.driver.db['courses'] - yield str(db.insert({'_etag': 'tag'})) + database = app.data.driver.db['courses'] + yield str(database.insert({'_etag': 'tag'})) @pytest.fixture -- GitLab From 6d1b326c8fb31167e04a0f74289c3e776c8a013e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 23 Nov 2017 22:58:49 +0100 Subject: [PATCH 10/11] Backend: update tox.ini --- Backend/tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Backend/tox.ini b/Backend/tox.ini index 030f25b..c3c409e 100644 --- a/Backend/tox.ini +++ b/Backend/tox.ini @@ -15,7 +15,8 @@ passenv = MONGO_HOST # Pylint config is a bit hacky, for some reason *.py doesnt work in tox [testenv:pylint] -commands = pylint app.py security.py settings.py setup.py tests +commands = pylint app.py security.py settings.py signups.py validation.py \ + setup.py tests deps = pylint pytest -- GitLab From 444fc8094b7f8f0d3da1571092bc59b41d4c50ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Dietm=C3=BCller?= Date: Thu, 23 Nov 2017 23:22:13 +0100 Subject: [PATCH 11/11] Backend/signups: When patching a signup, update signups of old course as well. --- Backend/signups.py | 6 ++++-- Backend/tests/test_signups.py | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Backend/signups.py b/Backend/signups.py index a1f1b1f..697b264 100644 --- a/Backend/signups.py +++ b/Backend/signups.py @@ -58,10 +58,12 @@ def deleted_signup(signup): update_signups(signup['course']) -def patched_signup(update, _): - """Update status of new course.""" +def patched_signup(update, original): + """Update status of all signups of the original and new course.""" + # Only need to do something if course is changed if 'course' in update: update_signups(str(update['course'])) + update_signups(str(original['course'])) def patched_course(update, original): diff --git a/Backend/tests/test_signups.py b/Backend/tests/test_signups.py index 9e45665..6ea2df7 100644 --- a/Backend/tests/test_signups.py +++ b/Backend/tests/test_signups.py @@ -211,12 +211,15 @@ def test_patch_signup_triggers_update(app, course, mock_update): fake = str(app.data.driver.db['signups'].insert({ '_etag': 'tag', 'nethz': 'lala', + 'course': 'oldcourse' })) app.client.patch('/signups/' + fake, data={'course': course}, headers={'If-Match': 'tag'}, assert_status=200) - mock_update.assert_called_with(course) + + # Both the signups of the old and new course are updated + mock_update.assert_has_calls([call(course), call('oldcourse')]) def test_delete_signup_triggers_update(app, course, mock_update): -- GitLab