refactor(backend): primera ronda de deduplicación, pruebas de caracterización y aislamientos lógicos

backend/app/clients/nominatim_client.py

@@ -7,7 +7,7 @@
 
 class NominatimClient:
     BASE_URL = "https://nominatim.openstreetmap.org/search"
-    USER_AGENT = "SkillStat/1.0 (proyecto academico UTCJ, contacto: eliaslucinoochoamalaga@gmail.com)"
+    USER_AGENT = "SkillStat/1.0 (proyecto academico UTCJ, contacto: 195959137+Ochoa-Stack@users.noreply.github.com)"
 
     # Valid types that represent a real city/town/village entity.
     VALID_TYPES = {"city", "town", "village", "municipality"}
@@ -31,20 +31,29 @@ class NominatimClient:
     }
 
     @classmethod
-    def geocode_city(cls, query: str) -> dict | None:
-        """Geocodes a city name using Nominatim API.
-        Returns a dict with 'name', 'state', 'lat', 'lon' or None if it fails, timeouts,
-        or doesn't meet the confidence threshold (must have state, must be a valid city type)"""
+    def _resolve_query(cls, query: str) -> str:
         import unicodedata
-
-        # Desambiguación de query
         normalized_query = query.strip().lower()
         normalized_query = ''.join(c for c in unicodedata.normalize('NFD', normalized_query) if unicodedata.category(c) != 'Mn')
-
-        # Eliminamos puntos comunes como en "mexico d.f." -> "mexico df" para que coincida con el diccionario
         normalized_query = normalized_query.replace(".", "")
-
-        actual_query = cls.QUERY_DISAMBIGUATION.get(normalized_query, query)
+        return cls.QUERY_DISAMBIGUATION.get(normalized_query, query)
+
+    @classmethod
+    def _is_valid_place_type(cls, result: dict) -> bool:
+        place_type = result.get("type", "").lower()
+        place_class = result.get("class", "").lower()
+        addresstype = result.get("addresstype", "").lower()
+        return place_type in cls.VALID_TYPES or place_class in cls.VALID_TYPES or addresstype in cls.VALID_TYPES
+
+    @classmethod
+    def _extract_city_name(cls, result: dict) -> str | None:
+        address = result.get("address", {})
+        return address.get("city") or address.get("town") or address.get("village") or address.get("municipality") or result.get("name")
+
+    @classmethod
+    def geocode_city(cls, query: str) -> dict | None:
+        """ Geocodes a city name using Nominatim API. Returns a dict with 'name', 'state', 'lat', 'lon' or None if it fails, timeouts, or doesn't meet the confidence threshold (must have state, must be a valid city type) """
+        actual_query = cls._resolve_query(query)
 
         # Sleep to respect Nominatim's strict 1 req/sec limit
         time.sleep(1.1)
@@ -70,11 +79,7 @@ def geocode_city(cls, query: str) -> dict | None:
 
             result = data[0]
 
-            place_type = result.get("type", "").lower()
-            place_class = result.get("class", "").lower()
-            addresstype = result.get("addresstype", "").lower()
-
-            if place_type not in cls.VALID_TYPES and place_class not in cls.VALID_TYPES and addresstype not in cls.VALID_TYPES:
+            if not cls._is_valid_place_type(result):
                 return None
 
             address = result.get("address", {})
@@ -83,7 +88,7 @@ def geocode_city(cls, query: str) -> dict | None:
             if not state:
                 return None
 
-            name = address.get("city") or address.get("town") or address.get("village") or address.get("municipality") or result.get("name")
+            name = cls._extract_city_name(result)
 
             if not name:
                 return None

backend/app/controllers/admin_bp.py

@@ -183,7 +183,7 @@ def update_user_role(user_id):
     new_role = payload["role"]
     # Si el actor se esta auto-modificando y la operacion lo saca de ADMIN, protegemos contra dejar el sistema sin ningun admin activo.
     if target.id == actor_id and target.role == "ADMIN" and new_role != "ADMIN":
-        if UserRepository.count_active_admins_for_update() <= 1:
+        if UserRepository.is_last_active_admin(actor_id):
             return error_response(
                 code="LAST_ADMIN_PROTECTED",
                 message="No puedes quitarte el rol de ADMIN: eres el unico administrador activo.",
@@ -228,7 +228,7 @@ def update_user_status(user_id):
     new_status = payload["is_active"]
     # Misma proteccion de ultimo-admin, aplicada a desactivacion en vez de cambio de rol.
     if target.id == actor_id and target.role == "ADMIN" and new_status is False:
-        if UserRepository.count_active_admins_for_update() <= 1:
+        if UserRepository.is_last_active_admin(actor_id):
             return error_response(
                 code="LAST_ADMIN_PROTECTED",
                 message="No puedes desactivar tu cuenta: eres el unico administrador activo.",
@@ -250,15 +250,11 @@ def update_user_status(user_id):
 
 @admin_bp.route("/trigger-pipeline", methods=["POST"])
 def trigger_pipeline():
-    """Dispara el pipeline diario (snapshots + evaluación de alertas) bajo demanda.
-    Autenticado exclusivamente via el header X-Pipeline-Trigger-Key, comparado
-    con PIPELINE_TRIGGER_SECRET usando tiempo constante para evitar timing attacks.
-    Diseñado para ser invocado por GitHub Actions, sin sesión de usuario."""
+    """Dispara el pipeline diario bajo demanda. Autenticado exclusivamente via el header X-Pipeline-Trigger-Key, comparado con PIPELINE_TRIGGER_SECRET usando tiempo constante para evitar timing attacks. Diseñado para ser invocado por GitHub Actions, sin sesión de usuario."""
     provided_key = request.headers.get("X-Pipeline-Trigger-Key", "")
     expected_key = current_app.config.get("PIPELINE_TRIGGER_SECRET", "")
 
-    # hmac.compare_digest previene timing attacks: el tiempo de comparación no
-    # varía según cuántos caracteres coincidan, a diferencia del operador ==.
+    # hmac.compare_digest previene timing attacks, el tiempo de comparación no varía según cuántos caracteres coincidan, a diferencia del operador ==.
     if not hmac.compare_digest(provided_key, expected_key):
         return error_response(
             code="UNAUTHORIZED",

backend/app/controllers/auth_bp.py

@@ -114,6 +114,7 @@ def get_profile():
 
 
 @auth_bp.route("/google", methods=["POST"])
+@limiter.limit("10 per 15 minutes")
 def google_login():
     data = request.get_json() or {}
     credential = data.get("credential")

backend/app/repositories/city_repository.py

@@ -7,6 +7,13 @@
 class CityRepository(BaseRepository):
     model = City
 
+    @classmethod
+    def _normalize(cls, text: str) -> str:
+        return ''.join(
+            c for c in unicodedata.normalize('NFD', text.strip().lower())
+            if unicodedata.category(c) != 'Mn'
+        )
+
     @classmethod
     def get_by_name(cls, name: str):
         return db.session.execute(
@@ -19,14 +26,12 @@ def get_or_create_city(cls, raw_location: str) -> tuple[City | None, bool]:
             return None, False
 
         # lowercase, sin acentos y trim (Normaliza)
-        normalized = raw_location.strip().lower()
-        normalized = ''.join(c for c in unicodedata.normalize('NFD', normalized) if unicodedata.category(c) != 'Mn')
+        normalized = cls._normalize(raw_location)
 
         # Búsqueda exhaustiva comparando el nombre normalizado
         all_cities = db.session.execute(db.select(City)).scalars().all()
         for city in all_cities:
-            city_norm = city.name.strip().lower()
-            city_norm = ''.join(c for c in unicodedata.normalize('NFD', city_norm) if unicodedata.category(c) != 'Mn')
+            city_norm = cls._normalize(city.name)
             if city_norm == normalized:
                 return city, False
 
@@ -37,12 +42,10 @@ def get_or_create_city(cls, raw_location: str) -> tuple[City | None, bool]:
 
         # Nominatim puede resolver un alias (ej: "Distrito Federal") a un nombre real (ej: "Ciudad de México"). Revisamos si ese nombre real ya existe en BD para evitar IntegrityError secuencial
         resolved_name = geo_data["name"]
-        resolved_norm = resolved_name.strip().lower()
-        resolved_norm = ''.join(c for c in unicodedata.normalize('NFD', resolved_norm) if unicodedata.category(c) != 'Mn')
+        resolved_norm = cls._normalize(resolved_name)
 
         for city in all_cities:
-            city_norm = city.name.strip().lower()
-            city_norm = ''.join(c for c in unicodedata.normalize('NFD', city_norm) if unicodedata.category(c) != 'Mn')
+            city_norm = cls._normalize(city.name)
             if city_norm == resolved_norm:
                 return city, False
 

backend/app/repositories/user_repository.py

@@ -9,22 +9,26 @@
 
 class UserRepository:
     # Encapsula el acceso a datos para la entidad User. Aísla las consultas SQLAlchemy de la lógica de negocio.
-
+    
     @classmethod
-    def create(cls, user_data: dict) -> User:
-        user = User(**user_data)
-        db.session.add(user)
+    def _commit_or_rollback(cls, user: User, action: str) -> User:
         try:
             db.session.commit()
             return user
         except IntegrityError as e:
             db.session.rollback()
-            logger.warning("Violacion de integridad al crear User: %s", str(e))
-            raise ConflictError("No se pudo crear el usuario: conflicto de integridad de datos.")
+            logger.warning(f"Violacion de integridad al {action} User: {e}")
+            raise ConflictError(f"No se pudo {action} el usuario: conflicto de integridad de datos.")
         except Exception as e:
             db.session.rollback()
-            logger.error("Fallo inesperado al crear User: %s", str(e))
-            raise AppError("Error interno al crear el usuario.", code="DATABASE_ERROR", status_code=500)
+            logger.error(f"Fallo inesperado al {action} User: {e}")
+            raise AppError(f"Error interno al {action} el usuario.", code="DATABASE_ERROR", status_code=500)
+
+    @classmethod
+    def create(cls, user_data: dict) -> User:
+        user = User(**user_data)
+        db.session.add(user)
+        return cls._commit_or_rollback(user, "crear")
 
     @classmethod
     def get_by_id(cls, user_id: int) -> User:
@@ -55,34 +59,17 @@ def get_paginated(cls, page: int = 1, per_page: int = 20):
     def save(cls, user: User) -> User:
         # Persiste cambios en una entidad ya existente, como el reseteo de password_hash; no crea un nuevo registro, solo hace commit.
         db.session.add(user)
-        try:
-            db.session.commit()
-            return user
-        except IntegrityError as e:
-            db.session.rollback()
-            logger.warning("Violacion de integridad al guardar User: %s", str(e))
-            raise ConflictError("No se pudo guardar el usuario: conflicto de integridad de datos.")
-        except Exception as e:
-            db.session.rollback()
-            logger.error("Fallo inesperado al guardar User: %s", str(e))
-            raise AppError("Error interno al guardar el usuario.", code="DATABASE_ERROR", status_code=500)
-
-    @classmethod
-    def count_active_admins(cls) -> int:
-        # Cuenta administradores activos para proteger contra que una operación deje al sistema sin ningún ADMIN capaz de operar el panel.
-        return db.session.execute(
-            db.select(db.func.count()).select_from(User).filter_by(
-                role="ADMIN", is_active=True
-            )
-        ).scalar_one()
+        return cls._commit_or_rollback(user, "guardar")
 
     @classmethod
     def count_active_admins_for_update(cls) -> int:
-        # Version con lock de fila explicito (SELECT ... FOR UPDATE) para proteger contra condiciones de carrera reales: dos requests concurrentes intentando degradar/desactivar a los dos ultimos administradores activos al mismo tiempo. El lock se retiene hasta el commit() de la transaccion actual (el que ya ocurre dentro de save()), forzando que la segunda request espere a que la primera termine antes de leer un conteo actualizado.
-        # Nota de implementacion: with_for_update() no es compatible directamente con func.count() como subquery en SQLAlchemy 2.x, por lo que se aplica FOR UPDATE sobre la query de filas y se cuenta el resultado en Python (equivalente semanticamente).
         rows = db.session.execute(
             db.select(User.id).filter_by(
                 role="ADMIN", is_active=True
             ).with_for_update()
         ).all()
         return len(rows)
+
+    @classmethod
+    def is_last_active_admin(cls, user_id: int) -> bool:
+        return cls.count_active_admins_for_update() <= 1

backend/app/services/alerts_service.py

@@ -1,11 +1,34 @@
+import logging
 from app.repositories.alert_repository import AlertRepository
 from app.repositories.trend_snapshot_repository import TrendSnapshotRepository
 from app.repositories.user_repository import UserRepository
 from app.repositories.skill_repository import SkillRepository
 from app.services.email_service import send_alert_email
 from app.utils.errors import AppError
 
+logger = logging.getLogger(__name__)
+
 class AlertsService:
+    @classmethod
+    def _build_notification_content(cls, alert, skill, latest_trend):
+        if alert.alert_type == "ABSOLUTE":
+            subject = f"Alerta SkillStat: {skill.name} ha superado tu umbral"
+            html_content = f"""
+            <h3>Alerta de Mercado Laboral</h3>
+            <p>Hola, tu alerta configurada para <strong>{skill.name}</strong> ha sido activada.</p>
+            <p>El mercado actual registra <strong>{latest_trend.demand_count}</strong> vacantes activas, superando tu umbral de {alert.threshold_value}.</p>
+            <p><a href="https://skillstat-ss.onrender.com/views/panorama.html">Ver en el Panorama</a></p>
+            """
+        else:
+            subject = f"Alerta SkillStat: {skill.name} está en tendencia de crecimiento"
+            html_content = f"""
+            <h3>Alerta de Mercado Laboral</h3>
+            <p>Hola, tu alerta de tendencia configurada para <strong>{skill.name}</strong> ha sido activada.</p>
+            <p>Esta habilidad ha crecido <strong>{latest_trend.growth_rate}%</strong> en la última semana, superando tu umbral de {alert.threshold_percentage}%.</p>
+            <p><a href="https://skillstat-ss.onrender.com/views/panorama.html">Ver en el Panorama</a></p>
+            """
+        return subject, html_content
+
     @classmethod
     def evaluate_and_notify(cls) -> int:
         active_alerts = AlertRepository.get_active()
@@ -21,7 +44,7 @@ def evaluate_and_notify(cls) -> int:
                 if latest_trend.demand_count is not None and latest_trend.demand_count >= alert.threshold_value:
                     triggered = True
             elif alert.alert_type == "TREND":
-                # Sin historial de 7 dias, growth_rate es None -- no evaluamos, no notificamos. Ausencia de dato no es lo mismo que "no se cumplio".
+                # Sin historial de 7 dias, growth_rate es None, no evaluamos, no notificamos. Ausencia de dato no es lo mismo que "no se cumplio".
                 if latest_trend.growth_rate is not None and latest_trend.growth_rate >= alert.threshold_percentage:
                     triggered = True
             if not triggered:
@@ -30,25 +53,16 @@ def evaluate_and_notify(cls) -> int:
             skill = SkillRepository.get_by_id(alert.skill_id)
             if not user or not skill:
                 continue
-            if alert.alert_type == "ABSOLUTE":
-                subject = f"Alerta SkillStat: {skill.name} ha superado tu umbral"
-                html_content = f"""
-                <h3>Alerta de Mercado Laboral</h3>
-                <p>Hola, tu alerta configurada para <strong>{skill.name}</strong> ha sido activada.</p>
-                <p>El mercado actual registra <strong>{latest_trend.demand_count}</strong> vacantes activas, superando tu umbral de {alert.threshold_value}.</p>
-                <p><a href="https://skillstat.mx/panorama">Ver en el Panorama</a></p>
-                """
-            else:
-                subject = f"Alerta SkillStat: {skill.name} está en tendencia de crecimiento"
-                html_content = f"""
-                <h3>Alerta de Mercado Laboral</h3>
-                <p>Hola, tu alerta de tendencia configurada para <strong>{skill.name}</strong> ha sido activada.</p>
-                <p>Esta habilidad ha crecido <strong>{latest_trend.growth_rate}%</strong> en la última semana, superando tu umbral de {alert.threshold_percentage}%.</p>
-                <p><a href="https://skillstat.mx/panorama">Ver en el Panorama</a></p>
-                """
+
+            subject, html_content = cls._build_notification_content(alert, skill, latest_trend)
+
             try:
                 send_alert_email(user.email, subject, html_content)
                 notifications_sent += 1
-            except AppError:
+            except AppError as e:
+                logger.warning(
+                    "Fallo al enviar correo de alerta: alert_id=%s user_id=%s. Error: %s",
+                    alert.id, alert.user_id, str(e)
+                )
                 continue
         return notifications_sent