From 091f6e520694360a0407ab0cf3bb036fb461e6e3 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Thu, 2 Aug 2018 11:00:25 +0200 Subject: Have StreamStack own the top and bottom layer Test-Information: Builds, unit tests and integration tests pass on macOS with clang 7.0 master. Change-Id: I0db411e49339ccb2301edd1a16612cb1ad2c927c diff --git a/Swiften/Session/BasicSessionStream.cpp b/Swiften/Session/BasicSessionStream.cpp index 3e65640..c44961d 100644 --- a/Swiften/Session/BasicSessionStream.cpp +++ b/Swiften/Session/BasicSessionStream.cpp @@ -35,7 +35,7 @@ BasicSessionStream::BasicSessionStream( tlsContextFactory(tlsContextFactory), timerFactory(timerFactory), tlsOptions_(tlsOptions) { - xmppLayer = new XMPPLayer(payloadParserFactories, payloadSerializers, xmlParserFactory, streamType); + auto xmppLayer = std::make_unique(payloadParserFactories, payloadSerializers, xmlParserFactory, streamType); xmppLayer->onStreamStart.connect(boost::bind(&BasicSessionStream::handleStreamStartReceived, this, _1)); xmppLayer->onStreamEnd.connect(boost::bind(&BasicSessionStream::handleStreamEndReceived, this)); xmppLayer->onElement.connect(boost::bind(&BasicSessionStream::handleElementReceived, this, _1)); @@ -44,10 +44,8 @@ BasicSessionStream::BasicSessionStream( xmppLayer->onWriteData.connect(boost::bind(&BasicSessionStream::handleDataWritten, this, _1)); connection->onDisconnected.connect(boost::bind(&BasicSessionStream::handleConnectionFinished, this, _1)); - connectionLayer = new ConnectionLayer(connection); - - streamStack = new StreamStack(xmppLayer, connectionLayer); + streamStack = std::make_unique(std::move(xmppLayer), std::unique_ptr(new ConnectionLayer(connection))); available = true; } @@ -57,37 +55,39 @@ BasicSessionStream::~BasicSessionStream() { tlsLayer->onError.disconnect(boost::bind(&BasicSessionStream::handleTLSError, this, _1)); tlsLayer->onConnected.disconnect(boost::bind(&BasicSessionStream::handleTLSConnected, this)); } - delete streamStack; connection->onDisconnected.disconnect(boost::bind(&BasicSessionStream::handleConnectionFinished, this, _1)); - delete connectionLayer; + auto xmppLayer = streamStack->getLayer(); xmppLayer->onStreamStart.disconnect(boost::bind(&BasicSessionStream::handleStreamStartReceived, this, _1)); xmppLayer->onStreamEnd.disconnect(boost::bind(&BasicSessionStream::handleStreamEndReceived, this)); xmppLayer->onElement.disconnect(boost::bind(&BasicSessionStream::handleElementReceived, this, _1)); xmppLayer->onError.disconnect(boost::bind(&BasicSessionStream::handleXMPPError, this)); xmppLayer->onDataRead.disconnect(boost::bind(&BasicSessionStream::handleDataRead, this, _1)); xmppLayer->onWriteData.disconnect(boost::bind(&BasicSessionStream::handleDataWritten, this, _1)); - delete xmppLayer; } void BasicSessionStream::writeHeader(const ProtocolHeader& header) { assert(available); + auto* xmppLayer = streamStack->getLayer(); xmppLayer->writeHeader(header); } void BasicSessionStream::writeElement(std::shared_ptr element) { assert(available); + auto* xmppLayer = streamStack->getLayer(); xmppLayer->writeElement(element); } void BasicSessionStream::writeFooter() { assert(available); + auto* xmppLayer = streamStack->getLayer(); xmppLayer->writeFooter(); } void BasicSessionStream::writeData(const std::string& data) { assert(available); + auto* xmppLayer = streamStack->getLayer(); xmppLayer->writeData(data); } @@ -162,6 +162,7 @@ void BasicSessionStream::setWhitespacePingEnabled(bool enabled) { } void BasicSessionStream::resetXMPPParser() { + auto* xmppLayer = streamStack->getLayer(); xmppLayer->resetParser(); } diff --git a/Swiften/Session/BasicSessionStream.h b/Swiften/Session/BasicSessionStream.h index 472b5cc..30a7e3b 100644 --- a/Swiften/Session/BasicSessionStream.h +++ b/Swiften/Session/BasicSessionStream.h @@ -83,9 +83,7 @@ namespace Swift { std::shared_ptr connection; TLSContextFactory* tlsContextFactory; TimerFactory* timerFactory; - XMPPLayer* xmppLayer; - ConnectionLayer* connectionLayer; - StreamStack* streamStack; + std::unique_ptr streamStack; TLSOptions tlsOptions_; }; diff --git a/Swiften/Session/Session.cpp b/Swiften/Session/Session.cpp index ebdb5d1..b1525b8 100644 --- a/Swiften/Session/Session.cpp +++ b/Swiften/Session/Session.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -22,16 +22,10 @@ Session::Session( payloadParserFactories(payloadParserFactories), payloadSerializers(payloadSerializers), xmlParserFactory(xmlParserFactory), - xmppLayer(nullptr), - connectionLayer(nullptr), - streamStack(nullptr), finishing(false) { } Session::~Session() { - delete streamStack; - delete connectionLayer; - delete xmppLayer; } void Session::startSession() { @@ -44,7 +38,7 @@ void Session::finishSession() { return; } finishing = true; - if (xmppLayer) { + if (auto xmppLayer = getXMPPLayer()) { xmppLayer->writeFooter(); } connection->disconnect(); @@ -55,14 +49,14 @@ void Session::finishSession(const SessionError& /*error*/) { return; } finishing = true; - if (xmppLayer) { + if (auto xmppLayer = getXMPPLayer()) { xmppLayer->writeFooter(); } connection->disconnect(); } void Session::initializeStreamStack() { - xmppLayer = new XMPPLayer(payloadParserFactories, payloadSerializers, xmlParserFactory, ClientStreamType); + auto xmppLayer = std::unique_ptr(new XMPPLayer(payloadParserFactories, payloadSerializers, xmlParserFactory, ClientStreamType)); xmppLayer->onStreamStart.connect( boost::bind(&Session::handleStreamStart, this, _1)); xmppLayer->onElement.connect(boost::bind(&Session::handleElement, this, _1)); @@ -72,12 +66,20 @@ void Session::initializeStreamStack() { xmppLayer->onWriteData.connect(boost::bind(boost::ref(onDataWritten), _1)); connection->onDisconnected.connect( boost::bind(&Session::handleDisconnected, this, _1)); - connectionLayer = new ConnectionLayer(connection); - streamStack = new StreamStack(xmppLayer, connectionLayer); + streamStack = std::unique_ptr(new StreamStack(std::move(xmppLayer), std::unique_ptr(new ConnectionLayer(connection)))); } +XMPPLayer* Session::getXMPPLayer() const { + return dynamic_cast(streamStack->getTopLayer()); +} + +StreamStack* Session::getStreamStack() const { + return streamStack.get(); +} + + void Session::sendElement(std::shared_ptr stanza) { - xmppLayer->writeElement(stanza); + getXMPPLayer()->writeElement(stanza); } void Session::handleDisconnected(const boost::optional& connectionError) { diff --git a/Swiften/Session/Session.h b/Swiften/Session/Session.h index 04153ec..e6a0d53 100644 --- a/Swiften/Session/Session.h +++ b/Swiften/Session/Session.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2017 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -85,13 +85,8 @@ namespace Swift { void initializeStreamStack(); - XMPPLayer* getXMPPLayer() const { - return xmppLayer; - } - - StreamStack* getStreamStack() const { - return streamStack; - } + XMPPLayer* getXMPPLayer() const; + StreamStack* getStreamStack() const; void setFinished(); @@ -105,9 +100,8 @@ namespace Swift { PayloadParserFactoryCollection* payloadParserFactories; PayloadSerializerCollection* payloadSerializers; XMLParserFactory* xmlParserFactory; - XMPPLayer* xmppLayer; - ConnectionLayer* connectionLayer; - StreamStack* streamStack; + + std::unique_ptr streamStack; bool finishing; }; } diff --git a/Swiften/StreamStack/StreamStack.cpp b/Swiften/StreamStack/StreamStack.cpp index 75fb1ce..cf80fb1 100644 --- a/Swiften/StreamStack/StreamStack.cpp +++ b/Swiften/StreamStack/StreamStack.cpp @@ -8,25 +8,25 @@ #include +#include #include #include -#include namespace Swift { -StreamStack::StreamStack(HighLayer* topLayer, LowLayer* bottomLayer) : topLayer_(topLayer), bottomLayer_(bottomLayer) { - bottomLayer_->setParentLayer(topLayer_); - topLayer_->setChildLayer(bottomLayer_); +StreamStack::StreamStack(std::unique_ptr topLayer, std::unique_ptr bottomLayer) : topLayer_(std::move(topLayer)), bottomLayer_(std::move(bottomLayer)) { + bottomLayer_->setParentLayer(topLayer_.get()); + topLayer_->setChildLayer(bottomLayer_.get()); } StreamStack::~StreamStack() { } void StreamStack::addLayer(std::unique_ptr streamLayer) { - LowLayer* lowLayer = layers_.empty() ? bottomLayer_ : layers_.rbegin()->get(); + auto* lowLayer = layers_.empty() ? bottomLayer_.get() : layers_.rbegin()->get(); topLayer_->setChildLayer(streamLayer.get()); - streamLayer->setParentLayer(topLayer_); + streamLayer->setParentLayer(topLayer_.get()); lowLayer->setParentLayer(streamLayer.get()); streamLayer->setChildLayer(lowLayer); diff --git a/Swiften/StreamStack/StreamStack.h b/Swiften/StreamStack/StreamStack.h index bd95811..263b1f5 100644 --- a/Swiften/StreamStack/StreamStack.h +++ b/Swiften/StreamStack/StreamStack.h @@ -21,13 +21,13 @@ namespace Swift { class SWIFTEN_API StreamStack { public: - StreamStack(HighLayer* topLayer, LowLayer* bottomLayer); + StreamStack(std::unique_ptr topLayer, std::unique_ptr bottomLayer); ~StreamStack(); void addLayer(std::unique_ptr /* streamLayer */); HighLayer* getTopLayer() const { - return topLayer_; + return topLayer_.get(); } template T* getLayer() const { @@ -37,12 +37,18 @@ namespace Swift { return layer; } } + if (T* layer = dynamic_cast(topLayer_.get())) { + return layer; + } + if (T* layer = dynamic_cast(bottomLayer_.get())) { + return layer; + } return nullptr; } private: - HighLayer* topLayer_; - LowLayer* bottomLayer_; + std::unique_ptr topLayer_; + std::unique_ptr bottomLayer_; std::vector> layers_; }; } diff --git a/Swiften/StreamStack/UnitTest/StreamStackTest.cpp b/Swiften/StreamStack/UnitTest/StreamStackTest.cpp index 0b520f1..b074736 100644 --- a/Swiften/StreamStack/UnitTest/StreamStackTest.cpp +++ b/Swiften/StreamStack/UnitTest/StreamStackTest.cpp @@ -39,19 +39,17 @@ class StreamStackTest : public CppUnit::TestFixture { public: void setUp() { - physicalStream_ = new TestLowLayer(); - xmppStream_ = new XMPPLayer(&parserFactories_, &serializers_, &xmlParserFactory_, ClientStreamType); + testling_ = std::make_unique(std::make_unique(&parserFactories_, &serializers_, &xmlParserFactory_, ClientStreamType), std::make_unique()); + physicalStream_ = testling_->getLayer(); + xmppStream_ = testling_->getLayer(); elementsReceived_ = 0; dataWriteReceived_ = 0; } void tearDown() { - delete physicalStream_; - delete xmppStream_; } void testWriteData_NoIntermediateStreamStack() { - StreamStack testling(xmppStream_, physicalStream_); xmppStream_->writeData("foo"); @@ -60,9 +58,8 @@ class StreamStackTest : public CppUnit::TestFixture { } void testWriteData_OneIntermediateStream() { - StreamStack testling(xmppStream_, physicalStream_); std::unique_ptr xStream(new MyStreamLayer("X")); - testling.addLayer(std::move(xStream)); + testling_->addLayer(std::move(xStream)); xmppStream_->writeData("foo"); @@ -71,11 +68,10 @@ class StreamStackTest : public CppUnit::TestFixture { } void testWriteData_TwoIntermediateStreamStack() { - StreamStack testling(xmppStream_, physicalStream_); std::unique_ptr xStream(new MyStreamLayer("X")); std::unique_ptr yStream(new MyStreamLayer("Y")); - testling.addLayer(std::move(xStream)); - testling.addLayer(std::move(yStream)); + testling_->addLayer(std::move(xStream)); + testling_->addLayer(std::move(yStream)); xmppStream_->writeData("foo"); @@ -84,7 +80,6 @@ class StreamStackTest : public CppUnit::TestFixture { } void testReadData_NoIntermediateStreamStack() { - StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1)); physicalStream_->onDataRead(createSafeByteArray("")); @@ -93,10 +88,9 @@ class StreamStackTest : public CppUnit::TestFixture { } void testReadData_OneIntermediateStream() { - StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1)); std::unique_ptr xStream(new MyStreamLayer("<")); - testling.addLayer(std::move(xStream)); + testling_->addLayer(std::move(xStream)); physicalStream_->onDataRead(createSafeByteArray("stream:stream xmlns:stream='http://etherx.jabber.org/streams'>")); @@ -104,12 +98,11 @@ class StreamStackTest : public CppUnit::TestFixture { } void testReadData_TwoIntermediateStreamStack() { - StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1)); std::unique_ptr xStream(new MyStreamLayer("s")); std::unique_ptr yStream(new MyStreamLayer("<")); - testling.addLayer(std::move(xStream)); - testling.addLayer(std::move(yStream)); + testling_->addLayer(std::move(xStream)); + testling_->addLayer(std::move(yStream)); physicalStream_->onDataRead(createSafeByteArray("tream:stream xmlns:stream='http://etherx.jabber.org/streams'>")); @@ -117,10 +110,9 @@ class StreamStackTest : public CppUnit::TestFixture { } void testAddLayer_ExistingOnWriteDataSlot() { - StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onWriteData.connect(boost::bind(&StreamStackTest::handleWriteData, this, _1)); std::unique_ptr xStream(new MyStreamLayer("X")); - testling.addLayer(std::move(xStream)); + testling_->addLayer(std::move(xStream)); xmppStream_->writeData("foo"); @@ -176,6 +168,7 @@ class StreamStackTest : public CppUnit::TestFixture { TestLowLayer* physicalStream_; PlatformXMLParserFactory xmlParserFactory_; XMPPLayer* xmppStream_; + std::unique_ptr testling_; int elementsReceived_; int dataWriteReceived_; }; -- cgit v0.10.2-6-g49f6