Zauważone niedociągnięcia w bibliotece .NET API KSeF (i "po sąsiedzku" - Javy)
To nie błędy, ale - parafrazując pozytywną nowomowę - "obszary do poprawy". Do każdego z nich proponuję poprawkę - czy możecie mi podpowiedzieć, czy się nie mylę, i czy warto to zgłaszać jako Issue w GitHub / helpdesku KSeF?
Więc po kolei:
#1 RestClient.cs
RestClient to bardzo ważna klasa biblioteki .NET (właściwie to ona obsługuje logikę każdego wywołania API). A wygląda jak coś zmienianego, na różnych etapach przez różnych autorów, którzy woleli szybko powielić jakiś kod zamiast się nad nim pochylić. Zawiera trzy metody o tej samej nazwie SendAsync, o coraz mniejszej liczbie parametrów. I każda z nich powstała chyba przez jakieś "copy/paste", bo zawierają (prawie) takie same bloki kodu.
*Wariant SendAsync() z największą liczbą parametrów jest podstawowym - wykorzystany w 47 miejscach. W IRestClient jest zdefiniowana tak:
Kopiuj
public Task<TResponse> SendAsync<TResponse, TRequest>(
HttpMethod method,
string url,
TRequest requestBody = default,
string bearerToken = null,
string contentType = "application/json",
CancellationToken cancellationToken = default,
Dictionary<string, string> additionalHeaders = null);
Aby logika jej implementacji pozwalała na obsługę pozostałych wariantów, wystarczyłoby zamienić wiersz 62 w obecnym pliku RestClient.cs
z takiego:
Kopiuj
var content = await response.Content.ReadAsStreamAsync(cancellationToken);
na taki:
Kopiuj
Stream content = null;
if (response.Content != null) content = await response.Content.ReadAsStreamAsync(cancellationToken);
*Pierwsze uproszczenie SendAsync(), używane w implementacji: CloseBatchSession, ClientRevokeCertificate, zwraca Task zamiast Task<TResponse> i nie ma opcjonalnego argumentu z dodatkowymi nagłówkami. Zamiast obecnego powielenia kodu, można byłoby potraktować jako szczególny przypadek metody poprzedniej:
Kopiuj
public Task SendAsync<TRequest>(
HttpMethod method,
string url,
TRequest requestBody = default,
string bearerToken = null,
string contentType = "application/json",
CancellationToken cancellationToken = default)
{
return SendAsync<object?, TRequest>(method,url,requestBody,bearerToken,contentType,cancellationToken);
}
*Drugie uproszczenie SendAsync(), użyte w implementacji: RevokeAccessToken, CloseOnlineSession, RevokeKsefToken, nie ma opcjonalnego TRequest, a contentType jest "na zicher" wpisany otwartym tekstem zamiast stałą (inna sprawa, że tak zdefiniowali go we wzorcowym interfejsie). Podobnie jak poprzednio, można to wyrazić jako szczególny przypadek poprzedniej metody:
Kopiuj
public Task SendAsync(
HttpMethod method,
string url,
string bearerToken = null,
string contentType = "application/json",
CancellationToken cancellationToken = default)
{
return SendAsync<object?>(method, url, null, bearerToken, contentType, cancellationToken);
}
(Na marginesie: argument bearerToken to accessToken. Tak się autorom interefejsu skojarzyło, bo nagłówek HTTP z tokenem JWT wygląda tak: "Authorization: Bearer eyJhbGciOiJIUzI1NiIXVCJ9TJV...r7E20RMHrHDcEfxjoYZgeFONFh7HgQ")
W IRestClient jest jeszcze metoda GetPemAsync(), która jest zwykłym pobraniem pliku publicznego certyfikatu (*.pem). Użyta tylko raz, w konstruktorze pomocniczej klasy CryptographyServices(.cs), wygląda na jakąś rozpaczliwą łatkę (por poniżej), która zniknie w docelowej implementacji. Nie warto jej optymalizować.
[Jak sądzicie - zgłosić taką optymalizację jako Issue do projektu .NET na GitHub?]
#2 CryptographyServices
W konstruktorze klasy umieszczono poprawny ("na oko") kod pobierający certyfikaty serwera za pomoca oficjalnej, dedykowanej metody API (kSeFClient.GetPublicCertificates(default) - czyli /api/v2/security/public-key-certificates). Ale ten kod jest skomentowany!
ZAMIAST tego plik certyfikatu jest "ściągany" zwykłym, statycznym HTTP:GET (por. metoda RestClient.GetPemAsync())
Żeby było ciekawiej: w równoległej bibliotece Java ekwiwalent tej funkcji też tak robi. Tylko tam jest to głębiej "zakopane" w DefaultKsefClient.getPublicKey() woła PublicKeyEnvironmentApi.apiV2PublicKeyGet(), a ta woła metodę o nazwie apiV2PublicKeyGetWithHttpInfo(), która ściąga plik *.pem.
[Tego bym na razie nie ruszał, bo wygląda że mają ze swoim API problem]. (Na marginesie: żeby taki "wrapper" jak v2/security/public-key-certificates, zwracający stałą odpowiedź, nie działał, to - moje uszanowanie :)).
#3 XadeSDummy(.cs):
po co do tych ogólnie użytecznych serwisów dodano coś takiego? Jedyna metoda tej klasy tworzy na lokalnym dysku plik xml do nazwie "ksefDummySign_{guid}.xml", i czeka przez 5 minut na pojawienie się nowego pliku z "ksefDummySign_{guid} (1).xml", który będzie z podpisem cyfrowym.
Ta klasa jest wykorzystywana przez środowisko (projekt) z przebiegami testowymi i przykładowy projekt portalu ASP .NET.
Gdyby chociaż gdzieś napisali jakiś komentarz, żeby ten dziwoląg ignorować! Coś takiego powinno się umieszczać w tych testach /
przykładzie, a nie w finalnej bibliotece. Tylko tworzy niepotrzebną zagadkę "po co to jest"? Zamiast takiego "kwiatka" mogliby ten plik wrzucić i do projektu testowego, i do przykładu. Tym bardziej, że jak pokazuje przypadek RestClient.cs, nie mają oporów z duplikowaniem o wiele ważniejszego kodu.
[Zgłosić to jako Issue do projektu .NET na GitHub?]
#4 Język komunikatów (błędów)
Język zwracanych tekstów nie jest ujednolicony. Programy z folderu Http "mówią" po angielsku, a te z Api/Services - w połowie po polsku. Przy czym moja ulubiona klasa XadeSDummy w jednej metodzie potrafi się odezwać raz tak, raz siak. Podobnie jest z biblioteką Java.
Uważam, że powinni to wszystko przestawić na angielski. (Dlaczego? Bo ten kod nie ma wersji językowych, a zakładam, że wszyscy polscy programiści, którzy się mogą nim zajmować, są obyci z komunikatami po angielsku. W drugą stronę to nie działa :)).
[Zgłosić to jako Issue do projektu .NET na GitHub?]