Saltar al contenido
Development

Limpiar un trimestre de código muerto en un solo PR

Por Victor Da Luz
railsrubyrefactoringdev-logblog-manager

Tenía acumulado un backlog de pequeños ítems de limpieza en blog-manager, columnas sobrantes de una integración con Postiz que había arrancado, una búsqueda de configuración que golpea la base de datos en cada llamada, y algunos filtros del lado de Ruby que deberían haber sido SQL desde el principio. Nada de eso era lo bastante urgente como para arreglarlo por separado, así que se seguía postergando. Este fue el issue de “hacerlo todo de una vez en un solo PR”.

Columnas muertas de Postiz

Antes de construir la publicación nativa a Medium y Dev.to, esta app usaba Postiz como intermediario de sindicación. Esa integración ya no existe, pero las columnas que dejó atrás sí seguían ahí: 7 en posts (postiz_channels, postiz_error, postiz_group_id, postiz_posted_at, postiz_publish, postiz_scheduled_for, postiz_status más su índice), 2 en app_settings (postiz_api_key, postiz_base_url). Primero busqué postiz en toda la app con grep, los únicos resultados que quedaban eran comentarios explicando por qué algo ya no se hace a la vieja manera de Postiz. Se podían eliminar sin riesgo. Una sola migración, siguiendo el mismo patrón de cuando eliminé las integraciones de Hashnode y las anteriores de Dev.to/Medium:

class RemovePostizIntegration < ActiveRecord::Migration[8.1]
  def change
    remove_index :posts, :postiz_status
    remove_column :posts, :postiz_channels, :text
    # ...six more remove_column calls
    remove_column :app_settings, :postiz_api_key, :string
    remove_column :app_settings, :postiz_base_url, :string
  end
end

Memoizar AppSetting.current, y por qué Rails.cache era la herramienta equivocada

AppSetting.current ejecuta first_or_create! en cada llamada. Hay 11 puntos de llamada repartidos entre controladores, jobs y vistas, así que el renderizado de una sola página puede golpear la tabla de configuración media docena de veces por una fila que casi nunca cambia.

Mi primer instinto fue Rails.cache.fetch. Después recordé que AppSetting tiene encrypts :dev_to_api_key, :medium_bridge_token, :listmonk_api_token, :pexels_api_key, y que el cache store de producción es solid_cache_store, que persiste en una tabla de base de datos. encrypts solo protege la columna en reposo dentro de su propia tabla. Una vez que el registro está cargado, esos atributos son texto plano en memoria. Si se cachea el objeto completo ya cargado, Rails serializa (Marshal) ese texto plano dentro de la tabla del cache store. Habría anulado el cifrado en silencio simplemente cacheando alrededor de él.

La app ya tenía la herramienta correcta ahí, sin usar para este propósito, Current < ActiveSupport::CurrentAttributes, que por ahora solo guardaba la sesión:

class Current < ActiveSupport::CurrentAttributes
  attribute :session, :app_setting
  delegate :user, to: :session, allow_nil: true
end
def self.current
  Current.app_setting ||= first_or_create!
end

CurrentAttributes es dentro del proceso, local al thread/fiber, y Rails lo reinicia automáticamente en cada request de controlador y en cada ejecución de Active Job, lo cual importa acá porque esta app corre un proceso worker de Solid Queue separado. Un cambio de configuración en el proceso web nunca queda desactualizado en el worker más allá del job que está corriendo en ese momento, y nada toca jamás un almacén persistente. La misma reducción en cantidad de llamadas, sin ninguno de los riesgos.

Mover filtros del lado de Ruby a SQL

Había tres lugares que cargaban posts y filtraban en Ruby en lugar de en la base de datos. Los filtros de sindicación del controlador mapeaban nombres de filtro a métodos predicado, y después hacían @posts.select(&predicate) después de cargar el scope completo (~80 filas). El dashboard cargaba cada post no eliminado en un array solo para hacer .count(&:needs_medium?) cuatro veces. Y el selector del newsletter era el que tenía un bug real: Post.kept.by_pub_date.limit(50).select { |post| post.live_url.present? }. Filtra después de limitar. Si 50 posts no publicados terminan ordenados antes que los publicables, el selector del newsletter muestra en silencio cero posts seleccionables aunque existan varios justo después de ese límite.

Agregué scopes que reflejan los predicados de instancia existentes:

scope :needs_medium,       -> { where(medium_status: [ :not_posted, :failed ]) }
scope :needs_devto,        -> { where(devto_status: [ :not_posted, :failed ]) }
scope :syndication_failed, -> { where(medium_status: :failed).or(where(devto_status: :failed)) }
scope :awaiting_publish,   -> { where(medium_status: :draft).or(where(devto_status: :draft)) }
scope :with_live_url,      -> { joins(:blog).where.not(pub_date: nil).where.not(blogs: { base_url: [ nil, "" ] }) }

y cambié el selector del newsletter para filtrar antes de limitar: Post.kept.includes(:blog).with_live_url.by_pub_date.limit(50).

Para asegurarme de que los scopes de SQL coincidieran realmente con los predicados de Ruby que reemplazaban (en vez de confiar sin más en mi propia traducción), escribí una prueba de equivalencia:

assert_equal Post.all.select(&:needs_medium?).map(&:id).sort, Post.needs_medium.pluck(:id).sort

Un seguro barato contra un valor de enum desfasado por uno que cambie el comportamiento en silencio.

Lo único que casi hice mal

El issue describía el código en publish_devto_article después de un redirect_to(...) and return temprano como “inalcanzable… eliminarlo”. Casi lo hice. Mirando más de cerca, ese no es código descartable, es todo el flujo nativo de publicación a Dev.to (verificaciones de API key, URL en vivo, imagen de portada, token de GitHub, encolado del job), construido antes y bloqueado a la espera de verificación contra la API real de Forem. Eliminarlo habría significado reconstruir todo desde el historial de git más tarde.

En su lugar, lo bloqueé con una bandera explícita:

DEVTO_PUBLISHING_ENABLED = false
# ...
def publish_devto_article
  return redirect_to(@post, alert: DEVTO_DISABLED_MESSAGE) unless DEVTO_PUBLISHING_ENABLED
  # full publish flow, untouched, ready to flip on once verified
end

Mismo comportamiento hoy, y activarlo más adelante se convierte en un cambio de una línea en vez de un proyecto de arqueología.

También en esta tanda: eliminé un hello_controller.js sin usar (boilerplate del generador de Stimulus, cero referencias), y arreglé un vacío de escapado HTML en el constructor de campañas del newsletter, los títulos de post se escapaban pero el href no, así que un título de post o una URL con un & habría producido HTML malformado en el cuerpo del newsletter.

Qué sigue

Nada de esto cambia el comportamiento visible para quien usa la app, salvo el arreglo del subllenado del selector del newsletter, que es una corrección de bug, no una funcionalidad. 282 tests, rubocop y brakeman en verde antes de fusionar.

Lecturas relacionadas