NetServices: use BBorrow<BDataIO> for custom body targets

Change-Id: Ib2d4b0ca3689338d906f943295278c086c6f2c83

docs/user/netservices/HttpResult.dox

@@ -114,9 +114,8 @@ namespace Network {
 	\brief Represents a HTTP response body.
 	
 	The HTTP response body is captured in this object. The body is either stored into a
-	\ref target, or into a \a text variable, depending on how you called the
+	target, or into a \ref text variable, depending on how you called the
-	\ref BHttpSession::Execute() method. If there is a \a target, the body will be empty,
+	\ref BHttpSession::Execute() method.
-	and vice versa.
 
 	You will usually get a reference to this object through the \ref BHttpResult::Body() method.
 	If you want to keep the contents of the body beyond the lifetime of the BHttpResult object,
@@ -127,17 +126,13 @@ namespace Network {
 
 
 /*!
-	\var std::unique_ptr<BDataIO> BHttpBody::target
+	\var std::optional<BString> BHttpBody::text
-	\brief An owned pointer to where the body has been written.
-
-	\since Haiku R1
-*/
-
-
-/*!
-	\var BString BHttpBody::text
 	\brief A string containing the body of the HTTP request.
 
+	The value of this class variable is set to \c std::nullopt if the target body was written to
+	a specified target. Otherwise, the response body is stored in this string. If the response
+	body was empty, then this will be an empty string.
+
 	\since Haiku R1
 */
 

docs/user/netservices/HttpSession.dox

@@ -165,14 +165,16 @@ namespace Network {
 
 /*!
 	\fn BHttpResult BHttpSession::Execute(BHttpRequest &&request,
-		std::unique_ptr< BDataIO > target=nullptr, BMessenger observer=BMessenger())
+		BBorrow< BDataIO > target=nullptr, BMessenger observer=BMessenger())
 	\brief Schedule and execute a \a request.
 
 	\param request The (valid) request to move from.
 	\param target An optional data buffer to write the incoming body of the request to. This can be
 		\c nullptr if you want to use the default internal storage. If you provide a buffer, it
-		must be wrapped in a \c std::unique_ptr. This means that you transfer ownership to the
+		must be wrapped in a \ref BBorrow object. This means that you exclusively borrow the
-		session. After the request is finished, you can regain ownership.
+		target to this session object. After the request is finished, you can regain usage of the
+		object through the matching \ref BExclusiveBorrow object. Use the \ref BHttpResult::Body()
+		method to synchronize when the target is available again.
 	\param observer An optional observer that will receive the progress and status messages for
 		this request.
 

headers/private/netservices2/HttpResult.h

@@ -23,8 +23,7 @@ struct HttpResultPrivate;
 
 struct BHttpBody
 {
-	std::unique_ptr<BDataIO>			target = nullptr;
+	std::optional<BString>	text;
-	BString								text;
 };
 
 

headers/private/netservices2/HttpSession.h

@@ -8,6 +8,7 @@
 
 #include <memory>
 
+#include <ExclusiveBorrow.h>
 #include <Messenger.h>
 
 class BUrl;
@@ -35,7 +36,7 @@ public:
 
 	// Requests
 	BHttpResult				Execute(BHttpRequest&& request,
-								std::unique_ptr<BDataIO> target = nullptr,
+								BBorrow<BDataIO> target = nullptr,
 								BMessenger observer = BMessenger());
 	void					Cancel(int32 identifier);
 	void					Cancel(const BHttpResult& request);

src/kits/network/libnetservices2/HttpResultPrivate.h

@@ -15,6 +15,7 @@
 #include <string>
 
 #include <DataIO.h>
+#include <ExclusiveBorrow.h>
 #include <OS.h> 
 #include <String.h>
 
@@ -45,10 +46,9 @@ struct HttpResultPrivate {
 			std::optional<BHttpBody>	body;
 			std::optional<std::exception_ptr>	error;
 
-	// Body storage
+	// Interim body storage (used while the request is running)
-			std::unique_ptr<BDataIO>	ownedBody = nullptr;
+			BString						bodyString;
-	//		std::shared_ptr<BMemoryRingIO>	shared_body = nullptr;
+			BBorrow<BDataIO>			bodyTarget;
-			BString						bodyText;
 
 	// Utility functions
 										HttpResultPrivate(int32 identifier);
@@ -98,6 +98,9 @@ HttpResultPrivate::SetCancel()
 inline void
 HttpResultPrivate::SetError(std::exception_ptr e)
 {
+	// Release any held body target borrow
+	bodyTarget.Return();
+
 	error = e;
 	atomic_set(&requestStatus, kError);
 	release_sem(data_wait);
@@ -125,7 +128,12 @@ HttpResultPrivate::SetFields(BHttpFields&& f)
 inline void
 HttpResultPrivate::SetBody()
 {
-	body = BHttpBody{std::move(ownedBody), std::move(bodyText)};
+	if (bodyTarget.HasValue()) {
+		body = BHttpBody{};
+		bodyTarget.Return();
+	} else
+		body = BHttpBody{std::move(bodyString)};
+
 	atomic_set(&requestStatus, kBodyReady);
 	release_sem(data_wait);
 }
@@ -136,14 +144,15 @@ HttpResultPrivate::WriteToBody(const void* buffer, size_t size)
 {
 	// TODO: when the support for a shared BMemoryRingIO is here, choose
 	// between one or the other depending on which one is available.
-	if (ownedBody == nullptr) {
+	if (bodyTarget.HasValue()) {
-		bodyText.Append(static_cast<const char*>(buffer), size);
+		auto result = bodyTarget->Write(buffer, size);
+		if (result < 0)
+			throw BSystemError("BDataIO::Write()", result);
+		return result;
+	} else {
+		bodyString.Append(reinterpret_cast<const char*>(buffer), size);
 		return size;
 	}
-	auto result = ownedBody->Write(buffer, size);
-	if (result < 0)
-		throw BSystemError("BDataIO::Write()", result);
-	return result;
 }
 
 

src/kits/network/libnetservices2/HttpSession.cpp

@@ -61,7 +61,7 @@ struct CounterDeleter {
 class BHttpSession::Request {
 public:
 									Request(BHttpRequest&& request,
-													std::unique_ptr<BDataIO> target,
+													BBorrow<BDataIO> target,
 													BMessenger observer);
 
 									Request(Request& original, const Redirect& redirect);
@@ -142,7 +142,7 @@ public:
 												~Impl() noexcept;
 
 			BHttpResult							Execute(BHttpRequest&& request,
-													std::unique_ptr<BDataIO> target,
+													BBorrow<BDataIO> target,
 													BMessenger observer);
 			void								Cancel(int32 identifier);
 			void								SetMaxConnectionsPerHost(size_t maxConnections);
@@ -232,7 +232,7 @@ BHttpSession::Impl::~Impl() noexcept
 
 
 BHttpResult
-BHttpSession::Impl::Execute(BHttpRequest&& request, std::unique_ptr<BDataIO> target,
+BHttpSession::Impl::Execute(BHttpRequest&& request, BBorrow<BDataIO> target,
 	BMessenger observer)
 {
 	auto wRequest = Request(std::move(request), std::move(target), observer);
@@ -635,7 +635,7 @@ BHttpSession::operator=(const BHttpSession&) noexcept = default;
 
 
 BHttpResult
-BHttpSession::Execute(BHttpRequest&& request, std::unique_ptr<BDataIO> target, BMessenger observer)
+BHttpSession::Execute(BHttpRequest&& request, BBorrow<BDataIO> target, BMessenger observer)
 {
 	return fImpl->Execute(std::move(request), std::move(target), observer);
 }
@@ -670,8 +670,7 @@ BHttpSession::SetMaxHosts(size_t maxConnections)
 
 
 // #pragma mark -- BHttpSession::Request (helpers)
-
+BHttpSession::Request::Request(BHttpRequest&& request, BBorrow<BDataIO> target,
-BHttpSession::Request::Request(BHttpRequest&& request, std::unique_ptr<BDataIO> target,
 		BMessenger observer)
 	: fRequest(std::move(request)), fObserver(observer)
 {
@@ -682,7 +681,10 @@ BHttpSession::Request::Request(BHttpRequest&& request, std::unique_ptr<BDataIO>
 
 	// create shared data
 	fResult = std::make_shared<HttpResultPrivate>(identifier);
-	fResult->ownedBody = std::move(target);
+
+	// check if there is a target
+	if (target.HasValue())
+		fResult->bodyTarget = std::move(target);
 
 	// inform the parser when we do a HEAD request, so not to expect content
 	if (fRequest.Method() == BHttpMethod::Head)

src/tests/kits/net/netservices2/HttpProtocolTest.cpp

@@ -14,6 +14,7 @@
 #include <tools/cppunit/ThreadedTestCaller.h>
 
 #include <DateTime.h>
+#include <ExclusiveBorrow.h>
 #include <HttpFields.h>
 #include <HttpRequest.h>
 #include <HttpResult.h>
@@ -23,6 +24,8 @@
 #include <Url.h>
 
 using BPrivate::BDateTime;
+using BPrivate::Network::BBorrow;
+using BPrivate::Network::BExclusiveBorrow;
 using BPrivate::Network::BHttpFields;
 using BPrivate::Network::BHttpMethod;
 using BPrivate::Network::BHttpRequest;
@@ -32,6 +35,7 @@ using BPrivate::Network::BHttpTime;
 using BPrivate::Network::BHttpTimeFormat;
 using BPrivate::Network::BNetworkRequestError;
 using BPrivate::Network::format_http_time;
+using BPrivate::Network::make_exclusive_borrow;
 using BPrivate::Network::parse_http_time;
 
 using namespace std::literals;
@@ -453,6 +457,7 @@ HttpIntegrationTest::AddTests(BTestSuite& parent)
 		testCaller->addThread("HostAndNetworkFailTest",
 			&HttpIntegrationTest::HostAndNetworkFailTest);
 		testCaller->addThread("GetTest", &HttpIntegrationTest::GetTest);
+		testCaller->addThread("GetWithBufferTest", &HttpIntegrationTest::GetWithBufferTest);
 		testCaller->addThread("HeadTest", &HttpIntegrationTest::HeadTest);
 		testCaller->addThread("NoContentTest", &HttpIntegrationTest::NoContentTest);
 		testCaller->addThread("AutoRedirectTest", &HttpIntegrationTest::AutoRedirectTest);
@@ -477,6 +482,7 @@ HttpIntegrationTest::AddTests(BTestSuite& parent)
 		testCaller->addThread("HostAndNetworkFailTest",
 			&HttpIntegrationTest::HostAndNetworkFailTest);
 		testCaller->addThread("GetTest", &HttpIntegrationTest::GetTest);
+		testCaller->addThread("GetWithBufferTest", &HttpIntegrationTest::GetWithBufferTest);
 		testCaller->addThread("HeadTest", &HttpIntegrationTest::HeadTest);
 		testCaller->addThread("NoContentTest", &HttpIntegrationTest::NoContentTest);
 		testCaller->addThread("AutoRedirectTest", &HttpIntegrationTest::AutoRedirectTest);
@@ -559,7 +565,25 @@ HttpIntegrationTest::GetTest()
 			CPPUNIT_ASSERT_EQUAL(field.Value(), (*expectedField).Value());
 		}
 		auto receivedBody = result.Body().text;
-		CPPUNIT_ASSERT_EQUAL(kExpectedGetBody, receivedBody.String());
+		CPPUNIT_ASSERT(receivedBody.has_value());
+		CPPUNIT_ASSERT_EQUAL(kExpectedGetBody, receivedBody.value().String());
+	} catch (const BPrivate::Network::BError& e) {
+		CPPUNIT_FAIL(e.DebugMessage().String());
+	}
+}
+
+
+void
+HttpIntegrationTest::GetWithBufferTest()
+{
+	auto request = BHttpRequest(BUrl(fTestServer.BaseUrl(), "/"));
+	auto body = make_exclusive_borrow<BMallocIO>();
+	auto result = fSession.Execute(std::move(request), BBorrow<BDataIO>(body), fLoggerMessenger);
+	try {
+		result.Body();
+		auto bodyString = std::string(reinterpret_cast<const char*>(body->Buffer()),
+			body->BufferLength());
+		CPPUNIT_ASSERT_EQUAL(kExpectedGetBody, bodyString);
 	} catch (const BPrivate::Network::BError& e) {
 		CPPUNIT_FAIL(e.DebugMessage().String());
 	}
@@ -584,8 +608,7 @@ HttpIntegrationTest::HeadTest()
 			CPPUNIT_ASSERT_EQUAL(field.Value(), (*expectedField).Value());
 		}
 
-		auto receivedBody = result.Body().text;
+		CPPUNIT_ASSERT(result.Body().text->Length() == 0);
-		CPPUNIT_ASSERT_EQUAL(receivedBody.Length(), 0);
 	} catch (const BPrivate::Network::BError& e) {
 		CPPUNIT_FAIL(e.DebugMessage().String());
 	}
@@ -618,8 +641,7 @@ HttpIntegrationTest::NoContentTest()
 			CPPUNIT_ASSERT_EQUAL(field.Value(), (*expectedField).Value());
 		}
 
-		auto receivedBody = result.Body().text;
+		CPPUNIT_ASSERT(result.Body().text->Length() == 0);
-		CPPUNIT_ASSERT_EQUAL(receivedBody.Length(), 0);
 	} catch (const BPrivate::Network::BError& e) {
 		CPPUNIT_FAIL(e.DebugMessage().String());
 	}
@@ -644,7 +666,8 @@ HttpIntegrationTest::AutoRedirectTest()
 			CPPUNIT_ASSERT_EQUAL(field.Value(), (*expectedField).Value());
 		}
 		auto receivedBody = result.Body().text;
-		CPPUNIT_ASSERT_EQUAL(kExpectedGetBody, receivedBody.String());
+		CPPUNIT_ASSERT(receivedBody.has_value());
+		CPPUNIT_ASSERT_EQUAL(kExpectedGetBody, receivedBody.value().String());
 	} catch (const BPrivate::Network::BError& e) {
 		CPPUNIT_FAIL(e.DebugMessage().String());
 	}
@@ -681,7 +704,7 @@ HttpIntegrationTest::StopOnErrorTest()
 	auto result = fSession.Execute(std::move(request),  nullptr, fLoggerMessenger);
 	CPPUNIT_ASSERT(result.Status().code == 400);
 	CPPUNIT_ASSERT(result.Fields().CountFields() == 0);
-	CPPUNIT_ASSERT(result.Body().text.Length() == 0);
+	CPPUNIT_ASSERT(result.Body().text->Length() == 0);
 }
 
 
@@ -764,8 +787,9 @@ HttpIntegrationTest::PostTest()
 
 	auto result = fSession.Execute(std::move(request), nullptr, BMessenger(observer));
 
-	CPPUNIT_ASSERT_EQUAL(kExpectedPostBody.Length(), result.Body().text.Length());
+	CPPUNIT_ASSERT(result.Body().text.has_value());
-	CPPUNIT_ASSERT(result.Body().text == kExpectedPostBody);
+	CPPUNIT_ASSERT_EQUAL(kExpectedPostBody.Length(), result.Body().text.value().Length());
+	CPPUNIT_ASSERT(result.Body().text.value() == kExpectedPostBody);
 
 	usleep(2000); // give some time to catch up on receiving all messages
 

src/tests/kits/net/netservices2/HttpProtocolTest.h

@@ -33,22 +33,23 @@ public:
 class HttpIntegrationTest : public BThreadedTestCase
 {
 public:
-					HttpIntegrationTest(TestServerMode mode);
+								HttpIntegrationTest(TestServerMode mode);
 
-	virtual	void		setUp() override;
+	virtual	void				setUp() override;
-	virtual void		tearDown() override;
+	virtual void				tearDown() override;
 
-			void	HostAndNetworkFailTest();
+			void				HostAndNetworkFailTest();
-			void	GetTest();
+			void				GetTest();
-			void	HeadTest();
+			void				GetWithBufferTest();
-			void	NoContentTest();
+			void				HeadTest();
-			void	AutoRedirectTest();
+			void				NoContentTest();
-			void	BasicAuthTest();
+			void				AutoRedirectTest();
-			void	StopOnErrorTest();
+			void				BasicAuthTest();
-			void	RequestCancelTest();
+			void				StopOnErrorTest();
-			void	PostTest();
+			void				RequestCancelTest();
+			void				PostTest();
 
-	static	void	AddTests(BTestSuite& suite);
+	static	void				AddTests(BTestSuite& suite);
 
 private:
 			TestServer			fTestServer;