From 92e8c8ec490628d75052c6d8029e7107148de6d4 Mon Sep 17 00:00:00 2001 From: Thomas Ives Date: Thu, 28 Aug 2025 15:06:03 +0100 Subject: [PATCH 1/5] catch2_event_old_client: Add test to catch duplicate event issue This test reproduces the issue described in #1521. There are three test cases added here which cover different places where we have the duplicate events issue. The first test case covers the case where events are being pushed manually both with and without change detection. This finds the duplicate event issue in the `ZmqEventSupplier::detect_and_push__event` methods along with the `ZmqEventSupplier::push_event_loop` method. The second test case covers events sent by the polling loop, this also triggers the issue with the `ZmqEventSupplier::detect_and_push__event` and is included mostly so that we can check periodic events work. The final test case covers attribute configuration events, which triggers the duplicate event issue with the `EventSupplier::push_att_config_event` method. --- tests/CMakeLists.txt | 1 + tests/catch2_event_old_client.cpp | 374 ++++++++++++++++++++++++++++++ 2 files changed, 375 insertions(+) create mode 100644 tests/catch2_event_old_client.cpp Index: tango/lib/cpp/tests/catch2_event_old_client.cpp =================================================================== --- /dev/null +++ tango/lib/cpp/tests/catch2_event_old_client.cpp @@ -0,0 +1,374 @@ +#include "catch2_common.h" + +#include "tango/internal/utils.h" + +#include + +using CallbackMockType = TangoTest::CallbackMock; + +constexpr const char *k_attr_name = "basicAttr"; +constexpr const char *k_no_detect_attr_name = "noDetectAttr"; +constexpr static Tango::DevLong k_polling_period = TANGO_TEST_CATCH2_DEFAULT_POLL_PERIOD; + +void simulate_old_clients_subscription(Tango::DeviceProxy *admin, + Tango::EventType ev_type, + int idlver, + std::string_view device_name, + std::string_view attr_name) +{ + // Here we trick the device server into thinking that an old client has + // subscribed by calling ZmqEventSubscriptionChange. The order here + // matters, we want to subscribe to idl5 first, otherwise the duplicates + // will get discarded on the client side. + for(int oldver = idlver - 1; oldver >= 4; --oldver) + { + std::vector request; + request.reserve(5); + request.emplace_back(device_name); + request.emplace_back(attr_name); + request.emplace_back("subscribe"); + { + std::stringstream ss; + if(oldver >= 5) + { + ss << "idl5_"; + } + ss << Tango::EventName[ev_type]; + request.emplace_back(ss.str()); + } + { + std::stringstream ss; + ss << oldver; + request.emplace_back(ss.str()); + } + Tango::DeviceData din; + din << request; + + REQUIRE_NOTHROW(admin->command_inout("ZmqEventSubscriptionChange", din)); + } +} + +template +class DuplicatePushedEvent : public Base +{ + public: + using Base::Base; + + ~DuplicatePushedEvent() override { } + + void init_device() override + { + value = 0; + } + + void read_attr(Tango::Attribute &att) override + { + att.set_value(&value); + } + + void push_change() + { + value += 1; + this->push_change_event(k_attr_name, &value); + this->push_change_event(k_no_detect_attr_name, &value); + } + + void push_archive() + { + value += 1; + this->push_archive_event(k_attr_name, &value); + this->push_archive_event(k_no_detect_attr_name, &value); + } + + void push_user() + { + value += 1; + std::vector filter_names; + std::vector filter_values; + this->push_event(k_attr_name, filter_names, filter_values, &value); + this->push_event(k_no_detect_attr_name, filter_names, filter_values, &value); + } + + static void attribute_factory(std::vector &attrs) + { + using Attr = TangoTest::AutoAttr<&DuplicatePushedEvent::read_attr>; + Tango::UserDefaultAttrProp props; + props.set_abs_change("1"); + props.set_archive_abs_change("1"); + + auto attr = new Attr(k_attr_name, Tango::DEV_LONG); + attr->set_default_properties(props); + attr->set_change_event(true, true); + attr->set_archive_event(true, true); + attrs.push_back(attr); + + auto no_detect_attr = new Attr(k_no_detect_attr_name, Tango::DEV_LONG); + no_detect_attr->set_change_event(true, false); + no_detect_attr->set_archive_event(true, false); + attrs.push_back(no_detect_attr); + } + + static void command_factory(std::vector &cmds) + { + cmds.push_back(new TangoTest::AutoCommand<&DuplicatePushedEvent::push_change>("push_change")); + cmds.push_back(new TangoTest::AutoCommand<&DuplicatePushedEvent::push_archive>("push_archive")); + cmds.push_back(new TangoTest::AutoCommand<&DuplicatePushedEvent::push_user>("push_user_event")); + } + + private: + Tango::DevLong value{0}; +}; + +TANGO_TEST_AUTO_DEV_TMPL_INSTANTIATE(DuplicatePushedEvent, 6) + +SCENARIO("Manually pushed events are not duplicated when old clients are connected") +{ + int idlver = GENERATE(TangoTest::idlversion(6)); + GIVEN("a device proxy to a simple IDLv" << idlver << " device") + { + TangoTest::Context ctx{"duplicate_event", "DuplicatePushedEvent", idlver}; + std::shared_ptr device = ctx.get_proxy(); + + REQUIRE(idlver == device->get_idl_version()); + + std::string attr_name = GENERATE(k_attr_name, k_no_detect_attr_name); + Tango::EventType ev_type = GENERATE(Tango::CHANGE_EVENT, Tango::ARCHIVE_EVENT, Tango::USER_EVENT); + + WHEN("some old clients subscribe to " << Tango::EventName[ev_type] << " events for " << attr_name) + { + std::unique_ptr admin = ctx.get_admin_proxy(); + + simulate_old_clients_subscription(admin.get(), ev_type, idlver, device->dev_name(), attr_name); + + AND_WHEN("we subscribe to " << Tango::EventName[ev_type] << " events for " << attr_name) + { + CallbackMockType callback; + + device->subscribe_event(attr_name, ev_type, &callback); + + auto maybe_initial_event = callback.pop_next_event(); + REQUIRE(maybe_initial_event != std::nullopt); + maybe_initial_event = callback.pop_next_event(); + + AND_WHEN("the server sends an single event") + { + { + std::stringstream ss; + ss << "push_" << Tango::EventName[ev_type]; + REQUIRE_NOTHROW(device->command_inout(ss.str())); + } + + THEN("we only receive one event") + { + using TangoTest::AnyLikeContains; + + auto maybe_event = callback.pop_next_event(); + REQUIRE(maybe_event != std::nullopt); + REQUIRE(maybe_event->event == Tango::EventName[ev_type]); + REQUIRE_THAT(*maybe_event->attr_value, AnyLikeContains(static_cast(1))); + + maybe_event = callback.pop_next_event(); + REQUIRE(maybe_event == std::nullopt); + } + } + } + } + } +} + +template +class DuplicatePollingEvent : public Base +{ + public: + using Base::Base; + + ~DuplicatePollingEvent() override { } + + void init_device() override + { + value = 0; + } + + void read_attr(Tango::Attribute &att) override + { + value += 1; + att.set_value(&value); + } + + static void attribute_factory(std::vector &attrs) + { + using Attr = TangoTest::AutoAttr<&DuplicatePollingEvent::read_attr>; + Tango::UserDefaultAttrProp props; + { + std::stringstream ss; + ss << k_polling_period; + props.set_period(ss.str().c_str()); + } + props.set_abs_change("1"); + props.set_archive_abs_change("1"); + + auto attr = new Attr(k_attr_name, Tango::DEV_LONG); + attr->set_default_properties(props); + attr->set_polling_period(k_polling_period); + attrs.push_back(attr); + } + + private: + Tango::DevLong value{0}; +}; + +TANGO_TEST_AUTO_DEV_TMPL_INSTANTIATE(DuplicatePollingEvent, 6) + +SCENARIO("Periodic events are not duplicated when old clients are connected") +{ + int idlver = GENERATE(TangoTest::idlversion(6)); + GIVEN("a device proxy to a simple IDLv" << idlver << " device") + { + TangoTest::Context ctx{"duplicate_event", "DuplicatePollingEvent", idlver}; + std::shared_ptr device = ctx.get_proxy(); + + REQUIRE(idlver == device->get_idl_version()); + + Tango::EventType ev_type = GENERATE(Tango::CHANGE_EVENT, Tango::ARCHIVE_EVENT, Tango::PERIODIC_EVENT); + + WHEN("some old clients subscribe to " << Tango::EventName[ev_type] << " events") + { + std::unique_ptr admin = ctx.get_admin_proxy(); + + simulate_old_clients_subscription(admin.get(), ev_type, idlver, device->dev_name(), k_attr_name); + + AND_WHEN("we subscribe to " << Tango::EventName[ev_type] << " events") + { + CallbackMockType callback; + + device->subscribe_event(k_attr_name, ev_type, &callback); + + // We only require one event for the initial subscription, we + // don't want to consume any events from the Zmq interface. + // Not quite sure why this matters for polling and not when + // pushing. I think the de-duplicate on the client side means + // we only see the duplicate event for the first events that are + // pushed. + auto maybe_initial_event = callback.pop_next_event(); + REQUIRE(maybe_initial_event != std::nullopt); + + AND_WHEN("we wait for three events") + { + // We wait for three events here as if there are duplicates + // two of these must have the same value, but we don't + // know which two. + auto maybe_event1 = callback.pop_next_event(); + auto maybe_event2 = callback.pop_next_event(); + auto maybe_event3 = callback.pop_next_event(); + + REQUIRE(maybe_event1 != std::nullopt); + REQUIRE(maybe_event2 != std::nullopt); + REQUIRE(maybe_event3 != std::nullopt); + + THEN("the events have different values") + { + Tango::DevLong value1; + *maybe_event1->attr_value >> value1; + Tango::DevLong value2; + *maybe_event2->attr_value >> value2; + Tango::DevLong value3; + *maybe_event3->attr_value >> value3; + + REQUIRE(value1 != value2); + REQUIRE(value2 != value3); + } + } + } + } + } +} + +template +class DuplicateAttrConfEvent : public Base +{ + public: + using Base::Base; + + ~DuplicateAttrConfEvent() override { } + + void init_device() override + { + value = 0; + } + + void read_attr(Tango::Attribute &att) override + { + value += 1; + att.set_value(&value); + } + + static void attribute_factory(std::vector &attrs) + { + using Attr = TangoTest::AutoAttr<&DuplicateAttrConfEvent::read_attr>; + Tango::UserDefaultAttrProp props; + props.set_abs_change("1"); + + auto attr = new Attr(k_attr_name, Tango::DEV_LONG); + attr->set_default_properties(props); + attrs.push_back(attr); + } + + private: + Tango::DevLong value{0}; +}; + +TANGO_TEST_AUTO_DEV_TMPL_INSTANTIATE(DuplicateAttrConfEvent, 6) + +SCENARIO("Attribute config events are not duplicated when old clients are connected") +{ + int idlver = GENERATE(TangoTest::idlversion(6)); + GIVEN("a device proxy to a simple IDLv" << idlver << " device") + { + TangoTest::Context ctx{"duplicate_event", "DuplicateAttrConfEvent", idlver}; + std::shared_ptr device = ctx.get_proxy(); + + REQUIRE(idlver == device->get_idl_version()); + + Tango::EventType ev_type = Tango::ATTR_CONF_EVENT; + + WHEN("some old clients subscribe to attr config") + { + std::unique_ptr admin = ctx.get_admin_proxy(); + + simulate_old_clients_subscription(admin.get(), ev_type, idlver, device->dev_name(), k_attr_name); + + AND_WHEN("we subscribe to attr config events") + { + TangoTest::CallbackMock callback; + + device->subscribe_event(k_attr_name, ev_type, &callback); + + auto maybe_initial_event = callback.pop_next_event(); + REQUIRE(maybe_initial_event != std::nullopt); + + maybe_initial_event = callback.pop_next_event(); + + AND_WHEN("we configure the attribute") + { + auto ai = device->attribute_query(k_attr_name); + ai.events.ch_event.abs_change = "Not specified"; + + Tango::AttributeInfoListEx ail; + ail.push_back(ai); + REQUIRE_NOTHROW(device->set_attribute_config(ail)); + + THEN("we only get a single attr conf event") + { + auto maybe_event = callback.pop_next_event(); + REQUIRE(maybe_event != std::nullopt); + REQUIRE(maybe_event->event == Tango::EventName[ev_type]); + REQUIRE(maybe_event->attr_conf != nullptr); + + maybe_event = callback.pop_next_event(); + REQUIRE(maybe_event == std::nullopt); + } + } + } + } + } +} Index: tango/lib/cpp/tests/CMakeLists.txt =================================================================== --- tango.orig/lib/cpp/tests/CMakeLists.txt +++ tango/lib/cpp/tests/CMakeLists.txt @@ -312,6 +312,7 @@ tango_catch2_tests_create( catch2_attr_polling.cpp catch2_cmd_polling.cpp catch2_connection.cpp + catch2_event_old_client.cpp catch2_event_reconnection.cpp catch2_test_dtypes.cpp catch2_dev_state.cpp