Files
itca_api_sap_wms/docs/PORTE_FUNCIONALIDAD_NUEVA.md
2026-10-05 17:53:41 -05:00

12 KiB

Porte de funcionalidad nueva desde la reescritura Python

Este documento (y los dos que lo acompañan, SOLICITUD_TRANSFERENCIA.md y PICKING.md) son la guía para agregar a este proyecto .NET Core 3.1 la funcionalidad de negocio nueva que se diseñó e implementó primero en la reescritura Python (/Users/george/Documents/Desarrollo/ITCG/itca_api_sap_wms_python), porque este .NET va a seguir corriendo en producción (IIS, Windows) más tiempo del previsto originalmente.

La lógica de negocio ya está validada en Python — acá es 100% traducción de lenguaje, no rediseño. El contrato HTTP (rutas, shapes de request/response, status codes) debe quedar idéntico al de la API Python, porque ambas corren sobre el mismo SAP Business One y el mismo WMS las va a consumir.

Alcance confirmado: solo la funcionalidad nueva (sin equivalente en este .NET hoy). No se portan acá las correcciones de bugs de contrato que sí se hicieron en la reescritura Python (ej. 201 real en los POST de creación, 400 explícito en Inventario/Recibo en vez de éxito falso) — quedan fuera de este trabajo salvo que se decida lo contrario más adelante.

Qué hay que agregar

  1. Módulo SolicitudTransferencia completo (no existe ningún controller ni repositorio hoy) — ver SOLICITUD_TRANSFERENCIA.md.
  2. Picking diurno/nocturno — campo Notificacion en el POST /api/Picking existente + nuevo POST /api/Picking/HojaRuta/{docEntry}/Cerrar — ver PICKING.md.

Dónde vive cada pieza hoy (mapa de archivos .NET)

Concepto Archivo
Controllers existentes Net.Business.Services/Controllers/*.cs
Repositorios de negocio (WMS) Net.Data/WMS/*/
Repositorios de Service Layer (SAP) Net.Data/ServicesLayer/*/
Cliente HTTP de Service Layer Net.Connection.ServiceLayer/ConnectionServiceLayer.cs
DTOs (shape del request HTTP) Net.Business.DTO/**/Dto_*.cs
Entidades de negocio Net.Business.Entities/**/BE_*.cs
Wrapper de inyección de dependencias Net.Data/DependencyInjection/{IRepositoryWrapper,RepositoryWrapper}.cs
Wrapper de resultado (BE_ResultadoTransaccion<T>) Net.Business.Entities/ResultadoTransaccion/BE_ResultadoTransaccion.cs + Net.CrossCotting/ResultadoTransaccion.cs
Logging de auditoría Net.Business.Services/Log/B_Log.cs (se llama desde cada controller, después del repositorio)
Handler global de excepciones Net.Business.Services/Extensions/ExceptionMiddlewareExtensions.cs

Patrón a seguir para cada pieza nueva (igual que TransferenciaController/ TransferenciaRepository/StockTransferRepository): DTO → RetornaModelo() → Entidad → Repositorio de negocio (usa ConnectionServiceLayer + helpers de HANA) → Controller (valida, llama al repo, llama a B_Log.Add, decide el status code).

Gaps de infraestructura compartida a tener en cuenta

Estos dos módulos nuevos necesitan cosas que ConnectionServiceLayer.cs hoy no soporta, y hay que agregarlas:

  1. Headers extra en PATCH. SolicitudTransferencia necesita mandar B1S-ReplaceCollectionsOnPatch: true en el PATCH a SAP. Ningún método de ConnectionServiceLayer (PostAsync, PatchAsyncSBA, etc.) acepta headers adicionales hoy — todos hardcodean Accept/Content-Type/Cookie únicamente. Hay que agregar un parámetro opcional (ej. Dictionary<string, string> extraHeaders = null) a PatchAsyncSBA<T>.
  2. GET de un solo recurso (no colección). SolicitudTransferencia necesita GET InventoryTransferRequests({docEntry}) (un objeto, no una lista .value). ConnectionServiceLayer.GetAsync<T> ya tiene un parámetro dinamico que deserializa T directo sin esperar el wrapper { "value": [...] } (ConnectionServiceLayer.cs:363-373) — reusar esa ruta (dinamico: true) en vez de escribir un método nuevo.
  3. Acción /Close sin body. Ni SolicitudTransferencia ni Picking/HojaRuta tienen hoy un método para POST {Recurso}({docEntry})/Close con body vacío — el único análogo es StockTransferRepository.StockTransfer (Net.Data/ServicesLayer/StockTransfer/StockTransferRepository.cs), que sí manda body. Hay que agregar un método nuevo (ej. CloseAsyncSBA) o un overload de PostAsync<T> que acepte obj: null/{}.

Bugs e inconsistencias encontrados en el código .NET existente

Auditado puntualmente el código que estos dos módulos nuevos van a tocar o reusar (ConnectionServiceLayer.cs, HelperProcesoPiciking.cs, BE_ResultadoTransaccion.cs). No es una auditoría completa del proyecto — solo lo relevante para este trabajo.

1. BE_ResultadoTransaccion<T>.NombreEstacion — setter con recursión infinita (bug serio, no relacionado al porte pero lo tocan ambos módulos nuevos)

Archivo: Net.Business.Entities/ResultadoTransaccion/BE_ResultadoTransaccion.cs, línea 36 (propiedad NombreEstacion).

public string NombreEstacion { get => System.Environment.MachineName; set => NombreEstacion = System.Environment.MachineName; }

El set asigna NombreEstacion = ..., lo que vuelve a invocar al propio setter — recursión infinita. Si alguna vez se invoca ese setter (por ejemplo, si algo deserializa un BE_ResultadoTransaccion<T> con JsonConvert.DeserializeObject y el JSON trae la propiedad NombreEstacion/nombreEstacion, o si cualquier código hace objeto.NombreEstacion = "algo" directamente), el proceso revienta con StackOverflowException — una excepción que .NET no puede atrapar con try/catch, tumba el proceso entero (el worker de IIS).

Hoy no se dispara porque nada construye BE_ResultadoTransaccion<T> por deserialización ni asigna esa propiedad directamente (todo pasa por ResultadoTransaccion<T>.GetInformacionResultado(...), que no la toca). Pero al escribir el código nuevo para SolicitudTransferencia/HojaRuta, nunca hacer .NombreEstacion = ... a mano ni deserializar un BE_ResultadoTransaccion<T> desde JSON — usar siempre GetInformacionResultado(...).

Solución propuesta: la propiedad solo necesita ser de lectura.

public string NombreEstacion => System.Environment.MachineName;

(o, si hace falta que el setter exista por compatibilidad con algo que no vimos en esta auditoría puntual, que el setter no haga nada: set { }).

2. ConnectionServiceLayer.PatchAsyncSBA<T> nunca marca éxito en un 204

Archivo: Net.Connection.ServiceLayer/ConnectionServiceLayer.cs, método PatchAsyncSBA<T>, líneas 739-856 (el caso puntual, líneas 840-848).

Con un 204 No Content real de SAP, el código no entra en la rama de éxito (la que solo cubre 200/201, líneas 788-796) ni en ninguna rama de error — el bloque if (response_.StatusCode == NoContent) { ... } (línea 840) está con el cuerpo comentado, así que cae directo a return default(T); (línea 848), o sea null. Cualquier caller que acceda a una propiedad del resultado (.Exito, .DocEntry, etc.) revienta con NullReferenceException. Ya es un bug conocido y documentado en el proyecto Python (CLAUDE.md, regla 4) — ahí se corrigió tratando un 204 real como éxito explícito.

Importante: confirmado que nadie llama a este método hoy (grep de PatchAsyncSBA en todo el proyecto no da otro resultado que su propia definición) — es seguro modificarlo directamente, no hace falta duplicarlo para no romper nada en producción.

Solución propuesta: al construir el repositorio de SolicitudTransferencia, modificar PatchAsyncSBA<T> para que un 204 (o 200) se trate como éxito real, sin intentar deserializar un body que puede venir vacío — por ejemplo, devolviendo un bool/un objeto liviano de éxito en vez de T deserializado a la fuerza, o agregando un chequeo explícito antes del return default(T) final.

3. ConnectionServiceLayer.PatchAsyncSBA<T> — un 401 con error.code distinto de "301" también devuelve null silenciosamente

Archivo: Net.Connection.ServiceLayer/ConnectionServiceLayer.cs, método PatchAsyncSBA<T>, líneas 797-822.

Dentro de la rama 401, si NumError < 2 y el código de error de SAP no es "301" (el que dispara relogin + reintento), no hay ningún goto band ni return — el flujo simplemente sale del if/else if (porque ya entró a la rama 401, la siguiente condición es else if y no se evalúa) y cae en el return default(T); del final del método. Mismo síntoma que el bug anterior (null silencioso, no un error explícito) pero por una causa distinta.

Solución propuesta: agregar un else final dentro de la rama 401 que construya y devuelva un resultado de error explícito, en vez de dejar que el flujo caiga al return default(T) genérico.

4. ConnectionServiceLayer — throw muerto después de return en 3 métodos

Archivo: Net.Connection.ServiceLayer/ConnectionServiceLayer.cs, métodos CancelAsyncSBA<T> (líneas 621-625), DeleteAsyncSBA<T> (líneas 733-737) y PatchAsyncSBA<T> (líneas 851-855):

catch (Exception ex)
{
    return default(T);
    throw new Exception(ex.Message.ToString());
}

El throw es código inalcanzable (el compilador de C# debería marcarlo con el warning CS0162) — la excepción original queda silenciada sin loguear ni propagar, y el caller recibe null sin ninguna pista de qué pasó.

Solución propuesta: sacar el throw muerto y decidir explícitamente el comportamiento — si se quiere tragar el error y devolver null/un resultado de error (dejar solo el return), o repropagar la excepción real (throw;, sin el return antes). Para los métodos nuevos de SolicitudTransferencia, no repetir este patrón: loguear o envolver el error en el BE_ResultadoTransaccion de salida, igual que hace el resto del proyecto (ver TransferenciaRepository.Create, que atrapa la excepción y la mete en GetInformacionResultado(ex.Message, entidad)).

5. ConnectionServiceLayer.Login() y .LoginPost() — código duplicado

Archivo: Net.Connection.ServiceLayer/ConnectionServiceLayer.cs, Login() (líneas 44-130) y LoginPost() (líneas 131-218).

Son copias casi idénticas línea por línea — la única diferencia real es un parámetro Language que está comentado en LoginPost. No es un bug funcional, es una inconsistencia de mantenimiento (dos lugares que hay que tocar a mano si cambia el login). No bloquea el porte de la funcionalidad nueva — se deja solo documentado por si se decide refactorizar en el futuro.

Solución propuesta: unificar en un solo método (LoginPost ya tiene el parámetro Language listo para habilitarse si hiciera falta).

6. HelperProcesoPiciking.Set — SQL injection real por interpolación de string

Archivo: Net.Data/HelperData/ProcesoPiciking/HelperProcesoPiciking.cs, ambas sobrecargas de Set (líneas 14 y 28):

cmd.CommandText = $"CALL\"B1_ARDI_PROD\".\"ITCA_WMS_SAP_SetProcesoPicking\"('{Proceso}', {IdEstado}, {DocEntry}, '', '', '{CodMotivo}', '{Code}')";

Proceso y CodMotivo vienen directo del body del request HTTP (entidad.Proceso, entidad.CodMotivo en PickingRepository.cs:96 y :374) e, interpolados así, son un vector de SQL injection real — mismo patrón de vulnerabilidad ya documentado y confirmado en el módulo Chatbot del .NET (CLAUDE.md del proyecto Python, sección "Pendiente"). Este método es justo el que hay que tocar para aislar la actualización de estado (ver PICKING.md), así que es un buen momento para corregirlo de una vez en vez de solo copiarlo.

Solución propuesta: parametrizar con HanaParameter/placeholders en vez de interpolar el string, por ejemplo:

cmd.CommandText = "CALL \"B1_ARDI_PROD\".\"ITCA_WMS_SAP_SetProcesoPicking\"(?, ?, ?, ?, ?, ?, ?)";
cmd.Parameters.Add(new HanaParameter("1", Proceso));
cmd.Parameters.Add(new HanaParameter("2", IdEstado));
cmd.Parameters.Add(new HanaParameter("3", DocEntry));
cmd.Parameters.Add(new HanaParameter("4", HoraInicio?.ToString() ?? ""));
cmd.Parameters.Add(new HanaParameter("5", HoraFin?.ToString() ?? ""));
cmd.Parameters.Add(new HanaParameter("6", CodMotivo));
cmd.Parameters.Add(new HanaParameter("7", Code));

(Ajustar la sintaxis exacta de binding de parámetros al driver Sap.Data.Hana realmente instalado — confirmar la forma correcta de parámetros posicionales de ese driver ADO.NET antes de aplicar, no asumida en esta auditoría.)