krishvishal commented on code in PR #3733:
URL: https://github.com/apache/iggy/pull/3733#discussion_r3728720681
##########
foreign/cpp/src/client.rs:
##########
@@ -73,37 +75,85 @@ pub struct Client {
/// (use-after-free).
/// - This function does not provide synchronisation. The pointer must not be
used concurrently
/// from multiple threads unless the caller serialises access externally.
-pub fn new_connection(connection_string: String) -> Result<*mut Client,
String> {
- let connection_str = connection_string.as_str();
- let client = match connection_str {
- "" => RustIggyClientBuilder::new()
- .with_tcp()
- .build()
- .map_err(|error| format!("Could not build default connection:
{error}"))?,
- s if s.starts_with("iggy://") || s.starts_with("iggy+") => {
- RustIggyClient::from_connection_string(s)
- .map_err(|error| format!("Could not parse connection string
'{s}': {error}"))?
+pub fn new_connection(config: ffi::IggyClientConfig) -> Result<*mut Client,
String> {
+ let mut builder = RustIggyClientBuilder::new().with_tcp();
+ if !config.server_address.is_empty() {
+ builder = builder.with_server_address(config.server_address);
+ }
+ match config.auto_login_kind.as_str() {
+ "" | "disabled" => {}
+ "username_password" => {
+ builder = builder.with_auto_sign_in(RustAutoLogin::Enabled(
+ RustCredentials::UsernamePassword(config.username,
config.password.into()),
+ ));
+ }
+ "personal_access_token" => {
+ builder = builder.with_auto_sign_in(RustAutoLogin::Enabled(
+
RustCredentials::PersonalAccessToken(config.personal_access_token.into()),
+ ));
}
- s => RustIggyClientBuilder::new()
- .with_tcp()
- .with_server_address(connection_string.clone())
- .build()
- .map_err(|error| format!("Could not build connection for address
'{s}': {error}"))?,
- };
+ _ => return Err("Unsupported automatic login kind".to_owned()),
+ }
+ if config.set_reconnection_max_retries {
+ builder = builder.with_reconnection_max_retries(
+ config
+ .has_reconnection_max_retries
+ .then_some(config.reconnection_max_retries),
+ );
+ }
+ if config.has_reconnection_interval {
+ builder = builder.with_reconnection_interval(RustIggyDuration::from(
+ config.reconnection_interval_micros,
+ ));
+ }
+ if config.has_reestablish_after {
+ builder =
+
builder.with_reestablish_after(RustIggyDuration::from(config.reestablish_after_micros));
+ }
+ if config.has_tls_enabled {
+ builder = builder.with_tls_enabled(config.tls_enabled);
+ if config.tls_enabled {
+ if !config.tls_domain.is_empty() {
+ builder = builder.with_tls_domain(config.tls_domain);
+ }
+ if !config.tls_ca_file.is_empty() {
+ builder = builder.with_tls_ca_file(config.tls_ca_file);
+ }
+ if config.has_tls_validate_certificate {
+ builder =
builder.with_tls_validate_certificate(config.tls_validate_certificate);
+ }
+ }
+ }
Review Comment:
`has_tls_enabled`: `Builder().WithTlsDomain(d).WithTlsCaFile(p).Build()`
without `WithTlsEnabled(true)` silently drops all three TLS settings and
connects in plaintext. The gate carries no information either, since
`TcpClientConfig::default()` is already `tls_enabled: false`. Suggest deleting
the field, calling `with_tls_enabled` unconditionally, and erroring when TLS
material is set with TLS off. - `set_reconnection_max_retries`
(`src/lib.rs:348`): dead. "Never set" and `WithoutReconnectionLimit()` are
indistinguishable, and `{set_=false, has_=true, value=n}` is representable and
silently ignored. `has_reconnection_max_retries` alone expresses `Option<u32>`.
--
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]
