You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/11/11 10:04:01 UTC

[GitHub] [pulsar-client-cpp] shibd opened a new pull request, #117: Fix getLastMessageId method dead recursion.

shibd opened a new pull request, #117:
URL: https://github.com/apache/pulsar-client-cpp/pull/117

   ### Motivation
   
   There is a problem with the `getLastMessageId` method currently implemented.
   
   He is recursively invoking himself.
   https://github.com/apache/pulsar-client-cpp/blob/ad79becf0814a992ad493bc625ba0a489900451f/lib/Consumer.cc#L293-L301
   
   ### Modifications
   
   - Let it call the actual implementation.
   
   **Note**:  `MultiTopicConsumer.getLastMessageId()` is not supported for the time being. I will support it later. Supporting `MultiTopicConsumer` requires the implementation of [MultiMessageId](https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiMessageIdImpl.java), which requires more thought and design.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
   
   - [x] `doc-not-needed` 
   (Please explain why)
   
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-cpp] BewareMyPower merged pull request #117: Fix getLastMessageId method dead recursion.

Posted by GitBox <gi...@apache.org>.
BewareMyPower merged PR #117:
URL: https://github.com/apache/pulsar-client-cpp/pull/117


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #117: Fix getLastMessageId method dead recursion.

Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #117:
URL: https://github.com/apache/pulsar-client-cpp/pull/117#discussion_r1023901373


##########
tests/ConsumerTest.cc:
##########
@@ -841,6 +841,30 @@ TEST(ConsumerTest, testPartitionsWithCloseUnblock) {
     thread.join();
 }
 
+TEST(ConsumerTest, testGetLastMessageId) {
+    Client client(lookupUrl);
+    const std::string topic = "testGetLastMessageId-" + std::to_string(time(nullptr));
+
+    Consumer consumer;
+    ASSERT_EQ(ResultOk, client.subscribe(topic, "test-sub", consumer));
+
+    MessageId msgId;
+    ASSERT_EQ(ResultOk, consumer.getLastMessageId(msgId));
+    ASSERT_EQ(msgId, MessageId(-1, -1, -1, -1));
+
+    Producer producer;
+    ASSERT_EQ(ResultOk, client.createProducer(topic, producer));
+    Message msg = MessageBuilder().setContent("message").build();
+    ASSERT_EQ(ResultOk, producer.send(msg));
+
+    ASSERT_EQ(ResultOk, consumer.getLastMessageId(msgId));
+    ASSERT_TRUE(msgId != MessageId(-1, -1, -1, -1));
+
+    std::cout << msgId << std::endl;

Review Comment:
   ```suggestion
   ```
   
   Don't use `std::cout` in tests



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-cpp] BewareMyPower commented on a diff in pull request #117: Fix getLastMessageId method dead recursion.

Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #117:
URL: https://github.com/apache/pulsar-client-cpp/pull/117#discussion_r1023900770


##########
tests/ConsumerTest.cc:
##########
@@ -841,6 +841,30 @@ TEST(ConsumerTest, testPartitionsWithCloseUnblock) {
     thread.join();
 }
 
+TEST(ConsumerTest, testGetLastMessageId) {
+    Client client(lookupUrl);
+    const std::string topic = "testGetLastMessageId-" + std::to_string(time(nullptr));
+
+    Consumer consumer;
+    ASSERT_EQ(ResultOk, client.subscribe(topic, "test-sub", consumer));
+
+    MessageId msgId;
+    ASSERT_EQ(ResultOk, consumer.getLastMessageId(msgId));
+    ASSERT_EQ(msgId, MessageId(-1, -1, -1, -1));
+
+    Producer producer;
+    ASSERT_EQ(ResultOk, client.createProducer(topic, producer));
+    Message msg = MessageBuilder().setContent("message").build();
+    ASSERT_EQ(ResultOk, producer.send(msg));
+
+    ASSERT_EQ(ResultOk, consumer.getLastMessageId(msgId));
+    ASSERT_TRUE(msgId != MessageId(-1, -1, -1, -1));

Review Comment:
   Use `ASSERT_NE`.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org