From 92e8c8ec490628d75052c6d8029e7107148de6d4 Mon Sep 17 00:00:00 2001
From: Thomas Ives <tri@observatorysciences.co.uk>
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_<x>_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_<x>_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 <memory>
+
+using CallbackMockType = TangoTest::CallbackMock<Tango::EventData>;
+
+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<std::string> 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 <typename Base>
+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<std::string> filter_names;
+        std::vector<double> 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<Tango::Attr *> &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<Tango::Command *> &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<Tango::DeviceProxy> 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<Tango::DeviceProxy> 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<Tango::DevLong>(1)));
+
+                        maybe_event = callback.pop_next_event();
+                        REQUIRE(maybe_event == std::nullopt);
+                    }
+                }
+            }
+        }
+    }
+}
+
+template <typename Base>
+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<Tango::Attr *> &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<Tango::DeviceProxy> 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<Tango::DeviceProxy> 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 <typename Base>
+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<Tango::Attr *> &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<Tango::DeviceProxy> 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<Tango::DeviceProxy> 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<Tango::AttrConfEventData> 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