Manage search model requests in SearchModel

Also make Client an ExplicitSingleton, and thus fix all known instances of Requests outliving RequestManager.

src/client/Client.cpp

@@ -976,120 +976,6 @@ SaveFile * Client::LoadSaveFile(ByteString filename)
 	return file;
 }
 
-std::vector<std::pair<ByteString, int> > * Client::GetTags(int start, int count, String query, int & resultCount)
-{
-	lastError = "";
-	resultCount = 0;
-	std::vector<std::pair<ByteString, int> > * tagArray = new std::vector<std::pair<ByteString, int> >();
-	ByteStringBuilder urlStream;
-	urlStream << SCHEME << SERVER << "/Browse/Tags.json?Start=" << start << "&Count=" << count;
-	if(query.length())
-	{
-		urlStream << "&Search_Query=";
-		if(query.length())
-			urlStream << format::URLEncode(query.ToUtf8());
-	}
-
-	auto [ dataStatus, data ] = http::Request::Simple(urlStream.Build());
-	if(dataStatus == 200 && data.size())
-	{
-		try
-		{
-			std::istringstream dataStream(data);
-			Json::Value objDocument;
-			dataStream >> objDocument;
-
-			resultCount = objDocument["TagTotal"].asInt();
-			Json::Value tagsArray = objDocument["Tags"];
-			for (Json::UInt j = 0; j < tagsArray.size(); j++)
-			{
-				int tagCount = tagsArray[j]["Count"].asInt();
-				ByteString tag = tagsArray[j]["Tag"].asString();
-				tagArray->push_back(std::pair<ByteString, int>(tag, tagCount));
-			}
-		}
-		catch (std::exception & e)
-		{
-			lastError = "Could not read response: " + ByteString(e.what()).FromUtf8();
-		}
-	}
-	else
-	{
-		lastError = http::StatusText(dataStatus);
-	}
-	return tagArray;
-}
-
-std::vector<SaveInfo*> * Client::SearchSaves(int start, int count, String query, ByteString sort, ByteString category, int & resultCount)
-{
-	lastError = "";
-	resultCount = 0;
-	std::vector<SaveInfo*> * saveArray = new std::vector<SaveInfo*>();
-	ByteStringBuilder urlStream;
-	ByteString data;
-	int dataStatus;
-	urlStream << SCHEME << SERVER << "/Browse.json?Start=" << start << "&Count=" << count;
-	if(query.length() || sort.length())
-	{
-		urlStream << "&Search_Query=";
-		if(query.length())
-			urlStream << format::URLEncode(query.ToUtf8());
-		if(sort == "date")
-		{
-			if(query.length())
-				urlStream << format::URLEncode(" ");
-			urlStream << format::URLEncode("sort:") << format::URLEncode(sort);
-		}
-	}
-	if(category.length())
-	{
-		urlStream << "&Category=" << format::URLEncode(category);
-	}
-	if(authUser.UserID)
-	{
-		ByteString userID = ByteString::Build(authUser.UserID);
-		std::tie(dataStatus, data) = http::Request::SimpleAuth(urlStream.Build(), userID, authUser.SessionID);
-	}
-	else
-	{
-		std::tie(dataStatus, data) = http::Request::Simple(urlStream.Build());
-	}
-	ParseServerReturn(data, dataStatus, true);
-	if (dataStatus == 200 && data.size())
-	{
-		try
-		{
-			std::istringstream dataStream(data);
-			Json::Value objDocument;
-			dataStream >> objDocument;
-
-			resultCount = objDocument["Count"].asInt();
-			Json::Value savesArray = objDocument["Saves"];
-			for (Json::UInt j = 0; j < savesArray.size(); j++)
-			{
-				int tempID = savesArray[j]["ID"].asInt();
-				int tempCreatedDate = savesArray[j]["Created"].asInt();
-				int tempUpdatedDate = savesArray[j]["Updated"].asInt();
-				int tempScoreUp = savesArray[j]["ScoreUp"].asInt();
-				int tempScoreDown = savesArray[j]["ScoreDown"].asInt();
-				ByteString tempUsername = savesArray[j]["Username"].asString();
-				String tempName = ByteString(savesArray[j]["Name"].asString()).FromUtf8();
-				int tempVersion = savesArray[j]["Version"].asInt();
-				bool tempPublished = savesArray[j]["Published"].asBool();
-				SaveInfo * tempSaveInfo = new SaveInfo(tempID, tempCreatedDate, tempUpdatedDate, tempScoreUp, tempScoreDown, tempUsername, tempName);
-				tempSaveInfo->Version = tempVersion;
-				tempSaveInfo->SetPublished(tempPublished);
-				saveArray->push_back(tempSaveInfo);
-			}
-		}
-		catch (std::exception &e)
-		{
-			lastError = "Could not read response: " + ByteString(e.what()).FromUtf8();
-		}
-	}
-	return saveArray;
-}
-
 std::list<ByteString> * Client::RemoveTag(int saveID, ByteString tag)
 {
 	lastError = "";

src/client/Client.h

@@ -129,8 +129,6 @@ public:
 	std::vector<char> GetSaveData(int saveID, int saveDate);
 
 	LoginStatus Login(ByteString username, ByteString password, User & user);
-	std::vector<SaveInfo*> * SearchSaves(int start, int count, String query, ByteString sort, ByteString category, int & resultCount);
-	std::vector<std::pair<ByteString, int> > * GetTags(int start, int count, String query, int & resultCount);
 
 	SaveInfo * GetSave(int saveID, int saveDate);
 	SaveFile * LoadSaveFile(ByteString filename);

src/client/http/Request.cpp

@@ -13,18 +13,10 @@ namespace http
 	Request::~Request()
 	{
 		if (handle->state != RequestHandle::ready)
-		{
-			// TODO: Fix bad design.
-			// Bad design: Client should not outlive RequestManager because it has its own requests, but
-			//             RequestManager needs Client because Client is also responsible for configuration >_>
-			// Problem:    Client outlives RequestManager, RequestManager doesn't necessarily exist at this point.
-			// Solution:   Check if it does >_> ExplicitSingleton::Exists exists for no other reason than this.
-			if (RequestManager::Exists())
 		{
 			RequestManager::Ref().UnregisterRequest(*this);
 		}
 	}
-	}
 
 	void Request::Verb(ByteString newVerb)
 	{
@@ -67,16 +59,8 @@ namespace http
 	{
 		assert(handle->state == RequestHandle::ready);
 		handle->state = RequestHandle::running;
-		// TODO: Fix bad design.
-		// Bad design: Client should not outlive RequestManager because it has its own requests, but
-		//             RequestManager needs Client because Client is also responsible for configuration >_>
-		// Problem:    Client outlives RequestManager, RequestManager doesn't necessarily exist at this point.
-		// Solution:   Check if it does >_> ExplicitSingleton::Exists exists for no other reason than this.
-		if (RequestManager::Exists())
-		{
 		RequestManager::Ref().RegisterRequest(*this);
 	}
-	}
 
 	bool Request::CheckDone() const
 	{
@@ -201,7 +185,6 @@ namespace http
 		case 607: return "Connection Refused";
 		case 608: return "Proxy Server Not Found";
 		case 609: return "SSL: Invalid Certificate Status";
-		case 610: return "Cancelled by Shutdown";
 		case 611: return "Too Many Redirects";
 		case 612: return "SSL: Connect Error";
 		case 613: return "SSL: Crypto Engine Not Found";

src/client/http/RequestManager.cpp

@@ -39,18 +39,6 @@ namespace http
 		{
 			{
 				std::lock_guard lk(sharedStateMx);
-				if (!running)
-				{
-					// TODO: Fix bad design.
-					// Bad design: Client should not outlive RequestManager because it has its own requests, but
-					//             RequestManager needs Client because Client is also responsible for configuration >_>
-					// Problem:    RequestManager's worker needs all requests to have been unregistered when it exits.
-					// Solution:   Knock out all live requests here on shutdown.
-					for (auto &requestHandle : requestHandles)
-					{
-						requestHandle->statusCode = 610;
-					}
-				}
 				for (auto &requestHandle : requestHandles)
 				{
 					if (requestHandle->statusCode)

src/common/ExplicitSingleton.h

@@ -28,9 +28,4 @@ public:
 	{
 		return *Instance();
 	}
-
-	static bool Exists()
-	{
-		return Instance();
-	}
 };

src/gui/search/SearchModel.cpp

@@ -2,6 +2,7 @@
 
 #include "SearchView.h"
 
+#include "Format.h"
 #include "client/SaveInfo.h"
 #include "client/Client.h"
 
@@ -17,14 +18,7 @@ SearchModel::SearchModel():
 	resultCount(0),
 	showOwn(false),
 	showFavourite(false),
-	showTags(true),
+	showTags(true)
-	saveListLoaded(false),
-	updateSaveListWorking(false),
-	updateSaveListFinished(false),
-	updateSaveListResult(nullptr),
-	updateTagListWorking(false),
-	updateTagListFinished(false),
-	updateTagListResult(nullptr)
 {
 }
 
@@ -38,32 +32,136 @@ bool SearchModel::GetShowTags()
 	return showTags;
 }
 
-void SearchModel::updateSaveListT()
+void SearchModel::BeginSearchSaves(int start, int count, String query, ByteString sort, ByteString category)
 {
-	ByteString category = "";
+	lastError = "";
-	if(showFavourite)
+	resultCount = 0;
-		category = "Favourites";
+	ByteStringBuilder urlStream;
-	if(showOwn && Client::Ref().GetAuthUser().UserID)
+	ByteString data;
-		category = "by:"+Client::Ref().GetAuthUser().Username;
+	urlStream << SCHEME << SERVER << "/Browse.json?Start=" << start << "&Count=" << count;
-	std::vector<SaveInfo*> * saveList = Client::Ref().SearchSaves((currentPage-1)*20, 20, lastQuery, currentSort=="new"?"date":"votes", category, thResultCount);
+	if(query.length() || sort.length())
-
+	{
-	updateSaveListResult = saveList;
+		urlStream << "&Search_Query=";
-	updateSaveListFinished = true;
+		if(query.length())
+			urlStream << format::URLEncode(query.ToUtf8());
+		if(sort == "date")
+		{
+			if(query.length())
+				urlStream << format::URLEncode(" ");
+			urlStream << format::URLEncode("sort:") << format::URLEncode(sort);
+		}
+	}
+	if(category.length())
+	{
+		urlStream << "&Category=" << format::URLEncode(category);
+	}
+	searchSaves = std::make_unique<http::Request>(urlStream.Build());
+	auto authUser = Client::Ref().GetAuthUser();
+	if (authUser.UserID)
+	{
+		searchSaves->AuthHeaders(ByteString::Build(Client::Ref().GetAuthUser().UserID), Client::Ref().GetAuthUser().SessionID);
+	}
+	searchSaves->Start();
 }
 
-void SearchModel::updateTagListT()
+std::vector<SaveInfo *> SearchModel::EndSearchSaves()
 {
-	int tagResultCount;
+	std::vector<SaveInfo *> saveArray;
-	std::vector<std::pair<ByteString, int> > * tagList = Client::Ref().GetTags(0, 24, "", tagResultCount);
+	auto [ dataStatus, data ] = searchSaves->Finish();
+	searchSaves.reset();
+	auto &client = Client::Ref();
+	client.ParseServerReturn(data, dataStatus, true);
+	if (dataStatus == 200 && data.size())
+	{
+		try
+		{
+			std::istringstream dataStream(data);
+			Json::Value objDocument;
+			dataStream >> objDocument;
 
-	updateTagListResult = tagList;
+			resultCount = objDocument["Count"].asInt();
-	updateTagListFinished = true;
+			Json::Value savesArray = objDocument["Saves"];
+			for (Json::UInt j = 0; j < savesArray.size(); j++)
+			{
+				int tempID = savesArray[j]["ID"].asInt();
+				int tempCreatedDate = savesArray[j]["Created"].asInt();
+				int tempUpdatedDate = savesArray[j]["Updated"].asInt();
+				int tempScoreUp = savesArray[j]["ScoreUp"].asInt();
+				int tempScoreDown = savesArray[j]["ScoreDown"].asInt();
+				ByteString tempUsername = savesArray[j]["Username"].asString();
+				String tempName = ByteString(savesArray[j]["Name"].asString()).FromUtf8();
+				int tempVersion = savesArray[j]["Version"].asInt();
+				bool tempPublished = savesArray[j]["Published"].asBool();
+				SaveInfo * tempSaveInfo = new SaveInfo(tempID, tempCreatedDate, tempUpdatedDate, tempScoreUp, tempScoreDown, tempUsername, tempName);
+				tempSaveInfo->Version = tempVersion;
+				tempSaveInfo->SetPublished(tempPublished);
+				saveArray.push_back(tempSaveInfo);
+			}
+		}
+		catch (std::exception &e)
+		{
+			lastError = "Could not read response: " + ByteString(e.what()).FromUtf8();
+		}
+	}
+	else
+	{
+		lastError = client.GetLastError();
+	}
+	return saveArray;
+}
+
+void SearchModel::BeginGetTags(int start, int count, String query)
+{
+	lastError = "";
+	ByteStringBuilder urlStream;
+	urlStream << SCHEME << SERVER << "/Browse/Tags.json?Start=" << start << "&Count=" << count;
+	if(query.length())
+	{
+		urlStream << "&Search_Query=";
+		if(query.length())
+			urlStream << format::URLEncode(query.ToUtf8());
+	}
+	getTags = std::make_unique<http::Request>(urlStream.Build());
+	getTags->Start();
+}
+
+std::vector<std::pair<ByteString, int>> SearchModel::EndGetTags()
+{
+	std::vector<std::pair<ByteString, int>> tagArray;
+	auto [ dataStatus, data ] = getTags->Finish();
+	getTags.reset();
+	if(dataStatus == 200 && data.size())
+	{
+		try
+		{
+			std::istringstream dataStream(data);
+			Json::Value objDocument;
+			dataStream >> objDocument;
+
+			Json::Value tagsArray = objDocument["Tags"];
+			for (Json::UInt j = 0; j < tagsArray.size(); j++)
+			{
+				int tagCount = tagsArray[j]["Count"].asInt();
+				ByteString tag = tagsArray[j]["Tag"].asString();
+				tagArray.push_back(std::pair<ByteString, int>(tag, tagCount));
+			}
+		}
+		catch (std::exception & e)
+		{
+			lastError = "Could not read response: " + ByteString(e.what()).FromUtf8();
+		}
+	}
+	else
+	{
+		lastError = http::StatusText(dataStatus);
+	}
+	return tagArray;
 }
 
 bool SearchModel::UpdateSaveList(int pageNumber, String query)
 {
 	//Threading
-	if (!updateSaveListWorking)
+	if (!searchSaves)
 	{
 		lastQuery = query;
 		lastError = "";
@@ -83,16 +181,17 @@ bool SearchModel::UpdateSaveList(int pageNumber, String query)
 		selected.clear();
 		notifySelectedChanged();
 
-		if(GetShowTags() && !tagList.size() && !updateTagListWorking)
+		if (GetShowTags() && !tagList.size() && !getTags)
 		{
-			updateTagListFinished = false;
+			BeginGetTags(0, 24, "");
-			updateTagListWorking = true;
-			std::thread([this]() { updateTagListT(); }).detach();
 		}
 
-		updateSaveListFinished = false;
+		ByteString category = "";
-		updateSaveListWorking = true;
+		if(showFavourite)
-		std::thread([this]() { updateSaveListT(); }).detach();
+			category = "Favourites";
+		if(showOwn && Client::Ref().GetAuthUser().UserID)
+			category = "by:"+Client::Ref().GetAuthUser().Username;
+		BeginSearchSaves((currentPage-1)*20, 20, lastQuery, currentSort=="new"?"date":"votes", category);
 		return true;
 	}
 	return false;
@@ -128,53 +227,21 @@ std::vector<std::pair<ByteString, int> > SearchModel::GetTagList()
 
 void SearchModel::Update()
 {
-	if(updateSaveListWorking)
+	if (searchSaves && searchSaves->CheckDone())
 	{
-		if(updateSaveListFinished)
-		{
-			updateSaveListWorking = false;
-			lastError = "";
 		saveListLoaded = true;
-
-			std::vector<SaveInfo *> *tempSaveList = updateSaveListResult;
-			updateSaveListResult = nullptr;
-
-			if(tempSaveList)
-			{
-				saveList = *tempSaveList;
-				delete tempSaveList;
-			}
-
-			if(!saveList.size())
-			{
-				lastError = Client::Ref().GetLastError();
-				if (lastError == "Unspecified Error")
 		lastError = "";
-			}
+		saveList = EndSearchSaves();
-
-			resultCount = thResultCount;
 		notifyPageChanged();
 		notifySaveListChanged();
 	}
-	}
+	if (getTags && getTags->CheckDone())
-	if(updateTagListWorking)
 	{
-		if(updateTagListFinished)
+		lastError = "";
-		{
+		tagList = EndGetTags();
-			updateTagListWorking = false;
-
-			std::vector<std::pair<ByteString, int>> *tempTagList = updateTagListResult;
-			updateTagListResult = nullptr;
-
-			if(tempTagList)
-			{
-				tagList = *tempTagList;
-				delete tempTagList;
-			}
 		notifyTagListChanged();
 	}
 }
-}
 
 void SearchModel::AddObserver(SearchView * observer)
 {

src/gui/search/SearchModel.h

@@ -1,15 +1,24 @@
 #pragma once
 #include "Config.h"
-
-#include <vector>
 #include "common/String.h"
+#include "client/http/Request.h"
+#include <vector>
 #include <atomic>
+#include <memory>
 
 class SaveInfo;
 class SearchView;
 class SearchModel
 {
 private:
+	std::unique_ptr<http::Request> searchSaves;
+	void BeginSearchSaves(int start, int count, String query, ByteString sort, ByteString category);
+	std::vector<SaveInfo *> EndSearchSaves();
+
+	void BeginGetTags(int start, int count, String query);
+	std::vector<std::pair<ByteString, int>> EndGetTags();
+	std::unique_ptr<http::Request> getTags;
+
 	SaveInfo * loadedSave;
 	ByteString currentSort;
 	String lastQuery;
@@ -20,7 +29,6 @@ private:
 	std::vector<std::pair<ByteString, int> > tagList;
 	int currentPage;
 	int resultCount;
-	int thResultCount;
 	bool showOwn;
 	bool showFavourite;
 	bool showTags;
@@ -33,19 +41,10 @@ private:
 	void notifyShowFavouriteChanged();
 
 	//Variables and methods for background save request
-	bool saveListLoaded;
+	bool saveListLoaded = false;
-	bool updateSaveListWorking;
-	std::atomic<bool> updateSaveListFinished;
-	void updateSaveListT();
-	std::vector<SaveInfo *> *updateSaveListResult;
-
-	bool updateTagListWorking;
-	std::atomic<bool> updateTagListFinished;
-	void updateTagListT();
-	std::vector<std::pair<ByteString, int>> *updateTagListResult;
 public:
     SearchModel();
-    virtual ~SearchModel();
+    ~SearchModel();
 
     void SetShowTags(bool show);
     bool GetShowTags();
@@ -57,11 +56,11 @@ public:
 	int GetPageCount();
 	int GetPageNum() { return currentPage; }
 	String GetLastQuery() { return lastQuery; }
-	void SetSort(ByteString sort) { if(!updateSaveListWorking) { currentSort = sort; } notifySortChanged(); }
+	void SetSort(ByteString sort) { if(!searchSaves) { currentSort = sort; } notifySortChanged(); }
 	ByteString GetSort() { return currentSort; }
-	void SetShowOwn(bool show) { if(!updateSaveListWorking) { if(show!=showOwn) { showOwn = show; } } notifyShowOwnChanged();  }
+	void SetShowOwn(bool show) { if(!searchSaves) { if(show!=showOwn) { showOwn = show; } } notifyShowOwnChanged();  }
 	bool GetShowOwn() { return showOwn; }
-	void SetShowFavourite(bool show) { if(show!=showFavourite && !updateSaveListWorking) { showFavourite = show; } notifyShowFavouriteChanged();  }
+	void SetShowFavourite(bool show) { if(show!=showFavourite && !searchSaves) { showFavourite = show; } notifyShowFavouriteChanged();  }
 	bool GetShowFavourite() { return showFavourite; }
 	void SetLoadedSave(SaveInfo * save);
 	SaveInfo * GetLoadedSave();