Remove dead code, remove locking from CopyTo, put locking into MoveTo.

scripts/ZoneMinder/lib/ZoneMinder/Event.pm

@@ -584,6 +584,7 @@ sub DiskSpace {
   return $_[0]{DiskSpace};
 }
 
+# Icon: I removed the locking from this. So we now have an assumption that the Event object is up to date.
 sub CopyTo {
   my ( $self, $NewStorage ) = @_;
 
@@ -614,16 +615,12 @@ sub CopyTo {
     Debug("$NewPath is good");
   }
 
-  $ZoneMinder::Database::dbh->begin_work();
-  $self->lock_and_load();
   # data is reloaded, so need to check that the move hasn't already happened.
   if ( $$self{StorageId} == $$NewStorage{Id} ) {
-    $ZoneMinder::Database::dbh->commit();
     return 'Event has already been moved by someone else.';
   }
 
   if ( $$OldStorage{Id} != $$self{StorageId} ) {
-    $ZoneMinder::Database::dbh->commit();
     return 'Old Storage path changed, Event has moved somewhere else.';
   }
 
@@ -661,39 +658,21 @@ sub CopyTo {
           }
 
           my $event_path = $subpath.$self->RelativePath();
-          if ( 0 ) { # Not neccessary
-            Debug("Making directory $event_path/");
-            if ( !$bucket->add_key($event_path.'/', '') ) {
-              Warning("Unable to add key for $event_path/ :". $s3->err . ': '. $s3->errstr());
-            }
-          }
 
           my @files = glob("$OldPath/*");
           Debug("Files to move @files");
-          foreach my $file ( @files ) {
+          foreach my $file (@files) {
             next if $file =~ /^\./;
-            ( $file ) = ( $file =~ /^(.*)$/ ); # De-taint
+            ($file) = ($file =~ /^(.*)$/); # De-taint
             my $starttime = [gettimeofday];
             Debug("Moving file $file to $NewPath");
             my $size = -s $file;
-            if ( ! $size ) {
+            if (!$size) {
               Info('Not moving file with 0 size');
             }
-            if ( 0 ) {
+            my $filename = $event_path.'/'.File::Basename::basename($file);
-              my $file_contents = File::Slurp::read_file($file);
+            if (!$bucket->add_key_filename($filename, $file)) {
-              if ( ! $file_contents ) {
+              die "Unable to add key for $filename " . $s3->err . ': '. $s3->errstr;
-                die 'Loaded empty file, but it had a size. Giving up';
-              }
-
-              my $filename = $event_path.'/'.File::Basename::basename($file);
-              if ( ! $bucket->add_key($filename, $file_contents) ) {
-                die "Unable to add key for $filename : ".$s3->err . ': ' . $s3->errstr;
-              }
-            } else {
-              my $filename = $event_path.'/'.File::Basename::basename($file);
-              if ( ! $bucket->add_key_filename($filename, $file) ) {
-                die "Unable to add key for $filename " . $s3->err . ': '. $s3->errstr;
-              }
             }
 
             my $duration = tv_interval($starttime);
@@ -704,16 +683,15 @@ sub CopyTo {
         };
         Error($@) if $@;
       } else {
-        Error("Unable to parse S3 Url into it's component parts.");
+        Error('Unable to parse S3 Url into it\'s component parts.');
       }
-      #die $@ if $@;
     } # end if Url
   } # end if s3
 
   my $error = '';
-  if ( !$moved ) {
+  if (!$moved) {
     File::Path::make_path($NewPath, {error => \my $err});
-    if ( @$err ) {
+    if (@$err) {
       for my $diag (@$err) {
         my ($file, $message) = %$diag;
         next if $message eq 'File exists';
@@ -724,23 +702,16 @@ sub CopyTo {
         }
       }
     }
-    if ( $error ) {
+    return $error if $error;
-      $ZoneMinder::Database::dbh->commit();
-      return $error;
-    }
     my @files = glob("$OldPath/*");
-    if ( ! @files ) {
+    return 'No files to move.' if !@files;
-      $ZoneMinder::Database::dbh->commit();
-      return 'No files to move.';
-    }
 
     for my $file (@files) {
       next if $file =~ /^\./;
-      ( $file ) = ( $file =~ /^(.*)$/ ); # De-taint
+      ($file) = ($file =~ /^(.*)$/); # De-taint
       my $starttime = [gettimeofday];
-      Debug("Moving file $file to $NewPath");
       my $size = -s $file;
-      if ( ! File::Copy::copy( $file, $NewPath ) ) {
+      if (!File::Copy::copy($file, $NewPath)) {
         $error .= "Copy failed: for $file to $NewPath: $!";
         last;
       }
@@ -749,10 +720,7 @@ sub CopyTo {
     } # end foreach file.
   } # end if ! moved
 
-  if ( $error ) {
+  return $error if $error;
-    $ZoneMinder::Database::dbh->commit();
-    return $error;
-  }
 } # end sub CopyTo
 
 sub MoveTo {
@@ -763,6 +731,10 @@ sub MoveTo {
     return 'No permission to move event.';
   }
 
+  my $was_in_transaction = !$ZoneMinder::Database::dbh->{AutoCommit};
+  $ZoneMinder::Database::dbh->begin_work() if !$was_in_transaction;
+  $self->lock_and_load(); # The fact that we are in a transaction might not imply locking
+
   my $OldStorage = $self->Storage(undef);
 
   my $error = $self->CopyTo($NewStorage);
@@ -772,6 +744,7 @@ sub MoveTo {
   $$self{StorageId} = $$NewStorage{Id};
   $self->Storage($NewStorage);
   $error .= $self->save();
+  $ZoneMinder::Database::dbh->commit() if !$was_in_transaction;
   if ($error) {
     return $error;
   }

web/api/app/Plugin/Crud

@@ -1 +1 @@
-Subproject commit 0bd63fb464957080ead342db58ca9e01532cf1ef
+Subproject commit 14292374ccf1328f2d5db20897bd06f99ba4d938