Refactor object_storage module to annotate with current Flask app. Up…

…date song/artwork upload code. Use test bucket in test and remove code that creates test data directories

rainfall/blueprint/file_blueprint_test.py

@@ -26,7 +26,7 @@ def test_delete_file(self, app, releases_user):
 
       rv = client.delete(f'/api/v1/file/{str(file_id)}')
 
-      assert rv.status == '204 NO CONTENT'
+      assert rv.status == '204 NO CONTENT', rv.json
 
     with app.app_context():
       updated_user = db.session.get(User, BASIC_USER_ID)

rainfall/blueprint/release.py

@@ -4,6 +4,7 @@
 
 import flask
 
+from rainfall import object_storage
 from rainfall.db import db
 from rainfall.decorators import with_current_user, with_validated_release
 from rainfall.models.release import Release
@@ -51,15 +52,6 @@ def create_release(user):
         error=f'A release with the name "{release.name}" already exists'), 400
 
   site.releases.append(release)
-  cur_release_path = release_path(flask.current_app.config['DATA_DIR'], release)
-  try:
-    os.makedirs(cur_release_path)
-  except FileExistsError:
-    db.session.rollback()
-    return flask.jsonify(
-        status=500,
-        error='Could not create that release (filesystem error)'), 500
-
   db.session.add(site)
   db.session.commit()
   return '', 204
@@ -132,8 +124,7 @@ def delete_release(release, user):
 
   try:
     path = release_path(flask.current_app.config['DATA_DIR'], release)
-    client = object_storage.connect(flask.current_app)
-    object_storage.rmtree(client, 'rainfall-data', path)
+    object_storage.rmtree(path)
   except Exception as e:
     db.session.rollback()
     return flask.jsonify(

rainfall/blueprint/release_test.py

@@ -3,6 +3,7 @@
 
 from uuid_extensions import uuid7
 
+from rainfall import object_storage
 from rainfall.conftest import BASIC_USER_ID
 from rainfall.db import db
 from rainfall.models.artwork import Artwork
@@ -25,7 +26,7 @@ def test_create(self, app, site_id):
               'name': 'Release Create Test',
               'site_id': site_id
           }})
-      assert rv.status == '204 NO CONTENT'
+      assert rv.status == '204 NO CONTENT', rv.json
 
     with app.app_context():
       user = db.session.get(User, BASIC_USER_ID)
@@ -301,23 +302,26 @@ def test_update_release_name_duplicate(self, mock_release_rename, app,
   def test_delete_release(self, app, releases_user):
     with app.app_context(), app.test_client() as client:
       db.session.add(releases_user)
-      release = releases_user.sites[0].releases[0]
+      release = releases_user.sites[0].releases[1]
       release_id = release.id
       files = release.files
 
+      object_client = object_storage.connect(app)
       with client.session_transaction() as sess:
         sess['user_id'] = BASIC_USER_ID
 
       rv = client.delete(f'/api/v1/release/{release_id}')
-      assert rv.status == '204 NO CONTENT'
+
+      assert rv.status == '204 NO CONTENT', rv.json
       assert release not in db.session
       for file in files:
         assert file not in db.session
-        assert not os.path.exists(file.path)
       assert release not in releases_user.sites[0].releases
+      assert not any(
+          object_client.list_objects(app.config['MINIO_BUCKET'],
+                                     prefix=release_path(
+                                         app.config['DATA_DIR'], release)))
       assert db.session.get(Release, release_id) is None
-      assert release_path(app.config['DATA_DIR'],
-                          release) not in os.listdir(app.config['DATA_DIR'])
 
   def test_delete_release_not_exist(self, app, basic_user):
     with app.app_context(), app.test_client() as client:
@@ -327,14 +331,15 @@ def test_delete_release_not_exist(self, app, basic_user):
       rv = client.delete(f'/api/v1/release/{uuid7()}')
       assert rv.status == '404 NOT FOUND'
 
-  @patch('rainfall.blueprint.release.shutil.rmtree')
+  @patch('rainfall.blueprint.release.object_storage.rmtree')
   def test_delete_release_filesystem_error(self, mock_rmtree, app,
                                            releases_user):
     with app.app_context(), app.test_client() as client:
       db.session.add(releases_user)
       release = releases_user.sites[0].releases[0]
       release_id = release.id
-      mock_rmtree.side_effect = Exception('Filesystem error')
+      mock_rmtree.side_effect = object_storage.ObjectStorageException(
+          'Test error')
 
       with client.session_transaction() as sess:
         sess['user_id'] = BASIC_USER_ID
@@ -347,8 +352,3 @@ def test_delete_release_filesystem_error(self, mock_rmtree, app,
       assert 'error' in rv.json
       assert db.session.get(Release, release_id) is not None
       assert release in releases_user.sites[0].releases
-
-      release_dir_name = os.path.basename(
-          release_path(app.config['DATA_DIR'], release))
-      site_data_path = site_path(app.config['DATA_DIR'], release.site)
-      assert release_dir_name in os.listdir(site_data_path)

rainfall/blueprint/site.py

@@ -4,11 +4,11 @@
 
 import flask
 
+from rainfall import object_storage
 from rainfall.db import db
-from rainfall.decorators import with_current_user, with_current_site
+from rainfall.decorators import with_current_site, with_current_user
 from rainfall.models.site import Site
-from rainfall import object_storage
-from rainfall.site import site_path, rename_site_dir
+from rainfall.site import rename_site_dir, site_path
 
 site = flask.Blueprint('site', __name__)
 
@@ -67,10 +67,8 @@ def delete_site(site, user):
 
   try:
     path = site_path(flask.current_app.config['DATA_DIR'], site)
-    client = object_storage.connect(flask.current_app)
-    object_storage.rmtree(client, 'rainfall-data', path)
+    object_storage.rmtree(path)
   except Exception as e:
-    print(e)
     db.session.rollback()
     return flask.jsonify(status=500,
                          error='Could not delete site (filesystem error)'), 500

rainfall/blueprint/site_test.py

@@ -5,10 +5,11 @@
 import pytest
 from uuid_extensions import uuid7
 
+from rainfall import object_storage
 from rainfall.conftest import BASIC_USER_ID
 from rainfall.db import db
-from rainfall.models.user import User
 from rainfall.models.site import Site
+from rainfall.models.user import User
 from rainfall.site import site_path
 
 
@@ -178,8 +179,7 @@ def test_delete_site(self, app, sites_user):
         sess['user_id'] = BASIC_USER_ID
 
       rv = client.delete(f'/api/v1/site/{site_id}')
-      print(rv.json)
-      assert rv.status == '204 NO CONTENT'
+      assert rv.status == '204 NO CONTENT', rv.json
 
       site = db.session.get(Site, site_id)
       assert site is None
@@ -202,12 +202,13 @@ def test_delete_site_no_site(self, app, sites_user):
       rv = client.delete(f'/api/v1/site/{uuid7()}')
       assert rv.status == '404 NOT FOUND'
 
-  @patch('rainfall.blueprint.site.shutil.rmtree')
+  @patch('rainfall.blueprint.site.object_storage.rmtree')
   def test_delete_site_filesystem_error(self, mock_rmtree, app, sites_user):
     with app.app_context(), app.test_client() as client:
       db.session.add(sites_user)
       site_id = sites_user.sites[0].id
 
+      object_client = object_storage.connect(app)
       with client.session_transaction() as sess:
         sess['user_id'] = BASIC_USER_ID
 
@@ -218,6 +219,3 @@ def test_delete_site_filesystem_error(self, mock_rmtree, app, sites_user):
 
       site = db.session.get(Site, site_id)
       assert site is not None
-      site_basename = os.path.basename(site_path(app.config['DATA_DIR'], site))
-      assert site_basename in os.listdir(
-          os.path.join(app.config['DATA_DIR'], str(site.user.id)))

rainfall/blueprint/upload.py

@@ -3,13 +3,14 @@
 
 import flask
 
+from rainfall import object_storage
 from rainfall.db import db
 from rainfall.decorators import with_current_user, with_validated_release
 from rainfall.models.artwork import Artwork
 from rainfall.models.file import File
 from rainfall.models.release import Release
-from rainfall import object_storage
-from rainfall.site import delete_file as delete_db_file, release_path, secure_filename
+from rainfall.site import delete_file as delete_db_file
+from rainfall.site import release_path, secure_filename
 
 upload = flask.Blueprint('upload', __name__)
 
@@ -34,23 +35,23 @@ def allowed_file(filename):
 
 
 def write_files(release, claz, *files):
-  for song in files:
-    name = secure_filename(song.filename)
+  for file in files:
+    name = secure_filename(file.filename)
     if len(name) > 1024:
       return flask.jsonify(status=400,
                            error=f'File name {name} is too long'), 400
 
-    file = claz(filename=name)
-    # Yield so that the calling function can properly save file to db.
-    yield file
+    obj = claz(filename=name)
+    # Yield so that the calling function can properly save metadata to db.
+    yield obj
 
-    # Write the file to the filesystem.
+    # Write the file to object storage.
     cur_release_path = release_path(flask.current_app.config['DATA_DIR'],
                                     release)
     object_path = os.path.join(cur_release_path, file.filename)
-    client = object_storage.connect(flask.current_app)
-    client.put_object('rainfall-data', object_path, song.stream,
-                      song.content_length, song.content_type)
+    file_size = file.seek(0, 2)
+    file.seek(0)  # Reset the file pointer to the beginning.
+    object_storage.put_object(object_path, file, file_size, file.content_type)
 
 
 @upload.route('upload/release/<release_id>/song', methods=['POST'])

rainfall/blueprint/upload_test.py

@@ -1,18 +1,28 @@
-from unittest.mock import patch
 import io
 import os
+import time
+from unittest.mock import patch
 
 import flask
 
-from rainfall.db import db
+from rainfall import object_storage
 from rainfall.conftest import BASIC_USER_ID
+from rainfall.db import db
 from rainfall.models.artwork import Artwork
 from rainfall.site import release_path, secure_filename
 
 
-def assert_file_contents(file_path, contents):
-  with open(file_path, 'r') as f:
-    assert f.read() == contents
+def assert_file_contents(app, path, contents):
+  object_client = object_storage.connect(app)
+  resp = None
+  try:
+    resp = object_client.get_object(app.config['MINIO_BUCKET'], path)
+    if resp:
+      resp_data = resp.read()
+  finally:
+    if resp is not None:
+      resp.close()
+  assert resp_data == contents
 
 
 class UploadTest:
@@ -32,12 +42,9 @@ def test_upload_single_file(self, app, releases_user):
 
       assert rv.status == '204 NO CONTENT', rv.text
 
-      song_path = os.path.join(app.config['DATA_DIR'], str(releases_user.id),
-                               secure_filename(site.name),
-                               secure_filename(release.name),
+      song_path = os.path.join(release_path(app.config['DATA_DIR'], release),
                                secure_filename('song1.wav'))
-      assert_file_contents(song_path, 'not-actually-a-song')
-
+      assert_file_contents(app, song_path, b'not-actually-a-song')
       assert len(release.files) == 1
       assert release.files[0].filename == 'song1.wav'
 
@@ -60,13 +67,13 @@ def test_upload_multiple_files(self, app, releases_user):
 
       assert rv.status == '204 NO CONTENT', rv.text
 
-      release_path = os.path.join(app.config['DATA_DIR'], str(releases_user.id),
-                                  secure_filename(site.name),
-                                  secure_filename(release.name))
-      song_1_path = os.path.join(release_path, secure_filename('song1.wav'))
-      song_2_path = os.path.join(release_path, secure_filename('song2.wav'))
-      assert_file_contents(song_1_path, 'not-actually-a-song')
-      assert_file_contents(song_2_path, 'not-actually-a-song-2')
+      song_release_path = release_path(app.config['DATA_DIR'], release)
+      song_1_path = os.path.join(song_release_path,
+                                 secure_filename('song1.wav'))
+      song_2_path = os.path.join(song_release_path,
+                                 secure_filename('song2.wav'))
+      assert_file_contents(app, song_1_path, b'not-actually-a-song')
+      assert_file_contents(app, song_2_path, b'not-actually-a-song-2')
 
       assert len(release.files) == 2
       assert release.files[0].filename == 'song1.wav'
@@ -138,7 +145,7 @@ def test_upload_release_art(self, app, releases_user):
       release_id = str(release.id)
       rv = client.post(f'/api/v1/upload/release/{release_id}/art',
                        data={
-                           'artwork': (io.BytesIO(b'not-actually-a-song'),
+                           'artwork': (io.BytesIO(b'not-actually-artwork'),
                                        'some_artwork.jpg')
                        })
 
@@ -149,7 +156,7 @@ def test_upload_release_art(self, app, releases_user):
       file_path = os.path.join(
           release_path(flask.current_app.config['DATA_DIR'], release),
           'some_artwork.jpg')
-      assert os.path.exists(file_path)
+      assert_file_contents(app, file_path, b'not-actually-artwork')
 
   def test_upload_release_art_no_file(self, app, releases_user):
     with app.app_context(), app.test_client() as client:
@@ -208,5 +215,4 @@ def test_upload_release_art_delete_existing(self, app, releases_user,
       file_path = os.path.join(
           release_path(flask.current_app.config['DATA_DIR'], release),
           'artwork.jpg')
-      print(file_path)
       assert not os.path.exists(file_path)