centralize abstraction for thumbnail generation

CHANGELOG.md

@@ -31,7 +31,7 @@ and releases in Jupiter project adheres to [Semantic Versioning](http://semver.o
 - Update UAL Logo [#1616](https://github.com/ualbertalib/jupiter/issues/1616)
 - Refactor `inactive` draft cleanup rake task to be sidekiq cron job [#1611](https://github.com/ualbertalib/jupiter/issues/1611)
 - Feature Image on Item show page need to be centered align within column [#1405](https://github.com/ualbertalib/jupiter/issues/1405)
-
+- Centralize Abstraction for Thumbnail Generation [#1343](https://github.com/ualbertalib/jupiter/issues/1343)
 
 ### Fixed
 - failing tests [#1376](https://github.com/ualbertalib/jupiter/issues/1376)

README.md

@@ -12,7 +12,7 @@
 - **PostgreSQL**
 - **Redis**
 - **Solr**
-- **ImageMagick**
+- **ImageMagick, Poppler, FFMpeg**
 - **Node.js** 10.13.0+
 - **Yarn**
 

app/helpers/page_layout_helper.rb

@@ -44,11 +44,34 @@ def page_description(description = nil)
   end
   # rubocop:enable Rails/HelperInstanceVariable
 
+  def thumbnail_path(logo, args = { resize: '100x100', auto_orient: true })
+    return nil if logo.blank?
+
+    # images have variants
+    Rails.application.routes.url_helpers.rails_representation_path(logo.variant(args).processed)
+  rescue ActiveStorage::InvariableError
+    begin
+      # pdfs and video have previews
+      Rails.application.routes.url_helpers.rails_representation_path(logo.preview(args).processed)
+    # ActiveStorage::UnpreviewableError and sometimes MiniMagick::Error gets thrown here
+    rescue StandardError => e
+      logger.warn("#{logo.record_type} with id: #{logo.record_id} and thumnail #{logo.name} \
+      threw an error after ActiveStorage::InvariableError.")
+      Rollbar.warn("#{logo.record_type} with id: #{logo.record_id} and thumnail #{logo.name} \
+      threw an error after ActiveStorage::InvariableError.", e)
+      nil
+    end
+  rescue StandardError => e
+    logger.warn("#{logo.record_type} with id: #{logo.record_id} and thumnail #{logo.name} threw an error.")
+    Rollbar.warn("#{logo.record_type} with id: #{logo.record_id} and thumnail #{logo.name} threw an error.", e)
+    nil
+  end
+
   # rubocop:disable Rails/HelperInstanceVariable
   def page_image_url
     default_url = asset_pack_url('media/images/era-logo.png')
     # We only have images on community and item/thesis show pages
-    image_path = @community&.thumbnail_path || @item&.thumbnail_path
+    image_path = thumbnail_path(@community&.thumbnail_file) || thumbnail_path(@item&.thumbnail_file)
 
     image_path ? request.base_url + image_path : default_url
   end

app/models/community.rb

@@ -48,13 +48,6 @@ def remove_logo=(val)
     logo_attachment.purge
   end
 
-  # compatibility with item thumbnail API
-  def thumbnail_path(args = { resize: '100x100', auto_orient: true })
-    return nil if logo_attachment.blank?
-
-    Rails.application.routes.url_helpers.rails_representation_path(logo_attachment.variant(args).processed)
-  end
-
   def thumbnail_file
     logo.attachment
   end

app/models/concerns/draft_properties.rb

@@ -50,19 +50,6 @@ def thumbnail
       files.first
     end
 
-    # compatibility with the thumbnail API used in Items/Theses and Communities
-    def thumbnail_path(args = { resize: '100x100', auto_orient: true })
-      return nil unless thumbnail.present? && thumbnail.blob.present?
-
-      Rails.application.routes.url_helpers.rails_representation_path(thumbnail.variant(args).processed)
-    rescue ActiveStorage::InvariableError
-      begin
-        Rails.application.routes.url_helpers.rails_representation_path(thumbnail.preview(args).processed)
-      rescue ActiveStorage::UnpreviewableError
-        nil
-      end
-    end
-
     def thumbnail_file
       thumbnail if thumbnail.present? && thumbnail.blob.present?
     end

app/models/jupiter_core/depositable.rb

@@ -104,19 +104,6 @@ def set_thumbnail(attachment)
   end
   # rubocop:enable Naming/AccessorMethodName
 
-  def thumbnail_path(args = { resize: '100x100', auto_orient: true })
-    logo = files.find_by(id: logo_id)
-    return nil if logo.blank?
-
-    Rails.application.routes.url_helpers.rails_representation_path(logo.variant(args).processed)
-  rescue ActiveStorage::InvariableError
-    begin
-      Rails.application.routes.url_helpers.rails_representation_path(logo.preview(args).processed)
-    rescue ActiveStorage::UnpreviewableError
-      nil
-    end
-  end
-
   def thumbnail_file
     files.find_by(id: logo_id)
   end

app/views/admin/theses/draft/_thumbnail.html.erb

@@ -1,18 +1,8 @@
-<% begin %>
-<%# NOTE: This is superficially similar to the logic in DraftItem#thumbnail, but this file is run against
-every file attached to an item via a JS callback rendering files/update_files_list.js.erb calling _files_list.html.erb
-whereas DraftItem#thumbnail specifically only deals with rendering the designated file for representing it as a
-thumbnail and is used on various other pages, like profiles, which use the application/_thumbnail partial.%>
-  <% thumbnail = rails_representation_path(file.variant(resize: '100x100', auto_orient: true).processed) %>
-  <%= safe_thumbnail_tag(thumbnail, alt: '', title: file.filename, size: '100x100') %>
-<% rescue  ActiveStorage::InvariableError %>
-  <% begin %>
-      <% thumbnail = rails_representation_path(file.preview(resize: '100x100', auto_orient: true).processed) %>
-      <%= safe_thumbnail_tag(thumbnail, alt: '', title: file.filename, size: '100x100') %>
-  <% rescue ActiveStorage::UnpreviewableError %>
-    <div class="text-muted text-center img-thumbnail p-3">
-      <%= icon('far', file_icon(file.content_type), class: 'fa-5x') %>
-      <span class="sr-only"><%= file.filename %></span>
-    </div>
-  <% end %>
+<% if thumbnail_path(file).present? %>
+  <%= safe_thumbnail_tag(thumbnail_path(file), alt: '', title: file.filename, size: '100x100') %>
+<% else %>
+  <div class="text-muted text-center img-thumbnail p-3">
+    <%= icon('far', file_icon(file.content_type), class: 'fa-5x') %>
+    <span class="sr-only"><%= file.filename %></span>
+  </div>
 <% end %>

app/views/application/_feature_image.html.erb

@@ -1,6 +1,6 @@
 <% if policy(object).thumbnail? %>
-  <% if object.thumbnail_path.present? %>
-    <%= safe_thumbnail_tag(object.thumbnail_path(resize: '350x350', auto_orient: true), alt: '', size: '350x350') %>
+  <% if thumbnail_path(object&.thumbnail_file).present? %>
+    <%= safe_thumbnail_tag(thumbnail_path(object&.thumbnail_file, resize: '350x350', auto_orient: true), alt: '', size: '350x350') %>
   <% else %>
     <div class="d-flex justify-content-center align-items-center img-thumbnail p-5">
       <%= icon('far', file_icon(object.thumbnail_file&.content_type), class: 'fa-5x text-muted') %>

app/views/application/_thumbnail.html.erb

@@ -1,6 +1,6 @@
 <% if policy(object).thumbnail? %>
-  <% if object.thumbnail_path.present? %>
-    <%= safe_thumbnail_tag(object.thumbnail_path, alt: '', title: object.title, size: '100x100') %>
+  <% if thumbnail_path(object&.thumbnail_file).present? %>
+    <%= safe_thumbnail_tag(thumbnail_path(object&.thumbnail_file), alt: '', title: object.title, size: '100x100') %>
   <% else %>
     <% if object.is_a? Community %>
       <%= image_pack_tag('era-logo-without-text.png', class: 'j-thumbnail img-thumbnail', alt: '', title: object.title, size: '100x100') %>

app/views/items/draft/_thumbnail.html.erb

@@ -1,18 +1,8 @@
-<% begin %>
-<%# NOTE: This is superficially similar to the logic in DraftItem#thumbnail, but this file is run against
-every file attached to an item via a JS callback rendering files/update_files_list.js.erb calling _files_list.html.erb
-whereas DraftItem#thumbnail specifically only deals with rendering the designated file for representing it as a
-thumbnail and is used on various other pages, like profiles, which use the application/_thumbnail partial.%>
-  <% thumbnail = rails_representation_path(file.variant(resize: '100x100', auto_orient: true).processed) %>
-  <%= safe_thumbnail_tag(thumbnail, alt: '', title: file.filename, size: '100x100') %>
-<% rescue  ActiveStorage::InvariableError %>
-  <% begin %>
-      <% thumbnail = rails_representation_path(file.preview(resize: '100x100', auto_orient: true).processed) %>
-      <%= safe_thumbnail_tag(thumbnail, alt: '', title: file.filename, size: '100x100') %>
-  <% rescue ActiveStorage::UnpreviewableError %>
-    <div class="text-muted text-center img-thumbnail p-3">
-      <%= icon('far', file_icon(file.content_type), class: 'fa-5x') %>
-      <span class="sr-only"><%= file.filename %></span>
-    </div>
-  <% end %>
+<% if thumbnail_path(file).present? %>
+  <%= safe_thumbnail_tag(thumbnail_path(file), alt: '', title: file.filename, size: '100x100') %>
+<% else %>
+  <div class="text-muted text-center img-thumbnail p-3">
+    <%= icon('far', file_icon(file.content_type), class: 'fa-5x') %>
+    <span class="sr-only"><%= file.filename %></span>
+  </div>
 <% end %>

test/fixtures/active_storage/attachments.yml

@@ -0,0 +1,5 @@
+logo:
+  name: 'files'
+  record_type: 'Item'
+  record_id: <%= ActiveRecord::FixtureSet.identify(:fancy) %>
+  blob_id: <%= ActiveRecord::FixtureSet.identify(:jpeg) %>

test/fixtures/active_storage/blobs.yml

@@ -0,0 +1,6 @@
+jpeg:
+  key: '12345'
+  filename: 'image-sample.jpeg'
+  content_type: 'image/jpeg'
+  byte_size: 12086
+  checksum: "GxpIjJsC4KnRoBKNjWnkJA=="

test/helpers/page_layout_helper_test.rb

@@ -16,7 +16,6 @@ def base_url
 
   # page_title
 
-  # page_title
   test 'should return the page title when given one' do
     assert_equal t('site_name'), page_title(t('site_name'))
   end
@@ -93,9 +92,66 @@ def base_url
     @community.logo.attach io: File.open(file_fixture('image-sample.jpeg')),
                            filename: 'image-sample.jpeg', content_type: 'image/jpeg'
 
-    assert_equal page_image_url, request.base_url + @community.thumbnail_path
+    assert_equal page_image_url, request.base_url + thumbnail_path(@community.thumbnail_file)
+  end
+
+  # thumbnail_path
+
+  def mupdf_exists?
+    ActiveStorage::Previewer::MuPDFPreviewer.mutool_exists?
+  end
+
+  def poppler_exists?
+    ActiveStorage::Previewer::PopplerPDFPreviewer.pdftoppm_exists?
+  end
+
+  test 'thumbnail_path should return preview for pdf (Invariable but Previewable)' do
+    skip 'this test requires the dependency MuPDF or Poppler to be installed' unless mupdf_exists? || poppler_exists?
+
+    item = items(:fancy)
+    File.open(file_fixture('pdf-sample.pdf'), 'r') do |file|
+      item.add_and_ingest_files([file])
+    end
+
+    logo = item.reload.files.first
+    expected = Rails.application.routes.url_helpers.rails_representation_path(
+      logo.preview(resize: '100x100', auto_orient: true).processed
+    )
+    assert_equal expected, thumbnail_path(logo)
   end
 
+  test 'thumbnail_path should return preview for image (Variable)' do
+    item = items(:fancy)
+    File.open(file_fixture('image-sample.jpeg'), 'r') do |file|
+      item.add_and_ingest_files([file])
+    end
+
+    logo = item.reload.files.first
+    expected = Rails.application.routes.url_helpers.rails_representation_path(
+      logo.variant(resize: '100x100', auto_orient: true).processed
+    )
+    assert_equal expected, thumbnail_path(logo)
+  end
+
+  test 'thumbnail_path should provide nil if no thumbnail is possible (StandardError on variable)' do
+    logo = active_storage_attachments(:logo)
+    logo.define_singleton_method(:variant) { |_| throw StandardError }
+
+    assert_nil thumbnail_path(logo)
+    # TODO: assert that the logger.warn was written
+  end
+
+  test 'thumbnail_path should return nil if both the variant and preview fail' do
+    logo = active_storage_attachments(:logo)
+    logo.define_singleton_method(:variant) { |_| throw ActiveStorage::InvariableError }
+    logo.define_singleton_method(:preview) { |_| throw StandardError }
+
+    assert_nil thumbnail_path(logo)
+    # TODO: assert that the logger.warn was written
+  end
+
+  # canonical_href
+
   test 'canonical_href is returning the correct canoncial url' do
     assert_equal Jupiter::PRODUCTION_URL, canonical_href(nil)
     assert_equal Jupiter::PRODUCTION_URL, canonical_href('/')