From: dklawren <dklawren@users.noreply.github.com>
Date: Thu, 12 Dec 2019 18:29:02 +0000 (-0500)
Subject: Revert "Bug 1588221 - Update current s3 storage controller to only upload attachments... 
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c5387936b0a2503468ddebcc3ee18bb800156ae5;p=thirdparty%2Fbugzilla.git

Revert "Bug 1588221 - Update current s3 storage controller to only upload attachments over a certain size"
---

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 1c15f44d3..e7b310834 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -316,7 +316,7 @@ the content of the attachment
 
 sub data {
   my $self = shift;
-  return $self->{data} //= $self->current_storage->get_data();
+  return $self->{data} //= current_storage()->retrieve($self->id);
 }
 
 =over
@@ -735,8 +735,7 @@ sub create {
     close($data);
     $data = $tmp;
   }
-
-  $attachment->current_storage->set_data($data)->set_class();
+  current_storage()->store($attachment->id, $data);
 
   # Return the new attachment object
   return $attachment;
@@ -828,7 +827,7 @@ sub remove_from_db {
               WHERE attach_id = ?', undef, ('text/plain', 0, 1, 0, $self->id)
   );
   $dbh->bz_commit_transaction();
-  $self->current_storage->remove_data()->remove_class();
+  current_storage()->remove($self->id);
 
   # As we don't call SUPER->remove_from_db we need to manually clear
   # memcached here.
@@ -894,31 +893,8 @@ sub get_content_type {
 }
 
 sub current_storage {
-  my ($self, $override_class) = @_;
-  my $dbh  = Bugzilla->dbh;
-
-  # Sometimes we might want to copy attachment data from one
-  # storage class to another. With this param, we can override
-  # the current class, and then call ->set_data().
-  if ($override_class) {
-    return $self->{current_storage} = $self->get_storage_by_name($override_class, $self);
-  }
-
-  if (!$self->{current_storage}) {
-    my ($current_storage)
-      = $dbh->selectrow_array(
-      "SELECT storage_class FROM attachment_storage_class WHERE id = ?",
-      undef, $self->id);
-    if (!$current_storage) {
-      $self->{current_storage}
-        = $self->get_storage_by_name(Bugzilla->params->{attachment_storage});
-    }
-    else {
-      $self->{current_storage} = $self->get_storage_by_name($current_storage);
-    }
-  }
-
-  return $self->{current_storage};
+  return state $storage
+    //= get_storage_by_name(Bugzilla->params->{attachment_storage});
 }
 
 sub get_storage_names {
@@ -931,20 +907,20 @@ sub get_storage_names {
 }
 
 sub get_storage_by_name {
-  my ($self, $name) = @_;
+  my ($name) = @_;
 
   # all options for attachment_storage need to be handled here
   if ($name eq 'database') {
-    require Bugzilla::Attachment::Storage::Database;
-    return Bugzilla::Attachment::Storage::Database->new({attachment => $self});
+    require Bugzilla::Attachment::Database;
+    return Bugzilla::Attachment::Database->new();
   }
   elsif ($name eq 'filesystem') {
-    require Bugzilla::Attachment::Storage::FileSystem;
-    return Bugzilla::Attachment::Storage::FileSystem->new({attachment => $self});
+    require Bugzilla::Attachment::FileSystem;
+    return Bugzilla::Attachment::FileSystem->new();
   }
   elsif ($name eq 's3') {
-    require Bugzilla::Attachment::Storage::S3;
-    return Bugzilla::Attachment::Storage::S3->new({attachment => $self});
+    require Bugzilla::Attachment::S3;
+    return Bugzilla::Attachment::S3->new();
   }
   else {
     return undef;
diff --git a/Bugzilla/Attachment/Database.pm b/Bugzilla/Attachment/Database.pm
new file mode 100644
index 000000000..fd8f5f000
--- /dev/null
+++ b/Bugzilla/Attachment/Database.pm
@@ -0,0 +1,50 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Attachment::Database;
+
+use 5.10.1;
+use strict;
+use warnings;
+
+sub new {
+  return bless({}, shift);
+}
+
+sub store {
+  my ($self, $attach_id, $data) = @_;
+  my $dbh = Bugzilla->dbh;
+  my $sth = $dbh->prepare(
+    "INSERT INTO attach_data (id, thedata) VALUES ($attach_id, ?)");
+  $sth->bind_param(1, $data, $dbh->BLOB_TYPE);
+  $sth->execute();
+}
+
+sub retrieve {
+  my ($self, $attach_id) = @_;
+  my $dbh = Bugzilla->dbh;
+  my ($data)
+    = $dbh->selectrow_array("SELECT thedata FROM attach_data WHERE id = ?",
+    undef, $attach_id);
+  return $data;
+}
+
+sub remove {
+  my ($self, $attach_id) = @_;
+  my $dbh = Bugzilla->dbh;
+  $dbh->do("DELETE FROM attach_data WHERE id = ?", undef, $attach_id);
+}
+
+sub exists {
+  my ($self, $attach_id) = @_;
+  my $dbh = Bugzilla->dbh;
+  my ($exists) = $dbh->selectrow_array("SELECT 1 FROM attach_data WHERE id = ?",
+    undef, $attach_id);
+  return !!$exists;
+}
+
+1;
diff --git a/Bugzilla/Attachment/FileSystem.pm b/Bugzilla/Attachment/FileSystem.pm
new file mode 100644
index 000000000..f10a994fd
--- /dev/null
+++ b/Bugzilla/Attachment/FileSystem.pm
@@ -0,0 +1,63 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Attachment::FileSystem;
+
+use 5.10.1;
+use strict;
+use warnings;
+
+use Bugzilla::Constants qw(bz_locations);
+
+sub new {
+  return bless({}, shift);
+}
+
+sub store {
+  my ($self, $attach_id, $data) = @_;
+  my $path = _local_path($attach_id);
+  mkdir($path, 0770) unless -d $path;
+  open(my $fh, '>', _local_file($attach_id));
+  binmode($fh);
+  print $fh $data;
+  close($fh);
+}
+
+sub retrieve {
+  my ($self, $attach_id) = @_;
+  if (open(my $fh, '<', _local_file($attach_id))) {
+    local $/;
+    binmode($fh);
+    my $data = <$fh>;
+    close($fh);
+    return $data;
+  }
+  return undef;
+}
+
+sub remove {
+  my ($self, $attach_id) = @_;
+  unlink(_local_file($attach_id));
+}
+
+sub exists {
+  my ($self, $attach_id) = @_;
+  return -e _local_file($attach_id);
+}
+
+sub _local_path {
+  my ($attach_id) = @_;
+  my $hash = sprintf('group.%03d', $attach_id % 1000);
+  return bz_locations()->{attachdir} . '/' . $hash;
+}
+
+sub _local_file {
+  my ($attach_id) = @_;
+  return _local_path($attach_id) . '/attachment.' . $attach_id;
+}
+
+1;
diff --git a/Bugzilla/Attachment/S3.pm b/Bugzilla/Attachment/S3.pm
new file mode 100644
index 000000000..7f4755720
--- /dev/null
+++ b/Bugzilla/Attachment/S3.pm
@@ -0,0 +1,62 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Attachment::S3;
+
+use 5.10.1;
+use strict;
+use warnings;
+
+use Bugzilla::Error;
+use Bugzilla::S3;
+
+sub new {
+  my $s3 = Bugzilla::S3->new({
+    aws_access_key_id     => Bugzilla->params->{aws_access_key_id},
+    aws_secret_access_key => Bugzilla->params->{aws_secret_access_key},
+    secure                => 1,
+  });
+  return
+    bless({s3 => $s3, bucket => $s3->bucket(Bugzilla->params->{s3_bucket}),},
+    shift);
+}
+
+sub store {
+  my ($self, $attach_id, $data) = @_;
+  unless ($self->{bucket}->add_key($attach_id, $data)) {
+    warn "Failed to add attachment $attach_id to S3: "
+      . $self->{bucket}->errstr . "\n";
+    ThrowCodeError('s3_add_failed',
+      {attach_id => $attach_id, reason => $self->{bucket}->errstr});
+  }
+}
+
+sub retrieve {
+  my ($self, $attach_id) = @_;
+  my $response = $self->{bucket}->get_key($attach_id);
+  if (!$response) {
+    warn "Failed to retrieve attachment $attach_id from S3: "
+      . $self->{bucket}->errstr . "\n";
+    ThrowCodeError('s3_get_failed',
+      {attach_id => $attach_id, reason => $self->{bucket}->errstr});
+  }
+  return $response->{value};
+}
+
+sub remove {
+  my ($self, $attach_id) = @_;
+  $self->{bucket}->delete_key($attach_id)
+    or warn "Failed to remove attachment $attach_id from S3: "
+    . $self->{bucket}->errstr . "\n";
+}
+
+sub exists {
+  my ($self, $attach_id) = @_;
+  return !!$self->{bucket}->head_key($attach_id);
+}
+
+1;
diff --git a/Bugzilla/Attachment/Storage/Base.pm b/Bugzilla/Attachment/Storage/Base.pm
deleted file mode 100644
index 4725f05c7..000000000
--- a/Bugzilla/Attachment/Storage/Base.pm
+++ /dev/null
@@ -1,38 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Attachment::Storage::Base;
-
-use 5.10.1;
-use Moo::Role;
-
-use Type::Utils qw(class_type);
-
-requires qw(set_data get_data remove_data data_exists data_type);
-
-has 'attachment' => (
-  is       => 'ro',
-  required => 1,
-  isa      => class_type({class => 'Bugzilla::Attachment'})
-);
-
-sub set_class {
-  my ($self) = @_;
-  Bugzilla->dbh->do(
-    "REPLACE INTO attachment_storage_class (id, storage_class) VALUES (?, ?)",
-    undef, $self->attachment->id, $self->data_type);
-  return $self;
-}
-
-sub remove_class {
-  my ($self) = @_;
-  Bugzilla->dbh->do("DELETE FROM attachment_storage_class WHERE id = ?",
-    undef, $self->attachment->id);
-  return $self;
-}
-
-1;
diff --git a/Bugzilla/Attachment/Storage/Database.pm b/Bugzilla/Attachment/Storage/Database.pm
deleted file mode 100644
index 9da52ef6c..000000000
--- a/Bugzilla/Attachment/Storage/Database.pm
+++ /dev/null
@@ -1,54 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Attachment::Storage::Database;
-
-use 5.10.1;
-use Moo;
-
-with 'Bugzilla::Attachment::Storage::Base';
-
-sub data_type { return 'database'; }
-
-sub set_data {
-  my ($self, $data) = @_;
-  my $dbh = Bugzilla->dbh;
-  my $sth
-    = $dbh->prepare(
-    "REPLACE INTO attach_data (id, thedata) VALUES (?, ?)"
-    );
-  $sth->bind_param(1, $self->attachment->id);
-  $sth->bind_param(2, $data, $dbh->BLOB_TYPE);
-  $sth->execute();
-  return $self;
-}
-
-sub get_data {
-  my ($self) = @_;
-  my $dbh = Bugzilla->dbh;
-  my ($data)
-    = $dbh->selectrow_array("SELECT thedata FROM attach_data WHERE id = ?",
-    undef, $self->attachment->id);
-  return $data;
-}
-
-sub remove_data {
-  my ($self) = @_;
-  my $dbh = Bugzilla->dbh;
-  $dbh->do("DELETE FROM attach_data WHERE id = ?", undef, $self->attachment->id);
-  return $self;
-}
-
-sub data_exists {
-  my ($self)   = @_;
-  my $dbh      = Bugzilla->dbh;
-  my ($exists) = $dbh->selectrow_array("SELECT 1 FROM attach_data WHERE id = ?",
-    undef, $self->attachment->id);
-  return !!$exists;
-}
-
-1;
diff --git a/Bugzilla/Attachment/Storage/FileSystem.pm b/Bugzilla/Attachment/Storage/FileSystem.pm
deleted file mode 100644
index d842a2476..000000000
--- a/Bugzilla/Attachment/Storage/FileSystem.pm
+++ /dev/null
@@ -1,63 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Attachment::Storage::FileSystem;
-
-use 5.10.1;
-use Moo;
-
-use Bugzilla::Constants qw(bz_locations);
-
-with 'Bugzilla::Attachment::Storage::Base';
-
-sub data_type { return 'filesystem'; }
-
-sub set_data {
-  my ($self, $data) = @_;
-  my $path = $self->_local_path();
-  mkdir($path, 0770) unless -d $path;
-  open(my $fh, '>', $self->_local_file());
-  binmode($fh);
-  print $fh $data;
-  close($fh);
-  return $self;
-}
-
-sub get_data {
-  my ($self) = @_;
-  if (open(my $fh, '<', $self->_local_file())) {
-    local $/;
-    binmode($fh);
-    my $data = <$fh>;
-    close($fh);
-    return $data;
-  }
-}
-
-sub remove_data {
-  my ($self) = @_;
-  unlink($self->_local_file());
-  return $self;
-}
-
-sub data_exists {
-  my ($self) = @_;
-  return -e $self->_local_file();
-}
-
-sub _local_path {
-  my ($self) = @_;
-  my $hash = sprintf('group.%03d', $self->attachment->id % 1000);
-  return bz_locations()->{attachdir} . '/' . $hash;
-}
-
-sub _local_file {
-  my ($self) = @_;
-  return $self->_local_path() . '/attachment.' . $self->attachment->id;
-}
-
-1;
diff --git a/Bugzilla/Attachment/Storage/S3.pm b/Bugzilla/Attachment/Storage/S3.pm
deleted file mode 100644
index aec5f440c..000000000
--- a/Bugzilla/Attachment/Storage/S3.pm
+++ /dev/null
@@ -1,90 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Attachment::Storage::S3;
-
-use 5.10.1;
-use Moo;
-
-use Bugzilla::Error;
-use Bugzilla::S3;
-
-with 'Bugzilla::Attachment::Storage::Base';
-
-has 's3'     => (is => 'ro', lazy => 1);
-has 'bucket' => (is => 'ro', lazy => 1);
-
-sub _build_s3 {
-  my $self = shift;
-  $self->{s3} ||= Bugzilla::S3->new({
-    aws_access_key_id     => Bugzilla->params->{aws_access_key_id},
-    aws_secret_access_key => Bugzilla->params->{aws_secret_access_key},
-    secure                => 1,
-  });
-  return $self->{s3};
-}
-
-sub _build_bucket {
-  my $self = shift;
-  $self->{bucket} ||= $self->s3->bucket(Bugzilla->params->{s3_bucket});
-  return $self->bucket;
-}
-
-sub data_type { return 's3'; }
-
-sub set_data {
-  my ($self, $data) = @_;
-  my $attach_id = $self->attachment->id;
-
-  # If the attachment is larger than attachment_s3_minsize,
-  # we instead store it in the database.
-  if (Bugzilla->params->{attachment_s3_minsize}
-    && $self->attachment->datasize < Bugzilla->params->{attachment_s3_minsize})
-  {
-    require Bugzilla::Attachment::Storage::Database;
-    return Bugzilla::Attachment::Storage::Database->new({attachment => $self->attachment})
-      ->set_data($data);
-  }
-
-  unless ($self->bucket->add_key($attach_id, $data)) {
-    warn "Failed to add attachment $attach_id to S3: "
-      . $self->bucket->errstr . "\n";
-    ThrowCodeError('s3_add_failed',
-      {attach_id => $attach_id, reason => $self->bucket->errstr});
-  }
-
-  return $self;
-}
-
-sub get_data {
-  my ($self)    = @_;
-  my $attach_id = $self->attachment->id;
-  my $response  = $self->bucket->get_key($attach_id);
-  if (!$response) {
-    warn "Failed to retrieve attachment $attach_id from S3: "
-      . $self->bucket->errstr . "\n";
-    ThrowCodeError('s3_get_failed',
-      {attach_id => $attach_id, reason => $self->bucket->errstr});
-  }
-  return $response->{value};
-}
-
-sub remove_data {
-  my ($self) = @_;
-  my $attach_id = $self->attachment->id;
-  $self->bucket->delete_key($attach_id)
-    or warn "Failed to remove attachment $attach_id from S3: "
-    . $self->bucket->errstr . "\n";
-  return $self;
-}
-
-sub data_exists {
-  my ($self) = @_;
-  return !!$self->bucket->head_key($self->attachment->id);
-}
-
-1;
diff --git a/Bugzilla/Config/Attachment.pm b/Bugzilla/Config/Attachment.pm
index 7aa0ca794..dc1a31d48 100644
--- a/Bugzilla/Config/Attachment.pm
+++ b/Bugzilla/Config/Attachment.pm
@@ -33,12 +33,6 @@ sub get_param_list {
       default => 'database',
       checker => \&check_storage
     },
-    {
-      name => 'attachment_s3_minsize',
-      type => 't',
-      default => '20000',
-      checker => \&check_numeric
-    },
     {name => 's3_bucket',             type => 't', default => '',},
     {name => 'aws_access_key_id',     type => 't', default => '',},
     {name => 'aws_secret_access_key', type => 't', default => '',},
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index a4daefd33..15ce3c337 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -538,7 +538,6 @@ use constant ABSTRACT_SCHEMA => {
       attachments_ispatch_idx           => ['ispatch'],
     ],
   },
-
   attach_data => {
     FIELDS => [
       id => {
@@ -552,20 +551,6 @@ use constant ABSTRACT_SCHEMA => {
     ],
   },
 
-  attachment_storage_class => {
-    FIELDS => [
-      id => {
-        TYPE       => 'INT3',
-        NOTNULL    => 1,
-        PRIMARYKEY => 1,
-        REFERENCES =>
-          {TABLE => 'attachments', COLUMN => 'attach_id', DELETE => 'CASCADE'}
-      },
-      storage_class => {TYPE => 'varchar(64)', NOTNULL => 1},
-      extra_data    => {TYPE => 'MEDIUMTEXT'}
-    ],
-  },
-
   duplicates => {
     FIELDS => [
       dupe_of => {
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 879bdf165..9425d95ec 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -805,9 +805,6 @@ sub update_table_definitions {
   # Bug 1576667 - dkl@mozilla.com
   _populate_api_keys_creation_ts();
 
-  # Bug 1588221 - dkl@mozilla.com
-  _populate_attachment_storage_class();
-
   ################################################################
   # New --TABLE-- changes should go *** A B O V E *** this point #
   ################################################################
@@ -4329,19 +4326,6 @@ sub _populate_api_keys_creation_ts {
     {TYPE => 'DATETIME', NOTNULL => 1});
 }
 
-sub _populate_attachment_storage_class {
-  my $dbh = Bugzilla->dbh;
-
-  # Return if we have already made these changes
-  if (!$dbh->selectrow_arrayref('SELECT COUNT(id) FROM attachment_storage_class'))
-  {
-    $dbh->do(
-      "INSERT INTO attachment_storage_class (id, storage_class) SELECT attachments.id, 'database' FROM attachments ORDER BY attachments.id"
-    );
-  }
-}
-
-
 1;
 
 __END__
diff --git a/scripts/attachment-data.pl b/scripts/attachment-data.pl
index 596d0844b..0e89618e6 100755
--- a/scripts/attachment-data.pl
+++ b/scripts/attachment-data.pl
@@ -62,7 +62,7 @@ elsif ($cmd eq 'import') {
     next unless $mem->{data_len};
     next unless check_attachment($attachment, $mem->{bug_id}, $mem->{data_len});
 
-    $attachment->current_storage->set_data($mem->{data})->set_class();
+    Bugzilla::Attachment::current_storage()->store($attachment->id, $mem->{data});
   }
 }
 elsif ($cmd eq 'check') {
@@ -79,8 +79,7 @@ elsif ($cmd eq 'remove') {
   while (my $mem = $archive->read_member) {
     warn "checking $mem->{attach_id}\n";
 
-    my $attachment
-      = Bugzilla::Attachment->new({id => $mem->{attach_id}, cache => 1});
+    my $attachment = Bugzilla::Attachment->new($mem->{attach_id});
     die "bad attachment\n"
       unless check_attachment($attachment, $mem->{bug_id}, $mem->{data_len});
     $remove_ok{$mem->{attach_id}} = 1;
@@ -89,8 +88,7 @@ elsif ($cmd eq 'remove') {
     chomp $attach_id;
     if ($remove_ok{$attach_id}) {
       warn "removing $attach_id\n";
-      my $attachment = Bugzilla::Attachment->new({id => $attach_id, cache => 1});
-      $attachment->current_storage->remove()->remove_class();
+      Bugzilla::Attachment::current_storage()->remove($attach_id);
     }
     else {
       warn "Unable to remove $attach_id, as it did not occur in the archive.\n";
diff --git a/scripts/migrate-attachments.pl b/scripts/migrate-attachments.pl
index 8dfd40cf6..7ef6bea78 100755
--- a/scripts/migrate-attachments.pl
+++ b/scripts/migrate-attachments.pl
@@ -21,23 +21,14 @@ use Getopt::Long qw(GetOptions);
 my @storage_names = Bugzilla::Attachment->get_storage_names();
 
 my %options;
-GetOptions(\%options, 'migrate=s@{2}', 'mirror=s@{2}', 'copy=s@{2}', 'delete=s') or exit(1);
-unless ($options{migrate} || $options{mirror} || $options{copy} || $options{delete}) {
+GetOptions(\%options, 'mirror=s@{2}', 'copy=s@{2}', 'delete=s') or exit(1);
+unless ($options{mirror} || $options{copy} || $options{delete}) {
   die <<EOF;
 Syntax:
-    migrate-attachments.pl --migrate source destination
     migrate-attachments.pl --mirror source destination
     migrate-attachments.pl --copy source destination
     migrate-attachments.pl --delete source
 
-'migrate'
-    Migrates all attachments from the specified source to the destination.
-    Attachments which already exist at the destination will be overwritten.
-    This also sets the storage class so that the system loads the data from the
-    new destination.
-
-    e.g. migrate-attachments.pl --migrate database s3
-
 'mirror'
     Copies all attachments from the specified source to the destination.
     Attachments which already exist at the destination will not be copied
@@ -68,68 +59,33 @@ EOF
 
 my $dbh = Bugzilla->dbh;
 
-if ($options{migrate}) {
-  if ($options{migrate}->[0] eq $options{migrate}->[1]) {
-    die "Source and destination must be different\n";
-  }
-  my ($source, $dest) = @{$options{migrate}};
-
-  my ($total) = $dbh->selectrow_array("SELECT COUNT(*) FROM attachments");
-  confirm(sprintf(
-    'Migrate %s attachments from %s to %s?', $total, @{$options{migrate}}));
-
-  my $sth
-    = $dbh->prepare("SELECT attach_id FROM attachments ORDER BY attach_id DESC");
-  $sth->execute();
-  my ($count, $migrated) = (0, 0);
-  while (my ($attach_id) = $sth->fetchrow_array()) {
-    indicate_progress({total => $total, current => ++$count});
-
-    my $attachment = Bugzilla::Attachment->new({id => $attach_id, cached => 1});
-
-    # skip deleted attachments
-    next if $attachment->datasize == 0;
-
-    # migrate the attachment
-    if (my $data = $attachment->current_storage($source)->get_data()) {
-      $attachment->current_storage($dest)->set_data($data)->set_class();
-      $migrated++;
-    }
-  }
-  print "\n";
-  print "Attachments migrated: $migrated\n";
-}
-
 if ($options{mirror}) {
   if ($options{mirror}->[0] eq $options{mirror}->[1]) {
     die "Source and destination must be different\n";
   }
-  my ($source, $dest) = @{$options{mirror}};
+  my ($source, $dest) = map { storage($_) } @{$options{mirror}};
 
   my ($total) = $dbh->selectrow_array("SELECT COUNT(*) FROM attachments");
   confirm(sprintf(
     'Mirror %s attachments from %s to %s?', $total, @{$options{mirror}}));
 
-  my $sth
-    = $dbh->prepare("SELECT attach_id FROM attachments ORDER BY attach_id DESC");
+  my $sth = $dbh->prepare(
+    "SELECT attach_id, attach_size FROM attachments ORDER BY attach_id DESC");
   $sth->execute();
   my ($count, $deleted, $stored) = (0, 0, 0);
-  while (my ($attach_id) = $sth->fetchrow_array()) {
+  while (my ($attach_id, $attach_size) = $sth->fetchrow_array()) {
     indicate_progress({total => $total, current => ++$count});
 
-    my $attachment = Bugzilla::Attachment->new({id => $attach_id, cached => 1});
-
     # remove deleted attachments
-    if ($attachment->datasize == 0 && $attachment->current_storage($dest)->data_exists()) {
-      $attachment->current_storage($dest)->remove_data();
+    if ($attach_size == 0 && $dest->exists($attach_id)) {
+      $dest->remove($attach_id);
       $deleted++;
     }
 
     # store attachments that don't already exist
-    elsif ($attachment->datasize != 0 && !$attachment->current_storage($dest)->data_exists())
-    {
-      if (my $data = $attachment->current_storage($source)->get_data()) {
-        $attachment->current_storage($dest)->set_data($data);
+    elsif ($attach_size != 0 && !$dest->exists($attach_id)) {
+      if (my $data = $source->retrieve($attach_id)) {
+        $dest->store($attach_id, $data);
         $stored++;
       }
     }
@@ -143,7 +99,7 @@ elsif ($options{copy}) {
   if ($options{copy}->[0] eq $options{copy}->[1]) {
     die "Source and destination must be different\n";
   }
-  my ($source, $dest) = @{$options{copy}};
+  my ($source, $dest) = map { storage($_) } @{$options{copy}};
 
   my ($total)
     = $dbh->selectrow_array(
@@ -153,19 +109,17 @@ elsif ($options{copy}) {
 
   my $sth
     = $dbh->prepare(
-    "SELECT attach_id FROM attachments WHERE attach_size != 0 ORDER BY attach_id DESC"
+    "SELECT attach_id, attach_size FROM attachments WHERE attach_size != 0 ORDER BY attach_id DESC"
     );
   $sth->execute();
   my ($count, $stored) = (0, 0);
-  while (my ($attach_id) = $sth->fetchrow_array()) {
+  while (my ($attach_id, $attach_size) = $sth->fetchrow_array()) {
     indicate_progress({total => $total, current => ++$count});
 
-    my $attachment = Bugzilla::Attachment->new({id => $attach_id, cached => 1});
-
     # store attachments that don't already exist
-    if (!$attachment->current_storage($dest)->data_exists()) {
-      if (my $data = $attachment->current_storage($source)->get_data()) {
-        $attachment->current_storage($dest)->set_data($data);
+    if (!$dest->exists($attach_id)) {
+      if (my $data = $source->retrieve($attach_id)) {
+        $dest->store($attach_id, $data);
         $stored++;
       }
     }
@@ -175,7 +129,7 @@ elsif ($options{copy}) {
 }
 
 elsif ($options{delete}) {
-  my $storage = $options{delete};
+  my $storage = storage($options{delete});
   my ($total)
     = $dbh->selectrow_array(
     "SELECT COUNT(*) FROM attachments WHERE attach_size != 0");
@@ -189,11 +143,8 @@ elsif ($options{delete}) {
   my ($count, $deleted) = (0, 0);
   while (my ($attach_id) = $sth->fetchrow_array()) {
     indicate_progress({total => $total, current => ++$count});
-
-    my $attachment = Bugzilla::Attachment->new({id => $attach_id, cached => 1});
-
-    if ($attachment->current_storage($storage)->data_exists()) {
-      $attachment->current_storage($storage)->remove_data();
+    if ($storage->exists($attach_id)) {
+      $storage->remove($attach_id);
       $deleted++;
     }
   }
@@ -201,6 +152,13 @@ elsif ($options{delete}) {
   print "Attachments deleted: $deleted\n";
 }
 
+sub storage {
+  my ($name) = @_;
+  my $storage = Bugzilla::Attachment::get_storage_by_name($name)
+    or die "Invalid attachment location: $name\n";
+  return $storage;
+}
+
 sub confirm {
   my ($prompt) = @_;
   print $prompt, "\n\nPress <Ctrl-C> to stop or <Enter> to continue..\n";
diff --git a/template/en/default/admin/params/attachment.html.tmpl b/template/en/default/admin/params/attachment.html.tmpl
index cfabd9d62..5f0d5844e 100644
--- a/template/en/default/admin/params/attachment.html.tmpl
+++ b/template/en/default/admin/params/attachment.html.tmpl
@@ -47,10 +47,5 @@
     "must use <tt>scripts/migrate-attachments</tt> to migrate existing " _
     "attachments.",
 
-  attachment_s3_minsize =>
-    "When attachment_storage is set to S3, only attachments larger than this " _
-    "value will be uploaded to S3 for performance reasons. Smaller attachments " _
-    "will be stored in the database."
-
   }
 %]