Revert HTTP cache

Partial revert of ee8f491

We've had report of failing updates recently, and this is the most likely culprit

Author: brliron

thcrap_test/src/repo_discovery.cpp

@@ -27,7 +27,7 @@ class FakeHttpHandle : public IHttpHandle
         files.clear();
     }
 
-    HttpStatus download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)>, DownloadCache*) override
+    HttpStatus download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)>) override
     {
         auto it = files.find(url);
         if (it == files.end()) {

thcrap_update/src/download_cache.cpp

@@ -22,8 +22,7 @@ std::list<DownloadUrl> Downloader::serversListToDownloadUrlList(const std::list<
 }
 
 void Downloader::addFile(const std::list<std::string>& serversUrl, std::string filePath,
-                         File::success_t successCallback, File::failure_t failureCallback, File::progress_t progressCallback,
-						 DownloadCache *cache)
+                         File::success_t successCallback, File::failure_t failureCallback, File::progress_t progressCallback)
 {
     std::scoped_lock lock(this->mutex);
 
@@ -32,7 +31,7 @@ void Downloader::addFile(const std::list<std::string>& serversUrl, std::string f
         current_++;
         return successCallback(url, data);
     };
-    std::shared_ptr<File> file = std::make_unique<File>(std::move(urls), successLambda, failureCallback, progressCallback, cache);
+    std::shared_ptr<File> file = std::make_unique<File>(std::move(urls), successLambda, failureCallback, progressCallback);
 
     this->futuresList.push_back(this->pool.enqueue([file]() {
         file->download();
@@ -42,15 +41,14 @@ void Downloader::addFile(const std::list<std::string>& serversUrl, std::string f
 }
 
 void Downloader::addFile(char** serversUrl, std::string filePath,
-                         File::success_t successCallback, File::failure_t failureCallback, File::progress_t progressCallback,
-						 DownloadCache *cache)
+                         File::success_t successCallback, File::failure_t failureCallback, File::progress_t progressCallback)
 {
     std::list<std::string> serversList;
 
     for (size_t i = 0; serversUrl && serversUrl[i]; i++) {
         serversList.push_back(serversUrl[i]);
     }
-    this->addFile(serversList, filePath, successCallback, failureCallback, progressCallback, cache);
+    this->addFile(serversList, filePath, successCallback, failureCallback, progressCallback);
 }
 
 size_t Downloader::current() const

thcrap_update/src/download_cache.h

@@ -6,7 +6,6 @@
 #include <atomic>
 #include <libs/include/ThreadPool.h>
 #include "file.h"
-#include "download_cache.h"
 
 class Downloader
 {
@@ -25,13 +24,11 @@ class Downloader
     void addFile(const std::list<std::string>& servers, std::string filename,
                  File::success_t successCallback = File::defaultSuccessFunction,
                  File::failure_t failureCallback = File::defaultFailureFunction,
-                 File::progress_t progressCallback = File::defaultProgressFunction,
-				 DownloadCache *cache = nullptr);
+                 File::progress_t progressCallback = File::defaultProgressFunction);
     void addFile(char** servers, std::string filename,
                  File::success_t successCallback = File::defaultSuccessFunction,
                  File::failure_t failureCallback = File::defaultFailureFunction,
-                 File::progress_t progressCallback = File::defaultProgressFunction,
-				 DownloadCache *cache = nullptr);
+                 File::progress_t progressCallback = File::defaultProgressFunction);
     size_t current() const;
     size_t total() const;
     void wait();

thcrap_update/src/downloader.cpp

@@ -10,23 +10,15 @@ using namespace std::string_literals;
 File::File(std::list<DownloadUrl>&& urls,
            success_t successCallback,
            failure_t failureCallback,
-           progress_t progressCallback,
-		   DownloadCache *cache)
-    : status(Status::Todo), urls(urls), cache(cache),
+           progress_t progressCallback)
+    : status(Status::Todo), urls(urls),
     userSuccessCallback(successCallback), userFailureCallback(failureCallback), userProgressCallback(progressCallback)
 {
     if (urls.empty()) {
         throw new std::invalid_argument("Input URL list must not be empty");
     }
 }
 
-File::~File()
-{
-	if (this->cache) {
-		delete this->cache;
-	}
-}
-
 size_t File::writeCallback(std::vector<uint8_t>& buffer, const uint8_t *data, size_t size)
 {
     buffer.insert(buffer.end(), data, data + size);
@@ -67,8 +59,7 @@ void File::download(IHttpHandle& http, const DownloadUrl& url)
         },
         [this, &url](size_t dlnow, size_t dltotal) {
             return this->progressCallback(url, dlnow, dltotal);
-        },
-		this->cache
+        }
     );
     if (!status) {
         if (status.get() == HttpStatus::ServerError || status.get() == HttpStatus::SystemError) {

thcrap_update/src/downloader.h

@@ -5,7 +5,6 @@
 #include <list>
 #include <mutex>
 #include <vector>
-#include "download_cache.h"
 #include "download_url.h"
 #include "http_interface.h"
 
@@ -34,7 +33,6 @@ class File
     // different servers.
     // When starting a download, we remove the corresponding URL from the list.
     std::list<DownloadUrl> urls;
-	DownloadCache *cache;
 
     // User-provided callbacks
     success_t userSuccessCallback;
@@ -50,13 +48,11 @@ class File
     File(std::list<DownloadUrl>&& urls,
          success_t successCallback = defaultSuccessFunction,
          failure_t failureCallback = defaultFailureFunction,
-         progress_t progressCallback = defaultProgressFunction,
-		 DownloadCache *cache = nullptr);
+         progress_t progressCallback = defaultProgressFunction);
     File(const File&) = delete;
     File(File&&) = delete;
     File& operator=(const File&) = delete;
     File& operator=(File&&) = delete;
-	~File();
 
     // Start a download thread.
     // The status of the download will be returned in the callbacks given

thcrap_update/src/file.cpp

@@ -70,59 +70,7 @@ static std::string encode_UTF8_url(std::string in)
     return out;
 }
 
-static curl_slist *setup_headers(DownloadCache *cache)
-{
-	if (!cache) {
-		return nullptr;
-	}
-	curl_slist *headers = nullptr;
-
-	std::optional<std::string> etag = cache->getEtag();
-	if (etag) {
-		BUILD_VLA_STR(char, header, "If-None-Match: ", etag->c_str());
-		headers = curl_slist_append(headers, header);
-		VLA_FREE(header);
-	}
-
-	std::optional<std::string> lastModifiedDate = cache->getLastModifiedDate();
-	if (lastModifiedDate) {
-		BUILD_VLA_STR(char, header, "If-Modified-Since: ", lastModifiedDate->c_str());
-		headers = curl_slist_append(headers, header);
-		VLA_FREE(header);
-	}
-
-	return headers;
-}
-
-void update_cache(CURL *curl, DownloadCache *cache)
-{
-	if (!cache) {
-		return ;
-	}
-
-	// The CURL doc says that the header struct is clobbered between
-	// subsequent calls, the strings inside it might also be clobbered
-	// (I don't think they are, but the doc doesn't guarantee it).
-	// Better play it safe and make a copy of the 1st header.
-	std::string etag;
-	const char *lastModifiedDate = nullptr;
-
-	curl_header *header;
-	if (curl_easy_header(curl, "ETag", 0, CURLH_HEADER, -1, &header) == CURLHE_OK) {
-		etag = header->value;
-	}
-
-	if (curl_easy_header(curl, "Last-Modified", 0, CURLH_HEADER, -1, &header) == CURLHE_OK) {
-		lastModifiedDate = header->value;
-		if (lastModifiedDate && !lastModifiedDate[0]) {
-			lastModifiedDate = nullptr;
-		}
-	}
-
-	cache->setCacheData(!etag.empty() ? etag.c_str() : nullptr, lastModifiedDate);
-}
-
-HttpStatus CurlHandle::download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)> progressCallback, DownloadCache *cache)
+HttpStatus CurlHandle::download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)> progressCallback)
 {
 	curl_easy_setopt(this->curl, CURLOPT_CAINFO, "bin/cacert.pem");
     curl_easy_setopt(this->curl, CURLOPT_FOLLOWLOCATION, 1);
@@ -142,9 +90,6 @@ HttpStatus CurlHandle::download(const std::string& url, std::function<size_t(con
     curl_easy_setopt(this->curl, CURLOPT_ERRORBUFFER, errbuf);
     errbuf[0] = 0;
 
-	curl_slist *headers = setup_headers(cache);
-	curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
-
     std::string url_encoded = encode_UTF8_url(url);
     curl_easy_setopt(this->curl, CURLOPT_URL, url_encoded.c_str());
 
@@ -172,28 +117,12 @@ HttpStatus CurlHandle::download(const std::string& url, std::function<size_t(con
         }
     }
     curl_easy_setopt(this->curl, CURLOPT_ERRORBUFFER, nullptr);
-    if (headers) {
-        curl_easy_setopt(this->curl, CURLOPT_HTTPHEADER, nullptr);
-        curl_slist_free_all(headers);
-    }
 
     long response_code = 0;
     curl_easy_getinfo(this->curl, CURLINFO_RESPONSE_CODE, &response_code);
-    if (response_code == 200) {
-        update_cache(this->curl, cache);
-        return HttpStatus::makeOk();
-    }
-    else if (response_code == 304) {
-        if (!cache) {
-            throw std::logic_error("Got 304 for a non-cached object");
-        }
-        std::vector<uint8_t> data = cache->getCachedFile();
-        progressCallback(data.size(), data.size());
-        writeCallback(data.data(), data.size());
-        update_cache(this->curl, cache);
-        return HttpStatus::makeOk();
-    }
-    else {
+    if (response_code != 200) {
         return HttpStatus::makeNetworkError(response_code);
     }
+
+    return HttpStatus::makeOk();
 }

thcrap_update/src/file.h

@@ -4,7 +4,6 @@
 #include <string>
 #include <curl/curl.h>
 #include "http_interface.h"
-#include "download_cache.h"
 
 class CurlHandle : public IHttpHandle
 {
@@ -21,5 +20,5 @@ class CurlHandle : public IHttpHandle
     CurlHandle& operator=(CurlHandle& other) = delete;
     ~CurlHandle();
 
-    HttpStatus download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)> progressCallback, DownloadCache *cache) override;
+    HttpStatus download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)> progressCallback) override;
 };

thcrap_update/src/http_curl.cpp

@@ -1,12 +1,10 @@
 #pragma once
 
-#include <string>
 #include "http_status.h"
-#include "download_cache.h"
 
 class IHttpHandle
 {
 public:
     virtual ~IHttpHandle() {}
-    virtual HttpStatus download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)> progressCallback, DownloadCache *cache) = 0;
+    virtual HttpStatus download(const std::string& url, std::function<size_t(const uint8_t*, size_t)> writeCallback, std::function<bool(size_t, size_t)> progressCallback) = 0;
 };