Skip to content

Commit bd31064

Browse files
amavashevamavashev
committed
feat(reservations): support sort_by + sort_dir on GET /v1/reservations
Implements cycles-protocol-v0.yaml revision 2026-04-16: clients can now request server-side ordering of list-reservations results by passing sort_by (one of reservation_id, tenant, scope_path, status, reserved, created_at_ms, expires_at_ms) and sort_dir (asc | desc, default desc). Invalid enum values return HTTP 400 INVALID_REQUEST. Design: dual-path. When both params are omitted the repository keeps its existing Redis-SCAN cursor loop byte-for-byte unchanged, so clients that never send the new params see zero wire or behaviour difference. When sort_by or sort_dir is present (or the incoming cursor is a sorted- path cursor), the repository runs a full SCAN pass with the existing filter predicates, sorts in memory via ReservationComparators with a stable reservation_id ASC tiebreaker, and emits an opaque slice cursor that binds the (sort_by, sort_dir, filters) tuple via an 8-byte SHA-256 filter hash. Reusing a cursor under a mismatched tuple returns 400. New support utilities under .repository.support: - SortedListCursor (base64url-no-pad Jackson JSON, legacy digit-only cursor falls through to the SCAN path by design) - FilterHasher (SHA-256 of canonical k=v|k=v|... over 8 filter fields, first 16 hex chars — collision-safe for server-side tuple detection) - ReservationComparators (per-key extractors with nullsLast, plus an extractSortValue helper used for cursor lsv encoding) Repository signature: RedisReservationRepository.listReservations now takes trailing sortBy, sortDir parameters (10 -> 12 args). The 10-arg overload was removed intentionally — keeping both caused Mockito stubs defined over 10 args to silently miss 12-arg call sites from the updated controller, producing null responses under test. Tests: - ReservationControllerTest: 4 new cases (invalid sort_by/sort_dir reject with 400, propagation to repo, all 7 spec enum values accepted). Pre-existing Mockito stubs updated to 12-arg signature. - ReservationComparatorsTest (new): all 7 primary sort keys, ASC/DESC, reservation_id tiebreaker, nullsLast across every key, null-safe extractSortValue, default-to-created_at_ms fallback. - SortedListCursorTest (new): round-trip, malformed/digit-only inputs (legacy SCAN passthrough), filter-hash stability across key ordering. - FilterHasherTest (new): determinism, null/empty equivalence, 16-hex shape. - RedisReservationQueryTest Testcontainers: new @nested sorted-path class covering 3-page pagination by created_at_ms ASC, cross-tuple cursor rejection (400), legacy numeric-cursor compatibility, scope_path lexicographic ordering. Wire-format invariance: omitting both params produces byte-identical responses to v0.1.25.11 (SCAN cursor, same has_more semantics). The sorted path is strictly additive.
1 parent 1f5909c commit bd31064

11 files changed

Lines changed: 1083 additions & 20 deletions

File tree

‎cycles-protocol-service/cycles-protocol-service-api/src/main/java/io/runcycles/protocol/api/controller/ReservationController.java‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,9 @@ public ResponseEntity<ReservationListResponse> list(
241241
@RequestParam(required = false) String agent,
242242
@RequestParam(required = false) String toolset,
243243
@RequestParam(defaultValue = "50") @Min(1) @Max(200) int limit,
244-
@RequestParam(required = false) String cursor) {
244+
@RequestParam(required = false) String cursor,
245+
@RequestParam(value = "sort_by", required = false) String sortBy,
246+
@RequestParam(value = "sort_dir", required = false) String sortDir) {
245247
// Validate status against ReservationStatus enum if provided
246248
if (status != null) {
247249
try {
@@ -251,6 +253,27 @@ public ResponseEntity<ReservationListResponse> list(
251253
"Invalid status filter: " + status + ". Must be one of: ACTIVE, COMMITTED, RELEASED, EXPIRED", 400);
252254
}
253255
}
256+
// v0.1.25.12 (cycles-protocol revision 2026-04-16): validate
257+
// sort_by / sort_dir at the controller boundary so clients get
258+
// a clean 400 INVALID_REQUEST on typos before the repo runs.
259+
// Enum values on the wire are lowercase (sort_by=created_at_ms);
260+
// uppercase them to match the Java enum constant.
261+
if (sortBy != null) {
262+
try {
263+
Enums.ReservationSortBy.valueOf(sortBy.toUpperCase());
264+
} catch (IllegalArgumentException e) {
265+
throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST,
266+
"Invalid sort_by: " + sortBy + ". Must be one of: reservation_id, tenant, scope_path, status, reserved, created_at_ms, expires_at_ms", 400);
267+
}
268+
}
269+
if (sortDir != null) {
270+
try {
271+
Enums.SortDirection.valueOf(sortDir.toUpperCase());
272+
} catch (IllegalArgumentException e) {
273+
throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST,
274+
"Invalid sort_dir: " + sortDir + ". Must be one of: asc, desc", 400);
275+
}
276+
}
254277
// v0.1.25.8 (cycles-protocol revision 2026-04-13): tenant param
255278
// semantics differ by auth type. ApiKeyAuth: optional, falls
256279
// back to authenticated tenant, validation-only when present.
@@ -271,8 +294,9 @@ public ResponseEntity<ReservationListResponse> list(
271294
effectiveTenant = tenant != null ? tenant : extractAuthTenantId();
272295
authorizeTenant(effectiveTenant);
273296
}
274-
LOG.info("GET /v1/reservations - tenant: {}, admin: {}", effectiveTenant, isAdminAuth());
297+
LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}",
298+
effectiveTenant, isAdminAuth(), sortBy, sortDir);
275299
return ResponseEntity.ok(repository.listReservations(effectiveTenant, idempotencyKey,
276-
status, workspace, app, workflow, agent, toolset, limit, cursor));
300+
status, workspace, app, workflow, agent, toolset, limit, cursor, sortBy, sortDir));
277301
}
278302
}

‎cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ void shouldListReservations() throws Exception {
498498
.reservations(Collections.emptyList())
499499
.hasMore(false)
500500
.build();
501-
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any()))
501+
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
502502
.thenReturn(resp);
503503

504504
mockMvc.perform(get("/v1/reservations"))
@@ -517,7 +517,7 @@ void shouldRejectInvalidStatusFilter() throws Exception {
517517
void shouldListWithActiveStatusFilter() throws Exception {
518518
ReservationListResponse resp = ReservationListResponse.builder()
519519
.reservations(Collections.emptyList()).hasMore(false).build();
520-
when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any()))
520+
when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
521521
.thenReturn(resp);
522522

523523
mockMvc.perform(get("/v1/reservations").param("status", "ACTIVE"))
@@ -529,7 +529,7 @@ void shouldListWithActiveStatusFilter() throws Exception {
529529
void shouldListWithCommittedStatusFilter() throws Exception {
530530
ReservationListResponse resp = ReservationListResponse.builder()
531531
.reservations(Collections.emptyList()).hasMore(false).build();
532-
when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any()))
532+
when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
533533
.thenReturn(resp);
534534

535535
mockMvc.perform(get("/v1/reservations").param("status", "COMMITTED"))
@@ -540,7 +540,7 @@ void shouldListWithCommittedStatusFilter() throws Exception {
540540
void shouldListWithExplicitTenantMatchingAuth() throws Exception {
541541
ReservationListResponse resp = ReservationListResponse.builder()
542542
.reservations(Collections.emptyList()).hasMore(false).build();
543-
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any()))
543+
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
544544
.thenReturn(resp);
545545

546546
mockMvc.perform(get("/v1/reservations").param("tenant", TENANT))
@@ -558,13 +558,63 @@ void shouldRejectListWithTenantMismatch() throws Exception {
558558
void shouldDefaultTenantFromAuth() throws Exception {
559559
ReservationListResponse resp = ReservationListResponse.builder()
560560
.reservations(Collections.emptyList()).hasMore(false).build();
561-
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any()))
561+
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
562562
.thenReturn(resp);
563563

564564
// No tenant param — should use auth tenant
565565
mockMvc.perform(get("/v1/reservations"))
566566
.andExpect(status().isOk());
567567
}
568+
569+
// v0.1.25.12 (cycles-protocol revision 2026-04-16): sort_by / sort_dir
570+
// query params on listReservations. Controller validates enum values;
571+
// invalid values MUST return 400 INVALID_REQUEST per spec.
572+
@Test
573+
@DisplayName("sort_by=bogus → 400 INVALID_REQUEST")
574+
void shouldRejectInvalidSortBy() throws Exception {
575+
mockMvc.perform(get("/v1/reservations").param("sort_by", "bogus"))
576+
.andExpect(status().isBadRequest())
577+
.andExpect(jsonPath("$.error").value("INVALID_REQUEST"))
578+
.andExpect(jsonPath("$.message").value(
579+
org.hamcrest.Matchers.containsString("Invalid sort_by")));
580+
}
581+
582+
@Test
583+
@DisplayName("sort_dir=sideways → 400 INVALID_REQUEST")
584+
void shouldRejectInvalidSortDir() throws Exception {
585+
mockMvc.perform(get("/v1/reservations").param("sort_dir", "sideways"))
586+
.andExpect(status().isBadRequest())
587+
.andExpect(jsonPath("$.error").value("INVALID_REQUEST"))
588+
.andExpect(jsonPath("$.message").value(
589+
org.hamcrest.Matchers.containsString("Invalid sort_dir")));
590+
}
591+
592+
@Test
593+
@DisplayName("sort_by=status&sort_dir=asc propagates to repository")
594+
void shouldPropagateSortParams() throws Exception {
595+
ReservationListResponse resp = ReservationListResponse.builder()
596+
.reservations(Collections.emptyList()).hasMore(false).build();
597+
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), eq("status"), eq("asc")))
598+
.thenReturn(resp);
599+
mockMvc.perform(get("/v1/reservations")
600+
.param("sort_by", "status")
601+
.param("sort_dir", "asc"))
602+
.andExpect(status().isOk());
603+
}
604+
605+
@Test
606+
@DisplayName("sort_by accepts all 7 spec-enum values case-insensitively")
607+
void shouldAcceptAllSpecSortByValues() throws Exception {
608+
ReservationListResponse resp = ReservationListResponse.builder()
609+
.reservations(Collections.emptyList()).hasMore(false).build();
610+
when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
611+
.thenReturn(resp);
612+
for (String value : new String[] {"reservation_id", "tenant", "scope_path",
613+
"status", "reserved", "created_at_ms", "expires_at_ms"}) {
614+
mockMvc.perform(get("/v1/reservations").param("sort_by", value))
615+
.andExpect(status().isOk());
616+
}
617+
}
568618
}
569619

570620
// v0.1.25.8 (cycles-protocol revision 2026-04-13): admin-on-behalf-of
@@ -583,7 +633,7 @@ void setAdminAuth() {
583633
void adminListWithTenantFilter() throws Exception {
584634
ReservationListResponse resp = ReservationListResponse.builder()
585635
.reservations(Collections.emptyList()).hasMore(false).build();
586-
when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), any()))
636+
when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any()))
587637
.thenReturn(resp);
588638
mockMvc.perform(get("/v1/reservations").param("tenant", "any-tenant"))
589639
.andExpect(status().isOk());

0 commit comments

Comments
 (0)