Skip to content

feat: add MaxKeys typed parameter with validation and ergonomic API - #212

Merged
harshavardhana merged 1 commit into
minio:masterfrom
HJLebbink:feat/issue-211-max-keys-validation
May 27, 2026
Merged

feat: add MaxKeys typed parameter with validation and ergonomic API#212
harshavardhana merged 1 commit into
minio:masterfrom
HJLebbink:feat/issue-211-max-keys-validation

Conversation

@HJLebbink

@HJLebbink HJLebbink commented Apr 28, 2026

Copy link
Copy Markdown
Member

feat: add MaxKeys typed parameter with validation and ergonomic API

Implements client-side validation for the max_keys parameter used in
ListObjects API calls, replacing implicit panics with explicit ValidationErr
handling. This ensures invalid user input is caught early with clear error
messages rather than causing runtime panics.

Key improvements:

  • Adds MaxKeys typed parameter with validation (range: 1-1000) that matches
    MinIO EOS hard limits
  • Provides ergonomic builder API: .max_keys(500) instead of requiring
    .max_keys(MaxKeys::new(500)?)
  • Defers validation to request building time via to_s3request(), allowing
    graceful error propagation instead of panicking in From
  • Updates client documentation to reflect client-side validation behavior
  • Adds 17 comprehensive unit tests covering boundary conditions, parsing,
    and conversions
  • Fixes error message redundancy to match project patterns
  • Relaxes integration test assertions to respect max_keys as an upper bound,
    preventing flakiness across S3 implementations
  • Removes dead code (unused MAX_KEYS_TESTING.md, new_unchecked())

The builder now accepts max_keys values via multiple ergonomic paths:

  • .max_keys(500) - accepts u16 directly
  • .max_keys(MaxKeys::new(500)?) - explicit pre-validation
  • .max_keys(None) - use server default (1000)

Validation ensures compliance with MinIO/S3 constraints while maintaining
clean error handling throughout the request lifecycle.

@HJLebbink
HJLebbink requested a review from Copilot April 28, 2026 12:00
@HJLebbink HJLebbink self-assigned this Apr 28, 2026
@HJLebbink HJLebbink added the enhancement Used in release doc generation label Apr 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a typed MaxKeys parameter (1–1000) for ListObjects APIs, updates the list-objects builder/client docs accordingly, and introduces tests/docs to validate pagination behavior.

Changes:

  • Introduce MaxKeys typed parameter with range validation and conversions (parsing/display).
  • Update ListObjects builder internals to accept ergonomic .max_keys(…) input and send validated max-keys query params.
  • Add integration/unit tests plus a documentation note for testing max_keys pagination behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/s3/types/typed_parameters.rs Adds the validated MaxKeys type and related conversions.
src/s3/error.rs Adds ValidationErr::InvalidMaxKeys for client-side validation failures.
src/s3/types/mod.rs Re-exports MaxKeys from types.
src/s3/builders/list_objects.rs Switches list-objects query building to use MaxKeys; adds an ergonomic wrapper type and unit tests.
src/s3/client/list_objects.rs Expands API docs explaining max_keys paging semantics/limits.
tests/s3/list_objects.rs Adds integration tests covering pagination with max_keys and the max_keys=1 edge case.
docs/MAX_KEYS_TESTING.md New doc describing how to test and reason about max_keys paging behavior.

Comment thread src/s3/builders/list_objects.rs
Comment thread src/s3/builders/list_objects.rs Outdated
Comment thread docs/MAX_KEYS_TESTING.md Outdated
Comment thread src/s3/client/list_objects.rs Outdated
Comment thread tests/s3/list_objects.rs
Comment thread docs/MAX_KEYS_TESTING.md Outdated
Comment thread docs/MAX_KEYS_TESTING.md Outdated
Comment thread src/s3/types/typed_parameters.rs Outdated
Comment thread docs/MAX_KEYS_TESTING.md Outdated
@HJLebbink
HJLebbink force-pushed the feat/issue-211-max-keys-validation branch from d77c019 to 1b80eb0 Compare April 28, 2026 20:30
@HJLebbink
HJLebbink requested a review from Copilot April 28, 2026 20:31

This comment was marked as resolved.

@HJLebbink
HJLebbink force-pushed the feat/issue-211-max-keys-validation branch 3 times, most recently from 01f1787 to 07c91f7 Compare April 28, 2026 20:48
@HJLebbink
HJLebbink requested a review from Copilot April 28, 2026 20:48
@HJLebbink
HJLebbink force-pushed the feat/issue-211-max-keys-validation branch from 07c91f7 to 0f15234 Compare April 28, 2026 20:54

This comment was marked as resolved.

@HJLebbink
HJLebbink force-pushed the feat/issue-211-max-keys-validation branch from 0f15234 to 7b46669 Compare April 29, 2026 09:12
  Implements client-side validation for the max_keys parameter used in
  ListObjects API calls, replacing implicit panics with explicit ValidationErr
  handling. This ensures invalid user input is caught early with clear error
  messages rather than causing runtime panics.

  Key improvements:

  - Adds MaxKeys typed parameter with validation (range: 1-1000) that matches
    MinIO EOS hard limits
  - Provides ergonomic builder API: .max_keys(500) instead of requiring
    .max_keys(MaxKeys::new(500)?)
  - Defers validation to request building time via to_s3request(), allowing
    graceful error propagation instead of panicking in From<u16>
  - Updates client documentation to reflect client-side validation behavior
  - Adds 17 comprehensive unit tests covering boundary conditions, parsing,
    and conversions
  - Fixes error message redundancy to match project patterns
  - Relaxes integration test assertions to respect max_keys as an upper bound,
    preventing flakiness across S3 implementations
  - Removes dead code (unused MAX_KEYS_TESTING.md, new_unchecked())

  The builder now accepts max_keys values via multiple ergonomic paths:
  - .max_keys(500) - accepts u16 directly
  - .max_keys(MaxKeys::new(500)?) - explicit pre-validation
  - .max_keys(None) - use server default (1000)

  Validation ensures compliance with MinIO/S3 constraints while maintaining
  clean error handling throughout the request lifecycle.
@HJLebbink
HJLebbink force-pushed the feat/issue-211-max-keys-validation branch from 7b46669 to d7472f2 Compare May 9, 2026 15:05
@harshavardhana
harshavardhana merged commit f56a08e into minio:master May 27, 2026
6 checks passed
@HJLebbink
HJLebbink deleted the feat/issue-211-max-keys-validation branch May 28, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Used in release doc generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants