[ https://issues.apache.org/jira/browse/PROTON-2308?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17461435#comment-17461435 ]
ASF GitHub Bot commented on PROTON-2308: ---------------------------------------- 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: dev-unsubscr...@qpid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [cpp] Add support for setting Dynamic Node Properties > ----------------------------------------------------- > > Key: PROTON-2308 > URL: https://issues.apache.org/jira/browse/PROTON-2308 > Project: Qpid Proton > Issue Type: Improvement > Components: cpp-binding > Affects Versions: proton-c-0.33.0 > Reporter: James Henry > Assignee: Rakhi Kumari > Priority: Major > Labels: api-addition > > Requesting support for setting the dynamic node properties be added to the > source and target options. > This would allow the setting of termini node properties for senders and > receivers. > Similar to the following request made for Python here: > https://issues.apache.org/jira/browse/PROTON-816 -- This message was sent by Atlassian Jira (v8.20.1#820001) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org