diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index 8fecf553..f2e69e73 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -30,7 +30,7 @@ class MetadataFile < MetadataTemplate # Determina si el archivo ya fue subido def uploaded? - value['path'].is_a?(String) && path? + value['path'].is_a?(String) end # Determina si la ruta es opcional pero deja pasar si la ruta se @@ -40,15 +40,17 @@ 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. def save - return true unless changed? - value['description'] = sanitize value['description'] value['path'] = nil unless path? - return true if uploaded? return true if path_optional? - return false unless hardlink.zero? + return false unless hardlink # Modificar el valor actual value['path'] = relative_destination_path @@ -56,60 +58,73 @@ class MetadataFile < MetadataTemplate true end - # Almacena el archivo en el sitio y lo devuelve + # Almacena el archivo en el sitio y lo devuelve o lo obtiene de la + # base de datos. # - # @return ActiveStorage::Attachment + # Existen tres casos: + # + # * El archivo fue subido a través de HTTP + # * 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 + # + # @return [ActiveStorage::Attachment] def static_file - return @static_file if @static_file return unless path? - ActiveRecord::Base.connection_pool.with_connection do - if uploaded? - blob = ActiveStorage::Blob.find_by(key: key_from_path) - @static_file ||= site.static_files.find_by(blob_id: blob.id) - elsif site.static_files.attach(value['path']) - @static_file ||= site.static_files.last + @static_file ||= + case value['path'] + when ActionDispatch::Http::UploadedFile + site.static_files.last if site.static_files.attach(value['path']) + when String + if (blob = ActiveStorage::Blob.where(key: key_from_path).pluck(:id).first) + site.static_files.find_by(blob_id: blob) + elsif site.static_files.attach(io: path.open, filename: path.basename) + site.static_files.last + end end - end end def key_from_path path.dirname.basename.to_s end - private - def path? value['path'].present? end + private + def filemagic @filemagic ||= FileMagic.new(FileMagic::MAGIC_MIME) end + # @return [Pathname] def path - @path ||= Pathname.new value['path'] + @path ||= Pathname.new(File.join(site.path, value['path'])) end - # Obtiene la ruta absoluta del archivo - # - # @return [String|Nil] def file - return unless path? - - if value['path'].is_a? ActionDispatch::Http::UploadedFile - value['path'].tempfile.path - else - File.join site.path, value['path'] - end + @file ||= + case value['path'] + when ActionDispatch::Http::UploadedFile then value['path'].tempfile.path + when String then File.join(site.path, value['path']) + end end # Hacemos un link duro para colocar el archivo dentro del repositorio # y no duplicar el espacio que ocupan. Esto requiere que ambos # directorios estén dentro del mismo punto de montaje. + # + # XXX: Asumimos que el archivo destino no existe porque siempre + # contiene una key única. + # + # @return [Boolean] def hardlink - FileUtils.mkdir_p File.dirname(destination_path) - FileUtils.ln uploaded_path, destination_path + FileUtils.mkdir_p(File.dirname(destination_path)) + FileUtils.ln(uploaded_path, destination_path).zero? end # Obtener la ruta al archivo @@ -118,16 +133,23 @@ class MetadataFile < MetadataTemplate ActiveStorage::Blob.service.path_for(static_file.key) end + # @return [String] def uploaded_path Rails.root.join uploaded_relative_path end + # La ruta del archivo mantiene el nombre original pero contiene el + # nombre interno y único del archivo para poder relacionarlo con el + # archivo subido en Sutty. + # + # @return [String] def relative_destination_path - File.join('public', static_file.key, static_file.filename.to_s) + @relative_destination_path ||= File.join('public', static_file.key, static_file.filename.to_s) end + # @return [String] def destination_path - File.join(site.path, relative_destination_path) + @destination_path ||= File.join(site.path, relative_destination_path) end # No hay archivo pero se lo describió diff --git a/app/models/site/static_file_migration.rb b/app/models/site/static_file_migration.rb index 614f63dd..36a882bf 100644 --- a/app/models/site/static_file_migration.rb +++ b/app/models/site/static_file_migration.rb @@ -2,15 +2,10 @@ class Site # Obtiene todos los archivos relacionados en artículos del sitio y los - # sube a Sutty de forma que los podamos seguir utilizando normalmente - # sin casos especiales (ej. soportar archivos locales al repositorio y - # remotos, alojados en Sutty) - # - # TODO: Convertir en reutilizable, por ejemplo correr en cada pull, no - # asumir que la migración se hizo una sola vez... + # sube a Sutty. class StaticFileMigration # Tipos de metadatos que contienen archivos - STATIC_TYPES = %w[file image].freeze + STATIC_TYPES = %i[file image].freeze attr_reader :site @@ -18,72 +13,20 @@ class Site @site = site end - # Recorre todos los artículos cuyos layouts contengan campos con - # archivos estáticos def migrate! - log = File.open(File.join(site.path, 'migration.log'), 'w') - modified = [] + modified = site.docs.map do |doc| + next unless STATIC_TYPES.map do |field| + next unless doc.attribute? field + next unless doc[field].path? + next unless doc[field].static_file - Dir.chdir site.path do - site.locales.each do |locale| - # Recorrer todos los documentos de todas las colecciones - site.posts(lang: locale).each do |doc| - # Ignoramos los documentos cuyo layout no contiene archivos - next unless layouts.include? doc.layout.name + true + end.any? - # Buscamos todos los campos con archivos - fields.each do |field| - next unless doc.attribute? field - next unless doc.document.data.key? field.to_s + log.write "#{doc.path.relative};no se pudo guardar\n" unless doc.save(validate: false) - # Traemos los metadatos, en este punto, Sutty cree que el - # archivo está subido, porque es una string apuntando a un - # archivo. - metadata = doc.public_send(field) - - next if metadata.value['path'].blank? - next if ActiveStorage::Blob.find_by(key: metadata.key_from_path) - - path = Pathname.new(metadata.value['path']) - - # Si no existe vaciamos el campo - unless path.exist? - metadata.value['path'] = nil - log.write "el archivo #{path} de #{doc.path.relative} no existe" - next - end - - # Agregamos el archivo al sitio y se lo asignamos al campo - # XXX: No usamos ActionDispatch::Http::UploadedFile porque - # no tenemos forma de crear un Tempfile o equivalente a - # partir de un archivo que exista. - metadata.value['path'] = { - io: path.open, - filename: path.basename - } - - # Copiar y analizar el archivo sincrónicamente - metadata.static_file.blob.upload path.open - metadata.static_file.blob.analyze - - dest = Pathname.new(metadata.send(:relative_destination_path)) - metadata.value['path'] = dest.to_s - metadata.send(:hardlink) - - # Eliminamos el archivo original y lo vinculamos al subido - # para mantener la ruta y no romper el sitio - FileUtils.rm_f path - # XXX: Link simbólico o duro? - FileUtils.ln_s dest.relative_path_from(path.dirname), path - end - - # Guardamos los cambios - log.write "#{doc.path.relative} no se pudo guardar\n" unless doc.save(validate: false) - - modified << doc.path.absolute - end - end - end + doc.path.absolute + end.compact log.close @@ -102,22 +45,8 @@ class Site name: 'Sutty' end - # Encuentra todos los layouts con campos estáticos - def layouts - @layouts ||= site.layouts.to_h.reject do |_, layout| - layout.metadata.select do |_, desc| - STATIC_TYPES.include? desc['type'] - end.empty? - end.keys - end - - # Encuentra todos los campos con archivos estáticos - def fields - @fields ||= layouts.map do |layout| - site.layouts[layout].metadata.select do |_, desc| - STATIC_TYPES.include? desc['type'] - end.keys - end.flatten.uniq.map(&:to_sym) + def log + @log ||= File.open(File.join(site.path, 'migration.csv'), 'w') end end end diff --git a/test/models/site/repository_test.rb b/test/models/site/repository_test.rb index cb084d83..39da8696 100644 --- a/test/models/site/repository_test.rb +++ b/test/models/site/repository_test.rb @@ -32,7 +32,7 @@ class RepositoryTest < ActiveSupport::TestCase .branches['master'].target.author[:name] Dir.chdir(@site.path) do - FileUtils.rm 'migration.log' + FileUtils.rm 'migration.csv' assert_equal 'nothing to commit, working tree clean', `LC_ALL=C git status`.strip.split("\n").last