Merge pull request #76 from alphagov/upload-csv-s3

Upload csv s3

app/main/uploader.py

@@ -0,0 +1,15 @@
+import os
+import uuid
+
+from boto3 import resource
+
+
+# TODO add service name to bucket name as well
+def s3upload(filepath):
+    filename = filepath.split(os.path.sep)[-1]
+    upload_id = str(uuid.uuid4())
+    s3 = resource('s3')
+    s3.create_bucket(Bucket=upload_id)
+    key = s3.Object(upload_id, filename)
+    key.put(Body=open(filepath, 'rb'), ServerSideEncryption='AES256')
+    return upload_id

app/main/views/jobs.py

@@ -1,11 +1,14 @@
 # -*- coding: utf-8 -*-
 
 import time
-from flask import render_template
+
+from flask import (
+    render_template,
+    session
+)
 from flask_login import login_required
 
 from app.main import main
-
 from ._jobs import jobs
 
 now = time.strftime('%H:%M')
@@ -55,6 +58,11 @@ def showjobs(service_id):
 @main.route("/services/<int:service_id>/jobs/<job_id>")
 @login_required
 def showjob(service_id, job_id):
+
+    # TODO the uploaded file name could be part of job definition
+    # so won't need to be passed on from last view via session
+    uploaded_file_name = session.get(job_id)
+
     return render_template(
         'views/job.html',
         messages=messages,
@@ -68,7 +76,7 @@ def showjob(service_id, job_id):
             ])
         },
         cost=u'£0.00',
-        uploaded_file_name='dispatch_20151114.csv',
+        uploaded_file_name=uploaded_file_name,
         uploaded_file_time=now,
         template_used='Test message 1',
         flash_message=u'We’ve started sending your messages',

app/main/views/sms.py

@@ -1,5 +1,8 @@
 import csv
 import re
+import os
+
+from datetime import date
 
 from flask import (
     request,
@@ -8,13 +11,16 @@ from flask import (
     url_for,
     session,
     flash,
-    current_app
+    current_app,
+    abort
 )
 
 from flask_login import login_required
+from werkzeug import secure_filename
 
 from app.main import main
 from app.main.forms import CsvUploadForm
+from app.main.uploader import s3upload
 
 # TODO move this to the templates directory
 message_templates = [
@@ -40,25 +46,25 @@ message_templates = [
 def sendsms(service_id):
     form = CsvUploadForm()
     if form.validate_on_submit():
-        csv_file = form.file.data
-
-        # in memory handing is temporary until next story to save csv file
         try:
-            results = _build_upload_result(csv_file)
+            csv_file = form.file.data
-        except Exception:
+            filename = _format_filename(csv_file.filename)
-            message = 'There was a problem with the file: {}'.format(
+            filepath = os.path.join(current_app.config['UPLOAD_FOLDER'],
+                                    filename)
+            csv_file.save(filepath)
+            _check_file(csv_file.filename, filepath)
+            return redirect(url_for('.checksms',
+                                    service_id=service_id,
+                                    recipients=filename))
+        except (IOError, ValueError) as e:
+            message = 'There was a problem uploading: {}'.format(
                       csv_file.filename)
             flash(message)
+            if isinstance(e, ValueError):
+                flash(str(e))
+            os.remove(filepath)
             return redirect(url_for('.sendsms', service_id=service_id))
 
-        if not results['valid'] and not results['rejects']:
-            message = "The file {} contained no data".format(csv_file.filename)
-            flash(message, 'error')
-            return redirect(url_for('.sendsms', service_id=service_id))
-
-        session[csv_file.filename] = results
-        return redirect(url_for('.checksms', service_id=service_id, recipients=csv_file.filename))
-
     return render_template('views/send-sms.html',
                            message_templates=message_templates,
                            form=form,
@@ -69,33 +75,68 @@ def sendsms(service_id):
 @login_required
 def checksms(service_id):
     if request.method == 'GET':
-
+        filename = request.args.get('recipients')
-        recipients_filename = request.args.get('recipients')
+        if not filename:
-        # upload results in session until file is persisted in next story
+            abort(400)
-        upload_result = session.get(recipients_filename)
+        filepath = os.path.join(current_app.config['UPLOAD_FOLDER'],
+                                filename)
+        upload_result = _build_upload_result(filepath)
         if upload_result.get('rejects'):
             flash('There was a problem with some of the numbers')
 
         return render_template(
             'views/check-sms.html',
             upload_result=upload_result,
+            filename=filename,
             message_template=message_templates[0]['body'],
             service_id=service_id
         )
     elif request.method == 'POST':
-        return redirect(url_for('.showjob', service_id=service_id, job_id=456))
+        filename = request.form['recipients']
+        filepath = os.path.join(current_app.config['UPLOAD_FOLDER'],
+                                filename)
+        try:
+            upload_id = s3upload(filepath)
+            # TODO when job is created record filename in job itself
+            # so downstream pages can get the original filename that way
+            session[upload_id] = filename
+            return redirect(url_for('main.showjob', service_id=service_id, job_id=upload_id))
+        except:
+            flash('There as a problem saving the file')
+            return redirect(url_for('.checksms', recipients=filename))
+
+
+def _check_file(filename, filepath):
+    if os.stat(filepath).st_size == 0:
+        message = 'The file {} contained no data'.format(filename)
+        raise ValueError(message)
+
+
+def _format_filename(filename):
+    d = date.today()
+    basename, extenstion = filename.split('.')
+    formatted_name = '{}_{}.csv'.format(basename, d.strftime('%Y%m%d'))
+    return secure_filename(formatted_name)
+
+
+def _open(file):
+    return open(file, 'r')
 
 
 def _build_upload_result(csv_file):
-    pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$')
+    try:
-    reader = csv.DictReader(
+        file = _open(csv_file, 'r')
-        csv_file.read().decode('utf-8').splitlines(),
+        pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$')
-        lineterminator='\n',
+        reader = csv.DictReader(
-        quoting=csv.QUOTE_NONE)
+            file.read().splitlines(),
-    valid, rejects = [], []
+            lineterminator='\n',
-    for i, row in enumerate(reader):
+            quoting=csv.QUOTE_NONE)
-        if pattern.match(row['phone']):
+        valid, rejects = [], []
-            valid.append(row)
+        for i, row in enumerate(reader):
-        else:
+            if pattern.match(row['phone']):
-            rejects.append({"line_number": i+2, "phone": row['phone']})
+                valid.append(row)
-    return {"valid": valid, "rejects": rejects}
+            else:
+                rejects.append({"line_number": i+2, "phone": row['phone']})
+        return {"valid": valid, "rejects": rejects}
+    finally:
+        file.close()

app/templates/views/check-sms.html

@@ -59,6 +59,8 @@
 					back_link = url_for(".sendsms", service_id=service_id)
 			)}}
 
+			<input type='hidden' name='recipients' value='{{filename}}'>
+
 		</form>
 	{% endif %}
 {% endblock %}

config.py

@@ -32,6 +32,7 @@ class Config(object):
     TOKEN_MAX_AGE_SECONDS = 3600
 
     MAX_CONTENT_LENGTH = 10 * 1024 * 1024  # 10mb
+    UPLOAD_FOLDER = '/tmp'
 
 
 class Development(Config):

requirements.txt

@@ -9,6 +9,7 @@ Flask-WTF==0.11
 Flask-Login==0.2.11
 Flask-Bcrypt==0.6.2
 credstash==1.8.0
+boto3==1.2.3
 
 git+https://github.com/alphagov/notify-api-client.git@0.1.4#egg=notify-api-client==0.1.4
 

tests/app/main/views/test_jobs.py

@@ -16,9 +16,12 @@ def test_should_return_list_of_all_jobs(notifications_admin, notifications_admin
 def test_should_show_page_for_one_job(notifications_admin, notifications_admin_db, notify_db_session):
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
-            user = create_test_user('active')
+            # TODO filename will be part of job metadata not in session
-            client.login(user)
+            with client.session_transaction() as s:
-            response = client.get('/services/123/jobs/456')
+                s[456] = 'dispatch_20151114.csv'
+        user = create_test_user('active')
+        client.login(user)
+        response = client.get('/services/123/jobs/456')
 
         assert response.status_code == 200
         assert 'dispatch_20151114.csv' in response.get_data(as_text=True)

tests/app/main/views/test_sms.py

@@ -1,16 +1,22 @@
 from io import BytesIO
+from unittest import mock
+from unittest.mock import mock_open
+
 from tests.app.main import create_test_user
 
 
-def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifications_admin_db, notify_db_session):
+def test_upload_empty_csvfile_returns_to_upload_page(
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    _setup_mocker_for_empty_file(mocker)
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-
             upload_data = {'file': (BytesIO(''.encode('utf-8')), 'emtpy.csv')}
-            response = client.post('/services/123/sms/send', data=upload_data,
+            response = client.post('/services/123/sms/send',
-                                   follow_redirects=True)
+                                   data=upload_data, follow_redirects=True)
 
         assert response.status_code == 200
         content = response.get_data(as_text=True)
@@ -18,19 +24,26 @@ def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifi
 
 
 def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(
-        notifications_admin, notifications_admin_db, notify_db_session):
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    contents = 'phone\n+44 123\n+44 456'
+    file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv')
+    m_open = mock_open(read_data=contents)
+    _setup_mocker_for_nonemtpy_file(mocker)
 
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-
+            upload_data = {'file': file_data}
-            file_contents = 'phone\n+44 123\n+44 456'.encode('utf-8')
+            with mock.patch('app.main.views.sms._open', m_open):
-            upload_data = {'file': (BytesIO(file_contents), 'invalid.csv')}
+                response = client.post('/services/123/sms/send',
-            response = client.post('/services/123/sms/send', data=upload_data,
+                                       data=upload_data,
-                                   follow_redirects=True)
+                                       follow_redirects=True)
         assert response.status_code == 200
         content = response.get_data(as_text=True)
+
         assert 'There was a problem with some of the numbers' in content
         assert 'The following numbers are invalid' in content
         assert '+44 123' in content
@@ -39,17 +52,24 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(
 
 
 def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers(
-        notifications_admin, notifications_admin_db, notify_db_session):
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989'  # noqa
+
+    file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv')
+    m_open = mock_open(read_data=contents)
+    _setup_mocker_for_nonemtpy_file(mocker)
 
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-            file_contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989'.encode('utf-8')  # noqa
+            upload_data = {'file': file_data}
-
+            with mock.patch('app.main.views.sms._open', m_open):
-            upload_data = {'file': (BytesIO(file_contents), 'valid.csv')}
+                response = client.post('/services/123/sms/send',
-            response = client.post('/services/123/sms/send', data=upload_data,
+                                       data=upload_data,
-                                   follow_redirects=True)
+                                       follow_redirects=True)
 
         content = response.get_data(as_text=True)
 
@@ -69,18 +89,24 @@ def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers(
 
 
 def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers(
-        notifications_admin, notifications_admin_db, notify_db_session):
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'  # noqa
+
+    file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv')
+    m_open = mock_open(read_data=contents)
+    _setup_mocker_for_nonemtpy_file(mocker)
 
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-
+            upload_data = {'file': file_data}
-            file_contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'.encode('utf-8')  # noqa
+            with mock.patch('app.main.views.sms._open', m_open):
-
+                response = client.post('/services/123/sms/send',
-            upload_data = {'file': (BytesIO(file_contents), 'valid.csv')}
+                                       data=upload_data,
-            response = client.post('/services/123/sms/send', data=upload_data,
+                                       follow_redirects=True)
-                                   follow_redirects=True)
 
         content = response.get_data(as_text=True)
 
@@ -96,13 +122,33 @@ def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers(
 
 
 def test_should_redirect_to_job(notifications_admin, notifications_admin_db,
-                                notify_db_session):
+                                notify_db_session, mocker):
+    _setup_mocker_for_check(mocker)
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
+            with client.session_transaction() as s:
+                s[456] = 'test.csv'
 
-        response = client.post('/services/123/sms/check')
+        response = client.post('/services/123/sms/check',
+                               data={'recipients': 'test.csv'})
 
         assert response.status_code == 302
-        assert response.location == 'http://localhost/services/123/jobs/456'
+
+
+def _setup_mocker_for_empty_file(mocker):
+    mocker.patch('werkzeug.datastructures.FileStorage.save')
+    mocker.patch('os.remove')
+    ret = ValueError('The file emtpy.csv contained no data')
+    mocker.patch('app.main.views.sms._check_file', side_effect=ret)
+
+
+def _setup_mocker_for_nonemtpy_file(mocker):
+    mocker.patch('werkzeug.datastructures.FileStorage.save')
+    mocker.patch('os.remove')
+    mocker.patch('app.main.views.sms._check_file')
+
+
+def _setup_mocker_for_check(mocker):
+    mocker.patch('app.main.views.sms.s3upload').return_value = 456