Merge pull request 'Fix/improve local ActiveStorage backend usage and handling of WebApp icons' (#162) from bugfix/local_web_app_icons into chore/update_dependencies

Reviewed-on: #162
Reviewed-by: greg <greg@noreply.kosmos.org>

app/components/app_catalog/web_app_icon_component.html.erb

@@ -0,0 +1,5 @@
+<% if @image_url %>
+  <%= image_tag @image_url, class: "h-full w-full" %>
+<% else %>
+  <%= render partial: "icons/remotestorage", locals: { custom_class: "h-full w-full p-0.5 text-gray-200" } %>
+<% end %>

app/components/app_catalog/web_app_icon_component.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module AppCatalog
+  class WebAppIconComponent < ViewComponent::Base
+    def initialize(web_app:)
+      if web_app&.icon&.attached?
+        @image_url = image_url_for(web_app.icon)
+      elsif web_app&.apple_touch_icon&.attached?
+        @image_url = image_url_for(web_app.apple_touch_icon)
+      end
+    end
+
+    def image_url_for(attachment)
+      if Setting.s3_enabled?
+        s3_image_url(attachment)
+      else
+        Rails.application.routes.url_helpers.rails_blob_path(attachment, only_path: true)
+      end
+    end
+  end
+end

app/components/rs_auth_component.html.erb

@@ -1,16 +1,10 @@
 <div class="flex items-center gap-4">
   <div class="h-16 w-16 flex-none">
-    <% if @web_app.icon.attached? %>
-    <%= image_tag s3_image_url(@web_app.icon), class: "h-full w-full" %>
-    <% elsif @web_app.apple_touch_icon.attached? %>
-    <%= image_tag s3_image_url(@web_app.apple_touch_icon), class: "h-full w-full" %>
-    <% else %>
-    <%= render partial: "icons/remotestorage", locals: { custom_class: "h-full w-full p-0.5 text-gray-200" } %>
-    <% end %>
+    <%= render AppCatalog::WebAppIconComponent.new(web_app: @web_app) %>
   </div>
   <div class="flex-grow">
     <h4 class="mb-1 text-lg font-bold">
-      <%= @web_app.name %>
+      <%= @web_app&.name || @auth.app_name %>
     </h4>
     <p class="text-sm text-gray-500">
       <%= @auth.client_id %>

app/models/app_catalog/web_app.rb

@@ -1,7 +1,7 @@
 class AppCatalog::WebApp < ApplicationRecord
   store :metadata, coder: JSON
 
-  has_many :remote_storage_authorizations
+  has_many :remote_storage_authorizations, dependent: :destroy
 
   has_one_attached :icon
   has_one_attached :apple_touch_icon

app/models/setting.rb

@@ -15,6 +15,9 @@ class Setting < RailsSettings::Base
   field :redis_url, type: :string,
     default: ENV["REDIS_URL"] || "redis://localhost:6379/0"
 
+  field :s3_enabled, type: :boolean,
+    default: ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false"
+
   #
   # Registrations
   #

app/services/app_catalog_manager/update_metadata.rb

@@ -18,6 +18,10 @@ module AppCatalogManager
         @app.metadata[prop] = metadata.send(prop) if prop
       end
 
+      @app.save!
+
+      # TODO move icon downloads to separate, async job
+
       if icon = metadata.select_icon(sizes: "256x256") ||
          icon = metadata.select_icon(sizes: "192x192")
         attach_remote_image(:icon, icon)
@@ -27,8 +31,6 @@ module AppCatalogManager
       if apple_touch_icon = metadata.select_icon(purpose: "apple-touch-icon")
         attach_remote_image(:apple_touch_icon, apple_touch_icon)
       end
-
-      @app.save!
     rescue Manifique::Error => e
       msg = "Fetching web app manifest failed for #{e.url}: #{e.type}"
       Rails.logger.warn(msg)
@@ -42,14 +44,19 @@ module AppCatalogManager
       else
         download_url = "#{@app.url}/#{icon["src"].gsub(/^\//,'')}"
       end
-      filename = "#{attachment_name}.png"
-      key = "web_apps/#{@app.id}/icons/#{attachment_name}.png"
+      filename = "#{attachment_name}-#{Time.now.to_i}.png"
+      key = "web_apps/#{@app.id}/icons/#{filename}"
 
       begin
         tempfile = Down.download(download_url)
         @app.send(attachment_name).attach(key: key, io: tempfile, filename: filename)
       rescue Down::NotFound
-        Rails.logger.warn "Icon download failed: NotFound error for #{download_url}"
+        msg = "Download of \"#{attachment_name}\" failed: NotFound error for #{download_url}"
+        Rails.logger.warn(msg)
+        Sentry.capture_message(msg)
+      rescue => e
+        Rails.logger.warn "Saving attachment \"#{attachment_name}\" failed: \"#{e.message}\""
+        Sentry.capture_exception(e) if Setting.sentry_enabled?
       end
     end
   end

app/views/icons/_remotestorage.html.erb

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<svg width="24" height="24" class="<%= custom_class %>" clip-rule="evenodd" fill-rule="evenodd" image-rendering="optimizeQuality" shape-rendering="geometricPrecision" text-rendering="geometricPrecision" version="1.1" viewBox="0 0 250 249.9" xml:space="preserve" xmlns="http://www.w3.org/2000/svg">
+<svg width="24" height="24" class="icon-remotestorage <%= custom_class %>" clip-rule="evenodd" fill-rule="evenodd" image-rendering="optimizeQuality" shape-rendering="geometricPrecision" text-rendering="geometricPrecision" version="1.1" viewBox="0 0 250 249.9" xml:space="preserve" xmlns="http://www.w3.org/2000/svg">
  <g transform="translate(-66.822 -.16484)">
   <polygon id="polygon1" fill="currentColor" transform="matrix(.29308 0 0 .29308 83.528 -.028385)" points="228 181 370 100 511 181 652 263 370 425 87 263 87 263 0 213 0 213 0 311 0 378 0 427 0 476 86 525 185 582 370 689 554 582 653 525 653 590 653 592 370 754 0 542 0 640 185 747 370 853 554 747 739 640 739 525 739 476 739 427 739 378 653 427 370 589 86 427 86 361 185 418 370 524 554 418 653 361 739 311 739 213 554 107 370 0 185 107 58 180 144 230"/>
  </g>

config/environments/development.rb

@@ -71,7 +71,7 @@ Rails.application.configure do
   # Allow requests from any IP
   config.web_console.permissions = '0.0.0.0/0'
 
-  if ENV["S3_ENABLED"]
+  if ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false"
     config.active_storage.service = :s3
   else
     config.active_storage.service = :local

config/environments/production.rb

@@ -110,7 +110,7 @@ Rails.application.configure do
   # Set this to true and configure the email server for immediate delivery to raise delivery errors.
   config.action_mailer.raise_delivery_errors = true
 
-  if ENV["S3_ENABLED"]
+  if ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false"
     config.active_storage.service = :s3
   else
     config.active_storage.service = :local

config/storage.yml

@@ -1,12 +1,12 @@
 local:
   service: Disk
-  root: <%= Rails.root.join("storage") %>
+  root: <%= ENV["ACTIVE_STORAGE_PATH"] || Rails.root.join("storage") %>
 
 test:
   service: Disk
   root: <%= Rails.root.join("tmp/storage") %>
 
-<% if ENV["S3_ENABLED"] %>
+<% if ENV["S3_ENABLED"] && ENV["S3_ENABLED"].to_s != "false" %>
 s3:
   service: S3
   endpoint: <%= ENV["S3_ENDPOINT"] %>

docker-compose.yml

@@ -42,8 +42,10 @@ services:
       LDAP_ADMIN_PASSWORD: passthebutter
       LDAP_USE_TLS: "false"
       REDIS_URL: redis://redis:6379/0
+      ACTIVE_STORAGE_PATH: "/akkounts/tmp/attachments"
       RS_REDIS_URL: redis://redis:6379/1
       RS_STORAGE_URL: "http://localhost:4567"
+      S3_ENABLED: false
     depends_on:
       - ldap
       - redis
@@ -67,6 +69,7 @@ services:
       REDIS_URL: redis://redis:6379/0
       RS_REDIS_URL: redis://redis:6379/1
       RS_STORAGE_URL: "http://localhost:4567"
+      S3_ENABLED: false
     depends_on:
       - ldap
       - redis

spec/components/app_catalog/web_app_icon_component_spec.rb

@@ -0,0 +1,11 @@
+require "rails_helper"
+
+RSpec.describe AppCatalog::WebAppIconComponent, type: :component do
+  describe "No web app given" do
+    it "renders the default icon" do
+      expect(
+        render_inline(described_class.new(web_app: nil)) {}.to_html
+      ).to include("icon-remotestorage")
+    end
+  end
+end