From 83f0807e5f978e1d9400a075c8270edc2a887e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Fri, 10 Aug 2018 10:22:38 -0400 Subject: [PATCH 1/6] [build] vendor std::expected polyfill --- include/mbgl/util/expected.hpp | 16 + scripts/vendor/expected.sh | 13 + vendor/expected/LICENSE.txt | 23 + vendor/expected/files.txt | 1 + vendor/expected/include/expected.hpp | 1708 ++++++++++++++++++++++++++ vendor/expected/version.txt | 1 + 6 files changed, 1762 insertions(+) create mode 100644 include/mbgl/util/expected.hpp create mode 100755 scripts/vendor/expected.sh create mode 100644 vendor/expected/LICENSE.txt create mode 100644 vendor/expected/files.txt create mode 100644 vendor/expected/include/expected.hpp create mode 100644 vendor/expected/version.txt diff --git a/include/mbgl/util/expected.hpp b/include/mbgl/util/expected.hpp new file mode 100644 index 00000000000..a45f071065b --- /dev/null +++ b/include/mbgl/util/expected.hpp @@ -0,0 +1,16 @@ +#pragma once + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wshadow" +#include +#pragma GCC diagnostic pop + +namespace mbgl { + +template +using expected = nonstd::expected; + +template +using unexpected = nonstd::unexpected_type; + +} // namespace mbgl diff --git a/scripts/vendor/expected.sh b/scripts/vendor/expected.sh new file mode 100755 index 00000000000..58c98902f6a --- /dev/null +++ b/scripts/vendor/expected.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +source "$(dirname "${BASH_SOURCE[0]}")/common.sh" + +NAME=expected +VERSION=a2359cf61478d735a308e952b1c9840714f61386 +ROOT=expected-lite-$VERSION + +download "https://github.com/martinmoene/expected-lite/archive/$VERSION.tar.gz" +init +extract "$ROOT/include" "$ROOT/LICENSE.txt" +mv include/nonstd/expected.hpp include +rm -rf include/nonstd +file_list include -name "*.hpp" diff --git a/vendor/expected/LICENSE.txt b/vendor/expected/LICENSE.txt new file mode 100644 index 00000000000..36b7cd93cdf --- /dev/null +++ b/vendor/expected/LICENSE.txt @@ -0,0 +1,23 @@ +Boost Software License - Version 1.0 - August 17th, 2003 + +Permission is hereby granted, free of charge, to any person or organization +obtaining a copy of the software and accompanying documentation covered by +this license (the "Software") to use, reproduce, display, distribute, +execute, and transmit the Software, and to prepare derivative works of the +Software, and to permit third-parties to whom the Software is furnished to +do so, all subject to the following: + +The copyright notices in the Software and this entire statement, including +the above license grant, this restriction and the following disclaimer, +must be included in all copies of the Software, in whole or in part, and +all derivative works of the Software, unless such copies or derivative +works are solely in the form of machine-executable object code generated by +a source language processor. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN NO EVENT +SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE BE LIABLE +FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR OTHERWISE, +ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. diff --git a/vendor/expected/files.txt b/vendor/expected/files.txt new file mode 100644 index 00000000000..258044be9a1 --- /dev/null +++ b/vendor/expected/files.txt @@ -0,0 +1 @@ +include/expected.hpp diff --git a/vendor/expected/include/expected.hpp b/vendor/expected/include/expected.hpp new file mode 100644 index 00000000000..70298d72372 --- /dev/null +++ b/vendor/expected/include/expected.hpp @@ -0,0 +1,1708 @@ +// This version targets C++11 and later. +// +// Copyright (C) 2016-2018 Martin Moene. +// +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// +// expected lite is based on: +// A proposal to add a utility class to represent expected monad +// by Vicente J. Botet Escriba and Pierre Talbot. http:://wg21.link/p0323 + +#ifndef NONSTD_EXPECTED_LITE_HPP +#define NONSTD_EXPECTED_LITE_HPP + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define expected_lite_VERSION "0.1.0" + +// expected-lite configuration +// +// DXXXXR0: -- +// N4015 : -2 (2014-05-26) +// N4109 : -1 (2014-06-29) +// P0323R0: 0 (2016-05-28) +// P0323R1: 1 (2016-10-12) +// -------: +// P0323R2: 2 (2017-06-15) +// P0323R3: 3 (2017-10-15) +// P0323R4: 4 (2017-11-26) +// P0323R5: 5 (2018-02-08) +// +// expected-lite uses 2 and higher + +#ifndef nsel_P0323R +# define nsel_P0323R 5 +#endif + +// Compiler detection (C++20 is speculative): +// Note: MSVC supports C++14 since it supports C++17. + +#ifdef _MSVC_LANG +# define type_MSVC_LANG _MSVC_LANG +#else +# define type_MSVC_LANG 0 +#endif + +#define type_CPP11 (__cplusplus == 201103L ) +#define nsel_CPP11_OR_GREATER (__cplusplus >= 201103L || type_MSVC_LANG >= 201103L ) +#define nsel_CPP14_OR_GREATER (__cplusplus >= 201402L || type_MSVC_LANG >= 201703L ) +#define nsel_CPP17_OR_GREATER (__cplusplus >= 201703L || type_MSVC_LANG >= 201703L ) +#define nsel_CPP20_OR_GREATER (__cplusplus >= 202000L || type_MSVC_LANG >= 202000L ) + +#if nsel_CPP14_OR_GREATER +# define nsel_constexpr14 constexpr +#else +# define nsel_constexpr14 /*constexpr*/ +#endif + +#if nsel_CPP17_OR_GREATER +# define nsel_inline17 inline +#else +# define nsel_inline17 /*inline*/ +#endif + +// Method enabling + +#define nsel_REQUIRES(...) \ + typename std::enable_if<__VA_ARGS__, void*>::type = 0 + +#define nsel_REQUIRES_0(...) \ + template< bool B = (__VA_ARGS__), typename std::enable_if::type = 0 > + +#define nsel_REQUIRES_T(...) \ + typename = typename std::enable_if< (__VA_ARGS__), nonstd::expected_detail::enabler >::type + +// Clang, GNUC, MSVC warning suppression macros: + +#if defined(_MSC_VER) && !defined(__clang__) +# define nsel_COMPILER_MSVC_VERSION (_MSC_VER / 10 - 10 * ( 5 + (_MSC_VER < 1900)) ) +#else +# define nsel_COMPILER_MSVC_VERSION 0 +#endif + +#ifdef __clang__ +# pragma clang diagnostic push +#elif defined __GNUC__ +# pragma GCC diagnostic push +#endif // __clang__ + +#if nsel_COMPILER_MSVC_VERSION >= 140 +# pragma warning( push ) +# define nsel_DISABLE_MSVC_WARNINGS(codes) __pragma( warning(disable: codes) ) +#else +# define nsel_DISABLE_MSVC_WARNINGS(codes) +#endif + +#ifdef __clang__ +# define nsel_RESTORE_WARNINGS() _Pragma("clang diagnostic pop") +#elif defined __GNUC__ +# define nsel_RESTORE_WARNINGS() _Pragma("GCC diagnostic pop") +#elif nsel_COMPILER_MSVC_VERSION >= 140 +# define nsel_RESTORE_WARNINGS() __pragma( warning( pop ) ) +#else +# define nsel_RESTORE_WARNINGS() +#endif + +// Suppress the following MSVC (GSL) warnings: +// - C26409: Avoid calling new and delete explicitly, use std::make_unique instead (r.11) + +nsel_DISABLE_MSVC_WARNINGS( 26409 ) + +namespace nonstd { + +namespace std20 { + +// type traits C++20: + +template< typename T > +struct remove_cvref +{ + typedef typename std::remove_cv< typename std::remove_reference::type >::type type; +}; + +} // namespace std20 + +// forward declaration: + +template< typename T, typename E > +class expected; + +namespace expected_detail { + +/// for nsel_REQUIRES_T + +enum class enabler{}; + +/// discriminated union to hold value or 'error'. + +template< typename T, typename E > +union storage_t +{ + friend class expected; + +private: + using value_type = T; + using error_type = E; + + // no-op construction + storage_t() {} + ~storage_t() {} + + void construct_value( value_type const & v ) + { + new( &m_value ) value_type( v ); + } + + void construct_value( value_type && v ) + { + new( &m_value ) value_type( std::move( v ) ); + } + + void destruct_value() + { + m_value.~value_type(); + } + + void construct_error( error_type const & e ) + { + new( &m_error ) error_type( e ); + } + + void construct_error( error_type && e ) + { + new( &m_error ) error_type( std::move( e ) ); + } + + void destruct_error() + { + m_error.~error_type(); + } + + constexpr value_type const & value() const & + { + return m_value; + } + + value_type & value() & + { + return m_value; + } + + constexpr value_type const && value() const && + { + return std::move( m_value ); + } + + nsel_constexpr14 value_type && value() && + { + return std::move( m_value ); + } + + value_type const * value_ptr() const + { + return &m_value; + } + + value_type * value_ptr() + { + return &m_value; + } + + error_type const & error() const + { + return m_error; + } + + error_type & error() + { + return m_error; + } + +private: + value_type m_value; + error_type m_error; +}; + +/// discriminated union to hold only 'error'. + +template< typename E > +union storage_t +{ + friend class expected; + +private: + using value_type = void; + using error_type = E; + + // no-op construction + storage_t() {} + ~storage_t() {} + + void construct_error( error_type const & e ) + { + new( &m_error ) error_type( e ); + } + + void construct_error( error_type && e ) + { + new( &m_error ) error_type( std::move( e ) ); + } + + void destruct_error() + { + m_error.~error_type(); + } + + error_type const & error() const + { + return m_error; + } + + error_type & error() + { + return m_error; + } + +private: + error_type m_error; +}; + +} // namespace expected_detail + +/// // x.x.3 Unexpected object type; unexpected_type; C++17 and later can also use aliased type unexpected. + +#if nsel_P0323R <= 2 +template< typename E = std::exception_ptr > +class unexpected_type +#else +template< typename E > +class unexpected_type +#endif // nsel_P0323R +{ +public: + using error_type = E; + + unexpected_type() = delete; + constexpr unexpected_type( unexpected_type const &) = default; + constexpr unexpected_type( unexpected_type &&) = default; + nsel_constexpr14 unexpected_type& operator=( unexpected_type const &) = default; + nsel_constexpr14 unexpected_type& operator=( unexpected_type &&) = default; + + template< typename E2, nsel_REQUIRES_T( + std::is_constructible::value ) + > + constexpr explicit unexpected_type( E2 && error ) + : m_error( std::forward( error ) ) + {} + + template< typename E2 > + constexpr explicit unexpected_type( unexpected_type const & error, nsel_REQUIRES( + std::is_constructible::value + && !std::is_convertible::value /*=> explicit */ ) + ) + : m_error( error ) + {} + + template< typename E2 > + constexpr /*non-explicit*/ unexpected_type( unexpected_type const & error, nsel_REQUIRES( + std::is_constructible::value + && std::is_convertible::value /*=> non-explicit */ ) + ) + : m_error( error ) + {} + + template< typename E2 > + constexpr explicit unexpected_type( unexpected_type && error, nsel_REQUIRES( + std::is_constructible::value + && !std::is_convertible::value /*=> explicit */ ) + ) + : m_error( error ) + {} + + template< typename E2 > + constexpr /*non-explicit*/ unexpected_type( unexpected_type && error, nsel_REQUIRES( + std::is_constructible::value + && std::is_convertible::value /*=> non-explicit */ ) + ) + : m_error( error ) + {} + + nsel_constexpr14 E & value() & noexcept + { + return m_error; + } + + constexpr E const & value() const & noexcept + { + return m_error; + } + + nsel_constexpr14 E && value() && noexcept + { + return std::move( m_error ); + } + + constexpr E const && value() const && noexcept + { + return std::move( m_error ); + } + +// , nsel_REQUIRES( +// std::is_move_constructible::value +// && std::is_swappable::value ) + + void swap( unexpected_type & rhs ) noexcept ( +#if nsel_CPP17_OR_GREATER + std::is_nothrow_move_constructible::value + && std::is_nothrow_swappable::value +#else + std::is_nothrow_move_constructible::value + && noexcept ( std::swap( std::declval(), std::declval() ) ) +#endif + ) + { + using std::swap; + swap( m_error, rhs.m_error ); + } + +private: + error_type m_error; +}; + +/// class unexpected_type, std::exception_ptr specialization (P0323R2) + +#if nsel_P0323R <= 2 + +template<> +class unexpected_type< std::exception_ptr > +{ +public: + using error_type = std::exception_ptr; + + unexpected_type() = delete; + + ~unexpected_type(){} + + explicit unexpected_type( std::exception_ptr const & error ) + : m_error( error ) + {} + + explicit unexpected_type(std::exception_ptr && error ) + : m_error( std::move( error ) ) + {} + + template< typename E > + explicit unexpected_type( E error ) + : m_error( std::make_exception_ptr( error ) ) + {} + + std::exception_ptr const & value() const + { + return m_error; + } + + std::exception_ptr & value() + { + return m_error; + } + +private: + std::exception_ptr m_error; +}; + +#endif // nsel_P0323R + +/// x.x.4, Unexpected equality operators + +template< typename E > +constexpr bool operator==( unexpected_type const & x, unexpected_type const & y ) +{ + return x.value() == y.value(); +} + +template< typename E > +constexpr bool operator!=( unexpected_type const & x, unexpected_type const & y ) +{ + return ! ( x == y ); +} + +#if nsel_P0323R <= 2 + +template< typename E > +constexpr bool operator<( unexpected_type const & x, unexpected_type const & y ) +{ + return x.value() < y.value(); +} + +template< typename E > +constexpr bool operator>( unexpected_type const & x, unexpected_type const & y ) +{ + return ( y < x ); +} + +template< typename E > +constexpr bool operator<=( unexpected_type const & x, unexpected_type const & y ) +{ + return ! ( y < x ); +} + +template< typename E > +constexpr bool operator>=( unexpected_type const & x, unexpected_type const & y ) +{ + return ! ( x < y ); +} + +/// x.x.5 Specialized algorithms + +template< typename E > +void swap( unexpected_type & x, unexpected_type & y) noexcept ( noexcept ( x.swap(y) ) ) +{ + x.swap( y ); +} + +// unexpected: relational operators for std::exception_ptr: + +inline constexpr bool operator<( unexpected_type const & /*x*/, unexpected_type const & /*y*/ ) +{ + return false; +} + +inline constexpr bool operator>( unexpected_type const & /*x*/, unexpected_type const & /*y*/ ) +{ + return false; +} + +inline constexpr bool operator<=( unexpected_type const & x, unexpected_type const & y ) +{ + return ( x == y ); +} + +inline constexpr bool operator>=( unexpected_type const & x, unexpected_type const & y ) +{ + return ( x == y ); +} + +#endif // nsel_P0323R + +// unexpected: traits + +#if nsel_P0323R <= 3 + +template +struct is_unexpected : std::false_type {}; + +template +struct is_unexpected< unexpected_type > : std::true_type {}; + +#endif // nsel_P0323R + +// unexpected: factory + +// keep make_unexpected() removed in p0323r2 for pre-C++17: + +template< typename E> +nsel_constexpr14 auto +make_unexpected( E && v) -> unexpected_type< typename std::decay::type > +{ + return unexpected_type< typename std::decay::type >( v ); +} + +#if nsel_P0323R <= 3 + +/*nsel_constexpr14*/ auto inline +make_unexpected_from_current_exception() -> unexpected_type< std::exception_ptr > +{ + return unexpected_type< std::exception_ptr >( std::current_exception() ); +} + +#endif // nsel_P0323R + +/// in-place tag: construct a value in-place (should come from std::experimental::optional) + +struct in_place_t{}; + +nsel_inline17 constexpr in_place_t in_place{}; + +/// unexpect tag, in_place_unexpected tag: construct an error + +struct unexpect_t{}; +using in_place_unexpected_t = unexpect_t; + +nsel_inline17 constexpr unexpect_t unexpect{}; +nsel_inline17 constexpr unexpect_t in_place_unexpected{}; + +/// expected access error + +template< typename E > +class bad_expected_access; + +template <> +class bad_expected_access< void > : public std::exception +{ +public: + explicit bad_expected_access() + : std::exception() + {} +}; + +template< typename E > +class bad_expected_access : public bad_expected_access< void > +{ +public: + using error_type = E; + + explicit bad_expected_access( error_type error ) + : m_error( error ) + {} + + virtual char const * what() const noexcept override + { + return "bad_expected_access"; + } + + nsel_constexpr14 error_type & error() & + { + return m_error; + } + + constexpr error_type const & error() const & + { + return m_error; + } + + nsel_constexpr14 error_type && error() && + { + return std::move( m_error ); + } + + constexpr error_type const && error() const && + { + return std::move( m_error ); + } + +private: + error_type m_error; +}; + +/// class error_traits + +template< typename Error > +struct error_traits +{ + static void rethrow( Error const & e ) + { + throw bad_expected_access{ e }; + } +}; + +template<> +struct error_traits< std::exception_ptr > +{ + static void rethrow( std::exception_ptr const & e ) + { + std::rethrow_exception( e ); + } +}; + +template<> +struct error_traits< std::error_code > +{ + static void rethrow( std::error_code const & e ) + { + throw std::system_error( e ); + } +}; + +/// class expected + +#if nsel_P0323R <= 2 +template< typename T, typename E = std::exception_ptr > +class expected +#else +template< typename T, typename E > +class expected +#endif // nsel_P0323R +{ +public: + using value_type = T; + using error_type = E; + using unexpected_type = nonstd::unexpected_type; + + template< typename U > + struct rebind + { + using type = expected; + }; + + // x.x.4.1 constructors + + nsel_REQUIRES_0( + std::is_default_constructible::value + ) + nsel_constexpr14 expected() noexcept + ( + std::is_nothrow_default_constructible::value + ) + : has_value_( true ) + { + contained.construct_value( value_type() ); + } + + nsel_constexpr14 expected( expected const & rhs +// , nsel_REQUIRES( +// std::is_copy_constructible::value +// && std::is_copy_constructible::value ) + ) + : has_value_( rhs.has_value_ ) + { + if ( has_value() ) contained.construct_value( rhs.contained.value() ); + else contained.construct_error( rhs.contained.error() ); + } + + nsel_constexpr14 expected( expected && rhs +// , nsel_REQUIRES( +// std::is_move_constructible::value +// && std::is_move_constructible::value ) + ) noexcept ( + std::is_nothrow_move_constructible::value + && std::is_nothrow_move_constructible::value + ) + : has_value_( rhs.has_value_ ) + { + if ( has_value() ) contained.construct_value( std::move( rhs.contained.value() ) ); + else contained.construct_error( std::move( rhs.contained.error() ) ); + } + + template< typename U, typename G > + nsel_constexpr14 explicit expected( expected const & rhs, nsel_REQUIRES( + std::is_constructible::value + && std::is_constructible::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && (!std::is_convertible::value || !std::is_convertible::value ) /*=> explicit */ ) + ) + : has_value_( rhs.has_value_ ) + { + if ( has_value() ) contained.construct_value( rhs.contained.value() ); + else contained.construct_error( rhs.contained.error() ); + } + + template< typename U, typename G > + nsel_constexpr14 /*non-explicit*/ expected( expected const & rhs, nsel_REQUIRES( + std::is_constructible::value + && std::is_constructible::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && !(!std::is_convertible::value || !std::is_convertible::value ) /*=> explicit */ ) + ) + : has_value_( rhs.has_value_ ) + { + if ( has_value() ) contained.construct_value( rhs.contained.value() ); + else contained.construct_error( rhs.contained.error() ); + } + + template< typename U, typename G > + nsel_constexpr14 explicit expected( expected && rhs, nsel_REQUIRES( + std::is_constructible::value + && std::is_constructible::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && (!std::is_convertible::value || !std::is_convertible::value ) /*=> explicit */ ) + ) + : has_value_( rhs.has_value_ ) + { + if ( has_value() ) contained.construct_value( std::move( rhs.contained.value() ) ); + else contained.construct_error( std::move( rhs.contained.error() ) ); + } + + template< typename U, typename G > + nsel_constexpr14 /*non-explicit*/ expected( expected && rhs, nsel_REQUIRES( + std::is_constructible::value + && std::is_constructible::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_constructible&>::value + && !std::is_constructible&&>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && !std::is_convertible&, T>::value + && !std::is_convertible&&, T>::value + && !(!std::is_convertible::value || !std::is_convertible::value ) /*=> non-explicit */ ) + ) + : has_value_( rhs.has_value_ ) + { + if ( has_value() ) contained.construct_value( std::move( rhs.contained.value() ) ); + else contained.construct_error( std::move( rhs.contained.error() ) ); + } + + nsel_constexpr14 expected( value_type const & v +// , nsel_REQUIRES( +// std::is_copy_constructible::value ) + ) + : has_value_( true ) + { + contained.construct_value( v ); + } + + template< typename U = T > + nsel_constexpr14 explicit expected( U && v, nsel_REQUIRES( + std::is_constructible::value + && !std::is_same::type, in_place_t>::value + && !std::is_same, typename std20::remove_cvref::type>::value + && !std::is_same, typename std20::remove_cvref::type>::value + && !std::is_convertible::value /*=> explicit */ +) + ) noexcept + ( + std::is_nothrow_move_constructible::value && + std::is_nothrow_move_constructible::value + ) + : has_value_( true ) + { + contained.construct_value( std::forward( v ) ); + } + + template< typename U = T > + nsel_constexpr14 expected( U && v, nsel_REQUIRES( + std::is_constructible::value + && !std::is_same::type, in_place_t>::value + && !std::is_same, typename std20::remove_cvref::type>::value + && !std::is_same, typename std20::remove_cvref::type>::value + && std::is_convertible::value /*=> non-explicit */ +) + ) noexcept + ( + std::is_nothrow_move_constructible::value && + std::is_nothrow_move_constructible::value + ) + : has_value_( true ) + { + contained.construct_value( std::forward( v ) ); + } + + template ::value ) > + + nsel_constexpr14 explicit expected( in_place_t, Args&&... args ) + : has_value_( true ) + { + contained.construct_value( std::forward( args )... ); + } + + template< typename U, typename... Args, nsel_REQUIRES_T( + std::is_constructible, Args&&...>::value ) > + + nsel_constexpr14 explicit expected( in_place_t, std::initializer_list il, Args&&... args ) + : has_value_( true ) + { + contained.construct_value( il, std::forward( args )... ); + } + + template< typename G = E > + nsel_constexpr14 explicit expected( nonstd::unexpected_type const & error, nsel_REQUIRES( + !std::is_convertible::value /*=> explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( error.value() ); + } + + template< typename G = E > + nsel_constexpr14 /*non-explicit*/ expected( nonstd::unexpected_type const & error, nsel_REQUIRES( + std::is_convertible::value /*=> non-explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( error.value() ); + } + + template< typename G = E > + nsel_constexpr14 explicit expected( nonstd::unexpected_type && error, nsel_REQUIRES( + !std::is_convertible::value /*=> explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( std::move( error.value() ) ); + } + + template< typename G = E > + nsel_constexpr14 /*non-explicit*/ expected( nonstd::unexpected_type && error, nsel_REQUIRES( + std::is_convertible::value /*=> non-explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( std::move( error.value() ) ); + } + + template< typename... Args, nsel_REQUIRES_T( + std::is_constructible::value ) + > + nsel_constexpr14 explicit expected( unexpect_t, Args&&... args ) + : has_value_( false ) + { + contained.construct_error( std::forward( args )... ); + } + + template< typename U, typename... Args, nsel_REQUIRES_T( + std::is_constructible, Args&&...>::value ) + > + nsel_constexpr14 explicit expected( unexpect_t, std::initializer_list il, Args&&... args ) + : has_value_( false ) + { + contained.construct_error( il, std::forward( args )... ); + } + + // x.x.4.2 destructor + + ~expected() + { + if ( has_value() ) contained.destruct_value(); + else contained.destruct_error(); + } + + // x.x.4.3 assignment + +// nsel_REQUIRES( +// std::is_copy_constructible::value && +// std::is_copy_assignable::value && +// std::is_copy_constructible::value && +// std::is_copy_assignable::value ) + + expected operator=( expected const & rhs ) + { + expected( rhs ).swap( *this ); + return *this; + } + +// nsel_REQUIRES( +// std::is_move_constructible::value && +// std::is_move_assignable::value && +// std::is_move_constructible::value && +// std::is_move_assignable::value ) + + expected & operator=( expected && rhs ) noexcept + ( + std::is_nothrow_move_assignable::value && + std::is_nothrow_move_constructible::value&& + std::is_nothrow_move_assignable::value && + std::is_nothrow_move_constructible::value ) + { + expected( std::move( rhs ) ).swap( *this ); + return *this; + } + + template< typename U, nsel_REQUIRES_T( + std::is_constructible::value && + std::is_assignable::value ) > + + expected & operator=( U && v ) + { + expected( std::forward( v ) ).swap( *this ); + return *this; + } + +// nsel_REQUIRES( +// std::is_copy_constructible::value && +// std::is_assignable::value ) + + expected & operator=( unexpected_type const & u ) + { + expected( std::move( u ) ).swap( *this ); + return *this; + } + +// nsel_REQUIRES( +// std::is_copy_constructible::value && +// std::is_assignable::value ) + + expected & operator=( unexpected_type && u ) + { + expected( std::move( u ) ).swap( *this ); + return *this; + } + + template< typename... Args, nsel_REQUIRES_T( + std::is_constructible::value ) > + + void emplace( Args &&... args ) + { + expected( in_place, std::forward(args)... ).swap( *this ); + } + + template< typename U, typename... Args, nsel_REQUIRES_T( + std::is_constructible&, Args&&...>::value ) > + + void emplace( std::initializer_list il, Args &&... args ) + { + expected( in_place, il, std::forward(args)... ).swap( *this ); + } + + // x.x.4.4 swap + +// nsel_REQUIRES( +// std::is_move_constructible::value && +// std::is_move_constructible::value ) + + void swap( expected & rhs ) noexcept + ( +#if nsel_CPP17_OR_GREATER + std::is_nothrow_move_constructible::value && std::is_nothrow_swappable::value && + std::is_nothrow_move_constructible::value && std::is_nothrow_swappable::value +#else + std::is_nothrow_move_constructible::value && noexcept ( std::swap( std::declval(), std::declval() ) ) && + std::is_nothrow_move_constructible::value && noexcept ( std::swap( std::declval(), std::declval() ) ) +#endif + ) + { + using std::swap; + + if ( bool(*this) && bool(rhs) ) { swap( contained.value(), rhs.contained.value() ); } + else if ( ! bool(*this) && ! bool(rhs) ) { swap( contained.error(), rhs.contained.error() ); } + else if ( bool(*this) && ! bool(rhs) ) { error_type t( std::move( rhs.error() ) ); + rhs.contained.destruct_error(); + rhs.contained.construct_value( std::move( contained.value() ) ); + contained.destruct_value(); + contained.construct_error( std::move( t ) ); + swap( has_value_, rhs.has_value_ ); } + else if ( ! bool(*this) && bool(rhs) ) { rhs.swap( *this ); } + } + + // x.x.4.5 observers + + constexpr value_type const * operator ->() const + { + return assert( has_value() ), contained.value_ptr(); + } + + value_type * operator ->() + { + return assert( has_value() ), contained.value_ptr(); + } + + constexpr value_type const & operator *() const & + { + return assert( has_value() ), contained.value(); + } + + value_type & operator *() & + { + return assert( has_value() ), contained.value(); + } + + constexpr value_type const && operator *() const && + { + return assert( has_value() ), std::move( contained.value() ); + } + + nsel_constexpr14 value_type && operator *() && + { + return assert( has_value() ), std::move( contained.value() ); + } + + constexpr explicit operator bool() const noexcept + { + return has_value(); + } + + constexpr bool has_value() const noexcept + { + return has_value_; + } + + constexpr value_type const & value() const & + { + return has_value() + ? ( contained.value() ) + : ( error_traits::rethrow( contained.error() ), contained.value() ); + } + + value_type & value() & + { + return has_value() + ? ( contained.value() ) + : ( error_traits::rethrow( contained.error() ), contained.value() ); + } + + constexpr value_type const && value() const && + { + return std::move( has_value() + ? ( contained.value() ) + : ( error_traits::rethrow( contained.error() ), contained.value() ) ); + } + + nsel_constexpr14 value_type && value() && + { + return std::move( has_value() + ? ( contained.value() ) + : ( error_traits::rethrow( contained.error() ), contained.value() ) ); + } + + constexpr error_type const & error() const & + { + return assert( ! has_value() ), contained.error(); + } + + error_type & error() & + { + return assert( ! has_value() ), contained.error(); + } + + constexpr error_type const && error() const && + { + return assert( ! has_value() ), std::move( contained.error() ); + } + + error_type && error() && + { + return assert( ! has_value() ), std::move( contained.error() ); + } + + constexpr unexpected_type get_unexpected() const + { + return make_unexpected( contained.error() ); + } + + template< typename Ex > + bool has_exception() const + { + using ContainedEx = typename std::remove_reference< decltype( get_unexpected().value() ) >::type; + return ! has_value() && std::is_base_of< Ex, ContainedEx>::value; + } + + template< typename U, nsel_REQUIRES_T( + std::is_copy_constructible::value && + std::is_convertible::value ) > + + value_type value_or( U && v ) const & + { + return has_value() + ? contained.value() + : static_cast( std::forward( v ) ); + } + + template< typename U, nsel_REQUIRES_T( + std::is_move_constructible::value && + std::is_convertible::value ) > + + value_type value_or( U && v ) && + { + return has_value() + ? std::move( contained.value() ) + : static_cast( std::forward( v ) ); + } + + // unwrap() + +// template +// constexpr expected expected,E>::unwrap() const&; + +// template +// constexpr expected expected::unwrap() const&; + +// template +// expected expected, E>::unwrap() &&; + +// template +// template expected expected::unwrap() &&; + + // factories + +// template +// expected catch_exception(F&& f); + +// template +// expected())),E> map(F&& func) ; + +// template +// 'see below' bind(F&& func); + +// template +// expected catch_error(F&& f); + +// template +// 'see below' then(F&& func); + +private: + bool has_value_; + expected_detail::storage_t contained; +}; + +/// class expected, void specialization + +template< typename E > +class expected +{ +public: + using value_type = void; + using error_type = E; + using unexpected_type = nonstd::unexpected_type; + + // x.x.4.1 constructors + + constexpr expected() noexcept + : has_value_( true ) + { + } + + nsel_constexpr14 expected( expected const & rhs ) + : has_value_( rhs.has_value_ ) + { + if ( ! has_value() ) contained.construct_error( rhs.contained.error() ); + } + + nsel_REQUIRES_0( + std::is_move_constructible::value ) + + nsel_constexpr14 expected( expected && rhs ) noexcept + ( + true // TBD - see also non-void specialization + ) + : has_value_( rhs.has_value_ ) + { + if ( ! has_value() ) contained.construct_error( std::move( rhs.contained.error() ) ); + } + + constexpr explicit expected( in_place_t ) + : has_value_( true ) + { + } + + template< typename G = E > + nsel_constexpr14 explicit expected( nonstd::unexpected_type const & error, nsel_REQUIRES( + !std::is_convertible::value /*=> explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( error.value() ); + } + + template< typename G = E > + nsel_constexpr14 /*non-explicit*/ expected( nonstd::unexpected_type const & error, nsel_REQUIRES( + std::is_convertible::value /*=> non-explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( error.value() ); + } + + template< typename G = E > + nsel_constexpr14 explicit expected( nonstd::unexpected_type && error, nsel_REQUIRES( + !std::is_convertible::value /*=> explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( std::move( error.value() ) ); + } + + template< typename G = E > + nsel_constexpr14 /*non-explicit*/ expected( nonstd::unexpected_type && error, nsel_REQUIRES( + std::is_convertible::value /*=> non-explicit */ ) + ) + : has_value_( false ) + { + contained.construct_error( std::move( error.value() ) ); + } + + template< typename... Args, nsel_REQUIRES_T( + std::is_constructible::value ) + > + nsel_constexpr14 explicit expected( unexpect_t, Args&&... args ) + : has_value_( false ) + { + contained.construct_error( std::forward( args )... ); + } + + template< typename U, typename... Args, nsel_REQUIRES_T( + std::is_constructible, Args&&...>::value ) + > + nsel_constexpr14 explicit expected( unexpect_t, std::initializer_list il, Args&&... args ) + : has_value_( false ) + { + contained.construct_error( il, std::forward( args )... ); + } + + + // destructor + + ~expected() + { + if ( ! has_value() ) contained.destruct_error(); + } + + // x.x.4.3 assignment + +// nsel_REQUIRES( +// std::is_copy_constructible::value && +// std::is_copy_assignable::value ) + + expected & operator=( expected const & rhs ) + { + expected( rhs ).swap( *this ); + return *this; + } + +// nsel_REQUIRES( +// std::is_move_constructible::value && +// std::is_move_assignable::value ) + + expected & operator=( expected && rhs ) noexcept + ( + std::is_nothrow_move_assignable::value && + std::is_nothrow_move_constructible::value ) + { + expected( std::move( rhs ) ).swap( *this ); + return *this; + } + + void emplace() + {} + + // x.x.4.4 swap + +// nsel_REQUIRES( +// std::is_move_constructible::value ) + + void swap( expected & rhs ) noexcept + ( +#if nsel_CPP17_OR_GREATER + std::is_nothrow_move_constructible::value && std::is_nothrow_swappable::value +#else + std::is_nothrow_move_constructible::value && noexcept ( std::swap( std::declval(), std::declval() ) ) +#endif + ) + { + using std::swap; + + if ( ! bool(*this) && ! bool(rhs) ) { swap( contained.error(), rhs.contained.error() ); } + else if ( bool(*this) && ! bool(rhs) ) { contained.construct_error( std::move( rhs.error() ) ); + swap( has_value_, rhs.has_value_ ); } + else if ( ! bool(*this) && bool(rhs) ) { rhs.swap( *this ); } + } + + // x.x.4.5 observers + + constexpr explicit operator bool() const noexcept + { + return has_value(); + } + + constexpr bool has_value() const noexcept + { + return has_value_; + } + + void value() const + {} + + constexpr error_type const & error() const & + { + return assert( ! has_value() ), contained.error(); + } + + error_type & error() & + { + return assert( ! has_value() ), contained.error(); + } + + constexpr error_type const && error() const && + { + return assert( ! has_value() ), std::move( contained.error() ); + } + + error_type && error() && + { + return assert( ! has_value() ), std::move( contained.error() ); + } + + constexpr unexpected_type get_unexpected() const + { + return make_unexpected( contained.error() ); + } + + template + bool has_exception() const + { + return ! has_value() && std::is_base_of< Ex, decltype( get_unexpected().value() ) >::value; + } + +// template constexpr 'see below' unwrap() const&; +// +// template 'see below' unwrap() &&; + + // factories + +// template +// expected catch_exception(F&& f); +// +// template +// expected map(F&& func) ; +// +// template +// 'see below' bind(F&& func) ; +// +// template +// expected catch_error(F&& f); +// +// template +// 'see below' then(F&& func); + +private: + bool has_value_; + expected_detail::storage_t contained; +}; + +// expected: relational operators + +template +constexpr bool operator==( expected const & x, expected const & y ) +{ + return bool(x) != bool(y) ? false : bool(x) == false ? true : *x == *y; +} + +template +constexpr bool operator!=( expected const & x, expected const & y ) +{ + return !(x == y); +} + +template +constexpr bool operator<( expected const & x, expected const & y ) +{ + return (!y) ? false : (!x) ? true : *x < *y; +} + +template +constexpr bool operator>( expected const & x, expected const & y ) +{ + return (y < x); +} + +template +constexpr bool operator<=( expected const & x, expected const & y ) +{ + return !(y < x); +} + +template +constexpr bool operator>=( expected const & x, expected const & y ) +{ + return !(x < y); +} + +// expected: comparison with unexpected_type + +template +constexpr bool operator==( expected const & x, unexpected_type const & u ) +{ + return (!x) ? x.get_unexpected() == u : false; +} + +template +constexpr bool operator==( unexpected_type const & u, expected const & x ) +{ + return ( x == u ); +} + +template +constexpr bool operator!=( expected const & x, unexpected_type const & u ) +{ + return ! ( x == u ); +} + +template +constexpr bool operator!=( unexpected_type const & u, expected const & x ) +{ + return ! ( x == u ); +} + +template +constexpr bool operator<( expected const & x, unexpected_type const & u ) +{ + return (!x) ? ( x.get_unexpected() < u ) : false; +} + +#if nsel_P0323R <= 2 + +template +constexpr bool operator<( unexpected_type const & u, expected const & x ) +{ + return (!x) ? ( u < x.get_unexpected() ) : true ; +} + +template +constexpr bool operator>( expected const & x, unexpected_type const & u ) +{ + return ( u < x ); +} + +template +constexpr bool operator>( unexpected_type const & u, expected const & x ) +{ + return ( x < u ); +} + +template +constexpr bool operator<=( expected const & x, unexpected_type const & u ) +{ + return ! ( u < x ); +} + +template +constexpr bool operator<=( unexpected_type const & u, expected const & x) +{ + return ! ( x < u ); +} + +template +constexpr bool operator>=( expected const & x, unexpected_type const & u ) +{ + return ! ( u > x ); +} + +template +constexpr bool operator>=( unexpected_type const & u, expected const & x ) +{ + return ! ( x > u ); +} + +#endif // nsel_P0323R + +// expected: comparison with T + +template +constexpr bool operator==( expected const & x, T const & v ) +{ + return bool(x) ? *x == v : false; +} + +template +constexpr bool operator==(T const & v, expected const & x ) +{ + return bool(x) ? v == *x : false; +} + +template +constexpr bool operator!=( expected const & x, T const & v ) +{ + return bool(x) ? *x != v : true; +} + +template +constexpr bool operator!=( T const & v, expected const & x ) +{ + return bool(x) ? v != *x : true; +} + +template +constexpr bool operator<( expected const & x, T const & v ) +{ + return bool(x) ? *x < v : true; +} + +template +constexpr bool operator<( T const & v, expected const & x ) +{ + return bool(x) ? v < *x : false; +} + +template +constexpr bool operator>( T const & v, expected const & x ) +{ + return bool(x) ? *x < v : false; +} + +template +constexpr bool operator>( expected const & x, T const & v ) +{ + return bool(x) ? v < *x : false; +} + +template +constexpr bool operator<=( T const & v, expected const & x ) +{ + return bool(x) ? ! ( *x < v ) : false; +} + +template +constexpr bool operator<=( expected const & x, T const & v ) +{ + return bool(x) ? ! ( v < *x ) : true; +} + +template +constexpr bool operator>=( expected const & x, T const & v ) +{ + return bool(x) ? ! ( *x < v ) : false; +} + +template +constexpr bool operator>=( T const & v, expected const & x ) +{ + return bool(x) ? ! ( v < *x ) : true; +} + +/// x.x.x Specialized algorithms + +template< typename T, typename E > +void swap( expected & x, expected & y ) noexcept ( noexcept ( x.swap(y) ) ) +{ + x.swap( y ); +} + +#if nsel_P0323R <= 3 + +template< typename T> +constexpr auto make_expected( T && v ) -> expected< typename std::decay::type > +{ + return expected< typename std::decay::type >( std::forward( v ) ); +} + +// expected specialization: + +auto inline make_expected() -> expected +{ + return expected( in_place ); +} + +template< typename T> +constexpr auto make_expected_from_current_exception() -> expected +{ + return expected( make_unexpected_from_current_exception() ); +} + +template< typename T> +auto make_expected_from_exception( std::exception_ptr v ) -> expected +{ + return expected( unexpected_type( std::forward( v ) ) ); +} + +template< typename T, typename E > +constexpr auto make_expected_from_error( E e ) -> expected::type> +{ + return expected::type>( make_unexpected( e ) ); +} + +template< typename F > +/*nsel_constexpr14*/ +auto make_expected_from_call( F f, + nsel_REQUIRES( ! std::is_same::type, void>::value ) +) -> expected< typename std::result_of::type > +{ + try + { + return make_expected( f() ); + } + catch (...) + { + return make_unexpected_from_current_exception(); + } +} + +template< typename F > +/*nsel_constexpr14*/ +auto make_expected_from_call( F f, + nsel_REQUIRES( std::is_same::type, void>::value ) +) -> expected +{ + try + { + f(); + return make_expected(); + } + catch (...) + { + return make_unexpected_from_current_exception(); + } +} + +#endif // nsel_P0323R + +} // namespace nonstd + +namespace std { + +// expected: hash support + +template< typename T, typename E > +struct hash< nonstd::expected > +{ + using result_type = typename hash::result_type; + using argument_type = nonstd::expected; + + constexpr result_type operator()(argument_type const & arg) const + { + return arg ? std::hash{}(*arg) : result_type{}; + } +}; + +// TBD - ?? remove? see spec. +template< typename T, typename E > +struct hash< nonstd::expected > +{ + using result_type = typename hash::result_type; + using argument_type = nonstd::expected; + + constexpr result_type operator()(argument_type const & arg) const + { + return arg ? std::hash{}(*arg) : result_type{}; + } +}; + +// TBD - implement +// bool(e), hash>()(e) shall evaluate to the hashing true; +// otherwise it evaluates to an unspecified value if E is exception_ptr or +// a combination of hashing false and hash()(e.error()). + +template< typename E > +struct hash< nonstd::expected > +{ +}; + +} // namespace std + + +namespace nonstd { + +// void unexpected() is deprecated && removed in C++17 + +#if nsel_CPP17_OR_GREATER && nsel_COMPILER_MSVC_VERSION > 141 +template< typename E > +using unexpected = unexpected_type; +#endif + +} // namespace nonstd + +#undef nsel_REQUIRES +#undef nsel_REQUIRES_0 +#undef nsel_REQUIRES_T + +nsel_RESTORE_WARNINGS() + +#endif // NONSTD_EXPECTED_LITE_HPP diff --git a/vendor/expected/version.txt b/vendor/expected/version.txt new file mode 100644 index 00000000000..52e41074409 --- /dev/null +++ b/vendor/expected/version.txt @@ -0,0 +1 @@ +a2359cf61478d735a308e952b1c9840714f61386 From be9d77c78834e4a7da141b74eeef54946b9b30d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 13 Jun 2018 15:50:23 +0200 Subject: [PATCH 2/6] [core] harden OfflineDatabase --- cmake/test-files.cmake | 2 + include/mbgl/storage/offline.hpp | 2 +- platform/default/default_file_source.cpp | 28 +- .../default/mbgl/storage/offline_database.cpp | 313 +++++---- .../default/mbgl/storage/offline_database.hpp | 24 +- .../default/mbgl/storage/offline_download.cpp | 31 +- platform/default/sqlite3.cpp | 48 +- platform/qt/src/sqlite3.cpp | 12 +- .../offline_database/corrupt-delayed.db | Bin 0 -> 19456 bytes .../offline_database/corrupt-immediate.db | Bin 0 -> 4096 bytes test/src/mbgl/test/fixture_log_observer.cpp | 6 +- test/src/mbgl/test/sqlite3_test_fs.cpp | 320 ++++++++++ test/src/mbgl/test/sqlite3_test_fs.hpp | 39 ++ test/storage/offline_database.test.cpp | 592 +++++++++++++----- test/storage/offline_download.test.cpp | 79 ++- test/storage/sqlite.test.cpp | 6 - 16 files changed, 1150 insertions(+), 352 deletions(-) create mode 100644 test/fixtures/offline_database/corrupt-delayed.db create mode 100644 test/fixtures/offline_database/corrupt-immediate.db create mode 100644 test/src/mbgl/test/sqlite3_test_fs.cpp create mode 100644 test/src/mbgl/test/sqlite3_test_fs.hpp diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index 2a679fc40be..ff659b1cd1a 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -95,6 +95,8 @@ set(MBGL_TEST_FILES test/src/mbgl/test/fixture_log_observer.hpp test/src/mbgl/test/getrss.cpp test/src/mbgl/test/getrss.hpp + test/src/mbgl/test/sqlite3_test_fs.cpp + test/src/mbgl/test/sqlite3_test_fs.hpp test/src/mbgl/test/stub_file_source.cpp test/src/mbgl/test/stub_file_source.hpp test/src/mbgl/test/stub_geometry_tile_feature.hpp diff --git a/include/mbgl/storage/offline.hpp b/include/mbgl/storage/offline.hpp index ef4a499e834..2193f8d09e3 100644 --- a/include/mbgl/storage/offline.hpp +++ b/include/mbgl/storage/offline.hpp @@ -187,11 +187,11 @@ class OfflineRegion { public: // Move-only; not publicly constructible. OfflineRegion(OfflineRegion&&); - OfflineRegion& operator=(OfflineRegion&&); ~OfflineRegion(); OfflineRegion() = delete; OfflineRegion(const OfflineRegion&) = delete; + OfflineRegion& operator=(OfflineRegion&&) = delete; OfflineRegion& operator=(const OfflineRegion&) = delete; int64_t getID() const; diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp index f0701214973..051e1b125ca 100644 --- a/platform/default/default_file_source.cpp +++ b/platform/default/default_file_source.cpp @@ -74,9 +74,9 @@ class DefaultFileSource::Impl { } void getRegionStatus(int64_t regionID, std::function)> callback) { - try { - callback({}, getDownload(regionID).getStatus()); - } catch (...) { + if (auto download = getDownload(regionID)) { + callback({}, download->getStatus()); + } else { callback(std::current_exception(), {}); } } @@ -92,11 +92,15 @@ class DefaultFileSource::Impl { } void setRegionObserver(int64_t regionID, std::unique_ptr observer) { - getDownload(regionID).setObserver(std::move(observer)); + if (auto download = getDownload(regionID)) { + download->setObserver(std::move(observer)); + } } void setRegionDownloadState(int64_t regionID, OfflineRegionDownloadState state) { - getDownload(regionID).setState(state); + if (auto download = getDownload(regionID)) { + download->setState(state); + } } void request(AsyncRequest* req, Resource resource, ActorRef ref) { @@ -181,13 +185,19 @@ class DefaultFileSource::Impl { } private: - OfflineDownload& getDownload(int64_t regionID) { + OfflineDownload* getDownload(int64_t regionID) { auto it = downloads.find(regionID); if (it != downloads.end()) { - return *it->second; + return it->second.get(); + } + auto definition = offlineDatabase->getRegionDefinition(regionID); + if (!definition) { + return nullptr; } - return *downloads.emplace(regionID, - std::make_unique(regionID, offlineDatabase->getRegionDefinition(regionID), *offlineDatabase, onlineFileSource)).first->second; + + auto download = std::make_unique(regionID, std::move(*definition), + *offlineDatabase, onlineFileSource); + return downloads.emplace(regionID, std::move(download)).first->second.get(); } // shared so that destruction is done on the creating thread diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index 8f7f0965f48..cfa7f529778 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -15,7 +15,14 @@ namespace mbgl { OfflineDatabase::OfflineDatabase(std::string path_, uint64_t maximumCacheSize_) : path(std::move(path_)), maximumCacheSize(maximumCacheSize_) { - ensureSchema(); + try { + initialize(); + } catch (const util::IOException& ex) { + handleError(ex, "open database"); + } catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "open database"); + } + // Assume that we can't open the database right now and work with an empty database object. } OfflineDatabase::~OfflineDatabase() { @@ -24,87 +31,71 @@ OfflineDatabase::~OfflineDatabase() { try { statements.clear(); db.reset(); - } catch (mapbox::sqlite::Exception& ex) { - Log::Error(Event::Database, (int)ex.code, ex.what()); + } catch (const util::IOException& ex) { + handleError(ex, "close database"); + } catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "close database"); } } -void OfflineDatabase::ensureSchema() { - auto result = mapbox::sqlite::Database::tryOpen(path, mapbox::sqlite::ReadWriteCreate); - if (result.is()) { - const auto& ex = result.get(); - if (ex.code == mapbox::sqlite::ResultCode::NotADB) { - // Corrupted; blow it away. - removeExisting(); - result = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate); - } else { - Log::Error(Event::Database, "Unexpected error connecting to database: %s", ex.what()); - throw ex; - } +void OfflineDatabase::initialize() { + assert(!db); + assert(statements.empty()); + + db = std::make_unique( + mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate)); + db->setBusyTimeout(Milliseconds::max()); + db->exec("PRAGMA foreign_keys = ON"); + + const auto userVersion = getPragma("PRAGMA user_version"); + switch (userVersion) { + case 0: + case 1: + // Newly created database, or old cache-only database; remove old table if it exists. + removeOldCacheTable(); + createSchema(); + return; + case 2: + migrateToVersion3(); + // fall through + case 3: + // Removed migration, see below. + // fall through + case 4: + migrateToVersion5(); + // fall through + case 5: + migrateToVersion6(); + // fall through + case 6: + // Happy path; we're done + return; + default: + // Downgrade: delete the database and try to reinitialize. + removeExisting(); + initialize(); } +} - try { - assert(result.is()); - db = std::make_unique(std::move(result.get())); - db->setBusyTimeout(Milliseconds::max()); - db->exec("PRAGMA foreign_keys = ON"); - - switch (userVersion()) { - case 0: - case 1: - // Newly created database, or old cache-only database; remove old table if it exists. - removeOldCacheTable(); - break; - case 2: - migrateToVersion3(); - // fall through - case 3: - case 4: - migrateToVersion5(); - // fall through - case 5: - migrateToVersion6(); - // fall through - case 6: - // happy path; we're done - return; - default: - // downgrade, delete the database - removeExisting(); - break; - } - } catch (const mapbox::sqlite::Exception& ex) { - // Unfortunately, SQLITE_NOTADB is not always reported upon opening the database. - // Apparently sometimes it is delayed until the first read operation. - if (ex.code == mapbox::sqlite::ResultCode::NotADB) { +void OfflineDatabase::handleError(const mapbox::sqlite::Exception& ex, const char* action) { + if (ex.code == mapbox::sqlite::ResultCode::NotADB || + ex.code == mapbox::sqlite::ResultCode::Corrupt) { + // Corrupted; delete database so that we have a clean slate for the next operation. + Log::Error(Event::Database, static_cast(ex.code), "Can't %s: %s", action, ex.what()); + try { removeExisting(); - } else { - throw; + } catch (const util::IOException& ioEx) { + handleError(ioEx, action); } - } - - try { - // When downgrading the database, or when the database is corrupt, we've deleted the old database handle, - // so we need to reopen it. - if (!db) { - db = std::make_unique(mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadWriteCreate)); - db->setBusyTimeout(Milliseconds::max()); - db->exec("PRAGMA foreign_keys = ON"); - } - - db->exec("PRAGMA auto_vacuum = INCREMENTAL"); - db->exec("PRAGMA journal_mode = DELETE"); - db->exec("PRAGMA synchronous = FULL"); - db->exec(offlineDatabaseSchema); - db->exec("PRAGMA user_version = 6"); - } catch (...) { - Log::Error(Event::Database, "Unexpected error creating database schema: %s", util::toString(std::current_exception()).c_str()); - throw; + } else { + // We treat the error as temporary, and pretend we have an inaccessible DB. + Log::Warning(Event::Database, static_cast(ex.code), "Can't %s: %s", action, ex.what()); } } -int OfflineDatabase::userVersion() { - return static_cast(getPragma("PRAGMA user_version")); +void OfflineDatabase::handleError(const util::IOException& ex, const char* action) { + // We failed to delete the database file. + Log::Error(Event::Database, ex.code, "Can't %s: %s", action, ex.what()); } void OfflineDatabase::removeExisting() { @@ -113,19 +104,28 @@ void OfflineDatabase::removeExisting() { statements.clear(); db.reset(); - try { - util::deleteFile(path); - } catch (util::IOException& ex) { - Log::Error(Event::Database, ex.code, ex.what()); - } + util::deleteFile(path); } void OfflineDatabase::removeOldCacheTable() { + assert(db); db->exec("DROP TABLE IF EXISTS http_cache"); db->exec("VACUUM"); } +void OfflineDatabase::createSchema() { + assert(db); + db->exec("PRAGMA auto_vacuum = INCREMENTAL"); + db->exec("PRAGMA journal_mode = DELETE"); + db->exec("PRAGMA synchronous = FULL"); + mapbox::sqlite::Transaction transaction(*db); + db->exec(offlineDatabaseSchema); + db->exec("PRAGMA user_version = 6"); + transaction.commit(); +} + void OfflineDatabase::migrateToVersion3() { + assert(db); db->exec("PRAGMA auto_vacuum = INCREMENTAL"); db->exec("VACUUM"); db->exec("PRAGMA user_version = 3"); @@ -138,12 +138,14 @@ void OfflineDatabase::migrateToVersion3() { // See: https://github.com/mapbox/mapbox-gl-native/pull/6320 void OfflineDatabase::migrateToVersion5() { + assert(db); db->exec("PRAGMA journal_mode = DELETE"); db->exec("PRAGMA synchronous = FULL"); db->exec("PRAGMA user_version = 5"); } void OfflineDatabase::migrateToVersion6() { + assert(db); mapbox::sqlite::Transaction transaction(*db); db->exec("ALTER TABLE resources ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0"); db->exec("ALTER TABLE tiles ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0"); @@ -152,6 +154,9 @@ void OfflineDatabase::migrateToVersion6() { } mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) { + if (!db) { + initialize(); + } auto it = statements.find(sql); if (it == statements.end()) { it = statements.emplace(sql, std::make_unique(*db, sql)).first; @@ -159,9 +164,15 @@ mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) { return *it->second; } -optional OfflineDatabase::get(const Resource& resource) { +optional OfflineDatabase::get(const Resource& resource) try { auto result = getInternal(resource); - return result ? result->first : optional(); + return result ? optional{ result->first } : nullopt; +} catch (const util::IOException& ex) { + handleError(ex, "read resource"); + return nullopt; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "read resource"); + return nullopt; } optional> OfflineDatabase::getInternal(const Resource& resource) { @@ -182,11 +193,17 @@ optional OfflineDatabase::hasInternal(const Resource& resource) { } } -std::pair OfflineDatabase::put(const Resource& resource, const Response& response) { +std::pair OfflineDatabase::put(const Resource& resource, const Response& response) try { + if (!db) { + initialize(); + } mapbox::sqlite::Transaction transaction(*db, mapbox::sqlite::Transaction::Immediate); auto result = putInternal(resource, response, true); transaction.commit(); return result; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "write resource"); + return { false, 0 }; } std::pair OfflineDatabase::putInternal(const Resource& resource, const Response& response, bool evict_) { @@ -227,11 +244,19 @@ std::pair OfflineDatabase::putInternal(const Resource& resource, optional> OfflineDatabase::getResource(const Resource& resource) { // Update accessed timestamp used for LRU eviction. - { + try { mapbox::sqlite::Query accessedQuery{ getStatement("UPDATE resources SET accessed = ?1 WHERE url = ?2") }; accessedQuery.bind(1, util::now()); accessedQuery.bind(2, resource.url); accessedQuery.run(); + } catch (const mapbox::sqlite::Exception& ex) { + if (ex.code == mapbox::sqlite::ResultCode::NotADB || + ex.code == mapbox::sqlite::ResultCode::Corrupt) { + throw; + } + + // If we don't have any indication that the database is corrupt, continue as usual. + Log::Warning(Event::Database, static_cast(ex.code), "Can't update timestamp: %s", ex.what()); } // clang-format off @@ -245,7 +270,7 @@ optional> OfflineDatabase::getResource(const Resou query.bind(1, resource.url); if (!query.run()) { - return {}; + return nullopt; } Response response; @@ -274,7 +299,7 @@ optional OfflineDatabase::hasResource(const Resource& resource) { mapbox::sqlite::Query query{ getStatement("SELECT length(data) FROM resources WHERE url = ?") }; query.bind(1, resource.url); if (!query.run()) { - return {}; + return nullopt; } return query.get>(0); @@ -366,7 +391,8 @@ bool OfflineDatabase::putResource(const Resource& resource, } optional> OfflineDatabase::getTile(const Resource::TileData& tile) { - { + // Update accessed timestamp used for LRU eviction. + try { // clang-format off mapbox::sqlite::Query accessedQuery{ getStatement( "UPDATE tiles " @@ -385,6 +411,13 @@ optional> OfflineDatabase::getTile(const Resource: accessedQuery.bind(5, tile.y); accessedQuery.bind(6, tile.z); accessedQuery.run(); + } catch (const mapbox::sqlite::Exception& ex) { + if (ex.code == mapbox::sqlite::ResultCode::NotADB || ex.code == mapbox::sqlite::ResultCode::Corrupt) { + throw; + } + + // If we don't have any indication that the database is corrupt, continue as usual. + Log::Warning(Event::Database, static_cast(ex.code), "Can't update timestamp: %s", ex.what()); } // clang-format off @@ -406,7 +439,7 @@ optional> OfflineDatabase::getTile(const Resource: query.bind(5, tile.z); if (!query.run()) { - return {}; + return nullopt; } Response response; @@ -450,7 +483,7 @@ optional OfflineDatabase::hasTile(const Resource::TileData& tile) { size.bind(5, tile.z); if (!size.run()) { - return {}; + return nullopt; } return size.get>(0); @@ -559,23 +592,30 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile, return true; } -std::vector OfflineDatabase::listRegions() { +std::vector OfflineDatabase::listRegions() try { mapbox::sqlite::Query query{ getStatement("SELECT id, definition, description FROM regions") }; - std::vector result; - while (query.run()) { - result.push_back(OfflineRegion( - query.get(0), - decodeOfflineRegionDefinition(query.get(1)), - query.get>(2))); + const auto id = query.get(0); + const auto definition = query.get(1); + const auto description = query.get>(2); + try { + OfflineRegion region(id, decodeOfflineRegionDefinition(definition), description); + result.emplace_back(std::move(region)); + } catch (const std::exception& ex) { + // Catch errors from malformed offline region definitions + // and skip them. + Log::Error(Event::General, "%s", ex.what()); + } } - return result; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "list regions"); + return {}; } -OfflineRegion OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, - const OfflineRegionMetadata& metadata) { +optional OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, + const OfflineRegionMetadata& metadata) try { // clang-format off mapbox::sqlite::Query query{ getStatement( "INSERT INTO regions (definition, description) " @@ -585,11 +625,13 @@ OfflineRegion OfflineDatabase::createRegion(const OfflineRegionDefinition& defin query.bind(1, encodeOfflineRegionDefinition(definition)); query.bindBlob(2, metadata); query.run(); - return OfflineRegion(query.lastInsertRowId(), definition, metadata); +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "create region"); + return nullopt; } -OfflineRegionMetadata OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) { +optional OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) try { // clang-format off mapbox::sqlite::Query query{ getStatement( "UPDATE regions SET description = ?1 " @@ -600,9 +642,12 @@ OfflineRegionMetadata OfflineDatabase::updateMetadata(const int64_t regionID, co query.run(); return metadata; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "update region metadata"); + return nullopt; } -void OfflineDatabase::deleteRegion(OfflineRegion&& region) { +void OfflineDatabase::deleteRegion(OfflineRegion&& region) try { { mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") }; query.bind(1, region.getID()); @@ -610,13 +655,17 @@ void OfflineDatabase::deleteRegion(OfflineRegion&& region) { } evict(0); + assert(db); db->exec("PRAGMA incremental_vacuum"); // Ensure that the cached offlineTileCount value is recalculated. offlineMapboxTileCount = {}; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "delete region"); + return; } -optional> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) { +optional> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) try { auto response = getInternal(resource); if (response) { @@ -624,9 +673,12 @@ optional> OfflineDatabase::getRegionResource(int64 } return response; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "read region resource"); + return nullopt; } -optional OfflineDatabase::hasRegionResource(int64_t regionID, const Resource& resource) { +optional OfflineDatabase::hasRegionResource(int64_t regionID, const Resource& resource) try { auto response = hasInternal(resource); if (response) { @@ -634,29 +686,52 @@ optional OfflineDatabase::hasRegionResource(int64_t regionID, const Res } return response; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "query region resource"); + return nullopt; } -uint64_t OfflineDatabase::putRegionResource(int64_t regionID, const Resource& resource, const Response& response) { +uint64_t OfflineDatabase::putRegionResource(int64_t regionID, + const Resource& resource, + const Response& response) try { + if (!db) { + initialize(); + } mapbox::sqlite::Transaction transaction(*db); auto size = putRegionResourceInternal(regionID, resource, response); transaction.commit(); return size; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "write region resource"); + return 0; } -void OfflineDatabase::putRegionResources(int64_t regionID, const std::list>& resources, OfflineRegionStatus& status) { +void OfflineDatabase::putRegionResources(int64_t regionID, + const std::list>& resources, + OfflineRegionStatus& status) try { + if (!db) { + initialize(); + } mapbox::sqlite::Transaction transaction(*db); + // Accumulate all statistics locally first before adding them to the OfflineRegionStatus object + // to ensure correctness when the transaction fails. + uint64_t completedResourceCount = 0; + uint64_t completedResourceSize = 0; + uint64_t completedTileCount = 0; + uint64_t completedTileSize = 0; + for (const auto& elem : resources) { const auto& resource = std::get<0>(elem); const auto& response = std::get<1>(elem); try { uint64_t resourceSize = putRegionResourceInternal(regionID, resource, response); - status.completedResourceCount++; - status.completedResourceSize += resourceSize; + completedResourceCount++; + completedResourceSize += resourceSize; if (resource.kind == Resource::Kind::Tile) { - status.completedTileCount += 1; - status.completedTileSize += resourceSize; + completedTileCount += 1; + completedTileSize += resourceSize; } } catch (const MapboxTileLimitExceededException&) { // Commit the rest of the batch and retrow @@ -667,6 +742,13 @@ void OfflineDatabase::putRegionResources(int64_t regionID, const std::list OfflineDatabase::getRegionDefinition(int64_t regionID) try { mapbox::sqlite::Query query{ getStatement("SELECT definition FROM regions WHERE id = ?1") }; query.bind(1, regionID); query.run(); return decodeOfflineRegionDefinition(query.get(0)); +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "load region"); + return nullopt; } -OfflineRegionStatus OfflineDatabase::getRegionCompletedStatus(int64_t regionID) { +optional OfflineDatabase::getRegionCompletedStatus(int64_t regionID) try { OfflineRegionStatus result; std::tie(result.completedResourceCount, result.completedResourceSize) @@ -786,6 +871,9 @@ OfflineRegionStatus OfflineDatabase::getRegionCompletedStatus(int64_t regionID) result.completedResourceSize += result.completedTileSize; return result; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "get region status"); + return nullopt; } std::pair OfflineDatabase::getCompletedResourceCountAndSize(int64_t regionID) { @@ -920,7 +1008,7 @@ bool OfflineDatabase::offlineMapboxTileCountLimitExceeded() { return getOfflineMapboxTileCount() >= offlineMapboxTileCountLimit; } -uint64_t OfflineDatabase::getOfflineMapboxTileCount() { +uint64_t OfflineDatabase::getOfflineMapboxTileCount() try { // Calculating this on every call would be much simpler than caching and // manually updating the value, but it would make offline downloads an O(n²) // operation, because the database query below involves an index scan of @@ -942,6 +1030,9 @@ uint64_t OfflineDatabase::getOfflineMapboxTileCount() { offlineMapboxTileCount = query.get(0); return *offlineMapboxTileCount; +} catch (const mapbox::sqlite::Exception& ex) { + handleError(ex, "get offline Mapbox tile count"); + return std::numeric_limits::max(); } bool OfflineDatabase::exceedsOfflineMapboxTileCountLimit(const Resource& resource) { diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp index 38eb3783ba8..cdad11b79e4 100644 --- a/platform/default/mbgl/storage/offline_database.hpp +++ b/platform/default/mbgl/storage/offline_database.hpp @@ -18,6 +18,7 @@ namespace sqlite { class Database; class Statement; class Query; +class Exception; } // namespace sqlite } // namespace mapbox @@ -26,6 +27,10 @@ namespace mbgl { class Response; class TileID; +namespace util { +struct IOException; +} // namespace util + struct MapboxTileLimitExceededException : util::Exception { MapboxTileLimitExceededException() : util::Exception("Mapbox tile limit exceeded") {} }; @@ -44,10 +49,10 @@ class OfflineDatabase : private util::noncopyable { std::vector listRegions(); - OfflineRegion createRegion(const OfflineRegionDefinition&, - const OfflineRegionMetadata&); + optional createRegion(const OfflineRegionDefinition&, + const OfflineRegionMetadata&); - OfflineRegionMetadata updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); + optional updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); void deleteRegion(OfflineRegion&&); @@ -57,8 +62,8 @@ class OfflineDatabase : private util::noncopyable { uint64_t putRegionResource(int64_t regionID, const Resource&, const Response&); void putRegionResources(int64_t regionID, const std::list>&, OfflineRegionStatus&); - OfflineRegionDefinition getRegionDefinition(int64_t regionID); - OfflineRegionStatus getRegionCompletedStatus(int64_t regionID); + optional getRegionDefinition(int64_t regionID); + optional getRegionCompletedStatus(int64_t regionID); void setOfflineMapboxTileCountLimit(uint64_t); uint64_t getOfflineMapboxTileCountLimit(); @@ -67,12 +72,15 @@ class OfflineDatabase : private util::noncopyable { bool exceedsOfflineMapboxTileCountLimit(const Resource&); private: - int userVersion(); - void ensureSchema(); + void initialize(); + void handleError(const mapbox::sqlite::Exception&, const char* action); + void handleError(const util::IOException&, const char* action); + void removeExisting(); void removeOldCacheTable(); - void migrateToVersion3(); + void createSchema(); void migrateToVersion5(); + void migrateToVersion3(); void migrateToVersion6(); mapbox::sqlite::Statement& getStatement(const char *); diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp index 4da51db6550..179d2d5f57d 100644 --- a/platform/default/mbgl/storage/offline_download.cpp +++ b/platform/default/mbgl/storage/offline_download.cpp @@ -62,39 +62,44 @@ OfflineRegionStatus OfflineDownload::getStatus() const { return status; } - OfflineRegionStatus result = offlineDatabase.getRegionCompletedStatus(id); + auto result = offlineDatabase.getRegionCompletedStatus(id); + if (!result) { + // We can't find this offline region because the database is unavailable, or the download + // does not exist. + return {}; + } - result.requiredResourceCount++; + result->requiredResourceCount++; optional styleResponse = offlineDatabase.get(Resource::style(definition.styleURL)); if (!styleResponse) { - return result; + return *result; } style::Parser parser; parser.parse(*styleResponse->data); - result.requiredResourceCountIsPrecise = true; + result->requiredResourceCountIsPrecise = true; for (const auto& source : parser.sources) { SourceType type = source->getType(); auto handleTiledSource = [&] (const variant& urlOrTileset, const uint16_t tileSize) { if (urlOrTileset.is()) { - result.requiredResourceCount += + result->requiredResourceCount += definition.tileCount(type, tileSize, urlOrTileset.get().zoomRange); } else { - result.requiredResourceCount += 1; + result->requiredResourceCount += 1; const auto& url = urlOrTileset.get(); optional sourceResponse = offlineDatabase.get(Resource::source(url)); if (sourceResponse) { style::conversion::Error error; optional tileset = style::conversion::convertJSON(*sourceResponse->data, error); if (tileset) { - result.requiredResourceCount += + result->requiredResourceCount += definition.tileCount(type, tileSize, (*tileset).zoomRange); } } else { - result.requiredResourceCountIsPrecise = false; + result->requiredResourceCountIsPrecise = false; } } }; @@ -121,7 +126,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { case SourceType::GeoJSON: { const auto& geojsonSource = *source->as(); if (geojsonSource.getURL()) { - result.requiredResourceCount += 1; + result->requiredResourceCount += 1; } break; } @@ -129,7 +134,7 @@ OfflineRegionStatus OfflineDownload::getStatus() const { case SourceType::Image: { const auto& imageSource = *source->as(); if (imageSource.getURL()) { - result.requiredResourceCount += 1; + result->requiredResourceCount += 1; } break; } @@ -142,14 +147,14 @@ OfflineRegionStatus OfflineDownload::getStatus() const { } if (!parser.glyphURL.empty()) { - result.requiredResourceCount += parser.fontStacks().size() * GLYPH_RANGES_PER_FONT_STACK; + result->requiredResourceCount += parser.fontStacks().size() * GLYPH_RANGES_PER_FONT_STACK; } if (!parser.spriteURL.empty()) { - result.requiredResourceCount += 2; + result->requiredResourceCount += 2; } - return result; + return *result; } void OfflineDownload::activateDownload() { diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp index 1a6045a9a87..fed5a3a1856 100644 --- a/platform/default/sqlite3.cpp +++ b/platform/default/sqlite3.cpp @@ -8,11 +8,38 @@ #include #include +#include #include namespace mapbox { namespace sqlite { +static_assert(mbgl::underlying_type(ResultCode::OK) == SQLITE_OK, "error"); +static_assert(mbgl::underlying_type(ResultCode::Error) == SQLITE_ERROR, "error"); +static_assert(mbgl::underlying_type(ResultCode::Internal) == SQLITE_INTERNAL, "error"); +static_assert(mbgl::underlying_type(ResultCode::Perm) == SQLITE_PERM, "error"); +static_assert(mbgl::underlying_type(ResultCode::Abort) == SQLITE_ABORT, "error"); +static_assert(mbgl::underlying_type(ResultCode::Busy) == SQLITE_BUSY, "error"); +static_assert(mbgl::underlying_type(ResultCode::Locked) == SQLITE_LOCKED, "error"); +static_assert(mbgl::underlying_type(ResultCode::NoMem) == SQLITE_NOMEM, "error"); +static_assert(mbgl::underlying_type(ResultCode::ReadOnly) == SQLITE_READONLY, "error"); +static_assert(mbgl::underlying_type(ResultCode::Interrupt) == SQLITE_INTERRUPT, "error"); +static_assert(mbgl::underlying_type(ResultCode::IOErr) == SQLITE_IOERR, "error"); +static_assert(mbgl::underlying_type(ResultCode::Corrupt) == SQLITE_CORRUPT, "error"); +static_assert(mbgl::underlying_type(ResultCode::NotFound) == SQLITE_NOTFOUND, "error"); +static_assert(mbgl::underlying_type(ResultCode::Full) == SQLITE_FULL, "error"); +static_assert(mbgl::underlying_type(ResultCode::CantOpen) == SQLITE_CANTOPEN, "error"); +static_assert(mbgl::underlying_type(ResultCode::Protocol) == SQLITE_PROTOCOL, "error"); +static_assert(mbgl::underlying_type(ResultCode::Schema) == SQLITE_SCHEMA, "error"); +static_assert(mbgl::underlying_type(ResultCode::TooBig) == SQLITE_TOOBIG, "error"); +static_assert(mbgl::underlying_type(ResultCode::Constraint) == SQLITE_CONSTRAINT, "error"); +static_assert(mbgl::underlying_type(ResultCode::Mismatch) == SQLITE_MISMATCH, "error"); +static_assert(mbgl::underlying_type(ResultCode::Misuse) == SQLITE_MISUSE, "error"); +static_assert(mbgl::underlying_type(ResultCode::NoLFS) == SQLITE_NOLFS, "error"); +static_assert(mbgl::underlying_type(ResultCode::Auth) == SQLITE_AUTH, "error"); +static_assert(mbgl::underlying_type(ResultCode::Range) == SQLITE_RANGE, "error"); +static_assert(mbgl::underlying_type(ResultCode::NotADB) == SQLITE_NOTADB, "error"); + class DatabaseImpl { public: DatabaseImpl(sqlite3* db_) @@ -24,7 +51,7 @@ class DatabaseImpl { { const int error = sqlite3_close(db); if (error != SQLITE_OK) { - mbgl::Log::Error(mbgl::Event::Database, "%s (Code %i)", sqlite3_errmsg(db), error); + mbgl::Log::Error(mbgl::Event::Database, error, "Failed to close database: %s", sqlite3_errmsg(db)); } } @@ -66,11 +93,8 @@ class StatementImpl { template using optional = std::experimental::optional; -static void errorLogCallback(void *, const int err, const char *msg) { - mbgl::Log::Record(mbgl::EventSeverity::Info, mbgl::Event::Database, err, "%s", msg); -} - -const static bool sqliteVersionCheck __attribute__((unused)) = []() { +__attribute__((constructor)) +static void initalize() { if (sqlite3_libversion_number() / 1000000 != SQLITE_VERSION_NUMBER / 1000000) { char message[96]; snprintf(message, 96, @@ -79,15 +103,17 @@ const static bool sqliteVersionCheck __attribute__((unused)) = []() { throw std::runtime_error(message); } +#ifndef NDEBUG // Enable SQLite logging before initializing the database. - sqlite3_config(SQLITE_CONFIG_LOG, errorLogCallback, nullptr); - - return true; -}(); + sqlite3_config(SQLITE_CONFIG_LOG, [](void *, const int err, const char *msg) { + mbgl::Log::Record(mbgl::EventSeverity::Debug, mbgl::Event::Database, err, "%s", msg); + }, nullptr); +#endif +} mapbox::util::variant Database::tryOpen(const std::string &filename, int flags) { sqlite3* db = nullptr; - const int error = sqlite3_open_v2(filename.c_str(), &db, flags, nullptr); + const int error = sqlite3_open_v2(filename.c_str(), &db, flags | SQLITE_OPEN_URI, nullptr); if (error != SQLITE_OK) { const auto message = sqlite3_errmsg(db); return Exception { error, message }; diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index 2ca09fd3ad0..96cee5fff8b 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -23,13 +23,6 @@ namespace mapbox { namespace sqlite { -// https://www.sqlite.org/rescode.html#ok -static_assert(mbgl::underlying_type(ResultCode::OK) == 0, "error"); -// https://www.sqlite.org/rescode.html#cantopen -static_assert(mbgl::underlying_type(ResultCode::CantOpen) == 14, "error"); -// https://www.sqlite.org/rescode.html#notadb -static_assert(mbgl::underlying_type(ResultCode::NotADB) == 26, "error"); - void checkQueryError(const QSqlQuery& query) { QSqlError lastError = query.lastError(); if (lastError.type() != QSqlError::NoError) { @@ -114,6 +107,11 @@ mapbox::util::variant Database::tryOpen(const std::string & connectOptions.append("QSQLITE_OPEN_READONLY"); } + if (filename.compare(0, 5, "file:") == 0) { + if (!connectOptions.isEmpty()) connectOptions.append(';'); + connectOptions.append("QSQLITE_OPEN_URI"); + } + db.setConnectOptions(connectOptions); db.setDatabaseName(QString(filename.c_str())); diff --git a/test/fixtures/offline_database/corrupt-delayed.db b/test/fixtures/offline_database/corrupt-delayed.db new file mode 100644 index 0000000000000000000000000000000000000000..04989dbf3687d5140d08b00bf487338919a6c801 GIT binary patch literal 19456 zcmeHP2Uru?+RhMqm1gKfP&!Bv5R~2oLazY?DWOOSLO^d*%T42w-2t-;dvme|1oRh!80fVT%c%>tH1v zbQ`)(0E2Gsw~VAGB!R(TKr|7?*3J!uadAU=;}T*OLp3F1HMp^oGC~cGD}wWp!{IIt za7|rfH4QaGxUQZtT-OAFfa@D->L?i^;o53QxRQyno~ACgvyPgsu@JU{kC&Sb2IcPQ zW{aJuv6?xy&-Z>{t*470%FV{h7UP2c)wp1Nz;BlN{bs5EZ5J2LV?|V~k)HxrN2lirq?zxvG`o)s|T_vA~tvgBx+mt^- z1R#NNasn3wabseqj`s1w4z>@VzZhq~_@BN#etlQCVkf${VEbf)EdJ*$CU&*N2#8^7 zod0OG_W7|9+k;s3(UM^kdP3&Oftx$V%jlK$nKFOu51t}^PX>+ zT4%mzaPoY2ZuBcf0R;Nb5_Qxssn}m51^T z1)kL^?G#jGtB z^~)(Corg1pE#8(y((!k&nxR8j(jVTE6KEKFu0XL2i@3^hKk0f_&MBn>EVtQ8w`!LO zZNe1|zQ8xcD+0KJE~d?```4l3BP}I*xH8^*{m^-__UC(Y^lmU_V9gkIFT9j$6R{z;UyRiXgo3nt@+a=@Di<7o5 zMyZZON{d)o7e)wt9(xxs8ZRp8CAC-?u+Gs$y)bff1H&G%{z+?nTS>M=Z=E7N&8B8C zbv|*`udAZ^jumyS_62_pc=CkFtK%}ph|=8pot%ru;jw%RIlJT16V;l_GW8ytQqs;y zGetJ-Ep!VELhGBb#S$;HVA(<*dd;oOm(@&8EHIy=o%Lx~Vd_-E$K&k^eZ(fWVA@NwR;X>ZVI>Q@ACzDGx zbMFwyg(_d9Szld{MwZZ+#0bQ9NwmALT0Kwk6Mq<7W+c*cMt&65oH`>%%d^^cQIg%c zTt&2l+>fcC*WWbc=@(@Et1vbN5AtM`(9HP#+6b;9>PK-h{EeBijmS*NkFkgS$nH%Y zfZkR;u{^qUG$(mpp@ayzLVD6L zBL2G)LZLpSMkf_W)7%Aq%1u+ZQyD)AyDn-+c~hQXeY;wVu`%oJ0G;C&l`IBcXHH>ydn;uLQ4E z@#aKV>eb!)%;Z%vd%x9gn}dzvwNi+@JE{iVGsU|y%(-vPI$wNF8zzZqK-MX^Ikxbx zP*#=3S+>S=uYS*i1c z^i%=z!=uGK^ka5k==Ys0gK)@Lk%$Gs5g1oaFUiir5xQ~8pc z=pvR#-k{4=PbSv_P2c*_3EhN_s7{+)(ykm1dC1|+u5l)$Y1L=pUP-Yu1#FsHJayHg zLUV&Upd;t(CPfs(n!4Z$!?sj0WUySBzO3I(YJle|S>?fY`4F*->CaJAod|;>MBJ3? z&{Lsy7(@4?3C1LLcPCHQ5n7q8>dH)0o~n3{POCye#siK+t5=+*s*(Mnh4FRbvxw#E z8U=~W2C^ozTTsP;5`s&?>qk4IKfR4yB-GiM%ugye>uXks@nuWDmYKlTn{Z6|0+iss z>VlOQQ%w89&YkQV*hY&-neRdTU-Iny!>(OVX;sU!SlomH1=dpEA ztSWii8?m?EF1AH8Ow2McS<31HoZ@m;*#qW5<%lC3uU1Y87LodhQFBSxQ_;q^2(S(V zhR^EA3XvCh^L$IIqm)M@2VK5+AYZgsq^Ezjij>=Z;sU=n+NUyN!?W}1JS4*+T;cl4 z;UV{QxOv|d62ynth0jMH#C%*3O?5V0By-c5W7`FexG6D()3&$7(p`PfG9Txy4L|=-0Pz8;=31-9E&J^$njUhq;DTtTJ}3$=AkK4NX`EY6Q+w zO;$0!^p8V)sH1XUoZBq8v6K5`BAHyYyboE>BWb2)6?-s0eCEa0)%XeG#uPfWII}8N zVswM%C(rzxk^@?$&yp`F)%IjCDmzLCh|FnZl8)ZCrYv42jk#G#pB%S44Ou>&w9DYP z)oDS|-8fE1dS+1T60|Jr7VYaZZN;AXx0(6Yp2Vmu>}Fl?6}`^bYlS)Mu5)W~liNm$ zKtrs>x`V)>_`x&x>~|q2%0l|=v~AJ}cvFrwOkw;Jtk+qS==Cb=od+@U=TV~qEpNl12fekKglJRlOIe=6{`l)uN# z?dzdsPbvA1e$#KJJ?Zf@f1w~w!#m|jD~H!$*Wf8ZsiiwXZ912g69*Now?-$cR?bou zwa;$d?6%X)IifGFduNN>fkZ8mquQ;a?K+*Mxtv(n%f6$?k6JXY_WlIB$&`t2R;VhB zNWRi(hBUs;&?YJAS0mIBp6K@4!i2b&>3t#B=$g?KObIQir#hg2=kqxM67P!a=(!Ip z8>bt{6%sc2DfKg=-ucFm@u@GAel2Yc;@)=I%n|o`E-O$mx%ioe|3%7c$d$Htx;@-f zi6j&}3*-HcQnIW9p;Rg%vDWa~k?fquV!2h&7*_@Nu=>Y!N6)jlHN=Y*J(I&!|5U6o zMaI!t@JSLSH0g=5cWZlOd|)+O$Ue@7Y2#%u``Y>HT{otiHmyQ09R@gUo(z#{x8|Wg zZRGXbU#ijPmhj@fAlDhVovOQGD*3Uum+_6d-t#qQ_8bcyskaKU)BK&|&oiw1>z@={ z*EL&fyEHwYC7(AJbmf>88*2FEWh1|g#Ugd{^RB2bB~^uOWg0h*#TiYZLPFO(o)JI3 zcnwnbcvZ}kYa1bv+3<*?VNy!KXG`f3m%@#XCpmFi9$Wt87eh_~R1SJ3{ART7m((r`@Vbg$dFb zb0I zRZSEI(>!t|$z1|X$(nsrZ@CUG<(Xp$C|h_crY5|Z%RD+?2GN}q&PD4!A`!7&I0>}Y zRMMDC9Lfo=7}s|m@s~JGr&(lGrLHkzY0Go1>)q+*haZl|RtRm3vZc3r&g(|?p{C^4 z1nA9m-&@jbExjXI6cN4?VR+>7<-t<2Gu7Ph1L#h9Yzp_zz&z9y*O^q=O!s|hruFTjxJx}B8v7chmXWg`m zBu7$-)*pB{N3eCro^!Thb`L`kcRa!9qqI6II2N8=%q>_jYvU{WN_lF!vZaBjU{r0& zCpJ3nrEitBfJp1yTPp0g00QU?1iA*DfsXBeE5`#o19%4h+zgOIp{!(HC?^**6#pl&q)@R1xPfC8JI06Eda%)6aWD>B_i9i|DnFWZ^w4RZ+Hgq4E(Pc zASNUO00vO)kpMj?_aJ}{)VmKr3ku!?H~=c%37`Qbe+HliHID?SK+zill%VRh0SZv| zssK5tU^##c6uuNd3M&67Kmtmi4N%n*OvP0F8fA5bWt6o5yc>2Jj60#TeMrKQ!+5 z?SCRV6=!x?Pp2ax?9dQ0YYn(gK9On%*#5n^IIA5ST&K0PL^90J` z9D&Dhen1hN8;}p@1?0jx0oiaqz=Jp!-~pTmkR0a#B;3>gziE>A)8HAvGw>h8fMr!g zvT8$FuA${|){hdJFU*G-I2fpRdXftY;-I3WY!x*|lB2?5#C9 z4+uzT4#oHVG_V})-cTBLM@507TL{ufo2Rg_`m{FnHKpEq`P=8&nF#a-b7Vt$zxYPK zpxX<{G^USR1)jw{&9M>mr7?%{)<@LhI!oEUEILOc)E=Yqqz781nK5Bb9A&9;sx#r! zuY(%IlER#>zAx>)M8!^}XNDA}DHv* z&V5u$i0VnWl;&ib;F9yilyYWxezI5DVTrag!5n#JNo-hY3PbLkaq9L(hMUG^)$%UQ zF}|PkjcA_RZdJ9-GXg`s{w9VLH#A&(UMZ!=Nu|h=qOLV$hF(Zex3_U`zErMm|@==oXq9X(4-90{Pwnz zD6y4sXhHDmwe6g%Nim~EY2l*=Z$(*N3uQy#2Al>7kf_4(Q4(=DX0tC*`GrrE`q`lx zid|xzLY{WTfUgS<<1Sp&o^~ImjpE-BLq9a{ZrYjB&^S&#$dou%f8XeVGx8Pbv}eN9 zx~!yVH3yM~P8h1^+^ty}<)xbI?q>uP5o~RKj%G%|DM>SrGxr$nazgGrL1Q|4Jmxqm9MB)Y)&CCsVvqnB!?J6u0XZwJ9!tkpEkTj^kM zmr^^5 zPZ62*$p)#*3vrpv>`YbhU}~@OM6wU*`c=W1y$W@0c(?Gz`@{9xFxHnJYUH`lhg_ZS_k4WQ z^`bB?!`S&`*^qEv8hNceB6cU6p1obnVGfYLuSE-$0mjGJzZvPb6U4UX6$$psJ$VpL zLPlNPRm-K)+*%VI!aBKoY1!dXOYNiS)4O99<`0;Sc1jWJ+ui2QJj*UtQb(t&0tPKF ztp>kSZ#kCvkgT4W8D;K5qD=?mzCjk8H6fF!}-1s!yisaMFLY32Ew_2Jk|bxS8Vps-%hz#;!X2gr`@mpvF8t` z->--B?Lq9&{JNCHFn$;Sh5!Tt$oZ^5uZK>n7a1ock{ zv4Rj35qJ`CV8Op|i&Xd?+UGPrw9o%tXdk${k2eN;cl+A9xnRQvp?(#%=9e*o{!kd8 ke|>D9KOCyYn=r!S=Wsw#P<%L`-v|LE^v~Y_*w8`$14hklVgLXD literal 0 HcmV?d00001 diff --git a/test/fixtures/offline_database/corrupt-immediate.db b/test/fixtures/offline_database/corrupt-immediate.db new file mode 100644 index 0000000000000000000000000000000000000000..8909c402b226547ece61086b4460889329da7d58 GIT binary patch literal 4096 zcmd5*XIPWlvQFq-sX=ONbde%WL25uCR1+W&K}vu~APAu&T?tjHbOGrCp`#!cdasHg zMI%+3B0aK|6TIp@=k9&~++X*5o>{Zzo${=icfRW;P&5t&cEozP+Ty^n04e|w2v7rq z0RVt1S=h<)`(q+ssD2qI$r?x&nqT@C{5Pcvkfc_lCc_kzd&f>T%0tRM3J~SNpU7}_ zDq0W-1iVL$v$eyZaA*w5^XEaMX$nz?LBKF|4JZWs69S7cfWc@7u&yBtq60An8ydmD zhHxkpY;3A)pl*r)>q8J=bvVpO*N~iS05OC~kQ2N-Fg7@pt2@S)+$aoULC*P=ckFdX z`=T&59=13%_HT8M{r>-m`u-#8_m3#b*B$MF^8D>PF6xSPKs%yQe>yqFP&iwspU*os z9Bgs6U=65|#<5|Kb#*5f@ci%iKOKOzAlmA1C=4uh9B6BA|IcB^PPn122^<0z`S)>1 z{CoQnV1Eg)uLRgnltEOKng*l-0s_!(4k%yG+ZghW*w}jEus>~^pJdob|5UWblr$i5 zaUkL+eZN9IQMbKNZsf=OSLSb%`adZBA5IZBTUV3>IaD-E5=aXY6a+?y|Ex)F9qZ*m zF81d_`5$%u!2jp<`1@A4kQ@C?*q>n^Px1dBnPj%4DQG|t!T*h|Kl8DX{(U9?eXR84 zgPrOWak%maYy-Qsw-{~xE_v-)aQ4*GVCR|uMGcbxrB5OoECwrmTP5^Y(K=5vYMt^1 zcX-vHt}nMdJvxK5%>y|#c?Vp)c;%~L)eG$JKcL2XX14pDW!vo`BL!t##_gDTFH_U= z2l)fna+BDiY92g~3x=Yz8b#uBG0$C2>+PmksQbDT+%z7m3T+eg6H(!9UQ+sf`T)L{ zT_&e1JO&eYv6Eiy5Q zu93R6S{k$Zdlz$Vo=w)^b3|Bi^ADNJ9*8q zK|HB1>s7=Wr(dfwZh*ob3OtW5$;i5 z6qR7m;_SK~Sy75$yvSxlB{4lfjoeuiwusXk-^r_b!A_)r}zm zC9yd3ye8~S0n4jsCDEqz%T0)Mxi9#0zVy%LPf|8$UD=o-UCc^YQ7sUm)Mn>(d!CVK z>)0^;#CdwP@!~K?+5MU(g=9=8t5#v}Jv&imIi=h~D32EW&WqW0>BO4Hb4%9_5RCQ) z%p;}ohq36zNr=yDOYH+;&bvKnjxcn#gpmS>TJ))`M6eg#b!An$6j!movQxC}G-s4S zB~o@wl~*bD4y*Jyn=+mbvpJFg$uosJ%Os~>6gK5HQhVy~dT8=)nU|#qQ}cH!mvI|W z)|S8c7G;-KxSTnNlV5(w8l3ZZb+yS`Rv1}$Fx#E<@Te|5VVmCGce~eyt0}Zb9-8w+ z%cOU)@JQ)Y_DA!sH(yiU(ndBSYE?0gt)g2@gt8mnlj#SM(tIn`HTvFme92B>AkUkW zO>=T=s)-07f$Lc&{!?1AW!l1QSf*-JRt$GfHj7YL& z{IJkN`Uav8ovqEs6$qPTx&?_$@z&5z-0R0jLR z-1su{ffm(BZ@$z=>9KtMv6nO=C@G%Hz0_Qn6gvyP9TED;uy_mksCkQza50otHGr8HGA#)|gf?-nT&0v0D0kM1*jTcX8ESjj8Za^sXD?O-FfZ>Q`jA z%Fz%S{A_AKbJ9lW$2%WDnq{bJ$=11X*Ho~@zyn(7XZ-5di!UO-tV$(0o37Dg43_zh zfafvl+@Y);t@so7>wmlt*xWsBs_7R1@lcNZ&QDbMVT6e!1{v3N9Kxo7TD@L0=ZsB9 z5<*-;%C|YYcdpjp3FEU?0XhLo%tQj`gkLoDb1k#$+VVl(;~&{Wvk44RWdn%3J~?v; z63>+zy7-3lFlLsfDUl5lZBF2&!8Yn{y60vUozyEHONdaf=}Y6(a8&S@T-Hgan`*FT zD%_xptgK*9h(1~XY}|-H;_xMPSu*xE&9Kqk9M!u|SsGH$`u=8np?hw_DUqF_NX^xw zj0kV363%`k?v|@T{o27<8+i&H=~nAb3WvfMW3HKVL03wH2JG~0QYnNJFE!5N{9>*5 zc;nfPD(akv(;b@wrNg_!m8{xlDnE0t>c+^zD>Ymw0rcLxcvH) zz;=UVjkr-YYtj^vuyu>6pks+t*=whpb>3LU@Ck{*fff=jP=zUPFJZH?P?7GQ7`TY| zqIbf@-jCuafhq387IXP^+HY*SK~3+|^l6I*AyftzW_vwIxFFY3qg@7_?nPbk#NeVj z=99)xzTOt2^(@c4xBQuB|3)K&YV3h1lX2SJIqyh%5$)CDZ^dnaXAjW_Su!54FN>8E z*S?+*eUtbe@SuIpudc^{{AFITU}nuAyfwIHGBc}3I-5Wl>7wcyQrA;^ z(T5My7$aRUrh=>bt59d2UZ5*)Qw}9TbVu1^+F!v=Zf8o^N851kPu$_(@u@n(a97&2 zNlZ8l3)&2g)9JV6U^n-3`kt>>8=sZ+I2)nT6>ykjxNj!+rN5u^gSOG@9cTV5OCkA> zs+Si;yJlXeSr66?<&_wk@3h}vSjo7WvmE%~5|R(~Ryp~)Z`xXcwuO%iYPX1x-(ITo z_)_%sc~nsFj@uYbPt+qoZO^u}`fkvXd82sckExv=ecg3uK_syH!${CX( zIA`AVJ*O0rT5q2*ah;6e51nG{pA#QCPU(L189Lk|2`{rb5a}+|mrkp_{jz>$j>`EC zE5$I|NUxT9eyl>;vND~TA(BMb+31I*}Kyp!r(M%QyJa1>0fLgn+bo~KIPx+VhIlrLms z4PVhp+O8@C+o~&0z-Lcqg_h44J5Tz_DzfPoAPL$!lUBAukGkh>w7mSRh%cAepW;hx zb6+vMJAj&3*%4#6F#Ke7g0wzIyC!+zNto&Rd8VW2uh<2J@DN(8@k%V?IxFA>$h06ivhe{lTO%P~Y-!8uIR> zS#Nt0TM5Fg{9Bw`AO7W?8p&)6!P7F$icY?h$YT3f$yVQhQT^@JTJqFJc%?@JGKENG z5RLHT6PGGd*-dv3aRs;aA5~w$HbgA=8)Cyr0U_dTRBaWVctEZ&|7J&HFPE_c>|B0C z(e#9$9pfEFOj~X%-Ho~vSH13yR!{RSz7t!yqR|{Yz9oEP+!^n_e2GQSra7QM(K5y3#^VsW#_G$w~G{fW4g9b(b`Cz VHwBsZ!rzOY@FrM`NwzJ2{1?*Q6Z!xE literal 0 HcmV?d00001 diff --git a/test/src/mbgl/test/fixture_log_observer.cpp b/test/src/mbgl/test/fixture_log_observer.cpp index d8a4b9edce3..d768c0284a1 100644 --- a/test/src/mbgl/test/fixture_log_observer.cpp +++ b/test/src/mbgl/test/fixture_log_observer.cpp @@ -32,7 +32,9 @@ bool FixtureLog::Observer::onRecord(EventSeverity severity, const std::string& msg) { std::lock_guard lock(messagesMutex); - messages.emplace_back(severity, event, code, msg); + if (severity != EventSeverity::Debug) { + messages.emplace_back(severity, event, code, msg); + } return true; } @@ -48,7 +50,7 @@ size_t FixtureLog::Observer::count(const Message& message, bool substring) const size_t message_count = 0; for (const auto& msg : messages) { - if (msg.matches(message, substring)) { + if (!msg.checked && msg.matches(message, substring)) { message_count++; msg.checked = true; } diff --git a/test/src/mbgl/test/sqlite3_test_fs.cpp b/test/src/mbgl/test/sqlite3_test_fs.cpp new file mode 100644 index 00000000000..16d411faffc --- /dev/null +++ b/test/src/mbgl/test/sqlite3_test_fs.cpp @@ -0,0 +1,320 @@ +#ifndef __QT__ // Qt doesn't expose SQLite VFS + +#include + +#include + +#include +#include +#include +#include +#include + +static bool sqlite3_test_fs_debug = false; +static bool sqlite3_test_fs_io = true; +static bool sqlite3_test_fs_file_open = true; +static bool sqlite3_test_fs_file_create = true; +static int64_t sqlite3_test_fs_read_limit = -1; +static int64_t sqlite3_test_fs_write_limit = -1; + +struct File { + sqlite3_file base; + sqlite3_file* real; +}; + +static int sqlite3_test_fs_close(sqlite3_file* pFile) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: close(%p)\n", pFile); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + const int rc = file->real->pMethods->xClose(file->real); + if (rc == SQLITE_OK) { + sqlite3_free((void*)file->base.pMethods); + file->base.pMethods = 0; + } + return rc; +} + +static int sqlite3_test_fs_read(sqlite3_file* pFile, void* zBuf, int iAmt, sqlite3_int64 iOfst) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: read(%p, amount=%d, offset=%lld)\n", pFile, iAmt, iOfst); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + if (sqlite3_test_fs_read_limit >= 0) { + if (iAmt > sqlite3_test_fs_read_limit) { + iAmt = 0; + return SQLITE_IOERR; + } + sqlite3_test_fs_read_limit -= iAmt; + } + File* file = (File*)pFile; + return file->real->pMethods->xRead(file->real, zBuf, iAmt, iOfst); +} + +static int sqlite3_test_fs_write(sqlite3_file* pFile, const void* zBuf, int iAmt, sqlite3_int64 iOfst) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: write(%p, amount=%d, offset=%lld)\n", pFile, iAmt, iOfst); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + if (sqlite3_test_fs_write_limit >= 0) { + if (iAmt > sqlite3_test_fs_write_limit) { + iAmt = 0; + return SQLITE_FULL; + } + sqlite3_test_fs_write_limit -= iAmt; + } + File* file = (File*)pFile; + return file->real->pMethods->xWrite(file->real, zBuf, iAmt, iOfst); +} + +static int sqlite3_test_fs_truncate(sqlite3_file* pFile, sqlite3_int64 size) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: truncate(%p, size=%lld)\n", pFile, size); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + return file->real->pMethods->xTruncate(file->real, size); +} + +static int sqlite3_test_fs_sync(sqlite3_file* pFile, int flags) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: sync(%p, flags=%d)\n", pFile, flags); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + return file->real->pMethods->xSync(file->real, flags); +} + +static int sqlite3_test_fs_file_size(sqlite3_file* pFile, sqlite3_int64* pSize) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: file_size(%p)\n", pFile); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + return file->real->pMethods->xFileSize(file->real, pSize); +} + +static int sqlite3_test_fs_lock(sqlite3_file* pFile, int eLock) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: lock(%p, %d)\n", pFile, eLock); + } + File* file = (File*)pFile; + return file->real->pMethods->xLock(file->real, eLock); +} + +static int sqlite3_test_fs_unlock(sqlite3_file* pFile, int eLock) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: unlock(%p, %d)\n", pFile, eLock); + } + File* file = (File*)pFile; + return file->real->pMethods->xUnlock(file->real, eLock); +} + +static int sqlite3_test_fs_check_reserved_lock(sqlite3_file* pFile, int* pResOut) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: check_reserved_lock(%p)\n", pFile); + } + File* file = (File*)pFile; + return file->real->pMethods->xCheckReservedLock(file->real, pResOut); +} + +static int sqlite3_test_fs_file_control(sqlite3_file* pFile, int op, void* pArg) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: file_control(%p, op=%d)\n", pFile, op); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + return file->real->pMethods->xFileControl(file->real, op, pArg); +} + +static int sqlite3_test_fs_sector_size(sqlite3_file* pFile) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: sector_size(%p)\n", pFile); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + return file->real->pMethods->xSectorSize(file->real); +} + +static int sqlite3_test_fs_device_characteristics(sqlite3_file* pFile) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: device_characteristics(%p)\n", pFile); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + File* file = (File*)pFile; + return file->real->pMethods->xDeviceCharacteristics(file->real); +} + +static int sqlite3_test_fs_open(sqlite3_vfs* vfs, const char* zName, sqlite3_file* pFile, int flags, int* pOutFlags) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: open(name=%s, flags=%d) -> %p\n", zName, flags, pFile); + } + if (!sqlite3_test_fs_io) { + pFile->pMethods = NULL; + return SQLITE_AUTH; + } + if (!sqlite3_test_fs_file_open) { + pFile->pMethods = NULL; + return SQLITE_CANTOPEN; + } + + File* file = (File*)pFile; + sqlite3_vfs* unix_fs = (sqlite3_vfs*)vfs->pAppData; + file->real = (sqlite3_file*)&file[1]; + + if (!sqlite3_test_fs_file_create) { + int res; + const int result = unix_fs->xAccess(vfs, zName, SQLITE_ACCESS_EXISTS, &res); + if (result != SQLITE_OK) { + pFile->pMethods = NULL; + return result; + } + if (res != 1) { + pFile->pMethods = NULL; + return SQLITE_CANTOPEN; + } + } + + const int status = unix_fs->xOpen(unix_fs, zName, file->real, flags, pOutFlags); + if (file->real->pMethods) { + sqlite3_io_methods* methods = (sqlite3_io_methods*)sqlite3_malloc(sizeof(sqlite3_io_methods)); + memset(methods, 0, sizeof(sqlite3_io_methods)); + methods->iVersion = 1; + methods->xClose = sqlite3_test_fs_close; + methods->xRead = sqlite3_test_fs_read; + methods->xWrite = sqlite3_test_fs_write; + methods->xTruncate = sqlite3_test_fs_truncate; + methods->xSync = sqlite3_test_fs_sync; + methods->xFileSize = sqlite3_test_fs_file_size; + methods->xLock = sqlite3_test_fs_lock; + methods->xUnlock = sqlite3_test_fs_unlock; + methods->xCheckReservedLock = sqlite3_test_fs_check_reserved_lock; + methods->xFileControl = sqlite3_test_fs_file_control; + methods->xSectorSize = sqlite3_test_fs_sector_size; + methods->xDeviceCharacteristics = sqlite3_test_fs_device_characteristics; + pFile->pMethods = methods; + } + return status; +} + +static int sqlite3_test_fs_delete(sqlite3_vfs* vfs, const char *zPath, int dirSync) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: delete(name=%s, sync=%d)\n", zPath, dirSync); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + sqlite3_vfs* unix_fs = (sqlite3_vfs*)vfs->pAppData; + return unix_fs->xDelete(unix_fs, zPath, dirSync); +} + +static int sqlite3_test_fs_access(sqlite3_vfs* vfs, const char *zPath, int flags, int *pResOut) { + if (sqlite3_test_fs_debug) { + fprintf(stderr, "SQLite3: access(name=%s, flags=%d)\n", zPath, flags); + } + if (!sqlite3_test_fs_io) { + return SQLITE_AUTH; + } + sqlite3_vfs* unix_fs = (sqlite3_vfs*)vfs->pAppData; + return unix_fs->xAccess(unix_fs, zPath, flags, pResOut); +} + +namespace mbgl { +namespace test { + +SQLite3TestFS::SQLite3TestFS() { + sqlite3_vfs* unix_fs = sqlite3_vfs_find("unix"); + if (unix_fs == 0) { + abort(); + } + + sqlite3_vfs* test_fs = (sqlite3_vfs*)sqlite3_malloc(sizeof(sqlite3_vfs)); + if (test_fs == 0) { + abort(); + } + memset(test_fs, 0, sizeof(sqlite3_vfs)); + test_fs->iVersion = 1; + test_fs->szOsFile = unix_fs->szOsFile + sizeof(File); + test_fs->mxPathname = unix_fs->mxPathname; + test_fs->zName = "test_fs"; + test_fs->pAppData = unix_fs; + test_fs->xOpen = sqlite3_test_fs_open; + test_fs->xDelete = sqlite3_test_fs_delete; + test_fs->xAccess = sqlite3_test_fs_access; + test_fs->xFullPathname = unix_fs->xFullPathname; + test_fs->xDlOpen = unix_fs->xDlOpen; + test_fs->xDlError = unix_fs->xDlError; + test_fs->xDlSym = unix_fs->xDlSym; + test_fs->xDlClose = unix_fs->xDlClose; + test_fs->xRandomness = unix_fs->xRandomness; + test_fs->xSleep = unix_fs->xSleep; + test_fs->xCurrentTime = unix_fs->xCurrentTime; + test_fs->xGetLastError = unix_fs->xGetLastError; + + sqlite3_vfs_register(test_fs, 0); +} + +SQLite3TestFS::~SQLite3TestFS() { + reset(); + sqlite3_vfs* test_fs = sqlite3_vfs_find("test_fs"); + if (test_fs) { + sqlite3_vfs_unregister(test_fs); + } +} + +void SQLite3TestFS::setDebug(bool value) { + sqlite3_test_fs_debug = value; +} + +void SQLite3TestFS::allowIO(bool value) { + sqlite3_test_fs_io = value; +} + +void SQLite3TestFS::allowFileOpen(bool value) { + sqlite3_test_fs_file_open = value; +} + +void SQLite3TestFS::allowFileCreate(bool value) { + sqlite3_test_fs_file_create = value; +} + +void SQLite3TestFS::setReadLimit(int64_t value) { + sqlite3_test_fs_read_limit = value; +} + +void SQLite3TestFS::setWriteLimit(int64_t value) { + sqlite3_test_fs_write_limit = value; +} + +void SQLite3TestFS::reset() { + setDebug(false); + allowIO(true); + allowFileOpen(true); + allowFileCreate(true); + setReadLimit(-1); + setWriteLimit(-1); +} + +} // namespace test +} // namespace mbgl + +#endif // __QT__ diff --git a/test/src/mbgl/test/sqlite3_test_fs.hpp b/test/src/mbgl/test/sqlite3_test_fs.hpp new file mode 100644 index 00000000000..00351f49ac2 --- /dev/null +++ b/test/src/mbgl/test/sqlite3_test_fs.hpp @@ -0,0 +1,39 @@ +#pragma once + +#include + +namespace mbgl { +namespace test { + +class SQLite3TestFS { +public: + SQLite3TestFS(); + ~SQLite3TestFS(); + + // When enabled, the VFS will log all I/O operations to stdout. + void setDebug(bool); + + // Allow any type of I/O. Will fail with SQLITE_AUTH if set to false. This is useful to simulate + // scenarios where the OS blocks an entire app's I/O, e.g. when it's in the background. + void allowIO(bool); + + // Allow files to be opened. Will fail with SQLITE_CANTOPEN if set to false. + void allowFileOpen(bool); + + // Allow files to be created. Will fail with SQLITE_CANTOPEN if set to false. + void allowFileCreate(bool); + + // Allow N bytes to be read, then fail reads with SQLITE_IOERR. -1 == unlimited + // This limit is global, not per file. + void setReadLimit(int64_t); + + // Allow N bytes to be written, then fail writes with SQLITE_FULL. -1 == unlimited + // This limit is global, not per file. + void setWriteLimit(int64_t); + + // Reset all restrictions. + void reset(); +}; + +} // namespace test +} // namespace mbgl diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 000f24e1cdd..de40d8caf70 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -13,51 +14,235 @@ using namespace std::literals::string_literals; using namespace mbgl; +using mapbox::sqlite::ResultCode; static constexpr const char* filename = "test/fixtures/offline_database/offline.db"; +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +static constexpr const char* filename_test_fs = "file:test/fixtures/offline_database/offline.db?vfs=test_fs"; +#endif -TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Create)) { - FixtureLog log; +static void deleteDatabaseFiles() { + // Delete leftover journaling files as well. util::deleteFile(filename); + util::deleteFile(filename + "-wal"s); + util::deleteFile(filename + "-journal"s); +} + +static FixtureLog::Message error(ResultCode code, const char* message) { + return { EventSeverity::Error, Event::Database, static_cast(code), message }; +} + +static FixtureLog::Message warning(ResultCode code, const char* message) { + return { EventSeverity::Warning, Event::Database, static_cast(code), message }; +} + +static int databasePageCount(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma page_count" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} + +static int databaseUserVersion(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma user_version" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} + +static std::string databaseJournalMode(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma journal_mode" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} +static int databaseSyncMode(const std::string& path) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + mapbox::sqlite::Statement stmt{ db, "pragma synchronous" }; + mapbox::sqlite::Query query{ stmt }; + query.run(); + return query.get(0); +} + +static std::vector databaseTableColumns(const std::string& path, const std::string& name) { + mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); + const auto sql = std::string("pragma table_info(") + name + ")"; + mapbox::sqlite::Statement stmt{ db, sql.c_str() }; + mapbox::sqlite::Query query{ stmt }; + std::vector columns; + while (query.run()) { + columns.push_back(query.get(1)); + } + return columns; +} + +namespace fixture { + +const Resource resource{ Resource::Style, "mapbox://test" }; +const Resource tile = Resource::tile("mapbox://test", 1, 0, 0, 0, Tileset::Scheme::XYZ); +const Response response = [] { + Response res; + res.data = std::make_shared("first"); + return res; +}(); + +} // namespace + +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Create)) { + FixtureLog log; + deleteDatabaseFiles(); OfflineDatabase db(filename); EXPECT_FALSE(bool(db.get({ Resource::Unknown, "mapbox://test" }))); EXPECT_EQ(0u, log.uncheckedCount()); } +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(CreateFail)) { + FixtureLog log; + deleteDatabaseFiles(); + test::SQLite3TestFS fs; + + // Opening the database will fail because our mock VFS returns a SQLITE_CANTOPEN error because + // it is not allowed to create the file. The OfflineDatabase object should handle this gracefully + // and treat it like an empty cache that can't be written to. + fs.allowFileCreate(false); + OfflineDatabase db(filename_test_fs); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't open database: unable to open database file"))); + + EXPECT_EQ(0u, log.uncheckedCount()); + + // We can try to insert things into the cache, but since the cache database isn't open, it won't be stored. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(false, uint64_t(0)), db.put(res, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't write resource: unable to open database file"))); + EXPECT_EQ(0u, log.uncheckedCount()); + } + + // We can also still "query" the database even though it is not open, and we will always get an empty result. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_FALSE(bool(db.get(res))); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't update timestamp: unable to open database file"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't read resource: unable to open database file"))); + EXPECT_EQ(0u, log.uncheckedCount()); + } + + // Now, we're "freeing up" some space on the disk, and try to insert and query again. This time, we should + // be opening the datbase, creating the schema, and writing the data so that we can retrieve it again. + fs.allowFileCreate(true); + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } + + // Next, set the file system to read only mode and try to read the data again. While we can't + // write anymore, we should still be able to read, and the query that tries to update the last + // accessed timestamp may fail without crashing. + fs.allowFileCreate(false); + fs.setWriteLimit(0); + for (const auto& res : { fixture::resource, fixture::tile }) { + auto result = db.get(res); + EXPECT_EQ(1u, log.count(warning(ResultCode::CantOpen, "Can't update timestamp: unable to open database file"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } + fs.setDebug(false); + + // We're allowing SQLite to create a journal file, but restrict the number of bytes it + // can write so that it can start writing the journal file, but eventually fails during the + // timestamp update. + fs.allowFileCreate(true); + fs.setWriteLimit(8192); + for (const auto& res : { fixture::resource, fixture::tile }) { + auto result = db.get(res); + EXPECT_EQ(1u, log.count(warning(ResultCode::Full, "Can't update timestamp: database or disk is full"))); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } + + // Lastly, we're disabling all I/O to simulate a backgrounded app that is restricted from doing + // any disk I/O at all. + fs.setWriteLimit(-1); + fs.allowIO(false); + for (const auto& res : { fixture::resource, fixture::tile }) { + // First, try reading. + auto result = db.get(res); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update timestamp: authorization denied"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't read resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + EXPECT_FALSE(result); + + // Now try inserting. + EXPECT_EQ(std::make_pair(false, uint64_t(0)), db.put(res, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + } + + // Allow deleting the database. + fs.reset(); +} +#endif // __QT__ + TEST(OfflineDatabase, TEST_REQUIRES_WRITE(SchemaVersion)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); { mapbox::sqlite::Database db = mapbox::sqlite::Database::open(filename, mapbox::sqlite::ReadWriteCreate); + db.setBusyTimeout(Milliseconds(1000)); db.exec("PRAGMA user_version = 1"); } + { + OfflineDatabase db(filename); + } + + EXPECT_EQ(6, databaseUserVersion(filename)); + OfflineDatabase db(filename); + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } EXPECT_EQ(0u, log.uncheckedCount()); } TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Invalid)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::write_file(filename, "this is an invalid file"); OfflineDatabase db(filename); - -#ifndef __QT__ - // Only non-Qt platforms are setting a logger on the SQLite object. // Checking two possibilities for the error string because it apparently changes between SQLite versions. - EXPECT_EQ(1u, - log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::NotADB), - "statement aborts at 1: [PRAGMA user_version] file is encrypted or is not a database" }, true) + - log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::NotADB), - "statement aborts at 1: [PRAGMA user_version] file is not a database" }, true)); -#endif - EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); - EXPECT_EQ(0u, log.uncheckedCount()); + EXPECT_EQ(1u, log.count(error(ResultCode::NotADB, "Can't open database: file is encrypted or is not a database"), true) + + log.count(error(ResultCode::NotADB, "Can't open database: file is not a database"), true)); + EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } } TEST(OfflineDatabase, PutDoesNotStoreConnectionErrors) { @@ -118,7 +303,7 @@ TEST(OfflineDatabase, PutResource) { TEST(OfflineDatabase, TEST_REQUIRES_WRITE(GetResourceFromOfflineRegion)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/satellite_test.db"); OfflineDatabase db(filename, mapbox::sqlite::ReadOnly); @@ -228,14 +413,15 @@ TEST(OfflineDatabase, CreateRegion) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); - EXPECT_EQ(definition.styleURL, region.getDefinition().styleURL); - EXPECT_EQ(definition.bounds, region.getDefinition().bounds); - EXPECT_EQ(definition.minZoom, region.getDefinition().minZoom); - EXPECT_EQ(definition.maxZoom, region.getDefinition().maxZoom); - EXPECT_EQ(definition.pixelRatio, region.getDefinition().pixelRatio); - EXPECT_EQ(metadata, region.getMetadata()); + EXPECT_EQ(definition.styleURL, region->getDefinition().styleURL); + EXPECT_EQ(definition.bounds, region->getDefinition().bounds); + EXPECT_EQ(definition.minZoom, region->getDefinition().minZoom); + EXPECT_EQ(definition.maxZoom, region->getDefinition().maxZoom); + EXPECT_EQ(definition.pixelRatio, region->getDefinition().pixelRatio); + EXPECT_EQ(metadata, region->getMetadata()); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -245,10 +431,11 @@ TEST(OfflineDatabase, UpdateMetadata) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); OfflineRegionMetadata newmetadata {{ 4, 5, 6 }}; - db.updateMetadata(region.getID(), newmetadata); + db.updateMetadata(region->getID(), newmetadata); EXPECT_EQ(db.listRegions().at(0).getMetadata(), newmetadata); EXPECT_EQ(0u, log.uncheckedCount()); @@ -260,11 +447,12 @@ TEST(OfflineDatabase, ListRegions) { OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); std::vector regions = db.listRegions(); ASSERT_EQ(1u, regions.size()); - EXPECT_EQ(region.getID(), regions.at(0).getID()); + EXPECT_EQ(region->getID(), regions.at(0).getID()); EXPECT_EQ(definition.styleURL, regions.at(0).getDefinition().styleURL); EXPECT_EQ(definition.bounds, regions.at(0).getDefinition().bounds); EXPECT_EQ(definition.minZoom, regions.at(0).getDefinition().minZoom); @@ -281,14 +469,16 @@ TEST(OfflineDatabase, GetRegionDefinition) { OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); - OfflineRegionDefinition result = db.getRegionDefinition(region.getID()); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); + auto result = db.getRegionDefinition(region->getID()); + ASSERT_TRUE(result); - EXPECT_EQ(definition.styleURL, result.styleURL); - EXPECT_EQ(definition.bounds, result.bounds); - EXPECT_EQ(definition.minZoom, result.minZoom); - EXPECT_EQ(definition.maxZoom, result.maxZoom); - EXPECT_EQ(definition.pixelRatio, result.pixelRatio); + EXPECT_EQ(definition.styleURL, result->styleURL); + EXPECT_EQ(definition.bounds, result->bounds); + EXPECT_EQ(definition.minZoom, result->minZoom); + EXPECT_EQ(definition.maxZoom, result->maxZoom); + EXPECT_EQ(definition.pixelRatio, result->pixelRatio); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -298,15 +488,16 @@ TEST(OfflineDatabase, DeleteRegion) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata {{ 1, 2, 3 }}; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); Response response; response.noContent = true; - db.putRegionResource(region.getID(), Resource::style("http://example.com/"), response); - db.putRegionResource(region.getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); + db.putRegionResource(region->getID(), Resource::style("http://example.com/"), response); + db.putRegionResource(region->getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); - db.deleteRegion(std::move(region)); + db.deleteRegion(std::move(*region)); ASSERT_EQ(0u, db.listRegions().size()); @@ -318,38 +509,35 @@ TEST(OfflineDatabase, CreateRegionInfiniteMaxZoom) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; OfflineRegionMetadata metadata; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); - EXPECT_EQ(0, region.getDefinition().minZoom); - EXPECT_EQ(INFINITY, region.getDefinition().maxZoom); + EXPECT_EQ(0, region->getDefinition().minZoom); + EXPECT_EQ(INFINITY, region->getDefinition().maxZoom); EXPECT_EQ(0u, log.uncheckedCount()); } TEST(OfflineDatabase, TEST_REQUIRES_WRITE(ConcurrentUse)) { FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); OfflineDatabase db1(filename); EXPECT_EQ(0u, log.uncheckedCount()); OfflineDatabase db2(filename); - Resource resource { Resource::Style, "http://example.com/" }; - Response response; - response.noContent = true; - std::thread thread1([&] { for (auto i = 0; i < 100; i++) { - db1.put(resource, response); - EXPECT_TRUE(bool(db1.get(resource))); + db1.put(fixture::resource, fixture::response); + EXPECT_TRUE(bool(db1.get(fixture::resource))); } }); std::thread thread2([&] { for (auto i = 0; i < 100; i++) { - db2.put(resource, response); - EXPECT_TRUE(bool(db2.get(resource))); + db2.put(fixture::resource, fixture::response); + EXPECT_TRUE(bool(db2.get(fixture::resource))); } }); @@ -411,13 +599,14 @@ TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Response response; response.data = randomString(1024); for (uint32_t i = 1; i <= 100; i++) { - db.putRegionResource(region.getID(), Resource::style("http://example.com/"s + util::toString(i)), response); + db.putRegionResource(region->getID(), Resource::style("http://example.com/"s + util::toString(i)), response); } EXPECT_TRUE(bool(db.get(Resource::style("http://example.com/1")))); @@ -448,32 +637,36 @@ TEST(OfflineDatabase, GetRegionCompletedStatus) { OfflineDatabase db(":memory:"); OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata; - OfflineRegion region = db.createRegion(definition, metadata); + auto region = db.createRegion(definition, metadata); + ASSERT_TRUE(region); - OfflineRegionStatus status1 = db.getRegionCompletedStatus(region.getID()); - EXPECT_EQ(0u, status1.completedResourceCount); - EXPECT_EQ(0u, status1.completedResourceSize); - EXPECT_EQ(0u, status1.completedTileCount); - EXPECT_EQ(0u, status1.completedTileSize); + auto status1 = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(status1); + EXPECT_EQ(0u, status1->completedResourceCount); + EXPECT_EQ(0u, status1->completedResourceSize); + EXPECT_EQ(0u, status1->completedTileCount); + EXPECT_EQ(0u, status1->completedTileSize); Response response; response.data = std::make_shared("data"); - uint64_t styleSize = db.putRegionResource(region.getID(), Resource::style("http://example.com/"), response); + uint64_t styleSize = db.putRegionResource(region->getID(), Resource::style("http://example.com/"), response); - OfflineRegionStatus status2 = db.getRegionCompletedStatus(region.getID()); - EXPECT_EQ(1u, status2.completedResourceCount); - EXPECT_EQ(styleSize, status2.completedResourceSize); - EXPECT_EQ(0u, status2.completedTileCount); - EXPECT_EQ(0u, status2.completedTileSize); + auto status2 = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(status2); + EXPECT_EQ(1u, status2->completedResourceCount); + EXPECT_EQ(styleSize, status2->completedResourceSize); + EXPECT_EQ(0u, status2->completedTileCount); + EXPECT_EQ(0u, status2->completedTileSize); - uint64_t tileSize = db.putRegionResource(region.getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); + uint64_t tileSize = db.putRegionResource(region->getID(), Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ), response); - OfflineRegionStatus status3 = db.getRegionCompletedStatus(region.getID()); - EXPECT_EQ(2u, status3.completedResourceCount); - EXPECT_EQ(styleSize + tileSize, status3.completedResourceSize); - EXPECT_EQ(1u, status3.completedTileCount); - EXPECT_EQ(tileSize, status3.completedTileSize); + auto status3 = db.getRegionCompletedStatus(region->getID()); + ASSERT_TRUE(status3); + EXPECT_EQ(2u, status3->completedResourceCount); + EXPECT_EQ(styleSize + tileSize, status3->completedResourceSize); + EXPECT_EQ(1u, status3->completedTileCount); + EXPECT_EQ(tileSize, status3->completedTileSize); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -482,21 +675,22 @@ TEST(OfflineDatabase, HasRegionResource) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); - EXPECT_FALSE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/1")))); - EXPECT_FALSE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/20")))); + EXPECT_FALSE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/1")))); + EXPECT_FALSE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/20")))); Response response; response.data = randomString(1024); for (uint32_t i = 1; i <= 100; i++) { - db.putRegionResource(region.getID(), Resource::style("http://example.com/"s + util::toString(i)), response); + db.putRegionResource(region->getID(), Resource::style("http://example.com/"s + util::toString(i)), response); } - EXPECT_TRUE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/1")))); - EXPECT_TRUE(bool(db.hasRegionResource(region.getID(), Resource::style("http://example.com/20")))); - EXPECT_EQ(1024, *(db.hasRegionResource(region.getID(), Resource::style("http://example.com/20")))); + EXPECT_TRUE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/1")))); + EXPECT_TRUE(bool(db.hasRegionResource(region->getID(), Resource::style("http://example.com/20")))); + EXPECT_EQ(1024, *(db.hasRegionResource(region->getID(), Resource::style("http://example.com/20")))); EXPECT_EQ(0u, log.uncheckedCount()); } @@ -505,7 +699,8 @@ TEST(OfflineDatabase, HasRegionResourceTile) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Resource resource { Resource::Tile, "http://example.com/" }; resource.tileData = Resource::TileData { @@ -519,15 +714,16 @@ TEST(OfflineDatabase, HasRegionResourceTile) { response.data = std::make_shared("first"); - EXPECT_FALSE(bool(db.hasRegionResource(region.getID(), resource))); - db.putRegionResource(region.getID(), resource, response); - EXPECT_TRUE(bool(db.hasRegionResource(region.getID(), resource))); - EXPECT_EQ(5, *(db.hasRegionResource(region.getID(), resource))); + EXPECT_FALSE(bool(db.hasRegionResource(region->getID(), resource))); + db.putRegionResource(region->getID(), resource, response); + EXPECT_TRUE(bool(db.hasRegionResource(region->getID(), resource))); + EXPECT_EQ(5, *(db.hasRegionResource(region->getID(), resource))); - OfflineRegion anotherRegion = db.createRegion(definition, OfflineRegionMetadata()); - EXPECT_LT(region.getID(), anotherRegion.getID()); - EXPECT_TRUE(bool(db.hasRegionResource(anotherRegion.getID(), resource))); - EXPECT_EQ(5, *(db.hasRegionResource(anotherRegion.getID(), resource))); + auto anotherRegion = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(anotherRegion); + EXPECT_LT(region->getID(), anotherRegion->getID()); + EXPECT_TRUE(bool(db.hasRegionResource(anotherRegion->getID(), resource))); + EXPECT_EQ(5, *(db.hasRegionResource(anotherRegion->getID(), resource))); EXPECT_EQ(0u, log.uncheckedCount()); @@ -539,8 +735,10 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { OfflineRegionDefinition definition { "http://example.com/style", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 2.0 }; OfflineRegionMetadata metadata; - OfflineRegion region1 = db.createRegion(definition, metadata); - OfflineRegion region2 = db.createRegion(definition, metadata); + auto region1 = db.createRegion(definition, metadata); + ASSERT_TRUE(region1); + auto region2 = db.createRegion(definition, metadata); + ASSERT_TRUE(region2); Resource nonMapboxTile = Resource::tile("http://example.com/", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); Resource mapboxTile1 = Resource::tile("mapbox://tiles/1", 1.0, 0, 0, 0, Tileset::Scheme::XYZ); @@ -553,27 +751,27 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a non-tile resource. - db.putRegionResource(region1.getID(), Resource::style("http://example.com/"), response); + db.putRegionResource(region1->getID(), Resource::style("http://example.com/"), response); EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a non-Mapbox tile. - db.putRegionResource(region1.getID(), nonMapboxTile, response); + db.putRegionResource(region1->getID(), nonMapboxTile, response); EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); // Count increases after putting a Mapbox tile not used by another region. - db.putRegionResource(region1.getID(), mapboxTile1, response); + db.putRegionResource(region1->getID(), mapboxTile1, response); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a Mapbox tile used by another region. - db.putRegionResource(region2.getID(), mapboxTile1, response); + db.putRegionResource(region2->getID(), mapboxTile1, response); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after putting a Mapbox tile used by the same region. - db.putRegionResource(region2.getID(), mapboxTile1, response); + db.putRegionResource(region2->getID(), mapboxTile1, response); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after deleting a region when the tile is still used by another region. - db.deleteRegion(std::move(region2)); + db.deleteRegion(std::move(*region2)); EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count stays the same after the putting a non-offline Mapbox tile. @@ -581,11 +779,11 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) { EXPECT_EQ(1u, db.getOfflineMapboxTileCount()); // Count increases after putting a pre-existing, but non-offline Mapbox tile. - db.putRegionResource(region1.getID(), mapboxTile2, response); + db.putRegionResource(region1->getID(), mapboxTile2, response); EXPECT_EQ(2u, db.getOfflineMapboxTileCount()); // Count decreases after deleting a region when the tiles are not used by other regions. - db.deleteRegion(std::move(region1)); + db.deleteRegion(std::move(*region1)); EXPECT_EQ(0u, db.getOfflineMapboxTileCount()); EXPECT_EQ(0u, log.uncheckedCount()); @@ -596,7 +794,8 @@ TEST(OfflineDatabase, BatchInsertion) { FixtureLog log; OfflineDatabase db(":memory:", 1024 * 100); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Response response; response.data = randomString(1024); @@ -607,7 +806,7 @@ TEST(OfflineDatabase, BatchInsertion) { } OfflineRegionStatus status; - db.putRegionResources(region.getID(), resources, status); + db.putRegionResources(region->getID(), resources, status); for (uint32_t i = 1; i <= 100; i++) { EXPECT_TRUE(bool(db.get(Resource::style("http://example.com/"s + util::toString(i))))); @@ -621,7 +820,8 @@ TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) { OfflineDatabase db(":memory:", 1024 * 100); db.setOfflineMapboxTileCountLimit(1); OfflineRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0 }; - OfflineRegion region = db.createRegion(definition, OfflineRegionMetadata()); + auto region = db.createRegion(definition, OfflineRegionMetadata()); + ASSERT_TRUE(region); Response response; response.data = randomString(1024); @@ -633,68 +833,26 @@ TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) { OfflineRegionStatus status; try { - db.putRegionResources(region.getID(), resources, status); + db.putRegionResources(region->getID(), resources, status); EXPECT_FALSE(true); } catch (const MapboxTileLimitExceededException&) { // Expected } - EXPECT_EQ(status.completedTileCount, 1u); - EXPECT_EQ(status.completedResourceCount, 2u); - EXPECT_EQ(db.getRegionCompletedStatus(region.getID()).completedTileCount, 1u); - EXPECT_EQ(db.getRegionCompletedStatus(region.getID()).completedResourceCount, 2u); + 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); EXPECT_EQ(0u, log.uncheckedCount()); } -static int databasePageCount(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma page_count" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} - -static int databaseUserVersion(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma user_version" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} - -static std::string databaseJournalMode(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma journal_mode" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} - -static int databaseSyncMode(const std::string& path) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - mapbox::sqlite::Statement stmt{ db, "pragma synchronous" }; - mapbox::sqlite::Query query{ stmt }; - query.run(); - return query.get(0); -} - -static std::vector databaseTableColumns(const std::string& path, const std::string& name) { - mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly); - const auto sql = std::string("pragma table_info(") + name + ")"; - mapbox::sqlite::Statement stmt{ db, sql.c_str() }; - mapbox::sqlite::Query query{ stmt }; - std::vector columns; - while (query.run()) { - columns.push_back(query.get(1)); - } - return columns; -} - TEST(OfflineDatabase, MigrateFromV2Schema) { // v2.db is a v2 database containing a single offline region with a small number of resources. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v2.db"); { @@ -715,7 +873,7 @@ TEST(OfflineDatabase, MigrateFromV2Schema) { TEST(OfflineDatabase, MigrateFromV3Schema) { // v3.db is a v3 database, migrated from v2. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v3.db"); { @@ -734,7 +892,7 @@ TEST(OfflineDatabase, MigrateFromV3Schema) { TEST(OfflineDatabase, MigrateFromV4Schema) { // v4.db is a v4 database, migrated from v2 & v3. This database used `journal_mode = WAL` and `synchronous = NORMAL`. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v4.db"); { @@ -760,7 +918,7 @@ TEST(OfflineDatabase, MigrateFromV4Schema) { TEST(OfflineDatabase, MigrateFromV5Schema) { // v5.db is a v5 database, migrated from v2, v3 & v4. FixtureLog log; - util::deleteFile(filename); + deleteDatabaseFiles(); util::copyFile(filename, "test/fixtures/offline_database/v5.db"); { @@ -808,3 +966,133 @@ TEST(OfflineDatabase, DowngradeSchema) { EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); EXPECT_EQ(0u, log.uncheckedCount()); } + +TEST(OfflineDatabase, CorruptDatabaseOnOpen) { + FixtureLog log; + util::deleteFile(filename); + util::copyFile(filename, "test/fixtures/offline_database/corrupt-immediate.db"); + + // This database is corrupt in a way that will prevent opening the database. + OfflineDatabase db(filename); + EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't open database: database disk image is malformed"), true)); + EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } +} + +TEST(OfflineDatabase, CorruptDatabaseOnQuery) { + FixtureLog log; + util::deleteFile(filename); + util::copyFile(filename, "test/fixtures/offline_database/corrupt-delayed.db"); + + // This database is corrupt in a way that won't manifest itself until we start querying it, + // so just opening it will not cause an error. + OfflineDatabase db(filename); + + // Just opening this corrupt database should not have produced an error yet, since + // PRAGMA user_version still succeeds with this database. + EXPECT_EQ(0u, log.uncheckedCount()); + + // The first request fails because the database is corrupt and has to be recreated. + EXPECT_EQ(nullopt, db.get(fixture::tile)); + EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't read resource: database disk image is malformed"), true)); + EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + // Now try inserting and reading back to make sure we have a valid database. + for (const auto& res : { fixture::resource, fixture::tile }) { + EXPECT_EQ(std::make_pair(true, uint64_t(5)), db.put(res, fixture::response)); + EXPECT_EQ(0u, log.uncheckedCount()); + auto result = db.get(res); + EXPECT_EQ(0u, log.uncheckedCount()); + ASSERT_TRUE(result && result->data); + EXPECT_EQ("first", *result->data); + } +} + +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +TEST(OfflineDatabase, TEST_REQUIRES_WRITE(DisallowedIO)) { + FixtureLog log; + deleteDatabaseFiles(); + test::SQLite3TestFS fs; + + OfflineDatabase db(filename_test_fs); + EXPECT_EQ(0u, log.uncheckedCount()); + + // First, create a region object so that we can try deleting it later. + OfflineTilePyramidRegionDefinition definition( + "mapbox://style", LatLngBounds::hull({ 37.66, -122.57 }, { 37.83, -122.32 }), 0, 8, 2); + auto region = db.createRegion(definition, {}); + ASSERT_TRUE(region); + + // Now forbid any type of IO on the database and test that none of the calls crashes. + fs.allowIO(false); + + EXPECT_EQ(nullopt, db.get(fixture::resource)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update timestamp: authorization denied"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't read resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(std::make_pair(false, uint64_t(0)), db.put(fixture::resource, fixture::response)); + 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_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_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_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update region metadata: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.getRegionResource(region->getID(), fixture::resource)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't update timestamp: authorization denied"))); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't read region resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(nullopt, db.hasRegionResource(region->getID(), fixture::resource)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't query region resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(0u, db.putRegionResource(region->getID(), fixture::resource, fixture::response)); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't write region resource: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + OfflineRegionStatus status; + db.putRegionResources(region->getID(), { std::make_tuple(fixture::resource, fixture::response) }, status); + 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_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_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_EQ(1u, log.count(warning(ResultCode::Auth, "Can't delete region: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + EXPECT_EQ(std::numeric_limits::max(), db.getOfflineMapboxTileCount()); + EXPECT_EQ(1u, log.count(warning(ResultCode::Auth, "Can't get offline Mapbox tile count: authorization denied"))); + EXPECT_EQ(0u, log.uncheckedCount()); + + fs.reset(); +} +#endif // __QT__ diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index 57780eba400..e87ad6c3709 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -42,7 +42,7 @@ class OfflineTest { OfflineDatabase db { ":memory:" }; std::size_t size = 0; - OfflineRegion createRegion() { + optional createRegion() { OfflineRegionDefinition definition { "", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 1.0 }; OfflineRegionMetadata metadata; return db.createRegion(definition, metadata); @@ -60,9 +60,10 @@ class OfflineTest { TEST(OfflineDownload, NoSubresources) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -100,9 +101,10 @@ TEST(OfflineDownload, NoSubresources) { TEST(OfflineDownload, InlineSource) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -140,9 +142,10 @@ TEST(OfflineDownload, InlineSource) { TEST(OfflineDownload, GeoJSONSource) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -175,9 +178,10 @@ TEST(OfflineDownload, GeoJSONSource) { TEST(OfflineDownload, Activate) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -250,9 +254,10 @@ TEST(OfflineDownload, Activate) { TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { FakeFileSource fileSource; OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, fileSource); @@ -272,9 +277,10 @@ TEST(OfflineDownload, DoesNotFloodTheFileSourceWithRequests) { TEST(OfflineDownload, GetStatusNoResources) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); OfflineRegionStatus status = download.getStatus(); @@ -289,9 +295,10 @@ TEST(OfflineDownload, GetStatusNoResources) { TEST(OfflineDownload, GetStatusStyleComplete) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -311,9 +318,10 @@ TEST(OfflineDownload, GetStatusStyleComplete) { TEST(OfflineDownload, GetStatusStyleAndSourceComplete) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -337,9 +345,10 @@ TEST(OfflineDownload, GetStatusStyleAndSourceComplete) { TEST(OfflineDownload, RequestError) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -365,9 +374,10 @@ TEST(OfflineDownload, RequestError) { TEST(OfflineDownload, RequestErrorsAreRetried) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -398,9 +408,10 @@ TEST(OfflineDownload, RequestErrorsAreRetried) { TEST(OfflineDownload, TileCountLimitExceededNoTileResponse) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -440,9 +451,10 @@ TEST(OfflineDownload, TileCountLimitExceededNoTileResponse) { TEST(OfflineDownload, TileCountLimitExceededWithTileResponse) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -494,9 +506,10 @@ TEST(OfflineDownload, TileCountLimitExceededWithTileResponse) { TEST(OfflineDownload, WithPreviouslyExistingTile) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -528,9 +541,10 @@ TEST(OfflineDownload, WithPreviouslyExistingTile) { TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -556,7 +570,7 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { test.loop.run(); OfflineDownload redownload( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); @@ -595,9 +609,10 @@ TEST(OfflineDownload, ReactivatePreviouslyCompletedDownload) { TEST(OfflineDownload, Deactivate) { OfflineTest test; - OfflineRegion region = test.createRegion(); + auto region = test.createRegion(); + ASSERT_TRUE(region); OfflineDownload download( - region.getID(), + region->getID(), OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), test.db, test.fileSource); diff --git a/test/storage/sqlite.test.cpp b/test/storage/sqlite.test.cpp index 22958c8bed2..cdbb7f26d7a 100644 --- a/test/storage/sqlite.test.cpp +++ b/test/storage/sqlite.test.cpp @@ -38,12 +38,6 @@ TEST(SQLite, TEST_REQUIRES_WRITE(TryOpen)) { auto result = mapbox::sqlite::Database::tryOpen("test/fixtures/offline_database/foobar123.db", mapbox::sqlite::ReadOnly); ASSERT_TRUE(result.is()); ASSERT_EQ(result.get().code, mapbox::sqlite::ResultCode::CantOpen); - -#ifndef __QT__ - // Only non-Qt platforms are setting a logger on the SQLite object. - EXPECT_EQ(1u, log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::CantOpen), "cannot open file" }, true)); - EXPECT_EQ(1u, log.count({ EventSeverity::Info, Event::Database, static_cast(mapbox::sqlite::ResultCode::CantOpen), "No such file or directory" }, true)); -#endif EXPECT_EQ(0u, log.uncheckedCount()); } From 369b70ef338137e06f1b1bc3bbd0c2baa875207d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 6 Aug 2018 16:36:03 +0200 Subject: [PATCH 3/6] [test] add test for pending offline download when the disk is full --- test/storage/offline_download.test.cpp | 78 +++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index e87ad6c3709..f979d42601e 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -1,5 +1,7 @@ #include #include +#include +#include #include #include @@ -10,11 +12,29 @@ #include #include +#include #include #include using namespace mbgl; using namespace std::literals::string_literals; +using mapbox::sqlite::ResultCode; + +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +static constexpr const char* filename = "test/fixtures/offline_download/offline.db"; +static constexpr const char* filename_test_fs = "file:test/fixtures/offline_download/offline.db?vfs=test_fs"; + +static void deleteDatabaseFiles() { + // Delete leftover journaling files as well. + util::deleteFile(filename); + util::deleteFile(filename + "-wal"s); + util::deleteFile(filename + "-journal"s); +} + +static FixtureLog::Message warning(ResultCode code, const char* message) { + return { EventSeverity::Warning, Event::Database, static_cast(code), message }; +} +#endif class MockObserver : public OfflineRegionObserver { public: @@ -37,9 +57,12 @@ class MockObserver : public OfflineRegionObserver { class OfflineTest { public: + OfflineTest(const std::string& path = ":memory:") : db(path) { + } + util::RunLoop loop; StubFileSource fileSource; - OfflineDatabase db { ":memory:" }; + OfflineDatabase db; std::size_t size = 0; optional createRegion() { @@ -636,3 +659,56 @@ TEST(OfflineDownload, Deactivate) { test.loop.run(); } + +#ifndef __QT__ // Qt doesn't expose the ability to register virtual file system handlers. +TEST(OfflineDownload, DiskFull) { + FixtureLog log; + deleteDatabaseFiles(); + test::SQLite3TestFS fs; + + OfflineTest test{ filename_test_fs }; + EXPECT_EQ(0u, log.uncheckedCount()); + + auto region = test.createRegion(); + ASSERT_TRUE(region); + EXPECT_EQ(0u, log.uncheckedCount()); + + // Simulate a full disk. + fs.setWriteLimit(8192); + + OfflineDownload download( + region->getID(), + OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0), + test.db, test.fileSource); + + bool hasRequestedStyle = false; + + test.fileSource.styleResponse = [&] (const Resource& resource) { + EXPECT_EQ("http://127.0.0.1:3000/style.json", resource.url); + hasRequestedStyle = true; + return test.response("empty.style.json"); + }; + + auto observer = std::make_unique(); + + observer->statusChangedFn = [&] (OfflineRegionStatus status) { + EXPECT_EQ(OfflineRegionDownloadState::Active, status.downloadState); + EXPECT_EQ(0u, status.completedResourceCount); + EXPECT_EQ(0u, status.completedResourceSize); + EXPECT_EQ(hasRequestedStyle, status.requiredResourceCountIsPrecise); + EXPECT_FALSE(status.complete()); + + if (hasRequestedStyle) { + EXPECT_EQ(1u, log.count(warning(ResultCode::Full, "Can't write region resources: database or disk is full"))); + EXPECT_EQ(0u, log.uncheckedCount()); + test.loop.stop(); + } + }; + + download.setObserver(std::move(observer)); + download.setState(OfflineRegionDownloadState::Active); + + test.loop.run(); + EXPECT_EQ(0u, log.uncheckedCount()); +} +#endif // __QT__ From 4d5a91f857caf931faa01cdfbf8035382aa7f868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 13 Aug 2018 16:29:14 -0700 Subject: [PATCH 4/6] WIP: use expected for passing on errors --- CMakeLists.txt | 8 ++- bin/offline.cpp | 6 +- cmake/core-files.cmake | 1 + cmake/filesource.cmake | 3 + cmake/mbgl.cmake | 55 ++++++------------ include/mbgl/storage/default_file_source.hpp | 15 +++-- include/mbgl/storage/offline.hpp | 2 + platform/android/config.cmake | 1 + .../android/src/offline/offline_manager.cpp | 23 ++++---- .../android/src/offline/offline_region.cpp | 20 +++---- platform/darwin/src/MGLOfflinePack.mm | 2 +- platform/darwin/src/MGLOfflineStorage.mm | 24 ++++---- platform/default/default_file_source.cpp | 58 +++++++------------ .../default/mbgl/storage/offline_database.cpp | 35 ++++++----- .../default/mbgl/storage/offline_database.hpp | 16 ++--- platform/ios/ios.xcodeproj/project.pbxproj | 10 +++- scripts/config.xcconfig.in | 8 +-- test/storage/offline_database.test.cpp | 37 ++++++------ test/storage/offline_download.test.cpp | 2 +- 19 files changed, 156 insertions(+), 170 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b6b86d9a8dd..a02b4b91738 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 8b42ab7b726..603f0b848a5 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 region_) { - if (error) { - std::cerr << "Error creating region: " << util::toString(error) << std::endl; + fileSource.createOfflineRegion(definition, metadata, [&] (mbgl::expected 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 e875f5b1423..02335fef7a7 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 ccd2192f390..9b7a4a11388 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 bb029a47dbb..3abc974feba 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 $<$,$>: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 b9c8de50528..e048d82af20 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -55,8 +56,7 @@ class DefaultFileSource : public FileSource { * 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 listOfflineRegions(std::function)>); /* * Create an offline region in the database. @@ -71,16 +71,14 @@ class DefaultFileSource : public FileSource { */ void createOfflineRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function)>); + std::function)>); /* * Update an offline region metadata in the database. */ void updateOfflineMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function)>); + std::function)>); /* * Register an observer to be notified when the state of the region changes. */ @@ -97,8 +95,9 @@ class DefaultFileSource : public FileSource { * 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)>) const; + void getOfflineRegionStatus( + OfflineRegion&, + std::function)>) 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 2193f8d09e3..62353446fac 100644 --- a/include/mbgl/storage/offline.hpp +++ b/include/mbgl/storage/offline.hpp @@ -210,4 +210,6 @@ class OfflineRegion { const OfflineRegionMetadata metadata; }; +using OfflineRegions = std::vector; + } // namespace mbgl diff --git a/platform/android/config.cmake b/platform/android/config.cmake index 001a45e6134..bd65bc517ff 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 4960ae2845d..4f94a1c3a5d 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(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()), jFileSource = std::shared_ptr(jFileSource_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional> regions) mutable { + ](mbgl::expected regions) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineManager::ListOfflineRegionsCallback::onError(*env, jni::Object(*callback), error); - } else if (regions) { - OfflineManager::ListOfflineRegionsCallback::onList(*env, jni::Object(*jFileSource), jni::Object(*callback), std::move(regions)); + if (regions) { + OfflineManager::ListOfflineRegionsCallback::onList( + *env, jni::Object(*jFileSource), + jni::Object(*callback), std::move(*regions)); + } else { + OfflineManager::ListOfflineRegionsCallback::onError( + *env, jni::Object(*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(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()), jFileSource = std::shared_ptr(jFileSource_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional region) mutable { + ](mbgl::expected region) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineManager::CreateOfflineRegionCallback::onError(*env, jni::Object(*callback), error); - } else if (region) { + if (region) { OfflineManager::CreateOfflineRegionCallback::onCreate( *env, jni::Object(*jFileSource), - jni::Object(*callback), std::move(region) + jni::Object(*callback), std::move(*region) ); + } else { + OfflineManager::CreateOfflineRegionCallback::onError(*env, jni::Object(*callback), region.error()); } }); } diff --git a/platform/android/src/offline/offline_region.cpp b/platform/android/src/offline/offline_region.cpp index 27de76fb003..fe4dbecf14e 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(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional status) mutable { + ](mbgl::expected status) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineRegionStatusCallback::onError(*env, jni::Object(*callback), error); - } else if (status) { - OfflineRegionStatusCallback::onStatus(*env, jni::Object(*callback), std::move(status)); + if (status) { + OfflineRegionStatusCallback::onStatus(*env, jni::Object(*callback), std::move(*status)); + } else { + OfflineRegionStatusCallback::onError(*env, jni::Object(*callback), status.error()); } }); } @@ -144,14 +144,14 @@ void OfflineRegion::updateOfflineRegionMetadata(jni::JNIEnv& env_, jni::ArraygetID(), metadata, [ //Ensure the object is not gc'd in the meanwhile callback = std::shared_ptr(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()) - ](std::exception_ptr error, mbgl::optional data) mutable { + ](mbgl::expected data) mutable { // Reattach, the callback comes from a different thread android::UniqueEnv env = android::AttachEnv(); - if (error) { - OfflineRegionUpdateMetadataCallback::onError(*env, jni::Object(*callback), error); - } else if (data) { - OfflineRegionUpdateMetadataCallback::onUpdate(*env, jni::Object(*callback), std::move(data)); + if (data) { + OfflineRegionUpdateMetadataCallback::onUpdate(*env, jni::Object(*callback), std::move(*data)); + } else { + OfflineRegionUpdateMetadataCallback::onError(*env, jni::Object(*callback), data.error()); } }); } diff --git a/platform/darwin/src/MGLOfflinePack.mm b/platform/darwin/src/MGLOfflinePack.mm index 9d903ee8416..7bbc681c880 100644 --- a/platform/darwin/src/MGLOfflinePack.mm +++ b/platform/darwin/src/MGLOfflinePack.mm @@ -141,7 +141,7 @@ - (void)requestProgress { mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource]; __weak MGLOfflinePack *weakSelf = self; - mbglFileSource->getOfflineRegionStatus(*_mbglOfflineRegion, [&, weakSelf](__unused std::exception_ptr exception, mbgl::optional status) { + mbglFileSource->getOfflineRegionStatus(*_mbglOfflineRegion, [&, weakSelf](mbgl::expected 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 e6c10e942b6..05e1b063384 100644 --- a/platform/darwin/src/MGLOfflineStorage.mm +++ b/platform/darwin/src/MGLOfflineStorage.mm @@ -288,16 +288,16 @@ - (void)_addPackForRegion:(id )region withContext:(NSData *)co const mbgl::OfflineTilePyramidRegionDefinition regionDefinition = [(id )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 mbglOfflineRegion) { + self.mbglFileSource->createOfflineRegion(regionDefinition, metadata, [&, completion](mbgl::expected 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 @@ - (void)reloadPacks { } - (void)getPacksWithCompletionHandler:(void (^)(NSArray *packs, NSError * _Nullable error))completion { - self.mbglFileSource->listOfflineRegions([&, completion](std::exception_ptr exception, mbgl::optional> regions) { + self.mbglFileSource->listOfflineRegions([&, completion](mbgl::expected 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 051e1b125ca..93f10eea729 100644 --- a/platform/default/default_file_source.cpp +++ b/platform/default/default_file_source.cpp @@ -45,61 +45,44 @@ class DefaultFileSource::Impl { onlineFileSource.setResourceTransform(std::move(transform)); } - void listRegions(std::function>)> callback) { - try { - callback({}, offlineDatabase->listRegions()); - } catch (...) { - callback(std::current_exception(), {}); - } + void listRegions(std::function)> callback) { + callback(offlineDatabase->listRegions()); } void createRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function)> callback) { - try { - callback({}, offlineDatabase->createRegion(definition, metadata)); - } catch (...) { - callback(std::current_exception(), {}); - } + std::function)> callback) { + callback(offlineDatabase->createRegion(definition, metadata)); } void updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function)> callback) { - try { - callback({}, offlineDatabase->updateMetadata(regionID, metadata)); - } catch (...) { - callback(std::current_exception(), {}); - } + std::function)> callback) { + callback(offlineDatabase->updateMetadata(regionID, metadata)); } - void getRegionStatus(int64_t regionID, std::function)> callback) { + void getRegionStatus(int64_t regionID, std::function)> callback) { if (auto download = getDownload(regionID)) { - callback({}, download->getStatus()); + callback(download.value()->getStatus()); } else { - callback(std::current_exception(), {}); + callback(unexpected(download.error())); } } void deleteRegion(OfflineRegion&& region, std::function 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 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 @@ class DefaultFileSource::Impl { } private: - OfflineDownload* getDownload(int64_t regionID) { + expected 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(definition.error()); } - - auto download = std::make_unique(regionID, std::move(*definition), + auto download = std::make_unique(regionID, std::move(definition.value()), *offlineDatabase, onlineFileSource); return downloads.emplace(regionID, std::move(download)).first->second.get(); } @@ -266,19 +248,19 @@ std::unique_ptr DefaultFileSource::request(const Resource& resourc return std::move(req); } -void DefaultFileSource::listOfflineRegions(std::function>)> callback) { +void DefaultFileSource::listOfflineRegions(std::function)> callback) { impl->actor().invoke(&Impl::listRegions, callback); } void DefaultFileSource::createOfflineRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function)> callback) { + std::function)> callback) { impl->actor().invoke(&Impl::createRegion, definition, metadata, callback); } void DefaultFileSource::updateOfflineMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function)> callback) { + std::function)> 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)> callback) const { +void DefaultFileSource::getOfflineRegionStatus(OfflineRegion& region, std::function)> 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 cfa7f529778..7516185f278 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 OfflineDatabase::listRegions() try { +expected OfflineDatabase::listRegions() try { mapbox::sqlite::Query query{ getStatement("SELECT id, definition, description FROM regions") }; - std::vector result; + OfflineRegions result; while (query.run()) { const auto id = query.get(0); const auto definition = query.get(1); const auto description = query.get>(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 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::current_exception()); } -optional OfflineDatabase::createRegion(const OfflineRegionDefinition& definition, - const OfflineRegionMetadata& metadata) try { +expected +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 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::current_exception()); } -optional OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata) try { +expected +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 OfflineDatabase::updateMetadata(const int64_t re return metadata; } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "update region metadata"); - return nullopt; + return unexpected(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> OfflineDatabase::getRegionResource(int64_t regionID, const Resource& resource) try { @@ -848,7 +853,7 @@ bool OfflineDatabase::markUsed(int64_t regionID, const Resource& resource) { } } -optional OfflineDatabase::getRegionDefinition(int64_t regionID) try { +expected 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 OfflineDatabase::getRegionDefinition(int64_t r return decodeOfflineRegionDefinition(query.get(0)); } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "load region"); - return nullopt; + return unexpected(std::current_exception()); } -optional OfflineDatabase::getRegionCompletedStatus(int64_t regionID) try { +expected OfflineDatabase::getRegionCompletedStatus(int64_t regionID) try { OfflineRegionStatus result; std::tie(result.completedResourceCount, result.completedResourceSize) @@ -873,7 +878,7 @@ optional OfflineDatabase::getRegionCompletedStatus(int64_t return result; } catch (const mapbox::sqlite::Exception& ex) { handleError(ex, "get region status"); - return nullopt; + return unexpected(std::current_exception()); } std::pair OfflineDatabase::getCompletedResourceCountAndSize(int64_t regionID) { diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp index cdad11b79e4..fbe6c707e1f 100644 --- a/platform/default/mbgl/storage/offline_database.hpp +++ b/platform/default/mbgl/storage/offline_database.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -47,14 +48,15 @@ class OfflineDatabase : private util::noncopyable { // Return value is (inserted, stored size) std::pair put(const Resource&, const Response&); - std::vector listRegions(); + expected listRegions(); - optional createRegion(const OfflineRegionDefinition&, - const OfflineRegionMetadata&); + expected createRegion(const OfflineRegionDefinition&, + const OfflineRegionMetadata&); - optional updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); + expected + updateMetadata(const int64_t regionID, const OfflineRegionMetadata&); - void deleteRegion(OfflineRegion&&); + std::exception_ptr deleteRegion(OfflineRegion&&); // Return value is (response, stored size) optional> getRegionResource(int64_t regionID, const Resource&); @@ -62,8 +64,8 @@ class OfflineDatabase : private util::noncopyable { uint64_t putRegionResource(int64_t regionID, const Resource&, const Response&); void putRegionResources(int64_t regionID, const std::list>&, OfflineRegionStatus&); - optional getRegionDefinition(int64_t regionID); - optional getRegionCompletedStatus(int64_t regionID); + expected getRegionDefinition(int64_t regionID); + expected 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 b77c67fbb36..48329cf63a2 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 357732c9aed..69ca2424a14 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 = "$," ">" +mbgl_core_LINK_LIBRARIES = "$" // mbgl-filesource -mbgl_filesource_INCLUDE_DIRECTORIES = "@mbgl_filesource_INCLUDE_DIRECTORIES@" -mbgl_filesource_LINK_LIBRARIES = "@mbgl_filesource_LINK_LIBRARIES@" +mbgl_filesource_INCLUDE_DIRECTORIES = "$," ">" +mbgl_filesource_LINK_LIBRARIES = "$" diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index de40d8caf70..a1fd2ea619a 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 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 f979d42601e..93b4dd623ae 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -65,7 +65,7 @@ class OfflineTest { OfflineDatabase db; std::size_t size = 0; - optional createRegion() { + auto createRegion() { OfflineRegionDefinition definition { "", LatLngBounds::hull({1, 2}, {3, 4}), 5, 6, 1.0 }; OfflineRegionMetadata metadata; return db.createRegion(definition, metadata); From 67755d13506186ab334aa09007c56f69796b11b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 14 Aug 2018 13:04:00 -0700 Subject: [PATCH 5/6] [core] recreate offline database when it is deleted out from under our feet --- .../default/mbgl/storage/offline_database.cpp | 7 ++++-- platform/default/sqlite3.cpp | 4 +++ platform/default/sqlite3.hpp | 25 ++++++++++--------- test/storage/offline_database.test.cpp | 8 +++--- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index 7516185f278..79bc3c8f271 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -79,8 +79,11 @@ void OfflineDatabase::initialize() { void OfflineDatabase::handleError(const mapbox::sqlite::Exception& ex, const char* action) { if (ex.code == mapbox::sqlite::ResultCode::NotADB || - ex.code == mapbox::sqlite::ResultCode::Corrupt) { - // Corrupted; delete database so that we have a clean slate for the next operation. + ex.code == mapbox::sqlite::ResultCode::Corrupt || + (ex.code == mapbox::sqlite::ResultCode::ReadOnly && + ex.extendedCode == mapbox::sqlite::ExtendedResultCode::ReadOnlyDBMoved)) { + // The database was corruped, moved away, or deleted. We're going to start fresh with a + // clean slate for the next operation. Log::Error(Event::Database, static_cast(ex.code), "Can't %s: %s", action, ex.what()); try { removeExisting(); diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp index fed5a3a1856..faaa85efd8e 100644 --- a/platform/default/sqlite3.cpp +++ b/platform/default/sqlite3.cpp @@ -45,6 +45,10 @@ class DatabaseImpl { DatabaseImpl(sqlite3* db_) : db(db_) { + const int error = sqlite3_extended_result_codes(db, true); + if (error != SQLITE_OK) { + mbgl::Log::Warning(mbgl::Event::Database, error, "Failed to enable extended result codes: %s", sqlite3_errmsg(db)); + } } ~DatabaseImpl() diff --git a/platform/default/sqlite3.hpp b/platform/default/sqlite3.hpp index 612e92acc67..16f76a0d1a5 100644 --- a/platform/default/sqlite3.hpp +++ b/platform/default/sqlite3.hpp @@ -15,7 +15,7 @@ enum OpenFlag : int { ReadWriteCreate = 0b110, }; -enum class ResultCode : int { +enum class ResultCode : uint8_t { OK = 0, Error = 1, Internal = 2, @@ -40,24 +40,25 @@ enum class ResultCode : int { NoLFS = 22, Auth = 23, Range = 25, - NotADB = 26 + NotADB = 26, +}; + +enum class ExtendedResultCode : uint8_t { + Unknown = 0, + ReadOnlyDBMoved = 4, }; class Exception : public std::runtime_error { public: - Exception(int err, const char* msg) - : std::runtime_error(msg), code(static_cast(err)) { - } - Exception(ResultCode err, const char* msg) - : std::runtime_error(msg), code(err) { - } + Exception(ResultCode err, const char* msg) : Exception(static_cast(err), msg) {} + Exception(int err, const char* msg) : Exception(err, std::string{ msg }) {} Exception(int err, const std::string& msg) - : std::runtime_error(msg), code(static_cast(err)) { - } - Exception(ResultCode err, const std::string& msg) - : std::runtime_error(msg), code(err) { + : std::runtime_error(msg), + code(static_cast(err)), + extendedCode(static_cast(err >> 8)) { } const ResultCode code = ResultCode::OK; + const ExtendedResultCode extendedCode = ExtendedResultCode::Unknown; }; class DatabaseImpl; diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index a1fd2ea619a..346cd6da807 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -32,7 +32,7 @@ static FixtureLog::Message error(ResultCode code, const char* message) { return { EventSeverity::Error, Event::Database, static_cast(code), message }; } -static FixtureLog::Message warning(ResultCode code, const char* message) { +static __attribute__((unused)) FixtureLog::Message warning(ResultCode code, const char* message) { return { EventSeverity::Warning, Event::Database, static_cast(code), message }; } @@ -232,7 +232,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(Invalid)) { // Checking two possibilities for the error string because it apparently changes between SQLite versions. EXPECT_EQ(1u, log.count(error(ResultCode::NotADB, "Can't open database: file is encrypted or is not a database"), true) + log.count(error(ResultCode::NotADB, "Can't open database: file is not a database"), true)); - EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); // Now try inserting and reading back to make sure we have a valid database. for (const auto& res : { fixture::resource, fixture::tile }) { @@ -977,7 +977,7 @@ TEST(OfflineDatabase, CorruptDatabaseOnOpen) { // This database is corrupt in a way that will prevent opening the database. OfflineDatabase db(filename); EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't open database: database disk image is malformed"), true)); - EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); EXPECT_EQ(0u, log.uncheckedCount()); // Now try inserting and reading back to make sure we have a valid database. @@ -1007,7 +1007,7 @@ TEST(OfflineDatabase, CorruptDatabaseOnQuery) { // The first request fails because the database is corrupt and has to be recreated. EXPECT_EQ(nullopt, db.get(fixture::tile)); EXPECT_EQ(1u, log.count(error(ResultCode::Corrupt, "Can't read resource: database disk image is malformed"), true)); - EXPECT_EQ(1u, log.count(warning(static_cast(-1), "Removing existing incompatible offline database"))); + EXPECT_EQ(1u, log.count({ EventSeverity::Warning, Event::Database, -1, "Removing existing incompatible offline database" })); EXPECT_EQ(0u, log.uncheckedCount()); // Now try inserting and reading back to make sure we have a valid database. From 0f35e05832f748a96baa60566c19b0872c53456c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 14 Aug 2018 13:28:28 -0700 Subject: [PATCH 6/6] [build] make sure we're also updating the mapbox-ios-events submodule --- cmake/mbgl.cmake | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmake/mbgl.cmake b/cmake/mbgl.cmake index 3abc974feba..233ae6e8b81 100644 --- a/cmake/mbgl.cmake +++ b/cmake/mbgl.cmake @@ -64,9 +64,14 @@ if(WITH_NODEJS) endfunction() # Run submodule update + set(MBGL_SUBMODULES mapbox-gl-js) + if (MBGL_PLATFORM STREQUAL "ios") + list(APPEND MBGL_SUBMODULES platform/ios/vendor/mapbox-events-ios) + endif() + message(STATUS "Updating submodules...") execute_process( - COMMAND git submodule update --init mapbox-gl-js + COMMAND git submodule update --init ${MBGL_SUBMODULES} WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}") if(NOT EXISTS "${CMAKE_SOURCE_DIR}/mapbox-gl-js/node_modules") @@ -80,7 +85,7 @@ if(WITH_NODEJS) # Add target for running submodule update during builds add_custom_target( update-submodules ALL - COMMAND git submodule update --init mapbox-gl-js + COMMAND git submodule update --init ${MBGL_SUBMODULES} WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" COMMENT "Updating submodules..." )