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<T>. 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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
73f01d0228
commit
225161e4ca
@@ -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))
|
||||
{
|
||||
|
||||
@@ -96,7 +96,23 @@ public sealed class IntegrationController : ControllerBase
|
||||
/// <summary>Upload an attachment for a document.</summary>
|
||||
[HttpPost("attachments")]
|
||||
public async Task<IActionResult> 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);
|
||||
}
|
||||
|
||||
/// <summary>Delete a single attachment by its id.</summary>
|
||||
[HttpDelete("attachments/{attachmentId:int}")]
|
||||
|
||||
@@ -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<string, object?>? 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(),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -5,18 +5,17 @@ using Newtonsoft.Json.Serialization;
|
||||
namespace Idoklad.Infrastructure;
|
||||
|
||||
/// <summary>
|
||||
/// Makes iDoklad SDK models serializable. Some SDK converters, attached through
|
||||
/// <see cref="JsonConverterAttribute"/>, only support reading and their <c>WriteJson</c> throws
|
||||
/// <see cref="NotImplementedException"/>, which turns any response carrying such a model into a
|
||||
/// 500. This resolver replaces them with <see cref="SdkWriteBypassJsonConverter"/>: 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
|
||||
/// <see cref="JsonConverterAttribute"/>, implement one direction and throw
|
||||
/// <see cref="NotImplementedException"/> 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 <see cref="DefaultContractResolver"/>.
|
||||
/// </summary>
|
||||
public sealed class SdkContractResolver : DefaultContractResolver
|
||||
{
|
||||
/// <summary>
|
||||
/// SDK converters that cannot write. Converters that do implement writing (for example
|
||||
/// <c>NullablePropertyJsonConverter</c>) are deliberately not listed here.
|
||||
/// SDK converters that cannot write, so responses carrying such a model would fail.
|
||||
/// </summary>
|
||||
private static readonly HashSet<string> WriteUnsupportedConverterTypeNames = new(StringComparer.Ordinal)
|
||||
{
|
||||
@@ -27,6 +26,11 @@ public sealed class SdkContractResolver : DefaultContractResolver
|
||||
"IdokladSdk.Serialization.NotificationJsonConverter",
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// SDK converters that cannot read, so a request body carrying such a member would fail.
|
||||
/// </summary>
|
||||
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<T> members of Patch models; writing works, reading throws.
|
||||
return typeName == NullablePropertyConverterTypeName
|
||||
? new SdkNullablePropertyConverter(converter!)
|
||||
: converter;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
using System.Reflection;
|
||||
using Newtonsoft.Json;
|
||||
|
||||
namespace Idoklad.Infrastructure;
|
||||
|
||||
/// <summary>
|
||||
/// Read support for the SDK's <c>NullableProperty<T></c>, used by Patch models to tell
|
||||
/// "not set" apart from "set to null". The SDK converter only writes: its <c>ReadJson</c> throws
|
||||
/// <see cref="NotImplementedException"/>, 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.
|
||||
/// </summary>
|
||||
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);
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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>(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);
|
||||
|
||||
/// <summary>
|
||||
/// Matches <see cref="DateTimeZoneHandling.Utc"/>: a value that carries an offset is converted,
|
||||
/// a value without one is taken as already being UTC and is only marked as such.
|
||||
/// </summary>
|
||||
private static DateTime ToUtc(DateTime value) => value.Kind switch
|
||||
{
|
||||
DateTimeKind.Utc => value,
|
||||
DateTimeKind.Local => value.ToUniversalTime(),
|
||||
_ => DateTime.SpecifyKind(value, DateTimeKind.Utc),
|
||||
};
|
||||
}
|
||||
@@ -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).
|
||||
|
||||
@@ -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<T>`, 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<DateTime>` 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.
|
||||
Reference in New Issue
Block a user