github-actions[bot] commented on code in PR #4397:
URL: https://github.com/apache/iggy/pull/4397#discussion_r4437898608


##########
foreign/cpp/src/type_conversions.cpp:
##########
@@ -849,4 +849,37 @@ MetadataChanges 
MetadataChanges::FromFfi(ffi::MetadataChanges changes) {
     return result;
 }
 
+SendMessagesConfirmation 
SendMessagesConfirmation::FromFfi(ffi::SendMessagesConfirmation confirmation) {
+    return SendMessagesConfirmation(confirmation.stream_id, 
confirmation.topic_id, confirmation.partition_id,
+                                    confirmation.base_offset);
+}
+
+SendMessagesResponse SendMessagesResponse::FromFfi(const 
ffi::SendMessagesResponse &response) {
+    std::vector<SendMessagesConfirmation> confirmations;
+    confirmations.reserve(response.confirmations.size());
+    for (const auto &confirmation : response.confirmations) {
+        
confirmations.push_back(SendMessagesConfirmation::FromFfi(confirmation));
+    }
+    return SendMessagesResponse(std::move(confirmations));
+}
+
+TransportEndpoints TransportEndpoints::FromFfi(ffi::TransportEndpoints 
endpoints) {
+    return TransportEndpoints(endpoints.tcp, endpoints.quic, endpoints.http, 
endpoints.websocket);
+}
+
+ClusterNode ClusterNode::FromFfi(ffi::ClusterNode node) {

Review Comment:
   warning: `ClusterNode::FromFfi()` drops `ffi::ClusterNode::replica_id`, so a 
`GetClusterMetadata()` caller cannot learn the ID that 
`SendBinaryRequestToReplica()` takes. Expose `ReplicaId()` and pass 
`node.replica_id` through the conversion.



##########
foreign/cpp/include/iggy.hpp:
##########
@@ -780,6 +789,17 @@ class Permissions final {
  * A consumer offset belongs either to an individual consumer or to a consumer
  * group. Create a value with Single() or Group(), then pass it to the consumer
  * offset operations on IggyBlockingClient.
+ *
+ * Two callers naming the same identifier share one offset, so under
+ * PollingStrategy::Next() with auto-commit each sees only the messages the
+ * other has not read yet. Give every independent consumer its own name.
+ *
+ * @code{.cpp}
+ * const auto polled{client.PollMessages(
+ *     stream, topic, std::nullopt,

Review Comment:
   warning: The `Consumer` example passes `std::nullopt` to `PollMessages()`, 
but the overload rejects an empty partition with `partition_id is required`, so 
the example always throws. Pass a concrete partition ID in the example and take 
`std::uint32_t` instead of `std::optional` in the overload.



##########
foreign/cpp/include/iggy.hpp:
##########
@@ -7427,6 +7746,181 @@ class IggyBlockingClient final {
      */
     Stats GetStats();
 
+    /**
+     * @brief Sends a non-empty batch of messages to one topic.
+     *
+     * The partitioning strategy selects one destination for the entire batch.
+     * Message and partitioning constraints are validated before the request is
+     * sent. The Rust SDK assigns generated IDs to messages whose ID is zero;
+     * the C++ input objects are not modified.
+     *
+     * Delivery is at least once. A failed or unknown transport outcome can
+     * leave the batch committed, so retrying may append duplicates. Successful
+     * responses can contain no confirmations; see SendMessagesResponse.
+     *
+     * @param stream Parent stream, addressed by numeric ID or name.
+     * @param topic Destination topic, addressed by numeric ID or name.
+     * @param partitioning Strategy used to select the destination partition.
+     * @param messages Messages to send. The collection must not be empty.
+     * @return Available per-partition commit confirmations.
+     * @throws IggyException if the client is unavailable or unauthenticated;
+     *         an identifier, partitioning strategy, message, or batch is
+     *         invalid; the stream, topic, or partition does not exist; the
+     *         caller lacks send permission; or the request fails.
+     */
+    SendMessagesResponse SendMessages(const Identifier &stream,
+                                      const Identifier &topic,
+                                      const Partitioning &partitioning,
+                                      const std::vector<IggyMessageToSend> 
&messages);
+
+    /**
+     * @brief Reads one partition with a message count and optional on-read 
commit.
+     *
+     * A partition is required. For additional read limits or a continuation,
+     * use the overload taking PollOptions.
+     */
+    PolledMessages PollMessages(const Identifier &stream,
+                                const Identifier &topic,
+                                std::optional<std::uint32_t> partition_id,
+                                const Consumer &consumer,
+                                const PollingStrategy &strategy,
+                                std::uint32_t count,
+                                bool auto_commit);
+
+    /**
+     * @brief Retrieves the server's option catalog for one resource type.
+     *
+     * The catalog describes accepted create-option keys, their HeaderKind
+     * encodings, defaults, and descriptions. A supported scope with no option
+     * keys returns an empty collection.
+     *
+     * @param scope Resource scope. Must be `topic`, `stream`, or `user`.
+     * @return Option specifications for the requested scope.
+     * @throws IggyException if the client is unavailable or unauthenticated;
+     *         @p scope is invalid; or the request fails.
+     */
+    std::vector<OptionSpec> DescribeOptions(std::string scope);
+
+    /**
+     * @brief Checks whether the configured server can answer a request.
+     *
+     * Ping does not require authentication and returns no server data.
+     *
+     * @throws IggyException if the client is unavailable or the request fails.
+     */
+    void Ping();
+
+    /**
+     * @brief Returns the client's configured heartbeat interval.
+     * @return Non-zero interval in microseconds.
+     * @note This reads client configuration and does not contact the server.
+     * @throws IggyException if this client has been moved from.
+     */
+    std::chrono::microseconds HeartbeatInterval();
+
+    /**
+     * @brief Captures server diagnostics as a ZIP archive.
+     *
+     * Duplicate snapshot types are removed. SystemSnapshotType::All() must be
+     * the only requested type and expands to all production diagnostic
+     * sections. The server runs only one snapshot collection at a time; a
+     * concurrent request fails. A section that cannot be captured can be
+     * omitted while the remaining archive is still returned.
+     *
+     * @param compression Compression method used for ZIP entries.
+     * @param types Non-empty collection of diagnostic sections to capture.
+     * @return Complete ZIP archive bytes owned by the caller.
+     * @throws IggyException if the client is unavailable or unauthenticated;
+     *         the caller lacks read-servers or manage-servers permission; the
+     *         requested types are invalid; another snapshot is in progress;
+     *         archive creation fails; or the request fails.
+     */
+    std::vector<std::uint8_t> Snapshot(const SnapshotCompression &compression,
+                                       const std::vector<SystemSnapshotType> 
&types);
+
+    /**
+     * @brief Sends one command through the raw Iggy binary protocol.
+     *
+     * The caller supplies only the command body and receives only the response
+     * body; the configured transport adds and removes protocol framing. The
+     * command keeps its normal authentication, authorization, and mutation
+     * semantics. Session-control commands for login, logout, and registration
+     * are rejected so that the SDK's session state cannot be bypassed.
+     *
+     * @param code Iggy binary-protocol command code.
+     * @param payload Command body encoded for @p code.
+     * @return Raw response body without protocol framing.
+     * @throws IggyException if the transport is HTTP; @p code is a
+     *         session-control or unsupported command; the payload is invalid;
+     *         the command's access checks fail; or the request fails.
+     */
+    std::vector<std::uint8_t> SendBinaryRequest(std::uint32_t code, const 
std::vector<std::uint8_t> &payload);
+
+    /**
+     * @brief Sends a non-replicated raw command to a specific cluster replica.
+     * @throws IggyException for an unknown replica, a replicated or 
session-control command,
+     *         or a failed request.
+     */
+    std::vector<std::uint8_t> SendBinaryRequestToReplica(std::uint32_t code,

Review Comment:
   nit: The class doc and README list the methods that populate 
`IggyException::Code()` and `Kind()`, but this method and `SendMessages()`, 
`UpdatePermissions()`, `ChangePassword()`, and `SendBinaryRequest()` populate 
them too. Add the five.



##########
foreign/cpp/src/client.cpp:
##########
@@ -553,6 +570,124 @@ Stats IggyBlockingClient::GetStats() {
     return RethrowAsIggyException([this] { return 
Stats::FromFfi(Handle()->get_stats()); });
 }
 
+SendMessagesResponse IggyBlockingClient::SendMessages(const Identifier &stream,
+                                                      const Identifier &topic,
+                                                      const Partitioning 
&partitioning,
+                                                      const 
std::vector<IggyMessageToSend> &messages) {
+    return RethrowAsIggyException([this, &stream, &topic, &partitioning, 
&messages](ffi::ErrorInfo &error_info) {
+        rust::Vec<ffi::IggyMessageToSend> ffi_messages;
+        ffi_messages.reserve(messages.size());
+        for (const auto &message : messages) {
+            ffi_messages.push_back(message.ToFfi());
+        }
+        rust::Vec<std::uint8_t> ffi_partitioning_value;

Review Comment:
   simplification: The new wrappers repeat the byte-copy loop three times, here 
and at lines 647 and 661. Call the existing `ToFfiBytes()` helper at line 68 
instead, which saves about twelve lines.



##########
foreign/cpp/include/iggy.hpp:
##########
@@ -998,10 +1038,11 @@ class IggyMessageToSend final {
      * @note Payload and header constraints are validated when the message is
      *       sent, not by this function.
      */
-    static IggyMessageToSend Create(std::vector<std::uint8_t> payload,
+    static IggyMessageToSend Create(std::string_view payload,

Review Comment:
   warning: `Create()` takes `std::string_view` and copies the payload, so a 
caller with a byte vector cannot move it and pays one extra copy. Restore the 
by-value vector overload alongside the view.



##########
foreign/cpp/src/client.cpp:
##########
@@ -280,14 +280,32 @@ TopicDetails IggyBlockingClient::CreateTopic(const 
Identifier &stream,
         }
         if (auto value = options.Durability()) {
             ffi_options.has_durability = true;
-            ffi_options.durability     = std::string(iggy::to_string(*value));
+            switch (*value) {

Review Comment:
   simplification: Two identical switch blocks map `Durability` to `replicated` 
or `persisted`, here and at line 299. Restore the removed public 
`to_string(Durability)` helper and call it for both fields, which saves about 
nine lines.



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