From bf930828e9175f03d44ff7f3a74ab2cb53283181 Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 17:57:02 +0100 Subject: [PATCH 01/52] Add basic file upload setup This commit adds the attachment module with some convenience methods and basic setup, storage with memory and file store classes, and an uploader base class. --- src/lucky/attachment.cr | 106 ++++++++ src/lucky/attachment/storage.cr | 62 +++++ src/lucky/attachment/storage/file_system.cr | 126 +++++++++ src/lucky/attachment/storage/memory.cr | 60 +++++ src/lucky/attachment/uploaded_file.cr | 275 ++++++++++++++++++++ src/lucky/attachment/uploader.cr | 181 +++++++++++++ 6 files changed, 810 insertions(+) create mode 100644 src/lucky/attachment.cr create mode 100644 src/lucky/attachment/storage.cr create mode 100644 src/lucky/attachment/storage/file_system.cr create mode 100644 src/lucky/attachment/storage/memory.cr create mode 100644 src/lucky/attachment/uploaded_file.cr create mode 100644 src/lucky/attachment/uploader.cr diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr new file mode 100644 index 000000000..da6e0d258 --- /dev/null +++ b/src/lucky/attachment.cr @@ -0,0 +1,106 @@ +require "habitat" +require "./attachment/uploaded_file" +require "./attachment/storage" +require "./attachment/storage/memory" +require "./attachment/storage/file_system" +require "./attachment/uploader" + +module Lucky::Attachment + Log = ::Log.for("lucky.attachment") + + Habitat.create do + # Storage configurations keyed by name ("cache", "store", etc.) + setting storages : Hash(String, Storage::Base) = {} of String => Storage::Base + end + + # Retrieves a storage by name, raising if not found. + # + # ``` + # Lucky::Attachment.find_storage("store") # => Storage::FileSystem + # Lucky::Attachment.find_storage("missing") # raises Lucky::Attachment::Error + # ``` + def self.find_storage(name : String) : Storage::Base + settings.storages[name]? || + raise Error.new( + "Storage #{name.inspect} is not registered." \ + "Available storages: #{settings.storages.keys.inspect}" + ) + end + + # Move a file from one storage to another (typically cache -> store). + # + # ``` + # stored = Lucky::Attachment.promote(cached, to: "store") + # ``` + def self.promote( + file : UploadedFile, + to storage : String, + delete_source : Bool = true, + ) : UploadedFile + file.open do |io| + find_storage(storage).upload(io, file.id, metadata: file.metadata) + promoted = UploadedFile.new( + id: file.id, + storage_key: storage, + metadata: file.metadata + ) + file.delete if delete_source + promoted + end + end + + # Deserialize an UploadedFile from various sources. + # + # ``` + # Lucky::Attachment.uploaded_file(json_string) + # Lucky::Attachment.uploaded_file(json_any) + # Lucky::Attachment.uploaded_file(uploaded_file) + # ``` + def self.uploaded_file(json : String) : UploadedFile + UploadedFile.from_json(json) + end + + def self.uploaded_file(json : JSON::Any) : UploadedFile + UploadedFile.from_json(json.to_json) + end + + def self.uploaded_file(file : UploadedFile) : UploadedFile + file + end + + def self.uploaded_file(value : Nil) : Nil + nil + end + + # Utility to work with a file IO. If the IO is already a File, yields it + # directly, Otherwise copies to a tempfile, yields it, then cleans up. + # + # ``` + # Lucky::Attachment.with_file(io) do |file| + # # file is guaranteed to be a File with a path + # end + # ``` + def self.with_file(io : IO, &) + if io.is_a?(File) + yield io + else + File.tempfile("lucky-attachment") do |tempfile| + IO.copy(io, tempfile) + tempfile.rewind + yield tempfile + end + end + end + + def self.with_file(uploaded_file : UploadedFile, &) + uploaded_file.download do |tempfile| + yield tempfile + end + end + + class Error < Exception; end + + class FileNotFound < Error; end + + class InvalidFile < Error; end +end diff --git a/src/lucky/attachment/storage.cr b/src/lucky/attachment/storage.cr new file mode 100644 index 000000000..7e5bab461 --- /dev/null +++ b/src/lucky/attachment/storage.cr @@ -0,0 +1,62 @@ +module Lucky::Attachment + # Storage backends handle the actual persistence of uploaded files. + # Implementations must provide methods for uploading, retreiving, checking + # existence, and deleting files. + # + abstract class Storage::Base + # Uploads an IO to the given location (id) in the storage. + # + # ``` + # storage.upload(io, "uploads/photo.jpg") + # storage.upload(io, "uploads/photo.jpg", metadata: {"filename" => "original.jpg"}) + # ``` + # + abstract def upload(io : IO, id : String, **options) : Nil + + # Opens the file at the given location and returns an IO for reading. + # + # ``` + # io = storage.open("uploads/photo.jpg") + # content = io.gets_to_end + # io.close + # ``` + # + # Raises `Lucky::Attachment::FileNotFound` if the file doesn't exist. + # + abstract def open(id : String, **options) : IO + + # Returns whether a file exists at the given location. + # + # ``` + # storage.exists?("uploads/photo.jpg") + # # => true + # ``` + # + abstract def exists?(id : String) : Bool + + # Returns the URL for accessing the file at the given location. + # + # ``` + # storage.url("uploads/photo.jpg") + # # => "/uploads/photo.jpg" + # storage.url("uploads/photo.jpg", host: "https://example.com") + # ``` + # + abstract def url(id : String, **options) : String + + # Deletes the file at the given location. + # + # ``` + # storage.delete("uploads/photo.jpg") + # ``` + # + # Does not raise if the file doesn't exist. + # + abstract def delete(id : String) : Nil + + # Moves a file from another location. + def move(io : IO, id : String, **options) : Nil + upload(io, id, **options) + end + end +end diff --git a/src/lucky/attachment/storage/file_system.cr b/src/lucky/attachment/storage/file_system.cr new file mode 100644 index 000000000..80c22868d --- /dev/null +++ b/src/lucky/attachment/storage/file_system.cr @@ -0,0 +1,126 @@ +require "../storage" + +module Lucky::Attachment + # Local filesystem storage backend. Files are stored in a directory on the + # local filesystem. Supports an optional prefix for organizing files. + # + # ``` + # Lucky::Attachment.configure do |settings| + # settings.storages["cache"] = Lucky::Attachment::Storage::FileSystem.new( + # directory: "uploads", + # prefix: "cache" + # ) + # settings.storages["store"] = Lucky::Attachment::Storage::FileSystem.new( + # directory: "uploads" + # ) + # end + # ``` + # + class Storage::FileSystem < Storage::Base + DEFAULT_PERMISSIONS = File::Permissions.new(0o644) + DEFAULT_DIRECTORY_PERMISSIONS = File::Permissions.new(0o755) + + getter directory : String + getter prefix : String? + getter? clean : Bool + getter permissions : File::Permissions + getter directory_permissions : File::Permissions + + def initialize( + @directory : String, + @prefix : String? = nil, + @clean : Bool = true, + @permissions : File::Permissions = DEFAULT_PERMISSIONS, + @directory_permissions : File::Permissions = DEFAULT_DIRECTORY_PERMISSIONS, + ) + Dir.mkdir_p(expanded_directory, mode: directory_permissions.value) + end + + # Returns the full expanded path including prefix. + # + # ``` + # storage.expanded_directory + # # => "/app/uploads/cache" + # ``` + # + def expanded_directory : String + return File.expand_path(directory) unless p = prefix + + File.expand_path(File.join(directory, p)) + end + + def upload(io : IO, id : String, move : Bool = false, **options) : Nil + path = path_for(id) + Dir.mkdir_p(File.dirname(path), mode: directory_permissions.value) + + if move && io.is_a?(File) + File.rename(io.path, path) + File.chmod(path, permissions) + else + File.open(path, "wb", perm: permissions) do |file| + IO.copy(io, file) + end + end + end + + def open(id : String, **options) : IO + File.open(path_for(id), "rb") + rescue ex : File::NotFoundError + raise FileNotFound.new("File not found: #{id}") + end + + def exists?(id : String) : Bool + File.exists?(path_for(id)) + end + + # Returns the full filesystem path for the given id. + # + # ``` + # storage.path_for("abc123.jpg") + # # => "/app/uploads/abc123.jpg" + # ``` + # + def path_for(id : String) : String + File.join(expanded_directory, id.gsub('/', File::SEPARATOR)) + end + + def url(id : String, host : String? = nil, **options) : String + String.build do |url| + url << host.rstrip('/') if host + url << '/' + url << prefix.lstrip('/') << '/' if prefix + url << id + end + end + + def delete(id : String) : Nil + path = path_for(id) + File.delete?(path) + clean_directories(path) if clean? + rescue ex : File::Error + # Ignore errors here + end + + # Override move for efficient file system rename + def move(io : IO, id : String, **options) : Nil + if io.is_a?(File) + upload(io, id, **options, move: true) + else + upload(io, id, **options) + end + end + + # Cleans empty parent directories up to the expanded_directory. + private def clean_directories(path : String) : Nil + current = File.dirname(path) + + while current != expanded_directory && current.starts_with?(expanded_directory) + break unless Dir.empty?(current) + Dir.delete(current) + current = File.dirname(current) + end + rescue ex : File::Error + # Ignore errors here + end + end +end diff --git a/src/lucky/attachment/storage/memory.cr b/src/lucky/attachment/storage/memory.cr new file mode 100644 index 000000000..12676922d --- /dev/null +++ b/src/lucky/attachment/storage/memory.cr @@ -0,0 +1,60 @@ +require "../storage" + +module Lucky::Attachment + # In-memory storage backend for testing purposes. Files are stored in a hash + # and are lost when the process exits. This is useful for testing without + # hitting the filesystem or network. + # + # ``` + # Lucky::Attachment.configure do |settings| + # settings.storages["cache"] = Lucky::Attachment::Storage::Memory.new + # settings.storages["store"] = Lucky::Attachment::Storage::Memory.new + # end + # ``` + # + class Storage::Memory < Storage::Base + getter store : Hash(String, Bytes) + getter base_url : String? + + def initialize(@base_url : String? = nil) + @store = {} of String => Bytes + end + + def upload(io : IO, id : String, **options) : Nil + @store[id] = io.getb_to_end + end + + def open(id : String, **options) : IO + if bytes = @store[id]? + IO::Memory.new(bytes) + else + raise FileNotFound.new("File not found: #{id}") + end + end + + def exists?(id : String) : Bool + @store.has_key?(id) + end + + def url(id : String, **options) : String + if base = @base_url + "#{base.rstrip('/')}/#{id}" + else + "/#{id}" + end + end + + def delete(id : String) : Nil + @store.delete(id) + end + + def clear! : Nil + @store.clear + end + + # Returns the number of stored files. + def size : Int32 + @store.size + end + end +end diff --git a/src/lucky/attachment/uploaded_file.cr b/src/lucky/attachment/uploaded_file.cr new file mode 100644 index 000000000..167176d9d --- /dev/null +++ b/src/lucky/attachment/uploaded_file.cr @@ -0,0 +1,275 @@ +require "json" +require "uuid" + +module Lucky::Attachment + alias MetadataValue = String | Int32 | Int64 | UInt32 | UInt64 | Float64 | Bool | Nil + alias MetadataHash = Hash(String, MetadataValue) + + # Represents a file that has been uploaded to a storage backend. + # + # This class is JSON serializable and stores the file's location (`id`), + # which storage it's in (`storage`), and associated metadata. + # + # NOTE: The JSON format is compatible with Shrine.rb/Shrine.cr: + # + # ```json + # { + # "id": "uploads/abc123.jpg", + # "storage": "store", + # "metadata": { + # "filename": "photo.jpg", + # "size": 102400, + # "mime_type": "image/jpeg" + # } + # } + # ``` + # + class UploadedFile + include JSON::Serializable + + getter id : String + + @[JSON::Field(key: "storage")] + getter storage_key : String + + getter metadata : MetadataHash + + @[JSON::Field(ignore: true)] + @io : IO? + + def initialize( + @id : String, + @storage_key : String, + @metadata : MetadataHash = MetadataHash.new, + ) + end + + # Returns the original filename from metadata. + # + # ``` + # file.original_filename + # # => "photo.jpg" + # ``` + # + def original_filename : String? + metadata["filename"]?.try(&.as(String)) + end + + # Returns the file extension based on the id or original filename. + # + # ``` + # file.extension + # # => "jpg" + # ``` + # + def extension : String? + ext = File.extname(id).lchop('.') + if ext.empty? && original_filename + ext = File.extname(original_filename.to_s).lchop('.') + end + ext.presence.try(&.downcase) + end + + # Returns the file size in bytes from metadata. + # + # ``` + # file.size + # # => 102400 + # ``` + # + def size : Int64? + case value = metadata["size"]? + when Int32 then value.to_i64 + when Int64 then value + when String then value.to_i64? + else nil + end + end + + # Returns the MIME type from metadata. + # + # ``` + # file.mime_type + # # => "image/jpeg" + # ``` + # + def mime_type : String? + metadata["mime_type"]?.try(&.as(String)) + end + + # Access arbitrary metadata values. + # + # ``` + # file["width"] + # # => 800 + # file["custom"] + # # => "value" + # ``` + # + def [](key : String) : MetadataValue + metadata[key]? + end + + # Returns the storage instance this file is stored in. + def storage : Storage::Base + Lucky::Attachment.find_storage(storage_key) + end + + # Returns the URL for accessing this file. + # + # ``` + # file.url + # # => "https://bucket.s3.amazonaws.com/uploads/abc123.jpg" + # + # # for presigned URLs + # file.url(expires_in: 1.hour) + # ``` + # + def url(**options) : String + storage.url(id, **options) + end + + # Returns whether this file exists in storage. + # + # ``` + # file.exists? # => true + # ``` + # + def exists? : Bool + storage.exists?(id) + end + + # Opens the file for reading. If a block is given, yields the IO and + # automatically closes it afterwards. Returns the block's return value. + # + # ``` + # file.open do |io| + # io.gets_to_end + # end + # ``` + # + def open(**options, &) + io = storage.open(id, **options) + begin + yield io + ensure + io.close + end + end + + # Opens the file and stores the IO handle internally for subsequent reads. + # Remember to call `close` when done. + # + # ``` + # file.open + # content = file.io.gets_to_end + # file.close + # ``` + def open(**options) : IO + close if @io + @io = storage.open(id, **options) + end + + # Returns the currently opened IO, or opens it if not already open. + def io : IO + @io || open + end + + # Closes the file if it is open. + def close : Nil + @io.try(&.close) + @io = nil + end + + # Tests whether the file has been opened or not. + def opened? : Bool + !@io.nil? + end + + # Downloads the file to a temporary file and returns it. As opposed to the + # block variant, this temporary file needs to be closed and deleted + # manually: + # + # ``` + # tempfile = file.download + # tempfile.path + # # => "/tmp/lucky-attachment123456789.jpg" + # tempfile.gets_to_end + # # => "file content" + # tempfile.close + # tempfile.delete + # ``` + # + def download(**options) : File + tempfile = File.tempfile("lucky-attachment", ".#{extension}") + stream(tempfile, **options) + tempfile.rewind + tempfile + end + + # Downloads to a tempfile, yields it to the block, then cleans up. + # + # ``` + # file.download do |tempfile| + # process(tempfile.path) + # end + # # tempfile is automatically deleted + # ``` + # + def download(**options, &) + tempfile = download(**options) + begin + yield tempfile + ensure + tempfile.close + tempfile.delete + end + end + + # Streams the file content to the given IO destination. + # + # ``` + # file.stream(response.output) + # ``` + # + def stream(destination : IO, **options) : Nil + if opened? + IO.copy(io, destination) + io.rewind if io.responds_to?(:rewind) + else + open(**options) do |io| + IO.copy(io, destination) + end + end + end + + # Deletes the file from storage. + # + # ``` + # file.delete + # ``` + # + def delete : Nil + storage.delete(id) + end + + # Returns a hash representation suitable for JSON serialization compatible + # with Shrine. + def data : Hash(String, String | MetadataHash) + { + "id" => id, + "metadata" => metadata, + "storage" => storage_key, + } + end + + # Compares two `UploadedFiles` by thier id and storage. + def ==(other : UploadedFile) : Bool + id == other.id && storage_key == other.storage_key + end + + def ==(other) : Bool + false + end + end +end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr new file mode 100644 index 000000000..3997977c8 --- /dev/null +++ b/src/lucky/attachment/uploader.cr @@ -0,0 +1,181 @@ +require "uuid" + +module Lucky::Attachment + # Base uploader class that handles file uploads with metadata extraction and + # location generation. + # + # ``` + # struct ImageUploader < Lucky::Attachment::Uploader + # def generate_location(io, metadata, **options) : String + # date = Time.utc.to_s("%Y/%m/%d") + # File.join("images", date, super) + # end + # end + # + # ImageUploader.new("store").upload(io) + # # => UploadedFile with id "images/2024/01/15/abc123.jpg" + # ``` + # + abstract struct Uploader + getter storage_key : String + + def initialize(@storage_key : String) + end + + # Returns the storage instance for this uploader. + def storage : Storage::Base + Lucky::Attachment.find_storage(storage_key) + end + + # Uploads a file and returns an UploadedFile. This method accepts + # additional metadata and arbitrary arguments for overrides. + # + # ``` + # uploader.upload(io) + # uploader.upload(io, metadata: {"custom" => "value"}) + # uploader.upload(io, location: "custom/path.jpg") + # ``` + # + def upload(io : IO, metadata : MetadataHash? = nil, **options) : UploadedFile + data = extract_metadata(io, metadata, **options) + data = data.merge(metadata) if metadata + location = options[:location]? || generate_location(io, data, **options) + + storage.upload(io, location, **options.merge(metadata: data)) + UploadedFile.new(id: location, storage_key: storage_key, metadata: data) + ensure + io.close if options[:close]?.nil? || options[:close]? + end + + # Uploads to the "cache" storage. + # + # ``` + # cached = ImageUploader.cache(io) + # ``` + def self.cache(io : IO, **options) : UploadedFile + new("cache").upload(io, **options) + end + + # Uploads to the "store" storage. + # + # ``` + # stored = ImageUploader.store(io) + # ``` + # + def self.store(io : IO, **options) : UploadedFile + new("store").upload(io, **options) + end + + # Promotes a file from cache to store. + # + # ``` + # cached = ImageUploader.cache(io) + # stored = ImageUploader.promote(cached) + # ``` + # + def self.promote( + file : UploadedFile, + to storage : String = "store", + delete_source : Bool = true, + **options, + ) : UploadedFile + Lucky::Attachment.promote( + file, + **options, + to: storage, + delete_source: delete_source + ) + end + + # Generates a unique location for the uploaded file. Override this in + # subclasses for custom locations. + # + # ``` + # class ImageUploader < Lucky::Attachment::Uploader + # def generate_location(io, metadata, **options) : String + # File.join("images", super) + # end + # end + # ``` + # + def generate_location(io : IO, metadata : MetadataHash) : String + extension = extract_extension(io, metadata) + basename = generate_uid + extension ? "#{basename}.#{extension}" : basename + end + + # Extracts metadata from the IO. Override in subclasses to add custom + # metadata extraction. + # + # ``` + # class ImageUploader < Lucky::Attachment::Uploader + # def extract_metadata(io, metadata : MetadataHash? = nil, **options) : MetadataHash + # data = super + # # Add custom metadata + # data["custom"] = "value" + # data + # end + # end + # ``` + # + def extract_metadata( + io : IO, + metadata : MetadataHash? = nil, + **options, + ) : MetadataHash + MetadataHash{ + "filename" => extract_filename(io), + "size" => extract_size(io), + "mime_type" => extract_mime_type(io), + } + end + + # Generates a unique identifier for file locations. + protected def generate_uid : String + UUID.random.to_s + end + + # Extracts the filename from the IO if available. + protected def extract_filename(io : IO) : String? + if io.responds_to?(:original_filename) + io.original_filename + elsif io.responds_to?(:filename) + io.filename + elsif io.responds_to?(:path) + File.basename(io.path) + end + end + + # Extracts the file size from the IO, if available. + protected def extract_size(io : IO) : Int64? + io.size.to_i64 if io.responds_to?(:size) + end + + # Extracts the MIME type from the IO if available. + # + # NOTE: This relies on the IO providing content_type, which typically comes + # from HTTP headers and may not be accurate. + # + protected def extract_mime_type(io : IO) : String? + return unless io.responds_to?(:content_type) && (type = io.content_type) + + type.split(';').first.strip + end + + # Extracts file extension from the IO or metadata. + protected def extract_extension( + io : IO, + metadata : MetadataHash, + ) : String? + if filename = metadata["filename"]?.try(&.as(String)) + ext = File.extname(filename).lchop('.') + return ext.downcase unless ext.empty? + end + + if io.responds_to?(:path) + ext = File.extname(io.path).lchop('.') + return ext.downcase unless ext.empty? + end + end + end +end From d0ead845330f49b2b296218c6538d2ca4c534333 Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 17:59:41 +0100 Subject: [PATCH 02/52] Add specs for uploader and memory store --- spec/lucky/attachment/storage/memory_spec.cr | 55 +++++++++++++++++ spec/lucky/attachment/uploaded_file_spec.cr | 59 ++++++++++++++++++ spec/lucky/attachment/uploader_spec.cr | 65 ++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 spec/lucky/attachment/storage/memory_spec.cr create mode 100644 spec/lucky/attachment/uploaded_file_spec.cr create mode 100644 spec/lucky/attachment/uploader_spec.cr diff --git a/spec/lucky/attachment/storage/memory_spec.cr b/spec/lucky/attachment/storage/memory_spec.cr new file mode 100644 index 000000000..4cda6b58e --- /dev/null +++ b/spec/lucky/attachment/storage/memory_spec.cr @@ -0,0 +1,55 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Storage::Memory do + describe "#upload and #open" do + it "stores and retrieves file content" do + storage = Lucky::Attachment::Storage::Memory.new + io = IO::Memory.new("hello world") + + storage.upload(io, "test.txt") + + result = storage.open("test.txt") + result.gets_to_end.should eq("hello world") + end + end + + describe "#exists?" do + it "returns true for existing files" do + storage = Lucky::Attachment::Storage::Memory.new + storage.upload(IO::Memory.new("test"), "test.txt") + + storage.exists?("test.txt").should be_true + end + + it "returns false for non-existing files" do + storage = Lucky::Attachment::Storage::Memory.new + + storage.exists?("missing.txt").should be_false + end + end + + describe "#delete" do + it "removes the file" do + storage = Lucky::Attachment::Storage::Memory.new + storage.upload(IO::Memory.new("test"), "test.txt") + + storage.delete("test.txt") + + storage.exists?("test.txt").should be_false + end + end + + describe "#url" do + it "returns path without base_url" do + storage = Lucky::Attachment::Storage::Memory.new + + storage.url("path/to/file.jpg").should eq("/path/to/file.jpg") + end + + it "includes base_url when set" do + storage = Lucky::Attachment::Storage::Memory.new(base_url: "https://example.com") + + storage.url("path/to/file.jpg").should eq("https://example.com/path/to/file.jpg") + end + end +end diff --git a/spec/lucky/attachment/uploaded_file_spec.cr b/spec/lucky/attachment/uploaded_file_spec.cr new file mode 100644 index 000000000..56dccdc59 --- /dev/null +++ b/spec/lucky/attachment/uploaded_file_spec.cr @@ -0,0 +1,59 @@ +require "../../spec_helper" + +describe Lucky::Attachment::UploadedFile do + describe ".from_json" do + it "deserializes from JSON" do + file = Lucky::Attachment::UploadedFile.from_json( + { + id: "test.jpg", + storage: "store", + metadata: {filename: "original.jpg", size: 1024}, + }.to_json + ) + + file.id.should eq("test.jpg") + file.storage_key.should eq("store") + file.original_filename.should eq("original.jpg") + file.size.should eq(1024) + end + end + + describe "#to_json" do + it "serializes to JSON" do + file = Lucky::Attachment::UploadedFile.new( + id: "test.jpg", + storage_key: "store", + metadata: Lucky::Attachment::MetadataHash{ + "filename" => "original.jpg", + "size" => 1024_i64, + } + ) + parsed = JSON.parse(file.to_json) + + parsed["id"].should eq("test.jpg") + parsed["storage"].should eq("store") + parsed["metadata"]["filename"].should eq("original.jpg") + end + end + + describe "#extension" do + it "extracts from id" do + file = Lucky::Attachment::UploadedFile.new( + id: "path/to/file.jpg", + storage_key: "store" + ) + + file.extension.should eq("jpg") + end + + it "falls back to filename metadata" do + file = Lucky::Attachment::UploadedFile.new( + id: "abc123", + storage_key: "store", + metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.png"} + ) + + file.extension.should eq("png") + end + end +end diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr new file mode 100644 index 000000000..42e2aaabd --- /dev/null +++ b/spec/lucky/attachment/uploader_spec.cr @@ -0,0 +1,65 @@ +require "../../spec_helper" + +describe Lucky::Attachment::Uploader do + before_each do + Lucky::Attachment.configure do |settings| + settings.storages["cache"] = Lucky::Attachment::Storage::Memory.new + settings.storages["store"] = Lucky::Attachment::Storage::Memory.new + end + end + + describe "#upload" do + it "uploads a file and returns UploadedFile" do + uploader = TestAttachmentUploader.new("store") + io = IO::Memory.new("test content") + + file = uploader.upload(io) + + file.should be_a(Lucky::Attachment::UploadedFile) + file.storage_key.should eq("store") + file.exists?.should be_true + end + + it "extracts metadata" do + uploader = TestAttachmentUploader.new("store") + io = IO::Memory.new("test content") + + file = uploader.upload(io) + + file.size.should eq(12) + end + + it "accepts custom metadata" do + uploader = TestAttachmentUploader.new("store") + io = IO::Memory.new("test") + + file = uploader.upload(io, metadata: {"custom" => "value"}) + + file["custom"].should eq("value") + end + end + + describe ".cache" do + it "uploads to cache storage" do + io = IO::Memory.new("test") + + file = TestAttachmentUploader.cache(io) + + file.storage_key.should eq("cache") + end + end + + describe ".promote" do + it "moves file from cache to store" do + cached = TestAttachmentUploader.cache(IO::Memory.new("test")) + + stored = TestAttachmentUploader.promote(cached) + + stored.storage_key.should eq("store") + cached.exists?.should be_false + end + end +end + +struct TestAttachmentUploader < Lucky::Attachment::Uploader +end From f8815a62a939f44a65911c961b927a7a8e245b8e Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 19:11:18 +0100 Subject: [PATCH 03/52] Add compatibility with `Lucky::UploadedFile` Makes sure `Lucky::UploadedFile` can be considered as an IO-ish object in uploader. --- spec/lucky/attachment/uploader_spec.cr | 224 +++++++++++++++++++++---- src/lucky/attachment/uploader.cr | 12 +- src/lucky/uploaded_file.cr | 11 ++ 3 files changed, 213 insertions(+), 34 deletions(-) diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index 42e2aaabd..766a56f8e 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -1,65 +1,229 @@ require "../../spec_helper" describe Lucky::Attachment::Uploader do + memory_cache = Lucky::Attachment::Storage::Memory.new + memory_store = Lucky::Attachment::Storage::Memory.new + before_each do + memory_cache.clear! + memory_store.clear! + Lucky::Attachment.configure do |settings| - settings.storages["cache"] = Lucky::Attachment::Storage::Memory.new - settings.storages["store"] = Lucky::Attachment::Storage::Memory.new + settings.storages["cache"] = memory_cache + settings.storages["store"] = memory_store end end describe "#upload" do - it "uploads a file and returns UploadedFile" do - uploader = TestAttachmentUploader.new("store") - io = IO::Memory.new("test content") + context "with a basic IO" do + it "uploads and returns an UploadedFile" do + io = IO::Memory.new("hello") + file = TestUploader.new("store").upload(io) + + file.should be_a(Lucky::Attachment::UploadedFile) + file.storage_key.should eq("store") + file.exists?.should be_true + end + + it "generates a unique location each time" do + file_a = TestUploader.new("store").upload(IO::Memory.new("a")) + file_b = TestUploader.new("store").upload(IO::Memory.new("b")) + + file_a.id.should_not eq(file_b.id) + end + + it "extracts size metadata" do + io = IO::Memory.new("hello world") + file = TestUploader.new("store").upload(io) + + file.size.should eq(11) + end + + it "preserves extension in the location" do + io = IO::Memory.new("data") + file = TestUploader.new("store").upload( + io, + metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.jpg"} + ) + + file.id.should end_with(".jpg") + end + + it "accepts a custom location" do + io = IO::Memory.new("data") + file = TestUploader.new("store").upload(io, location: "my/custom/path.jpg") + + file.id.should eq("my/custom/path.jpg") + end + + it "merges provided metadata with extracted metadata" do + io = IO::Memory.new("data") + file = TestUploader.new("store").upload( + io, + metadata: Lucky::Attachment::MetadataHash{ + "filename" => "override.png", + "custom" => "value", + } + ) + + file.original_filename.should eq("override.png") + file["custom"].should eq("value") + end + end - file = uploader.upload(io) + context "with a File IO" do + it "extracts filename from path" do + file = File.tempfile("myfile", ".txt") { |f| f.print("content") } + uploaded = TestUploader.new("store").upload(File.open(file.path)) - file.should be_a(Lucky::Attachment::UploadedFile) - file.storage_key.should eq("store") - file.exists?.should be_true - end + uploaded.original_filename.should eq(File.basename(file.path)) + ensure + file.try(&.delete) + end - it "extracts metadata" do - uploader = TestAttachmentUploader.new("store") - io = IO::Memory.new("test content") + it "extracts size" do + file = File.tempfile("myfile", ".txt") { |f| f.print("content") } + uploaded = TestUploader.new("store").upload(File.open(file.path)) - file = uploader.upload(io) + uploaded.size.should eq(7) + ensure + file.try(&.delete) + end + end - file.size.should eq(12) + context "error handling" do + it "raises Error when no storages are not configured" do + Lucky::Attachment.configure do |settings| + settings.storages = {} of String => Lucky::Attachment::Storage::Base + end + + expect_raises( + Lucky::Attachment::Error, + "There are no storages registered yet" + ) do + TestUploader.new("store").upload(IO::Memory.new("data")) + end + end + + it "raises Error when a storage is not configured" do + expect_raises( + Lucky::Attachment::Error, + %(Storage "missing" is not registered. The available storages are: "cache", "store") + ) do + TestUploader.new("missing").upload(IO::Memory.new("data")) + end + end end + end - it "accepts custom metadata" do - uploader = TestAttachmentUploader.new("store") - io = IO::Memory.new("test") + describe "custom uploader behaviour" do + it "uses overridden generate_location" do + file = CustomLocationUploader.new("store").upload(IO::Memory.new("data")) - file = uploader.upload(io, metadata: {"custom" => "value"}) + file.id.should start_with("custom/") + end - file["custom"].should eq("value") + it "uses overridden extract_metadata" do + file = CustomMetadataUploader.new("store").upload(IO::Memory.new("data")) + + file["custom_key"].should eq("custom_value") end end describe ".cache" do - it "uploads to cache storage" do - io = IO::Memory.new("test") - - file = TestAttachmentUploader.cache(io) + it "uploads to the cache storage" do + file = TestUploader.cache(IO::Memory.new("data")) file.storage_key.should eq("cache") + memory_cache.exists?(file.id).should be_true end end - describe ".promote" do - it "moves file from cache to store" do - cached = TestAttachmentUploader.cache(IO::Memory.new("test")) + describe ".store" do + it "uploads to the store storage" do + file = TestUploader.store(IO::Memory.new("data")) - stored = TestAttachmentUploader.promote(cached) + file.storage_key.should eq("store") + memory_store.exists?(file.id).should be_true + end + end + + describe ".promote" do + it "moves a cached file to the store" do + cached = TestUploader.cache(IO::Memory.new("data")) + stored = TestUploader.promote(cached) stored.storage_key.should eq("store") - cached.exists?.should be_false + memory_store.exists?(stored.id).should be_true + end + + it "deletes the source file by default" do + cached = TestUploader.cache(IO::Memory.new("data")) + cached_id = cached.id + TestUploader.promote(cached) + + memory_cache.exists?(cached_id).should be_false + end + + it "preserves the source when delete_source is false" do + cached = TestUploader.cache(IO::Memory.new("data")) + cached_id = cached.id + TestUploader.promote(cached, delete_source: false) + + memory_cache.exists?(cached_id).should be_true + end + + it "preserves the file id across storages" do + cached = TestUploader.cache(IO::Memory.new("data")) + stored = TestUploader.promote(cached) + + stored.id.should eq(cached.id) + end + + it "preserves metadata" do + cached = TestUploader.cache( + IO::Memory.new("data"), + metadata: Lucky::Attachment::MetadataHash{"filename" => "test.jpg"} + ) + stored = TestUploader.promote(cached) + + stored.original_filename.should eq("test.jpg") + end + + it "can promote to a custom storage key" do + Lucky::Attachment.configure do |settings| + settings.storages["cache"] = memory_cache + settings.storages["store"] = memory_store + settings.storages["offsite"] = Lucky::Attachment::Storage::Memory.new + end + cached = TestUploader.cache(IO::Memory.new("data")) + offsite = TestUploader.promote(cached, to: "offsite") + + offsite.storage_key.should eq("offsite") end end end -struct TestAttachmentUploader < Lucky::Attachment::Uploader +private struct TestUploader < Lucky::Attachment::Uploader +end + +private struct CustomLocationUploader < Lucky::Attachment::Uploader + def generate_location( + io : IO, + metadata : Lucky::Attachment::MetadataHash, + ) : String + "custom/#{super}" + end +end + +private struct CustomMetadataUploader < Lucky::Attachment::Uploader + def extract_metadata( + io : IO, + metadata : Lucky::Attachment::MetadataHash? = nil, + **options, + ) : Lucky::Attachment::MetadataHash + data = super + data["custom_key"] = "custom_value" + data + end end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 3997977c8..371d43c60 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -98,7 +98,7 @@ module Lucky::Attachment # end # ``` # - def generate_location(io : IO, metadata : MetadataHash) : String + def generate_location(io : IO, metadata : MetadataHash, **options) : String extension = extract_extension(io, metadata) basename = generate_uid extension ? "#{basename}.#{extension}" : basename @@ -140,7 +140,7 @@ module Lucky::Attachment if io.responds_to?(:original_filename) io.original_filename elsif io.responds_to?(:filename) - io.filename + io.filename.presence elsif io.responds_to?(:path) File.basename(io.path) end @@ -148,13 +148,17 @@ module Lucky::Attachment # Extracts the file size from the IO, if available. protected def extract_size(io : IO) : Int64? - io.size.to_i64 if io.responds_to?(:size) + if io.responds_to?(:tempfile) + io.tempfile.size + elsif io.responds_to?(:size) + io.size.to_i64 + end end # Extracts the MIME type from the IO if available. # # NOTE: This relies on the IO providing content_type, which typically comes - # from HTTP headers and may not be accurate. + # from HTTP headers and may not be accurate, but it's a good fallback. # protected def extract_mime_type(io : IO) : String? return unless io.responds_to?(:content_type) && (type = io.content_type) diff --git a/src/lucky/uploaded_file.cr b/src/lucky/uploaded_file.cr index 02ad0f49b..b5e8eff9d 100644 --- a/src/lucky/uploaded_file.cr +++ b/src/lucky/uploaded_file.cr @@ -39,6 +39,17 @@ class Lucky::UploadedFile tempfile.size.zero? end + # Attempts to extract the content type from the part's headers. + # + # ``` + # uploaded_file_object.content_type + # # => "image/png" + # ``` + # + def content_type : String? + @part.headers["Content-Type"]?.try { |t| t.split(';').first.strip } + end + # Avram::Uploadable needs to be updated when this is removed @[Deprecated("`metadata` deprecated. Each method on metadata is accessible directly on Lucky::UploadedFile")] def metadata : HTTP::FormData::FileMetadata From a27837919717bed9f9f2cec5a17bb14416915f8c Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 19:11:47 +0100 Subject: [PATCH 04/52] Add better error message for missing stores --- src/lucky/attachment.cr | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr index da6e0d258..343d46781 100644 --- a/src/lucky/attachment.cr +++ b/src/lucky/attachment.cr @@ -19,11 +19,19 @@ module Lucky::Attachment # Lucky::Attachment.find_storage("store") # => Storage::FileSystem # Lucky::Attachment.find_storage("missing") # raises Lucky::Attachment::Error # ``` + # def self.find_storage(name : String) : Storage::Base settings.storages[name]? || raise Error.new( - "Storage #{name.inspect} is not registered." \ - "Available storages: #{settings.storages.keys.inspect}" + String.build do |io| + if settings.storages.keys.empty? + io << "There are no storages registered yet" + else + io << %(Storage ) << name.inspect + io << %( is not registered. The available storages are: ) + io << settings.storages.keys.map { |s| s.inspect }.join(", ") + end + end ) end @@ -32,6 +40,7 @@ module Lucky::Attachment # ``` # stored = Lucky::Attachment.promote(cached, to: "store") # ``` + # def self.promote( file : UploadedFile, to storage : String, @@ -56,6 +65,7 @@ module Lucky::Attachment # Lucky::Attachment.uploaded_file(json_any) # Lucky::Attachment.uploaded_file(uploaded_file) # ``` + # def self.uploaded_file(json : String) : UploadedFile UploadedFile.from_json(json) end @@ -80,6 +90,7 @@ module Lucky::Attachment # # file is guaranteed to be a File with a path # end # ``` + # def self.with_file(io : IO, &) if io.is_a?(File) yield io From c09756236cdef2be105a4795a11a539ac048d1b0 Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 19:34:47 +0100 Subject: [PATCH 05/52] Fix bug in file system storage url builder --- src/lucky/attachment/storage/file_system.cr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lucky/attachment/storage/file_system.cr b/src/lucky/attachment/storage/file_system.cr index 80c22868d..b87b1e6ac 100644 --- a/src/lucky/attachment/storage/file_system.cr +++ b/src/lucky/attachment/storage/file_system.cr @@ -88,7 +88,9 @@ module Lucky::Attachment String.build do |url| url << host.rstrip('/') if host url << '/' - url << prefix.lstrip('/') << '/' if prefix + if p = prefix + url << p.lstrip('/') << '/' + end url << id end end From 077964a0ee67658b9034a5e7e407b7216a54bc9e Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 19:35:22 +0100 Subject: [PATCH 06/52] Add missing specs for attachment uploaded file --- spec/lucky/attachment/uploaded_file_spec.cr | 186 +++++++++++++++++++- 1 file changed, 183 insertions(+), 3 deletions(-) diff --git a/spec/lucky/attachment/uploaded_file_spec.cr b/spec/lucky/attachment/uploaded_file_spec.cr index 56dccdc59..eab316239 100644 --- a/spec/lucky/attachment/uploaded_file_spec.cr +++ b/spec/lucky/attachment/uploaded_file_spec.cr @@ -1,13 +1,27 @@ require "../../spec_helper" describe Lucky::Attachment::UploadedFile do + memory_store = Lucky::Attachment::Storage::Memory.new(base_url: "https://example.com") + + before_each do + memory_store.clear! + + Lucky::Attachment.configure do |settings| + settings.storages["store"] = memory_store + end + end + describe ".from_json" do it "deserializes from JSON" do file = Lucky::Attachment::UploadedFile.from_json( { id: "test.jpg", storage: "store", - metadata: {filename: "original.jpg", size: 1024}, + metadata: { + filename: "original.jpg", + size: 1024_i64, + mime_type: "image/jpeg", + }, }.to_json ) @@ -15,6 +29,7 @@ describe Lucky::Attachment::UploadedFile do file.storage_key.should eq("store") file.original_filename.should eq("original.jpg") file.size.should eq(1024) + file.mime_type.should eq("image/jpeg") end end @@ -24,15 +39,18 @@ describe Lucky::Attachment::UploadedFile do id: "test.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{ - "filename" => "original.jpg", - "size" => 1024_i64, + "filename" => "original.jpg", + "size" => 1024_i64, + "mime_type" => "image/jpeg", } ) parsed = JSON.parse(file.to_json) parsed["id"].should eq("test.jpg") parsed["storage"].should eq("store") + parsed["metadata"]["size"].should eq(1024_i64) parsed["metadata"]["filename"].should eq("original.jpg") + parsed["metadata"]["mime_type"].should eq("image/jpeg") end end @@ -55,5 +73,167 @@ describe Lucky::Attachment::UploadedFile do file.extension.should eq("png") end + + it "returns nil when no extension can be determined" do + file = Lucky::Attachment::UploadedFile.new( + id: "abc123", + storage_key: "store" + ) + + file.extension.should be_nil + end + end + + describe "#size" do + it "returns Int64 from integer metadata" do + file = Lucky::Attachment::UploadedFile.new( + id: "file.jpg", + storage_key: "store", + metadata: Lucky::Attachment::MetadataHash{"size" => 1024_i64} + ) + + file.size.should eq(1024_i64) + file.size.should be_a(Int64) + end + + it "coerces Int32 to Int64" do + file = Lucky::Attachment::UploadedFile.new( + id: "file.jpg", + storage_key: "store", + metadata: Lucky::Attachment::MetadataHash{"size" => 512_i32} + ) + + file.size.should eq(512_i64) + file.size.should be_a(Int64) + end + + it "returns nil when size is absent" do + file = Lucky::Attachment::UploadedFile.new( + id: "file.jpg", + storage_key: "store" + ) + + file.size.should be_nil + end + end + + describe "#url" do + it "delegates to storage" do + file = Lucky::Attachment::UploadedFile.new( + id: "uploads/photo.jpg", + storage_key: "store" + ) + + file.url.should eq("https://example.com/uploads/photo.jpg") + end + end + + describe "#exists?" do + it "returns true when file is in storage" do + memory_store.upload(IO::Memory.new("data"), "photo.jpg") + file = Lucky::Attachment::UploadedFile.new( + id: "photo.jpg", + storage_key: "store" + ) + + file.exists?.should be_true + end + + it "returns false when file is not in storage" do + file = Lucky::Attachment::UploadedFile.new( + id: "missing.jpg", + storage_key: "store" + ) + file.exists?.should be_false + end + end + + describe "#open" do + it "yields the file IO" do + memory_store.upload(IO::Memory.new("file content"), "test.txt") + file = Lucky::Attachment::UploadedFile.new( + id: "test.txt", + storage_key: "store" + ) + + file.open { |io| io.gets_to_end.should eq("file content") } + end + + it "closes the IO after the block" do + memory_store.upload(IO::Memory.new("data"), "test.txt") + file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + captured_io = nil + file.open { |io| captured_io = io } + + captured_io.not_nil!.closed?.should be_true + end + + it "closes the IO even if the block raises" do + memory_store.upload(IO::Memory.new("data"), "test.txt") + file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + captured_io = nil + + expect_raises(Exception) do + file.open do |io| + captured_io = io + raise "oops" + end + end + captured_io.not_nil!.closed?.should be_true + end + + describe "#download" do + it "returns a tempfile with file content" do + memory_store.upload(IO::Memory.new("downloaded content"), "test.txt") + file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + tempfile = file.download + + tempfile.gets_to_end.should eq("downloaded content") + tempfile.close + tempfile.delete + end + + it "cleans up the tempfile after the block" do + memory_store.upload(IO::Memory.new("data"), "test.txt") + file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + tempfile_path = "" + file.download { |tempfile| tempfile_path = tempfile.path } + + File.exists?(tempfile_path).should be_false + end + end + end + + describe "#delete" do + it "removes the file from storage" do + memory_store.upload(IO::Memory.new("data"), "test.txt") + file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + file.delete + + memory_store.exists?("test.txt").should be_false + end + end + + describe "#==" do + it "is equal when id and storage_key match" do + a = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "store") + b = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "store") + + (a == b).should be_true + end + + it "is not equal when id differs" do + a = Lucky::Attachment::UploadedFile.new(id: "a.jpg", storage_key: "store") + b = Lucky::Attachment::UploadedFile.new(id: "b.jpg", storage_key: "store") + + (a == b).should be_false + end + + it "is not equal when storage_key differs" do + a = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "cache") + b = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "store") + + (a == b).should be_false + end end end From a75d9f7d4e2443f89d2715c2c801d2ddee08018c Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 19:49:07 +0100 Subject: [PATCH 07/52] Add missing specs for memory storage --- spec/lucky/attachment/storage/memory_spec.cr | 49 ++++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/spec/lucky/attachment/storage/memory_spec.cr b/spec/lucky/attachment/storage/memory_spec.cr index 4cda6b58e..451e1bf68 100644 --- a/spec/lucky/attachment/storage/memory_spec.cr +++ b/spec/lucky/attachment/storage/memory_spec.cr @@ -5,12 +5,29 @@ describe Lucky::Attachment::Storage::Memory do it "stores and retrieves file content" do storage = Lucky::Attachment::Storage::Memory.new io = IO::Memory.new("hello world") - storage.upload(io, "test.txt") result = storage.open("test.txt") result.gets_to_end.should eq("hello world") end + + it "overwrites existing content" do + storage = Lucky::Attachment::Storage::Memory.new + storage.upload(IO::Memory.new("original"), "test.txt") + storage.upload(IO::Memory.new("updated"), "test.txt") + + storage.open("test.txt").gets_to_end.should eq("updated") + end + end + + describe "#open" do + it "raises FileNotFound for missing files" do + storage = Lucky::Attachment::Storage::Memory.new + + expect_raises(Lucky::Attachment::FileNotFound, /missing\.txt/) do + storage.open("missing.txt") + end + end end describe "#exists?" do @@ -32,11 +49,16 @@ describe Lucky::Attachment::Storage::Memory do it "removes the file" do storage = Lucky::Attachment::Storage::Memory.new storage.upload(IO::Memory.new("test"), "test.txt") - storage.delete("test.txt") storage.exists?("test.txt").should be_false end + + it "does not raise when deleting a missing file" do + storage = Lucky::Attachment::Storage::Memory.new + + storage.delete("nonexistent.txt") + end end describe "#url" do @@ -46,10 +68,27 @@ describe Lucky::Attachment::Storage::Memory do storage.url("path/to/file.jpg").should eq("/path/to/file.jpg") end - it "includes base_url when set" do - storage = Lucky::Attachment::Storage::Memory.new(base_url: "https://example.com") + it "prepends base_url when configured" do + storage = Lucky::Attachment::Storage::Memory.new(base_url: "https://cdn.example.com") + + storage.url("path/to/file.jpg").should eq("https://cdn.example.com/path/to/file.jpg") + end + + it "handles base_url with trailing slash" do + storage = Lucky::Attachment::Storage::Memory.new(base_url: "https://cdn.example.com/") + + storage.url("path/to/file.jpg").should eq("https://cdn.example.com/path/to/file.jpg") + end + end + + describe "#clear!" do + it "removes all files" do + storage = Lucky::Attachment::Storage::Memory.new + storage.upload(IO::Memory.new("a"), "a.txt") + storage.upload(IO::Memory.new("b"), "b.txt") + storage.clear! - storage.url("path/to/file.jpg").should eq("https://example.com/path/to/file.jpg") + storage.size.should eq(0) end end end From e8d529e48ddd408b4865b259bad03c63424bedcc Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 20:08:26 +0100 Subject: [PATCH 08/52] Add specs for file system storage --- .../attachment/storage/file_system_spec.cr | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 spec/lucky/attachment/storage/file_system_spec.cr diff --git a/spec/lucky/attachment/storage/file_system_spec.cr b/spec/lucky/attachment/storage/file_system_spec.cr new file mode 100644 index 000000000..e7bf8b937 --- /dev/null +++ b/spec/lucky/attachment/storage/file_system_spec.cr @@ -0,0 +1,131 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Storage::FileSystem do + temp_dir = File.tempname("lucky_attachment_spec") + + before_each do + Dir.mkdir_p(temp_dir) + end + + after_each do + FileUtils.rm_rf(temp_dir) + end + + describe "#upload and #open" do + it "writes and reads file content" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + storage.upload(IO::Memory.new("file content"), "test.txt") + + storage.open("test.txt").gets_to_end.should eq("file content") + end + + it "creates intermediate directories" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + storage.upload(IO::Memory.new("data"), "a/b/c/test.txt") + + Dir.exists?(File.join(temp_dir, "a/b/c")).should be_true + end + + it "respects the prefix" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir, prefix: "cache") + storage.upload(IO::Memory.new("data"), "test.txt") + + File.exists?(File.join(temp_dir, "cache", "test.txt")).should be_true + end + end + + describe "#open" do + it "raises FileNotFound for missing files" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + + expect_raises(Lucky::Attachment::FileNotFound, /missing\.txt/) do + storage.open("missing.txt") + end + end + end + + describe "#exists?" do + it "returns true for existing files" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + storage.upload(IO::Memory.new("data"), "test.txt") + + storage.exists?("test.txt").should be_true + end + + it "returns false for missing files" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + + storage.exists?("missing.txt").should be_false + end + end + + describe "#delete" do + it "removes the file" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + storage.upload(IO::Memory.new("data"), "test.txt") + storage.delete("test.txt") + + storage.exists?("test.txt").should be_false + end + + it "cleans up empty parent directories by default" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + storage.upload(IO::Memory.new("data"), "a/b/test.txt") + storage.delete("a/b/test.txt") + + Dir.exists?(File.join(temp_dir, "a/b")).should be_false + Dir.exists?(File.join(temp_dir, "a")).should be_false + end + + it "does not clean non-empty parent directories" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + storage.upload(IO::Memory.new("data"), "a/b/test1.txt") + storage.upload(IO::Memory.new("data"), "a/b/test2.txt") + storage.delete("a/b/test1.txt") + + Dir.exists?(File.join(temp_dir, "a/b")).should be_true + end + + it "skips cleanup when clean is false" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir, clean: false) + storage.upload(IO::Memory.new("data"), "a/b/test.txt") + storage.delete("a/b/test.txt") + + Dir.exists?(File.join(temp_dir, "a/b")).should be_true + end + + it "does not raise when deleting a missing file" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + + storage.delete("nonexistent.txt") + end + end + + describe "#url" do + it "returns a path from the root" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + + storage.url("test.txt").should eq("/test.txt") + end + + it "includes the prefix in the URL" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir, prefix: "uploads/cache") + + storage.url("test.txt").should eq("/uploads/cache/test.txt") + end + + it "prepends host when provided" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + + storage.url("test.txt", host: "https://example.com").should eq("https://example.com/test.txt") + end + end + + describe "#path_for" do + it "returns the full filesystem path" do + storage = Lucky::Attachment::Storage::FileSystem.new(temp_dir) + + storage.path_for("test.txt").should eq(File.join(File.expand_path(temp_dir), "test.txt")) + end + end +end From 1ed9c187c4ecfa212c7dfd4ef1f70e2451104982 Mon Sep 17 00:00:00 2001 From: Wout Date: Thu, 19 Feb 2026 20:09:32 +0100 Subject: [PATCH 09/52] Add specs for `Lucky::UploadedFile` integration Tests the integration between Lucky::UploadedFile and Lucky::Attachment::Uploader --- spec/lucky/attachment/uploader_spec.cr | 56 ++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index 766a56f8e..46ffff15b 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -204,9 +204,46 @@ describe Lucky::Attachment::Uploader do end end +describe "Lucky::UploadedFile integration" do + memory_store = Lucky::Attachment::Storage::Memory.new + + before_each do + memory_store.clear! + Lucky::Attachment.configure do |settings| + settings.storages["store"] = memory_store + end + end + + it "extracts filename from Lucky::UploadedFile" do + part = build_form_data_part("avatar", "photo.jpg", "image/jpeg", "data") + lucky_file = Lucky::UploadedFile.new(part) + uploaded = AvatarUploader.new("store").upload(lucky_file.tempfile) + + uploaded.original_filename.should eq(File.basename(lucky_file.tempfile.path)) + end + + it "extracts content_type when Lucky::UploadedFile exposes it" do + part = build_form_data_part("avatar", "photo.jpg", "image/jpeg", "data") + lucky_file = Lucky::UploadedFile.new(part) + uploaded = AvatarUploader.new("store").upload(lucky_file.tempfile) + + uploaded.mime_type.should be_nil + end + + it "handles blank files gracefully" do + part = build_form_data_part("avatar", "", "application/octet-stream", "") + lucky_file = Lucky::UploadedFile.new(part) + + lucky_file.blank?.should be_true + end +end + private struct TestUploader < Lucky::Attachment::Uploader end +private struct AvatarUploader < Lucky::Attachment::Uploader +end + private struct CustomLocationUploader < Lucky::Attachment::Uploader def generate_location( io : IO, @@ -227,3 +264,22 @@ private struct CustomMetadataUploader < Lucky::Attachment::Uploader data end end + +private def build_form_data_part( + name : String, + filename : String, + content_type : String, + body : String, +) : HTTP::FormData::Part + disposition = if filename.empty? + %(form-data; name="#{name}") + else + %(form-data; name="#{name}"; filename="#{filename}") + end + headers = HTTP::Headers{ + "Content-Disposition" => disposition, + "Content-Type" => content_type, + } + + HTTP::FormData::Part.new(headers: headers, body: IO::Memory.new(body)) +end From 9dbf953ef2dfe7dac41c3854e14219b42e0d574b Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 20 Feb 2026 19:39:19 +0100 Subject: [PATCH 10/52] Clean up storage code and add comments --- src/lucky/attachment/storage.cr | 1 + src/lucky/attachment/storage/file_system.cr | 10 +++++----- src/lucky/attachment/storage/memory.cr | 15 +++++++++++---- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/lucky/attachment/storage.cr b/src/lucky/attachment/storage.cr index 7e5bab461..a5fb69684 100644 --- a/src/lucky/attachment/storage.cr +++ b/src/lucky/attachment/storage.cr @@ -40,6 +40,7 @@ module Lucky::Attachment # storage.url("uploads/photo.jpg") # # => "/uploads/photo.jpg" # storage.url("uploads/photo.jpg", host: "https://example.com") + # # => "https://example.com/uploads/photo.jpg" # ``` # abstract def url(id : String, **options) : String diff --git a/src/lucky/attachment/storage/file_system.cr b/src/lucky/attachment/storage/file_system.cr index b87b1e6ac..f2389184b 100644 --- a/src/lucky/attachment/storage/file_system.cr +++ b/src/lucky/attachment/storage/file_system.cr @@ -49,6 +49,7 @@ module Lucky::Attachment File.expand_path(File.join(directory, p)) end + # Uploads an IO to the given location (id) in the storage. def upload(io : IO, id : String, move : Bool = false, **options) : Nil path = path_for(id) Dir.mkdir_p(File.dirname(path), mode: directory_permissions.value) @@ -63,12 +64,14 @@ module Lucky::Attachment end end + # Opens the file at the given location and returns an IO for reading. def open(id : String, **options) : IO File.open(path_for(id), "rb") rescue ex : File::NotFoundError raise FileNotFound.new("File not found: #{id}") end + # Returns whether a file exists at the given location. def exists?(id : String) : Bool File.exists?(path_for(id)) end @@ -95,6 +98,7 @@ module Lucky::Attachment end end + # Deletes the file at the given location. def delete(id : String) : Nil path = path_for(id) File.delete?(path) @@ -105,11 +109,7 @@ module Lucky::Attachment # Override move for efficient file system rename def move(io : IO, id : String, **options) : Nil - if io.is_a?(File) - upload(io, id, **options, move: true) - else - upload(io, id, **options) - end + upload(io, id, **options, move: io.is_a?(File)) end # Cleans empty parent directories up to the expanded_directory. diff --git a/src/lucky/attachment/storage/memory.cr b/src/lucky/attachment/storage/memory.cr index 12676922d..3d9e2ce99 100644 --- a/src/lucky/attachment/storage/memory.cr +++ b/src/lucky/attachment/storage/memory.cr @@ -20,10 +20,12 @@ module Lucky::Attachment @store = {} of String => Bytes end + # Uploads an IO to the given location (id) in the storage. def upload(io : IO, id : String, **options) : Nil @store[id] = io.getb_to_end end + # Opens the file at the given location and returns an IO for reading. def open(id : String, **options) : IO if bytes = @store[id]? IO::Memory.new(bytes) @@ -32,22 +34,27 @@ module Lucky::Attachment end end + # Returns whether a file exists at the given location. def exists?(id : String) : Bool @store.has_key?(id) end + # Returns the URL for accessing the file at the given location. def url(id : String, **options) : String - if base = @base_url - "#{base.rstrip('/')}/#{id}" - else - "/#{id}" + String.build do |io| + if base = @base_url + io << base.rstrip('/') + end + io << '/' << id end end + # Deletes the file at the given location. def delete(id : String) : Nil @store.delete(id) end + # Clears out the store. def clear! : Nil @store.clear end From 540c0376ca7349fedd94a34b1f95e127fa5af2a7 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 21 Feb 2026 17:10:37 +0100 Subject: [PATCH 11/52] Rename uploaded file to stored file for attachments --- ...oaded_file_spec.cr => stored_file_spec.cr} | 48 +++++++++---------- spec/lucky/attachment/uploader_spec.cr | 4 +- src/lucky/attachment.cr | 24 +++++----- .../{uploaded_file.cr => stored_file.cr} | 9 ++-- src/lucky/attachment/uploader.cr | 20 ++++---- 5 files changed, 54 insertions(+), 51 deletions(-) rename spec/lucky/attachment/{uploaded_file_spec.cr => stored_file_spec.cr} (77%) rename src/lucky/attachment/{uploaded_file.cr => stored_file.cr} (97%) diff --git a/spec/lucky/attachment/uploaded_file_spec.cr b/spec/lucky/attachment/stored_file_spec.cr similarity index 77% rename from spec/lucky/attachment/uploaded_file_spec.cr rename to spec/lucky/attachment/stored_file_spec.cr index eab316239..71edc90b4 100644 --- a/spec/lucky/attachment/uploaded_file_spec.cr +++ b/spec/lucky/attachment/stored_file_spec.cr @@ -1,6 +1,6 @@ require "../../spec_helper" -describe Lucky::Attachment::UploadedFile do +describe Lucky::Attachment::StoredFile do memory_store = Lucky::Attachment::Storage::Memory.new(base_url: "https://example.com") before_each do @@ -13,7 +13,7 @@ describe Lucky::Attachment::UploadedFile do describe ".from_json" do it "deserializes from JSON" do - file = Lucky::Attachment::UploadedFile.from_json( + file = Lucky::Attachment::StoredFile.from_json( { id: "test.jpg", storage: "store", @@ -35,7 +35,7 @@ describe Lucky::Attachment::UploadedFile do describe "#to_json" do it "serializes to JSON" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "test.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{ @@ -56,7 +56,7 @@ describe Lucky::Attachment::UploadedFile do describe "#extension" do it "extracts from id" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "path/to/file.jpg", storage_key: "store" ) @@ -65,7 +65,7 @@ describe Lucky::Attachment::UploadedFile do end it "falls back to filename metadata" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "abc123", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.png"} @@ -75,7 +75,7 @@ describe Lucky::Attachment::UploadedFile do end it "returns nil when no extension can be determined" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "abc123", storage_key: "store" ) @@ -86,7 +86,7 @@ describe Lucky::Attachment::UploadedFile do describe "#size" do it "returns Int64 from integer metadata" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "file.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{"size" => 1024_i64} @@ -97,7 +97,7 @@ describe Lucky::Attachment::UploadedFile do end it "coerces Int32 to Int64" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "file.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{"size" => 512_i32} @@ -108,7 +108,7 @@ describe Lucky::Attachment::UploadedFile do end it "returns nil when size is absent" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "file.jpg", storage_key: "store" ) @@ -119,7 +119,7 @@ describe Lucky::Attachment::UploadedFile do describe "#url" do it "delegates to storage" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "uploads/photo.jpg", storage_key: "store" ) @@ -131,7 +131,7 @@ describe Lucky::Attachment::UploadedFile do describe "#exists?" do it "returns true when file is in storage" do memory_store.upload(IO::Memory.new("data"), "photo.jpg") - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "photo.jpg", storage_key: "store" ) @@ -140,7 +140,7 @@ describe Lucky::Attachment::UploadedFile do end it "returns false when file is not in storage" do - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "missing.jpg", storage_key: "store" ) @@ -151,7 +151,7 @@ describe Lucky::Attachment::UploadedFile do describe "#open" do it "yields the file IO" do memory_store.upload(IO::Memory.new("file content"), "test.txt") - file = Lucky::Attachment::UploadedFile.new( + file = Lucky::Attachment::StoredFile.new( id: "test.txt", storage_key: "store" ) @@ -161,7 +161,7 @@ describe Lucky::Attachment::UploadedFile do it "closes the IO after the block" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") captured_io = nil file.open { |io| captured_io = io } @@ -170,7 +170,7 @@ describe Lucky::Attachment::UploadedFile do it "closes the IO even if the block raises" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") captured_io = nil expect_raises(Exception) do @@ -185,7 +185,7 @@ describe Lucky::Attachment::UploadedFile do describe "#download" do it "returns a tempfile with file content" do memory_store.upload(IO::Memory.new("downloaded content"), "test.txt") - file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") tempfile = file.download tempfile.gets_to_end.should eq("downloaded content") @@ -195,7 +195,7 @@ describe Lucky::Attachment::UploadedFile do it "cleans up the tempfile after the block" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") tempfile_path = "" file.download { |tempfile| tempfile_path = tempfile.path } @@ -207,7 +207,7 @@ describe Lucky::Attachment::UploadedFile do describe "#delete" do it "removes the file from storage" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::UploadedFile.new(id: "test.txt", storage_key: "store") + file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") file.delete memory_store.exists?("test.txt").should be_false @@ -216,22 +216,22 @@ describe Lucky::Attachment::UploadedFile do describe "#==" do it "is equal when id and storage_key match" do - a = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "store") - b = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "store") + a = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "store") + b = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "store") (a == b).should be_true end it "is not equal when id differs" do - a = Lucky::Attachment::UploadedFile.new(id: "a.jpg", storage_key: "store") - b = Lucky::Attachment::UploadedFile.new(id: "b.jpg", storage_key: "store") + a = Lucky::Attachment::StoredFile.new(id: "a.jpg", storage_key: "store") + b = Lucky::Attachment::StoredFile.new(id: "b.jpg", storage_key: "store") (a == b).should be_false end it "is not equal when storage_key differs" do - a = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "cache") - b = Lucky::Attachment::UploadedFile.new(id: "file.jpg", storage_key: "store") + a = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "cache") + b = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "store") (a == b).should be_false end diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index 46ffff15b..b7b9f1f2f 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -16,11 +16,11 @@ describe Lucky::Attachment::Uploader do describe "#upload" do context "with a basic IO" do - it "uploads and returns an UploadedFile" do + it "uploads and returns a stored file" do io = IO::Memory.new("hello") file = TestUploader.new("store").upload(io) - file.should be_a(Lucky::Attachment::UploadedFile) + file.should be_a(Lucky::Attachment::StoredFile) file.storage_key.should eq("store") file.exists?.should be_true end diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr index 343d46781..8089b1253 100644 --- a/src/lucky/attachment.cr +++ b/src/lucky/attachment.cr @@ -1,8 +1,8 @@ require "habitat" -require "./attachment/uploaded_file" require "./attachment/storage" -require "./attachment/storage/memory" require "./attachment/storage/file_system" +require "./attachment/storage/memory" +require "./attachment/stored_file" require "./attachment/uploader" module Lucky::Attachment @@ -42,13 +42,13 @@ module Lucky::Attachment # ``` # def self.promote( - file : UploadedFile, + file : StoredFile, to storage : String, delete_source : Bool = true, - ) : UploadedFile + ) : StoredFile file.open do |io| find_storage(storage).upload(io, file.id, metadata: file.metadata) - promoted = UploadedFile.new( + promoted = StoredFile.new( id: file.id, storage_key: storage, metadata: file.metadata @@ -58,7 +58,7 @@ module Lucky::Attachment end end - # Deserialize an UploadedFile from various sources. + # Deserialize an StoredFile from various sources. # # ``` # Lucky::Attachment.uploaded_file(json_string) @@ -66,15 +66,15 @@ module Lucky::Attachment # Lucky::Attachment.uploaded_file(uploaded_file) # ``` # - def self.uploaded_file(json : String) : UploadedFile - UploadedFile.from_json(json) + def self.uploaded_file(json : String) : StoredFile + StoredFile.from_json(json) end - def self.uploaded_file(json : JSON::Any) : UploadedFile - UploadedFile.from_json(json.to_json) + def self.uploaded_file(json : JSON::Any) : StoredFile + StoredFile.from_json(json.to_json) end - def self.uploaded_file(file : UploadedFile) : UploadedFile + def self.uploaded_file(file : StoredFile) : StoredFile file end @@ -103,7 +103,7 @@ module Lucky::Attachment end end - def self.with_file(uploaded_file : UploadedFile, &) + def self.with_file(uploaded_file : StoredFile, &) uploaded_file.download do |tempfile| yield tempfile end diff --git a/src/lucky/attachment/uploaded_file.cr b/src/lucky/attachment/stored_file.cr similarity index 97% rename from src/lucky/attachment/uploaded_file.cr rename to src/lucky/attachment/stored_file.cr index 167176d9d..4b7925f0a 100644 --- a/src/lucky/attachment/uploaded_file.cr +++ b/src/lucky/attachment/stored_file.cr @@ -24,14 +24,17 @@ module Lucky::Attachment # } # ``` # - class UploadedFile + class StoredFile include JSON::Serializable - getter id : String + # NOTE: This mimics the behavior of Avram's `JSON::Serializable` extension. + def self.adapter + Lucky(self) + end + getter id : String @[JSON::Field(key: "storage")] getter storage_key : String - getter metadata : MetadataHash @[JSON::Field(ignore: true)] diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 371d43c60..63d591680 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -13,7 +13,7 @@ module Lucky::Attachment # end # # ImageUploader.new("store").upload(io) - # # => UploadedFile with id "images/2024/01/15/abc123.jpg" + # # => Lucky::Attachment::StoredFile with id "images/2024/01/15/abc123.jpg" # ``` # abstract struct Uploader @@ -27,8 +27,8 @@ module Lucky::Attachment Lucky::Attachment.find_storage(storage_key) end - # Uploads a file and returns an UploadedFile. This method accepts - # additional metadata and arbitrary arguments for overrides. + # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method + # accepts additional metadata and arbitrary arguments for overrides. # # ``` # uploader.upload(io) @@ -36,13 +36,13 @@ module Lucky::Attachment # uploader.upload(io, location: "custom/path.jpg") # ``` # - def upload(io : IO, metadata : MetadataHash? = nil, **options) : UploadedFile + def upload(io : IO, metadata : MetadataHash? = nil, **options) : StoredFile data = extract_metadata(io, metadata, **options) data = data.merge(metadata) if metadata location = options[:location]? || generate_location(io, data, **options) storage.upload(io, location, **options.merge(metadata: data)) - UploadedFile.new(id: location, storage_key: storage_key, metadata: data) + StoredFile.new(id: location, storage_key: storage_key, metadata: data) ensure io.close if options[:close]?.nil? || options[:close]? end @@ -52,7 +52,7 @@ module Lucky::Attachment # ``` # cached = ImageUploader.cache(io) # ``` - def self.cache(io : IO, **options) : UploadedFile + def self.cache(io : IO, **options) : StoredFile new("cache").upload(io, **options) end @@ -62,7 +62,7 @@ module Lucky::Attachment # stored = ImageUploader.store(io) # ``` # - def self.store(io : IO, **options) : UploadedFile + def self.store(io : IO, **options) : StoredFile new("store").upload(io, **options) end @@ -74,11 +74,11 @@ module Lucky::Attachment # ``` # def self.promote( - file : UploadedFile, + file : StoredFile, to storage : String = "store", delete_source : Bool = true, **options, - ) : UploadedFile + ) : StoredFile Lucky::Attachment.promote( file, **options, @@ -124,7 +124,7 @@ module Lucky::Attachment **options, ) : MetadataHash MetadataHash{ - "filename" => extract_filename(io), + "filename" => options[:filename]? || extract_filename(io), "size" => extract_size(io), "mime_type" => extract_mime_type(io), } From dc9af32d65df239e6f691204bd44abd23e43dc87 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 21 Feb 2026 19:29:28 +0100 Subject: [PATCH 12/52] Restructure attachment directory --- src/lucky.cr | 2 + src/lucky/attachment.cr | 109 +---- src/lucky/attachment/config.cr | 31 ++ src/lucky/attachment/storage.cr | 110 +++-- src/lucky/attachment/storage/file_system.cr | 206 +++++---- src/lucky/attachment/storage/memory.cr | 102 ++--- src/lucky/attachment/stored_file.cr | 479 ++++++++++---------- src/lucky/attachment/uploader.cr | 316 +++++++------ src/lucky/attachment/utilities.cr | 75 +++ 9 files changed, 710 insertions(+), 720 deletions(-) create mode 100644 src/lucky/attachment/config.cr create mode 100644 src/lucky/attachment/utilities.cr diff --git a/src/lucky.cr b/src/lucky.cr index aadc3c554..4a6703084 100644 --- a/src/lucky.cr +++ b/src/lucky.cr @@ -10,6 +10,8 @@ require "./lucky/quick_def" require "./charms/*" require "http/server" require "lucky_router" +require "./lucky/attachment" +require "./lucky/attachment/**" require "./lucky/events/*" require "./lucky/support/*" require "./lucky/renderable_error" diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr index 8089b1253..e2f589f64 100644 --- a/src/lucky/attachment.cr +++ b/src/lucky/attachment.cr @@ -1,113 +1,10 @@ -require "habitat" require "./attachment/storage" -require "./attachment/storage/file_system" -require "./attachment/storage/memory" -require "./attachment/stored_file" -require "./attachment/uploader" module Lucky::Attachment - Log = ::Log.for("lucky.attachment") + alias MetadataValue = String | Int32 | Int64 | UInt32 | UInt64 | Float64 | Bool | Nil + alias MetadataHash = Hash(String, MetadataValue) - Habitat.create do - # Storage configurations keyed by name ("cache", "store", etc.) - setting storages : Hash(String, Storage::Base) = {} of String => Storage::Base - end - - # Retrieves a storage by name, raising if not found. - # - # ``` - # Lucky::Attachment.find_storage("store") # => Storage::FileSystem - # Lucky::Attachment.find_storage("missing") # raises Lucky::Attachment::Error - # ``` - # - def self.find_storage(name : String) : Storage::Base - settings.storages[name]? || - raise Error.new( - String.build do |io| - if settings.storages.keys.empty? - io << "There are no storages registered yet" - else - io << %(Storage ) << name.inspect - io << %( is not registered. The available storages are: ) - io << settings.storages.keys.map { |s| s.inspect }.join(", ") - end - end - ) - end - - # Move a file from one storage to another (typically cache -> store). - # - # ``` - # stored = Lucky::Attachment.promote(cached, to: "store") - # ``` - # - def self.promote( - file : StoredFile, - to storage : String, - delete_source : Bool = true, - ) : StoredFile - file.open do |io| - find_storage(storage).upload(io, file.id, metadata: file.metadata) - promoted = StoredFile.new( - id: file.id, - storage_key: storage, - metadata: file.metadata - ) - file.delete if delete_source - promoted - end - end - - # Deserialize an StoredFile from various sources. - # - # ``` - # Lucky::Attachment.uploaded_file(json_string) - # Lucky::Attachment.uploaded_file(json_any) - # Lucky::Attachment.uploaded_file(uploaded_file) - # ``` - # - def self.uploaded_file(json : String) : StoredFile - StoredFile.from_json(json) - end - - def self.uploaded_file(json : JSON::Any) : StoredFile - StoredFile.from_json(json.to_json) - end - - def self.uploaded_file(file : StoredFile) : StoredFile - file - end - - def self.uploaded_file(value : Nil) : Nil - nil - end - - # Utility to work with a file IO. If the IO is already a File, yields it - # directly, Otherwise copies to a tempfile, yields it, then cleans up. - # - # ``` - # Lucky::Attachment.with_file(io) do |file| - # # file is guaranteed to be a File with a path - # end - # ``` - # - def self.with_file(io : IO, &) - if io.is_a?(File) - yield io - else - File.tempfile("lucky-attachment") do |tempfile| - IO.copy(io, tempfile) - tempfile.rewind - yield tempfile - end - end - end - - def self.with_file(uploaded_file : StoredFile, &) - uploaded_file.download do |tempfile| - yield tempfile - end - end + # Log = ::Log.for("lucky.attachment") class Error < Exception; end diff --git a/src/lucky/attachment/config.cr b/src/lucky/attachment/config.cr new file mode 100644 index 000000000..66c42be07 --- /dev/null +++ b/src/lucky/attachment/config.cr @@ -0,0 +1,31 @@ +require "habitat" +require "./storage" + +module Lucky::Attachment + Habitat.create do + # Storage configurations keyed by name ("cache", "store", etc.) + setting storages : Hash(String, Storage::Base) = {} of String => Storage::Base + end + + # Retrieves a storage by name, raising if not found. + # + # ``` + # Lucky::Attachment.find_storage("store") # => Storage::FileSystem + # Lucky::Attachment.find_storage("missing") # raises Lucky::Attachment::Error + # ``` + # + def self.find_storage(name : String) : Storage::Base + settings.storages[name]? || + raise Error.new( + String.build do |io| + if settings.storages.keys.empty? + io << "There are no storages registered yet" + else + io << %(Storage ) << name.inspect + io << %( is not registered. The available storages are: ) + io << settings.storages.keys.map { |s| s.inspect }.join(", ") + end + end + ) + end +end diff --git a/src/lucky/attachment/storage.cr b/src/lucky/attachment/storage.cr index a5fb69684..8384527cb 100644 --- a/src/lucky/attachment/storage.cr +++ b/src/lucky/attachment/storage.cr @@ -1,63 +1,61 @@ -module Lucky::Attachment - # Storage backends handle the actual persistence of uploaded files. - # Implementations must provide methods for uploading, retreiving, checking - # existence, and deleting files. - # - abstract class Storage::Base - # Uploads an IO to the given location (id) in the storage. - # - # ``` - # storage.upload(io, "uploads/photo.jpg") - # storage.upload(io, "uploads/photo.jpg", metadata: {"filename" => "original.jpg"}) - # ``` - # - abstract def upload(io : IO, id : String, **options) : Nil +# Storage backends handle the actual persistence of uploaded files. +# Implementations must provide methods for uploading, retreiving, checking +# existence, and deleting files. +# +abstract class Lucky::Attachment::Storage::Base + # Uploads an IO to the given location (id) in the storage. + # + # ``` + # storage.upload(io, "uploads/photo.jpg") + # storage.upload(io, "uploads/photo.jpg", metadata: {"filename" => "original.jpg"}) + # ``` + # + abstract def upload(io : IO, id : String, **options) : Nil - # Opens the file at the given location and returns an IO for reading. - # - # ``` - # io = storage.open("uploads/photo.jpg") - # content = io.gets_to_end - # io.close - # ``` - # - # Raises `Lucky::Attachment::FileNotFound` if the file doesn't exist. - # - abstract def open(id : String, **options) : IO + # Opens the file at the given location and returns an IO for reading. + # + # ``` + # io = storage.open("uploads/photo.jpg") + # content = io.gets_to_end + # io.close + # ``` + # + # Raises `Lucky::Attachment::FileNotFound` if the file doesn't exist. + # + abstract def open(id : String, **options) : IO - # Returns whether a file exists at the given location. - # - # ``` - # storage.exists?("uploads/photo.jpg") - # # => true - # ``` - # - abstract def exists?(id : String) : Bool + # Returns whether a file exists at the given location. + # + # ``` + # storage.exists?("uploads/photo.jpg") + # # => true + # ``` + # + abstract def exists?(id : String) : Bool - # Returns the URL for accessing the file at the given location. - # - # ``` - # storage.url("uploads/photo.jpg") - # # => "/uploads/photo.jpg" - # storage.url("uploads/photo.jpg", host: "https://example.com") - # # => "https://example.com/uploads/photo.jpg" - # ``` - # - abstract def url(id : String, **options) : String + # Returns the URL for accessing the file at the given location. + # + # ``` + # storage.url("uploads/photo.jpg") + # # => "/uploads/photo.jpg" + # storage.url("uploads/photo.jpg", host: "https://example.com") + # # => "https://example.com/uploads/photo.jpg" + # ``` + # + abstract def url(id : String, **options) : String - # Deletes the file at the given location. - # - # ``` - # storage.delete("uploads/photo.jpg") - # ``` - # - # Does not raise if the file doesn't exist. - # - abstract def delete(id : String) : Nil + # Deletes the file at the given location. + # + # ``` + # storage.delete("uploads/photo.jpg") + # ``` + # + # Does not raise if the file doesn't exist. + # + abstract def delete(id : String) : Nil - # Moves a file from another location. - def move(io : IO, id : String, **options) : Nil - upload(io, id, **options) - end + # Moves a file from another location. + def move(io : IO, id : String, **options) : Nil + upload(io, id, **options) end end diff --git a/src/lucky/attachment/storage/file_system.cr b/src/lucky/attachment/storage/file_system.cr index f2389184b..2e903f709 100644 --- a/src/lucky/attachment/storage/file_system.cr +++ b/src/lucky/attachment/storage/file_system.cr @@ -1,128 +1,126 @@ require "../storage" -module Lucky::Attachment - # Local filesystem storage backend. Files are stored in a directory on the - # local filesystem. Supports an optional prefix for organizing files. +# Local filesystem storage backend. Files are stored in a directory on the +# local filesystem. Supports an optional prefix for organizing files. +# +# ``` +# Lucky::Attachment.configure do |settings| +# settings.storages["cache"] = Lucky::Attachment::Storage::FileSystem.new( +# directory: "uploads", +# prefix: "cache" +# ) +# settings.storages["store"] = Lucky::Attachment::Storage::FileSystem.new( +# directory: "uploads" +# ) +# end +# ``` +# +class Lucky::Attachment::Storage::FileSystem < Lucky::Attachment::Storage::Base + DEFAULT_PERMISSIONS = File::Permissions.new(0o644) + DEFAULT_DIRECTORY_PERMISSIONS = File::Permissions.new(0o755) + + getter directory : String + getter prefix : String? + getter? clean : Bool + getter permissions : File::Permissions + getter directory_permissions : File::Permissions + + def initialize( + @directory : String, + @prefix : String? = nil, + @clean : Bool = true, + @permissions : File::Permissions = DEFAULT_PERMISSIONS, + @directory_permissions : File::Permissions = DEFAULT_DIRECTORY_PERMISSIONS, + ) + Dir.mkdir_p(expanded_directory, mode: directory_permissions.value) + end + + # Returns the full expanded path including prefix. # # ``` - # Lucky::Attachment.configure do |settings| - # settings.storages["cache"] = Lucky::Attachment::Storage::FileSystem.new( - # directory: "uploads", - # prefix: "cache" - # ) - # settings.storages["store"] = Lucky::Attachment::Storage::FileSystem.new( - # directory: "uploads" - # ) - # end + # storage.expanded_directory + # # => "/app/uploads/cache" # ``` # - class Storage::FileSystem < Storage::Base - DEFAULT_PERMISSIONS = File::Permissions.new(0o644) - DEFAULT_DIRECTORY_PERMISSIONS = File::Permissions.new(0o755) - - getter directory : String - getter prefix : String? - getter? clean : Bool - getter permissions : File::Permissions - getter directory_permissions : File::Permissions - - def initialize( - @directory : String, - @prefix : String? = nil, - @clean : Bool = true, - @permissions : File::Permissions = DEFAULT_PERMISSIONS, - @directory_permissions : File::Permissions = DEFAULT_DIRECTORY_PERMISSIONS, - ) - Dir.mkdir_p(expanded_directory, mode: directory_permissions.value) - end + def expanded_directory : String + return File.expand_path(directory) unless p = prefix - # Returns the full expanded path including prefix. - # - # ``` - # storage.expanded_directory - # # => "/app/uploads/cache" - # ``` - # - def expanded_directory : String - return File.expand_path(directory) unless p = prefix - - File.expand_path(File.join(directory, p)) - end + File.expand_path(File.join(directory, p)) + end - # Uploads an IO to the given location (id) in the storage. - def upload(io : IO, id : String, move : Bool = false, **options) : Nil - path = path_for(id) - Dir.mkdir_p(File.dirname(path), mode: directory_permissions.value) + # Uploads an IO to the given location (id) in the storage. + def upload(io : IO, id : String, move : Bool = false, **options) : Nil + path = path_for(id) + Dir.mkdir_p(File.dirname(path), mode: directory_permissions.value) - if move && io.is_a?(File) - File.rename(io.path, path) - File.chmod(path, permissions) - else - File.open(path, "wb", perm: permissions) do |file| - IO.copy(io, file) - end + if move && io.is_a?(File) + File.rename(io.path, path) + File.chmod(path, permissions) + else + File.open(path, "wb", perm: permissions) do |file| + IO.copy(io, file) end end + end - # Opens the file at the given location and returns an IO for reading. - def open(id : String, **options) : IO - File.open(path_for(id), "rb") - rescue ex : File::NotFoundError - raise FileNotFound.new("File not found: #{id}") - end + # Opens the file at the given location and returns an IO for reading. + def open(id : String, **options) : IO + File.open(path_for(id), "rb") + rescue ex : File::NotFoundError + raise FileNotFound.new("File not found: #{id}") + end - # Returns whether a file exists at the given location. - def exists?(id : String) : Bool - File.exists?(path_for(id)) - end + # Returns whether a file exists at the given location. + def exists?(id : String) : Bool + File.exists?(path_for(id)) + end - # Returns the full filesystem path for the given id. - # - # ``` - # storage.path_for("abc123.jpg") - # # => "/app/uploads/abc123.jpg" - # ``` - # - def path_for(id : String) : String - File.join(expanded_directory, id.gsub('/', File::SEPARATOR)) - end + # Returns the full filesystem path for the given id. + # + # ``` + # storage.path_for("abc123.jpg") + # # => "/app/uploads/abc123.jpg" + # ``` + # + def path_for(id : String) : String + File.join(expanded_directory, id.gsub('/', File::SEPARATOR)) + end - def url(id : String, host : String? = nil, **options) : String - String.build do |url| - url << host.rstrip('/') if host - url << '/' - if p = prefix - url << p.lstrip('/') << '/' - end - url << id + def url(id : String, host : String? = nil, **options) : String + String.build do |url| + url << host.rstrip('/') if host + url << '/' + if p = prefix + url << p.lstrip('/') << '/' end + url << id end + end - # Deletes the file at the given location. - def delete(id : String) : Nil - path = path_for(id) - File.delete?(path) - clean_directories(path) if clean? - rescue ex : File::Error - # Ignore errors here - end + # Deletes the file at the given location. + def delete(id : String) : Nil + path = path_for(id) + File.delete?(path) + clean_directories(path) if clean? + rescue ex : File::Error + # Ignore errors here + end - # Override move for efficient file system rename - def move(io : IO, id : String, **options) : Nil - upload(io, id, **options, move: io.is_a?(File)) - end + # Override move for efficient file system rename + def move(io : IO, id : String, **options) : Nil + upload(io, id, **options, move: io.is_a?(File)) + end - # Cleans empty parent directories up to the expanded_directory. - private def clean_directories(path : String) : Nil - current = File.dirname(path) + # Cleans empty parent directories up to the expanded_directory. + private def clean_directories(path : String) : Nil + current = File.dirname(path) - while current != expanded_directory && current.starts_with?(expanded_directory) - break unless Dir.empty?(current) - Dir.delete(current) - current = File.dirname(current) - end - rescue ex : File::Error - # Ignore errors here + while current != expanded_directory && current.starts_with?(expanded_directory) + break unless Dir.empty?(current) + Dir.delete(current) + current = File.dirname(current) end + rescue ex : File::Error + # Ignore errors here end end diff --git a/src/lucky/attachment/storage/memory.cr b/src/lucky/attachment/storage/memory.cr index 3d9e2ce99..5f3038fd1 100644 --- a/src/lucky/attachment/storage/memory.cr +++ b/src/lucky/attachment/storage/memory.cr @@ -1,67 +1,65 @@ require "../storage" -module Lucky::Attachment - # In-memory storage backend for testing purposes. Files are stored in a hash - # and are lost when the process exits. This is useful for testing without - # hitting the filesystem or network. - # - # ``` - # Lucky::Attachment.configure do |settings| - # settings.storages["cache"] = Lucky::Attachment::Storage::Memory.new - # settings.storages["store"] = Lucky::Attachment::Storage::Memory.new - # end - # ``` - # - class Storage::Memory < Storage::Base - getter store : Hash(String, Bytes) - getter base_url : String? +# In-memory storage backend for testing purposes. Files are stored in a hash +# and are lost when the process exits. This is useful for testing without +# hitting the filesystem or network. +# +# ``` +# Lucky::Attachment.configure do |settings| +# settings.storages["cache"] = Lucky::Attachment::Storage::Memory.new +# settings.storages["store"] = Lucky::Attachment::Storage::Memory.new +# end +# ``` +# +class Lucky::Attachment::Storage::Memory < Lucky::Attachment::Storage::Base + getter store : Hash(String, Bytes) + getter base_url : String? - def initialize(@base_url : String? = nil) - @store = {} of String => Bytes - end + def initialize(@base_url : String? = nil) + @store = {} of String => Bytes + end - # Uploads an IO to the given location (id) in the storage. - def upload(io : IO, id : String, **options) : Nil - @store[id] = io.getb_to_end - end + # Uploads an IO to the given location (id) in the storage. + def upload(io : IO, id : String, **options) : Nil + @store[id] = io.getb_to_end + end - # Opens the file at the given location and returns an IO for reading. - def open(id : String, **options) : IO - if bytes = @store[id]? - IO::Memory.new(bytes) - else - raise FileNotFound.new("File not found: #{id}") - end + # Opens the file at the given location and returns an IO for reading. + def open(id : String, **options) : IO + if bytes = @store[id]? + IO::Memory.new(bytes) + else + raise FileNotFound.new("File not found: #{id}") end + end - # Returns whether a file exists at the given location. - def exists?(id : String) : Bool - @store.has_key?(id) - end + # Returns whether a file exists at the given location. + def exists?(id : String) : Bool + @store.has_key?(id) + end - # Returns the URL for accessing the file at the given location. - def url(id : String, **options) : String - String.build do |io| - if base = @base_url - io << base.rstrip('/') - end - io << '/' << id + # Returns the URL for accessing the file at the given location. + def url(id : String, **options) : String + String.build do |io| + if base = @base_url + io << base.rstrip('/') end + io << '/' << id end + end - # Deletes the file at the given location. - def delete(id : String) : Nil - @store.delete(id) - end + # Deletes the file at the given location. + def delete(id : String) : Nil + @store.delete(id) + end - # Clears out the store. - def clear! : Nil - @store.clear - end + # Clears out the store. + def clear! : Nil + @store.clear + end - # Returns the number of stored files. - def size : Int32 - @store.size - end + # Returns the number of stored files. + def size : Int32 + @store.size end end diff --git a/src/lucky/attachment/stored_file.cr b/src/lucky/attachment/stored_file.cr index 4b7925f0a..08e5b3fbe 100644 --- a/src/lucky/attachment/stored_file.cr +++ b/src/lucky/attachment/stored_file.cr @@ -1,278 +1,273 @@ require "json" require "uuid" -module Lucky::Attachment - alias MetadataValue = String | Int32 | Int64 | UInt32 | UInt64 | Float64 | Bool | Nil - alias MetadataHash = Hash(String, MetadataValue) +# Represents a file that has been uploaded to a storage backend. +# +# This class is JSON serializable and stores the file's location (`id`), +# which storage it's in (`storage`), and associated metadata. +# +# NOTE: The JSON format is compatible with Shrine.rb/Shrine.cr: +# +# ```json +# { +# "id": "uploads/abc123.jpg", +# "storage": "store", +# "metadata": { +# "filename": "photo.jpg", +# "size": 102400, +# "mime_type": "image/jpeg" +# } +# } +# ``` +# +class Lucky::Attachment::StoredFile + include JSON::Serializable - # Represents a file that has been uploaded to a storage backend. - # - # This class is JSON serializable and stores the file's location (`id`), - # which storage it's in (`storage`), and associated metadata. - # - # NOTE: The JSON format is compatible with Shrine.rb/Shrine.cr: - # - # ```json - # { - # "id": "uploads/abc123.jpg", - # "storage": "store", - # "metadata": { - # "filename": "photo.jpg", - # "size": 102400, - # "mime_type": "image/jpeg" - # } - # } - # ``` - # - class StoredFile - include JSON::Serializable - - # NOTE: This mimics the behavior of Avram's `JSON::Serializable` extension. - def self.adapter - Lucky(self) - end + # NOTE: This mimics the behavior of Avram's `JSON::Serializable` extension. + def self.adapter + Lucky(self) + end - getter id : String - @[JSON::Field(key: "storage")] - getter storage_key : String - getter metadata : MetadataHash + getter id : String + @[JSON::Field(key: "storage")] + getter storage_key : String + getter metadata : MetadataHash - @[JSON::Field(ignore: true)] - @io : IO? + @[JSON::Field(ignore: true)] + @io : IO? - def initialize( - @id : String, - @storage_key : String, - @metadata : MetadataHash = MetadataHash.new, - ) - end + def initialize( + @id : String, + @storage_key : String, + @metadata : MetadataHash = MetadataHash.new, + ) + end - # Returns the original filename from metadata. - # - # ``` - # file.original_filename - # # => "photo.jpg" - # ``` - # - def original_filename : String? - metadata["filename"]?.try(&.as(String)) - end + # Returns the original filename from metadata. + # + # ``` + # file.original_filename + # # => "photo.jpg" + # ``` + # + def original_filename : String? + metadata["filename"]?.try(&.as(String)) + end - # Returns the file extension based on the id or original filename. - # - # ``` - # file.extension - # # => "jpg" - # ``` - # - def extension : String? - ext = File.extname(id).lchop('.') - if ext.empty? && original_filename - ext = File.extname(original_filename.to_s).lchop('.') - end - ext.presence.try(&.downcase) + # Returns the file extension based on the id or original filename. + # + # ``` + # file.extension + # # => "jpg" + # ``` + # + def extension : String? + ext = File.extname(id).lchop('.') + if ext.empty? && original_filename + ext = File.extname(original_filename.to_s).lchop('.') end + ext.presence.try(&.downcase) + end - # Returns the file size in bytes from metadata. - # - # ``` - # file.size - # # => 102400 - # ``` - # - def size : Int64? - case value = metadata["size"]? - when Int32 then value.to_i64 - when Int64 then value - when String then value.to_i64? - else nil - end + # Returns the file size in bytes from metadata. + # + # ``` + # file.size + # # => 102400 + # ``` + # + def size : Int64? + case value = metadata["size"]? + when Int32 then value.to_i64 + when Int64 then value + when String then value.to_i64? + else nil end + end - # Returns the MIME type from metadata. - # - # ``` - # file.mime_type - # # => "image/jpeg" - # ``` - # - def mime_type : String? - metadata["mime_type"]?.try(&.as(String)) - end + # Returns the MIME type from metadata. + # + # ``` + # file.mime_type + # # => "image/jpeg" + # ``` + # + def mime_type : String? + metadata["mime_type"]?.try(&.as(String)) + end - # Access arbitrary metadata values. - # - # ``` - # file["width"] - # # => 800 - # file["custom"] - # # => "value" - # ``` - # - def [](key : String) : MetadataValue - metadata[key]? - end + # Access arbitrary metadata values. + # + # ``` + # file["width"] + # # => 800 + # file["custom"] + # # => "value" + # ``` + # + def [](key : String) : MetadataValue + metadata[key]? + end - # Returns the storage instance this file is stored in. - def storage : Storage::Base - Lucky::Attachment.find_storage(storage_key) - end + # Returns the storage instance this file is stored in. + def storage : Storage::Base + ::Lucky::Attachment.find_storage(storage_key) + end - # Returns the URL for accessing this file. - # - # ``` - # file.url - # # => "https://bucket.s3.amazonaws.com/uploads/abc123.jpg" - # - # # for presigned URLs - # file.url(expires_in: 1.hour) - # ``` - # - def url(**options) : String - storage.url(id, **options) - end + # Returns the URL for accessing this file. + # + # ``` + # file.url + # # => "https://bucket.s3.amazonaws.com/uploads/abc123.jpg" + # + # # for presigned URLs + # file.url(expires_in: 1.hour) + # ``` + # + def url(**options) : String + storage.url(id, **options) + end - # Returns whether this file exists in storage. - # - # ``` - # file.exists? # => true - # ``` - # - def exists? : Bool - storage.exists?(id) - end + # Returns whether this file exists in storage. + # + # ``` + # file.exists? # => true + # ``` + # + def exists? : Bool + storage.exists?(id) + end - # Opens the file for reading. If a block is given, yields the IO and - # automatically closes it afterwards. Returns the block's return value. - # - # ``` - # file.open do |io| - # io.gets_to_end - # end - # ``` - # - def open(**options, &) - io = storage.open(id, **options) - begin - yield io - ensure - io.close - end + # Opens the file for reading. If a block is given, yields the IO and + # automatically closes it afterwards. Returns the block's return value. + # + # ``` + # file.open do |io| + # io.gets_to_end + # end + # ``` + # + def open(**options, &) + io = storage.open(id, **options) + begin + yield io + ensure + io.close end + end - # Opens the file and stores the IO handle internally for subsequent reads. - # Remember to call `close` when done. - # - # ``` - # file.open - # content = file.io.gets_to_end - # file.close - # ``` - def open(**options) : IO - close if @io - @io = storage.open(id, **options) - end + # Opens the file and stores the IO handle internally for subsequent reads. + # Remember to call `close` when done. + # + # ``` + # file.open + # content = file.io.gets_to_end + # file.close + # ``` + def open(**options) : IO + close if @io + @io = storage.open(id, **options) + end - # Returns the currently opened IO, or opens it if not already open. - def io : IO - @io || open - end + # Returns the currently opened IO, or opens it if not already open. + def io : IO + @io || open + end - # Closes the file if it is open. - def close : Nil - @io.try(&.close) - @io = nil - end + # Closes the file if it is open. + def close : Nil + @io.try(&.close) + @io = nil + end - # Tests whether the file has been opened or not. - def opened? : Bool - !@io.nil? - end + # Tests whether the file has been opened or not. + def opened? : Bool + !@io.nil? + end - # Downloads the file to a temporary file and returns it. As opposed to the - # block variant, this temporary file needs to be closed and deleted - # manually: - # - # ``` - # tempfile = file.download - # tempfile.path - # # => "/tmp/lucky-attachment123456789.jpg" - # tempfile.gets_to_end - # # => "file content" - # tempfile.close - # tempfile.delete - # ``` - # - def download(**options) : File - tempfile = File.tempfile("lucky-attachment", ".#{extension}") - stream(tempfile, **options) - tempfile.rewind - tempfile - end + # Downloads the file to a temporary file and returns it. As opposed to the + # block variant, this temporary file needs to be closed and deleted + # manually: + # + # ``` + # tempfile = file.download + # tempfile.path + # # => "/tmp/lucky-attachment123456789.jpg" + # tempfile.gets_to_end + # # => "file content" + # tempfile.close + # tempfile.delete + # ``` + # + def download(**options) : File + tempfile = File.tempfile("lucky-attachment", ".#{extension}") + stream(tempfile, **options) + tempfile.rewind + tempfile + end - # Downloads to a tempfile, yields it to the block, then cleans up. - # - # ``` - # file.download do |tempfile| - # process(tempfile.path) - # end - # # tempfile is automatically deleted - # ``` - # - def download(**options, &) - tempfile = download(**options) - begin - yield tempfile - ensure - tempfile.close - tempfile.delete - end + # Downloads to a tempfile, yields it to the block, then cleans up. + # + # ``` + # file.download do |tempfile| + # process(tempfile.path) + # end + # # tempfile is automatically deleted + # ``` + # + def download(**options, &) + tempfile = download(**options) + begin + yield tempfile + ensure + tempfile.close + tempfile.delete end + end - # Streams the file content to the given IO destination. - # - # ``` - # file.stream(response.output) - # ``` - # - def stream(destination : IO, **options) : Nil - if opened? + # Streams the file content to the given IO destination. + # + # ``` + # file.stream(response.output) + # ``` + # + def stream(destination : IO, **options) : Nil + if opened? + IO.copy(io, destination) + io.rewind if io.responds_to?(:rewind) + else + open(**options) do |io| IO.copy(io, destination) - io.rewind if io.responds_to?(:rewind) - else - open(**options) do |io| - IO.copy(io, destination) - end end end + end - # Deletes the file from storage. - # - # ``` - # file.delete - # ``` - # - def delete : Nil - storage.delete(id) - end + # Deletes the file from storage. + # + # ``` + # file.delete + # ``` + # + def delete : Nil + storage.delete(id) + end - # Returns a hash representation suitable for JSON serialization compatible - # with Shrine. - def data : Hash(String, String | MetadataHash) - { - "id" => id, - "metadata" => metadata, - "storage" => storage_key, - } - end + # Returns a hash representation suitable for JSON serialization compatible + # with Shrine. + def data : Hash(String, String | MetadataHash) + { + "id" => id, + "metadata" => metadata, + "storage" => storage_key, + } + end - # Compares two `UploadedFiles` by thier id and storage. - def ==(other : UploadedFile) : Bool - id == other.id && storage_key == other.storage_key - end + # Compares two `StoredFile` by thier id and storage. + def ==(other : StoredFile) : Bool + id == other.id && storage_key == other.storage_key + end - def ==(other) : Bool - false - end + def ==(other) : Bool + false end end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 63d591680..a146d9749 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -1,185 +1,181 @@ require "uuid" -module Lucky::Attachment - # Base uploader class that handles file uploads with metadata extraction and - # location generation. +# Base uploader class that handles file uploads with metadata extraction and +# location generation. +# +# ``` +# struct ImageUploader < Lucky::Attachment::Uploader +# def generate_location(io, metadata, **options) : String +# date = Time.utc.to_s("%Y/%m/%d") +# File.join("images", date, super) +# end +# end +# +# ImageUploader.new("store").upload(io) +# # => Lucky::Attachment::StoredFile with id "images/2024/01/15/abc123.jpg" +# ``` +# +abstract struct Lucky::Attachment::Uploader + getter storage_key : String + + def initialize(@storage_key : String) + end + + # Returns the storage instance for this uploader. + def storage : Storage::Base + Lucky::Attachment.find_storage(storage_key) + end + + # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method + # accepts additional metadata and arbitrary arguments for overrides. # # ``` - # struct ImageUploader < Lucky::Attachment::Uploader - # def generate_location(io, metadata, **options) : String - # date = Time.utc.to_s("%Y/%m/%d") - # File.join("images", date, super) - # end - # end - # - # ImageUploader.new("store").upload(io) - # # => Lucky::Attachment::StoredFile with id "images/2024/01/15/abc123.jpg" + # uploader.upload(io) + # uploader.upload(io, metadata: {"custom" => "value"}) + # uploader.upload(io, location: "custom/path.jpg") # ``` # - abstract struct Uploader - getter storage_key : String + def upload(io : IO, metadata : MetadataHash? = nil, **options) : StoredFile + data = extract_metadata(io, metadata, **options) + data = data.merge(metadata) if metadata + location = options[:location]? || generate_location(io, data, **options) - def initialize(@storage_key : String) - end - - # Returns the storage instance for this uploader. - def storage : Storage::Base - Lucky::Attachment.find_storage(storage_key) - end - - # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method - # accepts additional metadata and arbitrary arguments for overrides. - # - # ``` - # uploader.upload(io) - # uploader.upload(io, metadata: {"custom" => "value"}) - # uploader.upload(io, location: "custom/path.jpg") - # ``` - # - def upload(io : IO, metadata : MetadataHash? = nil, **options) : StoredFile - data = extract_metadata(io, metadata, **options) - data = data.merge(metadata) if metadata - location = options[:location]? || generate_location(io, data, **options) - - storage.upload(io, location, **options.merge(metadata: data)) - StoredFile.new(id: location, storage_key: storage_key, metadata: data) - ensure - io.close if options[:close]?.nil? || options[:close]? - end + storage.upload(io, location, **options.merge(metadata: data)) + StoredFile.new(id: location, storage_key: storage_key, metadata: data) + end - # Uploads to the "cache" storage. - # - # ``` - # cached = ImageUploader.cache(io) - # ``` - def self.cache(io : IO, **options) : StoredFile - new("cache").upload(io, **options) - end + # Uploads to the "cache" storage. + # + # ``` + # cached = ImageUploader.cache(io) + # ``` + def self.cache(io : IO, **options) : StoredFile + new("cache").upload(io, **options) + end - # Uploads to the "store" storage. - # - # ``` - # stored = ImageUploader.store(io) - # ``` - # - def self.store(io : IO, **options) : StoredFile - new("store").upload(io, **options) - end + # Uploads to the "store" storage. + # + # ``` + # stored = ImageUploader.store(io) + # ``` + # + def self.store(io : IO, **options) : StoredFile + new("store").upload(io, **options) + end - # Promotes a file from cache to store. - # - # ``` - # cached = ImageUploader.cache(io) - # stored = ImageUploader.promote(cached) - # ``` - # - def self.promote( - file : StoredFile, - to storage : String = "store", - delete_source : Bool = true, + # Promotes a file from cache to store. + # + # ``` + # cached = ImageUploader.cache(io) + # stored = ImageUploader.promote(cached) + # ``` + # + def self.promote( + file : StoredFile, + to storage : String = "store", + delete_source : Bool = true, + **options, + ) : StoredFile + Lucky::Attachment.promote( + file, **options, - ) : StoredFile - Lucky::Attachment.promote( - file, - **options, - to: storage, - delete_source: delete_source - ) - end + to: storage, + delete_source: delete_source + ) + end - # Generates a unique location for the uploaded file. Override this in - # subclasses for custom locations. - # - # ``` - # class ImageUploader < Lucky::Attachment::Uploader - # def generate_location(io, metadata, **options) : String - # File.join("images", super) - # end - # end - # ``` - # - def generate_location(io : IO, metadata : MetadataHash, **options) : String - extension = extract_extension(io, metadata) - basename = generate_uid - extension ? "#{basename}.#{extension}" : basename - end + # Generates a unique location for the uploaded file. Override this in + # subclasses for custom locations. + # + # ``` + # class ImageUploader < Lucky::Attachment::Uploader + # def generate_location(io, metadata, **options) : String + # File.join("images", super) + # end + # end + # ``` + # + def generate_location(io : IO, metadata : MetadataHash, **options) : String + extension = extract_extension(io, metadata) + basename = generate_uid + extension ? "#{basename}.#{extension}" : basename + end - # Extracts metadata from the IO. Override in subclasses to add custom - # metadata extraction. - # - # ``` - # class ImageUploader < Lucky::Attachment::Uploader - # def extract_metadata(io, metadata : MetadataHash? = nil, **options) : MetadataHash - # data = super - # # Add custom metadata - # data["custom"] = "value" - # data - # end - # end - # ``` - # - def extract_metadata( - io : IO, - metadata : MetadataHash? = nil, - **options, - ) : MetadataHash - MetadataHash{ - "filename" => options[:filename]? || extract_filename(io), - "size" => extract_size(io), - "mime_type" => extract_mime_type(io), - } - end + # Extracts metadata from the IO. Override in subclasses to add custom + # metadata extraction. + # + # ``` + # class ImageUploader < Lucky::Attachment::Uploader + # def extract_metadata(io, metadata : MetadataHash? = nil, **options) : MetadataHash + # data = super + # # Add custom metadata + # data["custom"] = "value" + # data + # end + # end + # ``` + # + def extract_metadata( + io : IO, + metadata : MetadataHash? = nil, + **options, + ) : MetadataHash + MetadataHash{ + "filename" => options[:filename]? || extract_filename(io), + "size" => extract_size(io), + "mime_type" => extract_mime_type(io), + } + end - # Generates a unique identifier for file locations. - protected def generate_uid : String - UUID.random.to_s - end + # Generates a unique identifier for file locations. + protected def generate_uid : String + UUID.random.to_s + end - # Extracts the filename from the IO if available. - protected def extract_filename(io : IO) : String? - if io.responds_to?(:original_filename) - io.original_filename - elsif io.responds_to?(:filename) - io.filename.presence - elsif io.responds_to?(:path) - File.basename(io.path) - end + # Extracts the filename from the IO if available. + protected def extract_filename(io : IO) : String? + if io.responds_to?(:original_filename) + io.original_filename + elsif io.responds_to?(:filename) + io.filename.presence + elsif io.responds_to?(:path) + File.basename(io.path) end + end - # Extracts the file size from the IO, if available. - protected def extract_size(io : IO) : Int64? - if io.responds_to?(:tempfile) - io.tempfile.size - elsif io.responds_to?(:size) - io.size.to_i64 - end + # Extracts the file size from the IO, if available. + protected def extract_size(io : IO) : Int64? + if io.responds_to?(:tempfile) + io.tempfile.size + elsif io.responds_to?(:size) + io.size.to_i64 end + end - # Extracts the MIME type from the IO if available. - # - # NOTE: This relies on the IO providing content_type, which typically comes - # from HTTP headers and may not be accurate, but it's a good fallback. - # - protected def extract_mime_type(io : IO) : String? - return unless io.responds_to?(:content_type) && (type = io.content_type) + # Extracts the MIME type from the IO if available. + # + # NOTE: This relies on the IO providing content_type, which typically comes + # from HTTP headers and may not be accurate, but it's a good fallback. + # + protected def extract_mime_type(io : IO) : String? + return unless io.responds_to?(:content_type) && (type = io.content_type) + + type.split(';').first.strip + end - type.split(';').first.strip + # Extracts file extension from the IO or metadata. + protected def extract_extension( + io : IO, + metadata : MetadataHash, + ) : String? + if filename = metadata["filename"]?.try(&.as(String)) + ext = File.extname(filename).lchop('.') + return ext.downcase unless ext.empty? end - # Extracts file extension from the IO or metadata. - protected def extract_extension( - io : IO, - metadata : MetadataHash, - ) : String? - if filename = metadata["filename"]?.try(&.as(String)) - ext = File.extname(filename).lchop('.') - return ext.downcase unless ext.empty? - end - - if io.responds_to?(:path) - ext = File.extname(io.path).lchop('.') - return ext.downcase unless ext.empty? - end + if io.responds_to?(:path) + ext = File.extname(io.path).lchop('.') + return ext.downcase unless ext.empty? end end end diff --git a/src/lucky/attachment/utilities.cr b/src/lucky/attachment/utilities.cr new file mode 100644 index 000000000..75d83f1e5 --- /dev/null +++ b/src/lucky/attachment/utilities.cr @@ -0,0 +1,75 @@ +module Lucky::Attachment + # Move a file from one storage to another (typically cache -> store). + # + # ``` + # stored = Lucky::Attachment.promote(cached, to: "store") + # ``` + # + def self.promote( + file : StoredFile, + to storage : String = "store", + delete_source : Bool = true, + ) : StoredFile + file.open do |io| + find_storage(storage).upload(io, file.id, metadata: file.metadata) + promoted = StoredFile.new( + id: file.id, + storage_key: storage, + metadata: file.metadata + ) + file.delete if delete_source + promoted + end + end + + # Deserialize an StoredFile from various sources. + # + # ``` + # Lucky::Attachment.uploaded_file(json_string) + # Lucky::Attachment.uploaded_file(json_any) + # Lucky::Attachment.uploaded_file(uploaded_file) + # ``` + # + def self.uploaded_file(json : String) : StoredFile + StoredFile.from_json(json) + end + + def self.uploaded_file(json : JSON::Any) : StoredFile + StoredFile.from_json(json.to_json) + end + + def self.uploaded_file(file : StoredFile) : StoredFile + file + end + + def self.uploaded_file(value : Nil) : Nil + nil + end + + # Utility to work with a file IO. If the IO is already a File, yields it + # directly, Otherwise copies to a tempfile, yields it, then cleans up. + # + # ``` + # Lucky::Attachment.with_file(io) do |file| + # # file is guaranteed to be a File with a path + # end + # ``` + # + def self.with_file(io : IO, &) + if io.is_a?(File) + yield io + else + File.tempfile("lucky-attachment") do |tempfile| + IO.copy(io, tempfile) + tempfile.rewind + yield tempfile + end + end + end + + def self.with_file(uploaded_file : StoredFile, &) + uploaded_file.download do |tempfile| + yield tempfile + end + end +end From 62e947ad58571e57ae72b8fefa0f4c99ea9e6ada Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 22 Feb 2026 10:55:57 +0100 Subject: [PATCH 13/52] Add configurable path prefixes for attachments This feature makes it possible to configure dynamic path prefixes globally or per attachment. For example: ":model/:id/:attachment", which would then resolbve to somethign like "user_profile/123/avatar". --- src/lucky/attachment/config.cr | 15 ++++++++++++++- src/lucky/attachment/uploader.cr | 3 ++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/lucky/attachment/config.cr b/src/lucky/attachment/config.cr index 66c42be07..37ee8e4c9 100644 --- a/src/lucky/attachment/config.cr +++ b/src/lucky/attachment/config.cr @@ -3,8 +3,21 @@ require "./storage" module Lucky::Attachment Habitat.create do - # Storage configurations keyed by name ("cache", "store", etc.) + # Storage configurations keyed by name. The default storages are typically: + # - "cache" (temporary storage between requests to avoid re-uploads) + # - "store" (where uploads are moved from the cache after a commit) + # + # NOTE: Additional stores are not supported yet. Please reach out if that + # is something you need. + # setting storages : Hash(String, Storage::Base) = {} of String => Storage::Base + + # Path prefix for uploads. Possible keywords are: + # - `:model` (an underscored string of the model name) + # - `:id` (the record's primary key value) + # - `:attachment` (the name of the attachment; e.g. "avatar") + # + setting path_prefix : String = ":model/:id/:attachment" end # Retrieves a storage by name, raising if not found. diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index a146d9749..30fc34fcd 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -98,7 +98,8 @@ abstract struct Lucky::Attachment::Uploader def generate_location(io : IO, metadata : MetadataHash, **options) : String extension = extract_extension(io, metadata) basename = generate_uid - extension ? "#{basename}.#{extension}" : basename + filename = extension ? "#{basename}.#{extension}" : basename + File.join([options[:path_prefix]?, filename].compact) end # Extracts metadata from the IO. Override in subclasses to add custom From 25382fbe5c0ef919ea0cb2c3868baf2c8659fde8 Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 22 Feb 2026 15:40:12 +0100 Subject: [PATCH 14/52] Fix code styling issues --- spec/lucky/attachment/stored_file_spec.cr | 6 +++--- spec/lucky/attachment/uploader_spec.cr | 4 ++-- src/lucky/attachment/config.cr | 2 +- src/lucky/uploaded_file.cr | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/lucky/attachment/stored_file_spec.cr b/spec/lucky/attachment/stored_file_spec.cr index 71edc90b4..79b786982 100644 --- a/spec/lucky/attachment/stored_file_spec.cr +++ b/spec/lucky/attachment/stored_file_spec.cr @@ -156,7 +156,7 @@ describe Lucky::Attachment::StoredFile do storage_key: "store" ) - file.open { |io| io.gets_to_end.should eq("file content") } + file.open(&.gets_to_end.should(eq("file content"))) end it "closes the IO after the block" do @@ -165,7 +165,7 @@ describe Lucky::Attachment::StoredFile do captured_io = nil file.open { |io| captured_io = io } - captured_io.not_nil!.closed?.should be_true + captured_io.as(IO).closed?.should be_true end it "closes the IO even if the block raises" do @@ -179,7 +179,7 @@ describe Lucky::Attachment::StoredFile do raise "oops" end end - captured_io.not_nil!.closed?.should be_true + captured_io.as(IO).closed?.should be_true end describe "#download" do diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index b7b9f1f2f..3e4ee96bf 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -73,7 +73,7 @@ describe Lucky::Attachment::Uploader do context "with a File IO" do it "extracts filename from path" do - file = File.tempfile("myfile", ".txt") { |f| f.print("content") } + file = File.tempfile("myfile", ".txt", &.print("content")) uploaded = TestUploader.new("store").upload(File.open(file.path)) uploaded.original_filename.should eq(File.basename(file.path)) @@ -82,7 +82,7 @@ describe Lucky::Attachment::Uploader do end it "extracts size" do - file = File.tempfile("myfile", ".txt") { |f| f.print("content") } + file = File.tempfile("myfile", ".txt", &.print("content")) uploaded = TestUploader.new("store").upload(File.open(file.path)) uploaded.size.should eq(7) diff --git a/src/lucky/attachment/config.cr b/src/lucky/attachment/config.cr index 37ee8e4c9..ec0f25dff 100644 --- a/src/lucky/attachment/config.cr +++ b/src/lucky/attachment/config.cr @@ -36,7 +36,7 @@ module Lucky::Attachment else io << %(Storage ) << name.inspect io << %( is not registered. The available storages are: ) - io << settings.storages.keys.map { |s| s.inspect }.join(", ") + io << settings.storages.keys.map(&.inspect).join(", ") end end ) diff --git a/src/lucky/uploaded_file.cr b/src/lucky/uploaded_file.cr index b5e8eff9d..9f9d7a424 100644 --- a/src/lucky/uploaded_file.cr +++ b/src/lucky/uploaded_file.cr @@ -47,7 +47,7 @@ class Lucky::UploadedFile # ``` # def content_type : String? - @part.headers["Content-Type"]?.try { |t| t.split(';').first.strip } + @part.headers["Content-Type"]?.try(&.split(';').first.strip) end # Avram::Uploadable needs to be updated when this is removed From 87d1a0a3835395a305f14326abe34c093978f981 Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 22 Feb 2026 15:57:22 +0100 Subject: [PATCH 15/52] Add avram modules --- src/lucky/attachment/avram.cr | 157 ++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 src/lucky/attachment/avram.cr diff --git a/src/lucky/attachment/avram.cr b/src/lucky/attachment/avram.cr new file mode 100644 index 000000000..cbfaebaa9 --- /dev/null +++ b/src/lucky/attachment/avram.cr @@ -0,0 +1,157 @@ +module Avram::Attachment::Model + macro included + class ::{{ @type }}::SaveOperation < Avram::SaveOperation({{ @type }}) + include Avram::Attachment::SaveOperation + end + + macro finished + class ::{{ @type }}::DeleteOperation < Avram::DeleteOperation({{ @type }}) + include Avram::Attachment::DeleteOperation + end + end + end + + # Registers a serializable column for an attachment and takes and uploader + # class as the type. + # + # ``` + # attach avatar : ImageUploader + # # or + # attach avatar : ImageUploader? + # ``` + # + # It is assumed that a `jsonb` column exists with the same name. So in your + # migration, you'll need to add the column as follows: + # + # ``` + # add avatar : JSON::Any + # # or + # add avatar : JSON::Any? + # ``` + # + # The data of a stored file can then be accessed through the `avatar` method: + # + # ``` + # user.avatar.class + # # => Lucky::Attachment::StoredFile + # + # user.avatar.url + # # => "https://bucket.s3.amazonaws.com/user/1/avatar/abc123.jpg" + # + # # for presigned URLs + # user.avatar.url(expires_in: 1.hour) + # ``` + # + # The path prefix of an attachment can be customised globally in the + # settings, but also on attachment level: + # + # ``` + # attach avatar : ImageUploader?, path_prefix: ":model/images/:id" + # ``` + # + macro attach(type_declaration, path_prefix = nil) + {% name = type_declaration.var %} + {% if type_declaration.type.is_a?(Union) %} + {% uploader = type_declaration.type.types.first %} + {% nilable = true %} + {% else %} + {% uploader = type_declaration.type %} + {% nilable = false %} + {% end %} + + # Registers a path prefix for the attachment. + {% if !@type.constant(:ATTACHMENT_PREFIXES) %} + ATTACHMENT_PREFIXES = {} of Symbol => String + {% end %} + path_prefix = {{ path_prefix }} || ::Lucky::Attachment.settings.path_prefix + ATTACHMENT_PREFIXES[:{{ name }}] = path_prefix + .gsub(/:model/, {{ @type.stringify.gsub(/::/, "_").underscore }}) + .gsub(/:attachment/, {{ name.stringify }}) + + # Registers the configured uploader class for the attachment. + {% if !@type.constant(:ATTACHMENT_UPLOADERS) %} + ATTACHMENT_UPLOADERS = {} of Symbol => ::Lucky::Attachment::Uploader.class + {% end %} + ATTACHMENT_UPLOADERS[:{{ name }}] = {{ uploader }} + + column {{ name }} : ::Lucky::Attachment::StoredFile{% if nilable %}?{% end %}, serialize: true + end +end + +module Avram::Attachment::SaveOperation + # Registers a file attribute for an existing attachment on the model. + # + # ``` + # # The field name in the form will be "avatar_file" + # attach avatar + # + # # With a custom field name + # attach avatar, field_name: "avatar_upload" + # ``` + # + # The attachment will then be uploaded to the cache store, and after + # committing to the database the attachment will be moved to the permanent + # storage. + # + macro attach(name, field_name = nil) + {% + field_name = "#{name}_file".id if field_name.nil? + + unless column = T.constant(:COLUMNS).find { |c| c[:name].stringify == name.stringify } + raise %(The `#{T.name}` model does not have a column named `#{name}`) + end + %} + + file_attribute :{{ field_name }} + + {% if nilable = column[:nilable] %} + attribute delete_{{ name }} : Bool = false + {% end %} + + before_save __cache_{{ field_name }} + after_commit __process_{{ field_name }} + + # Moves uploaded file to the cache storage. + private def __cache_{{ field_name }} : Nil + {% if nilable %} + {{ name }}.value = nil if delete_{{ name }}.value + {% end %} + + return unless upload = {{ field_name }}.value + + record_id = {{ T.constant(:PRIMARY_KEY_NAME).id }}.value + {{ name }}.value = T::ATTACHMENT_UPLOADERS[:{{ name }}].cache( + upload.tempfile, + path_prefix: T::ATTACHMENT_PREFIXES[:{{ name }}].gsub(/:id/, record_id), + filename: upload.filename.presence + ) + end + + # Deletes or promotes the attachment and updates the record. + private def __process_{{ field_name }}(record) : Nil + {% if nilable %} + if delete_{{ name }}.value && (file = {{ name }}.original_value) + file.delete + end + {% end %} + + return unless {{ field_name }}.value && (cached = {{ name }}.value) + + stored = T::ATTACHMENT_UPLOADERS[:{{ name }}].promote(cached) + T::SaveOperation.update!(record, {{ name }}: stored) + end + end +end + +module Avram::Attachment::DeleteOperation + # Cleans up the files of any attachments this records still has. + macro included + after_delete do |_| + {% for name in T.constant(:ATTACHMENT_UPLOADERS) %} + if attachment = {{ name }}.value + attachment.delete + end + {% end %} + end + end +end From 442c0e3b0c78644e40a5d2967c60372c867be7db Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 22 Feb 2026 15:59:39 +0100 Subject: [PATCH 16/52] Fix code styling issues --- src/lucky/attachment/avram.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lucky/attachment/avram.cr b/src/lucky/attachment/avram.cr index cbfaebaa9..34beef287 100644 --- a/src/lucky/attachment/avram.cr +++ b/src/lucky/attachment/avram.cr @@ -97,11 +97,11 @@ module Avram::Attachment::SaveOperation {% field_name = "#{name}_file".id if field_name.nil? - unless column = T.constant(:COLUMNS).find { |c| c[:name].stringify == name.stringify } + unless column = T.constant(:COLUMNS).find { |col| col[:name].stringify == name.stringify } raise %(The `#{T.name}` model does not have a column named `#{name}`) end %} - + file_attribute :{{ field_name }} {% if nilable = column[:nilable] %} @@ -134,7 +134,7 @@ module Avram::Attachment::SaveOperation file.delete end {% end %} - + return unless {{ field_name }}.value && (cached = {{ name }}.value) stored = T::ATTACHMENT_UPLOADERS[:{{ name }}].promote(cached) From f5d6408ca1bfc0aa39b7cba3cc425bf259b0e365 Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 1 Mar 2026 13:26:32 +0100 Subject: [PATCH 17/52] Rename `Storage::Base` base class to `Storage` and add S3 shard --- shard.yml | 3 +++ spec/lucky/attachment/uploader_spec.cr | 2 +- src/lucky/attachment/config.cr | 4 ++-- src/lucky/attachment/storage.cr | 2 +- src/lucky/attachment/storage/file_system.cr | 2 +- src/lucky/attachment/storage/memory.cr | 2 +- src/lucky/attachment/stored_file.cr | 2 +- src/lucky/attachment/uploader.cr | 2 +- 8 files changed, 11 insertions(+), 8 deletions(-) diff --git a/shard.yml b/shard.yml index 4969a95ac..d109568c5 100644 --- a/shard.yml +++ b/shard.yml @@ -56,5 +56,8 @@ development_dependencies: ameba: github: crystal-ameba/ameba version: ~> 1.6.4 + awscr-s3: + github: taylorfinnell/awscr-s3 + version: ~> 0.10.0 license: MIT diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index 3e4ee96bf..84cf47834 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -94,7 +94,7 @@ describe Lucky::Attachment::Uploader do context "error handling" do it "raises Error when no storages are not configured" do Lucky::Attachment.configure do |settings| - settings.storages = {} of String => Lucky::Attachment::Storage::Base + settings.storages = {} of String => Lucky::Attachment::Storage end expect_raises( diff --git a/src/lucky/attachment/config.cr b/src/lucky/attachment/config.cr index ec0f25dff..4e0f9c2ca 100644 --- a/src/lucky/attachment/config.cr +++ b/src/lucky/attachment/config.cr @@ -10,7 +10,7 @@ module Lucky::Attachment # NOTE: Additional stores are not supported yet. Please reach out if that # is something you need. # - setting storages : Hash(String, Storage::Base) = {} of String => Storage::Base + setting storages : Hash(String, Storage) = {} of String => Storage # Path prefix for uploads. Possible keywords are: # - `:model` (an underscored string of the model name) @@ -27,7 +27,7 @@ module Lucky::Attachment # Lucky::Attachment.find_storage("missing") # raises Lucky::Attachment::Error # ``` # - def self.find_storage(name : String) : Storage::Base + def self.find_storage(name : String) : Storage settings.storages[name]? || raise Error.new( String.build do |io| diff --git a/src/lucky/attachment/storage.cr b/src/lucky/attachment/storage.cr index 8384527cb..8126e20db 100644 --- a/src/lucky/attachment/storage.cr +++ b/src/lucky/attachment/storage.cr @@ -2,7 +2,7 @@ # Implementations must provide methods for uploading, retreiving, checking # existence, and deleting files. # -abstract class Lucky::Attachment::Storage::Base +abstract class Lucky::Attachment::Storage # Uploads an IO to the given location (id) in the storage. # # ``` diff --git a/src/lucky/attachment/storage/file_system.cr b/src/lucky/attachment/storage/file_system.cr index 2e903f709..7eae43e2f 100644 --- a/src/lucky/attachment/storage/file_system.cr +++ b/src/lucky/attachment/storage/file_system.cr @@ -15,7 +15,7 @@ require "../storage" # end # ``` # -class Lucky::Attachment::Storage::FileSystem < Lucky::Attachment::Storage::Base +class Lucky::Attachment::Storage::FileSystem < Lucky::Attachment::Storage DEFAULT_PERMISSIONS = File::Permissions.new(0o644) DEFAULT_DIRECTORY_PERMISSIONS = File::Permissions.new(0o755) diff --git a/src/lucky/attachment/storage/memory.cr b/src/lucky/attachment/storage/memory.cr index 5f3038fd1..e8f504991 100644 --- a/src/lucky/attachment/storage/memory.cr +++ b/src/lucky/attachment/storage/memory.cr @@ -11,7 +11,7 @@ require "../storage" # end # ``` # -class Lucky::Attachment::Storage::Memory < Lucky::Attachment::Storage::Base +class Lucky::Attachment::Storage::Memory < Lucky::Attachment::Storage getter store : Hash(String, Bytes) getter base_url : String? diff --git a/src/lucky/attachment/stored_file.cr b/src/lucky/attachment/stored_file.cr index 08e5b3fbe..e8632ab26 100644 --- a/src/lucky/attachment/stored_file.cr +++ b/src/lucky/attachment/stored_file.cr @@ -110,7 +110,7 @@ class Lucky::Attachment::StoredFile end # Returns the storage instance this file is stored in. - def storage : Storage::Base + def storage : Storage ::Lucky::Attachment.find_storage(storage_key) end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 30fc34fcd..0b97b2512 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -22,7 +22,7 @@ abstract struct Lucky::Attachment::Uploader end # Returns the storage instance for this uploader. - def storage : Storage::Base + def storage : Storage Lucky::Attachment.find_storage(storage_key) end From 0323507ef41879b47adf4f176b96c4472a5e9a11 Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 6 Mar 2026 14:48:24 +0100 Subject: [PATCH 18/52] Add `Storage:S3` --- shard.yml | 5 +- spec/lucky/attachment/storage/s3_spec.cr | 434 +++++++++++++++++++++++ src/lucky/attachment/storage/s3.cr | 323 +++++++++++++++++ 3 files changed, 761 insertions(+), 1 deletion(-) create mode 100644 spec/lucky/attachment/storage/s3_spec.cr create mode 100644 src/lucky/attachment/storage/s3.cr diff --git a/shard.yml b/shard.yml index d109568c5..81fc227fe 100644 --- a/shard.yml +++ b/shard.yml @@ -1,7 +1,7 @@ name: lucky version: 1.4.0 -crystal: ">= 1.16.3" +crystal: '>= 1.16.3' authors: - Paul Smith @@ -59,5 +59,8 @@ development_dependencies: awscr-s3: github: taylorfinnell/awscr-s3 version: ~> 0.10.0 + webmock: + github: manastech/webmock.cr + branch: master license: MIT diff --git a/spec/lucky/attachment/storage/s3_spec.cr b/spec/lucky/attachment/storage/s3_spec.cr new file mode 100644 index 000000000..fffc20217 --- /dev/null +++ b/spec/lucky/attachment/storage/s3_spec.cr @@ -0,0 +1,434 @@ +require "../../../spec_helper" +require "webmock" + +describe Lucky::Attachment::Storage::S3 do + after_each do + WebMock.reset + end + + describe "#object_key" do + it "returns the id when no prefix is configured" do + storage = build_storage + + storage.object_key("photo.jpg").should eq("photo.jpg") + end + + it "prepends the prefix" do + storage = build_storage(prefix: "cache") + + storage.object_key("photo.jpg").should eq("cache/photo.jpg") + end + + it "strips extra slashes from prefix and id" do + storage = build_storage(prefix: "/cache/") + + storage.object_key("/photo.jpg").should eq("cache/photo.jpg") + end + end + + describe "#upload and #open" do + it "writes and reads file content" do + storage = build_storage + WebMock.stub(:put, "#{base_url}/test.txt") + .to_return(status: 200, headers: test_headers) + WebMock.stub(:get, "#{base_url}/test.txt") + .to_return(status: 200, body_io: test_file_io, headers: test_headers) + + storage.upload(test_file_io, "test.txt") + + storage.open("test.txt").gets_to_end.should eq("file content") + end + + it "respects the prefix when uploading" do + storage = build_storage(prefix: "cache") + WebMock.stub(:put, "#{base_url}/cache/test.txt") + .to_return(status: 200, headers: test_headers) + + storage.upload(IO::Memory.new("data"), "test.txt") + end + end + + describe "#open" do + it "raises FileNotFound when the object does not exist" do + storage = build_storage + WebMock.stub(:get, "#{base_url}/missing.txt") + .to_return(status: 404, body: s3_error_xml("NoSuchKey", "Missing.")) + + expect_raises(Lucky::Attachment::FileNotFound, /missing\.txt/) do + storage.open("missing.txt") + end + end + + it "returns a rewindable IO" do + storage = build_storage + WebMock.stub(:get, "#{base_url}/test.txt") + .to_return(status: 200, body_io: test_file_io("rewindable")) + + io = storage.open("test.txt") + io.gets_to_end.should eq("rewindable") + io.rewind + io.gets_to_end.should eq("rewindable") + end + end + + describe "#exists?" do + it "returns true when the object exists" do + storage = build_storage + WebMock.stub(:head, "#{base_url}/photo.jpg").to_return(status: 200) + + storage.exists?("photo.jpg").should be_true + end + + it "returns false when the object does not exist" do + storage = build_storage + WebMock.stub(:head, "#{base_url}/missing.txt").to_return(status: 404) + + storage.exists?("missing.txt").should be_false + end + end + + describe "#delete" do + it "sends a DELETE request for the object" do + storage = build_storage + WebMock.stub(:delete, "#{base_url}/photo.jpg").to_return(status: 204) + + storage.delete("photo.jpg") + end + + it "uses the prefix in the DELETE path" do + storage = build_storage(prefix: "cache") + WebMock.stub(:delete, "#{base_url}/cache/photo.jpg").to_return(status: 204) + + storage.delete("photo.jpg") + end + end + + describe "#url" do + describe "public URL (no expires_in)" do + it "returns a standard AWS S3 URL" do + storage = build_storage + + storage.url("photo.jpg").should eq( + "https://s3-eu-west-1.amazonaws.com/lucky-bucket/photo.jpg" + ) + end + + it "includes the prefix in the key" do + storage = build_storage(prefix: "store") + + storage.url("photo.jpg").should eq( + "https://s3-eu-west-1.amazonaws.com/lucky-bucket/store/photo.jpg" + ) + end + + it "returns a custom-emdpoint URL for S3-compatible services" do + storage = build_rustfs_storage + + storage.url("photo.jpg").should eq( + "http://localhost:9000/lucky-bucket/photo.jpg" + ) + end + + it "includes non-standard ports in the URL" do + storage = Lucky::Attachment::Storage::S3.new( + bucket: "lucky-bucket", + region: "eu-west-1", + access_key_id: "key", + secret_access_key: "secret", + endpoint: "http://localhost:9000" + ) + + storage.url("photo.jpg").should contain("http://localhost:9000") + end + + it "omits standard ports from the URL" do + storage = Lucky::Attachment::Storage::S3.new( + bucket: "lucky-bucket", + region: "eu-west-1", + access_key_id: "key", + secret_access_key: "secret", + endpoint: "https://s3.example.com:443" + ) + + storage.url("photo.jpg").should eq( + "https://s3.example.com/lucky-bucket/photo.jpg" + ) + end + end + + describe "#move" do + it "falls back to upload for a plain IO" do + storage = build_storage + WebMock.stub(:put, "#{base_url}/dest.txt") + .to_return(status: 200, headers: test_headers) + + storage.move(IO::Memory.new("data"), "dest.txt") + end + + it "uses server-side copy and deletes the source for a same-bucket StoredFile" do + storage = build_storage + WebMock.stub(:put, "#{base_url}/store/photo.jpg") + .to_return(status: 200, headers: test_headers, body: copy_object_xml) + WebMock.stub(:delete, "#{base_url}/cache/photo.jpg") + .to_return(status: 204) + Lucky::Attachment.settings.storages["cache"] = storage + + source = Lucky::Attachment::StoredFile.new( + id: "cache/photo.jpg", + storage_key: "cache", + metadata: Lucky::Attachment::MetadataHash.new + ) + + storage.move(source, "store/photo.jpg") + end + + it "falls back to upload for a StoredFile from a different bucket" do + storage = build_storage + other_storage = build_storage(bucket: "other-bucket") + Lucky::Attachment.settings.storages["other"] = other_storage + WebMock.stub(:get, "https://s3-eu-west-1.amazonaws.com/other-bucket/photo.jpg") + .to_return(status: 200, body_io: test_file_io("data"), headers: test_headers) + WebMock.stub(:put, "#{base_url}/photo.jpg") + .to_return(status: 200, headers: test_headers) + source = Lucky::Attachment::StoredFile.new( + id: "photo.jpg", + storage_key: "other", + metadata: Lucky::Attachment::MetadataHash.new + ) + + storage.move(source, "photo.jpg") + end + + it "uses only the object key (not double-prefixed) when copying a same-bucket StoredFile" do + storage = build_storage(prefix: "store") + Lucky::Attachment.settings.storages["store"] = storage + WebMock.stub(:put, "#{base_url}/store/photo.jpg?") + .to_return(status: 200, body: copy_object_xml) + WebMock.stub(:delete, "#{base_url}/store/photo.jpg?") + .to_return(status: 204) + + source = Lucky::Attachment::StoredFile.new( + id: "photo.jpg", + storage_key: "store", + metadata: Lucky::Attachment::MetadataHash.new + ) + + storage.move(source, "photo.jpg") + end + end + + describe "presigned URL (with expires_in)" do + it "returns a URL containing signature query parameters" do + storage = build_storage + url = storage.url("photo.jpg", expires_in: 3600) + + url.should contain("X-Amz-Signature") + url.should contain("X-Amz-Expires=3600") + url.should contain("photo.jpg") + end + + it "includes the prefix in the presigned key" do + storage = build_storage(prefix: "cache") + url = storage.url("photo.jpg", expires_in: 3600) + + url.should contain("cache/photo.jpg") + end + + it "uses the custom endpoint host for presigned URLs" do + storage = build_rustfs_storage + url = storage.url("photo.jpg", expires_in: 3600) + + url.should contain("localhost:9000") + end + end + end + + describe "upload headers" do + it "sets Content-Disposition from metadata filename" do + client = TestAwss3Client.new + build_storage(client: client).upload( + IO::Memory.new, "test-id", + metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.jpg"}, + ) + + client.headers["Content-Disposition"] + .should eq(%(inline; filename="photo.jpg")) + end + + it "sets Content-Type from metadata mime_type" do + client = TestAwss3Client.new + build_storage(client: client).upload( + IO::Memory.new, "test-id", + metadata: Lucky::Attachment::MetadataHash{"mime_type" => "image/jpeg"}, + ) + + client.headers["Content-Type"].should eq("image/jpeg") + end + + it "allows content_type to override metadata mime_type" do + client = TestAwss3Client.new + build_storage(client: client).upload( + IO::Memory.new, "test-id", + metadata: Lucky::Attachment::MetadataHash{"mime_type" => "image/jpeg"}, + content_type: "application/octet-stream" + ) + + client.headers["Content-Type"].should eq("application/octet-stream") + end + + it "allows content_disposition to override metadata filename" do + client = TestAwss3Client.new + build_storage(client: client).upload( + IO::Memory.new, "test-id", + metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.jpg"}, + content_disposition: "attachment" + ) + + client.headers["Content-Disposition"].should eq("attachment") + end + + it "sets x-amz-acl: public-read when storage is public" do + client = TestAwss3Client.new + build_storage(client: client, public: true) + .upload(IO::Memory.new, "test-id") + + client.headers["x-amz-acl"].should eq("public-read") + end + + it "merges upload_options" do + client = TestAwss3Client.new + build_storage( + client: client, + upload_options: {"Cache-Control" => "max-age=31536000"} + ).upload(IO::Memory.new, "test-id") + + client.headers["Cache-Control"].should eq("max-age=31536000") + end + + it "per-call options take precedence over upload_options" do + client = TestAwss3Client.new + build_storage( + client: client, + upload_options: {"Content-Type" => "application/octet-stream"} + ).upload(IO::Memory.new, "test-id", content_type: "image/jpeg") + + client.headers["Content-Type"].should eq("image/jpeg") + end + + it "upload_options do not override metadata Content-Disposition" do + client = TestAwss3Client.new + build_storage( + client: client, + upload_options: {"Content-Disposition" => "attachment"} + ).upload( + IO::Memory.new, "test-id", + metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.jpg"} + ) + + client.headers["Content-Disposition"] + .should eq(%(inline; filename="photo.jpg")) + end + end +end + +private class TestAwss3Client < Awscr::S3::Client + SIGNER = Awscr::Signer::Signers::V4.new("blah", "blah", "blah", "blah") + + getter headers = {} of String => String + + def initialize( + @region = "eu-west-1", + @aws_access_key = "test-key", + @aws_secret_key = "test-secret", + @endpoint = URI.new, + @signer = SIGNER, + @http = Awscr::S3::Http.new(SIGNER, URI.new), + ) + end + + def put_object(_bucket, _id, _io, @headers) + end +end + +private def bucket + "lucky-bucket" +end + +private def region + "eu-west-1" +end + +private def base_url + "https://s3-#{region}.amazonaws.com/#{bucket}" +end + +private def test_headers(headers = {} of String => String) + {"ETag" => %("abc123")}.merge(headers) +end + +private def test_file_io(content = "file content") + IO::Memory.new(content) +end + +private def copy_object_xml + <<-XML + + + "abc123" + 2026-03-01T10:08:56.000Z + + XML +end + +private def s3_error_xml(code : String, message : String) : String + <<-XML + + + #{code} + #{message} + + XML +end + +private def build_storage( + bucket = "lucky-bucket", + region = "eu-west-1", + access_key_id = "test-key", + secret_access_key = "test-secret", + prefix = nil, + public = false, + upload_options = Hash(String, String).new, + endpoint = nil, + client = nil, +) + if s3_client = client + Lucky::Attachment::Storage::S3.new( + bucket: bucket, + client: s3_client, + prefix: prefix, + public: public, + upload_options: upload_options + ) + else + Lucky::Attachment::Storage::S3.new( + bucket: bucket, + region: region, + access_key_id: "test-key", + secret_access_key: "test-secret", + prefix: prefix, + endpoint: endpoint, + public: public, + upload_options: upload_options + ) + end +end + +private def build_rustfs_storage(bucket = "lucky-bucket", prefix = nil) + build_storage( + bucket: bucket, + access_key_id: "rustfsadmin", + secret_access_key: "rustfsadmin", + prefix: prefix, + endpoint: "http://localhost:9000" + ) +end diff --git a/src/lucky/attachment/storage/s3.cr b/src/lucky/attachment/storage/s3.cr new file mode 100644 index 000000000..81e9ec66f --- /dev/null +++ b/src/lucky/attachment/storage/s3.cr @@ -0,0 +1,323 @@ +require "../storage" +require "awscr-s3" + +# S3-compatible storage backend. Supports AWS S3 and any S3-compatible service +# such as RustFS, Tigris, or Cloudflare R2 via a custom endpoint. +# +# Requires the `awscr-s3` shard to be added to your `shard.yml`: +# +# ```yaml +# dependencies: +# awscr-s3: +# github: taylorfinnell/awscr-s3 +# ``` +# +# ## AWS S3 +# +# ``` +# Lucky::Attachment::Storage::S3.new( +# bucket: "lucky-bucket", +# region: "eu-west-1", +# access_key_id: ENV["KEY"], +# secret_access_key: ENV["SECRET"] +# ) +# ``` +# +# ## RustFS or other S3-compatible services +# +# ``` +# Lucky::Attachment::Storage::S3.new( +# bucket: "lucky-bucket", +# region: "eu-west-1", +# access_key_id: ENV["KEY"], +# secret_access_key: ENV["SECRET"], +# endpoint: "http://localhost:9000" +# ) +# ``` +# +# ## Bring your own client +# +# ``` +# client = Awscr::S3::Client.new("eu-west-1", ENV["KEY"], ENV["SECRET"]) +# Lucky::Attachment::Storage::S3.new(bucket: "lucky-bucket", client: client) +# ``` +# +class Lucky::Attachment::Storage::S3 < Lucky::Attachment::Storage + getter bucket : String + getter prefix : String? + getter? public : Bool + getter upload_options : Hash(String, String) + getter client : Awscr::S3::Client + + # Initialises a storage using credentials. + # + # ``` + # storage = Lucky::Attachment::Storage::S3.new( + # bucket: "lucky-bucket", + # region: "eu-west-1", + # access_key_id: "key", + # secret_access_key: "secret", + # endpoint: "http://localhost:9000" + # ) + # ``` + # + def initialize( + @bucket : String, + region : String, + @access_key_id : String, + @secret_access_key : String, + @prefix : String? = nil, + endpoint : String? = nil, + @public : Bool = false, + @upload_options : Hash(String, String) = Hash(String, String).new, + ) + @client = Awscr::S3::Client.new( + region, + @access_key_id, + @secret_access_key, + endpoint: endpoint + ) + end + + # Initialises a storage with a pre-built `Awscr::S3::Client`. Useful when you + # need full control over the client configuration, or in tests for example. + # + # ``` + # client = Awscr::S3::Client.new("eu-west-1", "key", "secret") + # storage = Lucky::Attachment::Storage::S3.new( + # bucket: "lucky-bucket", + # client: client + # ) + # ``` + # + def initialize( + @bucket : String, + @client : Awscr::S3::Client, + @prefix : String? = nil, + @public : Bool = false, + @upload_options : Hash(String, String) = Hash(String, String).new, + ) + # NOTE: These attributes are protected on the client, so this is the only + # way to get them. + @access_key_id = @client.@aws_access_key + @secret_access_key = @client.@aws_secret_key + end + + # Uploads an IO to the given key in the bucket. Any additional keys in + # `options` are forwarded to the S3 client. + # + # ``` + # storage.upload(io, "uploads/photo.jpg") + # storage.upload(io, "uploads/photo.jpg", metadata: { + # "filename" => "photo.jpg", + # "mime_type" => "image/jpeg", + # }) + # ``` + # + def upload(io : IO, id : String, **options) : Nil + @client.put_object( + bucket, + object_key(id), + io.gets_to_end, + build_upload_headers(**options) + ) + end + + # Opens the S3 object and returns an `IO::Memory` for reading. + # + # ``` + # io = storage.open("uploads/photo.jpg") + # content = io.gets_to_end + # io.close + # ``` + # + # Raises `Lucky::Attachment::FileNotFound` if the object does not exist. + # + def open(id : String, **options) : IO + buffer = IO::Memory.new + @client.get_object(bucket, object_key(id)) do |response| + IO.copy(response.body_io, buffer) + end + buffer.rewind + buffer + rescue ex : Awscr::S3::NoSuchKey + raise FileNotFound.new("File not found: #{id}") + end + + # Tests if an object exists in the bucket. + # + # ``` + # storage.exists?("uploads/photo.jpg") + # # => true + # ``` + # + def exists?(id : String) : Bool + @client.head_object(bucket, object: object_key(id)) + true + rescue Awscr::S3::Exception + false + end + + # Returns the URL for accessing the object. When `expires_in` is provided + # (in seconds), a presigned URL is returned. Otherwise a plain public URL is + # constructed without any HTTP round-trip. + # + # ``` + # storage.url("uploads/photo.jpg") + # # => "https://s3-eu-west-1.amazonaws.com/lucky-bucket/uploads/photo.jpg" + # + # storage.url("uploads/photo.jpg", expires_in: 3600) + # # => "https://s3-eu-west-1.amazonaws.com/lucky-bucket/uploads/photo.jpg?X-Amz-Signature=..." + # ``` + # + def url(id : String, **options) : String + return public_url(id) unless expires_in = options[:expires_in]? + + presigned_url(id, expires_in: expires_in.to_i) + end + + # Deletes the object for the given key. Does not raise if the object does not + # exist. + # + # ``` + # storage.delete("uploads/photo.jpg") + # ``` + # + def delete(id : String) : Nil + @client.delete_object(bucket, object_key(id)) + end + + # Promotes a file efficiently using a server-side S3 copy when the source is + # a `StoredFile` in the same bucket, avoiding the download/re-upload. Falls + # back to a regular upload for plain `IO` sources. + # + def move(io : IO, id : String, **options) : Nil + upload(io, id, **options) + end + + def move(file : Lucky::Attachment::StoredFile, id : String, **options) : Nil + if same_bucket?(file) + copy_object( + **options, + source_key: object_key(file.id), + dest_key: object_key(id) + ) + file.delete + else + move(file.io, id, **options) + end + end + + # Returns the full object key including any configured prefix. + # + # ``` + # storage.object_key("photo.jpg") + # # => "photo.jpg" + # ``` + # + def object_key(id : String) : String + return id unless p = prefix + + "#{p.strip('/')}/#{id.lstrip('/')}" + end + + # Builds a header hash. + private def build_upload_headers(**options) : Hash(String, String) + Hash(String, String).new.tap do |headers| + if metadata = options[:metadata]?.try(&.as?(MetadataHash)) + if filename = metadata["filename"]?.try(&.as?(String)) + headers["Content-Disposition"] = %(inline; filename="#{filename}") + end + if mime_type = metadata["mime_type"]?.try(&.as?(String)) + headers["Content-Type"] = mime_type + end + end + + if content_type = options[:content_type]?.try(&.to_s.presence) + headers["Content-Type"] = content_type + end + + if content_disposition = options[:content_disposition]?.try(&.to_s.presence) + headers["Content-Disposition"] = content_disposition + end + + headers["x-amz-acl"] = "public-read" if public? + + @upload_options.each do |key, value| + headers[key] ||= value + end + end + end + + # Builds a public url considering custom endpoints configured in the client. + private def public_url(id : String) : String + String.build do |io| + if uri = endpoint_uri + scheme = uri.scheme || "https" + host = endpoint_host_with_port(uri) + else + scheme = "https" + host = "s3-#{@client.region}.amazonaws.com" + end + io << scheme << "://" << host << '/' << bucket << '/' << object_key(id) + end + end + + # Builds a presigned url. + private def presigned_url(id : String, expires_in : Int32) : String + options = Awscr::S3::Presigned::Url::Options.new( + aws_access_key: @access_key_id, + aws_secret_key: @secret_access_key, + region: @client.region, + object: "/#{object_key(id)}", + bucket: bucket, + host_name: presigned_url_host_name, + expires: expires_in, + ) + Awscr::S3::Presigned::Url.new(options).for(:get) + end + + # Builds the host name for the current endpoint. + private def presigned_url_host_name : String? + return unless uri = endpoint_uri + + endpoint_host_with_port(uri) + rescue URI::Error + raw_endpoint + end + + # Copies an object to a new location. + private def copy_object(source_key : String, dest_key : String, **options) : Nil + @client.copy_object( + bucket, + source_key, + dest_key, + build_upload_headers(**options) + ) + end + + # Determines if the given file is in the same bucket as the current one. + private def same_bucket?(file : Lucky::Attachment::StoredFile) : Bool + return false unless storage = file.storage.as?(S3) + + storage.bucket == bucket + end + + # Builds a host with port from a URI object. + private def endpoint_host_with_port(uri : URI) : String + host, port = uri.host.to_s, uri.port + (port && port != 80 && port != 443) ? "#{host}:#{port}" : host + end + + # Tries to parse the S3 client's endpoint to a URI object. + private def endpoint_uri : URI? + return unless endpoint = raw_endpoint + + URI.parse(endpoint) + end + + # Tries to retrieve the endpoint from the client. + private def raw_endpoint : String? + @client.endpoint.try(&.to_s) + end +end From 9789eb3eda3ad5f645fb6d0145083a8a5c9f6a98 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 11:19:47 +0100 Subject: [PATCH 19/52] Rework Uploader to use extractors to build the metadata hash --- src/lucky/attachment/extractor.cr | 15 ++++ .../attachment/extractor/filename_from_io.cr | 15 ++++ .../extractor/mime_from_extension.cr | 12 +++ .../attachment/extractor/mime_from_file.cr | 28 +++++++ .../attachment/extractor/mime_from_io.cr | 10 +++ .../attachment/extractor/size_from_io.cr | 12 +++ src/lucky/attachment/uploader.cr | 81 +++++++++---------- 7 files changed, 129 insertions(+), 44 deletions(-) create mode 100644 src/lucky/attachment/extractor.cr create mode 100644 src/lucky/attachment/extractor/filename_from_io.cr create mode 100644 src/lucky/attachment/extractor/mime_from_extension.cr create mode 100644 src/lucky/attachment/extractor/mime_from_file.cr create mode 100644 src/lucky/attachment/extractor/mime_from_io.cr create mode 100644 src/lucky/attachment/extractor/size_from_io.cr diff --git a/src/lucky/attachment/extractor.cr b/src/lucky/attachment/extractor.cr new file mode 100644 index 000000000..e485f3c80 --- /dev/null +++ b/src/lucky/attachment/extractor.cr @@ -0,0 +1,15 @@ +# Extractors try to extract metadata from the context they're given: the `IO` +# object, the current state of the resulting metadata hash, or arbitrary +# options passed to the `upload` method of the uploader. +# +module Lucky::Attachment::Extractor + # Extracts metadata and returns a `MetadataValue`. Alternatively, the + # metadata hash may be modified directly if multiple values need to be added + # (e.g. the dimensions of an image). + # + abstract def extract( + io : IO, + metadata : MetadataHash, + **options, + ) : MetadataValue? +end diff --git a/src/lucky/attachment/extractor/filename_from_io.cr b/src/lucky/attachment/extractor/filename_from_io.cr new file mode 100644 index 000000000..7ea201631 --- /dev/null +++ b/src/lucky/attachment/extractor/filename_from_io.cr @@ -0,0 +1,15 @@ +struct Lucky::Attachment::Extractor::FilenameFromIO + include Lucky::Attachment::Extractor + + # Returns the filename from the options or tries to extract the filename from + # the IO object. + def extract(io, metadata, **options) : String? + options[:filename]? || if io.responds_to?(:original_filename) + io.original_filename + elsif io.responds_to?(:filename) + io.filename.presence + elsif io.responds_to?(:path) + File.basename(io.path) + end + end +end diff --git a/src/lucky/attachment/extractor/mime_from_extension.cr b/src/lucky/attachment/extractor/mime_from_extension.cr new file mode 100644 index 000000000..40a1f7c2c --- /dev/null +++ b/src/lucky/attachment/extractor/mime_from_extension.cr @@ -0,0 +1,12 @@ +struct Lucky::Attachment::Extractor::MimeFromExtension + include Lucky::Attachment::Extractor + + # Extracts the MIME type from the extension of the filename. + def extract(io, metadata, **options) : String? + return unless filename = FilenameFromIO.new.extract(io, metadata, **options) + + MIME.from_filename(filename) + rescue KeyError + nil + end +end diff --git a/src/lucky/attachment/extractor/mime_from_file.cr b/src/lucky/attachment/extractor/mime_from_file.cr new file mode 100644 index 000000000..d86c93de1 --- /dev/null +++ b/src/lucky/attachment/extractor/mime_from_file.cr @@ -0,0 +1,28 @@ +struct Lucky::Attachment::Extractor::MimeFromFile + include Lucky::Attachment::Extractor + + # Extracts the MIME type using the `file` utility, and raises when it's not + # installed. + def extract(io, metadata, **options) : String? + # Avoids returning "application/x-empty" for empty files + return nil if io.size.try &.zero? + + stdout, stderr = IO::Memory.new, IO::Memory.new + command = Process.run( + "file", + args: ["--mime-type", "--brief", "-"], + output: stdout, + error: stderr, + input: io + ) + + if command.success? + io.rewind + stdout.to_s.strip + else + Log.debug { "Unable to extract MIME type using `file` utility (#{stderr})" } + end + rescue RuntimeError + raise Error.new("file command-line tool is not installed") + end +end diff --git a/src/lucky/attachment/extractor/mime_from_io.cr b/src/lucky/attachment/extractor/mime_from_io.cr new file mode 100644 index 000000000..035b50dd9 --- /dev/null +++ b/src/lucky/attachment/extractor/mime_from_io.cr @@ -0,0 +1,10 @@ +struct Lucky::Attachment::Extractor::MimeFromIO + include Lucky::Attachment::Extractor + + # Extracts the MIME type from the IO. + def extract(io, metadata, **options) : String? + return unless io.responds_to?(:content_type) && (type = io.content_type) + + type.split(';').first.strip + end +end diff --git a/src/lucky/attachment/extractor/size_from_io.cr b/src/lucky/attachment/extractor/size_from_io.cr new file mode 100644 index 000000000..e32ce09c0 --- /dev/null +++ b/src/lucky/attachment/extractor/size_from_io.cr @@ -0,0 +1,12 @@ +struct Lucky::Attachment::Extractor::SizeFromIO + include Lucky::Attachment::Extractor + + # Tries to extract the file size from the IO. + def extract(io, metadata, **options) : Int64? + if io.responds_to?(:tempfile) + io.tempfile.size + elsif io.responds_to?(:size) + io.size.to_i64 + end + end +end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 0b97b2512..8e24e4d2b 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -16,6 +16,17 @@ require "uuid" # ``` # abstract struct Lucky::Attachment::Uploader + EXTRACTORS = { + "filename" => Extractor::FilenameFromIO.new, + "mime_type" => Extractor::MimeFromIO.new, + "size" => Extractor::SizeFromIO.new, + } of String => Extractor + + # Registers an extractor for a given key. + macro extract(type_declaration) + EXTRACTORS["{{type_declaration.var.id}}"] = type_declaration.type.new + end + getter storage_key : String def initialize(@storage_key : String) @@ -49,6 +60,7 @@ abstract struct Lucky::Attachment::Uploader # ``` # cached = ImageUploader.cache(io) # ``` + # def self.cache(io : IO, **options) : StoredFile new("cache").upload(io, **options) end @@ -96,12 +108,27 @@ abstract struct Lucky::Attachment::Uploader # ``` # def generate_location(io : IO, metadata : MetadataHash, **options) : String - extension = extract_extension(io, metadata) - basename = generate_uid + extension = file_extension(io, metadata) + basename = generate_uid(io, metadata, **options) filename = extension ? "#{basename}.#{extension}" : basename File.join([options[:path_prefix]?, filename].compact) end + # Generates a unique identifier for file locations. Override this in + # subclasses for custom filenames in the storage. + # + # ``` + # class ImageUploader < Lucky::Attachment::Uploader + # def generate_uid(io, metadata, **options) : String + # "#{metadata["filename"]}-#{Time.local.to_unix}" + # end + # end + # ``` + # + def generate_uid(io : IO, metadata : MetadataHash, **options) : String + UUID.random.to_s + end + # Extracts metadata from the IO. Override in subclasses to add custom # metadata extraction. # @@ -121,51 +148,17 @@ abstract struct Lucky::Attachment::Uploader metadata : MetadataHash? = nil, **options, ) : MetadataHash - MetadataHash{ - "filename" => options[:filename]? || extract_filename(io), - "size" => extract_size(io), - "mime_type" => extract_mime_type(io), - } - end - - # Generates a unique identifier for file locations. - protected def generate_uid : String - UUID.random.to_s - end - - # Extracts the filename from the IO if available. - protected def extract_filename(io : IO) : String? - if io.responds_to?(:original_filename) - io.original_filename - elsif io.responds_to?(:filename) - io.filename.presence - elsif io.responds_to?(:path) - File.basename(io.path) + (metadata.try(&.dup) || MetadataHash.new).tap do |data| + EXTRACTORS.each do |key, extractor| + if value = extractor.extract(io, data, **options) + data[key] = value + end + end end end - # Extracts the file size from the IO, if available. - protected def extract_size(io : IO) : Int64? - if io.responds_to?(:tempfile) - io.tempfile.size - elsif io.responds_to?(:size) - io.size.to_i64 - end - end - - # Extracts the MIME type from the IO if available. - # - # NOTE: This relies on the IO providing content_type, which typically comes - # from HTTP headers and may not be accurate, but it's a good fallback. - # - protected def extract_mime_type(io : IO) : String? - return unless io.responds_to?(:content_type) && (type = io.content_type) - - type.split(';').first.strip - end - - # Extracts file extension from the IO or metadata. - protected def extract_extension( + # Tries to determine the file extension from the metadata or IO. + protected def file_extension( io : IO, metadata : MetadataHash, ) : String? From 90d890b08a7ad8cf209e867c7bbc840c4076623b Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 14:28:32 +0100 Subject: [PATCH 20/52] Install `file` utility in docker container for tests --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 33404cf33..5030e38b2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM crystallang/crystal:latest WORKDIR /data RUN apt-get update && \ - apt-get install -y curl libreadline-dev unzip && \ + apt-get install -y curl libreadline-dev unzip file && \ curl -fsSL https://bun.sh/install | bash && \ ln -s /root/.bun/bin/bun /usr/local/bin/bun && \ # Cleanup leftovers From b72b65d834d862797cb5242a66fda75387d28a62 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 14:28:47 +0100 Subject: [PATCH 21/52] Add test for extractors --- .../extractor/filename_from_io_spec.cr | 142 +++++++++++++++++ .../extractor/mime_from_extension_spec.cr | 149 ++++++++++++++++++ .../extractor/mime_from_file_spec.cr | 94 +++++++++++ .../attachment/extractor/mime_from_io_spec.cr | 77 +++++++++ .../attachment/extractor/size_from_io_spec.cr | 100 ++++++++++++ .../attachment/extractor/mime_from_file.cr | 2 +- 6 files changed, 563 insertions(+), 1 deletion(-) create mode 100644 spec/lucky/attachment/extractor/filename_from_io_spec.cr create mode 100644 spec/lucky/attachment/extractor/mime_from_extension_spec.cr create mode 100644 spec/lucky/attachment/extractor/mime_from_file_spec.cr create mode 100644 spec/lucky/attachment/extractor/mime_from_io_spec.cr create mode 100644 spec/lucky/attachment/extractor/size_from_io_spec.cr diff --git a/spec/lucky/attachment/extractor/filename_from_io_spec.cr b/spec/lucky/attachment/extractor/filename_from_io_spec.cr new file mode 100644 index 000000000..bfca8d424 --- /dev/null +++ b/spec/lucky/attachment/extractor/filename_from_io_spec.cr @@ -0,0 +1,142 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Extractor::FilenameFromIO do + describe "#extract" do + subject = Lucky::Attachment::Extractor::FilenameFromIO.new + + context "when a filename is provided in options" do + it "returns the filename from options" do + io = PlainIO.new + result = subject.extract(io, metadata: nil, filename: "override.txt") + + result.should eq("override.txt") + end + + it "prefers options filename over #original_filename" do + io = IOWithOriginalFilename.new("ignored.txt") + result = subject.extract(io, metadata: nil, filename: "override.txt") + + result.should eq("override.txt") + end + end + + context "when the IO responds to #original_filename" do + it "returns the original filename" do + io = IOWithOriginalFilename.new("photo.jpg") + result = subject.extract(io, metadata: nil) + + result.should eq("photo.jpg") + end + + it "returns nil when original_filename is nil" do + io = IOWithOriginalFilename.new(nil) + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the IO responds to #filename but not #original_filename" do + it "returns the filename" do + io = IOWithFilename.new("document.pdf") + result = subject.extract(io, metadata: nil) + + result.should eq("document.pdf") + end + + it "returns nil when filename is blank" do + io = IOWithFilename.new("") + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + + it "returns nil when filename is nil" do + io = IOWithFilename.new(nil) + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the IO responds to #path but not #original_filename or #filename" do + it "returns the basename of the path" do + io = IOWithPath.new("/uploads/tmp/archive.zip") + result = subject.extract(io, metadata: nil) + + result.should eq("archive.zip") + end + + it "returns just the filename when path has no directory component" do + io = IOWithPath.new("archive.zip") + result = subject.extract(io, metadata: nil) + + result.should eq("archive.zip") + end + end + + context "when the IO responds to none of the known methods" do + it "returns nil" do + io = PlainIO.new + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + end +end + +private class PlainIO < IO + def read(slice : Bytes) + raise "not implemented" + end + + def write(slice : Bytes) : Nil + raise "not implemented" + end +end + +private class IOWithOriginalFilename < IO + getter original_filename : String? + + def initialize(@original_filename : String?) + end + + def read(slice : Bytes) + raise "not implemented" + end + + def write(slice : Bytes) : Nil + raise "not implemented" + end +end + +private class IOWithFilename < IO + getter filename : String? + + def initialize(@filename : String?) + end + + def read(slice : Bytes) + raise "not implemented" + end + + def write(slice : Bytes) : Nil + raise "not implemented" + end +end + +private class IOWithPath < IO + getter path : String + + def initialize(@path : String) + end + + def read(slice : Bytes) + raise "not implemented" + end + + def write(slice : Bytes) : Nil + raise "not implemented" + end +end diff --git a/spec/lucky/attachment/extractor/mime_from_extension_spec.cr b/spec/lucky/attachment/extractor/mime_from_extension_spec.cr new file mode 100644 index 000000000..c817ec660 --- /dev/null +++ b/spec/lucky/attachment/extractor/mime_from_extension_spec.cr @@ -0,0 +1,149 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Extractor::MimeFromExtension do + describe "#extract" do + subject = Lucky::Attachment::Extractor::MimeFromExtension.new + + context "when a filename is passed in options" do + it "uses the filename from options over the IO filename" do + io = IOWithFilename.new("ignored.png") + result = subject.extract(io, metadata: nil, filename: "overridden.pdf") + + result.should eq("application/pdf") + end + end + + context "when the IO responds to #original_filename" do + it "returns the MIME type for a known extension" do + io = IOWithOriginalFilename.new("photo.png") + result = subject.extract(io, metadata: nil) + + result.should eq("image/png") + end + + it "returns nil when original_filename is nil" do + io = IOWithOriginalFilename.new(nil) + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the IO responds to #filename" do + it "returns the MIME type for a known extension" do + io = IOWithFilename.new("document.pdf") + result = subject.extract(io, metadata: nil) + + result.should eq("application/pdf") + end + + it "returns nil when filename is nil" do + io = IOWithFilename.new(nil) + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + + it "returns nil when filename is blank" do + io = IOWithFilename.new("") + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the IO responds to #path" do + it "returns the MIME type using the basename of the path" do + io = IOWithPath.new("/tmp/uploads/photo.png") + result = subject.extract(io, metadata: nil) + + result.should eq("image/png") + end + + it "handles a path with no directory component" do + io = IOWithPath.new("photo.jpg") + result = subject.extract(io, metadata: nil) + + result.should eq("image/jpeg") + end + end + + context "when the IO has no filename-related methods" do + it "returns nil" do + io = IO::Memory.new + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the filename has an unknown extension" do + it "returns nil" do + io = IOWithFilename.new("file.unknownextension") + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the filename has no extension" do + it "returns nil" do + io = IOWithFilename.new("Makefile") + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the filename has multiple dots" do + it "uses only the last extension" do + io = IOWithFilename.new("my.profile.photo.jpg") + result = subject.extract(io, metadata: nil) + + result.should eq("image/jpeg") + end + end + end +end + +private class IOWithOriginalFilename < IO + getter original_filename : String? + + def initialize(@original_filename : String?) + @io = IO::Memory.new + end + + delegate read, to: @io + + def write(slice : Bytes) : Nil + @io.write(slice) + end +end + +private class IOWithFilename < IO + getter filename : String? + + def initialize(@filename : String?) + @io = IO::Memory.new + end + + delegate read, to: @io + + def write(slice : Bytes) : Nil + @io.write(slice) + end +end + +private class IOWithPath < IO + getter path : String + + def initialize(@path : String) + @io = IO::Memory.new + end + + delegate read, to: @io + + def write(slice : Bytes) : Nil + @io.write(slice) + end +end diff --git a/spec/lucky/attachment/extractor/mime_from_file_spec.cr b/spec/lucky/attachment/extractor/mime_from_file_spec.cr new file mode 100644 index 000000000..6e715bfd0 --- /dev/null +++ b/spec/lucky/attachment/extractor/mime_from_file_spec.cr @@ -0,0 +1,94 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Extractor::MimeFromFile do + describe "#extract" do + subject = Lucky::Attachment::Extractor::MimeFromFile.new + + context "when the IO is empty" do + it "returns nil without invoking the file utility" do + io = IOWithSize.new("", size: 0_i64) + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + + context "when the file utility is not installed" do + it "raises Lucky::Attachment::Error" do + original_path = ENV["PATH"] + ENV["PATH"] = "" + io = IOWithSize.new("Hello, world!") + + begin + expect_raises( + Lucky::Attachment::Error, + "file command-line tool is not installed" + ) do + subject.extract(io, metadata: nil) + end + ensure + ENV["PATH"] = original_path + end + end + end + + context "when the file utility is installed" do + it "returns the MIME type for a PNG file" do + png_base64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwADhQGAWjR9awAAAABJRU5ErkJggg==" + png_bytes = Base64.decode(png_base64) + path = File.tempfile("test", ".png", &.write(png_bytes)).path + file = File.open(path) + result = subject.extract(file, metadata: nil) + + result.should eq("image/png") + ensure + File.delete(path) if path + end + + it "returns the MIME type for plain text" do + io = IOWithSize.new("Hello, world!") + result = subject.extract(io, metadata: nil) + + result.should eq("text/plain") + end + + it "strips surrounding whitespace from the output" do + io = IOWithSize.new("Hello, world!") + result = subject.extract(io, metadata: nil) + + result.should eq(result.try &.strip) + end + + it "rewinds the IO after reading" do + io = IOWithSize.new("Hello, world!") + subject.extract(io, metadata: nil) + + io.pos.should eq(0) + end + + it "returns nil for a nil-size IO" do + io = IOWithSize.new("hello", size: nil) + result = subject.extract(io, metadata: nil) + + result.should_not be_nil + end + end + end +end + +private class IOWithSize < IO + getter size : Int64? + + def initialize(content : String, @size : Int64? = nil) + @io = IO::Memory.new(content) + @size ||= content.bytesize.to_i64 + end + + delegate read, to: @io + delegate rewind, to: @io + delegate pos, to: @io + + def write(slice : Bytes) : Nil + @io.write(slice) + end +end diff --git a/spec/lucky/attachment/extractor/mime_from_io_spec.cr b/spec/lucky/attachment/extractor/mime_from_io_spec.cr new file mode 100644 index 000000000..2e4fef534 --- /dev/null +++ b/spec/lucky/attachment/extractor/mime_from_io_spec.cr @@ -0,0 +1,77 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Extractor::MimeFromIO do + describe "#extract" do + subject = Lucky::Attachment::Extractor::MimeFromIO.new + + context "when the IO responds to #content_type" do + context "and content_type is a plain MIME type" do + it "returns the MIME type" do + io = IOWithContentType.new("image/png") + result = subject.extract(io, metadata: nil) + + result.should eq("image/png") + end + end + + context "and content_type includes parameters (e.g. charset)" do + it "strips parameters and returns only the MIME type" do + io = IOWithContentType.new("text/plain; charset=utf-8") + result = subject.extract(io, metadata: nil) + + result.should eq("text/plain") + end + end + + context "and content_type includes multiple parameters" do + it "strips all parameters and returns only the MIME type" do + io = IOWithContentType.new("multipart/form-data; boundary=something; charset=utf-8") + result = subject.extract(io, metadata: nil) + + result.should eq("multipart/form-data") + end + end + + context "and content_type has surrounding whitespace" do + it "strips whitespace from the MIME type" do + io = IOWithContentType.new(" image/jpeg ; quality=80") + result = subject.extract(io, metadata: nil) + + result.should eq("image/jpeg") + end + end + + context "and content_type is nil" do + it "returns nil" do + io = IOWithContentType.new(nil) + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + end + + context "when the IO does not respond to #content_type" do + it "returns nil" do + io = IO::Memory.new + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + end +end + +private class IOWithContentType < IO + getter content_type : String? + + def initialize(@content_type : String?) + @io = IO::Memory.new + end + + delegate read, to: @io + + def write(slice : Bytes) : Nil + @io.write(slice) + end +end diff --git a/spec/lucky/attachment/extractor/size_from_io_spec.cr b/spec/lucky/attachment/extractor/size_from_io_spec.cr new file mode 100644 index 000000000..68d3de8a6 --- /dev/null +++ b/spec/lucky/attachment/extractor/size_from_io_spec.cr @@ -0,0 +1,100 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Extractor::SizeFromIO do + describe "#extract" do + subject = Lucky::Attachment::Extractor::SizeFromIO.new + + context "when the IO responds to #tempfile" do + it "returns the size of the tempfile" do + tempfile = File.tempfile("test") + tempfile.print("hello lucky") + tempfile.flush + + begin + io = IOWithTempfile.new(tempfile) + result = subject.extract(io, metadata: nil) + result.should eq(11_i64) + ensure + tempfile.delete + end + end + + it "prefers #tempfile over #size when both are present" do + tempfile = File.tempfile("test") + tempfile.print("hello") + tempfile.flush + + begin + io = IOWithTempfile.new(tempfile) + result = subject.extract(io, metadata: nil) + result.should eq(5_i64) + ensure + tempfile.delete + end + end + end + + context "when the IO responds to #size but not #tempfile" do + it "returns the size" do + io = IOWithSize.new(42_i64) + result = subject.extract(io, metadata: nil) + + result.should eq(42_i64) + end + + it "returns 0 for an empty IO" do + io = IOWithSize.new(0_i64) + result = subject.extract(io, metadata: nil) + + result.should eq(0_i64) + end + end + + context "when the IO responds to neither #tempfile nor #size" do + it "returns nil" do + io = PlainIO.new + result = subject.extract(io, metadata: nil) + + result.should be_nil + end + end + end +end + +private class IOWithTempfile < IO + getter tempfile : File + + def initialize(@tempfile : File) + end + + delegate read, to: @tempfile + + def write(slice : Bytes) : Nil + @tempfile.write(slice) + end +end + +private class IOWithSize < IO + getter size : Int64 + + def initialize(@size : Int64) + end + + def read(slice : Bytes) + raise "not implemented" + end + + def write(slice : Bytes) : Nil + raise "not implemented" + end +end + +private class PlainIO < IO + def read(slice : Bytes) + raise "not implemented" + end + + def write(slice : Bytes) : Nil + raise "not implemented" + end +end diff --git a/src/lucky/attachment/extractor/mime_from_file.cr b/src/lucky/attachment/extractor/mime_from_file.cr index d86c93de1..e0ff54f68 100644 --- a/src/lucky/attachment/extractor/mime_from_file.cr +++ b/src/lucky/attachment/extractor/mime_from_file.cr @@ -22,7 +22,7 @@ struct Lucky::Attachment::Extractor::MimeFromFile else Log.debug { "Unable to extract MIME type using `file` utility (#{stderr})" } end - rescue RuntimeError + rescue File::NotFoundError raise Error.new("file command-line tool is not installed") end end From a3c14c4e9f9471d7db17ed53f1deac38cf9f109b Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 16:56:08 +0100 Subject: [PATCH 22/52] Install imagemagick in test docker container --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 5030e38b2..ce8c88df5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM crystallang/crystal:latest WORKDIR /data RUN apt-get update && \ - apt-get install -y curl libreadline-dev unzip file && \ + apt-get install -y curl libreadline-dev unzip file imagemagick && \ curl -fsSL https://bun.sh/install | bash && \ ln -s /root/.bun/bin/bun /usr/local/bin/bun && \ # Cleanup leftovers From 3bc3cd25989924b7c14d8313bb18c41affda4fee Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 16:56:39 +0100 Subject: [PATCH 23/52] Add run_command helper for extractors --- src/lucky/attachment/extractor/run_command.cr | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 src/lucky/attachment/extractor/run_command.cr diff --git a/src/lucky/attachment/extractor/run_command.cr b/src/lucky/attachment/extractor/run_command.cr new file mode 100644 index 000000000..14c705b91 --- /dev/null +++ b/src/lucky/attachment/extractor/run_command.cr @@ -0,0 +1,26 @@ +module Lucky::Attachment::Extractor::RunCommand + # Helper method to execute command-line tools for metadata extractors. + private def run_command( + command : String, + args : Array(String), + input : IO, + ) : String? + stdout, stderr = IO::Memory.new, IO::Memory.new + result = Process.run( + command, + args: args.push("-"), + output: stdout, + error: stderr, + input: input + ) + input.rewind + + return stdout.to_s.strip if result.success? + + Log.debug do + "Unable to extract data with `#{command} #{args.join(' ')}` (#{stderr})" + end + rescue File::NotFoundError + raise Error.new("The `#{command}` command-line tool is not installed") + end +end From 8176c17e7b909b5cc644ab6692bcfa1ff439c614 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 16:57:08 +0100 Subject: [PATCH 24/52] Rework mime from file extractor to use the run command helper --- .../extractor/mime_from_file_spec.cr | 2 +- .../attachment/extractor/mime_from_file.cr | 24 ++++--------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/spec/lucky/attachment/extractor/mime_from_file_spec.cr b/spec/lucky/attachment/extractor/mime_from_file_spec.cr index 6e715bfd0..9a33af4b6 100644 --- a/spec/lucky/attachment/extractor/mime_from_file_spec.cr +++ b/spec/lucky/attachment/extractor/mime_from_file_spec.cr @@ -22,7 +22,7 @@ describe Lucky::Attachment::Extractor::MimeFromFile do begin expect_raises( Lucky::Attachment::Error, - "file command-line tool is not installed" + "The `file` command-line tool is not installed" ) do subject.extract(io, metadata: nil) end diff --git a/src/lucky/attachment/extractor/mime_from_file.cr b/src/lucky/attachment/extractor/mime_from_file.cr index e0ff54f68..0af8b0e53 100644 --- a/src/lucky/attachment/extractor/mime_from_file.cr +++ b/src/lucky/attachment/extractor/mime_from_file.cr @@ -1,28 +1,14 @@ +require "./run_command" + struct Lucky::Attachment::Extractor::MimeFromFile include Lucky::Attachment::Extractor + include Lucky::Attachment::Extractor::RunCommand - # Extracts the MIME type using the `file` utility, and raises when it's not - # installed. + # Extracts the MIME type using the `file` utility. def extract(io, metadata, **options) : String? # Avoids returning "application/x-empty" for empty files return nil if io.size.try &.zero? - stdout, stderr = IO::Memory.new, IO::Memory.new - command = Process.run( - "file", - args: ["--mime-type", "--brief", "-"], - output: stdout, - error: stderr, - input: io - ) - - if command.success? - io.rewind - stdout.to_s.strip - else - Log.debug { "Unable to extract MIME type using `file` utility (#{stderr})" } - end - rescue File::NotFoundError - raise Error.new("file command-line tool is not installed") + run_command("file", %w[--mime-type --brief], io) end end From 0c19c3f3080b79f782684c568e937e2e912a3283 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 16:57:34 +0100 Subject: [PATCH 25/52] Add small png logo fixture for dimensions from identify extractor --- spec/fixtures/lucky_logo_tiny.png | Bin 0 -> 1644 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 spec/fixtures/lucky_logo_tiny.png diff --git a/spec/fixtures/lucky_logo_tiny.png b/spec/fixtures/lucky_logo_tiny.png new file mode 100644 index 0000000000000000000000000000000000000000..8772c812a6099c0a769405f89a0af13a9101fd24 GIT binary patch literal 1644 zcmV-y29x=TP)004R> z004l5008;`004mK004C`008P>0026e000+ooVrmw00006VoOIv00000008+zyMF)x z00(qQO+^Rl0|yT<0Ci@e;{X5!Oi4sRR9M5^*K3SjRTT&D-@eajr_;`KT7*ier4M{o zY1Lo>6SW@{gT|;tMM@zBEX8+BG$e#b6k>eWgiwRk#-LI0RrsI=i)he*5h7wG23wTW zmeN+*PG|14bG9G$KAkhSy`_nw#+B@xeb!m~_22*XI6+0u+^~g62&Ttq9j?Oj@i*Ky zwsZQ%!R32LXTNf<2hLvS|5ZZSnZMz#vRjWa-hhvzbQi$193e zbD!|38MhoaVT`f&Vj-OL`~Qm6s)xz#LVTbK2=p&F)b3w+=)(_fU%qmnK5j|>-v8E~ z>nfM|Tu30XB*s{}%^i5_F^fq1v@t{H?$hc!YT3*^cdi+FY^ zO)p?3x}H-ARnf;DwkWF%IQhza=xT9CtJW^tpWapXUqMjjbrD{cgMg zua*)Dd<&avL6C3!Qk+-ac6+#E`dK4dwZe*_-3A(+@&xDSNd>{yc%$b)iVXZUl3U%2 zx8$3ChZOg|i7()=$vE?3d{J_V-(*1kTDgD?cnAZy5ucV=vmKvBr%}X05EfxwWn!Tb z^`CuEXcZRq@3&yuGvegunnby~qhiN)JO7o#_wvC43$Zx&H=2wANAPvYO}d%*f{M+T z@L1RPC-E66rj#U@$Ur^{wPvFka5a`zx7m04sP^2Bf%>>*Gxw;6qHAt1!c`p|L9i)a z`|PKmnvTntT9;Y%y(tClLUNbx@h5W4>e$t?n|H|%@V(0W&!pg-HgM$VkqdCylwGw7 z{R>8fdi25>`^;{QR#{S&HEr$XQ4}5RiNy(YqY~v!xr`Lo=H>n~676?M0r>5hUa>t7 z*Gbu^3gqAMZHXyw!beX4d`F5+hcmF7@_IrD8d6k0qk4qC=Z$F&jhofzSURv*(39iy zu(l?wk9t;REAYwO4|yY-@c>>T#j=BVT#6ZsF_ypY#CPxsya&_p6P%7USb}?`OjEA^ z`wUi@0k6Y@Qu?mS!3ow3iX#<*vxL01;y5cYNH>-qWe z+)5ddQsf3)m_fL{YwR6TU?utbVyWi1B)2LnZpYJ=^#cAOWvrJyDK za=*;N$CODT?KvII!Epoiaf|!+I|Zpbo}Lj8V}|4X{y+xiaq0HgOKj@HQ&Mg6Rg>=Z zl$3#fg||unVjcW8+ufh-wn_EQ!x@;zq+gW}No-h!Kbd6I0sKNLL8g@F;mj{@xd*R@ zP>Tl7-KTHfsAU6toIm{JserOM2vxhG8cbW2+v}E&$vUJi)gUd2(g#nnR=-sE z9g&z>jsE3&MV`NuK9gB>%+vs1)@0+Ds50dyoF!V^7&xLe+|gH?uxQ5qT)~|5H{;Lv zN)%6&1;$SWQHe=UcPq;cG9_z` z05UK#HZ3qSEigA!F*Q0gH99plD=;uRFfi=OiOv8303~!qSaf7zbY(hiZ)9m^c>ppn zGBqtRFfB7MR53U@H90ymI4dwPIxsLhF}1}2001a-MObuXVRU6WbZKp6b97;CZ~!te qGBzzRGc7PTR53L=G&njiFe@-HIxsLLq`KY!0000 Date: Sat, 7 Mar 2026 16:57:57 +0100 Subject: [PATCH 26/52] Add dimensions from identify extractor --- .../dimensions_from_identify_spec.cr | 54 +++++++++++++++++++ .../extractor/dimensions_from_identify.cr | 17 ++++++ 2 files changed, 71 insertions(+) create mode 100644 spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr create mode 100644 src/lucky/attachment/extractor/dimensions_from_identify.cr diff --git a/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr b/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr new file mode 100644 index 000000000..e69bc60af --- /dev/null +++ b/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr @@ -0,0 +1,54 @@ +require "../../../spec_helper" + +describe Lucky::Attachment::Extractor::DimensionsFromIdentify do + describe "#extract" do + subject = Lucky::Attachment::Extractor::DimensionsFromIdentify.new + png_path = "spec/fixtures/lucky_logo_tiny.png" + + context "when identify is not installed" do + it "raises Lucky::Attachment::Error" do + original_path = ENV["PATH"] + ENV["PATH"] = "" + io = IO::Memory.new + + begin + expect_raises( + Lucky::Attachment::Error, + "The `identify` command-line tool is not installed" + ) do + subject.extract(io, metadata: Lucky::Attachment::MetadataHash.new) + end + ensure + ENV["PATH"] = original_path + end + end + end + + context "when identify is installed" do + it "extracts width and height from a PNG file" do + file = File.open(png_path) + metadata = Lucky::Attachment::MetadataHash.new + subject.extract(file, metadata: metadata) + + metadata["width"].should eq(69) + metadata["height"].should eq(16) + end + + it "does not modify metadata when identify returns no output" do + io = IO::Memory.new + metadata = Lucky::Attachment::MetadataHash.new + subject.extract(io, metadata: metadata) + + metadata.should be_empty + end + + it "rewinds the IO after reading" do + file = File.open(png_path) + metadata = Lucky::Attachment::MetadataHash.new + subject.extract(file, metadata: metadata) + + file.pos.should eq(0) + end + end + end +end diff --git a/src/lucky/attachment/extractor/dimensions_from_identify.cr b/src/lucky/attachment/extractor/dimensions_from_identify.cr new file mode 100644 index 000000000..403cdacd2 --- /dev/null +++ b/src/lucky/attachment/extractor/dimensions_from_identify.cr @@ -0,0 +1,17 @@ +require "./run_command" + +struct Lucky::Attachment::Extractor::DimensionsFromIdentify + include Lucky::Attachment::Extractor + include Lucky::Attachment::Extractor::RunCommand + + # Extracts the dimensions of a file using ImageMagick's `identify` command. + def extract(io, metadata, **options) : Nil + if result = run_command("identify", ["-format", "%[fx:w] %[fx:h]"], io) + dimensions = result.split.map(&.to_i) + + metadata["hay"] = "sma" + metadata["width"] = dimensions[0] + metadata["height"] = dimensions[1] + end + end +end From 2ab8997fecac4cc94f5d9144dc64420692dce680 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 17:03:08 +0100 Subject: [PATCH 27/52] Remove debug line from dimensions extractor --- src/lucky/attachment/extractor/dimensions_from_identify.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lucky/attachment/extractor/dimensions_from_identify.cr b/src/lucky/attachment/extractor/dimensions_from_identify.cr index 403cdacd2..c6029c730 100644 --- a/src/lucky/attachment/extractor/dimensions_from_identify.cr +++ b/src/lucky/attachment/extractor/dimensions_from_identify.cr @@ -9,7 +9,6 @@ struct Lucky::Attachment::Extractor::DimensionsFromIdentify if result = run_command("identify", ["-format", "%[fx:w] %[fx:h]"], io) dimensions = result.split.map(&.to_i) - metadata["hay"] = "sma" metadata["width"] = dimensions[0] metadata["height"] = dimensions[1] end From b5d83bfdd37bf015609130ba46e5edf930c00e40 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 17:59:41 +0100 Subject: [PATCH 28/52] Add comments documenting the extract macro and run command module --- src/lucky/attachment/extractor/run_command.cr | 17 ++++++++++++++++- src/lucky/attachment/uploader.cr | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/lucky/attachment/extractor/run_command.cr b/src/lucky/attachment/extractor/run_command.cr index 14c705b91..7d072fd18 100644 --- a/src/lucky/attachment/extractor/run_command.cr +++ b/src/lucky/attachment/extractor/run_command.cr @@ -1,5 +1,20 @@ +# Some extractors may need to call on command-line tools to extract certain +# data. This module will provide a helper to simplify running commands. +# +# ``` +# struct ColourspaceFromIdentify +# include Lucky::Attachment::Extractor +# include Lucky::Attachment::Extractor::RunCommand +# +# def extract(io, metadata, **options) : String? +# run_command("identify", ["-format", "%[colorspace]"], io) +# end +# end +# ``` +# module Lucky::Attachment::Extractor::RunCommand - # Helper method to execute command-line tools for metadata extractors. + # Runs th given command on the given IO object and returns the resulting + # string if the command was successful. private def run_command( command : String, args : Array(String), diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 8e24e4d2b..6ce12454a 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -23,6 +23,23 @@ abstract struct Lucky::Attachment::Uploader } of String => Extractor # Registers an extractor for a given key. + # + # ``` + # struct PdfUploader < Lucky::Attachment::Uploader + # # Use a different MIME type extractor than the default one + # extract mime_type : Lucky::Attachment::Extractor::MimeFromExtension + # + # # Or use your own custom extractor to add arbitrary data + # extract pages : MyNumberOfPagesExtractor + # end + # ``` + # + # The result will then be added to the attachment's metadata after uploading: + # ``` + # invoice.pdf.metadata["pages"] + # # => 24 + # ``` + # macro extract(type_declaration) EXTRACTORS["{{type_declaration.var.id}}"] = type_declaration.type.new end From 1fbef59995d57d956be8d30a3e7bd84c6caf5ae7 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 18:48:34 +0100 Subject: [PATCH 29/52] Fix bug in extract macro --- src/lucky/attachment/uploader.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 6ce12454a..339d77adc 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -41,7 +41,7 @@ abstract struct Lucky::Attachment::Uploader # ``` # macro extract(type_declaration) - EXTRACTORS["{{type_declaration.var.id}}"] = type_declaration.type.new + EXTRACTORS["{{type_declaration.var.id}}"] = {{type_declaration.type}}.new end getter storage_key : String From ab6fdb3cd5429c670ceae83981318768b2aa0211 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 18:57:59 +0100 Subject: [PATCH 30/52] Install ImageMagick in CI flow --- .github/workflows/ci.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62889e8ad..a525e11ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,7 @@ on: push: branches: [main] pull_request: - branches: "*" + branches: '*' jobs: check_format: @@ -54,6 +54,15 @@ jobs: - uses: crystal-lang/install-crystal@v1 with: crystal: ${{matrix.crystal_version}} + - name: Install ImageMagick (Ubuntu) + if: runner.os == 'Linux' + run: sudo apt-get install -y imagemagick + - name: Install ImageMagick (macOS) + if: runner.os == 'macOS' + run: brew install imagemagick + - name: Install ImageMagick (Windows) + if: runner.os == 'Windows' + run: choco install imagemagick --no-progress - name: Install shards run: shards install --skip-postinstall --skip-executables env: From 11a9b2a6d7ecf3fde18a0b833562dcc0a73c851b Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 19:12:13 +0100 Subject: [PATCH 31/52] Make sure imagemagick is in the path on windows in CI --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a525e11ae..a4ba22488 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,7 +62,9 @@ jobs: run: brew install imagemagick - name: Install ImageMagick (Windows) if: runner.os == 'Windows' - run: choco install imagemagick --no-progress + run: | + $magickPath = (Get-Item "C:\Program Files\ImageMagick-*" | Select-Object -Last 1).FullName + echo "$magickPath" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - name: Install shards run: shards install --skip-postinstall --skip-executables env: From 6bd90b571269da0faa6808a3e488b60c31ef97a9 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 19:17:32 +0100 Subject: [PATCH 32/52] Debugging imagemagick installation on Windows --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4ba22488..1fe0e6a68 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,7 +64,14 @@ jobs: if: runner.os == 'Windows' run: | $magickPath = (Get-Item "C:\Program Files\ImageMagick-*" | Select-Object -Last 1).FullName + Write-Output "Found ImageMagick at: $magickPath" + Write-Output "Contents: $(Get-ChildItem $magickPath | Select-Object -ExpandProperty Name)" echo "$magickPath" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append + - name: Verify ImageMagick (Windows) + if: runner.os == 'Windows' + run: | + Write-Output "identify location: $(Get-Command identify -ErrorAction SilentlyContinue | Select-Object -ExpandProperty Source)" + identify --version - name: Install shards run: shards install --skip-postinstall --skip-executables env: From b6cb93326e0de51a0457e740fe1c2d990abfe0aa Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 19:50:15 +0100 Subject: [PATCH 33/52] Remove ImageMagick installation on Windows (already installed) --- .github/workflows/ci.yml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1fe0e6a68..650c1e874 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,18 +60,6 @@ jobs: - name: Install ImageMagick (macOS) if: runner.os == 'macOS' run: brew install imagemagick - - name: Install ImageMagick (Windows) - if: runner.os == 'Windows' - run: | - $magickPath = (Get-Item "C:\Program Files\ImageMagick-*" | Select-Object -Last 1).FullName - Write-Output "Found ImageMagick at: $magickPath" - Write-Output "Contents: $(Get-ChildItem $magickPath | Select-Object -ExpandProperty Name)" - echo "$magickPath" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append - - name: Verify ImageMagick (Windows) - if: runner.os == 'Windows' - run: | - Write-Output "identify location: $(Get-Command identify -ErrorAction SilentlyContinue | Select-Object -ExpandProperty Source)" - identify --version - name: Install shards run: shards install --skip-postinstall --skip-executables env: From 17fcb20969611d04e9d2dde07b232e0e1399b793 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 19:51:56 +0100 Subject: [PATCH 34/52] Rework the dimensions extractor to work with both `magick` and `identify` --- .../dimensions_from_identify_spec.cr | 12 +++++------ .../extractor/dimensions_from_identify.cr | 16 -------------- .../extractor/dimensions_from_magick.cr | 21 +++++++++++++++++++ src/lucky/attachment/extractor/run_command.cr | 7 +++++-- 4 files changed, 32 insertions(+), 24 deletions(-) delete mode 100644 src/lucky/attachment/extractor/dimensions_from_identify.cr create mode 100644 src/lucky/attachment/extractor/dimensions_from_magick.cr diff --git a/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr b/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr index e69bc60af..600408f92 100644 --- a/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr +++ b/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr @@ -1,11 +1,11 @@ require "../../../spec_helper" -describe Lucky::Attachment::Extractor::DimensionsFromIdentify do +describe Lucky::Attachment::Extractor::DimensionsFromMagick do describe "#extract" do - subject = Lucky::Attachment::Extractor::DimensionsFromIdentify.new + subject = Lucky::Attachment::Extractor::DimensionsFromMagick.new png_path = "spec/fixtures/lucky_logo_tiny.png" - context "when identify is not installed" do + context "when magick is not installed" do it "raises Lucky::Attachment::Error" do original_path = ENV["PATH"] ENV["PATH"] = "" @@ -14,7 +14,7 @@ describe Lucky::Attachment::Extractor::DimensionsFromIdentify do begin expect_raises( Lucky::Attachment::Error, - "The `identify` command-line tool is not installed" + /The `magick|identify` command-line tool is not installed/ ) do subject.extract(io, metadata: Lucky::Attachment::MetadataHash.new) end @@ -24,7 +24,7 @@ describe Lucky::Attachment::Extractor::DimensionsFromIdentify do end end - context "when identify is installed" do + context "when magick is installed" do it "extracts width and height from a PNG file" do file = File.open(png_path) metadata = Lucky::Attachment::MetadataHash.new @@ -34,7 +34,7 @@ describe Lucky::Attachment::Extractor::DimensionsFromIdentify do metadata["height"].should eq(16) end - it "does not modify metadata when identify returns no output" do + it "does not modify metadata when magick returns no output" do io = IO::Memory.new metadata = Lucky::Attachment::MetadataHash.new subject.extract(io, metadata: metadata) diff --git a/src/lucky/attachment/extractor/dimensions_from_identify.cr b/src/lucky/attachment/extractor/dimensions_from_identify.cr deleted file mode 100644 index c6029c730..000000000 --- a/src/lucky/attachment/extractor/dimensions_from_identify.cr +++ /dev/null @@ -1,16 +0,0 @@ -require "./run_command" - -struct Lucky::Attachment::Extractor::DimensionsFromIdentify - include Lucky::Attachment::Extractor - include Lucky::Attachment::Extractor::RunCommand - - # Extracts the dimensions of a file using ImageMagick's `identify` command. - def extract(io, metadata, **options) : Nil - if result = run_command("identify", ["-format", "%[fx:w] %[fx:h]"], io) - dimensions = result.split.map(&.to_i) - - metadata["width"] = dimensions[0] - metadata["height"] = dimensions[1] - end - end -end diff --git a/src/lucky/attachment/extractor/dimensions_from_magick.cr b/src/lucky/attachment/extractor/dimensions_from_magick.cr new file mode 100644 index 000000000..36282d7f2 --- /dev/null +++ b/src/lucky/attachment/extractor/dimensions_from_magick.cr @@ -0,0 +1,21 @@ +require "./run_command" + +struct Lucky::Attachment::Extractor::DimensionsFromMagick + include Lucky::Attachment::Extractor + include Lucky::Attachment::Extractor::RunCommand + + # Extracts the dimensions of a file using ImageMagick's `magick` command. + def extract(io, metadata, **options) : Nil + if result = run_magick_command(io, ["identify", "-format", "%[fx:w] %[fx:h]"]) + dimensions = result.split.map(&.to_i) + + metadata["width"] = dimensions[0] + metadata["height"] = dimensions[1] + end + end + + private def run_magick_command(io, args) + run_command("magick", args, io, fail_silent: true) || + run_command("identify", args[1..], io) + end +end diff --git a/src/lucky/attachment/extractor/run_command.cr b/src/lucky/attachment/extractor/run_command.cr index 7d072fd18..19a65eb2d 100644 --- a/src/lucky/attachment/extractor/run_command.cr +++ b/src/lucky/attachment/extractor/run_command.cr @@ -7,7 +7,7 @@ # include Lucky::Attachment::Extractor::RunCommand # # def extract(io, metadata, **options) : String? -# run_command("identify", ["-format", "%[colorspace]"], io) +# run_command("magick identify", ["-format", "%[colorspace]"], io) # end # end # ``` @@ -19,11 +19,12 @@ module Lucky::Attachment::Extractor::RunCommand command : String, args : Array(String), input : IO, + fail_silent : Bool = false, ) : String? stdout, stderr = IO::Memory.new, IO::Memory.new result = Process.run( command, - args: args.push("-"), + args: args + ["-"], output: stdout, error: stderr, input: input @@ -36,6 +37,8 @@ module Lucky::Attachment::Extractor::RunCommand "Unable to extract data with `#{command} #{args.join(' ')}` (#{stderr})" end rescue File::NotFoundError + return if fail_silent + raise Error.new("The `#{command}` command-line tool is not installed") end end From 396bbc255917a5739ac829725bea9c087d6713ea Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 7 Mar 2026 20:07:14 +0100 Subject: [PATCH 35/52] Properly handle fallback commands in dimension extractor --- src/lucky/attachment.cr | 2 ++ src/lucky/attachment/extractor/dimensions_from_magick.cr | 5 +++-- src/lucky/attachment/extractor/run_command.cr | 5 +---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr index e2f589f64..a5cd8d92b 100644 --- a/src/lucky/attachment.cr +++ b/src/lucky/attachment.cr @@ -11,4 +11,6 @@ module Lucky::Attachment class FileNotFound < Error; end class InvalidFile < Error; end + + class CliToolNotFound < Error; end end diff --git a/src/lucky/attachment/extractor/dimensions_from_magick.cr b/src/lucky/attachment/extractor/dimensions_from_magick.cr index 36282d7f2..782894ca6 100644 --- a/src/lucky/attachment/extractor/dimensions_from_magick.cr +++ b/src/lucky/attachment/extractor/dimensions_from_magick.cr @@ -15,7 +15,8 @@ struct Lucky::Attachment::Extractor::DimensionsFromMagick end private def run_magick_command(io, args) - run_command("magick", args, io, fail_silent: true) || - run_command("identify", args[1..], io) + run_command("magick", args, io) + rescue CliToolNotFound + run_command("identify", args[1..], io) end end diff --git a/src/lucky/attachment/extractor/run_command.cr b/src/lucky/attachment/extractor/run_command.cr index 19a65eb2d..e4acf3252 100644 --- a/src/lucky/attachment/extractor/run_command.cr +++ b/src/lucky/attachment/extractor/run_command.cr @@ -19,7 +19,6 @@ module Lucky::Attachment::Extractor::RunCommand command : String, args : Array(String), input : IO, - fail_silent : Bool = false, ) : String? stdout, stderr = IO::Memory.new, IO::Memory.new result = Process.run( @@ -37,8 +36,6 @@ module Lucky::Attachment::Extractor::RunCommand "Unable to extract data with `#{command} #{args.join(' ')}` (#{stderr})" end rescue File::NotFoundError - return if fail_silent - - raise Error.new("The `#{command}` command-line tool is not installed") + raise CliToolNotFound.new("The `#{command}` command-line tool is not installed") end end From 735598f73b0e4fe0d53fcc8c65687d14ed5e143a Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 8 Mar 2026 08:02:29 +0100 Subject: [PATCH 36/52] Fix error in run command example comment --- src/lucky/attachment/extractor/run_command.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lucky/attachment/extractor/run_command.cr b/src/lucky/attachment/extractor/run_command.cr index e4acf3252..5956a1025 100644 --- a/src/lucky/attachment/extractor/run_command.cr +++ b/src/lucky/attachment/extractor/run_command.cr @@ -7,7 +7,7 @@ # include Lucky::Attachment::Extractor::RunCommand # # def extract(io, metadata, **options) : String? -# run_command("magick identify", ["-format", "%[colorspace]"], io) +# run_command("magick", ["identify", "-format", "%[colorspace]"], io) # end # end # ``` From 4815328a3567fb59747eda17783417382a23a423 Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 13 Mar 2026 10:39:13 +0100 Subject: [PATCH 37/52] Change signature on `extract` macro for uploaders Replace the (misleading) type declaration syntax with a two argument approach with a `using` keyword for readability. --- src/lucky/attachment/uploader.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 339d77adc..f78c27401 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -27,10 +27,10 @@ abstract struct Lucky::Attachment::Uploader # ``` # struct PdfUploader < Lucky::Attachment::Uploader # # Use a different MIME type extractor than the default one - # extract mime_type : Lucky::Attachment::Extractor::MimeFromExtension + # extract mime_type, using: Lucky::Attachment::Extractor::MimeFromExtension # # # Or use your own custom extractor to add arbitrary data - # extract pages : MyNumberOfPagesExtractor + # extract pages, using: MyNumberOfPagesExtractor # end # ``` # @@ -40,8 +40,8 @@ abstract struct Lucky::Attachment::Uploader # # => 24 # ``` # - macro extract(type_declaration) - EXTRACTORS["{{type_declaration.var.id}}"] = {{type_declaration.type}}.new + macro extract(name, using) + EXTRACTORS["{{name.id}}"] = {{using}}.new end getter storage_key : String From 3f976107baa9471eb473a09ee782f6c576436a60 Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 13 Mar 2026 14:14:59 +0100 Subject: [PATCH 38/52] Rework type declaration for attach macro The type declaration now correctly reflects the type that is returned by the attach attribute. Uploaders now each have a dedicated `StoredFile` class that can be reopened to add metadata-specific methods. --- src/lucky/attachment.cr | 2 - src/lucky/attachment/avram.cr | 15 ++-- src/lucky/attachment/uploader.cr | 133 +++++++++++++++++------------- src/lucky/attachment/utilities.cr | 47 ----------- 4 files changed, 82 insertions(+), 115 deletions(-) diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr index a5cd8d92b..d7609d2a1 100644 --- a/src/lucky/attachment.cr +++ b/src/lucky/attachment.cr @@ -4,8 +4,6 @@ module Lucky::Attachment alias MetadataValue = String | Int32 | Int64 | UInt32 | UInt64 | Float64 | Bool | Nil alias MetadataHash = Hash(String, MetadataValue) - # Log = ::Log.for("lucky.attachment") - class Error < Exception; end class FileNotFound < Error; end diff --git a/src/lucky/attachment/avram.cr b/src/lucky/attachment/avram.cr index 34beef287..d6b109857 100644 --- a/src/lucky/attachment/avram.cr +++ b/src/lucky/attachment/avram.cr @@ -15,9 +15,9 @@ module Avram::Attachment::Model # class as the type. # # ``` - # attach avatar : ImageUploader + # attach avatar : ImageUploader::StoredFile # # or - # attach avatar : ImageUploader? + # attach avatar : ImageUploader::StoredFile? # ``` # # It is assumed that a `jsonb` column exists with the same name. So in your @@ -33,7 +33,7 @@ module Avram::Attachment::Model # # ``` # user.avatar.class - # # => Lucky::Attachment::StoredFile + # # => ImageUploader::StoredFile # # user.avatar.url # # => "https://bucket.s3.amazonaws.com/user/1/avatar/abc123.jpg" @@ -46,18 +46,19 @@ module Avram::Attachment::Model # settings, but also on attachment level: # # ``` - # attach avatar : ImageUploader?, path_prefix: ":model/images/:id" + # attach avatar : ImageUploader::StoredFile?, path_prefix: ":model/images/:id" # ``` # macro attach(type_declaration, path_prefix = nil) {% name = type_declaration.var %} {% if type_declaration.type.is_a?(Union) %} - {% uploader = type_declaration.type.types.first %} + {% stored_file = type_declaration.type.types.first %} {% nilable = true %} {% else %} - {% uploader = type_declaration.type %} + {% stored_file = type_declaration.type %} {% nilable = false %} {% end %} + {% uploader = stored_file.stringify.split("::")[0..-2].join("::").id %} # Registers a path prefix for the attachment. {% if !@type.constant(:ATTACHMENT_PREFIXES) %} @@ -74,7 +75,7 @@ module Avram::Attachment::Model {% end %} ATTACHMENT_UPLOADERS[:{{ name }}] = {{ uploader }} - column {{ name }} : ::Lucky::Attachment::StoredFile{% if nilable %}?{% end %}, serialize: true + column {{ name }} : ::{{ stored_file }}{% if nilable %}?{% end %}, serialize: true end end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index f78c27401..dc67da013 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -16,12 +16,86 @@ require "uuid" # ``` # abstract struct Lucky::Attachment::Uploader + alias MetadataHash = ::Lucky::Attachment::MetadataHash + EXTRACTORS = { "filename" => Extractor::FilenameFromIO.new, "mime_type" => Extractor::MimeFromIO.new, "size" => Extractor::SizeFromIO.new, } of String => Extractor + macro inherited + {% stored_file = "#{@type}::StoredFile".id %} + + class {{ stored_file }} < Lucky::Attachment::StoredFile + end + + # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method + # accepts additional metadata and arbitrary arguments for overrides. + # + # ``` + # uploader.upload(io) + # uploader.upload(io, metadata: {"custom" => "value"}) + # uploader.upload(io, location: "custom/path.jpg") + # ``` + # + def upload(io : IO, metadata : MetadataHash? = nil, **options) : {{ stored_file }} + data = extract_metadata(io, metadata, **options) + data = data.merge(metadata) if metadata + location = options[:location]? || generate_location(io, data, **options) + + storage.upload(io, location, **options.merge(metadata: data)) + {{ stored_file }}.new(id: location, storage_key: storage_key, metadata: data) + end + + # Uploads to the "cache" storage. + # + # ``` + # cached = ImageUploader.cache(io) + # ``` + # + def self.cache(io : IO, **options) : {{ stored_file }} + new("cache").upload(io, **options) + end + + # Uploads to the "store" storage. + # + # ``` + # stored = ImageUploader.store(io) + # ``` + # + def self.store(io : IO, **options) : {{ stored_file }} + new("store").upload(io, **options) + end + + # Promotes a file from cache to store. + # + # ``` + # cached = ImageUploader.cache(io) + # stored = ImageUploader.promote(cached) + # ``` + # + def self.promote( + file : {{ stored_file }}, + to storage : String = "store", + delete_source : Bool = true, + **options, + ) : {{ stored_file }} + file.open do |io| + ::Lucky::Attachment + .find_storage(storage) + .upload(io, file.id, metadata: file.metadata) + promoted = {{ stored_file }}.new( + id: file.id, + storage_key: storage, + metadata: file.metadata + ) + file.delete if delete_source + promoted + end + end + end + # Registers an extractor for a given key. # # ``` @@ -54,65 +128,6 @@ abstract struct Lucky::Attachment::Uploader Lucky::Attachment.find_storage(storage_key) end - # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method - # accepts additional metadata and arbitrary arguments for overrides. - # - # ``` - # uploader.upload(io) - # uploader.upload(io, metadata: {"custom" => "value"}) - # uploader.upload(io, location: "custom/path.jpg") - # ``` - # - def upload(io : IO, metadata : MetadataHash? = nil, **options) : StoredFile - data = extract_metadata(io, metadata, **options) - data = data.merge(metadata) if metadata - location = options[:location]? || generate_location(io, data, **options) - - storage.upload(io, location, **options.merge(metadata: data)) - StoredFile.new(id: location, storage_key: storage_key, metadata: data) - end - - # Uploads to the "cache" storage. - # - # ``` - # cached = ImageUploader.cache(io) - # ``` - # - def self.cache(io : IO, **options) : StoredFile - new("cache").upload(io, **options) - end - - # Uploads to the "store" storage. - # - # ``` - # stored = ImageUploader.store(io) - # ``` - # - def self.store(io : IO, **options) : StoredFile - new("store").upload(io, **options) - end - - # Promotes a file from cache to store. - # - # ``` - # cached = ImageUploader.cache(io) - # stored = ImageUploader.promote(cached) - # ``` - # - def self.promote( - file : StoredFile, - to storage : String = "store", - delete_source : Bool = true, - **options, - ) : StoredFile - Lucky::Attachment.promote( - file, - **options, - to: storage, - delete_source: delete_source - ) - end - # Generates a unique location for the uploaded file. Override this in # subclasses for custom locations. # diff --git a/src/lucky/attachment/utilities.cr b/src/lucky/attachment/utilities.cr index 75d83f1e5..b933d5121 100644 --- a/src/lucky/attachment/utilities.cr +++ b/src/lucky/attachment/utilities.cr @@ -1,51 +1,4 @@ module Lucky::Attachment - # Move a file from one storage to another (typically cache -> store). - # - # ``` - # stored = Lucky::Attachment.promote(cached, to: "store") - # ``` - # - def self.promote( - file : StoredFile, - to storage : String = "store", - delete_source : Bool = true, - ) : StoredFile - file.open do |io| - find_storage(storage).upload(io, file.id, metadata: file.metadata) - promoted = StoredFile.new( - id: file.id, - storage_key: storage, - metadata: file.metadata - ) - file.delete if delete_source - promoted - end - end - - # Deserialize an StoredFile from various sources. - # - # ``` - # Lucky::Attachment.uploaded_file(json_string) - # Lucky::Attachment.uploaded_file(json_any) - # Lucky::Attachment.uploaded_file(uploaded_file) - # ``` - # - def self.uploaded_file(json : String) : StoredFile - StoredFile.from_json(json) - end - - def self.uploaded_file(json : JSON::Any) : StoredFile - StoredFile.from_json(json.to_json) - end - - def self.uploaded_file(file : StoredFile) : StoredFile - file - end - - def self.uploaded_file(value : Nil) : Nil - nil - end - # Utility to work with a file IO. If the IO is already a File, yields it # directly, Otherwise copies to a tempfile, yields it, then cleans up. # From fb3a0fec2f46a28c217b4821155987e764523868 Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 13 Mar 2026 20:32:25 +0100 Subject: [PATCH 39/52] Make dimensions extract return a string value --- src/lucky/attachment/extractor/dimensions_from_magick.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lucky/attachment/extractor/dimensions_from_magick.cr b/src/lucky/attachment/extractor/dimensions_from_magick.cr index 782894ca6..774f81b4b 100644 --- a/src/lucky/attachment/extractor/dimensions_from_magick.cr +++ b/src/lucky/attachment/extractor/dimensions_from_magick.cr @@ -5,12 +5,13 @@ struct Lucky::Attachment::Extractor::DimensionsFromMagick include Lucky::Attachment::Extractor::RunCommand # Extracts the dimensions of a file using ImageMagick's `magick` command. - def extract(io, metadata, **options) : Nil + def extract(io, metadata, **options) : String? if result = run_magick_command(io, ["identify", "-format", "%[fx:w] %[fx:h]"]) dimensions = result.split.map(&.to_i) metadata["width"] = dimensions[0] metadata["height"] = dimensions[1] + "#{dimensions[0]}x#{dimensions[0]}" end end From eab92ed6f2ed1f44d4dded1824369391c9e0ce88 Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 13 Mar 2026 20:33:37 +0100 Subject: [PATCH 40/52] Make mime from IO extractor more resilient --- src/lucky/attachment/extractor/mime_from_io.cr | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lucky/attachment/extractor/mime_from_io.cr b/src/lucky/attachment/extractor/mime_from_io.cr index 035b50dd9..a4e2d859d 100644 --- a/src/lucky/attachment/extractor/mime_from_io.cr +++ b/src/lucky/attachment/extractor/mime_from_io.cr @@ -3,8 +3,11 @@ struct Lucky::Attachment::Extractor::MimeFromIO # Extracts the MIME type from the IO. def extract(io, metadata, **options) : String? - return unless io.responds_to?(:content_type) && (type = io.content_type) + return unless io.responds_to?(:content_type) + return unless type = io.content_type + return unless mime = type.split(';').first?.try(&.strip) + return if mime.empty? - type.split(';').first.strip + mime if mime.matches?(/\A\w+\/[\w\.\+\-]+\z/) end end From e0481c69f3b30323bf9c4188b700361d133c8ba1 Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 13 Mar 2026 20:34:20 +0100 Subject: [PATCH 41/52] Remove duplicate methods from stored file --- src/lucky/attachment/stored_file.cr | 51 ----------------------------- 1 file changed, 51 deletions(-) diff --git a/src/lucky/attachment/stored_file.cr b/src/lucky/attachment/stored_file.cr index e8632ab26..f1d61cef4 100644 --- a/src/lucky/attachment/stored_file.cr +++ b/src/lucky/attachment/stored_file.cr @@ -43,17 +43,6 @@ class Lucky::Attachment::StoredFile ) end - # Returns the original filename from metadata. - # - # ``` - # file.original_filename - # # => "photo.jpg" - # ``` - # - def original_filename : String? - metadata["filename"]?.try(&.as(String)) - end - # Returns the file extension based on the id or original filename. # # ``` @@ -69,46 +58,6 @@ class Lucky::Attachment::StoredFile ext.presence.try(&.downcase) end - # Returns the file size in bytes from metadata. - # - # ``` - # file.size - # # => 102400 - # ``` - # - def size : Int64? - case value = metadata["size"]? - when Int32 then value.to_i64 - when Int64 then value - when String then value.to_i64? - else nil - end - end - - # Returns the MIME type from metadata. - # - # ``` - # file.mime_type - # # => "image/jpeg" - # ``` - # - def mime_type : String? - metadata["mime_type"]?.try(&.as(String)) - end - - # Access arbitrary metadata values. - # - # ``` - # file["width"] - # # => 800 - # file["custom"] - # # => "value" - # ``` - # - def [](key : String) : MetadataValue - metadata[key]? - end - # Returns the storage instance this file is stored in. def storage : Storage ::Lucky::Attachment.find_storage(storage_key) From 5c6caaf41c2b8165c9b8b2b118254606e3b6c00d Mon Sep 17 00:00:00 2001 From: Wout Date: Fri, 13 Mar 2026 20:34:37 +0100 Subject: [PATCH 42/52] Create aliases for built-in extractors --- src/lucky/attachment/uploader.cr | 75 +++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 15 deletions(-) diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index dc67da013..c3149ad2d 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -17,12 +17,18 @@ require "uuid" # abstract struct Lucky::Attachment::Uploader alias MetadataHash = ::Lucky::Attachment::MetadataHash - - EXTRACTORS = { - "filename" => Extractor::FilenameFromIO.new, - "mime_type" => Extractor::MimeFromIO.new, - "size" => Extractor::SizeFromIO.new, - } of String => Extractor + {% for extractor in %w[ + DimensionsFromMagick + FilenameFromIO + MimeFromExtension + MimeFromFile + MimeFromIO + SizeFromIO + ] %} + alias {{ extractor.id }}Extractor = Lucky::Attachment::Extractor::{{ extractor.id }} + {% end %} + + EXTRACTORS = {} of String => Extractor macro inherited {% stored_file = "#{@type}::StoredFile".id %} @@ -30,6 +36,11 @@ abstract struct Lucky::Attachment::Uploader class {{ stored_file }} < Lucky::Attachment::StoredFile end + # Register default extractors + extract filename : String, using: Lucky::Attachment::Extractor::FilenameFromIO + extract mime_type : String, using: Lucky::Attachment::Extractor::MimeFromIO + extract size : Int64, using: Lucky::Attachment::Extractor::SizeFromIO + # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method # accepts additional metadata and arbitrary arguments for overrides. # @@ -101,21 +112,48 @@ abstract struct Lucky::Attachment::Uploader # ``` # struct PdfUploader < Lucky::Attachment::Uploader # # Use a different MIME type extractor than the default one - # extract mime_type, using: Lucky::Attachment::Extractor::MimeFromExtension + # extract mime_type : String, using: Lucky::Attachment::Extractor::MimeFromExtension # # # Or use your own custom extractor to add arbitrary data - # extract pages, using: MyNumberOfPagesExtractor + # extract pages : Int32, using: MyNumberOfPagesExtractor # end # ``` # # The result will then be added to the attachment's metadata after uploading: # ``` - # invoice.pdf.metadata["pages"] + # invoice.pdf.pages # # => 24 # ``` # - macro extract(name, using) - EXTRACTORS["{{name.id}}"] = {{using}}.new + macro extract(type_declaration, using) + {% + name = type_declaration.var + type = type_declaration.type.types.first + %} + + {% + if type_declaration.type.is_a?(Union) + raise <<-ERROR + Union types can't be used for extractors. + + Try this... + + ▸ extract #{name} : #{type}, using: #{using} + ERROR + end + %} + + class {{ @type }}::StoredFile < Lucky::Attachment::StoredFile + def {{ name }}? : {{ type }}? + metadata["{{ name }}"]?.try(&.as?({{ type }})) + end + + def {{ name }} : {{ type }} + metadata["{{ name }}"].as({{ type }}) + end + end + + EXTRACTORS["{{ name }}"] = {{ using }}.new end getter storage_key : String @@ -161,8 +199,8 @@ abstract struct Lucky::Attachment::Uploader UUID.random.to_s end - # Extracts metadata from the IO. Override in subclasses to add custom - # metadata extraction. + # Extracts metadata from the IO. Override in subclasses to add completely + # custom metadata extraction outside of the `extract` DSL. # # ``` # class ImageUploader < Lucky::Attachment::Uploader @@ -172,6 +210,13 @@ abstract struct Lucky::Attachment::Uploader # data["custom"] = "value" # data # end + # + # # Reopen the `StoredFile` class to add a method for the custom value. + # class StoredFile + # def custom : String + # metadata["custom"].as(String) + # end + # end # end # ``` # @@ -181,9 +226,9 @@ abstract struct Lucky::Attachment::Uploader **options, ) : MetadataHash (metadata.try(&.dup) || MetadataHash.new).tap do |data| - EXTRACTORS.each do |key, extractor| + EXTRACTORS.each do |name, extractor| if value = extractor.extract(io, data, **options) - data[key] = value + data[name] = value end end end From 58fbb96c25872d84635e340b57d1222ff5bbcd6e Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 11:59:14 +0100 Subject: [PATCH 43/52] Allow extractors to declare addtional methods for metadata properties --- src/lucky/attachment.cr | 5 ++- .../extractor/dimensions_from_magick.cr | 1 + src/lucky/attachment/uploader.cr | 39 ++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/lucky/attachment.cr b/src/lucky/attachment.cr index d7609d2a1..d0a93d509 100644 --- a/src/lucky/attachment.cr +++ b/src/lucky/attachment.cr @@ -1,9 +1,12 @@ require "./attachment/storage" module Lucky::Attachment - alias MetadataValue = String | Int32 | Int64 | UInt32 | UInt64 | Float64 | Bool | Nil + alias MetadataValue = String | Int64 | Int32 | Float64 | Bool | Nil alias MetadataHash = Hash(String, MetadataValue) + annotation MetadataMethods + end + class Error < Exception; end class FileNotFound < Error; end diff --git a/src/lucky/attachment/extractor/dimensions_from_magick.cr b/src/lucky/attachment/extractor/dimensions_from_magick.cr index 774f81b4b..3570392d1 100644 --- a/src/lucky/attachment/extractor/dimensions_from_magick.cr +++ b/src/lucky/attachment/extractor/dimensions_from_magick.cr @@ -1,5 +1,6 @@ require "./run_command" +@[Lucky::Attachment::MetadataMethods(width : Int32, height : Int32)] struct Lucky::Attachment::Extractor::DimensionsFromMagick include Lucky::Attachment::Extractor include Lucky::Attachment::Extractor::RunCommand diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index c3149ad2d..1366beaaf 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -17,6 +17,9 @@ require "uuid" # abstract struct Lucky::Attachment::Uploader alias MetadataHash = ::Lucky::Attachment::MetadataHash + + # Adds shorter local aliases for built-in extractors. + # e.g. `Lucky::Attachment::Extractor::SizeFromIO` -> `SizeFromIOExtractor` {% for extractor in %w[ DimensionsFromMagick FilenameFromIO @@ -145,12 +148,44 @@ abstract struct Lucky::Attachment::Uploader class {{ @type }}::StoredFile < Lucky::Attachment::StoredFile def {{ name }}? : {{ type }}? - metadata["{{ name }}"]?.try(&.as?({{ type }})) + {% if {Int32, Int64}.includes? type.resolve %} + if value = metadata["{{ name }}"]? + {{ type }}.new(value.as(Int32 | Int64)) + end + {% else %} + metadata["{{ name }}"]?.try(&.as?({{ type }})) + {% end %} end def {{ name }} : {{ type }} - metadata["{{ name }}"].as({{ type }}) + {% if {Int32, Int64}.includes? type.resolve %} + {{ type }}.new(metadata["{{ name }}"].as(Int32 | Int64)) + {% else %} + metadata["{{ name }}"].as({{ type }}) + {% end %} end + + {% if methods = using.resolve.annotation(Lucky::Attachment::MetadataMethods) %} + {% for td in methods.args %} + def {{ td.var }}? : {{ td.type }}? + {% if {Int32, Int64}.includes? td.type.resolve %} + if value = metadata["{{ td.var }}"]? + {{ td.type }}.new(value.as(Int32 | Int64)) + end + {% else %} + metadata["{{ td.var }}"]?.try(&.as?({{ td.type }})) + {% end %} + end + + def {{ td.var }} : {{ td.type }} + {% if {Int32, Int64}.includes? td.type.resolve %} + {{ td.type }}.new(metadata["{{ td.var }}"].as(Int32 | Int64)) + {% else %} + metadata["{{ td.var }}"].as({{ td.type }}) + {% end %} + end + {% end %} + {% end %} end EXTRACTORS["{{ name }}"] = {{ using }}.new From 69bca0922528d82daeab81ec205e7e6d528b8a58 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 13:51:49 +0100 Subject: [PATCH 44/52] Rework dimensions extractor to not return anything --- ...ns_from_identify_spec.cr => dimensions_from_magick_spec.cr} | 3 ++- src/lucky/attachment/extractor/dimensions_from_magick.cr | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) rename spec/lucky/attachment/extractor/{dimensions_from_identify_spec.cr => dimensions_from_magick_spec.cr} (94%) diff --git a/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr b/spec/lucky/attachment/extractor/dimensions_from_magick_spec.cr similarity index 94% rename from spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr rename to spec/lucky/attachment/extractor/dimensions_from_magick_spec.cr index 600408f92..84cecc065 100644 --- a/spec/lucky/attachment/extractor/dimensions_from_identify_spec.cr +++ b/spec/lucky/attachment/extractor/dimensions_from_magick_spec.cr @@ -28,8 +28,9 @@ describe Lucky::Attachment::Extractor::DimensionsFromMagick do it "extracts width and height from a PNG file" do file = File.open(png_path) metadata = Lucky::Attachment::MetadataHash.new - subject.extract(file, metadata: metadata) + result = subject.extract(file, metadata: metadata) + result.should be_nil metadata["width"].should eq(69) metadata["height"].should eq(16) end diff --git a/src/lucky/attachment/extractor/dimensions_from_magick.cr b/src/lucky/attachment/extractor/dimensions_from_magick.cr index 3570392d1..2af23298b 100644 --- a/src/lucky/attachment/extractor/dimensions_from_magick.cr +++ b/src/lucky/attachment/extractor/dimensions_from_magick.cr @@ -6,13 +6,12 @@ struct Lucky::Attachment::Extractor::DimensionsFromMagick include Lucky::Attachment::Extractor::RunCommand # Extracts the dimensions of a file using ImageMagick's `magick` command. - def extract(io, metadata, **options) : String? + def extract(io, metadata, **options) : Nil if result = run_magick_command(io, ["identify", "-format", "%[fx:w] %[fx:h]"]) dimensions = result.split.map(&.to_i) metadata["width"] = dimensions[0] metadata["height"] = dimensions[1] - "#{dimensions[0]}x#{dimensions[0]}" end end From b07307d5b663246bc404a5e0cbea7a76356f62e8 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 13:52:54 +0100 Subject: [PATCH 45/52] Add `[]?` and `extension?` methods to stored file class --- spec/lucky/attachment/stored_file_spec.cr | 54 ++++++++++++----------- spec/lucky/attachment/uploader_spec.cr | 16 +++---- src/lucky/attachment/stored_file.cr | 26 ++++++++--- 3 files changed, 57 insertions(+), 39 deletions(-) diff --git a/spec/lucky/attachment/stored_file_spec.cr b/spec/lucky/attachment/stored_file_spec.cr index 79b786982..433c3ab6c 100644 --- a/spec/lucky/attachment/stored_file_spec.cr +++ b/spec/lucky/attachment/stored_file_spec.cr @@ -13,7 +13,7 @@ describe Lucky::Attachment::StoredFile do describe ".from_json" do it "deserializes from JSON" do - file = Lucky::Attachment::StoredFile.from_json( + file = TestUploader::StoredFile.from_json( { id: "test.jpg", storage: "store", @@ -27,7 +27,7 @@ describe Lucky::Attachment::StoredFile do file.id.should eq("test.jpg") file.storage_key.should eq("store") - file.original_filename.should eq("original.jpg") + file.filename.should eq("original.jpg") file.size.should eq(1024) file.mime_type.should eq("image/jpeg") end @@ -35,7 +35,7 @@ describe Lucky::Attachment::StoredFile do describe "#to_json" do it "serializes to JSON" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "test.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{ @@ -56,7 +56,7 @@ describe Lucky::Attachment::StoredFile do describe "#extension" do it "extracts from id" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "path/to/file.jpg", storage_key: "store" ) @@ -65,7 +65,7 @@ describe Lucky::Attachment::StoredFile do end it "falls back to filename metadata" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "abc123", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{"filename" => "photo.png"} @@ -75,18 +75,18 @@ describe Lucky::Attachment::StoredFile do end it "returns nil when no extension can be determined" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "abc123", storage_key: "store" ) - file.extension.should be_nil + file.extension?.should be_nil end end describe "#size" do it "returns Int64 from integer metadata" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "file.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{"size" => 1024_i64} @@ -97,7 +97,7 @@ describe Lucky::Attachment::StoredFile do end it "coerces Int32 to Int64" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "file.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash{"size" => 512_i32} @@ -108,18 +108,18 @@ describe Lucky::Attachment::StoredFile do end it "returns nil when size is absent" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "file.jpg", storage_key: "store" ) - file.size.should be_nil + file.size?.should be_nil end end describe "#url" do it "delegates to storage" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "uploads/photo.jpg", storage_key: "store" ) @@ -131,7 +131,7 @@ describe Lucky::Attachment::StoredFile do describe "#exists?" do it "returns true when file is in storage" do memory_store.upload(IO::Memory.new("data"), "photo.jpg") - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "photo.jpg", storage_key: "store" ) @@ -140,7 +140,7 @@ describe Lucky::Attachment::StoredFile do end it "returns false when file is not in storage" do - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "missing.jpg", storage_key: "store" ) @@ -151,7 +151,7 @@ describe Lucky::Attachment::StoredFile do describe "#open" do it "yields the file IO" do memory_store.upload(IO::Memory.new("file content"), "test.txt") - file = Lucky::Attachment::StoredFile.new( + file = TestUploader::StoredFile.new( id: "test.txt", storage_key: "store" ) @@ -161,7 +161,7 @@ describe Lucky::Attachment::StoredFile do it "closes the IO after the block" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") + file = TestUploader::StoredFile.new(id: "test.txt", storage_key: "store") captured_io = nil file.open { |io| captured_io = io } @@ -170,7 +170,7 @@ describe Lucky::Attachment::StoredFile do it "closes the IO even if the block raises" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") + file = TestUploader::StoredFile.new(id: "test.txt", storage_key: "store") captured_io = nil expect_raises(Exception) do @@ -185,7 +185,7 @@ describe Lucky::Attachment::StoredFile do describe "#download" do it "returns a tempfile with file content" do memory_store.upload(IO::Memory.new("downloaded content"), "test.txt") - file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") + file = TestUploader::StoredFile.new(id: "test.txt", storage_key: "store") tempfile = file.download tempfile.gets_to_end.should eq("downloaded content") @@ -195,7 +195,7 @@ describe Lucky::Attachment::StoredFile do it "cleans up the tempfile after the block" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") + file = TestUploader::StoredFile.new(id: "test.txt", storage_key: "store") tempfile_path = "" file.download { |tempfile| tempfile_path = tempfile.path } @@ -207,7 +207,7 @@ describe Lucky::Attachment::StoredFile do describe "#delete" do it "removes the file from storage" do memory_store.upload(IO::Memory.new("data"), "test.txt") - file = Lucky::Attachment::StoredFile.new(id: "test.txt", storage_key: "store") + file = TestUploader::StoredFile.new(id: "test.txt", storage_key: "store") file.delete memory_store.exists?("test.txt").should be_false @@ -216,24 +216,26 @@ describe Lucky::Attachment::StoredFile do describe "#==" do it "is equal when id and storage_key match" do - a = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "store") - b = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "store") + a = TestUploader::StoredFile.new(id: "file.jpg", storage_key: "store") + b = TestUploader::StoredFile.new(id: "file.jpg", storage_key: "store") (a == b).should be_true end it "is not equal when id differs" do - a = Lucky::Attachment::StoredFile.new(id: "a.jpg", storage_key: "store") - b = Lucky::Attachment::StoredFile.new(id: "b.jpg", storage_key: "store") + a = TestUploader::StoredFile.new(id: "a.jpg", storage_key: "store") + b = TestUploader::StoredFile.new(id: "b.jpg", storage_key: "store") (a == b).should be_false end it "is not equal when storage_key differs" do - a = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "cache") - b = Lucky::Attachment::StoredFile.new(id: "file.jpg", storage_key: "store") + a = TestUploader::StoredFile.new(id: "file.jpg", storage_key: "cache") + b = TestUploader::StoredFile.new(id: "file.jpg", storage_key: "store") (a == b).should be_false end end end + +private struct TestUploader < Lucky::Attachment::Uploader; end diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index 84cf47834..7d10f0d93 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -20,7 +20,7 @@ describe Lucky::Attachment::Uploader do io = IO::Memory.new("hello") file = TestUploader.new("store").upload(io) - file.should be_a(Lucky::Attachment::StoredFile) + file.should be_a(TestUploader::StoredFile) file.storage_key.should eq("store") file.exists?.should be_true end @@ -66,8 +66,8 @@ describe Lucky::Attachment::Uploader do } ) - file.original_filename.should eq("override.png") - file["custom"].should eq("value") + file.filename.should eq("override.png") + file["custom"]?.should eq("value") end end @@ -76,7 +76,7 @@ describe Lucky::Attachment::Uploader do file = File.tempfile("myfile", ".txt", &.print("content")) uploaded = TestUploader.new("store").upload(File.open(file.path)) - uploaded.original_filename.should eq(File.basename(file.path)) + uploaded.filename.should eq(File.basename(file.path)) ensure file.try(&.delete) end @@ -126,7 +126,7 @@ describe Lucky::Attachment::Uploader do it "uses overridden extract_metadata" do file = CustomMetadataUploader.new("store").upload(IO::Memory.new("data")) - file["custom_key"].should eq("custom_value") + file["custom_key"]?.should eq("custom_value") end end @@ -187,7 +187,7 @@ describe Lucky::Attachment::Uploader do ) stored = TestUploader.promote(cached) - stored.original_filename.should eq("test.jpg") + stored.filename.should eq("test.jpg") end it "can promote to a custom storage key" do @@ -219,7 +219,7 @@ describe "Lucky::UploadedFile integration" do lucky_file = Lucky::UploadedFile.new(part) uploaded = AvatarUploader.new("store").upload(lucky_file.tempfile) - uploaded.original_filename.should eq(File.basename(lucky_file.tempfile.path)) + uploaded.filename.should eq(File.basename(lucky_file.tempfile.path)) end it "extracts content_type when Lucky::UploadedFile exposes it" do @@ -227,7 +227,7 @@ describe "Lucky::UploadedFile integration" do lucky_file = Lucky::UploadedFile.new(part) uploaded = AvatarUploader.new("store").upload(lucky_file.tempfile) - uploaded.mime_type.should be_nil + uploaded.mime_type?.should be_nil end it "handles blank files gracefully" do diff --git a/src/lucky/attachment/stored_file.cr b/src/lucky/attachment/stored_file.cr index f1d61cef4..a8239b186 100644 --- a/src/lucky/attachment/stored_file.cr +++ b/src/lucky/attachment/stored_file.cr @@ -46,18 +46,34 @@ class Lucky::Attachment::StoredFile # Returns the file extension based on the id or original filename. # # ``` - # file.extension + # file.extension? # # => "jpg" # ``` # - def extension : String? + def extension? : String? ext = File.extname(id).lchop('.') - if ext.empty? && original_filename - ext = File.extname(original_filename.to_s).lchop('.') - end + ext = File.extname(filename).lchop('.') if ext.empty? && filename? ext.presence.try(&.downcase) end + # The non-nilable variant of the `extension?` method. + def extension : String + extension?.as(String) + end + + # Aliases the `[]?` method on the metadata property. + # + # ``` + # file["width"]? + # # => 800 + # file["custom"]? + # # => "value" + # ``` + # + def []?(key : String) : MetadataValue + metadata[key]? + end + # Returns the storage instance this file is stored in. def storage : Storage ::Lucky::Attachment.find_storage(storage_key) From 63d9ff855dadb7c0dbdf33cd88e39ca6cbd0ae6e Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 14:20:26 +0100 Subject: [PATCH 46/52] Remove type declaration on extract macro signature Rather than having the user declare a return type, derive the return type from the `extract` method in the extractor class. --- src/lucky/attachment/uploader.cr | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 1366beaaf..570acd08a 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -40,9 +40,9 @@ abstract struct Lucky::Attachment::Uploader end # Register default extractors - extract filename : String, using: Lucky::Attachment::Extractor::FilenameFromIO - extract mime_type : String, using: Lucky::Attachment::Extractor::MimeFromIO - extract size : Int64, using: Lucky::Attachment::Extractor::SizeFromIO + extract filename, using: Lucky::Attachment::Extractor::FilenameFromIO + extract mime_type, using: Lucky::Attachment::Extractor::MimeFromIO + extract size, using: Lucky::Attachment::Extractor::SizeFromIO # Uploads a file and returns a `Lucky::Attachment::StoredFile`. This method # accepts additional metadata and arbitrary arguments for overrides. @@ -115,10 +115,10 @@ abstract struct Lucky::Attachment::Uploader # ``` # struct PdfUploader < Lucky::Attachment::Uploader # # Use a different MIME type extractor than the default one - # extract mime_type : String, using: Lucky::Attachment::Extractor::MimeFromExtension + # extract mime_type, using: Lucky::Attachment::Extractor::MimeFromExtension # # # Or use your own custom extractor to add arbitrary data - # extract pages : Int32, using: MyNumberOfPagesExtractor + # extract pages, using: MyNumberOfPagesExtractor # end # ``` # @@ -128,22 +128,11 @@ abstract struct Lucky::Attachment::Uploader # # => 24 # ``` # - macro extract(type_declaration, using) + macro extract(name, using) {% - name = type_declaration.var - type = type_declaration.type.types.first - %} - - {% - if type_declaration.type.is_a?(Union) - raise <<-ERROR - Union types can't be used for extractors. - - Try this... - - ▸ extract #{name} : #{type}, using: #{using} - ERROR - end + type = using.resolve.methods + .find { |method| method.name == :extract.id } + .return_type.types.first %} class {{ @type }}::StoredFile < Lucky::Attachment::StoredFile From 01b7492e8fbb865b49ee7b502f03c76e874ae682 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 15:38:41 +0100 Subject: [PATCH 47/52] Declare path prefix as a class method on uploaders --- src/lucky/attachment/avram.cr | 13 +++---------- src/lucky/attachment/uploader.cr | 6 ++++++ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/lucky/attachment/avram.cr b/src/lucky/attachment/avram.cr index d6b109857..3cd299b56 100644 --- a/src/lucky/attachment/avram.cr +++ b/src/lucky/attachment/avram.cr @@ -42,14 +42,7 @@ module Avram::Attachment::Model # user.avatar.url(expires_in: 1.hour) # ``` # - # The path prefix of an attachment can be customised globally in the - # settings, but also on attachment level: - # - # ``` - # attach avatar : ImageUploader::StoredFile?, path_prefix: ":model/images/:id" - # ``` - # - macro attach(type_declaration, path_prefix = nil) + macro attach(type_declaration) {% name = type_declaration.var %} {% if type_declaration.type.is_a?(Union) %} {% stored_file = type_declaration.type.types.first %} @@ -64,8 +57,8 @@ module Avram::Attachment::Model {% if !@type.constant(:ATTACHMENT_PREFIXES) %} ATTACHMENT_PREFIXES = {} of Symbol => String {% end %} - path_prefix = {{ path_prefix }} || ::Lucky::Attachment.settings.path_prefix - ATTACHMENT_PREFIXES[:{{ name }}] = path_prefix + + ATTACHMENT_PREFIXES[:{{ name }}] = {{ uploader }}.path_prefix .gsub(/:model/, {{ @type.stringify.gsub(/::/, "_").underscore }}) .gsub(/:attachment/, {{ name.stringify }}) diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index 570acd08a..f58c61791 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -18,6 +18,12 @@ require "uuid" abstract struct Lucky::Attachment::Uploader alias MetadataHash = ::Lucky::Attachment::MetadataHash + # Defines the path prefix for uploads in the storage. Overwrite this method + # in uploader subclasses to use custom path prefixes per uploader. + def self.path_prefix : String + Lucky::Attachment.settings.path_prefix + end + # Adds shorter local aliases for built-in extractors. # e.g. `Lucky::Attachment::Extractor::SizeFromIO` -> `SizeFromIOExtractor` {% for extractor in %w[ From 145f4dabc3d924b4ddef8731e5e83124f82c21d8 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 15:44:02 +0100 Subject: [PATCH 48/52] Move avram module to the Avram repo --- src/lucky/attachment/avram.cr | 151 ---------------------------------- 1 file changed, 151 deletions(-) delete mode 100644 src/lucky/attachment/avram.cr diff --git a/src/lucky/attachment/avram.cr b/src/lucky/attachment/avram.cr deleted file mode 100644 index 3cd299b56..000000000 --- a/src/lucky/attachment/avram.cr +++ /dev/null @@ -1,151 +0,0 @@ -module Avram::Attachment::Model - macro included - class ::{{ @type }}::SaveOperation < Avram::SaveOperation({{ @type }}) - include Avram::Attachment::SaveOperation - end - - macro finished - class ::{{ @type }}::DeleteOperation < Avram::DeleteOperation({{ @type }}) - include Avram::Attachment::DeleteOperation - end - end - end - - # Registers a serializable column for an attachment and takes and uploader - # class as the type. - # - # ``` - # attach avatar : ImageUploader::StoredFile - # # or - # attach avatar : ImageUploader::StoredFile? - # ``` - # - # It is assumed that a `jsonb` column exists with the same name. So in your - # migration, you'll need to add the column as follows: - # - # ``` - # add avatar : JSON::Any - # # or - # add avatar : JSON::Any? - # ``` - # - # The data of a stored file can then be accessed through the `avatar` method: - # - # ``` - # user.avatar.class - # # => ImageUploader::StoredFile - # - # user.avatar.url - # # => "https://bucket.s3.amazonaws.com/user/1/avatar/abc123.jpg" - # - # # for presigned URLs - # user.avatar.url(expires_in: 1.hour) - # ``` - # - macro attach(type_declaration) - {% name = type_declaration.var %} - {% if type_declaration.type.is_a?(Union) %} - {% stored_file = type_declaration.type.types.first %} - {% nilable = true %} - {% else %} - {% stored_file = type_declaration.type %} - {% nilable = false %} - {% end %} - {% uploader = stored_file.stringify.split("::")[0..-2].join("::").id %} - - # Registers a path prefix for the attachment. - {% if !@type.constant(:ATTACHMENT_PREFIXES) %} - ATTACHMENT_PREFIXES = {} of Symbol => String - {% end %} - - ATTACHMENT_PREFIXES[:{{ name }}] = {{ uploader }}.path_prefix - .gsub(/:model/, {{ @type.stringify.gsub(/::/, "_").underscore }}) - .gsub(/:attachment/, {{ name.stringify }}) - - # Registers the configured uploader class for the attachment. - {% if !@type.constant(:ATTACHMENT_UPLOADERS) %} - ATTACHMENT_UPLOADERS = {} of Symbol => ::Lucky::Attachment::Uploader.class - {% end %} - ATTACHMENT_UPLOADERS[:{{ name }}] = {{ uploader }} - - column {{ name }} : ::{{ stored_file }}{% if nilable %}?{% end %}, serialize: true - end -end - -module Avram::Attachment::SaveOperation - # Registers a file attribute for an existing attachment on the model. - # - # ``` - # # The field name in the form will be "avatar_file" - # attach avatar - # - # # With a custom field name - # attach avatar, field_name: "avatar_upload" - # ``` - # - # The attachment will then be uploaded to the cache store, and after - # committing to the database the attachment will be moved to the permanent - # storage. - # - macro attach(name, field_name = nil) - {% - field_name = "#{name}_file".id if field_name.nil? - - unless column = T.constant(:COLUMNS).find { |col| col[:name].stringify == name.stringify } - raise %(The `#{T.name}` model does not have a column named `#{name}`) - end - %} - - file_attribute :{{ field_name }} - - {% if nilable = column[:nilable] %} - attribute delete_{{ name }} : Bool = false - {% end %} - - before_save __cache_{{ field_name }} - after_commit __process_{{ field_name }} - - # Moves uploaded file to the cache storage. - private def __cache_{{ field_name }} : Nil - {% if nilable %} - {{ name }}.value = nil if delete_{{ name }}.value - {% end %} - - return unless upload = {{ field_name }}.value - - record_id = {{ T.constant(:PRIMARY_KEY_NAME).id }}.value - {{ name }}.value = T::ATTACHMENT_UPLOADERS[:{{ name }}].cache( - upload.tempfile, - path_prefix: T::ATTACHMENT_PREFIXES[:{{ name }}].gsub(/:id/, record_id), - filename: upload.filename.presence - ) - end - - # Deletes or promotes the attachment and updates the record. - private def __process_{{ field_name }}(record) : Nil - {% if nilable %} - if delete_{{ name }}.value && (file = {{ name }}.original_value) - file.delete - end - {% end %} - - return unless {{ field_name }}.value && (cached = {{ name }}.value) - - stored = T::ATTACHMENT_UPLOADERS[:{{ name }}].promote(cached) - T::SaveOperation.update!(record, {{ name }}: stored) - end - end -end - -module Avram::Attachment::DeleteOperation - # Cleans up the files of any attachments this records still has. - macro included - after_delete do |_| - {% for name in T.constant(:ATTACHMENT_UPLOADERS) %} - if attachment = {{ name }}.value - attachment.delete - end - {% end %} - end - end -end From 5764d6a9a32f56b0e6ee82434b8f535507ef79c6 Mon Sep 17 00:00:00 2001 From: Wout Date: Sat, 14 Mar 2026 15:59:03 +0100 Subject: [PATCH 49/52] Make `StoredFile` and abstract class --- spec/lucky/attachment/storage/s3_spec.cr | 8 +++++--- src/lucky/attachment/stored_file.cr | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/lucky/attachment/storage/s3_spec.cr b/spec/lucky/attachment/storage/s3_spec.cr index fffc20217..e302d9005 100644 --- a/spec/lucky/attachment/storage/s3_spec.cr +++ b/spec/lucky/attachment/storage/s3_spec.cr @@ -173,7 +173,7 @@ describe Lucky::Attachment::Storage::S3 do .to_return(status: 204) Lucky::Attachment.settings.storages["cache"] = storage - source = Lucky::Attachment::StoredFile.new( + source = TestUploader::StoredFile.new( id: "cache/photo.jpg", storage_key: "cache", metadata: Lucky::Attachment::MetadataHash.new @@ -190,7 +190,7 @@ describe Lucky::Attachment::Storage::S3 do .to_return(status: 200, body_io: test_file_io("data"), headers: test_headers) WebMock.stub(:put, "#{base_url}/photo.jpg") .to_return(status: 200, headers: test_headers) - source = Lucky::Attachment::StoredFile.new( + source = TestUploader::StoredFile.new( id: "photo.jpg", storage_key: "other", metadata: Lucky::Attachment::MetadataHash.new @@ -207,7 +207,7 @@ describe Lucky::Attachment::Storage::S3 do WebMock.stub(:delete, "#{base_url}/store/photo.jpg?") .to_return(status: 204) - source = Lucky::Attachment::StoredFile.new( + source = TestUploader::StoredFile.new( id: "photo.jpg", storage_key: "store", metadata: Lucky::Attachment::MetadataHash.new @@ -350,6 +350,8 @@ private class TestAwss3Client < Awscr::S3::Client end end +private struct TestUploader < Lucky::Attachment::Uploader; end + private def bucket "lucky-bucket" end diff --git a/src/lucky/attachment/stored_file.cr b/src/lucky/attachment/stored_file.cr index a8239b186..6986f0bed 100644 --- a/src/lucky/attachment/stored_file.cr +++ b/src/lucky/attachment/stored_file.cr @@ -20,7 +20,7 @@ require "uuid" # } # ``` # -class Lucky::Attachment::StoredFile +abstract class Lucky::Attachment::StoredFile include JSON::Serializable # NOTE: This mimics the behavior of Avram's `JSON::Serializable` extension. From a7d462f0e7618e43c131bf7fd2c2fdac9e356d76 Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 15 Mar 2026 10:30:23 +0100 Subject: [PATCH 50/52] Allow passing a custom location to the promote method --- spec/lucky/attachment/uploader_spec.cr | 15 +++++++++++++++ src/lucky/attachment/uploader.cr | 5 +++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/spec/lucky/attachment/uploader_spec.cr b/spec/lucky/attachment/uploader_spec.cr index 7d10f0d93..65bc9843f 100644 --- a/spec/lucky/attachment/uploader_spec.cr +++ b/spec/lucky/attachment/uploader_spec.cr @@ -201,6 +201,21 @@ describe Lucky::Attachment::Uploader do offsite.storage_key.should eq("offsite") end + + it "stores the file at the provided location" do + cached = TestUploader.cache(IO::Memory.new("data")) + stored = TestUploader.promote(cached, location: "custom/path/file.jpg") + + stored.id.should eq("custom/path/file.jpg") + memory_store.exists?("custom/path/file.jpg").should be_true + end + + it "uses the cached file id as location when none is provided" do + cached = TestUploader.cache(IO::Memory.new("data")) + stored = TestUploader.promote(cached) + + stored.id.should eq(cached.id) + end end end diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index f58c61791..f9865ddb2 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -102,11 +102,12 @@ abstract struct Lucky::Attachment::Uploader **options, ) : {{ stored_file }} file.open do |io| + store_location = options[:location]? || file.id ::Lucky::Attachment .find_storage(storage) - .upload(io, file.id, metadata: file.metadata) + .upload(io, store_location, **options, metadata: file.metadata) promoted = {{ stored_file }}.new( - id: file.id, + id: store_location, storage_key: storage, metadata: file.metadata ) From d8f96fb82e800975251130f94fdeddccd0968c96 Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 15 Mar 2026 12:49:36 +0100 Subject: [PATCH 51/52] Add move overload accepting stored files on the base storage class --- src/lucky/attachment/storage.cr | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lucky/attachment/storage.cr b/src/lucky/attachment/storage.cr index 8126e20db..27b2090c4 100644 --- a/src/lucky/attachment/storage.cr +++ b/src/lucky/attachment/storage.cr @@ -54,8 +54,13 @@ abstract class Lucky::Attachment::Storage # abstract def delete(id : String) : Nil - # Moves a file from another location. + # Moves an IO from another location. def move(io : IO, id : String, **options) : Nil upload(io, id, **options) end + + # Moves a file from another location. + def move(file : Lucky::Attachment::StoredFile, id : String, **options) : Nil + upload(file.io, id, **options) + end end From 84687a4f6ec68f9a75ddccdffa94f07a95abb1a5 Mon Sep 17 00:00:00 2001 From: Wout Date: Sun, 15 Mar 2026 12:51:46 +0100 Subject: [PATCH 52/52] Use the move method on the storage in uploaders to promote files more efficiently --- spec/lucky/attachment/storage/s3_spec.cr | 20 ++++++++++++++++++++ src/lucky/attachment/storage/s3.cr | 22 ++++++++-------------- src/lucky/attachment/uploader.cr | 24 +++++++++++------------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/spec/lucky/attachment/storage/s3_spec.cr b/spec/lucky/attachment/storage/s3_spec.cr index e302d9005..d2958fe80 100644 --- a/spec/lucky/attachment/storage/s3_spec.cr +++ b/spec/lucky/attachment/storage/s3_spec.cr @@ -215,6 +215,26 @@ describe Lucky::Attachment::Storage::S3 do storage.move(source, "photo.jpg") end + + it "uses the source storage prefix for the copy source key" do + cache_storage = build_storage(prefix: "cache") + store_storage = build_storage + + Lucky::Attachment.settings.storages["cache"] = cache_storage + Lucky::Attachment.settings.storages["store"] = store_storage + + WebMock.stub(:put, "#{base_url}/photo.jpg") + .with(headers: {"x-amz-copy-source" => "/lucky-bucket/cache/photo.jpg"}) + .to_return(status: 200, body: copy_object_xml) + + source = TestUploader::StoredFile.new( + id: "photo.jpg", + storage_key: "cache", + metadata: Lucky::Attachment::MetadataHash.new + ) + + store_storage.move(source, "photo.jpg") + end end describe "presigned URL (with expires_in)" do diff --git a/src/lucky/attachment/storage/s3.cr b/src/lucky/attachment/storage/s3.cr index 81e9ec66f..9e88aa1df 100644 --- a/src/lucky/attachment/storage/s3.cr +++ b/src/lucky/attachment/storage/s3.cr @@ -191,21 +191,15 @@ class Lucky::Attachment::Storage::S3 < Lucky::Attachment::Storage # a `StoredFile` in the same bucket, avoiding the download/re-upload. Falls # back to a regular upload for plain `IO` sources. # - def move(io : IO, id : String, **options) : Nil - upload(io, id, **options) - end - def move(file : Lucky::Attachment::StoredFile, id : String, **options) : Nil - if same_bucket?(file) - copy_object( - **options, - source_key: object_key(file.id), - dest_key: object_key(id) - ) - file.delete - else - move(file.io, id, **options) - end + return move(file.io, id, **options) unless same_bucket?(file) + + source_storage = file.storage.as(S3) + copy_object( + **options, + source_key: source_storage.object_key(file.id), + dest_key: object_key(id) + ) end # Returns the full object key including any configured prefix. diff --git a/src/lucky/attachment/uploader.cr b/src/lucky/attachment/uploader.cr index f9865ddb2..21294e660 100644 --- a/src/lucky/attachment/uploader.cr +++ b/src/lucky/attachment/uploader.cr @@ -101,19 +101,17 @@ abstract struct Lucky::Attachment::Uploader delete_source : Bool = true, **options, ) : {{ stored_file }} - file.open do |io| - store_location = options[:location]? || file.id - ::Lucky::Attachment - .find_storage(storage) - .upload(io, store_location, **options, metadata: file.metadata) - promoted = {{ stored_file }}.new( - id: store_location, - storage_key: storage, - metadata: file.metadata - ) - file.delete if delete_source - promoted - end + store_location = options[:location]? || file.id + store_storage = ::Lucky::Attachment.find_storage(storage) + store_storage.move(file, store_location, **options, metadata: file.metadata) + + promoted = {{ stored_file }}.new( + id: store_location, + storage_key: storage, + metadata: file.metadata + ) + file.delete if delete_source + promoted end end