ethanlin01x commented on code in PR #3733:
URL: https://github.com/apache/iggy/pull/3733#discussion_r3704718784


##########
foreign/cpp/include/iggy.hpp:
##########
@@ -252,4 +333,425 @@ class PollingStrategy final {
     std::uint64_t polling_strategy_value_;
 };
 
+/**
+ * @brief Exception thrown when an Iggy client operation fails.
+ */
+class IggyException : public std::runtime_error {
+  public:
+    explicit IggyException(const char *message) : std::runtime_error(message) 
{}
+    explicit IggyException(const std::string &message) : 
std::runtime_error(message) {}
+};
+
+/**
+ * @brief Owning client connection to an Apache Iggy server.
+ *
+ * Create instances with Builder or FromConnectionString(). The client owns the
+ * underlying Rust-backed connection and releases it when destroyed.
+ *
+ * Builder initializes a TCP client. To use QUIC, HTTP, or WebSocket, create 
the
+ * client with FromConnectionString().
+ *
+ * @code{.cpp}
+ * auto client = iggy::IggyBlockingClient::Builder()
+ *                   .WithServerAddress("127.0.0.1:8090")
+ *                   .Build();
+ * client.Connect();
+ * client.Login("iggy", "iggy");
+ * @endcode
+ */
+class IggyBlockingClient final {
+  public:
+    class Builder;
+
+    /** @brief IggyBlockingClient is move-only. */
+    IggyBlockingClient(const IggyBlockingClient &)            = delete;
+    IggyBlockingClient &operator=(const IggyBlockingClient &) = delete;
+
+    /**
+     * @brief Transfers ownership of a client.
+     * @param other Client whose connection ownership is transferred.
+     *
+     * The moved-from client may be destroyed or assigned a new value, but must
+     * not be used for client operations.
+     */
+    IggyBlockingClient(IggyBlockingClient &&other) noexcept;
+
+    /**
+     * @brief Replaces this client by taking ownership from another client.
+     * @param other Client whose connection ownership is transferred.
+     * @return Reference to this client.
+     *
+     * Any client currently owned by this object is released first. The
+     * moved-from client must not be used for client operations.
+     */
+    IggyBlockingClient &operator=(IggyBlockingClient &&other) noexcept;
+
+    /**
+     * @brief Releases the underlying Rust client.
+     *
+     * Cleanup errors cannot be reported from the destructor. Call Shutdown()
+     * explicitly when graceful transport shutdown must be observed.
+     */
+    ~IggyBlockingClient();
+
+    /**
+     * @brief Creates a client from an Iggy connection string.
+     *
+     * Connection strings use one of these forms:
+     *
+     * - `iggy://<credentials>@<host>:<port>[?<options>]` for TCP.
+     * - `iggy+tcp://<credentials>@<host>:<port>[?<options>]` for TCP.
+     * - `iggy+quic://<credentials>@<host>:<port>[?<options>]` for QUIC.
+     * - `iggy+http://<credentials>@<host>:<port>[?<options>]` for HTTP.
+     * - `iggy+ws://<credentials>@<host>:<port>[?<options>]` for WebSocket.
+     *
+     * Credentials are either `<username>:<password>` or a personal access
+     * token. Multiple query parameters are separated with `&`.
+     *
+     * Connection string examples:
+     *
+     * - Username and password:
+     *   `iggy+tcp://iggy:[email protected]:8090`
+     * - Personal access token:
+     *   `iggy+tcp://[email protected]:8090`
+     * - TCP with TLS:
+     *   `iggy+tcp://iggy:iggy@localhost:8090?tls=true&tls_domain=localhost`
+     *
+     * TCP accepts these query parameters:
+     *
+     * - `tls=<bool>`
+     * - `tls_domain=<string>`
+     * - `tls_ca_file=<path>`
+     * - `reconnection_retries=<uint32|unlimited>`
+     * - `reconnection_interval=<duration>`
+     * - `reestablish_after=<duration>`
+     * - `heartbeat_interval=<duration>`
+     * - `nodelay=<bool>`
+     *
+     * QUIC accepts these query parameters:
+     *
+     * - `response_buffer_size=<uint64>`
+     * - `max_concurrent_bidi_streams=<uint64>`
+     * - `datagram_send_buffer_size=<uint64>`
+     * - `initial_mtu=<uint16>`
+     * - `send_window=<uint64>`
+     * - `receive_window=<uint64>`
+     * - `keep_alive_interval=<uint64>`
+     * - `max_idle_timeout=<uint64>`
+     * - `validate_certificate=<bool>`
+     * - `heartbeat_interval=<duration>`
+     * - `reconnection_max_retries=<uint32|unlimited>`
+     * - `reconnection_interval=<duration>`
+     * - `reconnection_reestablish_after=<duration>`
+     *
+     * HTTP accepts these query parameters:
+     *
+     * - `heartbeat_interval=<duration>`
+     * - `retries=<uint32>`
+     *
+     * WebSocket accepts these query parameters:
+     *
+     * - `heartbeat_interval=<duration>`
+     * - `reconnection_retries=<uint32|unlimited>`
+     * - `reconnection_interval=<duration>`
+     * - `reestablish_after=<duration>`
+     * - `read_buffer_size=<unsigned integer>`
+     * - `write_buffer_size=<unsigned integer>`
+     * - `max_write_buffer_size=<unsigned integer>`
+     * - `max_message_size=<unsigned integer>`
+     * - `max_frame_size=<unsigned integer>`
+     * - `accept_unmasked_frames=<bool>`
+     * - `tls=<bool>`
+     * - `tls_domain=<string>`
+     * - `tls_ca_file=<path>`
+     * - `tls_validate_certificate=<bool>`
+     *
+     * Durations use Iggy duration syntax, such as `500ms`, `5s`, or `1min`.
+     * Boolean values are `true` or `false`.
+     *
+     * Credentials embedded in the connection string configure automatic login
+     * for Connect() and later reconnections. This method parses configuration
+     * but does not establish a network connection.
+     *
+     * @param connection_string Connection string containing client 
configuration.
+     * @return Configured, disconnected client.
+     * @throws IggyException if the connection string is invalid or the client
+     *         cannot be created.
+     */
+    static IggyBlockingClient FromConnectionString(std::string 
connection_string);
+
+    /**
+     * @brief Connects to the configured Iggy server.
+     *
+     * Establishes the configured transport connection and starts heartbeat
+     * processing. If automatic login was configured, authentication is also
+     * performed. Calling this on an already connected client has no effect.
+     *
+     * @note HTTP is stateless; connecting initializes heartbeat processing but
+     *       does not open a persistent transport connection.
+     * @throws IggyException if the connection or automatic authentication
+     *         fails.
+     */
+    void Connect() const;

Review Comment:
   Nit: These methods are `const` but clearly mutate connection state (it only 
compiles because the state lives behind `client_`). Marking them non-const 
would better communicate the semantics, since `const` usually implies no 
observable state change and often thread-safety.
   
   Applies to:
   - `Connect()`
   - `Disconnect()`
   - `Shutdown()`
   - `Login()`
   - `Logout()`



##########
foreign/cpp/include/iggy.hpp:
##########
@@ -252,4 +333,425 @@ class PollingStrategy final {
     std::uint64_t polling_strategy_value_;
 };
 
+/**
+ * @brief Exception thrown when an Iggy client operation fails.
+ */
+class IggyException : public std::runtime_error {
+  public:
+    explicit IggyException(const char *message) : std::runtime_error(message) 
{}
+    explicit IggyException(const std::string &message) : 
std::runtime_error(message) {}
+};
+
+/**
+ * @brief Owning client connection to an Apache Iggy server.
+ *
+ * Create instances with Builder or FromConnectionString(). The client owns the
+ * underlying Rust-backed connection and releases it when destroyed.
+ *
+ * Builder initializes a TCP client. To use QUIC, HTTP, or WebSocket, create 
the
+ * client with FromConnectionString().
+ *
+ * @code{.cpp}
+ * auto client = iggy::IggyBlockingClient::Builder()
+ *                   .WithServerAddress("127.0.0.1:8090")
+ *                   .Build();
+ * client.Connect();
+ * client.Login("iggy", "iggy");
+ * @endcode
+ */
+class IggyBlockingClient final {
+  public:
+    class Builder;
+
+    /** @brief IggyBlockingClient is move-only. */
+    IggyBlockingClient(const IggyBlockingClient &)            = delete;
+    IggyBlockingClient &operator=(const IggyBlockingClient &) = delete;
+
+    /**
+     * @brief Transfers ownership of a client.
+     * @param other Client whose connection ownership is transferred.
+     *
+     * The moved-from client may be destroyed or assigned a new value, but must
+     * not be used for client operations.
+     */
+    IggyBlockingClient(IggyBlockingClient &&other) noexcept;
+
+    /**
+     * @brief Replaces this client by taking ownership from another client.
+     * @param other Client whose connection ownership is transferred.
+     * @return Reference to this client.
+     *
+     * Any client currently owned by this object is released first. The
+     * moved-from client must not be used for client operations.
+     */
+    IggyBlockingClient &operator=(IggyBlockingClient &&other) noexcept;
+
+    /**
+     * @brief Releases the underlying Rust client.
+     *
+     * Cleanup errors cannot be reported from the destructor. Call Shutdown()
+     * explicitly when graceful transport shutdown must be observed.
+     */
+    ~IggyBlockingClient();
+
+    /**
+     * @brief Creates a client from an Iggy connection string.
+     *
+     * Connection strings use one of these forms:
+     *
+     * - `iggy://<credentials>@<host>:<port>[?<options>]` for TCP.
+     * - `iggy+tcp://<credentials>@<host>:<port>[?<options>]` for TCP.
+     * - `iggy+quic://<credentials>@<host>:<port>[?<options>]` for QUIC.
+     * - `iggy+http://<credentials>@<host>:<port>[?<options>]` for HTTP.
+     * - `iggy+ws://<credentials>@<host>:<port>[?<options>]` for WebSocket.
+     *
+     * Credentials are either `<username>:<password>` or a personal access
+     * token. Multiple query parameters are separated with `&`.
+     *
+     * Connection string examples:
+     *
+     * - Username and password:
+     *   `iggy+tcp://iggy:[email protected]:8090`
+     * - Personal access token:
+     *   `iggy+tcp://[email protected]:8090`
+     * - TCP with TLS:
+     *   `iggy+tcp://iggy:iggy@localhost:8090?tls=true&tls_domain=localhost`
+     *
+     * TCP accepts these query parameters:
+     *
+     * - `tls=<bool>`
+     * - `tls_domain=<string>`
+     * - `tls_ca_file=<path>`
+     * - `reconnection_retries=<uint32|unlimited>`
+     * - `reconnection_interval=<duration>`
+     * - `reestablish_after=<duration>`
+     * - `heartbeat_interval=<duration>`
+     * - `nodelay=<bool>`
+     *
+     * QUIC accepts these query parameters:
+     *
+     * - `response_buffer_size=<uint64>`
+     * - `max_concurrent_bidi_streams=<uint64>`
+     * - `datagram_send_buffer_size=<uint64>`
+     * - `initial_mtu=<uint16>`
+     * - `send_window=<uint64>`
+     * - `receive_window=<uint64>`
+     * - `keep_alive_interval=<uint64>`
+     * - `max_idle_timeout=<uint64>`
+     * - `validate_certificate=<bool>`
+     * - `heartbeat_interval=<duration>`
+     * - `reconnection_max_retries=<uint32|unlimited>`
+     * - `reconnection_interval=<duration>`
+     * - `reconnection_reestablish_after=<duration>`
+     *
+     * HTTP accepts these query parameters:
+     *
+     * - `heartbeat_interval=<duration>`
+     * - `retries=<uint32>`
+     *
+     * WebSocket accepts these query parameters:
+     *
+     * - `heartbeat_interval=<duration>`
+     * - `reconnection_retries=<uint32|unlimited>`
+     * - `reconnection_interval=<duration>`
+     * - `reestablish_after=<duration>`
+     * - `read_buffer_size=<unsigned integer>`
+     * - `write_buffer_size=<unsigned integer>`
+     * - `max_write_buffer_size=<unsigned integer>`
+     * - `max_message_size=<unsigned integer>`
+     * - `max_frame_size=<unsigned integer>`
+     * - `accept_unmasked_frames=<bool>`
+     * - `tls=<bool>`
+     * - `tls_domain=<string>`
+     * - `tls_ca_file=<path>`
+     * - `tls_validate_certificate=<bool>`
+     *
+     * Durations use Iggy duration syntax, such as `500ms`, `5s`, or `1min`.
+     * Boolean values are `true` or `false`.
+     *
+     * Credentials embedded in the connection string configure automatic login
+     * for Connect() and later reconnections. This method parses configuration
+     * but does not establish a network connection.
+     *
+     * @param connection_string Connection string containing client 
configuration.
+     * @return Configured, disconnected client.
+     * @throws IggyException if the connection string is invalid or the client
+     *         cannot be created.
+     */
+    static IggyBlockingClient FromConnectionString(std::string 
connection_string);
+
+    /**
+     * @brief Connects to the configured Iggy server.
+     *
+     * Establishes the configured transport connection and starts heartbeat
+     * processing. If automatic login was configured, authentication is also
+     * performed. Calling this on an already connected client has no effect.
+     *
+     * @note HTTP is stateless; connecting initializes heartbeat processing but
+     *       does not open a persistent transport connection.
+     * @throws IggyException if the connection or automatic authentication
+     *         fails.
+     */
+    void Connect() const;
+
+    /**
+     * @brief Disconnects from the configured Iggy server.
+     *
+     * Disconnect is temporary. It drops the active transport connection and
+     * changes the client state to disconnected, but keeps the client reusable.
+     * Call Connect() to establish a new connection. Configured automatic login
+     * is applied when reconnecting.
+     *
+     * @note The HTTP transport is stateless and treats this operation as a
+     *       no-op.
+     * @throws IggyException if the client cannot disconnect cleanly.
+     * @see Shutdown()
+     */
+    void Disconnect() const;
+
+    /**
+     * @brief Shuts down the client and its background tasks.
+     *
+     * Shutdown is terminal for stateful transports. It gracefully closes the
+     * active transport where supported, releases transport resources, and
+     * changes the client state to shutdown. Binary operations then fail with a
+     * client-shutdown error, which also causes the background heartbeat task 
to
+     * stop. Create a new client instead of reusing a shut-down client.
+     *
+     * @note The HTTP transport is stateless and treats this operation as a
+     *       no-op.
+     * @throws IggyException if shutdown fails.
+     * @see Disconnect()
+     */
+    void Shutdown() const;
+
+    /**
+     * @brief Authenticates with a username and password.
+     *
+     * For TCP, QUIC, and WebSocket, call Connect() first. A successful login
+     * leaves the transport connected and marks the session authenticated. For
+     * HTTP, the returned access token is stored by the client and used for
+     * subsequent authenticated requests.
+     *
+     * @param username Iggy user name.
+     * @param password Iggy user password.
+     * @return Information about the authenticated session.
+     * @throws IggyException if authentication fails.
+     */
+    LoginInfo Login(std::string username, std::string password) const;

Review Comment:
   `Login()` returning `ffi::LoginInfo` leaks the FFI's `has_access_token` 
bool-flag workaround into the high-level API. Since this is meant to be the 
primary API, a dedicated type with `std::optional` for the token would be 
cleaner, and avoids a breaking change later.



##########
foreign/cpp/src/client.rs:
##########
@@ -1085,6 +1135,90 @@ impl Client {
         })
     }
 
+    pub fn get_user(&self, user_id: ffi::Identifier) -> 
Result<ffi::UserInfoDetails, String> {
+        let rust_user_id = RustIdentifier::try_from(user_id)
+            .map_err(|error| format!("Could not get user: invalid user 
identifier: {error}"))?;
+
+        RUNTIME.block_on(async {
+            let user = self
+                .inner
+                .get_user(&rust_user_id)
+                .await
+                .map_err(|error| format!("Could not get user '{rust_user_id}': 
{error}"))?;
+            ffi::UserInfoDetails::try_from(user)
+                .map_err(|error| format!("Could not get user '{rust_user_id}': 
{error}"))
+        })
+    }
+
+    pub fn get_users(&self) -> Result<Vec<ffi::UserInfo>, String> {
+        RUNTIME.block_on(async {
+            let users = self
+                .inner
+                .get_users()
+                .await
+                .map_err(|error| format!("Could not get users: {error}"))?;
+            Ok(users.into_iter().map(ffi::UserInfo::from).collect())
+        })
+    }
+
+    pub fn create_user(
+        &self,
+        username: String,
+        password: String,
+        status: u8,
+        has_permissions: bool,
+        permissions: ffi::Permissions,
+    ) -> Result<ffi::UserInfoDetails, String> {
+        let rust_status = RustUserStatus::from_code(status)
+            .map_err(|error| format!("Could not create user '{username}': 
{error}"))?;
+        let rust_permissions = has_permissions
+            .then(|| RustPermissions::try_from(permissions))
+            .transpose()
+            .map_err(|error| format!("Could not create user '{username}': 
{error}"))?;
+
+        RUNTIME.block_on(async {
+            let user = self
+                .inner
+                .create_user(&username, &password, rust_status, 
rust_permissions)
+                .await
+                .map_err(|error| format!("Could not create user '{username}': 
{error}"))?;
+            Ok(ffi::UserInfoDetails::from(user))
+        })
+    }
+
+    pub fn delete_user(&self, user_id: ffi::Identifier) -> Result<(), String> {
+        let rust_user_id = RustIdentifier::try_from(user_id)
+            .map_err(|error| format!("Could not delete user: invalid user 
identifier: {error}"))?;
+
+        RUNTIME.block_on(async {
+            self.inner
+                .delete_user(&rust_user_id)
+                .await
+                .map_err(|error| format!("Could not delete user 
'{rust_user_id}': {error}"))?;
+            Ok(())
+        })
+    }
+
+    pub fn update_user(

Review Comment:
   Rust SDK allows updating username and status independently (Option params), 
but this forces both. Consider `has_username` / `has_status` flags like the 
existing `has_permissions` pattern



-- 
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]

Reply via email to