From a0f72f34f8a1db887207d7f83dd042b3ebb03021 Mon Sep 17 00:00:00 2001
From: Jonathan Rochkind <jrochkind@chemheritage.org>
Date: Thu, 17 May 2018 15:38:30 -0400
Subject: [PATCH] s3 driver includes file_name in link_for extras

Some client code expects this, such as [sufia](https://github.com/samvera/sufia/blob/63e3f1338fb409b7854491ab3c9467209e009aa6/app/assets/javascripts/sufia/browse_everything.js) and (identical) [hyrax](https://github.com/samvera/hyrax/blob/0fe39d4f4dcc24deb805c1e66527ac^C9df7741a/app/assets/javascripts/hyrax/browse_everything.js).

In sufia/hyrax if the filename isn't there, it ends up blank in the UI.

I also added the expires header (where applicable), because I noticed that the [box driver](https://github.com/samvera/browse-everything/blob/2c0a3447049f7ee925a0ee0759d246fa28c474ac/lib/browse_everything/driver/box.rb#L59) has it, so I figured it couldn't hurt, although I am not aware of client code using it.

The sufia/hyrax client code REALLY wants a `size` key as well, but there doesn't seem to be a way to get it in the current architecture without another S3 API call. The size was already there displayed in the listing, so it's already been fetched, and there ought to be a way for the client code to get it... but I can't figure out what it is, so punting on that for now.
---
 lib/browse_everything/driver/s3.rb            | 18 +++++++++++++-----
 spec/unit/browse_everything/driver/s3_spec.rb |  6 +++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lib/browse_everything/driver/s3.rb b/lib/browse_everything/driver/s3.rb
index 018bf607..dff2365e 100644
--- a/lib/browse_everything/driver/s3.rb
+++ b/lib/browse_everything/driver/s3.rb
@@ -61,11 +61,19 @@ def details(path)
 
       def link_for(path)
         obj = bucket.object(full_path(path))
-        case config[:response_type].to_sym
-        when :signed_url then obj.presigned_url(:get, expires_in: config[:expires_in])
-        when :public_url then obj.public_url
-        when :s3_uri     then "s3://#{obj.bucket_name}/#{obj.key}"
-        end
+
+        extras = {
+          file_name: File.basename(path),
+          expires: (config[:expires_in] if config[:response_type] == :signed_url)
+        }.compact
+
+        url = case config[:response_type].to_sym
+              when :signed_url then obj.presigned_url(:get, expires_in: config[:expires_in])
+              when :public_url then obj.public_url
+              when :s3_uri     then "s3://#{obj.bucket_name}/#{obj.key}"
+              end
+
+        [url, extras]
       end
 
       def authorized?
diff --git a/spec/unit/browse_everything/driver/s3_spec.rb b/spec/unit/browse_everything/driver/s3_spec.rb
index 52d8823e..02da50fc 100644
--- a/spec/unit/browse_everything/driver/s3_spec.rb
+++ b/spec/unit/browse_everything/driver/s3_spec.rb
@@ -133,17 +133,17 @@
 
         it ':signed_url' do
           provider.config[:response_type] = :signed_url
-          expect(provider.link_for('foo/quux.png')).to eq('https://s3.amazonaws.com/presigned_url')
+          expect(provider.link_for('foo/quux.png')).to eq ["https://s3.amazonaws.com/presigned_url", {:file_name=>"quux.png", :expires=>14400}]
         end
 
         it ':public_url' do
           provider.config[:response_type] = :public_url
-          expect(provider.link_for('foo/quux.png')).to eq('https://s3.amazonaws.com/public_url')
+          expect(provider.link_for('foo/quux.png')).to eq ["https://s3.amazonaws.com/public_url", {:file_name=>"quux.png"}]
         end
 
         it ':s3_uri' do
           provider.config[:response_type] = :s3_uri
-          expect(provider.link_for('foo/quux.png')).to eq('s3://s3.bucket/foo/quux.png')
+          expect(provider.link_for('foo/quux.png')).to eq ['s3://s3.bucket/foo/quux.png', {:file_name=>"quux.png"}]
         end
       end
     end