Skip to content

[3/N] [List API] feat(gateway): expose request List RPC#304

Open
albertywu wants to merge 1 commit into
wua/list-api-summary-storefrom
wua/list-api-gateway-rpc
Open

[3/N] [List API] feat(gateway): expose request List RPC#304
albertywu wants to merge 1 commit into
wua/list-api-summary-storefrom
wua/list-api-gateway-rpc

Conversation

@albertywu

@albertywu albertywu commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the public gateway List RPC and controller for queue-scoped request summaries with queue validation, status filtering, deterministic pagination, and admission-time sorting. Gateway write paths now persist log entries through the summary-aware helper so the read model stays updated.

Test Plan

make fmt && make build && make test && make check-mocks && make e2e-test

Issues

Stack

  1. [1/N] [List API] feat(requestlog): persist queue and change metadata #302
  2. [2/N] [List API] feat(storage): add request summary read model #303
  3. @ [3/N] [List API] feat(gateway): expose request List RPC #304
  4. [4/N] [List API] test(e2e): cover gateway List API #305
  5. [5/N] [List API] docs(service): document gateway List API #306

Summary:

Adds the public gateway List RPC and controller for queue-scoped request summaries with queue validation, status filtering, deterministic pagination, and admission-time sorting. Gateway write paths now persist log entries through the summary-aware helper so the read model stays updated.

Test Plan:

✅ `make fmt && make build && make test && make check-mocks && make e2e-test`
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 4980cb0 to 788ef8d Compare July 7, 2026 01:49
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from 96cfc64 to d308ee7 Compare July 7, 2026 01:49
@albertywu albertywu changed the title feat(gateway): expose request List RPC [3/N] [List API] feat(gateway): expose request List RPC Jul 7, 2026
@behinddwalls

Copy link
Copy Markdown
Collaborator

Few thoughts on this.

1. Group filters vs. pagination

ListRequest is a bit hard to read at a glance — filters, ordering, and pagination sit flat in one message, so it's unclear what's required, what's optional, and what's meant to combine. Grouping by role would make it more self-documenting.

On whether filters should be a generic Filter{key, value} list for extensibility: I'd lean against it. A proto message with named fields already is a typed key/value model — the keys are fields the schema knows. A Filter{key, value} list gives that up: keys move into runtime data, values collapse to strings, and we'd reinvent operators and validation. So I'd keep filters as named typed fields, grouped into a sub-message. (If we ever need open-ended user-composed predicates, the usual path is a single string filter grammar à la AIP-160 — but not yet.)

2. A possible restructure

A starting point:

message ListRequest {
  // Required: the queue to scan (storage partition key).
  string queue = 1;
  // Optional. Absent = whole queue.
  // Present fields AND together; a repeated field ORs within itself.
  ListFilter filter = 2;
  // Optional ordering; unspecified = admission order (FIFO).
  ListSort sort = 3;
  // Optional pagination, independent of filtering.
  Page page = 4;
}

message ListFilter {
  TimeWindow window = 1;          // either bound 0 = unbounded
  repeated string statuses = 2;   // OR-set; empty = all
}

// Half-open [start, end) in Unix epoch ms. 0 = unbounded.
message TimeWindow {
  int64 start_time_ms = 1;
  int64 end_time_ms = 2;
}

message Page {
  int32 size = 1;   // 0 = server default; server may cap
  string token = 2; // opaque continuation (see below)
}

enum ListSort {
  LIST_SORT_UNSPECIFIED  = 0;  // = admission ascending
  LIST_SORT_ADMITTED_ASC = 1;
  LIST_SORT_ADMITTED_DESC = 2;
}

Small things folded in:

  • proto3 optional on the time bounds would distinguish "no lower bound" from start_time_ms = 0.
  • A oneof could encode any mutually-exclusive filters in the schema rather than in prose.
  • I'd suggest dropping the terminal filter: it overlaps statuses, and terminal=true + statuses=[building] is a contradictory request the server then has to define. Terminality is derivable from the status set. Worth keeping the derived terminal bool on the response RequestSummary though — as output it saves clients from hardcoding which statuses are terminal.

3. In-flight requests in the time window

One edge case worth a look: the contract says "lifecycles overlap a time window," but the storage filter (parent PR) is started_at_ms < end AND completed_at_ms > start, and in-flight requests have completed_at_ms = 0. So 0 > start is false whenever start_time_ms > 0 — still-running requests get filtered out, which seems opposite to what a "queue right now" view wants. Treating 0 as open-ended (completed_at_ms = 0 OR completed_at_ms > ?) would fix it, and might argue for allowing an unbounded window. Let me know if I've misread the query.

4. Pagination token — a note and an open question

Embedding the query in the token looks safe: values go through bound ? parameters and sort/order comes from a whitelist switch, so no injection path, and the token carries no entitlements — a tampered one only moves the caller's own cursor within an already-authorized query. Might be worth stating as an invariant: the token can hold position + a query echo, never anything the server treats as permission.

Optional for later: if we ever want forged tokens to be impossible rather than just harmless, we could HMAC-sign the token with a server-side secret. The signature also guarantees integrity, so the token could shrink to position + a query hash instead of echoing the whole query. Cost is key management, so probably a follow-up unless the token needs to carry something sensitive. A length cap before decoding is a cheap safeguard regardless.

Open one, no strong opinion — token or cursor?

  • token matches the AIP convention and the internal listPageToken, and signals "opaque, tied to this query, don't introspect" — which fits, since it's position + frozen query and we reject on mismatch. Keeps it distinct from the storage RequestSummaryCursor (the actual position).
  • cursor is more familiar to consumers (GraphQL/Relay), but implies a plain reusable pointer, which is at odds with the mismatch rejection. If we preferred it, we'd probably make it a true position-only cursor.

Curious what you think — feels like a judgment call, and it interacts with the HMAC question.

5. Tiny things

  • ListSort may trip buf/AIP lint (zero-value naming + unprefixed values) — the LIST_SORT_UNSPECIFIED + prefixed form above should fix it.
  • statuses as free strings seems reasonable (mirrors StatusResponse.status), just worth documenting intent for unknown values: today a hard error, but we could treat them as ignored so an older client filtering on a renamed status doesn't break.

Let me know your thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants