diff options
Diffstat (limited to 'perllib/FixMyStreet')
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 59 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Photo.pm | 128 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 3 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Model/PhotoSet.pm | 269 | ||||
| -rw-r--r-- | perllib/FixMyStreet/DB/Result/Problem.pm | 29 | ||||
| -rw-r--r-- | perllib/FixMyStreet/TestMech.pm | 11 |
6 files changed, 358 insertions, 141 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index f9ea383f8..d7bca05a7 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -10,6 +10,7 @@ use Digest::SHA qw(sha1_hex); use mySociety::EmailUtil qw(is_valid_email); use if !$ENV{TRAVIS}, 'Image::Magick'; use DateTime::Format::Strptime; +use List::Util 'first'; use FixMyStreet::SendReport; @@ -672,8 +673,8 @@ sub report_edit : Path('report_edit') : Args(1) { ); } - if ( $c->get_param('rotate_photo') ) { - $c->forward('rotate_photo'); + if (my $rotate_photo_param = $self->_get_rotate_photo_param($c)) { + $self->rotate_photo($c, @$rotate_photo_param); return 1; } @@ -1370,36 +1371,28 @@ Rotate a photo 90 degrees left or right =cut +# returns index of photo to rotate, if any +sub _get_rotate_photo_param { + my ($self, $c) = @_; + my $params = $c->req->parameters; + my $key = first { /^rotate_photo/ } $c->req->param or return; + my ($index) = $key =~ /(\d+)$/; + my $direction = $c->req->param($key); + return [ $index || 0, $key, $direction ]; +} + sub rotate_photo : Private { - my ( $self, $c ) =@_; + my ( $self, $c, $index, $key, $direction ) = @_; - my $direction = $c->get_param('rotate_photo'); return unless $direction eq _('Rotate Left') or $direction eq _('Rotate Right'); - my $photo = $c->stash->{problem}->photo; - my $file; - - # If photo field contains a hash - if ( length($photo) == 40 ) { - $file = file( $c->config->{UPLOAD_DIR}, "$photo.jpeg" ); - $photo = $file->slurp; - } - - $photo = _rotate_image( $photo, $direction eq _('Rotate Left') ? -90 : 90 ); - return unless $photo; + my $problem = $c->stash->{problem}; + my $fileid = $problem->get_photoset($c)->rotate_image( + $index, + $direction eq _('Rotate Left') ? -90 : 90 + ) or return; - # Write out to new location - my $fileid = sha1_hex($photo); - $file = file( $c->config->{UPLOAD_DIR}, "$fileid.jpeg" ); - - my $fh = $file->open('w'); - print $fh $photo; - close $fh; - - unlink glob FixMyStreet->path_to( 'web', 'photo', $c->stash->{problem}->id . '.*' ); - - $c->stash->{problem}->photo( $fileid ); - $c->stash->{problem}->update(); + $problem->update({ photo => $fileid }); return 1; } @@ -1449,18 +1442,6 @@ sub trim { return $e; } -sub _rotate_image { - my ($photo, $direction) = @_; - my $image = Image::Magick->new; - $image->BlobToImage($photo); - my $err = $image->Rotate($direction); - return 0 if $err; - my @blobs = $image->ImageToBlob(); - undef $image; - return $blobs[0]; -} - - =head1 AUTHOR Struan Donald diff --git a/perllib/FixMyStreet/App/Controller/Photo.pm b/perllib/FixMyStreet/App/Controller/Photo.pm index a2ec7d4c8..7cf78cb2f 100644 --- a/perllib/FixMyStreet/App/Controller/Photo.pm +++ b/perllib/FixMyStreet/App/Controller/Photo.pm @@ -8,8 +8,8 @@ use DateTime::Format::HTTP; use Digest::SHA qw(sha1_hex); use File::Path; use File::Slurp; -use Image::Size; use Path::Class; +use FixMyStreet::App::Model::PhotoSet; use if !$ENV{TRAVIS}, 'Image::Magick'; =head1 NAME @@ -49,13 +49,13 @@ sub during :LocalRegex('^([0-9a-f]{40})\.(temp|fulltemp)\.jpeg$') { $c->forward( 'output', [ $photo ] ); } -sub index :LocalRegex('^(c/)?(\d+)(?:\.(full|tn|fp))?\.jpeg$') { +sub index :LocalRegex('^(c/)?(\d+)(?:\.(\d+))?(?:\.(full|tn|fp))?\.jpeg$') { my ( $self, $c ) = @_; - my ( $is_update, $id, $size ) = @{ $c->req->captures }; + my ( $is_update, $id, $photo_number, $size ) = @{ $c->req->captures }; - my @photo; + my $item; if ( $is_update ) { - @photo = $c->model('DB::Comment')->search( { + ($item) = $c->model('DB::Comment')->search( { id => $id, state => 'confirmed', photo => { '!=', undef }, @@ -67,35 +67,21 @@ sub index :LocalRegex('^(c/)?(\d+)(?:\.(full|tn|fp))?\.jpeg$') { } $c->detach( 'no_photo' ) if $id =~ /\D/; - @photo = $c->cobrand->problems->search( { + ($item) = $c->cobrand->problems->search( { id => $id, state => [ FixMyStreet::DB::Result::Problem->visible_states(), 'partial' ], photo => { '!=', undef }, } ); } - $c->detach( 'no_photo' ) unless @photo; + $c->detach( 'no_photo' ) unless $item; - my $item = $photo[0]; $c->detach( 'no_photo' ) unless $c->cobrand->allow_photo_display($item); # Should only be for reports, not updates - my $photo = $item->photo; - # If photo field contains a hash - if (length($photo) == 40) { - my $file = file( $c->config->{UPLOAD_DIR}, "$photo.jpeg" ); - $photo = $file->slurp; - } - - if ( $size eq 'tn' ) { - $photo = _shrink( $photo, 'x100' ); - } elsif ( $size eq 'fp' ) { - $photo = _crop( $photo ); - } elsif ( $size eq 'full' ) { - } elsif ( $c->cobrand->default_photo_resize ) { - $photo = _shrink( $photo, $c->cobrand->default_photo_resize ); - } else { - $photo = _shrink( $photo, '250x250' ); - } + my $photo = $item->get_photoset( $c ) + ->get_image_data( num => $photo_number, size => $size ) + + or $c->detach( 'no_photo' ); $c->forward( 'output', [ $photo ] ); } @@ -156,84 +142,28 @@ sub process_photo : Private { my ( $self, $c ) = @_; return - $c->forward('process_photo_upload') - || $c->forward('process_photo_cache') + $c->forward('process_photo_upload_or_cache') || 1; # always return true } -sub process_photo_upload : Private { +sub process_photo_upload_or_cache : Private { my ( $self, $c ) = @_; - - # check for upload or return - my $upload = $c->req->upload('photo') - || return; - - # check that the photo is a jpeg - my $ct = $upload->type; - $ct =~ s/x-citrix-//; # Thanks, Citrix - # Had a report of a JPEG from an Android 2.1 coming through as a byte stream - unless ( $ct eq 'image/jpeg' || $ct eq 'image/pjpeg' || $ct eq 'application/octet-stream' ) { - $c->log->info('Bad photo tried to upload, type=' . $ct); - $c->stash->{photo_error} = _('Please upload a JPEG image only'); - return; - } - - # get the photo into a variable - my $photo_blob = eval { - my $filename = $upload->tempname; - my $out = `jhead -se -autorot $filename 2>&1`; - unless (defined $out) { - my ($w, $h, $err) = Image::Size::imgsize($filename); - die _("Please upload a JPEG image only") . "\n" if !defined $w || $err ne 'JPG'; - } - die _("Please upload a JPEG image only") . "\n" if $out && $out =~ /Not JPEG:/; - my $photo = $upload->slurp; - return $photo; - }; - if ( my $error = $@ ) { - my $format = _( -"That image doesn't appear to have uploaded correctly (%s), please try again." - ); - $c->stash->{photo_error} = sprintf( $format, $error ); - return; - } - - # we have an image we can use - save it to the upload dir for storage - my $cache_dir = dir( $c->config->{UPLOAD_DIR} ); - $cache_dir->mkpath; - unless ( -d $cache_dir && -w $cache_dir ) { - warn "Can't find/write to photo cache directory '$cache_dir'"; - return; - } - - my $fileid = sha1_hex($photo_blob); - $upload->copy_to( file($cache_dir, $fileid . '.jpeg') ); - - # stick the hash on the stash, so don't have to reupload in case of error - $c->stash->{upload_fileid} = $fileid; - - return 1; -} - -=head2 process_photo_cache - -Look for the upload_fileid parameter and check it matches a file on disk. If it -does return true and put fileid on stash, otherwise false. - -=cut - -sub process_photo_cache : Private { - my ( $self, $c ) = @_; - - # get the fileid and make sure it is just a hex number - my $fileid = $c->get_param('upload_fileid') || ''; - $fileid =~ s{[^0-9a-f]}{}gi; - return unless $fileid; - - my $file = file( $c->config->{UPLOAD_DIR}, "$fileid.jpeg" ); - return unless -e $file; - - $c->stash->{upload_fileid} = $fileid; + my @items = ( + ( map { + /^photo/ ? # photo, photo1, photo2 etc. + ($c->req->upload($_)) : () + } sort $c->req->upload), + split /,/, ($c->req->param('upload_fileid') || '') + ); + + my $photoset = FixMyStreet::App::Model::PhotoSet->new({ + c => $c, + data_items => \@items, + }); + + my $fileid = $photoset->data; + + $c->stash->{upload_fileid} = $fileid or return; return 1; } diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index b55152693..6a884db7f 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -93,6 +93,7 @@ sub report_new : Path : Args(0) { $c->forward('check_for_category'); # deal with the user and report and check both are happy + return unless $c->forward('check_form_submitted'); $c->forward('process_user'); $c->forward('process_report'); @@ -290,7 +291,7 @@ sub report_import : Path('/import') { } # handle the photo upload - $c->forward( '/photo/process_photo_upload' ); + $c->forward( '/photo/process_photo' ); my $fileid = $c->stash->{upload_fileid}; if ( my $error = $c->stash->{photo_error} ) { push @errors, $error; diff --git a/perllib/FixMyStreet/App/Model/PhotoSet.pm b/perllib/FixMyStreet/App/Model/PhotoSet.pm new file mode 100644 index 000000000..fa6eb060b --- /dev/null +++ b/perllib/FixMyStreet/App/Model/PhotoSet.pm @@ -0,0 +1,269 @@ +package FixMyStreet::App::Model::PhotoSet; + +# TODO this isn't a Cat model, rename to something else + +use Moose; +use Path::Tiny 'path'; +use if !$ENV{TRAVIS}, 'Image::Magick'; +use Scalar::Util 'openhandle', 'blessed'; +use Digest::SHA qw(sha1_hex); +use Image::Size; + +has c => ( + is => 'ro', +); + +has object => ( + is => 'ro', +); + +has data => ( # generic data from DB field + is => 'ro', + lazy => 1, + default => sub { + # yes, this is a little circular: data -> data_items -> items -> data + # e.g. if not provided, then we're presumably uploading/etc., so calculate from + # the stored cached fileids + # (obviously if you provide none of these, then you'll get an infinite loop) + my $self = shift; + my $data = join ',', map { $_->[0] } $self->all_images; + return $data; + } +); + +has data_items => ( # either a) split from data or b) provided by photo upload + isa => 'ArrayRef', + is => 'rw', + traits => ['Array'], + lazy => 1, + handles => { + map_data_items => 'map', + }, + default => sub { + my $self = shift; + my $data = $self->data + or return []; + + return [$data] if (_jpeg_magic($data)); + + return [ split ',' => $data ]; + }, +); + +has upload_dir => ( + is => 'ro', + lazy => 1, + default => sub { + my $self = shift; + my $cache_dir = path( $self->c->config->{UPLOAD_DIR} ); + $cache_dir->mkpath; + unless ( -d $cache_dir && -w $cache_dir ) { + warn "Can't find/write to photo cache directory '$cache_dir'"; + return; + } + $cache_dir; + }, +); + +sub _jpeg_magic { + $_[0] =~ /^\x{ff}\x{d8}/; # JPEG + # NB: should we also handle \x{89}\x{50} (PNG, 15 results in live DB) ? + # and \x{49}\x{49} (Tiff, 3 results in live DB) ? +} + +has images => ( # jpeg data for actual image + isa => 'ArrayRef', + is => 'rw', + traits => ['Array'], + lazy => 1, + handles => { + num_images => 'count', + get_raw_image_data => 'get', + all_images => 'elements', + }, + default => sub { + my $self = shift; + my @photos = $self->map_data_items( sub { + my $part = $_; + + if (blessed $part and $part->isa('Catalyst::Request::Upload')) { + # check that the photo is a jpeg + my $upload = $part; + my $ct = $upload->type; + $ct =~ s/x-citrix-//; # Thanks, Citrix + # Had a report of a JPEG from an Android 2.1 coming through as a byte stream + unless ( $ct eq 'image/jpeg' || $ct eq 'image/pjpeg' || $ct eq 'application/octet-stream' ) { + my $c = $self->c; + $c->log->info('Bad photo tried to upload, type=' . $ct); + $c->stash->{photo_error} = _('Please upload a JPEG image only'); + return (); + } + + # get the photo into a variable + my $photo_blob = eval { + my $filename = $upload->tempname; + my $out = `jhead -se -autorot $filename 2>&1`; + unless (defined $out) { + my ($w, $h, $err) = Image::Size::imgsize($filename); + die _("Please upload a JPEG image only") . "\n" if !defined $w || $err ne 'JPG'; + } + die _("Please upload a JPEG image only") . "\n" if $out && $out =~ /Not JPEG:/; + my $photo = $upload->slurp; + }; + if ( my $error = $@ ) { + my $format = _( + "That image doesn't appear to have uploaded correctly (%s), please try again." + ); + $self->c->stash->{photo_error} = sprintf( $format, $error ); + 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); + $upload->copy_to( $file ); + return [$fileid, $photo_blob]; + + } + if (_jpeg_magic($part)) { + my $photo_blob = $part; + my $fileid = $self->get_fileid($photo_blob); + my $file = $self->get_file($fileid); + $file->spew_raw($photo_blob); + return [$fileid, $photo_blob]; + } + if (length($part) == 40) { + my $fileid = $part; + my $file = $self->get_file($fileid); + if ($file->exists) { + my $photo = $file->slurp_raw; + [$fileid, $photo]; + } + else { + warn "File $fileid doesn't exist"; + (); + } + } + else { + warn sprintf "Received bad photo hash of length %d", length($part); + (); + } + }); + return \@photos; + }, +); + +sub get_fileid { + my ($self, $photo_blob) = @_; + return sha1_hex($photo_blob); +} + +sub get_file { + my ($self, $fileid) = @_; + my $cache_dir = $self->upload_dir; + return path( $cache_dir, "$fileid.jpeg" ); +} + +sub get_image_data { + my ($self, %args) = @_; + my $num = $args{num} || 0; + + my $data = $self->get_raw_image_data( $num ) + or return; + + my ($fileid, $photo) = @$data; + + my $size = $args{size}; + if ( $size eq 'tn' ) { + $photo = _shrink( $photo, 'x100' ); + } elsif ( $size eq 'fp' ) { + $photo = _crop( $photo ); + } elsif ( $size eq 'full' ) { + # do nothing + } else { + $photo = _shrink( $photo, $self->c->cobrand->default_photo_resize || '250x250' ); + } + + return $photo; +} + +sub delete_cached { + my ($self) = @_; + my $object = $self->object or return; + + unlink glob FixMyStreet->path_to( + 'web', + 'photo', + $object->id . '.*' + ); +} + +sub rotate_image { + my ($self, $index, $direction) = @_; + + my @images = $self->all_images; + return if $index > $#images; + + my @items = map $_->[0], @images; + $items[$index] = _rotate_image( $images[$index][1], $direction ); + + my $new_set = (ref $self)->new({ + data_items => \@items, + c => $self->c, + object => $self->object, + }); + + $self->delete_cached(); + + return $new_set->data; # e.g. new comma-separated fileid +} + +sub _rotate_image { + my ($photo, $direction) = @_; + return $photo unless $Image::Magick::VERSION; + my $image = Image::Magick->new; + $image->BlobToImage($photo); + my $err = $image->Rotate($direction); + return 0 if $err; + my @blobs = $image->ImageToBlob(); + undef $image; + return $blobs[0]; +} + + + + + +# NB: These 2 subs stolen from A::C::Photo, should be purged from there! +# +# Shrinks a picture to the specified size, but keeping in proportion. +sub _shrink { + my ($photo, $size) = @_; + return $photo unless $Image::Magick::VERSION; + my $image = Image::Magick->new; + $image->BlobToImage($photo); + my $err = $image->Scale(geometry => "$size>"); + throw Error::Simple("resize failed: $err") if "$err"; + $image->Strip(); + my @blobs = $image->ImageToBlob(); + undef $image; + return $blobs[0]; +} + +# Shrinks a picture to 90x60, cropping so that it is exactly that. +sub _crop { + my ($photo) = @_; + return $photo unless $Image::Magick::VERSION; + my $image = Image::Magick->new; + $image->BlobToImage($photo); + my $err = $image->Resize( geometry => "90x60^" ); + throw Error::Simple("resize failed: $err") if "$err"; + $err = $image->Extent( geometry => '90x60', gravity => 'Center' ); + throw Error::Simple("resize failed: $err") if "$err"; + $image->Strip(); + my @blobs = $image->ImageToBlob(); + undef $image; + return $blobs[0]; +} + +1; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index 9706087aa..635c41382 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -483,11 +483,17 @@ sub url { =head2 get_photo_params -Returns a hashref of details of any attached photo for use in templates. +Returns a hashref of details of the attached photo, if any, for use in templates. + +NB: this method doesn't currently support multiple photos gracefully. + +Use get_photoset($c) instead to do the right thing with reports with 0, 1, or more photos. =cut sub get_photo_params { + # use Carp 'cluck'; + # cluck "get_photo_params called"; # TEMPORARY die to make sure I've done right thing with Zurich templates my $self = shift; return FixMyStreet::App::get_photo_params($self, 'id'); } @@ -828,6 +834,27 @@ sub latest_moderation_log_entry { return $self->admin_log_entries->search({ action => 'moderation' }, { order_by => 'id desc' })->first; } +=head2 get_photoset + +Return a PhotoSet object for all photos attached to this field + + my $photoset = $obj->get_photoset( $c ); + print $photoset->num_images; + return $photoset->get_image_data(num => 0, size => 'full'); + +=cut + +sub get_photoset { + my ($self, $c) = @_; + my $class = 'FixMyStreet::App::Model::PhotoSet'; + eval "use $class"; + return $class->new({ + c => $c, + data => $self->photo, + object => $self, + }); +} + __PACKAGE__->has_many( "admin_log_entries", "FixMyStreet::DB::Result::AdminLog", diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index bd2ca4096..ef60b0d36 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -631,7 +631,7 @@ sub create_problems_for_body { latitude => '51.5016605453401', longitude => '-0.142497580865087', user_id => $user->id, - photo => 1, + photo => $mech->get_photo_data, }; my %report_params = ( %$default_params, %$params ); @@ -646,4 +646,13 @@ sub create_problems_for_body { return @problems; } +sub get_photo_data { + my $mech = shift; + return $mech->{sample_photo} ||= do { + my $sample_file = FixMyStreet->path_to( 't/app/controller/sample.jpg' ); + $mech->builder->ok( -f "$sample_file", "sample file $sample_file exists" ); + $sample_file->slurp(iomode => '<:raw'); + }; +} + 1; |