gemmellr commented on a change in pull request #346:
URL: https://github.com/apache/qpid-proton/pull/346#discussion_r771394080
##########
File path: cpp/src/link_test.cpp
##########
@@ -23,9 +23,114 @@
#include <proton/sender_options.hpp>
#include <proton/receiver_options.hpp>
#include <proton/container.hpp>
+#include <proton/connection.hpp>
+#include <proton/connection_options.hpp>
+#include <proton/listen_handler.hpp>
+#include <proton/listener.hpp>
+#include <proton/messaging_handler.hpp>
+#include <proton/types.hpp>
+#include <proton/message.hpp>
+#include <proton/target_options.hpp>
+#include <proton/source_options.hpp>
+#include <proton/delivery.hpp>
#include <iostream>
+#include <map>
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+namespace {
+std::mutex m;
+std::condition_variable cv;
+bool listener_ready = false;
+int listener_port;
+} // namespace
+
+class test_recv : public proton::messaging_handler {
+ private:
+ class listener_ready_handler : public proton::listen_handler {
+ void on_open(proton::listener &l) override {
+ {
+ std::lock_guard<std::mutex> lk(m);
+ listener_port = l.port();
+ listener_ready = true;
+ }
+ cv.notify_one();
+ }
+ };
+
+ std::string url;
+ proton::listener listener;
+ listener_ready_handler listen_handler;
+
+ public:
+ test_recv(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) override {
+ listener = c.listen(url, listen_handler);
+ }
+
+ void on_message(proton::delivery &d, proton::message &msg) override {
+ proton::symbol sym = "symbol";
+ proton::value val = "value";
+ std::map<proton::symbol, proton::value> props =
d.receiver().target().dynamic_node_properties();
+
+ ASSERT(!props.empty());
+ for(auto it=props.begin(); it!=props.end(); it++) {
+ ASSERT_EQUAL(sym, it->first);
+ ASSERT_EQUAL(val, it->second);
+ }
+ d.receiver().close();
+ d.connection().close();
+ listener.stop();
+ }
+};
+
+class test_send : public proton::messaging_handler {
+ private:
+ std::string url;
+ proton::sender sender;
+
+ public:
+ test_send(const std::string &s) : url(s) {}
+
+ void on_container_start(proton::container &c) override {
+ proton::target_options opts;
+ std::map<proton::symbol,proton::value> m({{proton::symbol("symbol"),
proton::value("value")}});
+ opts.dynamic_node_properties(m);
+ sender = c.open_sender(url, proton::sender_options().target(opts));
+ }
+
+ void on_sendable(proton::sender &s) override {
+ proton::message msg;
+ msg.body("message");
+ proton::tracker t = s.send(msg);
+ s.connection().close();
+ }
+};
+
+int test_dynamic_node_properties() {
Review comment:
A dynamic consumer is by far the primary use case for dynamic, so if
only having one test it would be that (essentially the opposite of this test).
Having one test for a consumer, and another for a producer (from the client
perspective) wouldnt be a bad idea, they are separate things, but dynamic
senders are rare.
The test doesnt actually do all the steps I'd say it should. Its possible
another test is doing some of them somewhere, or that its just not tested for
the cpp binding. It should use a 'dynamic' source/target and verify the
behaviour in full, right now it isnt, but is instead attaching to a known
address. The 'dynamic' handling involves the original peer not setting an
address, and the other peer generating an address for the dynamic node and
supplying its detail back to the original side in its reply attach, which the
requesting peer can then read to determine the name of the dynamic node created
for it. The dynamic-node-properties adds to that mechanism by letting the
requester also ask for certain node details to be applied. The generating peer
can always use that same field to indicate properties actually in effect for
the dynamic node it created.
The test only looks to verify that the node properties were sent [illegally
due to not setting dynamic] from requestor to the other side. It doesnt check
it can handle reading the details that come back, i.e to verify any dynamic
address was given, or that any dynamic-node-properties arrived.
As an example, here is a Java test verifying the dynamic flag and address
propagation aspects (it doesnt test the dynamic node properties) for a dynamic
consumer:
https://github.com/vert-x3/vertx-proton/blob/4.2.2/src/test/java/io/vertx/proton/ProtonClientTest.java#L468-L545
I dont know if the 'built in' handshaking bits in proton-cpp that are
opening the link in your test will inspect the dynamic flag and generate an
address by themselves for the attach. If they do the test could just at least
verify there is a generated value. If not, or better still, the test could set
a generated address at the server side and check the client side can pick it up
when its told the link opened. It could do the same for the dynamic node
properties, e.g have the server apply something other than originally asked
for, verify they are picked up too at the client once the link opens.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
