Attachment downloads, bounded read retries, and Export Search fix by russellhoff · Pull Request #858 · cloudbees-oss/zendesk-java-client

russellhoff · 2026-09-25T07:07:12Z

The client currently has no API for saving attachment content to a file, and its default transport may replay requests without distinguishing reads from writes. This change adds attachment downloads and bounded retries at selected synchronous read boundaries.

Changes

Add downloadAttachment(long, Path) and downloadAttachment(Attachment, Path), refreshing metadata by ID and reusing the configured HTTP client.
Send Zendesk credentials only to the Zendesk origin. External content URLs do not receive Zendesk credentials.
Validate HTTP status before writing, create parent directories, and propagate checked IOException and InterruptedException from both metadata retrieval and content download while preserving the interrupt flag and request cancellation.
Disable method-agnostic retries in the default transport. Retry selected reads up to three attempts on transient transport errors and HTTP 500/502/503/504.
Honor HTTP 429 Retry-After values in seconds or any HTTP-date format (IMF-fixdate, RFC 850, or ANSI C asctime), including the RFC 850 fifty-year rule and space-padded asctime days. Retry only delays of at most 60 seconds; longer or invalid values are propagated without retrying. Injected clients retain their existing retry policy.
Correct Export Search to use filter[type]=ticket without appending type:ticket to the query.
Document the behavior and mark the new APIs and retry helper with @since 1.7.0.

Compatibility and limitations

Disabling default transport retries affects all requests using that transport. Explicit retries apply only to selected synchronous reads; writes and OAuth token requests are not retried by this policy. Async job methods do not gain these retries.

Attachment responses are buffered in memory, and the final file copy is not atomic.

Validation

JDK 11: mvn -B -ntp spotless:apply clean verify -Dsurefire.rerunFailingTestsCount=0 — 150 tests, zero failures/errors, one skipped. Spotless and the Java 8 API compatibility check passed.
Local HTTP tests cover byte-for-byte downloads, authentication boundaries, redirects, metadata and content transport failures, interruption in both phases, bounded retries, all three Retry-After date formats, the RFC 850 century window, malformed and excessive delays, pagination, and write non-replay.
No live Zendesk smoke tests were run.

Refresh attachment metadata and reuse the client transport without forwarding credentials to external origins. Limit retries to selected synchronous reads, honor Retry-After, and preserve interruption and injected client policies. Also fix Export Search to express the ticket type only through filter[type]. Cover downloads, pagination and retry behavior with local HTTP tests.

Copilot

Copilot review overview

🟡 Changes recommended

Two moderate issues and unresolved Javadoc placeholders remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Unwrap metadata request interruption and I/O exceptions · New
Parse all valid Retry-After HTTP-date formats · New

What changed in this PR

Adds attachment downloads, bounded retries for selected reads, safer transport behavior, and an Export Search query fix.

Changes:

Adds authenticated attachment download APIs with metadata refresh.
Adds bounded retries and Retry-After handling.
Disables method-agnostic default retries.
Corrects Export Search filtering and adds regression coverage.

File	Summary
`src/test/java/org/zendesk/client/v2/SearchExportTest.java`	Tests corrected Export Search parameters and pagination.
`src/test/java/org/zendesk/client/v2/ReadRetryTest.java`	Tests retry classification and interruption handling.
`src/test/java/org/zendesk/client/v2/ReadRetryHttpTest.java`	Tests HTTP retries and write non-replay behavior.
`src/test/java/org/zendesk/client/v2/AttachmentDownloadTest.java`	Tests downloads, redirects, and credential boundaries.
`src/main/java/org/zendesk/client/v2/ZendeskResponseRateLimitException.java`	Parses `Retry-After` values; moderate issue remains for RFC 850 and `asctime` dates.
`src/main/java/org/zendesk/client/v2/Zendesk.java`	Implements downloads, retries, and Export Search correction; metadata exception unwrapping and two Javadoc placeholders require changes.
`src/main/java/org/zendesk/client/v2/ReadRetry.java`	Defines bounded retry behavior.
`README.md`	Documents download and retry behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Refactor attachment retrieval to handle exceptions and improve error handling. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

PierreBtz

Thanks for the addition. We use an external client to download attachments but I think it's a good idea to offer the feature natively in the zendesk client.

Currently there is a compilation issue that prevents me from finishing up the review and testing.

About the @since FIXME, you can use 1.7.0.

PierreBtz · 2026-10-07T15:56:59Z

+      throw new IllegalArgumentException("Attachment id must be positive");
+    }
+Attachment attachment;
+try {


Unless I'm mistaken there is an issue here:

the indentation is incorrect

more importantly the method does not close properly. I think a call to downloadAttachmentContent is missing.

I think it's solved

Restore the missing content download return after metadata retrieval. Verify checked transport and interruption exceptions in both download phases. Parse legacy HTTP Retry-After dates with the fifty-year RFC 850 window and set contribution since tags to 1.7.0.

PierreBtz

Thanks for the contribution, I left a few comments but overall the change looks good.

PierreBtz · 2026-10-15T13:15:39Z

+          this.retryAfter = millis <= 0 ? 0L : 1 + (millis - 1) / 1000;
+        } catch (java.time.DateTimeException | ArithmeticException invalid) {
+          // An unusable header is not permission to retry earlier than requested.
+          this.retryAfter = Long.MAX_VALUE;


Before this change, the default retry would have been applied (DEFAULT_RETRY_AFTER). Is there a reason for not keeping this behavior ?

PierreBtz · 2026-10-15T13:19:52Z

+                RequestBuilder request = new RequestBuilder("GET").setUrl(contentUrl);
+                if (org.asynchttpclient.uri.Uri.create(url)
+                    .isSameBase(org.asynchttpclient.uri.Uri.create(contentUrl))) {
+                  request = reqBuilder("GET", contentUrl);


My understanding is that the intent is to reuse the auth header (since we are querying the ZD api again). But this call will actually reuse all the headers used for a ZD API call which does not seem to be what we want.
I'm especially thinking abbout the Accept header which will likely be application/json (API call) and might not be what the server expect for an attachment.

PierreBtz · 2026-10-15T13:38:03Z

+      try {
+        return operation.get();
+      } catch (ZendeskException failure) {
+        long delay = delayMillis(failure, attempt);


nit: you can invert this line and the next one. It's not much but each time attempt == 3 you'll compute a delay that will never be used.

russellhoff added 2 commits September 25, 2026 08:38

docs: complete contribution Javadocs

7cd9f41

Copilot AI lite review requested due to automatic review settings September 25, 2026 07:07

russellhoff requested review from PierreBtz and duemir as code owners September 25, 2026 07:07

Copilot started reviewing on behalf of russellhoff September 25, 2026 07:07

View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread src/main/java/org/zendesk/client/v2/Zendesk.java Outdated

Comment thread src/main/java/org/zendesk/client/v2/ZendeskResponseRateLimitException.java Outdated

Improve attachment retrieval with exception handling

9bc794e

Refactor attachment retrieval to handle exceptions and improve error handling. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

PierreBtz reviewed Oct 7, 2026

View reviewed changes

PierreBtz reviewed Oct 15, 2026

View reviewed changes

Navigation Menu

Repository navigation

Attachment downloads, bounded read retries, and Export Search fix - #858

Attachment downloads, bounded read retries, and Export Search fix#858
russellhoff wants to merge 4 commits into
cloudbees-oss:masterfrom
russellhoff:codex/upstream-attachment-read-retries

russellhoff commented Sep 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

PierreBtz left a comment

Uh oh!

PierreBtz Oct 7, 2026

Uh oh!

russellhoff Oct 15, 2026

Uh oh!

PierreBtz left a comment

Uh oh!

PierreBtz Oct 15, 2026

Uh oh!

PierreBtz Oct 15, 2026

Uh oh!

PierreBtz Oct 15, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

russellhoff commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Compatibility and limitations

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

PierreBtz left a comment

Choose a reason for hiding this comment

Uh oh!

PierreBtz Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

russellhoff Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

PierreBtz left a comment

Choose a reason for hiding this comment

Uh oh!

PierreBtz Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

PierreBtz Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

PierreBtz Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

russellhoff commented Sep 25, 2026 •

edited

Loading