From 225161e4ca75c728f077125ec02aee6653be6017 Mon Sep 17 00:00:00 2001 From: JiriUhlir <149317995+JiriUhlir@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:13:21 +0200 Subject: [PATCH] revize endpointu: cteni NullableProperty, filtr v UTC, attachments Kontrola vsech 180 operaci proti falesnemu iDoklad API a round trip 331 modelu SDK v obou smerech. - SdkNullablePropertyConverter doplnuje cteni NullableProperty. Konvertor SDK umi jen zapis, takze PATCH s takovou polozkou koncil prazdnou 500 uz pri cteni tela. Tykalo se 29 modelu. Zapis zustava na konvertoru SDK, odchozi payload se nemeni. - Datum uvnitr NullableProperty se normalizuje na UTC stejne jako zbytek serializace. - ListModifiers parsuje datum ve filtru s AdjustToUniversal a AssumeUniversal. Filtr se zonou se drive posouval o offset. - POST /attachments kontroluje FileName a FileBytes, SDK na ne sahalo bez kontroly na null a vracelo neosetrenou 500. - ExceptionHandlingMiddleware ma posledni zachyt, zadny request uz nekonci prazdnou 500. Co-Authored-By: Claude Opus 5 (1M context) --- Client/ListModifiers.cs | 5 +- Controllers/IntegrationController.cs | 18 ++++- Infrastructure/ExceptionHandlingMiddleware.cs | 9 +++ Infrastructure/SdkContractResolver.cs | 33 ++++++--- .../SdkNullablePropertyConverter.cs | 66 +++++++++++++++++ documentation/datumy-utc.md | 2 + documentation/revize-endpointu.md | 72 +++++++++++++++++++ 7 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 Infrastructure/SdkNullablePropertyConverter.cs create mode 100644 documentation/revize-endpointu.md diff --git a/Client/ListModifiers.cs b/Client/ListModifiers.cs index 50dd063..17f75a4 100644 --- a/Client/ListModifiers.cs +++ b/Client/ListModifiers.cs @@ -228,7 +228,10 @@ public static class ListModifiers } if (type == typeof(DateTime)) { - return DateTime.Parse(raw, CultureInfo.InvariantCulture, DateTimeStyles.None); + // iDoklad works in UTC, so a value carrying an offset is converted and a value + // without one is taken as already being UTC. Parsing without these flags turned + // "2024-01-01T00:00:00Z" into local time and shifted the filter by the offset. + return DateTime.Parse(raw, CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal); } if (type == typeof(bool)) { diff --git a/Controllers/IntegrationController.cs b/Controllers/IntegrationController.cs index c5dcca4..7b316c6 100644 --- a/Controllers/IntegrationController.cs +++ b/Controllers/IntegrationController.cs @@ -96,7 +96,23 @@ public sealed class IntegrationController : ControllerBase /// Upload an attachment for a document. [HttpPost("attachments")] public async Task UploadAttachment([FromBody] AttachmentUploadModel model, CancellationToken ct) - => Ok(await _service.UploadAttachmentAsync(model, ct)); + { + // AttachmentUploadModel carries no validation attributes and the SDK dereferences the file + // name without a null check, so an incomplete model would end as an unhandled 500. + if (string.IsNullOrWhiteSpace(model.FileName)) + { + ModelState.AddModelError(nameof(model.FileName), "The FileName field is required."); + } + + if (model.FileBytes is null || model.FileBytes.Length == 0) + { + ModelState.AddModelError(nameof(model.FileBytes), "The FileBytes field is required."); + } + + return ModelState.IsValid + ? Ok(await _service.UploadAttachmentAsync(model, ct)) + : ValidationProblem(ModelState); + } /// Delete a single attachment by its id. [HttpDelete("attachments/{attachmentId:int}")] diff --git a/Infrastructure/ExceptionHandlingMiddleware.cs b/Infrastructure/ExceptionHandlingMiddleware.cs index 81b96ec..421f2f5 100644 --- a/Infrastructure/ExceptionHandlingMiddleware.cs +++ b/Infrastructure/ExceptionHandlingMiddleware.cs @@ -79,6 +79,14 @@ public sealed class ExceptionHandlingMiddleware // Thrown by the SDK when required credential fields are blank/invalid. await WriteProblem(context, HttpStatusCode.BadRequest, ex.Message, null); } + catch (Exception ex) + { + // Last resort: without this the response is a 500 with an empty body, which tells the + // caller nothing. The exception type is enough to locate the cause in the container log; + // no stack trace or request data is exposed. + _logger.LogError(ex, "Unhandled error while processing the request."); + await WriteProblem(context, HttpStatusCode.InternalServerError, $"Unexpected error ({ex.GetType().Name}). See the service log for details.", null); + } } private static async Task WriteProblem(HttpContext context, HttpStatusCode status, string detail, IDictionary? extensions) @@ -114,6 +122,7 @@ public sealed class ExceptionHandlingMiddleware HttpStatusCode.Unauthorized => "Unauthorized", HttpStatusCode.BadRequest => "Bad Request", HttpStatusCode.BadGateway => "Upstream iDoklad API error", + HttpStatusCode.InternalServerError => "Internal Server Error", _ => status.ToString(), }; } diff --git a/Infrastructure/SdkContractResolver.cs b/Infrastructure/SdkContractResolver.cs index 52b4dc3..42679ce 100644 --- a/Infrastructure/SdkContractResolver.cs +++ b/Infrastructure/SdkContractResolver.cs @@ -5,18 +5,17 @@ using Newtonsoft.Json.Serialization; namespace Idoklad.Infrastructure; /// -/// Makes iDoklad SDK models serializable. Some SDK converters, attached through -/// , only support reading and their WriteJson throws -/// , which turns any response carrying such a model into a -/// 500. This resolver replaces them with : reading keeps -/// using the SDK converter, writing falls back to the standard serialization. Everything else is +/// Makes iDoklad SDK models usable in both directions. The SDK only ever serializes what it sends +/// and deserializes what it receives, so several of its converters, attached through +/// , implement one direction and throw +/// in the other. This service needs both directions, so this +/// resolver swaps those converters for wrappers that fill in the missing half. Everything else is /// left to . /// public sealed class SdkContractResolver : DefaultContractResolver { /// - /// SDK converters that cannot write. Converters that do implement writing (for example - /// NullablePropertyJsonConverter) are deliberately not listed here. + /// SDK converters that cannot write, so responses carrying such a model would fail. /// private static readonly HashSet WriteUnsupportedConverterTypeNames = new(StringComparer.Ordinal) { @@ -27,6 +26,11 @@ public sealed class SdkContractResolver : DefaultContractResolver "IdokladSdk.Serialization.NotificationJsonConverter", }; + /// + /// SDK converters that cannot read, so a request body carrying such a member would fail. + /// + private const string NullablePropertyConverterTypeName = "IdokladSdk.Clients.NullablePropertyJsonConverter"; + protected override JsonContract CreateContract(Type objectType) { var contract = base.CreateContract(objectType); @@ -49,8 +53,19 @@ public sealed class SdkContractResolver : DefaultContractResolver { var typeName = converter?.GetType().FullName; - return typeName is not null && WriteUnsupportedConverterTypeNames.Contains(typeName) - ? new SdkWriteBypassJsonConverter(converter!) + if (typeName is null) + { + return converter; + } + + if (WriteUnsupportedConverterTypeNames.Contains(typeName)) + { + return new SdkWriteBypassJsonConverter(converter!); + } + + // Attached to NullableProperty members of Patch models; writing works, reading throws. + return typeName == NullablePropertyConverterTypeName + ? new SdkNullablePropertyConverter(converter!) : converter; } } diff --git a/Infrastructure/SdkNullablePropertyConverter.cs b/Infrastructure/SdkNullablePropertyConverter.cs new file mode 100644 index 0000000..da7581c --- /dev/null +++ b/Infrastructure/SdkNullablePropertyConverter.cs @@ -0,0 +1,66 @@ +using System.Reflection; +using Newtonsoft.Json; + +namespace Idoklad.Infrastructure; + +/// +/// Read support for the SDK's NullableProperty<T>, used by Patch models to tell +/// "not set" apart from "set to null". The SDK converter only writes: its ReadJson throws +/// , because the SDK never deserializes a Patch model. This +/// service does, so a PATCH body carrying such a member failed while binding. Writing stays with +/// the SDK converter so the payload sent upstream is unchanged. +/// +public sealed class SdkNullablePropertyConverter : JsonConverter +{ + private readonly JsonConverter _sdkConverter; + + public SdkNullablePropertyConverter(JsonConverter sdkConverter) + { + _sdkConverter = sdkConverter; + } + + public override bool CanRead => true; + + public override bool CanWrite => _sdkConverter.CanWrite; + + public override bool CanConvert(Type objectType) => _sdkConverter.CanConvert(objectType); + + /// + /// Reads the JSON scalar the SDK converter writes: a value, or null for an explicit reset. + /// A member missing from the body is never read at all and keeps its unset default. + /// + public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer) + { + var propertyType = Nullable.GetUnderlyingType(objectType) ?? objectType; + var valueType = propertyType.GetGenericArguments()[0]; + + var value = reader.TokenType == JsonToken.Null + ? null + : serializer.Deserialize(reader, typeof(Nullable<>).MakeGenericType(valueType)); + + // The member converter puts the reader in DateParseHandling.None, so the date arrives as a + // string and the serializer's DateTimeZoneHandling never gets applied to it. The SDK + // rejects a DateTime whose Kind is not Utc, so apply the same normalization here. + if (value is DateTime date) + { + value = ToUtc(date); + } + + // NullableProperty(T? value) marks the member as set, including when the value is null. + return Activator.CreateInstance(propertyType, BindingFlags.Instance | BindingFlags.Public | BindingFlags.CreateInstance, null, new[] { value }, null); + } + + public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer) + => _sdkConverter.WriteJson(writer, value, serializer); + + /// + /// Matches : a value that carries an offset is converted, + /// a value without one is taken as already being UTC and is only marked as such. + /// + private static DateTime ToUtc(DateTime value) => value.Kind switch + { + DateTimeKind.Utc => value, + DateTimeKind.Local => value.ToUniversalTime(), + _ => DateTime.SpecifyKind(value, DateTimeKind.Utc), + }; +} diff --git a/documentation/datumy-utc.md b/documentation/datumy-utc.md index 4da79cf..a1b3ed2 100644 --- a/documentation/datumy-utc.md +++ b/documentation/datumy-utc.md @@ -69,3 +69,5 @@ POST https://services.csbot.cz/apps/idoklad/issued-invoices ``` s datumy ve tvaru `2026-08-25`. + +Navazující revize všech ostatních endpointů: [revize-endpointu.md](revize-endpointu.md). diff --git a/documentation/revize-endpointu.md b/documentation/revize-endpointu.md new file mode 100644 index 0000000..8e3a8ff --- /dev/null +++ b/documentation/revize-endpointu.md @@ -0,0 +1,72 @@ +# Revize všech endpointů po opravě datumů + +Kontrola navazuje na [datumy-utc.md](datumy-utc.md). Cílem bylo najít další místa se stejnou +povahou chyby, tedy vstup, který služba přijme, ale SDK nebo iDoklad ho odmítne, případně +tichý posun hodnoty. + +## Jak se to ověřovalo + +1. Reflexní round trip všech 331 modelů SDK (Post, Patch, Get) přesně tím nastavením + serializace, které používá služba: serializace odpovědi i deserializace requestu. +2. Běh služby proti falešnému iDoklad API, které zachytává odchozí requesty. Projeto všech + 180 operací z OpenAPI dokumentu a zkontrolováno, co se skutečně odesílá. +3. Porovnání klienta `iDoklad.cs` proti OpenAPI dokumentu služby, cesta i HTTP metoda. + +## Nálezy a opravy + +### 1. PATCH padal na NullableProperty (vážné) + +Patch modely používají `NullableProperty`, aby šlo odlišit nevyplněnou položku od položky +nastavené na null. Konvertor SDK `NullablePropertyJsonConverter` umí jen zápis, jeho `ReadJson` +vyhazuje `NotImplementedException`, protože SDK Patch modely nikdy nedeserializuje. Tato služba +je deserializovat musí, takže PATCH s takovou položkou skončil prázdnou 500 už při čtení těla. + +Týkalo se to 29 modelů, mimo jiné `IssuedInvoicePatchModel`, `ReceivedInvoicePatchModel`, +`ProformaInvoicePatchModel`, `ContactPatchModel` a `CreditNotePatchModel`. Endpoint spadl podle +toho, které položky klient poslal, takže se chyba projevovala nepravidelně. + +Oprava: `Infrastructure/SdkNullablePropertyConverter.cs` čtení doplňuje, zápis nechává na +konvertoru SDK, takže odchozí payload se nemění. Zaregistrováno v `SdkContractResolver`. + +Ověřeno, že sémantika zůstává správná: položka vynechaná v těle requestu se do iDokladu +neodešle. PATCH tedy nepřepisuje pole, která klient neuvedl. + +### 2. Datum uvnitř NullableProperty nebylo v UTC + +Když je na položce konvertor, Newtonsoft předá hodnotu jako řetězec a `DateTimeZoneHandling` +se na ni neuplatní. Datum uvnitř `NullableProperty` proto zůstávalo `Unspecified` +a narazilo by na stejnou kontrolu SDK jako u vydaných faktur. Konvertor hodnotu normalizuje +stejným pravidlem jako zbytek serializace. + +### 3. Filtr s časovou zónou posouval čas (tiché) + +`Client/ListModifiers.cs` parsoval datum ve filtru bez příznaků zóny. Filtr +`(DateOfTaxing~gte~2024-01-01T00:00:00Z)` se do iDokladu odeslal jako `2024-01-01 01:00:00`, +tedy posunutý o hodinu, a `+02:00` se posunulo o dvě. Chyba nic nenahlásila, jen vracela jiná +data. Opraveno na `AdjustToUniversal | AssumeUniversal`. + +### 4. POST /attachments vracel prázdnou 500 + +`AttachmentUploadModel` nemá žádné validační atributy a SDK sahá na jméno souboru bez kontroly +na null. Chybějící `FileName` nebo `FileBytes` tedy skončily neošetřenou `NullReferenceException`. +Doplněna kontrola v `IntegrationController`, chybějící pole nyní vrací 400. + +### 5. Poslední záchyt v middleware + +`ExceptionHandlingMiddleware` má nově i `catch (Exception)`. Neočekávaná chyba se zaloguje +a vrátí jako 400 nebo 500 s typem výjimky v těle. Žádný request už nekončí prázdnou 500, +ze které nebylo poznat vůbec nic. + +## Co bylo v pořádku + +- Query parametr `lastCheck` u `GET /system/code-books/changes`. Binder MVC normalizuje + hodnotu na UTC správně, `Z` i offset i tvar bez zóny dávají správný okamžik. +- Serializace odpovědí. Round trip 331 modelů proběhl bez chyby v obou směrech. +- Klient `iDoklad.cs`. Ze 177 rozpoznaných volání neodpovídá službě ani jedno špatnou cestou + nebo metodou. + +## Výsledek + +Všech 180 operací proti falešnému iDoklad API: 125 vrátilo 200, 55 vrátilo 400 kvůli +záměrně neúplnému testovacímu tělu, žádná nevrátila 5xx. Původní tělo z logu klienta, +které chybu odstartovalo, projde a do iDokladu odejde se správnými datumy.