ITADN

Use rusts type system for special integer parameters

#135Pull Requestchikko80 创建于 2025-09-23
C
chikko80commented
I’ve recently started using this library and noticed that it doesn’t make good use of Rust’s type system to describe the API. I’ll probably be working on my own fork most of the time, since I plan to introduce more significant changes. However, I was wondering if this prerequisite change might also be of interest for the main API. If this is not the direction you want to take this project, please feel free to close this MR, or let me know how we could make it compatible. Please see the MR description below: ### Title Refactor sentinel integers to typed enums; update Admin API for topic creation ### Summary This PR replaces several raw integer “special values” with strongly typed enums and adjusts the Admin API to use a typed `NewTopic` structure. This improves correctness, readability, and discoverability, and prevents misuse of magic numbers. ### Motivation - Remove error-prone magic numbers (e.g., `-1`, `-2`) scattered across the protocol and builder APIs. - Use rusts type system to provide compile-time guarantees and clear semantics for Kafka protocol flags. - Make the admin topic creation API explicit and future-proof. ### Key Changes 1. Produce Required Acks - Added `prelude::RequiredAcks` enum: `None (0)`, `Leader (1)`, `All (-1)`. - Refactored: - `protocol::produce::request::ProduceRequest` field `required_acks: RequiredAcks`. - `producer::ProduceParams` and `producer::produce` signature. - `producer_builder::ProducerBuilder::required_acks(RequiredAcks)`. - Encoding implemented via `ToByte` mapping to the correct INT16 values. 2. List Offsets - Added `protocol::list_offsets::request::ReplicaId` enum: `NormalConsumer (-1) | Broker(i32)`. - Added `protocol::list_offsets::request::TimestampQuery` enum: `Latest (-1) | Earliest (-2) | At(i64)`. - Updated request struct fields and `.new/.add` signatures to use enums. - Updated `consumer_builder::list_offsets(...)` and related tests to use typed enums. 3. Create Topics - Added `protocol::create_topics::request::NumPartitions`: `Default (-1) | Count(i32)`. - Added `protocol::create_topics::request::ReplicationFactor`: `Default (-1) | Factor(i16)`. - Updated `protocol::create_topics::request::Topic` and `.add(...)` to accept these enums. - Introduced `protocol::create_topics::NewTopic` (in `mod.rs`) with fields: - `topic_name: String` - `num_partitions: request::NumPartitions` - `replication_factor: request::ReplicationFactor` - Updated Admin helper `admin::create_topics` signature to accept `Vec<NewTopic>` and pass through values. - Re-exported `NewTopic` in `prelude` for convenience. ### Affected Files (high-level) - `src/lib.rs`: re-export `RequiredAcks`, `NewTopic` and expose protocol modules in prelude. - `src/producer.rs`, `src/producer_builder.rs`, `src/protocol/produce/request.rs`. - `src/protocol/list_offsets/request.rs`, `src/consumer_builder.rs`, `src/protocol/list_offsets/mod.rs` (tests). - `src/protocol/create_topics/request.rs`, `src/protocol/create_topics/mod.rs` (NewTopic), `src/admin.rs`. - Tests and benches updated accordingly. ### Backward Compatibility / Breaking Changes - Breaking: - `ProducerBuilder::required_acks(i16)` → `ProducerBuilder::required_acks(RequiredAcks)`. - `producer::produce(..., required_acks: i16, ...)` → `required_acks: RequiredAcks`. - `protocol::ProduceRequest::new(required_acks: i16, ...)` → `RequiredAcks`. - `protocol::ListOffsetsRequest::new(..., replica_id: i32)` → `ReplicaId`. - `protocol::ListOffsetsRequest::add(..., timestamp: i64)` → `TimestampQuery`. - `protocol::create_topics::request::Topic` fields and `.add(...)` now use enums. - `admin::create_topics(..., HashMap<&str, i32>)` → `create_topics(..., Vec<NewTopic>)`. ### Migration Guide - Produce: - Replace `.required_acks(1)` with `.required_acks(RequiredAcks::Leader)`. - Update direct `produce(...)` calls to pass `RequiredAcks`. - List Offsets: - Use `ReplicaId::NormalConsumer` or `ReplicaId::Broker(id)`. - Use `TimestampQuery::{Latest, Earliest, At(ms)}` instead of raw `i64`. - Create Topics / Admin: - Replace `HashMap<&str, i32>` with `Vec<NewTopic>`: ```rust use samsa::prelude::protocol::create_topics::request::{NumPartitions, ReplicationFactor}; use samsa::prelude::NewTopic; let topics = vec![NewTopic { topic_name: "my-topic".into(), num_partitions: NumPartitions::Count(3), replication_factor: ReplicationFactor::Factor(1), }]; create_topics(conn, corr_id, client_id, topics).await?; ``` ### Testing - Updated unit/integration tests and benches to use enums. - Ran `cargo check --tests` successfully. ### Rationale & Alternatives - Enums make the protocol intent explicit and prevent misuse. - We intentionally mirrored Kafka protocol sentinel values; encoding maps variants to the expected wire values. - Alternative considered: keep integers and add helper constants. Rejected due to lower type-safety and ergonomics. ### Checklist - [x] Compile-time safety for sentinel values - [x] Updated docs and examples - [x] Tests updated
合并状态:未合并 1 条评论