From e778def1d18bc63e049e4a56fb1d196f46d363d1 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Wed, 2 Oct 2024 01:29:37 -0300 Subject: [PATCH] Code adjustements and fixes --- .../src/cpp/http_file_source.cpp | 5 +-- .../android/http/NativeHttpRequest.java | 5 +-- .../android/module/http/HttpRequestImpl.java | 5 +-- .../testapp/utils/ExampleHttpRequestImpl.kt | 4 +-- .../src/mbgl/storage/pmtiles_file_source.cpp | 34 ++++++++----------- platform/qt/src/mbgl/http_request.cpp | 3 +- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp b/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp index 7d8cb41de5c..4bd80f1e78f 100644 --- a/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp @@ -96,9 +96,10 @@ HTTPRequest::HTTPRequest(jni::JNIEnv& env, const Resource& resource_, FileSource std::string modifiedStr; if (resource.dataRange) { - dataRangeStr = std::string("bytes=") + std::to_string(resource.dataRange->first) + std::string("-") + std::to_string(resource.dataRange->second); + dataRangeStr = std::string("bytes=") + std::to_string(resource.dataRange->first) + std::string("-") + + std::to_string(resource.dataRange->second); } - + if (resource.priorEtag) { etagStr = *resource.priorEtag; } else if (resource.priorModified) { diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/http/NativeHttpRequest.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/http/NativeHttpRequest.java index e41429cd3f5..0c4c43aab68 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/http/NativeHttpRequest.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/http/NativeHttpRequest.java @@ -19,7 +19,8 @@ public class NativeHttpRequest implements HttpResponder { private long nativePtr; @Keep - private NativeHttpRequest(long nativePtr, String resourceUrl, String etag, String modified, boolean offlineUsage) { + private NativeHttpRequest(long nativePtr, String resourceUrl, String dataRange, String etag, String modified, + boolean offlineUsage) { this.nativePtr = nativePtr; if (resourceUrl.startsWith("local://")) { @@ -27,7 +28,7 @@ private NativeHttpRequest(long nativePtr, String resourceUrl, String etag, Strin executeLocalRequest(resourceUrl); return; } - httpRequest.executeRequest(this, nativePtr, resourceUrl, etag, modified, offlineUsage); + httpRequest.executeRequest(this, nativePtr, resourceUrl, dataRange, etag, modified, offlineUsage); } public void cancel() { diff --git a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/module/http/HttpRequestImpl.java b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/module/http/HttpRequestImpl.java index d23e3a31782..e51c51de75a 100644 --- a/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/module/http/HttpRequestImpl.java +++ b/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/module/http/HttpRequestImpl.java @@ -57,7 +57,8 @@ public class HttpRequestImpl implements HttpRequest { @Override public void executeRequest(HttpResponder httpRequest, long nativePtr, @NonNull String resourceUrl, - @NonNull String dataRange, @NonNull String etag, @NonNull String modified, boolean offlineUsage) { + @NonNull String dataRange, @NonNull String etag, @NonNull String modified, + boolean offlineUsage) { OkHttpCallback callback = new OkHttpCallback(httpRequest); try { HttpUrl httpUrl = HttpUrl.parse(resourceUrl); @@ -73,7 +74,7 @@ public void executeRequest(HttpResponder httpRequest, long nativePtr, @NonNull S .url(resourceUrl) .tag(resourceUrl.toLowerCase(MapLibreConstants.MAPLIBRE_LOCALE)) .addHeader("User-Agent", userAgentString); - + if (dataRange.length() > 0) { builder.addHeader("Range", dataRange); } diff --git a/platform/android/MapLibreAndroidTestApp/src/main/java/org/maplibre/android/testapp/utils/ExampleHttpRequestImpl.kt b/platform/android/MapLibreAndroidTestApp/src/main/java/org/maplibre/android/testapp/utils/ExampleHttpRequestImpl.kt index a9d2840696f..e49f567f411 100644 --- a/platform/android/MapLibreAndroidTestApp/src/main/java/org/maplibre/android/testapp/utils/ExampleHttpRequestImpl.kt +++ b/platform/android/MapLibreAndroidTestApp/src/main/java/org/maplibre/android/testapp/utils/ExampleHttpRequestImpl.kt @@ -11,11 +11,11 @@ import org.maplibre.android.module.http.HttpRequestImpl */ class ExampleHttpRequestImpl : HttpRequest { override fun executeRequest(httpRequest: HttpResponder, nativePtr: Long, resourceUrl: String, - etag: String, modified: String, offlineUsage: Boolean) + dataRange: String, etag: String, modified: String, offlineUsage: Boolean) { // Load all json documents and any pbf ending with a 0. if (resourceUrl.endsWith(".json") || resourceUrl.endsWith("0.pbf")) { - impl.executeRequest(httpRequest, nativePtr, resourceUrl, etag, modified, offlineUsage) + impl.executeRequest(httpRequest, nativePtr, resourceUrl, dataRange, etag, modified, offlineUsage) } else { // All other requests get an instant 404! httpRequest.onResponse( diff --git a/platform/default/src/mbgl/storage/pmtiles_file_source.cpp b/platform/default/src/mbgl/storage/pmtiles_file_source.cpp index c661c01346a..dbfbafee8a6 100644 --- a/platform/default/src/mbgl/storage/pmtiles_file_source.cpp +++ b/platform/default/src/mbgl/storage/pmtiles_file_source.cpp @@ -29,10 +29,10 @@ #endif namespace { -const int MAX_DIRECTORY_CACHE_ENTRIES = 100; +constexpr int MAX_DIRECTORY_CACHE_ENTRIES = 100; bool acceptsURL(const std::string& url) { - return 0 == url.rfind(mbgl::util::PMTILES_PROTOCOL, 0); + return url.starts_with(mbgl::util::PMTILES_PROTOCOL); } std::string extract_url(const std::string& url) { @@ -74,7 +74,7 @@ class PMTilesFileSource::Impl { void request_tile(AsyncRequest* req, const Resource& resource, ActorRef ref) { auto url = extract_url(resource.url); - getHeaderAndRootDirectory(url, req, [=, this](std::unique_ptr error) { + getHeader(url, req, [=, this](std::unique_ptr error) { if (error) { Response response; response.noContent = true; @@ -206,20 +206,21 @@ class PMTilesFileSource::Impl { return fileSource; } - void getHeaderAndRootDirectory(const std::string& url, AsyncRequest* req, AsyncCallback callback) { + void getHeader(const std::string& url, AsyncRequest* req, AsyncCallback callback) { if (header_cache.find(url) != header_cache.end()) { callback(std::unique_ptr()); } Resource resource(Resource::Kind::Source, url); resource.loadingMethod = Resource::LoadingMethod::Network; - resource.dataRange = std::make_pair(0, 16383); + + // https://github.com/protomaps/PMTiles/blob/main/spec/v3/spec.md#3-header + resource.dataRange = std::make_pair(0, 127); tasks[req] = getFileSource()->request(resource, [=, this](const Response& response) { if (response.error) { callback(std::make_unique( - response.error->reason, - std::string("Error fetching PMTiles header and root directory: ") + response.error->message)); + response.error->reason, std::string("Error fetching PMTiles header: ") + response.error->message)); return; } @@ -234,19 +235,10 @@ class PMTilesFileSource::Impl { header_cache.emplace(url, header); - std::string directoryData = response.data->substr(header.root_dir_offset, header.root_dir_bytes); - - if (header.tile_compression == pmtiles::COMPRESSION_GZIP) { - directoryData = util::decompress(directoryData); - } - - storeDirectory(url, header.root_dir_offset, header.root_dir_bytes, directoryData); - callback(std::unique_ptr()); } catch (const std::exception& e) { - callback(std::make_unique( - Response::Error::Reason::Other, - std::string("Error parsing PMTiles header and root directory: ") + e.what())); + callback(std::make_unique(Response::Error::Reason::Other, + std::string("Error parsing PMTiles header: ") + e.what())); } }); } @@ -256,7 +248,7 @@ class PMTilesFileSource::Impl { callback(std::unique_ptr()); } - getHeaderAndRootDirectory(url, req, [=, this](std::unique_ptr error) { + getHeader(url, req, [=, this](std::unique_ptr error) { if (error) { callback(std::move(error)); return; @@ -406,7 +398,7 @@ class PMTilesFileSource::Impl { return; } - getHeaderAndRootDirectory(url, req, [=, this](std::unique_ptr error) { + getHeader(url, req, [=, this](std::unique_ptr error) { if (error) { callback(std::move(error)); return; @@ -458,6 +450,8 @@ class PMTilesFileSource::Impl { std::make_unique( Response::Error::Reason::Other, std::string("Error fetching PMTiles tile address: Maximum directory depth exceeded"))); + + return; } getDirectory(url, req, directoryOffset, directoryLength, [=, this](std::unique_ptr error) { diff --git a/platform/qt/src/mbgl/http_request.cpp b/platform/qt/src/mbgl/http_request.cpp index 30e60817632..973760a419a 100644 --- a/platform/qt/src/mbgl/http_request.cpp +++ b/platform/qt/src/mbgl/http_request.cpp @@ -50,7 +50,8 @@ QNetworkRequest HTTPRequest::networkRequest() const { #endif if (m_resource.dataRange) { - std::string range = std::string("bytes=") + std::to_string(resource.dataRange->first) + std::string("-") + std::to_string(resource.dataRange->second); + std::string range = std::string("bytes=") + std::to_string(m_resource.dataRange->first) + std::string("-") + + std::to_string(m_resource.dataRange->second); req.setRawHeader("Range", QByteArray(range.data(), static_cast(range.size()))); }