diff --git a/app/lib/active_storage/service/jekyll_service.rb b/app/lib/active_storage/service/jekyll_service.rb index 92b26e0e..88ffa83c 100644 --- a/app/lib/active_storage/service/jekyll_service.rb +++ b/app/lib/active_storage/service/jekyll_service.rb @@ -20,6 +20,18 @@ module ActiveStorage end end + # Solo copiamos el archivo si no existe + # + # @param :key [String] + # @param :io [IO] + # @param :checksum [String] + def upload(key, io, checksum: nil, **) + instrument :upload, key: key, checksum: checksum do + IO.copy_stream(io, make_path_for(key)) unless exist?(key) + ensure_integrity_of(key, checksum) if checksum + end + end + # Lo mismo que en DiskService agregando el nombre de archivo en la # firma. Esto permite que luego podamos guardar el archivo donde # corresponde. @@ -67,7 +79,9 @@ module ActiveStorage # @param :key [String] # @return [String] def filename_for(key) - ActiveStorage::Blob.where(key: key).limit(1).pluck(:filename).first + ActiveStorage::Blob.where(key: key).limit(1).pluck(:filename).first.tap do |filename| + raise ArgumentError, "Filename for key #{key} is blank" if filename.blank? + end end # Crea una ruta para la llave con un nombre conocido. diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index 71d3f049..3ac89c9b 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -23,7 +23,6 @@ class MetadataFile < MetadataTemplate errors << I18n.t("metadata.#{type}.site_invalid") if site.invalid? errors << I18n.t("metadata.#{type}.path_required") if path_missing? - errors << I18n.t("metadata.#{type}.no_file_for_description") if no_file_for_description? errors << I18n.t("metadata.#{type}.attachment_missing") if path? && !static_file errors.compact! @@ -41,14 +40,14 @@ class MetadataFile < MetadataTemplate end # Asociar la imagen subida al sitio y obtener la ruta - # - # XXX: Si evitamos guardar cambios con changed? no tenemos forma de - # saber que un archivo subido manualmente se convirtió en - # un Attachment y cada vez que lo editemos vamos a subir una imagen - # repetida. + # @return [Boolean] def save - value['description'] = sanitize value['description'] - value['path'] = static_file ? relative_destination_path_with_filename.to_s : nil + if value['path'].blank? + self[:value] = default_value + else + value['description'] = sanitize value['description'] + value['path'] = relative_destination_path_with_filename.to_s if static_file + end true end @@ -62,9 +61,6 @@ class MetadataFile < MetadataTemplate # * El archivo es una ruta que apunta a un archivo asociado al sitio # * El archivo es una ruta a un archivo dentro del repositorio # - # XXX: La última opción provoca archivos duplicados, pero es lo mejor - # que tenemos hasta que resolvamos https://0xacab.org/sutty/sutty/-/issues/213 - # # @todo encontrar una forma de obtener el attachment sin tener que # recurrir al último subido. # @@ -75,13 +71,7 @@ class MetadataFile < MetadataTemplate when ActionDispatch::Http::UploadedFile site.static_files.last if site.static_files.attach(value['path']) when String - if (blob_id = ActiveStorage::Blob.where(key: key_from_path).pluck(:id).first) - site.static_files.find_by(blob_id: blob_id) - elsif path? && pathname.exist? && site.static_files.attach(io: pathname.open, filename: pathname.basename) - site.static_files.last.tap do |s| - s.blob.update(key: key_from_path) - end - end + site.static_files.find_by(blob_id: blob_id) || migrate_static_file! end end @@ -98,7 +88,7 @@ class MetadataFile < MetadataTemplate # # @return [String] def key_from_path - pathname.dirname.basename.to_s + @key_from_path ||= pathname.dirname.basename.to_s end def path? @@ -127,13 +117,22 @@ class MetadataFile < MetadataTemplate # devolvemos la ruta original, que puede ser el archivo que no existe # o vacía si se está subiendo uno. rescue Errno::ENOENT => e - ExceptionNotifier.notify_exception(e) + ExceptionNotifier.notify_exception(e, data: { site: site.name, path: value['path'] }) - value['path'] + Pathname.new(File.join(site.path, value['path'])) end + # Obtener la ruta relativa al sitio. + # + # Si algo falla, devolver la ruta original para no romper el archivo. + # + # @return [String, nil] def relative_destination_path_with_filename destination_path_with_filename.relative_path_from(Pathname.new(site.path).realpath) + rescue ArgumentError => e + ExceptionNotifier.notify_exception(e, data: { site: site.name, path: value['path'] }) + + value['path'] end def static_file_path @@ -145,8 +144,31 @@ class MetadataFile < MetadataTemplate end end - # No hay archivo pero se lo describió - def no_file_for_description? - !path? && description? + # Obtiene el id del blob asociado + # + # @return [Integer,nil] + def blob_id + @blob_id ||= ActiveStorage::Blob.where(key: key_from_path, service_name: site.name).pluck(:id).first + end + + # Genera el blob para un archivo que ya se encuentra en el + # repositorio y lo agrega a la base de datos. + # + # @return [ActiveStorage::Attachment] + def migrate_static_file! + raise ArgumentError, 'El archivo no existe' unless path? && pathname.exist? + + Site.transaction do + blob = + ActiveStorage::Blob.create_after_unfurling!(key: key_from_path, + io: pathname.open, + filename: pathname.basename, + service_name: site.name) + + ActiveStorage::Attachment.create!(name: 'static_files', record: site, blob: blob) + end + rescue ArgumentError => e + ExceptionNotifier.notify_exception(e, data: { site: site.name, path: value['path'] }) + nil end end diff --git a/db/migrate/20220712135053_change_blob_key_uniqueness_to_include_service_name.rb b/db/migrate/20220712135053_change_blob_key_uniqueness_to_include_service_name.rb new file mode 100644 index 00000000..00bae7ea --- /dev/null +++ b/db/migrate/20220712135053_change_blob_key_uniqueness_to_include_service_name.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# Cambia el índice único para incluir el nombre del servicio, de forma +# que podamos tener varias copias del mismo sitio (por ejemplo para +# test) sin que falle la creación de archivos. +class ChangeBlobKeyUniquenessToIncludeServiceName < ActiveRecord::Migration[6.1] + def change + remove_index :active_storage_blobs, %i[key], unique: true + add_index :active_storage_blobs, %i[key service_name], unique: true + end +end