diff options
| -rw-r--r-- | CMakeLists.txt | 8 | ||||
| -rw-r--r-- | bin/offline.cpp | 6 | ||||
| -rw-r--r-- | cmake/core-files.cmake | 1 | ||||
| -rw-r--r-- | cmake/filesource.cmake | 3 | ||||
| -rw-r--r-- | cmake/mbgl.cmake | 55 | ||||
| -rw-r--r-- | include/mbgl/storage/default_file_source.hpp | 15 | ||||
| -rw-r--r-- | include/mbgl/storage/offline.hpp | 2 | ||||
| -rw-r--r-- | platform/android/config.cmake | 1 | ||||
| -rw-r--r-- | platform/android/src/offline/offline_manager.cpp | 23 | ||||
| -rw-r--r-- | platform/android/src/offline/offline_region.cpp | 20 | ||||
| -rw-r--r-- | platform/darwin/src/MGLOfflinePack.mm | 2 | ||||
| -rw-r--r-- | platform/darwin/src/MGLOfflineStorage.mm | 24 | ||||
| -rw-r--r-- | platform/default/default_file_source.cpp | 58 | ||||
| -rw-r--r-- | platform/default/mbgl/storage/offline_database.cpp | 35 | ||||
| -rw-r--r-- | platform/default/mbgl/storage/offline_database.hpp | 16 | ||||
| -rw-r--r-- | platform/ios/ios.xcodeproj/project.pbxproj | 10 | ||||
| -rw-r--r-- | scripts/config.xcconfig.in | 8 | ||||
| -rw-r--r-- | test/storage/offline_database.test.cpp | 37 | ||||
| -rw-r--r-- | test/storage/offline_download.test.cpp | 2 |
19 files changed, 156 insertions, 170 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index b6b86d9a8d..a02b4b9173 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -176,8 +176,10 @@ if(WITH_NODEJS AND COMMAND mbgl_platform_node) endif() if(CMAKE_GENERATOR STREQUAL "Xcode") - write_xcconfig_target_properties( - mbgl-core - mbgl-filesource + set_xcconfig_target_properties(mbgl-core) + set_xcconfig_target_properties(mbgl-filesource) + file(GENERATE + OUTPUT "${CMAKE_BINARY_DIR}/config.xcconfig" + INPUT "${CMAKE_SOURCE_DIR}/scripts/config.xcconfig.in" ) endif() diff --git a/bin/offline.cpp b/bin/offline.cpp index 8b42ab7b72..603f0b848a 100644 --- a/bin/offline.cpp +++ b/bin/offline.cpp @@ -136,9 +136,9 @@ int main(int argc, char *argv[]) { std::signal(SIGINT, [] (int) { stop(); }); - fileSource.createOfflineRegion(definition, metadata, [&] (std::exception_ptr error, optional<OfflineRegion> region_) { - if (error) { - std::cerr << "Error creating region: " << util::toString(error) << std::endl; + fileSource.createOfflineRegion(definition, metadata, [&] (mbgl::expected<OfflineRegion, std::exception_ptr> region_) { + if (!region_) { + std::cerr << "Error creating region: " << util::toString(region_.error()) << std::endl; loop.stop(); exit(1); } else { diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index e875f5b142..02335fef7a 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -664,6 +664,7 @@ set(MBGL_CORE_FILES include/mbgl/util/enum.hpp include/mbgl/util/event.hpp include/mbgl/util/exception.hpp + include/mbgl/util/expected.hpp include/mbgl/util/feature.hpp include/mbgl/util/font_stack.hpp include/mbgl/util/geo.hpp diff --git a/cmake/filesource.cmake b/cmake/filesource.cmake index ccd2192f39..9b7a4a1138 100644 --- a/cmake/filesource.cmake +++ b/cmake/filesource.cmake @@ -1,3 +1,5 @@ +add_vendor_target(expected INTERFACE) + add_library(mbgl-filesource STATIC # File source include/mbgl/storage/default_file_source.hpp @@ -39,6 +41,7 @@ target_include_directories(mbgl-filesource target_link_libraries(mbgl-filesource PUBLIC mbgl-core + PUBLIC expected ) mbgl_filesource() diff --git a/cmake/mbgl.cmake b/cmake/mbgl.cmake index bb029a47db..3abc974feb 100644 --- a/cmake/mbgl.cmake +++ b/cmake/mbgl.cmake @@ -115,13 +115,15 @@ endfunction() # Creates a library target for a vendored dependency function(add_vendor_target NAME TYPE) - add_library(${NAME} ${TYPE} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/empty.cpp") set(INCLUDE_TYPE "INTERFACE") set(SOURCE_TYPE "INTERFACE") if (TYPE STREQUAL "STATIC" OR TYPE STREQUAL "SHARED") + add_library(${NAME} ${TYPE} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/empty.cpp") set(INCLUDE_TYPE "PUBLIC") set(SOURCE_TYPE "PRIVATE") set_target_properties(${NAME} PROPERTIES SOURCES "") + else() + add_library(${NAME} ${TYPE}) endif() set_target_properties(${NAME} PROPERTIES INTERFACE_SOURCES "") file(STRINGS "${CMAKE_CURRENT_SOURCE_DIR}/vendor/${NAME}/files.txt" FILES) @@ -137,48 +139,25 @@ macro(set_xcode_property TARGET XCODE_PROPERTY XCODE_VALUE) set_property(TARGET ${TARGET} PROPERTY XCODE_ATTRIBUTE_${XCODE_PROPERTY} ${XCODE_VALUE}) endmacro (set_xcode_property) -function(_get_xcconfig_property target var) - get_property(result TARGET ${target} PROPERTY INTERFACE_${var} SET) - if (result) - get_property(result TARGET ${target} PROPERTY INTERFACE_${var}) - if (var STREQUAL "LINK_LIBRARIES") - # Remove target names from the list of linker flags, since Xcode can't deal with them. - set(link_flags) - foreach(item IN LISTS result) - if (NOT TARGET ${item}) - list(APPEND link_flags ${item}) - endif() - endforeach() - set(result "${link_flags}") +function(set_xcconfig_target_properties target) + # Create a list of linked libraries for use in the xcconfig generation script. + get_property(result TARGET ${target} PROPERTY INTERFACE_LINK_LIBRARIES) + string(GENEX_STRIP "${result}" result) + # Remove target names from the list of linker flags, since Xcode can't deal with them. + set(link_flags) + foreach(item IN LISTS result) + if (NOT TARGET ${item}) + list(APPEND link_flags ${item}) endif() - string(REPLACE ";-framework " ";-framework;" result "${result}") - string(REPLACE ";" "\" \"" result "${result}") - string(REPLACE "-" "_" target "${target}") - set(${target}_${var} "${result}" PARENT_SCOPE) - endif() -endfunction() - -if(MBGL_PLATFORM STREQUAL "ios") - execute_process( - COMMAND git submodule update --init platform/ios/vendor/mapbox-events-ios - WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}") -endif() - -function(write_xcconfig_target_properties) - foreach(target ${ARGN}) - _get_xcconfig_property(${target} INCLUDE_DIRECTORIES) - _get_xcconfig_property(${target} LINK_LIBRARIES) endforeach() - configure_file( - "${CMAKE_SOURCE_DIR}/scripts/config.xcconfig.in" - "${CMAKE_BINARY_DIR}/config.xcconfig" - @ONLY - ) + string(REPLACE ";-framework " ";-framework;" link_flags "${link_flags}") + string(REPLACE ";" "\" \"" link_flags "${link_flags}") + set_xcode_property(${target} XCCONFIG_LINK_LIBRARIES "${link_flags}") endfunction() # Set Xcode project build settings to be consistent with the CXX flags we're # using. (Otherwise, Xcode's defaults may override some of these.) -macro(initialize_xcode_cxx_build_settings target) +function(initialize_xcode_cxx_build_settings target) # -Wall set_xcode_property(${target} GCC_WARN_SIGN_COMPARE YES) set_xcode_property(${target} GCC_WARN_UNINITIALIZED_AUTOS YES) @@ -206,7 +185,7 @@ macro(initialize_xcode_cxx_build_settings target) # -flto set_xcode_property(${target} LLVM_LTO $<$<OR:$<CONFIG:Release>,$<CONFIG:RelWithDebugInfo>>:YES>) -endmacro(initialize_xcode_cxx_build_settings) +endfunction() # CMake 3.1 does not have this yet. set(CMAKE_CXX14_STANDARD_COMPILE_OPTION "-std=c++14") diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index b9c8de5052..e048d82af2 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -5,6 +5,7 @@ #include <mbgl/storage/offline.hpp> #include <mbgl/util/constants.hpp> #include <mbgl/util/optional.hpp> +#include <mbgl/util/expected.hpp> #include <vector> #include <mutex> @@ -55,8 +56,7 @@ public: * callback, which will be executed on the database thread; it is the responsibility * of the SDK bindings to re-execute a user-provided callback on the main thread. */ - void listOfflineRegions(std::function<void (std::exception_ptr, - optional<std::vector<OfflineRegion>>)>); + void listOfflineRegions(std::function<void (expected<OfflineRegions, std::exception_ptr>)>); /* * Create an offline region in the database. @@ -71,16 +71,14 @@ public: */ void createOfflineRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function<void (std::exception_ptr, - optional<OfflineRegion>)>); + std::function<void (expected<OfflineRegion, std::exception_ptr>)>); /* * Update an offline region metadata in the database. */ void updateOfflineMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function<void (std::exception_ptr, - optional<OfflineRegionMetadata>)>); + std::function<void (expected<OfflineRegionMetadata, std::exception_ptr>)>); /* * Register an observer to be notified when the state of the region changes. */ @@ -97,8 +95,9 @@ public: * executed on the database thread; it is the responsibility of the SDK bindings * to re-execute a user-provided callback on the main thread. */ - void getOfflineRegionStatus(OfflineRegion&, std::function<void (std::exception_ptr, - optional<OfflineRegionStatus>)>) const; + void getOfflineRegionStatus( + OfflineRegion&, + std::function<void (expected<OfflineRegionStatus, std::exception_ptr>)>) const; /* * Remove an offline region from the database and perform any resources evictions diff --git a/include/mbgl/storage/offline.hpp b/include/mbgl/storage/offline.hpp index 2193f8d09e..62353446fa 100644 --- a/include/mbgl/storage/offline.hpp +++ b/include/mbgl/storage/offline.hpp @@ -210,4 +210,6 @@ private: const OfflineRegionMetadata metadata; }; +using OfflineRegions = std::vector<OfflineRegion>; + } // namespace mbgl diff --git a/platform/android/config.cmake b/platform/android/config.cmake index 001a45e613..bd65bc517f 100644 --- a/platform/android/config.cmake +++ b/platform/android/config.cmake @@ -97,6 +97,7 @@ macro(mbgl_platform_core) target_link_libraries(mbgl-core PRIVATE nunicode + PUBLIC expected PUBLIC -llog PUBLIC -landroid PUBLIC -ljnigraphics diff --git a/platform/android/src/offline/offline_manager.cpp b/platform/android/src/offline/offline_manager.cpp index 4960ae2845..4f94a1c3a5 100644 --- a/platform/android/src/offline/offline_manager.cpp +++ b/platform/android/src/offline/offline_manager.cpp @@ -26,15 +26,18 @@ void OfflineManager::listOfflineRegions(jni::JNIEnv& env_, jni::Object<FileSourc //Keep a shared ptr to a global reference of the callback and file source so they are not GC'd in the meanwhile callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()), jFileSource = std::shared_ptr<jni::jobject>(jFileSource_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional<std::vector<mbgl::OfflineRegion>> regions) mutable { + ](mbgl::expected<mbgl::OfflineRegions, std::exception_ptr> regions) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineManager::ListOfflineRegionsCallback::onError(*env, jni::Object<ListOfflineRegionsCallback>(*callback), error); - } else if (regions) { - OfflineManager::ListOfflineRegionsCallback::onList(*env, jni::Object<FileSource>(*jFileSource), jni::Object<ListOfflineRegionsCallback>(*callback), std::move(regions)); + if (regions) { + OfflineManager::ListOfflineRegionsCallback::onList( + *env, jni::Object<FileSource>(*jFileSource), + jni::Object<ListOfflineRegionsCallback>(*callback), std::move(*regions)); + } else { + OfflineManager::ListOfflineRegionsCallback::onError( + *env, jni::Object<ListOfflineRegionsCallback>(*callback), regions.error()); } }); } @@ -59,19 +62,19 @@ void OfflineManager::createOfflineRegion(jni::JNIEnv& env_, //Keep a shared ptr to a global reference of the callback and file source so they are not GC'd in the meanwhile callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()), jFileSource = std::shared_ptr<jni::jobject>(jFileSource_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional<mbgl::OfflineRegion> region) mutable { + ](mbgl::expected<mbgl::OfflineRegion, std::exception_ptr> region) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineManager::CreateOfflineRegionCallback::onError(*env, jni::Object<CreateOfflineRegionCallback>(*callback), error); - } else if (region) { + if (region) { OfflineManager::CreateOfflineRegionCallback::onCreate( *env, jni::Object<FileSource>(*jFileSource), - jni::Object<CreateOfflineRegionCallback>(*callback), std::move(region) + jni::Object<CreateOfflineRegionCallback>(*callback), std::move(*region) ); + } else { + OfflineManager::CreateOfflineRegionCallback::onError(*env, jni::Object<CreateOfflineRegionCallback>(*callback), region.error()); } }); } diff --git a/platform/android/src/offline/offline_region.cpp b/platform/android/src/offline/offline_region.cpp index 27de76fb00..fe4dbecf14 100644 --- a/platform/android/src/offline/offline_region.cpp +++ b/platform/android/src/offline/offline_region.cpp @@ -107,14 +107,14 @@ void OfflineRegion::getOfflineRegionStatus(jni::JNIEnv& env_, jni::Object<Offlin fileSource.getOfflineRegionStatus(*region, [ //Ensure the object is not gc'd in the meanwhile callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional<mbgl::OfflineRegionStatus> status) mutable { + ](mbgl::expected<mbgl::OfflineRegionStatus, std::exception_ptr> status) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineRegionStatusCallback::onError(*env, jni::Object<OfflineRegionStatusCallback>(*callback), error); - } else if (status) { - OfflineRegionStatusCallback::onStatus(*env, jni::Object<OfflineRegionStatusCallback>(*callback), std::move(status)); + if (status) { + OfflineRegionStatusCallback::onStatus(*env, jni::Object<OfflineRegionStatusCallback>(*callback), std::move(*status)); + } else { + OfflineRegionStatusCallback::onError(*env, jni::Object<OfflineRegionStatusCallback>(*callback), status.error()); } }); } @@ -144,14 +144,14 @@ void OfflineRegion::updateOfflineRegionMetadata(jni::JNIEnv& env_, jni::Array<jn fileSource.updateOfflineMetadata(region->getID(), metadata, [ //Ensure the object is not gc'd in the meanwhile callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional<mbgl::OfflineRegionMetadata> data) mutable { + ](mbgl::expected<mbgl::OfflineRegionMetadata, std::exception_ptr> data) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineRegionUpdateMetadataCallback::onError(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), error); - } else if (data) { - OfflineRegionUpdateMetadataCallback::onUpdate(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), std::move(data)); + if (data) { + OfflineRegionUpdateMetadataCallback::onUpdate(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), std::move(*data)); + } else { + OfflineRegionUpdateMetadataCallback::onError(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), data.error()); } }); } diff --git a/platform/darwin/src/MGLOfflinePack.mm b/platform/darwin/src/MGLOfflinePack.mm index 9d903ee841..7bbc681c88 100644 --- a/platform/darwin/src/MGLOfflinePack.mm +++ b/platform/darwin/src/MGLOfflinePack.mm @@ -141,7 +141,7 @@ private: mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource]; __weak MGLOfflinePack *weakSelf = self; - mbglFileSource->getOfflineRegionStatus(*_mbglOfflineRegion, [&, weakSelf](__unused std::exception_ptr exception, mbgl::optional<mbgl::OfflineRegionStatus> status) { + mbglFileSource->getOfflineRegionStatus(*_mbglOfflineRegion, [&, weakSelf](mbgl::expected<mbgl::OfflineRegionStatus, std::exception_ptr> status) { if (status) { mbgl::OfflineRegionStatus checkedStatus = *status; dispatch_async(dispatch_get_main_queue(), ^{ diff --git a/platform/darwin/src/MGLOfflineStorage.mm b/platform/darwin/src/MGLOfflineStorage.mm index e6c10e942b..05e1b06338 100644 --- a/platform/darwin/src/MGLOfflineStorage.mm +++ b/platform/darwin/src/MGLOfflineStorage.mm @@ -288,16 +288,16 @@ const MGLExceptionName MGLUnsupportedRegionTypeException = @"MGLUnsupportedRegio const mbgl::OfflineTilePyramidRegionDefinition regionDefinition = [(id <MGLOfflineRegion_Private>)region offlineRegionDefinition]; mbgl::OfflineRegionMetadata metadata(context.length); [context getBytes:&metadata[0] length:metadata.size()]; - self.mbglFileSource->createOfflineRegion(regionDefinition, metadata, [&, completion](std::exception_ptr exception, mbgl::optional<mbgl::OfflineRegion> mbglOfflineRegion) { + self.mbglFileSource->createOfflineRegion(regionDefinition, metadata, [&, completion](mbgl::expected<mbgl::OfflineRegion, std::exception_ptr> mbglOfflineRegion) { NSError *error; - if (exception) { - NSString *errorDescription = @(mbgl::util::toString(exception).c_str()); + if (!mbglOfflineRegion) { + NSString *errorDescription = @(mbgl::util::toString(mbglOfflineRegion.error()).c_str()); error = [NSError errorWithDomain:MGLErrorDomain code:-1 userInfo:errorDescription ? @{ NSLocalizedDescriptionKey: errorDescription, } : nil]; } if (completion) { - MGLOfflinePack *pack = mbglOfflineRegion ? [[MGLOfflinePack alloc] initWithMBGLRegion:new mbgl::OfflineRegion(std::move(*mbglOfflineRegion))] : nil; + MGLOfflinePack *pack = mbglOfflineRegion ? [[MGLOfflinePack alloc] initWithMBGLRegion:new mbgl::OfflineRegion(std::move(mbglOfflineRegion.value()))] : nil; dispatch_async(dispatch_get_main_queue(), [&, completion, error, pack](void) { completion(pack, error); }); @@ -347,17 +347,17 @@ const MGLExceptionName MGLUnsupportedRegionTypeException = @"MGLUnsupportedRegio } - (void)getPacksWithCompletionHandler:(void (^)(NSArray<MGLOfflinePack *> *packs, NSError * _Nullable error))completion { - self.mbglFileSource->listOfflineRegions([&, completion](std::exception_ptr exception, mbgl::optional<std::vector<mbgl::OfflineRegion>> regions) { + self.mbglFileSource->listOfflineRegions([&, completion](mbgl::expected<mbgl::OfflineRegions, std::exception_ptr> result) { NSError *error; - if (exception) { + NSMutableArray *packs; + if (!result) { error = [NSError errorWithDomain:MGLErrorDomain code:-1 userInfo:@{ - NSLocalizedDescriptionKey: @(mbgl::util::toString(exception).c_str()), + NSLocalizedDescriptionKey: @(mbgl::util::toString(result.error()).c_str()), }]; - } - NSMutableArray *packs; - if (regions) { - packs = [NSMutableArray arrayWithCapacity:regions->size()]; - for (mbgl::OfflineRegion ®ion : *regions) { + } else { + auto& regions = result.value(); + packs = [NSMutableArray arrayWithCapacity:regions.size()]; + for (auto ®ion : regions) { MGLOfflinePack *pack = [[MGLOfflinePack alloc] initWithMBGLRegion:new mbgl::OfflineRegion(std::move(region))]; [packs addObject:pack]; } diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp index 051e1b125c..93f10eea72 100644 --- a/platform/default/default_file_source.cpp +++ b/platform/default/default_file_source.cpp @@ -45,61 +45,44 @@ public: onlineFileSource.setResourceTransform(std::move(transform)); } - void listRegions(std::function<void (std::exception_ptr, optional<std::vector<OfflineRegion>>)> callback) { - try { - callback({}, offlineDatabase->listRegions()); - } catch (...) { - callback(std::current_exception(), {}); - } + void listRegions(std::function<void (expected<OfflineRegions, std::exception_ptr>)> callback) { + callback(offlineDatabase->listRegions()); } void createRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function<void (std::exception_ptr, optional<OfflineRegion>)> callback) { - try { - callback({}, offlineDatabase->createRegion(definition, metadata)); - } catch (...) { - callback(std::current_exception(), {}); - } + std::function<void (expected<OfflineRegion, std::exception_ptr>)> callback) { + callback(offlineDatabase->createRegion(definition, metadata)); } void updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function<void (std::exception_ptr, optional<OfflineRegionMetadata>)> callback) { - try { - callback({}, offlineDatabase->updateMetadata(regionID, metadata)); - } catch (...) { - callback(std::current_exception(), {}); - } + std::function<void (expected<OfflineRegionMetadata, std::exception_ptr>)> callback) { + callback(offlineDatabase->updateMetadata(regionID, metadata)); } - void getRegionStatus(int64_t regionID, std::function<void (std::exception_ptr, optional<OfflineRegionStatus>)> callback) { + void getRegionStatus(int64_t regionID, std::function<void (expected<OfflineRegionStatus, std::exception_ptr>)> callback) { if (auto download = getDownload(regionID)) { - callback({}, download->getStatus()); + callback(download.value()->getStatus()); } else { - callback(std::current_exception(), {}); + callback(unexpected<std::exception_ptr>(download.error())); } } void deleteRegion(OfflineRegion&& region, std::function<void (std::exception_ptr)> callback) { - try { - downloads.erase(region.getID()); - offlineDatabase->deleteRegion(std::move(region)); - callback({}); - } catch (...) { - callback(std::current_exception()); - } + downloads.erase(region.getID()); + callback(offlineDatabase->deleteRegion(std::move(region))); } void setRegionObserver(int64_t regionID, std::unique_ptr<OfflineRegionObserver> observer) { if (auto download = getDownload(regionID)) { - download->setObserver(std::move(observer)); + download.value()->setObserver(std::move(observer)); } } void setRegionDownloadState(int64_t regionID, OfflineRegionDownloadState state) { if (auto download = getDownload(regionID)) { - download->setState(state); + download.value()->setState(state); } } @@ -185,17 +168,16 @@ public: } private: - OfflineDownload* getDownload(int64_t regionID) { + expected<OfflineDownload*, std::exception_ptr> getDownload(int64_t regionID) { auto it = downloads.find(regionID); if (it != downloads.end()) { return it->second.get(); } auto definition = offlineDatabase->getRegionDefinition(regionID); if (!definition) { - return nullptr; + return unexpected<std::exception_ptr>(definition.error()); } - - auto download = std::make_unique<OfflineDownload>(regionID, std::move(*definition), + auto download = std::make_unique<OfflineDownload>(regionID, std::move(definition.value()), *offlineDatabase, onlineFileSource); return downloads.emplace(regionID, std::move(download)).first->second.get(); } @@ -266,19 +248,19 @@ std::unique_ptr<AsyncRequest> DefaultFileSource::request(const Resource& resourc return std::move(req); } -void DefaultFileSource::listOfflineRegions(std::function<void (std::exception_ptr, optional<std::vector<OfflineRegion>>)> callback) { +void DefaultFileSource::listOfflineRegions(std::function<void (expected<OfflineRegions, std::exception_ptr>)> callback) { impl->actor().invoke(&Impl::listRegions, callback); } void DefaultFileSource::createOfflineRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function<void (std::exception_ptr, optional<OfflineRegion>)> callback) { + std::function<void (expected<OfflineRegion, std::exception_ptr>)> callback) { impl->actor().invoke(&Impl::createRegion, definition, metadata, callback); } void DefaultFileSource::updateOfflineMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function<void (std::exception_ptr, optional<OfflineRegionMetadata>)> callback) { + std::function<void (expected<OfflineRegionMetadata, std::exception_ptr>)> callback) { impl->actor().invoke(&Impl::updateMetadata, regionID, metadata, callback); } @@ -294,7 +276,7 @@ void DefaultFileSource::setOfflineRegionDownloadState(OfflineRegion& region, Off impl->actor().invoke(&Impl::setRegionDownloadState, region.getID(), state); } -void DefaultFileSource::getOfflineRegionStatus(OfflineRegion& region, std::function<void (std::exception_ptr, optional<OfflineRegionStatus>)> callback) const { +void DefaultFileSource::getOfflineRegionStatus(OfflineRegion& region, std::function<void (expected<OfflineRegionStatus, std::exception_ptr>)> callback) const { impl->actor().invoke(&Impl::getRegionStatus, region.getID(), callback); } diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index cfa7f52977..7516185f27 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -592,14 +592,15 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile, return true; } -std::vector<OfflineRegion> OfflineDatabase::listRegions() try { +expected<OfflineRegions, std::exception_ptr> OfflineDatabase::listRegions() try { mapbox::sqlite::Query query{ getStatement("SELECT id, definition, description FROM regions") }; - std::vector<OfflineRegion> result; + OfflineRegions result; while (query.run()) { const auto id = query.get<int64_t>(0); const auto definition = query.get<std::string>(1); const auto description = query.get<std::vector<uint8_t>>(2); try { + // Construct, then move because this constructor is private. OfflineRegion region(id, decodeOfflineRegionDefinition(definition), description); result.emplace_back(std::move(region)); } catch (const std::exception& ex) { @@ -608,14 +609,16 @@ std::vector<OfflineRegion> OfflineDatabase::listRegions() try { Log::Error(Event::General, "%s", ex.what()); } } - return result; + // Explicit move to avoid triggering the copy constructor. + return { std::move(result) }; } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "list regions"); - return {}; + return unexpected<std::exception_ptr>(std::current_exception()); } -optional<OfflineRegion> OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, - const OfflineRegionMetadata& metadata) try { +expected<OfflineRegion, std::exception_ptr> +OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, + const OfflineRegionMetadata& metadata) try { // clang-format off mapbox::sqlite::Query query{ getStatement( "INSERT INTO regions (definition, description) " @@ -628,10 +631,11 @@ optional<OfflineRegion> OfflineDatabase::createRegion(const OfflineRegionDefinit return OfflineRegion(query.lastInsertRowId(), definition, metadata); } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "create region"); - return nullopt; + return unexpected<std::exception_ptr>(std::current_exception()); } -optional<OfflineRegionMetadata> OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) try { +expected<OfflineRegionMetadata, std::exception_ptr> +OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) try { // clang-format off mapbox::sqlite::Query query{ getStatement( "UPDATE regions SET description = ?1 " @@ -644,10 +648,10 @@ optional<OfflineRegionMetadata> OfflineDatabase::updateMetadata(const int64_t re return metadata; } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "update region metadata"); - return nullopt; + return unexpected<std::exception_ptr>(std::current_exception()); } -void OfflineDatabase::deleteRegion(OfflineRegion&& region) try { +std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try { { mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") }; query.bind(1, region.getID()); @@ -660,9 +664,10 @@ void OfflineDatabase::deleteRegion(OfflineRegion&& region) try { // Ensure that the cached offlineTileCount value is recalculated. offlineMapboxTileCount = {}; + return nullptr; } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "delete region"); - return; + return std::current_exception(); } optional<std::pair<Response, uint64_t>> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) try { @@ -848,7 +853,7 @@ bool OfflineDatabase::markUsed(int64_t regionID, const Resource& resource) { } } -optional<OfflineRegionDefinition> OfflineDatabase::getRegionDefinition(int64_t regionID) try { +expected<OfflineRegionDefinition, std::exception_ptr> OfflineDatabase::getRegionDefinition(int64_t regionID) try { mapbox::sqlite::Query query{ getStatement("SELECT definition FROM regions WHERE id = ?1") }; query.bind(1, regionID); query.run(); @@ -856,10 +861,10 @@ optional<OfflineRegionDefinition> OfflineDatabase::getRegionDefinition(int64_t r return decodeOfflineRegionDefinition(query.get<std::string>(0)); } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "load region"); - return nullopt; + return unexpected<std::exception_ptr>(std::current_exception()); } -optional<OfflineRegionStatus> OfflineDatabase::getRegionCompletedStatus(int64_t regionID) try { +expected<OfflineRegionStatus, std::exception_ptr> OfflineDatabase::getRegionCompletedStatus(int64_t regionID) try { OfflineRegionStatus result; std::tie(result.completedResourceCount, result.completedResourceSize) @@ -873,7 +878,7 @@ optional<OfflineRegionStatus> OfflineDatabase::getRegionCompletedStatus(int64_t return result; } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "get region status"); - return nullopt; + return unexpected<std::exception_ptr>(std::current_exception()); } std::pair<int64_t, int64_t> OfflineDatabase::getCompletedResourceCountAndSize(int64_t regionID) { diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp index cdad11b79e..fbe6c707e1 100644 --- a/platform/default/mbgl/storage/offline_database.hpp +++ b/platform/default/mbgl/storage/offline_database.hpp @@ -7,6 +7,7 @@ #include <mbgl/util/optional.hpp> #include <mbgl/util/constants.hpp> #include <mbgl/util/mapbox.hpp> +#include <mbgl/util/expected.hpp> #include <unordered_map> #include <memory> @@ -47,14 +48,15 @@ public: // Return value is (inserted, stored size) std::pair<bool, uint64_t> put(const Resource&, const Response&); - std::vector<OfflineRegion> listRegions(); + expected<OfflineRegions, std::exception_ptr> listRegions(); - optional<OfflineRegion> createRegion(const OfflineRegionDefinition&, - const OfflineRegionMetadata&); + expected<OfflineRegion, std::exception_ptr> createRegion(const OfflineRegionDefinition&, + const OfflineRegionMetadata&); - optional<OfflineRegionMetadata> updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); + expected<OfflineRegionMetadata, std::exception_ptr> + updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); - void deleteRegion(OfflineRegion&&); + std::exception_ptr deleteRegion(OfflineRegion&&); // Return value is (response, stored size) optional<std::pair<Response, uint64_t>> getRegionResource(int64_t regionID, const Resource&); @@ -62,8 +64,8 @@ public: uint64_t putRegionResource(int64_t regionID, const Resource&, const Response&); void putRegionResources(int64_t regionID, const std::list<std::tuple<Resource, Response>>&, OfflineRegionStatus&); - optional<OfflineRegionDefinition> getRegionDefinition(int64_t regionID); - optional<OfflineRegionStatus> getRegionCompletedStatus(int64_t regionID); + expected<OfflineRegionDefinition, std::exception_ptr> getRegionDefinition(int64_t regionID); + expected<OfflineRegionStatus, std::exception_ptr> getRegionCompletedStatus(int64_t regionID); void setOfflineMapboxTileCountLimit(uint64_t); uint64_t getOfflineMapboxTileCountLimit(); diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj index b77c67fbb3..48329cf63a 100644 --- a/platform/ios/ios.xcodeproj/project.pbxproj +++ b/platform/ios/ios.xcodeproj/project.pbxproj @@ -3690,7 +3690,10 @@ baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */; buildSettings = { CLANG_ENABLE_MODULES = YES; - HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)"; + HEADER_SEARCH_PATHS = ( + "$(mbgl_core_INCLUDE_DIRECTORIES)", + "$(mbgl_filesource_INCLUDE_DIRECTORIES)", + ); INFOPLIST_FILE = test/Info.plist; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; OTHER_CFLAGS = "-fvisibility=hidden"; @@ -3714,7 +3717,10 @@ baseConfigurationReference = 55D8C9941D0F133500F42F10 /* config.xcconfig */; buildSettings = { CLANG_ENABLE_MODULES = YES; - HEADER_SEARCH_PATHS = "$(mbgl_core_INCLUDE_DIRECTORIES)"; + HEADER_SEARCH_PATHS = ( + "$(mbgl_core_INCLUDE_DIRECTORIES)", + "$(mbgl_filesource_INCLUDE_DIRECTORIES)", + ); INFOPLIST_FILE = test/Info.plist; LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks @loader_path/Frameworks"; OTHER_CFLAGS = "-fvisibility=hidden"; diff --git a/scripts/config.xcconfig.in b/scripts/config.xcconfig.in index 357732c9ae..69ca2424a1 100644 --- a/scripts/config.xcconfig.in +++ b/scripts/config.xcconfig.in @@ -1,9 +1,9 @@ // Do not edit -- generated by CMake // mbgl-core -mbgl_core_INCLUDE_DIRECTORIES = "@mbgl_core_INCLUDE_DIRECTORIES@" -mbgl_core_LINK_LIBRARIES = "@mbgl_core_LINK_LIBRARIES@" +mbgl_core_INCLUDE_DIRECTORIES = "$<JOIN:$<TARGET_PROPERTY:mbgl-core,INTERFACE_INCLUDE_DIRECTORIES>," ">" +mbgl_core_LINK_LIBRARIES = "$<TARGET_PROPERTY:mbgl-core,XCODE_ATTRIBUTE_XCCONFIG_LINK_LIBRARIES>" // mbgl-filesource -mbgl_filesource_INCLUDE_DIRECTORIES = "@mbgl_filesource_INCLUDE_DIRECTORIES@" -mbgl_filesource_LINK_LIBRARIES = "@mbgl_filesource_LINK_LIBRARIES@" +mbgl_filesource_INCLUDE_DIRECTORIES = "$<JOIN:$<TARGET_PROPERTY:mbgl-filesource,INTERFACE_INCLUDE_DIRECTORIES>," ">" +mbgl_filesource_LINK_LIBRARIES = "$<TARGET_PROPERTY:mbgl-filesource,XCODE_ATTRIBUTE_XCCONFIG_LINK_LIBRARIES>" diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index de40d8caf7..a1fd2ea619 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -436,7 +436,8 @@ TEST(OfflineDatabase, UpdateMetadata) { OfflineRegionMetadata newmetadata {{ 4, 5, 6 }}; db.updateMetadata(region->getID(), newmetadata); - EXPECT_EQ(db.listRegions().at(0).getMetadata(), newmetadata); + auto regions = db.listRegions().value(); + EXPECT_EQ(regions.at(0).getMetadata(), newmetadata); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -449,7 +450,7 @@ TEST(OfflineDatabase, ListRegions) { auto region = db.createRegion(definition, metadata); ASSERT_TRUE(region); - std::vector<OfflineRegion> regions = db.listRegions(); + auto regions = db.listRegions().value(); ASSERT_EQ(1u, regions.size()); EXPECT_EQ(region->getID(), regions.at(0).getID()); @@ -499,7 +500,8 @@ TEST(OfflineDatabase, DeleteRegion) { db.deleteRegion(std::move(*region)); - ASSERT_EQ(0u, db.listRegions().size()); + auto regions = db.listRegions().value(); + ASSERT_EQ(0u, regions.size()); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -841,10 +843,9 @@ TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) { EXPECT_EQ(0u, status.completedTileCount); EXPECT_EQ(0u, status.completedResourceCount); - const auto completedStatus = db.getRegionCompletedStatus(region->getID()); - ASSERT_TRUE(completedStatus); - EXPECT_EQ(1u, completedStatus->completedTileCount); - EXPECT_EQ(2u, completedStatus->completedResourceCount); + const auto completedStatus = db.getRegionCompletedStatus(region->getID()).value(); + EXPECT_EQ(1u, completedStatus.completedTileCount); + EXPECT_EQ(2u, completedStatus.completedResourceCount); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -858,7 +859,8 @@ TEST(OfflineDatabase, MigrateFromV2Schema) { { OfflineDatabase db(filename, 0); auto regions = db.listRegions(); - for (auto& region : regions) { + ASSERT_TRUE(regions); + for (auto& region : regions.value()) { db.deleteRegion(std::move(region)); } } @@ -878,7 +880,7 @@ TEST(OfflineDatabase, MigrateFromV3Schema) { { OfflineDatabase db(filename, 0); - auto regions = db.listRegions(); + auto regions = db.listRegions().value(); for (auto& region : regions) { db.deleteRegion(std::move(region)); } @@ -897,7 +899,7 @@ TEST(OfflineDatabase, MigrateFromV4Schema) { { OfflineDatabase db(filename, 0); - auto regions = db.listRegions(); + auto regions = db.listRegions().value(); for (auto& region : regions) { db.deleteRegion(std::move(region)); } @@ -923,7 +925,7 @@ TEST(OfflineDatabase, MigrateFromV5Schema) { { OfflineDatabase db(filename, 0); - auto regions = db.listRegions(); + auto regions = db.listRegions().value(); for (auto& region : regions) { db.deleteRegion(std::move(region)); } @@ -1046,16 +1048,15 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DisallowedIO)) { EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write resource: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); - const auto regions = db.listRegions(); - EXPECT_TRUE(regions.empty()); + EXPECT_FALSE(db.listRegions()); EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't list regions: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); - EXPECT_EQ(nullopt, db.createRegion(definition, {})); + EXPECT_FALSE(db.createRegion(definition, {})); EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't create region: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); - EXPECT_EQ(nullopt, db.updateMetadata(region->getID(), {})); + EXPECT_FALSE(db.updateMetadata(region->getID(), {})); EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update region metadata: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); @@ -1077,15 +1078,15 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DisallowedIO)) { EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write region resources: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); - EXPECT_EQ(nullopt, db.getRegionDefinition(region->getID())); + EXPECT_FALSE(db.getRegionDefinition(region->getID())); EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't load region: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); - EXPECT_EQ(nullopt, db.getRegionCompletedStatus(region->getID())); + EXPECT_FALSE(db.getRegionCompletedStatus(region->getID())); EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't get region status: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); - db.deleteRegion(std::move(*region)); + EXPECT_TRUE(db.deleteRegion(std::move(*region))); EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't delete region: authorization denied"))); EXPECT_EQ(0u, log.uncheckedCount()); diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index f979d42601..93b4dd623a 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -65,7 +65,7 @@ public: OfflineDatabase db; std::size_t size = 0; - optional<OfflineRegion> createRegion() { + auto createRegion() { OfflineRegionDefinition definition { "", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 1.0 }; OfflineRegionMetadata metadata; return db.createRegion(definition, metadata); |