From 5ec15f70b6469ab650f0b42c35c5c96d2389496d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Proch=C3=A1zka?= Date: Fri, 2 Oct 2026 15:32:34 +0000 Subject: [PATCH 1/2] feat: sync Java client with apify-client-js reference updates and spec v2-2026-10-01T153946Z - Add Build.getImageDigest() (spec apify-docs#3037). - Fix ScheduleClient.getLog(): was returning the raw response body as text; the endpoint returns a JSON envelope of ScheduleInvoked entries. - Fix DatasetClient.iterateItems() to paginate by the X-Apify-Pagination-Count scanned-row count (falling back to the returned count when the header is absent or implausibly low), fixing repeated/skipped items when combined with server-side item filters. - Add ApifyApiException subclasses by HTTP status (InvalidRequestError, UnauthorizedError, ForbiddenError, NotFoundError, ConflictError, RateLimitError, ServerError). - A 404 on a resource client reached through a run/build with no id of its own (run.dataset(), run.keyValueStore(), run.requestQueue(), run.log(), build.log()) now throws NotFoundError instead of resolving to empty/no-op. DatasetClient.getStatistics() and TaskClient.getInput() likewise throw on 404 instead of returning Optional.empty(). - Harden ResourceContext.toSafeId/encodePathSegment against path traversal (replace every slash, reject dot-only/empty path segments). - Skip request-body compression for content types that already carry their own compression (images, audio, video, archives, fonts). - Fix ApifyClientBuilder base URL normalization doubling /v2 when the caller already included it. - Add createItemsPublicUrl(options, expiresInSecs, format) overload. Bump Version.API_SPEC_VERSION to v2-2026-10-01T153946Z, CLIENT_VERSION to 0.7.0. --- CHANGELOG.md | 41 +++++ README.md | 38 ++++- docs/README.md | 8 +- docs/builds.md | 4 +- docs/misc.md | 4 +- docs/schedules.md | 5 +- docs/storages.md | 34 +++-- docs/tasks.md | 2 +- pom.xml | 2 +- .../java/com/apify/client/ApifyClient.java | 9 ++ .../com/apify/client/ApifyClientBuilder.java | 28 +++- .../java/com/apify/client/PaginationList.java | 26 ++++ src/main/java/com/apify/client/Version.java | 4 +- .../java/com/apify/client/build/Build.java | 10 ++ .../apify/client/dataset/DatasetClient.java | 82 +++++++--- .../apify/client/http/ApifyApiException.java | 8 + .../com/apify/client/http/ConflictError.java | 23 +++ .../com/apify/client/http/ForbiddenError.java | 23 +++ .../client/http/InvalidRequestError.java | 23 +++ .../com/apify/client/http/NotFoundError.java | 29 ++++ .../com/apify/client/http/RateLimitError.java | 24 +++ .../com/apify/client/http/ServerError.java | 25 ++++ .../apify/client/http/UnauthorizedError.java | 23 +++ .../internal/AsyncPaginatedPublisher.java | 26 +++- .../apify/client/internal/HttpClientCore.java | 140 +++++++++++++++++- .../client/internal/ResourceContext.java | 96 ++++++++++-- .../client/keyvalue/KeyValueStoreClient.java | 17 ++- .../java/com/apify/client/log/LogClient.java | 12 +- .../requestqueue/RequestQueueClient.java | 17 ++- .../apify/client/schedule/ScheduleClient.java | 22 +-- .../client/schedule/ScheduleInvoked.java | 28 ++++ .../com/apify/client/task/TaskClient.java | 18 ++- .../apify/client/AmbiguousNotFoundTest.java | 117 +++++++++++++++ .../client/BaseUrlNormalizationTest.java | 56 +++++++ .../client/ClientBehaviourRegressionTest.java | 80 ++++++++++ .../com/apify/client/CompressionTest.java | 59 ++++++++ .../client/DatasetItemsIteratorTest.java | 89 +++++++++++ .../apify/client/ExceptionHierarchyTest.java | 75 ++++++++++ .../java/com/apify/client/MockTransport.java | 26 +++- .../integration/ScheduleIntegrationTest.java | 7 +- .../integration/TaskIntegrationTest.java | 2 +- .../ResourceContextPathSafetyTest.java | 51 +++++++ 42 files changed, 1305 insertions(+), 108 deletions(-) create mode 100644 src/main/java/com/apify/client/http/ConflictError.java create mode 100644 src/main/java/com/apify/client/http/ForbiddenError.java create mode 100644 src/main/java/com/apify/client/http/InvalidRequestError.java create mode 100644 src/main/java/com/apify/client/http/NotFoundError.java create mode 100644 src/main/java/com/apify/client/http/RateLimitError.java create mode 100644 src/main/java/com/apify/client/http/ServerError.java create mode 100644 src/main/java/com/apify/client/http/UnauthorizedError.java create mode 100644 src/main/java/com/apify/client/schedule/ScheduleInvoked.java create mode 100644 src/test/java/com/apify/client/AmbiguousNotFoundTest.java create mode 100644 src/test/java/com/apify/client/BaseUrlNormalizationTest.java create mode 100644 src/test/java/com/apify/client/internal/ResourceContextPathSafetyTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c92a46..45349b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,47 @@ All notable changes to the Apify Java client are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.7.0] - 2026-10-02 + +### Added + +- `Build.getImageDigest()`, mirroring the OpenAPI spec's new `Build.imageDigest` field. +- `ApifyApiException` subclasses by HTTP status, matching the reference JS client: + `InvalidRequestError` (400), `UnauthorizedError` (401), `ForbiddenError` (403), `NotFoundError` + (404), `ConflictError` (409), `RateLimitError` (429), `ServerError` (5xx). +- `DatasetClient.createItemsPublicUrl(DatasetListItemsOptions, Long, DownloadItemsFormat)` overload + to set the public URL's serialization format. +- `ScheduleInvoked` model. + +### Fixed + +- `ScheduleClient.getLog()` was fetching the raw response body as text; the endpoint's response is + a JSON envelope wrapping an array of log entries. It now returns `List`. +- `DatasetClient.iterateItems`/the publisher driving it could repeat or silently skip items when + combined with server-side item filters (`clean`/`skipEmpty`/`skipHidden`): it now paginates by the + `X-Apify-Pagination-Count` scanned-row count the API reports (when that count is at least the + number of items returned; a smaller value is treated as the header not being populated and falls + back to the returned count, as before), not just by the number of items returned. +- `ApifyClientBuilder`'s `baseUrl`/`publicBaseUrl` doubled the `/v2` suffix when the caller already + included it (e.g. `"https://api.apify.com/v2"` became `".../v2/v2"`). +- `ResourceContext.toSafeId` replaced only the first `/` in a resource id; it now replaces all of + them, and `encodePathSegment` rejects an empty or dot-only (`.`/`..`) path segment instead of + encoding it, matching the reference client's URL path-traversal hardening. +- Request-body compression now skips content types that already carry their own compression + (images, audio, video, archives, office/zip packages, web fonts), matching the reference client. + +### Changed + +- A 404 on a resource client reached through a run/task/build without an explicit id of its own + (`run.dataset()`, `run.keyValueStore()`, `run.requestQueue()`, `run.log()`, `build.log()`) now + throws `NotFoundError` from `get()`/`delete()`/`log().get()` instead of resolving to an empty + `Optional`/no-op, since the 404 is ambiguous between the parent and the sub-resource being gone. + Resources addressed by an explicit id are unaffected. +- `DatasetClient.getStatistics()` and `TaskClient.getInput()` now throw `NotFoundError` on a 404 + (returning `JsonNode` directly) instead of resolving to `Optional.empty()`, for the same reason. +- Bumped `Version.API_SPEC_VERSION` to `v2-2026-10-01T153946Z` and `Version.CLIENT_VERSION` to + `0.7.0`. + ## [0.6.5] - 2026-09-29 ### Changed diff --git a/README.md b/README.md index 647896d..56208d5 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Maven (Maven Central is a default repository, so no extra configuration is neede com.apify apify-client - 0.6.5 + 0.7.0 ``` @@ -35,7 +35,7 @@ repositories { } dependencies { - implementation 'com.apify:apify-client:0.6.5' + implementation 'com.apify:apify-client:0.7.0' } ``` @@ -218,8 +218,9 @@ methods directly on top of the JDK `HttpClient`'s own `sendAsync`. ## Fetching single resources -Methods that fetch a single resource complete with an `Optional`: a missing resource is reported -by an empty `Optional` rather than an exception. +Methods that fetch a single resource **addressed by an explicit id** complete with an `Optional`: +a missing resource is reported by an empty `Optional` rather than an exception, and `delete()` +resolves without error if the resource is already gone. ```java client.actor("apify/hello-world").get() @@ -227,6 +228,23 @@ client.actor("apify/hello-world").get() .join(); ``` +Everywhere else, a 404 throws `NotFoundError` instead: a client chained off a run or build with no +id of its own (e.g. `client.run(id).dataset()`, `client.build(id).log()`), where the missing +resource could be the parent rather than the sub-resource, and a handful of fixed sub-paths where a +missing value is not a meaningful state distinct from "the parent is gone" (`DatasetClient +.getStatistics()`, `ScheduleClient.getLog()`, `TaskClient.getInput()`, `UserClient.monthlyUsage()` / +`limits()`, `WebhookClient.test()`): + +```java +try { + client.run("missing-run").dataset().get().join(); +} catch (CompletionException e) { + if (e.getCause() instanceof NotFoundError) { + // Either the run or its default dataset does not exist. + } +} +``` + ## Error handling Every exception this client throws for a request/transport failure is an unchecked @@ -239,7 +257,13 @@ original exception wrapped in an unchecked `CompletionException` (`.get()` wraps `ApifyApiException`/`ApifyTransportException`, or use `.handle(...)`/`.exceptionally(...)` to react to it without unwrapping at all: -- `ApifyApiException` — the request reached the API, which answered with a non-success status. +- `ApifyApiException` — the request reached the API, which answered with a non-success status. The + client throws the subclass matching the response's status code, so a `catch` can branch with + `instanceof` instead of comparing `getStatusCode()` by number — `InvalidRequestError` (400), + `UnauthorizedError` (401), `ForbiddenError` (403), `NotFoundError` (404), `ConflictError` (409), + `RateLimitError` (429) or `ServerError` (5xx). Any other status is the plain `ApifyApiException` + base class, which every subclass extends, so an existing `catch (ApifyApiException e)` keeps + working unchanged. - `ApifyTransportException` — the request never produced an API response at all (connection failure, DNS, timeout, or a local failure preparing the request/response, e.g. compression). `isTimeout()` reports whether the underlying cause was specifically a timeout (backed by @@ -293,10 +317,10 @@ try { The public `com.apify.client.Version` class (`import com.apify.client.Version;`) exposes two constants: -- `Version.CLIENT_VERSION` — the semantic version of this client (`0.6.5`). +- `Version.CLIENT_VERSION` — the semantic version of this client (`0.7.0`). - `Version.API_SPEC_VERSION` — the version of the [Apify OpenAPI specification](https://docs.apify.com/api/openapi.json) (its `info.version` field) that this client's endpoints, parameters and models were last generated - and checked against (`v2-2026-09-28T115051Z`). It is a snapshot, not a live compatibility + and checked against (`v2-2026-10-01T153946Z`). It is a snapshot, not a live compatibility guarantee: the client keeps working against newer, backward-compatible spec revisions, but a feature added to the API after this snapshot has no corresponding method here yet. diff --git a/docs/README.md b/docs/README.md index 6a2e645..e21c877 100644 --- a/docs/README.md +++ b/docs/README.md @@ -76,9 +76,11 @@ transitive dependency of this client, so it is already on your classpath. A few methods return data whose shape is not modelled by this client and is instead exposed as a Jackson `JsonNode` (or accept an arbitrary `Object` serialized to JSON): -- Read, returning a required `JsonNode` (never absent): `me().monthlyUsage(...)`, `me().limits()`. -- Read, returning `Optional` (empty when the underlying resource has none): `dataset(id).getStatistics()`, - `task(id).getInput()`, `build(id).getOpenApiDefinition()`. +- Read, returning a required `JsonNode` (never absent; a 404 throws rather than returning empty — + see [error handling](../README.md#error-handling)): `me().monthlyUsage(...)`, `me().limits()`, + `dataset(id).getStatistics()`, `task(id).getInput()`. +- Read, returning `Optional` (empty when the underlying resource has none): + `build(id).getOpenApiDefinition()`. - Write: `task(id).updateInput(...)` (itself returning a required `JsonNode`, the updated input) and `me().updateLimits(...)` accept an arbitrary JSON-serializable value, as do definition/`update`/`create` arguments generally — a `Map`, a `JsonNode`, or your own POJO. diff --git a/docs/builds.md b/docs/builds.md index 88a7ace..cc71c16 100644 --- a/docs/builds.md +++ b/docs/builds.md @@ -23,7 +23,9 @@ builds) and a single build with `client.build(id)`. | `log()` | A `LogClient` for the build's log. | `Build` fields: `getId()`, `getActId()`, `getUserId()`, `getStatus()`, `getStartedAt()`, -`getFinishedAt()`, `getBuildNumber()`, `getMeta()` (`BuildMeta` — `getOrigin()`, `getClientIp()`, +`getFinishedAt()`, `getBuildNumber()`, `getImageDigest()` (`String`, nullable — the built Docker +image manifest's digest, without the `sha256:` prefix; compare two builds' digests to tell whether +their image contents differ), `getMeta()` (`BuildMeta` — `getOrigin()`, `getClientIp()`, `getUserAgent()`), `getStats()` (`BuildStats` — `getDurationMillis()`/`getRunTimeSecs()` as `Long`, `getComputeUnits()` as `Double`, `getImageSizeBytes()` as `Long`), `getOptions()` (`BuildOptions` — `getUseCache()`/`getBetaPackages()` as `Boolean`, `getMemoryMbytes()`/`getDiskMbytes()` as `Long`), diff --git a/docs/misc.md b/docs/misc.md index 33bf9d9..ecda77c 100644 --- a/docs/misc.md +++ b/docs/misc.md @@ -78,8 +78,8 @@ Access a build's or run's log directly, or via `client.run(id).log()` / `client. | Method | Description | |---|---| -| `get()` / `get(LogOptions)` | The whole log as text. Completes with `Optional`. | -| `stream()` / `stream(LogOptions)` | A live `InputStream` over the log (for redirection). Completes with `InputStream`. | +| `get()` / `get(LogOptions)` | The whole log as text. Completes with `Optional` when addressed by an explicit id (`client.log(id)`); on `run.log()`/`build.log()` (no log id of its own), a 404 is ambiguous and throws `NotFoundError` instead — see [Fetching single resources](../README.md#fetching-single-resources). | +| `stream()` / `stream(LogOptions)` | A live `InputStream` over the log (for redirection). Completes with `InputStream`; throws on any error response, including a 404. | `LogOptions` fields: `raw(Boolean)`, `download(Boolean)`. diff --git a/docs/schedules.md b/docs/schedules.md index 6c5f8ce..05330ad 100644 --- a/docs/schedules.md +++ b/docs/schedules.md @@ -29,7 +29,7 @@ Schedule schedule = client.schedules().create(Map.of( | Method | Description | |---|---| | `get()` / `update(Object)` / `delete()` | CRUD. | -| `getLog()` | The schedule's invocation log. Completes with `Optional`. | +| `getLog()` | Up to the last 1000 entries of the schedule's invocation log. Completes with `List`; throws `NotFoundError` if the schedule itself no longer exists. | ```java Optional s = client.schedule("SCHEDULE_ID").get().join(); @@ -44,3 +44,6 @@ s.ifPresent(sched -> System.out.println(sched.getCronExpression())); (`ScheduleNotifications`, exposing `isEmail()`). Any field not covered by a typed getter is still available via the inherited `getExtra()` (see [the docs index](README.md#model-fields-and-unmodeled-data-getextra)). + +`ScheduleInvoked` (one entry of `getLog()`): `getMessage()`, `getLevel()` (e.g. `INFO`, `ERROR`), +`getCreatedAt()` (`Instant`). diff --git a/docs/storages.md b/docs/storages.md index 6ae6511..5bc4ec1 100644 --- a/docs/storages.md +++ b/docs/storages.md @@ -38,25 +38,26 @@ any field not covered by a typed getter above is still available via the inherit | Method | Description | |---|---| -| `get()` / `update(Object)` / `delete()` | Metadata CRUD. | +| `get()` / `update(Object)` / `delete()` | Metadata CRUD. On a client reached through a run/task with no dataset id of its own (e.g. `run.dataset()`), a 404 is ambiguous (parent vs. sub-resource gone) and throws `NotFoundError` instead of resolving to empty/no-op — see [Fetching single resources](../README.md#fetching-single-resources). | | `listItems(DatasetListItemsOptions)` | List items. Completes with `PaginationList`. | | `listItems(DatasetListItemsOptions, Class)` | List items decoded into `T`. Completes with `PaginationList`. | | `iterateItems(DatasetListItemsOptions)` / `iterateItems(DatasetListItemsOptions, Long chunkSize)` | A lazy `Flow.Publisher` over all items; the options' `limit` caps the total yielded (`null`/unset or non-positive = all), the optional `chunkSize` sets the per-request page size (omitted/`null` = server default). | | `iterateItems(DatasetListItemsOptions, Long chunkSize, Class)` | As above, decoded into `T`. Returns `Flow.Publisher`. For typed iteration at the server-default page size, pass a `null` chunk size: `iterateItems(opts, null, T.class)`. | | `downloadItems(DownloadItemsFormat, DatasetDownloadOptions)` | Serialized bytes. `DownloadItemsFormat` is one of `JSON`, `JSONL`, `CSV`, `XLSX`, `XML`, `RSS`, `HTML`. Completes with `byte[]`. | | `pushItems(Object)` | Push a single item or a list of items. Completes with no value (`CompletableFuture`). | -| `getStatistics()` | Dataset statistics. Completes with `Optional`. | -| `createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs)` | A public (optionally signed) items URL. Completes with `String`. | +| `getStatistics()` | Dataset statistics. Completes with `JsonNode`; throws `NotFoundError` if the dataset is gone (no separate "statistics absent" state). | +| `createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs)` | A public (optionally signed) items URL, served as `json`. Completes with `String`. | +| `createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs, DownloadItemsFormat format)` | As above, with the URL's serialization `format` set explicitly. | > **Server-side item filters and iteration.** The dataset-items endpoint applies `offset`/`limit` to > the raw items and then drops those removed by a server-side filter (`skipEmpty`, `skipHidden`, -> `clean`, `simplified`), so a page can contain fewer items than requested. Because `iterateItems` -> advances the offset by the number of items actually returned, combining it with those filters over a -> multi-page dataset has two failure modes: page windows can overlap and **repeat items**, and — more -> severely — if an entire offset window is filtered out the endpoint returns an empty page, which the -> iterator treats as the end, so iteration **stops early and silently skips the remaining data** (an -> all-filtered first page yields nothing at all). Prefer paging without server-side item filters when -> iterating, or fetch pages explicitly with `listItems` and filter client-side. +> `clean`, `simplified`), so a page can *scan* up to `limit` rows while *returning* fewer, or none at +> all. Where the API reports the number of rows it scanned (`X-Apify-Pagination-Count`), `iterateItems` +> advances by that number rather than by the number of items returned, so a fully-filtered page +> neither repeats already-seen items nor ends iteration early. A reported count smaller than the +> returned one (seen in practice as a flat `0` even on pages that did return items) is treated as the +> header not being populated for that request and falls back to the returned count, so this is safe +> whether or not a given deployment of the API sends the header yet. ```java Dataset ds = client.datasets().getOrCreate("my-dataset").join(); @@ -83,10 +84,11 @@ adds `attachment` (`Boolean`), `bom` (`Boolean`), `delimiter` (`String`), `skipH (`Boolean`), `xmlRoot` (`String`), `xmlRow` (`String`), `feedTitle` (`String`), `feedDescription` (`String`). -`createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs)` completes with a `String` URL. -If the dataset is private, the client fetches it, reads its URL-signing secret, and appends an -HMAC-SHA256 signature (bounded by `expiresInSecs`, or non-expiring when `null`); for public datasets -the URL is unsigned. +`createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs[, DownloadItemsFormat format])` +completes with a `String` URL. If the dataset is private, the client fetches it, reads its +URL-signing secret, and appends an HMAC-SHA256 signature (bounded by `expiresInSecs`, or +non-expiring when `null`); for public datasets the URL is unsigned. The optional `format` sets the +serialization the URL serves items in (default `json` when omitted/`null`). > **Public-URL limitation (matches the JavaScript reference client).** The public-URL builders > (`createItemsPublicUrl`, and the key-value-store `getRecordPublicUrl` / `createKeysPublicUrl`) @@ -108,7 +110,7 @@ datasets. | Method | Description | |---|---| -| `get()` / `update(Object)` / `delete()` | Metadata CRUD. | +| `get()` / `update(Object)` / `delete()` | Metadata CRUD. On a client reached through a run/task with no store id of its own (e.g. `run.keyValueStore()`), a 404 is ambiguous (parent vs. sub-resource gone) and throws `NotFoundError` instead of resolving to empty/no-op — see [Fetching single resources](../README.md#fetching-single-resources). | | `listKeys(ListKeysOptions)` | List keys. Completes with `KeyValueStoreKeysPage`. | | `iterateKeys(ListKeysOptions)` / `iterateKeys(ListKeysOptions, Long chunkSize)` | A lazy `Flow.Publisher` over all keys, paging with the cursor (`exclusiveStartKey`). Note: here the options' `limit` caps the **total** number of keys yielded (`null`/unset or non-positive = all), whereas for `listKeys`/`createKeysPublicUrl` the same `ListKeysOptions.limit` is a single-request page size. `chunkSize` sets the per-request page size (`null` = server default). | | `recordExists(String key)` | Whether a record exists. Completes with `boolean`. | @@ -158,7 +160,7 @@ overload here — the request-queue creation endpoint does not accept a creation | Method | Description | |---|---| -| `get()` / `update(Object)` / `delete()` | Metadata CRUD. | +| `get()` / `update(Object)` / `delete()` | Metadata CRUD. On a client reached through a run/task with no queue id of its own (e.g. `run.requestQueue()`), a 404 is ambiguous (parent vs. sub-resource gone) and throws `NotFoundError` instead of resolving to empty/no-op — see [Fetching single resources](../README.md#fetching-single-resources). | | `withClientKey(String)` | A copy that identifies its requests with a stable client key (required for lock operations). | | `listHead(Long limit)` | Requests at the head. Completes with `RequestQueueHead`. | | `addRequest(RequestQueueRequest, boolean forefront)` | Add a request. Completes with `RequestQueueOperationInfo`. | diff --git a/docs/tasks.md b/docs/tasks.md index 6d19bd0..fbef5e6 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -28,7 +28,7 @@ Task task = client.tasks().create(Map.of( | `start(Object input, TaskStartOptions)` | Start a task run (input overrides stored input; `null` uses it). Completes with `ActorRun`. | | `call(Object input, TaskStartOptions, Long waitSecs)` | Start and poll until finished; does **not** stream the run's log. Completes with `ActorRun`. | | `call(Object input, TaskCallOptions, Long waitSecs)` | As above, additionally streaming the run's log for the duration of the wait by default (matching the reference client's `call` defaulting `options.log` to `'default'`). Use `TaskCallOptions.disableLogStreaming()` to opt out, or `logOptions(StreamedLogOptions)` for a custom destination. | -| `getInput()` | The stored input. Completes with `Optional`. | +| `getInput()` | The stored input. Completes with `JsonNode`; throws `NotFoundError` if the task is gone (no separate "input absent" state). | | `updateInput(Object)` | Replace the stored input. Completes with `JsonNode`. | | `lastRun(String status)` / `lastRun(LastRunOptions)` | A `RunClient` for the last run (see [`LastRunOptions`](actors.md#actorclient)). | | `runs()` | Nested run collection client. | diff --git a/pom.xml b/pom.xml index 9d1bdd0..9c1829d 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.apify apify-client - 0.6.5 + 0.7.0 jar Apify Java Client diff --git a/src/main/java/com/apify/client/ApifyClient.java b/src/main/java/com/apify/client/ApifyClient.java index 731513f..7dc81e3 100644 --- a/src/main/java/com/apify/client/ApifyClient.java +++ b/src/main/java/com/apify/client/ApifyClient.java @@ -116,6 +116,15 @@ String getApiBaseUrl() { return baseUrl; } + /** + * Returns the fully-qualified public API base URL this client builds shareable URLs against + * (including the {@code /v2} suffix). Not part of the public API, for the same reason as {@link + * #getUserAgent()}. + */ + String getPublicApiBaseUrl() { + return publicBaseUrl; + } + // ----- Actor accessors ----------------------------------------------------- /** A client for the Actor collection (list & create Actors). */ diff --git a/src/main/java/com/apify/client/ApifyClientBuilder.java b/src/main/java/com/apify/client/ApifyClientBuilder.java index 1938f97..5f572c9 100644 --- a/src/main/java/com/apify/client/ApifyClientBuilder.java +++ b/src/main/java/com/apify/client/ApifyClientBuilder.java @@ -4,6 +4,8 @@ import com.apify.client.http.HttpTransport; import com.apify.client.http.RetryConfig; import com.apify.client.internal.HttpClientCore; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Duration; import java.util.Locale; import java.util.function.BooleanSupplier; @@ -166,9 +168,31 @@ public ApifyClient build() { return new ApifyClient(http, apiBase, publicBase); } - /** Strips any trailing slashes and appends {@link #API_VERSION_PATH}. */ + /** + * Strips any trailing slashes and appends {@link #API_VERSION_PATH}, unless the URL's path + * already ends with it (e.g. a caller passing {@code "https://api.apify.com/v2"} does not end up + * with {@code ".../v2/v2"}). Only an exact {@code /v2} path suffix counts; another {@code /vN} is + * left alone, since this client's endpoints only exist under {@code /v2}. + * + *

Checks the URL's parsed path, not just whether the full string ends with {@code "/v2"}: a + * bare {@code "https://v2"} (host {@code v2}, no path at all) would otherwise be mistaken for an + * already-versioned URL, because the {@code "//"} of the scheme happens to precede a host that + * reads "v2" - {@code "https://v2"} must still become {@code "https://v2/v2"}. + */ private static String normalizeApiUrl(String url) { - return trimTrailingSlash(url) + API_VERSION_PATH; + String trimmed = trimTrailingSlash(url); + String path = pathOf(trimmed); + return path.endsWith(API_VERSION_PATH) ? trimmed : trimmed + API_VERSION_PATH; + } + + /** The path component of a URL, or {@code ""} if it does not parse as one. */ + private static String pathOf(String url) { + try { + String path = new URI(url).getRawPath(); + return path == null ? "" : path; + } catch (URISyntaxException e) { + return ""; + } } private static String trimTrailingSlash(String s) { diff --git a/src/main/java/com/apify/client/PaginationList.java b/src/main/java/com/apify/client/PaginationList.java index 2308644..558c1f1 100644 --- a/src/main/java/com/apify/client/PaginationList.java +++ b/src/main/java/com/apify/client/PaginationList.java @@ -22,6 +22,7 @@ public final class PaginationList extends ApifyResource { private long count; private boolean desc; private List items = List.of(); + private Long scannedCount; /** Total number of items available across all pages. */ public long getTotal() { @@ -48,6 +49,27 @@ public boolean isDesc() { return desc; } + /** + * The number of rows the API scanned to produce this page, before any item-level filtering (e.g. + * dataset items' {@code clean}/{@code skipEmpty}/{@code skipHidden}), as reported by the {@code + * X-Apify-Pagination-Count} response header. {@code null} when the endpoint does not send that + * header, in which case {@link #getCount()} (the number of items actually returned) is the right + * value to advance pagination by - the two always agree outside the dataset items endpoint. + * + *

A value smaller than {@link #getCount()} cannot be a genuine "rows scanned" answer (scanning + * can never produce more items than it scanned), so it is treated as the header not actually + * being populated for that request rather than as real data - callers reading this field directly + * should apply the same {@code scannedCount != null && scannedCount >= getCount() ? scannedCount + * : getCount()} fallback that {@link com.apify.client.dataset.DatasetClient#iterateItems} uses. + * + *

Internal pagination-engine detail, exposed here (rather than hidden) only because {@link + * #getItems()} and this metadata necessarily travel together on the same page object; most + * callers never need it directly - {@code iterateItems} already accounts for it. + */ + public Long getScannedCount() { + return scannedCount; + } + /** The items of this page (never {@code null}; unmodifiable). */ public List getItems() { // Null-coalesce: Jackson binds directly to the (private) `items` field for deserialization @@ -80,6 +102,10 @@ public void setDesc(boolean desc) { this.desc = desc; } + public void setScannedCount(Long scannedCount) { + this.scannedCount = scannedCount; + } + public void setItems(List items) { // Copy defensively: items is set once by the client from a freshly-parsed/collected list, but // taking an immutable copy means a caller can never mutate this page's items afterwards by diff --git a/src/main/java/com/apify/client/Version.java b/src/main/java/com/apify/client/Version.java index e30e989..c637bc4 100644 --- a/src/main/java/com/apify/client/Version.java +++ b/src/main/java/com/apify/client/Version.java @@ -13,13 +13,13 @@ public final class Version { * The semantic version of this client library (see SemVer). * Changes to the public interface other than additive ones are considered breaking changes. */ - public static final String CLIENT_VERSION = "0.6.5"; + public static final String CLIENT_VERSION = "0.7.0"; /** * The version of the Apify OpenAPI specification this client was generated and verified against. * Corresponds to the {@code info.version} field of the Apify OpenAPI document. */ - public static final String API_SPEC_VERSION = "v2-2026-09-28T115051Z"; + public static final String API_SPEC_VERSION = "v2-2026-10-01T153946Z"; private Version() {} } diff --git a/src/main/java/com/apify/client/build/Build.java b/src/main/java/com/apify/client/build/Build.java index 8a5fa98..cd12040 100644 --- a/src/main/java/com/apify/client/build/Build.java +++ b/src/main/java/com/apify/client/build/Build.java @@ -13,6 +13,7 @@ public final class Build extends ApifyResource { private Instant startedAt; private Instant finishedAt; private String buildNumber; + private String imageDigest; private BuildMeta meta; private BuildStats stats; private BuildOptions options; @@ -59,6 +60,15 @@ public String getBuildNumber() { return buildNumber; } + /** + * Digest of the built Docker image manifest, without the {@code sha256:} prefix. Compare the + * digests of two builds to find out whether their image contents differ. {@code null} if the + * digest is not available. + */ + public String getImageDigest() { + return imageDigest; + } + /** Metadata about how the build was initiated. */ public BuildMeta getMeta() { return meta; diff --git a/src/main/java/com/apify/client/dataset/DatasetClient.java b/src/main/java/com/apify/client/dataset/DatasetClient.java index 71053ac..46b4d35 100644 --- a/src/main/java/com/apify/client/dataset/DatasetClient.java +++ b/src/main/java/com/apify/client/dataset/DatasetClient.java @@ -29,6 +29,13 @@ public final class DatasetClient { /** Header reporting the effective page size limit applied to this page. */ private static final String HEADER_PAGINATION_LIMIT = "X-Apify-Pagination-Limit"; + /** + * Header reporting the number of rows the API scanned to produce this page, before any item-level + * filtering ({@code clean}/{@code skipEmpty}/{@code skipHidden}) is applied. Absent on older API + * responses; see {@link PaginationList#getScannedCount()}. + */ + private static final String HEADER_PAGINATION_COUNT = "X-Apify-Pagination-Count"; + private final HttpClientCore http; private final ResourceContext ctx; @@ -65,9 +72,15 @@ public static DatasetClient nested( http, ResourceContext.nestedCollection(http, base, subPath, inherited)); } - /** Fetches the dataset metadata, or empty if it does not exist. */ + /** + * Fetches the dataset metadata, or empty if it does not exist. + * + *

On a client reached through a run/task without an explicit dataset id (e.g. {@code + * run.dataset()}), a 404 is ambiguous between "the run is gone" and "the run has no dataset", so + * it throws {@link com.apify.client.http.NotFoundError} instead of resolving to empty. + */ public CompletableFuture> get() { - return ctx.getResource("", new QueryParams(), Dataset.class); + return ctx.getResourceUnlessAmbiguous("", new QueryParams(), Dataset.class); } /** Updates the dataset metadata (e.g. name, title) and returns the updated object. */ @@ -75,9 +88,12 @@ public CompletableFuture update(Object newFields) { return ctx.updateResource("", newFields, Dataset.class); } - /** Deletes the dataset. */ + /** + * Deletes the dataset. See {@link #get()} for why a client without an explicit dataset id throws + * on a 404 instead of treating it as a no-op. + */ public CompletableFuture delete() { - return ctx.deleteResource(""); + return ctx.deleteResourceUnlessAmbiguous(""); } /** @@ -122,12 +138,16 @@ public Flow.Publisher iterateItems(DatasetListItemsOptions options) { * of items yielded ({@code null} or non-positive = all); {@code chunkSize} is the per-request * page size ({@code null} = server default). * - *

Note: server-side item filters ({@code skipEmpty}, {@code skipHidden}, {@code clean}, {@code - * simplified}) are applied after {@code offset}/{@code limit}, so a page can return fewer items - * than requested. Combining those filters with iteration can repeat items (overlapping windows) - * and, if a whole offset window is filtered out, the endpoint returns an empty page which ends - * iteration early — silently skipping the remaining items. Iterate without server-side item - * filters, or page explicitly with {@link #listItems} and filter client-side. + *

Server-side item filters ({@code skipEmpty}, {@code skipHidden}, {@code clean}, {@code + * simplified}) are applied after {@code offset}/{@code limit}, so a page can scan up to {@code + * limit} rows while returning fewer, or none at all. Where the API reports the number of rows it + * scanned ({@code X-Apify-Pagination-Count}), this iterator advances by that number rather than + * by the number of items returned, so a fully-filtered page neither repeats already-seen items + * nor ends iteration early. A header value smaller than the returned count (observed in practice + * as a flat {@code 0} even on pages that did return real items) is treated as the header not + * being populated for that request, falling back to the returned count - exactly as if the header + * were absent - so this is safe whether or not a given deployment of the API actually sends it + * yet. */ public Flow.Publisher iterateItems( DatasetListItemsOptions options, Long chunkSize, Class itemClass) { @@ -169,6 +189,7 @@ private CompletableFuture> fetchItemsPage( result.setTotal(headerLong(resp, HEADER_PAGINATION_TOTAL, count)); result.setOffset(headerLong(resp, HEADER_PAGINATION_OFFSET, 0)); result.setLimit(headerLong(resp, HEADER_PAGINATION_LIMIT, count)); + result.setScannedCount(headerLongOrNull(resp, HEADER_PAGINATION_COUNT)); if (desc != null) { result.setDesc(desc); } @@ -207,14 +228,15 @@ public CompletableFuture pushItems(Object items) { .thenApply(resp -> null); } - /** Returns statistical information about the dataset, or empty if unavailable. */ - public CompletableFuture> getStatistics() { - return ctx.getRaw("statistics", new QueryParams()) - .thenApply( - resp -> - resp == null - ? Optional.empty() - : Optional.of(Json.parseData(resp.body(), JsonNode.class))); + /** + * Returns statistical information about the dataset. + * + *

A 404 throws {@link com.apify.client.http.NotFoundError} rather than resolving to empty: + * unlike {@link #get()}, there is no meaningful "statistics are absent" state distinct from "the + * dataset is gone", so a missing dataset is reported as a failure here too. + */ + public CompletableFuture getStatistics() { + return ctx.getResourceRequired("statistics", new QueryParams(), JsonNode.class); } /** @@ -227,8 +249,21 @@ public CompletableFuture> getStatistics() { */ public CompletableFuture createItemsPublicUrl( DatasetListItemsOptions options, Long expiresInSecs) { + return createItemsPublicUrl(options, expiresInSecs, null); + } + + /** + * As {@link #createItemsPublicUrl(DatasetListItemsOptions, Long)}, additionally setting the + * {@code format} the URL serves items in (e.g. {@code csv}, {@code xml}); {@code null} leaves it + * unset, which the API serves as {@code json}. + */ + public CompletableFuture createItemsPublicUrl( + DatasetListItemsOptions options, Long expiresInSecs, DownloadItemsFormat format) { QueryParams params = new QueryParams(); options.apply(params); + if (format != null) { + params.addString("format", format.wireValue()); + } return get() .thenApply( dataset -> { @@ -253,4 +288,15 @@ public CompletableFuture createItemsPublicUrl( private static long headerLong(ApiResponse resp, String name, long fallback) { return resp.headers().firstValueAsLong(name).orElse(fallback); } + + /** + * As {@link #headerLong}, but returns {@code null} (rather than a fallback value) when the header + * is absent, so callers can distinguish "not reported" from any particular number - notably + * {@code 0}, a value the header can legitimately carry (see {@link + * PaginationList#getScannedCount()}). + */ + private static Long headerLongOrNull(ApiResponse resp, String name) { + var value = resp.headers().firstValueAsLong(name); + return value.isPresent() ? value.getAsLong() : null; + } } diff --git a/src/main/java/com/apify/client/http/ApifyApiException.java b/src/main/java/com/apify/client/http/ApifyApiException.java index 0efafbd..b82fd9b 100644 --- a/src/main/java/com/apify/client/http/ApifyApiException.java +++ b/src/main/java/com/apify/client/http/ApifyApiException.java @@ -12,6 +12,14 @@ * #getStatusCode() status code}, the number of the final {@link #getAttempt() attempt}, and the * request {@link #getHttpMethod() method}/{@link #getPath() path}. * + *

The client throws the subclass matching the response's status code - {@link + * InvalidRequestError} (400), {@link UnauthorizedError} (401), {@link ForbiddenError} (403), {@link + * NotFoundError} (404), {@link ConflictError} (409), {@link RateLimitError} (429) or {@link + * ServerError} (5xx) - so a {@code catch} block can branch with {@code instanceof} instead of + * comparing {@link #getStatusCode()} by number. Any other status is this plain class. Every + * subclass extends this one, so an existing {@code catch (ApifyApiException e)} keeps working + * unchanged. + * *

It is an unchecked exception so callers are not forced to wrap every call; recover from it * with a normal {@code try}/{@code catch} where relevant. */ diff --git a/src/main/java/com/apify/client/http/ConflictError.java b/src/main/java/com/apify/client/http/ConflictError.java new file mode 100644 index 0000000..8a0a11f --- /dev/null +++ b/src/main/java/com/apify/client/http/ConflictError.java @@ -0,0 +1,23 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 409 Conflict} response: the request conflicts + * with the resource's current state (e.g. a duplicate name). + */ +public final class ConflictError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 409} API error response. */ + public ConflictError( + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(409, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/http/ForbiddenError.java b/src/main/java/com/apify/client/http/ForbiddenError.java new file mode 100644 index 0000000..9a5e1d0 --- /dev/null +++ b/src/main/java/com/apify/client/http/ForbiddenError.java @@ -0,0 +1,23 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 403 Forbidden} response: the API token is valid + * but lacks permission for the requested operation. + */ +public final class ForbiddenError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 403} API error response. */ + public ForbiddenError( + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(403, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/http/InvalidRequestError.java b/src/main/java/com/apify/client/http/InvalidRequestError.java new file mode 100644 index 0000000..c16721e --- /dev/null +++ b/src/main/java/com/apify/client/http/InvalidRequestError.java @@ -0,0 +1,23 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 400 Bad Request} response: the request itself was + * malformed or failed server-side validation. + */ +public final class InvalidRequestError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 400} API error response. */ + public InvalidRequestError( + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(400, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/http/NotFoundError.java b/src/main/java/com/apify/client/http/NotFoundError.java new file mode 100644 index 0000000..57ce8cf --- /dev/null +++ b/src/main/java/com/apify/client/http/NotFoundError.java @@ -0,0 +1,29 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 404 Not Found} response: the addressed resource + * does not exist (or the token cannot see it). + * + *

Most resource clients swallow this into an empty {@link java.util.Optional}/no-op when a + * resource is addressed by an explicit id (see each client's {@code get()}/{@code delete()} + * Javadoc); it reaches the caller as this exception everywhere the 404 is ambiguous, e.g. a client + * chained off a run or build without an id of its own ({@code run.dataset()}, {@code build.log()}, + * ...), where the missing resource may be the parent rather than the sub-resource. + */ +public final class NotFoundError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 404} API error response. */ + public NotFoundError( + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(404, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/http/RateLimitError.java b/src/main/java/com/apify/client/http/RateLimitError.java new file mode 100644 index 0000000..f3aa9a1 --- /dev/null +++ b/src/main/java/com/apify/client/http/RateLimitError.java @@ -0,0 +1,24 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 429 Too Many Requests} response. The client + * already retries this status internally (see {@code RetryConfig}); it reaches the caller only once + * every retry has been exhausted. + */ +public final class RateLimitError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 429} API error response. */ + public RateLimitError( + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(429, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/http/ServerError.java b/src/main/java/com/apify/client/http/ServerError.java new file mode 100644 index 0000000..8012f7c --- /dev/null +++ b/src/main/java/com/apify/client/http/ServerError.java @@ -0,0 +1,25 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 5xx} response: the API itself failed. The client + * already retries these statuses internally (see {@code RetryConfig}); it reaches the caller only + * once every retry has been exhausted. + */ +public final class ServerError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 5xx} API error response. */ + public ServerError( + int statusCode, + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(statusCode, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/http/UnauthorizedError.java b/src/main/java/com/apify/client/http/UnauthorizedError.java new file mode 100644 index 0000000..5a30d86 --- /dev/null +++ b/src/main/java/com/apify/client/http/UnauthorizedError.java @@ -0,0 +1,23 @@ +package com.apify.client.http; + +import java.util.Map; + +/** + * An {@link ApifyApiException} for an HTTP {@code 401 Unauthorized} response: no valid API token + * was supplied. + */ +public final class UnauthorizedError extends ApifyApiException { + + private static final long serialVersionUID = 1L; + + /** Constructs an exception describing a {@code 401} API error response. */ + public UnauthorizedError( + String type, + String message, + int attempt, + String httpMethod, + String path, + Map data) { + super(401, type, message, attempt, httpMethod, path, data); + } +} diff --git a/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java b/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java index 39ae2a0..45d435b 100644 --- a/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java +++ b/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java @@ -225,7 +225,23 @@ private void applyPage(PaginationList page) { buffer = page.getItems(); pos = 0; int count = buffer.size(); - offset += count; + // Advance the offset (and decide exhaustion) by the number of rows the API scanned, not the + // number of items it returned: an endpoint with item-level filtering (dataset items' clean / + // skipEmpty / skipHidden) can scan up to the requested page size while returning fewer rows, + // or none at all. Using the returned count here would re-scan (and re-yield) already-seen + // rows, or stop iteration early on a page that filtered out everything but still had more + // data ahead. + // + // getScannedCount() is null on every endpoint that does not report the header at all + // (everything but dataset items). It is also only trusted when it is at least the returned + // count: a scanned count can never be smaller than what it produced, so a smaller value (in + // practice, observed as a flat 0 on every page against the live API at the time of writing, + // even where real items were both scanned and returned) means the header is not actually + // populated yet rather than a real "nothing scanned" answer - falling back to the returned + // count there keeps this exactly as safe as before the header existed. + Long reported = page.getScannedCount(); + long scanned = reported != null && reported >= count ? reported : count; + offset += scanned; yielded += count; // Defensively trim the last page to the cap in case the server returned more than requested, // so the subscriber never sees more than `totalLimit` items. @@ -233,9 +249,11 @@ private void applyPage(PaginationList page) { buffer = buffer.subList(0, count - (int) (yielded - totalLimit)); yielded = totalLimit; } - // Stop on the cap or an empty page; never on a short page (the API clamps large page sizes) - // or the reported total (unreliable on some endpoints — see class doc). - if (count == 0 || (totalLimit != null && yielded >= totalLimit)) { + // Stop on the cap or a page that scanned nothing (the source is exhausted); never on a short + // *returned* page (the API clamps large page sizes, and filtering can legitimately return + // fewer items than were scanned) or the reported total (unreliable on some endpoints — see + // class doc). + if (scanned == 0 || (totalLimit != null && yielded >= totalLimit)) { exhausted = true; } } diff --git a/src/main/java/com/apify/client/internal/HttpClientCore.java b/src/main/java/com/apify/client/internal/HttpClientCore.java index 2483012..cd14bc9 100644 --- a/src/main/java/com/apify/client/internal/HttpClientCore.java +++ b/src/main/java/com/apify/client/internal/HttpClientCore.java @@ -5,8 +5,15 @@ import com.apify.client.http.ApiResponse; import com.apify.client.http.ApifyApiException; import com.apify.client.http.ApifyTransportException; +import com.apify.client.http.ConflictError; +import com.apify.client.http.ForbiddenError; import com.apify.client.http.HttpTransport; +import com.apify.client.http.InvalidRequestError; +import com.apify.client.http.NotFoundError; +import com.apify.client.http.RateLimitError; import com.apify.client.http.RetryConfig; +import com.apify.client.http.ServerError; +import com.apify.client.http.UnauthorizedError; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -16,7 +23,10 @@ import java.net.http.HttpResponse; import java.time.Duration; import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; @@ -176,7 +186,7 @@ public CompletableFuture call( // Compress the body once, up front, so every retry reuses the same encoded payload. byte[] requestBody = body; Map headers = extraHeaders; - if (shouldCompress(requestBody, extraHeaders)) { + if (shouldCompress(requestBody, contentType, extraHeaders)) { Compressed compressed = compress(requestBody, BROTLI_AVAILABLE); requestBody = compressed.body(); headers = new LinkedHashMap<>(extraHeaders == null ? Map.of() : extraHeaders); @@ -351,10 +361,12 @@ private Duration attemptTimeout(Duration base, int attempt) { /** * Reports whether a request body should be compressed: it must be present, at least {@link - * #MIN_COMPRESS_BYTES} bytes, and the caller must not have already set a {@code Content-Encoding} - * header (which would mean the body is pre-encoded). + * #MIN_COMPRESS_BYTES} bytes, the caller must not have already set a {@code Content-Encoding} + * header (which would mean the body is pre-encoded), and {@code contentType} must not already + * carry its own compression (see {@link #isCompressibleContentType}). */ - private static boolean shouldCompress(byte[] body, Map extraHeaders) { + private static boolean shouldCompress( + byte[] body, String contentType, Map extraHeaders) { if (body == null || body.length < MIN_COMPRESS_BYTES) { return false; } @@ -365,6 +377,97 @@ private static boolean shouldCompress(byte[] body, Map extraHead } } } + return isCompressibleContentType(contentType); + } + + /** Media type prefixes whose payloads already carry their own compression. */ + private static final List ALREADY_COMPRESSED_MEDIA_TYPE_PREFIXES = + List.of("audio/", "image/", "video/"); + + /** Exact media types whose payloads already carry their own compression. */ + private static final Set ALREADY_COMPRESSED_MEDIA_TYPES = + Set.of( + "application/epub+zip", + "application/gzip", + "application/java-archive", + "application/vnd.android.package-archive", + "application/vnd.openxmlformats-officedocument.presentationml.presentation", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "application/vnd.rar", + "application/x-7z-compressed", + "application/x-bzip", + "application/x-bzip2", + "application/x-gzip", + "application/x-rar-compressed", + "application/x-xz", + "application/x-zip-compressed", + "application/zip", + "application/zstd", + "font/woff", + "font/woff2"); + + /** Uncompressed media types that sit under an already-compressed prefix but are still raw. */ + private static final Set COMPRESSIBLE_MEDIA_TYPES = + Set.of( + "audio/aiff", + "audio/basic", + "audio/l16", + "audio/l24", + "audio/midi", + "audio/vnd.wave", + "audio/wav", + "audio/wave", + "audio/x-aiff", + "audio/x-wav", + "image/bmp", + "image/tiff", + "image/vnd.adobe.photoshop", + "image/vnd.microsoft.icon", + "image/x-icon", + "image/x-ms-bmp"); + + /** Structured syntax suffixes that mark a media type as text even under a compressed prefix. */ + private static final List COMPRESSIBLE_MEDIA_TYPE_SUFFIXES = List.of("+json", "+xml"); + + /** + * Reports whether a request body with the given {@code Content-Type} is worth compressing. + * Images, audio, video and archives already carry their own compression: running them through + * brotli or gzip burns CPU, holds a second full copy of the body in memory, and usually produces + * output slightly larger than the input. A content type that is raw despite such a media type + * (e.g. {@code image/bmp}, {@code audio/wav}) is still compressed, as is a structured-syntax type + * ending in {@code +json}/{@code +xml} (e.g. {@code image/svg+xml}). A missing content type is + * assumed compressible. Matches the reference JS client's {@code isCompressibleContentType}. + * + *

Public (like {@link #compress} and {@link #brotliAvailable}) so {@code CompressionTest}, + * outside this non-exported package, can exercise the classification directly. + */ + public static boolean isCompressibleContentType(String contentType) { + if (contentType == null || contentType.isEmpty()) { + return true; + } + // Content-Type is case-insensitive and may carry parameters (e.g. "text/plain; charset=utf-8"). + int semicolon = contentType.indexOf(';'); + String mediaType = + (semicolon >= 0 ? contentType.substring(0, semicolon) : contentType) + .trim() + .toLowerCase(Locale.ROOT); + if (COMPRESSIBLE_MEDIA_TYPES.contains(mediaType)) { + return true; + } + for (String suffix : COMPRESSIBLE_MEDIA_TYPE_SUFFIXES) { + if (mediaType.endsWith(suffix)) { + return true; + } + } + if (ALREADY_COMPRESSED_MEDIA_TYPES.contains(mediaType)) { + return false; + } + for (String prefix : ALREADY_COMPRESSED_MEDIA_TYPE_PREFIXES) { + if (mediaType.startsWith(prefix)) { + return false; + } + } return true; } @@ -504,7 +607,34 @@ public static ApifyApiException buildApiError( ? "unexpected error with status " + status : "unexpected error: " + new String(body, java.nio.charset.StandardCharsets.UTF_8); } - return new ApifyApiException(status, type, message, attempt, method, path, data); + return newApiException(status, type, message, attempt, method, path, data); + } + + /** + * Builds the {@link ApifyApiException} subclass matching {@code status}, so callers can branch + * with {@code instanceof} on the specific error (see the class Javadoc). A status with no + * dedicated subclass falls back to the plain base class. + */ + private static ApifyApiException newApiException( + int status, + String type, + String message, + int attempt, + String method, + String path, + Map data) { + return switch (status) { + case 400 -> new InvalidRequestError(type, message, attempt, method, path, data); + case 401 -> new UnauthorizedError(type, message, attempt, method, path, data); + case 403 -> new ForbiddenError(type, message, attempt, method, path, data); + case 404 -> new NotFoundError(type, message, attempt, method, path, data); + case 409 -> new ConflictError(type, message, attempt, method, path, data); + case RATE_LIMIT_EXCEEDED -> new RateLimitError(type, message, attempt, method, path, data); + default -> + status >= MIN_SERVER_ERROR + ? new ServerError(status, type, message, attempt, method, path, data) + : new ApifyApiException(status, type, message, attempt, method, path, data); + }; } /** Returns the path+query portion of a URL, for error reporting. */ diff --git a/src/main/java/com/apify/client/internal/ResourceContext.java b/src/main/java/com/apify/client/internal/ResourceContext.java index 9b521ff..b49d4ad 100644 --- a/src/main/java/com/apify/client/internal/ResourceContext.java +++ b/src/main/java/com/apify/client/internal/ResourceContext.java @@ -75,6 +75,17 @@ public final class ResourceContext { /** Origin used to build public, shareable URLs (defaults to {@link #apiOrigin}). */ final String publicOrigin; + /** + * Whether this context addresses its resource by an explicit id of its own ({@link #single}), as + * opposed to a nested path with no id ({@link #collection}/{@link #nestedCollection}, e.g. a + * run's {@code .../dataset}). A 404 is unambiguous for the former (that id does not exist) but + * not for the latter (either the parent or the sub-resource could be gone), which is why {@link + * #getResourceUnlessAmbiguous} and {@link #deleteResourceUnlessAmbiguous} key their behavior on + * this flag - see those methods and each affected resource client's {@code get()}/{@code + * delete()} Javadoc. + */ + private final boolean ownId; + /** * Immutable: every field is set once here. {@link #withPublicOrigin} and {@link #seedParams} * return a new instance rather than mutating this one, so a {@code ResourceContext} (and the @@ -85,22 +96,24 @@ private ResourceContext( String url, QueryParams baseParams, String apiOrigin, - String publicOrigin) { + String publicOrigin, + boolean ownId) { this.http = http; this.url = url; this.baseParams = baseParams; this.apiOrigin = apiOrigin; this.publicOrigin = publicOrigin; + this.ownId = ownId; } - private ResourceContext(HttpClientCore http, String url, String baseUrl) { - this(http, url, new QueryParams(), originOf(baseUrl), originOf(baseUrl)); + private ResourceContext(HttpClientCore http, String url, String baseUrl, boolean ownId) { + this(http, url, new QueryParams(), originOf(baseUrl), originOf(baseUrl), ownId); } /** Creates a context for a collection endpoint: {@code {base}/{resourcePath}}. */ public static ResourceContext collection( HttpClientCore http, String baseUrl, String resourcePath) { - return new ResourceContext(http, baseUrl + "/" + resourcePath, baseUrl); + return new ResourceContext(http, baseUrl + "/" + resourcePath, baseUrl, false); } /** @@ -117,12 +130,13 @@ public static ResourceContext nestedCollection( /** Creates a context for a single resource: {@code {base}/{resourcePath}/{safeId}}. */ public static ResourceContext single( HttpClientCore http, String baseUrl, String resourcePath, String id) { - return new ResourceContext(http, baseUrl + "/" + resourcePath + "/" + toSafeId(id), baseUrl); + return new ResourceContext( + http, baseUrl + "/" + resourcePath + "/" + toSafeId(id), baseUrl, true); } /** A copy of this context with the origin used to build public URLs overridden. */ public ResourceContext withPublicOrigin(String publicBaseUrl) { - return new ResourceContext(http, url, baseParams, apiOrigin, originOf(publicBaseUrl)); + return new ResourceContext(http, url, baseParams, apiOrigin, originOf(publicBaseUrl), ownId); } /** This resource's URL with an optional extra path segment appended. */ @@ -160,7 +174,7 @@ public ResourceContext seedParams(QueryParams inherited) { return this; } return new ResourceContext( - http, url, baseParams.copy().extend(inherited), apiOrigin, publicOrigin); + http, url, baseParams.copy().extend(inherited), apiOrigin, publicOrigin, ownId); } // ---- CRUD primitives ------------------------------------------------------ @@ -203,6 +217,27 @@ public CompletableFuture getResourceRequired( return getResourceRequired(subPath, params, Json.type(dataClass)); } + /** + * Like {@link #getResource}, but a 404 is only swallowed to {@link Optional#empty()} when this + * context {@link #ownId addresses its resource by an explicit id}. Otherwise (a nested path with + * no id of its own, e.g. a run's default dataset) the 404 is ambiguous - it may mean the parent + * is gone rather than the sub-resource - so it is rethrown as an {@link ApifyApiException} + * ({@link com.apify.client.http.NotFoundError}) instead of being hidden behind an empty result. + * Used by every storage/log resource client's {@code get()}. + */ + public CompletableFuture> getResourceUnlessAmbiguous( + String subPath, QueryParams params, JavaType dataType) { + if (ownId) { + return getResource(subPath, params, dataType); + } + return this.getResourceRequired(subPath, params, dataType).thenApply(Optional::ofNullable); + } + + public CompletableFuture> getResourceUnlessAmbiguous( + String subPath, QueryParams params, Class dataClass) { + return getResourceUnlessAmbiguous(subPath, params, Json.type(dataClass)); + } + public CompletableFuture updateResource(String subPath, Object body, Class dataClass) { String u = mergedParams(new QueryParams()).applyToUrl(subUrl(subPath)); return http.call("PUT", u, Json.toBytes(body), CONTENT_TYPE_JSON, http.baseRequestTimeout()) @@ -226,6 +261,19 @@ public CompletableFuture deleteResource(String subPath) { }); } + /** + * Like {@link #deleteResource}, but - mirroring {@link #getResourceUnlessAmbiguous} - a 404 is + * only treated as a successful no-op when this context addresses its resource by an explicit id. + * Otherwise it is rethrown, since it may mean the parent (not the sub-resource) is gone. + */ + public CompletableFuture deleteResourceUnlessAmbiguous(String subPath) { + if (ownId) { + return deleteResource(subPath); + } + String u = mergedParams(new QueryParams()).applyToUrl(subUrl(subPath)); + return http.call("DELETE", u, null, "", http.baseRequestTimeout()).thenApply(resp -> null); + } + public CompletableFuture> listResource( String subPath, QueryParams params, Class itemClass) { JavaType listType = Json.parametric(PaginationList.class, Json.type(itemClass)); @@ -374,6 +422,22 @@ public CompletableFuture getRaw(String subPath, QueryParams params) }); } + /** + * As {@link #getRaw}, but a 404 is not swallowed - it propagates as an {@link ApifyApiException}. + */ + public CompletableFuture getRawRequired(String subPath, QueryParams params) { + String u = mergedParams(params).applyToUrl(subUrl(subPath)); + return http.call("GET", u, null, "", http.baseRequestTimeout()); + } + + /** + * Like {@link #getRaw}, but - mirroring {@link #getResourceUnlessAmbiguous} - a 404 is only + * swallowed to {@code null} when this context addresses its resource by an explicit id. + */ + public CompletableFuture getRawUnlessAmbiguous(String subPath, QueryParams params) { + return ownId ? getRaw(subPath, params) : getRawRequired(subPath, params); + } + /** HEAD request; completes with whether the resource exists. */ public CompletableFuture headExists(String subPath, QueryParams params) { String u = mergedParams(params).applyToUrl(subUrl(subPath)); @@ -548,7 +612,9 @@ private static RuntimeException asRuntimeException(Throwable cause) { /** * Encodes a resource id so it is safe to embed in a URL path. Apify uses the {@code - * username~resourcename} form, so the first {@code /} of an id is replaced with {@code ~}. + * username~resourcename} form, so every {@code /} in an id is replaced with {@code ~} (not just + * the first: an id with more than one slash must not leave any literal {@code /} that could + * restructure the request path). * *

Public (not just intra-package) because {@code RunClient#metamorph} in the {@code .run} * package needs to apply the same normalization to a resource id supplied as an argument value @@ -558,15 +624,25 @@ private static RuntimeException asRuntimeException(Throwable cause) { * this class's other narrowly-public members). */ public static String toSafeId(String id) { - int slash = id.indexOf('/'); - return slash < 0 ? id : id.substring(0, slash) + "~" + id.substring(slash + 1); + return id.replace('/', '~'); } /** * Percent-encodes a single URL path segment, so that values interpolated into the path (record * keys, request IDs) cannot break out of the segment. + * + *

Rejects an empty string or a bare {@code "."}/{@code ".."} segment instead of encoding it: a + * URL parser (client-side, or a proxy/server in front of the API) resolves dot segments + * after percent-decoding, so an encoded {@code "%2E%2E"} would still collapse against + * its neighbours exactly like a literal {@code ".."} once decoded. Rejecting it outright - rather + * than silently encoding something that cannot safely address any record/request - mirrors the + * reference JS client's path-segment sanitization. */ public static String encodePathSegment(String input) { + if (input == null || input.isEmpty() || ".".equals(input) || "..".equals(input)) { + throw new IllegalArgumentException( + "a URL path segment must not be empty or a dot segment (\".\" or \"..\"), was: " + input); + } return URLEncoder.encode(input, StandardCharsets.UTF_8).replace("+", "%20"); } diff --git a/src/main/java/com/apify/client/keyvalue/KeyValueStoreClient.java b/src/main/java/com/apify/client/keyvalue/KeyValueStoreClient.java index fb651f6..0084f3c 100644 --- a/src/main/java/com/apify/client/keyvalue/KeyValueStoreClient.java +++ b/src/main/java/com/apify/client/keyvalue/KeyValueStoreClient.java @@ -51,9 +51,15 @@ public static KeyValueStoreClient nested( ResourceContext.nestedCollection(http, base, subPath, inherited)); } - /** Fetches the store metadata, or empty if it does not exist. */ + /** + * Fetches the store metadata, or empty if it does not exist. + * + *

On a client reached through a run/task without an explicit store id (e.g. {@code + * run.keyValueStore()}), a 404 is ambiguous between "the run is gone" and "the run has no store", + * so it throws {@link com.apify.client.http.NotFoundError} instead of resolving to empty. + */ public CompletableFuture> get() { - return ctx.getResource("", new QueryParams(), KeyValueStore.class); + return ctx.getResourceUnlessAmbiguous("", new QueryParams(), KeyValueStore.class); } /** Updates the store metadata (e.g. name) and returns the updated object. */ @@ -61,9 +67,12 @@ public CompletableFuture update(Object newFields) { return ctx.updateResource("", newFields, KeyValueStore.class); } - /** Deletes the store. */ + /** + * Deletes the store. See {@link #get()} for why a client without an explicit store id throws on a + * 404 instead of treating it as a no-op. + */ public CompletableFuture delete() { - return ctx.deleteResource(""); + return ctx.deleteResourceUnlessAmbiguous(""); } /** Lists the keys stored in this key-value store. */ diff --git a/src/main/java/com/apify/client/log/LogClient.java b/src/main/java/com/apify/client/log/LogClient.java index 45b28d8..d7bcced 100644 --- a/src/main/java/com/apify/client/log/LogClient.java +++ b/src/main/java/com/apify/client/log/LogClient.java @@ -35,16 +35,22 @@ public static LogClient nested(HttpClientCore http, String base, QueryParams inh return new LogClient(ResourceContext.nestedCollection(http, base, "log", inherited)); } - /** Fetches the entire log as text, or empty if the log does not exist. */ + /** + * Fetches the entire log as text, or empty if the log does not exist. + * + *

On a client reached through a run/build without an explicit log id (e.g. {@code run.log()}, + * {@code build.log()}), a 404 is ambiguous between "the run/build is gone" and "it has no log + * yet", so it throws {@link com.apify.client.http.NotFoundError} instead of resolving to empty. + */ public CompletableFuture> get() { return get(new LogOptions()); } - /** Fetches the log with explicit options (raw, download). */ + /** Fetches the log with explicit options (raw, download). See {@link #get()} for 404 handling. */ public CompletableFuture> get(LogOptions options) { QueryParams params = new QueryParams(); options.apply(params); - return ctx.getRaw("", params) + return ctx.getRawUnlessAmbiguous("", params) .thenApply( resp -> resp == null diff --git a/src/main/java/com/apify/client/requestqueue/RequestQueueClient.java b/src/main/java/com/apify/client/requestqueue/RequestQueueClient.java index dd46483..2363e01 100644 --- a/src/main/java/com/apify/client/requestqueue/RequestQueueClient.java +++ b/src/main/java/com/apify/client/requestqueue/RequestQueueClient.java @@ -94,9 +94,15 @@ private QueryParams withClientKey(QueryParams params) { return params; } - /** Fetches the queue metadata, or empty if it does not exist. */ + /** + * Fetches the queue metadata, or empty if it does not exist. + * + *

On a client reached through a run/task without an explicit queue id (e.g. {@code + * run.requestQueue()}), a 404 is ambiguous between "the run is gone" and "the run has no queue", + * so it throws {@link com.apify.client.http.NotFoundError} instead of resolving to empty. + */ public CompletableFuture> get() { - return ctx.getResource("", new QueryParams(), RequestQueue.class); + return ctx.getResourceUnlessAmbiguous("", new QueryParams(), RequestQueue.class); } /** Updates the queue metadata (e.g. name) and returns the updated object. */ @@ -104,9 +110,12 @@ public CompletableFuture update(Object newFields) { return ctx.updateResource("", newFields, RequestQueue.class); } - /** Deletes the queue. */ + /** + * Deletes the queue. See {@link #get()} for why a client without an explicit queue id throws on a + * 404 instead of treating it as a no-op. + */ public CompletableFuture delete() { - return ctx.deleteResource(""); + return ctx.deleteResourceUnlessAmbiguous(""); } /** diff --git a/src/main/java/com/apify/client/schedule/ScheduleClient.java b/src/main/java/com/apify/client/schedule/ScheduleClient.java index 81a81c8..8c1d06b 100644 --- a/src/main/java/com/apify/client/schedule/ScheduleClient.java +++ b/src/main/java/com/apify/client/schedule/ScheduleClient.java @@ -2,11 +2,13 @@ import com.apify.client.internal.ApiPaths; import com.apify.client.internal.HttpClientCore; +import com.apify.client.internal.Json; import com.apify.client.internal.QueryParams; import com.apify.client.internal.ResourceContext; -import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import tools.jackson.databind.JavaType; /** A client for a specific schedule ({@code /v2/schedules/{scheduleId}}). */ public final class ScheduleClient { @@ -31,13 +33,15 @@ public CompletableFuture delete() { return ctx.deleteResource(""); } - /** Fetches the schedule's invocation log as text, or empty if absent. */ - public CompletableFuture> getLog() { - return ctx.getRaw("log", new QueryParams()) - .thenApply( - resp -> - resp == null - ? Optional.empty() - : Optional.of(new String(resp.body(), StandardCharsets.UTF_8))); + /** + * Fetches up to the last 1000 entries of the schedule's invocation log. + * + *

A 404 (the schedule itself no longer exists) throws {@link + * com.apify.client.http.NotFoundError} rather than resolving to an empty list, since an empty + * list would otherwise be indistinguishable from "no invocations yet". + */ + public CompletableFuture> getLog() { + JavaType listType = Json.parametric(List.class, Json.type(ScheduleInvoked.class)); + return ctx.getResourceRequired("log", new QueryParams(), listType); } } diff --git a/src/main/java/com/apify/client/schedule/ScheduleInvoked.java b/src/main/java/com/apify/client/schedule/ScheduleInvoked.java new file mode 100644 index 0000000..5999668 --- /dev/null +++ b/src/main/java/com/apify/client/schedule/ScheduleInvoked.java @@ -0,0 +1,28 @@ +package com.apify.client.schedule; + +import com.apify.client.ApifyResource; +import java.time.Instant; + +/** + * A single entry of a schedule's invocation log, as returned by {@link ScheduleClient#getLog()}. + */ +public final class ScheduleInvoked extends ApifyResource { + private String message; + private String level; + private Instant createdAt; + + /** The human-readable log message. */ + public String getMessage() { + return message; + } + + /** The log level (e.g. {@code "INFO"}, {@code "ERROR"}). */ + public String getLevel() { + return level; + } + + /** When this entry was recorded. */ + public Instant getCreatedAt() { + return createdAt; + } +} diff --git a/src/main/java/com/apify/client/task/TaskClient.java b/src/main/java/com/apify/client/task/TaskClient.java index 2cf11fb..8e05bdf 100644 --- a/src/main/java/com/apify/client/task/TaskClient.java +++ b/src/main/java/com/apify/client/task/TaskClient.java @@ -118,14 +118,16 @@ public CompletableFuture call(Object input, TaskCallOptions options, L waitSecs); } - /** Fetches the task's stored input, or empty if none is set. */ - public CompletableFuture> getInput() { - return ctx.getRaw("input", new QueryParams()) - .thenApply( - resp -> - resp == null - ? Optional.empty() - : Optional.of(Json.parse(resp.body(), JsonNode.class))); + /** + * Fetches the task's stored input. + * + *

A 404 throws {@link com.apify.client.http.NotFoundError} rather than resolving to empty: + * unlike {@link #get()}, there is no meaningful "input is absent" state distinct from "the task + * is gone", so a missing task is reported as a failure here too. + */ + public CompletableFuture getInput() { + return ctx.getRawRequired("input", new QueryParams()) + .thenApply(resp -> Json.parse(resp.body(), JsonNode.class)); } /** Replaces the task's stored input and returns the updated input. */ diff --git a/src/test/java/com/apify/client/AmbiguousNotFoundTest.java b/src/test/java/com/apify/client/AmbiguousNotFoundTest.java new file mode 100644 index 0000000..1fcaee9 --- /dev/null +++ b/src/test/java/com/apify/client/AmbiguousNotFoundTest.java @@ -0,0 +1,117 @@ +package com.apify.client; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.apify.client.http.NotFoundError; +import java.time.Duration; +import java.util.concurrent.CompletionException; +import org.junit.jupiter.api.Test; + +/** + * Hermetic tests for the ambiguous-404 rule (mirrors apify-client-js#1042): a resource + * client addressed by an explicit id swallows a 404 to an empty {@code Optional}/no-op, but a + * client reached through a run/build with no id of its own throws {@link NotFoundError} instead, + * since the 404 could mean either the parent or the sub-resource is gone. + */ +class AmbiguousNotFoundTest { + + private static ApifyClient client(int status) { + return ApifyClient.builder() + .token("test-token") + .httpTransport(MockTransport.ofConstant(status, notFoundBody())) + .maxRetries(0) + .minDelayBetweenRetries(Duration.ofMillis(1)) + .build(); + } + + private static String notFoundBody() { + return "{\"error\":{\"type\":\"record-not-found\",\"message\":\"not found\"}}"; + } + + // ---- Addressed by an explicit id: swallow to empty/no-op ------------------------------- + + @Test + void datasetByIdSwallows404ToEmpty() { + assertTrue(client(404).dataset("missing").get().join().isEmpty()); + } + + @Test + void keyValueStoreByIdSwallows404ToEmpty() { + assertTrue(client(404).keyValueStore("missing").get().join().isEmpty()); + } + + @Test + void requestQueueByIdSwallows404ToEmpty() { + assertTrue(client(404).requestQueue("missing").get().join().isEmpty()); + } + + @Test + void datasetByIdDeleteIsNoOpOn404() { + client(404).dataset("missing").delete().join(); // must not throw + } + + @Test + void logByIdSwallows404ToEmpty() { + assertTrue(client(404).log("missing").get().join().isEmpty()); + } + + // ---- Reached through a run/build with no id of its own: throw ------------------------- + + @Test + void runDatasetThrowsNotFoundError() { + CompletionException e = + assertThrows(CompletionException.class, () -> client(404).run("r1").dataset().get().join()); + assertInstanceOf(NotFoundError.class, e.getCause()); + } + + @Test + void runKeyValueStoreThrowsNotFoundError() { + CompletionException e = + assertThrows( + CompletionException.class, () -> client(404).run("r1").keyValueStore().get().join()); + assertInstanceOf(NotFoundError.class, e.getCause()); + } + + @Test + void runRequestQueueThrowsNotFoundError() { + CompletionException e = + assertThrows( + CompletionException.class, () -> client(404).run("r1").requestQueue().get().join()); + assertInstanceOf(NotFoundError.class, e.getCause()); + } + + @Test + void runDatasetDeleteThrowsNotFoundError() { + CompletionException e = + assertThrows( + CompletionException.class, () -> client(404).run("r1").dataset().delete().join()); + assertInstanceOf(NotFoundError.class, e.getCause()); + } + + @Test + void runLogThrowsNotFoundError() { + CompletionException e = + assertThrows(CompletionException.class, () -> client(404).run("r1").log().get().join()); + assertInstanceOf(NotFoundError.class, e.getCause()); + } + + @Test + void buildLogThrowsNotFoundError() { + CompletionException e = + assertThrows(CompletionException.class, () -> client(404).build("b1").log().get().join()); + assertInstanceOf(NotFoundError.class, e.getCause()); + } + + // ---- Non-404 errors must still propagate from the id-addressed ("swallow") path too ---- + + @Test + void datasetByIdStillThrowsOnNon404Error() { + CompletionException e = + assertThrows(CompletionException.class, () -> client(500).dataset("d1").get().join()); + assertFalse(e.getCause() instanceof NotFoundError); + } +} diff --git a/src/test/java/com/apify/client/BaseUrlNormalizationTest.java b/src/test/java/com/apify/client/BaseUrlNormalizationTest.java new file mode 100644 index 0000000..30643a3 --- /dev/null +++ b/src/test/java/com/apify/client/BaseUrlNormalizationTest.java @@ -0,0 +1,56 @@ +package com.apify.client; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +/** + * Offline tests for {@code ApifyClientBuilder}'s {@code baseUrl}/{@code publicBaseUrl} + * normalization: trailing slashes are stripped and {@code /v2} is appended, unless the URL's path + * already ends with it. Mirrors the reference JS client's table-driven regression test for the same + * bug (a caller-supplied {@code .../v2} used to become {@code .../v2/v2}). + */ +class BaseUrlNormalizationTest { + + @ParameterizedTest + @CsvSource({ + "https://example.com, https://example.com/v2", + "https://example.com/, https://example.com/v2", + "https://example.com/v2, https://example.com/v2", + "https://example.com/v2/, https://example.com/v2", + "https://example.com/v2//, https://example.com/v2", + "https://example.com/proxy/v2, https://example.com/proxy/v2", + "https://example.com/apiv2, https://example.com/apiv2/v2", + // A bare "v2" host, not a "/v2" path: must still gain the version path, even though the + // scheme's own "//" happens to precede a host spelled "v2" (a naive string-suffix check on the + // whole URL would be fooled by this). + "https://v2, https://v2/v2", + }) + void normalizesBaseUrl(String input, String expected) { + ApifyClient client = ApifyClient.builder().token("t").baseUrl(input).build(); + assertEquals(expected, client.getApiBaseUrl()); + } + + @ParameterizedTest + @CsvSource({ + "https://example.com, https://example.com/v2", + "https://example.com/v2, https://example.com/v2", + }) + void normalizesPublicBaseUrlIndependently(String input, String expected) { + ApifyClient client = + ApifyClient.builder() + .token("t") + .baseUrl("https://api.apify.com") + .publicBaseUrl(input) + .build(); + assertEquals(expected, client.getPublicApiBaseUrl()); + } + + @Test + void defaultBaseUrlIsVersioned() { + ApifyClient client = ApifyClient.builder().token("t").build(); + assertEquals("https://api.apify.com/v2", client.getApiBaseUrl()); + } +} diff --git a/src/test/java/com/apify/client/ClientBehaviourRegressionTest.java b/src/test/java/com/apify/client/ClientBehaviourRegressionTest.java index ad6dde6..0ffaef7 100644 --- a/src/test/java/com/apify/client/ClientBehaviourRegressionTest.java +++ b/src/test/java/com/apify/client/ClientBehaviourRegressionTest.java @@ -13,6 +13,7 @@ import com.apify.client.dataset.DownloadItemsFormat; import com.apify.client.http.ApifyApiException; import com.apify.client.http.ApifyTransportException; +import com.apify.client.http.NotFoundError; import com.apify.client.keyvalue.KeyValueStore; import com.apify.client.keyvalue.KeyValueStoreRecord; import com.apify.client.keyvalue.ListKeysOptions; @@ -25,6 +26,7 @@ import com.apify.client.run.MetamorphOptions; import com.apify.client.run.RunChargeOptions; import com.apify.client.run.SetStatusMessageOptions; +import com.apify.client.schedule.ScheduleInvoked; import com.apify.client.store.ActorStoreListItem; import com.apify.client.store.StoreListOptions; import com.apify.client.webhook.NestedWebhookCollectionClient; @@ -34,6 +36,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.CompletionException; import java.util.concurrent.Flow; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -754,4 +757,81 @@ void versionsIterateHonorsTotalLimitCap() { List yielded = items.stream().map(ActorVersion::getVersionNumber).toList(); assertEquals(List.of("0.1", "0.2"), yielded, "limit caps the number yielded"); } + + @Test + void scheduleGetLogDecodesEnvelopeArray() { + MockTransport backend = + MockTransport.ofConstant( + 200, + "{\"data\":[" + + "{\"message\":\"Schedule invoked\",\"level\":\"INFO\",\"createdAt\":\"2019-03-26T12:28:00.370Z\"}," + + "{\"message\":\"boom\",\"level\":\"ERROR\",\"createdAt\":\"2019-03-26T12:30:00.325Z\"}]}"); + List log = client(backend).schedule("s1").getLog().join(); + assertEquals(2, log.size()); + assertEquals("Schedule invoked", log.get(0).getMessage()); + assertEquals("INFO", log.get(0).getLevel()); + assertTrue(backend.lastUrl.contains("schedules/s1/log"), backend.lastUrl); + } + + @Test + void scheduleGetLogThrowsNotFoundOn404() { + MockTransport backend = + MockTransport.ofConstant( + 404, "{\"error\":{\"type\":\"record-not-found\",\"message\":\"not found\"}}"); + assertThrows( + CompletionException.class, () -> client(backend).schedule("missing").getLog().join()); + } + + @Test + void datasetGetStatisticsThrowsNotFoundOn404InsteadOfEmpty() { + MockTransport backend = + MockTransport.ofConstant( + 404, "{\"error\":{\"type\":\"record-not-found\",\"message\":\"not found\"}}"); + CompletionException e = + assertThrows( + CompletionException.class, + () -> client(backend).dataset("missing").getStatistics().join()); + assertTrue(e.getCause() instanceof NotFoundError); + } + + @Test + void taskGetInputThrowsNotFoundOn404() { + MockTransport backend = + MockTransport.ofConstant( + 404, "{\"error\":{\"type\":\"record-not-found\",\"message\":\"not found\"}}"); + CompletionException e = + assertThrows( + CompletionException.class, () -> client(backend).task("missing").getInput().join()); + assertTrue(e.getCause() instanceof NotFoundError); + } + + @Test + void taskGetInputReturnsRawJsonOnSuccess() { + MockTransport backend = MockTransport.ofConstant(200, "{\"foo\":\"bar\"}"); + var input = client(backend).task("t1").getInput().join(); + assertEquals("bar", input.get("foo").asString()); + assertTrue(backend.lastUrl.contains("actor-tasks/t1/input"), backend.lastUrl); + } + + @Test + void createItemsPublicUrlWithFormatSetsQueryParam() { + MockTransport backend = MockTransport.ofConstant(200, "{\"data\":{\"id\":\"d1\"}}"); + String url = + client(backend) + .dataset("d1") + .createItemsPublicUrl(new DatasetListItemsOptions(), null, DownloadItemsFormat.CSV) + .join(); + assertTrue(url.contains("format=csv"), url); + } + + @Test + void createItemsPublicUrlWithoutFormatOmitsQueryParam() { + MockTransport backend = MockTransport.ofConstant(200, "{\"data\":{\"id\":\"d1\"}}"); + String url = + client(backend) + .dataset("d1") + .createItemsPublicUrl(new DatasetListItemsOptions(), null) + .join(); + assertFalse(url.contains("format="), url); + } } diff --git a/src/test/java/com/apify/client/CompressionTest.java b/src/test/java/com/apify/client/CompressionTest.java index 1d2fa01..4392dd2 100644 --- a/src/test/java/com/apify/client/CompressionTest.java +++ b/src/test/java/com/apify/client/CompressionTest.java @@ -149,6 +149,65 @@ void smallBodyIsNotCompressed() { assertArrayEquals(payload, backend.lastBodyBytes, "small body must be sent verbatim"); } + // --- Content-type classification: skip compressing payloads that are already compressed. --- + + @Test + void isCompressibleContentTypeClassifiesKnownMediaTypes() { + // Missing/empty/text content types are compressible. + assertTrue(HttpClientCore.isCompressibleContentType(null)); + assertTrue(HttpClientCore.isCompressibleContentType("")); + assertTrue(HttpClientCore.isCompressibleContentType("application/json")); + assertTrue(HttpClientCore.isCompressibleContentType("text/plain; charset=utf-8")); + assertTrue(HttpClientCore.isCompressibleContentType("application/octet-stream")); + // A structured-syntax suffix overrides an already-compressed prefix. + assertTrue(HttpClientCore.isCompressibleContentType("image/svg+xml")); + assertTrue(HttpClientCore.isCompressibleContentType("IMAGE/SVG+XML; charset=utf-8")); + // A raw format under an already-compressed prefix is still compressible. + assertTrue(HttpClientCore.isCompressibleContentType("image/bmp")); + assertTrue(HttpClientCore.isCompressibleContentType("audio/wav")); + + // Media/archive prefixes and exact types already carry their own compression. + assertFalse(HttpClientCore.isCompressibleContentType("image/png")); + assertFalse(HttpClientCore.isCompressibleContentType("video/mp4")); + assertFalse(HttpClientCore.isCompressibleContentType("audio/mpeg")); + assertFalse(HttpClientCore.isCompressibleContentType("application/zip")); + assertFalse(HttpClientCore.isCompressibleContentType("application/x-gzip")); + assertFalse(HttpClientCore.isCompressibleContentType("font/woff2")); + assertFalse( + HttpClientCore.isCompressibleContentType( + "application/vnd.openxmlformats-officedocument.wordprocessingml.document")); + // Case-insensitive and whitespace-tolerant. + assertFalse(HttpClientCore.isCompressibleContentType(" Image/PNG ")); + } + + @Test + void alreadyCompressedContentTypeSkipsCompression() { + MockTransport backend = MockTransport.ofConstant(201, ""); + byte[] payload = payload(4096, (byte) 'a'); // trivially compressible, well above the threshold + + client(backend).keyValueStore(STORE_ID).setRecord(RECORD_KEY, payload, "image/png").join(); + + assertFalse( + backend.lastHeaders.firstValue("Content-Encoding").isPresent(), + "an already-compressed media type must not be run through the compressor"); + assertArrayEquals(payload, backend.lastBodyBytes, "body must be sent verbatim, uncompressed"); + } + + @Test + void rawFormatUnderCompressedPrefixIsStillCompressed() throws IOException { + MockTransport backend = MockTransport.ofConstant(201, ""); + byte[] payload = payload(4096, (byte) 'a'); + + // image/bmp sits under the "image/" prefix but is raw, so it should still be compressed. + client(backend).keyValueStore(STORE_ID).setRecord(RECORD_KEY, payload, "image/bmp").join(); + + boolean brotli = HttpClientCore.brotliAvailable(); + assertEquals( + brotli ? "br" : "gzip", backend.lastHeaders.firstValue("Content-Encoding").orElse(null)); + byte[] recovered = brotli ? unbrotli(backend.lastBodyBytes) : gunzip(backend.lastBodyBytes); + assertArrayEquals(payload, recovered); + } + @Test void bodyExactlyAtThresholdIsCompressed() throws IOException { MockTransport backend = MockTransport.ofConstant(201, ""); diff --git a/src/test/java/com/apify/client/DatasetItemsIteratorTest.java b/src/test/java/com/apify/client/DatasetItemsIteratorTest.java index 76ba1ac..89438d3 100644 --- a/src/test/java/com/apify/client/DatasetItemsIteratorTest.java +++ b/src/test/java/com/apify/client/DatasetItemsIteratorTest.java @@ -7,6 +7,7 @@ import com.apify.client.dataset.DatasetListItemsOptions; import java.time.Duration; import java.util.List; +import java.util.Map; import java.util.concurrent.Flow; import org.junit.jupiter.api.Test; import tools.jackson.databind.JsonNode; @@ -90,6 +91,94 @@ void totalCapTrimsDatasetItems() { /** A typed item for {@link #decodesIntoRequestedType()}. */ public record Item(int n) {} + @Test + void listItemsExposesScannedCountFromHeader() { + MockTransport backend = + new MockTransport( + List.of(MockTransport.ok(200, "[{\"n\":1}]", Map.of("X-Apify-Pagination-Count", "5")))); + PaginationList page = + client(backend).dataset("d1").listItems(new DatasetListItemsOptions()).join(); + assertEquals(1, page.getCount(), "count stays the number of items actually returned"); + assertEquals(5L, page.getScannedCount(), "scannedCount comes from X-Apify-Pagination-Count"); + } + + @Test + void listItemsScannedCountNullWhenHeaderAbsent() { + MockTransport backend = MockTransport.ofConstant(200, "[{\"n\":1}]"); + PaginationList page = + client(backend).dataset("d1").listItems(new DatasetListItemsOptions()).join(); + assertEquals(null, page.getScannedCount(), "no header -> no scanned count reported"); + } + + @Test + void advancesByScannedCountNotReturnedCount() { + // Regression for the fully-filtered-page bug: a page can scan `limit` rows (reported via + // X-Apify-Pagination-Count) while a server-side filter (clean/skipEmpty/skipHidden) drops all + // of them. Advancing by the *returned* item count (0) would re-request the same offset forever + // (or, with the old "stop on empty page" rule, end iteration early and silently skip the + // remaining items). Advancing by the *scanned* count must skip past the filtered-out window and + // reach the real data on the next page. + MockTransport backend = + new MockTransport( + List.of( + // Page 1: scanned 2 rows, server-side filter dropped both. + MockTransport.ok(200, "[]", Map.of("X-Apify-Pagination-Count", "2")), + // Page 2: scanned 2 rows, 1 survived the filter. + MockTransport.ok(200, "[{\"n\":3}]", Map.of("X-Apify-Pagination-Count", "2")), + // Page 3: nothing left to scan - iteration ends. + MockTransport.ok(200, "[]", Map.of("X-Apify-Pagination-Count", "0")))); + List seen = + collect( + client(backend) + .dataset("d1") + .iterateItems(new DatasetListItemsOptions().clean(true), 2L)); + List ns = seen.stream().map(n -> n.get("n").asInt()).toList(); + assertEquals(List.of(3), ns, "the filtered-out page must not be skipped over or looped on"); + // 3 calls proves iteration did not stop after the fully-filtered first page: advancing (and + // deciding exhaustion) by the *returned* count would read page 1 as "0 scanned" and stop right + // there, silently losing item 3 on page 2. + assertEquals(3, backend.calls); + } + + @Test + void fallsBackToReturnedCountWhenHeaderMissing() { + // Endpoints/mocks that do not send X-Apify-Pagination-Count (every collection but dataset + // items, and this one deliberately omitting it) must keep behaving exactly as before: advance + // by the number of items actually returned. + MockTransport backend = + new MockTransport( + List.of(MockTransport.ok(200, "[{\"n\":1},{\"n\":2}]"), MockTransport.ok(200, "[]"))); + List seen = + collect(client(backend).dataset("d1").iterateItems(new DatasetListItemsOptions(), 2L)); + List ns = seen.stream().map(n -> n.get("n").asInt()).toList(); + assertEquals(List.of(1, 2), ns); + assertEquals( + 2, backend.calls, "falls back to items.length, same as before this header existed"); + } + + @Test + void fallsBackToReturnedCountWhenHeaderIsImplausiblyLow() { + // Regression for a real finding against the live API: it was observed sending + // X-Apify-Pagination-Count: 0 on every page of an *unfiltered* listing, even though each page + // genuinely scanned and returned real items. A scanned count can never be smaller than the + // number of items it produced, so a header value below the returned count cannot be a real + // "nothing scanned" answer - it means the header is not actually populated for this + // request/endpoint yet. Trusting it anyway would advance the offset by 0 and re-request (or, + // worse, treat the page as exhausted and stop) even though there is more data. + MockTransport backend = + new MockTransport( + List.of( + MockTransport.ok( + 200, "[{\"n\":1},{\"n\":2}]", Map.of("X-Apify-Pagination-Count", "0")), + MockTransport.ok(200, "[{\"n\":3}]", Map.of("X-Apify-Pagination-Count", "0")), + MockTransport.ok(200, "[]", Map.of("X-Apify-Pagination-Count", "0")))); + List seen = + collect(client(backend).dataset("d1").iterateItems(new DatasetListItemsOptions(), 2L)); + List ns = seen.stream().map(n -> n.get("n").asInt()).toList(); + assertEquals(List.of(1, 2, 3), ns, "must not stop early just because the header reads 0"); + assertEquals(3, backend.calls); + } + @Test void decodesIntoRequestedType() { MockTransport backend = diff --git a/src/test/java/com/apify/client/ExceptionHierarchyTest.java b/src/test/java/com/apify/client/ExceptionHierarchyTest.java index a49b1f4..a5d94b8 100644 --- a/src/test/java/com/apify/client/ExceptionHierarchyTest.java +++ b/src/test/java/com/apify/client/ExceptionHierarchyTest.java @@ -1,13 +1,25 @@ package com.apify.client; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import com.apify.client.http.ApifyApiException; import com.apify.client.http.ApifyClientException; import com.apify.client.http.ApifyTransportException; +import com.apify.client.http.ConflictError; +import com.apify.client.http.ForbiddenError; +import com.apify.client.http.InvalidRequestError; +import com.apify.client.http.NotFoundError; +import com.apify.client.http.RateLimitError; +import com.apify.client.http.ServerError; +import com.apify.client.http.UnauthorizedError; +import com.apify.client.internal.HttpClientCore; import java.io.IOException; +import java.time.Duration; +import java.util.List; import java.util.Map; +import java.util.concurrent.CompletionException; import org.junit.jupiter.api.Test; /** @@ -41,4 +53,67 @@ void transportExceptionIsCatchableAsClientException() { }); assertInstanceOf(ApifyTransportException.class, caught); } + + /** + * {@link HttpClientCore#buildApiError} must construct the subclass matching each status code, so + * callers can branch with {@code instanceof} instead of comparing {@code getStatusCode()} by + * number (see the reference JS client's per-status {@code ApifyApiError} subclasses). + */ + @Test + void buildApiErrorPicksSubclassByStatus() { + assertInstanceOf(InvalidRequestError.class, apiError(400)); + assertInstanceOf(UnauthorizedError.class, apiError(401)); + assertInstanceOf(ForbiddenError.class, apiError(403)); + assertInstanceOf(NotFoundError.class, apiError(404)); + assertInstanceOf(ConflictError.class, apiError(409)); + assertInstanceOf(RateLimitError.class, apiError(429)); + assertInstanceOf(ServerError.class, apiError(500)); + assertInstanceOf(ServerError.class, apiError(503)); + // A status with no dedicated subclass falls back to the plain base class (and nothing more + // specific - the exact type matters here, not just assignability). + ApifyApiException teapot = apiError(418); + assertEquals(ApifyApiException.class, teapot.getClass()); + } + + @Test + void everySubclassExtendsApifyApiException() { + // Every subclass must stay catchable by an existing `catch (ApifyApiException e)`. + for (ApifyApiException e : + List.of( + apiError(400), + apiError(401), + apiError(403), + apiError(404), + apiError(409), + apiError(429), + apiError(500))) { + assertInstanceOf(ApifyApiException.class, e); + assertInstanceOf(ApifyClientException.class, e); + } + } + + /** + * End-to-end: a 404 API response surfaces through the real HTTP pipeline (not just the factory in + * isolation) as a {@link NotFoundError}. + */ + @Test + void realRequestSurfacesTypedSubclass() { + MockTransport backend = + MockTransport.ofConstant( + 404, "{\"error\":{\"type\":\"record-not-found\",\"message\":\"not found\"}}"); + ApifyClient client = + ApifyClient.builder() + .token("test-token") + .httpTransport(backend) + .maxRetries(0) + .minDelayBetweenRetries(Duration.ofMillis(1)) + .build(); + CompletionException wrapper = + assertThrows(CompletionException.class, () -> client.task("nope").getInput().join()); + assertInstanceOf(NotFoundError.class, wrapper.getCause()); + } + + private static ApifyApiException apiError(int status) { + return HttpClientCore.buildApiError(status, new byte[0], 1, "GET", "/x"); + } } diff --git a/src/test/java/com/apify/client/MockTransport.java b/src/test/java/com/apify/client/MockTransport.java index 0c5b3d4..3363e68 100644 --- a/src/test/java/com/apify/client/MockTransport.java +++ b/src/test/java/com/apify/client/MockTransport.java @@ -28,16 +28,22 @@ */ final class MockTransport implements HttpTransport { - /** One scripted response: an HTTP status + body, or a transport error. */ + /** One scripted response: an HTTP status + body (+ optional headers), or a transport error. */ static final class Scripted { final int status; final byte[] body; final Exception error; + final Map headers; Scripted(int status, String body, Exception error) { + this(status, body, error, Map.of()); + } + + Scripted(int status, String body, Exception error, Map headers) { this.status = status; this.body = body == null ? new byte[0] : body.getBytes(StandardCharsets.UTF_8); this.error = error; + this.headers = headers; } } @@ -73,6 +79,11 @@ static Scripted ok(int status, String body) { return new Scripted(status, body, null); } + /** As {@link #ok(int, String)}, additionally setting response headers (e.g. pagination ones). */ + static Scripted ok(int status, String body, Map headers) { + return new Scripted(status, body, null, headers); + } + static Scripted networkError() { return new Scripted(0, null, new IOException("connection refused")); } @@ -105,7 +116,8 @@ public synchronized CompletableFuture> sendAsync(HttpReques failed.completeExceptionally(r.error); return failed; } - return CompletableFuture.completedFuture(new FakeResponse(request.uri(), r.status, r.body)); + return CompletableFuture.completedFuture( + new FakeResponse(request.uri(), r.status, r.body, r.headers)); } /** Scripts the next {@link #sendStreamingResponse} call to return the given status and body. */ @@ -187,11 +199,17 @@ private static final class FakeResponse implements HttpResponse { private final URI uri; private final int status; private final byte[] body; + private final Map headers; FakeResponse(URI uri, int status, byte[] body) { + this(uri, status, body, Map.of()); + } + + FakeResponse(URI uri, int status, byte[] body, Map headers) { this.uri = uri; this.status = status; this.body = body; + this.headers = headers; } @Override @@ -211,7 +229,9 @@ public Optional> previousResponse() { @Override public HttpHeaders headers() { - return HttpHeaders.of(Map.of(), (a, b) -> true); + Map> multi = new java.util.LinkedHashMap<>(); + headers.forEach((k, v) -> multi.put(k, List.of(v))); + return HttpHeaders.of(multi, (a, b) -> true); } @Override diff --git a/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java b/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java index 90d00df..b97df52 100644 --- a/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java +++ b/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java @@ -51,9 +51,10 @@ void getScheduleLog() { ApifyClient client = requireClient(); Schedule sch = client.schedules().create(scheduleDef(uniqueName("sch-log"))).join(); try { - // Simple GET on the schedule-log endpoint; a fresh schedule may have no log yet (empty - // Optional), which is a valid result — we only assert the call itself succeeds. - client.schedule(sch.getId()).getLog().join(); + // Simple GET on the schedule-log endpoint; a fresh schedule has no invocations yet (an + // empty list), which is a valid result — we only assert the call itself succeeds and + // decodes the response envelope (a non-null list). + assertTrue(client.schedule(sch.getId()).getLog().join() != null); } finally { client.schedule(sch.getId()).delete().join(); } diff --git a/src/test/java/com/apify/client/integration/TaskIntegrationTest.java b/src/test/java/com/apify/client/integration/TaskIntegrationTest.java index 4976c4f..8426364 100644 --- a/src/test/java/com/apify/client/integration/TaskIntegrationTest.java +++ b/src/test/java/com/apify/client/integration/TaskIntegrationTest.java @@ -66,7 +66,7 @@ void taskCrudFlow() { TaskClient tc = client.task(task.getId()); assertTrue(tc.get().join().isPresent()); tc.updateInput(Map.of("message", "updated")).join(); - assertTrue(tc.getInput().join().isPresent()); + assertTrue(tc.getInput().join() != null); tc.update(Map.of("name", uniqueName("task-renamed"))).join(); tc.runs().list(new ListOptions(), new RunListOptions()).join(); diff --git a/src/test/java/com/apify/client/internal/ResourceContextPathSafetyTest.java b/src/test/java/com/apify/client/internal/ResourceContextPathSafetyTest.java new file mode 100644 index 0000000..9187ef1 --- /dev/null +++ b/src/test/java/com/apify/client/internal/ResourceContextPathSafetyTest.java @@ -0,0 +1,51 @@ +package com.apify.client.internal; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +/** + * Pins {@link ResourceContext#toSafeId} and {@link ResourceContext#encodePathSegment} against + * path-traversal / path-restructuring inputs, mirroring the reference JS client's hardening (apify-client-js#1011). + */ +class ResourceContextPathSafetyTest { + + @Test + void toSafeIdReplacesEveryExtraSlash() { + // Not just the first: an id with more than one slash must not leave any literal '/' that could + // restructure the request path. + assertEquals("apify~hello-world", ResourceContext.toSafeId("apify/hello-world")); + assertEquals("user~name~extra", ResourceContext.toSafeId("user/name/extra")); + assertEquals("no-slash", ResourceContext.toSafeId("no-slash")); + } + + @Test + void encodePathSegmentPercentEncodesSlashesAndSpecialChars() { + // A key/id containing slashes must become a single opaque path segment - no literal '/' must + // survive to restructure the request path, e.g. by walking into a sibling resource. + assertEquals( + "..%2F..%2Factor-runs%2FVICTIM%2Fabort", + ResourceContext.encodePathSegment("../../actor-runs/VICTIM/abort")); + assertEquals("foo%3Finjected%3D1", ResourceContext.encodePathSegment("foo?injected=1")); + assertEquals("foo%23frag", ResourceContext.encodePathSegment("foo#frag")); + } + + @Test + void encodePathSegmentRejectsDotSegmentsAndEmpty() { + // A bare "." or ".." (or empty string) is rejected outright rather than encoded: a URL parser + // resolves dot segments *after* percent-decoding, so an encoded ".." would still collapse + // against its neighbours once decoded by a client-side or intermediary URL parser. + assertThrows(IllegalArgumentException.class, () -> ResourceContext.encodePathSegment(".")); + assertThrows(IllegalArgumentException.class, () -> ResourceContext.encodePathSegment("..")); + assertThrows(IllegalArgumentException.class, () -> ResourceContext.encodePathSegment("")); + assertThrows(IllegalArgumentException.class, () -> ResourceContext.encodePathSegment(null)); + } + + @Test + void encodePathSegmentKeepsOrdinaryValuesReadable() { + assertEquals("my-key", ResourceContext.encodePathSegment("my-key")); + assertEquals("a%20b", ResourceContext.encodePathSegment("a b")); + } +} From d37d5b22e4707e96a4281f3cdf573c5ae813d591 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Proch=C3=A1zka?= Date: Fri, 2 Oct 2026 15:53:47 +0000 Subject: [PATCH 2/2] fix: correct dataset iterateItems pagination guard for unwind, address review feedback The X-Apify-Pagination-Count guard added in the previous commit discarded the reported scanned count whenever it was smaller than the returned item count. That is wrong for unwind, which legitimately returns more items than were scanned (one scanned row's array field splits into several output items) - the guard then advanced the offset by the larger returned count and silently dropped rows on the next page. Narrow the guard to only distrust a reported zero when the page also returned real items (the one combination that can never be genuine, since scanning zero rows can't produce items); any other nonzero reported value - including one smaller than the returned count - is now trusted, matching the reference client's behavior for both the filtered-page and unwind cases. Also: - Strengthen the getLog/getInput integration assertions to check decoded content instead of just non-null. - Add a regression test and MockTransport request-URL tracking covering the unwind case specifically (asserts the actual offset sent, since the mock otherwise serves scripted pages regardless of the request). - Consolidate the "flat zero observed on the live API" rationale to its one canonical location instead of restating it in five places. --- CHANGELOG.md | 9 +-- docs/storages.md | 18 ++--- .../java/com/apify/client/PaginationList.java | 24 +++---- .../apify/client/dataset/DatasetClient.java | 18 +++-- .../internal/AsyncPaginatedPublisher.java | 18 +++-- .../client/DatasetItemsIteratorTest.java | 65 +++++++++++++++---- .../java/com/apify/client/MockTransport.java | 2 + .../integration/ScheduleIntegrationTest.java | 12 ++-- .../integration/TaskIntegrationTest.java | 2 +- 9 files changed, 109 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45349b5..bcdc8e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,10 +22,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `ScheduleClient.getLog()` was fetching the raw response body as text; the endpoint's response is a JSON envelope wrapping an array of log entries. It now returns `List`. - `DatasetClient.iterateItems`/the publisher driving it could repeat or silently skip items when - combined with server-side item filters (`clean`/`skipEmpty`/`skipHidden`): it now paginates by the - `X-Apify-Pagination-Count` scanned-row count the API reports (when that count is at least the - number of items returned; a smaller value is treated as the header not being populated and falls - back to the returned count, as before), not just by the number of items returned. + combined with server-side item filters (`clean`/`skipEmpty`/`skipHidden`) or `unwind`: it now + paginates by the `X-Apify-Pagination-Count` scanned-row count the API reports whenever that count + is nonzero (a reported `0` is trusted only when the page also returned nothing, since scanning zero + rows can never produce items; any other `0` falls back to the returned count, as before), not just + by the number of items returned. - `ApifyClientBuilder`'s `baseUrl`/`publicBaseUrl` doubled the `/v2` suffix when the caller already included it (e.g. `"https://api.apify.com/v2"` became `".../v2/v2"`). - `ResourceContext.toSafeId` replaced only the first `/` in a resource id; it now replaces all of diff --git a/docs/storages.md b/docs/storages.md index 5bc4ec1..7440a18 100644 --- a/docs/storages.md +++ b/docs/storages.md @@ -49,15 +49,15 @@ any field not covered by a typed getter above is still available via the inherit | `createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs)` | A public (optionally signed) items URL, served as `json`. Completes with `String`. | | `createItemsPublicUrl(DatasetListItemsOptions, Long expiresInSecs, DownloadItemsFormat format)` | As above, with the URL's serialization `format` set explicitly. | -> **Server-side item filters and iteration.** The dataset-items endpoint applies `offset`/`limit` to -> the raw items and then drops those removed by a server-side filter (`skipEmpty`, `skipHidden`, -> `clean`, `simplified`), so a page can *scan* up to `limit` rows while *returning* fewer, or none at -> all. Where the API reports the number of rows it scanned (`X-Apify-Pagination-Count`), `iterateItems` -> advances by that number rather than by the number of items returned, so a fully-filtered page -> neither repeats already-seen items nor ends iteration early. A reported count smaller than the -> returned one (seen in practice as a flat `0` even on pages that did return items) is treated as the -> header not being populated for that request and falls back to the returned count, so this is safe -> whether or not a given deployment of the API sends the header yet. +> **Server-side item filters, `unwind`, and iteration.** The dataset-items endpoint applies +> `offset`/`limit` to the raw rows and then transforms them: a filter (`skipEmpty`, `skipHidden`, +> `clean`, `simplified`) can drop rows (fewer items returned than scanned), and `unwind` can split a +> row's array field into several items (more items returned than scanned). Where the API reports the +> number of rows it scanned (`X-Apify-Pagination-Count`), `iterateItems` advances by that number +> rather than by the number of items returned, so neither case repeats already-seen items, skips +> rows, nor ends iteration early. (A reported `0` is trusted only when the page also returned +> nothing, since scanning zero rows can never produce items; any other combination falls back to the +> returned count, as if the header were absent.) ```java Dataset ds = client.datasets().getOrCreate("my-dataset").join(); diff --git a/src/main/java/com/apify/client/PaginationList.java b/src/main/java/com/apify/client/PaginationList.java index 558c1f1..6caf693 100644 --- a/src/main/java/com/apify/client/PaginationList.java +++ b/src/main/java/com/apify/client/PaginationList.java @@ -50,21 +50,23 @@ public boolean isDesc() { } /** - * The number of rows the API scanned to produce this page, before any item-level filtering (e.g. - * dataset items' {@code clean}/{@code skipEmpty}/{@code skipHidden}), as reported by the {@code - * X-Apify-Pagination-Count} response header. {@code null} when the endpoint does not send that - * header, in which case {@link #getCount()} (the number of items actually returned) is the right - * value to advance pagination by - the two always agree outside the dataset items endpoint. + * The number of rows the API scanned to produce this page, before any item-level transform (e.g. + * dataset items' {@code clean}/{@code skipEmpty}/{@code skipHidden} filters, which can make this + * larger than {@link #getCount()}, or {@code unwind}, which can make it smaller), as reported by + * the {@code X-Apify-Pagination-Count} response header. {@code null} when the endpoint does not + * send that header, in which case {@link #getCount()} is the right value to advance pagination by + * - the two always agree outside the dataset items endpoint. * - *

A value smaller than {@link #getCount()} cannot be a genuine "rows scanned" answer (scanning - * can never produce more items than it scanned), so it is treated as the header not actually - * being populated for that request rather than as real data - callers reading this field directly - * should apply the same {@code scannedCount != null && scannedCount >= getCount() ? scannedCount - * : getCount()} fallback that {@link com.apify.client.dataset.DatasetClient#iterateItems} uses. + *

A reported {@code 0} together with a nonzero {@link #getCount()} cannot be a genuine answer + * (scanning zero rows can never produce items), so that specific combination is treated as the + * header not actually being populated for the request rather than as real data. See {@code + * AsyncPaginatedPublisher.applyPage} for the exact fallback callers reading this field directly + * should mirror. * *

Internal pagination-engine detail, exposed here (rather than hidden) only because {@link * #getItems()} and this metadata necessarily travel together on the same page object; most - * callers never need it directly - {@code iterateItems} already accounts for it. + * callers never need it directly - {@link com.apify.client.dataset.DatasetClient#iterateItems} + * already accounts for it. */ public Long getScannedCount() { return scannedCount; diff --git a/src/main/java/com/apify/client/dataset/DatasetClient.java b/src/main/java/com/apify/client/dataset/DatasetClient.java index 46b4d35..b63643d 100644 --- a/src/main/java/com/apify/client/dataset/DatasetClient.java +++ b/src/main/java/com/apify/client/dataset/DatasetClient.java @@ -138,16 +138,14 @@ public Flow.Publisher iterateItems(DatasetListItemsOptions options) { * of items yielded ({@code null} or non-positive = all); {@code chunkSize} is the per-request * page size ({@code null} = server default). * - *

Server-side item filters ({@code skipEmpty}, {@code skipHidden}, {@code clean}, {@code - * simplified}) are applied after {@code offset}/{@code limit}, so a page can scan up to {@code - * limit} rows while returning fewer, or none at all. Where the API reports the number of rows it - * scanned ({@code X-Apify-Pagination-Count}), this iterator advances by that number rather than - * by the number of items returned, so a fully-filtered page neither repeats already-seen items - * nor ends iteration early. A header value smaller than the returned count (observed in practice - * as a flat {@code 0} even on pages that did return real items) is treated as the header not - * being populated for that request, falling back to the returned count - exactly as if the header - * were absent - so this is safe whether or not a given deployment of the API actually sends it - * yet. + *

A server-side item filter ({@code skipEmpty}, {@code skipHidden}, {@code clean}, {@code + * simplified}) or {@code unwind} can make a page's returned item count diverge from the number of + * rows actually scanned - a filter drops rows (returns fewer), {@code unwind} splits one row's + * array field into several items (returns more). Where the API reports the scanned count ({@code + * X-Apify-Pagination-Count}), this iterator advances by that number rather than by the number of + * items returned, so neither case repeats already-seen items, skips rows, nor ends iteration + * early. See {@link PaginationList#getScannedCount()} for the one case that header value is not + * trusted (and why). */ public Flow.Publisher iterateItems( DatasetListItemsOptions options, Long chunkSize, Class itemClass) { diff --git a/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java b/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java index 45d435b..155cfbf 100644 --- a/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java +++ b/src/main/java/com/apify/client/internal/AsyncPaginatedPublisher.java @@ -233,14 +233,18 @@ private void applyPage(PaginationList page) { // data ahead. // // getScannedCount() is null on every endpoint that does not report the header at all - // (everything but dataset items). It is also only trusted when it is at least the returned - // count: a scanned count can never be smaller than what it produced, so a smaller value (in - // practice, observed as a flat 0 on every page against the live API at the time of writing, - // even where real items were both scanned and returned) means the header is not actually - // populated yet rather than a real "nothing scanned" answer - falling back to the returned - // count there keeps this exactly as safe as before the header existed. + // (everything but dataset items). Where it is reported, a *nonzero* value is always trusted: + // the returned count can legitimately fall on either side of it (a filter drops rows, so + // returned < scanned; unwind splits one scanned row's array field into several output items, + // so returned > scanned), and the scanned count is what offset must advance by in both cases. + // A reported *zero* is trusted only when the page also returned nothing: scanning zero rows + // can never produce items, so "0 scanned, N>0 returned" is not a real answer the API can give + // - it means the header is not actually populated for this request yet (observed in practice + // as a flat 0 on every page of an unfiltered, non-unwound live-API request that did return + // real items), and falling back to the returned count there keeps this exactly as safe as + // before the header existed. Long reported = page.getScannedCount(); - long scanned = reported != null && reported >= count ? reported : count; + long scanned = reported != null && (reported > 0 || count == 0) ? reported : count; offset += scanned; yielded += count; // Defensively trim the last page to the cap in case the server returned more than requested, diff --git a/src/test/java/com/apify/client/DatasetItemsIteratorTest.java b/src/test/java/com/apify/client/DatasetItemsIteratorTest.java index 89438d3..393f10a 100644 --- a/src/test/java/com/apify/client/DatasetItemsIteratorTest.java +++ b/src/test/java/com/apify/client/DatasetItemsIteratorTest.java @@ -1,6 +1,7 @@ package com.apify.client; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import com.apify.client.dataset.DatasetClient; @@ -112,12 +113,11 @@ void listItemsScannedCountNullWhenHeaderAbsent() { @Test void advancesByScannedCountNotReturnedCount() { - // Regression for the fully-filtered-page bug: a page can scan `limit` rows (reported via - // X-Apify-Pagination-Count) while a server-side filter (clean/skipEmpty/skipHidden) drops all - // of them. Advancing by the *returned* item count (0) would re-request the same offset forever - // (or, with the old "stop on empty page" rule, end iteration early and silently skip the - // remaining items). Advancing by the *scanned* count must skip past the filtered-out window and - // reach the real data on the next page. + // Regression for the fully-filtered-page bug (see AsyncPaginatedPublisher.applyPage): a + // server-side filter (clean/skipEmpty/skipHidden) can drop every item of a scanned page. + // Advancing by the *returned* count (0) would stop iteration right there; advancing by the + // *scanned* count (reported via X-Apify-Pagination-Count) must skip past the filtered-out + // window and reach the real data on the next page. MockTransport backend = new MockTransport( List.of( @@ -140,6 +140,45 @@ void advancesByScannedCountNotReturnedCount() { assertEquals(3, backend.calls); } + @Test + void advancesByScannedCountEvenWhenUnwindReturnsMoreThanScanned() { + // Regression for the mirror-image bug the naive "trust the header only when it's >= the + // returned count" guard introduced: `unwind` splits one scanned row's array field into several + // output items, so returnedCount > scannedCount is a legitimate, common shape - not a sign the + // header is unpopulated. A guard that falls back to the (larger) returned count here advances + // the offset too far and silently drops the rows in between on the next page. + MockTransport backend = + new MockTransport( + List.of( + // Page 1: scanned 2 rows, each with a 2-element array that unwind splits in two. + MockTransport.ok( + 200, + "[{\"n\":1},{\"n\":2},{\"n\":3},{\"n\":4}]", + Map.of("X-Apify-Pagination-Count", "2")), + // Page 2 (offset must be 2, the scanned count - not 4, the returned count): the + // remaining 2 scanned rows, unwound into 4 items. + MockTransport.ok( + 200, + "[{\"n\":5},{\"n\":6},{\"n\":7},{\"n\":8}]", + Map.of("X-Apify-Pagination-Count", "2")), + // Nothing left to scan. + MockTransport.ok(200, "[]", Map.of("X-Apify-Pagination-Count", "0")))); + List seen = + collect( + client(backend) + .dataset("d1") + .iterateItems(new DatasetListItemsOptions().unwind(List.of("arr")), 2L)); + List ns = seen.stream().map(n -> n.get("n").asInt()).toList(); + assertEquals(List.of(1, 2, 3, 4, 5, 6, 7, 8), ns, "no rows dropped between the unwound pages"); + assertEquals(3, backend.calls); + // The decisive assertion: the second request's offset must be 2 (the scanned count), not 4 + // (the returned count) - MockTransport serves its scripted pages in call order regardless of + // what offset is requested, so only inspecting the actual request URLs catches a guard that + // picks the wrong (larger) value. + assertTrue(backend.urls.get(1).contains("offset=2"), backend.urls.get(1)); + assertFalse(backend.urls.get(1).contains("offset=4"), backend.urls.get(1)); + } + @Test void fallsBackToReturnedCountWhenHeaderMissing() { // Endpoints/mocks that do not send X-Apify-Pagination-Count (every collection but dataset @@ -158,13 +197,13 @@ void fallsBackToReturnedCountWhenHeaderMissing() { @Test void fallsBackToReturnedCountWhenHeaderIsImplausiblyLow() { - // Regression for a real finding against the live API: it was observed sending - // X-Apify-Pagination-Count: 0 on every page of an *unfiltered* listing, even though each page - // genuinely scanned and returned real items. A scanned count can never be smaller than the - // number of items it produced, so a header value below the returned count cannot be a real - // "nothing scanned" answer - it means the header is not actually populated for this - // request/endpoint yet. Trusting it anyway would advance the offset by 0 and re-request (or, - // worse, treat the page as exhausted and stop) even though there is more data. + // Regression for a real finding against the live API (see PaginationList.getScannedCount): it + // was observed sending X-Apify-Pagination-Count: 0 on every page of an *unfiltered* listing, + // even though each page genuinely scanned and returned real items - a combination that can + // never + // be a real "nothing scanned" answer (0 scanned rows can't produce items), so it must mean the + // header is not populated for this request/endpoint yet. Trusting it anyway would advance the + // offset by 0 and re-request (or, worse, treat the page as exhausted and stop). MockTransport backend = new MockTransport( List.of( diff --git a/src/test/java/com/apify/client/MockTransport.java b/src/test/java/com/apify/client/MockTransport.java index 3363e68..162779b 100644 --- a/src/test/java/com/apify/client/MockTransport.java +++ b/src/test/java/com/apify/client/MockTransport.java @@ -55,6 +55,7 @@ static final class Scripted { String lastBody; byte[] lastBodyBytes; final List bodies = new ArrayList<>(); + final List urls = new ArrayList<>(); /** * Optional scripted response for {@link #sendStreamingResponse}; defaults to the first {@code @@ -101,6 +102,7 @@ public synchronized CompletableFuture> sendAsync(HttpReques int idx = calls++; lastHeaders = request.headers(); lastUrl = request.uri().toString(); + urls.add(lastUrl); lastMethod = request.method(); lastBodyBytes = readBody(request); lastBody = lastBodyBytes == null ? null : new String(lastBodyBytes, StandardCharsets.UTF_8); diff --git a/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java b/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java index b97df52..eea9704 100644 --- a/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java +++ b/src/test/java/com/apify/client/integration/ScheduleIntegrationTest.java @@ -51,10 +51,14 @@ void getScheduleLog() { ApifyClient client = requireClient(); Schedule sch = client.schedules().create(scheduleDef(uniqueName("sch-log"))).join(); try { - // Simple GET on the schedule-log endpoint; a fresh schedule has no invocations yet (an - // empty list), which is a valid result — we only assert the call itself succeeds and - // decodes the response envelope (a non-null list). - assertTrue(client.schedule(sch.getId()).getLog().join() != null); + // A fresh schedule has no invocations yet, so the only reachable real-API assertion is an + // empty (not null/absent) list: the endpoint's "no entries" response must still decode + // through the {"data": [...]} envelope rather than erroring or resolving to null. Decoding of + // populated entries (message/level/createdAt) is covered against a real wire-format example + // in ClientBehaviourRegressionTest#scheduleGetLogDecodesEnvelopeArray — producing a non-empty + // log here would require waiting for an actual cron invocation, which isn't practical in a + // short-lived integration test. + assertEquals(List.of(), client.schedule(sch.getId()).getLog().join()); } finally { client.schedule(sch.getId()).delete().join(); } diff --git a/src/test/java/com/apify/client/integration/TaskIntegrationTest.java b/src/test/java/com/apify/client/integration/TaskIntegrationTest.java index 8426364..61d21c5 100644 --- a/src/test/java/com/apify/client/integration/TaskIntegrationTest.java +++ b/src/test/java/com/apify/client/integration/TaskIntegrationTest.java @@ -66,7 +66,7 @@ void taskCrudFlow() { TaskClient tc = client.task(task.getId()); assertTrue(tc.get().join().isPresent()); tc.updateInput(Map.of("message", "updated")).join(); - assertTrue(tc.getInput().join() != null); + assertEquals("updated", tc.getInput().join().get("message").asString()); tc.update(Map.of("name", uniqueName("task-renamed"))).join(); tc.runs().list(new ListOptions(), new RunListOptions()).join();