From 9e531e49955801bd41a4a8a48ca8d764ebfe342a Mon Sep 17 00:00:00 2001 From: f Date: Tue, 12 Jul 2022 15:38:16 -0300 Subject: [PATCH 1/4] no cambiar el valor si no se pudo cargar el archivo esto hace que sutilmente desaparezcan archivos --- app/models/metadata_file.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index 71d3f049..2b628a57 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -41,14 +41,10 @@ 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 + value['path'] = relative_destination_path_with_filename.to_s if static_file true end From 0b06bf35417b1f23176f798611c70ffc49822985 Mon Sep 17 00:00:00 2001 From: f Date: Tue, 12 Jul 2022 15:39:08 -0300 Subject: [PATCH 2/4] notificar cuando no se pudo cargar el archivo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no perder información silenciosamente --- app/models/metadata_file.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index 2b628a57..942b448d 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -58,9 +58,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. # @@ -77,8 +74,12 @@ class MetadataFile < MetadataTemplate site.static_files.last.tap do |s| s.blob.update(key: key_from_path) end + else + raise ArgumentError, 'No se pudo subir el archivo' end end + rescue ArgumentError => e + ExceptionNotifier.notify_exception(e, data: { site: site.name, path: value['path'] }) end # Obtiene la ruta absoluta al archivo From def4402313e0d142b72321192c1b09783fdfe520 Mon Sep 17 00:00:00 2001 From: f Date: Tue, 12 Jul 2022 15:39:42 -0300 Subject: [PATCH 3/4] permitir cargar archivos con la misma key podemos tener sitios distintos con la misma llave identificatoria. ahora permitimos solo una llave por cada sitio distinto. si subimos un archivo duplicado en un mismo sitio el error va a seguir siendo el mismo. --- app/models/metadata_file.rb | 2 +- ...nge_blob_key_uniqueness_to_include_service_name.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220712135053_change_blob_key_uniqueness_to_include_service_name.rb diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index 942b448d..fa570ac0 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -68,7 +68,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) + if (blob_id = ActiveStorage::Blob.where(key: key_from_path, service_name: site.name).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| 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 From cd523babade074fd69471e9c54cf1c391a2730da Mon Sep 17 00:00:00 2001 From: f Date: Tue, 12 Jul 2022 15:58:31 -0300 Subject: [PATCH 4/4] siempre devolver Pathname closes #6982 closes #6979 closes #6974 closes #6973 closes #6972 closes #6970 closes #6968 closes #6967 --- app/models/metadata_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index fa570ac0..4ea026b4 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -126,7 +126,7 @@ class MetadataFile < MetadataTemplate rescue Errno::ENOENT => e ExceptionNotifier.notify_exception(e) - value['path'] + Pathname.new(value['path']) end def relative_destination_path_with_filename