Skip to content

refactor: replace FromResponse trait with fetch_inner + fetch_conditional - #373

Closed
JanKaul wants to merge 1 commit into
rest-cache-primitivesfrom
refactor-fetch-no-from-response-trait
Closed

refactor: replace FromResponse trait with fetch_inner + fetch_conditional#373
JanKaul wants to merge 1 commit into
rest-cache-primitivesfrom
refactor-fetch-no-from-response-trait

Conversation

@JanKaul

@JanKaul JanKaul commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Builds on #372.

Summary

  • Removes the FromResponse single-method async trait entirely
  • Extracts all HTTP mechanics into a private fetch_inner that returns a raw reqwest::Response
  • fetch becomes a thin wrapper over fetch_inner with T: DeserializeOwned, absorbing fetch_empty (which is deleted)
  • fetch_conditional is a new thin wrapper that handles 304 Not Modified and returns Conditional<T>
  • Conditional::Modified now carries the full reqwest::header::HeaderMap instead of just an extracted etag: Option<String>, giving callers access to any response header
  • load_table is no longer generic — it always returns Conditional<LoadTableResult> and calls fetch_conditional directly

Motivation

FromResponse is a single-method trait with no state, which is the textbook case for replacement with a function. The fetch_inner + two thin wrappers pattern achieves the same zero-duplication benefit with no public trait surface to maintain or explain. Returning the full HeaderMap from Conditional::Modified is strictly more general than pre-extracting only the ETag.

…onal

- Extract HTTP mechanics into private fetch_inner returning reqwest::Response
- fetch becomes a thin wrapper (T: DeserializeOwned), absorbing fetch_empty
- fetch_conditional returns Conditional<T> for 304-aware endpoints
- Conditional::Modified now carries the full HeaderMap instead of a single
  extracted etag string, letting callers access any response header
- load_table is non-generic, always returns Conditional<LoadTableResult>
- No FromResponse trait needed; no fetch_empty function needed
@JanKaul JanKaul closed this Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant