Clean up leftover file path references

This change removes and direct references to the filesystem from the
pyfbin code other that file_storage.filesystem. This should hopyfully be
the last changes needed for pyfbin to be successfully run using the
file_storage.s3 module without any issues.

4 files changed, 42 insertions, 70 deletions

diff --git a/fbin-scanner.py b/fbin-scanner.py
index 76bc33b..05f8739 100644
--- a/fbin-scanner.py
+++ b/fbin-scanner.py
@@ -83,10 +83,10 @@ def main():
     files = deque(db.session.query(File).filter(File.scanned == False).all())
     while len(files):
         dbfile = files.pop()
-        if not dbfile.get_size():
+        if not dbfile.size:
             logger.info('Ignoring file %s/%s due to unknown size', dbfile.filename, dbfile.hash)
             continue
-        if dbfile.get_size() > 32*10**6:
+        if dbfile.size > 32*10**6:
             logger.info('Ignoring file %s/%s due to size (%s)', dbfile.filename, dbfile.hash, dbfile.formatted_size)
             continue
         logger.info('Checking file %s/%s (%s)', dbfile.filename, dbfile.hash, dbfile.formatted_size)
diff --git a/fbin/db.py b/fbin/db.py
index 876efeb..53b0d8b 100644
--- a/fbin/db.py
+++ b/fbin/db.py
@@ -71,23 +71,9 @@ class File(db.Model):
                 continue
         return '{} B'.format(size)
 
-    def get_path(self):
-        return os.path.join(current_app.config['FILE_DIRECTORY'], self.hash + os.path.splitext(self.filename)[1])
-
-    def get_thumb_path(self):
-        return os.path.join(current_app.config['THUMB_DIRECTORY'], self.hash + '.jpg')
-
-    def get_size(self):
-        try:
-            if self.size:
-                return self.size
-            return os.path.getsize(self.get_path())
-        except OSError:
-            return None
-
     @property
     def formatted_size(self):
-        return self.pretty_size(self.get_size())
+        return self.pretty_size(self.size)
 
     @property
     def formatted_date(self):
@@ -105,7 +91,3 @@ class File(db.Model):
     @property
     def ext(self):
         return os.path.splitext(self.filename)[1]
-
-    @property
-    def exists(self):
-        return os.path.exists(self.get_path())
diff --git a/fbin/fbin.py b/fbin/fbin.py
index ac4569e..04857d9 100755
--- a/fbin/fbin.py
+++ b/fbin/fbin.py
@@ -36,14 +36,6 @@ base62_alphabet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXY
 if not os.path.isdir(current_app.config['THUMB_DIRECTORY']):
     os.mkdir(current_app.config['THUMB_DIRECTORY'])
 
-try:
-    # Throws OSError if mogrify doesn't exist.
-    subprocess.call(['mogrify', '-quiet'])
-except OSError:
-    has_mogrify = False
-else:
-    has_mogrify = True
-
 def get_or_create_user(username, jab_id):
     try:
         return db.session.query(User).filter(User.jab_id == jab_id).one()
@@ -84,11 +76,7 @@ def get_files(user):
 def delete_file(file):
     db.session.delete(file)
     db.session.commit()
-    filename = file.get_path()
     storage.delete_file(file)
-    thumbfile = file.get_thumb_path()
-    if os.path.exists(thumbfile):
-        os.unlink(thumbfile)
 
 app = Blueprint('fbin', __name__)
 
@@ -138,14 +126,6 @@ def upload(api=False, user=None):
     except StorageError as e:
         return error(str(e))
 
-    mime = new_file.get_mime_type()
-    # TODO: Apparently TIFF also supports EXIF, test this.
-    if has_mogrify and mime == 'image/jpeg':
-        # NOTE: PIL doesn't support lossless rotation, so we call mogrify to do this.
-        # NOTE: This changes the file, so the file_hash applies to the ORIGINAL file contents only.
-        # NOTE: The file hash is only used to detect duplicates when uploading, so this should not be a problem.
-        subprocess.call(['mogrify', '-auto-orient', new_file.get_path()])
-
     if api:
         return jsonify({
             'status': True,
@@ -196,7 +176,7 @@ def _file(hash, ext=None, filename=None):
         return path
     if not path or not os.path.exists(path):
         abort(404)
-    return send_file(path)
+    return send_file(path, attachment_filename=f.filename)
 
 @app.route('/l')
 @app.route('/login')
@@ -287,7 +267,7 @@ def files():
     context = {
         'title': 'Files',
         'files': files,
-        'total_size': File.pretty_size(sum(size for size in (f.get_size() for f in files) if size is not None)),
+        'total_size': File.pretty_size(sum(size for size in (f.size for f in files) if size is not None)),
     }
     return render_template('files.html', **context)
 
@@ -300,22 +280,8 @@ def file_edit():
         flash('File not found.', 'error')
         return redirect(url_for('.files'))
     if 'filename' in request.form:
-        old_path = f.get_path()
         filename = request.form.get('filename', f.filename)
         f.filename = filename
-        new_path = f.get_path()
-        # If extension changed, the local filename also changes. We could just store the file without the extension,
-        # but that would break the existing files, requiring a manual rename.
-        if old_path != new_path:
-            try:
-                if os.path.exists(new_path):
-                    # This shouldn't happen unless we have two files with the same hash, which should be impossible.
-                    raise RuntimeError()
-                else:
-                    os.rename(old_path, new_path)
-            except:
-                flash(Markup('Internal rename failed; file may have become unreachable. '
-                'Please contact an admin and specify <strong>hash={}</strong>.'.format(f.hash)), 'error')
         db.session.add(f)
         flash('Filename changed to "{}".'.format(f.filename), 'success')
     elif 'delete' in request.form:
@@ -338,7 +304,7 @@ def images():
         'title': 'Images',
         'fullwidth': True,
         'files': files,
-        'total_size': File.pretty_size(sum(size for size in (f.get_size() for f in files) if size is not None)),
+        'total_size': File.pretty_size(sum(size for size in (f.size for f in files) if size is not None)),
     }
     return render_template('images.html', **context)
 
@@ -351,7 +317,7 @@ def videos():
         'title': 'Videos',
         'fullwidth': True,
         'files': files,
-        'total_size': File.pretty_size(sum(size for size in (f.get_size() for f in files) if size is not None)),
+        'total_size': File.pretty_size(sum(size for size in (f.size for f in files) if size is not None)),
     }
     return render_template('images.html', **context)
 
diff --git a/fbin/file_storage/filesystem.py b/fbin/file_storage/filesystem.py
index 7951d88..87d908e 100644
--- a/fbin/file_storage/filesystem.py
+++ b/fbin/file_storage/filesystem.py
@@ -10,6 +10,24 @@ class Storage(BaseStorage):
         os.makedirs(self.app.config['FILE_DIRECTORY'], exist_ok=True)
         os.makedirs(self.app.config['THUMB_DIRECTORY'], exist_ok=True)
 
+    def _get_legacy_path(self, file_hash, filename):
+        return os.path.join(self.app.config['FILE_DIRECTORY'], file_hash + os.path.splitext(filename)[1])
+
+    def _get_path(self, file_hash):
+        return os.path.join(self.app.config['FILE_DIRECTORY'], file_hash)
+
+    def _find_path(self, f):
+        path = self._get_legacy_path(f.hash, f.filename)
+        if path and os.path.exists(path):
+            return path
+        path = self._get_path(f.hash)
+        if path and os.path.exists(path):
+            return path
+        return None
+
+    def get_thumb_path(self, f):
+        return os.path.join(self.app.config['THUMB_DIRECTORY'], f.hash + '.jpg')
+
     def upload_file(self, uploaded_file, file_hash, user):
         size = uploaded_file.content_length
         if hasattr(uploaded_file.stream, 'file'):
@@ -20,7 +38,7 @@ class Storage(BaseStorage):
             uploaded_file.save(temp.file)
             temp_path = temp.name
             size = os.path.getsize(temp_path)
-        new_path = os.path.join(self.app.config['FILE_DIRECTORY'], file_hash + os.path.splitext(uploaded_file.filename)[1])
+        new_path = self._get_path(file_hash)
         os.rename(temp_path, new_path)
         if self.app.config.get('DESTINATION_MODE'):
             os.chmod(new_path, self.app.config.get('DESTINATION_MODE'))
@@ -36,33 +54,39 @@ class Storage(BaseStorage):
             raise
 
     def file_exists(self, f):
-        path = f.get_path()
-        return os.path.exists(path)
+        path = self._find_path(f)
+        return path and os.path.exists(path)
 
     def get_file(self, f):
-        path = f.get_path()
-        if not os.path.exists(path):
+        path = self._find_path(f)
+        if not path or not os.path.exists(path):
             return
         return path
 
     def delete_file(self, f):
-        path = f.get_path()
-        if os.path.exists(path):
+        path = self._find_path(f)
+        if path and os.path.exists(path):
             os.unlink(path)
+        thumb_path = self.get_thumb_path(f)
+        if thumb_path and os.path.exists(thumb_path):
+            os.unlink(thumb_path)
 
     @contextlib.contextmanager
     def temp_file(self, f):
-        with open(f.get_path(), 'rb') as f:
+        path = self._find_path(f)
+        if not path or not os.path.exists(path):
+            raise FileNotFoundError(path)
+        with open(path, 'rb') as f:
             yield f
 
     def get_thumbnail(self, f):
-        path = f.get_thumb_path()
-        if not os.path.exists(path):
+        path = self.get_thumb_path(f)
+        if not path or not os.path.exists(path):
             return
         return path
 
     def store_thumbnail(self, f, stream):
-        path = f.get_thumb_path()
+        path = self.get_thumb_path(f)
         with open(path, 'wb') as f:
             buf = stream.read(1024*10)
             while buf: