BewareMyPower commented on code in PR #357:
URL:
https://github.com/apache/pulsar-client-node/pull/357#discussion_r1441615671
##########
src/ConsumerConfig.h:
##########
@@ -21,14 +21,17 @@
#define CONSUMER_CONFIG_H
#include <pulsar/c/consumer_configuration.h>
+#include "ThreadSafeDeferred.h"
#include "MessageListener.h"
#define MIN_ACK_TIMEOUT_MILLIS 10000
class ConsumerConfig {
public:
- ConsumerConfig(const Napi::Object &consumerConfig, pulsar_message_listener
messageListener);
+ ConsumerConfig();
~ConsumerConfig();
+ void InitConfig(const std::shared_ptr<ThreadSafeDeferred> deferred, const
Napi::Object &consumerConfig,
Review Comment:
Don't pass const value as the parameter type. For example,
```c++
struct Foo {
void f1() const {}
void f2() {}
};
void f(const std::shared_ptr<Foo> foo) {
foo->f1();
foo->f2(); // OK, even though f2() is a non-const method
}
```
`f()` can only guarantee `foo` itself cannot be modified like
`foo.set(nullptr)`.
However, since this method only needs a reference to the
`ThreadSafeDeferred` object and does not transfer or share the ownership of
`deferred` to another thread, I suggest passing a reference here.
```c++
void InitConfig(ThreadSafeDeferred &deferred, const Napi::Object
&consumerConfig,
pulsar_message_listener messageListener);
```
Then call `deferred.Reject` instead of `deferred->Reject` in the
implementation.
BTW, I still don't get why this refactoring is needed in this PR.
##########
tests/consumer.test.js:
##########
@@ -315,6 +346,47 @@ const Pulsar = require('../index');
consumer.close();
dlqConsumer.close();
});
+
+ test('Batch Receive', async () => {
+ const topicName = 'batch-receive-test-topic';
+ const producer = await client.createProducer({
+ topic: topicName,
+ });
+
+ const consumer = await client.subscribe({
+ topic: topicName,
+ subscription: 'sub1',
+ subscriptionType: 'Shared',
+ batchReceivePolicy: {
+ maxNumMessages: 10,
+ maxNumBytes: -1,
+ timeoutMs: 500,
Review Comment:
+1. You'd better test the `timeoutMs` config works.
--
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]
