En nuestro día a día como developers, todas/os nos encontramos o hemos encontrado con proyectos de hace unos años escritos en un código que no se concibió con la idea de ser mantenible, sobre el que nos toca trabajar aunque no nos inspire confianza para hacerlo sin romper nada.

Aunque es menos común, hay ocasiones en las que este spaghetti code está implementado en forma de sentencias SQL de 200-300 líneas, con toda la lógica de dominio incrustada en las propias consultas, sobre un modelo de datos innecesariamente enrevesado que gana complejidad con los años por desconocimiento, y por miedo a hacer modificaciones para no romper un sistema que funciona. Nos hemos encontrado este tipo de situación, por ejemplo, al trabajar en un proyecto basado en añadir personalizaciones sobre un ERP que originalmente no estaba pensado para ello.

En este artículo, propondremos una pequeña estrategia para enfrentar esa situación, en la que nos piden desarrollar una funcionalidad nueva sobre este código, sin morir en el intento :skull:.

Cómo atacar el problema

1. Estudia su comportamiento

Parece obvio, pero es muy probable que en estos casos los autores originales del código ya no se encuentren en la empresa, y nadie sepa exactamente cómo funciona, a excepción de alguna idea vaga de gente que ha trabajado en desarrollos relacionados.

Esto nos lleva a que, el primer paso sea un spike (investigación) sobre las clases que contienen las consultas, en el que tendremos que responder preguntas como:

• ¿En qué flujo de procesos de negocio se está ejecutando?
• ¿Cuáles son las tablas (o vistas) implicadas en la query?¿Cuál es su dominio de negocio?¿Qué significan cada uno de sus campos y qué valores pueden tomar?
• Si se trata de tablas que se usan a modo de tabla temporal, ¿en qué punto se rellenan y se borran sus entradas?
• ¿Qué tipo de JOIN se está aplicando en cada una?
• ¿Qué campos se están recuperando en cada consulta anidada y qué están aportando a la consulta principal?
• ¿Qué filtros se están aplicando en las cláusulas WHERE?¿Qué lógica se está aplicando sobre los campos seleccionados?

Para este tipo de spike, viene muy bien usar herramientas como Notion para ir documentando, a la vez que generamos más conocimiento sobre el sistema.

2. Tests arnés

Aunque lo situemos como segundo paso, en realidad escribir tests a modo de arnés de seguridad también sirve para tener una imagen del comportamiento de las consultas en base a su input y output, e incluso para ponerlas a prueba con casos límite que se nos puedan ocurrir.

Para ello tendremos que tener en cuenta algunas consideraciones:

• En casos como el del ERP que comentábamos, donde la lógica de conexión a la base de datos se gestiona mediante métodos de código propietario al que no tenemos acceso, puede que sea necesario crear una costura en nuestros tests: una nueva clase "testable" que hereda de la clase productiva, donde hacemos override del método que ejecuta la query y gestiona la conexión, aplicando nuestra lógica de forma que se respete su comportamiento original. Esto nos da la posibilidad de gestionar nosotros la conexión, dándonos el control total del entorno de base de datos que usarán los tests.

    class TestableOriginalClass extends OriginalClass {

        public TestableOriginalClass(Session session) {
            super(session);
        }

        @Override
        protected int executeQuery(String query, Object... parameters) {
            // ...
    	    // Our database connection and query execution logic
    	    // ...
        }
    }

• Los tests pueden no garantizar una seguridad absoluta:
  • Para seguir esta estrategia, será necesaria la tediosa labor de preparar conjuntos de datos de pruebas para todas las tablas implicadas. Esto significa que tendremos un modelo de datos falso y estático, que puede sesgar nuestra visión del comportamiento del código al estar diseñado en base a los escenarios de test.
  • Si se trata de un entorno de gran incertidumbre y desconocimiento del sistema o dominio, es bastante probable que los escenarios de test planteados estén incompletos y no cubran todas las casuísticas, lo que puede provocar que en productivo se den comportamientos inesperados o no contemplados. Para hacer frente a esto puede ser útil que los equipos de de negocio que trabajan en su día a día con el flujo de procesos afectado, ayuden a plantear los escenarios que suelen encontrarse.

3. Continuar el desarrollo

Una vez identificado y testeado el comportamiento de las queries, puede que sea deseable pasar al refactor de las mismas, hacer un poco de scouting, ya que estamos por ahí para aclararnos. Evidentemente lo ideal sería extraer la lógica de las consultas a código, pero no siempre se puede por motivos de tiempo, complejidad o restricciones de hardware.

El refactor más sencillo (y que puede ser de gran ayuda incluso desde el primer paso) es mejorar la legibilidad del string donde se formatea la query.

En este tipo de situaciones, es común encontrarse con una identación confusa o bloques de comentarios de hace años que no aportan nada. Darle un formato limpio que sea fácil de leer, ayuda a identificar mejor el comportamiento de un vistazo y a trabajar de forma más cómoda con la query.

Puede ser tentador pensar en extraer variables de partes del string, como cálculos de negocio, para dar contexto con un buen naming, pero en la práctica puede no ser una buena opción. Por ejemplo, suponiendo el siguiente fragmento de query original:

...
.append("  CASE ")
.append("    WHEN (field_name = 'some random value') ")
.append("      THEN ")
.append("        CASE ")
.append("          WHEN (field_A <= field_B) ")
.append("            THEN 0 ")
.append("          WHEN (field_A > field_B) ")
.append("            THEN (cast((field_A + 2 * (field_C- field_B)) / 1000 as decimal (19,10)) ")
.append("                  / datediff(dd,date_from,date_to)) )")
.append("        END ")
.append("    ELSE ")
...

Podrían extraerse variables para dar contexto de la siguiente forma:

...
String isThisACaseX = " WHEN (field_name = 'some random value') ";
String resultOfSomeBusinessLogic = "cast((field_A + 2 * (field_C- field_B)) / 1000 as decimal (19,10))";
String elapsedDays = "datediff(dd,date_from,date_to)) )";
...
// Dozens of query lines
...
.append("  CASE ")
.append(isThisACaseX)
.append("      THEN ")
.append("        CASE ")
.append("          WHEN (field_A <= field_B) ")
.append("            THEN 0 ")
.append("          WHEN (field_A > field_B) ")
.append("            THEN  " + resultOfSomeBusinessLogic + "/" + elapsedDays)
.append("        END ")
.append("    ELSE ")
...

Y efectivamente, podemos comprobar que la consulta se vuelve más amigable al desarrollador que la está leyendo. El problema es que cuando se necesita saber qué se está haciendo en esos fragmentos de forma concreta, no queda más remedio que dar saltos navegando en el código o dividir la pantalla, impidiendo también seleccionar y copiar toda la query de una sola vez en caso de que necesitemos ejecutarla manualmente.

Puede acabar siendo más práctico añadir pequeños comentarios para dar contexto en puntos concretos:

...
.append("  CASE ")
//           Is this business variable on X case? 
.append("    WHEN (field_name = 'some random value') ")
.append("      THEN ")
//               Apply according logic
.append("        CASE ")
.append("          WHEN (field_A <= field_B) ")
.append("            THEN 0 ")
.append("          WHEN (field_A > field_B) ")
.append("            THEN (cast((field_A + 2 * (field_C- field_B)) / 1000 as decimal (19,10)) ")
.append("                  / datediff(dd,date_from,date_to)) )")
.append("        END ")
.append("    ELSE ")
...

Además del formato, también podemos intentar ir granulando los métodos de las clases implicadas para cumplir un poco de SOLID, o refactorizar condiciones o campos que se calculan y luego no se usan, como veremos en el apartado extra.

Tras el refactor, procedemos a añadir la nueva funcionalidad que necesitemos a la query, como siempre añadiendo tests que cubran los nuevos escenarios, y comprobando que no rompemos los tests anteriores (a no ser que cubran casuísticas que ahora dejan de aplicarse).

Extra: Tratar problemas de rendimiento

Al trabajar en un sistema basado en legacy queries que da servicio a un número de usuarias/os concurrente en aumento, con un volumen de datos cada vez mayor, es bastante probable que se acaben dando problemas de rendimiento en la ejecución de flujos de negocio en productivo.

Aunque puede que no haya más que hacer y se trate de que se está llegando a un límite en relación al volumen de datos y tráfico y la capacidad del hardware, puede ser útil tener en cuenta un procedimiento básico a seguir para mejorar el rendimiento:

• Identificar los procesos que están afectando al rendimiento de la base de datos. Suelen ser sospechosos los que llevan demasiado tiempo en ejecución.
• Identificar qué query es la que está ejecutando el proceso que está bloqueando al resto.
• Se pueden optimizar las queries moviendo condiciones del WHERE a las condiciones de los JOINs, si la tabla implicada en las condiciones se corresponde con la que se está recuperando en el JOIN en cuestión. De esta forma se aplicarán antes más filtros restrictivos y se mejorará el rendimiento al trabajar con menos entradas.
• Puede que a veces se encuentren fragmentos de la query redundantes o que se calculan en una subquery para luego no usarse en niveles superiores, podemos eliminarlos (con la seguridad de nuestros tests :ok_hand:) para quitar lastre del procesamiento.
• Identificar las tablas implicadas en la query, y comprobar si se pueden añadir índices a sus campos principales, especialmente a aquellos con valores más susceptibles de cambio en cada entrada.
• También puede ser útil hacer uso de EXPLAIN, una herramienta incluida en todos los motores de base de datos (aunque puede cambiar la sintaxis de un dialecto SQL a otro) para inspeccionar la ejecución de la query paso a paso. El inconveniente de esta herramienta es que si se trata de consultas demasiado complejas, o hay vistas demasiado grandes implicadas, que se deben calcular previamente, interpretar la ejecución puede no ser tan obvio.