refactor!: package split + issue #8 review fixes; sync spec to v2-2026-07-20T094852Z#10
Open
Pijukatel wants to merge 10 commits into
Open
refactor!: package split + issue #8 review fixes; sync spec to v2-2026-07-20T094852Z#10Pijukatel wants to merge 10 commits into
Pijukatel wants to merge 10 commits into
Conversation
…6-07-20T094852Z Addresses the expert review at #8: splits the single com.apify.client package into resource-scoped sub-packages, renames the replaceable HTTP transport (HttpBackend -> HttpTransport), reworks the exception hierarchy (new ApifyClientException base, public ApifyTransportException, backend-agnostic HttpTimeoutException), makes ResourceContext/DatasetClient/KeyValueStoreClient immutable, centralizes resource path segments into ApiPaths, removes setStatusMessage() and the public getUserAgent()/getApiBaseUrl(), makes brotli opt-in and adds slf4j logging. Several review suggestions are explicitly skipped where they would conflict with the reference-client naming/behavior parity requirement (see notes.md); documented with rationale. Bumps to 0.4.0 (breaking) and syncs Version.API_SPEC_VERSION to v2-2026-07-20T094852Z (response-only spec delta, no behavior change needed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…estore setStatusMessage Fixes the headline correctness bug (ApifyApiException now extends ApifyClientException) and a PaginationList NPE on "items": null. Hides implementation plumbing behind a non-exported com.apify.client.internal package with a module-info.java, restores ApifyClient.setStatusMessage (the prior removal was based on a false claim about the reference client), and removes the duplicated AI-disclaimer from the 10 docs pages. Also covers the accumulated code-quality, docs, changelog, test-coverage, parity-gap and issue #8 follow-ups from the review; see notes.md "Implementer — loop 2" for the itemized list and the explicit skip-with-justification entries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
…ns, log-streaming call(), DRY collection clients Closes every item from the loop-2 review's remaining-actionable list: batchAddRequests now also splits by payload byte size (not just count) to avoid 413s; RequestQueueClient's lock/list operations return typed models instead of raw JsonNode; Schedule/Webhook/Task/ActorRun gained typed getters for fields previously reachable only via getExtra(); new ActorCallOptions/ TaskCallOptions call() overloads stream the run's log by default, matching the reference client; extracted AbstractCollectionClient to de-duplicate the 8 collection clients' list/iterate boilerplate; plus the requested docs completions and style nits. See notes.md "Implementer — loop 3" in apify-client-orchestration for the itemized list and test evidence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
…ver-throws, parity getters, call() DRY Closes every item from the loop-3 review's residual list: removed the duplicated AI/experimental disclaimer from ApifyClient's class javadoc (README-only now); RequestQueueClient.batchAddRequests no longer throws on a non-retryable 4xx, matching the reference client's never-throws contract (unprocessed requests reported via BatchAddResult instead); added offline mock-backend coverage for RunClient.metamorph/reboot and corrected the stale ActorRunIntegrationTest comment; added the remaining typed getters (ActorRun.usageTotalUsd/buildNumber/exitCode/isContainerServerReady/ gitBranchName/storageIds, Webhook.shouldInterpolateStrings/lastDispatch); extracted StartOptionsLike/CallOptionsLike + a RunStartSupport helper so ActorClient/TaskClient's start/call machinery stops duplicating ~185 lines; ApifyClientBuilder.timeout now rejects zero/negative durations; RequestQueueRequest.userData is defensively deep-copied; RequestQueueClient's cursor iterator now trims an overshot page to totalLimit; tightened spotbugs-exclude.xml to entries that actually reproduce a finding; plus the requested docs completions and CI permissions block. See notes.md "Implementer — loop 4" in apify-client-orchestration for the itemized list and test evidence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
…ocs contradictions, uniqueKey caveat Closes the loop-4 review's residual MAJORs and nits: - N1: RunStartSupport now takes Consumer<QueryParams>/plain values instead of the public CallOptionsLike/StartOptionsLike marker interfaces, so ActorClient/TaskClient can keep apply()/contentTypeOrDefault()/toStartOptions()/logStreamingEnabledValue()/logOptionsValue() package-private on ActorStartOptions/TaskStartOptions/ActorCallOptions/TaskCallOptions instead of leaking them (and the internal QueryParams type) into the options builders' public API. - N2: reconciled the examples.md contradiction across README.md/docs/README.md/docs/examples.md — all three now agree that the fragments are fragments and the runnable programs live under src/test/java/com/apify/client/examples/. - N3: added the missing WebhookLastDispatch import to docs/README.md's package table. - N4: added an offline mock-backend test for ApifyClient.setStatusMessage's ACTOR_RUN_ID-unset IllegalStateException path (previously zero coverage); documented why the ACTOR_RUN_ID-set path isn't offline- or live-testable, mirroring the metamorph/reboot/charge convention. - N5: added a javadoc caveat on batchAddRequests/RequestQueueRequest.getUniqueKey() about uniqueKey-based retry reconciliation falsely reporting a succeeded request as unprocessed. - Nits: isShouldInterpolateStrings() -> getShouldInterpolateStrings() (field name unchanged, so no wire-format effect); named the unused RequestQueueClient catch binding `ignored`; fixed the "one raw-JSON field" framing in docs/README.md; clarified the request-queue getOrCreate overload gap in docs/storages.md; documented SetStatusMessageOptions's primitive boolean; trimmed boilerplate javadoc on trivial RequestQueueRequest setters; documented DefaultHttpTransport(Duration) and evened out import disclosure in README fragments; strengthened the nested-webhook test comment to point at the real multi-item iteration coverage. mvn spotless:check/compile/spotbugs:check/test all green: 183/183 tests passing (unit + live integration + ExamplesTest + DocSnippetsTest), 0 failures, 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
…setter trim Addresses the two concrete findings from this loop's coding-style review: - The near-identical "package-private on purpose" paragraph was copy-pasted across ActorStartOptions/ActorCallOptions/TaskStartOptions/TaskCallOptions's class javadoc. Condensed each to a one-line pointer at RunStartSupport, whose own javadoc (now with @param docs on call/callWithLogStreaming) carries the full rationale once. - RequestQueueRequest's setter-javadoc trim was inconsistent: setId/setUrl/setMethod/setPayload had their boilerplate one-liner removed, but setHandledAt/setRetryCount/setLoadedUrl (equally trivial restatements of their getters) did not. Trimmed those three too for consistency; setUserData/ setHeaders/setNoRetry/setUniqueKey/setErrorMessages are untouched (still the mandated keep-as-informative set). mvn spotless:check/compile/spotbugs:check/test all green: 183/183 tests passing, 0 failures, 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
…doc gaps Addresses the second independent /review-client pass's residual findings on top of f705ae6: - P1 (CI, merge-blocking): ActorRunIntegrationTest.streamedLogRedirection failed in CI on f705ae6 (the same live race as loop 1/2 — a live-tail stream opened before the run finishes can reach EOF a hair before the final log bytes are flushed to that connection). Fix: after the bounded catch-up window, if the run produced a log but the first stream still came up empty, retry once with a brand-new StreamedLog opened strictly after the run is already finished — a GET against a static, fully-persisted log rather than a live tail, so it cannot race the run's own writer. Verified green 3x locally against the live API. - P2: fixed a factually wrong RunStartSupport javadoc claim that ActorStartOptions/TaskStartOptions/ ActorCallOptions/TaskCallOptions "stay package-private" — those classes are public (they must be, as public method parameter/return types); only 5 of their accessor methods are package-private. - P5: AbstractCollectionClient.list(options) NPE'd on null options while iterate(options) tolerated it — now consistent. BatchAddResult.getProcessedRequests()/getUnprocessedRequests()/merge() now null-coalesce, matching PaginationList's existing pattern, so an explicit "processedRequests": null / "unprocessedRequests": null API response can no longer NPE. - P7 (docs): documented that ValidateInputOptions lives in com.apify.client.task despite configuring an ActorClient method; added java.time.Instant to the standard-import list; noted that setStatusMessage/UserClient limits methods/ApifyClientBuilder setters throw plain JDK exceptions outside the ApifyClientException hierarchy; clarified waitForFinish/call's behavior when the wait budget expires before the run reaches a terminal state. mvn spotless:check/compile/spotbugs:check/test all green: 183/183 tests passing, 0 failures, 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
…d-Actor race, DTO null-guards, RunClient.metamorph validation, package move, docs Addresses every item from the loop-6 /review-client verdict: RequestQueueClient.batchAddChunkWithRetries now catches ApifyClientException (not just ApifyApiException) so transport failures also stay within the never-throws contract; ActorRunIntegrationTest#lastRunAccess no longer asserts run identity against the shared apify/hello-world Actor (only presence/status/origin); added last-run- and run-scoped nested storage GET coverage; null-coalesced every flagged list-returning DTO getter; moved ValidateInputOptions to com.apify.client.actor next to its only consumer; RunClient.metamorph validates targetActorId; various docs completeness, comment, and DRY nits. See notes.md "Implementer — loop 7" in apify-client-orchestration for the itemized list and test evidence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HuTYpvY6WkVvmsXzupzEk9
- RequestQueueClient: null-coalesce BatchDeleteResult's list getters and RequestQueueRequest.getErrorMessages() to avoid NPEs on an explicit JSON null, mirroring BatchAddResult. - RunClient.metamorph: normalize targetActorId via ResourceContext.toSafeId before sending (matches the reference client's _toSafeId); update the offline mock test's expected query param. - RunClient.metamorph/resurrect and ActorClient.build: throw a clear IllegalArgumentException on a null required options argument instead of an NPE deep inside the method. - RunClient's default log-redirection prefix now uses the resolved run's real id instead of the field id (which is the literal "last" for a runs/last accessor). - Box ActorRunOptions.timeoutSecs/memoryMbytes/diskMbytes (Long) so an unset value is distinguishable from 0, matching the boxed sibling fields.
Flesh out response DTOs to match the JS reference's typed interfaces, resolving the recurring thin-DTO review category: - WebhookDispatch: typed status, eventType, userId, createdAt, calls (WebhookDispatchCall), webhook (WebhookDispatchWebhookInfo), eventData (WebhookDispatchEventData). - Actor: wire in getActorStandby() (ActorStandby, now also carrying the Actor-only isEnabled field), plus stats (ActorStats), versions, pricingInfos, defaultRunOptions (ActorDefaultRunOptions), taggedBuilds, actorPermissionLevel, categories, isDeprecated, deploymentKey, seoTitle/seoDescription. - Build: userId, meta (BuildMeta), stats (BuildStats), options (BuildOptions), usage/usageUsd/usageTotalUsd (BuildUsage). - User: profile (UserProfile), email, proxy (UserProxy), plan (UserPlan), effectivePlatformFeatures, createdAt, isPaying. - ActorStoreListItem: description, stats, currentPricingInfo (PricingInfo), pictureUrl, userPictureUrl, url, readmeSummary. Task/ActorRun were already complete from an earlier loop; verified against the JS reference and left unchanged. Every new field name/type was checked against apify-client-js's resource_clients interfaces before modelling. getExtra() is kept for forward-compat on every touched model. Docs field lists updated to match (actors.md, builds.md, misc.md, webhooks.md, examples.md); the GetAccount example and its doc snippet now read the typed getEmail() instead of reaching into getExtra(). spotbugs-exclude.xml: added the new pure-DTO classes to the existing UWF_UNWRITTEN_FIELD allow-list (same false-positive rationale as the pre-existing entries); rewrote the "historical note" to state only the current timeless rationale, dropping the loop-numbered narrative.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Syncs the Java client to Apify OpenAPI spec
v2-2026-07-20T094852Zand releases0.4.0, applying the structural changes from the expert review in #8. This is a breaking release (explicitly authorized).Structure & public API
com.apify.clientpackage into resource-scoped sub-packages (.actor,.build,.run,.dataset,.keyvalue,.requestqueue,.task,.schedule,.webhook,.user,.store,.log,.http). Amodule-info.javaexports only these and keeps implementation plumbing in a non-exportedcom.apify.client.internalpackage.HttpBackend→HttpTransport,DefaultHttpBackend→DefaultHttpTransport,sendStreaming→sendStreamingResponse.ApifyClientExceptionbase, withApifyApiExceptionand a transport-agnosticApifyTransportException/HttpTimeoutException.ApiPaths; makesResourceContextand the storage clients immutable.ValidateInputOptionslives incom.apify.client.actor, alongside its only consumer (ActorClient.validateInput).Behavior & correctness
RequestQueueClient.batchAddRequestschunks by both request count and cumulative payload byte size (so a batch of large requests can't 413), and never throws on a hard 4xx or a transport-level failure (timeout, connection error) — failed requests are returned viaBatchAddResult.getUnprocessedRequests(), matching the reference client's contract.JsonNode/getExtra()access across the request-queue lock operations and theSchedule/Webhook/Task/ActorRunmodels; every list-returning DTO getter tolerates an explicit JSONnullwithout throwing.call(..., ActorCallOptions / TaskCallOptions, ...)overloads that stream the run's log during the wait (matching the reference client); the plaincall(..., *StartOptions, ...)overload is unchanged.RunClient.metamorphvalidatestargetActorId;ApifyClientBuildervalidates inputs (rejects a zero/negative timeout at build time); brotli compression is opt-in; logging goes through SLF4J.Docs & tests
apify/hello-worldstore Actor.mvn test(unit + live integration +ExamplesTest+DocSnippetsTest): 186/186 passing.Review points from #8 intentionally not applied
ApifyClient.setStatusMessage(...)is kept — the reference JS client has it, so removing it would break the required convenience-method parity.ApifyApiExceptionstays aRuntimeException-rooted type (parity with the JS error model, which is a plain unchecked error).Test plan
mvn spotless:checkmvn compilemvn spotbugs:checkmvn test(offline unit + live integration +ExamplesTest+DocSnippetsTest) — 186/186 passing