Merge pull request #11661 from mkllnk/report-file-storage

Store report files on the local disk

app/controllers/admin/reports_controller.rb

@@ -80,11 +80,10 @@ module Admin
           block: "start"
         ).broadcast
 
-      blob = ReportBlob.create_for_upload_later!(report_filename)
-
       ReportJob.perform_later(
         report_class:, user: spree_current_user, params:,
-        format:, blob:, channel: ScopedChannel.for_id(params[:uuid]),
+        format:, filename: report_filename,
+        channel: ScopedChannel.for_id(params[:uuid]),
       )
 
       head :no_content

app/jobs/application_job.rb

@@ -12,6 +12,6 @@ class ApplicationJob < ActiveJob::Base
 
   def enable_active_storage_urls
     ActiveStorage::Current.url_options ||=
-      Rails.application.config.action_controller.default_url_options
+      Rails.application.routes.default_url_options
   end
 end

app/jobs/report_job.rb

@@ -9,12 +9,12 @@ class ReportJob < ApplicationJob
 
   NOTIFICATION_TIME = 5.seconds
 
-  def perform(report_class:, user:, params:, format:, blob:, channel: nil)
+  def perform(report_class:, user:, params:, format:, filename:, channel: nil)
     start_time = Time.zone.now
 
     report = report_class.new(user, params, render: true)
     result = report.render_as(format)
-    blob.store(result)
+    blob = ReportBlob.create!(filename, result)
 
     execution_time = Time.zone.now - start_time
 

app/models/report_blob.rb

@@ -5,34 +5,20 @@ class ReportBlob < ActiveStorage::Blob
   # AWS S3 limits URL expiry to one week.
   LIFETIME = 1.week
 
-  def self.create_for_upload_later!(filename)
-    # ActiveStorage discourages modifying a blob later but we need a blob
-    # before we know anything about the report file. It enables us to use the
-    # same blob in the controller to read the result.
-    create_before_direct_upload!(
+  def self.create!(filename, content)
+    create_and_upload!(
+      io: StringIO.new(content),
       filename:,
-      byte_size: 0,
-      checksum: "0",
       content_type: content_type(filename),
-    ).tap do |blob|
-      ActiveStorage::PurgeJob.set(wait: LIFETIME).perform_later(blob)
-    end
+      identify: false,
+      service_name: :local,
+    )
   end
 
   def self.content_type(filename)
     MIME::Types.of(filename).first&.to_s || "application/octet-stream"
   end
 
-  def store(content)
-    io = StringIO.new(content)
-    upload(io, identify: false)
-    save!
-  end
-
-  def content_stored?
-    @content_stored ||= reload.checksum != "0"
-  end
-
   def result
     @result ||= download.force_encoding(Encoding::UTF_8)
   end

config/environments/test.rb

@@ -56,6 +56,4 @@ Openfoodnetwork::Application.configure do
   config.active_support.deprecation = :stderr
 
   config.active_job.queue_adapter = :test
-
-  config.active_storage.service = :test
 end

config/storage.yml

@@ -1,10 +1,6 @@
 local:
   service: Disk
-  root: <%= Rails.root.join("storage") %>
-
-test:
-  service: Disk
-  root: <%= Rails.root.join("tmp/storage") %>
+  root: <%= Rails.root.join(Rails.env.test? ? "tmp/storage" : "storage") %>
 
 test_amazon:
   service: S3

spec/jobs/report_job_spec.rb

@@ -3,16 +3,17 @@
 require 'spec_helper'
 
 describe ReportJob do
+  include CableReady::Broadcaster
+
   let(:report_args) {
-    { report_class:, user:, params:, format:,
-      blob: }
+    { report_class:, user:, params:, format:, filename: }
   }
   let(:report_class) { Reporting::Reports::UsersAndEnterprises::Base }
   let(:user) { enterprise.owner }
   let(:enterprise) { create(:enterprise) }
   let(:params) { {} }
   let(:format) { :csv }
-  let(:blob) { ReportBlob.create_for_upload_later!("report.csv") }
+  let(:filename) { "report.csv" }
 
   it "generates a report" do
     job = perform_enqueued_jobs(only: ReportJob) do
@@ -22,15 +23,30 @@ describe ReportJob do
   end
 
   it "enqueues a job for async processing" do
-    job = ReportJob.perform_later(**report_args)
-    expect(blob.content_stored?).to eq false
+    expect {
+      ReportJob.perform_later(**report_args)
+    }.to_not change { ActiveStorage::Blob.count }
 
-    perform_enqueued_jobs(only: ReportJob)
+    expect {
+      perform_enqueued_jobs(only: ReportJob)
+    }.to change { ActiveStorage::Blob.count }
 
-    expect(blob.content_stored?).to eq true
     expect_csv_report
   end
 
+  it "notifies Cable Ready when the report is done" do
+    channel = ScopedChannel.for_id("123")
+    with_channel = report_args.merge(channel:)
+
+    ReportJob.perform_later(**with_channel)
+
+    expect(cable_ready[channel]).to receive(:broadcast).and_call_original
+
+    expect {
+      perform_enqueued_jobs(only: ReportJob)
+    }.to change { ActiveStorage::Blob.count }
+  end
+
   it "triggers an email when the report is done" do
     # Setup test data which also triggers emails:
     report_args
@@ -44,10 +60,9 @@ describe ReportJob do
       ReportJob.perform_later(**report_args)
       perform_enqueued_jobs(only: ReportJob)
     }.to enqueue_mail(ReportMailer, :report_ready).with(
-      params: {
+      params: hash_including(
         to: user.email,
-        blob:,
-      },
+      ),
       args: [],
     )
   end
@@ -76,7 +91,7 @@ describe ReportJob do
   end
 
   def expect_csv_report
-    blob.reload
+    blob = ReportBlob.last
     expect(blob.filename.to_s).to eq "report.csv"
     expect(blob.content_type).to eq "text/csv"
 

spec/mailers/report_mailer_spec.rb

@@ -10,7 +10,7 @@ describe ReportMailer do
         blob:,
       ).report_ready
     }
-    let(:blob) { ReportBlob.create_for_upload_later!("customers.csv") }
+    let(:blob) { ReportBlob.create!("customers.csv", "report content") }
 
     it "notifies about a report" do
       expect(email.subject).to eq "Report ready"

spec/models/report_blob_spec.rb

@@ -4,11 +4,10 @@ require 'spec_helper'
 
 describe ReportBlob, type: :model do
   it "preserves UTF-8 content" do
-    blob = ReportBlob.create_for_upload_later!("customers.html")
     content = "This works. ✓"
 
     expect do
-      blob.store(content)
+      blob = ReportBlob.create!("customers.html", content)
       content = blob.result
     end.to_not change { content.encoding }.from(Encoding::UTF_8)
   end

spec/system/admin/reports_spec.rb

@@ -80,14 +80,13 @@ describe '
       email = ActionMailer::Base.deliveries.last
       expect(email.body).to have_link(
         "customers",
-        href: %r"^http://test\.host/rails/active_storage/disk/.*/customers_[0-9]+\.html$"
+        href: %r"^http://.*/rails/active_storage/disk/.*/customers_[0-9]+\.html$"
       )
 
       # ActiveStorage links usually expire after 5 minutes.
       # But we want a longer expiry in emailed links.
       parsed_email = Capybara::Node::Simple.new(email.body.to_s)
-      email_link_href = parsed_email.find(:link, "customers")[:href]
-      report_link = email_link_href.sub("test.host", Rails.application.default_url_options[:host])
+      report_link = parsed_email.find(:link, "customers")[:href]
       content = URI.parse(report_link).read
       expect(content).to match "<th>\nFirst Name\n</th>"