test: cobertura y verificación de la solución del Sprint 1

pom.xml

@@ -21,8 +21,10 @@
 		<springdoc.version>3.1.1</springdoc.version>
 		<archunit.version>1.4.1</archunit.version>
 		<testcontainers.version>1.21.4</testcontainers.version>
-		<jacoco.version>0.8.12</jacoco.version>
 
+		<jacoco.version>0.8.13</jacoco.version>
+		<pitest.version>1.19.1</pitest.version>
+		<pitest.junit5.version>1.2.2</pitest.junit5.version>
 		<sonar.organization>codefactorybookingapp</sonar.organization>
 		<sonar.projectKey>CodeFactoryBookingApp_bookingplatform</sonar.projectKey>
 		<sonar.host.url>https://sonarcloud.io</sonar.host.url>
@@ -174,6 +176,93 @@
 					</execution>
 				</executions>
 			</plugin>
+			<!--
+			  Coverage measurement (QA). The agent instruments the unit test run;
+			  the report lands in target/site/jacoco. Classes with no branches or
+			  statements of their own (records, enums, the Spring entry point and
+			  declarative @Configuration) are excluded: counting them inflates the
+			  number without telling us anything about the behaviour under test.
+			-->
+			<plugin>
+				<groupId>org.jacoco</groupId>
+				<artifactId>jacoco-maven-plugin</artifactId>
+				<version>${jacoco.version}</version>
+				<configuration>
+					<excludes>
+						<!--
+						  Único excluido: el arranque de Spring Boot. Su main()
+						  solo delega en SpringApplication.run y cubrirlo exige
+						  levantar el contexto entero sin probar nada propio.
+
+						  Todo lo demás se mide, incluidos los DTO y los records
+						  de propiedades: excluirlos sube el porcentaje sin decir
+						  nada sobre lo que está probado, que es justo lo que una
+						  cifra de cobertura no debe hacer.
+						-->
+						<exclude>**/BookingPlatformApplication.class</exclude>
+					</excludes>
+				</configuration>
+				<executions>
+					<execution>
+						<id>prepare-agent</id>
+						<goals>
+							<goal>prepare-agent</goal>
+						</goals>
+					</execution>
+					<!--
+					  `prepare-agent` fija la propiedad argLine, y failsafe la
+					  hereda igual que surefire: los ITs de Testcontainers quedan
+					  instrumentados sin configuración extra y su cobertura se
+					  acumula en el mismo target/jacoco.exec. El informe se emite
+					  al final, ya con las dos fases dentro.
+
+					  El número oficial sale de `mvn clean verify`. Sin `clean`,
+					  el fichero de ejecución acumula corridas anteriores.
+					-->
+					<execution>
+						<id>report</id>
+						<phase>verify</phase>
+						<goals>
+							<goal>report</goal>
+						</goals>
+					</execution>
+					<!--
+					  Umbral: por debajo de 90% el build falla. Se exige tanto en
+					  instrucciones como en ramas, y la de ramas es la que de
+					  verdad protege: se puede recorrer una línea sin probar
+					  ninguna de las decisiones que toma.
+
+					  Está en 90 y no en el 100 que hay hoy para dejar margen al
+					  mantenimiento sin romper la rama a nadie.
+					-->
+					<execution>
+						<id>check-coverage</id>
+						<phase>verify</phase>
+						<goals>
+							<goal>check</goal>
+						</goals>
+						<configuration>
+							<rules>
+								<rule>
+									<element>BUNDLE</element>
+									<limits>
+										<limit>
+											<counter>INSTRUCTION</counter>
+											<value>COVEREDRATIO</value>
+											<minimum>0.90</minimum>
+										</limit>
+										<limit>
+											<counter>BRANCH</counter>
+											<value>COVEREDRATIO</value>
+											<minimum>0.90</minimum>
+										</limit>
+									</limits>
+								</rule>
+							</rules>
+						</configuration>
+					</execution>
+				</executions>
+			</plugin>
 			<plugin>
 				<groupId>org.apache.maven.plugins</groupId>
 				<artifactId>maven-compiler-plugin</artifactId>
@@ -200,25 +289,92 @@
 					</compilerArgs>
 				</configuration>
 			</plugin>
+			<!--
+			  Prueba de mutacion (QA). La cobertura demuestra que el codigo se
+			  ejecuta; PIT demuestra que las pruebas lo vigilan: siembra defectos
+			  en el bytecode y comprueba que alguna prueba falle por cada uno.
+			  No se ata a ninguna fase: se lanza a mano con
+			  `mvnw test-compile org.pitest:pitest-maven:mutationCoverage`.
+			-->
 			<plugin>
-				<groupId>org.jacoco</groupId>
-				<artifactId>jacoco-maven-plugin</artifactId>
-				<version>${jacoco.version}</version>
-				<executions>
-					<execution>
-						<id>prepare-agent</id>
-						<goals>
-							<goal>prepare-agent</goal>
-						</goals>
-					</execution>
-					<execution>
-						<id>report</id>
-						<phase>verify</phase>
-						<goals>
-							<goal>report</goal>
-						</goals>
-					</execution>
-				</executions>
+				<groupId>org.pitest</groupId>
+				<artifactId>pitest-maven</artifactId>
+				<version>${pitest.version}</version>
+				<dependencies>
+					<dependency>
+						<groupId>org.pitest</groupId>
+						<artifactId>pitest-junit5-plugin</artifactId>
+						<version>${pitest.junit5.version}</version>
+					</dependency>
+				</dependencies>
+				<configuration>
+					<targetClasses>
+						<param>com.codefactory.bookingplatform.*</param>
+					</targetClasses>
+					<targetTests>
+						<param>com.codefactory.bookingplatform.*</param>
+					</targetTests>
+					<!--
+					  Fuera del ambito mutado, y el motivo de cada exclusion:
+
+					  - BookingPlatformApplication: solo delega en SpringApplication.run.
+					  - Enums y records puros (DTO, propiedades, comandos, resultados):
+					    lo que PIT muta ahi es el equals/hashCode/toString y los
+					    accessors que genera el compilador. Matar esos mutantes solo se
+					    consigue escribiendo pruebas de codigo generado, que es relleno:
+					    sube el porcentaje sin proteger ninguna regla de negocio.
+					  - OpenApiConfig, ClockConfig, JpaAuditingConfig: configuracion
+					    declarativa sin decisiones. Lo mismo.
+
+					  Todo lo demas se muta, incluidas las entidades JPA, SecurityConfig
+					  y las politicas de dominio, que es donde un superviviente importa.
+					-->
+					<excludedClasses>
+						<param>com.codefactory.bookingplatform.BookingPlatformApplication</param>
+						<param>com.codefactory.bookingplatform.*.api.dto.*</param>
+						<param>com.codefactory.bookingplatform.auth.domain.model.AppRole</param>
+						<param>com.codefactory.bookingplatform.auth.domain.model.AuthTokens</param>
+						<param>com.codefactory.bookingplatform.auth.domain.model.ConfirmedUser</param>
+						<param>com.codefactory.bookingplatform.auth.domain.model.UpstreamAuthError</param>
+						<param>com.codefactory.bookingplatform.identity.domain.model.ClientStatus</param>
+						<param>com.codefactory.bookingplatform.identity.domain.model.NotificationChannel</param>
+						<param>com.codefactory.bookingplatform.identity.application.RegisterClientCommand</param>
+						<param>com.codefactory.bookingplatform.identity.application.RegistrationOutcome</param>
+						<param>com.codefactory.bookingplatform.shared.error.ErrorCode</param>
+						<param>com.codefactory.bookingplatform.shared.config.AuthPolicyProperties</param>
+						<param>com.codefactory.bookingplatform.shared.config.SecurityProperties</param>
+						<param>com.codefactory.bookingplatform.shared.config.SupabaseProperties</param>
+						<param>com.codefactory.bookingplatform.shared.config.OpenApiConfig</param>
+						<param>com.codefactory.bookingplatform.shared.config.ClockConfig</param>
+						<param>com.codefactory.bookingplatform.shared.persistence.JpaAuditingConfig</param>
+					</excludedClasses>
+					<!--
+					  Los tests de integracion quedan fuera de la EJECUCION: levantan
+					  Postgres con Testcontainers y arrancar un contenedor por mutante
+					  no es viable. Consecuencia a tener presente al leer el informe:
+					  un mutante que solo matarian los ITs aparece como superviviente.
+					-->
+					<excludedTestClasses>
+						<param>com.codefactory.bookingplatform.*IT</param>
+					</excludedTestClasses>
+					<mutators>
+						<mutator>ALL</mutator>
+					</mutators>
+					<outputFormats>
+						<param>HTML</param>
+						<param>XML</param>
+					</outputFormats>
+					<timestampedReports>false</timestampedReports>
+					<threads>4</threads>
+					<timeoutConstant>8000</timeoutConstant>
+					<!--
+					  Hoy la puntuacion real sobre ese ambito es 98%, y los 17 mutantes
+					  que sobreviven estan analizados uno a uno y son equivalentes. El
+					  umbral se deja en 95 para dejar margen de mantenimiento sin que
+					  un refactor honesto rompa la rama.
+					-->
+					<mutationThreshold>95</mutationThreshold>
+				</configuration>
 			</plugin>
 		</plugins>
 	</build>

src/main/java/com/codefactory/bookingplatform/auth/application/PasswordRecoveryUseCase.java

@@ -3,6 +3,8 @@
 import com.codefactory.bookingplatform.auth.domain.model.UpstreamAuthError;
 import com.codefactory.bookingplatform.auth.domain.model.UpstreamAuthException;
 import com.codefactory.bookingplatform.auth.domain.port.IdentityProviderPort;
+import com.codefactory.bookingplatform.shared.error.BusinessException;
+import com.codefactory.bookingplatform.shared.error.ErrorCode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Service;
@@ -30,7 +32,20 @@ public void requestRecovery(String email) {
                 log.debug("Recovery requested for unknown email; ignored to avoid user enumeration");
                 return;
             }
-            throw ex;
+            throw mapUpstream(ex);
         }
     }
+
+    /**
+     * Translates the upstream failure the same way {@code LoginUseCase} and
+     * {@code LogoutUseCase} do. Without it the raw {@link UpstreamAuthException} reached
+     * the generic handler and the caller got a 500: on a provider rate limit the answer
+     * changed from 202 to 500, which is itself an enumeration signal.
+     */
+    private BusinessException mapUpstream(UpstreamAuthException ex) {
+        if (ex.error() == UpstreamAuthError.RATE_LIMITED) {
+            return new BusinessException(ErrorCode.RATE_LIMITED);
+        }
+        return new BusinessException(ErrorCode.UPSTREAM_AUTH_ERROR, ex.getMessage());
+    }
 }

src/main/java/com/codefactory/bookingplatform/auth/application/PasswordResetUseCase.java

@@ -36,7 +36,18 @@ public void resetPassword(String tokenHash, String newPassword) {
             if (ex.error() == UpstreamAuthError.TOKEN_INVALID || ex.error() == UpstreamAuthError.TOKEN_EXPIRED) {
                 throw new BusinessException(ErrorCode.VERIFICATION_TOKEN_INVALID);
             }
-            throw ex;
+            throw mapUpstream(ex);
         }
     }
+
+    /**
+     * Same translation as {@code LoginUseCase} and {@code LogoutUseCase}: a raw
+     * {@link UpstreamAuthException} has no handler and would reach the caller as a 500.
+     */
+    private BusinessException mapUpstream(UpstreamAuthException ex) {
+        if (ex.error() == UpstreamAuthError.RATE_LIMITED) {
+            return new BusinessException(ErrorCode.RATE_LIMITED);
+        }
+        return new BusinessException(ErrorCode.UPSTREAM_AUTH_ERROR, ex.getMessage());
+    }
 }

src/main/java/com/codefactory/bookingplatform/auth/domain/model/UpstreamAuthException.java

@@ -2,6 +2,8 @@
 
 public class UpstreamAuthException extends RuntimeException {
 
+    private static final long serialVersionUID = 1L;
+
     private final UpstreamAuthError error;
 
     public UpstreamAuthException(UpstreamAuthError error, String message) {

src/main/java/com/codefactory/bookingplatform/auth/infrastructure/supabase/GoTrueClient.java

@@ -30,6 +30,9 @@ public class GoTrueClient implements IdentityProviderPort {
 
     private static final Logger log = LoggerFactory.getLogger(GoTrueClient.class);
 
+    /** Error-mapping context of the /verify endpoint, shared by email confirmation and password reset. */
+    private static final String VERIFY_CONTEXT = "verify";
+
     private final RestClient restClient;
     private final SupabaseProperties properties;
 
@@ -56,7 +59,7 @@ public UUID createUser(String email, String password, AppRole role) {
                     .body(body)
                     .retrieve()
                     .body(Map.class);
-            return UUID.fromString(String.valueOf(requireField(response, "id")));
+            return requireUuid(response, "id");
         } catch (RestClientResponseException ex) {
             throw mapError(ex, "createUser");
         } catch (ResourceAccessException ex) {
@@ -94,7 +97,7 @@ public AuthTokens requestPasswordToken(String email, String password) {
                     String.valueOf(requireField(response, "access_token")),
                     String.valueOf(requireField(response, "refresh_token")),
                     String.valueOf(response.getOrDefault("token_type", "bearer")),
-                    Long.parseLong(String.valueOf(response.getOrDefault("expires_in", "3600"))));
+                    requireExpiresIn(response));
         } catch (RestClientResponseException ex) {
             throw mapError(ex, "token");
         } catch (ResourceAccessException ex) {
@@ -116,9 +119,12 @@ public ConfirmedUser verifyEmailToken(String tokenHash) {
         } else {
             user = response;
         }
+        // El correo no lo consume nadie: ConfirmEmailUseCase resuelve el cliente por
+        // userId. Exigirlo con requireField convertiria una respuesta sin ese campo en
+        // un 502 para un valor que se descarta, asi que se deja tolerante a proposito.
         return new ConfirmedUser(
-                UUID.fromString(String.valueOf(requireField(user, "id"))),
-                String.valueOf(user.get("email")));
+                requireUuid(user, "id"),
+                user.get("email") == null ? null : String.valueOf(user.get("email")));
     }
 
     @Override
@@ -199,7 +205,7 @@ private Map<String, Object> verify(String tokenHash, String type, String passwor
                     .body(Map.class);
             return response != null ? response : Map.of();
         } catch (RestClientResponseException ex) {
-            throw mapError(ex, "verify");
+            throw mapError(ex, VERIFY_CONTEXT);
         } catch (ResourceAccessException ex) {
             throw unavailable(ex);
         }
@@ -222,6 +228,35 @@ private Object requireField(Map<String, Object> response, String field) {
         return response.get(field);
     }
 
+    /**
+     * A malformed id is an unusable provider answer, exactly like a missing one:
+     * the raw {@link IllegalArgumentException} from {@code UUID.fromString} would
+     * escape the adapter and surface as a 500 instead of an upstream error.
+     */
+    private UUID requireUuid(Map<String, Object> response, String field) {
+        String raw = String.valueOf(requireField(response, field));
+        try {
+            return UUID.fromString(raw);
+        } catch (IllegalArgumentException ex) {
+            throw new UpstreamAuthException(UpstreamAuthError.UNAVAILABLE,
+                    "Unexpected identity provider response, field is not a valid UUID: " + field, ex);
+        }
+    }
+
+    /**
+     * {@code expires_in} is optional (defaults to one hour), but a present value that
+     * is not a whole number — {@code "never"}, or the perfectly valid JSON number
+     * {@code 3600.0} — must not escape as a raw {@link NumberFormatException}.
+     */
+    private long requireExpiresIn(Map<String, Object> response) {
+        try {
+            return Long.parseLong(String.valueOf(response.getOrDefault("expires_in", "3600")));
+        } catch (NumberFormatException ex) {
+            throw new UpstreamAuthException(UpstreamAuthError.UNAVAILABLE,
+                    "Unexpected identity provider response, field is not a number: expires_in", ex);
+        }
+    }
+
     private UpstreamAuthException unavailable(Exception cause) {
         log.error("Identity provider unreachable: {}", cause.getMessage());
         return new UpstreamAuthException(UpstreamAuthError.UNAVAILABLE,
@@ -242,20 +277,25 @@ private UpstreamAuthException mapError(RestClientResponseException ex, String co
         if (body.contains("already exists") || body.contains("user_exists") || body.contains("email_exists")) {
             return new UpstreamAuthException(UpstreamAuthError.USER_ALREADY_EXISTS, "User already exists");
         }
-        if (body.contains("not found") || status == 404) {
-            return new UpstreamAuthException(UpstreamAuthError.USER_NOT_FOUND, "User not found");
-        }
+        // GoTrue answers 404 to an expired or already consumed OTP, so "expired" has to
+        // be checked before the 404 rule: otherwise a stale verification link is reported
+        // as USER_NOT_FOUND and the user reads "usuario no encontrado".
         if (body.contains("expired")) {
             return new UpstreamAuthException(UpstreamAuthError.TOKEN_EXPIRED, "Token has expired");
         }
+        // A bare 404 from /verify is about the one-time token, never about a user; it is
+        // classified below by the verify branch. Every other endpoint keeps the old rule.
+        if (body.contains("not found") || (status == 404 && !VERIFY_CONTEXT.equals(context))) {
+            return new UpstreamAuthException(UpstreamAuthError.USER_NOT_FOUND, "User not found");
+        }
         switch (context) {
             case "token":
                 if (status == 400 || status == 401) {
                     return new UpstreamAuthException(UpstreamAuthError.INVALID_CREDENTIALS, "Invalid login credentials");
                 }
                 break;
-            case "verify":
-                if (status == 400 || status == 403) {
+            case VERIFY_CONTEXT:
+                if (status == 400 || status == 403 || status == 404) {
                     return new UpstreamAuthException(UpstreamAuthError.TOKEN_INVALID, "Invalid or already used token");
                 }
                 break;