fresh-borzoni commented on code in PR #531:
URL: https://github.com/apache/fluss-rust/pull/531#discussion_r3212827427
##########
crates/fluss/src/rpc/server_connection.rs:
##########
@@ -1030,4 +1171,126 @@ mod tests {
"in-flight gauge must return to zero after API error"
);
}
+
+ #[tokio::test]
+ async fn server_api_versions_negotiation() {
+ assert_eq!(
+ resolve_api_version_for(None, ApiKey::ApiVersion).unwrap(),
+ ApiVersion(0)
+ );
+
+ assert_eq!(
+ resolve_api_version_for(None, ApiKey::PutKv).unwrap(),
+ ApiVersion(0)
+ );
+
+ let server_versions = vec![
+ // PutKv: server v0..v3, client v0 only (v1 key encoding not yet
implemented) → negotiated v0
+ PbApiVersion {
+ api_key: 1016,
+ min_version: 0,
+ max_version: 3,
+ },
+ // ProduceLog: server v0..v2, client v0 only → negotiated v0
+ PbApiVersion {
+ api_key: 1014,
+ min_version: 0,
+ max_version: 2,
+ },
+ // Disjoint: server v5..v7, client v0 only → error
+ PbApiVersion {
+ api_key: 1015,
+ min_version: 5,
+ max_version: 7,
+ },
+ // Unknown key (9999) → skipped
+ PbApiVersion {
+ api_key: 9999,
+ min_version: 0,
+ max_version: 5,
+ },
+ ];
+ let negotiated = ServerApiVersions::new(&server_versions);
+
+ // Successful negotiation cases
+ assert_eq!(
+ negotiated.highest_available_version(ApiKey::PutKv).unwrap(),
+ ApiVersion(0)
+ );
+ assert_eq!(
+ negotiated
+ .highest_available_version(ApiKey::ProduceLog)
+ .unwrap(),
+ ApiVersion(0)
+ );
+
+ // Disjoint range → error
+ assert!(
+ negotiated
+ .highest_available_version(ApiKey::FetchLog)
+ .unwrap_err()
+ .to_string()
+ .contains(&format!(
+ "The server does not support {:?}",
+ ApiKey::FetchLog
+ ))
+ );
+
+ // Unknown key is skipped → not in map → error
+ assert!(
+ negotiated
+ .highest_available_version(ApiKey::Unknown(9999))
+ .is_err()
+ );
+
+ // Key not advertised by server → error
+ assert!(
+ ServerApiVersions::new(&[])
+ .highest_available_version(ApiKey::FetchLog)
+ .is_err()
+ );
+ }
+
+ #[test]
+ fn server_type_validation() {
+ // Happy path: server advertises the expected type.
+ assert!(
+ validate_server_type(
+ &ServerType::CoordinatorServer,
+ Some(ServerType::CoordinatorServer.to_type_id()),
+ )
+ .is_ok()
+ );
+ assert!(
+ validate_server_type(
+ &ServerType::TabletServer,
+ Some(ServerType::TabletServer.to_type_id()),
+ )
+ .is_ok()
+ );
+
+ // Mismatch: connected to a coordinator while expecting a tablet server
+ // (and vice versa).
+ let err = validate_server_type(
+ &ServerType::TabletServer,
+ Some(ServerType::CoordinatorServer.to_type_id()),
+ )
+ .unwrap_err();
+ assert!(
+ matches!(err, Error::InvalidServerType { .. }),
+ "expected InvalidServerType, got: {err:?}"
+ );
+
+ assert!(matches!(
+ validate_server_type(
+ &ServerType::CoordinatorServer,
+ Some(ServerType::TabletServer.to_type_id()),
+ ),
+ Err(Error::InvalidServerType { .. })
+ ));
+
+ // Unknown / unmapped type id still fails, with the raw id surfaced so
Review Comment:
None is the skip case, not the unknown case. Probably we want two
assertions: one for None (returns Ok) and one for Some(99) (returns Err)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
