diff --git a/CHANGELOG.md b/CHANGELOG.md index 2031e7c..6052112 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,149 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). -## Unreleased +## 0.4.3 — 2026-08-13 + +### Changed (breaking) + +- **`server_timezone` is now a required client argument** (`GLPI_SERVER_TIMEZONE` + for `from_env`). It takes an IANA zone name — `"Europe/Paris"` — or a + `tzinfo`. + + GLPI 11 sends most timestamps with the correct historical offset, but not + all of them. Measured against a live instance: 19 of the 20 datetime fields + across every resource are offset-bearing, and `KBArticle.revisions[].date` + is not. One response therefore carries both kinds, and comparing them raises + `TypeError: can't compare offset-naive and offset-aware datetimes` — sorting + an article's revision history against the article's own dates was enough to + trigger it. + + There is deliberately **no default**. Every candidate is wrong somewhere: + against a Europe/Paris instance, assuming UTC shifts the affected timestamps + by one or two hours *and stops raising*, turning a loud failure into a quiet + wrong answer. An IANA name is preferred over a fixed offset because a name + follows DST — the same instance emits both `+01:00` and `+02:00`. + + An offset already on the wire always wins over the configured zone, and a + model built outside the client (no validation context) keeps its naive values + rather than being stamped with a guess. + + Adds `tzdata` as a dependency on Windows, which ships no system timezone + database; without it `zoneinfo` resolves on Linux CI and raises on a + developer machine. + +- **Search endpoints now raise on a 4xx instead of returning `[]`.** The seven + `search_*` helpers passed no `failure_message` to `_resource_list`, which + skipped the status check entirely, so a 400, 401, 403 or 404 came back as an + empty list — indistinguishable from a filter that legitimately matched + nothing. (5xx already raised.) It composed badly with the batch iterators: + they stop on a page shorter than `batch_size`, so a 403 on the first page + ended the walk having yielded nothing and the caller saw a *successful* + empty result. This reverses decision D2 of the 0.4.0 error work, which chose + tolerance deliberately; the silent-empty failure mode has proved worse than + the exception. An empty list now means the server said the result set is + empty. **Callers that relied on `[]` after a permission error must catch + `GlpiStatusError`.** + +### Added + +- **`glpi_python_client.rsql`** — public date builders for the v2 filter + grammar: `created_between`, `date_window` and `changed_since`, all exported + from the package root. The end-of-day detail on a window's upper bound is + easy to get wrong and impossible to notice, since GLPI answers a malformed + filter by ignoring it and returning the whole table. + +- **`find_user_by_email(email)`** — resolves a person by address. It scans, + because GLPI exposes addresses as the nested array `User.emails` and the v2 + filter engine cannot join a nested array. Narrow it with `rsql_filter` and + cache the id; do not hand-roll an RSQL e-mail filter. + +- **`stream_document_content(document_id, chunk_size=...)`** — yields a + document body in chunks instead of buffering it whole, as + `download_document_content` does. Upload still buffers. + +- **Batch iterators for the four resources that lacked one**: + `iter_search_kb_articles`, `iter_search_kb_categories`, + `iter_search_documents` and `iter_search_locations`. + +### Fixed + +- **Every `datetime` write raised `TypeError`.** `model_to_payload` dumped in + Pydantic's python mode, so a request body reached `json.dumps` still holding + a live `datetime`. The failure landed at the encoder — after the model had + validated and outside any transport stub — which is why the suite never saw + it. The dump now runs in JSON mode. + +- **GLPI discards the offset on every datetime it is sent, so aware values + were written as the wrong moment.** Measured against a live Europe/Paris + instance: `2026-08-01T12:30:00` written bare, as `...Z`, and with `+02:00`, + `+09:00`, `-08:00` and `+14:00` all store 12:30 Paris. The server reads the + naive prefix, interprets it in its own timezone, and throws the rest away — + with a 200. It does parse the offset first, since `+99:99` answers HTTP 500, + which is the worst combination: a malformed offset crashes, a well-formed + wrong one is silent. `12:30-08:00` is 21:30 in Paris and landed nine hours + early. + + An aware `datetime` is now converted onto the server's clock and the offset + dropped, via a serialisation context mirroring the validation context used + on the inbound half. Naive values are untouched — they already mean the + server's clock — and no context means no conversion, so a model dumped + outside the client is unchanged. This is the second half of the + `server_timezone` contract above; `mode="json"` alone would have shipped + writes wrong by up to twelve hours. + +- **`get_ticket_statistics` silently truncated at 200 tickets**, on an + instance whose own docstring records 59,690. It issued one `search_tickets` + call with no paging loop, so every statistic was computed over whichever + 200 tickets came back first and reported as if it covered the corpus. The + two entity and one user name-resolution sites had the same shape. All four + now page through `iter_search_*`. + +- **`from_transport` silently deleted text.** The HTML path was taken whenever + the content held both `<` and `>`, so `"use the key"` became + `"use the key"` — an unknown tag's markup is dropped and its empty body + kept, removing the word with nothing left to show it was ever there. + `"cmd out"` and `"if x0"` lost text the same way. The + decision is now made on the element *name*. + +- **Fenced code blocks, tables and prose punctuation were mangled.** Without + the `fenced_code` extension a fence rendered as inline ``, which the + GLPI web UI shows as one run-on line and which a later read wrote back as + inline code — so a pasted log degraded further on every edit. Without + `tables`, a table rendered as literal pipes. Inbound, `markdownify` escaped + underscores and asterisks, so `snake_case` came back as `snake\_case` and + accumulated a backslash on every read-modify-write cycle. + +- **`_MAX_DATETIME` was naive, so `to_markdown()` raised on a mixed-awareness + timeline.** Sorting events padded absent timestamps with `datetime.max`, + which cannot be compared against the offset-bearing values GLPI sends. The + sort key now normalises both sides to UTC. The live probe confirmed the + mixed population is real, not hypothetical. + +- **`require_response_int` rejected create responses GLPI actually returns.** + A numeric-string id, an id nested under a `data` envelope, and a create that + reports only a `Location` header all raised a protocol error over a + perfectly usable identifier. It now probes top-level keys, then the + envelope, then the header. + +- **`sort="date_mod desc"` — the library's own documented example — is HTTP + 400.** Found while running the wire-format probe. The accepted syntax is + `field:direction`; a bare `date_mod` is accepted but sorts *ascending*, and + `order=` is ignored entirely. + +- **Three client construction examples had `server_timezone` inserted twice + and misindented**, by the sweep that added it. A repeated keyword argument + is a `SyntaxError`, so those examples could not be copied at all. Two older + documentation defects surfaced alongside them: three lines of expected + output stranded inside a `code-block:: python` (they belong to the example + above), and an import indented four spaces inside a three-space block. A new + test compiles all 77 Python snippets in the skills, the guide and the + README. + +## 0.4.0 – 0.4.2 + +These three releases were tagged without the changelog ever being +sectioned, so their notes accumulated under a single `Unreleased` +heading. They are grouped here rather than split retroactively. ### Fixed @@ -314,6 +456,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - The `requests` intersphinx mapping is removed; it survived the transport swap and made every docs build fetch an inventory nothing referenced. +## Pre-0.4.0 notes + +Kept for history. Written before the httpx and unasync rewrites, so the +status they describe is superseded by everything above — the transport is +no longer `requests`, and tolerant searches no longer swallow a 4xx. + ### Unchanged (deliberately) - Retry semantics: 5xx retried 3 times with a 3-second fixed wait, 4xx never diff --git a/README.md b/README.md index 261c068..188d330 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ from glpi_python_client import GlpiClient, PostTicket with GlpiClient( glpi_api_url="https://glpi.example.com/api.php/v2", + server_timezone="Europe/Paris", client_id="oauth-client-id", client_secret="oauth-client-secret", username="api-user", @@ -83,6 +84,7 @@ from glpi_python_client import AsyncGlpiClient, PostTicket async def main() -> None: async with AsyncGlpiClient( glpi_api_url="https://glpi.example.com/api.php/v2", + server_timezone="Europe/Paris", client_id="oauth-client-id", client_secret="oauth-client-secret", username="api-user", diff --git a/docs/user_guide.rst b/docs/user_guide.rst index 9a2fb9f..834b518 100644 --- a/docs/user_guide.rst +++ b/docs/user_guide.rst @@ -68,6 +68,7 @@ pair. The OAuth password grant accepts either ``client_id`` / with GlpiClient( glpi_api_url="https://glpi.example.com/api.php/v2", + server_timezone="Europe/Paris", client_id="oauth-client-id", client_secret="oauth-client-secret", username="api-user", @@ -92,6 +93,7 @@ The asynchronous client takes the same arguments and is used inside an async def main() -> None: async with AsyncGlpiClient( glpi_api_url="https://glpi.example.com/api.php/v2", + server_timezone="Europe/Paris", client_id="oauth-client-id", client_secret="oauth-client-secret", username="api-user", @@ -134,6 +136,11 @@ build the client for you: * ``GLPI_CLIENT_ID`` and ``GLPI_CLIENT_SECRET`` * ``GLPI_USERNAME`` and ``GLPI_PASSWORD`` * ``GLPI_ENTITY``, ``GLPI_PROFILE``, ``GLPI_ENTITY_RECURSIVE`` +* ``GLPI_SERVER_TIMEZONE`` -- **required**. IANA name of the timezone the + GLPI server runs in (e.g. ``Europe/Paris``). GLPI does not advertise it, + and it is needed to interpret the timestamps the server sends without an + offset. There is no default: guessing UTC against a Europe/Paris instance + shifts those values by an hour or two and never raises. * ``GLPI_LANGUAGE``, ``GLPI_VERIFY_SSL`` * ``GLPI_V1_BASE_URL``, ``GLPI_V1_USER_TOKEN``, ``GLPI_V1_APP_TOKEN`` @@ -628,6 +635,9 @@ so the client sets it through a legacy fallback — see Example output:: ['Networking'] + 42 Reset a Wi-Fi controller + 42 Reset a Wi-Fi controller + 1 revision(s) Assigning categories ^^^^^^^^^^^^^^^^^^^^^ @@ -659,9 +669,6 @@ article's full category set; passing an empty list clears every category. # Or set them explicitly at any time. client.set_kb_article_categories(article_id, [14]) # replace the full set client.set_kb_article_categories(article_id, []) # clear all - 42 Reset a Wi-Fi controller - 42 Reset a Wi-Fi controller - 1 revision(s) Enums ~~~~~ @@ -710,10 +717,11 @@ internal container and field names: .. code-block:: python - from glpi_python_client import GlpiClient + from glpi_python_client import GlpiClient with GlpiClient( glpi_api_url="https://glpi.example.com/api.php/v2", + server_timezone="Europe/Paris", client_id="oauth-client-id", client_secret="oauth-client-secret", username="api-user", diff --git a/glpi_python_client/__init__.py b/glpi_python_client/__init__.py index 66ef03d..13e3eb8 100644 --- a/glpi_python_client/__init__.py +++ b/glpi_python_client/__init__.py @@ -103,8 +103,13 @@ PostUser, TicketMarkdownOptions, ) +from glpi_python_client.rsql import ( + changed_since, + created_between, + date_window, +) -__version__ = "0.4.2" +__version__ = "0.4.3" __all__ = [ "AsyncGlpiClient", @@ -190,4 +195,7 @@ "PostUser", "TicketMarkdownOptions", "__version__", + "changed_since", + "created_between", + "date_window", ] diff --git a/glpi_python_client/_async/clients/_base_client.py b/glpi_python_client/_async/clients/_base_client.py index 90c9c40..cf12737 100644 --- a/glpi_python_client/_async/clients/_base_client.py +++ b/glpi_python_client/_async/clients/_base_client.py @@ -25,10 +25,12 @@ from glpi_python_client._async.clients.commons._config import ( build_client_env_config, build_client_resources, + resolve_server_timezone, ) if TYPE_CHECKING: from collections.abc import Mapping + from datetime import tzinfo logger = logging.getLogger(__name__) @@ -45,6 +47,7 @@ def __init__( self, *, glpi_api_url: str, + server_timezone: str | tzinfo, client_id: str | None = None, client_secret: str | None = None, username: str | None = None, @@ -66,6 +69,25 @@ def __init__( glpi_api_url : str Base URL of the GLPI v2 REST API, e.g. ``https://glpi.example.com/api.php/v2``. + server_timezone : str | tzinfo + IANA name of the timezone the GLPI server runs in (e.g. + ``"Europe/Paris"``), or a ``tzinfo``. **Required**: GLPI does + not advertise it, and it governs both directions of every + timestamp the client exchanges. + + Reading, it interprets the timestamps the server sends without + an offset. Writing, it is what makes an aware ``datetime`` + arrive as the moment it names: GLPI reads the naive prefix of a + timestamp and discards the offset, so the value has to be + converted onto the server's clock before it is sent. Measured on + a live instance, offsets from ``-08:00`` to ``+14:00`` written + to one field all stored the same wall clock. + + There is no default because every candidate is wrong somewhere + -- guessing UTC against a Europe/Paris instance shifts those + timestamps by an hour or two and never raises. Prefer a name + over a fixed offset: a name follows DST, and one instance emits + both ``+01:00`` and ``+02:00``. client_id : str | None, optional OAuth client identifier used to obtain access tokens. client_secret : str | None, optional @@ -103,6 +125,7 @@ def __init__( missing OAuth credentials together with no v1 fallback). """ + self.server_timezone = resolve_server_timezone(server_timezone) resources = build_client_resources( glpi_api_url=glpi_api_url, client_name=type(self).__name__, @@ -142,7 +165,8 @@ def from_env( ``GLPI_USERNAME``, ``GLPI_PASSWORD``, ``GLPI_VERIFY_SSL``, ``GLPI_V1_BASE_URL``, ``GLPI_V1_USER_TOKEN``, ``GLPI_V1_APP_TOKEN``, ``GLPI_ENTITY``, ``GLPI_PROFILE``, ``GLPI_ENTITY_RECURSIVE``, - ``GLPI_LANGUAGE``, ``GLPI_AUTH_TOKEN_REFRESH``). + ``GLPI_LANGUAGE``, ``GLPI_AUTH_TOKEN_REFRESH``, + ``GLPI_SERVER_TIMEZONE``). Parameters ---------- diff --git a/glpi_python_client/_async/clients/api/administration/_user.py b/glpi_python_client/_async/clients/api/administration/_user.py index 57e6fbc..5cf5fc4 100644 --- a/glpi_python_client/_async/clients/api/administration/_user.py +++ b/glpi_python_client/_async/clients/api/administration/_user.py @@ -12,6 +12,7 @@ from glpi_python_client._async.clients.commons._constants import USER_ENDPOINT, GlpiId from glpi_python_client._async.clients.commons._transport import TransportMixin +from glpi_python_client._errors import GlpiValidationError from glpi_python_client.models.api_schema.administration._user import ( DeleteUser, GetUser, @@ -110,6 +111,81 @@ async def iter_search_users( break start += batch_size + async def find_user_by_email( + self, + email: str, + *, + rsql_filter: str = "", + batch_size: int = 100, + skip_entity: bool = True, + ) -> GetUser | None: + """Return the first user holding ``email``, or ``None``. + + **This scans.** GLPI exposes e-mail addresses as ``User.emails``, a + nested *array*, and the v2 filter engine cannot join a nested array + -- the structurally identical ``Ticket.team`` answers HTTP 500 for + its declared subfields and is silently ignored for every other + spelling. So there is no server-side e-mail filter to use, and the + addresses have to be compared client-side. + + Nothing about that is cheap: the scan costs one request per + ``batch_size`` users until it matches, so it is meant for + occasional resolution, not for a per-request lookup. Narrow it with + ``rsql_filter`` when you can (``"is_active==true"`` is the usual + one), and cache the resulting id rather than calling this again. + + A server-side fast path is deliberately *not* attempted. GLPI v2 + ignores a filter field it does not recognise and answers with the + whole unfiltered table, so a guessed e-mail filter would not fail + -- it would return a plausible non-empty page whose first row is + the wrong person. Guessing is the one thing this helper exists to + stop each caller doing separately. + + Parameters + ---------- + email : str + Address to look for. Compared case-insensitively after + trimming surrounding whitespace, the way mail systems treat it. + rsql_filter : str, optional + Raw RSQL filter narrowing the population scanned. Empty by + default, which scans every visible user. + batch_size : int, optional + Users fetched per request while scanning (defaults to 100). + skip_entity : bool, optional + When ``True`` (the default) the ``GLPI-Entity`` header is + omitted so the scan spans every entity the caller can see. A + user whose account lives outside the client's configured entity + is invisible otherwise, and the helper would answer ``None`` + for somebody who exists. + + Returns + ------- + GetUser | None + The first user with a matching address, or ``None`` when the + scanned population holds none. + + Raises + ------ + GlpiValidationError + If ``email`` is blank -- which would otherwise scan the whole + directory and match nothing. + """ + + needle = email.strip().casefold() + if not needle: + raise GlpiValidationError("find_user_by_email requires a non-empty address") + + async for batch in self.iter_search_users( + rsql_filter, + batch_size=batch_size, + skip_entity=skip_entity, + ): + for user in batch: + for entry in user.emails or (): + if entry.email and entry.email.strip().casefold() == needle: + return user + return None + async def get_user(self, user_id: GlpiId) -> GetUser: """Fetch one GLPI user by identifier. diff --git a/glpi_python_client/_async/clients/api/administration/tests/test_user.py b/glpi_python_client/_async/clients/api/administration/tests/test_user.py index b378e05..a3bc1f6 100644 --- a/glpi_python_client/_async/clients/api/administration/tests/test_user.py +++ b/glpi_python_client/_async/clients/api/administration/tests/test_user.py @@ -192,3 +192,158 @@ async def fake_search( batches = [batch async for batch in client.iter_search_users("", batch_size=2)] assert call_count == 2 assert sum(len(b) for b in batches) == 3 + + +async def test_find_user_by_email_matches_a_nested_email_entry(client: Any) -> None: + """The address is matched inside the ``emails`` array, not on a top field.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + if start: + return [] + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "alice@x.test"}]} + ), + GetUser.model_validate( + {"id": 2, "username": "bob", "emails": [{"email": "bob@x.test"}]} + ), + ] + + client.search_users = fake_search # type: ignore[method-assign] + + found = await client.find_user_by_email("bob@x.test") + + assert found is not None + assert found.id == 2 + + +async def test_find_user_by_email_is_case_insensitive(client: Any) -> None: + """Addresses are compared case-insensitively, as mail systems treat them.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + if start: + return [] + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "Alice@X.test"}]} + ) + ] + + client.search_users = fake_search # type: ignore[method-assign] + + found = await client.find_user_by_email(" ALICE@x.TEST ") + + assert found is not None + assert found.id == 1 + + +async def test_find_user_by_email_returns_none_when_absent(client: Any) -> None: + """No match is ``None`` rather than an arbitrary first user.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + if start: + return [] + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "alice@x.test"}]} + ) + ] + + client.search_users = fake_search # type: ignore[method-assign] + + assert await client.find_user_by_email("nobody@x.test") is None + + +async def test_find_user_by_email_scans_past_the_first_page(client: Any) -> None: + """The match may live on any page, so the scan pages until it finds one.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + pages = [ + [GetUser.model_validate({"id": i, "username": f"u{i}"}) for i in range(1, 3)], + [ + GetUser.model_validate( + {"id": 9, "username": "zoe", "emails": [{"email": "zoe@x.test"}]} + ), + ], + ] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_users = fake_search # type: ignore[method-assign] + + found = await client.find_user_by_email("zoe@x.test", batch_size=2) + + assert found is not None + assert found.id == 9 + + +async def test_find_user_by_email_stops_at_the_matching_page(client: Any) -> None: + """The scan stops as soon as it matches instead of walking the directory.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + starts.append(start) + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "alice@x.test"}]} + ) + ] * limit + + client.search_users = fake_search # type: ignore[method-assign] + + await client.find_user_by_email("alice@x.test", batch_size=2) + + assert starts == [0] + + +async def test_find_user_by_email_scans_every_entity_by_default(client: Any) -> None: + """The scan spans entities, or a match outside the header scope is missed.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + seen: dict[str, Any] = {} + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + seen.update(kwargs) + seen["filter"] = rsql_filter + return [] + + client.search_users = fake_search # type: ignore[method-assign] + + await client.find_user_by_email("alice@x.test", rsql_filter="is_active==true") + + assert seen["skip_entity"] is True + assert seen["filter"] == "is_active==true" + + +async def test_find_user_by_email_rejects_a_blank_address(client: Any) -> None: + """A blank address would scan the whole directory and match nothing.""" + + from glpi_python_client import GlpiValidationError + + with pytest.raises(GlpiValidationError): + await client.find_user_by_email(" ") diff --git a/glpi_python_client/_async/clients/api/assistance/_team.py b/glpi_python_client/_async/clients/api/assistance/_team.py index 1d52375..16538fb 100644 --- a/glpi_python_client/_async/clients/api/assistance/_team.py +++ b/glpi_python_client/_async/clients/api/assistance/_team.py @@ -15,7 +15,6 @@ GlpiId, ) from glpi_python_client._async.clients.commons._http import ensure_response_status -from glpi_python_client._async.clients.commons._payloads import model_to_payload from glpi_python_client._async.clients.commons._transport import TransportMixin from glpi_python_client.models.api_schema.assistance._team import ( GetTeamMember, @@ -79,7 +78,7 @@ async def add_ticket_team_member( """ endpoint = f"{TICKET_ENDPOINT}/{ticket_id}/{TEAM_MEMBER_SUFFIX}" - response = await self._post_request(endpoint, model_to_payload(member)) + response = await self._post_request(endpoint, self._body(member)) ensure_response_status( response, success_statuses=(200, 201), @@ -114,7 +113,7 @@ async def remove_ticket_team_member( f"{TICKET_ENDPOINT}/{ticket_id}/{TEAM_MEMBER_SUFFIX}", failure_message=f"Failed to remove team member on ticket {ticket_id}", log_message=f"GLPI API removed team member on ticket {ticket_id}", - body=model_to_payload(member), + body=self._body(member), ) diff --git a/glpi_python_client/_async/clients/api/assistance/_ticket.py b/glpi_python_client/_async/clients/api/assistance/_ticket.py index e080333..dcad77a 100644 --- a/glpi_python_client/_async/clients/api/assistance/_ticket.py +++ b/glpi_python_client/_async/clients/api/assistance/_ticket.py @@ -49,7 +49,11 @@ async def search_tickets( start : int, optional Zero-based offset of the first record returned. sort : str | None, optional - ``sort`` query parameter forwarded as-is, e.g. ``"date_mod desc"``. + ``sort`` query parameter, spelled ``field`` or ``field:direction`` + (e.g. ``"date_mod:desc"``). Measured against GLPI 11: a space + before the direction answers **HTTP 400** ("Invalid property for + sorting"), a bare ``field`` sorts *ascending*, and a separate + ``order`` parameter is ignored. fields : tuple[str, ...], optional Restricted set of contract field names to request. Empty tuple lets the GLPI server pick its default field set. @@ -94,7 +98,9 @@ async def iter_search_tickets( the ``limit`` parameter on each underlying :meth:`search_tickets` call. sort : str | None, optional - ``sort`` query parameter forwarded as-is to each page request. + ``sort`` query parameter forwarded to each page request, + spelled ``field`` or ``field:direction`` (e.g. ``"date_mod:desc"``). + A space before the direction answers HTTP 400. fields : tuple[str, ...], optional Restricted set of contract field names to request. diff --git a/glpi_python_client/_async/clients/api/assistance/tests/test_ticket.py b/glpi_python_client/_async/clients/api/assistance/tests/test_ticket.py index 989cdcd..1675254 100644 --- a/glpi_python_client/_async/clients/api/assistance/tests/test_ticket.py +++ b/glpi_python_client/_async/clients/api/assistance/tests/test_ticket.py @@ -29,14 +29,14 @@ async def test_search_tickets_forwards_sort_and_fields(client: Any) -> None: rec = TransportRecorder(get_payload=[{"id": 1, "name": "n", "content": "c"}]) rec.install(client) tickets = await client.search_tickets( - "status==1", limit=5, start=10, sort="date_mod desc", fields=("id", "name") + "status==1", limit=5, start=10, sort="date_mod:desc", fields=("id", "name") ) assert len(tickets) == 1 assert rec.calls[0]["params"]["filter"] == "status==1" assert rec.calls[0]["params"]["limit"] == 5 assert rec.calls[0]["params"]["start"] == 10 - assert rec.calls[0]["params"]["sort"] == "date_mod desc" + assert rec.calls[0]["params"]["sort"] == "date_mod:desc" assert rec.calls[0]["params"]["fields"] == "id,name" diff --git a/glpi_python_client/_async/clients/api/dropdowns/_location.py b/glpi_python_client/_async/clients/api/dropdowns/_location.py index 8cc9916..8698fe4 100644 --- a/glpi_python_client/_async/clients/api/dropdowns/_location.py +++ b/glpi_python_client/_async/clients/api/dropdowns/_location.py @@ -7,6 +7,8 @@ from __future__ import annotations +from collections.abc import AsyncIterator + from glpi_python_client._async.clients.commons._constants import ( LOCATION_ENDPOINT, GlpiId, @@ -52,6 +54,48 @@ async def search_locations( params["filter"] = rsql_filter return await self._resource_list(LOCATION_ENDPOINT, GetLocation, params=params) + async def iter_search_locations( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + ) -> AsyncIterator[list[GetLocation]]: + """Yield successive pages of GLPI locations until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_locations` + call. + + Yields + ------ + list[GetLocation] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = await self.search_locations( + rsql_filter, + limit=batch_size, + start=start, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + async def get_location(self, location_id: GlpiId) -> GetLocation: """Fetch one GLPI location by identifier. diff --git a/glpi_python_client/_async/clients/api/dropdowns/tests/test_location.py b/glpi_python_client/_async/clients/api/dropdowns/tests/test_location.py index bff93c9..d5d7957 100644 --- a/glpi_python_client/_async/clients/api/dropdowns/tests/test_location.py +++ b/glpi_python_client/_async/clients/api/dropdowns/tests/test_location.py @@ -126,3 +126,64 @@ async def test_delete_helpers_raise_on_failure_status( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): await call(client) + + +async def test_iter_search_locations_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.dropdowns import GetLocation + + pages = [[GetLocation(id=i) for i in range(3)], [GetLocation(id=99)]] + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetLocation]: + starts.append(start) + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_locations = fake_search # type: ignore[method-assign] + + batches = [ + batch async for batch in client.iter_search_locations("name==x", batch_size=3) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + + +async def test_iter_search_locations_stops_on_a_single_short_page(client: Any) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.dropdowns import GetLocation + + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetLocation]: + starts.append(start) + return [GetLocation(id=1)] + + client.search_locations = fake_search # type: ignore[method-assign] + + batches = [batch async for batch in client.iter_search_locations(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +async def test_iter_search_locations_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.dropdowns import GetLocation + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetLocation]: + return [] + + client.search_locations = fake_search # type: ignore[method-assign] + + assert [batch async for batch in client.iter_search_locations()] == [] diff --git a/glpi_python_client/_async/clients/api/knowledgebase/_article.py b/glpi_python_client/_async/clients/api/knowledgebase/_article.py index 19dac76..1929eb7 100644 --- a/glpi_python_client/_async/clients/api/knowledgebase/_article.py +++ b/glpi_python_client/_async/clients/api/knowledgebase/_article.py @@ -8,7 +8,7 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import AsyncIterator, Sequence from glpi_python_client._async.clients.commons._constants import ( KB_ARTICLE_ENDPOINT, @@ -50,7 +50,11 @@ async def search_kb_articles( start : int, optional Zero-based offset of the first record returned. sort : str | None, optional - ``sort`` query parameter forwarded as-is. + ``sort`` query parameter, spelled ``field`` or ``field:direction`` + (e.g. ``"date_mod:desc"``). Measured against GLPI 11: a space + before the direction answers **HTTP 400** ("Invalid property for + sorting"), a bare ``field`` sorts *ascending*, and a separate + ``order`` parameter is ignored. language : str | None, optional GLPI language code forwarded as the ``language`` query parameter to select a translated view. @@ -72,6 +76,59 @@ async def search_kb_articles( KB_ARTICLE_ENDPOINT, GetKBArticle, params=params ) + async def iter_search_kb_articles( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + sort: str | None = None, + language: str | None = None, + ) -> AsyncIterator[list[GetKBArticle]]: + """Yield successive pages of GLPI knowledge base articles until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_kb_articles` + call. + sort : str | None, optional + ``sort`` query parameter forwarded to each page request, + spelled ``field`` or ``field:direction`` (e.g. ``"date_mod:desc"``). + A space before the direction answers HTTP 400. + language : str | None, optional + GLPI language code forwarded to each page request to select + a translated view. + + Yields + ------ + list[GetKBArticle] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = await self.search_kb_articles( + rsql_filter, + limit=batch_size, + start=start, + sort=sort, + language=language, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + async def get_kb_article(self, article_id: GlpiId) -> GetKBArticle: """Fetch one knowledge base article by identifier. diff --git a/glpi_python_client/_async/clients/api/knowledgebase/_category.py b/glpi_python_client/_async/clients/api/knowledgebase/_category.py index c1a063b..10453c4 100644 --- a/glpi_python_client/_async/clients/api/knowledgebase/_category.py +++ b/glpi_python_client/_async/clients/api/knowledgebase/_category.py @@ -7,6 +7,8 @@ from __future__ import annotations +from collections.abc import AsyncIterator + from glpi_python_client._async.clients.commons._constants import ( KB_CATEGORY_ENDPOINT, GlpiId, @@ -43,7 +45,11 @@ async def search_kb_categories( start : int, optional Zero-based offset of the first record returned. sort : str | None, optional - ``sort`` query parameter forwarded as-is, e.g. ``"name asc"``. + ``sort`` query parameter, spelled ``field`` or ``field:direction`` + (e.g. ``"name:asc"``). Measured against GLPI 11: a space + before the direction answers **HTTP 400** ("Invalid property for + sorting"), a bare ``field`` sorts *ascending*, and a separate + ``order`` parameter is ignored. language : str | None, optional GLPI language code forwarded as the ``language`` query parameter to select a translated view. @@ -65,6 +71,59 @@ async def search_kb_categories( KB_CATEGORY_ENDPOINT, GetKBCategory, params=params ) + async def iter_search_kb_categories( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + sort: str | None = None, + language: str | None = None, + ) -> AsyncIterator[list[GetKBCategory]]: + """Yield successive pages of GLPI knowledge base categories until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_kb_categories` + call. + sort : str | None, optional + ``sort`` query parameter forwarded to each page request, + spelled ``field`` or ``field:direction`` (e.g. ``"date_mod:desc"``). + A space before the direction answers HTTP 400. + language : str | None, optional + GLPI language code forwarded to each page request to select + a translated view. + + Yields + ------ + list[GetKBCategory] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = await self.search_kb_categories( + rsql_filter, + limit=batch_size, + start=start, + sort=sort, + language=language, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + async def get_kb_category(self, category_id: GlpiId) -> GetKBCategory: """Fetch one knowledge base category by identifier. diff --git a/glpi_python_client/_async/clients/api/knowledgebase/tests/test_article.py b/glpi_python_client/_async/clients/api/knowledgebase/tests/test_article.py index 4a22076..7f7f4bd 100644 --- a/glpi_python_client/_async/clients/api/knowledgebase/tests/test_article.py +++ b/glpi_python_client/_async/clients/api/knowledgebase/tests/test_article.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Callable +from datetime import timedelta from typing import Any import pytest @@ -13,7 +14,10 @@ PatchKBArticle, PostKBArticle, ) -from glpi_python_client._async._testing import FailingTransportRecorder +from glpi_python_client._async._testing import ( + FailingTransportRecorder, + TransportRecorder, +) from glpi_python_client.testing.utils import FakeResponse @@ -111,7 +115,7 @@ async def test_search_kb_articles_forwards_params(client: Any) -> None: rec = _Recorder(get_payload=[{"id": 1, "name": "Reset", "content": "

c

"}]) rec.install(client) result = await client.search_kb_articles( - "is_faq==1", limit=3, start=1, sort="date_mod desc", language="en_GB" + "is_faq==1", limit=3, start=1, sort="date_mod:desc", language="en_GB" ) assert result[0].id == 1 call = rec.calls[0] @@ -119,7 +123,7 @@ async def test_search_kb_articles_forwards_params(client: Any) -> None: assert call["params"]["filter"] == "is_faq==1" assert call["params"]["limit"] == 3 assert call["params"]["start"] == 1 - assert call["params"]["sort"] == "date_mod desc" + assert call["params"]["sort"] == "date_mod:desc" assert call["params"]["language"] == "en_GB" @@ -345,3 +349,119 @@ async def test_delete_helpers_raise_on_failure( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): await call(client) + + +async def test_iter_search_kb_articles_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBArticle + + pages = [[GetKBArticle(id=i) for i in range(3)], [GetKBArticle(id=99)]] + starts: list[int] = [] + forwarded: dict[str, Any] = {} + + async def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBArticle]: + starts.append(start) + forwarded["sort"] = sort + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_kb_articles = fake_search # type: ignore[method-assign] + + batches = [ + batch + async for batch in client.iter_search_kb_articles( + "name==x", batch_size=3, sort="name:asc" + ) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + assert forwarded["sort"] == "name:asc" + + +async def test_iter_search_kb_articles_stops_on_a_single_short_page( + client: Any, +) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBArticle + + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBArticle]: + starts.append(start) + return [GetKBArticle(id=1)] + + client.search_kb_articles = fake_search # type: ignore[method-assign] + + batches = [batch async for batch in client.iter_search_kb_articles(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +async def test_iter_search_kb_articles_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBArticle + + async def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBArticle]: + return [] + + client.search_kb_articles = fake_search # type: ignore[method-assign] + + assert [batch async for batch in client.iter_search_kb_articles()] == [] + + +async def test_get_kb_article_localises_the_naive_revision_date(client: Any) -> None: + """A KB article's revision dates come back comparable with its own. + + This is the shape GLPI 11 actually sends, measured on a live instance: + the article's own timestamps carry an offset and the nested revision + dates do not. Before the server timezone was threaded through, sorting + an article's history against the article itself raised + ``TypeError: can't compare offset-naive and offset-aware datetimes``. + """ + + rec = TransportRecorder( + get_payload={ + "id": 1, + "name": "article", + "date_creation": "2018-04-06T17:38:15+02:00", + "revisions": [{"id": 9, "date": "2018-04-06 17:39:44"}], + } + ) + rec.install(client) + + article = await client.get_kb_article(1) + + assert article.revisions is not None + revision_date = article.revisions[0].date + assert revision_date is not None + assert revision_date.tzinfo is not None + # The comparison itself is the regression: it used to raise. + assert article.date_creation is not None + assert revision_date > article.date_creation + assert revision_date - article.date_creation == timedelta(seconds=89) diff --git a/glpi_python_client/_async/clients/api/knowledgebase/tests/test_category.py b/glpi_python_client/_async/clients/api/knowledgebase/tests/test_category.py index 4fe4cea..b82f570 100644 --- a/glpi_python_client/_async/clients/api/knowledgebase/tests/test_category.py +++ b/glpi_python_client/_async/clients/api/knowledgebase/tests/test_category.py @@ -73,7 +73,7 @@ async def test_search_kb_categories_forwards_filter_and_language(client: Any) -> rec = _Recorder(get_payload=[{"id": 1, "name": "Network"}]) rec.install(client) result = await client.search_kb_categories( - "name==Network", limit=5, start=2, sort="name asc", language="fr_FR" + "name==Network", limit=5, start=2, sort="name:asc", language="fr_FR" ) assert result[0].id == 1 call = rec.calls[0] @@ -81,7 +81,7 @@ async def test_search_kb_categories_forwards_filter_and_language(client: Any) -> assert call["params"]["filter"] == "name==Network" assert call["params"]["limit"] == 5 assert call["params"]["start"] == 2 - assert call["params"]["sort"] == "name asc" + assert call["params"]["sort"] == "name:asc" assert call["params"]["language"] == "fr_FR" @@ -183,3 +183,87 @@ async def test_delete_helpers_raise_on_failure( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): await call(client) + + +async def test_iter_search_kb_categories_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBCategory + + pages = [[GetKBCategory(id=i) for i in range(3)], [GetKBCategory(id=99)]] + starts: list[int] = [] + forwarded: dict[str, Any] = {} + + async def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBCategory]: + starts.append(start) + forwarded["language"] = language + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_kb_categories = fake_search # type: ignore[method-assign] + + batches = [ + batch + async for batch in client.iter_search_kb_categories( + "name==x", batch_size=3, language="fr_FR" + ) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + assert forwarded["language"] == "fr_FR" + + +async def test_iter_search_kb_categories_stops_on_a_single_short_page( + client: Any, +) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBCategory + + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBCategory]: + starts.append(start) + return [GetKBCategory(id=1)] + + client.search_kb_categories = fake_search # type: ignore[method-assign] + + batches = [batch async for batch in client.iter_search_kb_categories(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +async def test_iter_search_kb_categories_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBCategory + + async def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBCategory]: + return [] + + client.search_kb_categories = fake_search # type: ignore[method-assign] + + assert [batch async for batch in client.iter_search_kb_categories()] == [] diff --git a/glpi_python_client/_async/clients/api/management/_document.py b/glpi_python_client/_async/clients/api/management/_document.py index 11a32bb..0c3d1aa 100644 --- a/glpi_python_client/_async/clients/api/management/_document.py +++ b/glpi_python_client/_async/clients/api/management/_document.py @@ -8,6 +8,7 @@ from __future__ import annotations import logging +from collections.abc import AsyncIterator from glpi_python_client._async.clients.commons._constants import ( DOCUMENT_ENDPOINT, @@ -63,6 +64,48 @@ async def search_documents( DOCUMENT_ENDPOINT, GetDocument, params=params, skip_entity=True ) + async def iter_search_documents( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + ) -> AsyncIterator[list[GetDocument]]: + """Yield successive pages of GLPI documents until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_documents` + call. + + Yields + ------ + list[GetDocument] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = await self.search_documents( + rsql_filter, + limit=batch_size, + start=start, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + async def get_document(self, document_id: GlpiId) -> GetDocument: """Fetch one GLPI document by identifier. @@ -214,6 +257,55 @@ async def download_document_content(self, document_id: GlpiId) -> bytes: ) return response.content + async def stream_document_content( + self, + document_id: GlpiId, + *, + chunk_size: int = 65536, + ) -> AsyncIterator[bytes]: + """Stream the binary payload of one GLPI document in chunks. + + Use this instead of :meth:`download_document_content` when the file + may be large: that method holds the whole body in memory before + returning, so a 500 MB attachment costs 500 MB of process memory + even if the caller only writes it straight to disk. + + Parameters + ---------- + document_id : GlpiId + Numeric identifier of the document whose binary content is + requested. + chunk_size : int, optional + Bytes requested per chunk (defaults to 64 KiB). + + Yields + ------ + bytes + Successive chunks of the document body. The final chunk may be + shorter than ``chunk_size``. + + Raises + ------ + GlpiStatusError + If the GLPI server returns a non-success HTTP status. + + Examples + -------- + Writing a document to disk without buffering it:: + + with open("attachment.pdf", "wb") as handle: + async for chunk in client.stream_document_content(42): + handle.write(chunk) + """ + + async for chunk in self._stream_request( + f"{DOCUMENT_ENDPOINT}/{document_id}/Download", + chunk_size=chunk_size, + skip_entity=True, + failure_message=f"Failed to download document {document_id}", + ): + yield chunk + async def upload_document( self, *, diff --git a/glpi_python_client/_async/clients/api/management/tests/test_document.py b/glpi_python_client/_async/clients/api/management/tests/test_document.py index 4a1bd31..1e7f1a1 100644 --- a/glpi_python_client/_async/clients/api/management/tests/test_document.py +++ b/glpi_python_client/_async/clients/api/management/tests/test_document.py @@ -9,7 +9,7 @@ from __future__ import annotations from collections.abc import Callable -from typing import Any +from typing import Any, ClassVar import pytest @@ -224,3 +224,146 @@ async def test_delete_helpers_raise_on_failure_status( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): await call(client) + + +async def test_iter_search_documents_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.management import GetDocument + + pages = [[GetDocument(id=i) for i in range(3)], [GetDocument(id=99)]] + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetDocument]: + starts.append(start) + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_documents = fake_search # type: ignore[method-assign] + + batches = [ + batch async for batch in client.iter_search_documents("name==x", batch_size=3) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + + +async def test_iter_search_documents_stops_on_a_single_short_page(client: Any) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.management import GetDocument + + starts: list[int] = [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetDocument]: + starts.append(start) + return [GetDocument(id=1)] + + client.search_documents = fake_search # type: ignore[method-assign] + + batches = [batch async for batch in client.iter_search_documents(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +async def test_iter_search_documents_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.management import GetDocument + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetDocument]: + return [] + + client.search_documents = fake_search # type: ignore[method-assign] + + assert [batch async for batch in client.iter_search_documents()] == [] + + +def _pin_token(client: Any) -> None: + """Pretend a valid token is held so no OAuth round trip happens. + + The transport recorders stub above ``_ensure_token``; the streaming + helper runs through it, so the token has to be supplied here instead. + """ + + from datetime import datetime, timedelta, timezone + + client._auth.access_token = "stub-token" + client._auth.token_expires_at = datetime.now(tz=timezone.utc) + timedelta(days=1) + + +async def test_stream_document_content_yields_chunks(client: Any) -> None: + """The generator yields the body in chunks instead of buffering it.""" + + seen: dict[str, Any] = {} + + class _Stream: + async def __aenter__(self) -> Any: + return self + + async def __aexit__(self, *exc: Any) -> None: + return None + + status_code = 200 + headers: ClassVar[dict[str, str]] = {} + + async def aread(self) -> bytes: + return b"" + + async def aiter_bytes(self, chunk_size: int | None = None) -> Any: + seen["chunk_size"] = chunk_size + for chunk in (b"abc", b"def"): + yield chunk + + def _stream(method: str, url: str, **kwargs: Any) -> Any: + seen["method"] = method + seen["url"] = url + return _Stream() + + _pin_token(client) + client._session.stream = _stream # type: ignore[method-assign] + + chunks = [c async for c in client.stream_document_content(7, chunk_size=4)] + + assert chunks == [b"abc", b"def"] + assert seen["method"] == "GET" + assert seen["url"].endswith("Management/Document/7/Download") + assert seen["chunk_size"] == 4 + + +async def test_stream_document_content_raises_on_error_status(client: Any) -> None: + """A non-success status raises rather than yielding an error page as bytes.""" + + class _Stream: + async def __aenter__(self) -> Any: + return self + + async def __aexit__(self, *exc: Any) -> None: + return None + + status_code = 404 + headers: ClassVar[dict[str, str]] = {} + url = "https://glpi.example.test/Management/Document/7/Download" + text = "not found" + + async def aread(self) -> bytes: + return b"not found" + + async def aiter_bytes(self, chunk_size: int | None = None) -> Any: + yield b"not found" + + _pin_token(client) + client._session.stream = lambda method, url, **kw: _Stream() # type: ignore[method-assign] + + from glpi_python_client import GlpiStatusError + + with pytest.raises(GlpiStatusError): + [c async for c in client.stream_document_content(7)] diff --git a/glpi_python_client/_async/clients/api/plugins/_fields.py b/glpi_python_client/_async/clients/api/plugins/_fields.py index 616e953..3a25c93 100644 --- a/glpi_python_client/_async/clients/api/plugins/_fields.py +++ b/glpi_python_client/_async/clients/api/plugins/_fields.py @@ -33,6 +33,7 @@ import json from typing import Any +from glpi_python_client._async.clients.commons._payloads import model_from_payload from glpi_python_client._async.clients.commons._transport import TransportMixin from glpi_python_client._errors import GlpiProtocolError, GlpiValidationError from glpi_python_client.models.api_schema.plugins import ( @@ -147,7 +148,12 @@ async def list_plugin_fields_containers( failure_message="Failed to list PluginFieldsContainer", ) rows = payload if isinstance(payload, list) else [] - containers = [GetPluginFieldsContainer.model_validate(row) for row in rows] + containers = [ + model_from_payload( + GetPluginFieldsContainer, row, server_timezone=self.server_timezone + ) + for row in rows + ] if itemtype is None: return containers return [c for c in containers if _container_targets_itemtype(c, itemtype)] @@ -179,7 +185,12 @@ async def list_plugin_fields_fields( failure_message="Failed to list PluginFieldsField", ) rows = payload if isinstance(payload, list) else [] - fields = [GetPluginFieldsField.model_validate(row) for row in rows] + fields = [ + model_from_payload( + GetPluginFieldsField, row, server_timezone=self.server_timezone + ) + for row in rows + ] if container_id is None: return fields return [f for f in fields if f.plugin_fields_containers_id == container_id] @@ -219,7 +230,12 @@ async def list_item_plugin_field_rows( failure_message=f"Failed to list {endpoint}", ) rows = payload if isinstance(payload, list) else [] - return [GetPluginFieldsValueRow.model_validate(row) for row in rows] + return [ + model_from_payload( + GetPluginFieldsValueRow, row, server_timezone=self.server_timezone + ) + for row in rows + ] async def create_item_plugin_field_row( self, diff --git a/glpi_python_client/_async/clients/commons/_config.py b/glpi_python_client/_async/clients/commons/_config.py index 8a7300e..0236345 100644 --- a/glpi_python_client/_async/clients/commons/_config.py +++ b/glpi_python_client/_async/clients/commons/_config.py @@ -9,7 +9,9 @@ from collections.abc import Mapping from dataclasses import dataclass +from datetime import tzinfo from typing import TYPE_CHECKING, Protocol +from zoneinfo import ZoneInfo, ZoneInfoNotFoundError import httpx @@ -183,6 +185,63 @@ def build_client_resources( ) +def resolve_server_timezone(value: object) -> tzinfo: + """Resolve the configured GLPI server timezone into a ``tzinfo``. + + The timezone is **required** and has no default. GLPI does not + advertise it anywhere in the API, so only the operator knows it, and + every candidate default is wrong somewhere: guessing UTC against a + Europe/Paris instance shifts every naive timestamp by one or two hours + *without raising*, which is worse than the ``TypeError`` a naive value + produces on comparison. + + An IANA name is preferred over a fixed offset because a fixed offset + cannot follow DST, and a single GLPI instance demonstrably emits both + ``+01:00`` and ``+02:00`` depending on the date. + + Parameters + ---------- + value : object + An IANA zone name (``"Europe/Paris"``) or a ``tzinfo`` instance. + + Returns + ------- + tzinfo + The resolved timezone. + + Raises + ------ + GlpiValidationError + When the value is missing, blank, not a string or ``tzinfo``, or + names a zone the system database does not know. + """ + + if isinstance(value, tzinfo): + return value + if value is None: + raise GlpiValidationError( + "server_timezone is required: GLPI does not advertise its own " + "timezone, so it has to be declared (e.g. 'Europe/Paris', or " + "GLPI_SERVER_TIMEZONE in the environment)." + ) + if not isinstance(value, str): + raise GlpiValidationError( + f"server_timezone must be an IANA name or a tzinfo; got {value!r}" + ) + name = value.strip() + if not name: + raise GlpiValidationError( + "server_timezone is empty: set it to an IANA name (e.g. 'Europe/Paris')." + ) + try: + return ZoneInfo(name) + except (ZoneInfoNotFoundError, ValueError) as exc: + raise GlpiValidationError( + f"Unknown server_timezone {name!r}. Use an IANA zone name such as " + "'Europe/Paris' or 'UTC'." + ) from exc + + def parse_optional_env_int(value: object) -> int | None: """Parse one optional integer from an environment-style value. @@ -248,6 +307,7 @@ def build_client_env_config( config: dict[str, object] = { "glpi_api_url": env.get(f"{prefix}API_URL"), + "server_timezone": env.get(f"{prefix}SERVER_TIMEZONE"), "client_id": env.get(f"{prefix}CLIENT_ID"), "client_secret": env.get(f"{prefix}CLIENT_SECRET"), "username": env.get(f"{prefix}USERNAME"), @@ -313,5 +373,6 @@ def validate_v1_document_config( "normalize_client_api_url", "parse_optional_env_bool", "parse_optional_env_int", + "resolve_server_timezone", "validate_v1_document_config", ] diff --git a/glpi_python_client/_async/clients/commons/_filters.py b/glpi_python_client/_async/clients/commons/_filters.py index 12f9b49..cdf31bc 100644 --- a/glpi_python_client/_async/clients/commons/_filters.py +++ b/glpi_python_client/_async/clients/commons/_filters.py @@ -4,6 +4,30 @@ for GLPI endpoints that accept RSQL-like query expressions. All functions return ``None`` when the supplied input is empty so callers can compose filters without sprinkling conditional blocks at every call site. + +Two properties of the v2 filter engine shape everything here, and both +fail *open*: + +* **An unrecognised filter field is ignored, not rejected.** The query + then returns the entire unfiltered table with a 200. A typo in a field + name does not raise -- it succeeds and answers with far too many rows, + which is why a filter that returns results is not evidence the filter + was applied. When validating a new expression against a live instance, + check that it returns *fewer* rows than the unfiltered baseline. +* **``;`` (AND) binds tighter than ``,`` (OR)**, so an unparenthesised + OR group silently drops every preceding AND clause for all but its + first alternative. See :func:`rsql_any_filter`, which exists for that + reason and carries the measured numbers. + +Nested *object* subfields can be joined (``status.id==1``, +``entity.id==3``). Nested *arrays* cannot: ``Ticket.team`` answers HTTP +500 for its contract-declared subfields and is silently ignored for every +other spelling, so actor selection has to go through the v1 search engine. +The same is expected of ``User.emails``. + +Date windows live on the public surface instead, in +:mod:`glpi_python_client.rsql`, because their grammar has an end-of-day +detail that is easy to get wrong and impossible to notice. """ from __future__ import annotations diff --git a/glpi_python_client/_async/clients/commons/_http.py b/glpi_python_client/_async/clients/commons/_http.py index 765cb40..7f75887 100644 --- a/glpi_python_client/_async/clients/commons/_http.py +++ b/glpi_python_client/_async/clients/commons/_http.py @@ -310,6 +310,45 @@ def response_json_mapping(response: httpx.Response) -> Mapping[str, object]: return result if isinstance(result, Mapping) else {} +def coerce_response_int(value: object) -> int | None: + """Return ``value`` as an ``int`` when it plainly denotes one. + + Native integers pass through. A string is accepted when it parses + cleanly, because GLPI is PHP-backed and PHP-backed APIs routinely + render integers as strings; refusing one would be a protocol error + raised over a perfectly usable identifier. ``bool`` is rejected even + though it is an ``int`` subclass in Python, and floats are rejected + because a fractional identifier is a sign of a misread payload rather + than a value to round. + """ + + if isinstance(value, bool): + return None + if isinstance(value, int): + return value + if isinstance(value, str): + try: + return int(value.strip()) + except ValueError: + return None + return None + + +def response_id_from_location(response: httpx.Response) -> int | None: + """Return the trailing identifier of the response ``Location`` header. + + A create that answers with an empty body and the URL of the new + resource is an ordinary REST shape. Reading the last path segment + recovers the identifier instead of failing a request that succeeded. + Returns ``None`` when there is no header or its tail is not numeric. + """ + + location = response.headers.get("Location") or response.headers.get("location") + if not location: + return None + return coerce_response_int(location.rstrip("/").rsplit("/", 1)[-1]) + + def require_response_int( response: httpx.Response, *, @@ -322,17 +361,35 @@ def require_response_int( set of keys. Callers list the candidate keys explicitly so the behaviour stays predictable. + Three shapes beyond a plain top-level integer are accepted, because + each is a response the server considers successful and each cost the + caller the identifier of a record that already exists: a numeric + string, an id nested one level under ``data``, and an empty body + whose ``Location`` header names the new resource. Top-level keys are + probed before nested ones, and the header is consulted last. + Raises ------ GlpiProtocolError - When none of ``keys`` maps to an integer value in the response. + When no candidate key and no ``Location`` header yields an + identifier. """ result = response_json_mapping(response) - for key in keys: - value = result.get(key) - if isinstance(value, int) and not isinstance(value, bool): - return value + candidates: list[Mapping[str, object]] = [result] + nested = result.get("data") + if isinstance(nested, Mapping): + candidates.append(nested) + + for mapping in candidates: + for key in keys: + found = coerce_response_int(mapping.get(key)) + if found is not None: + return found + + from_location = response_id_from_location(response) + if from_location is not None: + return from_location raise GlpiProtocolError(missing_message) @@ -380,6 +437,7 @@ def unwrap_timeline_items(payload: object) -> list[dict[str, object]]: __all__ = [ "build_request_headers", "build_request_url", + "coerce_response_int", "ensure_response_status", "finalize_request_response", "list_payload_items", @@ -387,6 +445,7 @@ def unwrap_timeline_items(payload: object) -> list[dict[str, object]]: "request_params", "require_access_token", "require_response_int", + "response_id_from_location", "response_json_mapping", "response_json_or_empty", "transport_error_from", diff --git a/glpi_python_client/_async/clients/commons/_payloads.py b/glpi_python_client/_async/clients/commons/_payloads.py index 15e24b3..69e1232 100644 --- a/glpi_python_client/_async/clients/commons/_payloads.py +++ b/glpi_python_client/_async/clients/commons/_payloads.py @@ -7,36 +7,89 @@ from __future__ import annotations +from datetime import tzinfo from typing import TypeVar -from glpi_python_client.models._base import GlpiModel +from glpi_python_client.models._base import ( + SERVER_TIMEZONE_CONTEXT_KEY, + GlpiModel, +) ModelT = TypeVar("ModelT", bound=GlpiModel) -def model_to_payload(model: GlpiModel) -> dict[str, object]: +def model_to_payload( + model: GlpiModel, + *, + server_timezone: tzinfo | None = None, +) -> dict[str, object]: """Serialise one :class:`GlpiModel` into a request body. ``None`` fields are omitted, the meta ``extra_payload`` field is excluded from the dump, and any user-provided ``extra_payload`` keys are merged on top so callers can inject contract-validated extras the package does not yet model. + + The dump runs in JSON mode. The returned mapping is handed to the HTTP + library as a JSON body, and its encoder is :func:`json.dumps`, which + cannot represent a ``datetime`` -- python mode leaves those as live + objects and every write of a date field then fails at the encoder, past + the point any transport stub can see. JSON mode renders them as ISO-8601 + strings instead, so what the model validated is what GLPI receives. + + ``server_timezone`` is passed as a Pydantic serialisation context, the + mirror of the validation context in :func:`model_from_payload` and + threaded the same way, so it reaches nested submodels. It is needed + because JSON mode alone renders an aware datetime *with* its offset, and + GLPI 11 does not read that offset: it takes the naive prefix, interprets + it in the server's own timezone, and discards the rest -- measured across + offsets from ``-08:00`` to ``+14:00``, which all stored the same moment. + The context lets the value be converted onto that clock first -- see + :meth:`GlpiModel._render_datetimes_on_the_server_clock`. ``None`` + converts nothing, so a model dumped outside the client is unchanged. """ - body = model.model_dump(exclude_none=True, exclude={"extra_payload"}) + context = ( + {SERVER_TIMEZONE_CONTEXT_KEY: server_timezone} + if server_timezone is not None + else None + ) + body = model.model_dump( + mode="json", + exclude_none=True, + exclude={"extra_payload"}, + context=context, + ) if model.extra_payload: body.update(model.extra_payload) return body -def model_from_payload(model_class: type[ModelT], payload: object) -> ModelT: +def model_from_payload( + model_class: type[ModelT], + payload: object, + *, + server_timezone: tzinfo | None = None, +) -> ModelT: """Validate one raw GLPI payload into the requested ``GlpiModel`` class. - The helper is a thin wrapper around ``model_validate`` that keeps the - mixin call sites concise and consistent with :func:`model_to_payload`. + The helper wraps ``model_validate`` so the mixin call sites stay concise + and consistent with :func:`model_to_payload`, and so the server timezone + is threaded through one place instead of forty field declarations. + + ``server_timezone`` is passed as a Pydantic validation context, which + reaches nested submodels as well as the top-level one -- necessary + because the timestamps GLPI sends without an offset are nested + (``KBArticle.revisions[].date``). ``None`` supplies no context at all, + so naive values stay naive rather than being stamped with a guess. """ - return model_class.model_validate(payload) + context = ( + {SERVER_TIMEZONE_CONTEXT_KEY: server_timezone} + if server_timezone is not None + else None + ) + return model_class.model_validate(payload, context=context) __all__ = ["model_from_payload", "model_to_payload"] diff --git a/glpi_python_client/_async/clients/commons/_transport.py b/glpi_python_client/_async/clients/commons/_transport.py index 4303eea..a0706c3 100644 --- a/glpi_python_client/_async/clients/commons/_transport.py +++ b/glpi_python_client/_async/clients/commons/_transport.py @@ -25,7 +25,8 @@ from __future__ import annotations import logging -from collections.abc import Callable +from collections.abc import AsyncIterator, Callable +from datetime import tzinfo from typing import TYPE_CHECKING, Any, TypeVar import httpx @@ -108,6 +109,7 @@ class TransportMixin: glpi_entity: int | None glpi_profile: int | None language: str + server_timezone: tzinfo def _ensure_open(self) -> None: """Raise when the client has already been closed. @@ -192,6 +194,65 @@ async def _send_request( except httpx.HTTPError as exc: raise transport_error_from(exc, method=method, url=url) from exc + async def _stream_request( + self, + endpoint: str, + *, + chunk_size: int, + skip_entity: bool = False, + failure_message: str, + ) -> AsyncIterator[bytes]: + """Stream one authenticated GLPI ``GET`` body in chunks. + + The non-streaming helpers materialise the whole body before the + caller sees any of it, which is fine for JSON and wrong for a + document that may be hundreds of megabytes. + + Two details differ from the buffered path and both are load-bearing. + The status has to be checked *inside* the context manager, and the + body read first: the error helpers format the response text, and + reading text off an unread stream raises rather than reporting the + status. And no retry decorator belongs here -- tenacity does not + wrap an async generator, so a decorator would silently degrade to + the sync path instead of failing loudly. + + Raises + ------ + GlpiStatusError + When the response status is not 200. + GlpiTransportError + When the request never produced a response. + """ + + await self._ensure_token() + access_token = require_access_token(self._auth.access_token) + url = build_request_url(self.glpi_api_url, endpoint) + headers = build_request_headers( + access_token=access_token, + language=self.language, + glpi_entity=self.glpi_entity, + glpi_profile=self.glpi_profile, + entity_recursive=self.entity_recursive, + include_content_type=False, + skip_entity=skip_entity, + ) + + try: + async with self._session.stream( + "GET", url, headers=headers, timeout=30 + ) as response: + if response.status_code != 200: + await response.aread() + ensure_response_status( + response, + success_statuses=(200,), + failure_message=failure_message, + ) + async for chunk in response.aiter_bytes(chunk_size): + yield chunk + except httpx.HTTPError as exc: + raise transport_error_from(exc, method="get", url=url) from exc + async def _execute_request( self, *, @@ -354,9 +415,9 @@ async def _resource_list( skip_entity : bool, optional When ``True`` the ``GLPI-Entity`` header is omitted. failure_message : str | None, optional - When provided, response status is checked with this message; - search-style endpoints that tolerate empty results pass - ``None``. + Message embedded in the raised ``GlpiStatusError``. ``None`` + derives one from the endpoint; the status is checked either + way. success_statuses : tuple[int, ...], optional HTTP status codes considered successful when ``failure_message`` is set. @@ -373,19 +434,28 @@ async def _resource_list( response = await self._get_request( endpoint, params=params, skip_entity=skip_entity ) - if failure_message is not None: - ensure_response_status( - response, - success_statuses=success_statuses, - failure_message=failure_message, - ) + # The status is checked on every list call, search included. It used + # not to be, and a refused search then came back as ``[]``: a 403 was + # indistinguishable from a filter that matched nothing. It composed + # badly with the batch iterators, which stop on a page shorter than + # ``batch_size`` -- so a 403 on page one ended the walk having + # yielded nothing and the caller saw a successful empty result. An + # empty list now means the server said the result set is empty. + ensure_response_status( + response, + success_statuses=success_statuses, + failure_message=failure_message or f"Failed to list {endpoint}", + ) payload = response.json() items = ( unwrap_timeline_items(payload) if unwrap_envelope else list_payload_items(payload) ) - return [model_from_payload(model, item) for item in items] + return [ + model_from_payload(model, item, server_timezone=self.server_timezone) + for item in items + ] async def _resource_get( self, @@ -421,7 +491,21 @@ async def _resource_get( success_statuses=(200, 206), failure_message=failure_message, ) - return model_from_payload(model, response.json()) + return model_from_payload( + model, response.json(), server_timezone=self.server_timezone + ) + + def _body(self, model: GlpiModel) -> dict[str, object]: + """Serialise one model into a request body on the server's clock. + + Every write in the package goes through here rather than calling + :func:`model_to_payload` directly, because the timezone argument is + not optional in practice and a call site that forgets it produces no + error -- just a timestamp GLPI silently reinterprets. Binding it once + leaves nothing to remember at the next endpoint. + """ + + return model_to_payload(model, server_timezone=self.server_timezone) async def _resource_create( self, @@ -466,7 +550,7 @@ async def _resource_create( """ response = await self._post_request( - endpoint, model_to_payload(body_model), skip_entity=skip_entity + endpoint, self._body(body_model), skip_entity=skip_entity ) ensure_response_status( response, @@ -507,7 +591,7 @@ async def _resource_update( None """ - response = await self._update_request(endpoint, model_to_payload(body_model)) + response = await self._update_request(endpoint, self._body(body_model)) ensure_response_status( response, success_statuses=(200, 204), @@ -558,7 +642,7 @@ async def _resource_delete( request_body = body if request_body is None and delete_model_cls is not None and force is not None: - request_body = model_to_payload(delete_model_cls(force=force)) # type: ignore[call-arg] + request_body = self._body(delete_model_cls(force=force)) # type: ignore[call-arg] response = await self._delete_request( endpoint, request_body, skip_entity=skip_entity ) diff --git a/glpi_python_client/_async/clients/commons/tests/test_config.py b/glpi_python_client/_async/clients/commons/tests/test_config.py index 5c47974..038f0d7 100644 --- a/glpi_python_client/_async/clients/commons/tests/test_config.py +++ b/glpi_python_client/_async/clients/commons/tests/test_config.py @@ -7,6 +7,8 @@ from __future__ import annotations +from datetime import datetime, timedelta, timezone + import pytest from glpi_python_client import GlpiValidationError @@ -15,6 +17,7 @@ normalize_client_api_url, parse_optional_env_bool, parse_optional_env_int, + resolve_server_timezone, validate_v1_document_config, ) @@ -160,3 +163,47 @@ def test_build_client_env_config_overrides_win() -> None: assert config["glpi_api_url"] == "https://override" assert config["verify_ssl"] is False assert config["language"] == "en_GB" + + +def test_resolve_server_timezone_accepts_an_iana_name() -> None: + """An IANA zone name resolves to a DST-aware timezone. + + A name rather than a fixed offset is what makes winter and summer both + correct: the same GLPI instance sends ``+01:00`` and ``+02:00``. + """ + + resolved = resolve_server_timezone("Europe/Paris") + + assert datetime(2019, 1, 15, tzinfo=resolved).utcoffset() == timedelta(hours=1) + assert datetime(2018, 7, 15, tzinfo=resolved).utcoffset() == timedelta(hours=2) + + +def test_resolve_server_timezone_passes_a_tzinfo_through() -> None: + """A caller holding a tzinfo object may supply it directly.""" + + fixed = timezone(timedelta(hours=2)) + + assert resolve_server_timezone(fixed) is fixed + + +def test_resolve_server_timezone_rejects_an_unknown_name() -> None: + """A typo fails at construction, not silently at the first timestamp.""" + + with pytest.raises(GlpiValidationError) as excinfo: + resolve_server_timezone("Europe/Pariss") + + assert "Europe/Pariss" in str(excinfo.value) + + +def test_resolve_server_timezone_rejects_a_missing_value() -> None: + """The timezone is required; GLPI does not advertise it.""" + + with pytest.raises(GlpiValidationError): + resolve_server_timezone(None) + + +def test_resolve_server_timezone_rejects_a_blank_name() -> None: + """An empty environment variable is a missing value, not a valid one.""" + + with pytest.raises(GlpiValidationError): + resolve_server_timezone(" ") diff --git a/glpi_python_client/_async/clients/commons/tests/test_http.py b/glpi_python_client/_async/clients/commons/tests/test_http.py index 2862214..989276e 100644 --- a/glpi_python_client/_async/clients/commons/tests/test_http.py +++ b/glpi_python_client/_async/clients/commons/tests/test_http.py @@ -195,3 +195,52 @@ def test_4xx_raises_a_typed_status_error_from_ensure_response_status() -> None: assert excinfo.value.status_code == 404 assert isinstance(excinfo.value, ValueError) assert str(excinfo.value) == "Failed to fetch ticket 1: 404 nope" + + +def test_require_response_int_accepts_a_numeric_string_id() -> None: + """A quoted identifier is still an identifier. + + GLPI is PHP-backed and PHP-backed APIs commonly render integers as + strings. Rejecting one is a protocol error raised over a usable value. + """ + + response = FakeResponse(payload={"id": "4242"}) + + assert require_response_int(response, keys=("id",), missing_message="x") == 4242 + + +def test_require_response_int_ignores_a_non_numeric_string_id() -> None: + """A string that is not a number is not silently coerced.""" + + response = FakeResponse(payload={"id": "not-a-number"}) + + with pytest.raises(GlpiProtocolError): + require_response_int(response, keys=("id",), missing_message="x") + + +def test_require_response_int_reads_a_nested_data_envelope() -> None: + """An id wrapped in a ``data`` envelope is found rather than refused.""" + + response = FakeResponse(payload={"data": {"id": 4242}}) + + assert require_response_int(response, keys=("id",), missing_message="x") == 4242 + + +def test_require_response_int_falls_back_to_the_location_header() -> None: + """A 201 whose id lives only in ``Location`` still yields the id. + + A create that answers with an empty body and a resource URL is a normal + REST shape; without this the client raises a protocol error on a request + that succeeded, and the caller loses the id of a record that now exists. + """ + + response = FakeResponse( + status_code=201, + payload=None, + content=b"", + headers={ + "Location": "https://glpi.example.test/api.php/v2/Assistance/Ticket/4242" + }, + ) + + assert require_response_int(response, keys=("id",), missing_message="x") == 4242 diff --git a/glpi_python_client/_async/clients/commons/tests/test_payloads.py b/glpi_python_client/_async/clients/commons/tests/test_payloads.py index 10a29ce..cbb6251 100644 --- a/glpi_python_client/_async/clients/commons/tests/test_payloads.py +++ b/glpi_python_client/_async/clients/commons/tests/test_payloads.py @@ -2,6 +2,9 @@ from __future__ import annotations +import json +from datetime import datetime, timedelta, timezone + from glpi_python_client._async.clients.commons._payloads import ( model_from_payload, model_to_payload, @@ -10,6 +13,9 @@ GetUser, PostUser, ) +from glpi_python_client.models.api_schema.assistance.timeline._task import ( + PostTicketTask, +) def test_model_to_payload_excludes_none_and_extra_payload_meta() -> None: @@ -62,3 +68,54 @@ def test_capture_extra_keys_merges_with_existing_extra_payload_dict() -> None: "stranger": "explicit", "caller_wins": True, } + + +def test_model_to_payload_body_is_json_encodable() -> None: + """Every value in the body survives the JSON encoder httpx will use. + + ``_execute_request`` hands the mapping straight to ``httpx`` as ``json=``, + whose encoder is ``json.dumps``. A body holding a live ``datetime`` object + raises ``TypeError`` there -- after the model validated, and outside any + transport stub -- so the assertion has to be encodability, not shape. + """ + + body = model_to_payload(PostTicketTask(planned_begin=datetime(2024, 1, 1, 12, 0))) + + assert json.dumps(body) + + +def test_model_to_payload_renders_naive_datetime_without_offset() -> None: + """A naive datetime reaches GLPI as the bare timestamp it was given.""" + + body = model_to_payload(PostTicketTask(planned_begin=datetime(2024, 1, 1, 12, 0))) + + assert body["planned_begin"] == "2024-01-01T12:00:00" + + +def test_model_to_payload_rewrites_an_aware_datetime_onto_the_server_clock() -> None: + """An aware datetime is converted to server-local time and sent bare. + + GLPI 11 ignores the offset it is sent -- measured, three spellings of one + moment all stored the same wall clock -- so writing ``12:00Z`` to a + ``+01:00`` instance stores 12:00 there and loses an hour silently. The + offset has to be spent on the conversion instead of written out. + """ + + aware = datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc) + + body = model_to_payload( + PostTicketTask(planned_begin=aware), + server_timezone=timezone(timedelta(hours=1)), + ) + + assert body["planned_begin"] == "2024-01-01T13:00:00" + + +def test_model_to_payload_leaves_an_aware_datetime_alone_without_a_timezone() -> None: + """Outside the client there is no server clock to convert onto.""" + + aware = datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc) + + body = model_to_payload(PostTicketTask(planned_begin=aware)) + + assert body["planned_begin"] == "2024-01-01T12:00:00Z" diff --git a/glpi_python_client/_async/clients/commons/tests/test_transport.py b/glpi_python_client/_async/clients/commons/tests/test_transport.py index b3d35a4..677f0c5 100644 --- a/glpi_python_client/_async/clients/commons/tests/test_transport.py +++ b/glpi_python_client/_async/clients/commons/tests/test_transport.py @@ -25,6 +25,7 @@ AsyncGlpiClient, GlpiError, GlpiServerError, + GlpiStatusError, GlpiTimeoutError, GlpiTransportError, ) @@ -330,20 +331,56 @@ async def _send(method: str, url: str, **kw: Any) -> FakeResponse: assert response.status_code == 404 -async def test_tolerant_search_still_returns_empty_on_4xx(retry_client: Any) -> None: - """Search endpoints that pass no ``failure_message`` still swallow a 4xx. - - Guards the 7 tolerant ``_resource_list`` call sites against the 4xx raise - being moved into ``finalize_request_response``. +@pytest.mark.parametrize("status", [400, 401, 403, 404]) +async def test_search_raises_on_4xx_instead_of_returning_empty( + retry_client: Any, status: int +) -> None: + """A refused search is an error, not an empty result set. + + Search endpoints pass no ``failure_message``, and that used to mean the + status went unchecked: a 403 came back as ``[]``, indistinguishable from + a filter that matched nothing. It composed badly with the batch + iterators, which stop on a short page -- so a 403 on page one ended the + walk having yielded nothing and the caller saw a successful empty + result. """ async def _send(method: str, url: str, **kw: Any) -> FakeResponse: - return FakeResponse(status_code=400, payload=[], text="[]") + return FakeResponse(status_code=status, payload=[], text="[]") + + retry_client._send_request = _send # type: ignore[method-assign] + + with pytest.raises(GlpiStatusError): + await retry_client.search_tickets() + + +async def test_search_still_returns_empty_on_a_genuine_empty_result( + retry_client: Any, +) -> None: + """A 200 carrying an empty list is still an ordinary empty result.""" + + async def _send(method: str, url: str, **kw: Any) -> FakeResponse: + return FakeResponse(status_code=200, payload=[], text="[]") retry_client._send_request = _send # type: ignore[method-assign] + assert await retry_client.search_tickets() == [] +async def test_iterators_surface_a_4xx_instead_of_ending_silently( + retry_client: Any, +) -> None: + """The iterator raises rather than yielding nothing and looking finished.""" + + async def _send(method: str, url: str, **kw: Any) -> FakeResponse: + return FakeResponse(status_code=403, payload=[], text="[]") + + retry_client._send_request = _send # type: ignore[method-assign] + + with pytest.raises(GlpiStatusError): + [batch async for batch in retry_client.iter_search_tickets()] + + @pytest.mark.parametrize("method_name", _RETRIED_METHODS) async def test_network_errors_are_still_retried( retry_client: Any, method_name: str diff --git a/glpi_python_client/_async/clients/custom/_statistics.py b/glpi_python_client/_async/clients/custom/_statistics.py index e3e075f..1958f57 100644 --- a/glpi_python_client/_async/clients/custom/_statistics.py +++ b/glpi_python_client/_async/clients/custom/_statistics.py @@ -33,6 +33,7 @@ GlpiPriority, GlpiTicketType, ) +from glpi_python_client.rsql import created_between #: The GLPI v2 ticket search includes soft-deleted ("trashed") tickets by #: default, while the v1 search excludes them. Every aggregation here is @@ -382,27 +383,34 @@ async def get_ticket_statistics( entity_filter = f"entity.id=={entity_id}" elif entity_name is not None: name_filter = rsql_contains_filter("name", entity_name) or "" - entities = await self.search_entities( # type: ignore[attr-defined] - rsql_filter=name_filter, - limit=200, - ) + entities = [] + async for entity_batch in self.iter_search_entities( # type: ignore[attr-defined] + name_filter, + batch_size=200, + ): + entities.extend(entity_batch) if not entities: return {"entities": {}} entity_filter = rsql_any_filter( *(f"entity.id=={e.id}" for e in entities if e.id is not None) ) - date_filter = f"date_creation=ge={start.isoformat()};" - date_filter += f"date_creation=le={end.isoformat()} 23:59:59" + date_filter = created_between(start, end) query = rsql_all_filter( date_filter, entity_filter, _LIVE_TICKETS, extra_filter, ) - tickets: list[GetTicket] = await self.search_tickets( # type: ignore[attr-defined] - rsql_filter=query or "", - limit=200, - ) + # Paged rather than fetched in one call. A single ``search_tickets`` + # returns one page and nothing else, so any corpus larger than the + # page size was silently truncated and the helper reported a + # plausible number that was really just the page size. + tickets: list[GetTicket] = [] + async for batch in self.iter_search_tickets( # type: ignore[attr-defined] + query or "", + batch_size=200, + ): + tickets.extend(batch) return _summarize_tickets(tickets) async def get_task_statistics( @@ -518,18 +526,19 @@ async def get_task_durations( end_date=end_date, default_days=default_days, ) - date_filter = f"date_creation=ge={start.isoformat()};" - date_filter += f"date_creation=le={end.isoformat()} 23:59:59" + date_filter = created_between(start, end) entity_filter: str | None = None if entity_id is not None: entity_filter = f"entity.id=={entity_id}" elif entity_name is not None: name_filter = rsql_contains_filter("name", entity_name) or "" - entities = await self.search_entities( # type: ignore[attr-defined] - rsql_filter=name_filter, - limit=200, - ) + entities = [] + async for entity_batch in self.iter_search_entities( # type: ignore[attr-defined] + name_filter, + batch_size=200, + ): + entities.extend(entity_batch) if not entities: return TaskDurationsResult( start_date=start.isoformat(), @@ -714,10 +723,12 @@ async def get_user_activity( rsql_contains_filter("firstname", firstname) if firstname else None, ] user_rsql = rsql_all_filter(*name_parts) or "" - matched_users = await self.search_users( # type: ignore[attr-defined] - rsql_filter=user_rsql, - limit=200, - ) + matched_users = [] + async for user_batch in self.iter_search_users( # type: ignore[attr-defined] + user_rsql, + batch_size=200, + ): + matched_users.extend(user_batch) if not matched_users: raise GlpiValidationError("No users matched the supplied criteria") resolved_user_ids = [u.id for u in matched_users if u.id is not None] @@ -731,8 +742,7 @@ async def get_user_activity( if u.id is not None } - date_range = f"date_creation=ge={start.isoformat()};" - date_range += f"date_creation=le={end.isoformat()} 23:59:59" + date_range = created_between(start, end) # The date window is resolved once for every user rather than once # per user per role. Previously each user drove two full pagings of diff --git a/glpi_python_client/_async/clients/custom/tests/test_statistics.py b/glpi_python_client/_async/clients/custom/tests/test_statistics.py index 46fdc79..f46a67f 100644 --- a/glpi_python_client/_async/clients/custom/tests/test_statistics.py +++ b/glpi_python_client/_async/clients/custom/tests/test_statistics.py @@ -105,7 +105,7 @@ async def test_get_ticket_statistics_aggregates_by_entity_status_priority_type( captured: dict[str, Any] = {} async def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [ @@ -154,7 +154,7 @@ async def test_get_ticket_statistics_default_window_uses_today( captured: dict[str, Any] = {} async def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -267,7 +267,7 @@ async def test_get_ticket_statistics_entity_id_filter(client: Any) -> None: captured: dict[str, Any] = {} async def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -299,7 +299,7 @@ async def fake_search_entities( return [GetEntity(id=3, name="Acme"), GetEntity(id=4, name="Acme Sub")] async def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured_tickets["filter"] = rsql_filter return [] @@ -344,7 +344,7 @@ async def test_get_ticket_statistics_extra_filter_appended(client: Any) -> None: captured: dict[str, Any] = {} async def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -367,7 +367,7 @@ async def test_get_ticket_statistics_default_days_window(client: Any) -> None: captured: dict[str, Any] = {} async def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -784,3 +784,102 @@ async def fake_task_stats(ticket_ids: list[int]) -> dict[str, Any]: assert result["task_count"] == 0 assert calls == [[1]] + + +async def test_get_ticket_statistics_counts_beyond_one_page(client: Any) -> None: + """Aggregates cover every matching ticket, not just the first page. + + A single ``search_tickets`` call returns one page and nothing else, so a + corpus larger than the page size was silently truncated: the helper + reported a plausible number that happened to be the page size. The stub + answers a full page followed by a short one, which is the only shape that + tells a paging walk apart from a single request. + """ + + pages = [[_ticket() for _ in range(200)], [_ticket() for _ in range(30)]] + calls: list[int] = [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetTicket]: + calls.append(start) + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_tickets = fake_search # type: ignore[method-assign] + + result = await client.get_ticket_statistics(default_days=7) + + assert calls == [0, 200] + assert result["entities"]["1"]["total"] == 230 + + +async def test_get_ticket_statistics_resolves_entities_beyond_one_page( + client: Any, +) -> None: + """Entity-name resolution walks every page of matches. + + A name prefix shared by many entities matches more than one page. A + single ``search_entities`` call drops the rest, so tickets belonging to + the unlisted entities vanish from the aggregate with no error. + """ + + from glpi_python_client.models.api_schema.administration._entity import GetEntity + + pages = [ + [GetEntity(id=i, name="Acme") for i in range(1, 201)], + [GetEntity(id=201, name="Acme Sub")], + ] + captured_tickets: dict[str, Any] = {} + + async def fake_search_entities( + rsql_filter: str = "", *, limit: int = 200, start: int = 0, **kwargs: Any + ) -> list[GetEntity]: + index = start // limit + return pages[index] if index < len(pages) else [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetTicket]: + captured_tickets["filter"] = rsql_filter + return [] + + client.search_entities = fake_search_entities # type: ignore[method-assign] + client.search_tickets = fake_search # type: ignore[method-assign] + + await client.get_ticket_statistics(default_days=7, entity_name="Acme") + + assert "entity.id==201" in captured_tickets["filter"] + + +async def test_get_task_durations_resolves_entities_beyond_one_page( + client: Any, +) -> None: + """``get_task_durations`` pages entity-name resolution the same way.""" + + from glpi_python_client.models.api_schema.administration._entity import GetEntity + + pages = [ + [GetEntity(id=i, name="Acme") for i in range(1, 201)], + [GetEntity(id=201, name="Acme Sub")], + ] + captured_tickets: dict[str, Any] = {} + + async def fake_search_entities( + rsql_filter: str = "", *, limit: int = 200, start: int = 0, **kwargs: Any + ) -> list[GetEntity]: + index = start // limit + return pages[index] if index < len(pages) else [] + + async def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetTicket]: + captured_tickets["filter"] = rsql_filter + return [] + + client.search_entities = fake_search_entities # type: ignore[method-assign] + client.search_tickets = fake_search # type: ignore[method-assign] + + await client.get_task_durations(default_days=7, entity_name="Acme") + + assert "entity.id==201" in captured_tickets["filter"] diff --git a/glpi_python_client/_async/clients/tests/test_client.py b/glpi_python_client/_async/clients/tests/test_client.py index 99ff47c..4b80471 100644 --- a/glpi_python_client/_async/clients/tests/test_client.py +++ b/glpi_python_client/_async/clients/tests/test_client.py @@ -21,6 +21,7 @@ async def test_glpi_client_from_env_uses_overrides_and_defaults( "GLPI_API_URL": "https://glpi.example.test/api.php/v2", "GLPI_USERNAME": "u", "GLPI_PASSWORD": "p", + "GLPI_SERVER_TIMEZONE": "Europe/Paris", } client = AsyncGlpiClient.from_env(env=env) try: @@ -34,6 +35,7 @@ async def test_glpi_client_close_is_idempotent() -> None: client = AsyncGlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", ) @@ -61,7 +63,10 @@ async def test_glpi_client_rejects_invalid_credentials() -> None: """Constructor refuses to build a client with no usable credentials.""" with pytest.raises(ValueError): - AsyncGlpiClient(glpi_api_url="https://glpi.example.test/api.php/v2") + AsyncGlpiClient( + glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", + ) async def test_glpi_client_v1_session_built_when_configured() -> None: @@ -69,6 +74,7 @@ async def test_glpi_client_v1_session_built_when_configured() -> None: client = AsyncGlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", v1_base_url="https://glpi.example.test/apirest.php", @@ -87,6 +93,7 @@ async def test_glpi_client_rejects_partial_v1_config() -> None: with pytest.raises(ValueError, match="v1_base_url and v1_user_token"): AsyncGlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", v1_base_url="https://glpi.example.test/apirest.php", @@ -101,6 +108,7 @@ async def test_environ_default_is_used_when_env_argument_omitted( monkeypatch.setenv("GLPI_API_URL", "https://from-environ.example/api.php/v2") monkeypatch.setenv("GLPI_USERNAME", "u") monkeypatch.setenv("GLPI_PASSWORD", "p") + monkeypatch.setenv("GLPI_SERVER_TIMEZONE", "Europe/Paris") client = AsyncGlpiClient.from_env() try: assert client.glpi_api_url.endswith("/api.php/v2") @@ -113,6 +121,7 @@ async def test_async_transport_ensure_open_blocks_after_close() -> None: client = AsyncGlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", ) @@ -149,6 +158,7 @@ def _track_init(self: httpx.AsyncClient, *args: Any, **kwargs: Any) -> None: with pytest.raises(ValueError): AsyncGlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", client_id="only-id-no-secret", ) assert constructed == [], "a transport session was built for a rejected config" diff --git a/glpi_python_client/_sync/clients/_base_client.py b/glpi_python_client/_sync/clients/_base_client.py index d5ee4c3..a30b8c2 100644 --- a/glpi_python_client/_sync/clients/_base_client.py +++ b/glpi_python_client/_sync/clients/_base_client.py @@ -25,10 +25,12 @@ from glpi_python_client._sync.clients.commons._config import ( build_client_env_config, build_client_resources, + resolve_server_timezone, ) if TYPE_CHECKING: from collections.abc import Mapping + from datetime import tzinfo logger = logging.getLogger(__name__) @@ -45,6 +47,7 @@ def __init__( self, *, glpi_api_url: str, + server_timezone: str | tzinfo, client_id: str | None = None, client_secret: str | None = None, username: str | None = None, @@ -66,6 +69,25 @@ def __init__( glpi_api_url : str Base URL of the GLPI v2 REST API, e.g. ``https://glpi.example.com/api.php/v2``. + server_timezone : str | tzinfo + IANA name of the timezone the GLPI server runs in (e.g. + ``"Europe/Paris"``), or a ``tzinfo``. **Required**: GLPI does + not advertise it, and it governs both directions of every + timestamp the client exchanges. + + Reading, it interprets the timestamps the server sends without + an offset. Writing, it is what makes an aware ``datetime`` + arrive as the moment it names: GLPI reads the naive prefix of a + timestamp and discards the offset, so the value has to be + converted onto the server's clock before it is sent. Measured on + a live instance, offsets from ``-08:00`` to ``+14:00`` written + to one field all stored the same wall clock. + + There is no default because every candidate is wrong somewhere + -- guessing UTC against a Europe/Paris instance shifts those + timestamps by an hour or two and never raises. Prefer a name + over a fixed offset: a name follows DST, and one instance emits + both ``+01:00`` and ``+02:00``. client_id : str | None, optional OAuth client identifier used to obtain access tokens. client_secret : str | None, optional @@ -103,6 +125,7 @@ def __init__( missing OAuth credentials together with no v1 fallback). """ + self.server_timezone = resolve_server_timezone(server_timezone) resources = build_client_resources( glpi_api_url=glpi_api_url, client_name=type(self).__name__, @@ -142,7 +165,8 @@ def from_env( ``GLPI_USERNAME``, ``GLPI_PASSWORD``, ``GLPI_VERIFY_SSL``, ``GLPI_V1_BASE_URL``, ``GLPI_V1_USER_TOKEN``, ``GLPI_V1_APP_TOKEN``, ``GLPI_ENTITY``, ``GLPI_PROFILE``, ``GLPI_ENTITY_RECURSIVE``, - ``GLPI_LANGUAGE``, ``GLPI_AUTH_TOKEN_REFRESH``). + ``GLPI_LANGUAGE``, ``GLPI_AUTH_TOKEN_REFRESH``, + ``GLPI_SERVER_TIMEZONE``). Parameters ---------- diff --git a/glpi_python_client/_sync/clients/api/administration/_user.py b/glpi_python_client/_sync/clients/api/administration/_user.py index 2a9cb19..11eb40a 100644 --- a/glpi_python_client/_sync/clients/api/administration/_user.py +++ b/glpi_python_client/_sync/clients/api/administration/_user.py @@ -12,6 +12,7 @@ from glpi_python_client._sync.clients.commons._constants import USER_ENDPOINT, GlpiId from glpi_python_client._sync.clients.commons._transport import TransportMixin +from glpi_python_client._errors import GlpiValidationError from glpi_python_client.models.api_schema.administration._user import ( DeleteUser, GetUser, @@ -110,6 +111,81 @@ def iter_search_users( break start += batch_size + def find_user_by_email( + self, + email: str, + *, + rsql_filter: str = "", + batch_size: int = 100, + skip_entity: bool = True, + ) -> GetUser | None: + """Return the first user holding ``email``, or ``None``. + + **This scans.** GLPI exposes e-mail addresses as ``User.emails``, a + nested *array*, and the v2 filter engine cannot join a nested array + -- the structurally identical ``Ticket.team`` answers HTTP 500 for + its declared subfields and is silently ignored for every other + spelling. So there is no server-side e-mail filter to use, and the + addresses have to be compared client-side. + + Nothing about that is cheap: the scan costs one request per + ``batch_size`` users until it matches, so it is meant for + occasional resolution, not for a per-request lookup. Narrow it with + ``rsql_filter`` when you can (``"is_active==true"`` is the usual + one), and cache the resulting id rather than calling this again. + + A server-side fast path is deliberately *not* attempted. GLPI v2 + ignores a filter field it does not recognise and answers with the + whole unfiltered table, so a guessed e-mail filter would not fail + -- it would return a plausible non-empty page whose first row is + the wrong person. Guessing is the one thing this helper exists to + stop each caller doing separately. + + Parameters + ---------- + email : str + Address to look for. Compared case-insensitively after + trimming surrounding whitespace, the way mail systems treat it. + rsql_filter : str, optional + Raw RSQL filter narrowing the population scanned. Empty by + default, which scans every visible user. + batch_size : int, optional + Users fetched per request while scanning (defaults to 100). + skip_entity : bool, optional + When ``True`` (the default) the ``GLPI-Entity`` header is + omitted so the scan spans every entity the caller can see. A + user whose account lives outside the client's configured entity + is invisible otherwise, and the helper would answer ``None`` + for somebody who exists. + + Returns + ------- + GetUser | None + The first user with a matching address, or ``None`` when the + scanned population holds none. + + Raises + ------ + GlpiValidationError + If ``email`` is blank -- which would otherwise scan the whole + directory and match nothing. + """ + + needle = email.strip().casefold() + if not needle: + raise GlpiValidationError("find_user_by_email requires a non-empty address") + + for batch in self.iter_search_users( + rsql_filter, + batch_size=batch_size, + skip_entity=skip_entity, + ): + for user in batch: + for entry in user.emails or (): + if entry.email and entry.email.strip().casefold() == needle: + return user + return None + def get_user(self, user_id: GlpiId) -> GetUser: """Fetch one GLPI user by identifier. diff --git a/glpi_python_client/_sync/clients/api/administration/tests/test_user.py b/glpi_python_client/_sync/clients/api/administration/tests/test_user.py index 56d94bf..af25eef 100644 --- a/glpi_python_client/_sync/clients/api/administration/tests/test_user.py +++ b/glpi_python_client/_sync/clients/api/administration/tests/test_user.py @@ -192,3 +192,158 @@ def fake_search( batches = [batch for batch in client.iter_search_users("", batch_size=2)] assert call_count == 2 assert sum(len(b) for b in batches) == 3 + + +def test_find_user_by_email_matches_a_nested_email_entry(client: Any) -> None: + """The address is matched inside the ``emails`` array, not on a top field.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + if start: + return [] + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "alice@x.test"}]} + ), + GetUser.model_validate( + {"id": 2, "username": "bob", "emails": [{"email": "bob@x.test"}]} + ), + ] + + client.search_users = fake_search # type: ignore[method-assign] + + found = client.find_user_by_email("bob@x.test") + + assert found is not None + assert found.id == 2 + + +def test_find_user_by_email_is_case_insensitive(client: Any) -> None: + """Addresses are compared case-insensitively, as mail systems treat them.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + if start: + return [] + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "Alice@X.test"}]} + ) + ] + + client.search_users = fake_search # type: ignore[method-assign] + + found = client.find_user_by_email(" ALICE@x.TEST ") + + assert found is not None + assert found.id == 1 + + +def test_find_user_by_email_returns_none_when_absent(client: Any) -> None: + """No match is ``None`` rather than an arbitrary first user.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + if start: + return [] + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "alice@x.test"}]} + ) + ] + + client.search_users = fake_search # type: ignore[method-assign] + + assert client.find_user_by_email("nobody@x.test") is None + + +def test_find_user_by_email_scans_past_the_first_page(client: Any) -> None: + """The match may live on any page, so the scan pages until it finds one.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + pages = [ + [GetUser.model_validate({"id": i, "username": f"u{i}"}) for i in range(1, 3)], + [ + GetUser.model_validate( + {"id": 9, "username": "zoe", "emails": [{"email": "zoe@x.test"}]} + ), + ], + ] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_users = fake_search # type: ignore[method-assign] + + found = client.find_user_by_email("zoe@x.test", batch_size=2) + + assert found is not None + assert found.id == 9 + + +def test_find_user_by_email_stops_at_the_matching_page(client: Any) -> None: + """The scan stops as soon as it matches instead of walking the directory.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + starts.append(start) + return [ + GetUser.model_validate( + {"id": 1, "username": "alice", "emails": [{"email": "alice@x.test"}]} + ) + ] * limit + + client.search_users = fake_search # type: ignore[method-assign] + + client.find_user_by_email("alice@x.test", batch_size=2) + + assert starts == [0] + + +def test_find_user_by_email_scans_every_entity_by_default(client: Any) -> None: + """The scan spans entities, or a match outside the header scope is missed.""" + + from glpi_python_client.models.api_schema.administration._user import GetUser + + seen: dict[str, Any] = {} + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetUser]: + seen.update(kwargs) + seen["filter"] = rsql_filter + return [] + + client.search_users = fake_search # type: ignore[method-assign] + + client.find_user_by_email("alice@x.test", rsql_filter="is_active==true") + + assert seen["skip_entity"] is True + assert seen["filter"] == "is_active==true" + + +def test_find_user_by_email_rejects_a_blank_address(client: Any) -> None: + """A blank address would scan the whole directory and match nothing.""" + + from glpi_python_client import GlpiValidationError + + with pytest.raises(GlpiValidationError): + client.find_user_by_email(" ") diff --git a/glpi_python_client/_sync/clients/api/assistance/_team.py b/glpi_python_client/_sync/clients/api/assistance/_team.py index 4247412..9771971 100644 --- a/glpi_python_client/_sync/clients/api/assistance/_team.py +++ b/glpi_python_client/_sync/clients/api/assistance/_team.py @@ -15,7 +15,6 @@ GlpiId, ) from glpi_python_client._sync.clients.commons._http import ensure_response_status -from glpi_python_client._sync.clients.commons._payloads import model_to_payload from glpi_python_client._sync.clients.commons._transport import TransportMixin from glpi_python_client.models.api_schema.assistance._team import ( GetTeamMember, @@ -79,7 +78,7 @@ def add_ticket_team_member( """ endpoint = f"{TICKET_ENDPOINT}/{ticket_id}/{TEAM_MEMBER_SUFFIX}" - response = self._post_request(endpoint, model_to_payload(member)) + response = self._post_request(endpoint, self._body(member)) ensure_response_status( response, success_statuses=(200, 201), @@ -114,7 +113,7 @@ def remove_ticket_team_member( f"{TICKET_ENDPOINT}/{ticket_id}/{TEAM_MEMBER_SUFFIX}", failure_message=f"Failed to remove team member on ticket {ticket_id}", log_message=f"GLPI API removed team member on ticket {ticket_id}", - body=model_to_payload(member), + body=self._body(member), ) diff --git a/glpi_python_client/_sync/clients/api/assistance/_ticket.py b/glpi_python_client/_sync/clients/api/assistance/_ticket.py index 546d6ea..0b39cc1 100644 --- a/glpi_python_client/_sync/clients/api/assistance/_ticket.py +++ b/glpi_python_client/_sync/clients/api/assistance/_ticket.py @@ -49,7 +49,11 @@ def search_tickets( start : int, optional Zero-based offset of the first record returned. sort : str | None, optional - ``sort`` query parameter forwarded as-is, e.g. ``"date_mod desc"``. + ``sort`` query parameter, spelled ``field`` or ``field:direction`` + (e.g. ``"date_mod:desc"``). Measured against GLPI 11: a space + before the direction answers **HTTP 400** ("Invalid property for + sorting"), a bare ``field`` sorts *ascending*, and a separate + ``order`` parameter is ignored. fields : tuple[str, ...], optional Restricted set of contract field names to request. Empty tuple lets the GLPI server pick its default field set. @@ -94,7 +98,9 @@ def iter_search_tickets( the ``limit`` parameter on each underlying :meth:`search_tickets` call. sort : str | None, optional - ``sort`` query parameter forwarded as-is to each page request. + ``sort`` query parameter forwarded to each page request, + spelled ``field`` or ``field:direction`` (e.g. ``"date_mod:desc"``). + A space before the direction answers HTTP 400. fields : tuple[str, ...], optional Restricted set of contract field names to request. diff --git a/glpi_python_client/_sync/clients/api/assistance/tests/test_ticket.py b/glpi_python_client/_sync/clients/api/assistance/tests/test_ticket.py index e7cfeba..ec25ebf 100644 --- a/glpi_python_client/_sync/clients/api/assistance/tests/test_ticket.py +++ b/glpi_python_client/_sync/clients/api/assistance/tests/test_ticket.py @@ -29,14 +29,14 @@ def test_search_tickets_forwards_sort_and_fields(client: Any) -> None: rec = TransportRecorder(get_payload=[{"id": 1, "name": "n", "content": "c"}]) rec.install(client) tickets = client.search_tickets( - "status==1", limit=5, start=10, sort="date_mod desc", fields=("id", "name") + "status==1", limit=5, start=10, sort="date_mod:desc", fields=("id", "name") ) assert len(tickets) == 1 assert rec.calls[0]["params"]["filter"] == "status==1" assert rec.calls[0]["params"]["limit"] == 5 assert rec.calls[0]["params"]["start"] == 10 - assert rec.calls[0]["params"]["sort"] == "date_mod desc" + assert rec.calls[0]["params"]["sort"] == "date_mod:desc" assert rec.calls[0]["params"]["fields"] == "id,name" diff --git a/glpi_python_client/_sync/clients/api/dropdowns/_location.py b/glpi_python_client/_sync/clients/api/dropdowns/_location.py index 99c1f1f..283428b 100644 --- a/glpi_python_client/_sync/clients/api/dropdowns/_location.py +++ b/glpi_python_client/_sync/clients/api/dropdowns/_location.py @@ -7,6 +7,8 @@ from __future__ import annotations +from collections.abc import Iterator + from glpi_python_client._sync.clients.commons._constants import ( LOCATION_ENDPOINT, GlpiId, @@ -52,6 +54,48 @@ def search_locations( params["filter"] = rsql_filter return self._resource_list(LOCATION_ENDPOINT, GetLocation, params=params) + def iter_search_locations( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + ) -> Iterator[list[GetLocation]]: + """Yield successive pages of GLPI locations until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_locations` + call. + + Yields + ------ + list[GetLocation] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = self.search_locations( + rsql_filter, + limit=batch_size, + start=start, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + def get_location(self, location_id: GlpiId) -> GetLocation: """Fetch one GLPI location by identifier. diff --git a/glpi_python_client/_sync/clients/api/dropdowns/tests/test_location.py b/glpi_python_client/_sync/clients/api/dropdowns/tests/test_location.py index 72c13a8..3f17866 100644 --- a/glpi_python_client/_sync/clients/api/dropdowns/tests/test_location.py +++ b/glpi_python_client/_sync/clients/api/dropdowns/tests/test_location.py @@ -126,3 +126,64 @@ def test_delete_helpers_raise_on_failure_status( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): call(client) + + +def test_iter_search_locations_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.dropdowns import GetLocation + + pages = [[GetLocation(id=i) for i in range(3)], [GetLocation(id=99)]] + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetLocation]: + starts.append(start) + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_locations = fake_search # type: ignore[method-assign] + + batches = [ + batch for batch in client.iter_search_locations("name==x", batch_size=3) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + + +def test_iter_search_locations_stops_on_a_single_short_page(client: Any) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.dropdowns import GetLocation + + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetLocation]: + starts.append(start) + return [GetLocation(id=1)] + + client.search_locations = fake_search # type: ignore[method-assign] + + batches = [batch for batch in client.iter_search_locations(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +def test_iter_search_locations_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.dropdowns import GetLocation + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetLocation]: + return [] + + client.search_locations = fake_search # type: ignore[method-assign] + + assert [batch for batch in client.iter_search_locations()] == [] diff --git a/glpi_python_client/_sync/clients/api/knowledgebase/_article.py b/glpi_python_client/_sync/clients/api/knowledgebase/_article.py index fe1bc4d..753ebd5 100644 --- a/glpi_python_client/_sync/clients/api/knowledgebase/_article.py +++ b/glpi_python_client/_sync/clients/api/knowledgebase/_article.py @@ -8,7 +8,7 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Iterator, Sequence from glpi_python_client._sync.clients.commons._constants import ( KB_ARTICLE_ENDPOINT, @@ -50,7 +50,11 @@ def search_kb_articles( start : int, optional Zero-based offset of the first record returned. sort : str | None, optional - ``sort`` query parameter forwarded as-is. + ``sort`` query parameter, spelled ``field`` or ``field:direction`` + (e.g. ``"date_mod:desc"``). Measured against GLPI 11: a space + before the direction answers **HTTP 400** ("Invalid property for + sorting"), a bare ``field`` sorts *ascending*, and a separate + ``order`` parameter is ignored. language : str | None, optional GLPI language code forwarded as the ``language`` query parameter to select a translated view. @@ -72,6 +76,59 @@ def search_kb_articles( KB_ARTICLE_ENDPOINT, GetKBArticle, params=params ) + def iter_search_kb_articles( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + sort: str | None = None, + language: str | None = None, + ) -> Iterator[list[GetKBArticle]]: + """Yield successive pages of GLPI knowledge base articles until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_kb_articles` + call. + sort : str | None, optional + ``sort`` query parameter forwarded to each page request, + spelled ``field`` or ``field:direction`` (e.g. ``"date_mod:desc"``). + A space before the direction answers HTTP 400. + language : str | None, optional + GLPI language code forwarded to each page request to select + a translated view. + + Yields + ------ + list[GetKBArticle] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = self.search_kb_articles( + rsql_filter, + limit=batch_size, + start=start, + sort=sort, + language=language, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + def get_kb_article(self, article_id: GlpiId) -> GetKBArticle: """Fetch one knowledge base article by identifier. diff --git a/glpi_python_client/_sync/clients/api/knowledgebase/_category.py b/glpi_python_client/_sync/clients/api/knowledgebase/_category.py index 3e597bc..73bd4e5 100644 --- a/glpi_python_client/_sync/clients/api/knowledgebase/_category.py +++ b/glpi_python_client/_sync/clients/api/knowledgebase/_category.py @@ -7,6 +7,8 @@ from __future__ import annotations +from collections.abc import Iterator + from glpi_python_client._sync.clients.commons._constants import ( KB_CATEGORY_ENDPOINT, GlpiId, @@ -43,7 +45,11 @@ def search_kb_categories( start : int, optional Zero-based offset of the first record returned. sort : str | None, optional - ``sort`` query parameter forwarded as-is, e.g. ``"name asc"``. + ``sort`` query parameter, spelled ``field`` or ``field:direction`` + (e.g. ``"name:asc"``). Measured against GLPI 11: a space + before the direction answers **HTTP 400** ("Invalid property for + sorting"), a bare ``field`` sorts *ascending*, and a separate + ``order`` parameter is ignored. language : str | None, optional GLPI language code forwarded as the ``language`` query parameter to select a translated view. @@ -65,6 +71,59 @@ def search_kb_categories( KB_CATEGORY_ENDPOINT, GetKBCategory, params=params ) + def iter_search_kb_categories( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + sort: str | None = None, + language: str | None = None, + ) -> Iterator[list[GetKBCategory]]: + """Yield successive pages of GLPI knowledge base categories until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_kb_categories` + call. + sort : str | None, optional + ``sort`` query parameter forwarded to each page request, + spelled ``field`` or ``field:direction`` (e.g. ``"date_mod:desc"``). + A space before the direction answers HTTP 400. + language : str | None, optional + GLPI language code forwarded to each page request to select + a translated view. + + Yields + ------ + list[GetKBCategory] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = self.search_kb_categories( + rsql_filter, + limit=batch_size, + start=start, + sort=sort, + language=language, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + def get_kb_category(self, category_id: GlpiId) -> GetKBCategory: """Fetch one knowledge base category by identifier. diff --git a/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_article.py b/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_article.py index a0a2f67..01817e2 100644 --- a/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_article.py +++ b/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_article.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Callable +from datetime import timedelta from typing import Any import pytest @@ -13,7 +14,10 @@ PatchKBArticle, PostKBArticle, ) -from glpi_python_client._sync._testing import FailingTransportRecorder +from glpi_python_client._sync._testing import ( + FailingTransportRecorder, + TransportRecorder, +) from glpi_python_client.testing.utils import FakeResponse @@ -111,7 +115,7 @@ def test_search_kb_articles_forwards_params(client: Any) -> None: rec = _Recorder(get_payload=[{"id": 1, "name": "Reset", "content": "

c

"}]) rec.install(client) result = client.search_kb_articles( - "is_faq==1", limit=3, start=1, sort="date_mod desc", language="en_GB" + "is_faq==1", limit=3, start=1, sort="date_mod:desc", language="en_GB" ) assert result[0].id == 1 call = rec.calls[0] @@ -119,7 +123,7 @@ def test_search_kb_articles_forwards_params(client: Any) -> None: assert call["params"]["filter"] == "is_faq==1" assert call["params"]["limit"] == 3 assert call["params"]["start"] == 1 - assert call["params"]["sort"] == "date_mod desc" + assert call["params"]["sort"] == "date_mod:desc" assert call["params"]["language"] == "en_GB" @@ -345,3 +349,119 @@ def test_delete_helpers_raise_on_failure( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): call(client) + + +def test_iter_search_kb_articles_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBArticle + + pages = [[GetKBArticle(id=i) for i in range(3)], [GetKBArticle(id=99)]] + starts: list[int] = [] + forwarded: dict[str, Any] = {} + + def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBArticle]: + starts.append(start) + forwarded["sort"] = sort + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_kb_articles = fake_search # type: ignore[method-assign] + + batches = [ + batch + for batch in client.iter_search_kb_articles( + "name==x", batch_size=3, sort="name:asc" + ) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + assert forwarded["sort"] == "name:asc" + + +def test_iter_search_kb_articles_stops_on_a_single_short_page( + client: Any, +) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBArticle + + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBArticle]: + starts.append(start) + return [GetKBArticle(id=1)] + + client.search_kb_articles = fake_search # type: ignore[method-assign] + + batches = [batch for batch in client.iter_search_kb_articles(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +def test_iter_search_kb_articles_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBArticle + + def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBArticle]: + return [] + + client.search_kb_articles = fake_search # type: ignore[method-assign] + + assert [batch for batch in client.iter_search_kb_articles()] == [] + + +def test_get_kb_article_localises_the_naive_revision_date(client: Any) -> None: + """A KB article's revision dates come back comparable with its own. + + This is the shape GLPI 11 actually sends, measured on a live instance: + the article's own timestamps carry an offset and the nested revision + dates do not. Before the server timezone was threaded through, sorting + an article's history against the article itself raised + ``TypeError: can't compare offset-naive and offset-aware datetimes``. + """ + + rec = TransportRecorder( + get_payload={ + "id": 1, + "name": "article", + "date_creation": "2018-04-06T17:38:15+02:00", + "revisions": [{"id": 9, "date": "2018-04-06 17:39:44"}], + } + ) + rec.install(client) + + article = client.get_kb_article(1) + + assert article.revisions is not None + revision_date = article.revisions[0].date + assert revision_date is not None + assert revision_date.tzinfo is not None + # The comparison itself is the regression: it used to raise. + assert article.date_creation is not None + assert revision_date > article.date_creation + assert revision_date - article.date_creation == timedelta(seconds=89) diff --git a/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_category.py b/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_category.py index f4deceb..fa8c5d6 100644 --- a/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_category.py +++ b/glpi_python_client/_sync/clients/api/knowledgebase/tests/test_category.py @@ -73,7 +73,7 @@ def test_search_kb_categories_forwards_filter_and_language(client: Any) -> None: rec = _Recorder(get_payload=[{"id": 1, "name": "Network"}]) rec.install(client) result = client.search_kb_categories( - "name==Network", limit=5, start=2, sort="name asc", language="fr_FR" + "name==Network", limit=5, start=2, sort="name:asc", language="fr_FR" ) assert result[0].id == 1 call = rec.calls[0] @@ -81,7 +81,7 @@ def test_search_kb_categories_forwards_filter_and_language(client: Any) -> None: assert call["params"]["filter"] == "name==Network" assert call["params"]["limit"] == 5 assert call["params"]["start"] == 2 - assert call["params"]["sort"] == "name asc" + assert call["params"]["sort"] == "name:asc" assert call["params"]["language"] == "fr_FR" @@ -183,3 +183,87 @@ def test_delete_helpers_raise_on_failure( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): call(client) + + +def test_iter_search_kb_categories_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBCategory + + pages = [[GetKBCategory(id=i) for i in range(3)], [GetKBCategory(id=99)]] + starts: list[int] = [] + forwarded: dict[str, Any] = {} + + def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBCategory]: + starts.append(start) + forwarded["language"] = language + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_kb_categories = fake_search # type: ignore[method-assign] + + batches = [ + batch + for batch in client.iter_search_kb_categories( + "name==x", batch_size=3, language="fr_FR" + ) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + assert forwarded["language"] == "fr_FR" + + +def test_iter_search_kb_categories_stops_on_a_single_short_page( + client: Any, +) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBCategory + + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBCategory]: + starts.append(start) + return [GetKBCategory(id=1)] + + client.search_kb_categories = fake_search # type: ignore[method-assign] + + batches = [batch for batch in client.iter_search_kb_categories(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +def test_iter_search_kb_categories_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.knowledgebase import GetKBCategory + + def fake_search( + rsql_filter: str = "", + *, + limit: int = 50, + start: int = 0, + sort: str | None = None, + language: str | None = None, + ) -> list[GetKBCategory]: + return [] + + client.search_kb_categories = fake_search # type: ignore[method-assign] + + assert [batch for batch in client.iter_search_kb_categories()] == [] diff --git a/glpi_python_client/_sync/clients/api/management/_document.py b/glpi_python_client/_sync/clients/api/management/_document.py index 71cda34..74c4627 100644 --- a/glpi_python_client/_sync/clients/api/management/_document.py +++ b/glpi_python_client/_sync/clients/api/management/_document.py @@ -8,6 +8,7 @@ from __future__ import annotations import logging +from collections.abc import Iterator from glpi_python_client._sync.clients.commons._constants import ( DOCUMENT_ENDPOINT, @@ -63,6 +64,48 @@ def search_documents( DOCUMENT_ENDPOINT, GetDocument, params=params, skip_entity=True ) + def iter_search_documents( + self, + rsql_filter: str = "", + *, + batch_size: int = 50, + ) -> Iterator[list[GetDocument]]: + """Yield successive pages of GLPI documents until exhausted. + + The generator drives pagination automatically by advancing the + ``start`` offset after each batch. Iteration stops when the server + returns fewer items than ``batch_size``, which signals the last page. + + Parameters + ---------- + rsql_filter : str, optional + Raw RSQL filter forwarded as the ``filter`` query parameter. + Empty by default, which lists every visible record. + batch_size : int, optional + Number of records requested per page (default 50). Acts as the + ``limit`` parameter on each underlying :meth:`search_documents` + call. + + Yields + ------ + list[GetDocument] + One page per iteration. The last yielded batch may be shorter + than ``batch_size``. + """ + + start = 0 + while True: + batch = self.search_documents( + rsql_filter, + limit=batch_size, + start=start, + ) + if batch: + yield batch + if len(batch) < batch_size: + break + start += batch_size + def get_document(self, document_id: GlpiId) -> GetDocument: """Fetch one GLPI document by identifier. @@ -214,6 +257,55 @@ def download_document_content(self, document_id: GlpiId) -> bytes: ) return response.content + def stream_document_content( + self, + document_id: GlpiId, + *, + chunk_size: int = 65536, + ) -> Iterator[bytes]: + """Stream the binary payload of one GLPI document in chunks. + + Use this instead of :meth:`download_document_content` when the file + may be large: that method holds the whole body in memory before + returning, so a 500 MB attachment costs 500 MB of process memory + even if the caller only writes it straight to disk. + + Parameters + ---------- + document_id : GlpiId + Numeric identifier of the document whose binary content is + requested. + chunk_size : int, optional + Bytes requested per chunk (defaults to 64 KiB). + + Yields + ------ + bytes + Successive chunks of the document body. The final chunk may be + shorter than ``chunk_size``. + + Raises + ------ + GlpiStatusError + If the GLPI server returns a non-success HTTP status. + + Examples + -------- + Writing a document to disk without buffering it:: + + with open("attachment.pdf", "wb") as handle: + async for chunk in client.stream_document_content(42): + handle.write(chunk) + """ + + for chunk in self._stream_request( + f"{DOCUMENT_ENDPOINT}/{document_id}/Download", + chunk_size=chunk_size, + skip_entity=True, + failure_message=f"Failed to download document {document_id}", + ): + yield chunk + def upload_document( self, *, diff --git a/glpi_python_client/_sync/clients/api/management/tests/test_document.py b/glpi_python_client/_sync/clients/api/management/tests/test_document.py index 57dd25e..87fb963 100644 --- a/glpi_python_client/_sync/clients/api/management/tests/test_document.py +++ b/glpi_python_client/_sync/clients/api/management/tests/test_document.py @@ -9,7 +9,7 @@ from __future__ import annotations from collections.abc import Callable -from typing import Any +from typing import Any, ClassVar import pytest @@ -224,3 +224,146 @@ def test_delete_helpers_raise_on_failure_status( FailingTransportRecorder(500).install(client) with pytest.raises(ValueError): call(client) + + +def test_iter_search_documents_yields_every_page(client: Any) -> None: + """The generator advances ``start`` until a short page ends the walk.""" + + from glpi_python_client.models.api_schema.management import GetDocument + + pages = [[GetDocument(id=i) for i in range(3)], [GetDocument(id=99)]] + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetDocument]: + starts.append(start) + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_documents = fake_search # type: ignore[method-assign] + + batches = [ + batch for batch in client.iter_search_documents("name==x", batch_size=3) + ] + + assert starts == [0, 3] + assert [len(b) for b in batches] == [3, 1] + + +def test_iter_search_documents_stops_on_a_single_short_page(client: Any) -> None: + """One short page is the last page; no second request is made.""" + + from glpi_python_client.models.api_schema.management import GetDocument + + starts: list[int] = [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetDocument]: + starts.append(start) + return [GetDocument(id=1)] + + client.search_documents = fake_search # type: ignore[method-assign] + + batches = [batch for batch in client.iter_search_documents(batch_size=50)] + + assert starts == [0] + assert len(batches) == 1 + + +def test_iter_search_documents_yields_nothing_when_empty(client: Any) -> None: + """An empty first page yields no batch at all rather than one empty list.""" + + from glpi_python_client.models.api_schema.management import GetDocument + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0 + ) -> list[GetDocument]: + return [] + + client.search_documents = fake_search # type: ignore[method-assign] + + assert [batch for batch in client.iter_search_documents()] == [] + + +def _pin_token(client: Any) -> None: + """Pretend a valid token is held so no OAuth round trip happens. + + The transport recorders stub above ``_ensure_token``; the streaming + helper runs through it, so the token has to be supplied here instead. + """ + + from datetime import datetime, timedelta, timezone + + client._auth.access_token = "stub-token" + client._auth.token_expires_at = datetime.now(tz=timezone.utc) + timedelta(days=1) + + +def test_stream_document_content_yields_chunks(client: Any) -> None: + """The generator yields the body in chunks instead of buffering it.""" + + seen: dict[str, Any] = {} + + class _Stream: + def __enter__(self) -> Any: + return self + + def __exit__(self, *exc: Any) -> None: + return None + + status_code = 200 + headers: ClassVar[dict[str, str]] = {} + + def read(self) -> bytes: + return b"" + + def iter_bytes(self, chunk_size: int | None = None) -> Any: + seen["chunk_size"] = chunk_size + for chunk in (b"abc", b"def"): + yield chunk + + def _stream(method: str, url: str, **kwargs: Any) -> Any: + seen["method"] = method + seen["url"] = url + return _Stream() + + _pin_token(client) + client._session.stream = _stream # type: ignore[method-assign] + + chunks = [c for c in client.stream_document_content(7, chunk_size=4)] + + assert chunks == [b"abc", b"def"] + assert seen["method"] == "GET" + assert seen["url"].endswith("Management/Document/7/Download") + assert seen["chunk_size"] == 4 + + +def test_stream_document_content_raises_on_error_status(client: Any) -> None: + """A non-success status raises rather than yielding an error page as bytes.""" + + class _Stream: + def __enter__(self) -> Any: + return self + + def __exit__(self, *exc: Any) -> None: + return None + + status_code = 404 + headers: ClassVar[dict[str, str]] = {} + url = "https://glpi.example.test/Management/Document/7/Download" + text = "not found" + + def read(self) -> bytes: + return b"not found" + + def iter_bytes(self, chunk_size: int | None = None) -> Any: + yield b"not found" + + _pin_token(client) + client._session.stream = lambda method, url, **kw: _Stream() # type: ignore[method-assign] + + from glpi_python_client import GlpiStatusError + + with pytest.raises(GlpiStatusError): + [c for c in client.stream_document_content(7)] diff --git a/glpi_python_client/_sync/clients/api/plugins/_fields.py b/glpi_python_client/_sync/clients/api/plugins/_fields.py index 46f1b3d..d96dd86 100644 --- a/glpi_python_client/_sync/clients/api/plugins/_fields.py +++ b/glpi_python_client/_sync/clients/api/plugins/_fields.py @@ -33,6 +33,7 @@ import json from typing import Any +from glpi_python_client._sync.clients.commons._payloads import model_from_payload from glpi_python_client._sync.clients.commons._transport import TransportMixin from glpi_python_client._errors import GlpiProtocolError, GlpiValidationError from glpi_python_client.models.api_schema.plugins import ( @@ -147,7 +148,12 @@ def list_plugin_fields_containers( failure_message="Failed to list PluginFieldsContainer", ) rows = payload if isinstance(payload, list) else [] - containers = [GetPluginFieldsContainer.model_validate(row) for row in rows] + containers = [ + model_from_payload( + GetPluginFieldsContainer, row, server_timezone=self.server_timezone + ) + for row in rows + ] if itemtype is None: return containers return [c for c in containers if _container_targets_itemtype(c, itemtype)] @@ -179,7 +185,12 @@ def list_plugin_fields_fields( failure_message="Failed to list PluginFieldsField", ) rows = payload if isinstance(payload, list) else [] - fields = [GetPluginFieldsField.model_validate(row) for row in rows] + fields = [ + model_from_payload( + GetPluginFieldsField, row, server_timezone=self.server_timezone + ) + for row in rows + ] if container_id is None: return fields return [f for f in fields if f.plugin_fields_containers_id == container_id] @@ -219,7 +230,12 @@ def list_item_plugin_field_rows( failure_message=f"Failed to list {endpoint}", ) rows = payload if isinstance(payload, list) else [] - return [GetPluginFieldsValueRow.model_validate(row) for row in rows] + return [ + model_from_payload( + GetPluginFieldsValueRow, row, server_timezone=self.server_timezone + ) + for row in rows + ] def create_item_plugin_field_row( self, diff --git a/glpi_python_client/_sync/clients/commons/_config.py b/glpi_python_client/_sync/clients/commons/_config.py index b611cc3..140e5e7 100644 --- a/glpi_python_client/_sync/clients/commons/_config.py +++ b/glpi_python_client/_sync/clients/commons/_config.py @@ -9,7 +9,9 @@ from collections.abc import Mapping from dataclasses import dataclass +from datetime import tzinfo from typing import TYPE_CHECKING, Protocol +from zoneinfo import ZoneInfo, ZoneInfoNotFoundError import httpx @@ -183,6 +185,63 @@ def build_client_resources( ) +def resolve_server_timezone(value: object) -> tzinfo: + """Resolve the configured GLPI server timezone into a ``tzinfo``. + + The timezone is **required** and has no default. GLPI does not + advertise it anywhere in the API, so only the operator knows it, and + every candidate default is wrong somewhere: guessing UTC against a + Europe/Paris instance shifts every naive timestamp by one or two hours + *without raising*, which is worse than the ``TypeError`` a naive value + produces on comparison. + + An IANA name is preferred over a fixed offset because a fixed offset + cannot follow DST, and a single GLPI instance demonstrably emits both + ``+01:00`` and ``+02:00`` depending on the date. + + Parameters + ---------- + value : object + An IANA zone name (``"Europe/Paris"``) or a ``tzinfo`` instance. + + Returns + ------- + tzinfo + The resolved timezone. + + Raises + ------ + GlpiValidationError + When the value is missing, blank, not a string or ``tzinfo``, or + names a zone the system database does not know. + """ + + if isinstance(value, tzinfo): + return value + if value is None: + raise GlpiValidationError( + "server_timezone is required: GLPI does not advertise its own " + "timezone, so it has to be declared (e.g. 'Europe/Paris', or " + "GLPI_SERVER_TIMEZONE in the environment)." + ) + if not isinstance(value, str): + raise GlpiValidationError( + f"server_timezone must be an IANA name or a tzinfo; got {value!r}" + ) + name = value.strip() + if not name: + raise GlpiValidationError( + "server_timezone is empty: set it to an IANA name (e.g. 'Europe/Paris')." + ) + try: + return ZoneInfo(name) + except (ZoneInfoNotFoundError, ValueError) as exc: + raise GlpiValidationError( + f"Unknown server_timezone {name!r}. Use an IANA zone name such as " + "'Europe/Paris' or 'UTC'." + ) from exc + + def parse_optional_env_int(value: object) -> int | None: """Parse one optional integer from an environment-style value. @@ -248,6 +307,7 @@ def build_client_env_config( config: dict[str, object] = { "glpi_api_url": env.get(f"{prefix}API_URL"), + "server_timezone": env.get(f"{prefix}SERVER_TIMEZONE"), "client_id": env.get(f"{prefix}CLIENT_ID"), "client_secret": env.get(f"{prefix}CLIENT_SECRET"), "username": env.get(f"{prefix}USERNAME"), @@ -313,5 +373,6 @@ def validate_v1_document_config( "normalize_client_api_url", "parse_optional_env_bool", "parse_optional_env_int", + "resolve_server_timezone", "validate_v1_document_config", ] diff --git a/glpi_python_client/_sync/clients/commons/_filters.py b/glpi_python_client/_sync/clients/commons/_filters.py index 12f9b49..cdf31bc 100644 --- a/glpi_python_client/_sync/clients/commons/_filters.py +++ b/glpi_python_client/_sync/clients/commons/_filters.py @@ -4,6 +4,30 @@ for GLPI endpoints that accept RSQL-like query expressions. All functions return ``None`` when the supplied input is empty so callers can compose filters without sprinkling conditional blocks at every call site. + +Two properties of the v2 filter engine shape everything here, and both +fail *open*: + +* **An unrecognised filter field is ignored, not rejected.** The query + then returns the entire unfiltered table with a 200. A typo in a field + name does not raise -- it succeeds and answers with far too many rows, + which is why a filter that returns results is not evidence the filter + was applied. When validating a new expression against a live instance, + check that it returns *fewer* rows than the unfiltered baseline. +* **``;`` (AND) binds tighter than ``,`` (OR)**, so an unparenthesised + OR group silently drops every preceding AND clause for all but its + first alternative. See :func:`rsql_any_filter`, which exists for that + reason and carries the measured numbers. + +Nested *object* subfields can be joined (``status.id==1``, +``entity.id==3``). Nested *arrays* cannot: ``Ticket.team`` answers HTTP +500 for its contract-declared subfields and is silently ignored for every +other spelling, so actor selection has to go through the v1 search engine. +The same is expected of ``User.emails``. + +Date windows live on the public surface instead, in +:mod:`glpi_python_client.rsql`, because their grammar has an end-of-day +detail that is easy to get wrong and impossible to notice. """ from __future__ import annotations diff --git a/glpi_python_client/_sync/clients/commons/_http.py b/glpi_python_client/_sync/clients/commons/_http.py index 550922e..e4dca06 100644 --- a/glpi_python_client/_sync/clients/commons/_http.py +++ b/glpi_python_client/_sync/clients/commons/_http.py @@ -310,6 +310,45 @@ def response_json_mapping(response: httpx.Response) -> Mapping[str, object]: return result if isinstance(result, Mapping) else {} +def coerce_response_int(value: object) -> int | None: + """Return ``value`` as an ``int`` when it plainly denotes one. + + Native integers pass through. A string is accepted when it parses + cleanly, because GLPI is PHP-backed and PHP-backed APIs routinely + render integers as strings; refusing one would be a protocol error + raised over a perfectly usable identifier. ``bool`` is rejected even + though it is an ``int`` subclass in Python, and floats are rejected + because a fractional identifier is a sign of a misread payload rather + than a value to round. + """ + + if isinstance(value, bool): + return None + if isinstance(value, int): + return value + if isinstance(value, str): + try: + return int(value.strip()) + except ValueError: + return None + return None + + +def response_id_from_location(response: httpx.Response) -> int | None: + """Return the trailing identifier of the response ``Location`` header. + + A create that answers with an empty body and the URL of the new + resource is an ordinary REST shape. Reading the last path segment + recovers the identifier instead of failing a request that succeeded. + Returns ``None`` when there is no header or its tail is not numeric. + """ + + location = response.headers.get("Location") or response.headers.get("location") + if not location: + return None + return coerce_response_int(location.rstrip("/").rsplit("/", 1)[-1]) + + def require_response_int( response: httpx.Response, *, @@ -322,17 +361,35 @@ def require_response_int( set of keys. Callers list the candidate keys explicitly so the behaviour stays predictable. + Three shapes beyond a plain top-level integer are accepted, because + each is a response the server considers successful and each cost the + caller the identifier of a record that already exists: a numeric + string, an id nested one level under ``data``, and an empty body + whose ``Location`` header names the new resource. Top-level keys are + probed before nested ones, and the header is consulted last. + Raises ------ GlpiProtocolError - When none of ``keys`` maps to an integer value in the response. + When no candidate key and no ``Location`` header yields an + identifier. """ result = response_json_mapping(response) - for key in keys: - value = result.get(key) - if isinstance(value, int) and not isinstance(value, bool): - return value + candidates: list[Mapping[str, object]] = [result] + nested = result.get("data") + if isinstance(nested, Mapping): + candidates.append(nested) + + for mapping in candidates: + for key in keys: + found = coerce_response_int(mapping.get(key)) + if found is not None: + return found + + from_location = response_id_from_location(response) + if from_location is not None: + return from_location raise GlpiProtocolError(missing_message) @@ -380,6 +437,7 @@ def unwrap_timeline_items(payload: object) -> list[dict[str, object]]: __all__ = [ "build_request_headers", "build_request_url", + "coerce_response_int", "ensure_response_status", "finalize_request_response", "list_payload_items", @@ -387,6 +445,7 @@ def unwrap_timeline_items(payload: object) -> list[dict[str, object]]: "request_params", "require_access_token", "require_response_int", + "response_id_from_location", "response_json_mapping", "response_json_or_empty", "transport_error_from", diff --git a/glpi_python_client/_sync/clients/commons/_payloads.py b/glpi_python_client/_sync/clients/commons/_payloads.py index 15e24b3..69e1232 100644 --- a/glpi_python_client/_sync/clients/commons/_payloads.py +++ b/glpi_python_client/_sync/clients/commons/_payloads.py @@ -7,36 +7,89 @@ from __future__ import annotations +from datetime import tzinfo from typing import TypeVar -from glpi_python_client.models._base import GlpiModel +from glpi_python_client.models._base import ( + SERVER_TIMEZONE_CONTEXT_KEY, + GlpiModel, +) ModelT = TypeVar("ModelT", bound=GlpiModel) -def model_to_payload(model: GlpiModel) -> dict[str, object]: +def model_to_payload( + model: GlpiModel, + *, + server_timezone: tzinfo | None = None, +) -> dict[str, object]: """Serialise one :class:`GlpiModel` into a request body. ``None`` fields are omitted, the meta ``extra_payload`` field is excluded from the dump, and any user-provided ``extra_payload`` keys are merged on top so callers can inject contract-validated extras the package does not yet model. + + The dump runs in JSON mode. The returned mapping is handed to the HTTP + library as a JSON body, and its encoder is :func:`json.dumps`, which + cannot represent a ``datetime`` -- python mode leaves those as live + objects and every write of a date field then fails at the encoder, past + the point any transport stub can see. JSON mode renders them as ISO-8601 + strings instead, so what the model validated is what GLPI receives. + + ``server_timezone`` is passed as a Pydantic serialisation context, the + mirror of the validation context in :func:`model_from_payload` and + threaded the same way, so it reaches nested submodels. It is needed + because JSON mode alone renders an aware datetime *with* its offset, and + GLPI 11 does not read that offset: it takes the naive prefix, interprets + it in the server's own timezone, and discards the rest -- measured across + offsets from ``-08:00`` to ``+14:00``, which all stored the same moment. + The context lets the value be converted onto that clock first -- see + :meth:`GlpiModel._render_datetimes_on_the_server_clock`. ``None`` + converts nothing, so a model dumped outside the client is unchanged. """ - body = model.model_dump(exclude_none=True, exclude={"extra_payload"}) + context = ( + {SERVER_TIMEZONE_CONTEXT_KEY: server_timezone} + if server_timezone is not None + else None + ) + body = model.model_dump( + mode="json", + exclude_none=True, + exclude={"extra_payload"}, + context=context, + ) if model.extra_payload: body.update(model.extra_payload) return body -def model_from_payload(model_class: type[ModelT], payload: object) -> ModelT: +def model_from_payload( + model_class: type[ModelT], + payload: object, + *, + server_timezone: tzinfo | None = None, +) -> ModelT: """Validate one raw GLPI payload into the requested ``GlpiModel`` class. - The helper is a thin wrapper around ``model_validate`` that keeps the - mixin call sites concise and consistent with :func:`model_to_payload`. + The helper wraps ``model_validate`` so the mixin call sites stay concise + and consistent with :func:`model_to_payload`, and so the server timezone + is threaded through one place instead of forty field declarations. + + ``server_timezone`` is passed as a Pydantic validation context, which + reaches nested submodels as well as the top-level one -- necessary + because the timestamps GLPI sends without an offset are nested + (``KBArticle.revisions[].date``). ``None`` supplies no context at all, + so naive values stay naive rather than being stamped with a guess. """ - return model_class.model_validate(payload) + context = ( + {SERVER_TIMEZONE_CONTEXT_KEY: server_timezone} + if server_timezone is not None + else None + ) + return model_class.model_validate(payload, context=context) __all__ = ["model_from_payload", "model_to_payload"] diff --git a/glpi_python_client/_sync/clients/commons/_transport.py b/glpi_python_client/_sync/clients/commons/_transport.py index ec24d51..6ec2e12 100644 --- a/glpi_python_client/_sync/clients/commons/_transport.py +++ b/glpi_python_client/_sync/clients/commons/_transport.py @@ -25,7 +25,8 @@ from __future__ import annotations import logging -from collections.abc import Callable +from collections.abc import Iterator, Callable +from datetime import tzinfo from typing import TYPE_CHECKING, Any, TypeVar import httpx @@ -108,6 +109,7 @@ class TransportMixin: glpi_entity: int | None glpi_profile: int | None language: str + server_timezone: tzinfo def _ensure_open(self) -> None: """Raise when the client has already been closed. @@ -192,6 +194,65 @@ def _send_request( except httpx.HTTPError as exc: raise transport_error_from(exc, method=method, url=url) from exc + def _stream_request( + self, + endpoint: str, + *, + chunk_size: int, + skip_entity: bool = False, + failure_message: str, + ) -> Iterator[bytes]: + """Stream one authenticated GLPI ``GET`` body in chunks. + + The non-streaming helpers materialise the whole body before the + caller sees any of it, which is fine for JSON and wrong for a + document that may be hundreds of megabytes. + + Two details differ from the buffered path and both are load-bearing. + The status has to be checked *inside* the context manager, and the + body read first: the error helpers format the response text, and + reading text off an unread stream raises rather than reporting the + status. And no retry decorator belongs here -- tenacity does not + wrap an async generator, so a decorator would silently degrade to + the sync path instead of failing loudly. + + Raises + ------ + GlpiStatusError + When the response status is not 200. + GlpiTransportError + When the request never produced a response. + """ + + self._ensure_token() + access_token = require_access_token(self._auth.access_token) + url = build_request_url(self.glpi_api_url, endpoint) + headers = build_request_headers( + access_token=access_token, + language=self.language, + glpi_entity=self.glpi_entity, + glpi_profile=self.glpi_profile, + entity_recursive=self.entity_recursive, + include_content_type=False, + skip_entity=skip_entity, + ) + + try: + with self._session.stream( + "GET", url, headers=headers, timeout=30 + ) as response: + if response.status_code != 200: + response.read() + ensure_response_status( + response, + success_statuses=(200,), + failure_message=failure_message, + ) + for chunk in response.iter_bytes(chunk_size): + yield chunk + except httpx.HTTPError as exc: + raise transport_error_from(exc, method="get", url=url) from exc + def _execute_request( self, *, @@ -354,9 +415,9 @@ def _resource_list( skip_entity : bool, optional When ``True`` the ``GLPI-Entity`` header is omitted. failure_message : str | None, optional - When provided, response status is checked with this message; - search-style endpoints that tolerate empty results pass - ``None``. + Message embedded in the raised ``GlpiStatusError``. ``None`` + derives one from the endpoint; the status is checked either + way. success_statuses : tuple[int, ...], optional HTTP status codes considered successful when ``failure_message`` is set. @@ -373,19 +434,28 @@ def _resource_list( response = self._get_request( endpoint, params=params, skip_entity=skip_entity ) - if failure_message is not None: - ensure_response_status( - response, - success_statuses=success_statuses, - failure_message=failure_message, - ) + # The status is checked on every list call, search included. It used + # not to be, and a refused search then came back as ``[]``: a 403 was + # indistinguishable from a filter that matched nothing. It composed + # badly with the batch iterators, which stop on a page shorter than + # ``batch_size`` -- so a 403 on page one ended the walk having + # yielded nothing and the caller saw a successful empty result. An + # empty list now means the server said the result set is empty. + ensure_response_status( + response, + success_statuses=success_statuses, + failure_message=failure_message or f"Failed to list {endpoint}", + ) payload = response.json() items = ( unwrap_timeline_items(payload) if unwrap_envelope else list_payload_items(payload) ) - return [model_from_payload(model, item) for item in items] + return [ + model_from_payload(model, item, server_timezone=self.server_timezone) + for item in items + ] def _resource_get( self, @@ -421,7 +491,21 @@ def _resource_get( success_statuses=(200, 206), failure_message=failure_message, ) - return model_from_payload(model, response.json()) + return model_from_payload( + model, response.json(), server_timezone=self.server_timezone + ) + + def _body(self, model: GlpiModel) -> dict[str, object]: + """Serialise one model into a request body on the server's clock. + + Every write in the package goes through here rather than calling + :func:`model_to_payload` directly, because the timezone argument is + not optional in practice and a call site that forgets it produces no + error -- just a timestamp GLPI silently reinterprets. Binding it once + leaves nothing to remember at the next endpoint. + """ + + return model_to_payload(model, server_timezone=self.server_timezone) def _resource_create( self, @@ -466,7 +550,7 @@ def _resource_create( """ response = self._post_request( - endpoint, model_to_payload(body_model), skip_entity=skip_entity + endpoint, self._body(body_model), skip_entity=skip_entity ) ensure_response_status( response, @@ -507,7 +591,7 @@ def _resource_update( None """ - response = self._update_request(endpoint, model_to_payload(body_model)) + response = self._update_request(endpoint, self._body(body_model)) ensure_response_status( response, success_statuses=(200, 204), @@ -558,7 +642,7 @@ def _resource_delete( request_body = body if request_body is None and delete_model_cls is not None and force is not None: - request_body = model_to_payload(delete_model_cls(force=force)) # type: ignore[call-arg] + request_body = self._body(delete_model_cls(force=force)) # type: ignore[call-arg] response = self._delete_request( endpoint, request_body, skip_entity=skip_entity ) diff --git a/glpi_python_client/_sync/clients/commons/tests/test_config.py b/glpi_python_client/_sync/clients/commons/tests/test_config.py index 58884ae..3ac66c1 100644 --- a/glpi_python_client/_sync/clients/commons/tests/test_config.py +++ b/glpi_python_client/_sync/clients/commons/tests/test_config.py @@ -7,6 +7,8 @@ from __future__ import annotations +from datetime import datetime, timedelta, timezone + import pytest from glpi_python_client import GlpiValidationError @@ -15,6 +17,7 @@ normalize_client_api_url, parse_optional_env_bool, parse_optional_env_int, + resolve_server_timezone, validate_v1_document_config, ) @@ -160,3 +163,47 @@ def test_build_client_env_config_overrides_win() -> None: assert config["glpi_api_url"] == "https://override" assert config["verify_ssl"] is False assert config["language"] == "en_GB" + + +def test_resolve_server_timezone_accepts_an_iana_name() -> None: + """An IANA zone name resolves to a DST-aware timezone. + + A name rather than a fixed offset is what makes winter and summer both + correct: the same GLPI instance sends ``+01:00`` and ``+02:00``. + """ + + resolved = resolve_server_timezone("Europe/Paris") + + assert datetime(2019, 1, 15, tzinfo=resolved).utcoffset() == timedelta(hours=1) + assert datetime(2018, 7, 15, tzinfo=resolved).utcoffset() == timedelta(hours=2) + + +def test_resolve_server_timezone_passes_a_tzinfo_through() -> None: + """A caller holding a tzinfo object may supply it directly.""" + + fixed = timezone(timedelta(hours=2)) + + assert resolve_server_timezone(fixed) is fixed + + +def test_resolve_server_timezone_rejects_an_unknown_name() -> None: + """A typo fails at construction, not silently at the first timestamp.""" + + with pytest.raises(GlpiValidationError) as excinfo: + resolve_server_timezone("Europe/Pariss") + + assert "Europe/Pariss" in str(excinfo.value) + + +def test_resolve_server_timezone_rejects_a_missing_value() -> None: + """The timezone is required; GLPI does not advertise it.""" + + with pytest.raises(GlpiValidationError): + resolve_server_timezone(None) + + +def test_resolve_server_timezone_rejects_a_blank_name() -> None: + """An empty environment variable is a missing value, not a valid one.""" + + with pytest.raises(GlpiValidationError): + resolve_server_timezone(" ") diff --git a/glpi_python_client/_sync/clients/commons/tests/test_http.py b/glpi_python_client/_sync/clients/commons/tests/test_http.py index b34fec1..7348c7f 100644 --- a/glpi_python_client/_sync/clients/commons/tests/test_http.py +++ b/glpi_python_client/_sync/clients/commons/tests/test_http.py @@ -195,3 +195,52 @@ def test_4xx_raises_a_typed_status_error_from_ensure_response_status() -> None: assert excinfo.value.status_code == 404 assert isinstance(excinfo.value, ValueError) assert str(excinfo.value) == "Failed to fetch ticket 1: 404 nope" + + +def test_require_response_int_accepts_a_numeric_string_id() -> None: + """A quoted identifier is still an identifier. + + GLPI is PHP-backed and PHP-backed APIs commonly render integers as + strings. Rejecting one is a protocol error raised over a usable value. + """ + + response = FakeResponse(payload={"id": "4242"}) + + assert require_response_int(response, keys=("id",), missing_message="x") == 4242 + + +def test_require_response_int_ignores_a_non_numeric_string_id() -> None: + """A string that is not a number is not silently coerced.""" + + response = FakeResponse(payload={"id": "not-a-number"}) + + with pytest.raises(GlpiProtocolError): + require_response_int(response, keys=("id",), missing_message="x") + + +def test_require_response_int_reads_a_nested_data_envelope() -> None: + """An id wrapped in a ``data`` envelope is found rather than refused.""" + + response = FakeResponse(payload={"data": {"id": 4242}}) + + assert require_response_int(response, keys=("id",), missing_message="x") == 4242 + + +def test_require_response_int_falls_back_to_the_location_header() -> None: + """A 201 whose id lives only in ``Location`` still yields the id. + + A create that answers with an empty body and a resource URL is a normal + REST shape; without this the client raises a protocol error on a request + that succeeded, and the caller loses the id of a record that now exists. + """ + + response = FakeResponse( + status_code=201, + payload=None, + content=b"", + headers={ + "Location": "https://glpi.example.test/api.php/v2/Assistance/Ticket/4242" + }, + ) + + assert require_response_int(response, keys=("id",), missing_message="x") == 4242 diff --git a/glpi_python_client/_sync/clients/commons/tests/test_payloads.py b/glpi_python_client/_sync/clients/commons/tests/test_payloads.py index 443dbc2..3bcf86f 100644 --- a/glpi_python_client/_sync/clients/commons/tests/test_payloads.py +++ b/glpi_python_client/_sync/clients/commons/tests/test_payloads.py @@ -2,6 +2,9 @@ from __future__ import annotations +import json +from datetime import datetime, timedelta, timezone + from glpi_python_client._sync.clients.commons._payloads import ( model_from_payload, model_to_payload, @@ -10,6 +13,9 @@ GetUser, PostUser, ) +from glpi_python_client.models.api_schema.assistance.timeline._task import ( + PostTicketTask, +) def test_model_to_payload_excludes_none_and_extra_payload_meta() -> None: @@ -62,3 +68,54 @@ def test_capture_extra_keys_merges_with_existing_extra_payload_dict() -> None: "stranger": "explicit", "caller_wins": True, } + + +def test_model_to_payload_body_is_json_encodable() -> None: + """Every value in the body survives the JSON encoder httpx will use. + + ``_execute_request`` hands the mapping straight to ``httpx`` as ``json=``, + whose encoder is ``json.dumps``. A body holding a live ``datetime`` object + raises ``TypeError`` there -- after the model validated, and outside any + transport stub -- so the assertion has to be encodability, not shape. + """ + + body = model_to_payload(PostTicketTask(planned_begin=datetime(2024, 1, 1, 12, 0))) + + assert json.dumps(body) + + +def test_model_to_payload_renders_naive_datetime_without_offset() -> None: + """A naive datetime reaches GLPI as the bare timestamp it was given.""" + + body = model_to_payload(PostTicketTask(planned_begin=datetime(2024, 1, 1, 12, 0))) + + assert body["planned_begin"] == "2024-01-01T12:00:00" + + +def test_model_to_payload_rewrites_an_aware_datetime_onto_the_server_clock() -> None: + """An aware datetime is converted to server-local time and sent bare. + + GLPI 11 ignores the offset it is sent -- measured, three spellings of one + moment all stored the same wall clock -- so writing ``12:00Z`` to a + ``+01:00`` instance stores 12:00 there and loses an hour silently. The + offset has to be spent on the conversion instead of written out. + """ + + aware = datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc) + + body = model_to_payload( + PostTicketTask(planned_begin=aware), + server_timezone=timezone(timedelta(hours=1)), + ) + + assert body["planned_begin"] == "2024-01-01T13:00:00" + + +def test_model_to_payload_leaves_an_aware_datetime_alone_without_a_timezone() -> None: + """Outside the client there is no server clock to convert onto.""" + + aware = datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc) + + body = model_to_payload(PostTicketTask(planned_begin=aware)) + + assert body["planned_begin"] == "2024-01-01T12:00:00Z" diff --git a/glpi_python_client/_sync/clients/commons/tests/test_transport.py b/glpi_python_client/_sync/clients/commons/tests/test_transport.py index c1c24c8..e4d4a69 100644 --- a/glpi_python_client/_sync/clients/commons/tests/test_transport.py +++ b/glpi_python_client/_sync/clients/commons/tests/test_transport.py @@ -25,6 +25,7 @@ GlpiClient, GlpiError, GlpiServerError, + GlpiStatusError, GlpiTimeoutError, GlpiTransportError, ) @@ -330,20 +331,56 @@ def _send(method: str, url: str, **kw: Any) -> FakeResponse: assert response.status_code == 404 -def test_tolerant_search_still_returns_empty_on_4xx(retry_client: Any) -> None: - """Search endpoints that pass no ``failure_message`` still swallow a 4xx. - - Guards the 7 tolerant ``_resource_list`` call sites against the 4xx raise - being moved into ``finalize_request_response``. +@pytest.mark.parametrize("status", [400, 401, 403, 404]) +def test_search_raises_on_4xx_instead_of_returning_empty( + retry_client: Any, status: int +) -> None: + """A refused search is an error, not an empty result set. + + Search endpoints pass no ``failure_message``, and that used to mean the + status went unchecked: a 403 came back as ``[]``, indistinguishable from + a filter that matched nothing. It composed badly with the batch + iterators, which stop on a short page -- so a 403 on page one ended the + walk having yielded nothing and the caller saw a successful empty + result. """ def _send(method: str, url: str, **kw: Any) -> FakeResponse: - return FakeResponse(status_code=400, payload=[], text="[]") + return FakeResponse(status_code=status, payload=[], text="[]") + + retry_client._send_request = _send # type: ignore[method-assign] + + with pytest.raises(GlpiStatusError): + retry_client.search_tickets() + + +def test_search_still_returns_empty_on_a_genuine_empty_result( + retry_client: Any, +) -> None: + """A 200 carrying an empty list is still an ordinary empty result.""" + + def _send(method: str, url: str, **kw: Any) -> FakeResponse: + return FakeResponse(status_code=200, payload=[], text="[]") retry_client._send_request = _send # type: ignore[method-assign] + assert retry_client.search_tickets() == [] +def test_iterators_surface_a_4xx_instead_of_ending_silently( + retry_client: Any, +) -> None: + """The iterator raises rather than yielding nothing and looking finished.""" + + def _send(method: str, url: str, **kw: Any) -> FakeResponse: + return FakeResponse(status_code=403, payload=[], text="[]") + + retry_client._send_request = _send # type: ignore[method-assign] + + with pytest.raises(GlpiStatusError): + [batch for batch in retry_client.iter_search_tickets()] + + @pytest.mark.parametrize("method_name", _RETRIED_METHODS) def test_network_errors_are_still_retried( retry_client: Any, method_name: str diff --git a/glpi_python_client/_sync/clients/custom/_statistics.py b/glpi_python_client/_sync/clients/custom/_statistics.py index 0e2a25b..b493a4b 100644 --- a/glpi_python_client/_sync/clients/custom/_statistics.py +++ b/glpi_python_client/_sync/clients/custom/_statistics.py @@ -33,6 +33,7 @@ GlpiPriority, GlpiTicketType, ) +from glpi_python_client.rsql import created_between #: The GLPI v2 ticket search includes soft-deleted ("trashed") tickets by #: default, while the v1 search excludes them. Every aggregation here is @@ -382,27 +383,34 @@ def get_ticket_statistics( entity_filter = f"entity.id=={entity_id}" elif entity_name is not None: name_filter = rsql_contains_filter("name", entity_name) or "" - entities = self.search_entities( # type: ignore[attr-defined] - rsql_filter=name_filter, - limit=200, - ) + entities = [] + for entity_batch in self.iter_search_entities( # type: ignore[attr-defined] + name_filter, + batch_size=200, + ): + entities.extend(entity_batch) if not entities: return {"entities": {}} entity_filter = rsql_any_filter( *(f"entity.id=={e.id}" for e in entities if e.id is not None) ) - date_filter = f"date_creation=ge={start.isoformat()};" - date_filter += f"date_creation=le={end.isoformat()} 23:59:59" + date_filter = created_between(start, end) query = rsql_all_filter( date_filter, entity_filter, _LIVE_TICKETS, extra_filter, ) - tickets: list[GetTicket] = self.search_tickets( # type: ignore[attr-defined] - rsql_filter=query or "", - limit=200, - ) + # Paged rather than fetched in one call. A single ``search_tickets`` + # returns one page and nothing else, so any corpus larger than the + # page size was silently truncated and the helper reported a + # plausible number that was really just the page size. + tickets: list[GetTicket] = [] + for batch in self.iter_search_tickets( # type: ignore[attr-defined] + query or "", + batch_size=200, + ): + tickets.extend(batch) return _summarize_tickets(tickets) def get_task_statistics( @@ -518,18 +526,19 @@ def get_task_durations( end_date=end_date, default_days=default_days, ) - date_filter = f"date_creation=ge={start.isoformat()};" - date_filter += f"date_creation=le={end.isoformat()} 23:59:59" + date_filter = created_between(start, end) entity_filter: str | None = None if entity_id is not None: entity_filter = f"entity.id=={entity_id}" elif entity_name is not None: name_filter = rsql_contains_filter("name", entity_name) or "" - entities = self.search_entities( # type: ignore[attr-defined] - rsql_filter=name_filter, - limit=200, - ) + entities = [] + for entity_batch in self.iter_search_entities( # type: ignore[attr-defined] + name_filter, + batch_size=200, + ): + entities.extend(entity_batch) if not entities: return TaskDurationsResult( start_date=start.isoformat(), @@ -714,10 +723,12 @@ def get_user_activity( rsql_contains_filter("firstname", firstname) if firstname else None, ] user_rsql = rsql_all_filter(*name_parts) or "" - matched_users = self.search_users( # type: ignore[attr-defined] - rsql_filter=user_rsql, - limit=200, - ) + matched_users = [] + for user_batch in self.iter_search_users( # type: ignore[attr-defined] + user_rsql, + batch_size=200, + ): + matched_users.extend(user_batch) if not matched_users: raise GlpiValidationError("No users matched the supplied criteria") resolved_user_ids = [u.id for u in matched_users if u.id is not None] @@ -731,8 +742,7 @@ def get_user_activity( if u.id is not None } - date_range = f"date_creation=ge={start.isoformat()};" - date_range += f"date_creation=le={end.isoformat()} 23:59:59" + date_range = created_between(start, end) # The date window is resolved once for every user rather than once # per user per role. Previously each user drove two full pagings of diff --git a/glpi_python_client/_sync/clients/custom/tests/test_statistics.py b/glpi_python_client/_sync/clients/custom/tests/test_statistics.py index 6939b83..4daf77d 100644 --- a/glpi_python_client/_sync/clients/custom/tests/test_statistics.py +++ b/glpi_python_client/_sync/clients/custom/tests/test_statistics.py @@ -105,7 +105,7 @@ def test_get_ticket_statistics_aggregates_by_entity_status_priority_type( captured: dict[str, Any] = {} def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [ @@ -154,7 +154,7 @@ def test_get_ticket_statistics_default_window_uses_today( captured: dict[str, Any] = {} def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -267,7 +267,7 @@ def test_get_ticket_statistics_entity_id_filter(client: Any) -> None: captured: dict[str, Any] = {} def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -299,7 +299,7 @@ def fake_search_entities( return [GetEntity(id=3, name="Acme"), GetEntity(id=4, name="Acme Sub")] def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured_tickets["filter"] = rsql_filter return [] @@ -344,7 +344,7 @@ def test_get_ticket_statistics_extra_filter_appended(client: Any) -> None: captured: dict[str, Any] = {} def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -367,7 +367,7 @@ def test_get_ticket_statistics_default_days_window(client: Any) -> None: captured: dict[str, Any] = {} def fake_search( - rsql_filter: str = "", *, limit: int = 50, start: int = 0 + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any ) -> list[GetTicket]: captured["filter"] = rsql_filter return [] @@ -784,3 +784,102 @@ def fake_task_stats(ticket_ids: list[int]) -> dict[str, Any]: assert result["task_count"] == 0 assert calls == [[1]] + + +def test_get_ticket_statistics_counts_beyond_one_page(client: Any) -> None: + """Aggregates cover every matching ticket, not just the first page. + + A single ``search_tickets`` call returns one page and nothing else, so a + corpus larger than the page size was silently truncated: the helper + reported a plausible number that happened to be the page size. The stub + answers a full page followed by a short one, which is the only shape that + tells a paging walk apart from a single request. + """ + + pages = [[_ticket() for _ in range(200)], [_ticket() for _ in range(30)]] + calls: list[int] = [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetTicket]: + calls.append(start) + index = start // limit + return pages[index] if index < len(pages) else [] + + client.search_tickets = fake_search # type: ignore[method-assign] + + result = client.get_ticket_statistics(default_days=7) + + assert calls == [0, 200] + assert result["entities"]["1"]["total"] == 230 + + +def test_get_ticket_statistics_resolves_entities_beyond_one_page( + client: Any, +) -> None: + """Entity-name resolution walks every page of matches. + + A name prefix shared by many entities matches more than one page. A + single ``search_entities`` call drops the rest, so tickets belonging to + the unlisted entities vanish from the aggregate with no error. + """ + + from glpi_python_client.models.api_schema.administration._entity import GetEntity + + pages = [ + [GetEntity(id=i, name="Acme") for i in range(1, 201)], + [GetEntity(id=201, name="Acme Sub")], + ] + captured_tickets: dict[str, Any] = {} + + def fake_search_entities( + rsql_filter: str = "", *, limit: int = 200, start: int = 0, **kwargs: Any + ) -> list[GetEntity]: + index = start // limit + return pages[index] if index < len(pages) else [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetTicket]: + captured_tickets["filter"] = rsql_filter + return [] + + client.search_entities = fake_search_entities # type: ignore[method-assign] + client.search_tickets = fake_search # type: ignore[method-assign] + + client.get_ticket_statistics(default_days=7, entity_name="Acme") + + assert "entity.id==201" in captured_tickets["filter"] + + +def test_get_task_durations_resolves_entities_beyond_one_page( + client: Any, +) -> None: + """``get_task_durations`` pages entity-name resolution the same way.""" + + from glpi_python_client.models.api_schema.administration._entity import GetEntity + + pages = [ + [GetEntity(id=i, name="Acme") for i in range(1, 201)], + [GetEntity(id=201, name="Acme Sub")], + ] + captured_tickets: dict[str, Any] = {} + + def fake_search_entities( + rsql_filter: str = "", *, limit: int = 200, start: int = 0, **kwargs: Any + ) -> list[GetEntity]: + index = start // limit + return pages[index] if index < len(pages) else [] + + def fake_search( + rsql_filter: str = "", *, limit: int = 50, start: int = 0, **kwargs: Any + ) -> list[GetTicket]: + captured_tickets["filter"] = rsql_filter + return [] + + client.search_entities = fake_search_entities # type: ignore[method-assign] + client.search_tickets = fake_search # type: ignore[method-assign] + + client.get_task_durations(default_days=7, entity_name="Acme") + + assert "entity.id==201" in captured_tickets["filter"] diff --git a/glpi_python_client/_sync/clients/tests/test_client.py b/glpi_python_client/_sync/clients/tests/test_client.py index 70624f8..d4980fc 100644 --- a/glpi_python_client/_sync/clients/tests/test_client.py +++ b/glpi_python_client/_sync/clients/tests/test_client.py @@ -21,6 +21,7 @@ def test_glpi_client_from_env_uses_overrides_and_defaults( "GLPI_API_URL": "https://glpi.example.test/api.php/v2", "GLPI_USERNAME": "u", "GLPI_PASSWORD": "p", + "GLPI_SERVER_TIMEZONE": "Europe/Paris", } client = GlpiClient.from_env(env=env) try: @@ -34,6 +35,7 @@ def test_glpi_client_close_is_idempotent() -> None: client = GlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", ) @@ -61,7 +63,10 @@ def test_glpi_client_rejects_invalid_credentials() -> None: """Constructor refuses to build a client with no usable credentials.""" with pytest.raises(ValueError): - GlpiClient(glpi_api_url="https://glpi.example.test/api.php/v2") + GlpiClient( + glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", + ) def test_glpi_client_v1_session_built_when_configured() -> None: @@ -69,6 +74,7 @@ def test_glpi_client_v1_session_built_when_configured() -> None: client = GlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", v1_base_url="https://glpi.example.test/apirest.php", @@ -87,6 +93,7 @@ def test_glpi_client_rejects_partial_v1_config() -> None: with pytest.raises(ValueError, match="v1_base_url and v1_user_token"): GlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", v1_base_url="https://glpi.example.test/apirest.php", @@ -101,6 +108,7 @@ def test_environ_default_is_used_when_env_argument_omitted( monkeypatch.setenv("GLPI_API_URL", "https://from-environ.example/api.php/v2") monkeypatch.setenv("GLPI_USERNAME", "u") monkeypatch.setenv("GLPI_PASSWORD", "p") + monkeypatch.setenv("GLPI_SERVER_TIMEZONE", "Europe/Paris") client = GlpiClient.from_env() try: assert client.glpi_api_url.endswith("/api.php/v2") @@ -113,6 +121,7 @@ def test_async_transport_ensure_open_blocks_after_close() -> None: client = GlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", username="u", password="p", ) @@ -149,6 +158,7 @@ def _track_init(self: httpx.Client, *args: Any, **kwargs: Any) -> None: with pytest.raises(ValueError): GlpiClient( glpi_api_url="https://glpi.example.test/api.php/v2", + server_timezone="Europe/Paris", client_id="only-id-no-secret", ) assert constructed == [], "a transport session was built for a rejected config" diff --git a/glpi_python_client/content/conversion.py b/glpi_python_client/content/conversion.py index dce7c3d..bacfeb4 100644 --- a/glpi_python_client/content/conversion.py +++ b/glpi_python_client/content/conversion.py @@ -6,9 +6,73 @@ from __future__ import annotations +import re + from markdown import markdown as markdown_to_html from markdownify import markdownify as html_to_markdown +#: Element names that make a ``<...>`` sequence markup rather than text. +#: +#: The HTML5 element set, which is what the parser behind ``markdownify`` +#: will actually recognise. Anything outside it -- ````, ````, +#: ```` -- parses as an *unknown* tag, whose markup is dropped +#: while its (usually empty) body is kept, so the token silently vanishes +#: from the middle of a sentence. +_HTML_ELEMENTS = frozenset( + """ + a abbr address area article aside audio b base bdi bdo blockquote body br + button canvas caption cite code col colgroup data datalist dd del details + dfn dialog div dl dt em embed fieldset figcaption figure footer form h1 h2 + h3 h4 h5 h6 head header hgroup hr html i iframe img input ins kbd label + legend li link main map mark menu meta meter nav noscript object ol optgroup + option output p param picture pre progress q rp rt ruby s samp script search + section select slot small source span strong style sub summary sup table + tbody td template textarea tfoot th thead time title tr track u ul var video + wbr + """.split() +) + +#: python-markdown extensions applied when rendering outbound content. +#: +#: ``fenced_code`` and ``tables`` are here because without them the two +#: constructs do not survive at all. A fence rendered without +#: ``fenced_code`` becomes inline ````, which the GLPI web UI shows +#: as one run-on line and which a later read writes back as inline code -- +#: so a pasted log degrades a little more on every edit. A table without +#: ``tables`` renders as literal pipe characters. +#: +#: A language tag is still lost: ``markdownify`` drops the +#: ``class="language-python"`` that ``fenced_code`` emits, so ```` ```python ```` +#: comes back as a bare fence. That is a limitation of the pair of +#: libraries, not something an extension list can fix. +_MARKDOWN_EXTENSIONS = ["nl2br", "sane_lists", "fenced_code", "tables"] + +#: One candidate tag: ``<`` or `` 1`` and ``x <= y`` text: a space after ``<`` +#: means no tag, so arithmetic never reaches the HTML path in the first +#: place. +_CANDIDATE_TAG = re.compile(r"]*>") + + +def _looks_like_html(content: str) -> bool: + """Return whether ``content`` carries at least one real HTML element. + + Deciding on the element *name* rather than on the presence of angle + brackets is what separates markup from prose that merely contains + ``<`` and ``>``. It cannot separate them perfectly: ``ac`` is + genuinely ambiguous, because ``b`` is both a real element and a + plausible variable, and no probe reading the text alone can resolve + that. It resolves every case where the name is not an element at all, + which is where the silent deletions came from. + """ + + return any( + match.group(1).lower() in _HTML_ELEMENTS + for match in _CANDIDATE_TAG.finditer(content) + ) + class GlpiContentConverter: """Convert content between GLPI HTML payloads and canonical Markdown. @@ -23,12 +87,18 @@ def from_transport(value: object) -> str: Empty input stays empty, plain text is preserved, and HTML content is normalized through ``markdownify`` with the package's preferred options. + + The HTML path is taken only when :func:`_looks_like_html` finds a real + element. Both directions of that decision matter, because this method + is also wired as the inbound validator for caller-authored content: + text sent down the HTML path loses whatever the parser does not + recognise, and Markdown sent down it comes back escaped. """ content = str(value or "") if not content.strip(): return "" - if "<" not in content or ">" not in content: + if not _looks_like_html(content): return content.strip() markdown = html_to_markdown( @@ -36,6 +106,8 @@ def from_transport(value: object) -> str: heading_style="ATX", bullets="-", strip=["script", "style"], + escape_underscores=False, + escape_asterisks=False, ) return str(markdown).strip() @@ -52,7 +124,7 @@ def to_transport(value: object) -> str: return "" html = markdown_to_html( markdown, - extensions=["nl2br", "sane_lists"], + extensions=_MARKDOWN_EXTENSIONS, output_format="html5", ) return str(html).strip() diff --git a/glpi_python_client/content/tests/test_conversion.py b/glpi_python_client/content/tests/test_conversion.py index 5802d10..e708c95 100644 --- a/glpi_python_client/content/tests/test_conversion.py +++ b/glpi_python_client/content/tests/test_conversion.py @@ -1,5 +1,7 @@ from __future__ import annotations +import pytest + from glpi_python_client.content.conversion import GlpiContentConverter @@ -11,3 +13,98 @@ def test_content_converter_uses_markdown_in_python_and_html_for_glpi() -> None: assert markdown == "Hello **world**" assert html == "

Hello world

" + + +@pytest.mark.parametrize( + "text", + [ + "use the key", + "cmd out", + "if x0", + "tempmin", + "a b", + "", + "generic in the signature", + ], +) +def test_from_transport_preserves_text_whose_tags_are_not_html(text: str) -> None: + """Angle brackets around a non-element name are text, not markup. + + ```` parses as an unknown tag, and an unknown tag's markup is + dropped while its (empty) body is kept -- so the word disappears from the + middle of a sentence with nothing to show it was ever there. + """ + + assert GlpiContentConverter.from_transport(text) == text + + +def test_from_transport_still_converts_real_html() -> None: + """Tightening the probe must not stop genuine HTML being normalised.""" + + html = "

The printer is offline.

" + + assert GlpiContentConverter.from_transport(html) == "The printer is **offline**." + + +def test_from_transport_leaves_caller_markdown_untouched() -> None: + """Markdown authored by a caller survives the inbound normaliser. + + ``from_transport`` is wired as a Pydantic ``BeforeValidator``, so it also + runs on outbound content. Anything that sends caller Markdown down the + HTML path escapes it, and the ticket reaches GLPI showing literal + asterisks. + """ + + markdown = "The printer is **offline** and 5 * 3 = 15." + + assert GlpiContentConverter.from_transport(markdown) == markdown + + +def test_fenced_code_block_survives_the_round_trip() -> None: + """A fence stays a fence. Pasted logs are the common case for this.""" + + markdown = "```\nblock\n```" + + assert ( + GlpiContentConverter.from_transport(GlpiContentConverter.to_transport(markdown)) + == markdown + ) + + +def test_fenced_code_block_renders_as_a_pre_block() -> None: + """Outbound, a fence becomes ``
`` rather than inline code.
+
+    Inline ```` is what collapsed a multi-line log into one line in the
+    GLPI web UI, and what a read-modify-write then wrote back as inline code.
+    """
+
+    assert GlpiContentConverter.to_transport("```\nblock\n```") == (
+        "
block\n
"
+    )
+
+
+def test_table_survives_the_round_trip() -> None:
+    """A Markdown table stays a table instead of degrading to text."""
+
+    rendered = GlpiContentConverter.from_transport(
+        GlpiContentConverter.to_transport("| a | b |\n| - | - |\n| 1 | 2 |")
+    )
+
+    assert rendered == "| a | b |\n| --- | --- |\n| 1 | 2 |"
+
+
+@pytest.mark.parametrize(
+    ("html", "expected"),
+    [
+        ("
snake_case name
", "snake_case name"),
+        ("
5 * 3 = 15
", "5 * 3 = 15"),
+    ],
+)
+def test_incoming_text_is_not_backslash_escaped(html: str, expected: str) -> None:
+    r"""Underscores and asterisks in prose stay readable.
+
+    Escaping them turns ``snake_case`` into ``snake\_case`` on every read,
+    and the backslash accumulates across read-modify-write cycles.
+    """
+
+    assert GlpiContentConverter.from_transport(html) == expected
diff --git a/glpi_python_client/models/_base.py b/glpi_python_client/models/_base.py
index 4c27872..d534a1f 100644
--- a/glpi_python_client/models/_base.py
+++ b/glpi_python_client/models/_base.py
@@ -16,9 +16,27 @@
 
 from __future__ import annotations
 
+from datetime import datetime
 from typing import Any
 
-from pydantic import BaseModel, ConfigDict, Field, model_validator
+from pydantic import (
+    BaseModel,
+    ConfigDict,
+    Field,
+    SerializationInfo,
+    SerializerFunctionWrapHandler,
+    ValidationInfo,
+    model_serializer,
+    model_validator,
+)
+
+#: Validation-context key carrying the GLPI server's timezone.
+#:
+#: Set by ``model_from_payload`` in the client's ``commons._payloads``
+#: module from the client's ``server_timezone``. Absent when a model is built
+#: outside the client, which is deliberate -- see
+#: :meth:`GlpiModel._localise_naive_datetimes`.
+SERVER_TIMEZONE_CONTEXT_KEY = "server_timezone"
 
 
 class GlpiModel(BaseModel):
@@ -59,3 +77,100 @@ def _capture_unknown_fields(cls, data: Any) -> Any:
                 merged.update(existing_extras)
             data["extra_payload"] = merged
         return data
+
+    @model_validator(mode="after")
+    def _localise_naive_datetimes(self, info: ValidationInfo) -> GlpiModel:
+        """Stamp the server's timezone onto timestamps that arrived without one.
+
+        GLPI 11 sends most timestamps with the correct historical offset --
+        measured on a live instance, 19 of the 20 datetime fields across
+        every resource, and the same article carries ``+02:00`` in summer
+        and ``+01:00`` in winter. ``KBArticle.revisions[].date`` is the
+        exception and arrives bare, so one response can hold both kinds and
+        comparing them raises ``TypeError``. This closes that gap.
+
+        Two rules make it safe:
+
+        * **An offset already on the wire wins.** GLPI's own offset follows
+          DST; a single configured zone does not, so overwriting would
+          corrupt half the year.
+        * **No context means no guess.** A model built outside the client
+          keeps its naive values. Stamping an arbitrary offset on an unknown
+          timestamp would convert a loud ``TypeError`` into a quietly wrong
+          answer, which is strictly worse.
+
+        The stamped values are written onto a copy rather than assigned in
+        place, because a ``mode="after"`` validator receives the instance
+        itself -- and ``model_validate`` accepts an existing model, so
+        mutating would reach back into an object the caller still holds.
+        """
+
+        context = info.context
+        if not isinstance(context, dict):
+            return self
+        tzinfo = context.get(SERVER_TIMEZONE_CONTEXT_KEY)
+        if tzinfo is None:
+            return self
+
+        localised = {
+            name: value.replace(tzinfo=tzinfo)
+            for name in type(self).model_fields
+            if isinstance(value := getattr(self, name, None), datetime)
+            and value.tzinfo is None
+        }
+        if not localised:
+            return self
+        return self.model_copy(update=localised)
+
+    @model_serializer(mode="wrap")
+    def _render_datetimes_on_the_server_clock(
+        self,
+        handler: SerializerFunctionWrapHandler,
+        info: SerializationInfo,
+    ) -> Any:
+        """Convert aware datetimes to server-local time and drop the offset.
+
+        GLPI 11 does not read the offset it sends. Measured on a live
+        Europe/Paris instance, ``12:30:00`` written bare, as ``...Z``, and
+        with ``+02:00``, ``+09:00``, ``-08:00`` and ``+14:00`` all store the
+        same moment -- 12:30 Paris. The server takes the naive prefix,
+        interprets it in its own timezone, and discards the rest. It is not
+        ignoring the offset unparsed either: ``+99:99`` answers HTTP 500. So
+        the value is read and then thrown away, and ``12:30-08:00`` -- 21:30
+        in Paris -- lands nine hours early with a 200 and nothing to read
+        back that looks wrong.
+
+        An offset is therefore not something to preserve on the way out. The
+        only spelling GLPI cannot misread is one whose naive prefix is
+        already server-local, so the offset is spent converting the value
+        and then removed.
+
+        The two rules mirror the inbound half:
+
+        * **Naive values are left alone.** A naive datetime already means
+          "the server's clock" -- converting it would require guessing which
+          zone the caller meant.
+        * **No context means no conversion.** A model dumped outside the
+          client has no server to be local to.
+
+        Converted values go onto a copy for the same reason the inbound
+        validator copies: the caller still holds the model being dumped, and
+        serialising is not allowed to change it.
+        """
+
+        context = info.context
+        if not isinstance(context, dict):
+            return handler(self)
+        tzinfo = context.get(SERVER_TIMEZONE_CONTEXT_KEY)
+        if tzinfo is None:
+            return handler(self)
+
+        server_local = {
+            name: value.astimezone(tzinfo).replace(tzinfo=None)
+            for name in type(self).model_fields
+            if isinstance(value := getattr(self, name, None), datetime)
+            and value.tzinfo is not None
+        }
+        if not server_local:
+            return handler(self)
+        return handler(self.model_copy(update=server_local))
diff --git a/glpi_python_client/models/api_schema/_content.py b/glpi_python_client/models/api_schema/_content.py
index 6110af4..61ea729 100644
--- a/glpi_python_client/models/api_schema/_content.py
+++ b/glpi_python_client/models/api_schema/_content.py
@@ -16,10 +16,19 @@
   the Markdown value is rendered back to HTML so GLPI receives the format
   it expects.
 
-Plain-text content (no ``<...>`` markup) is preserved verbatim on the
-inbound path and rendered as HTML paragraphs on the outbound path, matching
-the converter's default behaviour. ``None`` values are passed through
-unchanged so optional fields and ``exclude_none`` semantics keep working.
+Plain-text content is preserved verbatim on the inbound path and rendered
+as HTML paragraphs on the outbound path, matching the converter's default
+behaviour. "Plain text" means text carrying no recognised HTML element:
+``use the  key`` and ``if x0`` are text, because ``Enter``
+and ``y`` are not elements, while ``ac`` is treated as markup because
+``b`` is. ``None`` values are passed through unchanged so optional fields
+and ``exclude_none`` semantics keep working.
+
+Note that the inbound converter also runs on **outbound** content: the
+``BeforeValidator`` below fires when a caller constructs a ``Post*`` model,
+so caller-authored Markdown passes through it before the serializer renders
+it. That is why the plain-text path has to stay verbatim -- routing Markdown
+through the HTML normaliser escapes it, and GLPI receives literal asterisks.
 """
 
 from __future__ import annotations
diff --git a/glpi_python_client/models/custom_schema/_ticket_context.py b/glpi_python_client/models/custom_schema/_ticket_context.py
index b00eb57..07d9d1e 100644
--- a/glpi_python_client/models/custom_schema/_ticket_context.py
+++ b/glpi_python_client/models/custom_schema/_ticket_context.py
@@ -9,7 +9,7 @@
 from __future__ import annotations
 
 from dataclasses import dataclass, field
-from datetime import datetime
+from datetime import datetime, timezone
 from enum import Enum
 from typing import Any
 
@@ -29,7 +29,7 @@
 )
 from glpi_python_client.models.api_schema.management._document import GetDocument
 
-_MAX_DATETIME = datetime.max
+_MAX_DATETIME = datetime.max.replace(tzinfo=timezone.utc)
 
 
 def _ref_label(ref: IdNameRef | None) -> str | None:
@@ -164,9 +164,24 @@ def _event_sort_key(event: Any) -> datetime:
     are therefore always ordered by ``date_creation`` and items missing a
     creation timestamp are pushed to the end while preserving the sort's
     stability.
+
+    Every key is returned as an aware datetime. Awareness is not uniform
+    across a timeline: GLPI omits the offset on some resources and sends
+    it on others, so one response can carry both spellings, and the
+    end-of-time sentinel has to compare against whichever arrives.
+    Comparing the two kinds raises ``TypeError``, which would surface as a
+    crash in :meth:`GlpiTicketContext.to_markdown` rather than as a
+    mis-ordering. A naive value is the server's own wall clock, so it is
+    read as UTC here; that assumption orders events and never reaches the
+    rendered output, which prints the original value.
     """
 
-    return getattr(event, "date_creation", None) or _MAX_DATETIME
+    created: datetime | None = getattr(event, "date_creation", None)
+    if created is None:
+        return _MAX_DATETIME
+    if created.tzinfo is None:
+        return created.replace(tzinfo=timezone.utc)
+    return created
 
 
 class GlpiTicketContext(GlpiModel):
diff --git a/glpi_python_client/models/custom_schema/tests/test_ticket_context.py b/glpi_python_client/models/custom_schema/tests/test_ticket_context.py
index b64c658..a9ad98a 100644
--- a/glpi_python_client/models/custom_schema/tests/test_ticket_context.py
+++ b/glpi_python_client/models/custom_schema/tests/test_ticket_context.py
@@ -396,3 +396,61 @@ def test_options_default_reproduces_original_output() -> None:
 
     context = GlpiTicketContext.model_validate(_FULL_PAYLOAD)
     assert context.to_markdown() == context.to_markdown(TicketMarkdownOptions())
+
+
+def test_to_markdown_sorts_aware_events_when_one_lacks_a_creation_date() -> None:
+    """An event with no ``date_creation`` sorts last among aware timestamps.
+
+    The undated event falls back to the sentinel used by ``_event_sort_key``.
+    When the server sends offset-bearing timestamps -- the format GLPI emits
+    for the knowledge base -- a naive sentinel makes that comparison raise
+    ``TypeError`` instead of ordering.
+    """
+
+    context = GlpiTicketContext.model_validate(
+        {
+            "ticket": {"id": 1, "name": "x"},
+            "followups": [
+                {"id": 2, "content": "undated note"},
+                {
+                    "id": 1,
+                    "content": "dated note",
+                    "date_creation": datetime(2024, 1, 1, tzinfo=timezone.utc),
+                },
+            ],
+        }
+    )
+
+    rendered = context.to_markdown()
+
+    assert rendered.index("dated note") < rendered.index("undated note")
+
+
+def test_to_markdown_orders_events_across_mixed_datetime_awareness() -> None:
+    """Naive and aware timestamps in one timeline still order chronologically.
+
+    Nothing guarantees one wire format per response: a naive value is read as
+    the server's wall clock, so it is treated as UTC for ordering purposes.
+    """
+
+    context = GlpiTicketContext.model_validate(
+        {
+            "ticket": {"id": 1, "name": "x"},
+            "followups": [
+                {
+                    "id": 2,
+                    "content": "aware second",
+                    "date_creation": datetime(2024, 1, 2, tzinfo=timezone.utc),
+                },
+                {
+                    "id": 1,
+                    "content": "naive first",
+                    "date_creation": datetime(2024, 1, 1),
+                },
+            ],
+        }
+    )
+
+    rendered = context.to_markdown()
+
+    assert rendered.index("naive first") < rendered.index("aware second")
diff --git a/glpi_python_client/models/tests/__init__.py b/glpi_python_client/models/tests/__init__.py
new file mode 100644
index 0000000..87301ea
--- /dev/null
+++ b/glpi_python_client/models/tests/__init__.py
@@ -0,0 +1 @@
+"""Unit tests for the shared model base."""
diff --git a/glpi_python_client/models/tests/test_base.py b/glpi_python_client/models/tests/test_base.py
new file mode 100644
index 0000000..bdb6188
--- /dev/null
+++ b/glpi_python_client/models/tests/test_base.py
@@ -0,0 +1,183 @@
+"""Unit tests for :mod:`glpi_python_client.models._base`.
+
+The timezone tests here pin the inbound half of the server-timezone
+contract: GLPI sends most timestamps with an offset but not all of them,
+and a payload carrying both kinds is what makes a plain comparison raise.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime, timedelta, timezone
+
+from glpi_python_client.models._base import GlpiModel
+
+_PARIS_SUMMER = timezone(timedelta(hours=2))
+_PARIS_WINTER = timezone(timedelta(hours=1))
+
+
+class _Stamped(GlpiModel):
+    """Model with one optional datetime field, used by the tests below."""
+
+    id: int | None = None
+    date: datetime | None = None
+
+
+class _Nested(GlpiModel):
+    """Model holding a submodel, to prove the context reaches nested values."""
+
+    id: int | None = None
+    inner: _Stamped | None = None
+
+
+def test_naive_datetime_gains_the_server_timezone() -> None:
+    """A timestamp sent without an offset is stamped with the server's."""
+
+    parsed = _Stamped.model_validate(
+        {"id": 1, "date": "2018-04-06 17:39:44"},
+        context={"server_timezone": _PARIS_SUMMER},
+    )
+
+    assert parsed.date is not None
+    assert parsed.date.utcoffset() == timedelta(hours=2)
+    assert parsed.date.isoformat() == "2018-04-06T17:39:44+02:00"
+
+
+def test_aware_datetime_keeps_the_offset_the_server_sent() -> None:
+    """An offset already on the wire wins over the configured timezone.
+
+    GLPI sends the correct historical offset -- the same instance emits
+    ``+02:00`` in summer and ``+01:00`` in winter -- so overwriting it with
+    a single configured zone would corrupt half the year.
+    """
+
+    parsed = _Stamped.model_validate(
+        {"id": 1, "date": "2019-01-15T09:00:00+01:00"},
+        context={"server_timezone": _PARIS_SUMMER},
+    )
+
+    assert parsed.date is not None
+    assert parsed.date.utcoffset() == timedelta(hours=1)
+
+
+def test_naive_datetime_stays_naive_without_a_context() -> None:
+    """No configured timezone means no guess.
+
+    Stamping an arbitrary offset on an unknown value would turn a loud
+    ``TypeError`` on comparison into a silently wrong result, so a model
+    validated outside the client keeps what it was given.
+    """
+
+    parsed = _Stamped.model_validate({"id": 1, "date": "2018-04-06 17:39:44"})
+
+    assert parsed.date is not None
+    assert parsed.date.tzinfo is None
+
+
+def test_the_timezone_reaches_a_nested_model() -> None:
+    """Submodels are stamped too, which is where the naive values live.
+
+    The one field GLPI 11 sends naive is ``KBArticle.revisions[].date`` --
+    nested inside an article whose own timestamps are aware.
+    """
+
+    parsed = _Nested.model_validate(
+        {"id": 1, "inner": {"id": 2, "date": "2018-04-06 17:39:44"}},
+        context={"server_timezone": _PARIS_SUMMER},
+    )
+
+    assert parsed.inner is not None
+    assert parsed.inner.date is not None
+    assert parsed.inner.date.utcoffset() == timedelta(hours=2)
+
+
+def test_none_datetime_is_left_alone() -> None:
+    """An absent timestamp stays absent rather than becoming an epoch."""
+
+    parsed = _Stamped.model_validate(
+        {"id": 1}, context={"server_timezone": _PARIS_SUMMER}
+    )
+
+    assert parsed.date is None
+
+
+def test_non_datetime_fields_are_untouched() -> None:
+    """Only datetime fields are considered; the rest pass through."""
+
+    parsed = _Stamped.model_validate(
+        {"id": 7, "date": None}, context={"server_timezone": _PARIS_SUMMER}
+    )
+
+    assert parsed.id == 7
+
+
+def test_validating_an_existing_instance_does_not_mutate_the_original() -> None:
+    """Re-validation returns a stamped copy and leaves the caller's object.
+
+    A ``mode="after"`` validator receives the model itself, so assigning to
+    it in place would reach back into an object the caller still holds.
+    """
+
+    original = _Stamped(id=1, date=datetime(2018, 4, 6, 17, 39, 44))
+
+    revalidated = _Stamped.model_validate(
+        original, context={"server_timezone": _PARIS_SUMMER}
+    )
+
+    assert revalidated.date is not None
+    assert revalidated.date.utcoffset() == timedelta(hours=2)
+    assert original.date is not None
+    assert original.date.tzinfo is None
+
+
+def test_aware_datetime_is_converted_to_the_server_clock_and_stripped() -> None:
+    """An offset is not information GLPI keeps, so the value must carry none.
+
+    Measured against GLPI 11: the server reads the naive prefix of a
+    timestamp, interprets it in its own timezone, and discards whatever
+    offset followed. ``12:30:00Z`` written to a Europe/Paris instance is
+    stored as 12:30 Paris -- two hours before the moment that was sent, with
+    a 200 and no complaint. The only spelling that survives is one whose
+    naive prefix is already server-local, so the offset has to be spent on
+    the conversion rather than written out.
+    """
+
+    stamped = _Stamped(id=1, date=datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc))
+
+    dumped = stamped.model_dump(mode="json", context={"server_timezone": _PARIS_WINTER})
+
+    assert dumped["date"] == "2024-01-01T13:00:00"
+
+
+def test_naive_datetime_serialises_unchanged() -> None:
+    """A naive value already means "the server's clock" and is left alone."""
+
+    stamped = _Stamped(id=1, date=datetime(2024, 1, 1, 12, 0))
+
+    dumped = stamped.model_dump(mode="json", context={"server_timezone": _PARIS_WINTER})
+
+    assert dumped["date"] == "2024-01-01T12:00:00"
+
+
+def test_serialisation_without_a_context_leaves_the_offset_alone() -> None:
+    """No timezone means no conversion, matching the inbound half.
+
+    A model dumped outside the client has no server to be local to. Guessing
+    one would be the same silent shift the conversion exists to prevent.
+    """
+
+    stamped = _Stamped(id=1, date=datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc))
+
+    assert stamped.model_dump(mode="json")["date"] == "2024-01-01T12:00:00Z"
+
+
+def test_the_serialisation_timezone_reaches_a_nested_model() -> None:
+    """Submodels are converted too, since request bodies nest."""
+
+    nested = _Nested(
+        id=1,
+        inner=_Stamped(id=2, date=datetime(2024, 1, 1, 12, 0, tzinfo=timezone.utc)),
+    )
+
+    dumped = nested.model_dump(mode="json", context={"server_timezone": _PARIS_WINTER})
+
+    assert dumped["inner"]["date"] == "2024-01-01T13:00:00"
diff --git a/glpi_python_client/rsql.py b/glpi_python_client/rsql.py
new file mode 100644
index 0000000..0c1bc03
--- /dev/null
+++ b/glpi_python_client/rsql.py
@@ -0,0 +1,188 @@
+"""Public RSQL builders for GLPI v2 date filters.
+
+GLPI v2 accepts RSQL expressions on the ``filter`` query parameter. Its
+date grammar has two details that are easy to get subtly wrong and
+impossible to notice when you do:
+
+* the upper bound of a day-granular window has to be spelled through
+  ``23:59:59``, because ``date_creation=le=2026-01-31`` compares against
+  midnight and silently excludes everything that happened on the 31st;
+* a filter field the server does not recognise is **ignored**, and the
+  query then returns the whole unfiltered table rather than an error.
+  A window built with a typo does not fail -- it succeeds, loudly and
+  wrongly, with far too many rows.
+
+That second point is why these live on the public surface. Concatenating
+the grammar at each call site means each call site is a fresh chance to
+produce a filter that quietly matches everything.
+
+This module is deliberately tree-neutral: it holds pure string logic with
+no I/O, so unlike the internal composition helpers it is not duplicated
+into the generated synchronous tree and can be imported from one place on
+either surface.
+
+Examples
+--------
+Counting one month of tickets::
+
+    from glpi_python_client import created_between
+
+    window = created_between("2026-01-01", "2026-01-31")
+    tickets = await client.search_tickets(window)
+
+Incremental sync since the last run::
+
+    from glpi_python_client import changed_since
+
+    async for batch in client.iter_search_tickets(changed_since(last_run)):
+        ...
+"""
+
+from __future__ import annotations
+
+from datetime import date, datetime, timezone
+
+from glpi_python_client._errors import GlpiValidationError
+
+#: Spelled-out end of day, appended to the upper bound of a date window.
+#:
+#: Without it GLPI compares against midnight and the final day of the
+#: window is excluded -- a window that looks inclusive and is not.
+_END_OF_DAY = "23:59:59"
+
+
+def _coerce_date(value: date | str, *, parameter: str) -> date:
+    """Return ``value`` as a :class:`datetime.date`.
+
+    ``datetime`` is accepted and truncated, because a day-granular window
+    built from ``datetime.now()`` is a normal thing to write.
+
+    Raises
+    ------
+    GlpiValidationError
+        When a string is not an ISO ``YYYY-MM-DD`` date. Failing here is
+        the point: an unparsable value concatenated into a filter yields
+        an expression GLPI ignores, and the query returns everything.
+    """
+
+    if isinstance(value, datetime):
+        return value.date()
+    if isinstance(value, date):
+        return value
+    try:
+        return date.fromisoformat(value)
+    except (TypeError, ValueError) as exc:
+        raise GlpiValidationError(
+            f"{parameter} must be a date or an ISO YYYY-MM-DD string; got {value!r}"
+        ) from exc
+
+
+def _render_moment(value: date | datetime | str, *, parameter: str) -> str:
+    """Render one date or datetime the way GLPI's date columns expect.
+
+    An aware ``datetime`` is converted to UTC and rendered without its
+    offset. GLPI stores naive server-local timestamps and does not accept
+    an offset suffix, so passing one through would produce a filter the
+    server ignores -- and an ignored filter returns the whole table.
+    """
+
+    if isinstance(value, datetime):
+        moment = value.astimezone(timezone.utc) if value.tzinfo else value
+        return moment.strftime("%Y-%m-%d %H:%M:%S")
+    return _coerce_date(value, parameter=parameter).isoformat()
+
+
+def date_window(
+    field: str,
+    start: date | str,
+    end: date | str,
+) -> str:
+    """Build an inclusive day-granular RSQL window on one date field.
+
+    Parameters
+    ----------
+    field : str
+        Name of the GLPI date column, e.g. ``"date_creation"``.
+    start : date | str
+        First day included, as a ``date`` or an ISO ``YYYY-MM-DD`` string.
+    end : date | str
+        Last day included. Rendered through ``23:59:59`` so the day itself
+        is inside the window.
+
+    Returns
+    -------
+    str
+        An RSQL expression joining the two bounds with ``;`` (AND), ready
+        to pass as ``rsql_filter``.
+
+    Raises
+    ------
+    GlpiValidationError
+        If either bound is unparsable, or ``start`` falls after ``end``.
+
+    Examples
+    --------
+    >>> date_window("date_creation", "2026-01-01", "2026-01-31")
+    'date_creation=ge=2026-01-01;date_creation=le=2026-01-31 23:59:59'
+    """
+
+    first = _coerce_date(start, parameter="start")
+    last = _coerce_date(end, parameter="end")
+    if first > last:
+        raise GlpiValidationError(
+            f"start ({first.isoformat()}) must not be after end ({last.isoformat()}); "
+            "GLPI answers a contradictory window with zero rows rather than an error."
+        )
+    return f"{field}=ge={first.isoformat()};{field}=le={last.isoformat()} {_END_OF_DAY}"
+
+
+def created_between(start: date | str, end: date | str) -> str:
+    """Build an inclusive window on ``date_creation``.
+
+    The common case, and the one every reporting helper in this package
+    uses. See :func:`date_window` for the parameters.
+
+    Examples
+    --------
+    >>> created_between("2026-01-01", "2026-01-31")
+    'date_creation=ge=2026-01-01;date_creation=le=2026-01-31 23:59:59'
+    """
+
+    return date_window("date_creation", start, end)
+
+
+def changed_since(moment: date | datetime | str, *, field: str = "date_mod") -> str:
+    """Build an open-ended lower bound on a date field.
+
+    Intended for incremental sync: fetch what changed since the last run,
+    with no upper bound.
+
+    Parameters
+    ----------
+    moment : date | datetime | str
+        Lower bound. A ``datetime`` keeps its time component so a re-sync
+        does not re-read a whole day; an aware one is converted to UTC and
+        rendered without its offset, which is the only form GLPI accepts.
+    field : str, optional
+        Date column to compare (defaults to ``"date_mod"``).
+
+    Returns
+    -------
+    str
+        An RSQL expression ready to pass as ``rsql_filter``.
+
+    Raises
+    ------
+    GlpiValidationError
+        If ``moment`` is a string that is not an ISO date.
+
+    Examples
+    --------
+    >>> changed_since("2026-01-01")
+    'date_mod=ge=2026-01-01'
+    """
+
+    return f"{field}=ge={_render_moment(moment, parameter='moment')}"
+
+
+__all__ = ["changed_since", "created_between", "date_window"]
diff --git a/glpi_python_client/testing/tests/test_content_roundtrip.py b/glpi_python_client/testing/tests/test_content_roundtrip.py
index b05e65d..817aeb0 100644
--- a/glpi_python_client/testing/tests/test_content_roundtrip.py
+++ b/glpi_python_client/testing/tests/test_content_roundtrip.py
@@ -100,3 +100,90 @@ def test_outgoing_empty_string_renders_empty() -> None:
     payload = model_to_payload(PostTicket(content=""))
 
     assert payload["content"] == ""
+
+
+# ---------------------------------------------------------------------------
+# Round-trip corpus
+# ---------------------------------------------------------------------------
+#
+# ``from_transport(to_transport(m)) == m`` is the property the content layer
+# would like to hold. It does not hold universally, and cannot: the two
+# libraries either side of the wire disagree about a handful of constructs,
+# and no option on either fixes them.
+#
+# So the corpus is an inventory rather than a property test. Every case is
+# listed, the lossy ones carry ``xfail(strict=True)``, and that strictness is
+# the point -- fixing one of them turns its xfail into an XPASS and fails the
+# suite, forcing the inventory to be updated rather than quietly drifting out
+# of date. A regression in a passing case fails immediately.
+
+
+def _lossy(reason: str) -> pytest.MarkDecorator:
+    """Mark one corpus entry as a known, recorded round-trip loss."""
+
+    return pytest.mark.xfail(strict=True, reason=reason)
+
+
+ROUND_TRIP_CORPUS = [
+    pytest.param("The printer is offline.", id="plain"),
+    pytest.param("The printer is **offline**.", id="bold"),
+    pytest.param("This is *emphasis*.", id="italic"),
+    pytest.param("Run `systemctl restart` now.", id="inline-code"),
+    pytest.param("# Title\n\nBody text.", id="heading"),
+    pytest.param("## Section\n\nBody text.", id="subheading"),
+    pytest.param("First para.\n\nSecond para.", id="paragraphs"),
+    pytest.param("line one  \nline two", id="hard-break"),
+    pytest.param("- alpha\n- beta\n- gamma", id="bullets"),
+    pytest.param("1. one\n2. two", id="numbered"),
+    pytest.param("> quoted text", id="blockquote"),
+    pytest.param("See [the doc](https://example.test/doc).", id="link"),
+    pytest.param("```\nx = 1\n```", id="fence"),
+    pytest.param("| a | b |\n| --- | --- |\n| 1 | 2 |", id="table"),
+    pytest.param("The snake_case name.", id="underscore"),
+    pytest.param("5 * 3 = 15", id="asterisk"),
+    pytest.param("# Title\n\n- alpha\n- beta\n\nClosing **note**.", id="mixed"),
+    pytest.param(
+        "line one\nline two",
+        id="soft-newline",
+        marks=_lossy(
+            "nl2br renders a lone newline as 
, which markdownify reads "
+            "back as a hard break (two trailing spaces). Semantically "
+            "equivalent and stable after one cycle; see issue #32."
+        ),
+    ),
+    pytest.param(
+        "- alpha\n    - inner\n- beta",
+        id="nested-list",
+        marks=_lossy(
+            "markdownify indents nested items by 2 spaces; python-markdown "
+            "needs 4 to keep the nesting, so a second cycle flattens it."
+        ),
+    ),
+    pytest.param(
+        "```python\nx = 1\n```",
+        id="fence-with-language",
+        marks=_lossy(
+            "fenced_code emits class='language-python' and markdownify drops "
+            "the class, so the language tag cannot survive."
+        ),
+    ),
+    pytest.param(
+        "use the  key",
+        id="angle-bracket-text",
+        marks=_lossy(
+            "to_transport does not escape raw markup, so the text reaches "
+            "GLPI as a live unknown tag -- which the web UI drops too. "
+            "Escaping it is a separate change to the outbound direction."
+        ),
+    ),
+]
+
+
+@pytest.mark.parametrize("markdown", ROUND_TRIP_CORPUS)
+def test_round_trip_corpus(markdown: str) -> None:
+    """Markdown survives a full write-then-read cycle through GLPI's HTML."""
+
+    outgoing = model_to_payload(PostTicket(name="Round trip", content=markdown))
+    incoming = GetTicket.model_validate({"name": "Round trip", **outgoing})
+
+    assert incoming.content == markdown
diff --git a/glpi_python_client/testing/tests/test_documentation_snippets.py b/glpi_python_client/testing/tests/test_documentation_snippets.py
new file mode 100644
index 0000000..1f2ee70
--- /dev/null
+++ b/glpi_python_client/testing/tests/test_documentation_snippets.py
@@ -0,0 +1,99 @@
+"""Parse every Python snippet in the skills, the guide and the README.
+
+The snippets are the package's primary teaching surface -- nine ``SKILL.md``
+files an agent reads before writing any GLPI code -- and nothing compiles
+them. A sweep that adds an argument to every client construction example can
+therefore leave a duplicated keyword or a broken indent in a block that still
+*looks* right in a diff, and the first thing to notice is whoever copies it.
+
+This guard only compiles: a snippet is checked for syntax, not executed and
+not type-checked. That is deliberate -- most blocks are fragments referring
+to names they never define -- but it is enough to catch the class of damage a
+mechanical edit does.
+
+It uses :func:`compile` rather than :func:`ast.parse` because the two differ
+on exactly the case that prompted it. ``ast.parse`` accepts a call with the
+same keyword twice; the duplicate is rejected later, when the tree is
+compiled. A sweep that inserts an argument into every construction example
+can produce precisely that, so parsing alone would have passed the file it
+was written to catch.
+
+``PyCF_ALLOW_TOP_LEVEL_AWAIT`` is set because most async examples are
+fragments -- ``ticket = await client.get_ticket(1)`` with no surrounding
+``async def``, which is how they are meant to be read. Without the flag the
+guard would reject 28 perfectly good snippets and teach whoever hit it to
+wrap examples in scaffolding no reader needs.
+
+Blocks that are illustrative rather than runnable opt out with a
+``# doc: no-parse`` comment on the fence line.
+"""
+
+from __future__ import annotations
+
+import ast
+import pathlib
+import re
+import textwrap
+
+import pytest
+
+_REPO_ROOT = pathlib.Path(__file__).resolve().parents[3]
+
+#: A fenced block introduced as Python, capturing any fence-line flags.
+_MARKDOWN_BLOCK = re.compile(
+    r"^```py(?:thon)?[^\S\n]*(?P[^\n]*)\n(?P.*?)^```",
+    re.MULTILINE | re.DOTALL,
+)
+
+#: A Sphinx ``code-block:: python`` and everything indented beneath it.
+_RST_BLOCK = re.compile(
+    r"^\.\.[^\S\n]+code-block::[^\S\n]+python\n"
+    r"(?:^[^\S\n]+:[a-z]+:[^\n]*\n)*"
+    r"\n"
+    r"(?P(?:^(?:[^\S\n]+[^\n]*)?\n)+)",
+    re.MULTILINE,
+)
+
+
+def _documents() -> list[pathlib.Path]:
+    """Return every file whose Python snippets are meant to be copied."""
+
+    return [
+        *sorted((_REPO_ROOT / "skills").rglob("SKILL.md")),
+        *sorted((_REPO_ROOT / "docs").rglob("*.rst")),
+        _REPO_ROOT / "README.md",
+    ]
+
+
+def _snippets() -> list[tuple[str, str]]:
+    """Return every ``(location, source)`` pair worth parsing."""
+
+    found: list[tuple[str, str]] = []
+    for path in _documents():
+        text = path.read_text(encoding="utf-8")
+        where = path.relative_to(_REPO_ROOT).as_posix()
+        pattern = _RST_BLOCK if path.suffix == ".rst" else _MARKDOWN_BLOCK
+        for match in pattern.finditer(text):
+            if "no-parse" in (match.groupdict().get("flags") or ""):
+                continue
+            line = text[: match.start()].count("\n") + 1
+            found.append((f"{where}:{line}", textwrap.dedent(match["code"])))
+    return found
+
+
+def test_the_documents_actually_contain_snippets() -> None:
+    """Guard the guard: a broken regex must not silently check nothing."""
+
+    assert len(_snippets()) > 40
+
+
+@pytest.mark.parametrize(
+    ("location", "source"), _snippets(), ids=[where for where, _ in _snippets()]
+)
+def test_documentation_snippet_compiles(location: str, source: str) -> None:
+    """Every documented snippet is syntactically valid Python."""
+
+    try:
+        compile(source, location, "exec", flags=ast.PyCF_ALLOW_TOP_LEVEL_AWAIT)
+    except SyntaxError as error:  # pragma: no cover - the message is the point
+        pytest.fail(f"{location} does not compile: {error}")
diff --git a/glpi_python_client/testing/tests/test_method_invocation.py b/glpi_python_client/testing/tests/test_method_invocation.py
index 23cb896..5aa2f07 100644
--- a/glpi_python_client/testing/tests/test_method_invocation.py
+++ b/glpi_python_client/testing/tests/test_method_invocation.py
@@ -23,8 +23,10 @@
 
 import asyncio
 import inspect
+from collections.abc import AsyncIterator, Iterator
+from contextlib import asynccontextmanager, contextmanager
 from datetime import datetime, timedelta, timezone
-from typing import Any, get_type_hints
+from typing import Any, ClassVar, get_type_hints
 
 import pytest
 
@@ -167,6 +169,46 @@ async def _arequest(method: str, url: str, **kwargs: Any) -> _StubResponse:
         return _stub_response_for(method, url)
 
     client._session.request = _arequest if is_async else _request  # type: ignore[method-assign,union-attr,assignment]
+
+    # Streaming does NOT route through ``session.request`` on either
+    # surface, so stubbing that seam alone leaves ``session.stream`` on the
+    # real transport -- and the "made no HTTP call" assertion below then
+    # fails while the test tries to open a socket to the fake host. The two
+    # context managers are written out by hand because this file is not
+    # unasync-generated.
+    class _StubStream:
+        status_code = 200
+        headers: ClassVar[dict[str, str]] = {}
+        url = "https://glpi.example.test/stub"
+        text = ""
+
+        def read(self) -> bytes:
+            return b""
+
+        async def aread(self) -> bytes:
+            return b""
+
+        def iter_bytes(self, chunk_size: int | None = None) -> Iterator[bytes]:
+            yield b"stub"
+
+        async def aiter_bytes(
+            self, chunk_size: int | None = None
+        ) -> AsyncIterator[bytes]:
+            yield b"stub"
+
+    @contextmanager
+    def _stream(method: str, url: str, **kwargs: Any) -> Iterator[_StubStream]:
+        calls.append(f"{method} {url}")
+        yield _StubStream()
+
+    @asynccontextmanager
+    async def _astream(
+        method: str, url: str, **kwargs: Any
+    ) -> AsyncIterator[_StubStream]:
+        calls.append(f"{method} {url}")
+        yield _StubStream()
+
+    client._session.stream = _astream if is_async else _stream  # type: ignore[method-assign,union-attr,assignment]
     # Pretend a valid, non-expiring token is already held so no OAuth round
     # trip happens and the call log contains only endpoint traffic.
     client._auth.access_token = "stub-token"
diff --git a/glpi_python_client/testing/tests/test_unasync_codegen.py b/glpi_python_client/testing/tests/test_unasync_codegen.py
index 1e6ccb2..9a5f8c7 100644
--- a/glpi_python_client/testing/tests/test_unasync_codegen.py
+++ b/glpi_python_client/testing/tests/test_unasync_codegen.py
@@ -52,6 +52,8 @@
     "AsyncHTTPTransport",
     "aclose",
     "aread",
+    # httpx's streaming body reader, called by the document stream helper.
+    "aiter_bytes",
 }
 
 
diff --git a/glpi_python_client/testing/tests/test_version_agreement.py b/glpi_python_client/testing/tests/test_version_agreement.py
new file mode 100644
index 0000000..4e2e55b
--- /dev/null
+++ b/glpi_python_client/testing/tests/test_version_agreement.py
@@ -0,0 +1,89 @@
+"""The version is written down in eleven places; they must agree.
+
+``pyproject.toml`` is the source of truth -- ``release.yml`` validates the
+git tag against it, and that is the only version check anything performs.
+Nothing checks the other ten, so they drift silently, and they had: at the
+time this was written ``__version__`` was one release behind and all nine
+skills were two.
+
+That matters most for the skills. ``metadata.version`` tells a consumer
+which release the skill describes, and nine of them claimed 0.4.1 while
+documenting a branch that had since made ``server_timezone`` required and
+turned a swallowed 4xx into a raise. An agent trusting the stamp would have
+written client constructions that raise ``TypeError`` on the first call.
+"""
+
+from __future__ import annotations
+
+import pathlib
+import re
+import sys
+
+if sys.version_info >= (3, 11):
+    import tomllib
+else:  # pragma: no cover - exercised on 3.10 only
+    import tomli as tomllib
+
+import glpi_python_client
+
+_REPO_ROOT = pathlib.Path(__file__).resolve().parents[3]
+
+#: ``version:`` under the ``metadata:`` key of a SKILL.md front-matter block.
+_SKILL_VERSION = re.compile(r'^\s+version:\s*"(?P[^"]+)"', re.MULTILINE)
+
+
+def _declared_version() -> str:
+    """Return the version in ``pyproject.toml``, the source of truth."""
+
+    with (_REPO_ROOT / "pyproject.toml").open("rb") as handle:
+        config = tomllib.load(handle)
+    version = config["project"]["version"]
+    assert isinstance(version, str)
+    return version
+
+
+def _skills() -> list[pathlib.Path]:
+    """Return every skill definition that stamps a package version."""
+
+    return sorted((_REPO_ROOT / "skills").rglob("SKILL.md"))
+
+
+def test_package_dunder_version_matches_pyproject() -> None:
+    """``glpi_python_client.__version__`` is what the build publishes."""
+
+    assert glpi_python_client.__version__ == _declared_version()
+
+
+def test_every_skill_stamps_the_released_version() -> None:
+    """No skill claims to describe a release other than this one."""
+
+    expected = _declared_version()
+    stamped = {
+        path.relative_to(_REPO_ROOT).as_posix(): match["version"]
+        for path in _skills()
+        if (match := _SKILL_VERSION.search(path.read_text(encoding="utf-8")))
+    }
+
+    assert len(stamped) == len(_skills()), f"a skill has no metadata.version: {stamped}"
+    assert set(stamped.values()) == {expected}, (
+        f"skills disagree with pyproject ({expected}): "
+        f"{ {k: v for k, v in stamped.items() if v != expected} }"
+    )
+
+
+def test_the_changelog_records_the_released_version() -> None:
+    """A release has a section; the notes are not left under 'Unreleased'.
+
+    Every release through 0.4.2 was tagged with its notes still under an
+    ``## Unreleased`` heading, so three releases' entries accumulated into
+    one undifferentiated block.
+    """
+
+    changelog = (_REPO_ROOT / "CHANGELOG.md").read_text(encoding="utf-8")
+    expected = _declared_version()
+
+    headings = re.findall(r"^## +(.+)$", changelog, re.MULTILINE)
+
+    assert any(head.startswith(expected) for head in headings), (
+        f"CHANGELOG.md has no section for {expected}; found {headings}"
+    )
diff --git a/glpi_python_client/testing/tests/test_write_timezone_audit.py b/glpi_python_client/testing/tests/test_write_timezone_audit.py
new file mode 100644
index 0000000..881c8a6
--- /dev/null
+++ b/glpi_python_client/testing/tests/test_write_timezone_audit.py
@@ -0,0 +1,73 @@
+"""Audit that every library write serialises on the server's clock.
+
+This is a structural guard, not a behavioural one, and it exists because
+the failure it prevents is silent. :func:`model_to_payload` takes
+``server_timezone`` as an optional keyword, so a call site that omits it
+still returns a valid body, still passes every transport stub, and still
+gets a 200 from GLPI. What it does not do is name the right moment: GLPI 11
+reads the naive prefix of a timestamp and discards the offset, so an aware
+datetime written without the conversion is wrong by that offset -- measured
+at up to twelve hours, with nothing in the response to show it.
+
+The rule is that library code calls ``self._body(...)`` on the transport,
+which binds the client's timezone once. Tests and the helper's own module
+are exempt; nothing else calls :func:`model_to_payload` directly.
+"""
+
+from __future__ import annotations
+
+import ast
+import pathlib
+
+_PACKAGE_ROOT = pathlib.Path(__file__).resolve().parents[2]
+
+#: Modules allowed to name ``model_to_payload`` directly.
+#:
+#: ``_payloads`` defines it, and ``_transport`` wraps it in the one helper
+#: that supplies the timezone.
+_EXEMPT = {"_payloads.py", "_transport.py"}
+
+
+def _library_modules() -> list[pathlib.Path]:
+    """Return every non-test, non-testing module in the package."""
+
+    return [
+        path
+        for path in sorted(_PACKAGE_ROOT.rglob("*.py"))
+        if "tests" not in path.parts
+        and "testing" not in path.parts
+        and not path.name.startswith("test_")
+        and path.name not in _EXEMPT
+    ]
+
+
+def test_no_library_module_serialises_a_body_without_the_server_timezone() -> None:
+    """Only the transport helper may call ``model_to_payload``."""
+
+    offenders = [
+        f"{path.relative_to(_PACKAGE_ROOT)}:{node.lineno}"
+        for path in _library_modules()
+        for node in ast.walk(ast.parse(path.read_text(encoding="utf-8")))
+        if isinstance(node, ast.Call)
+        and isinstance(node.func, ast.Name)
+        and node.func.id == "model_to_payload"
+    ]
+
+    assert not offenders, (
+        "these call model_to_payload directly and so send timestamps GLPI "
+        f"will reinterpret; use self._body(...) instead: {offenders}"
+    )
+
+
+def test_the_transport_helper_passes_the_client_timezone() -> None:
+    """The one exempt call site actually supplies what the others delegate.
+
+    Without this, the audit above could pass while the helper it points
+    everyone at had quietly dropped the argument.
+    """
+
+    source = (
+        _PACKAGE_ROOT / "_async" / "clients" / "commons" / "_transport.py"
+    ).read_text(encoding="utf-8")
+
+    assert "model_to_payload(model, server_timezone=self.server_timezone)" in source
diff --git a/glpi_python_client/testing/utils.py b/glpi_python_client/testing/utils.py
index 6fff29c..d6c87b0 100644
--- a/glpi_python_client/testing/utils.py
+++ b/glpi_python_client/testing/utils.py
@@ -21,6 +21,7 @@
     "client_secret": "client-secret",
     "username": "api-user",
     "password": "api-password",
+    "server_timezone": "Europe/Paris",
 }
 
 
diff --git a/glpi_python_client/tests/test_rsql.py b/glpi_python_client/tests/test_rsql.py
new file mode 100644
index 0000000..f8cbed4
--- /dev/null
+++ b/glpi_python_client/tests/test_rsql.py
@@ -0,0 +1,95 @@
+"""Unit tests for the public RSQL date builders."""
+
+from __future__ import annotations
+
+from datetime import date, datetime, timezone
+
+import pytest
+
+from glpi_python_client import GlpiValidationError
+from glpi_python_client.rsql import changed_since, created_between, date_window
+
+
+def test_created_between_builds_an_inclusive_window() -> None:
+    """The window covers both endpoints, the end through end-of-day."""
+
+    assert created_between(date(2026, 1, 1), date(2026, 1, 31)) == (
+        "date_creation=ge=2026-01-01;date_creation=le=2026-01-31 23:59:59"
+    )
+
+
+def test_created_between_accepts_iso_strings() -> None:
+    """ISO ``YYYY-MM-DD`` strings are accepted alongside date objects."""
+
+    assert created_between("2026-01-01", "2026-01-31") == created_between(
+        date(2026, 1, 1), date(2026, 1, 31)
+    )
+
+
+def test_created_between_accepts_a_single_day() -> None:
+    """A one-day window is a legitimate window, not an empty one."""
+
+    assert created_between(date(2026, 1, 1), date(2026, 1, 1)) == (
+        "date_creation=ge=2026-01-01;date_creation=le=2026-01-01 23:59:59"
+    )
+
+
+def test_created_between_rejects_a_reversed_window() -> None:
+    """A start after the end is a caller error, not an empty result set.
+
+    GLPI answers a contradictory window with zero rows, which reads as "no
+    tickets matched" rather than "your dates are backwards".
+    """
+
+    with pytest.raises(GlpiValidationError):
+        created_between(date(2026, 1, 31), date(2026, 1, 1))
+
+
+def test_created_between_rejects_a_malformed_date() -> None:
+    """A string that is not an ISO date fails here, not on the server."""
+
+    with pytest.raises(GlpiValidationError):
+        created_between("31/01/2026", "2026-01-31")
+
+
+def test_date_window_targets_any_field() -> None:
+    """The field is a parameter so the same grammar serves other columns."""
+
+    assert date_window("date_mod", date(2026, 1, 1), date(2026, 1, 2)) == (
+        "date_mod=ge=2026-01-01;date_mod=le=2026-01-02 23:59:59"
+    )
+
+
+def test_changed_since_builds_an_open_ended_lower_bound() -> None:
+    """Incremental sync needs a lower bound with no end."""
+
+    assert changed_since(date(2026, 1, 1)) == "date_mod=ge=2026-01-01"
+
+
+def test_changed_since_renders_a_datetime_to_the_second() -> None:
+    """A datetime keeps its time component so a re-sync does not re-read a day."""
+
+    moment = datetime(2026, 1, 1, 13, 45, 30)
+
+    assert changed_since(moment) == "date_mod=ge=2026-01-01 13:45:30"
+
+
+def test_changed_since_drops_the_offset_of_an_aware_datetime() -> None:
+    """GLPI stores naive server-local timestamps and rejects an offset.
+
+    An aware value is converted to UTC and rendered without the offset
+    rather than being refused, so callers holding aware datetimes do not
+    have to strip the tzinfo at every call site.
+    """
+
+    aware = datetime(2026, 1, 1, 13, 45, 30, tzinfo=timezone.utc)
+
+    assert changed_since(aware) == "date_mod=ge=2026-01-01 13:45:30"
+
+
+def test_changed_since_targets_any_field() -> None:
+    """``field`` is overridable for columns other than ``date_mod``."""
+
+    assert changed_since(date(2026, 1, 1), field="date_creation") == (
+        "date_creation=ge=2026-01-01"
+    )
diff --git a/integration_tests/probe_wire_format.py b/integration_tests/probe_wire_format.py
new file mode 100644
index 0000000..401a972
--- /dev/null
+++ b/integration_tests/probe_wire_format.py
@@ -0,0 +1,332 @@
+"""Record what GLPI 11 actually puts on the wire, for two open decisions.
+
+Run this against preprod and paste the output into issues #31 and #35. It
+answers two questions the repository cannot answer from inside, and both
+decide whether a proposed change is worth building at all:
+
+**#31 -- do datetimes arrive naive?** The proposal to attach a server
+timezone assumes GLPI omits the UTC offset. Nothing here records a real v2
+response, and the project's own fixtures disagree: the knowledge base tests
+use ``+00:00`` (which already parses aware) while the timeline, management
+and administration tests use a bare ``2024-01-02T03:04:05``. If the live
+server sends an offset, #31 can be closed unbuilt.
+
+**#35 -- what does a POST return?** ``_resource_create`` parses the whole
+body and keeps one integer. If the body already carries the full record,
+the fix is to stop discarding it, not to add a second request behind a
+``create_*_and_fetch`` helper.
+
+**#22 follow-up -- does GLPI honour an offset on write?** The fix for #22
+changed outbound serialisation to Pydantic's JSON mode, so an aware
+``datetime`` now goes out as ``...+02:00`` or ``...Z``. Accepting it and
+honouring it are different questions, and only the second is dangerous to
+get wrong: a truncated offset moves the moment silently, with a 200.
+
+This is a **read-mostly** probe. It creates exactly one ticket -- reused by
+both write probes -- and deletes it again in a ``finally``. Nothing else is
+written.
+
+Usage
+-----
+    python integration_tests/probe_wire_format.py
+
+Credentials load the same way the integration suite loads them: from
+``secrets/`` files, falling back to ``GLPI_*`` environment variables.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import re
+import socket
+import sys
+from pathlib import Path
+from typing import Any
+from urllib.parse import urlparse
+
+import httpx
+
+_REPO_ROOT = Path(__file__).resolve().parents[1]
+_SECRETS_DIR = _REPO_ROOT / "secrets"
+
+#: Wire values that look like a timestamp, offset-bearing or not.
+_TIMESTAMP = re.compile(r"^\d{4}-\d{2}-\d{2}[ T]\d{2}:\d{2}:\d{2}")
+
+
+def _read_value(secret_name: str, env_name: str) -> str | None:
+    """Return the secret file's contents, or the environment fallback."""
+
+    path = _SECRETS_DIR / secret_name
+    if path.exists():
+        value = path.read_text(encoding="utf-8").strip()
+        if value:
+            return value
+    env_value = os.environ.get(env_name)
+    return env_value.strip() if env_value else None
+
+
+def _load() -> dict[str, str]:
+    """Resolve the live configuration or exit with what is missing."""
+
+    wanted = {
+        "api_url": ("glpi_api_url", "GLPI_API_URL"),
+        "client_id": ("glpi_client_id_test", "GLPI_CLIENT_ID"),
+        "client_secret": ("glpi_client_secret_test", "GLPI_CLIENT_SECRET"),
+        "username": ("glpi_username", "GLPI_USERNAME"),
+        "password": ("glpi_password", "GLPI_PASSWORD"),
+    }
+    config: dict[str, str] = {}
+    missing: list[str] = []
+    for key, (secret, env) in wanted.items():
+        value = _read_value(secret, env)
+        if value is None:
+            missing.append(secret)
+        else:
+            config[key] = value
+    if missing:
+        sys.exit("missing credentials: " + ", ".join(missing))
+    return config
+
+
+def _check_reachable(api_url: str) -> None:
+    """Exit with a diagnosis when the API host does not resolve.
+
+    The GLPI instance lives on an internal ``.local`` name, so running this
+    off the corporate VPN fails during DNS with a forty-line httpx traceback
+    ending in ``getaddrinfo failed`` -- which looks like a bug in the probe
+    rather than a missing network. Checking first turns that into one line.
+    """
+
+    host = urlparse(api_url).hostname
+    if not host:
+        sys.exit(f"glpi_api_url is not a URL: {api_url!r}")
+    try:
+        socket.getaddrinfo(host, None)
+    except socket.gaierror:
+        try:
+            socket.getaddrinfo("github.com", None)
+        except socket.gaierror:
+            sys.exit(f"cannot resolve {host} -- and public DNS is down too.")
+        sys.exit(
+            f"cannot resolve {host}.\n"
+            "Public DNS works, so this is name resolution for the internal "
+            "domain: connect to the corporate VPN and run this again.\n"
+            "(The integration suite cannot reach the instance either while "
+            "this fails.)"
+        )
+
+
+def _token(client: httpx.Client, config: dict[str, str]) -> str:
+    """Obtain an access token with the password grant."""
+
+    response = client.post(
+        f"{config['api_url'].rstrip('/')}/token",
+        data={
+            "grant_type": "password",
+            "client_id": config["client_id"],
+            "client_secret": config["client_secret"],
+            "username": config["username"],
+            "password": config["password"],
+            "scope": "api",
+        },
+        timeout=30,
+    )
+    response.raise_for_status()
+    return str(response.json()["access_token"])
+
+
+def _walk_timestamps(payload: Any, prefix: str = "") -> list[tuple[str, str]]:
+    """Return every ``(path, value)`` in ``payload`` that looks like a timestamp."""
+
+    found: list[tuple[str, str]] = []
+    if isinstance(payload, dict):
+        for key, value in payload.items():
+            found.extend(_walk_timestamps(value, f"{prefix}.{key}" if prefix else key))
+    elif isinstance(payload, list):
+        for index, value in enumerate(payload[:2]):
+            found.extend(_walk_timestamps(value, f"{prefix}[{index}]"))
+    elif isinstance(payload, str) and _TIMESTAMP.match(payload):
+        found.append((prefix, payload))
+    return found
+
+
+def _report_timestamps(label: str, payload: Any) -> None:
+    """Print every timestamp in one payload and whether it carries an offset."""
+
+    stamps = _walk_timestamps(payload)
+    if not stamps:
+        print(f"  {label}: no timestamp-shaped values found")
+        return
+    for path, value in stamps:
+        aware = bool(re.search(r"(Z|[+-]\d{2}:?\d{2})$", value))
+        verdict = "AWARE (carries an offset)" if aware else "NAIVE (no offset)"
+        print(f"  {label}.{path} = {value!r}  -> {verdict}")
+
+
+#: The three ways this package can now render one moment onto the wire.
+#:
+#: These are not hypothetical spellings -- they are the literal bytes
+#: ``model_to_payload`` produces for a ``PatchTicket(date=...)`` built with a
+#: UTC ``tzinfo``, a ``Europe/Paris`` one, and none at all. Pydantic's JSON
+#: mode writes UTC as ``Z`` and every other zone as a numeric offset.
+#:
+#: ``12:30Z`` is the discriminating case, because it is ``14:30`` in Paris:
+#: a server that honours the offset stores a different wall clock than one
+#: that ignores it, and one read-back separates them. The ``+02:00`` case
+#: cannot discriminate -- it names the same wall clock either way -- but it
+#: does answer whether an offset-bearing string is accepted at all.
+_WRITE_CASES = (
+    ("naive   (control)", "2026-08-01T12:30:00"),
+    ("UTC     (Z form) ", "2026-08-01T12:30:00Z"),
+    ("Paris   (+02:00) ", "2026-08-01T12:30:00+02:00"),
+    ("Tokyo   (+09:00) ", "2026-08-01T12:30:00+09:00"),
+    ("LA      (-08:00) ", "2026-08-01T12:30:00-08:00"),
+    ("Kiritim (+14:00) ", "2026-08-01T12:30:00+14:00"),
+    ("nonsense(+99:99) ", "2026-08-01T12:30:00+99:99"),
+)
+
+
+def _probe_write_offsets(
+    client: httpx.Client,
+    base: str,
+    read_headers: dict[str, str],
+    write_headers: dict[str, str],
+    ticket_id: int,
+) -> None:
+    """Write each spelling of one moment and report what GLPI stored.
+
+    Issue #22 changed outbound serialisation to Pydantic's JSON mode, so an
+    aware ``datetime`` now leaves as ``...+02:00`` or ``...Z`` where it
+    previously left as a live object that ``json.dumps`` refused. That fixed
+    the crash but moved the question rather than answering it: nothing here
+    had ever recorded whether GLPI *accepts* an offset, nor -- the part that
+    matters more -- whether it *honours* one.
+
+    A rejection is loud and harmless. Silent truncation is neither: writing
+    ``12:30Z`` and having 12:30 stored as Paris local time moves the moment
+    two hours with a 200 and no complaint.
+    """
+
+    for label, wire in _WRITE_CASES:
+        patch = client.request(
+            "PATCH",
+            f"{base}/Assistance/Ticket/{ticket_id}",
+            headers=write_headers,
+            json={"date": wire},
+        )
+        if patch.status_code >= 400:
+            print(f"  {label} {wire!r}")
+            print(f"      PATCH -> HTTP {patch.status_code} REJECTED")
+            print(f"      body  -> {patch.text[:200]!r}")
+            continue
+        read = client.get(f"{base}/Assistance/Ticket/{ticket_id}", headers=read_headers)
+        stored = read.json().get("date") if read.status_code < 400 else None
+        print(f"  {label} {wire!r}")
+        print(f"      PATCH -> HTTP {patch.status_code}   read back -> {stored!r}")
+
+
+def main() -> None:
+    """Run every probe and print a report to paste into the issues."""
+
+    config = _load()
+    base = config["api_url"].rstrip("/")
+    _check_reachable(base)
+
+    with httpx.Client(verify=False, follow_redirects=True, timeout=30) as client:
+        token = _token(client, config)
+        # A GET must NOT advertise a JSON content type. GLPI sees the header,
+        # tries to parse the (absent) body, and answers 400 "Contenu du JSON
+        # invalide" -- an error about the request body on a request that has
+        # none. The library avoids this with its `include_content_type` flag,
+        # which is False for GET; the two header sets here mirror that.
+        read_headers = {"Authorization": f"Bearer {token}"}
+        write_headers = {
+            "Authorization": f"Bearer {token}",
+            "Content-Type": "application/json",
+        }
+
+        print("=" * 72)
+        print("PROBE 1 (issue #31) -- datetime wire format")
+        print("=" * 72)
+        for label, path in (
+            ("Ticket", "/Assistance/Ticket?limit=1"),
+            ("User", "/Administration/User?limit=1"),
+            ("KBArticle", "/Knowledgebase/Article?limit=1"),
+        ):
+            response = client.get(f"{base}{path}", headers=read_headers)
+            if response.status_code >= 400:
+                print(f"  {label}: HTTP {response.status_code} -- skipped")
+                continue
+            _report_timestamps(label, response.json())
+
+        print()
+        print("=" * 72)
+        print("PROBE 2 (issue #35) -- what a create returns")
+        print("=" * 72)
+        created_id: int | None = None
+        try:
+            response = client.post(
+                f"{base}/Assistance/Ticket",
+                headers=write_headers,
+                json={
+                    "name": "py_glpi wire-format probe (safe to delete)",
+                    "content": "
Automated probe. Deleted immediately.
",
+                },
+            )
+            print(f"  status      : {response.status_code}")
+            print(f"  Location    : {response.headers.get('Location', '')}")
+            print(f"  body bytes  : {len(response.content)}")
+            try:
+                body = response.json()
+            except ValueError:
+                print(f"  body        :  {response.text[:200]!r}")
+            else:
+                keys = sorted(body) if isinstance(body, dict) else ""
+                print(f"  body keys   : {keys}")
+                print("  body        :")
+                print(json.dumps(body, indent=4, ensure_ascii=False)[:2000])
+                if isinstance(body, dict):
+                    created_id = body.get("id")
+                    print()
+                    print(
+                        "  VERDICT: "
+                        + (
+                            "id only -- a fetch is genuinely needed (#35)"
+                            if set(body) <= {"id", "href"}
+                            else f"{len(body)} fields returned -- the client is "
+                            "DISCARDING a fuller record; fix that instead of "
+                            "adding create_*_and_fetch (#35)"
+                        )
+                    )
+            if created_id:
+                print()
+                print("=" * 72)
+                print("PROBE 3 (#22 follow-up) -- does GLPI honour an offset on write?")
+                print("=" * 72)
+                _probe_write_offsets(
+                    client, base, read_headers, write_headers, created_id
+                )
+        finally:
+            if created_id:
+                # `client.delete(...)` rejects `json=` -- httpx exposes a body
+                # only on `request()`, because DELETE-with-a-body is unusual.
+                # GLPI needs one for `force`, so this must not be "simplified"
+                # back to the convenience method; the library's own
+                # `_delete_request` goes through `session.request` for the
+                # same reason.
+                cleanup = client.request(
+                    "DELETE",
+                    f"{base}/Assistance/Ticket/{created_id}",
+                    headers=write_headers,
+                    json={"force": True},
+                )
+                print()
+                print(f"  cleanup: deleted {created_id} -> {cleanup.status_code}")
+            else:
+                print()
+                print("  cleanup: nothing to delete (no id parsed from the response)")
+
+
+if __name__ == "__main__":
+    main()
diff --git a/integration_tests/test_integration.py b/integration_tests/test_integration.py
index 8c599b8..4546b39 100644
--- a/integration_tests/test_integration.py
+++ b/integration_tests/test_integration.py
@@ -52,6 +52,7 @@ class _LiveGlpiConfig:
     v1_user_token: str | None
     v1_app_token: str | None
     team_member_role: str
+    server_timezone: str
 
 
 def _read_value(secret_name: str, *env_names: str) -> str | None:
@@ -138,6 +139,8 @@ def _load_config() -> _LiveGlpiConfig:
         v1_base_url=_read_value("glpi_api_v1_url", "GLPI_API_V1_URL"),
         v1_user_token=_read_value("glpi_api_v1_token_user", "GLPI_V1_USER_TOKEN"),
         v1_app_token=_read_value("glpi_api_v1_app_token", "GLPI_V1_APP_TOKEN"),
+        server_timezone=_read_value("glpi_server_timezone", "GLPI_SERVER_TIMEZONE")
+        or "UTC",
         team_member_role=_read_value("glpi_team_member_role", "GLPI_TEAM_MEMBER_ROLE")
         or "assigned",
     )
@@ -162,6 +165,7 @@ def client(
 
     glpi_client = GlpiClient(
         glpi_api_url=live_config.api_url,
+        server_timezone=live_config.server_timezone,
         client_id=live_config.client_id,
         client_secret=live_config.client_secret,
         username=live_config.username,
diff --git a/integration_tests/test_integration_async.py b/integration_tests/test_integration_async.py
index d35570d..02521cf 100644
--- a/integration_tests/test_integration_async.py
+++ b/integration_tests/test_integration_async.py
@@ -58,6 +58,7 @@ def _build_async_client(config: _LiveGlpiConfig) -> AsyncGlpiClient:
 
     return AsyncGlpiClient(
         glpi_api_url=config.api_url,
+        server_timezone=config.server_timezone,
         client_id=config.client_id,
         client_secret=config.client_secret,
         username=config.username,
diff --git a/integration_tests/test_integration_kb.py b/integration_tests/test_integration_kb.py
index 7b0c519..222479c 100644
--- a/integration_tests/test_integration_kb.py
+++ b/integration_tests/test_integration_kb.py
@@ -45,6 +45,7 @@ def client(live_config: _LiveGlpiConfig) -> Iterator[GlpiClient]:
 
     glpi_client = GlpiClient(
         glpi_api_url=live_config.api_url,
+        server_timezone=live_config.server_timezone,
         client_id=live_config.client_id,
         client_secret=live_config.client_secret,
         username=live_config.username,
diff --git a/pyproject.toml b/pyproject.toml
index df59d3d..66835b9 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -35,7 +35,7 @@ exclude = [
 
 [project]
 name = "glpi-python-client"
-version = "0.4.2"
+version = "0.4.3"
 description = "A typed Python client for GLPI ITSM APIs."
 readme = "README.md"
 requires-python = ">=3.10"
@@ -74,6 +74,13 @@ dependencies = [
     # 3354ms without it, 1304ms with. `pip check` reports no broken
     # requirements either way, so this pin is the only thing holding the fix.
     "sniffio>=1.3",
+    # Required because `server_timezone` takes an IANA zone name, and a name
+    # is what makes DST correct -- a fixed offset would be wrong for half the
+    # year against an instance that emits both +01:00 and +02:00. Windows
+    # ships no system tz database, so `zoneinfo` finds nothing there while
+    # Linux reads /usr/share/zoneinfo: without this pin the client works in
+    # CI and raises ZoneInfoNotFoundError on a developer machine.
+    "tzdata>=2024.1; platform_system == 'Windows'",
     "tenacity>=8.2",
     "typing-extensions>=4.7; python_version < '3.11'",
 ]
diff --git a/skills/glpi-client-setup/SKILL.md b/skills/glpi-client-setup/SKILL.md
index 2550fcd..aef3468 100644
--- a/skills/glpi-client-setup/SKILL.md
+++ b/skills/glpi-client-setup/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to a GLPI v2 API, and valid GLPI credentials."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Client Setup
@@ -85,6 +85,7 @@ call `client.close()` (or `await client.close()`) when finished.
 - `GLPI_CLIENT_ID` and `GLPI_CLIENT_SECRET`
 - `GLPI_USERNAME` and `GLPI_PASSWORD`
 - `GLPI_ENTITY`, `GLPI_PROFILE`, `GLPI_ENTITY_RECURSIVE`
+- `GLPI_SERVER_TIMEZONE` -- **required**. IANA name of the GLPI server's timezone (e.g. `Europe/Paris`). GLPI does not advertise it, and it is needed to interpret the timestamps the server sends without an offset -- there is no default, because guessing UTC against a Europe/Paris instance shifts those values silently.
 - `GLPI_LANGUAGE`, `GLPI_VERIFY_SSL`, `GLPI_AUTH_TOKEN_REFRESH`
 - `GLPI_V1_BASE_URL`, `GLPI_V1_USER_TOKEN`, `GLPI_V1_APP_TOKEN`
 
@@ -101,6 +102,7 @@ from glpi_python_client import GlpiClient
 def main() -> None:
     with GlpiClient(
         glpi_api_url="https://glpi.example.com/api.php/v2",
+        server_timezone="Europe/Paris",
         client_id="oauth-client-id",
         client_secret="oauth-client-secret",
         username="api-user",
@@ -125,6 +127,7 @@ from glpi_python_client import AsyncGlpiClient
 async def main() -> None:
     async with AsyncGlpiClient(
         glpi_api_url="https://glpi.example.com/api.php/v2",
+        server_timezone="Europe/Paris",
         client_id="oauth-client-id",
         client_secret="oauth-client-secret",
         username="api-user",
diff --git a/skills/glpi-document-workflow/SKILL.md b/skills/glpi-document-workflow/SKILL.md
index 56f73c4..bb93d36 100644
--- a/skills/glpi-document-workflow/SKILL.md
+++ b/skills/glpi-document-workflow/SKILL.md
@@ -5,25 +5,25 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to the GLPI v2 API, and v1 credentials configured on the client for binary uploads."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Document Workflow
 > The snippets below use `AsyncGlpiClient` (`async with` + `await`). Every method shown also exists on the synchronous `GlpiClient` with the same signature -- replace `async with` with `with`, drop the `await` keyword, and skip the surrounding `async def`/`asyncio.run` scaffolding.
 
-Document metadata uses the standard `Get`/`Post`/`Patch`/`Delete` shape on `/Management/Document`. Binary content uses two dedicated helpers: `download_document_content` for downloads and `upload_document` for uploads through the legacy v1 fallback session (the v2 contract does not advertise a binary upload endpoint).
+Document metadata uses the standard `Get`/`Post`/`Patch`/`Delete` shape on `/Management/Document`. Binary content uses three dedicated helpers: `download_document_content` for whole-file downloads, `stream_document_content` for chunked ones, and `upload_document` for uploads through the legacy v1 fallback session (the v2 contract does not advertise a binary upload endpoint).
 
 The `GLPIV1Session` class is no longer part of the public surface; the v1 session is fully internal and is configured by passing `v1_base_url` and `v1_user_token` to `GlpiClient`.
 
 ## Procedure
 
 1. Create a `GlpiClient`. For uploads, also pass `v1_base_url` and `v1_user_token` (and optionally `v1_app_token`).
-2. Search documents with `await client.search_documents(rsql_filter, limit=..., start=...)`.
+2. Search documents with `await client.search_documents(rsql_filter, limit=..., start=...)`, or walk every page with `async for batch in client.iter_search_documents(rsql_filter, batch_size=50)`.
 3. Fetch one document's metadata with `await client.get_document(document_id)`.
 4. Create a metadata-only record with `PostDocument(...)` and `await client.create_document(document)`. The method returns the new ID.
 5. Update with `PatchDocument(...)` and `await client.update_document(document_id, document)`.
 6. Delete with `await client.delete_document(document_id, force=True|False|None)`.
-7. Download bytes with `content = await client.download_document_content(document_id)`.
+7. Download bytes with `content = await client.download_document_content(document_id)`, which buffers the whole body. For a large attachment use `async for chunk in client.stream_document_content(document_id, chunk_size=65536)` and write each chunk out as it arrives.
 8. Upload bytes with `await client.upload_document(filename=..., content=..., mime_type=..., ticket_id=..., entity_id=...)`.
 9. To put a file on a ticket timeline, use `upload_document(..., ticket_id=...)` -- it creates the document *and* the ticket link in one call. `link_ticket_timeline_document` from the timeline skill cannot be told **which** existing document to link: `PostTimelineDocument` declares only `extra_payload` and `timeline_position`, and the POST URL carries only the ticket id, so there is no typed slot for a document id. See the timeline skill for the `extra_payload` escape hatch and its caveat.
 
@@ -64,10 +64,10 @@ document_id = await client.create_document(PostDocument(name="Diagnostic notes")
 
 ## Gotchas
 
-- **`search_documents` swallows 4xx and returns `[]`.** This is a library-wide contract, not a document peculiarity: `_resource_list` checks the response status only when the caller passes a `failure_message`, and none of the seven `search_*` helpers (`search_documents`, `search_tickets`, `search_users`, `search_locations`, `search_entities`, `search_kb_articles`, `search_kb_categories`) passes one -- a GLPI error body is not a JSON list, so it is coerced to `[]`. A malformed RSQL filter, a 403 on `/Management/Document`, a missing route and "no such document" all look identical. `get_document`, `download_document_content` and every `list_*` helper do pass a `failure_message` and raise `GlpiStatusError` (narrowed to `GlpiAuthError` / `GlpiNotFoundError` / `GlpiServerError`) normally. So never conclude from an empty `search_documents` that a file is not on the server and re-upload it -- that is how duplicate documents get created; corroborate with a call that raises first.
+- **`search_documents` raises `GlpiStatusError` on a 4xx.** This changed: `_resource_list` used to check the response status only when the caller passed a `failure_message`, which none of the seven `search_*` helpers does, so a GLPI error body was coerced to `[]` and a malformed RSQL filter, a 403 on `/Management/Document`, a missing route and "no such document" all looked identical. The status is now checked on every call, so **an empty list means the server said there are no matches**. The *other* fail-open path is unchanged: GLPI v2 ignores a filter field it does not recognise and answers 200 with the whole unfiltered table, so a search that returns rows is still not proof the filter applied. Never conclude from a search that a file is absent and re-upload it without corroborating -- that is how duplicate documents get created.
 - `upload_document` raises `RuntimeError` when the v1 session is not configured. Pass `v1_base_url` and `v1_user_token` to the client constructor or `from_env`.
 - `upload_document` requires a non-empty `filename`. On the async client the multipart POST is awaited like any other call, so the event loop is not blocked.
-- `download_document_content` returns `bytes` and raises on non-200 responses.
+- `download_document_content` returns `bytes` and raises on non-200 responses. It holds the entire file in memory, so a 500 MB attachment costs 500 MB of process memory even when the caller only writes it straight to disk -- prefer `stream_document_content(document_id, chunk_size=65536)`, an async generator on `AsyncGlpiClient` (`async for`) and a plain generator on `GlpiClient` (`for`), which yields the body in chunks and raises the same `GlpiStatusError` on a non-200. Uploads still buffer.
 - `mime_type` defaults to `application/octet-stream` when omitted on `upload_document`.
 - The snippets above use `AsyncGlpiClient`, so every call is awaited. The same methods on the synchronous `GlpiClient` are plain blocking calls -- drop the `await`.
 - The `delete_document(force=True)` flag permanently deletes; omit (or `False`) to move to the trash.
\ No newline at end of file
diff --git a/skills/glpi-knowledge-base/SKILL.md b/skills/glpi-knowledge-base/SKILL.md
index ebe30fa..43bd1dc 100644
--- a/skills/glpi-knowledge-base/SKILL.md
+++ b/skills/glpi-knowledge-base/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to the GLPI v2 API, and — for category writes only — a legacy v1 session (v1_base_url + v1_user_token)."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Knowledge Base
@@ -16,7 +16,7 @@ The GLPI knowledge base lives under `/Knowledgebase/*` on the v2 API and covers
 ## Procedure
 
 1. Create a client from the `glpi-client-setup` skill. Add `v1_base_url` and `v1_user_token` **only** if you will write article categories.
-2. Articles: `search_kb_articles(rsql_filter, limit=..., start=..., sort=..., language=...)` for lists and `get_kb_article(article_id)` for one. Write with `create_kb_article(PostKBArticle(...))` (returns the new id), `update_kb_article(article_id, PatchKBArticle(...))` and `delete_kb_article(article_id, force=...)` (both return `None`).
+2. Articles: `search_kb_articles(rsql_filter, limit=..., start=..., sort=..., language=...)` for lists, `iter_search_kb_articles(rsql_filter, batch_size=50, sort=..., language=...)` to walk every page without managing `start` yourself, and `get_kb_article(article_id)` for one. `sort` is spelled `field` or `field:direction` (`"date_mod:desc"`); a space before the direction is HTTP 400, and a bare field sorts *ascending*. Write with `create_kb_article(PostKBArticle(...))` (returns the new id), `update_kb_article(article_id, PatchKBArticle(...))` and `delete_kb_article(article_id, force=...)` (both return `None`).
 3. Article categories: `set_kb_article_categories(article_id, category_ids)`. The ids **replace** the whole set; an empty sequence clears it. Ids are not validated against the server -- an unknown id is simply not linked.
 4. Categories: `search_kb_categories(...)` (same parameters as the article search), `get_kb_category(category_id)`, `create_kb_category(PostKBCategory(...))`, `update_kb_category(category_id, PatchKBCategory(...))`, `delete_kb_category(category_id, force=...)`. `completename` and `level` are server-managed and absent from the write models.
 5. Comments: `list_kb_article_comments(article_id)`, `get_kb_article_comment(article_id, comment_id)`, `create_kb_article_comment(article_id, PostKBArticleComment(...))` (returns the new id), `update_kb_article_comment(article_id, comment_id, PatchKBArticleComment(...))`, `delete_kb_article_comment(article_id, comment_id, force=...)`. The parent article comes from the URL, so `PostKBArticleComment` has no `kbarticle` field.
@@ -32,6 +32,7 @@ from glpi_python_client import AsyncGlpiClient, IdNameRef, PostKBArticle, PostKB
 
 async with AsyncGlpiClient(
     glpi_api_url="https://glpi.example.com/api.php/v2",
+    server_timezone="Europe/Paris",
     client_id="oauth-client-id",
     client_secret="oauth-client-secret",
     v1_base_url="https://glpi.example.com/api.php/v1",
@@ -89,7 +90,7 @@ Search, and disambiguate an empty result. `language` is a query parameter on bot
 from glpi_python_client import GlpiNotFoundError
 
 faq = await client.search_kb_articles(
-    "is_faq==1", limit=25, start=0, sort="date_mod desc", language="fr_FR"
+    "is_faq==1", limit=25, start=0, sort="date_mod:desc", language="fr_FR"
 )
 categories = await client.search_kb_categories("name==Network", limit=10)
 print([(c.id, c.completename) for c in categories])
@@ -159,7 +160,7 @@ await client.delete_kb_article(42, force=True)
 - `update_kb_article` does not wrap the failure the way create does -- the raw error propagates (`GlpiValidationError` for a category reference with no `id`, `RuntimeError` for a missing v1 session, `GlpiStatusError` for a legacy non-success). The v2 field changes are already applied and are not reverted.
 - `categories=[]` means opposite things on create and update. On create it is skipped entirely: no v1 call, no v1 session needed. On update it *clears* every category, which is a legacy write and does need v1. Only `categories=None` (the default) is a no-op on both.
 - `categories` is still sent inside the v2 POST/PATCH body and GLPI silently ignores it; the body is never stripped. So a create-with-categories against a client with no v1 session yields an uncategorised article *and* an error, not a clean rejection -- and no code path persists a category through v2 alone.
-- **Every `search_*` helper in the library swallows 4xx and returns `[]`. This is a library-wide contract, not a KB peculiarity.** `_resource_list` checks the response status only when the caller passes a `failure_message`, and none of the seven searches -- `search_kb_articles`, `search_kb_categories`, `search_tickets`, `search_users`, `search_locations`, `search_entities`, `search_documents` -- passes one. A GLPI error body is not a JSON list, so it is coerced to `[]`. Every `list_*` and `get_*` helper does pass a `failure_message` and raises normally: here that is `list_kb_article_comments`, `list_kb_article_revisions`, `get_kb_article`, `get_kb_category` and `get_kb_article_revision` (`GlpiNotFoundError` on a 404). So an empty list from a search means "no matches" *or* "bad RSQL filter" *or* "403" *or* "this GLPI serves no `/Knowledgebase` routes at all" (they need High-Level API >= 2.2.0), indistinguishably; an empty list from a list helper is unambiguous. Never treat `[]` from a search as proof a record is absent before creating one.
+- **Every `search_*` helper raises `GlpiStatusError` on a 4xx.** This changed: `_resource_list` used to check the response status only when the caller passed a `failure_message`, which none of the seven searches does, so a GLPI error body was coerced to `[]`. An empty list from a search meant "no matches" *or* "bad RSQL filter" *or* "403" *or* "this GLPI serves no `/Knowledgebase` routes at all" (they need High-Level API >= 2.2.0), indistinguishably. The status is now checked on every call, so **an empty list means the server said there are no matches** -- and a GLPI without the Knowledgebase routes raises rather than looking like an empty knowledge base. The *other* fail-open path is unchanged: v2 ignores a filter field it does not recognise and answers 200 with the whole unfiltered table, so a search that returns rows is still not proof the filter applied.
 - `language` has two different mechanics in this family. On `search_kb_articles`/`search_kb_categories` it is a **query parameter**. On `list_kb_article_revisions`/`get_kb_article_revision` it is a **path segment** between the id and `Revision` (`Knowledgebase/Article/5/fr_FR/Revision`). `get_kb_article`, the comment helpers and every write helper take no `language` at all; they inherit the client-level value, sent as `Accept-Language` (default `en_GB`).
 - KB write models use `IdNameRef` for every foreign key -- `categories[]`, `entity`, `user`, `parent` -- not `IdRef`. Passing `IdRef(id=4)` raises a pydantic `ValidationError`. (`GetKBArticleComment.parent` is the one KB field genuinely typed `IdRef`, and it is read-only.)
 - Article `content`/`description` and revision `content` are Markdown on the Python side and HTML on the wire; the conversion is automatic, so never author HTML. Comment `comment` is a plain `str` with no conversion at all -- the inconsistency is real, not an omission here.
diff --git a/skills/glpi-plugin-fields/SKILL.md b/skills/glpi-plugin-fields/SKILL.md
index 9f424a4..f3bdcdb 100644
--- a/skills/glpi-plugin-fields/SKILL.md
+++ b/skills/glpi-plugin-fields/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, the GLPI Fields plugin installed server-side, and a legacy v1 session (v1_base_url + v1_user_token) — every method in this family goes over the v1 API."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Plugin Fields
@@ -63,6 +63,7 @@ PLUGIN_ABSENT = "ERROR_RESOURCE_NOT_FOUND_NOR_COMMONDBTM"
 async def main() -> None:
     async with AsyncGlpiClient(
         glpi_api_url="https://glpi.example.com/api.php/v2",
+        server_timezone="Europe/Paris",
         client_id="oauth-client-id",
         client_secret="oauth-client-secret",
         v1_base_url="https://glpi.example.com/api.php/v1",
diff --git a/skills/glpi-reporting-and-context/SKILL.md b/skills/glpi-reporting-and-context/SKILL.md
index db93bce..6a1055c 100644
--- a/skills/glpi-reporting-and-context/SKILL.md
+++ b/skills/glpi-reporting-and-context/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to the GLPI v2 API, and credentials allowed to read tickets, tasks, users, entities, and timeline records."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Reporting And Context
@@ -18,7 +18,7 @@ Custom helpers on `GlpiClient` build on top of the contract-aligned API mixins:
 - `get_task_statistics(ticket_ids)` returns task duration totals grouped by user and ticket for a caller-supplied list of ticket IDs.
 - `get_task_durations(...)` is a higher-level helper that internally iterates `iter_search_tickets` with a date/entity RSQL filter, computes per-user and per-entity duration totals, and optionally returns a flat per-task list when `return_task_details=True`. `user_id` is **not** part of the RSQL filter: the v2 `team` array cannot be joined by the RSQL engine, so the ticket ids for that actor are resolved through the legacy v1 search engine (searchOptions 5 `Technicien` and 4 `Demandeur`, OR-ed) and intersected client-side. Passing `user_id` therefore requires a client built with `v1_base_url` + `v1_user_token`, or the call raises `RuntimeError`; a non-positive or non-`int` id raises `GlpiValidationError`. Once the matched ticket set reaches 25 tickets and a v1 session is present, task aggregation switches from the per-ticket v2 fan-out to one bulk sweep of the v1 `TicketTask` collection (paged 1000 rows at a time); the aggregate is identical either way.
 - `get_user_activity(...)` aggregates per-user activity (tickets as technician, tickets as recipient, task durations) over a date window; resolves users by `user_id`, `username`, `realname`, or `firstname` and merges users that share the same display key. The technician and recipient counts have **no v2 equivalent** and are resolved through the legacy v1 search engine (searchOption 5 `Technicien`, 4 `Demandeur`), intersected with the ids returned by a single walk of the date window. This helper therefore **always** requires a client built with `v1_base_url` + `v1_user_token` and raises `RuntimeError` naming the missing options when they are absent.
-- `iter_search_tickets`, `iter_search_users`, `iter_search_entities` yield successive `list[...]` batches of contract models and stop on the first short batch. They handle pagination so callers do not manage `start` cursors manually.
+- `iter_search_tickets`, `iter_search_users`, `iter_search_entities`, `iter_search_locations`, `iter_search_documents`, `iter_search_kb_articles` and `iter_search_kb_categories` yield successive `list[...]` batches of contract models and stop on the first short batch. They handle pagination so callers do not manage `start` cursors manually. A 4xx raises `GlpiStatusError` rather than ending the walk quietly, so an empty walk does mean the filter matched nothing.
 
 Returned identifiers are raw GLPI numeric values; resolve them with the appropriate `search_*` helpers when human-readable labels are needed.
 
@@ -29,7 +29,7 @@ Returned identifiers are raw GLPI numeric values; resolve them with the appropri
 3. For ticket counts, call `await client.get_ticket_statistics(start_date=..., end_date=..., default_days=..., entity_id=..., entity_name=..., extra_filter=...)`. All keyword arguments are optional; the default window is the last 30 days ending today.
 4. For task duration totals on a known ticket list, call `await client.get_task_statistics(ticket_ids)`. For an end-to-end "duration over a window with filters" report, call `await client.get_task_durations(...)` instead; it gathers the ticket IDs internally.
 5. For a per-user activity report, call `await client.get_user_activity(username=..., start_date=..., end_date=...)`. Supply at least one of `user_id`, `username`, `realname`, `firstname`.
-6. For memory-bounded pagination over large result sets, iterate `iter_search_tickets` / `iter_search_users` / `iter_search_entities` with `async for batch in client.iter_search_*(...): ...`.
+6. For memory-bounded pagination over large result sets, iterate any `iter_search_*` helper with `async for batch in client.iter_search_*(...): ...`.
 7. Use the public enums when composing additional RSQL filters. There are eight, all exported from `glpi_python_client`, and this is the whole list: `GlpiTicketStatus` (`NEW = 1`, `ASSIGNED = 2`, `PLANNED = 3`, `PENDING = 4`, `SOLVED = 5`, `CLOSED = 6`, `VALIDATION = 10`), `GlpiTicketType` (`INCIDENT = 1`, `REQUEST = 2`), `GlpiPriority` (`VERY_LOW = 1` .. `VERY_HIGH = 5`, `MAJOR = 6`), `GlpiGlobalValidation` and `GlpiSolutionStatus` (both `NONE = 1`, `WAITING = 2`, `ACCEPTED = 3`, `REFUSED = 4`), `GlpiTaskState` (`INFORMATION = 0`, `TODO = 1`, `DONE = 2`), `GlpiTimelinePosition` (`INVALID = -1`, `NONE = 0`, `LEFT = 1`, `RIGHT = 2`, `LEFT_BIG = 3`, `RIGHT_BIG = 4`) and `GlpiUserAuthType` (`LOCAL = 1`, `LDAP = 2`, `MAIL = 3`, `CAS = 4`, `X509 = 5`, `EXTERNAL = 6`). All eight subclass `GlpiEnum`, which is exported too and is a plain `IntEnum` with two conveniences for filter building: `.glpi_id` returns the number, and `.rsql_equals("status")` returns the RSQL fragment, so `GlpiTicketStatus.NEW.rsql_equals("status")` replaces the hand-written `f"status=={int(GlpiTicketStatus.NEW)}"` below.
 
 ## Examples
diff --git a/skills/glpi-team-members/SKILL.md b/skills/glpi-team-members/SKILL.md
index f6777e9..bfcc1db 100644
--- a/skills/glpi-team-members/SKILL.md
+++ b/skills/glpi-team-members/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to the GLPI v2 API, and credentials allowed to manage ticket teams."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Team Members
diff --git a/skills/glpi-ticket-timeline/SKILL.md b/skills/glpi-ticket-timeline/SKILL.md
index b1f65d0..bb59cf1 100644
--- a/skills/glpi-ticket-timeline/SKILL.md
+++ b/skills/glpi-ticket-timeline/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, and network access to the GLPI v2 API."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Ticket Timeline
diff --git a/skills/glpi-ticket-workflow/SKILL.md b/skills/glpi-ticket-workflow/SKILL.md
index 2009c73..101fdc3 100644
--- a/skills/glpi-ticket-workflow/SKILL.md
+++ b/skills/glpi-ticket-workflow/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to the GLPI v2 API, and credentials accepted by GlpiClient."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI Ticket Workflow
@@ -71,7 +71,7 @@ ticket = PostTicket(
 
 - On `AsyncGlpiClient` every ticket method is a coroutine and must be awaited; on `GlpiClient` the same methods are ordinary blocking calls and must not be awaited. `iter_search_tickets` is the exception to the shape: it is an async generator on `AsyncGlpiClient` (`async for`) and a plain generator on `GlpiClient` (`for`), so it is iterated, not awaited, on either surface.
 - `search_tickets` accepts a raw RSQL filter string; pagination is via the keyword-only `limit` and `start` (it also takes `sort` and `fields`). To walk a whole result set use the batch iterator `iter_search_tickets(rsql_filter, batch_size=50, sort=..., fields=...)`, which advances `start` itself and yields one `list[GetTicket]` page per step, stopping when a page comes back shorter than `batch_size` — `async for batch in client.iter_search_tickets(...)` on `AsyncGlpiClient`, `for batch in client.iter_search_tickets(...)` on `GlpiClient`.
-- **`search_tickets` swallows 4xx and returns `[]`.** This is a library-wide contract, not a ticket peculiarity: `_resource_list` checks the response status only when the caller passes a `failure_message`, and none of the seven `search_*` helpers (`search_tickets`, `search_users`, `search_locations`, `search_entities`, `search_documents`, `search_kb_articles`, `search_kb_categories`) passes one -- a GLPI error body is not a JSON list, so it is coerced to `[]`. A malformed RSQL filter, a 403, a missing route and a genuinely empty result set are therefore indistinguishable at the call site. `iter_search_tickets` inherits it: a swallowed 4xx yields a short first page, so the loop simply ends and you process nothing. `get_ticket` and every `list_*`/`get_*` helper do pass a `failure_message` and raise `GlpiStatusError` (narrowed to `GlpiAuthError` / `GlpiNotFoundError` / `GlpiServerError`) normally, so probe with one of those before believing an empty search -- and never treat `[]` as proof a ticket does not exist before creating a replacement.
+- **`search_tickets` and every other `search_*` raise `GlpiStatusError` on a 4xx.** This changed: they used to check the response status only when the caller passed a `failure_message`, which none of the seven `search_*` helpers does, so a GLPI error body was coerced to `[]` and a malformed RSQL filter, a 403, a missing route and a genuinely empty result set were indistinguishable. `_resource_list` now checks the status on every call, so **an empty list means the server said the result set is empty**. The iterators inherit that: a 4xx raises instead of making the first page short and ending the walk silently. Note the *other* fail-open path is unchanged and still bites -- GLPI v2 ignores a filter field it does not recognise and answers 200 with the whole unfiltered table, so a filter that returns rows is still not proof it was applied.
 - `create_ticket` returns the new ticket ID. `update_ticket` and `delete_ticket` return `None`.
 - The GLPI server is the authoritative validator. Extra keys returned by the server flow into `ticket.extra_payload` rather than raising. Caller-provided `extra_payload` keys win on conflicts.
 - Read-only fields such as `status` are intentionally absent from `PostTicket`/`PatchTicket`; the server controls those transitions. `global_validation` is the exception -- it *is* declared on both write models, typed `GlpiGlobalValidation | None`, whose members are `NONE = 1`, `WAITING = 2`, `ACCEPTED = 3`, `REFUSED = 4`. `GlpiGlobalValidation` is exported from the package root alongside `GlpiTicketStatus` (`NEW = 1`, `ASSIGNED = 2`, `PLANNED = 3`, `PENDING = 4`, `SOLVED = 5`, `CLOSED = 6`, `VALIDATION = 10`), `GlpiTicketType` (`INCIDENT = 1`, `REQUEST = 2`) and `GlpiPriority`; `status` and the rest stay readable on `GetTicket`.
\ No newline at end of file
diff --git a/skills/glpi-user-location-provisioning/SKILL.md b/skills/glpi-user-location-provisioning/SKILL.md
index 2a17260..742de53 100644
--- a/skills/glpi-user-location-provisioning/SKILL.md
+++ b/skills/glpi-user-location-provisioning/SKILL.md
@@ -5,7 +5,7 @@ license: MIT
 compatibility: "Requires Python 3.10+, glpi-python-client, network access to the GLPI v2 API, and credentials allowed to read or write users, locations, and entities."
 metadata:
   package: glpi-python-client
-  version: "0.4.1"
+  version: "0.4.3"
 ---
 
 # GLPI User, Location, And Entity Provisioning
@@ -17,6 +17,7 @@ Users live under `/Administration/User`, entities under `/Administration/Entity`
 
 1. Create a `GlpiClient` with the correct entity/profile scope.
 2. Search before creating duplicates: `search_users(rsql_filter, limit=..., start=..., skip_entity=False)`, `search_locations(rsql_filter, limit=..., start=...)`, `search_entities(rsql_filter, limit=..., start=...)`. Scope matters here: `search_users` and `search_locations` are narrowed by the client's `GLPI-Entity` / `GLPI-Profile` headers, so pass `skip_entity=True` to `search_users` (the only one of the three `search_*` helpers that has the flag — `iter_search_users` takes it too) to look across every entity the caller can see before deciding a user does not exist; `search_entities` always bypasses those headers.
+   To resolve a person by e-mail use `await client.find_user_by_email("a@b.test")`, which returns `GetUser | None` and defaults to `skip_entity=True` for the reason just given.
 3. Fetch one record with `get_user(user_id)`, `get_location(location_id)`, or `get_entity(entity_id)`.
 4. Create with `create_user(PostUser(...))`, `create_location(PostLocation(...))`, or `create_entity(PostEntity(...))`. Each returns the new ID.
 5. Update with `update_user(user_id, PatchUser(...))`, `update_location(location_id, PatchLocation(...))`, or `update_entity(entity_id, PatchEntity(...))`.
@@ -71,13 +72,14 @@ async def find_or_create_location(
     if matches and matches[0].id is not None:
         return matches[0].id
 
-    # Guard 2. Empty is not proof of absence. Re-run the same route with no
-    # filter at all -- same URL, same auth, same entity scope, nothing to
-    # reject -- so a swallowed 403/404/5xx shows up here too. An empty
-    # answer here has exactly two causes: the search layer failed, or the
-    # Locations dropdown is genuinely empty (a fresh GLPI ships it empty).
-    # This cannot tell them apart either, so it fails closed and makes the
-    # second one something the caller states on purpose.
+    # Guard 2. Empty is not proof of absence. A 4xx now raises, so the
+    # failure this used to catch is loud -- but the *other* fail-open path
+    # is not: v2 silently ignores a filter field it does not know and
+    # answers 200 with the whole table, and a filter that is dropped
+    # entirely can still yield nothing useful. Re-running with no filter
+    # at all distinguishes "this dropdown is empty" from "my filter was
+    # the problem", and it fails closed so the empty case is something the
+    # caller states on purpose.
     if not await client.search_locations("", limit=1) and not dropdown_may_be_empty:
         raise RuntimeError(
             "search_locations returned nothing even unfiltered: assume a failed "
@@ -97,10 +99,11 @@ for entity in entities:
 
 ## Gotchas
 
-- **`search_users`, `search_locations` and `search_entities` swallow 4xx and return `[]`, and this family is where that hurts most.** It is a library-wide contract, not a peculiarity of these three: `_resource_list` checks the response status only when the caller passes a `failure_message`, and none of the seven `search_*` helpers (`search_users`, `search_locations`, `search_entities`, `search_tickets`, `search_documents`, `search_kb_articles`, `search_kb_categories`) passes one -- a GLPI error body is not a JSON list, so it is coerced to `[]`. A malformed RSQL filter, a 403, a missing route and "no such record" are therefore indistinguishable. `iter_search_users` / `iter_search_entities` inherit it: a swallowed 4xx makes the first page short, so the loop ends having yielded nothing. **The find-or-create pattern is the trap**: `matches[0].id if matches else create(...)` provisions a duplicate user or location every time the search fails, and the duplicate is then a data-repair job, not an exception someone sees. Guard both halves as the example above does -- validate anything you interpolate into the filter, and corroborate an empty result with an unfiltered control search or with a `get_user`/`get_location`/`get_entity` on a known id, all of which do pass a `failure_message` and raise `GlpiStatusError` (narrowed to `GlpiAuthError` / `GlpiNotFoundError` / `GlpiServerError`) normally.
+- **`search_users`, `search_locations` and `search_entities` raise `GlpiStatusError` on a 4xx.** This changed: they used to check the response status only when the caller passed a `failure_message`, which none of the seven `search_*` helpers does, so a GLPI error body was coerced to `[]` and a malformed RSQL filter, a 403, a missing route and a genuinely empty result set were indistinguishable. `_resource_list` now checks the status on every call, so **an empty list means the server said the result set is empty**. The iterators inherit that: a 4xx raises instead of making the first page short and ending the walk silently. Note the *other* fail-open path is unchanged and still bites -- GLPI v2 ignores a filter field it does not recognise and answers 200 with the whole unfiltered table, so a filter that returns rows is still not proof it was applied. **The find-or-create pattern was the trap this protected against**: `matches[0].id if matches else create(...)` provisioned a duplicate every time the search failed. A 403 now raises there. It is still worth validating anything you interpolate into a filter, because the silent-drop path returns a *non-empty* wrong answer that no status check can catch.
 - On `AsyncGlpiClient` every method shown is a coroutine -- always `await` it -- and `iter_search_users` / `iter_search_entities` are async generators consumed with `async for`, not `await`. The generated `GlpiClient` carries the same names and signatures as ordinary blocking calls: no `await`, and a plain `for` over the iterators.
 - `PostUser` has **no** client-side required fields: every declared field defaults to `None` (bar `extra_payload`, which defaults to an empty dict) and `model_dump(exclude_none=True)` strips the unset ones, so `PostUser()` validates fine and it is the GLPI server that rejects a create without `username` and enforces the `password`/`password2` pair for local accounts. Tweak according to your auth backend: `authtype` on `PostUser`/`PatchUser`/`GetUser` is typed `GlpiUserAuthType | None`, exported from the package root, with members `LOCAL = 1`, `LDAP = 2`, `MAIL = 3`, `CAS = 4`, `X509 = 5`, `EXTERNAL = 6` -- pass the member (`authtype=GlpiUserAuthType.LDAP`) rather than a bare integer. Like every public enum in the package it subclasses `GlpiEnum`, itself an `IntEnum`, so it serialises as its number and compares equal to one. `password`/`password2` are `SecretStr` (plain `str` is coerced) and are masked in `repr` and logs, unmasked only when the request body is serialised.
-- Search filters are raw RSQL strings. `search_*` pages manually with `limit` and `start`, but for users and entities the client drives pagination for you: `async for page in client.iter_search_users(rsql_filter, batch_size=50, skip_entity=False)` and `iter_search_entities(rsql_filter, batch_size=50)` yield successive pages and stop on the first short page (plain `for` on `GlpiClient`). `iter_search_users` carries the same `skip_entity` flag as `search_users`, so pass `skip_entity=True` there too when paging across every entity; `iter_search_entities` has no such parameter and always spans them. There is no `iter_search_locations` -- page `search_locations` yourself with `limit`/`start`.
+- Search filters are raw RSQL strings. `search_*` pages manually with `limit` and `start`, but all three resources now have a batch iterator that drives pagination for you: `iter_search_users(rsql_filter, batch_size=50, skip_entity=False)`, `iter_search_entities(rsql_filter, batch_size=50)` and `iter_search_locations(rsql_filter, batch_size=50)` yield successive pages and stop on the first short page (plain `for` on `GlpiClient`). `iter_search_users` carries the same `skip_entity` flag as `search_users`, so pass `skip_entity=True` there too when paging across every entity; the other two have no such parameter.
+- `find_user_by_email("a@b.test")` resolves a person by address and returns `GetUser | None`. It **scans** -- GLPI exposes addresses as the nested array `User.emails`, which the v2 filter engine cannot join -- so narrow it with `rsql_filter="is_active==true"` where you can and cache the id instead of calling it per request. Do not hand-roll an RSQL e-mail filter: v2 ignores a field it does not recognise and answers with the whole table, so the first row would be the wrong person.
 - Extra keys returned by the live server (`display_name`, plugin fields, ...) flow into `record.extra_payload` rather than raising.
 - `delete_*(force=True)` permanently deletes the record; omit (or `False`/`None`) to move it to the trash.
 - If the user provides a name rather than an ID, search first and confirm the ID before changing or deleting records.
\ No newline at end of file
diff --git a/unasync_build.py b/unasync_build.py
index a2f7a7f..6d9c9f7 100644
--- a/unasync_build.py
+++ b/unasync_build.py
@@ -96,6 +96,14 @@
     "AsyncHTTPTransport": "HTTPTransport",
     "aclose": "close",
     "aread": "read",
+    # httpx spells the streaming body readers with an ``a`` prefix and
+    # defines both surfaces' versions on one ``Response`` class. Left
+    # un-rewritten, the generated tree calls ``aiter_bytes`` on a sync
+    # response and fails at *iteration* with "'async_generator' object is
+    # not iterable" -- not with an AttributeError at the call, which is why
+    # this is easy to miss. ``aiter_text``/``aiter_lines``/``aiter_raw``
+    # are equally un-rewritten and would each need an entry here.
+    "aiter_bytes": "iter_bytes",
 }