Use ActiveStorage for attachments, add attachment size limit

.github/workflows/rubyonrails.yml

@@ -56,7 +56,7 @@ jobs:
           cp config/school.yml.template config/school.yml
           cp config/autogradeConfig.rb.template config/autogradeConfig.rb
           cp .env.template .env
-          mkdir attachments/ tmp/
+          mkdir tmp/
 
       - name: Set up database
         run: |

.gitignore

@@ -23,6 +23,7 @@ config/lti_platform_jwk.json
 app/views/home/_topannounce.html.erb
 attachments/
 doc/
+storage/
 out.txt
 
 # compiled assets

README.md

@@ -51,7 +51,7 @@ We released new documentation! Check it out [here](https://docs.autolabproject.c
 3. Create necessary directories.
 
 	```
-	mkdir attachments/ tmp/
+	mkdir tmp/
 	```
 
 ### Running Tests

app/controllers/attachments_controller.rb

@@ -33,7 +33,7 @@ def create
 
     if @attachment.update(attachment_params)
       flash[:success] = "Attachment created"
-      redirect_to_attachment_list
+      redirect_to_index
     else
       error_msg = "Attachment create failed:"
       if !@attachment.valid?
@@ -57,23 +57,33 @@ def create
 
   action_auth_level :show, :student
   def show
-    filename = Rails.root.join("attachments", @attachment.filename)
-    unless File.exist?(filename)
-      COURSE_LOGGER.log("Cannot find the file '#{@attachment.filename}' for"\
-                        " attachment #{@attachment.name}")
-
-      flash[:error] = "Error loading #{@attachment.name} from #{@attachment.filename}"
-      redirect_to([@course, :attachments]) && return
-    end
     if @cud.instructor? || @attachment.released?
-      # Set to application/octet-stream to force download
-      send_file(filename, disposition: "inline",
-                          type: "application/octet-stream",
-                          filename: @attachment.filename) && return
+      begin
+        attached_file = @attachment.attachment_file
+        if attached_file.attached?
+          send_data attached_file.download, filename: @attachment.filename,
+                                            type: @attachment.mime_type
+          return
+        end
+
+        old_attachment_path = Rails.root.join("attachments", @attachment.filename)
+        if File.exist?(old_attachment_path)
+          send_file old_attachment_path, filename: @attachment.filename, type: @attachment.mime_type
+          return
+        else
+          COURSE_LOGGER.log("No file attached to attachment '#{@attachment.name}'")
+          flash[:error] = "No file attached to attachment '#{@attachment.name}'"
+          redirect_to_index && return
+        end
+      rescue StandardError
+        COURSE_LOGGER.log("Error viewing attachment '#{@attachment.name}'")
+        flash[:error] = "Error viewing attachment '#{@attachment.name}'"
+        redirect_to_index && return
+      end
     end
 
     flash[:error] = "You are unauthorized to view this attachment"
-    redirect_to([@course, @assessment])
+    redirect_to_index
   end
 
   action_auth_level :edit, :instructor
@@ -83,7 +93,7 @@ def edit; end
   def update
     if @attachment.update(attachment_params)
       flash[:success] = "Attachment updated"
-      redirect_to_attachment_list
+      redirect_to_index
     else
       error_msg = "Attachment update failed:"
       if !@attachment.valid?
@@ -109,7 +119,7 @@ def update
   def destroy
     @attachment.destroy
     flash[:success] = "Attachment deleted"
-    redirect_to_attachment_list
+    redirect_to_index
   end
 
 private
@@ -129,10 +139,10 @@ def set_attachment
 
     COURSE_LOGGER.log("Cannot find attachment with id: #{params[:id]}")
     flash[:error] = "Could not find Attachment \##{params[:id]}"
-    redirect_to_attachment_list
+    redirect_to_index
   end
 
-  def redirect_to_attachment_list
+  def redirect_to_index
     if @is_assessment
       redirect_to course_assessment_path(@course, @assessment)
     else
@@ -142,10 +152,10 @@ def redirect_to_attachment_list
 
   def add_attachments_breadcrumb
     @breadcrumbs << if @is_assessment
-                      (view_context.link_to "Assessment Attachments",
-                                            [@course, @assessment, :attachments])
+                      view_context.link_to "Assessment Attachments",
+                                           course_assessment_attachments_path(@course, @assessment)
                     else
-                      (view_context.link_to "Course Attachments", [@course, :attachments])
+                      view_context.link_to "Course Attachments", course_attachments_path(@course)
                     end
   end
 

app/models/attachment.rb

@@ -9,10 +9,18 @@ class Attachment < ApplicationRecord
   validates :category_name, presence: true
   validates :filename, presence: true
   validates :release_at, presence: true
+  validate :file_size_limit
+  has_one_attached :attachment_file
 
   belongs_to :course
   belongs_to :assessment
 
+  def file_size_limit
+    return unless attachment_file.attached? && attachment_file.byte_size > 1.gigabyte
+
+    errors.add(:attachment_file, "must be less than 1GB")
+  end
+
   # Constants
   ORDERING = "release_at ASC, name ASC".freeze
 
@@ -30,21 +38,8 @@ def released?
   end
 
   def file=(upload)
-    directory = "attachments"
-    filename = File.basename(upload.original_filename)
-    dir_path = Rails.root.join(directory)
-    FileUtils.mkdir_p(dir_path) unless File.exist?(dir_path)
-
-    path = Rails.root.join(directory, filename)
-    addendum = 1
-
-    # Deal with duplicate filenames on disk
-    while File.exist?(path)
-      path = Rails.root.join(directory, "#{filename}.#{addendum}")
-      addendum += 1
-    end
-    self.filename = File.basename(path)
-    File.open(path, "wb") { |f| f.write(upload.read) }
+    self.filename = File.basename(upload.original_filename)
+    attachment_file.attach(upload)
     self.mime_type = upload.content_type
   end
 

app/views/attachments/_form.html.erb

@@ -32,7 +32,7 @@
     <% end %>
     <li>
       <% if @attachment.new_record? %>
-        <%= f.file_field :file, button_text: "Upload Attachment" %>
+        <%= f.file_field :file, button_text: "Upload Attachment", help_text: "Max Size: 1GB" %>
       <% else %>
         <%= f.text_field :mime_type,
                          help_text: "Note: If you change the Mime type of a file, you might need to clear your browser's cache in order for you to see the change." %>

config/environments/development.rb

@@ -61,7 +61,7 @@
   # config.file_watcher = ActiveSupport::EventedFileUpdateChecker
 
   # OAuth2 Application Configuration for Github
-  # See (TODO replace with doc link)
+  # See https://docs.autolabproject.com/installation/github_integration/
   config.x.github.client_id = ENV['GITHUB_CLIENT_ID']
   config.x.github.client_secret = ENV['GITHUB_CLIENT_SECRET']
 

config/environments/production.rb.template

@@ -55,6 +55,9 @@ Autolab3::Application.configure do
   # Enable serving of images, stylesheets, and JavaScripts from an asset server
   # config.action_controller.asset_host = "http://assets.example.com"
 
+  # Store uploaded files on the local file system (see config/storage.yml for options).
+  config.active_storage.service = :local
+
   # Disable delivery errors, bad email addresses will be ignored
   # config.action_mailer.raise_delivery_errors = false
 
@@ -113,7 +116,7 @@ Autolab3::Application.configure do
   config.x.google_analytics_id = nil
 
   # OAuth2 Application Configuration for Github
-  # See (TODO replace with doc link)
+  # See https://docs.autolabproject.com/installation/github_integration/
   config.x.github.client_id = ENV['GITHUB_CLIENT_ID']
   config.x.github.client_secret = ENV['GITHUB_CLIENT_SECRET']
   # LTI configuration file storage location. Needs to be in a private folder

config/environments/test.rb

@@ -34,6 +34,9 @@
   # Store uploaded files on the local file system in a temporary directory.
   config.active_storage.service = :test
 
+  # Perform ActiveStorage jobs immediately.
+  config.active_job.queue_adapter = :inline
+
   # Don't care if the mailer can't send.
   config.action_mailer.raise_delivery_errors = false
 

db/migrate/20231208034215_create_active_storage_tables.active_storage.rb

@@ -0,0 +1,27 @@
+# This migration comes from active_storage (originally 20170806125915)
+class CreateActiveStorageTables < ActiveRecord::Migration[5.2]
+  def change
+    create_table :active_storage_blobs do |t|
+      t.string   :key,        null: false
+      t.string   :filename,   null: false
+      t.string   :content_type
+      t.text     :metadata
+      t.bigint   :byte_size,  null: false
+      t.string   :checksum,   null: false
+      t.datetime :created_at, null: false
+
+      t.index [ :key ], unique: true
+    end
+
+    create_table :active_storage_attachments do |t|
+      t.string     :name,     null: false
+      t.references :record,   null: false, polymorphic: true, index: false
+      t.references :blob,     null: false
+
+      t.datetime :created_at, null: false
+
+      t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true
+      t.foreign_key :active_storage_blobs, column: :blob_id
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,28 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2023_09_24_163059) do
+ActiveRecord::Schema.define(version: 2023_12_08_034215) do
+
+  create_table "active_storage_attachments", force: :cascade do |t|
+    t.string "name", null: false
+    t.string "record_type", null: false
+    t.bigint "record_id", null: false
+    t.bigint "blob_id", null: false
+    t.datetime "created_at", null: false
+    t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id"
+    t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true
+  end
+
+  create_table "active_storage_blobs", force: :cascade do |t|
+    t.string "key", null: false
+    t.string "filename", null: false
+    t.string "content_type"
+    t.text "metadata"
+    t.bigint "byte_size", null: false
+    t.string "checksum", null: false
+    t.datetime "created_at", null: false
+    t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
+  end
 
   create_table "annotations", force: :cascade do |t|
     t.integer "submission_id"