diff --git a/cpp/implementations/plain/incoming_call_handler.h b/cpp/implementations/plain/incoming_call_handler.h index 78cd02e..c031bf7 100644 --- a/cpp/implementations/plain/incoming_call_handler.h +++ b/cpp/implementations/plain/incoming_call_handler.h @@ -32,11 +32,25 @@ public: virtual void onMethods(const MethodsMessage& req, MethodsReply reply) = 0; - // `sink` stays alive until onUnsubscribe fires or the connection dies. - // The handler must call `sink(evt)` on every matching emission. - virtual void onSubscribe(const SubscribeMessage& req, EventSink sink) = 0; + // `sink` stays alive until onUnsubscribe fires or the connection + // dies. The handler must call `sink(evt)` on every matching emission. + // + // `connectionId` is an opaque per-connection token (the rpc layer + // passes the connection's `this` pointer). The handler keys sinks + // by it so a subsequent onUnsubscribe / onConnectionClosed can + // remove only the sinks belonging to that connection — sub/unsub + // frames don't carry a subscriber identifier on the wire. + virtual void onSubscribe(const SubscribeMessage& req, EventSink sink, + const void* connectionId) = 0; - virtual void onUnsubscribe(const UnsubscribeMessage& req) = 0; + virtual void onUnsubscribe(const UnsubscribeMessage& req, + const void* connectionId) = 0; + + // Called when a connection is torn down (graceful close or error) + // so the handler can drop any sinks still keyed to it. Without + // this, a dropped client leaks subscriptions and the host keeps + // fanning events into dead sinks. + virtual void onConnectionClosed(const void* connectionId) = 0; virtual void onToken(const TokenMessage& req) = 0; }; diff --git a/cpp/implementations/plain/json_mapping.cpp b/cpp/implementations/plain/json_mapping.cpp index 90ca322..f79d687 100644 --- a/cpp/implementations/plain/json_mapping.cpp +++ b/cpp/implementations/plain/json_mapping.cpp @@ -1,5 +1,7 @@ #include "json_mapping.h" +#include + namespace logos::plain { using json = nlohmann::json; diff --git a/cpp/implementations/plain/plain_logos_object.cpp b/cpp/implementations/plain/plain_logos_object.cpp index 3048385..fb9b752 100644 --- a/cpp/implementations/plain/plain_logos_object.cpp +++ b/cpp/implementations/plain/plain_logos_object.cpp @@ -2,10 +2,14 @@ #include "qvariant_rpc_value.h" +#include #include +#include +#include #include #include +#include #include namespace logos::plain { @@ -51,6 +55,32 @@ QVariant PlainLogosObject::callMethod(const QString& authToken, return rpcValueToQVariant(res.value); } +namespace { + +// Hand `callback(result)` over to the Qt event loop so PlainLogosObject's +// async path matches LogosObject's interface contract: callbacks are +// always delivered on a subsequent event-loop iteration, on the Qt +// thread, never synchronously and never racing with QObjects/UI code. +// +// Using QCoreApplication::instance() as the anchor means the queued +// invocation lands on whichever thread runs the Qt event loop in this +// process, regardless of which worker thread completed the future. +// If the application has shut down (instance() is null), we drop the +// callback rather than invoke it from an arbitrary thread. +void postToQtEventLoop(PlainLogosObject::AsyncResultCallback callback, + QVariant result) +{ + QCoreApplication* app = QCoreApplication::instance(); + if (!app) return; + QMetaObject::invokeMethod(app, + [callback = std::move(callback), result = std::move(result)]() mutable { + callback(result); + }, + Qt::QueuedConnection); +} + +} // anonymous namespace + void PlainLogosObject::callMethodAsync(const QString& authToken, const QString& methodName, const QVariantList& args, @@ -59,7 +89,9 @@ void PlainLogosObject::callMethodAsync(const QString& authToken, { if (!callback) return; if (!m_conn || !m_conn->isOpen()) { - callback(QVariant()); + // Defer even the failure path — LogosObject's contract requires + // callbacks on a subsequent event-loop iteration, never inline. + postToQtEventLoop(std::move(callback), QVariant()); return; } @@ -73,17 +105,20 @@ void PlainLogosObject::callMethodAsync(const QString& authToken, auto fut = std::make_shared>( m_conn->sendCall(std::move(msg))); - // Simple waiter thread. Acceptable here because callMethodAsync is - // already an infrequent path and spawns no-ops if the caller never - // awaits. A future iteration can fold this into the io_context. + // Waiter thread is per-call but the callback hops back to the Qt + // event loop before running, so it never races with Qt objects. A + // future iteration can fold this wait into the shared Asio + // io_context (the connection already runs on it) so we don't spin + // up a thread per pending RPC. std::thread([fut, timeoutMs, callback = std::move(callback)]() mutable { if (fut->wait_for(std::chrono::milliseconds(timeoutMs)) != std::future_status::ready) { - callback(QVariant()); + postToQtEventLoop(std::move(callback), QVariant()); return; } auto res = fut->get(); - callback(res.ok ? rpcValueToQVariant(res.value) : QVariant()); + QVariant value = res.ok ? rpcValueToQVariant(res.value) : QVariant(); + postToQtEventLoop(std::move(callback), std::move(value)); }).detach(); } diff --git a/cpp/implementations/plain/plain_logos_object.h b/cpp/implementations/plain/plain_logos_object.h index f5bc198..c088b60 100644 --- a/cpp/implementations/plain/plain_logos_object.h +++ b/cpp/implementations/plain/plain_logos_object.h @@ -8,6 +8,8 @@ #include #include #include +#include +#include namespace logos::plain { diff --git a/cpp/implementations/plain/plain_transport_connection.cpp b/cpp/implementations/plain/plain_transport_connection.cpp index 95e4447..f293d15 100644 --- a/cpp/implementations/plain/plain_transport_connection.cpp +++ b/cpp/implementations/plain/plain_transport_connection.cpp @@ -11,8 +11,11 @@ #include #include #include +#include #include +#include + namespace logos::plain { namespace { @@ -77,6 +80,25 @@ bool PlainTransportConnection::connectToHost() if (m_cfg.protocol == LogosProtocol::TcpSsl) { auto ctx = buildClientSslCtx(m_cfg); SslStream stream(ioc, ctx); + // Set SNI: TLS clients must advertise the target host name + // in the ClientHello so the server picks the right cert + // (and so any intermediate proxy can route correctly). Without + // this, vhost-style deployments would terminate the handshake. + // Cast through the OpenSSL macro because Asio doesn't expose + // SNI configuration at the wrapper level. + if (!SSL_set_tlsext_host_name(stream.native_handle(), + m_cfg.host.c_str())) { + qWarning() << "PlainTransportConnection: SSL_set_tlsext_host_name failed"; + } + // Verify the peer's certificate name matches the host we + // dialed when verifyPeer is on. verify_peer alone only + // validates the chain — without host-name verification a + // valid cert for a *different* name would still pass, which + // is exactly the MITM hole verify_peer is meant to close. + if (m_cfg.verifyPeer) { + stream.set_verify_callback( + boost::asio::ssl::host_name_verification(m_cfg.host)); + } boost::asio::connect(stream.lowest_layer(), endpoints); stream.handshake(boost::asio::ssl::stream_base::client); auto conn = std::make_shared( diff --git a/cpp/implementations/plain/plain_transport_host.cpp b/cpp/implementations/plain/plain_transport_host.cpp index ac2f03b..9cf528a 100644 --- a/cpp/implementations/plain/plain_transport_host.cpp +++ b/cpp/implementations/plain/plain_transport_host.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include #include #include @@ -377,25 +379,49 @@ void PlainTransportHost::onMethods(const MethodsMessage& req, MethodsReply reply }, Qt::QueuedConnection); } -void PlainTransportHost::onSubscribe(const SubscribeMessage& req, EventSink sink) +void PlainTransportHost::onSubscribe(const SubscribeMessage& req, EventSink sink, + const void* connectionId) { std::lock_guard g(m_mu); auto it = m_published.find(req.object); if (it == m_published.end()) return; - // Sinks are keyed by functor-shared-pointer address. The caller (RpcConnection) - // moves the sink in; we key by its move-captured lambda-object's address, - // which is unique per subscription. - static std::atomic counter{0}; - it->second.sinksByEvent[req.eventName][ - reinterpret_cast(counter.fetch_add(1) + 1)] = std::move(sink); + // Sinks are keyed by the originating connection so that + // onUnsubscribe / onConnectionClosed can remove only sinks + // belonging to that connection — sub/unsub frames don't carry a + // subscriber id on the wire. + it->second.sinksByEvent[req.eventName][connectionId] = std::move(sink); } -void PlainTransportHost::onUnsubscribe(const UnsubscribeMessage& req) +void PlainTransportHost::onUnsubscribe(const UnsubscribeMessage& req, + const void* connectionId) { std::lock_guard g(m_mu); auto it = m_published.find(req.object); if (it == m_published.end()) return; - it->second.sinksByEvent.erase(req.eventName); + auto evtIt = it->second.sinksByEvent.find(req.eventName); + if (evtIt == it->second.sinksByEvent.end()) return; + // Only drop the requesting connection's sink — other clients + // subscribed to the same (object, event) keep theirs. Previously + // this erased the entire eventName entry, taking every other + // subscriber down with it. + evtIt->second.erase(connectionId); + if (evtIt->second.empty()) it->second.sinksByEvent.erase(evtIt); +} + +void PlainTransportHost::onConnectionClosed(const void* connectionId) +{ + std::lock_guard g(m_mu); + // Sweep every published object's per-event sink table and drop any + // entries belonging to the closed connection. Without this, a + // crashing/disconnecting client (which never sends Unsubscribe) + // leaves dead sinks in the table forever. + for (auto& [_name, pub] : m_published) { + for (auto evtIt = pub.sinksByEvent.begin(); evtIt != pub.sinksByEvent.end(); ) { + evtIt->second.erase(connectionId); + if (evtIt->second.empty()) evtIt = pub.sinksByEvent.erase(evtIt); + else ++evtIt; + } + } } void PlainTransportHost::onToken(const TokenMessage& req) diff --git a/cpp/implementations/plain/plain_transport_host.h b/cpp/implementations/plain/plain_transport_host.h index 27c3e7e..ef69b98 100644 --- a/cpp/implementations/plain/plain_transport_host.h +++ b/cpp/implementations/plain/plain_transport_host.h @@ -49,8 +49,11 @@ public: // IncomingCallHandler void onCall(const CallMessage& req, CallReply reply) override; void onMethods(const MethodsMessage& req, MethodsReply reply) override; - void onSubscribe(const SubscribeMessage& req, EventSink sink) override; - void onUnsubscribe(const UnsubscribeMessage& req) override; + void onSubscribe(const SubscribeMessage& req, EventSink sink, + const void* connectionId) override; + void onUnsubscribe(const UnsubscribeMessage& req, + const void* connectionId) override; + void onConnectionClosed(const void* connectionId) override; void onToken(const TokenMessage& req) override; // Internal: deliver an event emitted by the wrapped QObject to every diff --git a/cpp/implementations/plain/qvariant_rpc_value.cpp b/cpp/implementations/plain/qvariant_rpc_value.cpp index 8761a6b..31730d0 100644 --- a/cpp/implementations/plain/qvariant_rpc_value.cpp +++ b/cpp/implementations/plain/qvariant_rpc_value.cpp @@ -4,6 +4,9 @@ #include +#include +#include + namespace logos::plain { namespace { diff --git a/cpp/implementations/plain/rpc_connection.h b/cpp/implementations/plain/rpc_connection.h index 0deb901..7613bc0 100644 --- a/cpp/implementations/plain/rpc_connection.h +++ b/cpp/implementations/plain/rpc_connection.h @@ -264,13 +264,19 @@ void RpcConnection::dispatchIncoming(AnyMessage msg) } else if constexpr (std::is_same_v) { if (!m_handler) return; - auto self = this->shared_from_this(); - m_handler->onSubscribe(m, [self](EventMessage evt) { - self->sendEvent(std::move(evt)); - }); + // weak_ptr capture so the host's stored sink doesn't keep the + // connection alive past its natural lifetime — without this, + // `[self]` would leak every subscribed connection until + // unsubscribe (which a crashing client never sends). + std::weak_ptr> weak = this->shared_from_this(); + const void* connId = static_cast(this); + m_handler->onSubscribe(m, [weak](EventMessage evt) { + if (auto self = weak.lock()) self->sendEvent(std::move(evt)); + }, connId); } else if constexpr (std::is_same_v) { - if (m_handler) m_handler->onUnsubscribe(m); + if (m_handler) + m_handler->onUnsubscribe(m, static_cast(this)); } else if constexpr (std::is_same_v) { if (m_handler) m_handler->onToken(m); @@ -420,6 +426,14 @@ void RpcConnection::fail(const std::string& reason) m_stream.lowest_layer().close(ignore); } catch (...) {} + // Notify the dispatch handler so it can drop any subscriptions still + // keyed to this connection. Without this, a connection that drops + // without sending Unsubscribe leaks sinks in the host's per-event map. + if (m_handler) { + try { m_handler->onConnectionClosed(static_cast(this)); } + catch (...) {} + } + if (errCb) errCb(reason); } diff --git a/cpp/implementations/plain/rpc_message.cpp b/cpp/implementations/plain/rpc_message.cpp index 7cdb44e..e4be921 100644 --- a/cpp/implementations/plain/rpc_message.cpp +++ b/cpp/implementations/plain/rpc_message.cpp @@ -1,5 +1,7 @@ #include "rpc_message.h" +#include + namespace logos::plain { MessageType messageTypeOf(const AnyMessage& m) diff --git a/cpp/logos_api.cpp b/cpp/logos_api.cpp index a9baa57..b36e3f2 100644 --- a/cpp/logos_api.cpp +++ b/cpp/logos_api.cpp @@ -43,18 +43,11 @@ LogosAPIProvider* LogosAPI::getProvider() const LogosAPIClient* LogosAPI::getClient(const QString& target_module) const { - // Check if we already have a client for this target module - if (m_clients.contains(target_module)) { - return m_clients.value(target_module); - } - - // Create a new client for this target module - LogosAPIClient* client = new LogosAPIClient(target_module, m_module_name, m_token_manager, const_cast(this)); - - // Cache it for future use - m_clients.insert(target_module, client); - - return client; + // The no-transport overload is just shorthand for "use the + // process-global default" — the explicit-transport overload below + // is the single resolution path. Mode-awareness lives in the + // factory, so this delegation preserves Mock/Local semantics. + return getClient(target_module, LogosTransportConfigGlobal::getDefault()); } LogosAPIClient* LogosAPI::getClient(const std::string& target_module) const @@ -65,21 +58,24 @@ LogosAPIClient* LogosAPI::getClient(const std::string& target_module) const LogosAPIClient* LogosAPI::getClient(const QString& target_module, const LogosTransportConfig& transport) const { - // Separate cache from the default-transport path. Caching by - // (target, protocol) keeps `getClient(x, tcp_ssl)` and - // `getClient(x, local)` from aliasing onto the same object, which - // would double-open connections / confuse reuse. - const QString key = target_module + "#" + - QString::number(static_cast(transport.protocol)) + ":" + - QString::fromStdString(transport.host) + ":" + - QString::number(transport.port); - if (m_clientsByTransport.contains(key)) - return m_clientsByTransport.value(key); + // Single cache, single construction path. Key composition mirrors + // the factory's resolution rule (see LogosAPIClientCacheKey in + // logos_api.h): + // - Mock/Local mode: every cfg collapses to one cache slot per + // target — switching cfg returns the same MockTransport-backed + // client instead of allocating a duplicate. + // - Remote mode: every distinguishing field of cfg matters, so + // two callers with different TLS/codec settings get separate + // clients (no risk of silently reusing an insecure transport). + const LogosAPIClientCacheKey key{ + target_module, LogosModeConfig::getMode(), transport}; + auto it = m_clients.constFind(key); + if (it != m_clients.constEnd()) return it.value(); LogosAPIClient* client = new LogosAPIClient( target_module, m_module_name, m_token_manager, transport, const_cast(this)); - m_clientsByTransport.insert(key, client); + m_clients.insert(key, client); return client; } diff --git a/cpp/logos_api.h b/cpp/logos_api.h index 78e2d40..3ee32fb 100644 --- a/cpp/logos_api.h +++ b/cpp/logos_api.h @@ -1,18 +1,84 @@ #ifndef LOGOS_API_H #define LOGOS_API_H +#include "logos_mode.h" #include "logos_transport_config.h" #include "logos_types.h" +#include +#include #include #include -#include + +#include #include class LogosAPIClient; class LogosAPIProvider; class TokenManager; +// qHash for LogosTransportConfig — combined with operator== from +// logos_transport_config.h, this lets QHash use it as a key. Lives here +// rather than in logos_transport_config.h so that header stays Qt-free +// (the SDK is being de-Qt'd; only Qt-using consumers like the cache +// here pull in the QHash adapter). +// +// Every field that distinguishes one explicit-transport client from +// another contributes to the hash; otherwise two callers with different +// TLS or codec settings could land on the same bucket and cache-alias +// onto a single client. +inline size_t qHash(const LogosTransportConfig& cfg, size_t seed = 0) noexcept +{ + return qHashMulti(seed, + static_cast(cfg.protocol), + std::hash{}(cfg.host), + cfg.port, + std::hash{}(cfg.caFile), + std::hash{}(cfg.certFile), + std::hash{}(cfg.keyFile), + cfg.verifyPeer, + static_cast(cfg.codec)); +} + +// LogosAPIClient cache key. Mirrors the factory's transport-resolution +// rule so two callers that would observe the same connection share a +// cached client: +// +// - Mock / Local mode → transport is ignored at construction; key +// ignores it too. Switching mode changes the +// key (so cached clients don't bleed across +// mode switches in tests). +// - Remote mode → cfg picks the wire endpoint; key includes +// the full LogosTransportConfig. +// +// Without the mode-aware comparison, calling +// `getClient(x, tcp)` and `getClient(x, tcp_ssl)` in Mock mode would +// allocate two clients pointing at functionally identical +// MockTransportConnections. +struct LogosAPIClientCacheKey { + QString target; + LogosMode mode; + LogosTransportConfig transport; // only compared when mode == Remote +}; + +inline bool operator==(const LogosAPIClientCacheKey& a, + const LogosAPIClientCacheKey& b) noexcept +{ + if (a.target != b.target) return false; + if (a.mode != b.mode) return false; + return a.mode == LogosMode::Remote ? a.transport == b.transport : true; +} + +inline size_t qHash(const LogosAPIClientCacheKey& k, size_t seed = 0) noexcept +{ + if (k.mode == LogosMode::Remote) { + return qHashMulti(seed, k.target, static_cast(k.mode), k.transport); + } + // Mock / Local: transport is irrelevant — leave it out of the hash + // so it can't bias which bucket the key lands in. + return qHashMulti(seed, k.target, static_cast(k.mode)); +} + /** * @brief LogosAPI provides a unified interface to the Logos SDK * @@ -93,9 +159,14 @@ public: * that would also flip the same process's `LogosAPIProvider` into * trying to bind a tcp_ssl server, which the CLI has no cert for. * - * Cached per (target_module, transport) pair so repeat calls with - * the same config return the same client; a request with a - * different transport to the same target creates a separate client. + * Cached per (target_module, full LogosTransportConfig) — repeat + * calls with the same target *and* the same transport return the + * same client. The cache key covers every config field that can + * distinguish two clients (protocol, host, port, codec, all TLS + * settings), via the operator== / qHash defined alongside + * LogosTransportConfig, so two callers with different TLS or codec + * settings always get separate clients — no risk of silently + * reusing an insecure connection where a secure one was asked for. */ LogosAPIClient* getClient(const QString& target_module, const LogosTransportConfig& transport) const; @@ -116,8 +187,14 @@ public: private: QString m_module_name; LogosAPIProvider* m_provider; - mutable QHash m_clients; // Cache of default-transport clients per target module - mutable QHash m_clientsByTransport; // Cache for explicit-transport clients, keyed by target+transport + // Single cache for both getClient overloads. Keyed by a + // mode-aware composite (LogosAPIClientCacheKey above) so that: + // - Mock/Local mode buckets ignore transport (the factory does too) + // - Remote mode keys include the full LogosTransportConfig + // - the no-transport overload resolves to the same key as an + // explicit caller passing LogosTransportConfigGlobal::getDefault() + // - mode switches don't return stale clients from the previous mode + mutable QHash m_clients; TokenManager* m_token_manager; }; diff --git a/cpp/logos_api_client.cpp b/cpp/logos_api_client.cpp index 1641d7a..500b056 100644 --- a/cpp/logos_api_client.cpp +++ b/cpp/logos_api_client.cpp @@ -5,9 +5,14 @@ #include #include -LogosAPIClient::LogosAPIClient(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, QObject *parent) +LogosAPIClient::LogosAPIClient(const QString& module_to_talk_to, + const QString& origin_module, + TokenManager* token_manager, + const LogosTransportConfig& transport, + QObject *parent) : QObject(parent) - , m_consumer(new LogosAPIConsumer(module_to_talk_to, origin_module, token_manager, this)) + , m_consumer(new LogosAPIConsumer(module_to_talk_to, origin_module, + token_manager, transport, this)) , m_token_manager(token_manager) , m_origin_module(origin_module) { @@ -16,12 +21,9 @@ LogosAPIClient::LogosAPIClient(const QString& module_to_talk_to, const QString& LogosAPIClient::LogosAPIClient(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, - const LogosTransportConfig& transport, QObject *parent) - : QObject(parent) - , m_consumer(new LogosAPIConsumer(module_to_talk_to, origin_module, token_manager, transport, this)) - , m_token_manager(token_manager) - , m_origin_module(origin_module) + : LogosAPIClient(module_to_talk_to, origin_module, token_manager, + LogosTransportConfigGlobal::getDefault(), parent) { } diff --git a/cpp/logos_api_client.h b/cpp/logos_api_client.h index fc192f8..4ec9406 100644 --- a/cpp/logos_api_client.h +++ b/cpp/logos_api_client.h @@ -27,19 +27,30 @@ class LogosAPIClient : public QObject Q_OBJECT public: - explicit LogosAPIClient(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, QObject *parent = nullptr); /** - * Explicit-transport overload. The client (and the consumer it - * owns) will use `transport` for its connection, bypassing - * `LogosTransportConfigGlobal::getDefault`. See - * `LogosAPIConsumer`'s explicit-transport constructor for the - * full rationale. + * @brief Construct a client (with its underlying consumer) using + * `transport`, honoring the process-wide LogosMode. + * + * Transport resolution is delegated to LogosAPIConsumer, which in + * turn goes through the single LogosTransportFactory rule combining + * LogosMode + LogosTransportConfig. See LogosAPIConsumer's explicit + * constructor doc-comment for the resolution table. */ LogosAPIClient(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, const LogosTransportConfig& transport, QObject *parent = nullptr); + + /** + * @brief Convenience constructor that uses the process-global default + * LogosTransportConfig. Equivalent to the explicit constructor above + * with `LogosTransportConfigGlobal::getDefault()`. + */ + explicit LogosAPIClient(const QString& module_to_talk_to, + const QString& origin_module, + TokenManager* token_manager, + QObject *parent = nullptr); ~LogosAPIClient(); /** diff --git a/cpp/logos_api_consumer.cpp b/cpp/logos_api_consumer.cpp index 49560e6..19d3a3c 100644 --- a/cpp/logos_api_consumer.cpp +++ b/cpp/logos_api_consumer.cpp @@ -14,15 +14,6 @@ #include #include -LogosAPIConsumer::LogosAPIConsumer(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, QObject *parent) - : QObject(parent) - , m_registryUrl(LogosInstance::id(module_to_talk_to)) - , m_token_manager(token_manager) -{ - m_transport = LogosTransportFactory::createConnection(m_registryUrl); - m_transport->connectToHost(); -} - LogosAPIConsumer::LogosAPIConsumer(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, @@ -32,14 +23,24 @@ LogosAPIConsumer::LogosAPIConsumer(const QString& module_to_talk_to, , m_registryUrl(LogosInstance::id(module_to_talk_to)) , m_token_manager(token_manager) { - // Explicit-config path. Bypasses `LogosTransportConfigGlobal::getDefault` - // so the caller's choice of transport for THIS consumer doesn't bleed - // into the rest of the process (in particular, any `LogosAPIProvider` - // in the same LogosAPI still creates its host from the global default). + // Single transport-resolution path: the factory combines LogosMode + // + LogosTransportConfig (mode wins for Mock/Local; transport + // chooses the wire protocol in Remote mode). The choice scopes to + // this consumer only — any LogosAPIProvider in the same LogosAPI + // still constructs its host from the global default. m_transport = LogosTransportFactory::createConnection(transport, m_registryUrl); m_transport->connectToHost(); } +LogosAPIConsumer::LogosAPIConsumer(const QString& module_to_talk_to, + const QString& origin_module, + TokenManager* token_manager, + QObject *parent) + : LogosAPIConsumer(module_to_talk_to, origin_module, token_manager, + LogosTransportConfigGlobal::getDefault(), parent) +{ +} + LogosAPIConsumer::~LogosAPIConsumer() { } diff --git a/cpp/logos_api_consumer.h b/cpp/logos_api_consumer.h index 1f6761e..0409313 100644 --- a/cpp/logos_api_consumer.h +++ b/cpp/logos_api_consumer.h @@ -31,21 +31,38 @@ class LogosAPIConsumer : public QObject Q_OBJECT public: - explicit LogosAPIConsumer(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, QObject *parent = nullptr); /** - * Explicit-transport overload. Use this when the caller needs a - * specific transport for this consumer and does *not* want the - * process-global default (which also drives provider bind-URLs). - * Typical case: a pure client that only reaches one remote module - * over a particular protocol — e.g. the logoscore CLI dialing - * `core_service` over tcp_ssl without side-effecting the client's - * own LogosAPI provider into also trying to bind TLS. + * @brief Construct a consumer connected via `transport`, honoring the + * process-wide LogosMode. + * + * Transport resolution is done in one place — LogosTransportFactory — + * by combining LogosMode + the supplied LogosTransportConfig: + * - LogosMode::Mock → MockTransportConnection (transport ignored) + * - LogosMode::Local → LocalTransportConnection (transport ignored) + * - LogosMode::Remote → wire protocol picked by `transport.protocol` + * + * Use this overload when the caller wants a specific transport for + * this consumer without side-effecting the rest of the process + * (e.g. the logoscore CLI dialing `core_service` over tcp_ssl + * without also flipping the in-process LogosAPIProvider into + * binding TLS). */ LogosAPIConsumer(const QString& module_to_talk_to, const QString& origin_module, TokenManager* token_manager, const LogosTransportConfig& transport, QObject *parent = nullptr); + + /** + * @brief Convenience constructor that uses the process-global default + * LogosTransportConfig. Equivalent to passing + * `LogosTransportConfigGlobal::getDefault()` to the explicit-transport + * constructor above. + */ + explicit LogosAPIConsumer(const QString& module_to_talk_to, + const QString& origin_module, + TokenManager* token_manager, + QObject *parent = nullptr); ~LogosAPIConsumer(); /** diff --git a/cpp/logos_transport_config.h b/cpp/logos_transport_config.h index 001692b..95bde91 100644 --- a/cpp/logos_transport_config.h +++ b/cpp/logos_transport_config.h @@ -3,6 +3,7 @@ #include #include +#include #include // ----------------------------------------------------------------------------- @@ -50,6 +51,31 @@ struct LogosTransportConfig { LogosWireCodec codec = LogosWireCodec::Json; }; +// Field-wise equality. Used as the equality predicate for hashed +// containers keyed by LogosTransportConfig (e.g. the explicit-transport +// LogosAPIClient cache in logos_api.h). Every field that can plausibly +// distinguish one transport-attached client from another belongs here — +// missing one would let two callers with different security or codec +// settings alias onto the same cached client. +inline bool operator==(const LogosTransportConfig& a, + const LogosTransportConfig& b) noexcept +{ + return a.protocol == b.protocol + && a.port == b.port + && a.verifyPeer == b.verifyPeer + && a.codec == b.codec + && a.host == b.host + && a.caFile == b.caFile + && a.certFile == b.certFile + && a.keyFile == b.keyFile; +} + +inline bool operator!=(const LogosTransportConfig& a, + const LogosTransportConfig& b) noexcept +{ + return !(a == b); +} + using LogosTransportSet = std::vector; // ----------------------------------------------------------------------------- diff --git a/cpp/logos_transport_factory.cpp b/cpp/logos_transport_factory.cpp index b12f709..dbf0709 100644 --- a/cpp/logos_transport_factory.cpp +++ b/cpp/logos_transport_factory.cpp @@ -8,45 +8,23 @@ #include "implementations/plain/plain_transport_connection.h" #include "implementations/plain/plain_transport_host.h" +#include + namespace LogosTransportFactory { -namespace { - -// Pick the remote-transport backend based on the process-global -// LogosTransportConfig. A future iteration (full multi-transport publish) -// threads a config per LogosAPI instance through createHost/createConnection. -std::unique_ptr createRemoteHost(const QString& registryUrl) -{ - const auto& cfg = LogosTransportConfigGlobal::getDefault(); - switch (cfg.protocol) { - case LogosProtocol::Tcp: - case LogosProtocol::TcpSsl: { - auto host = std::make_unique(cfg); - host->start(); - return host; - } - case LogosProtocol::LocalSocket: - default: - return std::make_unique(registryUrl); - } -} - -std::unique_ptr createRemoteConnection(const QString& registryUrl) -{ - const auto& cfg = LogosTransportConfigGlobal::getDefault(); - switch (cfg.protocol) { - case LogosProtocol::Tcp: - case LogosProtocol::TcpSsl: - return std::make_unique(cfg); - case LogosProtocol::LocalSocket: - default: - return std::make_unique(registryUrl); - } -} - -} // anonymous namespace - -std::unique_ptr createHost(const QString& registryUrl) +// Single resolution rule for both `createHost` overloads: +// LogosMode::Mock → MockTransportHost (cfg ignored) +// LogosMode::Local → LocalTransportHost (cfg ignored) +// LogosMode::Remote + LocalSocket → RemoteTransportHost (QRO) +// LogosMode::Remote + Tcp/TcpSsl → PlainTransportHost(cfg) +// +// Mode is consulted *first* so test fixtures setting Mock/Local always +// get the right transport regardless of which createHost overload (or +// LogosAPIProvider constructor) was used. The no-cfg overload below +// just delegates with `LogosTransportConfigGlobal::getDefault()` so +// there's exactly one path. +std::unique_ptr +createHost(const LogosTransportConfig& cfg, const QString& registryUrl) { if (LogosModeConfig::isLocal()) { return std::make_unique(); @@ -54,17 +32,14 @@ std::unique_ptr createHost(const QString& registryUrl) if (LogosModeConfig::isMock()) { return std::make_unique(); } - return createRemoteHost(registryUrl); -} - -std::unique_ptr -createHost(const LogosTransportConfig& cfg, const QString& registryUrl) -{ switch (cfg.protocol) { case LogosProtocol::Tcp: case LogosProtocol::TcpSsl: { auto host = std::make_unique(cfg); - host->start(); + if (!host->start()) { + qCritical() << "LogosTransportFactory: PlainTransportHost::start() failed"; + return nullptr; + } return host; } case LogosProtocol::LocalSocket: @@ -73,7 +48,14 @@ createHost(const LogosTransportConfig& cfg, const QString& registryUrl) } } -std::unique_ptr createConnection(const QString& registryUrl) +std::unique_ptr createHost(const QString& registryUrl) +{ + return createHost(LogosTransportConfigGlobal::getDefault(), registryUrl); +} + +// Same resolution rule as createHost — see the comment block above. +std::unique_ptr +createConnection(const LogosTransportConfig& cfg, const QString& registryUrl) { if (LogosModeConfig::isLocal()) { return std::make_unique(); @@ -81,12 +63,6 @@ std::unique_ptr createConnection(const QString& regist if (LogosModeConfig::isMock()) { return std::make_unique(); } - return createRemoteConnection(registryUrl); -} - -std::unique_ptr -createConnection(const LogosTransportConfig& cfg, const QString& registryUrl) -{ switch (cfg.protocol) { case LogosProtocol::Tcp: case LogosProtocol::TcpSsl: @@ -97,4 +73,9 @@ createConnection(const LogosTransportConfig& cfg, const QString& registryUrl) } } +std::unique_ptr createConnection(const QString& registryUrl) +{ + return createConnection(LogosTransportConfigGlobal::getDefault(), registryUrl); +} + } diff --git a/cpp/logos_transport_factory.h b/cpp/logos_transport_factory.h index 7ec45fd..3069580 100644 --- a/cpp/logos_transport_factory.h +++ b/cpp/logos_transport_factory.h @@ -12,33 +12,48 @@ class LogosTransportConnection; namespace LogosTransportFactory { /** - * @brief Create the appropriate transport host for the current mode - * @param registryUrl The URL used by the remote transport (ignored in local mode) - * @return Owning pointer to the transport host - */ - std::unique_ptr createHost(const QString& registryUrl); - - /** - * @brief Create a transport host from an explicit config (bypasses the - * process-global default). Used by LogosAPIProvider when a per-instance - * transport override is supplied (e.g. daemon publishing core_service - * on TCP while modules stay on local sockets). + * @brief Create a transport host for `cfg`, honoring the process-wide + * LogosMode. + * + * Resolution rule: + * - LogosMode::Mock → MockTransportHost (cfg ignored) + * - LogosMode::Local → LocalTransportHost (cfg ignored) + * - LogosMode::Remote + LocalSocket → RemoteTransportHost (QRO) + * - LogosMode::Remote + Tcp/TcpSsl → PlainTransportHost(cfg) + * + * Mode wins over `cfg.protocol` so test fixtures that switch the + * process into Mock/Local always get the test transport, regardless + * of which overload (or which LogosAPIProvider constructor) was + * used. In Remote mode, `cfg` chooses the wire protocol and + * carries the bind/dial address + TLS material. */ std::unique_ptr createHost(const LogosTransportConfig& cfg, const QString& registryUrl); /** - * @brief Create the appropriate transport connection for the current mode - * @param registryUrl The URL to connect to (ignored in local mode) - * @return Owning pointer to the transport connection + * @brief Convenience: createHost using the process-global default + * LogosTransportConfig. Equivalent to + * `createHost(LogosTransportConfigGlobal::getDefault(), registryUrl)`. */ - std::unique_ptr createConnection(const QString& registryUrl); + std::unique_ptr createHost(const QString& registryUrl); + /** + * @brief Create a transport connection for `cfg`, honoring the + * process-wide LogosMode. Same resolution rule as createHost — see + * its doc-comment for the full table. + */ std::unique_ptr createConnection(const LogosTransportConfig& cfg, const QString& registryUrl); + /** + * @brief Convenience: createConnection using the process-global default + * LogosTransportConfig. Equivalent to + * `createConnection(LogosTransportConfigGlobal::getDefault(), registryUrl)`. + */ + std::unique_ptr createConnection(const QString& registryUrl); + } #endif // LOGOS_TRANSPORT_FACTORY_H diff --git a/tests/sdk/CMakeLists.txt b/tests/sdk/CMakeLists.txt index 5320477..01a2766 100644 --- a/tests/sdk/CMakeLists.txt +++ b/tests/sdk/CMakeLists.txt @@ -43,7 +43,12 @@ add_executable(sdk_tests test_rpc_framing.cpp test_json_codec.cpp test_cbor_codec.cpp - test_plain_transport_tcp.cpp + # test_plain_transport_tcp.cpp — entirely #if 0 in this iteration + # (in-process Qt-event-loop deadlock between consumer + provider + # under nix's test sandbox). The plain transport is exercised + # cross-process by the logos-logoscore-py integration matrix + # instead. Re-add once the in-process scaffold runs host + consumer + # in separate QCoreApplication instances / processes. fixtures/sample_provider.cpp ${GENERATED_DISPATCH} )