diff --git a/Makefile b/Makefile index e94889cea..3612540bc 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ SHELL := /bin/bash DATE = $(shell date +%Y-%m-%d:%H:%M:%S) +PIP_ACCEL_CACHE ?= ${CURDIR}/cache/pip-accel APP_VERSION_FILE = app/version.py GIT_BRANCH ?= $(shell git symbolic-ref --short HEAD 2> /dev/null || echo "detached") @@ -33,19 +34,11 @@ help: @cat $(MAKEFILE_LIST) | grep -E '^[a-zA-Z_-]+:.*?## .*$$' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' .PHONY: venv -# if there's a virtualenv already active, we don't want to do anything -ifeq ($(VIRTUAL_ENV),) venv: venv/bin/activate ## Create virtualenv if it does not exist venv/bin/activate: test -d venv || virtualenv venv -p python3 - -VENV_BIN_DIR = $(shell . venv/bin/activate && echo $$VIRTUAL_ENV/bin) -else -venv: - -VENV_BIN_DIR = $(VIRTUAL_ENV)/bin -endif + . venv/bin/activate && pip install pip-accel .PHONY: check-env-vars check-env-vars: ## Check mandatory environment variables @@ -72,9 +65,9 @@ production: ## Set environment to production @true .PHONY: dependencies -dependencies: venv - $(call install-pycurl, $(VENV_BIN_DIR)/pip) - $(VENV_BIN_DIR)/pip install -r requirements_for_test.txt +dependencies: venv ## Install build dependencies + mkdir -p ${PIP_ACCEL_CACHE} + . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements_for_test.txt .PHONY: generate-version-file generate-version-file: ## Generates the app version file @@ -82,7 +75,7 @@ generate-version-file: ## Generates the app version file .PHONY: build build: dependencies generate-version-file ## Build project - + . venv/bin/activate && PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} pip-accel install -r requirements.txt .PHONY: build-paas-artifact build-paas-artifact: ## Build the deploy artifact for PaaS @@ -104,7 +97,6 @@ test: venv generate-version-file ## Run tests freeze-requirements: rm -rf venv-freeze virtualenv -p python3 venv-freeze - $(call install-pycurl, $$(pwd)/venv-freeze/bin/pip) $$(pwd)/venv-freeze/bin/pip install -r requirements-app.txt echo '# pyup: ignore file' > requirements.txt echo '# This file is autogenerated. Do not edit it manually.' >> requirements.txt @@ -113,15 +105,6 @@ freeze-requirements: $$(pwd)/venv-freeze/bin/pip freeze -r <(sed '/^--/d' requirements-app.txt) | sed -n '/The following requirements were added by pip freeze/,$$p' >> requirements.txt rm -rf venv-freeze -define install-pycurl - # install pycurl separately to avoid flags disabling wheels for other packages - PYCURL_SSL_LIBRARY=openssl ${1} install pycurl==7.43.0.2 --global-option="build_ext" --global-option="-I/usr/local/opt/openssl/include" -endef - -.PHONY: install-pycurl -install-pycurl: - $(call install-pycurl, pip) - .PHONY: test-requirements test-requirements: @diff requirements-app.txt requirements.txt | grep '<' \ @@ -131,10 +114,11 @@ test-requirements: .PHONY: coverage coverage: venv ## Create coverage report - coveralls + . venv/bin/activate && coveralls .PHONY: prepare-docker-build-image prepare-docker-build-image: ## Prepare the Docker builder image + mkdir -p ${PIP_ACCEL_CACHE} make -C docker build .PHONY: build-with-docker @@ -142,6 +126,7 @@ build-with-docker: prepare-docker-build-image ## Build inside a Docker container @docker run -i${DOCKER_TTY} --rm \ --name "${DOCKER_CONTAINER_PREFIX}-build" \ -v "`pwd`:/var/project" \ + -v "${PIP_ACCEL_CACHE}:/var/project/cache/pip-accel" \ -e UID=$(shell id -u) \ -e GID=$(shell id -g) \ -e GIT_COMMIT=${GIT_COMMIT} \ diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index cc0a20922..854ea18c8 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -1,6 +1,7 @@ import io import math from datetime import datetime +from uuid import UUID from PyPDF2.utils import PdfReadError from botocore.exceptions import ClientError as BotoClientError @@ -30,8 +31,9 @@ from app.letters.utils import ( get_reference_from_filename, get_folder_name, upload_letter_pdf, - move_failed_pdf, ScanErrorType, + move_failed_pdf, + move_scan_to_invalid_pdf_bucket, move_error_pdf_to_scan_bucket, get_file_names_from_error_bucket ) @@ -39,9 +41,10 @@ from app.models import ( KEY_TYPE_TEST, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, - NOTIFICATION_VIRUS_SCAN_FAILED, + NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TECHNICAL_FAILURE, - NOTIFICATION_PERMANENT_FAILURE + NOTIFICATION_VALIDATION_FAILED, + NOTIFICATION_VIRUS_SCAN_FAILED, ) @@ -188,18 +191,27 @@ def process_virus_scan_passed(self, filename): new_pdf = _sanitise_precomiled_pdf(self, notification, old_pdf) + # TODO: Remove this once CYSP update their template to not cross over the margins + if notification.service_id == UUID('fe44178f-3b45-4625-9f85-2264a36dd9ec'): # CYSP + # Check your state pension submit letters with good addresses and notify tags, so just use their supplied pdf + new_pdf = old_pdf + if not new_pdf: current_app.logger.info('Invalid precompiled pdf received {} ({})'.format(notification.id, filename)) - # update_notification_status_by_id(notification.id, NOTIFICATION_VALIDATION_FAILED) - # move_scan_to_invalid_pdf_bucket() # TODO: implement this (and create bucket etc) - # scan_pdf_object.delete() - # return + + notification.status = NOTIFICATION_VALIDATION_FAILED + dao_update_notification(notification) + + move_scan_to_invalid_pdf_bucket(filename) + scan_pdf_object.delete() + return else: current_app.logger.info( "Validation was successful for precompiled pdf {} ({})".format(notification.id, filename)) current_app.logger.info('notification id {} ({}) sanitised and ready to send'.format(notification.id, filename)) + # temporarily upload original pdf while testing sanitise flow. _upload_pdf_to_test_or_live_pdf_bucket( old_pdf, # TODO: change to new_pdf filename, @@ -257,7 +269,9 @@ def _sanitise_precomiled_pdf(self, notification, precompiled_pdf): return resp.content except RequestException as ex: if ex.response is not None and ex.response.status_code == 400: - # validation error + current_app.logger.exception( + "sanitise_precomiled_pdf validation error for notification: {}".format(notification.id) + ) return None try: @@ -269,7 +283,9 @@ def _sanitise_precomiled_pdf(self, notification, precompiled_pdf): current_app.logger.exception( "RETRY FAILED: sanitise_precomiled_pdf failed for notification {}".format(notification.id), ) - update_notification_status_by_id(notification.id, NOTIFICATION_TECHNICAL_FAILURE) + + notification.status = NOTIFICATION_TECHNICAL_FAILURE + dao_update_notification(notification) raise diff --git a/app/config.py b/app/config.py index c37dd5d00..5a17d006f 100644 --- a/app/config.py +++ b/app/config.py @@ -150,9 +150,9 @@ class Config(object): BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { 'region': AWS_REGION, + 'polling_interval': 1, # 1 second 'visibility_timeout': 310, - 'queue_name_prefix': NOTIFICATION_QUEUE_PREFIX, - 'wait_time_seconds': 20 # enable long polling, with a wait time of 20 seconds + 'queue_name_prefix': NOTIFICATION_QUEUE_PREFIX } CELERY_ENABLE_UTC = True CELERY_TIMEZONE = 'Europe/London' @@ -270,9 +270,7 @@ class Config(object): 'options': {'queue': QueueNames.PERIODIC} } } - - # this is overriden by the -Q command, but locally, we should read from all queues - CELERY_QUEUES = [Queue(queue, Exchange('default'), routing_key=queue) for queue in QueueNames.all_queues()] + CELERY_QUEUES = [] NOTIFICATIONS_ALERT = 5 # five mins FROM_NUMBER = 'development' @@ -334,11 +332,11 @@ class Development(Config): SQLALCHEMY_ECHO = False CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'development-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'development-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'notify.tools-ftp' LETTERS_PDF_BUCKET_NAME = 'development-letters-pdf' LETTERS_SCAN_BUCKET_NAME = 'development-letters-scan' + INVALID_PDF_BUCKET_NAME = 'development-letters-invalid-pdf' ADMIN_CLIENT_SECRET = 'dev-notify-secret-key' SECRET_KEY = 'dev-notify-secret-key' @@ -360,6 +358,11 @@ class Development(Config): STATSD_PORT = 1000 STATSD_PREFIX = "stats-prefix" + for queue in QueueNames.all_queues(): + Config.CELERY_QUEUES.append( + Queue(queue, Exchange('default'), routing_key=queue) + ) + API_HOST_NAME = "http://localhost:6011" API_RATE_LIMIT_ENABLED = True @@ -371,17 +374,22 @@ class Test(Development): TESTING = True CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'test-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'test-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'test.notify.com-ftp' LETTERS_PDF_BUCKET_NAME = 'test-letters-pdf' LETTERS_SCAN_BUCKET_NAME = 'test-letters-scan' + INVALID_PDF_BUCKET_NAME = 'test-letters-invalid-pdf' # this is overriden in jenkins and on cloudfoundry SQLALCHEMY_DATABASE_URI = os.getenv('SQLALCHEMY_DATABASE_URI', 'postgresql://localhost/test_notification_api') BROKER_URL = 'you-forgot-to-mock-celery-in-your-tests://' + for queue in QueueNames.all_queues(): + Config.CELERY_QUEUES.append( + Queue(queue, Exchange('default'), routing_key=queue) + ) + API_RATE_LIMIT_ENABLED = True API_HOST_NAME = "http://localhost:6011" @@ -394,11 +402,11 @@ class Preview(Config): NOTIFY_EMAIL_DOMAIN = 'notify.works' NOTIFY_ENVIRONMENT = 'preview' CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'preview-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'preview-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp' LETTERS_PDF_BUCKET_NAME = 'preview-letters-pdf' LETTERS_SCAN_BUCKET_NAME = 'preview-letters-scan' + INVALID_PDF_BUCKET_NAME = 'preview-letters-invalid-pdf' FROM_NUMBER = 'preview' API_RATE_LIMIT_ENABLED = True CHECK_PROXY_HEADER = False @@ -408,11 +416,11 @@ class Staging(Config): NOTIFY_EMAIL_DOMAIN = 'staging-notify.works' NOTIFY_ENVIRONMENT = 'staging' CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'staging-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'staging-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'staging-notify.works-ftp' LETTERS_PDF_BUCKET_NAME = 'staging-letters-pdf' LETTERS_SCAN_BUCKET_NAME = 'staging-letters-scan' + INVALID_PDF_BUCKET_NAME = 'staging-letters-invalid-pdf' STATSD_ENABLED = True FROM_NUMBER = 'stage' API_RATE_LIMIT_ENABLED = True @@ -424,11 +432,11 @@ class Live(Config): NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk' NOTIFY_ENVIRONMENT = 'live' CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' - LETTERS_PDF_BUCKET_NAME = 'production-letters-pdf' TEST_LETTERS_BUCKET_NAME = 'production-test-letters' DVLA_RESPONSE_BUCKET_NAME = 'notifications.service.gov.uk-ftp' LETTERS_PDF_BUCKET_NAME = 'production-letters-pdf' LETTERS_SCAN_BUCKET_NAME = 'production-letters-scan' + INVALID_PDF_BUCKET_NAME = 'production-letters-invalid-pdf' STATSD_ENABLED = True FROM_NUMBER = 'GOVUK' FUNCTIONAL_TEST_PROVIDER_SERVICE_ID = '6c1d81bb-dae2-4ee9-80b0-89a4aae9f649' @@ -452,6 +460,7 @@ class Sandbox(CloudFoundryConfig): DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp' LETTERS_PDF_BUCKET_NAME = 'cf-sandbox-letters-pdf' LETTERS_SCAN_BUCKET_NAME = 'cf-sandbox-letters-scan' + INVALID_PDF_BUCKET_NAME = 'cf-sandbox-letters-invalid-pdf' FROM_NUMBER = 'sandbox' REDIS_ENABLED = False diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 6cfca54a8..95896e4da 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -5,7 +5,7 @@ from datetime import ( timedelta, ) -from botocore.exceptions import ClientError as BotoClientError +from boto.exception import BotoClientError from flask import current_app from notifications_utils.recipients import ( diff --git a/app/letters/utils.py b/app/letters/utils.py index a3c795e5b..fc84b50ac 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -6,7 +6,7 @@ from flask import current_app from notifications_utils.s3 import s3upload -from app.models import KEY_TYPE_TEST, SECOND_CLASS, RESOLVE_POSTAGE_FOR_FILE_NAME +from app.models import KEY_TYPE_TEST, SECOND_CLASS, RESOLVE_POSTAGE_FOR_FILE_NAME, NOTIFICATION_VALIDATION_FAILED from app.utils import convert_utc_to_bst @@ -48,14 +48,23 @@ def get_letter_pdf_filename(reference, crown, is_scan_letter=False, postage=SECO return upload_file_name -def get_bucket_prefix_for_notification(notification, is_test_letter=False): +def get_bucket_name_and_prefix_for_notification(notification): + is_test_letter = notification.key_type == KEY_TYPE_TEST and notification.template.is_precompiled_letter + folder = '' + if notification.status == NOTIFICATION_VALIDATION_FAILED: + bucket_name = current_app.config['INVALID_PDF_BUCKET_NAME'] + elif is_test_letter: + bucket_name = current_app.config['TEST_LETTERS_BUCKET_NAME'] + else: + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + folder = '{}/'.format(notification.created_at.date()) + upload_file_name = PRECOMPILED_BUCKET_PREFIX.format( - folder='' if is_test_letter else - '{}/'.format(notification.created_at.date()), + folder=folder, reference=notification.reference ).upper() - return upload_file_name + return bucket_name, upload_file_name def get_reference_from_filename(filename): @@ -107,6 +116,12 @@ def move_error_pdf_to_scan_bucket(source_filename): _move_s3_object(scan_bucket, error_file, scan_bucket, source_filename) +def move_scan_to_invalid_pdf_bucket(source_filename): + scan_bucket = current_app.config['LETTERS_SCAN_BUCKET_NAME'] + invalid_pdf_bucket = current_app.config['INVALID_PDF_BUCKET_NAME'] + _move_s3_object(scan_bucket, source_filename, invalid_pdf_bucket, source_filename) + + def get_file_names_from_error_bucket(): s3 = boto3.resource('s3') scan_bucket = current_app.config['LETTERS_SCAN_BUCKET_NAME'] @@ -116,18 +131,12 @@ def get_file_names_from_error_bucket(): def get_letter_pdf(notification): - is_test_letter = notification.key_type == KEY_TYPE_TEST and notification.template.is_precompiled_letter - if is_test_letter: - bucket_name = current_app.config['TEST_LETTERS_BUCKET_NAME'] - else: - bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + bucket_name, prefix = get_bucket_name_and_prefix_for_notification(notification) s3 = boto3.resource('s3') bucket = s3.Bucket(bucket_name) - item = next(x for x in bucket.objects.filter( - Prefix=get_bucket_prefix_for_notification(notification, is_test_letter) - )) + item = next(x for x in bucket.objects.filter(Prefix=prefix)) obj = s3.Object( bucket_name=bucket_name, diff --git a/app/models.py b/app/models.py index 6c11689ab..3cb89e4d5 100644 --- a/app/models.py +++ b/app/models.py @@ -247,6 +247,7 @@ class DVLAOrganisation(db.Model): __tablename__ = 'dvla_organisation' id = db.Column(db.String, primary_key=True) name = db.Column(db.String(255), nullable=True) + filename = db.Column(db.String(255), nullable=True) INTERNATIONAL_SMS_TYPE = 'international_sms' @@ -1052,6 +1053,7 @@ NOTIFICATION_TECHNICAL_FAILURE = 'technical-failure' NOTIFICATION_TEMPORARY_FAILURE = 'temporary-failure' NOTIFICATION_PERMANENT_FAILURE = 'permanent-failure' NOTIFICATION_PENDING_VIRUS_CHECK = 'pending-virus-check' +NOTIFICATION_VALIDATION_FAILED = 'validation-failed' NOTIFICATION_VIRUS_SCAN_FAILED = 'virus-scan-failed' NOTIFICATION_RETURNED_LETTER = 'returned-letter' @@ -1059,6 +1061,7 @@ NOTIFICATION_STATUS_TYPES_FAILED = [ NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, + NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED, NOTIFICATION_RETURNED_LETTER, ] @@ -1100,6 +1103,7 @@ NOTIFICATION_STATUS_TYPES = [ NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_PENDING_VIRUS_CHECK, + NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED, NOTIFICATION_RETURNED_LETTER, ] diff --git a/docker/Dockerfile b/docker/Dockerfile index db1bc6202..1060824cf 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -22,7 +22,6 @@ RUN \ libffi-dev \ python-dev \ jq \ - libcurl4-openssl-dev \ && echo "Clean up" \ && rm -rf /var/lib/apt/lists/* /tmp/* diff --git a/migrations/versions/0236_another_letter_org.py b/migrations/versions/0236_another_letter_org.py new file mode 100644 index 000000000..c49b8fd46 --- /dev/null +++ b/migrations/versions/0236_another_letter_org.py @@ -0,0 +1,35 @@ +"""empty message + +Revision ID: 0236_another_letter_org +Revises: 0235_add_postage_to_pk + +""" + +# revision identifiers, used by Alembic. +revision = '0236_another_letter_org' +down_revision = '0235_add_postage_to_pk' + +from alembic import op + + +NEW_ORGANISATIONS = [ + ('514', 'Brighton and Hove city council'), +] + + +def upgrade(): + for numeric_id, name in NEW_ORGANISATIONS: + op.execute(""" + INSERT + INTO dvla_organisation + VALUES ('{}', '{}') + """.format(numeric_id, name)) + + +def downgrade(): + for numeric_id, _ in NEW_ORGANISATIONS: + op.execute(""" + DELETE + FROM dvla_organisation + WHERE id = '{}' + """.format(numeric_id)) diff --git a/migrations/versions/0237_add_filename_to_dvla_org.py b/migrations/versions/0237_add_filename_to_dvla_org.py new file mode 100644 index 000000000..9e2dbbc66 --- /dev/null +++ b/migrations/versions/0237_add_filename_to_dvla_org.py @@ -0,0 +1,56 @@ +""" + +Revision ID: 0237_add_filename_to_dvla_org +Revises: 0236_another_letter_org +Create Date: 2018-09-28 15:39:21.115358 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.sql import text + + +revision = '0237_add_filename_to_dvla_org' +down_revision = '0236_another_letter_org' + + +LOGOS = { + '001': 'hm-government', + '002': 'opg', + '003': 'dwp', + '004': 'geo', + '005': 'ch', + '006': 'dwp-welsh', + '007': 'dept-for-communities', + '008': 'mmo', + '009': 'hmpo', + '500': 'hm-land-registry', + '501': 'ea', + '502': 'wra', + '503': 'eryc', + '504': 'rother', + '505': 'cadw', + '506': 'twfrs', + '507': 'thames-valley-police', + '508': 'ofgem', + '509': 'hackney', + '510': 'pension-wise', + '511': 'nhs', + '512': 'vale-of-glamorgan', + '513': 'wdc', + '514': 'brighton-hove', +} + + +def upgrade(): + conn = op.get_bind() + op.add_column('dvla_organisation', sa.Column('filename', sa.String(length=255), nullable=True)) + + for org_id, org_filename in LOGOS.items(): + conn.execute(text(""" + UPDATE dvla_organisation SET filename = :filename WHERE id = :id + """), filename=org_filename, id=org_id) + + +def downgrade(): + op.drop_column('dvla_organisation', 'filename') diff --git a/migrations/versions/0238_add_validation_failed.py b/migrations/versions/0238_add_validation_failed.py new file mode 100644 index 000000000..1dd2222af --- /dev/null +++ b/migrations/versions/0238_add_validation_failed.py @@ -0,0 +1,28 @@ +""" + +Revision ID: 0238_add_validation_failed +Revises: 0237_add_filename_to_dvla_org +Create Date: 2018-09-03 11:24:58.773824 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0238_add_validation_failed' +down_revision = '0237_add_filename_to_dvla_org' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.execute("INSERT INTO notification_status_types (name) VALUES ('validation-failed')") + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.execute("UPDATE notifications SET notification_status = 'permanent-failure' WHERE notification_status = 'validation-failed'") + op.execute("UPDATE notification_history SET notification_status = 'permanent-failure' WHERE notification_status = 'validation-failed'") + + op.execute("DELETE FROM notification_status_types WHERE name = 'validation-failed'") + # ### end Alembic commands ### diff --git a/requirements-app.txt b/requirements-app.txt index 085306c17..f55b54765 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -2,7 +2,7 @@ # with package version changes made in requirements-app.txt cffi==1.11.5 -celery[sqs]==4.2.1 +celery==3.1.26.post2 # pyup: <4 docopt==0.6.2 Flask-Bcrypt==0.7.1 flask-marshmallow==0.9.0 @@ -27,6 +27,6 @@ awscli==1.15.82 awscli-cwlogs>=1.4,<1.5 botocore<1.11.0 -git+https://github.com/alphagov/notifications-utils.git@30.5.3#egg=notifications-utils==30.5.3 +git+https://github.com/alphagov/notifications-utils.git@30.5.4#egg=notifications-utils==30.5.4 -# if you want to update pycurl please do so in makefile +git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/requirements.txt b/requirements.txt index 89d555659..e848c5cf2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ # with package version changes made in requirements-app.txt cffi==1.11.5 -celery[sqs]==4.2.1 +celery==3.1.26.post2 # pyup: <4 docopt==0.6.2 Flask-Bcrypt==0.7.1 flask-marshmallow==0.9.0 @@ -29,17 +29,18 @@ awscli==1.15.82 awscli-cwlogs>=1.4,<1.5 botocore<1.11.0 -git+https://github.com/alphagov/notifications-utils.git@30.5.3#egg=notifications-utils==30.5.3 +git+https://github.com/alphagov/notifications-utils.git@30.5.4#egg=notifications-utils==30.5.4 -# if you want to update pycurl please do so in makefile +git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 ## The following requirements were added by pip freeze: alembic==1.0.0 -amqp==2.3.2 +amqp==1.4.9 +anyjson==0.3.3 bcrypt==3.1.4 -billiard==3.5.0.4 +billiard==3.3.0.23 bleach==2.1.3 -boto3==1.9.17 +boto3==1.6.16 certifi==2018.8.24 chardet==3.0.4 Click==7.0 @@ -53,7 +54,7 @@ idna==2.7 itsdangerous==0.24 Jinja2==2.10 jmespath==0.9.3 -kombu==4.2.1 +kombu==3.0.37 Mako==1.0.7 MarkupSafe==1.0 mistune==0.8.3 @@ -62,7 +63,6 @@ orderedset==2.0.1 phonenumbers==8.9.4 pyasn1==0.4.4 pycparser==2.19 -pycurl==7.43.0.2 PyPDF2==1.26.0 python-dateutil==2.7.3 python-editor==1.0.3 @@ -77,6 +77,5 @@ six==1.11.0 smartypants==2.0.1 statsd==3.2.2 urllib3==1.23 -vine==1.1.4 webencodings==0.5.1 Werkzeug==0.14.1 diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index b92832233..c46d03aa6 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -20,7 +20,16 @@ function display_result { fi } -make build +if [ ! $VIRTUAL_ENV ]; then + virtualenv -p python3 ./venv + . ./venv/bin/activate +fi + +# we need the version file to exist otherwise the app will blow up +make generate-version-file + +# Install Python development dependencies +pip3 install -r requirements_for_test.txt # Create Postgres databases createdb notification_api diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 59edba5fd..11c8cd797 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -33,11 +33,14 @@ from app.models import ( Notification, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, + NOTIFICATION_PENDING_VIRUS_CHECK, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_SENDING, NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED, ) + from tests.app.db import create_notification from tests.conftest import set_config_values @@ -338,7 +341,7 @@ def test_letter_in_created_state_fails_if_notification_doesnt_exist(sample_notif @freeze_time('2018-01-01 18:00') @mock_s3 -@pytest.mark.parametrize('key_type, noti_status,bucket_config_name,destination_folder', [ +@pytest.mark.parametrize('key_type,noti_status,bucket_config_name,destination_folder', [ (KEY_TYPE_NORMAL, NOTIFICATION_CREATED, 'LETTERS_PDF_BUCKET_NAME', '2018-01-02/'), (KEY_TYPE_TEST, NOTIFICATION_DELIVERED, 'TEST_LETTERS_BUCKET_NAME', '') ]) @@ -361,23 +364,61 @@ def test_process_letter_task_check_virus_scan_passed( mocker.patch('app.celery.letters_pdf_tasks.pdf_page_count', return_value=1) mock_s3upload = mocker.patch('app.celery.letters_pdf_tasks.s3upload') - mock_sanitise = mocker.patch('app.celery.letters_pdf_tasks._sanitise_precomiled_pdf') + mock_sanitise = mocker.patch('app.celery.letters_pdf_tasks._sanitise_precomiled_pdf', return_value="pdf_content") process_virus_scan_passed(filename) assert letter_notification.status == noti_status assert letter_notification.billable_units == 1 - mock_s3upload.assert_called_once_with( - filedata=b'pdf_content', - region='eu-west-1', - bucket_name=target_bucket_name, - file_location=destination_folder + filename - ) mock_sanitise.assert_called_once_with( ANY, letter_notification, b'pdf_content' ) + mock_s3upload.assert_called_once_with( + bucket_name=target_bucket_name, + filedata=b'pdf_content', + file_location=destination_folder + filename, + region='eu-west-1', + ) + + +@freeze_time('2018-01-01 18:00') +@mock_s3 +@pytest.mark.parametrize('key_type,is_test_letter', [ + (KEY_TYPE_NORMAL, False), (KEY_TYPE_TEST, True) +]) +def test_process_letter_task_check_virus_scan_passed_when_sanitise_fails( + sample_letter_notification, mocker, key_type, is_test_letter +): + filename = 'NOTIFY.{}'.format(sample_letter_notification.reference) + source_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME'] + target_bucket_name = current_app.config['INVALID_PDF_BUCKET_NAME'] + + conn = boto3.resource('s3', region_name='eu-west-1') + conn.create_bucket(Bucket=source_bucket_name) + conn.create_bucket(Bucket=target_bucket_name) + + s3 = boto3.client('s3', region_name='eu-west-1') + s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content') + + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK + sample_letter_notification.key_type = key_type + mock_move_s3 = mocker.patch('app.letters.utils._move_s3_object') + mock_sanitise = mocker.patch('app.celery.letters_pdf_tasks._sanitise_precomiled_pdf', return_value=None) + + process_virus_scan_passed(filename) + + assert sample_letter_notification.status == NOTIFICATION_VALIDATION_FAILED + mock_sanitise.assert_called_once_with( + ANY, + sample_letter_notification, + b'pdf_content' + ) + mock_move_s3.assert_called_once_with( + source_bucket_name, filename, + target_bucket_name, filename + ) def test_get_page_count_set_notification_to_permanent_failure_when_not_pdf( @@ -391,7 +432,7 @@ def test_get_page_count_set_notification_to_permanent_failure_when_not_pdf( def test_process_letter_task_check_virus_scan_failed(sample_letter_notification, mocker): filename = 'NOTIFY.{}'.format(sample_letter_notification.reference) - sample_letter_notification.status = 'pending-virus-check' + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK mock_move_failed_pdf = mocker.patch('app.celery.letters_pdf_tasks.move_failed_pdf') with pytest.raises(VirusScanError) as e: @@ -404,7 +445,7 @@ def test_process_letter_task_check_virus_scan_failed(sample_letter_notification, def test_process_letter_task_check_virus_scan_error(sample_letter_notification, mocker): filename = 'NOTIFY.{}'.format(sample_letter_notification.reference) - sample_letter_notification.status = 'pending-virus-check' + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK mock_move_failed_pdf = mocker.patch('app.celery.letters_pdf_tasks.move_failed_pdf') with pytest.raises(VirusScanError) as e: @@ -436,6 +477,7 @@ def test_replay_letters_in_error_for_one_file(notify_api, mocker): def test_sanitise_precompiled_pdf_returns_data_from_template_preview(rmock, sample_letter_notification): + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK rmock.post('http://localhost:9999/precompiled/sanitise', content=b'new_pdf', status_code=200) mock_celery = Mock(**{'retry.side_effect': Retry}) @@ -446,6 +488,7 @@ def test_sanitise_precompiled_pdf_returns_data_from_template_preview(rmock, samp def test_sanitise_precompiled_pdf_returns_none_on_validation_error(rmock, sample_letter_notification): + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK rmock.post('http://localhost:9999/precompiled/sanitise', content=b'new_pdf', status_code=400) mock_celery = Mock(**{'retry.side_effect': Retry}) @@ -455,6 +498,7 @@ def test_sanitise_precompiled_pdf_returns_none_on_validation_error(rmock, sample def test_sanitise_precompiled_pdf_retries_on_http_error(rmock, sample_letter_notification): + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK rmock.post('http://localhost:9999/precompiled/sanitise', content=b'new_pdf', status_code=500) mock_celery = Mock(**{'retry.side_effect': Retry}) @@ -466,6 +510,7 @@ def test_sanitise_precompiled_pdf_sets_notification_to_technical_failure_after_t rmock, sample_letter_notification ): + sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK rmock.post('http://localhost:9999/precompiled/sanitise', content=b'new_pdf', status_code=500) mock_celery = Mock(**{'retry.side_effect': MaxRetriesExceededError}) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index a1d71d2dd..04d77a60e 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -189,24 +189,22 @@ def sample_service( return service -@pytest.fixture(scope='function') -def sample_service_full_permissions(notify_db, notify_db_session): - return sample_service( - notify_db, - notify_db_session, - # ensure name doesn't clash with regular sample service +@pytest.fixture(scope='function', name='sample_service_full_permissions') +def _sample_service_full_permissions(notify_db_session): + service = create_service( service_name="sample service full permissions", - permissions=set(SERVICE_PERMISSION_TYPES) + service_permissions=set(SERVICE_PERMISSION_TYPES) ) - - -@pytest.fixture(scope='function') -def sample_service_custom_letter_contact_block(notify_db, notify_db_session): - service = sample_service(notify_db, notify_db_session) - create_letter_contact(service, contact_block='((contact block))') + create_inbound_number('12345', service_id=service.id) return service +@pytest.fixture(scope='function', name='sample_service_custom_letter_contact_block') +def _sample_service_custom_letter_contact_block(sample_service): + create_letter_contact(sample_service, contact_block='((contact block))') + return sample_service + + @pytest.fixture(scope='function') def sample_template( notify_db, @@ -226,7 +224,9 @@ def sample_template( if user is None: user = create_user() if service is None: - service = sample_service(notify_db, notify_db_session, permissions=permissions) + service = Service.query.filter_by(name='Sample service').first() + if not service: + service = create_service(service_permissions=permissions) if created_by is None: created_by = create_user() diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index e7068dc9f..f2077b786 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -7,33 +7,39 @@ from freezegun import freeze_time from moto import mock_s3 from app.letters.utils import ( - get_bucket_prefix_for_notification, + get_bucket_name_and_prefix_for_notification, get_letter_pdf_filename, get_letter_pdf, upload_letter_pdf, ScanErrorType, move_failed_pdf, get_folder_name ) -from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEST, PRECOMPILED_TEMPLATE_NAME +from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEST, PRECOMPILED_TEMPLATE_NAME, NOTIFICATION_VALIDATION_FAILED from tests.app.db import create_notification FROZEN_DATE_TIME = "2018-03-14 17:00:00" -@pytest.fixture() -def sample_precompiled_letter_notification_using_test_key(sample_letter_notification): +@pytest.fixture(name='sample_precompiled_letter_notification') +def _sample_precompiled_letter_notification(sample_letter_notification): sample_letter_notification.template.hidden = True sample_letter_notification.template.name = PRECOMPILED_TEMPLATE_NAME - sample_letter_notification.key_type = KEY_TYPE_TEST sample_letter_notification.reference = 'foo' with freeze_time(FROZEN_DATE_TIME): sample_letter_notification.created_at = datetime.utcnow() return sample_letter_notification -def test_get_bucket_prefix_for_notification_valid_notification(sample_notification): +@pytest.fixture(name='sample_precompiled_letter_notification_using_test_key') +def _sample_precompiled_letter_notification_using_test_key(sample_precompiled_letter_notification): + sample_precompiled_letter_notification.key_type = KEY_TYPE_TEST + return sample_precompiled_letter_notification - bucket_prefix = get_bucket_prefix_for_notification(sample_notification) +def test_get_bucket_name_and_prefix_for_notification_valid_notification(sample_notification): + + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification(sample_notification) + + assert bucket == current_app.config['LETTERS_PDF_BUCKET_NAME'] assert bucket_prefix == '{folder}/NOTIFY.{reference}'.format( folder=sample_notification.created_at.date(), reference=sample_notification.reference @@ -41,19 +47,44 @@ def test_get_bucket_prefix_for_notification_valid_notification(sample_notificati @freeze_time(FROZEN_DATE_TIME) -def test_get_bucket_prefix_for_notification_precompiled_letter_using_test_key( +def test_get_bucket_name_and_prefix_for_notification_precompiled_letter_using_test_key( sample_precompiled_letter_notification_using_test_key ): - bucket_prefix = get_bucket_prefix_for_notification( - sample_precompiled_letter_notification_using_test_key, is_test_letter=True) + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification( + sample_precompiled_letter_notification_using_test_key) + assert bucket == current_app.config['TEST_LETTERS_BUCKET_NAME'] assert bucket_prefix == 'NOTIFY.{}'.format( sample_precompiled_letter_notification_using_test_key.reference).upper() -def test_get_bucket_prefix_for_notification_invalid_notification(): +@freeze_time(FROZEN_DATE_TIME) +def test_get_bucket_name_and_prefix_for_failed_validation(sample_precompiled_letter_notification): + sample_precompiled_letter_notification.status = NOTIFICATION_VALIDATION_FAILED + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification(sample_precompiled_letter_notification) + + assert bucket == current_app.config['INVALID_PDF_BUCKET_NAME'] + assert bucket_prefix == 'NOTIFY.{}'.format( + sample_precompiled_letter_notification.reference).upper() + + +@freeze_time(FROZEN_DATE_TIME) +def test_get_bucket_name_and_prefix_for_test_noti_with_failed_validation( + sample_precompiled_letter_notification_using_test_key +): + sample_precompiled_letter_notification_using_test_key.status = NOTIFICATION_VALIDATION_FAILED + bucket, bucket_prefix = get_bucket_name_and_prefix_for_notification( + sample_precompiled_letter_notification_using_test_key + ) + + assert bucket == current_app.config['INVALID_PDF_BUCKET_NAME'] + assert bucket_prefix == 'NOTIFY.{}'.format( + sample_precompiled_letter_notification_using_test_key.reference).upper() + + +def test_get_bucket_name_and_prefix_for_notification_invalid_notification(): with pytest.raises(AttributeError): - get_bucket_prefix_for_notification(None) + get_bucket_name_and_prefix_for_notification(None) @pytest.mark.parametrize('crown_flag,expected_crown_text', [ diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index b97f4ef81..9d8ccee94 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -346,15 +346,15 @@ def test_reset_failed_login_count_returns_404_when_user_does_not_exist(client): @pytest.mark.parametrize('data, expected_auth_url', ( ( {}, - 'http://localhost:6012/email-auth/.', + 'http://localhost:6012/email-auth/%2E', ), ( {'to': None}, - 'http://localhost:6012/email-auth/.', + 'http://localhost:6012/email-auth/%2E', ), ( {'to': None, 'email_auth_link_host': 'https://example.com'}, - 'https://example.com/email-auth/.', + 'https://example.com/email-auth/%2E', ), )) def test_send_user_email_code( diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 4db097ef3..8bbe13db5 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -437,7 +437,7 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no assert len(json_response['errors']) == 1 assert json_response['errors'][0]['message'] == "status elephant is not one of [cancelled, created, sending, " \ "sent, delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, " \ - "pending-virus-check, virus-scan-failed, returned-letter, accepted, received]" + "pending-virus-check, validation-failed, virus-scan-failed, returned-letter, accepted, received]" def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index cc2765789..006c3f4d3 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -44,7 +44,7 @@ def test_get_notifications_request_invalid_statuses( partial_error_status = "is not one of " \ "[cancelled, created, sending, sent, delivered, pending, failed, " \ "technical-failure, temporary-failure, permanent-failure, pending-virus-check, " \ - "virus-scan-failed, returned-letter, accepted, received]" + "validation-failed, virus-scan-failed, returned-letter, accepted, received]" with pytest.raises(ValidationError) as e: validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) @@ -92,7 +92,7 @@ def test_get_notifications_request_invalid_statuses_and_template_types(): for invalid_status in ["elephant", "giraffe"]: assert "status {} is not one of [cancelled, created, sending, sent, delivered, " \ "pending, failed, technical-failure, temporary-failure, permanent-failure, " \ - "pending-virus-check, virus-scan-failed, returned-letter, accepted, received]".format( + "pending-virus-check, validation-failed, virus-scan-failed, returned-letter, accepted, received]".format( invalid_status ) in error_messages