Factor out photo storage into PhotoStorage::FileSystem backend

4 files changed, 157 insertions, 49 deletions

diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm
index 160d2851e..afc8bd918 100644
--- a/perllib/FixMyStreet/App.pm
+++ b/perllib/FixMyStreet/App.pm
@@ -129,10 +129,18 @@ __PACKAGE__->log->disable('debug')    #
   unless __PACKAGE__->debug;
 
 # Check upload_dir
-my $cache_dir = path(FixMyStreet->config('UPLOAD_DIR'))->absolute(FixMyStreet->path_to());
-$cache_dir->mkpath;
-unless ( -d $cache_dir && -w $cache_dir ) {
-    warn "\x1b[31mCan't find/write to photo cache directory '$cache_dir'\x1b[0m\n";
+# TODO: Should this check be part of PhotoStorage::FileSystem?
+if (
+    FixMyStreet->config('PHOTO_STORAGE_BACKEND') eq 'FileSystem' ||
+    !defined FixMyStreet->config('PHOTO_STORAGE_BACKEND') # Backwards compatibility
+   ) {
+    my $cache_dir = FixMyStreet->config('PHOTO_STORAGE_OPTIONS')->{UPLOAD_DIR}
+                    || FixMyStreet->config('UPLOAD_DIR');
+    $cache_dir = path($cache_dir)->absolute(FixMyStreet->path_to());
+    $cache_dir->mkpath;
+    unless ( -d $cache_dir && -w $cache_dir ) {
+        warn "\x1b[31mCan't find/write to photo cache directory '$cache_dir'\x1b[0m\n";
+    }
 }
 
 =head1 NAME
diff --git a/perllib/FixMyStreet/App/Model/PhotoSet.pm b/perllib/FixMyStreet/App/Model/PhotoSet.pm
index 8fcc1700e..58e1a135d 100644
--- a/perllib/FixMyStreet/App/Model/PhotoSet.pm
+++ b/perllib/FixMyStreet/App/Model/PhotoSet.pm
@@ -3,7 +3,6 @@ package FixMyStreet::App::Model::PhotoSet;
 # TODO this isn't a Cat model, rename to something else
 
 use Moose;
-use Path::Tiny 'path';
 
 my $IM = eval {
     require Image::Magick;
@@ -12,7 +11,6 @@ my $IM = eval {
 };
 
 use Scalar::Util 'openhandle', 'blessed';
-use Digest::SHA qw(sha1_hex);
 use Image::Size;
 use IPC::Cmd qw(can_run);
 use IPC::Open3;
@@ -57,28 +55,23 @@ has data_items => ( # either a) split from db_data or b) provided by photo uploa
         my $self = shift;
         my $data = $self->db_data or return [];
 
-        return [$data] if (detect_type($data));
+        return [$data] if ($self->storage->detect_type($data));
 
         return [ split ',' => $data ];
     },
 );
 
-has upload_dir => (
+has storage => (
     is => 'ro',
     lazy => 1,
     default => sub {
-        path(FixMyStreet->config('UPLOAD_DIR'))->absolute(FixMyStreet->path_to());
-    },
+        my $class = 'FixMyStreet::PhotoStorage::';
+        $class .= FixMyStreet->config('PHOTO_STORAGE_BACKEND') || 'FileSystem';
+        eval "use $class";
+        return $class->new();
+    }
 );
 
-sub detect_type {
-    return 'jpeg' if $_[0] =~ /^\x{ff}\x{d8}/;
-    return 'png' if $_[0] =~ /^\x{89}\x{50}/;
-    return 'tiff' if $_[0] =~ /^II/;
-    return 'gif' if $_[0] =~ /^GIF/;
-    return '';
-}
-
 =head2 C<ids>, C<num_images>, C<get_id>, C<all_ids>
 
 C<$photoset-E<GT>ids> is an arrayref containing the fileid data.
@@ -166,25 +159,20 @@ has ids => ( #  Arrayref of $fileid tuples (always, so post upload/raw data proc
                     return ();
                 }
 
-                # we have an image we can use - save it to the upload dir for storage
-                my $fileid = $self->get_fileid($photo_blob);
-                my $file = $self->get_file($fileid, $type);
-                $upload->copy_to( $file );
-                return $file->basename;
-
+                # we have an image we can use - save it to storage
+                return $self->storage->store_photo($photo_blob);
             }
-            if (my $type = detect_type($part)) {
+
+            # It might be a raw file stored in the DB column...
+            if (my $type = $self->storage->detect_type($part)) {
                 my $photo_blob = $part;
-                my $fileid = $self->get_fileid($photo_blob);
-                my $file = $self->get_file($fileid, $type);
-                $file->spew_raw($photo_blob);
-                return $file->basename;
+                return $self->storage->store_photo($photo_blob);
+                # TODO: Should this update the DB record with a pointer to the
+                # newly-stored file, instead of leaving it in the DB?
             }
-            my ($fileid, $type) = split /\./, $part;
-            $type ||= 'jpeg';
-            if ($fileid && length($fileid) == 40) {
-                my $file = $self->get_file($fileid, $type);
-                $file->basename;
+
+            if (my $key = $self->storage->validate_key($part)) {
+                $key;
             } else {
                 # A bad hash, probably a bot spamming with bad data.
                 ();
@@ -194,24 +182,11 @@ has ids => ( #  Arrayref of $fileid tuples (always, so post upload/raw data proc
     },
 );
 
-sub get_fileid {
-    my ($self, $photo_blob) = @_;
-    return sha1_hex($photo_blob);
-}
-
-sub get_file {
-    my ($self, $fileid, $type) = @_;
-    my $cache_dir = $self->upload_dir;
-    return path( $cache_dir, "$fileid.$type" );
-}
-
 sub get_raw_image {
     my ($self, $index) = @_;
     my $filename = $self->get_id($index);
-    my ($fileid, $type) = split /\./, $filename;
-    my $file = $self->get_file($fileid, $type);
-    if ($file->exists) {
-        my $photo = $file->slurp_raw;
+    my ($photo, $type) = $self->storage->retrieve_photo($filename);
+    if ($photo) {
         return {
             data => $photo,
             content_type => "image/$type",
diff --git a/perllib/FixMyStreet/PhotoStorage.pm b/perllib/FixMyStreet/PhotoStorage.pm
new file mode 100644
index 000000000..99f0bdab6
--- /dev/null
+++ b/perllib/FixMyStreet/PhotoStorage.pm
@@ -0,0 +1,30 @@
+package FixMyStreet::PhotoStorage;
+
+use Moose;
+use Digest::SHA qw(sha1_hex);
+
+
+sub detect_type {
+    my ($self, $photo) = @_;
+    return 'jpeg' if $photo =~ /^\x{ff}\x{d8}/;
+    return 'png' if $photo =~ /^\x{89}\x{50}/;
+    return 'tiff' if $photo =~ /^II/;
+    return 'gif' if $photo =~ /^GIF/;
+    return '';
+}
+
+=head2 get_fileid
+
+Calculates an identifier for a binary blob of photo data.
+This is just the SHA1 hash of the blob currently.
+
+=cut
+
+sub get_fileid {
+    my ($self, $photo_blob) = @_;
+    return sha1_hex($photo_blob);
+}
+
+
+
+1;
diff --git a/perllib/FixMyStreet/PhotoStorage/FileSystem.pm b/perllib/FixMyStreet/PhotoStorage/FileSystem.pm
new file mode 100644
index 000000000..0600f867d
--- /dev/null
+++ b/perllib/FixMyStreet/PhotoStorage/FileSystem.pm
@@ -0,0 +1,95 @@
+package FixMyStreet::PhotoStorage::FileSystem;
+
+use Moose;
+use parent 'FixMyStreet::PhotoStorage';
+
+use Path::Tiny 'path';
+
+
+has upload_dir => (
+    is => 'ro',
+    lazy => 1,
+    default => sub {
+        my $dir = FixMyStreet->config('PHOTO_STORAGE_OPTIONS')->{UPLOAD_DIR} ||
+                  FixMyStreet->config('UPLOAD_DIR');
+        return path($dir)->absolute(FixMyStreet->path_to());
+    },
+);
+
+
+=head2 get_file
+
+Returns a Path::Tiny path to a file on disk identified by an ID and type.
+File may or may not exist. This handle is then used to read photo data or
+write to disk.
+
+=cut
+
+sub get_file {
+    my ($self, $fileid, $type) = @_;
+    my $cache_dir = $self->upload_dir;
+    return path( $cache_dir, "$fileid.$type" );
+}
+
+
+=head2 store_photo
+
+Stores a blob of binary data representing a photo on disk.
+Returns a key which is used in the future to get the contents of the file.
+
+=cut
+
+sub store_photo {
+    my ($self, $photo_blob) = @_;
+
+    my $type = $self->detect_type($photo_blob) || 'jpeg';
+    my $fileid = $self->get_fileid($photo_blob);
+    my $file = $self->get_file($fileid, $type);
+    $file->spew_raw($photo_blob);
+
+    return $file->basename;
+}
+
+
+=head2 retrieve_photo
+
+Fetches the file content of a particular photo from storage.
+Returns the binary blob and the filetype, if the photo exists in storage.
+
+=cut
+
+sub retrieve_photo {
+    my ($self, $filename) = @_;
+
+    my ($fileid, $type) = split /\./, $filename;
+    my $file = $self->get_file($fileid, $type);
+    if ($file->exists) {
+        my $photo = $file->slurp_raw;
+        return ($photo, $type);
+    }
+}
+
+
+=head2 validate_key
+
+A long-running FMS instance might have reports whose photo IDs in the DB
+don't include the file extension. This function takes a value from the DB and
+returns a 'tidied' version that can be used when calling photo_exists
+or retrieve_photo.
+
+If the passed key doesn't seem like it'll result in a valid filename (i.e.
+it's not a 40-char SHA1 hash) returns undef.
+
+=cut
+
+sub validate_key {
+    my ($self, $key) = @_;
+
+    my ($fileid, $type) = split /\./, $key;
+    $type ||= 'jpeg';
+    if ($fileid && length($fileid) == 40) {
+        return "$fileid.$type";
+    }
+}
+
+1;