Pagando la deuda técnica de mi plugin de Moodle: troceando dos "archivos-dios"

Pagando la deuda técnica de mi plugin de Moodle: troceando dos "archivos-dios"
ContenidoContents

Si usas mod_jitsi y has notado que últimamente no saca novedades, no es que lo haya aparcado: es justo lo contrario. Estos días estoy metido en la limpieza de deuda técnica más grande que le he hecho al plugin —el que integra videollamadas de Jitsi en Moodle—, y eso se está llevando todo el tiempo que normalmente iría a funciones nuevas. Todavía está todo en la rama dev, sin publicar: es trabajo en curso.

Y ya me ha pasado algo que resume por qué vale la pena hacerlo con los tests por delante. Escribí un test para una función trivial —un botón que enciende y apaga la cámara— y el test me encontró un bug que llevaba ahí, agazapado, quién sabe cuánto tiempo: la función intentaba leer una propiedad de un objeto que nunca llegaba a existir. En producción no peta de milagro. Y ha sido solo la primera de seis o siete sorpresas. Os cuento cómo va.

Dos archivos que nadie quiere tocar

Todo plugin que crece durante años acumula sus rincones oscuros. Los míos son dos:

  • classes/external.php: 2.698 líneas, una única clase con 126 métodos (41 endpoints AJAX, cada uno con sus tres métodos al estilo Moodle antiguo). Añadir un endpoint significa editar un archivo de 2.700 líneas y rezar para no chocar en el siguiente merge.
  • lib.php: 3.216 líneas, 47 funciones globales donde conviven callbacks de Moodle, helpers, lógica de base de datos, integración con YouTube/Google, notificaciones push… y una función de 924 líneas que genera el HTML y el JS de la videollamada a base de 526 echo.

Lo irónico es que el resto del plugin ya usa namespaces modernos. Solo estos dos siguen anclados en 2015.

El plan: tests primero, a trocitos

La tentación con un refactor así es hacerlo “de un tirón”. Mala idea. Lo estoy haciendo en tandas pequeñas y con los tests por delante: escribir el test contra el código actual → verlo pasar → mover el código → volver a verde → phpcs, subir versión, correr toda la suite → commit. Llevo repetido el ciclo una veintena de veces.

Esta disciplina, que parece lenta, es justo la que va destapando los bugs.

De un archivo de 2.700 líneas a 40 archivos pequeños

external.php lo estoy partiendo en una clase por endpoint, siguiendo el estándar moderno de Moodle (core_external\external_api, autocarga, sin el viejo classpath). El nombre público de cada web service no cambia, así que el JavaScript y la app móvil siguen funcionando sin tocar nada.

¿Y no deja un hueco borrar external.php entero? No. En el Moodle moderno ese archivo ya no pinta nada: no es un nombre reservado —como sí lo son lib.php o db/services.php—, era solo el sitio donde por tradición se metía la clase con todas las funciones dentro. Desde Moodle 4.2, cada función vive en su propia clase autocargada bajo classes/external/ y se declara en db/services.php. El mecanismo viejo (una clase gigante cargada a mano vía classpath) está incluso marcado para deprecación a partir de Moodle 4.6. O sea, que el monolito no solo sobraba: estaba en vías de quedar obsoleto.

Y lib.php va ya de 3.216 a 1.086 líneas (−66%): sus helpers se están repartiendo en clases con responsabilidad clara (room, attendance, recording, google, youtube, notification…). El archivo gigante external.php ha llegado a desaparecer (2.698 líneas → 0).

Los bugs que no sabía que tenía

Aquí está lo interesante: no busco bugs, busco ordenar. Pero al escribir un test para cada función antes de moverla, van cayendo solos:

1// El botón de la cámara, ANTES (roto): $jitsi es un int, no un objeto,
2// y $jitsiob no se llega a definir nunca
3$event->add_record_snapshot('course', $jitsi->course);
4$event->add_record_snapshot('jitsi', $jitsiob);
5
6// DESPUÉS:
7$jitsiob = $DB->get_record('jitsi', ['id' => $jitsi]);
8$course  = $DB->get_record('course', ['id' => $jitsiob->course]);

Llevo seis o siete fallos latentes. Muchos son inofensivos en producción (esa validación de Moodle solo corre en modo debug), pero están mal. Y uno da miedo de verdad: una función muerta llamada get_participants que, de ejecutarse, hacía $jitsiob->name = 'modificado' y guardaba el registro… es decir, renombraría la actividad a “modificado”. No la llama nadie, pero ahí está, como una mina.

El casi-desastre: cuando tu propio script te miente

Esta es mi favorita de la semana. Tras mover la función getminutes() a su nueva clase, quise comprobar que no quedaban llamadas antiguas. Usé un grep “listo”:

1grep "getminutes(" -r . | grep -v "::"   # ignorar las ya migradas (attendance::minutes)

La idea: descartar las líneas con :: porque ya estaban migradas. Pero había una llamada vieja olvidada en view.php:

1html_writer::tag('div', getminutes($id, $USER->id), ...)

¿Veis el problema? Esa línea contiene :: (por html_writer::tag), así que mi grep “inteligente” la ocultó. Los 102 tests unitarios tampoco la pillaron, porque no cargan view.php. La encontró una simple prueba de humo en el navegador: la página reventaba con Call to undefined function getminutes().

Doble moraleja, barata de aprender: no filtres tu verificación con un grep -v demasiado astuto, y los tests unitarios no cargan tus páginas — hay que abrir la vista real aunque sea una vez.

(Bonus para fans del detalle: el sed de macOS no entiende \b, así que un reemplazo falla en silencio y deja clases sin su prefijo de namespace. Otra que solo se ve si miras.)

Mover no es mejorar

Una confesión sobre lo que llevo decidido. Esa función de 924 líneas que genera la videollamada la estoy moviendo tal cual a una clase nueva, sin tocar sus 526 echo. Evalué reescribirla con plantillas Mustache y módulos AMD… y he decidido no hacerlo ahora: es alto riesgo (hay que probar videollamadas reales) y el objetivo de esta fase es vaciar lib.php, no rediseñar el reproductor. Eso queda para más adelante.

Conviene ser honesto: sacar un monstruo de un archivo y meterlo en otro reduce el ruido, pero no arregla el monstruo. Es deuda reubicada, no saldada. Eso sí, antes de moverla la he blindado con tests para los cuatro tipos de servidor, incluida la verificación de la firma del token JWT (lo más crítico del plugin, y que antes no tenía ni una prueba).

Dónde estoy ahora

Esto es una foto del trabajo en curso, todo en dev:

  • lib.php: −66% hasta ahora (3.216 → 1.086 líneas). external.php: eliminado (2.698 → 0).
  • 64 → 105 tests (+41), con la generación del JWT por fin cubierta.
  • 6-7 bugs latentes corregidos por el camino y tres funciones muertas (una peligrosa) fuera.
  • Compatibilidad total: ningún cliente externo nota el cambio.

Y lo que queda por delante: reescribir de verdad esa función de 924 líneas (Mustache + AMD), seguir bajando el acoplamiento y, cuando todo esté verde y probado en llamadas reales, llevarlo de dev a master.

Si algo me estoy llevando de momento: refactorizar con los tests por delante no solo te protege, te encuentra bugs que no sabías que tenías; desconfía de tus propios atajos de verificación; y sé honesto sobre qué deuda saldas y cuál solo cambias de sitio.


Continúa en la segunda parte: cumplo la promesa de “mover no es mejorar” y desmonto de verdad esa función de 924 líneas en datos, plantillas y módulos.

CompartirShare