From 1d1f89b8cd8f14f9022898ea41c7d980199af650 Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 18 Dec 2024 12:48:59 +0100 Subject: [PATCH 1/3] DataStorm fixes for C++ lints, nodiscard, and Ice qualification --- .clang-tidy | 7 +- cpp/include/DataStorm/DataStorm.h | 47 ++-- cpp/include/DataStorm/InternalI.h | 28 +-- cpp/include/DataStorm/InternalT.h | 49 ++-- cpp/include/DataStorm/Node.h | 4 +- cpp/include/DataStorm/Types.h | 16 +- cpp/include/Ice/LoggerUtil.h | 2 +- cpp/src/DataStorm/CallbackExecutor.cpp | 4 +- cpp/src/DataStorm/CallbackExecutor.h | 4 +- cpp/src/DataStorm/ConnectionManager.cpp | 11 +- cpp/src/DataStorm/DataElementI.cpp | 105 +++++---- cpp/src/DataStorm/DataElementI.h | 64 +++--- cpp/src/DataStorm/ForwarderManager.cpp | 20 +- cpp/src/DataStorm/ForwarderManager.h | 16 +- cpp/src/DataStorm/Instance.cpp | 24 +- cpp/src/DataStorm/Instance.h | 38 ++- cpp/src/DataStorm/LookupI.cpp | 21 +- cpp/src/DataStorm/Node.cpp | 35 +-- cpp/src/DataStorm/NodeI.cpp | 110 ++++----- cpp/src/DataStorm/NodeI.h | 36 +-- cpp/src/DataStorm/NodeSessionI.cpp | 45 ++-- cpp/src/DataStorm/NodeSessionI.h | 10 +- cpp/src/DataStorm/NodeSessionManager.cpp | 95 ++++---- cpp/src/DataStorm/NodeSessionManager.h | 14 +- cpp/src/DataStorm/SessionI.cpp | 268 +++++++++++----------- cpp/src/DataStorm/SessionI.h | 85 ++++--- cpp/src/DataStorm/TopicFactoryI.cpp | 47 ++-- cpp/src/DataStorm/TopicFactoryI.h | 20 +- cpp/src/DataStorm/TopicI.cpp | 72 +++--- cpp/src/DataStorm/TopicI.h | 77 ++++--- cpp/src/DataStorm/TraceUtil.cpp | 3 +- cpp/src/DataStorm/TraceUtil.h | 32 +-- cpp/src/Ice/LoggerUtil.cpp | 2 +- cpp/src/dsnode/Node.cpp | 9 +- cpp/test/DataStorm/api/Writer.cpp | 16 +- cpp/test/DataStorm/events/Reader.cpp | 3 +- cpp/test/DataStorm/reliability/Reader.cpp | 1 + cpp/test/DataStorm/reliability/Writer.cpp | 6 +- cpp/test/DataStorm/reliability/test.py | 8 +- 39 files changed, 723 insertions(+), 731 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index c6325ca55ba..1f185aa8325 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,15 +9,16 @@ Checks: -modernize-use-trailing-return-type, -modernize-concat-nested-namespaces, performance-*, - -performance-avoid-endl + -performance-avoid-endl, + -modernize-use-constraints ' WarningsAsErrors: '*' HeaderFilterRegex: '.*' ExcludeHeaderFilterRegex: 'include/lmdb.h|Grammar.h' UseColor: true FormatStyle: 'file' -ExtraArgs: ['-std=c++20'] +ExtraArgs: ['-std=c++17'] CheckOptions: modernize-use-nullptr.NullMacros: 'NULL' # std::exception_ptr is a cheap to copy, pointer-like type; we pass it by value all the time. - performance-unnecessary-value-param.AllowedTypes: 'exception_ptr$;' + performance-unnecessary-value-param.AllowedTypes: 'exception_ptr$;std::function$' diff --git a/cpp/include/DataStorm/DataStorm.h b/cpp/include/DataStorm/DataStorm.h index ad5191a670a..2d2e37bb297 100644 --- a/cpp/include/DataStorm/DataStorm.h +++ b/cpp/include/DataStorm/DataStorm.h @@ -264,7 +264,7 @@ namespace DataStorm * * @return The unread samples. */ - std::vector> getAllUnread() noexcept; + [[nodiscard]] std::vector> getAllUnread() noexcept; /** * Wait for given number of unread samples to be available. @@ -286,7 +286,7 @@ namespace DataStorm * @return The unread sample. * @throws NodeShutdownException If the node is shut down while waiting. */ - Sample getNextUnread(); + [[nodiscard]] Sample getNextUnread(); /** * Calls the given functions to provide the initial set of connected keys and when a key is added or @@ -420,14 +420,14 @@ namespace DataStorm * @return The last written sample. * @throws std::logic_error If there's no sample. **/ - Sample getLast(); + [[nodiscard]] Sample getLast(); /** * Get all the written sample kept in the writer history. * * @return The sample history. **/ - std::vector> getAll() noexcept; + [[nodiscard]] std::vector> getAll() noexcept; /** * Calls the given functions to provide the initial set of connected keys and when a key is added or @@ -551,7 +551,7 @@ namespace DataStorm * * @return True if data writers are connected, false otherwise. */ - bool hasWriters() const noexcept; + [[nodiscard]] bool hasWriters() const noexcept; /** * Wait for given number of data writers to be online. @@ -580,7 +580,7 @@ namespace DataStorm * * @return True if data readers are connected, false otherwise. */ - bool hasReaders() const noexcept; + [[nodiscard]] bool hasReaders() const noexcept; /** * Wait for given number of data readers to be online. @@ -818,7 +818,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - SingleKeyReader makeSingleKeyReader( + [[nodiscard]] SingleKeyReader makeSingleKeyReader( const Topic& topic, const typename Topic::KeyType& key, std::string name = std::string(), @@ -838,7 +838,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - SingleKeyReader makeSingleKeyReader( + [[nodiscard]] SingleKeyReader makeSingleKeyReader( const Topic& topic, const typename Topic::KeyType& key, const Filter& sampleFilter, @@ -860,7 +860,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - MultiKeyReader makeMultiKeyReader( + [[nodiscard]] MultiKeyReader makeMultiKeyReader( const Topic& topic, const std::vector::KeyType>& keys, std::string name = std::string(), @@ -882,7 +882,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - MultiKeyReader makeMultiKeyReader( + [[nodiscard]] MultiKeyReader makeMultiKeyReader( const Topic& topic, const std::vector::KeyType>& keys, const Filter& sampleFilter, @@ -903,7 +903,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - MultiKeyReader makeAnyKeyReader( + [[nodiscard]] MultiKeyReader makeAnyKeyReader( const Topic& topic, std::string name = std::string(), const ReaderConfig& config = ReaderConfig()) @@ -923,7 +923,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - MultiKeyReader makeAnyKeyReader( + [[nodiscard]] MultiKeyReader makeAnyKeyReader( const Topic& topic, const Filter& sampleFilter, std::string name = std::string(), @@ -1006,7 +1006,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - FilteredKeyReader makeFilteredKeyReader( + [[nodiscard]] FilteredKeyReader makeFilteredKeyReader( const Topic& topic, const Filter& filter, std::string name = std::string(), @@ -1026,7 +1026,7 @@ namespace DataStorm * @param config The optional reader configuration. */ template - FilteredKeyReader makeFilteredKeyReader( + [[nodiscard]] FilteredKeyReader makeFilteredKeyReader( const Topic& topic, const Filter& keyFilter, const Filter& sampleFilter, @@ -1100,7 +1100,7 @@ namespace DataStorm * @param tag The partial update tag. */ template - std::function partialUpdate(const UpdateTag& tag) noexcept; + [[nodiscard]] std::function partialUpdate(const UpdateTag& tag) noexcept; /** * Remove the data element. This generates a {@link Remove} sample. @@ -1176,7 +1176,7 @@ namespace DataStorm * @param tag The partial update tag. */ template - std::function partialUpdate(const UpdateTag& tag) noexcept; + [[nodiscard]] std::function partialUpdate(const UpdateTag& tag) noexcept; /** * Remove the data element. This generates a {@link Remove} sample. @@ -1219,7 +1219,7 @@ namespace DataStorm * @param config The optional writer configuration. */ template - MultiKeyWriter makeMultiKeyWriter( + [[nodiscard]] MultiKeyWriter makeMultiKeyWriter( const Topic& topic, const std::vector::KeyType>& keys, std::string name = std::string(), @@ -1237,7 +1237,7 @@ namespace DataStorm * @param config The optional writer configuration. */ template - MultiKeyWriter makeAnyKeyWriter( + [[nodiscard]] MultiKeyWriter makeAnyKeyWriter( const Topic& topic, std::string name = std::string(), const WriterConfig& config = WriterConfig()) noexcept @@ -1245,14 +1245,9 @@ namespace DataStorm return MultiKeyWriter(topic, {}, std::move(name), config); } -} - -// -// Public template based API implementation -// - -namespace DataStorm -{ + // + // Public template based API implementation + // // // Sample template implementation diff --git a/cpp/include/DataStorm/InternalI.h b/cpp/include/DataStorm/InternalI.h index df59e34a1bd..c3638265884 100644 --- a/cpp/include/DataStorm/InternalI.h +++ b/cpp/include/DataStorm/InternalI.h @@ -52,7 +52,7 @@ namespace DataStormI public: virtual ~KeyFactory() = default; [[nodiscard]] virtual std::shared_ptr get(std::int64_t) const = 0; - virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0; + [[nodiscard]] virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0; }; class Tag : public virtual Element @@ -64,7 +64,7 @@ namespace DataStormI public: virtual ~TagFactory() = default; [[nodiscard]] virtual std::shared_ptr get(std::int64_t) const = 0; - virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0; + [[nodiscard]] virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0; }; class Sample : public Filterable @@ -96,8 +96,8 @@ namespace DataStormI virtual void setValue(const std::shared_ptr&) = 0; virtual void decode(const Ice::CommunicatorPtr&) = 0; - virtual const Ice::ByteSeq& encode(const Ice::CommunicatorPtr&) = 0; - virtual Ice::ByteSeq encodeValue(const Ice::CommunicatorPtr&) = 0; + [[nodiscard]] virtual const Ice::ByteSeq& encode(const Ice::CommunicatorPtr&) = 0; + [[nodiscard]] virtual Ice::ByteSeq encodeValue(const Ice::CommunicatorPtr&) = 0; [[nodiscard]] const Ice::ByteSeq& getEncodedValue() const { return _encodedValue; } @@ -118,7 +118,7 @@ namespace DataStormI public: virtual ~SampleFactory() = default; - virtual std::shared_ptr create( + [[nodiscard]] virtual std::shared_ptr create( std::string, std::string, std::int64_t, @@ -151,7 +151,7 @@ namespace DataStormI [[nodiscard]] virtual std::shared_ptr get(const std::string&, std::int64_t) const = 0; - virtual std::shared_ptr + [[nodiscard]] virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const std::string&, const Ice::ByteSeq&) = 0; }; @@ -178,14 +178,14 @@ namespace DataStormI class DataReader : public virtual DataElement { public: - virtual bool hasWriters() = 0; + [[nodiscard]] virtual bool hasWriters() = 0; virtual void waitForWriters(int) = 0; [[nodiscard]] virtual int getInstanceCount() const = 0; - virtual std::vector> getAllUnread() = 0; + [[nodiscard]] virtual std::vector> getAllUnread() = 0; virtual void waitForUnread(unsigned int) const = 0; [[nodiscard]] virtual bool hasUnread() const = 0; - virtual std::shared_ptr getNextUnread() = 0; + [[nodiscard]] virtual std::shared_ptr getNextUnread() = 0; virtual void onSamples( std::function>&)>, @@ -224,14 +224,14 @@ namespace DataStormI class TopicReader : public virtual Topic { public: - virtual std::shared_ptr createFiltered( + [[nodiscard]] virtual std::shared_ptr createFiltered( const std::shared_ptr&, std::string, DataStorm::ReaderConfig, std::string = std::string(), Ice::ByteSeq = {}) = 0; - virtual std::shared_ptr create( + [[nodiscard]] virtual std::shared_ptr create( const std::vector>&, std::string, DataStorm::ReaderConfig, @@ -246,7 +246,7 @@ namespace DataStormI class TopicWriter : public virtual Topic { public: - virtual std::shared_ptr + [[nodiscard]] virtual std::shared_ptr create(const std::vector>&, std::string, DataStorm::WriterConfig) = 0; virtual void setDefaultConfig(DataStorm::WriterConfig) = 0; @@ -259,7 +259,7 @@ namespace DataStormI public: virtual ~TopicFactory() = default; - virtual std::shared_ptr createTopicReader( + [[nodiscard]] virtual std::shared_ptr createTopicReader( std::string, std::shared_ptr, std::shared_ptr, @@ -267,7 +267,7 @@ namespace DataStormI std::shared_ptr, std::shared_ptr) = 0; - virtual std::shared_ptr createTopicWriter( + [[nodiscard]] virtual std::shared_ptr createTopicWriter( std::string, std::shared_ptr, std::shared_ptr, diff --git a/cpp/include/DataStorm/InternalT.h b/cpp/include/DataStorm/InternalT.h index b5c564e217a..1fa73dfe275 100644 --- a/cpp/include/DataStorm/InternalT.h +++ b/cpp/include/DataStorm/InternalT.h @@ -162,13 +162,13 @@ namespace DataStormI } template - std::shared_ptr create(F&& value, Args&&... args) + [[nodiscard]] std::shared_ptr create(F&& value, Args&&... args) { std::lock_guard lock(_mutex); return createImpl(std::forward(value), std::forward(args)...); } - std::vector> create(std::vector values) + [[nodiscard]] std::vector> create(std::vector values) { std::lock_guard lock(_mutex); std::vector> seq; @@ -182,7 +182,7 @@ namespace DataStormI protected: friend struct Deleter; - std::shared_ptr getImpl(std::int64_t id) const + [[nodiscard]] std::shared_ptr getImpl(std::int64_t id) const { std::lock_guard lock(_mutex); auto p = _elementsById.find(id); @@ -193,7 +193,7 @@ namespace DataStormI return nullptr; } - template std::shared_ptr createImpl(F&& value, Args&&... args) + template [[nodiscard]] std::shared_ptr createImpl(F&& value, Args&&... args) { // Called with _mutex locked @@ -261,12 +261,13 @@ namespace DataStormI return AbstractFactoryT>::getImpl(id); } - std::shared_ptr decode(const Ice::CommunicatorPtr& communicator, const Ice::ByteSeq& data) final + [[nodiscard]] std::shared_ptr + decode(const Ice::CommunicatorPtr& communicator, const Ice::ByteSeq& data) final { return AbstractFactoryT>::create(DecoderT::decode(communicator, data)); } - static std::shared_ptr> createFactory() + [[nodiscard]] static std::shared_ptr> createFactory() { auto f = std::make_shared>(); f->init(); @@ -293,12 +294,13 @@ namespace DataStormI return AbstractFactoryT>::getImpl(id); } - std::shared_ptr decode(const Ice::CommunicatorPtr& communicator, const Ice::ByteSeq& data) final + [[nodiscard]] std::shared_ptr + decode(const Ice::CommunicatorPtr& communicator, const Ice::ByteSeq& data) final { return AbstractFactoryT>::create(DecoderT::decode(communicator, data)); } - static std::shared_ptr> createFactory() + [[nodiscard]] static std::shared_ptr> createFactory() { auto f = std::make_shared>(); f->init(); @@ -335,21 +337,24 @@ namespace DataStormI _encodedValue = std::move(value); } - DataStorm::Sample get() + [[nodiscard]] DataStorm::Sample get() { auto impl = std::enable_shared_from_this>::shared_from_this(); return DataStorm::Sample(impl); } - const Key& getKey() + [[nodiscard]] const Key& getKey() { assert(key); return std::static_pointer_cast>(key)->get(); } - const Value& getValue() const { return _value; } + [[nodiscard]] const Value& getValue() const { return _value; } - UpdateTag getTag() const { return tag ? std::static_pointer_cast>(tag)->get() : UpdateTag(); } + [[nodiscard]] UpdateTag getTag() const + { + return tag ? std::static_pointer_cast>(tag)->get() : UpdateTag(); + } void setValue(Value value) { @@ -373,7 +378,7 @@ namespace DataStormI _hasValue = true; } - const Ice::ByteSeq& encode(const Ice::CommunicatorPtr& communicator) final + [[nodiscard]] const Ice::ByteSeq& encode(const Ice::CommunicatorPtr& communicator) final { if (_encodedValue.empty()) { @@ -382,7 +387,7 @@ namespace DataStormI return _encodedValue; } - Ice::ByteSeq encodeValue(const Ice::CommunicatorPtr& communicator) final + [[nodiscard]] Ice::ByteSeq encodeValue(const Ice::CommunicatorPtr& communicator) final { assert(_hasValue || event == DataStorm::SampleEvent::Remove); return EncoderT::encode(communicator, _value); @@ -406,7 +411,7 @@ namespace DataStormI template class SampleFactoryT final : public SampleFactory { public: - std::shared_ptr create( + [[nodiscard]] std::shared_ptr create( std::string session, std::string origin, std::int64_t id, @@ -466,7 +471,7 @@ namespace DataStormI return AbstractFactoryT>::getImpl(id); } - static std::shared_ptr> createFactory() + [[nodiscard]] static std::shared_ptr> createFactory() { auto f = std::make_shared>(); f->init(); @@ -484,7 +489,7 @@ namespace DataStormI [[nodiscard]] virtual std::shared_ptr get(std::int64_t) const = 0; - virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0; + [[nodiscard]] virtual std::shared_ptr decode(const Ice::CommunicatorPtr&, const Ice::ByteSeq&) = 0; }; template struct FactoryT final : Factory @@ -495,7 +500,7 @@ namespace DataStormI { } - std::shared_ptr create(Criteria criteria) + [[nodiscard]] std::shared_ptr create(Criteria criteria) { return std::static_pointer_cast>( filterFactory.create(criteria, name, lambda(criteria))); @@ -503,7 +508,8 @@ namespace DataStormI [[nodiscard]] std::shared_ptr get(std::int64_t id) const final { return filterFactory.get(id); } - std::shared_ptr decode(const Ice::CommunicatorPtr& communicator, const Ice::ByteSeq& data) final + [[nodiscard]] std::shared_ptr + decode(const Ice::CommunicatorPtr& communicator, const Ice::ByteSeq& data) final { return create(DecoderT::decode(communicator, data)); } @@ -514,7 +520,8 @@ namespace DataStormI }; public: - template std::shared_ptr create(const std::string& name, const Criteria& criteria) + template + [[nodiscard]] std::shared_ptr create(const std::string& name, const Criteria& criteria) { auto p = _factories.find(name); if (p == _factories.end()) @@ -531,7 +538,7 @@ namespace DataStormI return factory->create(criteria); } - std::shared_ptr + [[nodiscard]] std::shared_ptr decode(const Ice::CommunicatorPtr& communicator, const std::string& name, const Ice::ByteSeq& data) final { auto p = _factories.find(name); diff --git a/cpp/include/DataStorm/Node.h b/cpp/include/DataStorm/Node.h index 8a0c9868333..8a22477bc32 100644 --- a/cpp/include/DataStorm/Node.h +++ b/cpp/include/DataStorm/Node.h @@ -140,7 +140,7 @@ namespace DataStorm * provided the Node will use the default callback executor that executes callback in a dedicated thread. */ Node( - Ice::CommunicatorPtr communicator, + const Ice::CommunicatorPtr& communicator, std::function call)> customExecutor = nullptr); /** @@ -197,7 +197,7 @@ namespace DataStorm private: Node( - Ice::CommunicatorPtr, + const Ice::CommunicatorPtr&, std::function call)> customExecutor, bool ownsCommunicator); diff --git a/cpp/include/DataStorm/Types.h b/cpp/include/DataStorm/Types.h index 9e02a1b970f..14131830d14 100644 --- a/cpp/include/DataStorm/Types.h +++ b/cpp/include/DataStorm/Types.h @@ -83,9 +83,9 @@ namespace DataStorm std::optional sampleCount = std::nullopt, std::optional sampleLifetime = std::nullopt, std::optional clearHistory = std::nullopt) noexcept - : sampleCount(std::move(sampleCount)), - sampleLifetime(std::move(sampleLifetime)), - clearHistory(std::move(clearHistory)) + : sampleCount{sampleCount}, + sampleLifetime{sampleLifetime}, + clearHistory{clearHistory} { } @@ -134,8 +134,8 @@ namespace DataStorm std::optional sampleLifetime = std::nullopt, std::optional clearHistory = std::nullopt, std::optional discardPolicy = std::nullopt) noexcept - : Config(std::move(sampleCount), std::move(sampleLifetime), std::move(clearHistory)), - discardPolicy(std::move(discardPolicy)) + : Config{sampleCount, sampleLifetime, clearHistory}, + discardPolicy{discardPolicy} { } @@ -171,8 +171,8 @@ namespace DataStorm std::optional sampleLifetime = std::nullopt, std::optional clearHistory = std::nullopt, std::optional priority = std::nullopt) noexcept - : Config(std::move(sampleCount), std::move(sampleLifetime), std::move(clearHistory)), - priority(std::move(priority)) + : Config{sampleCount, sampleLifetime, clearHistory}, + priority{priority} { } @@ -265,7 +265,7 @@ namespace DataStorm * Cloner template specialization to clone shared Ice values using ice_clone. */ template - struct Cloner, typename std::enable_if::value>::type> + struct Cloner, typename std::enable_if_t>> { static std::shared_ptr clone(const std::shared_ptr& value) noexcept { return value->ice_clone(); } }; diff --git a/cpp/include/Ice/LoggerUtil.h b/cpp/include/Ice/LoggerUtil.h index 809f378dfcf..5bbcf38c88f 100644 --- a/cpp/include/Ice/LoggerUtil.h +++ b/cpp/include/Ice/LoggerUtil.h @@ -130,7 +130,7 @@ namespace Ice class ICE_API Trace : public LoggerOutputBase { public: - Trace(const LoggerPtr&, const std::string&); + Trace(LoggerPtr, std::string); ~Trace(); void flush(); diff --git a/cpp/src/DataStorm/CallbackExecutor.cpp b/cpp/src/DataStorm/CallbackExecutor.cpp index 6baad52c105..4975b475020 100644 --- a/cpp/src/DataStorm/CallbackExecutor.cpp +++ b/cpp/src/DataStorm/CallbackExecutor.cpp @@ -8,9 +8,7 @@ using namespace std; using namespace DataStormI; CallbackExecutor::CallbackExecutor(function call)> customExecutor) - : _flush(false), - _destroyed(false), - _customExecutor(std::move(customExecutor)) + : _customExecutor(std::move(customExecutor)) { _thread = thread( [this] diff --git a/cpp/src/DataStorm/CallbackExecutor.h b/cpp/src/DataStorm/CallbackExecutor.h index 51d04176232..1030a4f0201 100644 --- a/cpp/src/DataStorm/CallbackExecutor.h +++ b/cpp/src/DataStorm/CallbackExecutor.h @@ -27,8 +27,8 @@ namespace DataStormI std::mutex _mutex; std::thread _thread; std::condition_variable _cond; - bool _flush; - bool _destroyed; + bool _flush{false}; + bool _destroyed{false}; std::vector> _queue; // An optional executor or null if no custom executor is provided during Node construction. std::function call)> _customExecutor; diff --git a/cpp/src/DataStorm/ConnectionManager.cpp b/cpp/src/DataStorm/ConnectionManager.cpp index 3ffe27fc287..32cf9af2f10 100644 --- a/cpp/src/DataStorm/ConnectionManager.cpp +++ b/cpp/src/DataStorm/ConnectionManager.cpp @@ -9,26 +9,27 @@ using namespace std; using namespace DataStormI; +using namespace Ice; ConnectionManager::ConnectionManager(const shared_ptr& executor) : _executor(executor) {} void ConnectionManager::add( - const Ice::ConnectionPtr& connection, + const ConnectionPtr& connection, shared_ptr object, - function callback) + function callback) { lock_guard lock(_mutex); auto& objects = _connections[connection]; if (objects.empty()) { - connection->setCloseCallback([self = shared_from_this()](const Ice::ConnectionPtr& con) { self->remove(con); }); + connection->setCloseCallback([self = shared_from_this()](const ConnectionPtr& con) { self->remove(con); }); } objects.emplace(std::move(object), std::move(callback)); } void -ConnectionManager::remove(const shared_ptr& object, const Ice::ConnectionPtr& connection) +ConnectionManager::remove(const shared_ptr& object, const ConnectionPtr& connection) { lock_guard lock(_mutex); auto p = _connections.find(connection); @@ -46,7 +47,7 @@ ConnectionManager::remove(const shared_ptr& object, const Ice::ConnectionP } void -ConnectionManager::remove(const Ice::ConnectionPtr& connection) +ConnectionManager::remove(const ConnectionPtr& connection) { map, Callback> objects; { diff --git a/cpp/src/DataStorm/DataElementI.cpp b/cpp/src/DataStorm/DataElementI.cpp index 92e53627fb0..126b632fd23 100644 --- a/cpp/src/DataStorm/DataElementI.cpp +++ b/cpp/src/DataStorm/DataElementI.cpp @@ -13,15 +13,16 @@ using namespace std; using namespace DataStormI; using namespace DataStormContract; +using namespace Ice; namespace { - DataSample toSample(const shared_ptr& sample, const Ice::CommunicatorPtr& communicator, bool marshalKey) + DataSample toSample(const shared_ptr& sample, const CommunicatorPtr& communicator, bool marshalKey) { return DataSample{ .id = sample->id, .keyId = marshalKey ? 0 : sample->key->getId(), - .keyValue = marshalKey ? sample->key->encode(communicator) : Ice::ByteSeq{}, + .keyValue = marshalKey ? sample->key->encode(communicator) : ByteSeq{}, .timestamp = chrono::time_point_cast(sample->timestamp).time_since_epoch().count(), .tag = sample->tag ? sample->tag->getId() : 0, .event = sample->event, @@ -51,15 +52,11 @@ DataElementI::DataElementI(TopicI* parent, string name, int64_t id, const DataSt _id(id), _config(make_shared()), _executor(parent->instance()->getCallbackExecutor()), - _listenerCount(0), // The collocated forwarder is initalized here to avoid using a nullable proxy. The forwarder is only used by // the instance that owns it and is removed in destroy implementation. _forwarder{parent->instance()->getCollocatedForwarder()->add( - [this](Ice::ByteSeq inParams, const Ice::Current& current) { forward(inParams, current); })}, - _parent(parent->shared_from_this()), - _waiters(0), - _notified(0), - _destroyed(false) + [this](const ByteSeq& inParams, const Current& current) { forward(inParams, current); })}, + _parent(parent->shared_from_this()) { _config->sampleCount = config.sampleCount; _config->sampleLifetime = config.sampleLifetime; @@ -127,8 +124,10 @@ DataElementI::attach( } // Attach the key or filter, and if attach success compute the ACK data to send to the peer. - if ((id > 0 && attachKey(topicId, data.id, key, sampleFilter, session, prx, facet, id, name, priority)) || - (id < 0 && attachFilter(topicId, data.id, key, sampleFilter, session, prx, facet, id, filter, name, priority))) + if ((id > 0 && + attachKey(topicId, data.id, key, sampleFilter, session, std::move(prx), facet, id, name, priority)) || + (id < 0 && + attachFilter(topicId, data.id, key, sampleFilter, session, std::move(prx), facet, id, filter, name, priority))) { auto q = data.lastIds.find(_id); int64_t lastId = q != data.lastIds.end() ? q->second : 0; @@ -182,8 +181,10 @@ DataElementI::attach( // - If this data element is a data reader, the computed samples will be empty. // - If this data element is a data writer, the computed samples will contain the samples to send to the peer // based on the peer reader configuration. - if ((id > 0 && attachKey(topicId, data.id, key, sampleFilter, session, prx, facet, id, name, priority)) || - (id < 0 && attachFilter(topicId, data.id, key, sampleFilter, session, prx, facet, id, filter, name, priority))) + if ((id > 0 && + attachKey(topicId, data.id, key, sampleFilter, session, std::move(prx), facet, id, name, priority)) || + (id < 0 && + attachFilter(topicId, data.id, key, sampleFilter, session, std::move(prx), facet, id, filter, name, priority))) { auto q = data.lastIds.find(_id); int64_t lastId = q != data.lastIds.end() ? q->second : 0; @@ -215,11 +216,11 @@ DataElementI::attachKey( { // No locking necessary, called by the session with the mutex locked - ListenerKey listenerKey{session, facet}; + ListenerKey listenerKey{.session = session, .facet = facet}; auto p = _listeners.find(listenerKey); if (p == _listeners.end()) { - p = _listeners.emplace(std::move(listenerKey), Listener(std::move(prx), facet)).first; + p = _listeners.emplace(std::move(listenerKey), Listener{std::move(prx), facet}).first; } bool added = false; @@ -240,7 +241,7 @@ DataElementI::attachKey( if (_traceLevels->data > 1) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": attach e" << elementId << ":" << name; if (!facet.empty()) { @@ -297,7 +298,7 @@ DataElementI::detachKey( if (_traceLevels->data > 1) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": detach e" << elementId << ":" << subscriber->name; if (!facet.empty()) { @@ -333,7 +334,7 @@ DataElementI::attachFilter( auto p = _listeners.find({session, facet}); if (p == _listeners.end()) { - p = _listeners.emplace(ListenerKey{session, facet}, Listener(prx, facet)).first; + p = _listeners.emplace(ListenerKey{.session = session, .facet = facet}, Listener{std::move(prx), facet}).first; } bool added = false; @@ -351,7 +352,7 @@ DataElementI::attachFilter( } if (_traceLevels->data > 1) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": attach e" << elementId << ":" << name; if (!facet.empty()) { @@ -408,7 +409,7 @@ DataElementI::detachFilter( if (_traceLevels->data > 1) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": detach e" << elementId << ":" << subscriber->name; if (!facet.empty()) { @@ -432,6 +433,7 @@ DataElementI::getConnectedKeys() const { unique_lock lock(_parent->_mutex); vector> keys; + keys.reserve(_connectedKeys.size()); for (const auto& key : _connectedKeys) { keys.push_back(key.first); @@ -444,7 +446,6 @@ DataElementI::getConnectedElements() const { unique_lock lock(_parent->_mutex); vector elements; - elements.reserve(_listeners.size()); for (const auto& listener : _listeners) { for (const auto& subscriber : listener.second.subscribers) @@ -465,11 +466,12 @@ DataElementI::onConnectedKeys( if (init) { vector> keys; - for (const auto& key : _connectedKeys) + keys.reserve(_connectedKeys.size()); + for (const auto& [key, _] : _connectedKeys) { - keys.push_back(key.first); + keys.push_back(key); } - _executor->queue([self = shared_from_this(), init, keys = std::move(keys)] { init(std::move(keys)); }, true); + _executor->queue([init = std::move(init), keys = std::move(keys)]() mutable { init(std::move(keys)); }, true); } } @@ -485,12 +487,14 @@ DataElementI::onConnectedElements( vector elements; for (const auto& listener : _listeners) { - for (const auto& subscriber : listener.second.subscribers) + for (const auto& [_, subscriber] : listener.second.subscribers) { - elements.push_back(subscriber.second->name); + elements.push_back(subscriber->name); } } - _executor->queue([init, elements = std::move(elements)] { init(std::move(elements)); }, true); + _executor->queue( + [init = std::move(init), elements = std::move(elements)]() mutable { init(std::move(elements)); }, + true); } } @@ -564,7 +568,7 @@ DataElementI::hasListeners() const return _listenerCount > 0; } -Ice::CommunicatorPtr +CommunicatorPtr DataElementI::getCommunicator() const { return _parent->instance()->getCommunicator(); @@ -649,7 +653,7 @@ DataElementI::disconnect() } void -DataElementI::forward(const Ice::ByteSeq& inParams, const Ice::Current& current) const +DataElementI::forward(const ByteSeq& inParams, const Current& current) const { for (const auto& [_, listener] : _listeners) { @@ -668,7 +672,7 @@ DataReaderI::DataReaderI( string name, int64_t id, string sampleFilterName, - Ice::ByteSeq sampleFilterCriteria, + ByteSeq sampleFilterCriteria, const DataStorm::ReaderConfig& config) : DataElementI(topic, std::move(name), id, config), _parent(topic), @@ -676,7 +680,8 @@ DataReaderI::DataReaderI( { if (!sampleFilterName.empty()) { - _config->sampleFilter = FilterInfo{std::move(sampleFilterName), std::move(sampleFilterCriteria)}; + _config->sampleFilter = + FilterInfo{.name = std::move(sampleFilterName), .criteria = std::move(sampleFilterCriteria)}; } } @@ -743,7 +748,7 @@ DataReaderI::initSamples( { if (_traceLevels->data > 1) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": initialized " << samples.size() << " samples from `" << element << '@' << topic << "'"; } @@ -784,7 +789,7 @@ DataReaderI::initSamples( if (_traceLevels->data > 2 && valid.size() < samples.size()) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": discarded " << samples.size() - valid.size() << " samples from `" << element << '@' << topic << "'"; } @@ -817,7 +822,7 @@ DataReaderI::initSamples( if (*_config->sampleCount > 0) { size_t count = _samples.size(); - size_t maxCount = static_cast(*_config->sampleCount); + auto maxCount = static_cast(*_config->sampleCount); if (count + valid.size() > maxCount) { count = count + valid.size() - maxCount; @@ -873,7 +878,7 @@ DataReaderI::queue( { if (_traceLevels->data > 2) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": skipped sample " << sample->id << " (facet doesn't match)"; } return; @@ -882,7 +887,7 @@ DataReaderI::queue( { if (_traceLevels->data > 2) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": skipped sample " << sample->id << " (key doesn't match)"; } return; @@ -890,7 +895,7 @@ DataReaderI::queue( if (_traceLevels->data > 2) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": queued sample " << sample->id << " listeners=" << _listenerCount; } @@ -900,7 +905,7 @@ DataReaderI::queue( { if (_traceLevels->data > 2) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": discarded sample" << sample->id; } return; @@ -935,7 +940,7 @@ DataReaderI::queue( if (*_config->sampleCount > 0) { size_t count = _samples.size(); - size_t maxCount = static_cast(*_config->sampleCount); + auto maxCount = static_cast(*_config->sampleCount); if (count + 1 > maxCount) { if (!_samples.empty()) @@ -1004,7 +1009,7 @@ DataReaderI::addConnectedKey(const shared_ptr& key, const shared_ptr(_forwarder)} + _subscribers{uncheckedCast(_forwarder)} { _config->priority = config.priority; } @@ -1024,7 +1029,7 @@ DataWriterI::publish(const shared_ptr& key, const shared_ptr& sampl if (_traceLevels->data > 2) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": publishing sample " << sample->id << " listeners=" << _listenerCount; } send(key, sample); @@ -1069,14 +1074,14 @@ KeyDataReaderI::KeyDataReaderI( int64_t id, const vector>& keys, string sampleFilterName, - const Ice::ByteSeq sampleFilterCriteria, + ByteSeq sampleFilterCriteria, const DataStorm::ReaderConfig& config) - : DataReaderI(topic, std::move(name), id, std::move(sampleFilterName), sampleFilterCriteria, config), + : DataReaderI(topic, std::move(name), id, std::move(sampleFilterName), std::move(sampleFilterCriteria), config), _keys(keys) { if (_traceLevels->data > 0) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": created key reader"; } @@ -1094,7 +1099,7 @@ KeyDataReaderI::destroyImpl() { if (_traceLevels->data > 0) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": destroyed key reader"; } try @@ -1159,7 +1164,7 @@ KeyDataWriterI::KeyDataWriterI( { if (_traceLevels->data > 0) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": created key writer"; } } @@ -1169,7 +1174,7 @@ KeyDataWriterI::destroyImpl() { if (_traceLevels->data > 0) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": destroyed key writer"; } try @@ -1336,7 +1341,7 @@ KeyDataWriterI::send(const shared_ptr& key, const shared_ptr& sampl } void -KeyDataWriterI::forward(const Ice::ByteSeq& inParams, const Ice::Current& current) const +KeyDataWriterI::forward(const ByteSeq& inParams, const Current& current) const { for (const auto& [_, listener] : _listeners) { @@ -1362,14 +1367,14 @@ FilteredDataReaderI::FilteredDataReaderI( int64_t id, shared_ptr filter, string sampleFilterName, - Ice::ByteSeq sampleFilterCriteria, + ByteSeq sampleFilterCriteria, const DataStorm::ReaderConfig& config) : DataReaderI(topic, std::move(name), id, std::move(sampleFilterName), std::move(sampleFilterCriteria), config), _filter(std::move(filter)) { if (_traceLevels->data > 0) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": created filtered reader"; } @@ -1387,7 +1392,7 @@ FilteredDataReaderI::destroyImpl() { if (_traceLevels->data > 0) { - Trace out(_traceLevels, _traceLevels->dataCat); + Trace out(_traceLevels->logger, _traceLevels->dataCat); out << this << ": destroyed filter reader"; } try diff --git a/cpp/src/DataStorm/DataElementI.h b/cpp/src/DataStorm/DataElementI.h index 1968f28cbb0..40e757e611a 100644 --- a/cpp/src/DataStorm/DataElementI.h +++ b/cpp/src/DataStorm/DataElementI.h @@ -107,7 +107,7 @@ namespace DataStormI return false; } - std::shared_ptr addOrGet( + [[nodiscard]] std::shared_ptr addOrGet( std::int64_t topicId, std::int64_t elementId, std::int64_t id, @@ -128,12 +128,12 @@ namespace DataStormI return p->second; } - std::shared_ptr get(std::int64_t topicId, std::int64_t elementId) + [[nodiscard]] std::shared_ptr get(std::int64_t topicId, std::int64_t elementId) { return subscribers.find(std::make_pair(topicId, elementId))->second; } - bool remove(std::int64_t topicId, std::int64_t elementId) + [[nodiscard]] bool remove(std::int64_t topicId, std::int64_t elementId) { subscribers.erase(std::make_pair(topicId, elementId)); return subscribers.empty(); @@ -199,7 +199,7 @@ namespace DataStormI /// - For a publisher, this method always returns a `nullptr` function. /// - For a subscriber, this method returns a function that initializes the reader with samples provided by the /// peer. - std::function attach( + [[nodiscard]] std::function attach( std::int64_t topicId, std::int64_t id, const std::shared_ptr& key, @@ -210,7 +210,7 @@ namespace DataStormI const std::chrono::time_point& now, DataStormContract::DataSamplesSeq& samples); - bool attachKey( + [[nodiscard]] bool attachKey( std::int64_t, std::int64_t, const std::shared_ptr&, @@ -230,7 +230,7 @@ namespace DataStormI const std::string&, bool); - bool attachFilter( + [[nodiscard]] bool attachFilter( std::int64_t, std::int64_t, const std::shared_ptr&, @@ -251,8 +251,8 @@ namespace DataStormI const std::string&, bool); - std::vector> getConnectedKeys() const override; - std::vector getConnectedElements() const override; + [[nodiscard]] std::vector> getConnectedKeys() const override; + [[nodiscard]] std::vector getConnectedElements() const override; void onConnectedKeys( std::function>)>, @@ -287,16 +287,16 @@ namespace DataStormI virtual std::string toString() const = 0; - Ice::CommunicatorPtr getCommunicator() const override; + [[nodiscard]] Ice::CommunicatorPtr getCommunicator() const override; - std::int64_t getId() const { return _id; } + [[nodiscard]] std::int64_t getId() const { return _id; } - std::shared_ptr getConfig() const; + [[nodiscard]] std::shared_ptr getConfig() const; void waitForListeners(int count) const; - bool hasListeners() const; + [[nodiscard]] bool hasListeners() const; - TopicI* getTopic() const { return _parent.get(); } + [[nodiscard]] TopicI* getTopic() const { return _parent.get(); } protected: virtual bool addConnectedKey(const std::shared_ptr& key, const std::shared_ptr& subscriber); @@ -312,7 +312,7 @@ namespace DataStormI const std::shared_ptr _config; const std::shared_ptr _executor; - size_t _listenerCount; + size_t _listenerCount{0}; mutable std::shared_ptr _sample; DataStormContract::SessionPrx _forwarder; // A map containing the connected keys, these are keys attached to a peer key. @@ -327,9 +327,9 @@ namespace DataStormI virtual void forward(const Ice::ByteSeq&, const Ice::Current&) const; const std::shared_ptr _parent; - mutable size_t _waiters; - mutable size_t _notified; - bool _destroyed; + mutable size_t _waiters{0}; + mutable size_t _notified{0}; + bool _destroyed{false}; std::function)> _onConnectedKeys; std::function _onConnectedElements; @@ -346,12 +346,12 @@ namespace DataStormI Ice::ByteSeq, const DataStorm::ReaderConfig&); - int getInstanceCount() const override; + [[nodiscard]] int getInstanceCount() const override; - std::vector> getAllUnread() override; + [[nodiscard]] std::vector> getAllUnread() override; void waitForUnread(unsigned int) const override; - bool hasUnread() const override; - std::shared_ptr getNextUnread() override; + [[nodiscard]] bool hasUnread() const override; + [[nodiscard]] std::shared_ptr getNextUnread() override; void initSamples( const std::vector>&, @@ -375,7 +375,7 @@ namespace DataStormI protected: virtual bool matchKey(const std::shared_ptr&) const = 0; - bool addConnectedKey(const std::shared_ptr&, const std::shared_ptr&) override; + [[nodiscard]] bool addConnectedKey(const std::shared_ptr&, const std::shared_ptr&) override; TopicReaderI* _parent; @@ -418,9 +418,9 @@ namespace DataStormI void destroyImpl() final; void waitForWriters(int) final; - bool hasWriters() final; + [[nodiscard]] bool hasWriters() final; - std::string toString() const final; + [[nodiscard]] std::string toString() const final; private: bool matchKey(const std::shared_ptr&) const final; @@ -441,14 +441,14 @@ namespace DataStormI void destroyImpl() final; void waitForReaders(int) const final; - bool hasReaders() const final; + [[nodiscard]] bool hasReaders() const final; - std::shared_ptr getLast() const final; - std::vector> getAll() const final; + [[nodiscard]] std::shared_ptr getLast() const final; + [[nodiscard]] std::vector> getAll() const final; - std::string toString() const final; + [[nodiscard]] std::string toString() const final; - DataStormContract::DataSamples getSamples( + [[nodiscard]] DataStormContract::DataSamples getSamples( const std::shared_ptr&, const std::shared_ptr&, const std::shared_ptr&, @@ -477,12 +477,12 @@ namespace DataStormI void destroyImpl() final; void waitForWriters(int) final; - bool hasWriters() final; + [[nodiscard]] bool hasWriters() final; - std::string toString() const final; + [[nodiscard]] std::string toString() const final; private: - bool matchKey(const std::shared_ptr&) const final; + [[nodiscard]] bool matchKey(const std::shared_ptr&) const final; const std::shared_ptr _filter; }; diff --git a/cpp/src/DataStorm/ForwarderManager.cpp b/cpp/src/DataStorm/ForwarderManager.cpp index 5b2c51dfad5..2be15952d84 100644 --- a/cpp/src/DataStorm/ForwarderManager.cpp +++ b/cpp/src/DataStorm/ForwarderManager.cpp @@ -6,16 +6,16 @@ using namespace std; using namespace DataStormI; +using namespace Ice; -ForwarderManager::ForwarderManager(const Ice::ObjectAdapterPtr& adapter, const string& category) - : _adapter(adapter), - _category(category), - _nextId(0) +ForwarderManager::ForwarderManager(ObjectAdapterPtr adapter, string category) + : _adapter(std::move(adapter)), + _category(std::move(category)) { } void -ForwarderManager::remove(const Ice::Identity& id) +ForwarderManager::remove(const Identity& id) { lock_guard lock(_mutex); _forwarders.erase(id.name); @@ -30,18 +30,18 @@ ForwarderManager::destroy() void ForwarderManager::ice_invokeAsync( - Ice::ByteSeq inParams, - function response, + ByteSeq inParams, + function response, function exception, - const Ice::Current& current) + const Current& current) { - std::function forwarder; + std::function forwarder; { lock_guard lock(_mutex); auto p = _forwarders.find(current.id.name); if (p == _forwarders.end()) { - throw Ice::ObjectNotExistException{__FILE__, __LINE__}; + throw ObjectNotExistException{__FILE__, __LINE__}; } forwarder = p->second; } diff --git a/cpp/src/DataStorm/ForwarderManager.h b/cpp/src/DataStorm/ForwarderManager.h index 455b1972baa..ce025664f77 100644 --- a/cpp/src/DataStorm/ForwarderManager.h +++ b/cpp/src/DataStorm/ForwarderManager.h @@ -19,25 +19,25 @@ namespace DataStormI using Response = std::function; using Exception = std::function; - ForwarderManager(const Ice::ObjectAdapterPtr&, const std::string&); + ForwarderManager(Ice::ObjectAdapterPtr, std::string); - template::value, bool> = true> + template, bool> = true> Prx add(std::function forwarder) { std::lock_guard lock(_mutex); - const Ice::Identity id = {std::to_string(_nextId++), _category}; + const Ice::Identity id{.name = std::to_string(_nextId++), .category = _category}; _forwarders.emplace(id.name, std::move(forwarder)); - return _adapter->createProxy(id); + return _adapter->createProxy(std::move(id)); } - template::value, bool> = true> + template, bool> = true> Prx add(std::function forwarder) { return add( [forwarder = std::move(forwarder)]( Ice::ByteSeq inParams, - Response response, - Exception exception, + std::function response, + std::function exception, const Ice::Current& current) { try @@ -68,7 +68,7 @@ namespace DataStormI std::mutex _mutex; std::map> _forwarders; - unsigned int _nextId; + unsigned int _nextId{0}; }; } #endif diff --git a/cpp/src/DataStorm/Instance.cpp b/cpp/src/DataStorm/Instance.cpp index f87d78144dd..3a7c3a87834 100644 --- a/cpp/src/DataStorm/Instance.cpp +++ b/cpp/src/DataStorm/Instance.cpp @@ -14,12 +14,12 @@ using namespace std; using namespace DataStormI; +using namespace Ice; -Instance::Instance(const Ice::CommunicatorPtr& communicator, function call)> customExecutor) - : _communicator(communicator), - _shutdown(false) +Instance::Instance(CommunicatorPtr communicator, function call)> customExecutor) + : _communicator(std::move(communicator)) { - Ice::PropertiesPtr properties = _communicator->getProperties(); + PropertiesPtr properties = _communicator->getProperties(); if (properties->getIcePropertyAsInt("DataStorm.Node.Server.Enabled") > 0) { @@ -33,10 +33,10 @@ Instance::Instance(const Ice::CommunicatorPtr& communicator, functioncreateObjectAdapter("DataStorm.Node.Server"); } - catch (const Ice::LocalException& ex) + catch (const LocalException& ex) { ostringstream os; - os << "failed to listen on server endpoints `"; + os << "failed to listen on server endpoints '"; os << properties->getIceProperty("DataStorm.Node.Server.Endpoints") << "':\n"; os << ex.what(); throw invalid_argument(os.str()); @@ -62,10 +62,10 @@ Instance::Instance(const Ice::CommunicatorPtr& communicator, functioncreateObjectAdapter("DataStorm.Node.Multicast"); } - catch (const Ice::LocalException& ex) + catch (const LocalException& ex) { ostringstream os; - os << "failed to listen on multicast endpoints `"; + os << "failed to listen on multicast endpoints '"; os << properties->getIceProperty("DataStorm.Node.Multicast.Endpoints") << "':\n"; os << ex.what(); throw invalid_argument(os.str()); @@ -81,7 +81,7 @@ Instance::Instance(const Ice::CommunicatorPtr& communicator, functionsetProperty(collocated + ".AdapterId", collocated); _collocatedAdapter = _communicator->createObjectAdapter(collocated); @@ -108,10 +108,12 @@ Instance::init() _nodeSessionManager->init(); auto lookupI = make_shared(_nodeSessionManager, _topicFactory, _node->getProxy()); - _adapter->add(lookupI, {"Lookup", "DataStorm"}); + _adapter->add(lookupI, Identity{.name = "Lookup", .category = "DataStorm"}); if (_multicastAdapter) { - auto lookup = _multicastAdapter->add(lookupI, {"Lookup", "DataStorm"}); + auto lookup = _multicastAdapter->add( + lookupI, + Identity{.name = "Lookup", .category = "DataStorm"}); // The lookup proxy can be customized by setting the property DataStorm.Node.Multicast.Proxy. if (!_communicator->getProperties()->getIceProperty("DataStorm.Node.Multicast.Proxy").empty()) { diff --git a/cpp/src/DataStorm/Instance.h b/cpp/src/DataStorm/Instance.h index 05e6fc17e62..ee3c108fbc7 100644 --- a/cpp/src/DataStorm/Instance.h +++ b/cpp/src/DataStorm/Instance.h @@ -25,77 +25,75 @@ namespace DataStormI class Instance final : public std::enable_shared_from_this { public: - Instance( - const Ice::CommunicatorPtr& communicator, - std::function call)> customExecutor); + Instance(Ice::CommunicatorPtr communicator, std::function call)> customExecutor); void init(); - std::shared_ptr getConnectionManager() const + [[nodiscard]] std::shared_ptr getConnectionManager() const { assert(_connectionManager); return _connectionManager; } - std::shared_ptr getNodeSessionManager() const + [[nodiscard]] std::shared_ptr getNodeSessionManager() const { assert(_nodeSessionManager); return _nodeSessionManager; } - Ice::CommunicatorPtr getCommunicator() const + [[nodiscard]] Ice::CommunicatorPtr getCommunicator() const { assert(_communicator); return _communicator; } - Ice::ObjectAdapterPtr getObjectAdapter() const + [[nodiscard]] Ice::ObjectAdapterPtr getObjectAdapter() const { assert(_adapter); return _adapter; } - std::shared_ptr getCollocatedForwarder() const + [[nodiscard]] std::shared_ptr getCollocatedForwarder() const { assert(_collocatedForwarder); return _collocatedForwarder; } - std::optional getLookup() const { return _lookup; } + [[nodiscard]] std::optional getLookup() const { return _lookup; } - std::shared_ptr getTopicFactory() const + [[nodiscard]] std::shared_ptr getTopicFactory() const { assert(_topicFactory); return _topicFactory; } - std::shared_ptr getTraceLevels() const + [[nodiscard]] std::shared_ptr getTraceLevels() const { assert(_traceLevels); return _traceLevels; } - std::shared_ptr getNode() const + [[nodiscard]] std::shared_ptr getNode() const { assert(_node); return _node; } - std::shared_ptr getCallbackExecutor() const + [[nodiscard]] std::shared_ptr getCallbackExecutor() const { assert(_executor); return _executor; } - std::chrono::milliseconds getRetryDelay(int count) const + [[nodiscard]] std::chrono::milliseconds getRetryDelay(int count) const { return _retryDelay * static_cast(std::pow(_retryMultiplier, std::min(count, _retryCount))); } - int getRetryCount() const { return _retryCount; } + [[nodiscard]] int getRetryCount() const { return _retryCount; } void shutdown(); - bool isShutdown() const; + [[nodiscard]] bool isShutdown() const; void checkShutdown() const; void waitForShutdown() const; @@ -114,16 +112,16 @@ namespace DataStormI } } - void scheduleTimerTask(IceInternal::TimerTaskPtr task, const std::chrono::milliseconds& delay) + void scheduleTimerTask(const IceInternal::TimerTaskPtr& task, const std::chrono::milliseconds& delay) { std::unique_lock lock(_mutex); if (_timer) { - _timer->schedule(std::move(task), delay); + _timer->schedule(task, delay); } } - void cancelTimerTask(IceInternal::TimerTaskPtr task) + void cancelTimerTask(const IceInternal::TimerTaskPtr& task) { std::unique_lock lock(_mutex); if (_timer) @@ -152,7 +150,7 @@ namespace DataStormI mutable std::mutex _mutex; mutable std::condition_variable _cond; - bool _shutdown; + bool _shutdown{false}; }; } #endif diff --git a/cpp/src/DataStorm/LookupI.cpp b/cpp/src/DataStorm/LookupI.cpp index fb18138f5c5..dfebf9807f5 100644 --- a/cpp/src/DataStorm/LookupI.cpp +++ b/cpp/src/DataStorm/LookupI.cpp @@ -10,6 +10,7 @@ using namespace std; using namespace DataStormContract; using namespace DataStormI; +using namespace Ice; LookupI::LookupI( shared_ptr nodeSessionManager, @@ -22,9 +23,9 @@ LookupI::LookupI( } void -LookupI::announceTopicReader(string name, optional subscriber, const Ice::Current& current) +LookupI::announceTopicReader(string name, optional subscriber, const Current& current) { - Ice::checkNotNull(subscriber, __FILE__, __LINE__, current); + checkNotNull(subscriber, __FILE__, __LINE__, current); // Forward the announcement to known nodes via the node session manager. _nodeSessionManager->announceTopicReader(name, *subscriber, current.con); @@ -34,9 +35,9 @@ LookupI::announceTopicReader(string name, optional subscriber, const Ic } void -LookupI::announceTopicWriter(string name, optional publisher, const Ice::Current& current) +LookupI::announceTopicWriter(string name, optional publisher, const Current& current) { - Ice::checkNotNull(publisher, __FILE__, __LINE__, current); + checkNotNull(publisher, __FILE__, __LINE__, current); // Forward the announcement to known nodes via the node session manager. _nodeSessionManager->announceTopicWriter(name, *publisher, current.con); @@ -46,13 +47,9 @@ LookupI::announceTopicWriter(string name, optional publisher, const Ice } void -LookupI::announceTopics( - Ice::StringSeq readers, - Ice::StringSeq writers, - optional proxy, - const Ice::Current& current) +LookupI::announceTopics(StringSeq readers, StringSeq writers, optional proxy, const Current& current) { - Ice::checkNotNull(proxy, __FILE__, __LINE__, current); + checkNotNull(proxy, __FILE__, __LINE__, current); // Forward the announcement to known nodes via the node session manager. _nodeSessionManager->announceTopics(readers, writers, *proxy, current.con); @@ -72,9 +69,9 @@ LookupI::announceTopics( } optional -LookupI::createSession(optional node, const Ice::Current& current) +LookupI::createSession(optional node, const Current& current) { - Ice::checkNotNull(node, __FILE__, __LINE__, current); + checkNotNull(node, __FILE__, __LINE__, current); _nodeSessionManager->createOrGet(std::move(*node), current.con, true); return _nodePrx; } diff --git a/cpp/src/DataStorm/Node.cpp b/cpp/src/DataStorm/Node.cpp index e6b54e2dfab..dabc9610cf0 100644 --- a/cpp/src/DataStorm/Node.cpp +++ b/cpp/src/DataStorm/Node.cpp @@ -10,55 +10,56 @@ using namespace std; using namespace DataStorm; +using namespace Ice; namespace { - Ice::CommunicatorPtr argsToCommunicator(int& argc, const char* argv[], optional configFile) + CommunicatorPtr argsToCommunicator(int& argc, const char* argv[], optional configFile) { - Ice::PropertiesPtr properties = make_shared(vector{"DataStorm"}); + PropertiesPtr properties = make_shared(vector{"DataStorm"}); if (configFile) { properties->load(*configFile); } - Ice::InitializationData initData; + InitializationData initData; initData.properties = properties; - return Ice::initialize(argc, argv, initData); + return initialize(argc, argv, initData); } #ifdef _WIN32 - Ice::CommunicatorPtr argsToCommunicator(int& argc, const wchar_t* argv[], optional configFile) + CommunicatorPtr argsToCommunicator(int& argc, const wchar_t* argv[], optional configFile) { - Ice::PropertiesPtr properties = make_shared(vector{"DataStorm"}); + PropertiesPtr properties = make_shared(vector{"DataStorm"}); if (configFile) { properties->load(*configFile); } - Ice::InitializationData initData; + InitializationData initData; initData.properties = properties; - return Ice::initialize(argc, argv, initData); + return initialize(argc, argv, initData); } #endif - Ice::CommunicatorPtr configToCommunicator(optional configFile) + CommunicatorPtr configToCommunicator(optional configFile) { - Ice::PropertiesPtr properties = make_shared(vector{"DataStorm"}); + PropertiesPtr properties = make_shared(vector{"DataStorm"}); if (configFile) { properties->load(*configFile); } - Ice::InitializationData initData; + InitializationData initData; initData.properties = properties; - return Ice::initialize(initData); + return initialize(initData); } } @@ -93,13 +94,13 @@ Node::Node(optional configFile, function call { } -Node::Node(Ice::CommunicatorPtr communicator, function call)> customExecutor) - : Node(std::move(communicator), std::move(customExecutor), false) +Node::Node(const CommunicatorPtr& communicator, function call)> customExecutor) + : Node(communicator, std::move(customExecutor), false) { } Node::Node( - Ice::CommunicatorPtr communicator, + const CommunicatorPtr& communicator, std::function call)> customExecutor, bool ownsCommunicator) : _ownsCommunicator(ownsCommunicator) @@ -162,13 +163,13 @@ Node::operator=(Node&& node) noexcept return *this; } -Ice::CommunicatorPtr +CommunicatorPtr Node::getCommunicator() const noexcept { return _instance ? _instance->getCommunicator() : nullptr; } -Ice::ConnectionPtr +ConnectionPtr Node::getSessionConnection(string_view ident) const noexcept { return _instance ? _instance->getNode()->getSessionConnection(ident) : nullptr; diff --git a/cpp/src/DataStorm/NodeI.cpp b/cpp/src/DataStorm/NodeI.cpp index 6cb143b11c5..c8e26a5c998 100644 --- a/cpp/src/DataStorm/NodeI.cpp +++ b/cpp/src/DataStorm/NodeI.cpp @@ -14,10 +14,11 @@ using namespace std; using namespace DataStormI; using namespace DataStormContract; +using namespace Ice; namespace { - class SessionDispatcher : public Ice::Object + class SessionDispatcher : public Object { public: SessionDispatcher(shared_ptr node, shared_ptr executor) @@ -26,7 +27,7 @@ namespace { } - void dispatch(Ice::IncomingRequest& request, std::function sendResponse) final + void dispatch(IncomingRequest& request, std::function sendResponse) final { if (auto session = _node->getSession(request.current().id)) { @@ -35,7 +36,7 @@ namespace } else { - throw Ice::ObjectNotExistException{__FILE__, __LINE__}; + throw ObjectNotExistException{__FILE__, __LINE__}; } } @@ -47,15 +48,13 @@ namespace NodeI::NodeI(const shared_ptr& instance) : _instance(instance), - _nextPublisherSessionId{0}, - _nextSubscriberSessionId{0}, - _proxy{instance->getObjectAdapter()->createProxy({Ice::generateUUID(), ""})}, + _proxy{instance->getObjectAdapter()->createProxy(Identity{.name = generateUUID(), .category = ""})}, // The subscriber and publisher collocated forwarders are initalized here to avoid using a nullable proxy. These // objects are only used after the node is initialized and are removed in destroy implementation. _publisherForwarder{instance->getCollocatedForwarder()->add( - [this](Ice::ByteSeq inParams, const Ice::Current& current) { forwardToPublishers(inParams, current); })}, + [this](const ByteSeq& inParams, const Current& current) { forwardToPublishers(inParams, current); })}, _subscriberForwarder{instance->getCollocatedForwarder()->add( - [this](Ice::ByteSeq inParams, const Ice::Current& current) { forwardToSubscribers(inParams, current); })} + [this](const ByteSeq& inParams, const Current& current) { forwardToSubscribers(inParams, current); })} { } @@ -106,7 +105,7 @@ NodeI::destroy(bool ownsCommunicator) // Notify subscriber session of the disconnection, don't need to wait for the result. session->disconnectedAsync(nullptr); } - catch (const Ice::LocalException&) + catch (const LocalException&) { } } @@ -121,7 +120,7 @@ NodeI::destroy(bool ownsCommunicator) // Notify publisher session of the disconnection, don't need to wait for the result. session->disconnectedAsync(nullptr); } - catch (const Ice::LocalException&) + catch (const LocalException&) { } } @@ -134,11 +133,11 @@ NodeI::destroy(bool ownsCommunicator) } void -NodeI::initiateCreateSession(optional publisher, const Ice::Current& current) +NodeI::initiateCreateSession(optional publisher, const Current& current) { - Ice::checkNotNull(publisher, __FILE__, __LINE__, current); + checkNotNull(publisher, __FILE__, __LINE__, current); // Create a session with the given publisher. - createPublisherSession(std::move(*publisher), current.con, nullptr); + createPublisherSession(*publisher, current.con, nullptr); } void @@ -146,10 +145,10 @@ NodeI::createSession( optional subscriber, optional subscriberSession, bool fromRelay, - const Ice::Current& current) + const Current& current) { - Ice::checkNotNull(subscriber, __FILE__, __LINE__, current); - Ice::checkNotNull(subscriberSession, __FILE__, __LINE__, current); + checkNotNull(subscriber, __FILE__, __LINE__, current); + checkNotNull(subscriberSession, __FILE__, __LINE__, current); auto instance = _instance.lock(); assert(instance); @@ -163,7 +162,7 @@ NodeI::createSession( // If the call is from a relay, we check if we already have a connection to this node and eventually re-use // it. Otherwise, we'll try to establish a connection to the node if it has endpoints. If it doesn't, we'll // re-use the current connection to send the confirmation. - s = getNodeWithExistingConnection(std::move(instance), s, current.con); + s = getNodeWithExistingConnection(instance, s, current.con); } else if (current.con) { @@ -178,7 +177,7 @@ NodeI::createSession( } s->ice_getConnectionAsync( - [=, self = shared_from_this()](auto connection) mutable + [=, self = shared_from_this()](const auto& connection) mutable { if (session->checkSession()) { @@ -199,7 +198,7 @@ NodeI::createSession( // Must be called before connected s->confirmCreateSessionAsync( self->_proxy, - Ice::uncheckedCast(session->getProxy()), + uncheckedCast(session->getProxy()), nullptr, [self, subscriber, session](auto ex) { self->removePublisherSession(*subscriber, session, ex); }); @@ -208,7 +207,7 @@ NodeI::createSession( // Session::connected informs the subscriber session of all the topic writers in the current node. session->connected(*subscriberSession, connection, instance->getTopicFactory()->getTopicWriters()); } - catch (const Ice::LocalException&) + catch (const LocalException&) { self->removePublisherSession(*subscriber, session, current_exception()); } @@ -216,7 +215,7 @@ NodeI::createSession( [self = shared_from_this(), subscriber, session](auto ex) { self->removePublisherSession(*subscriber, session, ex); }); } - catch (const Ice::LocalException&) + catch (const LocalException&) { removePublisherSession(*subscriber, session, current_exception()); } @@ -226,10 +225,10 @@ void NodeI::confirmCreateSession( optional publisher, optional publisherSession, - const Ice::Current& current) + const Current& current) { - Ice::checkNotNull(publisher, __FILE__, __LINE__, current); - Ice::checkNotNull(publisherSession, __FILE__, __LINE__, current); + checkNotNull(publisher, __FILE__, __LINE__, current); + checkNotNull(publisherSession, __FILE__, __LINE__, current); unique_lock lock(_mutex); auto p = _subscribers.find(publisher->ice_getIdentity()); @@ -259,7 +258,7 @@ NodeI::confirmCreateSession( void NodeI::createSubscriberSession( NodePrx subscriber, - const Ice::ConnectionPtr& subscriberConnection, + const ConnectionPtr& subscriberConnection, const shared_ptr& session) { auto instance = _instance.lock(); @@ -267,10 +266,10 @@ NodeI::createSubscriberSession( try { - subscriber = getNodeWithExistingConnection(std::move(instance), subscriber, subscriberConnection); + subscriber = getNodeWithExistingConnection(instance, subscriber, subscriberConnection); subscriber->ice_getConnectionAsync( - [=, self = shared_from_this()](auto connection) + [=, self = shared_from_this()](const auto& connection) { if (connection && !connection->getAdapter()) { @@ -284,7 +283,7 @@ NodeI::createSubscriberSession( [subscriber, session, self = shared_from_this()](auto ex) { self->removePublisherSession(subscriber, session, ex); }); } - catch (const Ice::LocalException&) + catch (const LocalException&) { removePublisherSession(subscriber, session, current_exception()); } @@ -292,8 +291,8 @@ NodeI::createSubscriberSession( void NodeI::createPublisherSession( - NodePrx publisher, - const Ice::ConnectionPtr& publisherConnection, + const NodePrx& publisher, + const ConnectionPtr& publisherConnection, shared_ptr session) { auto instance = _instance.lock(); @@ -301,7 +300,7 @@ NodeI::createPublisherSession( try { - auto p = getNodeWithExistingConnection(std::move(instance), publisher, publisherConnection); + auto p = getNodeWithExistingConnection(instance, publisher, publisherConnection); unique_lock lock(_mutex); if (!session) @@ -314,7 +313,7 @@ NodeI::createPublisherSession( } p->ice_getConnectionAsync( - [=, self = shared_from_this()](auto connection) + [=, self = shared_from_this()](const auto& connection) { if (session->checkSession()) { @@ -330,12 +329,12 @@ NodeI::createPublisherSession( { p->createSessionAsync( self->_proxy, - Ice::uncheckedCast(session->getProxy()), + uncheckedCast(session->getProxy()), false, nullptr, [=](exception_ptr ex) { self->removeSubscriberSession(publisher, session, ex); }); } - catch (const Ice::LocalException&) + catch (const LocalException&) { self->removeSubscriberSession(publisher, session, current_exception()); } @@ -343,14 +342,14 @@ NodeI::createPublisherSession( [publisher, session, self = shared_from_this()](exception_ptr ex) { self->removeSubscriberSession(publisher, session, ex); }); } - catch (const Ice::LocalException&) + catch (const LocalException&) { removeSubscriberSession(publisher, session, current_exception()); } } void -NodeI::removeSubscriberSession(NodePrx node, const shared_ptr& session, exception_ptr ex) +NodeI::removeSubscriberSession(const NodePrx& node, const shared_ptr& session, exception_ptr ex) { if (session && !session->retry(node, ex)) { @@ -370,7 +369,7 @@ NodeI::removeSubscriberSession(NodePrx node, const shared_ptr& session, exception_ptr ex) +NodeI::removePublisherSession(const NodePrx& node, const shared_ptr& session, exception_ptr ex) { if (session && !session->retry(node, ex)) { @@ -389,10 +388,10 @@ NodeI::removePublisherSession(NodePrx node, const shared_ptr& } } -Ice::ConnectionPtr +ConnectionPtr NodeI::getSessionConnection(string_view id) const { - auto session = getSession(Ice::stringToIdentity(id)); + auto session = getSession(stringToIdentity(id)); if (session) { return session->getConnection(); @@ -404,7 +403,7 @@ NodeI::getSessionConnection(string_view id) const } shared_ptr -NodeI::getSession(const Ice::Identity& ident) const +NodeI::getSession(const Identity& ident) const { unique_lock lock(_mutex); if (ident.category == "s") @@ -427,7 +426,7 @@ NodeI::getSession(const Ice::Identity& ident) const } shared_ptr -NodeI::createSubscriberSessionServant(NodePrx node) +NodeI::createSubscriberSessionServant(const NodePrx& node) { // Called with mutex locked auto instance = _instance.lock(); @@ -447,13 +446,15 @@ NodeI::createSubscriberSessionServant(NodePrx node) instance, shared_from_this(), node, - instance->getObjectAdapter()->createProxy({to_string(id), "s"})->ice_oneway()); + instance->getObjectAdapter() + ->createProxy(Identity{.name = to_string(id), .category = "s"}) + ->ice_oneway()); session->init(); _subscribers.emplace(node->ice_getIdentity(), session); _subscriberSessions.emplace(session->getProxy()->ice_getIdentity(), session); return session; } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { return nullptr; } @@ -461,7 +462,7 @@ NodeI::createSubscriberSessionServant(NodePrx node) } shared_ptr -NodeI::createPublisherSessionServant(NodePrx node) +NodeI::createPublisherSessionServant(const NodePrx& node) { // Called with mutex locked auto instance = _instance.lock(); @@ -481,13 +482,15 @@ NodeI::createPublisherSessionServant(NodePrx node) instance, shared_from_this(), node, - instance->getObjectAdapter()->createProxy({to_string(id), "p"})->ice_oneway()); + instance->getObjectAdapter() + ->createProxy(Identity{.name = to_string(id), .category = "p"}) + ->ice_oneway()); session->init(); _publishers.emplace(node->ice_getIdentity(), session); _publisherSessions.emplace(session->getProxy()->ice_getIdentity(), session); return session; } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { return nullptr; } @@ -495,7 +498,7 @@ NodeI::createPublisherSessionServant(NodePrx node) } void -NodeI::forwardToSubscribers(const Ice::ByteSeq& inParams, const Ice::Current& current) const +NodeI::forwardToSubscribers(const ByteSeq& inParams, const Current& current) const { // Forward the invocation to all subscribers with an active session, don't need to wait for the result. lock_guard lock(_mutex); @@ -510,7 +513,7 @@ NodeI::forwardToSubscribers(const Ice::ByteSeq& inParams, const Ice::Current& cu } void -NodeI::forwardToPublishers(const Ice::ByteSeq& inParams, const Ice::Current& current) const +NodeI::forwardToPublishers(const ByteSeq& inParams, const Current& current) const { // Forward the invocation to all publishers with an active session, don't need to wait for the result. assert(current.id == _publisherForwarder->ice_getIdentity()); @@ -524,11 +527,14 @@ NodeI::forwardToPublishers(const Ice::ByteSeq& inParams, const Ice::Current& cur } NodePrx -NodeI::getNodeWithExistingConnection(const shared_ptr& instance, NodePrx node, const Ice::ConnectionPtr& con) +NodeI::getNodeWithExistingConnection( + const shared_ptr& instance, + const NodePrx& node, + const ConnectionPtr& newConnection) { - Ice::ConnectionPtr connection; + ConnectionPtr connection; - // If the node has a session with this node, use a bi-dir proxy associated with node session's connection. + // If the node has a session with this node, use a fixed proxy associated with node session's connection. if (auto nodeSession = instance->getNodeSessionManager()->getSession(node->ice_getIdentity())) { connection = nodeSession->getConnection(); @@ -565,7 +571,7 @@ NodeI::getNodeWithExistingConnection(const shared_ptr& instance, NodeP if (!connection && node->ice_getEndpoints().empty() && node->ice_getAdapterId().empty()) { - connection = con; + connection = newConnection; } if (connection) diff --git a/cpp/src/DataStorm/NodeI.h b/cpp/src/DataStorm/NodeI.h index d1fbed52eeb..83d4174f778 100644 --- a/cpp/src/DataStorm/NodeI.h +++ b/cpp/src/DataStorm/NodeI.h @@ -48,47 +48,55 @@ namespace DataStormI const std::shared_ptr&); void createPublisherSession( - DataStormContract::NodePrx, + const DataStormContract::NodePrx&, const Ice::ConnectionPtr&, std::shared_ptr); void removeSubscriberSession( - DataStormContract::NodePrx, + const DataStormContract::NodePrx&, const std::shared_ptr&, std::exception_ptr); void removePublisherSession( - DataStormContract::NodePrx, + const DataStormContract::NodePrx&, const std::shared_ptr&, std::exception_ptr); - Ice::ConnectionPtr getSessionConnection(std::string_view) const; + [[nodiscard]] Ice::ConnectionPtr getSessionConnection(std::string_view) const; - std::shared_ptr getSession(const Ice::Identity&) const; + [[nodiscard]] std::shared_ptr getSession(const Ice::Identity&) const; - DataStormContract::NodePrx getNodeWithExistingConnection( + [[nodiscard]] DataStormContract::NodePrx getNodeWithExistingConnection( const std::shared_ptr& instance, - DataStormContract::NodePrx node, + const DataStormContract::NodePrx& node, const Ice::ConnectionPtr& connection); - DataStormContract::NodePrx getProxy() const { return _proxy; } + [[nodiscard]] DataStormContract::NodePrx getProxy() const { return _proxy; } - DataStormContract::PublisherSessionPrx getPublisherForwarder() const { return _publisherForwarder; } + [[nodiscard]] DataStormContract::PublisherSessionPrx getPublisherForwarder() const + { + return _publisherForwarder; + } - DataStormContract::SubscriberSessionPrx getSubscriberForwarder() const { return _subscriberForwarder; } + [[nodiscard]] DataStormContract::SubscriberSessionPrx getSubscriberForwarder() const + { + return _subscriberForwarder; + } private: - std::shared_ptr createSubscriberSessionServant(DataStormContract::NodePrx); + [[nodiscard]] std::shared_ptr + createSubscriberSessionServant(const DataStormContract::NodePrx&); - std::shared_ptr createPublisherSessionServant(DataStormContract::NodePrx); + [[nodiscard]] std::shared_ptr + createPublisherSessionServant(const DataStormContract::NodePrx&); void forwardToSubscribers(const Ice::ByteSeq&, const Ice::Current&) const; void forwardToPublishers(const Ice::ByteSeq&, const Ice::Current&) const; std::weak_ptr _instance; mutable std::mutex _mutex; - std::int64_t _nextPublisherSessionId; - std::int64_t _nextSubscriberSessionId; + std::int64_t _nextPublisherSessionId{0}; + std::int64_t _nextSubscriberSessionId{0}; // The proxy for this node. DataStormContract::NodePrx _proxy; diff --git a/cpp/src/DataStorm/NodeSessionI.cpp b/cpp/src/DataStorm/NodeSessionI.cpp index 796cc4bcb31..38265dd08ff 100644 --- a/cpp/src/DataStorm/NodeSessionI.cpp +++ b/cpp/src/DataStorm/NodeSessionI.cpp @@ -11,6 +11,7 @@ using namespace std; using namespace DataStormI; using namespace DataStormContract; +using namespace Ice; namespace { @@ -26,17 +27,17 @@ namespace public: NodeForwarder( shared_ptr nodeSessionManager, - shared_ptr nodeSession, + const shared_ptr& nodeSession, NodePrx node) : _nodeSessionManager(std::move(nodeSessionManager)), - _nodeSession(std::move(nodeSession)), + _nodeSession(nodeSession), _node(std::move(node)) { } - void initiateCreateSession(optional publisher, const Ice::Current& current) final + void initiateCreateSession(optional publisher, const Current& current) final { - Ice::checkNotNull(publisher, __FILE__, __LINE__, current); + checkNotNull(publisher, __FILE__, __LINE__, current); if (auto nodeSession = _nodeSession.lock()) { try @@ -46,7 +47,7 @@ namespace // Forward the call to the target Node object, don't need to wait for the result. _node->initiateCreateSessionAsync(publisher, nullptr); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { } } @@ -56,10 +57,10 @@ namespace optional subscriber, optional subscriberSession, bool /* fromRelay */, - const Ice::Current& current) final + const Current& current) final { - Ice::checkNotNull(subscriber, __FILE__, __LINE__, current); - Ice::checkNotNull(subscriberSession, __FILE__, __LINE__, current); + checkNotNull(subscriber, __FILE__, __LINE__, current); + checkNotNull(subscriberSession, __FILE__, __LINE__, current); if (auto nodeSession = _nodeSession.lock()) { @@ -70,7 +71,7 @@ namespace // Forward the call to the target Node object, don't need to wait for the result. _node->createSessionAsync(subscriber, subscriberSession, true, nullptr); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { } } @@ -79,10 +80,10 @@ namespace void confirmCreateSession( optional publisher, optional publisherSession, - const Ice::Current& current) final + const Current& current) final { - Ice::checkNotNull(publisher, __FILE__, __LINE__, current); - Ice::checkNotNull(publisherSession, __FILE__, __LINE__, current); + checkNotNull(publisher, __FILE__, __LINE__, current); + checkNotNull(publisherSession, __FILE__, __LINE__, current); if (auto nodeSession = _nodeSession.lock()) { @@ -93,7 +94,7 @@ namespace // Forward the call to the target Node object, don't need to wait for the result. _node->confirmCreateSessionAsync(publisher, publisherSession, nullptr); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { } } @@ -102,8 +103,7 @@ namespace private: // This helper method is used to replace the Node and Session proxies with forwarders when the calling Node // doesn't have a public endpoint. - template - void updateNodeAndSessionProxy(NodePrx& node, optional& session, const Ice::Current& current) + template void updateNodeAndSessionProxy(NodePrx& node, optional& session, const Current& current) { if (node->ice_getEndpoints().empty() && node->ice_getAdapterId().empty()) { @@ -125,7 +125,7 @@ namespace NodeSessionI::NodeSessionI( shared_ptr instance, NodePrx node, - Ice::ConnectionPtr connection, + ConnectionPtr connection, bool forwardAnnouncements) : _instance(std::move(instance)), _node(std::move(node)), @@ -133,7 +133,7 @@ NodeSessionI::NodeSessionI( { if (forwardAnnouncements) { - _lookup = _connection->createProxy({"Lookup", "DataStorm"}); + _lookup = _connection->createProxy(Identity{.name = "Lookup", .category = "DataStorm"}); } } @@ -158,7 +158,7 @@ NodeSessionI::init() if (_instance->getTraceLevels()->session > 0) { - Trace out(_instance->getTraceLevels(), _instance->getTraceLevels()->sessionCat); + Trace out(_instance->getTraceLevels()->logger, _instance->getTraceLevels()->sessionCat); out << "created node session (peer = `" << _publicNode << "'):\n" << _connection->toString(); } } @@ -181,16 +181,16 @@ NodeSessionI::destroy() session->disconnectedAsync(nullptr); } } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { } if (_instance->getTraceLevels()->session > 0) { - Trace out(_instance->getTraceLevels(), _instance->getTraceLevels()->sessionCat); + Trace out(_instance->getTraceLevels()->logger, _instance->getTraceLevels()->sessionCat); out << "destroyed node session (peer = `" << _publicNode << "')"; } } @@ -199,5 +199,6 @@ void NodeSessionI::addSession(SessionPrx session) { lock_guard lock(_mutex); - _sessions.insert_or_assign(session->ice_getIdentity(), session); + Identity id = session->ice_getIdentity(); + _sessions.insert_or_assign(std::move(id), std::move(session)); } diff --git a/cpp/src/DataStorm/NodeSessionI.h b/cpp/src/DataStorm/NodeSessionI.h index a6470c22980..f608a07ecde 100644 --- a/cpp/src/DataStorm/NodeSessionI.h +++ b/cpp/src/DataStorm/NodeSessionI.h @@ -20,22 +20,22 @@ namespace DataStormI void destroy(); void addSession(DataStormContract::SessionPrx); - DataStormContract::NodePrx getPublicNode() const + [[nodiscard]] DataStormContract::NodePrx getPublicNode() const { // always set after init assert(_publicNode); return *_publicNode; } - std::optional getLookup() const { return _lookup; } - const Ice::ConnectionPtr& getConnection() const { return _connection; } + [[nodiscard]] std::optional getLookup() const { return _lookup; } + [[nodiscard]] const Ice::ConnectionPtr& getConnection() const { return _connection; } // Helper method to create a forwarder proxy for a subscriber or publisher session proxy. - template Prx forwarder(Prx session) const + template [[nodiscard]] Prx forwarder(const Prx& session) const { auto id = session->ice_getIdentity(); auto proxy = _instance->getObjectAdapter()->createProxy( - {id.name + '-' + _node->ice_getIdentity().name, id.category + 'f'}); + Ice::Identity{.name = id.name + '-' + _node->ice_getIdentity().name, .category = id.category + 'f'}); return proxy->ice_oneway(); } diff --git a/cpp/src/DataStorm/NodeSessionManager.cpp b/cpp/src/DataStorm/NodeSessionManager.cpp index 42529d1a56c..7acb2b166cf 100644 --- a/cpp/src/DataStorm/NodeSessionManager.cpp +++ b/cpp/src/DataStorm/NodeSessionManager.cpp @@ -13,10 +13,11 @@ using namespace std; using namespace DataStormI; using namespace DataStormContract; +using namespace Ice; namespace { - class SessionForwarder : public Ice::Blobject + class SessionForwarder : public Blobject { public: SessionForwarder(shared_ptr nodeSessionManager) @@ -24,27 +25,30 @@ namespace { } - bool ice_invoke(Ice::ByteSeq inParams, Ice::ByteSeq&, const Ice::Current& current) final + bool ice_invoke(ByteSeq inParams, ByteSeq&, const Current& current) final { auto pos = current.id.name.find('-'); if (pos != string::npos && pos < current.id.name.length()) { - if (auto session = _nodeSessionManager->getSession(Ice::Identity{current.id.name.substr(pos + 1), ""})) + if (auto session = _nodeSessionManager->getSession( + Identity{.name = current.id.name.substr(pos + 1), .category = ""})) { // Forward the call to the target session, don't need to wait for the result. - Ice::Identity id{current.id.name.substr(0, pos), current.id.category.substr(0, 1)}; - session->getConnection()->createProxy(id)->ice_invokeAsync( - current.operation, - current.mode, - inParams, - nullptr, - nullptr, - nullptr, - current.ctx); + Identity id{.name = current.id.name.substr(0, pos), .category = current.id.category.substr(0, 1)}; + session->getConnection() + ->createProxy(std::move(id)) + ->ice_invokeAsync( + current.operation, + current.mode, + inParams, + nullptr, + nullptr, + nullptr, + current.ctx); return true; } } - throw Ice::ObjectNotExistException{__FILE__, __LINE__}; + throw ObjectNotExistException{__FILE__, __LINE__}; } private: @@ -59,9 +63,8 @@ NodeSessionManager::NodeSessionManager(const shared_ptr& instance, con _forwardToMulticast( instance->getCommunicator()->getProperties()->getIcePropertyAsInt( "DataStorm.Node.Server.ForwardDiscoveryToMulticast") > 0), - _retryCount(0), _forwarder(instance->getCollocatedForwarder()->add( - [this](Ice::ByteSeq inParams, const Ice::Current& current) { forward(inParams, current); })) + [this](const ByteSeq& inParams, const Current& current) { forward(inParams, current); })) { } @@ -83,7 +86,7 @@ NodeSessionManager::init() } shared_ptr -NodeSessionManager::createOrGet(NodePrx node, const Ice::ConnectionPtr& connection, bool forwardAnnouncements) +NodeSessionManager::createOrGet(NodePrx node, const ConnectionPtr& connection, bool forwardAnnouncements) { unique_lock lock(_mutex); @@ -117,14 +120,14 @@ NodeSessionManager::createOrGet(NodePrx node, const Ice::ConnectionPtr& connecti instance->getConnectionManager()->add( connection, make_shared(node), - [self = shared_from_this(), node](const Ice::ConnectionPtr&, std::exception_ptr) - { self->destroySession(std::move(node)); }); + [self = shared_from_this(), node = std::move(node)](const ConnectionPtr&, exception_ptr) mutable + { self->destroySession(node); }); return session; } void -NodeSessionManager::announceTopicReader(const string& topic, NodePrx node, const Ice::ConnectionPtr& connection) const +NodeSessionManager::announceTopicReader(const string& topic, NodePrx node, const ConnectionPtr& connection) const { unique_lock lock(_mutex); if (connection && node->ice_getIdentity() == _nodePrx->ice_getIdentity()) @@ -134,7 +137,7 @@ NodeSessionManager::announceTopicReader(const string& topic, NodePrx node, const if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); if (connection) { out << "topic reader `" << topic << "' announced (peer = `" << node << "')"; @@ -171,7 +174,7 @@ NodeSessionManager::announceTopicReader(const string& topic, NodePrx node, const } void -NodeSessionManager::announceTopicWriter(const string& topic, NodePrx node, const Ice::ConnectionPtr& connection) const +NodeSessionManager::announceTopicWriter(const string& topic, NodePrx node, const ConnectionPtr& connection) const { unique_lock lock(_mutex); if (connection && node->ice_getIdentity() == _nodePrx->ice_getIdentity()) @@ -181,7 +184,7 @@ NodeSessionManager::announceTopicWriter(const string& topic, NodePrx node, const if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); if (connection) { out << "topic writer `" << topic << "' announced (peer = `" << node << "')"; @@ -219,10 +222,10 @@ NodeSessionManager::announceTopicWriter(const string& topic, NodePrx node, const void NodeSessionManager::announceTopics( - const Ice::StringSeq& readers, - const Ice::StringSeq& writers, + const StringSeq& readers, + const StringSeq& writers, NodePrx node, - const Ice::ConnectionPtr& connection) const + const ConnectionPtr& connection) const { unique_lock lock(_mutex); if (connection && node->ice_getIdentity() == _nodePrx->ice_getIdentity()) @@ -232,7 +235,7 @@ NodeSessionManager::announceTopics( if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); if (connection) { if (!readers.empty()) @@ -283,7 +286,7 @@ NodeSessionManager::announceTopics( } shared_ptr -NodeSessionManager::getSession(const Ice::Identity& node) const +NodeSessionManager::getSession(const Identity& node) const { unique_lock lock(_mutex); auto p = _sessions.find(node); @@ -291,7 +294,7 @@ NodeSessionManager::getSession(const Ice::Identity& node) const } void -NodeSessionManager::forward(const Ice::ByteSeq& inParams, const Ice::Current& current) const +NodeSessionManager::forward(const ByteSeq& inParams, const Current& current) const { // Called while holding the mutex lock to ensure `_exclude` is not updated concurrently. @@ -323,7 +326,7 @@ NodeSessionManager::forward(const Ice::ByteSeq& inParams, const Ice::Current& cu } void -NodeSessionManager::connect(LookupPrx lookup, NodePrx proxy) +NodeSessionManager::connect(const LookupPrx& lookup, const NodePrx& proxy) { auto instance = _instance.lock(); if (!instance) @@ -336,7 +339,7 @@ NodeSessionManager::connect(LookupPrx lookup, NodePrx proxy) { lookup->createSessionAsync( proxy, - [=, self = shared_from_this()](optional node) + [self = shared_from_this(), lookup](optional node) { // createSession must return a non null proxy. assert(node); @@ -349,20 +352,20 @@ NodeSessionManager::connect(LookupPrx lookup, NodePrx proxy) self->disconnected(lookup); } }, - [=, self = shared_from_this()](std::exception_ptr) { self->disconnected(lookup); }); + [self = shared_from_this(), lookup](exception_ptr) { self->disconnected(lookup); }); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { // Ignore node is being shutdown. } - catch (const std::exception&) + catch (const exception&) { disconnected(lookup); } } void -NodeSessionManager::connected(NodePrx node, LookupPrx lookup) +NodeSessionManager::connected(const NodePrx& node, const LookupPrx& lookup) { unique_lock lock(_mutex); auto instance = _instance.lock(); @@ -381,21 +384,17 @@ NodeSessionManager::connected(NodePrx node, LookupPrx lookup) if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); out << "established node session (peer = `" << node << "'):\n" << connection->toString(); } instance->getConnectionManager()->add( connection, make_shared(lookup), - [=, self = shared_from_this()](const Ice::ConnectionPtr&, std::exception_ptr) + [self = shared_from_this(), node, lookup](const ConnectionPtr&, exception_ptr) { self->disconnected(node, lookup); }); - if (p != _sessions.end()) - { - lookup = lookup->ice_fixed(connection); - } - _connectedTo = lookup; + _connectedTo = p == _sessions.end() ? lookup : lookup->ice_fixed(connection); auto readerNames = instance->getTopicFactory()->getTopicReaderNames(); auto writerNames = instance->getTopicFactory()->getTopicWriterNames(); @@ -403,9 +402,9 @@ NodeSessionManager::connected(NodePrx node, LookupPrx lookup) { try { - lookup->announceTopicsAsync(readerNames, writerNames, _nodePrx, nullptr); + _connectedTo->announceTopicsAsync(readerNames, writerNames, _nodePrx, nullptr); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { // Ignore node is being shutdown. } @@ -413,7 +412,7 @@ NodeSessionManager::connected(NodePrx node, LookupPrx lookup) } void -NodeSessionManager::disconnected(NodePrx node, LookupPrx lookup) +NodeSessionManager::disconnected(const NodePrx& node, const LookupPrx& lookup) { unique_lock lock(_mutex); auto instance = _instance.lock(); @@ -422,7 +421,7 @@ NodeSessionManager::disconnected(NodePrx node, LookupPrx lookup) _retryCount = 0; if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); out << "disconnected node session (peer = `" << node << "')"; } _connectedTo = nullopt; @@ -432,20 +431,20 @@ NodeSessionManager::disconnected(NodePrx node, LookupPrx lookup) } void -NodeSessionManager::disconnected(LookupPrx lookup) +NodeSessionManager::disconnected(const LookupPrx& lookup) { unique_lock lock(_mutex); auto instance = _instance.lock(); if (instance) { instance->scheduleTimerTask( - [=, self = shared_from_this()] { self->connect(lookup, self->_nodePrx); }, + [self = shared_from_this(), lookup] { self->connect(lookup, self->_nodePrx); }, instance->getRetryDelay(_retryCount++)); } } void -NodeSessionManager::destroySession(NodePrx node) +NodeSessionManager::destroySession(const NodePrx& node) { unique_lock lock(_mutex); auto p = _sessions.find(node->ice_getIdentity()); diff --git a/cpp/src/DataStorm/NodeSessionManager.h b/cpp/src/DataStorm/NodeSessionManager.h index 18a6824496a..8f49a7b9136 100644 --- a/cpp/src/DataStorm/NodeSessionManager.h +++ b/cpp/src/DataStorm/NodeSessionManager.h @@ -37,19 +37,19 @@ namespace DataStormI DataStormContract::NodePrx, const Ice::ConnectionPtr& = nullptr) const; - std::shared_ptr getSession(const Ice::Identity&) const; + [[nodiscard]] std::shared_ptr getSession(const Ice::Identity&) const; void forward(const Ice::ByteSeq&, const Ice::Current&) const; private: - void connect(DataStormContract::LookupPrx, DataStormContract::NodePrx); + void connect(const DataStormContract::LookupPrx&, const DataStormContract::NodePrx&); - void connected(DataStormContract::NodePrx, DataStormContract::LookupPrx); + void connected(const DataStormContract::NodePrx&, const DataStormContract::LookupPrx&); - void disconnected(DataStormContract::NodePrx, DataStormContract::LookupPrx); - void disconnected(DataStormContract::LookupPrx); + void disconnected(const DataStormContract::NodePrx&, const DataStormContract::LookupPrx&); + void disconnected(const DataStormContract::LookupPrx&); - void destroySession(DataStormContract::NodePrx); + void destroySession(const DataStormContract::NodePrx&); std::weak_ptr _instance; const std::shared_ptr _traceLevels; @@ -58,7 +58,7 @@ namespace DataStormI mutable std::mutex _mutex; - int _retryCount; + int _retryCount{0}; // A map containing the `NodeSessionI` servants for all nodes that have an active session with this node. // The map is indexed by the identity of the nodes. diff --git a/cpp/src/DataStorm/SessionI.cpp b/cpp/src/DataStorm/SessionI.cpp index e6ca9ee1c0c..7091a24ec58 100644 --- a/cpp/src/DataStorm/SessionI.cpp +++ b/cpp/src/DataStorm/SessionI.cpp @@ -14,19 +14,20 @@ using namespace std; using namespace DataStormI; using namespace DataStormContract; +using namespace Ice; namespace { - class DispatchInterceptorI : public Ice::Object + class DispatchInterceptorI : public Object { public: - DispatchInterceptorI(Ice::ObjectPtr servant, shared_ptr executor) + DispatchInterceptorI(ObjectPtr servant, shared_ptr executor) : _servant(std::move(servant)), _executor(std::move(executor)) { } - void dispatch(Ice::IncomingRequest& request, std::function sendResponse) final + void dispatch(IncomingRequest& request, std::function sendResponse) final { _servant->dispatch(request, sendResponse); // Flush the callback executor to ensure that any callbacks queued by the dispatch are executed. @@ -34,7 +35,7 @@ namespace } private: - Ice::ObjectPtr _servant; + ObjectPtr _servant; shared_ptr _executor; }; } @@ -44,11 +45,8 @@ SessionI::SessionI(shared_ptr instance, shared_ptr parent, Node _traceLevels{_instance->getTraceLevels()}, _parent{std::move(parent)}, _proxy{std::move(proxy)}, - _id{Ice::identityToString(_proxy->ice_getIdentity())}, - _node{std::move(node)}, - _destroyed{false}, - _sessionInstanceId{0}, - _retryCount{0} + _id{identityToString(_proxy->ice_getIdentity())}, + _node{std::move(node)} { } @@ -63,13 +61,13 @@ SessionI::init() if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": created session (peer = `" << _node << "')"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": created session (peer = '" << _node << "')"; } } void -SessionI::announceTopics(TopicInfoSeq topics, bool, const Ice::Current&) +SessionI::announceTopics(TopicInfoSeq topics, bool, const Current&) { // Retain topics outside the synchronization. This is necessary to ensure the topic destructor doesn't get called // within the synchronization. The topic destructor can callback on the session to disconnect. @@ -83,8 +81,8 @@ SessionI::announceTopics(TopicInfoSeq topics, bool, const Ice::Current&) if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": announcing topics `" << topics << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": announcing topics '" << topics << "'"; } for (const auto& info : topics) @@ -124,7 +122,7 @@ SessionI::announceTopics(TopicInfoSeq topics, bool, const Ice::Current&) } void -SessionI::attachTopic(TopicSpec spec, const Ice::Current&) +SessionI::attachTopic(TopicSpec spec, const Current&) { // Retain topics outside the synchronization. This is necessary to ensure the topic destructor doesn't get called // within the synchronization. The topic destructor can callback on the session to disconnect. @@ -144,8 +142,8 @@ SessionI::attachTopic(TopicSpec spec, const Ice::Current&) { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": attaching topic `" << spec << "' to `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": attaching topic '" << spec << "' to '" << topic << "'"; } // Attach local topics with the given name to the remote topic. @@ -175,8 +173,8 @@ SessionI::attachTopic(TopicSpec spec, const Ice::Current&) { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": matched elements `" << spec << "' on `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": matched elements '" << spec << "' on '" << topic << "'"; } // Don't wait for the response here, the remote session will send an ack. _session->attachElementsAsync(topic->getId(), specs, true, nullptr); @@ -186,7 +184,7 @@ SessionI::attachTopic(TopicSpec spec, const Ice::Current&) } void -SessionI::detachTopic(int64_t id, const Ice::Current&) +SessionI::detachTopic(int64_t id, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -200,8 +198,8 @@ SessionI::detachTopic(int64_t id, const Ice::Current&) { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": detaching topic `" << id << "' from `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": detaching topic '" << id << "' from '" << topic << "'"; } topic->detach(id, shared_from_this()); }); @@ -211,7 +209,7 @@ SessionI::detachTopic(int64_t id, const Ice::Current&) } void -SessionI::attachTags(int64_t topicId, ElementInfoSeq tags, bool initialize, const Ice::Current&) +SessionI::attachTags(int64_t topicId, ElementInfoSeq tags, bool initialize, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -225,8 +223,8 @@ SessionI::attachTags(int64_t topicId, ElementInfoSeq tags, bool initialize, cons { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": attaching tags `[" << tags << "]@" << topicId << "' on topic `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": attaching tags '[" << tags << "]@" << topicId << "' on topic '" << topic << "'"; } if (initialize) @@ -242,7 +240,7 @@ SessionI::attachTags(int64_t topicId, ElementInfoSeq tags, bool initialize, cons } void -SessionI::detachTags(int64_t topicId, Ice::LongSeq tags, const Ice::Current&) +SessionI::detachTags(int64_t topicId, LongSeq tags, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -256,8 +254,8 @@ SessionI::detachTags(int64_t topicId, Ice::LongSeq tags, const Ice::Current&) { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": detaching tags `[" << tags << "]@" << topicId << "' on topic `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": detaching tags '[" << tags << "]@" << topicId << "' on topic '" << topic << "'"; } for (auto tag : tags) @@ -268,7 +266,7 @@ SessionI::detachTags(int64_t topicId, Ice::LongSeq tags, const Ice::Current&) } void -SessionI::announceElements(int64_t topicId, ElementInfoSeq elements, const Ice::Current&) +SessionI::announceElements(int64_t topicId, ElementInfoSeq elements, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -282,8 +280,8 @@ SessionI::announceElements(int64_t topicId, ElementInfoSeq elements, const Ice:: { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": announcing elements `[" << elements << "]@" << topicId << "' on topic `" << topic + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": announcing elements '[" << elements << "]@" << topicId << "' on topic '" << topic << "'"; } @@ -292,8 +290,8 @@ SessionI::announceElements(int64_t topicId, ElementInfoSeq elements, const Ice:: { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": announcing elements matched `[" << specs << "]@" << topicId << "' on topic `" + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": announcing elements matched '[" << specs << "]@" << topicId << "' on topic '" << topic << "'"; } _session->attachElementsAsync(topic->getId(), specs, false, nullptr); @@ -302,7 +300,7 @@ SessionI::announceElements(int64_t topicId, ElementInfoSeq elements, const Ice:: } void -SessionI::attachElements(int64_t topicId, ElementSpecSeq elements, bool initialize, const Ice::Current&) +SessionI::attachElements(int64_t topicId, ElementSpecSeq elements, bool initialize, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -320,8 +318,8 @@ SessionI::attachElements(int64_t topicId, ElementSpecSeq elements, bool initiali { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": attaching elements `[" << elements << "]@" << topicId << "' on topic `" << topic + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": attaching elements '[" << elements << "]@" << topicId << "' on topic '" << topic << "'"; if (initialize) { @@ -343,8 +341,8 @@ SessionI::attachElements(int64_t topicId, ElementSpecSeq elements, bool initiali { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": attaching elements matched `[" << specAck << "]@" << topicId << "' on topic `" + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": attaching elements matched '[" << specAck << "]@" << topicId << "' on topic '" << topic << "'"; } _session->attachElementsAckAsync(topic->getId(), specAck, nullptr); @@ -353,7 +351,7 @@ SessionI::attachElements(int64_t topicId, ElementSpecSeq elements, bool initiali } void -SessionI::attachElementsAck(int64_t topicId, ElementSpecAckSeq elements, const Ice::Current&) +SessionI::attachElementsAck(int64_t topicId, ElementSpecAckSeq elements, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -367,19 +365,19 @@ SessionI::attachElementsAck(int64_t topicId, ElementSpecAckSeq elements, const I { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": attaching elements ack `[" << elements << "]@" << topicId << "' on topic `" << topic + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": attaching elements ack '[" << elements << "]@" << topicId << "' on topic '" << topic << "'"; } - Ice::LongSeq removedIds; + LongSeq removedIds; auto samples = topic->attachElementsAck(topicId, elements, shared_from_this(), *_session, now, removedIds); if (!samples.empty()) { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": initializing elements `[" << samples << "]@" << topicId << "' on topic `" << topic + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": initializing elements '[" << samples << "]@" << topicId << "' on topic '" << topic << "'"; } _session->initSamplesAsync(topic->getId(), samples, nullptr); @@ -393,7 +391,7 @@ SessionI::attachElementsAck(int64_t topicId, ElementSpecAckSeq elements, const I } void -SessionI::detachElements(int64_t id, Ice::LongSeq elements, const Ice::Current&) +SessionI::detachElements(int64_t id, LongSeq elements, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -407,8 +405,8 @@ SessionI::detachElements(int64_t id, Ice::LongSeq elements, const Ice::Current&) { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": detaching elements `[" << elements << "]@" << id << "' on topic `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": detaching elements '[" << elements << "]@" << id << "' on topic '" << topic << "'"; } for (const auto& element : elements) @@ -434,7 +432,7 @@ SessionI::detachElements(int64_t id, Ice::LongSeq elements, const Ice::Current&) } void -SessionI::initSamples(int64_t topicId, DataSamplesSeq samplesSeq, const Ice::Current&) +SessionI::initSamples(int64_t topicId, DataSamplesSeq samplesSeq, const Current&) { lock_guard lock(_mutex); if (!_session) @@ -455,8 +453,8 @@ SessionI::initSamples(int64_t topicId, DataSamplesSeq samplesSeq, const Ice::Cur { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": initializing samples from `" << samples.id << "'" + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": initializing samples from '" << samples.id << "'" << " on ["; for (auto q = elementSubscribers->getSubscribers().begin(); q != elementSubscribers->getSubscribers().end(); @@ -477,7 +475,7 @@ SessionI::initSamples(int64_t topicId, DataSamplesSeq samplesSeq, const Ice::Cur vector> samplesI; samplesI.reserve(samples.samples.size()); - auto sampleFactory = topic->getSampleFactory(); + const auto& sampleFactory = topic->getSampleFactory(); for (const auto& sample : samples.samples) { shared_ptr key; @@ -526,7 +524,7 @@ SessionI::initSamples(int64_t topicId, DataSamplesSeq samplesSeq, const Ice::Cur } void -SessionI::disconnected(const Ice::Current& current) +SessionI::disconnected(const Current& current) { if (disconnected(current.con, nullptr)) { @@ -538,7 +536,7 @@ SessionI::disconnected(const Ice::Current& current) } void -SessionI::connected(SessionPrx session, const Ice::ConnectionPtr& newConnection, const TopicInfoSeq& topics) +SessionI::connected(SessionPrx session, const ConnectionPtr& newConnection, const TopicInfoSeq& topics) { lock_guard lock(_mutex); if (_destroyed || _session) @@ -547,7 +545,7 @@ SessionI::connected(SessionPrx session, const Ice::ConnectionPtr& newConnection, return; } - _session = session; + _session = std::move(session); _connection = newConnection; if (newConnection) { @@ -557,7 +555,7 @@ SessionI::connected(SessionPrx session, const Ice::ConnectionPtr& newConnection, _instance->getConnectionManager()->add( newConnection, self, - [self](auto connection, auto ex) + [self](const auto& connection, auto ex) { if (self->disconnected(connection, ex)) { @@ -579,8 +577,8 @@ SessionI::connected(SessionPrx session, const Ice::ConnectionPtr& newConnection, if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": session `" << _session->ice_getIdentity() << "' connected"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": session '" << _session->ice_getIdentity() << "' connected"; if (_connection) { out << "\n" << _connection->toString(); @@ -594,7 +592,7 @@ SessionI::connected(SessionPrx session, const Ice::ConnectionPtr& newConnection, // Announce the topics to the peer, don't wait for the result. _session->announceTopicsAsync(topics, true, nullptr); } - catch (const Ice::LocalException&) + catch (const LocalException&) { // Ignore } @@ -602,7 +600,7 @@ SessionI::connected(SessionPrx session, const Ice::ConnectionPtr& newConnection, } bool -SessionI::disconnected(const Ice::ConnectionPtr& connection, exception_ptr ex) +SessionI::disconnected(const ConnectionPtr& connection, exception_ptr ex) { lock_guard lock(_mutex); if (_destroyed) @@ -631,12 +629,12 @@ SessionI::disconnected(const Ice::ConnectionPtr& connection, exception_ptr ex) } else { - throw Ice::CloseConnectionException{__FILE__, __LINE__}; + throw CloseConnectionException{__FILE__, __LINE__}; } } catch (const std::exception& e) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); out << _id << ": session '" << _session->ice_getIdentity() << "' disconnected:\n"; out << (_connection ? _connection->toString() : "") << "\n"; out << e.what(); @@ -645,9 +643,9 @@ SessionI::disconnected(const Ice::ConnectionPtr& connection, exception_ptr ex) // Detach all topics from the session. auto self = shared_from_this(); - for (const auto& t : _topics) + for (const auto& [topicId, _] : _topics) { - runWithTopics(t.first, [id = t.first, self](TopicI* topic, TopicSubscriber&) { topic->detach(id, self); }); + runWithTopics(topicId, [topicId, self](TopicI* topic, TopicSubscriber&) { topic->detach(topicId, self); }); } _session = nullopt; @@ -668,11 +666,11 @@ SessionI::retry(NodePrx node, exception_ptr exception) { rethrow_exception(exception); } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { return false; } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { return false; } @@ -696,8 +694,8 @@ SessionI::retry(NodePrx node, exception_ptr exception) if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": can't retry connecting to `" << node->ice_toString() << "`, waiting " << delay.count() + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": can't retry connecting to '" << node->ice_toString() << "', waiting " << delay.count() << " (ms) for peer to reconnect"; } @@ -714,16 +712,16 @@ SessionI::retry(NodePrx node, exception_ptr exception) if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); if (_retryCount <= _instance->getRetryCount()) { - out << _id << ": retrying connecting to `" << node->ice_toString() << "` in " << delay.count(); + out << _id << ": retrying connecting to '" << node->ice_toString() << "' in " << delay.count(); out << " (ms), retry " << _retryCount << '/' << _instance->getRetryCount(); } else { - out << _id << ": connection to `" << node->ice_toString() - << "` failed and the retry limit has been reached"; + out << _id << ": connection to '" << node->ice_toString() + << "' failed and the retry limit has been reached"; } if (exception) @@ -744,8 +742,8 @@ SessionI::retry(NodePrx node, exception_ptr exception) return false; } - _retryTask = - make_shared([node, self = shared_from_this()] { self->reconnect(node); }); + _retryTask = make_shared([node = std::move(node), self = shared_from_this()] + { self->reconnect(node); }); _instance->scheduleTimerTask(_retryTask, delay); } return true; @@ -760,7 +758,7 @@ SessionI::destroyImpl(const exception_ptr& ex) if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); + Trace out(_traceLevels->logger, _traceLevels->sessionCat); out << _id << ": destroyed session"; if (ex) { @@ -768,7 +766,7 @@ SessionI::destroyImpl(const exception_ptr& ex) { rethrow_exception(ex); } - catch (const Ice::Exception& e) + catch (const Exception& e) { out << "\n:" << e.what() << "\n" << e.ice_stackTrace(); } @@ -805,12 +803,12 @@ SessionI::destroyImpl(const exception_ptr& ex) { _instance->getObjectAdapter()->remove(_proxy->ice_getIdentity()); } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { } } -Ice::ConnectionPtr +ConnectionPtr SessionI::getConnection() const { lock_guard lock(_mutex); @@ -835,7 +833,7 @@ SessionI::checkSession() { _connection->throwException(); } - catch (const Ice::LocalException&) + catch (const LocalException&) { auto connection = _connection; lock.unlock(); @@ -873,26 +871,26 @@ void SessionI::setNode(NodePrx node) { lock_guard lock(_mutex); - _node = node; + _node = std::move(node); } void -SessionI::subscribe(int64_t id, TopicI* topic) +SessionI::subscribe(int64_t topicId, TopicI* topic) { if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": subscribed topic `" << id << "' to topic `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": subscribed topic '" << topicId << "' to topic '" << topic << "'"; } - // Add the topic as a subscriber of the remote topic with the given id. - _topics[id].addSubscriber(topic, _sessionInstanceId); + // Add the topic as a subscriber of the remote topic with the given ID. + _topics[topicId].addSubscriber(topic, _sessionInstanceId); } void -SessionI::unsubscribe(int64_t id, TopicI* topic) +SessionI::unsubscribe(int64_t topicId, TopicI* topic) { - assert(_topics.find(id) != _topics.end()); - auto& subscriber = _topics.at(id).getSubscriber(topic); + assert(_topics.find(topicId) != _topics.end()); + auto& subscriber = _topics.at(topicId).getSubscriber(topic); for (auto& [elementId, elementSubscribers] : subscriber.getAll()) { for (auto& [element, elementSubscriber] : elementSubscribers.getSubscribers()) @@ -901,11 +899,11 @@ SessionI::unsubscribe(int64_t id, TopicI* topic) { if (elementId > 0) { - element->detachKey(id, elementId, key, shared_from_this(), elementSubscriber.facet, false); + element->detachKey(topicId, elementId, key, shared_from_this(), elementSubscriber.facet, false); } else { - element->detachFilter(id, -elementId, key, shared_from_this(), elementSubscriber.facet, false); + element->detachFilter(topicId, -elementId, key, shared_from_this(), elementSubscriber.facet, false); } } } @@ -913,13 +911,13 @@ SessionI::unsubscribe(int64_t id, TopicI* topic) if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": unsubscribed topic `" << id << "' from topic `" << topic << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": unsubscribed topic '" << topicId << "' from topic '" << topic << "'"; } } void -SessionI::disconnect(int64_t id, TopicI* topic) +SessionI::disconnect(int64_t topicId, TopicI* topic) { lock_guard lock(_mutex); // Called by TopicI::destroy if (!_session) @@ -927,18 +925,18 @@ SessionI::disconnect(int64_t id, TopicI* topic) return; } - if (_topics.find(id) == _topics.end()) + if (_topics.find(topicId) == _topics.end()) { return; // Peer topic detached first. } - runWithTopic(id, topic, [&](TopicSubscriber&) { unsubscribe(id, topic); }); + runWithTopic(topicId, topic, [&](TopicSubscriber&) { unsubscribe(topicId, topic); }); - auto& subscriber = _topics.at(id); + auto& subscriber = _topics.at(topicId); subscriber.removeSubscriber(topic); if (subscriber.getSubscribers().empty()) { - _topics.erase(id); + _topics.erase(topicId); } } @@ -958,8 +956,8 @@ SessionI::subscribeToKey( TopicSubscriber& subscriber = _topics.at(topicId).getSubscriber(element->getTopic()); if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": subscribed `e" << elementId << ":[k" << keyId << "]@" << topicId << "' to `" << element << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": subscribed 'e" << elementId << ":[k" << keyId << "]@" << topicId << "' to '" << element << "'"; if (!facet.empty()) { out << " (facet=" << facet << ')'; @@ -984,30 +982,30 @@ SessionI::unsubscribeFromKey( int64_t keyId) { assert(_topics.find(topicId) != _topics.end()); - auto& subscriber = _topics.at(topicId).getSubscriber(element->getTopic()); - auto k = subscriber.get(elementId); - if (k) + TopicSubscriber& topicSubscriber = _topics.at(topicId).getSubscriber(element->getTopic()); + ElementSubscribers* elementSubscribers = topicSubscriber.get(elementId); + if (elementSubscribers) { if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": unsubscribed `e" << elementId << "[k" << keyId << "]@" << topicId << "' from `" << element + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": unsubscribed 'e" << elementId << "[k" << keyId << "]@" << topicId << "' from '" << element << "'"; } - k->removeSubscriber(element); - if (k->getSubscribers().empty()) + elementSubscribers->removeSubscriber(element); + if (elementSubscribers->getSubscribers().empty()) { - subscriber.remove(elementId); + topicSubscriber.remove(elementId); } } - auto& p = subscriber.keys[keyId]; - if (--p.second[elementId] == 0) + auto& elementSubscribersMap = topicSubscriber.keys[keyId].second; + if (--elementSubscribersMap[elementId] == 0) { - p.second.erase(elementId); - if (p.second.empty()) + elementSubscribersMap.erase(elementId); + if (elementSubscribersMap.empty()) { - subscriber.keys.erase(keyId); + topicSubscriber.keys.erase(keyId); } } } @@ -1045,8 +1043,8 @@ SessionI::subscribeToFilter( auto& subscriber = _topics.at(topicId).getSubscriber(element->getTopic()); if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": subscribed `e" << elementId << '@' << topicId << "' to `" << element << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": subscribed 'e" << elementId << '@' << topicId << "' to '" << element << "'"; if (!facet.empty()) { out << " (facet=" << facet << ')'; @@ -1069,8 +1067,8 @@ SessionI::unsubscribeFromFilter( { if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": unsubscribed `e" << elementId << '@' << topicId << "' from `" << element << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": unsubscribed 'e" << elementId << '@' << topicId << "' from '" << element << "'"; } f->removeSubscriber(element); if (f->getSubscribers().empty()) @@ -1132,8 +1130,8 @@ SessionI::subscriberInitialized( if (_traceLevels->session > 1) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": initialized `" << element << "' from `e" << elementId << '@' << topicId << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": initialized '" << element << "' from 'e" << elementId << '@' << topicId << "'"; } elementSubscriber->initialized = true; elementSubscriber->lastId = samples.empty() ? 0 : samples.back().id; @@ -1164,10 +1162,10 @@ void SessionI::runWithTopics( const std::string& name, vector>& retained, - function&)> fn) + function&)> callback) { auto topics = getTopics(name); - for (auto topic : topics) + for (const auto& topic : topics) { retained.push_back(topic); unique_lock lock(topic->getMutex()); @@ -1176,15 +1174,15 @@ SessionI::runWithTopics( continue; } _topicLock = &lock; - fn(topic); + callback(topic); _topicLock = nullptr; } } void -SessionI::runWithTopics(int64_t id, function fn) +SessionI::runWithTopics(int64_t topicId, function callback) { - auto t = _topics.find(id); + auto t = _topics.find(topicId); if (t != _topics.end()) { for (auto& [topic, subscriber] : t->second.getSubscribers()) @@ -1195,16 +1193,16 @@ SessionI::runWithTopics(int64_t id, function fn continue; } _topicLock = &lock; - fn(topic, subscriber); + callback(topic, subscriber); _topicLock = nullptr; } } } void -SessionI::runWithTopic(int64_t id, TopicI* topic, function fn) +SessionI::runWithTopic(int64_t topicId, TopicI* topic, function callback) { - auto t = _topics.find(id); + auto t = _topics.find(topicId); if (t != _topics.end()) { auto p = t->second.getSubscribers().find(topic); @@ -1216,7 +1214,7 @@ SessionI::runWithTopic(int64_t id, TopicI* topic, functionsecond); + callback(p->second); _topicLock = nullptr; } } @@ -1238,15 +1236,15 @@ SubscriberSessionI::getTopics(const string& name) const } void -SubscriberSessionI::s(int64_t topicId, int64_t elementId, DataSample dataSample, const Ice::Current& current) +SubscriberSessionI::s(int64_t topicId, int64_t elementId, DataSample dataSample, const Current& current) { lock_guard lock(_mutex); if (!_session || current.con != _connection) { if (current.con != _connection) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": discarding sample `" << dataSample.id << "' from `e" << elementId << '@' << topicId + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": discarding sample '" << dataSample.id << "' from 'e" << elementId << '@' << topicId << "'\n"; if (_connection) { @@ -1271,8 +1269,8 @@ SubscriberSessionI::s(int64_t topicId, int64_t elementId, DataSample dataSample, { if (_traceLevels->session > 2) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": queuing sample `" << dataSample.id << "[k" << dataSample.keyId << "]' from `e" + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": queuing sample '" << dataSample.id << "[k" << dataSample.keyId << "]' from 'e" << elementId << '@' << topicId << "'"; if (!current.facet.empty()) { @@ -1341,8 +1339,8 @@ SubscriberSessionI::reconnect(NodePrx node) { if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": trying to reconnect session with `" << node->ice_toString() << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": trying to reconnect session with '" << node->ice_toString() << "'"; } _parent->createPublisherSession(node, nullptr, dynamic_pointer_cast(shared_from_this())); } @@ -1373,8 +1371,8 @@ PublisherSessionI::reconnect(NodePrx node) { if (_traceLevels->session > 0) { - Trace out(_traceLevels, _traceLevels->sessionCat); - out << _id << ": trying to reconnect session with `" << node->ice_toString() << "'"; + Trace out(_traceLevels->logger, _traceLevels->sessionCat); + out << _id << ": trying to reconnect session with '" << node->ice_toString() << "'"; } _parent->createSubscriberSession(node, nullptr, dynamic_pointer_cast(shared_from_this())); } diff --git a/cpp/src/DataStorm/SessionI.h b/cpp/src/DataStorm/SessionI.h index 7246a96b391..05aec24bd1c 100644 --- a/cpp/src/DataStorm/SessionI.h +++ b/cpp/src/DataStorm/SessionI.h @@ -31,19 +31,17 @@ namespace DataStormI /// element. struct ElementSubscriber { - ElementSubscriber(const std::string& facet, const std::shared_ptr& key, int sessionInstanceId) - : facet(facet), - initialized(false), - lastId(0), + ElementSubscriber(std::string facet, std::shared_ptr key, int sessionInstanceId) + : facet(std::move(facet)), sessionInstanceId(sessionInstanceId) { - keys.insert(key); + keys.insert(std::move(key)); } const std::string facet; - bool initialized; + bool initialized{false}; // The ID of the last processed sample. - std::int64_t lastId; + std::int64_t lastId{0}; std::set> keys; int sessionInstanceId; }; @@ -51,12 +49,7 @@ namespace DataStormI class ElementSubscribers { public: - ElementSubscribers(std::string name, int priority) - : name(std::move(name)), - priority(priority), - _sessionInstanceId(0) - { - } + ElementSubscribers(std::string name, int priority) : name(std::move(name)), priority(priority) {} void addSubscriber( const std::shared_ptr& element, @@ -80,9 +73,12 @@ namespace DataStormI void removeSubscriber(const std::shared_ptr& element) { _subscribers.erase(element); } - std::map, ElementSubscriber>& getSubscribers() { return _subscribers; } + [[nodiscard]] std::map, ElementSubscriber>& getSubscribers() + { + return _subscribers; + } - ElementSubscriber* getSubscriber(const std::shared_ptr& element) + [[nodiscard]] ElementSubscriber* getSubscriber(const std::shared_ptr& element) { auto p = _subscribers.find(element); if (p != _subscribers.end()) @@ -92,7 +88,7 @@ namespace DataStormI return nullptr; } - bool reap(int sessionInstanceId) + [[nodiscard]] bool reap(int sessionInstanceId) { if (_sessionInstanceId != sessionInstanceId) { @@ -122,7 +118,7 @@ namespace DataStormI // local data element subscribing to the remote data element, and the ElementSubscriber object contains the // subscription details. std::map, ElementSubscriber> _subscribers; - int _sessionInstanceId; + int _sessionInstanceId{0}; }; /// Represents the subscription from a local topic object to a remote topic. @@ -131,7 +127,7 @@ namespace DataStormI public: TopicSubscriber(int sessionInstanceId) : sessionInstanceId(sessionInstanceId) {} - ElementSubscribers* add(std::int64_t id, std::string name, int priority) + [[nodiscard]] ElementSubscribers* add(std::int64_t id, std::string name, int priority) { auto p = _elements.find(id); if (p == _elements.end()) @@ -141,7 +137,7 @@ namespace DataStormI return &p->second; } - ElementSubscribers* get(std::int64_t elementId) + [[nodiscard]] ElementSubscribers* get(std::int64_t elementId) { auto p = _elements.find(elementId); if (p == _elements.end()) @@ -160,10 +156,10 @@ namespace DataStormI _elements.erase(p); return tmp; } - return ElementSubscribers("", 0); + return {"", 0}; } - std::map& getAll() { return _elements; } + [[nodiscard]] std::map& getAll() { return _elements; } void reap(int id) { @@ -217,7 +213,7 @@ namespace DataStormI } } - TopicSubscriber& getSubscriber(TopicI* topic) + [[nodiscard]] TopicSubscriber& getSubscriber(TopicI* topic) { assert(_subscribers.find(topic) != _subscribers.end()); return _subscribers.at(topic); @@ -228,7 +224,7 @@ namespace DataStormI std::map& getSubscribers() { return _subscribers; } // Determine if the subscriber should be reaped. - bool reap(int sessionInstanceId) + [[nodiscard]] bool reap(int sessionInstanceId) { if (sessionInstanceId != _sessionInstanceId) { @@ -260,7 +256,8 @@ namespace DataStormI // object contains the subscription details. std::map _subscribers; - // The session instance id for the last subscription. + // The session instance id is incremented each time a session is reconnected. Subscribers with a different + // session instance id can be discarded. int _sessionInstanceId; }; @@ -290,22 +287,22 @@ namespace DataStormI void connected(DataStormContract::SessionPrx, const Ice::ConnectionPtr&, const DataStormContract::TopicInfoSeq&); - bool disconnected(const Ice::ConnectionPtr&, std::exception_ptr); - bool retry(DataStormContract::NodePrx, std::exception_ptr); + [[nodiscard]] bool disconnected(const Ice::ConnectionPtr&, std::exception_ptr); + [[nodiscard]] bool retry(DataStormContract::NodePrx, std::exception_ptr); void destroyImpl(const std::exception_ptr&); - const std::string& getId() const { return _id; } + [[nodiscard]] const std::string& getId() const { return _id; } - Ice::ConnectionPtr getConnection() const; - std::optional getSession() const; - bool checkSession(); + [[nodiscard]] Ice::ConnectionPtr getConnection() const; + [[nodiscard]] std::optional getSession() const; + [[nodiscard]] bool checkSession(); - DataStormContract::SessionPrx getProxy() const { return _proxy; } + [[nodiscard]] DataStormContract::SessionPrx getProxy() const { return _proxy; } - DataStormContract::NodePrx getNode() const; + [[nodiscard]] DataStormContract::NodePrx getNode() const; void setNode(DataStormContract::NodePrx); - std::unique_lock& getTopicLock() { return *_topicLock; } + [[nodiscard]] std::unique_lock& getTopicLock() { return *_topicLock; } void subscribe(std::int64_t, TopicI*); void unsubscribe(std::int64_t, TopicI*); @@ -342,10 +339,10 @@ namespace DataStormI /// @param element The data element for which the last sample IDs are retrieved. /// @return A map where the key represents the remote element ID, and the value represents the last sample ID /// read by the specified data element. - DataStormContract::LongLongDict + [[nodiscard]] DataStormContract::LongLongDict getLastIds(std::int64_t topic, std::int64_t key, const std::shared_ptr& element); - std::vector> subscriberInitialized( + [[nodiscard]] std::vector> subscriberInitialized( std::int64_t, std::int64_t, const DataStormContract::DataSampleSeq&, @@ -370,18 +367,18 @@ namespace DataStormI /// Runs the provided callback function for each subscriber of the specified topic. /// The callback is executed with the topic's mutex locked. /// - /// @param id The ID of the topic to process. + /// @param topicId The ID of the topic to process. /// @param callback The callback function to execute for each subscriber. - void runWithTopics(std::int64_t id, std::function callback); + void runWithTopics(std::int64_t topicId, std::function callback); /// Runs the provided callback function for the specified topic, if it is among the subscribers for the given /// topic ID. /// The callback is executed with the topic's mutex locked. /// - /// @param id The ID of the topic to process. + /// @param topicId The ID of the topic to process. /// @param topic The topic to process. /// @param callback The callback function to execute for the subscriber. - void runWithTopic(std::int64_t id, TopicI* topic, std::function callback); + void runWithTopic(std::int64_t topicId, TopicI* topic, std::function callback); /// Returns the topics that match the specified name. /// @@ -416,13 +413,13 @@ namespace DataStormI DataStormContract::NodePrx _node; // Indicates whether the session has been destroyed. - bool _destroyed; + bool _destroyed{false}; // The instance ID of the session, incremented each time the session is reconnected. - int _sessionInstanceId; + int _sessionInstanceId{0}; // The number of attempts made to reconnect the session. - int _retryCount; + int _retryCount{0}; // A retry task, scheduled if an attempt to reconnect the session is underway; nullptr if no retry is scheduled. IceInternal::TimerTaskPtr _retryTask; @@ -454,7 +451,7 @@ namespace DataStormI void s(std::int64_t, std::int64_t, DataStormContract::DataSample, const Ice::Current&) final; private: - std::vector> getTopics(const std::string&) const final; + [[nodiscard]] std::vector> getTopics(const std::string&) const final; void reconnect(DataStormContract::NodePrx) final; void remove() final; }; @@ -469,7 +466,7 @@ namespace DataStormI DataStormContract::SessionPrx); private: - std::vector> getTopics(const std::string&) const final; + [[nodiscard]] std::vector> getTopics(const std::string&) const final; void reconnect(DataStormContract::NodePrx) final; void remove() final; }; diff --git a/cpp/src/DataStorm/TopicFactoryI.cpp b/cpp/src/DataStorm/TopicFactoryI.cpp index c10d79a67b3..97939250a52 100644 --- a/cpp/src/DataStorm/TopicFactoryI.cpp +++ b/cpp/src/DataStorm/TopicFactoryI.cpp @@ -11,13 +11,10 @@ using namespace std; using namespace DataStormI; +using namespace DataStormContract; +using namespace Ice; -TopicFactoryI::TopicFactoryI(shared_ptr instance) - : _instance{std::move(instance)}, - _nextReaderId{0}, - _nextWriterId{0} -{ -} +TopicFactoryI::TopicFactoryI(const shared_ptr& instance) : _instance{instance} {} shared_ptr TopicFactoryI::createTopicReader( @@ -47,7 +44,7 @@ TopicFactoryI::createTopicReader( _readers[name].push_back(reader); if (instance->getTraceLevels()->topic > 0) { - Trace out(instance->getTraceLevels(), instance->getTraceLevels()->topicCat); + Trace out(instance->getTraceLevels()->logger, instance->getTraceLevels()->topicCat); out << name << ": created topic reader"; } @@ -62,13 +59,13 @@ TopicFactoryI::createTopicReader( { node->createSubscriberSession(nodePrx, nullptr, nullptr); } - node->getSubscriberForwarder()->announceTopics({{name, {reader->getId()}}}, false); + node->getSubscriberForwarder()->announceTopics({TopicInfo{.name = name, .ids = {reader->getId()}}}, false); instance->getNodeSessionManager()->announceTopicReader(name, nodePrx); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { } return reader; @@ -103,7 +100,7 @@ TopicFactoryI::createTopicWriter( if (instance->getTraceLevels()->topic > 0) { - Trace out(instance->getTraceLevels(), instance->getTraceLevels()->topicCat); + Trace out(instance->getTraceLevels()->logger, instance->getTraceLevels()->topicCat); out << name << ": created topic writer"; } @@ -118,13 +115,13 @@ TopicFactoryI::createTopicWriter( { node->createPublisherSession(nodePrx, nullptr, nullptr); } - node->getPublisherForwarder()->announceTopics({{name, {writer->getId()}}}, false); + node->getPublisherForwarder()->announceTopics({TopicInfo{.name = name, .ids = {writer->getId()}}}, false); instance->getNodeSessionManager()->announceTopicWriter(name, nodePrx); } - catch (const Ice::CommunicatorDestroyedException&) + catch (const CommunicatorDestroyedException&) { } - catch (const Ice::ObjectAdapterDestroyedException&) + catch (const ObjectAdapterDestroyedException&) { } @@ -139,7 +136,7 @@ TopicFactoryI::removeTopicReader(const string& name, const shared_ptr& r assert(instance); if (instance->getTraceLevels()->topic > 0) { - Trace out(instance->getTraceLevels(), instance->getTraceLevels()->topicCat); + Trace out(instance->getTraceLevels()->logger, instance->getTraceLevels()->topicCat); out << name << ": destroyed topic reader"; } auto& readers = _readers[name]; @@ -158,7 +155,7 @@ TopicFactoryI::removeTopicWriter(const string& name, const shared_ptr& w assert(instance); if (instance->getTraceLevels()->topic > 0) { - Trace out(instance->getTraceLevels(), instance->getTraceLevels()->topicCat); + Trace out(instance->getTraceLevels()->logger, instance->getTraceLevels()->topicCat); out << name << ": destroyed topic writer"; } auto& writers = _writers[name]; @@ -176,7 +173,7 @@ TopicFactoryI::getTopicReaders(const string& name) const auto p = _readers.find(name); if (p == _readers.end()) { - return vector>(); + return {}; } return p->second; } @@ -188,7 +185,7 @@ TopicFactoryI::getTopicWriters(const string& name) const auto p = _writers.find(name); if (p == _writers.end()) { - return vector>(); + return {}; } return p->second; } @@ -197,7 +194,7 @@ void TopicFactoryI::createPublisherSession( const string& topic, DataStormContract::NodePrx publisher, - const Ice::ConnectionPtr& connection) + const ConnectionPtr& connection) { auto readers = getTopicReaders(topic); if (!readers.empty()) @@ -212,7 +209,7 @@ void TopicFactoryI::createSubscriberSession( const string& topic, DataStormContract::NodePrx subscriber, - const Ice::ConnectionPtr& connection) + const ConnectionPtr& connection) { auto writers = getTopicWriters(topic); if (!writers.empty()) @@ -263,11 +260,11 @@ TopicFactoryI::getTopicWriters() const return writers; } -Ice::StringSeq +StringSeq TopicFactoryI::getTopicReaderNames() const { lock_guard lock(_mutex); - Ice::StringSeq readers; + StringSeq readers; readers.reserve(_readers.size()); for (const auto& [name, _] : _readers) { @@ -276,11 +273,11 @@ TopicFactoryI::getTopicReaderNames() const return readers; } -Ice::StringSeq +StringSeq TopicFactoryI::getTopicWriterNames() const { lock_guard lock(_mutex); - Ice::StringSeq writers; + StringSeq writers; writers.reserve(_writers.size()); for (const auto& [name, _] : _writers) { @@ -310,7 +307,7 @@ TopicFactoryI::shutdown() const } } -Ice::CommunicatorPtr +CommunicatorPtr TopicFactoryI::getCommunicator() const { auto instance = _instance.lock(); diff --git a/cpp/src/DataStorm/TopicFactoryI.h b/cpp/src/DataStorm/TopicFactoryI.h index 9552cafcb7c..cc61016c520 100644 --- a/cpp/src/DataStorm/TopicFactoryI.h +++ b/cpp/src/DataStorm/TopicFactoryI.h @@ -18,9 +18,9 @@ namespace DataStormI class TopicFactoryI final : public TopicFactory, public std::enable_shared_from_this { public: - TopicFactoryI(std::shared_ptr); + TopicFactoryI(const std::shared_ptr&); - std::shared_ptr createTopicReader( + [[nodiscard]] std::shared_ptr createTopicReader( std::string, std::shared_ptr, std::shared_ptr, @@ -28,7 +28,7 @@ namespace DataStormI std::shared_ptr, std::shared_ptr) final; - std::shared_ptr createTopicWriter( + [[nodiscard]] std::shared_ptr createTopicWriter( std::string, std::shared_ptr, std::shared_ptr, @@ -36,7 +36,7 @@ namespace DataStormI std::shared_ptr, std::shared_ptr) final; - Ice::CommunicatorPtr getCommunicator() const final; + [[nodiscard]] Ice::CommunicatorPtr getCommunicator() const final; void removeTopicReader(const std::string&, const std::shared_ptr&); void removeTopicWriter(const std::string&, const std::shared_ptr&); @@ -47,19 +47,19 @@ namespace DataStormI void createSubscriberSession(const std::string&, DataStormContract::NodePrx, const Ice::ConnectionPtr&); void createPublisherSession(const std::string&, DataStormContract::NodePrx, const Ice::ConnectionPtr&); - DataStormContract::TopicInfoSeq getTopicReaders() const; - DataStormContract::TopicInfoSeq getTopicWriters() const; + [[nodiscard]] DataStormContract::TopicInfoSeq getTopicReaders() const; + [[nodiscard]] DataStormContract::TopicInfoSeq getTopicWriters() const; - Ice::StringSeq getTopicReaderNames() const; - Ice::StringSeq getTopicWriterNames() const; + [[nodiscard]] Ice::StringSeq getTopicReaderNames() const; + [[nodiscard]] Ice::StringSeq getTopicWriterNames() const; void shutdown() const; private: std::weak_ptr _instance; mutable std::mutex _mutex; - std::int64_t _nextReaderId; - std::int64_t _nextWriterId; + std::int64_t _nextReaderId{0}; + std::int64_t _nextWriterId{0}; // A map of topic readers indexed by the topic name. // Each key is a topic name, and the corresponding value is a vector of readers for that topic. diff --git a/cpp/src/DataStorm/TopicI.cpp b/cpp/src/DataStorm/TopicI.cpp index 788bad487c3..ee6bc77d44c 100644 --- a/cpp/src/DataStorm/TopicI.cpp +++ b/cpp/src/DataStorm/TopicI.cpp @@ -10,11 +10,12 @@ using namespace std; using namespace DataStormI; using namespace DataStormContract; +using namespace Ice; namespace { static Topic::Updater noOpUpdater = // NOLINT:cert-err58-cpp - [](const shared_ptr& previous, const shared_ptr& next, const Ice::CommunicatorPtr&) + [](const shared_ptr& previous, const shared_ptr& next, const CommunicatorPtr&) { next->setValue(previous); }; // The always match filter always matches the value, it's used by the any key reader/writer. @@ -29,7 +30,7 @@ namespace return alwaysmatch; } - [[nodiscard]] Ice::ByteSeq encode(const Ice::CommunicatorPtr&) const final { return {}; } + [[nodiscard]] ByteSeq encode(const CommunicatorPtr&) const final { return {}; } [[nodiscard]] int64_t getId() const final { @@ -64,7 +65,7 @@ namespace } else { - throw Ice::ParseException(__FILE__, __LINE__, "Invalid clear history policy: " + value); + throw ParseException(__FILE__, __LINE__, "Invalid clear history policy: " + value); } } @@ -84,14 +85,14 @@ namespace } else { - throw Ice::ParseException(__FILE__, __LINE__, "Invalid discard policy: " + value); + throw ParseException(__FILE__, __LINE__, "Invalid discard policy: " + value); } } } TopicI::TopicI( shared_ptr instance, - shared_ptr factory, + const shared_ptr& factory, shared_ptr keyFactory, shared_ptr tagFactory, shared_ptr sampleFactory, @@ -99,7 +100,7 @@ TopicI::TopicI( shared_ptr sampleFilterFactories, string name, int64_t id) - : _factory(std::move(factory)), + : _factory(factory), _keyFactory(std::move(keyFactory)), _tagFactory(std::move(tagFactory)), _sampleFactory(std::move(sampleFactory)), @@ -112,14 +113,7 @@ TopicI::TopicI( // The collocated forwarder is initalized here to avoid using a nullable proxy. The forwarder is only used by // the instance that owns it and is removed in destroy implementation. _forwarder{_instance->getCollocatedForwarder()->add( - [this](Ice::ByteSeq inParams, const Ice::Current& current) { forward(inParams, current); })}, - _destroyed(false), - _listenerCount(0), - _waiters(0), - _notified(0), - _nextId(0), - _nextFilteredId(0), - _nextSampleId(0) + [this](const ByteSeq& inParams, const Current& current) { forward(inParams, current); })} { } @@ -169,11 +163,11 @@ TopicI::getTopicSpec() const spec.id = _id; spec.name = _name; spec.elements.reserve(_keyElements.size() + _filteredElements.size()); - for (auto k : _keyElements) + for (const auto& k : _keyElements) { spec.elements.push_back({k.first->getId(), "", k.first->encode(_instance->getCommunicator())}); } - for (auto f : _filteredElements) + for (const auto& f : _filteredElements) { spec.elements.push_back({-f.first->getId(), f.first->getName(), f.first->encode(_instance->getCommunicator())}); } @@ -186,9 +180,9 @@ TopicI::getTags() const { ElementInfoSeq tags; tags.reserve(_updaters.size()); - for (auto u : _updaters) + for (const auto& [tag, _] : _updaters) { - tags.push_back({u.first->getId(), "", u.first->encode(_instance->getCommunicator())}); + tags.push_back({tag->getId(), "", tag->encode(_instance->getCommunicator())}); } return tags; } @@ -373,7 +367,7 @@ TopicI::attachElements( int64_t topicId, const ElementSpecSeq& elements, const shared_ptr& session, - SessionPrx prx, + const SessionPrx& prx, const chrono::time_point& now) { // Called by the session holding the session and topic locks. @@ -424,7 +418,7 @@ TopicI::attachElements( .elements = std::move(acks), .id = key->getId(), .name = "", - .value = spec.id < 0 ? key->encode(_instance->getCommunicator()) : Ice::ByteSeq{}, + .value = spec.id < 0 ? key->encode(_instance->getCommunicator()) : ByteSeq{}, .peerId = spec.id, .peerName = spec.name}); } @@ -473,7 +467,7 @@ TopicI::attachElements( .elements = std::move(acks), .id = -filter->getId(), .name = filter->getName(), - .value = spec.id > 0 ? filter->encode(_instance->getCommunicator()) : Ice::ByteSeq{}, + .value = spec.id > 0 ? filter->encode(_instance->getCommunicator()) : ByteSeq{}, .peerId = spec.id, .peerName = spec.name}); } @@ -489,9 +483,9 @@ TopicI::attachElementsAck( int64_t topicId, const ElementSpecAckSeq& elements, const shared_ptr& session, - SessionPrx prx, + const SessionPrx& prx, const chrono::time_point& now, - Ice::LongSeq& removedIds) + LongSeq& removedIds) { DataSamplesSeq samples; vector> initCallbacks; @@ -625,7 +619,7 @@ TopicI::attachElementsAck( // Initialize samples on data elements once all the elements have been attached. This is important for the priority // configuration in case 2 writers with different priorities are attached from the same session. - for (auto initCb : initCallbacks) + for (const auto& initCb : initCallbacks) { initCb(); } @@ -731,7 +725,7 @@ TopicI::remove(const shared_ptr& element, const vector& element, const shared_ptr instance, - shared_ptr factory, + const shared_ptr& factory, shared_ptr keyFactory, shared_ptr tagFactory, shared_ptr sampleFactory, @@ -913,7 +907,7 @@ TopicReaderI::TopicReaderI( int64_t id) : TopicI( std::move(instance), - std::move(factory), + factory, std::move(keyFactory), std::move(tagFactory), std::move(sampleFactory), @@ -931,7 +925,7 @@ TopicReaderI::createFiltered( string name, DataStorm::ReaderConfig config, string sampleFilterName, - Ice::ByteSeq sampleFilterCriteria) + ByteSeq sampleFilterCriteria) { lock_guard lock(_mutex); auto element = make_shared( @@ -941,7 +935,7 @@ TopicReaderI::createFiltered( filter, std::move(sampleFilterName), std::move(sampleFilterCriteria), - mergeConfigs(std::move(config))); + mergeConfigs(config)); addFiltered(element, filter); return element; } @@ -952,7 +946,7 @@ TopicReaderI::create( string name, DataStorm::ReaderConfig config, string sampleFilterName, - Ice::ByteSeq sampleFilterCriteria) + ByteSeq sampleFilterCriteria) { lock_guard lock(_mutex); auto element = make_shared( @@ -962,7 +956,7 @@ TopicReaderI::create( keys, std::move(sampleFilterName), std::move(sampleFilterCriteria), - mergeConfigs(std::move(config))); + mergeConfigs(config)); add(element, keys); return element; } @@ -971,7 +965,7 @@ void TopicReaderI::setDefaultConfig(DataStorm::ReaderConfig config) { lock_guard lock(_mutex); - _defaultConfig = mergeConfigs(std::move(config)); + _defaultConfig = mergeConfigs(config); } void @@ -1040,7 +1034,7 @@ TopicReaderI::mergeConfigs(DataStorm::ReaderConfig config) const TopicWriterI::TopicWriterI( shared_ptr instance, - shared_ptr factory, + const shared_ptr& factory, shared_ptr keyFactory, shared_ptr tagFactory, shared_ptr sampleFactory, @@ -1050,7 +1044,7 @@ TopicWriterI::TopicWriterI( int64_t id) : TopicI( std::move(instance), - std::move(factory), + factory, std::move(keyFactory), std::move(tagFactory), std::move(sampleFactory), @@ -1066,7 +1060,7 @@ shared_ptr TopicWriterI::create(const vector>& keys, string name, DataStorm::WriterConfig config) { lock_guard lock(_mutex); - auto element = make_shared(this, std::move(name), ++_nextId, keys, mergeConfigs(std::move(config))); + auto element = make_shared(this, std::move(name), ++_nextId, keys, mergeConfigs(config)); add(element, keys); return element; } @@ -1075,7 +1069,7 @@ void TopicWriterI::setDefaultConfig(DataStorm::WriterConfig config) { lock_guard lock(_mutex); - _defaultConfig = mergeConfigs(std::move(config)); + _defaultConfig = mergeConfigs(config); } void diff --git a/cpp/src/DataStorm/TopicI.h b/cpp/src/DataStorm/TopicI.h index 743f0f70abc..4c660253881 100644 --- a/cpp/src/DataStorm/TopicI.h +++ b/cpp/src/DataStorm/TopicI.h @@ -28,7 +28,7 @@ namespace DataStormI public: TopicI( std::shared_ptr, - std::shared_ptr, + const std::shared_ptr&, std::shared_ptr, std::shared_ptr, std::shared_ptr, @@ -39,16 +39,16 @@ namespace DataStormI ~TopicI() override; - std::string getName() const override; + [[nodiscard]] std::string getName() const override; void destroy() override; void shutdown(); // const getter for _instance - const std::shared_ptr& instance() const noexcept { return _instance; } + [[nodiscard]] const std::shared_ptr& instance() const noexcept { return _instance; } - DataStormContract::TopicSpec getTopicSpec() const; - DataStormContract::ElementInfoSeq getTags() const; + [[nodiscard]] DataStormContract::TopicSpec getTopicSpec() const; + [[nodiscard]] DataStormContract::ElementInfoSeq getTags() const; /// Compute the element specs for the local elements that match the given element infos. /// @@ -56,7 +56,7 @@ namespace DataStormI /// @param infos The element infos to match. /// @param session The session that requested the element specs. /// @return The element specs for the local elements that match the given element infos. - DataStormContract::ElementSpecSeq getElementSpecs( + [[nodiscard]] DataStormContract::ElementSpecSeq getElementSpecs( std::int64_t topicId, const DataStormContract::ElementInfoSeq& infos, const std::shared_ptr& session); @@ -74,40 +74,43 @@ namespace DataStormI /// @param session The session servant in the current node. void detach(std::int64_t topicId, const std::shared_ptr& session); - DataStormContract::ElementSpecAckSeq attachElements( + [[nodiscard]] DataStormContract::ElementSpecAckSeq attachElements( std::int64_t, const DataStormContract::ElementSpecSeq&, const std::shared_ptr&, - DataStormContract::SessionPrx, + const DataStormContract::SessionPrx&, const std::chrono::time_point&); - DataStormContract::DataSamplesSeq attachElementsAck( + [[nodiscard]] DataStormContract::DataSamplesSeq attachElementsAck( std::int64_t, const DataStormContract::ElementSpecAckSeq&, const std::shared_ptr&, - DataStormContract::SessionPrx, + const DataStormContract::SessionPrx&, const std::chrono::time_point&, Ice::LongSeq&); void setUpdater(const std::shared_ptr&, Updater) override; - const Updater& getUpdater(const std::shared_ptr&) const; + [[nodiscard]] const Updater& getUpdater(const std::shared_ptr&) const; void setUpdaters(std::map, Updater>) override; - std::map, Updater> getUpdaters() const override; + [[nodiscard]] std::map, Updater> getUpdaters() const override; - bool isDestroyed() const { return _destroyed; } + [[nodiscard]] bool isDestroyed() const { return _destroyed; } - std::int64_t getId() const { return _id; } + [[nodiscard]] std::int64_t getId() const { return _id; } - std::mutex& getMutex() { return _mutex; } + [[nodiscard]] std::mutex& getMutex() { return _mutex; } - const std::shared_ptr& getKeyFactory() const { return _keyFactory; } + [[nodiscard]] const std::shared_ptr& getKeyFactory() const { return _keyFactory; } - const std::shared_ptr& getTagFactory() const { return _tagFactory; } + [[nodiscard]] const std::shared_ptr& getTagFactory() const { return _tagFactory; } - const std::shared_ptr& getSampleFactory() const { return _sampleFactory; } + [[nodiscard]] const std::shared_ptr& getSampleFactory() const { return _sampleFactory; } - const std::shared_ptr& getSampleFilterFactories() const { return _sampleFilterFactories; } + [[nodiscard]] const std::shared_ptr& getSampleFilterFactories() const + { + return _sampleFilterFactories; + } void incListenerCount(const std::shared_ptr&); void decListenerCount(const std::shared_ptr&); @@ -118,7 +121,7 @@ namespace DataStormI protected: void waitForListeners(int count) const; - bool hasListeners() const; + [[nodiscard]] bool hasListeners() const; void notifyListenerWaiters(std::unique_lock&) const; void disconnect(); @@ -150,7 +153,7 @@ namespace DataStormI mutable std::mutex _mutex; mutable std::condition_variable _cond; - bool _destroyed; + bool _destroyed{false}; // A map containing the data readers or data writers for this topic. // The map's key is a pointer returned by the topic's key factory, and the value is a set of data elements @@ -176,14 +179,14 @@ namespace DataStormI std::map, Updater> _updaters; // The number of connected listeners. - size_t _listenerCount; + size_t _listenerCount{0}; // The number of threads waiting for a listener notification. See waitForListeners(). - mutable size_t _waiters; - mutable size_t _notified; - std::int64_t _nextId; - std::int64_t _nextFilteredId; - std::int64_t _nextSampleId; + mutable size_t _waiters{0}; + mutable size_t _notified{0}; + std::int64_t _nextId{0}; + std::int64_t _nextFilteredId{0}; + std::int64_t _nextSampleId{0}; }; class TopicReaderI final : public TopicReader, public TopicI @@ -191,7 +194,7 @@ namespace DataStormI public: TopicReaderI( std::shared_ptr, - std::shared_ptr, + const std::shared_ptr&, std::shared_ptr, std::shared_ptr, std::shared_ptr, @@ -200,11 +203,11 @@ namespace DataStormI std::string, std::int64_t); - std::shared_ptr + [[nodiscard]] std::shared_ptr createFiltered(const std::shared_ptr&, std::string, DataStorm::ReaderConfig, std::string, Ice::ByteSeq) final; - std::shared_ptr create( + [[nodiscard]] std::shared_ptr create( const std::vector>&, std::string, DataStorm::ReaderConfig, @@ -213,12 +216,12 @@ namespace DataStormI void setDefaultConfig(DataStorm::ReaderConfig) final; void waitForWriters(int) const final; - bool hasWriters() const final; + [[nodiscard]] bool hasWriters() const final; void destroy() final; private: - DataStorm::ReaderConfig parseConfig() const; - DataStorm::ReaderConfig mergeConfigs(DataStorm::ReaderConfig) const; + [[nodiscard]] DataStorm::ReaderConfig parseConfig() const; + [[nodiscard]] DataStorm::ReaderConfig mergeConfigs(DataStorm::ReaderConfig) const; DataStorm::ReaderConfig _defaultConfig; }; @@ -228,7 +231,7 @@ namespace DataStormI public: TopicWriterI( std::shared_ptr, - std::shared_ptr, + const std::shared_ptr&, std::shared_ptr, std::shared_ptr, std::shared_ptr, @@ -237,7 +240,7 @@ namespace DataStormI std::string, std::int64_t); - std::shared_ptr + [[nodiscard]] std::shared_ptr create(const std::vector>&, std::string, DataStorm::WriterConfig) final; void setDefaultConfig(DataStorm::WriterConfig) final; @@ -246,8 +249,8 @@ namespace DataStormI void destroy() final; private: - DataStorm::WriterConfig parseConfig() const; - DataStorm::WriterConfig mergeConfigs(DataStorm::WriterConfig) const; + [[nodiscard]] DataStorm::WriterConfig parseConfig() const; + [[nodiscard]] DataStorm::WriterConfig mergeConfigs(DataStorm::WriterConfig) const; DataStorm::WriterConfig _defaultConfig; }; diff --git a/cpp/src/DataStorm/TraceUtil.cpp b/cpp/src/DataStorm/TraceUtil.cpp index eb8ba4ca524..0dde936fcd4 100644 --- a/cpp/src/DataStorm/TraceUtil.cpp +++ b/cpp/src/DataStorm/TraceUtil.cpp @@ -6,6 +6,7 @@ using namespace std; using namespace DataStormI; +using namespace Ice; #if defined(__clang__) # pragma clang diagnostic push @@ -15,7 +16,7 @@ using namespace DataStormI; # pragma GCC diagnostic ignored "-Wshadow" #endif -TraceLevels::TraceLevels(const Ice::PropertiesPtr& properties, Ice::LoggerPtr logger) +TraceLevels::TraceLevels(const PropertiesPtr& properties, LoggerPtr logger) : topic(properties->getIcePropertyAsInt("DataStorm.Trace.Topic")), topicCat("Topic"), data(properties->getIcePropertyAsInt("DataStorm.Trace.Data")), diff --git a/cpp/src/DataStorm/TraceUtil.h b/cpp/src/DataStorm/TraceUtil.h index faefb6ded92..5c760e026c1 100644 --- a/cpp/src/DataStorm/TraceUtil.h +++ b/cpp/src/DataStorm/TraceUtil.h @@ -12,7 +12,8 @@ #include "TopicI.h" // TODO: explain why we need to use namespace std here. -namespace std // NOLINT:cert-dcl58-cpp +// NOLINTBEGIN:cert-dcl58-cpp +namespace std { template inline std::ostream& operator<<(std::ostream& os, const std::vector& p) { @@ -46,6 +47,7 @@ namespace std // NOLINT:cert-dcl58-cpp return os; } } +// NOLINTEND:cert-dcl58-cpp namespace Ice { @@ -143,32 +145,28 @@ namespace DataStormContract namespace DataStormI { - template::value>::type* = nullptr> + template>* = nullptr> inline std::ostream& operator<<(std::ostream& os, const std::shared_ptr& p) { os << (p ? p->toString() : ""); return os; } - template< - typename T, - typename ::std::enable_if<::std::is_base_of::value>::type* = nullptr> + template>* = nullptr> inline std::ostream& operator<<(std::ostream& os, T* element) { os << (element ? element->toString() : ""); return os; } - template< - typename T, - typename ::std::enable_if<::std::is_base_of::value>::type* = nullptr> + template>* = nullptr> inline std::ostream& operator<<(std::ostream& os, const std::shared_ptr& element) { os << element.get(); return os; } - template::value>::type* = nullptr> + template>* = nullptr> inline std::ostream& operator<<(std::ostream& os, T* session) { if (session) @@ -184,8 +182,7 @@ namespace DataStormI template< typename T, - typename ::std::enable_if< - ::std::is_base_of::type>::value>::type* = nullptr> + typename std::enable_if_t>>* = nullptr> inline std::ostream& operator<<(std::ostream& os, T topic) { if (topic) @@ -199,7 +196,7 @@ namespace DataStormI return os; } - template::value>::type* = nullptr> + template>* = nullptr> inline std::ostream& operator<<(std::ostream& os, const std::shared_ptr& topic) { os << topic.get(); @@ -226,16 +223,7 @@ namespace DataStormI class Trace : public Ice::Trace { public: - Trace(std::shared_ptr traceLevels, const std::string& category) - : Ice::Trace(traceLevels->logger, category) - { - } - }; - - class Warning : public Ice::Warning - { - public: - Warning(std::shared_ptr traceLevels) : Ice::Warning(traceLevels->logger) {} + Trace(Ice::LoggerPtr logger, std::string category) : Ice::Trace(std::move(logger), std::move(category)) {} }; } #endif diff --git a/cpp/src/Ice/LoggerUtil.cpp b/cpp/src/Ice/LoggerUtil.cpp index 4870e065ac4..b836fc9d9c0 100644 --- a/cpp/src/Ice/LoggerUtil.cpp +++ b/cpp/src/Ice/LoggerUtil.cpp @@ -37,7 +37,7 @@ Ice::loggerInsert(Ice::LoggerOutputBase& out, const Ice::Exception& ex) return out; } -Ice::Trace::Trace(const LoggerPtr& logger, const string& category) : _logger(logger), _category(category) {} +Ice::Trace::Trace(LoggerPtr logger, string category) : _logger(std::move(logger)), _category(std::move(category)) {} Ice::Trace::~Trace() { flush(); } diff --git a/cpp/src/dsnode/Node.cpp b/cpp/src/dsnode/Node.cpp index df60145ba2f..c3b302227b9 100644 --- a/cpp/src/dsnode/Node.cpp +++ b/cpp/src/dsnode/Node.cpp @@ -22,9 +22,7 @@ main(int argc, char* argv[]) { try { - // // Parse arguments. - // for (int i = 0; i < argc; ++i) { string arg = argv[i]; @@ -40,9 +38,7 @@ main(int argc, char* argv[]) } } - // // CtrlCHandler must be created before the node is created or any other threads are started. - // Ice::CtrlCHandler ctrlCHandler; DataStorm::Node node{argc, argv}; @@ -53,14 +49,11 @@ main(int argc, char* argv[]) usage(argv[0]); return 1; } - // + // Shutdown the node on Ctrl-C. - // ctrlCHandler.setCallback([&node](int) { node.shutdown(); }); - // // Exit once the user hits Ctrl-C to shutdown the node. - // node.waitForShutdown(); } catch (const std::exception& ex) diff --git a/cpp/test/DataStorm/api/Writer.cpp b/cpp/test/DataStorm/api/Writer.cpp index ae002c15380..fe6edd04ec3 100644 --- a/cpp/test/DataStorm/api/Writer.cpp +++ b/cpp/test/DataStorm/api/Writer.cpp @@ -110,7 +110,7 @@ void ::Writer::run(int argc, char* argv[]) testException([&reader] { reader.waitForWriters(); }); testException([&reader] { reader.waitForNoWriters(); }); testException([&reader] { reader.waitForUnread(); }); - testException([&reader] { reader.getNextUnread(); }); + testException([&reader] { [[maybe_unused]] auto _ = reader.getNextUnread(); }); } } cout << "ok" << endl; @@ -170,12 +170,12 @@ void ::Writer::run(int argc, char* argv[]) test(writer.getAll().empty()); try { - writer.getLast(); + [[maybe_unused]] auto last = writer.getLast(); } catch (const std::logic_error&) { } - writer.getAll(); + [[maybe_unused]] auto all = writer.getAll(); writer.onConnectedKeys([](vector) {}, [](CallbackReason, string) {}); }; @@ -230,13 +230,9 @@ void ::Writer::run(int argc, char* argv[]) reader.waitForNoWriters(); [[maybe_unused]] auto _ = reader.getConnectedWriters(); [[maybe_unused]] auto connectedKeys = reader.getConnectedKeys(); - reader.getAllUnread(); + [[maybe_unused]] auto allUnread = reader.getAllUnread(); reader.waitForUnread(0); [[maybe_unused]] bool hasUnread = reader.hasUnread(); - if (false) - { - reader.getNextUnread(); - } reader.onConnectedKeys([](vector) {}, [](CallbackReason, string) {}); reader.onSamples([](vector>) {}, [](Sample) {}); }; @@ -284,7 +280,7 @@ void ::Writer::run(int argc, char* argv[]) try { - makeFilteredKeyReader(topic, Filter("unknown", "")); + [[maybe_unused]] auto unknownFilteredReader = makeFilteredKeyReader(topic, Filter("unknown", "")); test(false); } catch (const std::invalid_argument&) @@ -293,7 +289,7 @@ void ::Writer::run(int argc, char* argv[]) try { - makeFilteredKeyReader(topic, Filter("_regex", "(")); + [[maybe_unused]] auto regexFilteredReader = makeFilteredKeyReader(topic, Filter("_regex", "(")); test(false); } catch (const std::invalid_argument&) diff --git a/cpp/test/DataStorm/events/Reader.cpp b/cpp/test/DataStorm/events/Reader.cpp index f76109beb0c..ca388bfb0f7 100644 --- a/cpp/test/DataStorm/events/Reader.cpp +++ b/cpp/test/DataStorm/events/Reader.cpp @@ -254,7 +254,8 @@ void ::Reader::run(int argc, char* argv[]) auto reader = makeFilteredKeyReader(topic, Filter("startswith", "val"), "", config); reader.waitForWriters(1); test(reader.hasWriters()); - reader.getNextUnread(); + auto sample = reader.getNextUnread(); + test(sample.getKey().find("val") == 0); } } diff --git a/cpp/test/DataStorm/reliability/Reader.cpp b/cpp/test/DataStorm/reliability/Reader.cpp index 8c0c09538b4..0b0856ef001 100644 --- a/cpp/test/DataStorm/reliability/Reader.cpp +++ b/cpp/test/DataStorm/reliability/Reader.cpp @@ -51,6 +51,7 @@ void ::Reader::run(int argc, char* argv[]) cerr << "unexpected sample: " << sample.getValue() << " expected:" << i << endl; test(false); } + if ((i % 50) == 0) { auto connection = node.getSessionConnection(sample.getSession()); diff --git a/cpp/test/DataStorm/reliability/Writer.cpp b/cpp/test/DataStorm/reliability/Writer.cpp index b0384e17191..e0517fd641b 100644 --- a/cpp/test/DataStorm/reliability/Writer.cpp +++ b/cpp/test/DataStorm/reliability/Writer.cpp @@ -36,9 +36,9 @@ void ::Writer::run(int argc, char* argv[]) test(connection); connection->close().get(); writer.update("update2"); - barrier.getNextUnread(); + sample = barrier.getNextUnread(); writer.update("update3"); - barrier.getNextUnread(); + sample = barrier.getNextUnread(); } cout << "ok" << endl; @@ -51,7 +51,7 @@ void ::Writer::run(int argc, char* argv[]) { writer.update(i); } - makeSingleKeyReader(topic, "barrier").getNextUnread(); + [[maybe_unused]] auto _ = makeSingleKeyReader(topic, "barrier").getNextUnread(); } cout << "ok" << endl; } diff --git a/cpp/test/DataStorm/reliability/test.py b/cpp/test/DataStorm/reliability/test.py index 73a28dabbc8..463a703734e 100644 --- a/cpp/test/DataStorm/reliability/test.py +++ b/cpp/test/DataStorm/reliability/test.py @@ -9,34 +9,38 @@ "DataStorm.Trace.Topic" : 1, "DataStorm.Trace.Session" : 3, "DataStorm.Trace.Data" : 2, + "Ice.Trace.Protocol" : 1, } +# A client connected to the default server clientProps = { "DataStorm.Node.Multicast.Enabled": 0, "DataStorm.Node.Server.Enabled": 0, "DataStorm.Node.ConnectTo": "tcp -p {port1}" } +# A client connected to the second server client2Props = { "DataStorm.Node.Multicast.Enabled": 0, "DataStorm.Node.Server.Enabled": 0, "DataStorm.Node.ConnectTo": "tcp -p {port2}" } +# The default server, not connected to any other server. serverProps = { "DataStorm.Node.Multicast.Enabled": 0, - "DataStorm.Node.Server.Enabled": 1, "DataStorm.Node.Server.Endpoints": "tcp -p {port1}", "DataStorm.Node.ConnectTo": "" } +# A second server connected to the first server server2Props = { "DataStorm.Node.Multicast.Enabled": 0, - "DataStorm.Node.Server.Enabled": 1, "DataStorm.Node.Server.Endpoints": "tcp -p {port2}", "DataStorm.Node.ConnectTo": "tcp -p {port1}" } +# A server without a fixed endpoint, connected to the first server. serverAnyProps = { "DataStorm.Node.Multicast.Enabled": 0, "DataStorm.Node.Server.Endpoints": "tcp", From 331a6407ae96a257d334682a94363f1b32174afa Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 19 Dec 2024 12:54:47 +0100 Subject: [PATCH 2/3] cleanup clang-tidy --- .clang-tidy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 1f185aa8325..be0a3b5d9af 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,8 +9,7 @@ Checks: -modernize-use-trailing-return-type, -modernize-concat-nested-namespaces, performance-*, - -performance-avoid-endl, - -modernize-use-constraints + -performance-avoid-endl ' WarningsAsErrors: '*' HeaderFilterRegex: '.*' From e40bb81741e2d2951f146ef165b72f6178959088 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 19 Dec 2024 16:02:03 +0100 Subject: [PATCH 3/3] Review fixes --- .clang-tidy | 2 +- cpp/include/DataStorm/Node.h | 4 ++-- cpp/src/DataStorm/ForwarderManager.cpp | 6 +++--- cpp/src/DataStorm/ForwarderManager.h | 15 ++++++++------- cpp/src/DataStorm/Node.cpp | 6 +++--- cpp/src/DataStorm/SessionI.cpp | 4 ++-- cpp/test/Ice/span/.clang-tidy | 6 ++++++ 7 files changed, 25 insertions(+), 18 deletions(-) create mode 100644 cpp/test/Ice/span/.clang-tidy diff --git a/.clang-tidy b/.clang-tidy index be0a3b5d9af..a330c09b07d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -20,4 +20,4 @@ ExtraArgs: ['-std=c++17'] CheckOptions: modernize-use-nullptr.NullMacros: 'NULL' # std::exception_ptr is a cheap to copy, pointer-like type; we pass it by value all the time. - performance-unnecessary-value-param.AllowedTypes: 'exception_ptr$;std::function$' + performance-unnecessary-value-param.AllowedTypes: 'exception_ptr$;' diff --git a/cpp/include/DataStorm/Node.h b/cpp/include/DataStorm/Node.h index 8a22477bc32..8a0c9868333 100644 --- a/cpp/include/DataStorm/Node.h +++ b/cpp/include/DataStorm/Node.h @@ -140,7 +140,7 @@ namespace DataStorm * provided the Node will use the default callback executor that executes callback in a dedicated thread. */ Node( - const Ice::CommunicatorPtr& communicator, + Ice::CommunicatorPtr communicator, std::function call)> customExecutor = nullptr); /** @@ -197,7 +197,7 @@ namespace DataStorm private: Node( - const Ice::CommunicatorPtr&, + Ice::CommunicatorPtr, std::function call)> customExecutor, bool ownsCommunicator); diff --git a/cpp/src/DataStorm/ForwarderManager.cpp b/cpp/src/DataStorm/ForwarderManager.cpp index 2be15952d84..1d3a598ceb8 100644 --- a/cpp/src/DataStorm/ForwarderManager.cpp +++ b/cpp/src/DataStorm/ForwarderManager.cpp @@ -9,8 +9,8 @@ using namespace DataStormI; using namespace Ice; ForwarderManager::ForwarderManager(ObjectAdapterPtr adapter, string category) - : _adapter(std::move(adapter)), - _category(std::move(category)) + : _adapter{std::move(adapter)}, + _category{std::move(category)} { } @@ -45,5 +45,5 @@ ForwarderManager::ice_invokeAsync( } forwarder = p->second; } - forwarder(std::move(inParams), std::move(response), std::move(exception), current); + forwarder(inParams, response, exception, current); } diff --git a/cpp/src/DataStorm/ForwarderManager.h b/cpp/src/DataStorm/ForwarderManager.h index ce025664f77..4a73a0bb985 100644 --- a/cpp/src/DataStorm/ForwarderManager.h +++ b/cpp/src/DataStorm/ForwarderManager.h @@ -22,7 +22,7 @@ namespace DataStormI ForwarderManager(Ice::ObjectAdapterPtr, std::string); template, bool> = true> - Prx add(std::function forwarder) + Prx add(std::function forwarder) { std::lock_guard lock(_mutex); const Ice::Identity id{.name = std::to_string(_nextId++), .category = _category}; @@ -31,18 +31,18 @@ namespace DataStormI } template, bool> = true> - Prx add(std::function forwarder) + Prx add(std::function forwarder) { return add( [forwarder = std::move(forwarder)]( - Ice::ByteSeq inParams, - std::function response, - std::function exception, + const Ice::ByteSeq& inParams, + const Response& response, + const Exception& exception, const Ice::Current& current) { try { - forwarder(std::move(inParams), current); + forwarder(inParams, current); response(true, {}); } catch (...) @@ -67,7 +67,8 @@ namespace DataStormI const std::string _category; std::mutex _mutex; - std::map> _forwarders; + std::map> + _forwarders; unsigned int _nextId{0}; }; } diff --git a/cpp/src/DataStorm/Node.cpp b/cpp/src/DataStorm/Node.cpp index dabc9610cf0..88b59c575a2 100644 --- a/cpp/src/DataStorm/Node.cpp +++ b/cpp/src/DataStorm/Node.cpp @@ -94,13 +94,13 @@ Node::Node(optional configFile, function call { } -Node::Node(const CommunicatorPtr& communicator, function call)> customExecutor) - : Node(communicator, std::move(customExecutor), false) +Node::Node(CommunicatorPtr communicator, function call)> customExecutor) + : Node(std::move(communicator), std::move(customExecutor), false) { } Node::Node( - const CommunicatorPtr& communicator, + CommunicatorPtr communicator, std::function call)> customExecutor, bool ownsCommunicator) : _ownsCommunicator(ownsCommunicator) diff --git a/cpp/src/DataStorm/SessionI.cpp b/cpp/src/DataStorm/SessionI.cpp index 7091a24ec58..166e555d49f 100644 --- a/cpp/src/DataStorm/SessionI.cpp +++ b/cpp/src/DataStorm/SessionI.cpp @@ -22,8 +22,8 @@ namespace { public: DispatchInterceptorI(ObjectPtr servant, shared_ptr executor) - : _servant(std::move(servant)), - _executor(std::move(executor)) + : _servant{std::move(servant)}, + _executor{std::move(executor)} { } diff --git a/cpp/test/Ice/span/.clang-tidy b/cpp/test/Ice/span/.clang-tidy new file mode 100644 index 00000000000..cbbc2a4cdce --- /dev/null +++ b/cpp/test/Ice/span/.clang-tidy @@ -0,0 +1,6 @@ +Checks: +' + -modernize-use-constraints +' +InheritParentConfig: true +ExtraArgs: ['-std=c++20']