A lot of changes

This commit is contained in:
2026-05-14 14:32:19 +02:00
parent 7160266763
commit f2febb898e
43 changed files with 2941 additions and 444 deletions
+165
View File
@@ -0,0 +1,165 @@
# MusifyBackend — Resumen de cambios (2026-05-14)
Este documento resume **todos los cambios aplicados hasta ahora** en la rama `master` durante esta iteración.
> Nota: El proyecto compila en `Release` al cierre de estos cambios.
---
## 1) Mensajería robusta: MassTransit + EF Outbox
**Objetivo**: hacer la publicación de eventos confiable y transaccional (DB + mensajes) y tolerar redelivery en consumidores.
### Cambios principales
- Se integró **MassTransit EntityFramework Outbox** con PostgreSQL (`UsePostgres` + `UseBusOutbox`).
- Se añadieron entidades/tablas de **Inbox/Outbox** en el `DbContext`.
- Se generó y añadió una **migración** para las tablas de outbox/inbox.
- Se ajustó la configuración de endpoints para aplicar:
- **Retry** (exponential backoff)
- **Idempotencia** en consumo usando `UseEntityFrameworkOutbox<Database>()`
- Se ajustó el orden de operaciones para el patrón outbox: **publicar evento y luego `SaveChanges`**.
### Archivos involucrados (principalmente)
- `Infrastructure/MassTransit/DependencyInjection/MassTransitDependencyInjection.cs`
- `Infrastructure/MassTransit/DependencyInjection/MassTransitDependencyInjection.Endpoints.cs`
- `Infrastructure/Persistence/Database.cs`
- `Infrastructure/Persistence/Migrations/20260514095113_AddMassTransitOutbox.cs`
- `Application/Abstractions/Infrastructure/IEventBus.cs`
- `Infrastructure/MassTransit/EventBus/MassTransitEventBus.cs`
---
## 2) Workflows: Courier RoutingSlip para imágenes y audio
**Objetivo**: mantener el pipeline asíncrono (descarga, resize/transcode, upload, update de entidades) con reintentos e idempotencia.
### Fix importante en workflow de imágenes
- Se corrigió el nombre de actividad del paso de **Upload del tamaño Medium** (antes duplicaba el nombre de `ResizeMedium`).
Archivos:
- `Infrastructure/MassTransit/RoutingSlip/Builders/PictureWorkflowRoutingSlipBuilder.cs`
- `Infrastructure/MassTransit/RoutingSlip/ActivityNames.cs`
### Fix importante en compensación de uploads
- `UploadFileToBucketActivity` ahora registra correctamente el **object key completo** (ruta + filename) para que la compensación borre el objeto correcto.
Archivo:
- `Infrastructure/MassTransit/Activities/Files/UploadFileToBucketActivity.cs`
---
## 3) Uploads (Opción B): Presigned PUT URL + Create/Update con nombres
**Objetivo**: separar la subida de binarios (S3) de la operación de dominio (Create/Update), evitando rollbacks complejos y haciendo el flujo más robusto.
### Endpoint/Commands añadidos (request presigned URL)
- Playlist picture:
- `Application/PlayLists/Commands/RequestPlayListPictureUploadCommand.cs`
- `Application/PlayLists/Handlers/RequestPlayListPictureUploadCommandHandler.cs`
- `Application/PlayLists/Responses/PlayListPictureUploadResponse.cs`
- Track picture + audio:
- `Application/Tracks/Commands/RequestTrackUploadUrlsCommand.cs`
- `Application/Tracks/Handler/RequestTrackUploadUrlsCommandHandler.cs`
- `Application/Tracks/Responses/TrackUploadUrlsResponse.cs`
### Storage: presigned PUT
- Se añadió soporte para generar **presigned PUT URLs**:
- `Application/Abstractions/Infrastructure/IStorageService.cs`
- `Infrastructure/Services/StorageService.cs` (`GetUploadUrlAsync`)
---
## 4) Limpieza: eliminar V1 y normalizar nombres (V2 → “normal”)
**Objetivo**: eliminar comandos/handlers “V1” (subida directa por stream/multipart) y dejar un único set de comandos “normales” con el contrato final.
### Resultado
- `CreatePlayListCommand`, `UpdatePlayListCommand` y `CreateTrackCommand` quedaron con el contrato nuevo (nombres/metadata, no streams).
- Se eliminaron archivos `*V2*` ya innecesarios.
Archivos clave:
- `Application/PlayLists/Commands/CreatePlayListCommand.cs`
- `Application/PlayLists/Handlers/CreatePlayListCommandHandler.cs`
- `Application/PlayLists/Commands/UpdatePlayListCommand.cs`
- `Application/PlayLists/Handlers/UpdatePlayListCommandHandler.cs`
- `Application/Tracks/Commands/CreateTrackCommand.cs`
- `Application/Tracks/Handler/CreateTrackCommandHandler.cs`
---
## 5) Prefijo de seguridad: `uploads/{userId}/...` para originales
**Objetivo**: reducir superficie (y colisiones) haciendo que los objetos originales queden naturalmente “namespaced” por usuario.
### Cambios
- Las **keys de originales** (playlist original picture, track original picture, track original audio) ahora se generan bajo:
- `uploads/{userId}/...`
- Se añadieron overloads de rutas para construir keys con `userId`.
- Los handlers de request presigned URL ahora generan keys usando el prefijo con `userId`.
- Los eventos de workflows (sourceKey) se publican con la key prefijada.
- El borrado de playlist borra la key original prefijada.
- El borrado de track (`DeleteTrackActivity`) borra originales usando el `userId` presente en `DeleteTrackArguments`.
Archivos:
- `Application/Configuration/PlayListConfiguration.cs`
- `Application/Configuration/TrackConfiguration.cs`
- `Application/PlayLists/Handlers/RequestPlayListPictureUploadCommandHandler.cs`
- `Application/Tracks/Handler/RequestTrackUploadUrlsCommandHandler.cs`
- `Application/PlayLists/Handlers/CreatePlayListCommandHandler.cs`
- `Application/PlayLists/Handlers/UpdatePlayListCommandHandler.cs`
- `Application/Tracks/Handler/CreateTrackCommandHandler.cs`
- `Application/PlayLists/Handlers/DeletePlayListCommandHandler.cs`
- `Infrastructure/MassTransit/Activities/Tracks/DeleteTrackActivity.cs`
---
## 6) Normalización de separadores en keys S3 (evitar `\\` en Windows)
**Objetivo**: evitar que `Path.Combine` introduzca separadores `\\` en keys S3 (lo cual es incorrecto/inesperado en S3, y rompe interoperabilidad).
### Cambios
- Se reemplazaron combinaciones de keys que usaban `Path.Combine` por joins con `/` en puntos críticos.
Archivos:
- `Application/Configuration/PlayListConfiguration.cs`
- `Application/Configuration/TrackConfiguration.cs`
- `Infrastructure/MassTransit/Activities/Files/UploadFileToBucketActivity.cs`
- `Infrastructure/Services/StorageService.cs`
---
## 7) Validaciones en Application eliminadas (delegadas a API)
**Objetivo**: evitar duplicar validaciones de formato (extensiones/strings) si la capa API ya se encarga.
### Cambios
- Se eliminó el helper de validación creado inicialmente y los checks en handlers.
Archivos:
- Eliminado: `Application/Validation/UploadObjectNameValidator.cs`
- Ajustados:
- `Application/PlayLists/Handlers/CreatePlayListCommandHandler.cs`
- `Application/PlayLists/Handlers/UpdatePlayListCommandHandler.cs`
- `Application/Tracks/Handler/CreateTrackCommandHandler.cs`
---
## 8) Documento de mitigaciones (presigned uploads)
Se añadió documentación de hardening/mitigaciones contra abuso/spam en endpoints presigned.
Archivo:
- `docs/presigned-upload-hardening.md`
---
## 9) Notas de compatibilidad / impacto
- Este cambio **rompe compatibilidad** con la API anterior si existían endpoints que aceptaban multipart/streams para Create/Update (ahora se espera el flujo presigned y luego Create/Update con nombres).
- Los objetos originales ahora viven bajo `uploads/{userId}/...`. Objetos antiguos fuera de ese prefijo pueden quedar huérfanos si ya existían de antes.
---
## 10) Estado final
- `dotnet build MusifyBackend.slnx -c Release` ✅
+175
View File
@@ -0,0 +1,175 @@
# Plan — Unificar Deletes con Workflows (RoutingSlip)
## Contexto
Actualmente hay dos problemas de diseño en los deletes:
**PlayLists:** el borrado dispara múltiples `RemoveFileEvent` sueltos desde `DeletePlayListCommandHandler` (4 llamadas separadas para original/small/medium/large) y luego elimina la entidad en DB. Sin workflow, sin trazabilidad.
**Tracks:** ya existe un workflow (`DeleteTrackEvent` → consumer → RoutingSlip → `DeleteTrackActivity`), pero `DeleteTrackActivity` hace **demasiado en un único paso**: carga la entidad, borra todos los archivos de S3 (pictures + audios), y elimina de DB. Esto rompe el principio de actividades granulares y reutilizables, hace que el slip no sea "seguible" paso a paso, y dificulta los reintentos parciales.
La idea es **homogeneizar y corregir ambos**: que los deletes de Track y PlayList sigan un RoutingSlip con actividades pequeñas, ordenadas, repetibles e independientes.
---
## Objetivos
- **Un único camino** de borrado por aggregate (Track / PlayList), basado en **RoutingSlip con actividades granulares**.
- El handler de comando hace lo mínimo: validar ownership/reglas, cambiar estado a `Removing`, y publicar el evento de borrado (outbox).
- El workflow ejecuta pasos **ordenados, reutilizables y reiniciables** individualmente.
- **Reutilizar** `RemoveFileFromBucketActivity` (ya existente) para cada archivo a borrar, en lugar de lógica ad-hoc dentro de una activity monolítica.
- Eliminar `RemoveFileEvent`, `RemoveFileConsumer` y su registration si no tienen otros usos.
No objetivos (por ahora):
- No introducir observabilidad extra (tracing/telemetría) fuera de lo ya existente.
---
## Problemas del enfoque actual
### PlayLists
- El handler implementa orquestación (decide qué archivos borrar y cómo) → acoplamiento alto.
- Publica múltiples `RemoveFileEvent` sueltos, sin agruparlos en un proceso único.
- No hay log de RoutingSlip que permita seguir el progreso/estado.
### Tracks ← **nuevo problema identificado**
- `DeleteTrackActivity` es monolítica: carga entidad, borra pictures, borra audios, elimina de DB, todo en un solo `Execute`.
- No reutiliza `RemoveFileFromBucketActivity` ya existente.
- Si falla a mitad, no hay granularidad de reintento (no se sabe qué paso falló).
- El RoutingSlip tiene un único nodo → no es realmente un "slip" con pasos.
---
## Diseño propuesto
### Actividades compartidas / reutilizables
Ya existe (y se mantiene):
- `RemoveFileFromBucketActivity` — borra un único objeto de S3. Se usará N veces en cada slip.
Se añaden actividades de DB compartibles por aggregate:
- `MarkAsRemovingActivity<TArgs>` — cambia estado de la entidad a `Removing` y persiste. (O una por aggregate si la lógica difiere.)
- `DeleteTrackFromDbActivity` — elimina el Track de EF (cascade elimina relaciones).
- `DeletePlayListFromDbActivity` — elimina la PlayList de EF (cascade elimina relaciones).
### Estructura del RoutingSlip para Track
```
DeleteTrackEvent
└─► DeleteTrackConsumer
└─► DeleteTrackRoutingSlipBuilder
├─ MarkTrackAsRemovingActivity (cambia estado → Removing, persiste)
├─ RemoveFileFromBucketActivity (original picture)
├─ RemoveFileFromBucketActivity (small picture) ← solo si IsPicturesProcessed
├─ RemoveFileFromBucketActivity (medium picture) ← solo si IsPicturesProcessed
├─ RemoveFileFromBucketActivity (large picture) ← solo si IsPicturesProcessed
├─ RemoveFileFromBucketActivity (original audio)
├─ RemoveFileFromBucketActivity (processed audio) ← solo si IsAudioProcessed
└─ DeleteTrackFromDbActivity (Remove() + SaveChanges)
```
> El builder carga la entidad para construir el plan (keys condicionales) antes de arrancar el slip. Las actividades de S3 son idempotentes (NotFound → log Information + continue).
### Estructura del RoutingSlip para PlayList
```
DeletePlayListEvent
└─► DeletePlayListConsumer
└─► DeletePlayListRoutingSlipBuilder
├─ MarkPlayListAsRemovingActivity (cambia estado → Removing, persiste)
├─ RemoveFileFromBucketActivity (original — uploads/{userId}/...) ← solo si no es preset
├─ RemoveFileFromBucketActivity (small) ← solo si no es preset
├─ RemoveFileFromBucketActivity (medium) ← solo si no es preset
├─ RemoveFileFromBucketActivity (large) ← solo si no es preset
└─ DeletePlayListFromDbActivity (Remove() + SaveChanges, cascade elimina PlayListHasTrack)
```
### Handler de comando (ambos aggregates)
```csharp
// DeletePlayListCommandHandler — ANTES
await PublishRemoveFileEventsAsync(...); // 4 eventos sueltos
database.PlayLists.Remove(playlist);
await database.SaveChangesAsync();
// DESPUÉS
await eventBus.PublishAsync(new DeletePlayListEvent(command.PlayListId, command.UserId));
// Nada más. El workflow se encarga de todo.
```
```csharp
// DeleteTrackCommandHandler — ANTES
// (ya publicaba DeleteTrackEvent, se mantiene igual)
await eventBus.PublishAsync(new DeleteTrackEvent(command.TrackId, command.UserId));
// La diferencia es que ahora el slip tiene pasos granulares, no una activity monolítica.
```
---
## Reglas de negocio a preservar
- Ownership se valida en el handler del comando (igual que ahora), antes de publicar el evento.
- PlayList: solo borrar original/sizes si no son presets.
- PlayList: el original está bajo `uploads/{userId}/...` (cambio reciente).
- Track: respetar estados de procesamiento — si audio está `Pending/Processing`, el handler ya bloquea el delete antes de llegar al workflow.
- EF cascade elimina `PlayListHasTrack` al borrar la PlayList → no hace falta activity explícita para ello.
---
## Idempotencia / reintentos
- RoutingSlip + reintentos de endpoint ya configurados en MassTransit.
- Operaciones S3: idempotentes por diseño. Si el key no existe → log `Information` y `Completed()` (no throw).
- DB: si la entidad ya no existe cuando llega `DeleteTrackFromDbActivity` / `DeletePlayListFromDbActivity`, el workflow se considera `Completed` (misma lógica que ahora).
- Estado `Removing`: permite identificar entidades en proceso de borrado si el sistema se reinicia a mitad del slip.
---
## Plan de implementación (paso a paso)
### 1. Inventario
- Confirmar todos los usos de `RemoveFileEvent` / `RemoveFileConsumer` en la solución.
- Si no tienen otros usos fuera de playlist delete → marcarlos para eliminación en el paso 7.
### 2. Refactorizar Track delete ← **nuevo**
- Crear `MarkTrackAsRemovingActivity` (execute-activity): carga Track, setea estado `Removing`, persiste.
- Crear `DeleteTrackFromDbActivity` (execute-activity): carga Track, llama `Remove()`, persiste.
- Actualizar `DeleteTrackRoutingSlipBuilder`:
- Carga el Track para calcular el plan (keys condicionales según `IsPicturesProcessed`, `IsAudioProcessed`).
- Encadena: `MarkTrackAsRemoving` → N × `RemoveFileFromBucket` → `DeleteTrackFromDb`.
- Eliminar `DeleteTrackActivity` monolítica (ya no se usa).
- Ajustar registration DI de MassTransit (quitar activity vieja, registrar las nuevas).
### 3. Crear PlayList delete: evento + consumer + builder
- Añadir `DeletePlayListEvent(PlayListId, UserId)` en `Application/Events`.
- Crear `DeletePlayListConsumer` en Infrastructure: recibe el evento y ejecuta el builder.
- Crear `DeletePlayListRoutingSlipBuilder`:
- Carga la PlayList para calcular el plan (keys condicionales por presets, ruta de original).
- Encadena: `MarkPlayListAsRemoving` → N × `RemoveFileFromBucket` → `DeletePlayListFromDb`.
### 4. Crear actividades de PlayList
- `MarkPlayListAsRemovingActivity` (execute-activity): carga PlayList, setea `Removing`, persiste.
- `DeletePlayListFromDbActivity` (execute-activity): carga PlayList, llama `Remove()`, persiste (cascade).
### 5. Actualizar `RemoveFileFromBucketActivity` para NotFound
- Si el SDK lanza excepción por key inexistente → capturar, log `Information("Key {Key} not found, skipping")`, retornar `Completed()`.
- Si es otro error → log `Error` + throw (reintento normal).
### 6. Cambiar `DeletePlayListCommandHandler`
- Reemplazar `PublishRemoveFileEventsAsync` por `eventBus.PublishAsync(new DeletePlayListEvent(...))`.
- No publicar más `RemoveFileEvent` para playlist delete.
### 7. Ajustar DI / registration MassTransit
- Registrar: `DeletePlayListConsumer`, `MarkPlayListAsRemovingActivity`, `DeletePlayListFromDbActivity`.
- Registrar: `MarkTrackAsRemovingActivity`, `DeleteTrackFromDbActivity`.
- Desregistrar: `DeleteTrackActivity` (monolítica), y si procede `RemoveFileConsumer`.
### 8. Eliminar código muerto
- `DeleteTrackActivity` monolítica.
- `RemoveFileEvent` y `RemoveFileConsumer` (si no tienen otros usos).
- Endpoints y registrations asociados.
### 9. Build y verificación
- `dotnet build -c Release` sin warnings de referencias muertas.
- Smoke test manual: borrar un Track y una PlayList, verificar logs de cada activity en orden.
+95
View File
@@ -0,0 +1,95 @@
# Hardening: abuso de pre-signed uploads (Opción B)
## Contexto
Con la Opción B el backend genera URLs pre-firmadas (pre-signed) para que el cliente suba ficheros directamente a S3, y luego el backend crea/actualiza entidades en DB referenciando `pictureName/audioName`.
Esto mejora latencia y descarga la API, pero abre un vector de abuso: un usuario puede llamar muchas veces a `RequestPlayListPictureUpload` / `RequestTrackUploadUrls` y subir muchos objetos a S3 sin llegar a crear la entidad final (objetos “huérfanos”).
## Qué pasa hoy si el usuario spamea
- Se generan muchas URLs pre-firmadas (costo bajo pero no cero).
- El usuario puede subir muchos objetos a S3 sin relación con una entidad en DB.
- El impacto principal es:
- coste/uso de S3 (storage + requests)
- posible saturación del endpoint de generación de URLs
## Objetivo
Reducir abuso y coste, manteniendo:
- UX simple
- backend sin streams
- workflows asíncronos + MassTransit Outbox
## Mitigaciones recomendadas (pueden coexistir)
### 1) Rate limiting por usuario/IP (primera línea de defensa)
Aplicar limitación en el host/API (no en `Application`). Ejemplos:
- `N` requests/min por `UserId`
- opcional: `M` requests/min por IP
Ventajas:
- Corta abuso sin tocar DB/S3.
- No cambia dominio.
Nota: requiere acceso al proyecto host (controllers/minimal API) para aplicarlo.
### 2) “Upload intents” en DB (control y conciliación)
Persistir una intención de upload cuando se genera un pre-signed URL.
Modelo sugerido (mínimo):
- `UploadIntent`
- `Id`
- `UserId`
- `Bucket`
- `Key`
- `ContentType`
- `ExpiresAt`
- `Status`: `Issued | Consumed | Expired`
- `CreatedAt`
Reglas:
- Al pedir URL (Request*Upload*):
- insertar intent `Issued`
- imponer límite por usuario: p.ej. `MaxActiveIntentsPerUser = 5`
- si supera el límite: devolver `429 Too Many Requests` (o `Conflict`) según convención del API
- Al crear/actualizar entidad (Create*/Update* V2):
- validar que el `pictureName/audioName` recibido corresponde a un intent `Issued` del mismo `UserId` y no expirado
- marcar intent como `Consumed`
Ventajas:
- Previene spam con reglas duras y auditables.
- Permite cleanup de huérfanos con certeza.
Coste:
- entidad + migración + lógica de validación.
### 3) Limitar el scope del pre-signed URL (reducción de daño)
Aplicar restricciones conservadoras:
- Reducir TTL:
- `ExpiresInSeconds` más bajo (p.ej. 60–120s en vez de 600s)
- Validar/limitar:
- `ContentType` permitido
- extensiones permitidas (`FileType`)
- Usar keys temporales por usuario:
- `PlayLists/TempUploads/{userId}/{guid}.{ext}`
- `Tracks/TempUploads/{userId}/{guid}.{ext}`
Luego, en el workflow/worker:
- mover/copiar de `TempUploads/...` a la ruta final (`OriginalPictures/...` / `OriginalAudios/...`)
- borrar el objeto temporal
Ventajas:
- Cleanup masivo sencillo (por prefijo `TempUploads/{userId}`)
- Reduce el impacto de huérfanos.
## Orden de implementación sugerido
1) Rate limiting en API (rápido + mayor impacto)
2) Upload intents en DB (límite de activos + validación al crear/actualizar)
3) Reducir TTL y mover a rutas temporales (`TempUploads/...`) + cleanup
## Parámetros recomendados (config)
- `UploadUrlExpiresInSeconds` (default: 120)
- `MaxActiveUploadIntentsPerUser` (default: 5)
- `AllowedPictureExtensions` / `AllowedAudioExtensions`
- `AllowedContentTypes` (lista blanca)
## Notas
- Estas defensas complementan el Outbox: el Outbox garantiza entrega de eventos tras commit, pero no evita el abuso de generación de URLs o almacenamiento de objetos huérfanos.