From 2d629a34c26a8528ae00f404237e89b3cd57bdb5 Mon Sep 17 00:00:00 2001
From: Josh McArthur <joshua.mcarthur@gmail.com>
Date: Thu, 11 Jan 2024 13:10:23 +1300
Subject: [PATCH 1/5] Add aws-sdk-s3 to allow presigned URLs to be generated

---
 Gemfile      |  1 +
 Gemfile.lock | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/Gemfile b/Gemfile
index 9237e5c8c..d30da7236 100644
--- a/Gemfile
+++ b/Gemfile
@@ -11,6 +11,7 @@ gem 'pg', '~>1.2'
 # Use SQLite to access signs from a Signbank dictionary export
 gem 'sqlite3'
 
+gem 'aws-sdk-s3'
 gem 'bootsnap', '>= 1.1.0', require: false
 gem 'haml'
 gem 'jquery-rails'
diff --git a/Gemfile.lock b/Gemfile.lock
index ebf20cd6c..7d8530b37 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -71,6 +71,22 @@ GEM
     ast (2.4.2)
     autoprefixer-rails (10.3.3.0)
       execjs (~> 2)
+    aws-eventstream (1.3.0)
+    aws-partitions (1.878.0)
+    aws-sdk-core (3.190.2)
+      aws-eventstream (~> 1, >= 1.3.0)
+      aws-partitions (~> 1, >= 1.651.0)
+      aws-sigv4 (~> 1.8)
+      jmespath (~> 1, >= 1.6.1)
+    aws-sdk-kms (1.76.0)
+      aws-sdk-core (~> 3, >= 3.188.0)
+      aws-sigv4 (~> 1.1)
+    aws-sdk-s3 (1.142.0)
+      aws-sdk-core (~> 3, >= 3.189.0)
+      aws-sdk-kms (~> 1)
+      aws-sigv4 (~> 1.8)
+    aws-sigv4 (1.8.0)
+      aws-eventstream (~> 1, >= 1.0.2)
     babel-source (5.8.35)
     babel-transpiler (0.7.0)
       babel-source (>= 4.0, < 6)
@@ -175,6 +191,7 @@ GEM
       multi_xml (>= 0.5.2)
     i18n (1.14.1)
       concurrent-ruby (~> 1.0)
+    jmespath (1.6.2)
     jquery-rails (4.4.0)
       rails-dom-testing (>= 1, < 3)
       railties (>= 4.2.0)
@@ -420,6 +437,7 @@ PLATFORMS
 
 DEPENDENCIES
   autoprefixer-rails
+  aws-sdk-s3
   bootsnap (>= 1.1.0)
   brakeman
   bundle-audit

From bae5dac2fd163f466970e445b45dc4b9f4f8d94b Mon Sep 17 00:00:00 2001
From: Josh McArthur <joshua.mcarthur@gmail.com>
Date: Thu, 11 Jan 2024 13:54:02 +1300
Subject: [PATCH 2/5] Import Signbank::AssetURL model to handle presigning sign
 assets where required

---
 app/models/signbank/asset.rb           |  6 +++
 app/models/signbank/asset_url.rb       | 69 ++++++++++++++++++++++++
 config/initializers/inflections.rb     |  8 +--
 spec/models/signbank/asset_spec.rb     | 12 +++++
 spec/models/signbank/asset_url_spec.rb | 72 ++++++++++++++++++++++++++
 5 files changed, 163 insertions(+), 4 deletions(-)
 create mode 100644 app/models/signbank/asset_url.rb
 create mode 100644 spec/models/signbank/asset_url_spec.rb

diff --git a/app/models/signbank/asset.rb b/app/models/signbank/asset.rb
index 9de160f3e..985e82a32 100644
--- a/app/models/signbank/asset.rb
+++ b/app/models/signbank/asset.rb
@@ -7,5 +7,11 @@ class Asset < Signbank::Record
     default_scope -> { order(display_order: :asc) }
 
     scope :image, -> { where("filename LIKE '%.png'") }
+
+    def url
+      return unless super
+
+      AssetURL.new(super).url
+    end
   end
 end
diff --git a/app/models/signbank/asset_url.rb b/app/models/signbank/asset_url.rb
new file mode 100644
index 000000000..ed053fd1e
--- /dev/null
+++ b/app/models/signbank/asset_url.rb
@@ -0,0 +1,69 @@
+module Signbank
+  class AssetURL
+    attr_reader :asset_url
+
+    class S3Adapter
+      cattr_accessor :region, :access_key_id, :secret_access_key, :endpoint
+      self.region = ENV.fetch('DICTIONARY_AWS_REGION', ENV.fetch('AWS_REGION', nil))
+      self.access_key_id = ENV.fetch('DICTIONARY_AWS_ACCESS_KEY_ID', nil)
+      self.secret_access_key = ENV.fetch('DICTIONARY_AWS_SECRET_ACCESS_KEY', nil)
+      self.endpoint = 's3.amazonaws.com'
+
+      def initialize(asset)
+        @asset = asset
+      end
+
+      def self.configured?
+        region && access_key_id && secret_access_key && client
+      end
+
+      def self.client
+        @client ||= Aws::S3::Client.new(region:, access_key_id:,
+                                        secret_access_key:)
+      rescue Aws::Errors::MissingCredentialsError, Aws::Errors::MissingRegionError
+        nil
+      end
+
+      def bucket_name
+        bucket_name, hostname = @asset.asset_url.host.split('.', 2)
+        raise ArgumentError, "Invalid hostname #{@asset.asset_url.host}" unless hostname == endpoint
+
+        bucket_name
+      end
+
+      def url(expires_in: 1.hour)
+        return unless self.class.configured?
+
+        object_key =  @asset.asset_url.path[1..]
+
+        URI.parse(
+          Aws::S3::Object.new(bucket_name, object_key, client: self.class.client)
+          .presigned_url(:get, expires_in: expires_in.to_i)
+        )
+      end
+    end
+
+    class PassthroughUrlAdapter
+      def initialize(asset)
+        @asset = asset
+      end
+
+      def self.configured?
+        true
+      end
+
+      def url(*)
+        return unless self.class.configured?
+
+        @asset.asset_url
+      end
+    end
+
+    delegate :url, to: :@adapter
+
+    def initialize(asset_url, adapter: nil)
+      @asset_url = URI.parse(asset_url)
+      @adapter = (adapter || [S3Adapter, PassthroughUrlAdapter].find(&:configured?)).new(self)
+    end
+  end
+end
diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb
index aa7435fbc..34f00586c 100644
--- a/config/initializers/inflections.rb
+++ b/config/initializers/inflections.rb
@@ -1,4 +1,5 @@
 # frozen_string_literal: true
+
 # Be sure to restart your server when you modify this file.
 
 # Add new inflection rules using the following format. Inflections
@@ -11,7 +12,6 @@
 #   inflect.uncountable %w( fish sheep )
 # end
 
-# These inflection rules are supported but not enabled by default:
-# ActiveSupport::Inflector.inflections(:en) do |inflect|
-#   inflect.acronym 'RESTful'
-# end
+ActiveSupport::Inflector.inflections(:en) do |inflect|
+  inflect.acronym 'URL'
+end
diff --git a/spec/models/signbank/asset_spec.rb b/spec/models/signbank/asset_spec.rb
index ab7113fe6..a27ce7143 100644
--- a/spec/models/signbank/asset_spec.rb
+++ b/spec/models/signbank/asset_spec.rb
@@ -17,6 +17,18 @@
     end
   end
 
+  describe '#url' do
+    it 'is a Signbank::AssetURL' do
+      asset = Signbank::Asset.new(filename: 'test.png', url: '/test.png')
+      expect(asset.url).to be_a(URI)
+    end
+
+    it 'is nil when the URL is nil' do
+      asset = Signbank::Asset.new(filename: 'test.png', url: nil)
+      expect(asset.url).to be_nil
+    end
+  end
+
   describe '.scoped' do
     it 'is ordered by display_order' do
       sign = Signbank::Sign.create!(id: SecureRandom.uuid)
diff --git a/spec/models/signbank/asset_url_spec.rb b/spec/models/signbank/asset_url_spec.rb
new file mode 100644
index 000000000..0934f2d0f
--- /dev/null
+++ b/spec/models/signbank/asset_url_spec.rb
@@ -0,0 +1,72 @@
+require 'rails_helper'
+
+RSpec.describe Signbank::AssetURL do
+  describe '#url' do
+    context 'when using S3Adapter' do
+      let(:asset_url) { 'https://example.s3.amazonaws.com/assets/asset.mp4' }
+      let(:adapter) { Signbank::AssetURL::S3Adapter }
+
+      it 'returns the presigned URL for the asset' do
+        asset = Signbank::AssetURL.new(asset_url, adapter:)
+        presigned_url = 'https://s3.amazonaws.com/bucket-name/asset.mp4?expires=1234567890'
+
+        allow(adapter).to receive(:configured?).and_return(true)
+        allow(adapter).to receive(:client).and_return(instance_double(Aws::S3::Client))
+        allow_any_instance_of(Aws::S3::Object).to receive(:presigned_url).and_return(presigned_url)
+
+        expect(asset.url).to eq(URI.parse(presigned_url))
+      end
+
+      it 'returns nil if S3Adapter is not configured' do
+        asset = Signbank::AssetURL.new(asset_url, adapter:)
+
+        allow(adapter).to receive(:configured?).and_return(false)
+
+        expect(asset.url).to be_nil
+      end
+
+      it 'raises an error if the URL does not have the expected hostname' do
+        asset_url = 'https://example.com/assets/asset.mp4'
+        asset = Signbank::AssetURL.new(asset_url, adapter:)
+        allow(adapter).to receive(:configured?).and_return(true)
+
+        expect { asset.url }.to raise_error(ArgumentError)
+      end
+    end
+
+    context 'when using PassthroughUrlAdapter' do
+      let(:asset_url) { URI.parse('https://example.com/assets/asset.mp4') }
+      let(:adapter) { Signbank::AssetURL::PassthroughUrlAdapter }
+
+      it 'returns the original asset URL' do
+        asset = Signbank::AssetURL.new(asset_url.to_s, adapter:)
+
+        allow(adapter).to receive(:configured?).and_return(true)
+
+        expect(asset.url).to eq(asset_url)
+      end
+    end
+
+    context 'when no adapter is specified' do
+      let(:asset_url) { URI.parse('https://example.com/assets/asset.mp4') }
+
+      it 'uses the first configured adapter' do
+        asset = Signbank::AssetURL.new(asset_url.to_s)
+
+        allow(Signbank::AssetURL::S3Adapter).to receive(:configured?).and_return(false)
+        allow(Signbank::AssetURL::PassthroughUrlAdapter).to receive(:configured?).and_return(true)
+
+        expect(asset.url).to eq(asset_url)
+      end
+
+      it 'returns nil if no adapter is configured' do
+        asset = Signbank::AssetURL.new(asset_url.to_s)
+
+        allow(Signbank::AssetURL::S3Adapter).to receive(:configured?).and_return(false)
+        allow(Signbank::AssetURL::PassthroughUrlAdapter).to receive(:configured?).and_return(false)
+
+        expect(asset.url).to be_nil
+      end
+    end
+  end
+end

From 2bc14f8a4321f60302d59e1c530a448828cc35e7 Mon Sep 17 00:00:00 2001
From: Josh McArthur <joshua.mcarthur@gmail.com>
Date: Thu, 11 Jan 2024 14:24:51 +1300
Subject: [PATCH 3/5] Use Signbank::AssetURL for examples and sign video

---
 app/models/signbank/asset.rb         |  2 +-
 app/models/signbank/example.rb       |  6 ++++++
 app/models/signbank/sign.rb          |  6 ++++++
 spec/models/signbank/asset_spec.rb   | 10 ++++++----
 spec/models/signbank/example_spec.rb | 14 ++++++++++++++
 spec/models/signbank/sign_spec.rb    | 14 ++++++++++++++
 6 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/app/models/signbank/asset.rb b/app/models/signbank/asset.rb
index 985e82a32..b6e21b9ec 100644
--- a/app/models/signbank/asset.rb
+++ b/app/models/signbank/asset.rb
@@ -11,7 +11,7 @@ class Asset < Signbank::Record
     def url
       return unless super
 
-      AssetURL.new(super).url
+      AssetURL.new(super).url.to_s
     end
   end
 end
diff --git a/app/models/signbank/example.rb b/app/models/signbank/example.rb
index 5618d1142..0e15d9656 100644
--- a/app/models/signbank/example.rb
+++ b/app/models/signbank/example.rb
@@ -7,5 +7,11 @@ class Example < Signbank::Record
                       foreign_key: :word_id,
                       inverse_of: :examples
     default_scope -> { order(display_order: :asc).where.not(video: nil) }
+
+    def video
+      return unless super
+
+      AssetURL.new(super).url.to_s
+    end
   end
 end
diff --git a/app/models/signbank/sign.rb b/app/models/signbank/sign.rb
index 6f4053224..27b57b4e8 100644
--- a/app/models/signbank/sign.rb
+++ b/app/models/signbank/sign.rb
@@ -43,6 +43,12 @@ def picture_url
       picture&.url
     end
 
+    def video
+      return unless super
+
+      AssetURL.new(super).url.to_s
+    end
+
     ##
     # These are all aliases for the object shape that
     # existing code is expecting
diff --git a/spec/models/signbank/asset_spec.rb b/spec/models/signbank/asset_spec.rb
index a27ce7143..6cf1ea7c8 100644
--- a/spec/models/signbank/asset_spec.rb
+++ b/spec/models/signbank/asset_spec.rb
@@ -18,13 +18,15 @@
   end
 
   describe '#url' do
-    it 'is a Signbank::AssetURL' do
-      asset = Signbank::Asset.new(filename: 'test.png', url: '/test.png')
-      expect(asset.url).to be_a(URI)
+    it 'uses Signbank::AssetURL' do
+      double = instance_double(Signbank::AssetURL, url: URI.parse('/test.png'))
+      allow(Signbank::AssetURL).to receive(:new).and_return(double)
+      asset = Signbank::Asset.new(url: 'test.png')
+      expect(asset.url).to eq '/test.png'
     end
 
     it 'is nil when the URL is nil' do
-      asset = Signbank::Asset.new(filename: 'test.png', url: nil)
+      asset = Signbank::Asset.new(url: nil)
       expect(asset.url).to be_nil
     end
   end
diff --git a/spec/models/signbank/example_spec.rb b/spec/models/signbank/example_spec.rb
index 7f2e57aad..18ea214b9 100644
--- a/spec/models/signbank/example_spec.rb
+++ b/spec/models/signbank/example_spec.rb
@@ -18,4 +18,18 @@
       expect(sign.examples).to be_empty
     end
   end
+
+  describe '#video' do
+    it 'uses Signbank::AssetURL' do
+      double = instance_double(Signbank::AssetURL, url: URI.parse('/test.png'))
+      allow(Signbank::AssetURL).to receive(:new).and_return(double)
+      example = Signbank::Example.new(video: 'test.png')
+      expect(example.video).to eq '/test.png'
+    end
+
+    it 'is nil when the URL is nil' do
+      example = Signbank::Example.new(video: nil)
+      expect(example.video).to be_nil
+    end
+  end
 end
diff --git a/spec/models/signbank/sign_spec.rb b/spec/models/signbank/sign_spec.rb
index 60201852c..cded2e60f 100644
--- a/spec/models/signbank/sign_spec.rb
+++ b/spec/models/signbank/sign_spec.rb
@@ -101,4 +101,18 @@ module Signbank
       end
     end
   end
+
+  describe '#url' do
+    it 'uses Signbank::AssetURL' do
+      double = instance_double(Signbank::AssetURL, url: URI.parse('/test.png'))
+      allow(Signbank::AssetURL).to receive(:new).and_return(double)
+      sign = Signbank::Sign.new(video: 'test.png')
+      expect(sign.video).to eq '/test.png'
+    end
+
+    it 'is nil when the URL is nil' do
+      sign = Signbank::Sign.new(video: nil)
+      expect(sign.video).to be_nil
+    end
+  end
 end

From 64b8f319e5468d7ca440b006b84032077535c61a Mon Sep 17 00:00:00 2001
From: Josh McArthur <joshua.mcarthur@gmail.com>
Date: Mon, 15 Jan 2024 13:52:17 +1300
Subject: [PATCH 4/5] Apply patch to handle empty string column values for sign
 media

---
 app/models/signbank/asset.rb         | 2 +-
 app/models/signbank/example.rb       | 2 +-
 app/models/signbank/sign.rb          | 2 +-
 spec/models/signbank/asset_spec.rb   | 5 +++++
 spec/models/signbank/example_spec.rb | 5 +++++
 spec/models/signbank/sign_spec.rb    | 5 +++++
 6 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/app/models/signbank/asset.rb b/app/models/signbank/asset.rb
index b6e21b9ec..cd0b1bac1 100644
--- a/app/models/signbank/asset.rb
+++ b/app/models/signbank/asset.rb
@@ -9,7 +9,7 @@ class Asset < Signbank::Record
     scope :image, -> { where("filename LIKE '%.png'") }
 
     def url
-      return unless super
+      return unless super.presence
 
       AssetURL.new(super).url.to_s
     end
diff --git a/app/models/signbank/example.rb b/app/models/signbank/example.rb
index 0e15d9656..1c0a5e13d 100644
--- a/app/models/signbank/example.rb
+++ b/app/models/signbank/example.rb
@@ -9,7 +9,7 @@ class Example < Signbank::Record
     default_scope -> { order(display_order: :asc).where.not(video: nil) }
 
     def video
-      return unless super
+      return unless super.presence
 
       AssetURL.new(super).url.to_s
     end
diff --git a/app/models/signbank/sign.rb b/app/models/signbank/sign.rb
index 27b57b4e8..847b3c6be 100644
--- a/app/models/signbank/sign.rb
+++ b/app/models/signbank/sign.rb
@@ -44,7 +44,7 @@ def picture_url
     end
 
     def video
-      return unless super
+      return unless super.presence
 
       AssetURL.new(super).url.to_s
     end
diff --git a/spec/models/signbank/asset_spec.rb b/spec/models/signbank/asset_spec.rb
index 6cf1ea7c8..f421e9670 100644
--- a/spec/models/signbank/asset_spec.rb
+++ b/spec/models/signbank/asset_spec.rb
@@ -29,6 +29,11 @@
       asset = Signbank::Asset.new(url: nil)
       expect(asset.url).to be_nil
     end
+
+    it 'is nil when the URL is blank' do
+      asset = Signbank::Asset.new(url: "")
+      expect(asset.url).to be_nil
+    end
   end
 
   describe '.scoped' do
diff --git a/spec/models/signbank/example_spec.rb b/spec/models/signbank/example_spec.rb
index 18ea214b9..90c83e522 100644
--- a/spec/models/signbank/example_spec.rb
+++ b/spec/models/signbank/example_spec.rb
@@ -31,5 +31,10 @@
       example = Signbank::Example.new(video: nil)
       expect(example.video).to be_nil
     end
+
+    it 'is nil when the URL is blank' do
+      example = Signbank::Example.new(video: "")
+      expect(example.video).to be_nil
+    end
   end
 end
diff --git a/spec/models/signbank/sign_spec.rb b/spec/models/signbank/sign_spec.rb
index cded2e60f..cfc61da55 100644
--- a/spec/models/signbank/sign_spec.rb
+++ b/spec/models/signbank/sign_spec.rb
@@ -114,5 +114,10 @@ module Signbank
       sign = Signbank::Sign.new(video: nil)
       expect(sign.video).to be_nil
     end
+
+    it 'is nil when the URL is blank' do
+      sign = Signbank::Sign.new(video: "")
+      expect(sign.video).to be_nil
+    end
   end
 end

From e3e1422e3e965c096b14cbf175b9c0a4a3125f57 Mon Sep 17 00:00:00 2001
From: Josh McArthur <joshua.mcarthur@gmail.com>
Date: Mon, 15 Jan 2024 13:56:38 +1300
Subject: [PATCH 5/5] Correct string formatting violations with rubocop

---
 spec/models/signbank/asset_spec.rb   | 2 +-
 spec/models/signbank/example_spec.rb | 2 +-
 spec/models/signbank/sign_spec.rb    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/models/signbank/asset_spec.rb b/spec/models/signbank/asset_spec.rb
index f421e9670..93fc0e120 100644
--- a/spec/models/signbank/asset_spec.rb
+++ b/spec/models/signbank/asset_spec.rb
@@ -31,7 +31,7 @@
     end
 
     it 'is nil when the URL is blank' do
-      asset = Signbank::Asset.new(url: "")
+      asset = Signbank::Asset.new(url: '')
       expect(asset.url).to be_nil
     end
   end
diff --git a/spec/models/signbank/example_spec.rb b/spec/models/signbank/example_spec.rb
index 90c83e522..03fcade03 100644
--- a/spec/models/signbank/example_spec.rb
+++ b/spec/models/signbank/example_spec.rb
@@ -33,7 +33,7 @@
     end
 
     it 'is nil when the URL is blank' do
-      example = Signbank::Example.new(video: "")
+      example = Signbank::Example.new(video: '')
       expect(example.video).to be_nil
     end
   end
diff --git a/spec/models/signbank/sign_spec.rb b/spec/models/signbank/sign_spec.rb
index cfc61da55..8aaf62750 100644
--- a/spec/models/signbank/sign_spec.rb
+++ b/spec/models/signbank/sign_spec.rb
@@ -116,7 +116,7 @@ module Signbank
     end
 
     it 'is nil when the URL is blank' do
-      sign = Signbank::Sign.new(video: "")
+      sign = Signbank::Sign.new(video: '')
       expect(sign.video).to be_nil
     end
   end