[Rt-commit] rt branch, 4.4/external-storage, created. rt-4.2.11-13-g08b1636

Shawn Moore shawn at bestpractical.com
Fri Jun 5 14:51:31 EDT 2015


The branch, 4.4/external-storage has been created
        at  08b1636199ad8b107bc45a9a5adc1771d906aacf (commit)

- Log -----------------------------------------------------------------
commit 428c9c45c4c552c2c469a3d14acef766882742fb
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu May 21 19:56:46 2015 +0000

    Verbatim copy of RT::Extension::ExternalStorage
    
        at a9df4596ba43cd64552a2c9f3173e82bd0690b78

diff --git a/lib/RT/Extension/ExternalStorage.pm b/lib/RT/Extension/ExternalStorage.pm
new file mode 100644
index 0000000..cfac2d8
--- /dev/null
+++ b/lib/RT/Extension/ExternalStorage.pm
@@ -0,0 +1,283 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use 5.008003;
+use warnings;
+use strict;
+
+package RT::Extension::ExternalStorage;
+
+our $VERSION = '0.61';
+
+use Digest::SHA qw//;
+
+require RT::Extension::ExternalStorage::Backend;
+
+=head1 NAME
+
+RT::Extension::ExternalStorage - Store attachments outside the database
+
+=head1 SYNOPSIS
+
+    Set( @Plugins, 'RT::Extension::ExternalStorage' );
+
+    Set(%ExternalStorage,
+        Type => 'Disk',
+        Path => '/opt/rt4/var/attachments',
+    );
+
+=head1 DESCRIPTION
+
+By default, RT stores attachments in the database.  This extension moves
+all attachments that RT does not need efficient access to (which include
+textual content and images) to outside of the database.  This may either
+be on local disk, or to a cloud storage solution.  This decreases the
+size of RT's database, in turn decreasing the burden of backing up RT's
+database, at the cost of adding additional locations which must be
+configured or backed up.  Attachment storage paths are calculated based
+on file contents; this provides de-duplication.
+
+The files are initially stored in the database when RT receives them;
+this guarantees that the user does not need to wait for the file to be
+transferred to disk or to the cloud, and makes it durable to transient
+failures of cloud connectivity.  The provided C<bin/extract-attachments>
+script, to be run regularly via cron, takes care of moving attachments
+out of the database at a later time.
+
+=head1 INSTALLATION
+
+=over
+
+=item C<perl Makefile.PL>
+
+=item C<make>
+
+=item C<make install>
+
+May need root permissions
+
+=item Edit your F</opt/rt4/etc/RT_SiteConfig.pm>
+
+If you are using RT 4.2 or greater, add this line:
+
+    Plugin('RT::Extension::ExternalStorage');
+
+For RT 4.0, add this line:
+
+    Set(@Plugins, qw(RT::Extension::ExternalStorage));
+
+or add C<RT::Extension::ExternalStorage> to your existing C<@Plugins> line.
+
+You will also need to configure the C<%ExternalStorage> option,
+depending on how and where you want your data stored; see
+L</CONFIGURATION>.
+
+=item Restart your webserver
+
+Restarting the webserver before the next step (extracting existing
+attachments) is important to ensure that files remain available as they
+are extracted.
+
+=item Extract existing attachments
+
+Run C<bin/extract-attachments>; this may take some time, depending on
+the existing size of the database.  This task may be safely cancelled
+and re-run to resume.
+
+=item Schedule attachments extraction
+
+Schedule C<bin/extract-attachments> to run at regular intervals via
+cron.  For instance, the following F</etc/cron.d/rt> entry will run it
+daily, which may be good to concentrate network or disk usage to times
+when RT is less in use:
+
+    0 0 * * * root /opt/rt4/local/plugins/RT-Extension-ExternalStorage/bin/extract-attachments
+
+=back
+
+=head1 CONFIGURATION
+
+This module comes with a number of possible backends; see the
+documentation in each for necessary configuration details:
+
+=over
+
+=item L<RT::Extension::ExternalStorage::Disk>
+
+=item L<RT::Extension::ExternalStorage::Dropbox>
+
+=item L<RT::Extension::ExternalStorage::AmazonS3>
+
+=back
+
+=head1 CAVEATS
+
+This extension is not currently compatibile with RT's C<shredder> tool;
+attachments which are shredded will not be removed from external
+storage.
+
+=cut
+
+our $BACKEND;
+our $WRITE;
+$RT::Config::META{ExternalStorage} = {
+    Type => 'HASH',
+    PostLoadCheck => sub {
+        my $self = shift;
+        my %hash = $self->Get('ExternalStorage');
+        return unless keys %hash;
+        $hash{Write} = $WRITE;
+        $BACKEND = RT::Extension::ExternalStorage::Backend->new( %hash );
+    },
+};
+
+sub Store {
+    my $class = shift;
+    my $content = shift;
+
+    my $key = Digest::SHA::sha256_hex( $content );
+    my ($ok, $msg) = $BACKEND->Store( $key => $content );
+    return ($ok, $msg) unless defined $ok;
+
+    return ($key);
+}
+
+
+package RT::Record;
+
+no warnings 'redefine';
+my $__DecodeLOB = __PACKAGE__->can('_DecodeLOB');
+*_DecodeLOB = sub {
+    my $self            = shift;
+    my $ContentType     = shift || '';
+    my $ContentEncoding = shift || 'none';
+    my $Content         = shift;
+    my $Filename        = @_;
+
+    return $__DecodeLOB->($self, $ContentType, $ContentEncoding, $Content, $Filename)
+        unless $ContentEncoding eq "external";
+
+    unless ($BACKEND) {
+        RT->Logger->error( "Failed to load $Content; external storage not configured" );
+        return ("");
+    };
+
+    my ($ok, $msg) = $BACKEND->Get( $Content );
+    unless (defined $ok) {
+        RT->Logger->error( "Failed to load $Content from external storage: $msg" );
+        return ("");
+    }
+
+    return $__DecodeLOB->($self, $ContentType, 'none', $ok, $Filename);
+};
+
+package RT::ObjectCustomFieldValue;
+
+sub StoreExternally {
+    my $self = shift;
+    my $type = $self->CustomFieldObj->Type;
+    my $length = length($self->LargeContent || '');
+
+    return 0 if $length == 0;
+
+    return 1 if $type eq "Binary";
+
+    return 1 if $type eq "Image" and $length > 10 * 1024 * 1024;
+
+    return 0;
+}
+
+package RT::Attachment;
+
+sub StoreExternally {
+    my $self = shift;
+    my $type = $self->ContentType;
+    my $length = $self->ContentLength;
+
+    return 0 if $length == 0;
+
+    if ($type =~ m{^multipart/}) {
+        return 0;
+    } elsif ($type =~ m{^(text|message)/}) {
+        # If textual, we only store externally if it's _large_ (> 10M)
+        return 1 if $length > 10 * 1024 * 1024;
+        return 0;
+    } elsif ($type =~ m{^image/}) {
+        # Ditto images, which may be displayed inline
+        return 1 if $length > 10 * 1024 * 1024;
+        return 0;
+    } else {
+        return 1;
+    }
+}
+
+=head1 AUTHOR
+
+Best Practical Solutions, LLC E<lt>modules at bestpractical.comE<gt>
+
+=head1 BUGS
+
+All bugs should be reported via email to
+
+    L<bug-RT-Extension-ExternalStorage at rt.cpan.org|mailto:bug-RT-Extension-ExternalStorage at rt.cpan.org>
+
+or via the web at
+
+    L<rt.cpan.org|http://rt.cpan.org/Public/Dist/Display.html?Name=RT-Extension-ExternalStorage>.
+
+=head1 COPYRIGHT
+
+This extension is Copyright (C) 2009-2015 Best Practical Solutions, LLC.
+
+This is free software, licensed under:
+
+  The GNU General Public License, Version 2, June 1991
+
+=cut
+
+1;
diff --git a/lib/RT/Extension/ExternalStorage/AmazonS3.pm b/lib/RT/Extension/ExternalStorage/AmazonS3.pm
new file mode 100644
index 0000000..773ab4d
--- /dev/null
+++ b/lib/RT/Extension/ExternalStorage/AmazonS3.pm
@@ -0,0 +1,197 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use 5.008003;
+use warnings;
+use strict;
+
+package RT::Extension::ExternalStorage::AmazonS3;
+
+use Role::Basic qw/with/;
+with 'RT::Extension::ExternalStorage::Backend';
+
+our( $S3, $BUCKET);
+sub Init {
+    my $self = shift;
+    my %self = %{$self};
+
+    if (not Amazon::S3->require) {
+        RT->Logger->error("Required module Amazon::S3 is not installed");
+        return;
+    } elsif (not $self{AccessKeyId}) {
+        RT->Logger->error("AccessKeyId not provided for AmazonS3");
+        return;
+    } elsif (not $self{SecretAccessKey}) {
+        RT->Logger->error("SecretAccessKey not provided for AmazonS3");
+        return;
+    } elsif (not $self{Bucket}) {
+        RT->Logger->error("Bucket not provided for AmazonS3");
+        return;
+    }
+
+
+    $S3 = Amazon::S3->new( {
+        aws_access_key_id     => $self{AccessKeyId},
+        aws_secret_access_key => $self{SecretAccessKey},
+        retry                 => 1,
+    } );
+
+    my $buckets = $S3->bucket( $self{Bucket} );
+    unless ( $buckets ) {
+        RT->Logger->error("Can't list buckets of AmazonS3: ".$S3->errstr);
+        return;
+    }
+    unless ( grep {$_->bucket eq $self{Bucket}} @{$buckets->{buckets}} ) {
+        my $ok = $S3->add_bucket( {
+            bucket    => $self{Bucket},
+            acl_short => 'private',
+        } );
+        unless ($ok) {
+            RT->Logger->error("Can't create new bucket '$self{Bucket}' on AmazonS3: ".$S3->errstr);
+            return;
+        }
+    }
+
+    return $self;
+}
+
+sub Get {
+    my $self = shift;
+    my ($sha) = @_;
+
+    my $ok = $S3->bucket($self->{Bucket})->get_key( $sha );
+    return (undef, "Could not retrieve from AmazonS3:" . $S3->errstr)
+        unless $ok;
+    return ($ok->{value});
+}
+
+sub Store {
+    my $self = shift;
+    my ($sha, $content) = @_;
+
+    # No-op if the path exists already
+    return (1) if $S3->bucket($self->{Bucket})->head_key( $sha );
+
+    $S3->bucket($self->{Bucket})->add_key(
+        $sha => $content
+    ) or return (undef, "Failed to write to AmazonS3: " . $S3->errstr);
+
+    return (1);
+}
+
+=head1 NAME
+
+RT::Extension::ExternalStorage::Dropbox - Store files in the Dropbox cloud
+
+=head1 SYNOPSIS
+
+    Set(%ExternalStorage,
+        Type => 'Dropbox',
+        AccessToken => '...',
+    );
+
+=head1 DESCRIPTION
+
+This storage option places attachments in the Dropbox shared file
+service.  The files are de-duplicated when they are saved; as such, if
+the same file appears in multiple transactions, only one copy will be
+stored on in Dropbox.
+
+Files in Dropbox C<must not be modified or removed>; doing so may cause
+internal inconsistency.
+
+=head1 SETUP
+
+In order to use this stoage type, a new application must be registered
+with Dropbox:
+
+=over
+
+=item 1.
+
+Log into Dropbox as the user you wish to store files as.
+
+=item 2.
+
+Click C<Create app> on L<https://www.dropbox.com/developers/apps>
+
+=item 3.
+
+Choose B<Dropbox API app> as the type of app.
+
+=item 4.
+
+Choose the B<Files and datastores> as the type of data to store.
+
+=item 5.
+
+Choose B<Yes>, your application only needs access to files it creates.
+
+=item 6.
+
+Enter a descriptive name -- C<Request Tracker files> is fine.
+
+=item 7.
+
+Under C<Generated access token>, click the C<Generate> button.
+
+=item 8.
+
+Copy the provided value into your F<RT_SiteConfig.pm> file as the
+C<AccessToken>:
+
+    Set(%ExternalStorage,
+        Type => 'Dropbox',
+        AccessToken => '...',   # Replace the value here, between the quotes
+    );
+
+=back
+
+=cut
+
+1;
diff --git a/lib/RT/Extension/ExternalStorage/Backend.pm b/lib/RT/Extension/ExternalStorage/Backend.pm
new file mode 100644
index 0000000..03b0898
--- /dev/null
+++ b/lib/RT/Extension/ExternalStorage/Backend.pm
@@ -0,0 +1,89 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use 5.008003;
+use warnings;
+use strict;
+
+package RT::Extension::ExternalStorage::Backend;
+
+use Role::Basic;
+
+requires 'Init';
+requires 'Get';
+requires 'Store';
+
+sub new {
+    my $class = shift;
+    my %args = @_;
+
+    $class = delete $args{Type};
+    if (not $class) {
+        RT->Logger->error("No storage engine type provided");
+        return undef;
+    } elsif ($class->require) {
+    } else {
+        my $long = "RT::Extension::ExternalStorage::$class";
+        if ($long->require) {
+            $class = $long;
+        } else {
+            RT->Logger->error("Can't load external storage engine $class: $@");
+            return undef;
+        }
+    }
+
+    unless ($class->DOES("RT::Extension::ExternalStorage::Backend")) {
+        RT->Logger->error("External storage engine $class doesn't implement RT::Extension::ExternalStorage::Backend");
+        return undef;
+    }
+
+    my $self = bless \%args, $class;
+    $self->Init;
+}
+
+1;
diff --git a/lib/RT/Extension/ExternalStorage/Disk.pm b/lib/RT/Extension/ExternalStorage/Disk.pm
new file mode 100644
index 0000000..e93fb84
--- /dev/null
+++ b/lib/RT/Extension/ExternalStorage/Disk.pm
@@ -0,0 +1,144 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use 5.008003;
+use warnings;
+use strict;
+
+package RT::Extension::ExternalStorage::Disk;
+
+use File::Path qw//;
+
+use Role::Basic qw/with/;
+with 'RT::Extension::ExternalStorage::Backend';
+
+sub Init {
+    my $self = shift;
+
+    my %self = %{$self};
+    if (not $self{Path}) {
+        RT->Logger->error("No path provided for local storage");
+        return;
+    } elsif (not -e $self{Path}) {
+        RT->Logger->error("Path provided for local storage ($self{Path}) does not exist");
+        return;
+    } elsif ($self{Write} and not -w $self{Path}) {
+        RT->Logger->error("Path provided for local storage ($self{Path}) is not writable");
+        return;
+    }
+
+    return $self;
+}
+
+sub Get {
+    my $self = shift;
+    my ($sha) = @_;
+
+    $sha =~ m{^(...)(...)(.*)};
+    my $path = $self->{Path} . "/$1/$2/$3";
+
+    return (undef, "File does not exist") unless -e $path;
+
+    open(my $fh, "<", $path) or return (undef, "Cannot read file on disk: $!");
+    my $content = do {local $/; <$fh>};
+    $content = "" unless defined $content;
+    close $fh;
+
+    return ($content);
+}
+
+sub Store {
+    my $self = shift;
+    my ($sha, $content) = @_;
+
+    $sha =~ m{^(...)(...)(.*)};
+    my $dir  = $self->{Path} . "/$1/$2";
+    my $path = "$dir/$3";
+
+    return (1) if -f $path;
+
+    File::Path::make_path($dir, {error => \my $err});
+    return (undef, "Making directory failed") if @{$err};
+
+    open( my $fh, ">:raw", $path ) or return (undef, "Cannot write file on disk: $!");
+    print $fh $content or return (undef, "Cannot write file to disk: $!");
+    close $fh or return (undef, "Cannot write file to disk: $!");
+
+    return (1);
+}
+
+=head1 NAME
+
+RT::Extension::ExternalStorage::Disk - On-disk storage of attachments
+
+=head1 SYNOPSIS
+
+    Set(%ExternalStorage,
+        Type => 'Disk',
+        Path => '/opt/rt4/var/attachments',
+    );
+
+=head1 DESCRIPTION
+
+This storage option places attachments on disk under the given C<Path>,
+uncompressed.  The files are de-duplicated when they are saved; as such,
+if the same file appears in multiple transactions, only one copy will be
+stored on disk.
+
+The C<Path> must be readable by the webserver, and writable by the
+C<bin/extract-attachments> script.  Because the majority of the
+attachments are in the filesystem, a simple database backup is thus
+incomplete.  It is B<extremely important> that I<backups include the
+on-disk attachments directory>.
+
+Files also C<must not be modified or removed>; doing so may cause
+internal inconsistency.
+
+=cut
+
+1;
diff --git a/lib/RT/Extension/ExternalStorage/Dropbox.pm b/lib/RT/Extension/ExternalStorage/Dropbox.pm
new file mode 100644
index 0000000..d57bf6b
--- /dev/null
+++ b/lib/RT/Extension/ExternalStorage/Dropbox.pm
@@ -0,0 +1,184 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use 5.008003;
+use warnings;
+use strict;
+
+package RT::Extension::ExternalStorage::Dropbox;
+
+use Role::Basic qw/with/;
+with 'RT::Extension::ExternalStorage::Backend';
+
+our $DROPBOX;
+sub Init {
+    my $self = shift;
+    my %self = %{$self};
+
+    if (not File::Dropbox->require) {
+        RT->Logger->error("Required module File::Dropbox is not installed");
+        return;
+    } elsif (not $self{AccessToken}) {
+        RT->Logger->error("AccessToken not provided for Dropbox.  Register a new application"
+                      . " at https://www.dropbox.com/developers/apps and generate an access token.");
+        return;
+    }
+
+
+    $DROPBOX = File::Dropbox->new(
+        oauth2       => 1,
+        access_token => $self{AccessToken},
+        root         => 'sandbox',
+        furlopts     => { timeout => 60 },
+    );
+
+    return $self;
+}
+
+sub Get {
+    my $self = shift;
+    my ($sha) = @_;
+
+    open( $DROPBOX, "<", $sha)
+        or return (undef, "Failed to retrieve file from dropbox: $!");
+    my $content = do {local $/; <$DROPBOX>};
+    close $DROPBOX;
+
+    return ($content);
+}
+
+sub Store {
+    my $self = shift;
+    my ($sha, $content) = @_;
+
+    # No-op if the path exists already.  This forces a metadata read.
+    return (1) if open( $DROPBOX, "<", $sha);
+
+    open( $DROPBOX, ">", $sha )
+        or return (undef, "Open for write on dropbox failed: $!");
+    print $DROPBOX $content
+        or return (undef, "Write to dropbox failed: $!");
+    close $DROPBOX
+        or return (undef, "Flush to dropbox failed: $!");
+
+    return (1);
+}
+
+=head1 NAME
+
+RT::Extension::ExternalStorage::Dropbox - Store files in the Dropbox cloud
+
+=head1 SYNOPSIS
+
+    Set(%ExternalStorage,
+        Type => 'Dropbox',
+        AccessToken => '...',
+    );
+
+=head1 DESCRIPTION
+
+This storage option places attachments in the Dropbox shared file
+service.  The files are de-duplicated when they are saved; as such, if
+the same file appears in multiple transactions, only one copy will be
+stored on in Dropbox.
+
+Files in Dropbox C<must not be modified or removed>; doing so may cause
+internal inconsistency.  It is also important to ensure that the Dropbox
+account used has sufficient space for the attachments, and to monitor
+its space usage.
+
+=head1 SETUP
+
+In order to use this stoage type, a new application must be registered
+with Dropbox:
+
+=over
+
+=item 1.
+
+Log into Dropbox as the user you wish to store files as.
+
+=item 2.
+
+Click C<Create app> on L<https://www.dropbox.com/developers/apps>
+
+=item 3.
+
+Choose B<Dropbox API app> as the type of app.
+
+=item 4.
+
+Choose the B<Files and datastores> as the type of data to store.
+
+=item 5.
+
+Choose B<Yes>, your application only needs access to files it creates.
+
+=item 6.
+
+Enter a descriptive name -- C<Request Tracker files> is fine.
+
+=item 7.
+
+Under C<Generated access token>, click the C<Generate> button.
+
+=item 8.
+
+Copy the provided value into your F<RT_SiteConfig.pm> file as the
+C<AccessToken>:
+
+    Set(%ExternalStorage,
+        Type => 'Dropbox',
+        AccessToken => '...',   # Replace the value here, between the quotes
+    );
+
+=back
+
+=cut
+
+1;
diff --git a/lib/RT/Extension/ExternalStorage/Test.pm.in b/lib/RT/Extension/ExternalStorage/Test.pm.in
new file mode 100644
index 0000000..9032217
--- /dev/null
+++ b/lib/RT/Extension/ExternalStorage/Test.pm.in
@@ -0,0 +1,98 @@
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+
+use strict;
+use warnings;
+
+### after: use lib qw(@RT_LIB_PATH@);
+use lib qw(/opt/rt4/local/lib /opt/rt4/lib);
+
+package RT::Extension::ExternalStorage::Test;
+
+=head2 RT::Extension::ExternalStorage::Test
+
+Initialization for testing.
+
+=cut
+
+use base qw(RT::Test);
+use File::Spec;
+use File::Path 'mkpath';
+
+sub import {
+    my $class = shift;
+    my %args  = @_;
+
+    $args{'requires'} ||= [];
+    if ( $args{'testing'} ) {
+        unshift @{ $args{'requires'} }, 'RT::Extension::ExternalStorage';
+    } else {
+        $args{'testing'} = 'RT::Extension::ExternalStorage';
+    }
+
+    $class->SUPER::import( %args );
+    $class->export_to_level(1);
+
+    require RT::Extension::ExternalStorage;
+}
+
+sub attachments_dir {
+    my $dir = File::Spec->catdir( RT::Test->temp_directory, qw(attachments) );
+    mkpath($dir);
+    return $dir;
+}
+
+sub bootstrap_more_config {
+    my $self = shift;
+    my ($config) = @_;
+
+    my $dir = $self->attachments_dir;
+    print $config qq|Set( %ExternalStorage, Type => 'Disk', Path => '$dir' );\n|;
+}
+
+1;
diff --git a/sbin/extract-attachments.in b/sbin/extract-attachments.in
new file mode 100755
index 0000000..ce5dedb
--- /dev/null
+++ b/sbin/extract-attachments.in
@@ -0,0 +1,167 @@
+#!/usr/bin/env perl
+### before: #!@PERL@
+
+# BEGIN BPS TAGGED BLOCK {{{
+#
+# COPYRIGHT:
+#
+# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+#                                          <sales at bestpractical.com>
+#
+# (Except where explicitly superseded by other copyright notices)
+#
+#
+# LICENSE:
+#
+# This work is made available to you under the terms of Version 2 of
+# the GNU General Public License. A copy of that license should have
+# been provided with this software, but in any event can be snarfed
+# from www.gnu.org.
+#
+# This work is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 or visit their web page on the internet at
+# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+#
+#
+# CONTRIBUTION SUBMISSION POLICY:
+#
+# (The following paragraph is not intended to limit the rights granted
+# to you to modify and distribute this software under the terms of
+# the GNU General Public License and is only of importance to you if
+# you choose to contribute your changes and enhancements to the
+# community by submitting them to Best Practical Solutions, LLC.)
+#
+# By intentionally submitting any modifications, corrections or
+# derivatives to this work, or any other work intended for use with
+# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+# you are the copyright holder for those contributions and you grant
+# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+# royalty-free, perpetual, license to use, copy, create derivative
+# works based on those contributions, and sublicense and distribute
+# those contributions and any derivatives thereof.
+#
+# END BPS TAGGED BLOCK }}}
+use strict;
+use warnings;
+
+### after: use lib qw(@RT_LIB_PATH@);
+use lib qw(/opt/rt4/local/lib /opt/rt4/lib);
+
+BEGIN { $RT::Extension::ExternalStorage::WRITE = 1 };
+use RT -init;
+
+# Ensure we only run one of these processes at once
+use Fcntl ':flock';
+exit unless flock main::DATA, LOCK_EX | LOCK_NB;
+
+die "\%ExternalStorage is not configured\n"
+    unless RT->Config->Get("ExternalStorage");
+
+exit unless $RT::Extension::ExternalStorage::BACKEND;
+
+my $last = RT->System->FirstAttribute("ExternalStorage");
+$last = $last ? $last->Content : {};
+
+for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
+    my $column = $class eq 'RT::Attachments' ? "Content" : "LargeContent";
+    my $id = $last->{$class} || 0;
+
+    while (1) {
+        my $attach = $class->new($RT::SystemUser);
+        $attach->Limit(
+            FIELD    => 'id',
+            OPERATOR => '>',
+            VALUE    => $id,
+        );
+        $attach->Limit(
+            FIELD           => 'ContentEncoding',
+            OPERATOR        => '!=',
+            VALUE           => 'external',
+            ENTRYAGGREGATOR => 'AND',
+        );
+        if ($class eq "RT::Attachments") {
+            $attach->_OpenParen('applies');
+            $attach->Limit(
+                FIELD     => 'ContentType',
+                OPERATOR  => 'NOT STARTSWITH',
+                VALUE     => $_,
+                SUBCLAUSE => 'applies',
+                ENTRYAGGREGATOR => "AND",
+            ) for "text/", "message/", "image/", "multipart/";
+            $attach->_CloseParen('applies');
+            $attach->Limit(
+                FUNCTION  => 'LENGTH(main.Content)',
+                OPERATOR  => '>',
+                VALUE     => 10*1024*1024,
+                SUBCLAUSE => 'applies',
+                ENTRYAGGREGATOR => 'OR',
+            );
+        } else {
+            my $cfs = $attach->Join(
+                ALIAS1 => 'main',
+                FIELD1 => 'CustomField',
+                TABLE2 => 'CustomFields',
+                FIELD2 => 'id',
+            );
+            # TODO: use IN operator once we increase required RT version to 4.2
+            $attach->Limit(
+                ALIAS => $cfs,
+                FIELD => "Type",
+                VALUE => $_,
+            ) for qw(Binary Image);
+            $attach->{'find_expired_rows'} = 1;
+        }
+
+        $attach->RowsPerPage(100);
+        $RT::Handle->dbh->begin_work;
+        while ( my $a = $attach->Next ) {
+            $id = $a->id;
+            next unless $a->StoreExternally;
+
+            # Explicitly get bytes (not characters, which ->$column would do)
+            my $content = $a->_DecodeLOB(
+                "application/octet-stream",
+                $a->ContentEncoding,
+                $a->_Value( $column, decode_utf8 => 0),
+            );
+
+            # Attempt to write that out
+            my ($key, $msg) = RT::Extension::ExternalStorage->Store( $content );
+            unless ($key) {
+                RT->Logger->error("Failed to store $class $id: $msg");
+                exit 1;
+            }
+
+            (my $status, $msg ) = $a->__Set(
+                Field => $column, Value => $key
+            );
+            unless ($status) {
+                RT->Logger->error("Failed to update $column of $class $id: $msg");
+                exit 2;
+            }
+
+            ( $status, $msg ) = $a->__Set(
+                Field => 'ContentEncoding', Value => 'external',
+            );
+            unless ($status) {
+                RT->Logger->error("Failed to update ContentEncoding of $class $id: $msg");
+                exit 2;
+            }
+        }
+        $RT::Handle->dbh->commit;
+
+        last unless $attach->Count;
+    }
+    $last->{$class} = $id;
+}
+
+RT->System->SetAttribute( Name => "ExternalStorage", Content => $last );
+
+__DATA__
diff --git a/t/externalstorage/basic.t b/t/externalstorage/basic.t
new file mode 100644
index 0000000..281d2e7
--- /dev/null
+++ b/t/externalstorage/basic.t
@@ -0,0 +1,76 @@
+use strict;
+use warnings;
+
+use RT::Extension::ExternalStorage::Test tests => undef;
+
+my $queue = RT::Test->load_or_create_queue(Name => 'General');
+ok $queue && $queue->id;
+
+my $message = MIME::Entity->build(
+    From    => 'root at localhost',
+    Subject => 'test',
+    Data    => 'test',
+);
+$message->attach(
+    Type     => 'image/special',
+    Filename => 'afile.special',
+    Data     => 'boo',
+);
+$message->attach(
+    Type     => 'application/octet-stream',
+    Filename => 'otherfile.special',
+    Data     => 'thing',
+);
+my $ticket = RT::Ticket->new( RT->SystemUser );
+my ($id) = $ticket->Create(
+    Queue => $queue,
+    Subject => 'test',
+    MIMEObj => $message,
+);
+
+ok $id, 'created a ticket';
+
+my @attachs = @{ $ticket->Transactions->First->Attachments->ItemsArrayRef };
+is scalar @attachs, 4, "Contains a multipart and two sub-parts";
+
+is $attachs[0]->ContentType, "multipart/mixed", "Found the top multipart";
+ok !$attachs[0]->StoreExternally, "Shouldn't store multipart part on disk";
+
+is $attachs[1]->ContentType, "text/plain", "Found the text part";
+is $attachs[1]->Content, 'test', "Can get the text part content";
+is $attachs[1]->ContentEncoding, "none", "Content is not encoded";
+ok !$attachs[1]->StoreExternally, "Won't store text part on disk";
+
+is $attachs[2]->ContentType, "image/special", "Found the image part";
+is $attachs[2]->Content, 'boo',  "Can get the image content";
+is $attachs[2]->ContentEncoding, "none", "Content is not encoded";
+ok !$attachs[2]->StoreExternally, "Won't store images on disk";
+
+is $attachs[3]->ContentType, "application/octet-stream", "Found the binary part";
+is $attachs[3]->Content, 'thing',  "Can get the binary content";
+is $attachs[3]->ContentEncoding, "none", "Content is not encoded";
+ok $attachs[3]->StoreExternally, "Will store binary data on disk";
+
+my $dir = RT::Extension::ExternalStorage::Test->attachments_dir;
+ok !<$dir/*>, "Attachments directory is empty";
+
+
+ok -e 'sbin/extract-attachments', "Found extract-attachments script";
+ok -x 'sbin/extract-attachments', "extract-attachments is executable";
+ok !system('sbin/extract-attachments'), "extract-attachments ran successfully";
+
+ at attachs = @{ $ticket->Transactions->First->Attachments->ItemsArrayRef };
+is $attachs[1]->Content, 'test', "Can still get the text part content";
+is $attachs[1]->ContentEncoding, "none", "Content is not encoded";
+
+is $attachs[2]->Content, 'boo',  "Can still get the image content";
+is $attachs[2]->ContentEncoding, "none", "Content is not encoded";
+
+is $attachs[3]->ContentType, "application/octet-stream", "Found the binary part";
+is $attachs[3]->Content, 'thing',  "Can still get the binary content";
+isnt $attachs[3]->__Value('Content'), "thing", "Content in database is not the raw content";
+is $attachs[3]->ContentEncoding, "external", "Content encoding is 'external'";
+
+ok <$dir/*>, "Attachments directory contains files";
+
+done_testing();

commit b4bebf8db1d3a1ae0be09c041573a0bc8b6e0688
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu May 21 20:02:15 2015 +0000

    Move ExternalStorage files into proper namespaces
    
        This includes only the bare minimum of code changes in order to make
        this look like it was part of core RT all along.

diff --git a/.gitignore b/.gitignore
index 855514b..7d7d823 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,6 +27,7 @@
 /sbin/rt-email-dashboards
 /sbin/rt-email-digest
 /sbin/rt-email-group-admin
+/sbin/rt-externalize-attachments
 /sbin/rt-fulltext-indexer
 /sbin/rt-preferences-viewer
 /sbin/rt-server
diff --git a/Makefile.in b/Makefile.in
index b5ebfb9..3d997c8 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -144,6 +144,7 @@ SYSTEM_BINARIES		=	rt-attributes-viewer \
 				rt-email-dashboards \
 				rt-email-digest \
 				rt-email-group-admin \
+				rt-externalize-attachments \
 				rt-fulltext-indexer \
 				rt-importer \
 				rt-preferences-viewer \
diff --git a/configure.ac b/configure.ac
index 1bcf2ba..7df7b37 100755
--- a/configure.ac
+++ b/configure.ac
@@ -436,6 +436,7 @@ AC_CONFIG_FILES([
                  sbin/rt-test-dependencies
                  sbin/rt-email-digest
                  sbin/rt-email-dashboards
+                 sbin/rt-externalize-attachments
                  sbin/rt-clean-sessions
                  sbin/rt-shredder
                  sbin/rt-validator
diff --git a/lib/RT/Extension/ExternalStorage.pm b/lib/RT/ExternalStorage.pm
similarity index 83%
rename from lib/RT/Extension/ExternalStorage.pm
rename to lib/RT/ExternalStorage.pm
index cfac2d8..66bd2fb 100644
--- a/lib/RT/Extension/ExternalStorage.pm
+++ b/lib/RT/ExternalStorage.pm
@@ -50,21 +50,21 @@ use 5.008003;
 use warnings;
 use strict;
 
-package RT::Extension::ExternalStorage;
+package RT::ExternalStorage;
 
 our $VERSION = '0.61';
 
 use Digest::SHA qw//;
 
-require RT::Extension::ExternalStorage::Backend;
+require RT::ExternalStorage::Backend;
 
 =head1 NAME
 
-RT::Extension::ExternalStorage - Store attachments outside the database
+RT::ExternalStorage - Store attachments outside the database
 
 =head1 SYNOPSIS
 
-    Set( @Plugins, 'RT::Extension::ExternalStorage' );
+    Set( @Plugins, 'RT::ExternalStorage' );
 
     Set(%ExternalStorage,
         Type => 'Disk',
@@ -82,12 +82,12 @@ database, at the cost of adding additional locations which must be
 configured or backed up.  Attachment storage paths are calculated based
 on file contents; this provides de-duplication.
 
-The files are initially stored in the database when RT receives them;
-this guarantees that the user does not need to wait for the file to be
-transferred to disk or to the cloud, and makes it durable to transient
-failures of cloud connectivity.  The provided C<bin/extract-attachments>
-script, to be run regularly via cron, takes care of moving attachments
-out of the database at a later time.
+The files are initially stored in the database when RT receives
+them; this guarantees that the user does not need to wait for
+the file to be transferred to disk or to the cloud, and makes it
+durable to transient failures of cloud connectivity.  The provided
+C<sbin/rt-externalize-attachments> script, to be run regularly via cron,
+takes care of moving attachments out of the database at a later time.
 
 =head1 INSTALLATION
 
@@ -105,13 +105,13 @@ May need root permissions
 
 If you are using RT 4.2 or greater, add this line:
 
-    Plugin('RT::Extension::ExternalStorage');
+    Plugin('RT::ExternalStorage');
 
 For RT 4.0, add this line:
 
-    Set(@Plugins, qw(RT::Extension::ExternalStorage));
+    Set(@Plugins, qw(RT::ExternalStorage));
 
-or add C<RT::Extension::ExternalStorage> to your existing C<@Plugins> line.
+or add C<RT::ExternalStorage> to your existing C<@Plugins> line.
 
 You will also need to configure the C<%ExternalStorage> option,
 depending on how and where you want your data stored; see
@@ -125,18 +125,18 @@ are extracted.
 
 =item Extract existing attachments
 
-Run C<bin/extract-attachments>; this may take some time, depending on
-the existing size of the database.  This task may be safely cancelled
+Run C<sbin/rt-externalize-attachments>; this may take some time, depending
+on the existing size of the database.  This task may be safely cancelled
 and re-run to resume.
 
 =item Schedule attachments extraction
 
-Schedule C<bin/extract-attachments> to run at regular intervals via
+Schedule C<sbin/rt-externalize-attachments> to run at regular intervals via
 cron.  For instance, the following F</etc/cron.d/rt> entry will run it
 daily, which may be good to concentrate network or disk usage to times
 when RT is less in use:
 
-    0 0 * * * root /opt/rt4/local/plugins/RT-Extension-ExternalStorage/bin/extract-attachments
+    0 0 * * * root /opt/rt4/sbin/rt-externalize-attachments
 
 =back
 
@@ -147,11 +147,11 @@ documentation in each for necessary configuration details:
 
 =over
 
-=item L<RT::Extension::ExternalStorage::Disk>
+=item L<RT::ExternalStorage::Disk>
 
-=item L<RT::Extension::ExternalStorage::Dropbox>
+=item L<RT::ExternalStorage::Dropbox>
 
-=item L<RT::Extension::ExternalStorage::AmazonS3>
+=item L<RT::ExternalStorage::AmazonS3>
 
 =back
 
@@ -172,7 +172,7 @@ $RT::Config::META{ExternalStorage} = {
         my %hash = $self->Get('ExternalStorage');
         return unless keys %hash;
         $hash{Write} = $WRITE;
-        $BACKEND = RT::Extension::ExternalStorage::Backend->new( %hash );
+        $BACKEND = RT::ExternalStorage::Backend->new( %hash );
     },
 };
 
diff --git a/lib/RT/Extension/ExternalStorage/AmazonS3.pm b/lib/RT/ExternalStorage/AmazonS3.pm
similarity index 96%
rename from lib/RT/Extension/ExternalStorage/AmazonS3.pm
rename to lib/RT/ExternalStorage/AmazonS3.pm
index 773ab4d..c36c1d0 100644
--- a/lib/RT/Extension/ExternalStorage/AmazonS3.pm
+++ b/lib/RT/ExternalStorage/AmazonS3.pm
@@ -50,10 +50,10 @@ use 5.008003;
 use warnings;
 use strict;
 
-package RT::Extension::ExternalStorage::AmazonS3;
+package RT::ExternalStorage::AmazonS3;
 
 use Role::Basic qw/with/;
-with 'RT::Extension::ExternalStorage::Backend';
+with 'RT::ExternalStorage::Backend';
 
 our( $S3, $BUCKET);
 sub Init {
@@ -126,7 +126,7 @@ sub Store {
 
 =head1 NAME
 
-RT::Extension::ExternalStorage::Dropbox - Store files in the Dropbox cloud
+RT::ExternalStorage::Dropbox - Store files in the Dropbox cloud
 
 =head1 SYNOPSIS
 
diff --git a/lib/RT/Extension/ExternalStorage/Backend.pm b/lib/RT/ExternalStorage/Backend.pm
similarity index 91%
rename from lib/RT/Extension/ExternalStorage/Backend.pm
rename to lib/RT/ExternalStorage/Backend.pm
index 03b0898..976f3b8 100644
--- a/lib/RT/Extension/ExternalStorage/Backend.pm
+++ b/lib/RT/ExternalStorage/Backend.pm
@@ -50,7 +50,7 @@ use 5.008003;
 use warnings;
 use strict;
 
-package RT::Extension::ExternalStorage::Backend;
+package RT::ExternalStorage::Backend;
 
 use Role::Basic;
 
@@ -68,7 +68,7 @@ sub new {
         return undef;
     } elsif ($class->require) {
     } else {
-        my $long = "RT::Extension::ExternalStorage::$class";
+        my $long = "RT::ExternalStorage::$class";
         if ($long->require) {
             $class = $long;
         } else {
@@ -77,8 +77,8 @@ sub new {
         }
     }
 
-    unless ($class->DOES("RT::Extension::ExternalStorage::Backend")) {
-        RT->Logger->error("External storage engine $class doesn't implement RT::Extension::ExternalStorage::Backend");
+    unless ($class->DOES("RT::ExternalStorage::Backend")) {
+        RT->Logger->error("External storage engine $class doesn't implement RT::ExternalStorage::Backend");
         return undef;
     }
 
diff --git a/lib/RT/Extension/ExternalStorage/Disk.pm b/lib/RT/ExternalStorage/Disk.pm
similarity index 94%
rename from lib/RT/Extension/ExternalStorage/Disk.pm
rename to lib/RT/ExternalStorage/Disk.pm
index e93fb84..4713a0b 100644
--- a/lib/RT/Extension/ExternalStorage/Disk.pm
+++ b/lib/RT/ExternalStorage/Disk.pm
@@ -50,12 +50,12 @@ use 5.008003;
 use warnings;
 use strict;
 
-package RT::Extension::ExternalStorage::Disk;
+package RT::ExternalStorage::Disk;
 
 use File::Path qw//;
 
 use Role::Basic qw/with/;
-with 'RT::Extension::ExternalStorage::Backend';
+with 'RT::ExternalStorage::Backend';
 
 sub Init {
     my $self = shift;
@@ -114,7 +114,7 @@ sub Store {
 
 =head1 NAME
 
-RT::Extension::ExternalStorage::Disk - On-disk storage of attachments
+RT::ExternalStorage::Disk - On-disk storage of attachments
 
 =head1 SYNOPSIS
 
@@ -131,7 +131,7 @@ if the same file appears in multiple transactions, only one copy will be
 stored on disk.
 
 The C<Path> must be readable by the webserver, and writable by the
-C<bin/extract-attachments> script.  Because the majority of the
+C<sbin/rt-externalize-attachments> script.  Because the majority of the
 attachments are in the filesystem, a simple database backup is thus
 incomplete.  It is B<extremely important> that I<backups include the
 on-disk attachments directory>.
diff --git a/lib/RT/Extension/ExternalStorage/Dropbox.pm b/lib/RT/ExternalStorage/Dropbox.pm
similarity index 96%
rename from lib/RT/Extension/ExternalStorage/Dropbox.pm
rename to lib/RT/ExternalStorage/Dropbox.pm
index d57bf6b..cf6b4ac 100644
--- a/lib/RT/Extension/ExternalStorage/Dropbox.pm
+++ b/lib/RT/ExternalStorage/Dropbox.pm
@@ -50,10 +50,10 @@ use 5.008003;
 use warnings;
 use strict;
 
-package RT::Extension::ExternalStorage::Dropbox;
+package RT::ExternalStorage::Dropbox;
 
 use Role::Basic qw/with/;
-with 'RT::Extension::ExternalStorage::Backend';
+with 'RT::ExternalStorage::Backend';
 
 our $DROPBOX;
 sub Init {
@@ -111,7 +111,7 @@ sub Store {
 
 =head1 NAME
 
-RT::Extension::ExternalStorage::Dropbox - Store files in the Dropbox cloud
+RT::ExternalStorage::Dropbox - Store files in the Dropbox cloud
 
 =head1 SYNOPSIS
 
diff --git a/lib/RT/Extension/ExternalStorage/Test.pm.in b/lib/RT/Test/ExternalStorage.pm
similarity index 77%
rename from lib/RT/Extension/ExternalStorage/Test.pm.in
rename to lib/RT/Test/ExternalStorage.pm
index 9032217..dee8440 100644
--- a/lib/RT/Extension/ExternalStorage/Test.pm.in
+++ b/lib/RT/Test/ExternalStorage.pm
@@ -46,41 +46,13 @@
 #
 # END BPS TAGGED BLOCK }}}
 
+package RT::Test::ExternalStorage;
 use strict;
 use warnings;
-
-### after: use lib qw(@RT_LIB_PATH@);
-use lib qw(/opt/rt4/local/lib /opt/rt4/lib);
-
-package RT::Extension::ExternalStorage::Test;
-
-=head2 RT::Extension::ExternalStorage::Test
-
-Initialization for testing.
-
-=cut
-
 use base qw(RT::Test);
 use File::Spec;
 use File::Path 'mkpath';
 
-sub import {
-    my $class = shift;
-    my %args  = @_;
-
-    $args{'requires'} ||= [];
-    if ( $args{'testing'} ) {
-        unshift @{ $args{'requires'} }, 'RT::Extension::ExternalStorage';
-    } else {
-        $args{'testing'} = 'RT::Extension::ExternalStorage';
-    }
-
-    $class->SUPER::import( %args );
-    $class->export_to_level(1);
-
-    require RT::Extension::ExternalStorage;
-}
-
 sub attachments_dir {
     my $dir = File::Spec->catdir( RT::Test->temp_directory, qw(attachments) );
     mkpath($dir);
@@ -89,10 +61,13 @@ sub attachments_dir {
 
 sub bootstrap_more_config {
     my $self = shift;
-    my ($config) = @_;
+    my $handle = shift;
+    my $args = shift;
+
+    $self->SUPER::bootstrap_more_config($handle, $args, @_);
 
     my $dir = $self->attachments_dir;
-    print $config qq|Set( %ExternalStorage, Type => 'Disk', Path => '$dir' );\n|;
+    print $handle qq|Set( %ExternalStorage, Type => 'Disk', Path => '$dir' );\n|;
 }
 
 1;
diff --git a/sbin/extract-attachments.in b/sbin/rt-externalize-attachments.in
similarity index 89%
rename from sbin/extract-attachments.in
rename to sbin/rt-externalize-attachments.in
index ce5dedb..8a18878 100755
--- a/sbin/extract-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -1,6 +1,4 @@
-#!/usr/bin/env perl
-### before: #!@PERL@
-
+#!@PERL@
 # BEGIN BPS TAGGED BLOCK {{{
 #
 # COPYRIGHT:
@@ -51,10 +49,24 @@
 use strict;
 use warnings;
 
-### after: use lib qw(@RT_LIB_PATH@);
-use lib qw(/opt/rt4/local/lib /opt/rt4/lib);
+# fix lib paths, some may be relative
+BEGIN { # BEGIN RT CMD BOILERPLATE
+    require File::Spec;
+    require Cwd;
+    my @libs = ("@RT_LIB_PATH@", "@LOCAL_LIB_PATH@");
+    my $bin_path;
+
+    for my $lib (@libs) {
+        unless ( File::Spec->file_name_is_absolute($lib) ) {
+            $bin_path ||= ( File::Spec->splitpath(Cwd::abs_path(__FILE__)) )[1];
+            $lib = File::Spec->catfile( $bin_path, File::Spec->updir, $lib );
+        }
+        unshift @INC, $lib;
+    }
+
+}
 
-BEGIN { $RT::Extension::ExternalStorage::WRITE = 1 };
+BEGIN { $RT::ExternalStorage::WRITE = 1 };
 use RT -init;
 
 # Ensure we only run one of these processes at once
@@ -64,7 +76,7 @@ exit unless flock main::DATA, LOCK_EX | LOCK_NB;
 die "\%ExternalStorage is not configured\n"
     unless RT->Config->Get("ExternalStorage");
 
-exit unless $RT::Extension::ExternalStorage::BACKEND;
+exit unless $RT::ExternalStorage::BACKEND;
 
 my $last = RT->System->FirstAttribute("ExternalStorage");
 $last = $last ? $last->Content : {};
@@ -133,7 +145,7 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             );
 
             # Attempt to write that out
-            my ($key, $msg) = RT::Extension::ExternalStorage->Store( $content );
+            my ($key, $msg) = RT::ExternalStorage->Store( $content );
             unless ($key) {
                 RT->Logger->error("Failed to store $class $id: $msg");
                 exit 1;
diff --git a/t/externalstorage/basic.t b/t/externalstorage/basic.t
index 281d2e7..497f68c 100644
--- a/t/externalstorage/basic.t
+++ b/t/externalstorage/basic.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Extension::ExternalStorage::Test tests => undef;
+use RT::Test::ExternalStorage tests => undef;
 
 my $queue = RT::Test->load_or_create_queue(Name => 'General');
 ok $queue && $queue->id;
@@ -51,13 +51,13 @@ is $attachs[3]->Content, 'thing',  "Can get the binary content";
 is $attachs[3]->ContentEncoding, "none", "Content is not encoded";
 ok $attachs[3]->StoreExternally, "Will store binary data on disk";
 
-my $dir = RT::Extension::ExternalStorage::Test->attachments_dir;
+my $dir = RT::Test::ExternalStorage->attachments_dir;
 ok !<$dir/*>, "Attachments directory is empty";
 
 
-ok -e 'sbin/extract-attachments', "Found extract-attachments script";
-ok -x 'sbin/extract-attachments', "extract-attachments is executable";
-ok !system('sbin/extract-attachments'), "extract-attachments ran successfully";
+ok -e 'sbin/rt-externalize-attachments', "Found rt-externalize-attachments script";
+ok -x 'sbin/rt-externalize-attachments', "rt-externalize-attachments is executable";
+ok !system('sbin/rt-externalize-attachments'), "rt-externalize-attachments ran successfully";
 
 @attachs = @{ $ticket->Transactions->First->Attachments->ItemsArrayRef };
 is $attachs[1]->Content, 'test', "Can still get the text part content";

commit 713c3f2872612f9d8b2b9922020cbb31c8b1bd0c
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu May 21 21:29:00 2015 +0000

    Update ExternalStorage documentation
    
        Dropbox has deprecated the filestore API, so that step is omitted.

diff --git a/README b/README
index bdd8e09..ba3221e 100644
--- a/README
+++ b/README
@@ -190,14 +190,18 @@ GENERAL INSTALLATION
  9) Set up automated recurring tasks (cronjobs):
 
     To generate email digest messages, you must arrange for the provided
-    utility to be run once daily, and once weekly. You may also want to
-    arrange for the rt-email-dashboards utility to be run hourly.  For
-    example, if your task scheduler is cron, you can configure it by
+    utility to be run once daily, and once weekly. You may also want
+    to arrange for the rt-email-dashboards utility to be run hourly. If
+    you choose to use the external storage feature, that must process
+    new attachments on a regular basis.
+
+    If your task scheduler is cron, you can configure it by
     adding the following lines as /etc/cron.d/rt:
 
         0 0 * * * root /opt/rt4/sbin/rt-email-digest -m daily
         0 0 * * 0 root /opt/rt4/sbin/rt-email-digest -m weekly
         0 * * * * root /opt/rt4/sbin/rt-email-dashboards
+        0 0 * * * root /opt/rt4/sbin/rt-externalize-attachments
 
 10) Configure the RT email gateway.  To let email flow to your RT
     server, you need to add a few lines of configuration to your mail
diff --git a/docs/backups.pod b/docs/backups.pod
index 554336f..768e7d5 100644
--- a/docs/backups.pod
+++ b/docs/backups.pod
@@ -188,6 +188,19 @@ the default cronjobs in place, it's one less piece to forget during a restore.
 If you have custom L<< C<rt-crontool> >> invocations, you don't want to have to
 recreate those.
 
+=item External storage
+
+If you use L<RT::ExternalStorage>, you will want to backup
+the attachments in your chosen storage engine.
+
+If you're using L<RT::ExternalStorage::Disk>, then you need only back
+up the files under the C<Path> option under C<%ExternalStorage> in your
+RT_SiteConfig.pm.
+
+If you're using a cloud storage engine like
+L<RT::ExternalStorage::AmazonS3>, consult that service's documentation
+regarding backups.
+
 =back
 
 Simply saving a tarball should be sufficient, with something like:
diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 7ece63f..c341ccd 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -2512,7 +2512,52 @@ Set(%GnuPGOptions,
 
 =back
 
+=head1 External storage
 
+By default, RT stores attachments in the database.  ExternalStorage moves
+all attachments that RT does not need efficient access to (which include
+textual content and images) to outside of the database.  This may either
+be on local disk, or to a cloud storage solution.  This decreases the
+size of RT's database, in turn decreasing the burden of backing up RT's
+database, at the cost of adding additional locations which must be
+configured or backed up.  Attachment storage paths are calculated based
+on file contents; this provides de-duplication.
+
+A full description of external storage can be found by running the command
+`perldoc L<RT::ExternalStorage>` (or `perldoc lib/RT/ExternalStorage.pm`
+from your RT install directory).
+
+Note that simply configuring L<RT::ExternalStorage> is insufficient; there
+are additional steps required (including setup of a regularly-scheduled
+upload job) to enable RT to make use of external storage.
+
+=over 4
+
+=item C<%ExternalStorage>
+
+This selects which storage engine is used, as well as options for
+configuring that specific storage engine. RT ships with the following
+storage engines:
+
+L<RT::ExternalStorage::Disk>, which stores files on directly onto disk.
+
+L<RT::ExternalStorage::AmazonS3>, which stores files on Amazon's S3 service.
+
+L<RT::ExternalStorage::Dropbox>, which stores files in Dropbox.
+
+See each storage engine's documentation for the configuration it requires
+and accepts.
+
+    Set(%ExternalStorage,
+        Type => 'Disk',
+        Path => '/opt/rt4/var/attachments',
+    );
+
+=cut
+
+Set(%ExternalStorage, ());
+
+=back
 
 =head1 Lifecycles
 
diff --git a/lib/RT/ExternalStorage.pm b/lib/RT/ExternalStorage.pm
index 66bd2fb..ddf7229 100644
--- a/lib/RT/ExternalStorage.pm
+++ b/lib/RT/ExternalStorage.pm
@@ -64,8 +64,6 @@ RT::ExternalStorage - Store attachments outside the database
 
 =head1 SYNOPSIS
 
-    Set( @Plugins, 'RT::ExternalStorage' );
-
     Set(%ExternalStorage,
         Type => 'Disk',
         Path => '/opt/rt4/var/attachments',
@@ -73,7 +71,7 @@ RT::ExternalStorage - Store attachments outside the database
 
 =head1 DESCRIPTION
 
-By default, RT stores attachments in the database.  This extension moves
+By default, RT stores attachments in the database.  ExternalStorage moves
 all attachments that RT does not need efficient access to (which include
 textual content and images) to outside of the database.  This may either
 be on local disk, or to a cloud storage solution.  This decreases the
@@ -89,47 +87,39 @@ durable to transient failures of cloud connectivity.  The provided
 C<sbin/rt-externalize-attachments> script, to be run regularly via cron,
 takes care of moving attachments out of the database at a later time.
 
-=head1 INSTALLATION
-
-=over
-
-=item C<perl Makefile.PL>
-
-=item C<make>
+=head1 SETUP
 
-=item C<make install>
+=head2 Edit F</opt/rt4/etc/RT_SiteConfig.pm>
 
-May need root permissions
+You will need to configure the C<%ExternalStorage> option,
+depending on how and where you want your data stored.
 
-=item Edit your F</opt/rt4/etc/RT_SiteConfig.pm>
-
-If you are using RT 4.2 or greater, add this line:
+RT comes with a number of possible storage backends; see the
+documentation in each for necessary configuration details:
 
-    Plugin('RT::ExternalStorage');
+=over
 
-For RT 4.0, add this line:
+=item L<RT::ExternalStorage::Disk>
 
-    Set(@Plugins, qw(RT::ExternalStorage));
+=item L<RT::ExternalStorage::Dropbox>
 
-or add C<RT::ExternalStorage> to your existing C<@Plugins> line.
+=item L<RT::ExternalStorage::AmazonS3>
 
-You will also need to configure the C<%ExternalStorage> option,
-depending on how and where you want your data stored; see
-L</CONFIGURATION>.
+=back
 
-=item Restart your webserver
+=head2 Restart your webserver
 
 Restarting the webserver before the next step (extracting existing
 attachments) is important to ensure that files remain available as they
 are extracted.
 
-=item Extract existing attachments
+=head2 Extract existing attachments
 
 Run C<sbin/rt-externalize-attachments>; this may take some time, depending
 on the existing size of the database.  This task may be safely cancelled
 and re-run to resume.
 
-=item Schedule attachments extraction
+=head2 Schedule attachments extraction
 
 Schedule C<sbin/rt-externalize-attachments> to run at regular intervals via
 cron.  For instance, the following F</etc/cron.d/rt> entry will run it
@@ -138,26 +128,9 @@ when RT is less in use:
 
     0 0 * * * root /opt/rt4/sbin/rt-externalize-attachments
 
-=back
-
-=head1 CONFIGURATION
-
-This module comes with a number of possible backends; see the
-documentation in each for necessary configuration details:
-
-=over
-
-=item L<RT::ExternalStorage::Disk>
-
-=item L<RT::ExternalStorage::Dropbox>
-
-=item L<RT::ExternalStorage::AmazonS3>
-
-=back
-
 =head1 CAVEATS
 
-This extension is not currently compatibile with RT's C<shredder> tool;
+This feature is not currently compatible with RT's C<shredder> tool;
 attachments which are shredded will not be removed from external
 storage.
 
@@ -256,28 +229,4 @@ sub StoreExternally {
     }
 }
 
-=head1 AUTHOR
-
-Best Practical Solutions, LLC E<lt>modules at bestpractical.comE<gt>
-
-=head1 BUGS
-
-All bugs should be reported via email to
-
-    L<bug-RT-Extension-ExternalStorage at rt.cpan.org|mailto:bug-RT-Extension-ExternalStorage at rt.cpan.org>
-
-or via the web at
-
-    L<rt.cpan.org|http://rt.cpan.org/Public/Dist/Display.html?Name=RT-Extension-ExternalStorage>.
-
-=head1 COPYRIGHT
-
-This extension is Copyright (C) 2009-2015 Best Practical Solutions, LLC.
-
-This is free software, licensed under:
-
-  The GNU General Public License, Version 2, June 1991
-
-=cut
-
 1;
diff --git a/lib/RT/ExternalStorage/AmazonS3.pm b/lib/RT/ExternalStorage/AmazonS3.pm
index c36c1d0..f30ec46 100644
--- a/lib/RT/ExternalStorage/AmazonS3.pm
+++ b/lib/RT/ExternalStorage/AmazonS3.pm
@@ -126,68 +126,76 @@ sub Store {
 
 =head1 NAME
 
-RT::ExternalStorage::Dropbox - Store files in the Dropbox cloud
+RT::ExternalStorage::AmazonS3 - Store files in Amazon's S3 cloud
 
 =head1 SYNOPSIS
 
     Set(%ExternalStorage,
-        Type => 'Dropbox',
-        AccessToken => '...',
+        Type            => 'AmazonS3',
+        AccessKeyId     => '...',
+        SecretAccessKey => '...',
+        Bucket          => '...',
     );
 
 =head1 DESCRIPTION
 
-This storage option places attachments in the Dropbox shared file
+This storage option places attachments in the S3 cloud file storage
 service.  The files are de-duplicated when they are saved; as such, if
 the same file appears in multiple transactions, only one copy will be
-stored on in Dropbox.
+stored in S3.
 
-Files in Dropbox C<must not be modified or removed>; doing so may cause
-internal inconsistency.
+Files in S3 B<must not be modified or removed>; doing so may cause
+internal inconsistency.  It is also important to ensure that the S3
+account used maintains sufficient funds for your RT's B<storage and
+bandwidth> needs.
 
 =head1 SETUP
 
-In order to use this stoage type, a new application must be registered
-with Dropbox:
+In order to use this storage type, you must grant RT access to your
+S3 account.
 
 =over
 
 =item 1.
 
-Log into Dropbox as the user you wish to store files as.
+Log into Amazon S3, L<https://aws.amazon.com/s3/>, as the account you wish
+to store files under.
 
 =item 2.
 
-Click C<Create app> on L<https://www.dropbox.com/developers/apps>
+Navigate to "Security Credentials" under your account name in the menu bar.
 
 =item 3.
 
-Choose B<Dropbox API app> as the type of app.
+Open the "Access Keys" pane.
 
 =item 4.
 
-Choose the B<Files and datastores> as the type of data to store.
+Click "Create New Access Key".
 
 =item 5.
 
-Choose B<Yes>, your application only needs access to files it creates.
+Copy the provided values for Access Key ID and Secret Access Key into
+ your F<RT_SiteConfig.pm> file:
 
-=item 6.
-
-Enter a descriptive name -- C<Request Tracker files> is fine.
-
-=item 7.
-
-Under C<Generated access token>, click the C<Generate> button.
+    Set(%ExternalStorage,
+        Type            => 'AmazonS3',
+        AccessKeyId     => '...', # Put Access Key ID between quotes
+        SecretAccessKey => '...', # Put Secret Access Key between quotes
+        Bucket          => '...',
+    );
 
-=item 8.
+=item 6.
 
-Copy the provided value into your F<RT_SiteConfig.pm> file as the
-C<AccessToken>:
+Set up a Bucket for RT to use. You can either create and configure it
+in the S3 web interface, or let RT create one itself. Either way, tell
+RT what bucket name to use in your F<RT_SiteConfig.pm> file:
 
     Set(%ExternalStorage,
-        Type => 'Dropbox',
-        AccessToken => '...',   # Replace the value here, between the quotes
+        Type            => 'AmazonS3',
+        AccessKeyId     => '...',
+        SecretAccessKey => '...',
+        Bucket          => '...', # Put bucket name between quotes
     );
 
 =back
diff --git a/lib/RT/ExternalStorage/Disk.pm b/lib/RT/ExternalStorage/Disk.pm
index 4713a0b..4971989 100644
--- a/lib/RT/ExternalStorage/Disk.pm
+++ b/lib/RT/ExternalStorage/Disk.pm
@@ -130,7 +130,7 @@ uncompressed.  The files are de-duplicated when they are saved; as such,
 if the same file appears in multiple transactions, only one copy will be
 stored on disk.
 
-The C<Path> must be readable by the webserver, and writable by the
+The C<Path> must be readable by the web server, and writable by the
 C<sbin/rt-externalize-attachments> script.  Because the majority of the
 attachments are in the filesystem, a simple database backup is thus
 incomplete.  It is B<extremely important> that I<backups include the
diff --git a/lib/RT/ExternalStorage/Dropbox.pm b/lib/RT/ExternalStorage/Dropbox.pm
index cf6b4ac..beef360 100644
--- a/lib/RT/ExternalStorage/Dropbox.pm
+++ b/lib/RT/ExternalStorage/Dropbox.pm
@@ -125,16 +125,16 @@ RT::ExternalStorage::Dropbox - Store files in the Dropbox cloud
 This storage option places attachments in the Dropbox shared file
 service.  The files are de-duplicated when they are saved; as such, if
 the same file appears in multiple transactions, only one copy will be
-stored on in Dropbox.
+stored in Dropbox.
 
-Files in Dropbox C<must not be modified or removed>; doing so may cause
+Files in Dropbox B<must not be modified or removed>; doing so may cause
 internal inconsistency.  It is also important to ensure that the Dropbox
 account used has sufficient space for the attachments, and to monitor
 its space usage.
 
 =head1 SETUP
 
-In order to use this stoage type, a new application must be registered
+In order to use this storage type, a new application must be registered
 with Dropbox:
 
 =over
@@ -153,21 +153,17 @@ Choose B<Dropbox API app> as the type of app.
 
 =item 4.
 
-Choose the B<Files and datastores> as the type of data to store.
-
-=item 5.
-
 Choose B<Yes>, your application only needs access to files it creates.
 
-=item 6.
+=item 5.
 
 Enter a descriptive name -- C<Request Tracker files> is fine.
 
-=item 7.
+=item 6.
 
 Under C<Generated access token>, click the C<Generate> button.
 
-=item 8.
+=item 7.
 
 Copy the provided value into your F<RT_SiteConfig.pm> file as the
 C<AccessToken>:

commit 686daae6af4c209a913f1c763c2753f6690dbc3a
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu May 21 21:24:31 2015 +0000

    Cleanup pass of ExternalStorage code
    
        Highlights:
    
        - Move monkeypatches directly into their respective classes
    
        - Use accessors instead of direct hash access
    
        - Import overlays
    
        - Replace globals with accessors, method calls, etc.
            In particular, the external storage backend object now hangs
            off of RT->System.
    
        - Die with a message if backend is missing
            Due to %ExternalStorage now being part of core config, we must
            check that external storage was actually loaded correctly.
    
        - Move Store routine from RT::ExternalStorage to script
            This eliminates all API from RT::ExternalStorage
            itself; instead, developers should be talking to
            RT->System->ExternalStorage. Furthermore, there are all sorts of
            consistency (like updating ContentEncoding to external) and safety
            checks (like $WRITE=1 performing a -w check) that the script does
            that RT::ExternalStorage->Store doesn't.
    
        - Handle a single bucket in a region
            ->{buckets} only seems to be populated when there are multiple
            sibling buckets in a region.
    
            Without this fallback, we try to create a bucket that already
            exists (but we can't tell it exists because ->{buckets} is empty).
    
        - Limit the transaction in rt-externalize-attachments to each upload
            This way if upload #99 fails, we won't bother trying to re-upload
            1-98 since they are marked as external.

diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 154d161..9570447 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -1178,6 +1178,29 @@ sub __DependsOn {
     return $self->SUPER::__DependsOn( %args );
 }
 
+sub ShouldStoreExternally {
+    my $self = shift;
+    my $type = $self->ContentType;
+    my $length = $self->ContentLength;
+
+    return 0 if $length == 0;
+
+    if ($type =~ m{^multipart/}) {
+        return 0;
+    } elsif ($type =~ m{^(text|message)/}) {
+        # If textual, we only store externally if it's _large_ (> 10M)
+        return 1 if $length > 10 * 1024 * 1024;
+        return 0;
+    } elsif ($type =~ m{^image/}) {
+        # Ditto images, which may be displayed inline
+        return 1 if $length > 10 * 1024 * 1024;
+        return 0;
+    } else {
+        return 1;
+    }
+}
+
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 3a0c16b..47ef2da 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -985,6 +985,19 @@ our %META;
             $config->Set( CustomFieldGroupings => %$groups );
         },
     },
+    ExternalStorage => {
+        Type            => 'HASH',
+        PostLoadCheck   => sub {
+            my $self = shift;
+            my %hash = $self->Get('ExternalStorage');
+            return unless keys %hash;
+
+            require RT::ExternalStorage;
+
+            my $backend = RT::ExternalStorage::Backend->new(%hash);
+            RT->System->ExternalStorage($backend);
+        },
+    },
     ChartColors => {
         Type    => 'ARRAY',
     },
diff --git a/lib/RT/ExternalStorage.pm b/lib/RT/ExternalStorage.pm
index ddf7229..d0a7012 100644
--- a/lib/RT/ExternalStorage.pm
+++ b/lib/RT/ExternalStorage.pm
@@ -46,16 +46,11 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-use 5.008003;
 use warnings;
 use strict;
 
 package RT::ExternalStorage;
 
-our $VERSION = '0.61';
-
-use Digest::SHA qw//;
-
 require RT::ExternalStorage::Backend;
 
 =head1 NAME
@@ -136,97 +131,6 @@ storage.
 
 =cut
 
-our $BACKEND;
-our $WRITE;
-$RT::Config::META{ExternalStorage} = {
-    Type => 'HASH',
-    PostLoadCheck => sub {
-        my $self = shift;
-        my %hash = $self->Get('ExternalStorage');
-        return unless keys %hash;
-        $hash{Write} = $WRITE;
-        $BACKEND = RT::ExternalStorage::Backend->new( %hash );
-    },
-};
-
-sub Store {
-    my $class = shift;
-    my $content = shift;
-
-    my $key = Digest::SHA::sha256_hex( $content );
-    my ($ok, $msg) = $BACKEND->Store( $key => $content );
-    return ($ok, $msg) unless defined $ok;
-
-    return ($key);
-}
-
-
-package RT::Record;
-
-no warnings 'redefine';
-my $__DecodeLOB = __PACKAGE__->can('_DecodeLOB');
-*_DecodeLOB = sub {
-    my $self            = shift;
-    my $ContentType     = shift || '';
-    my $ContentEncoding = shift || 'none';
-    my $Content         = shift;
-    my $Filename        = @_;
-
-    return $__DecodeLOB->($self, $ContentType, $ContentEncoding, $Content, $Filename)
-        unless $ContentEncoding eq "external";
-
-    unless ($BACKEND) {
-        RT->Logger->error( "Failed to load $Content; external storage not configured" );
-        return ("");
-    };
-
-    my ($ok, $msg) = $BACKEND->Get( $Content );
-    unless (defined $ok) {
-        RT->Logger->error( "Failed to load $Content from external storage: $msg" );
-        return ("");
-    }
-
-    return $__DecodeLOB->($self, $ContentType, 'none', $ok, $Filename);
-};
-
-package RT::ObjectCustomFieldValue;
-
-sub StoreExternally {
-    my $self = shift;
-    my $type = $self->CustomFieldObj->Type;
-    my $length = length($self->LargeContent || '');
-
-    return 0 if $length == 0;
-
-    return 1 if $type eq "Binary";
-
-    return 1 if $type eq "Image" and $length > 10 * 1024 * 1024;
-
-    return 0;
-}
-
-package RT::Attachment;
-
-sub StoreExternally {
-    my $self = shift;
-    my $type = $self->ContentType;
-    my $length = $self->ContentLength;
-
-    return 0 if $length == 0;
-
-    if ($type =~ m{^multipart/}) {
-        return 0;
-    } elsif ($type =~ m{^(text|message)/}) {
-        # If textual, we only store externally if it's _large_ (> 10M)
-        return 1 if $length > 10 * 1024 * 1024;
-        return 0;
-    } elsif ($type =~ m{^image/}) {
-        # Ditto images, which may be displayed inline
-        return 1 if $length > 10 * 1024 * 1024;
-        return 0;
-    } else {
-        return 1;
-    }
-}
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/ExternalStorage/AmazonS3.pm b/lib/RT/ExternalStorage/AmazonS3.pm
index f30ec46..c563938 100644
--- a/lib/RT/ExternalStorage/AmazonS3.pm
+++ b/lib/RT/ExternalStorage/AmazonS3.pm
@@ -46,7 +46,6 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-use 5.008003;
 use warnings;
 use strict;
 
@@ -55,44 +54,74 @@ package RT::ExternalStorage::AmazonS3;
 use Role::Basic qw/with/;
 with 'RT::ExternalStorage::Backend';
 
-our( $S3, $BUCKET);
+sub S3 {
+    my $self = shift;
+    if (@_) {
+        $self->{S3} = shift;
+    }
+    return $self->{S3};
+}
+
+sub Bucket {
+    my $self = shift;
+    return $self->{Bucket};
+}
+
+sub AccessKeyId {
+    my $self = shift;
+    return $self->{AccessKeyId};
+}
+
+sub SecretAccessKey {
+    my $self = shift;
+    return $self->{SecretAccessKey};
+}
+
+sub BucketObj {
+    my $self = shift;
+    return $self->S3->bucket($self->Bucket);
+}
+
 sub Init {
     my $self = shift;
-    my %self = %{$self};
 
     if (not Amazon::S3->require) {
         RT->Logger->error("Required module Amazon::S3 is not installed");
         return;
-    } elsif (not $self{AccessKeyId}) {
-        RT->Logger->error("AccessKeyId not provided for AmazonS3");
-        return;
-    } elsif (not $self{SecretAccessKey}) {
-        RT->Logger->error("SecretAccessKey not provided for AmazonS3");
-        return;
-    } elsif (not $self{Bucket}) {
-        RT->Logger->error("Bucket not provided for AmazonS3");
-        return;
     }
 
+    for my $key (qw/AccessKeyId SecretAccessKey Bucket/) {
+        if (not $self->$key) {
+            RT->Logger->error("Required option '$key' not provided for AmazonS3 external storage. See the documentation for " . __PACKAGE__ . " for setting up this integration.");
+            return;
+        }
+    }
 
-    $S3 = Amazon::S3->new( {
-        aws_access_key_id     => $self{AccessKeyId},
-        aws_secret_access_key => $self{SecretAccessKey},
+    my $S3 = Amazon::S3->new( {
+        aws_access_key_id     => $self->AccessKeyId,
+        aws_secret_access_key => $self->SecretAccessKey,
         retry                 => 1,
     } );
+    $self->S3($S3);
 
-    my $buckets = $S3->bucket( $self{Bucket} );
+    my $buckets = $S3->bucket( $self->Bucket );
     unless ( $buckets ) {
         RT->Logger->error("Can't list buckets of AmazonS3: ".$S3->errstr);
         return;
     }
-    unless ( grep {$_->bucket eq $self{Bucket}} @{$buckets->{buckets}} ) {
+
+    my @buckets = $buckets->{buckets} ? @{$buckets->{buckets}} : ($buckets);
+
+    unless ( grep {$_->bucket eq $self->Bucket} @buckets ) {
         my $ok = $S3->add_bucket( {
-            bucket    => $self{Bucket},
+            bucket    => $self->Bucket,
             acl_short => 'private',
         } );
-        unless ($ok) {
-            RT->Logger->error("Can't create new bucket '$self{Bucket}' on AmazonS3: ".$S3->errstr);
+        if ($ok) {
+            RT->Logger->debug("Created new bucket '".$self->Bucket."' on AmazonS3");
+        }
+        else {
+            RT->Logger->error("Can't create new bucket '".$self->Bucket."' on AmazonS3: ".$S3->errstr);
             return;
         }
     }
@@ -104,8 +133,8 @@ sub Get {
     my $self = shift;
     my ($sha) = @_;
 
-    my $ok = $S3->bucket($self->{Bucket})->get_key( $sha );
-    return (undef, "Could not retrieve from AmazonS3:" . $S3->errstr)
+    my $ok = $self->BucketObj->get_key( $sha );
+    return (undef, "Could not retrieve from AmazonS3:" . $self->S3->errstr)
         unless $ok;
     return ($ok->{value});
 }
@@ -115,11 +144,11 @@ sub Store {
     my ($sha, $content) = @_;
 
     # No-op if the path exists already
-    return (1) if $S3->bucket($self->{Bucket})->head_key( $sha );
+    return (1) if $self->BucketObj->head_key( $sha );
 
-    $S3->bucket($self->{Bucket})->add_key(
+    $self->BucketObj->add_key(
         $sha => $content
-    ) or return (undef, "Failed to write to AmazonS3: " . $S3->errstr);
+    ) or return (undef, "Failed to write to AmazonS3: " . $self->S3->errstr);
 
     return (1);
 }
@@ -202,4 +231,6 @@ RT what bucket name to use in your F<RT_SiteConfig.pm> file:
 
 =cut
 
+RT::Base->_ImportOverlays();
+
 1;
diff --git a/lib/RT/ExternalStorage/Backend.pm b/lib/RT/ExternalStorage/Backend.pm
index 976f3b8..f69403d 100644
--- a/lib/RT/ExternalStorage/Backend.pm
+++ b/lib/RT/ExternalStorage/Backend.pm
@@ -46,7 +46,6 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-use 5.008003;
 use warnings;
 use strict;
 
@@ -67,6 +66,7 @@ sub new {
         RT->Logger->error("No storage engine type provided");
         return undef;
     } elsif ($class->require) {
+        # no action needed; $class was loaded
     } else {
         my $long = "RT::ExternalStorage::$class";
         if ($long->require) {
@@ -86,4 +86,6 @@ sub new {
     $self->Init;
 }
 
+RT::Base->_ImportOverlays();
+
 1;
diff --git a/lib/RT/ExternalStorage/Disk.pm b/lib/RT/ExternalStorage/Disk.pm
index 4971989..305f35e 100644
--- a/lib/RT/ExternalStorage/Disk.pm
+++ b/lib/RT/ExternalStorage/Disk.pm
@@ -46,7 +46,6 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-use 5.008003;
 use warnings;
 use strict;
 
@@ -57,30 +56,41 @@ use File::Path qw//;
 use Role::Basic qw/with/;
 with 'RT::ExternalStorage::Backend';
 
+sub Path {
+    my $self = shift;
+    return $self->{Path};
+}
+
 sub Init {
     my $self = shift;
 
-    my %self = %{$self};
-    if (not $self{Path}) {
-        RT->Logger->error("No path provided for local storage");
+    if (not $self->Path) {
+        RT->Logger->error("Required option 'Path' not provided for Disk external storage.");
         return;
-    } elsif (not -e $self{Path}) {
-        RT->Logger->error("Path provided for local storage ($self{Path}) does not exist");
-        return;
-    } elsif ($self{Write} and not -w $self{Path}) {
-        RT->Logger->error("Path provided for local storage ($self{Path}) is not writable");
+    } elsif (not -e $self->Path) {
+        RT->Logger->error("Path provided for Disk external storage (".$self->Path.") does not exist");
         return;
     }
 
     return $self;
 }
 
+sub IsWriteable {
+    my $self = shift;
+
+    if (not -w $self->Path) {
+        return (undef, "Path provided for local storage (".$self->Path.") is not writable");
+    }
+
+    return (1);
+}
+
 sub Get {
     my $self = shift;
     my ($sha) = @_;
 
     $sha =~ m{^(...)(...)(.*)};
-    my $path = $self->{Path} . "/$1/$2/$3";
+    my $path = $self->Path . "/$1/$2/$3";
 
     return (undef, "File does not exist") unless -e $path;
 
@@ -96,8 +106,9 @@ sub Store {
     my $self = shift;
     my ($sha, $content) = @_;
 
+    # fan out to avoid one gigantic directory which slows down all file access
     $sha =~ m{^(...)(...)(.*)};
-    my $dir  = $self->{Path} . "/$1/$2";
+    my $dir  = $self->Path . "/$1/$2";
     my $path = "$dir/$3";
 
     return (1) if -f $path;
@@ -141,4 +152,6 @@ internal inconsistency.
 
 =cut
 
+RT::Base->_ImportOverlays();
+
 1;
diff --git a/lib/RT/ExternalStorage/Dropbox.pm b/lib/RT/ExternalStorage/Dropbox.pm
index beef360..bb1ff5b 100644
--- a/lib/RT/ExternalStorage/Dropbox.pm
+++ b/lib/RT/ExternalStorage/Dropbox.pm
@@ -46,7 +46,6 @@
 #
 # END BPS TAGGED BLOCK }}}
 
-use 5.008003;
 use warnings;
 use strict;
 
@@ -55,27 +54,38 @@ package RT::ExternalStorage::Dropbox;
 use Role::Basic qw/with/;
 with 'RT::ExternalStorage::Backend';
 
-our $DROPBOX;
+sub Dropbox {
+    my $self = shift;
+    if (@_) {
+        $self->{Dropbox} = shift;
+    }
+    return $self->{Dropbox};
+}
+
+sub AccessToken {
+    my $self = shift;
+    return $self->{AccessToken};
+}
+
 sub Init {
     my $self = shift;
-    my %self = %{$self};
 
     if (not File::Dropbox->require) {
         RT->Logger->error("Required module File::Dropbox is not installed");
         return;
-    } elsif (not $self{AccessToken}) {
-        RT->Logger->error("AccessToken not provided for Dropbox.  Register a new application"
-                      . " at https://www.dropbox.com/developers/apps and generate an access token.");
+    } elsif (not $self->AccessToken) {
+        RT->Logger->error("Required option 'AccessToken' not provided for Dropbox external storage. See the documentation for " . __PACKAGE__ . " for setting up this integration.");
         return;
     }
 
 
-    $DROPBOX = File::Dropbox->new(
+    my $dropbox = File::Dropbox->new(
         oauth2       => 1,
-        access_token => $self{AccessToken},
+        access_token => $self->AccessToken,
         root         => 'sandbox',
         furlopts     => { timeout => 60 },
     );
+    $self->Dropbox($dropbox);
 
     return $self;
 }
@@ -84,10 +94,12 @@ sub Get {
     my $self = shift;
     my ($sha) = @_;
 
-    open( $DROPBOX, "<", $sha)
+    my $dropbox = $self->Dropbox;
+
+    open( $dropbox, "<", $sha)
         or return (undef, "Failed to retrieve file from dropbox: $!");
-    my $content = do {local $/; <$DROPBOX>};
-    close $DROPBOX;
+    my $content = do {local $/; <$dropbox>};
+    close $dropbox;
 
     return ($content);
 }
@@ -96,14 +108,16 @@ sub Store {
     my $self = shift;
     my ($sha, $content) = @_;
 
+    my $dropbox = $self->Dropbox;
+
     # No-op if the path exists already.  This forces a metadata read.
-    return (1) if open( $DROPBOX, "<", $sha);
+    return (1) if open( $dropbox, "<", $sha);
 
-    open( $DROPBOX, ">", $sha )
+    open( $dropbox, ">", $sha )
         or return (undef, "Open for write on dropbox failed: $!");
-    print $DROPBOX $content
+    print $dropbox $content
         or return (undef, "Write to dropbox failed: $!");
-    close $DROPBOX
+    close $dropbox
         or return (undef, "Flush to dropbox failed: $!");
 
     return (1);
@@ -177,4 +191,6 @@ C<AccessToken>:
 
 =cut
 
+RT::Base->_ImportOverlays();
+
 1;
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 2434d6c..66a096d 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -733,6 +733,20 @@ sub FindDependencies {
     $deps->Add( out => $self->Object );
 }
 
+sub ShouldStoreExternally {
+    my $self = shift;
+    my $type = $self->CustomFieldObj->Type;
+    my $length = length($self->LargeContent || '');
+
+    return 0 if $length == 0;
+
+    return 1 if $type eq "Binary";
+
+    return 1 if $type eq "Image" and $length > 10 * 1024 * 1024;
+
+    return 0;
+}
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 7cf116a..69d06d6 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -860,24 +860,28 @@ sub _EncodeLOB {
 Unpacks data stored in the database, which may be base64 or QP encoded
 because of our need to store binary and badly encoded data in columns
 marked as UTF-8.  Databases such as PostgreSQL and Oracle care that you
-are feeding them invalid UTF-8 and will refuse the content.  This
-function handles unpacking the encoded data.
+are feeding them invalid UTF-8 and will refuse the content.  This function
+handles unpacking the encoded data.
 
-It returns textual data as a UTF-8 string which has been processed by Encode's
-PERLQQ filter which will replace the invalid bytes with \x{HH} so you can see
-the invalid byte but won't run into problems treating the data as UTF-8 later.
+Alternatively, if the data lives in external storage, it will be read
+(or downloaded) and returned.
+
+C<_DecodeLOB> returns textual data as a UTF-8 string which has been
+processed by L<Encode>'s PERLQQ filter which will replace the invalid bytes
+with C<\x{HH}> so you can see the invalid byte but won't run into problems
+treating the data as UTF-8 later.
 
 This is similar to how we filter all data coming in via the web UI in
-RT::Interface::Web::DecodeARGS. This filter should only end up being
+L<RT::Interface::Web/DecodeARGS>. This filter should only end up being
 applied to old data from less UTF-8-safe versions of RT.
 
 If the passed C<ContentType> includes a character set, that will be used
 to decode textual data; the default character set is UTF-8.  This is
 necessary because while we attempt to store textual data as UTF-8, the
 definition of "textual" has migrated over time, and thus we may now need
-to attempt to decode data that was previously not trancoded on insertion.
+to attempt to decode data that was previously not transcoded on insertion.
 
-Important Note - This function expects an octet string and returns a
+Important note: This function expects an octet string and returns a
 character string for non-binary data.
 
 =cut
@@ -896,9 +900,26 @@ sub _DecodeLOB {
     elsif ( $ContentEncoding eq 'quoted-printable' ) {
         $Content = MIME::QuotedPrint::decode($Content);
     }
+    elsif ( $ContentEncoding eq 'external' ) {
+        my $Digest = $Content;
+        my $Storage = RT->System->ExternalStorage;
+        unless ($Storage) {
+            RT->Logger->error( "Failed to load $Content; external storage not configured" );
+            return ("");
+        };
+
+        ($Content, my $msg) = $Storage->Get( $Digest );
+        unless (defined $Content) {
+            RT->Logger->error( "Failed to load $Digest from external storage: $msg" );
+            return ("");
+        }
+
+        return ($Content);
+    }
     elsif ( $ContentEncoding && $ContentEncoding ne 'none' ) {
         return ( $self->loc( "Unknown ContentEncoding [_1]", $ContentEncoding ) );
     }
+
     if ( RT::I18N::IsTextualContentType($ContentType) ) {
         my $entity = MIME::Entity->new();
         $entity->head->add("Content-Type", $ContentType);
diff --git a/lib/RT/System.pm b/lib/RT/System.pm
index b487cf2..32c2866 100644
--- a/lib/RT/System.pm
+++ b/lib/RT/System.pm
@@ -327,6 +327,13 @@ sub ParsedUpgradeHistory {
     return ($version_status, @lines);
 }
 
+sub ExternalStorage {
+    my $self = shift;
+    if (@_) {
+        $self->{ExternalStorage} = shift;
+    }
+    return $self->{ExternalStorage};
+}
 
 RT::Base->_ImportOverlays();
 
diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
old mode 100755
new mode 100644
index 8a18878..c125d4d
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -66,18 +66,25 @@ BEGIN { # BEGIN RT CMD BOILERPLATE
 
 }
 
-BEGIN { $RT::ExternalStorage::WRITE = 1 };
 use RT -init;
 
 # Ensure we only run one of these processes at once
 use Fcntl ':flock';
 exit unless flock main::DATA, LOCK_EX | LOCK_NB;
 
+use Digest::SHA qw//;
+
+my $ExternalStorage = RT->System->ExternalStorage;
+
 die "\%ExternalStorage is not configured\n"
-    unless RT->Config->Get("ExternalStorage");
+    unless $ExternalStorage;
 
-exit unless $RT::ExternalStorage::BACKEND;
+if ($ExternalStorage->can('IsWriteable')) {
+    my ($ok, $msg) = $ExternalStorage->IsWriteable;
+    die $msg if !$ok;
+}
 
+# pull out the previous high-water mark for each object type
 my $last = RT->System->FirstAttribute("ExternalStorage");
 $last = $last ? $last->Content : {};
 
@@ -122,20 +129,19 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
                 TABLE2 => 'CustomFields',
                 FIELD2 => 'id',
             );
-            # TODO: use IN operator once we increase required RT version to 4.2
             $attach->Limit(
-                ALIAS => $cfs,
-                FIELD => "Type",
-                VALUE => $_,
-            ) for qw(Binary Image);
+                ALIAS    => $cfs,
+                FIELD    => 'Type',
+                OPERATOR => 'IN',
+                VALUE    => [ qw(Binary Image) ],
+            );
             $attach->{'find_expired_rows'} = 1;
         }
 
         $attach->RowsPerPage(100);
-        $RT::Handle->dbh->begin_work;
         while ( my $a = $attach->Next ) {
             $id = $a->id;
-            next unless $a->StoreExternally;
+            next unless $a->ShouldStoreExternally;
 
             # Explicitly get bytes (not characters, which ->$column would do)
             my $content = $a->_DecodeLOB(
@@ -145,12 +151,14 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             );
 
             # Attempt to write that out
-            my ($key, $msg) = RT::ExternalStorage->Store( $content );
+            my ($key, $msg) = Store( $content );
             unless ($key) {
                 RT->Logger->error("Failed to store $class $id: $msg");
                 exit 1;
             }
 
+            $RT::Handle->dbh->begin_work;
+
             (my $status, $msg ) = $a->__Set(
                 Field => $column, Value => $key
             );
@@ -166,14 +174,27 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
                 RT->Logger->error("Failed to update ContentEncoding of $class $id: $msg");
                 exit 2;
             }
+
+            $RT::Handle->dbh->commit;
         }
-        $RT::Handle->dbh->commit;
 
         last unless $attach->Count;
     }
     $last->{$class} = $id;
 }
 
+# update high-water mark for each object type
 RT->System->SetAttribute( Name => "ExternalStorage", Content => $last );
 
+sub Store {
+    my $content = shift;
+
+    my $key = Digest::SHA::sha256_hex( $content );
+    my ($ok, $msg) = $ExternalStorage->Store( $key => $content );
+    return ($ok, $msg) unless defined $ok;
+
+    return ($key);
+}
+
+# don't remove; for locking (see call to flock above)
 __DATA__
diff --git a/t/externalstorage/basic.t b/t/externalstorage/basic.t
index 497f68c..c71935d 100644
--- a/t/externalstorage/basic.t
+++ b/t/externalstorage/basic.t
@@ -34,22 +34,22 @@ my @attachs = @{ $ticket->Transactions->First->Attachments->ItemsArrayRef };
 is scalar @attachs, 4, "Contains a multipart and two sub-parts";
 
 is $attachs[0]->ContentType, "multipart/mixed", "Found the top multipart";
-ok !$attachs[0]->StoreExternally, "Shouldn't store multipart part on disk";
+ok !$attachs[0]->ShouldStoreExternally, "Shouldn't store multipart part on disk";
 
 is $attachs[1]->ContentType, "text/plain", "Found the text part";
 is $attachs[1]->Content, 'test', "Can get the text part content";
 is $attachs[1]->ContentEncoding, "none", "Content is not encoded";
-ok !$attachs[1]->StoreExternally, "Won't store text part on disk";
+ok !$attachs[1]->ShouldStoreExternally, "Won't store text part on disk";
 
 is $attachs[2]->ContentType, "image/special", "Found the image part";
 is $attachs[2]->Content, 'boo',  "Can get the image content";
 is $attachs[2]->ContentEncoding, "none", "Content is not encoded";
-ok !$attachs[2]->StoreExternally, "Won't store images on disk";
+ok !$attachs[2]->ShouldStoreExternally, "Won't store images on disk";
 
 is $attachs[3]->ContentType, "application/octet-stream", "Found the binary part";
 is $attachs[3]->Content, 'thing',  "Can get the binary content";
 is $attachs[3]->ContentEncoding, "none", "Content is not encoded";
-ok $attachs[3]->StoreExternally, "Will store binary data on disk";
+ok $attachs[3]->ShouldStoreExternally, "Will store binary data on disk";
 
 my $dir = RT::Test::ExternalStorage->attachments_dir;
 ok !<$dir/*>, "Attachments directory is empty";

commit ae65f3a31e802a0280a365d57e39a6bb07d23aee
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri May 22 19:24:23 2015 +0000

    Add --with-attachment-store to ./configure
    
        This sets up defaults for rt-test-dependencies to install Amazon::S3
        or File::Dropbox as appropriate

diff --git a/configure.ac b/configure.ac
index 7df7b37..1a611d3 100755
--- a/configure.ac
+++ b/configure.ac
@@ -324,6 +324,16 @@ fi
 AC_SUBST(RT_SMIME_DEPS)
 AC_SUBST(RT_SMIME)
 
+dnl ExternalStorage
+AC_ARG_WITH(attachment-store,
+	    AC_HELP_STRING([--with-attachment-store=TYPE],
+	    		   [which attachment storage RT will use for attachments (default: database) (database, disk, S3 and Dropbox are valid)]), 
+            ATTACHMENT_STORE=$withval,
+            ATTACHMENT_STORE=database)
+if test "$ATTACHMENT_STORE" != 'database' -a "$ATTACHMENT_STORE" != 'disk' -a "$ATTACHMENT_STORE" != 'S3' -a "$ATTACHMENT_STORE" != 'Dropbox' ; then
+	AC_MSG_ERROR([Only database, disk, S3 and Dropbox are valid db types])
+fi
+AC_SUBST(ATTACHMENT_STORE)
 
 dnl This section maps the variable names this script 'natively' generates
 dnl to their existing names. They should be removed from here as the .in
diff --git a/sbin/rt-test-dependencies.in b/sbin/rt-test-dependencies.in
index 363177b..bbaf87b 100644
--- a/sbin/rt-test-dependencies.in
+++ b/sbin/rt-test-dependencies.in
@@ -78,6 +78,8 @@ GetOptions(
     'with-USERLOGO',
     'with-HTML-DOC',
 
+    'with-S3', 'with-DROPBOX',
+
     'list-deps',
     'siteinstall!',
     'help|h',
@@ -91,18 +93,20 @@ if ( $args{help} ) {
 
 # Set up defaults
 my %default = (
-    'with-CORE' => 1,
-    'with-CLI' => 1,
-    'with-MAILGATE' => 1, 
-    'with-DEVELOPER' => @RT_DEVELOPER@,
-    'with-GPG' => @RT_GPG_DEPS@,
-    'with-SMIME' => @RT_SMIME_DEPS@,
-    'with-ICAL' => 1,
-    'with-GRAPHVIZ' => @RT_GRAPHVIZ@,
-    'with-GD' => @RT_GD@,
+    'with-CORE'       => 1,
+    'with-CLI'        => 1,
+    'with-MAILGATE'   => 1,
+    'with-DEVELOPER'  => @RT_DEVELOPER@,
+    'with-GPG'        => @RT_GPG_DEPS@,
+    'with-SMIME'      => @RT_SMIME_DEPS@,
+    'with-ICAL'       => 1,
+    'with-GRAPHVIZ'   => @RT_GRAPHVIZ@,
+    'with-GD'         => @RT_GD@,
     'with-DASHBOARDS' => 1,
-    'with-USERLOGO' => 1,
-    'with-HTML-DOC' => @RT_DEVELOPER@,
+    'with-USERLOGO'   => 1,
+    'with-HTML-DOC'   => @RT_DEVELOPER@,
+    'with-S3'         => (uc(q{@ATTACHMENT_STORE@}) eq 'S3'),
+    'with-DROPBOX'    => (uc(q{@ATTACHMENT_STORE@}) eq 'DROPBOX'),
 );
 $args{$_} = $default{$_} foreach grep !exists $args{$_}, keys %default;
 
@@ -378,6 +382,14 @@ HTML::Entities
 Pod::Simple 3.24
 .
 
+$deps{'S3'} = [ text_to_hash( <<'.') ];
+Amazon::S3
+.
+
+$deps{'DROPBOX'} = [ text_to_hash( <<'.') ];
+File::Dropbox
+.
+
 my %AVOID = (
     'DBD::Oracle' => [qw(1.23)],
     'Devel::StackTrace' => [qw(1.28 1.29)],

commit ead55c854d7f995a00289176654aeb9b432bafdd
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Thu May 21 23:32:49 2015 +0000

    New config for ExternalStorageCutoffSize
    
        This lets you adjust how large attachments must be to go into
        external storage. Its value is changed from 10 MiB to 10 MB for
        parity with MaxAttachmentSize.

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index c341ccd..5dd768e 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -2557,6 +2557,22 @@ and accepts.
 
 Set(%ExternalStorage, ());
 
+=item C<$ExternalStorageCutoffSize>
+
+Certain object types, like values for Binary (aka file upload) custom
+fields, are always put into external storage. However, for other
+object types, like images and text, there is a line in the sand where
+you want small objects in the database but large objects in external
+storage. By default, objects larger than 10 MB will be put into external
+storage. C<$ExternalStorageCutoffSize> adjusts that line in the sand.
+
+Note that changing this setting does not affect existing attachments, only
+the new ones that C<sbin/rt-externalize-attachments> hasn't seen yet.
+
+=cut
+
+Set($ExternalStorageCutoffSize, 10_000_000);
+
 =back
 
 =head1 Lifecycles
diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 9570447..195d60d 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -1189,11 +1189,11 @@ sub ShouldStoreExternally {
         return 0;
     } elsif ($type =~ m{^(text|message)/}) {
         # If textual, we only store externally if it's _large_ (> 10M)
-        return 1 if $length > 10 * 1024 * 1024;
+        return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
         return 0;
     } elsif ($type =~ m{^image/}) {
         # Ditto images, which may be displayed inline
-        return 1 if $length > 10 * 1024 * 1024;
+        return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
         return 0;
     } else {
         return 1;
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 66a096d..963b795 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -742,7 +742,7 @@ sub ShouldStoreExternally {
 
     return 1 if $type eq "Binary";
 
-    return 1 if $type eq "Image" and $length > 10 * 1024 * 1024;
+    return 1 if $type eq "Image" and $length > RT->Config->Get('ExternalStorageCutoffSize');
 
     return 0;
 }
diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index c125d4d..1895971 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -118,7 +118,7 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             $attach->Limit(
                 FUNCTION  => 'LENGTH(main.Content)',
                 OPERATOR  => '>',
-                VALUE     => 10*1024*1024,
+                VALUE     => RT->Config->Get('ExternalStorageCutoffSize'),
                 SUBCLAUSE => 'applies',
                 ENTRYAGGREGATOR => 'OR',
             );

commit 2d16571a18694c745280230d817782a559c6d24a
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri May 22 22:03:10 2015 +0000

    New config for ExternalStorageDirectLink
    
        This lets you link directly to files in external storage, which
        means RT does not have to be involved in the downloading of such
        attachments.
    
        This feature is currently for AmazonS3 only. Dropbox does have a
        way to generate sharing hyperlinks, but it's not supported in the
        module we use (unfortunately, File::Dropbox::metadata doesn't include
        the link).

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index 5dd768e..cbf855b 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -2573,6 +2573,24 @@ the new ones that C<sbin/rt-externalize-attachments> hasn't seen yet.
 
 Set($ExternalStorageCutoffSize, 10_000_000);
 
+=item C<$ExternalStorageDirectLink>
+
+Certain ExternalStorage backends can serve files over HTTP.  For such
+backends, RT can link directly to those files in external storage.  This
+cuts down download time and relieves resource pressure because RT's web
+server is no longer involved in retrieving and then immediately serving
+each attachment.
+
+Of the storage engines that RT ships, only
+L<RT::ExternalStorage::AmazonS3> supports this feature, and you must
+manually configure it to allow direct linking.
+
+Set this to 1 to link directly to files in external storage.
+
+=cut
+
+Set($ExternalStorageDirectLink, 0);
+
 =back
 
 =head1 Lifecycles
diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 195d60d..c7995b7 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -1200,6 +1200,12 @@ sub ShouldStoreExternally {
     }
 }
 
+sub ExternalStoreDigest {
+    my $self = shift;
+
+    return undef if $self->ContentEncoding ne 'external';
+    return $self->_Value('Content');
+}
 
 RT::Base->_ImportOverlays();
 
diff --git a/lib/RT/ExternalStorage/AmazonS3.pm b/lib/RT/ExternalStorage/AmazonS3.pm
index c563938..7f8ecbd 100644
--- a/lib/RT/ExternalStorage/AmazonS3.pm
+++ b/lib/RT/ExternalStorage/AmazonS3.pm
@@ -153,6 +153,19 @@ sub Store {
     return (1);
 }
 
+sub DownloadURLFor {
+    my $self = shift;
+    my $object = shift;
+
+    my $column = $object->isa('RT::Attachment') ? 'Content' : 'LargeContent';
+    my $digest = $object->__Value($column);
+
+    # "If you make a request to the http://BUCKET.s3.amazonaws.com
+    # endpoint, the DNS has sufficient information to route your request
+    # directly to the region where your bucket resides."
+    return "https://" . $self->Bucket . ".s3.amazonaws.com/" . $digest;
+}
+
 =head1 NAME
 
 RT::ExternalStorage::AmazonS3 - Store files in Amazon's S3 cloud
@@ -229,6 +242,51 @@ RT what bucket name to use in your F<RT_SiteConfig.pm> file:
 
 =back
 
+=head2 Direct Linking
+
+This storage engine supports direct linking. This means that RT can link
+I<directly> to S3 when listing attachments, showing image previews, etc.
+This relieves some bandwidth pressure from RT because ordinarily it would
+have to download each attachment from S3 to be able to serve it.
+
+To enable direct linking you must first make all content in your bucket
+publicly viewable.
+
+B<Beware that this could have serious implications for billing and
+privacy>. RT cannot enforce its access controls for content on S3. This
+is tempered somewhat by the fact that users must be able to guess the
+SHA-256 digest of the file to be able to access it. But there is nothing
+stopping someone from tweeting a URL to a file hosted on your S3. These
+concerns do not arise when using an RT-mediated link to S3, since RT
+uses an access key to upload to and download from S3.
+
+To make all content in an S3 bucket publicly viewable, navigate to
+the bucket in the S3 web UI. Select the "Properties" tab and inside
+"Permissions" there is a button to "Add bucket policy". Paste the
+following content in the provided textbox:
+
+    {
+        "Version": "2008-10-17",
+        "Statement": [
+            {
+                "Sid": "AllowPublicRead",
+                "Effect": "Allow",
+                "Principal": {
+                    "AWS": "*"
+                },
+                "Action": "s3:GetObject",
+                "Resource": "arn:aws:s3:::BUCKET/*"
+            }
+        ]
+    }
+
+Replace C<BUCKET> with the bucket name that is used by your RT instance.
+
+Finally, set C<$ExternalStorageDirectLink> to 1 in your
+F<RT_SiteConfig.pm> file:
+
+    Set($ExternalStorageDirectLink, 1);
+
 =cut
 
 RT::Base->_ImportOverlays();
diff --git a/lib/RT/ExternalStorage/Backend.pm b/lib/RT/ExternalStorage/Backend.pm
index f69403d..0f67996 100644
--- a/lib/RT/ExternalStorage/Backend.pm
+++ b/lib/RT/ExternalStorage/Backend.pm
@@ -56,6 +56,7 @@ use Role::Basic;
 requires 'Init';
 requires 'Get';
 requires 'Store';
+requires 'DownloadURLFor';
 
 sub new {
     my $class = shift;
diff --git a/lib/RT/ExternalStorage/Disk.pm b/lib/RT/ExternalStorage/Disk.pm
index 305f35e..63991bf 100644
--- a/lib/RT/ExternalStorage/Disk.pm
+++ b/lib/RT/ExternalStorage/Disk.pm
@@ -123,6 +123,10 @@ sub Store {
     return (1);
 }
 
+sub DownloadURLFor {
+    return;
+}
+
 =head1 NAME
 
 RT::ExternalStorage::Disk - On-disk storage of attachments
diff --git a/lib/RT/ExternalStorage/Dropbox.pm b/lib/RT/ExternalStorage/Dropbox.pm
index bb1ff5b..ea9691a 100644
--- a/lib/RT/ExternalStorage/Dropbox.pm
+++ b/lib/RT/ExternalStorage/Dropbox.pm
@@ -123,6 +123,10 @@ sub Store {
     return (1);
 }
 
+sub DownloadURLFor {
+    return;
+}
+
 =head1 NAME
 
 RT::ExternalStorage::Dropbox - Store files in the Dropbox cloud
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 963b795..b9e1430 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -747,6 +747,13 @@ sub ShouldStoreExternally {
     return 0;
 }
 
+sub ExternalStoreDigest {
+    my $self = shift;
+
+    return undef if $self->ContentEncoding ne 'external';
+    return $self->_Value( 'LargeContent' );
+}
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/System.pm b/lib/RT/System.pm
index 32c2866..9103d14 100644
--- a/lib/RT/System.pm
+++ b/lib/RT/System.pm
@@ -327,6 +327,13 @@ sub ParsedUpgradeHistory {
     return ($version_status, @lines);
 }
 
+=head2 ExternalStorage
+
+Accessor for the storage engine selected by L<RT::ExternalStorage>. Will
+be undefined if external storage is not configured.
+
+=cut
+
 sub ExternalStorage {
     my $self = shift;
     if (@_) {
@@ -335,6 +342,30 @@ sub ExternalStorage {
     return $self->{ExternalStorage};
 }
 
+=head2 ExternalStorageURLFor object
+
+Returns a URL for direct linking to an L<RT::ExternalStorage>
+engine. Will return C<undef> if external storage is not configured, or
+if direct linking is disabled in config (C<$ExternalStorageDirectLink>),
+or if the external storage engine doesn't support hyperlinking (as in
+L<RT::ExternalStorage::Disk>), or finally, if the object is for whatever
+reason not present in external storage.
+
+=cut
+
+sub ExternalStorageURLFor {
+    my $self = shift;
+    my $Object = shift;
+
+    # external storage not configured
+    return undef if !$self->ExternalStorage;
+
+    # external storage direct links disabled
+    return undef if !RT->Config->Get('ExternalStorageDirectLink');
+
+    return $self->ExternalStorage->DownloadURLFor($Object);
+}
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/share/html/Elements/EditCustomFieldBinary b/share/html/Elements/EditCustomFieldBinary
index fc6ee3f..38f312e 100644
--- a/share/html/Elements/EditCustomFieldBinary
+++ b/share/html/Elements/EditCustomFieldBinary
@@ -47,7 +47,17 @@
 %# END BPS TAGGED BLOCK }}}
 % while ( $Values and my $value = $Values->Next ) {
 %# XXX - let user download the file(s) here?
-<input type="checkbox" name="<%$delete_name%>" class="checkbox CF-<%$CustomField->id%>-Edit" value="<% $value->Id %>" /><a href="<%RT->Config->Get('WebPath')%>/Download/CustomFieldValue/<% $value->Id %>/<% $value->Content |un %>"><% $value->Content %></a><br />
+<input type="checkbox" name="<%$delete_name%>" class="checkbox CF-<%$CustomField->id%>-Edit" value="<% $value->Id %>" />
+
+% if (my $url = RT->System->ExternalStorageURLFor($value)) {
+<a href="<%$url%>">
+% } else {
+<a href="<%RT->Config->Get('WebPath')%>/Download/CustomFieldValue/<% $value->Id %>/<% $value->Content |un %>">
+% }
+
+<% $value->Content %>
+</a>
+<br />
 % }
 % if ($MaxValues && $Values && $Values->Count >= $MaxValues ) {
 <div class="hints">
diff --git a/share/html/Elements/ShowCustomFieldBinary b/share/html/Elements/ShowCustomFieldBinary
index 544b54d..1a5672f 100644
--- a/share/html/Elements/ShowCustomFieldBinary
+++ b/share/html/Elements/ShowCustomFieldBinary
@@ -45,7 +45,14 @@
 %# those contributions and any derivatives thereof.
 %#
 %# END BPS TAGGED BLOCK }}}
-<a href="<%RT->Config->Get('WebPath')%>/Download/CustomFieldValue/<% $Object->Id %>/<% $Object->Content |un %>"><% $Object->Content %></a>
+% if (my $url = RT->System->ExternalStorageURLFor($Object)) {
+<a href="<%$url%>">
+% } else {
+<a href="<%RT->Config->Get('WebPath')%>/Download/CustomFieldValue/<% $Object->Id %>/<% $Object->Content |un %>">
+% }
+
+<% $Object->Content %>
+</a>
 <%ARGS>
 $Object => undef
 </%ARGS>
diff --git a/share/html/Elements/ShowCustomFieldImage b/share/html/Elements/ShowCustomFieldImage
index f5a1886..cc3a2aa 100644
--- a/share/html/Elements/ShowCustomFieldImage
+++ b/share/html/Elements/ShowCustomFieldImage
@@ -45,9 +45,12 @@
 %# those contributions and any derivatives thereof.
 %#
 %# END BPS TAGGED BLOCK }}}
-%    my $url = RT->Config->Get('WebPath') . "/Download/CustomFieldValue/".$Object->Id.'/'.$m->interp->apply_escapes($Object->Content, 'u');
 <a href="<% $url %>"><% $Object->Content %></a><br>
 <img type="<% $Object->ContentType %>" height="64" src="<% $url %>" align="middle" />
 <%ARGS>
 $Object
 </%ARGS>
+<%INIT>
+my $url = RT->System->ExternalStorageURLFor($Object)
+       || RT->Config->Get('WebPath') . "/Download/CustomFieldValue/".$Object->Id.'/'.$m->interp->apply_escapes($Object->Content, 'u');
+</%INIT>
diff --git a/share/html/Elements/ShowTransactionAttachments b/share/html/Elements/ShowTransactionAttachments
index 7aeded6..dcc28f2 100644
--- a/share/html/Elements/ShowTransactionAttachments
+++ b/share/html/Elements/ShowTransactionAttachments
@@ -59,7 +59,12 @@ foreach my $message ( @{ $Attachments->{ $Parent || 0 } || [] } ) {
     if ( $message->ContentLength or $name ) {
 </%PERL>
 <div class="downloadattachment">
-<a href="<% $AttachmentPath %>/<% $Transaction->Id %>/<% $message->Id %>/<% $name | u%>"><&|/l&>Download</&> <% length $name ? $name : loc('(untitled)') %></a>\
+% if (my $url = RT->System->ExternalStorageURLFor($message)) {
+<a href="<% $url %>">
+% } else {
+<a href="<% $AttachmentPath %>/<% $Transaction->Id %>/<% $message->Id %>/<% $name | u%>">
+% }
+<&|/l&>Download</&> <% length $name ? $name : loc('(untitled)') %></a>\
 % if ( $DownloadableHeaders && ! length $name && $message->ContentType =~ /text/  ) {
  / <a href="<% $AttachmentPath %>/WithHeaders/<% $message->Id %>"><% loc('with headers') %></a>
 % }
@@ -275,11 +280,13 @@ my $render_attachment = sub {
 
         my $filename = length $name ? $name : loc('(untitled)');
         my $efilename = $m->interp->apply_escapes( $filename, 'h' );
+
+        my $url = RT->System->ExternalStorageURLFor($message)
+               || $AttachmentPath .'/'. $Transaction->Id .'/'. $message->Id .'/'
+                . $m->interp->apply_escapes( $filename, 'u', 'h' );
+
         $m->out(
-            qq{<img alt="$efilename" title="$efilename"}
-            . ' src="'. $AttachmentPath .'/'. $Transaction->Id .'/'. $message->Id .'/'
-                . $m->interp->apply_escapes( $filename, 'u', 'h' )
-            . '" />'
+            qq{<img alt="$efilename" title="$efilename" src="$url" />}
         );
     }
     elsif ( $message->ContentLength && $message->ContentLength > 0 ) {
diff --git a/share/html/Ticket/Elements/ShowAttachments b/share/html/Ticket/Elements/ShowAttachments
index a567844..8116db5 100644
--- a/share/html/Ticket/Elements/ShowAttachments
+++ b/share/html/Ticket/Elements/ShowAttachments
@@ -58,7 +58,11 @@
 % foreach my $rev (@{$documents{$key}}) {
 % if ($rev->ContentLength) {
 <li><font size="-2">
+% if (my $url = RT->System->ExternalStorageURLFor($rev)) {
+<a href="<%$url%>">
+% } else {
 <a href="<%RT->Config->Get('WebPath')%>/Ticket/Attachment/<%$rev->TransactionId%>/<%$rev->Id%>/<%$rev->Filename | un %>">
+% }
 % my $desc = loc("[_1] ([_2]) by [_3]", $rev->CreatedAsString, $rev->FriendlyContentLength, $m->scomp('/Elements/ShowUser', User => $rev->CreatorObj));
 <% $desc |n%>
 </a>
diff --git a/share/html/Ticket/ShowEmailRecord.html b/share/html/Ticket/ShowEmailRecord.html
index bdb2119..6742271 100644
--- a/share/html/Ticket/ShowEmailRecord.html
+++ b/share/html/Ticket/ShowEmailRecord.html
@@ -62,9 +62,10 @@ my $show_content = sub {
         $m->out( $content );
         return;
     }
-    my $href = RT->Config->Get('WebPath') .'/Ticket/Attachment/'
-        . $attach->TransactionId .'/'. $attach->id .'/'
-        . $m->interp->apply_escapes( $attach->Filename, 'u' );
+    my $href = RT->System->ExternalStorageURLFor($attach)
+            || RT->Config->Get('WebPath') .'/Ticket/Attachment/'
+             . $attach->TransactionId .'/'. $attach->id .'/'
+             . $m->interp->apply_escapes( $attach->Filename, 'u' );
     $m->out( '<a href="'. $href  .'">'. loc('download') .'</a>' );
 };
 
diff --git a/share/html/m/ticket/show b/share/html/m/ticket/show
index ebf39ac..e0a8cf0 100644
--- a/share/html/m/ticket/show
+++ b/share/html/m/ticket/show
@@ -315,7 +315,11 @@ my $print_value = sub {
 % foreach my $rev (@{$documents{$key}}) {
 % if ($rev->ContentLength) {
 <li><font size="-2">
+% if (my $url = RT->System->ExternalStorageURLFor($rev)) {
+<a href="<%$url%>">
+% } else {
 <a href="<%RT->Config->Get('WebPath')%>/Ticket/Attachment/<%$rev->TransactionId%>/<%$rev->Id%>/<%$rev->Filename | un %>">
+% }
 <&|/l, $rev->CreatedAsString, $rev->FriendlyContentLength, $rev->CreatorObj->Name &>[_1] ([_2]) by [_3]</&>
 </a>
 </font></li>

commit 71006844d810124950f6e026d767bfc4df2651b6
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Jun 5 18:46:09 2015 +0000

    Add --verbose to rt-externalize-attachments

diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index c7995b7..8b5e2d6 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -1183,19 +1183,23 @@ sub ShouldStoreExternally {
     my $type = $self->ContentType;
     my $length = $self->ContentLength;
 
-    return 0 if $length == 0;
-
     if ($type =~ m{^multipart/}) {
-        return 0;
-    } elsif ($type =~ m{^(text|message)/}) {
-        # If textual, we only store externally if it's _large_ (> 10M)
+        return (0, "attachment is multipart");
+    }
+    elsif ($length == 0) {
+        return (0, "zero length");
+    }
+    elsif ($type =~ m{^(text|message)/}) {
+        # If textual, we only store externally if it's _large_
         return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
-        return 0;
-    } elsif ($type =~ m{^image/}) {
+        return (0, "text length ($length) does not exceed ExternalStorageCutoffSize (" . RT->Config->Get('ExternalStorageCutoffSize') . ")");
+    }
+    elsif ($type =~ m{^image/}) {
         # Ditto images, which may be displayed inline
         return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
-        return 0;
-    } else {
+        return (0, "image size ($length) does not exceed ExternalStorageCutoffSize (" . RT->Config->Get('ExternalStorageCutoffSize') . ")");
+    }
+    else {
         return 1;
     }
 }
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index b9e1430..92a0e02 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -738,13 +738,17 @@ sub ShouldStoreExternally {
     my $type = $self->CustomFieldObj->Type;
     my $length = length($self->LargeContent || '');
 
-    return 0 if $length == 0;
+    return (0, "zero length") if $length == 0;
 
     return 1 if $type eq "Binary";
 
-    return 1 if $type eq "Image" and $length > RT->Config->Get('ExternalStorageCutoffSize');
+    if ($type eq "Image") {
+        # We only store externally if it's _large_
+        return 1 if $length > RT->Config->Get('ExternalStorageCutoffSize');
+        return (0, "image size ($length) does not exceed ExternalStorageCutoffSize (" . RT->Config->Get('ExternalStorageCutoffSize') . ")");
+    }
 
-    return 0;
+    return (0, "Only custom fields of type Binary or Image go into external storage (not $type)");
 }
 
 sub ExternalStoreDigest {
diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index 1895971..adb5790 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -66,6 +66,19 @@ BEGIN { # BEGIN RT CMD BOILERPLATE
 
 }
 
+# Read in the options
+my %opts;
+use Getopt::Long;
+GetOptions( \%opts,
+    "help|h", "verbose|v",
+);
+
+if ($opts{'help'}) {
+    require Pod::Usage;
+    print Pod::Usage::pod2usage(-verbose => 2);
+    exit;
+}
+
 use RT -init;
 
 # Ensure we only run one of these processes at once
@@ -141,7 +154,14 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
         $attach->RowsPerPage(100);
         while ( my $a = $attach->Next ) {
             $id = $a->id;
-            next unless $a->ShouldStoreExternally;
+
+            my ($ok, $why) = $a->ShouldStoreExternally;
+            if (!$ok) {
+                if ($opts{verbose}) {
+                    RT->Logger->info("Skipping $class $id because: $why");
+                }
+                next;
+            }
 
             # Explicitly get bytes (not characters, which ->$column would do)
             my $content = $a->_DecodeLOB(
@@ -151,6 +171,10 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             );
 
             # Attempt to write that out
+            if ($opts{verbose}) {
+                RT->Logger->info("Storing $class $id");
+            }
+
             my ($key, $msg) = Store( $content );
             unless ($key) {
                 RT->Logger->error("Failed to store $class $id: $msg");
@@ -176,6 +200,10 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             }
 
             $RT::Handle->dbh->commit;
+
+            if ($opts{verbose}) {
+                RT->Logger->info("Stored $class $id as $key");
+            }
         }
 
         last unless $attach->Count;
@@ -196,5 +224,36 @@ sub Store {
     return ($key);
 }
 
+=head1 NAME
+
+rt-externalize-attachments - Move attachments from database to external storage
+
+=head1 SYNOPSIS
+
+    rt-externalize-attachments [options]
+
+=head1 OPTIONS
+
+This tool supports a few options. Most are for debugging.
+
+=over 8
+
+=item -h
+
+=item --help
+
+Display this documentation
+
+=item -v
+
+=item --verbose
+
+Log a message (at level "info") for each attachment, both before its
+upload begins and after it completes.
+
+=back
+
+=cut
+
 # don't remove; for locking (see call to flock above)
 __DATA__
diff --git a/t/externalstorage/basic.t b/t/externalstorage/basic.t
index c71935d..2fc5528 100644
--- a/t/externalstorage/basic.t
+++ b/t/externalstorage/basic.t
@@ -34,22 +34,29 @@ my @attachs = @{ $ticket->Transactions->First->Attachments->ItemsArrayRef };
 is scalar @attachs, 4, "Contains a multipart and two sub-parts";
 
 is $attachs[0]->ContentType, "multipart/mixed", "Found the top multipart";
-ok !$attachs[0]->ShouldStoreExternally, "Shouldn't store multipart part on disk";
+my ($ok, $msg) = $attachs[0]->ShouldStoreExternally;
+ok !$ok, "Shouldn't store multipart part on disk";
+like $msg, qr/attachment is multipart/, "Shouldn't store multipart part on disk";
 
 is $attachs[1]->ContentType, "text/plain", "Found the text part";
 is $attachs[1]->Content, 'test', "Can get the text part content";
 is $attachs[1]->ContentEncoding, "none", "Content is not encoded";
-ok !$attachs[1]->ShouldStoreExternally, "Won't store text part on disk";
+($ok, $msg) = $attachs[1]->ShouldStoreExternally;
+ok !$ok, "Won't store text part on disk";
+like $msg, qr/text length.*does not exceed/, "Won't store text part on disk";
 
 is $attachs[2]->ContentType, "image/special", "Found the image part";
 is $attachs[2]->Content, 'boo',  "Can get the image content";
 is $attachs[2]->ContentEncoding, "none", "Content is not encoded";
-ok !$attachs[2]->ShouldStoreExternally, "Won't store images on disk";
+($ok, $msg) = $attachs[2]->ShouldStoreExternally;
+ok !$ok, "Won't store images on disk";
+like $msg, qr/image size.*does not exceed/, "Won't store images on disk";
 
 is $attachs[3]->ContentType, "application/octet-stream", "Found the binary part";
 is $attachs[3]->Content, 'thing',  "Can get the binary content";
 is $attachs[3]->ContentEncoding, "none", "Content is not encoded";
-ok $attachs[3]->ShouldStoreExternally, "Will store binary data on disk";
+($ok, $msg) = $attachs[3]->ShouldStoreExternally;
+ok $ok, "Will store binary data on disk";
 
 my $dir = RT::Test::ExternalStorage->attachments_dir;
 ok !<$dir/*>, "Attachments directory is empty";

commit 08b1636199ad8b107bc45a9a5adc1771d906aacf
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Wed May 27 18:06:52 2015 +0000

    Filter should-store-externally? in Perl, not SQL
    
        This reduces performance, but it improves visibility and flexibility.
        The code for answering "should store externally?" is no longer
        duplicated across Perl and SQL; this means overlays and extensions can
        adjust how to answer the question. Finally the improved visibility
        helps sysadmins because the skip reason can be logged (as opposed
        to skipping in the database).
    
        Performance should be fine because it will largely be dominated by
        uploading, and the high-water marks mean we won't re-examine
        attachments.

diff --git a/sbin/rt-externalize-attachments.in b/sbin/rt-externalize-attachments.in
index adb5790..18a9c43 100644
--- a/sbin/rt-externalize-attachments.in
+++ b/sbin/rt-externalize-attachments.in
@@ -118,36 +118,8 @@ for my $class (qw/RT::Attachments RT::ObjectCustomFieldValues/) {
             VALUE           => 'external',
             ENTRYAGGREGATOR => 'AND',
         );
-        if ($class eq "RT::Attachments") {
-            $attach->_OpenParen('applies');
-            $attach->Limit(
-                FIELD     => 'ContentType',
-                OPERATOR  => 'NOT STARTSWITH',
-                VALUE     => $_,
-                SUBCLAUSE => 'applies',
-                ENTRYAGGREGATOR => "AND",
-            ) for "text/", "message/", "image/", "multipart/";
-            $attach->_CloseParen('applies');
-            $attach->Limit(
-                FUNCTION  => 'LENGTH(main.Content)',
-                OPERATOR  => '>',
-                VALUE     => RT->Config->Get('ExternalStorageCutoffSize'),
-                SUBCLAUSE => 'applies',
-                ENTRYAGGREGATOR => 'OR',
-            );
-        } else {
-            my $cfs = $attach->Join(
-                ALIAS1 => 'main',
-                FIELD1 => 'CustomField',
-                TABLE2 => 'CustomFields',
-                FIELD2 => 'id',
-            );
-            $attach->Limit(
-                ALIAS    => $cfs,
-                FIELD    => 'Type',
-                OPERATOR => 'IN',
-                VALUE    => [ qw(Binary Image) ],
-            );
+
+        if ($class eq "RT::ObjectCustomFieldValues") {
             $attach->{'find_expired_rows'} = 1;
         }
 

-----------------------------------------------------------------------


More information about the rt-commit mailing list