Merge branch 'jekyll-service' into 'rails'

permitir tener sitios duplicados

Closes #6967, #6968, #6970, #6972, #6973, #6974, #6979, #6982, #6966, #6969, #6985, #6990, #6991, #9362, #9364, #9377, #9378, #7611, #7612, #7844, #7845, #8098, #8099, #8674, #8764, #8765, #8888, #9366, #9374, #9375, #9379, #9380, #9376, #9363, #8887, #8690, #8262, #7998, #7807, #7053, and #6984

See merge request sutty/sutty!101

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.

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

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