From a3aedcbfcd95aed279dd9754434b6409462b4bc1 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Mon, 28 Feb 2022 14:48:10 +0100 Subject: [PATCH 01/14] Add writable flag to meta store classes --- terracotta/drivers/base_classes.py | 1 + .../drivers/sqlite_remote_meta_store.py | 2 ++ terracotta/drivers/terracotta_driver.py | 29 +++++++++++++++---- terracotta/exceptions.py | 4 +++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/terracotta/drivers/base_classes.py b/terracotta/drivers/base_classes.py index bae3f206..5ff36dac 100644 --- a/terracotta/drivers/base_classes.py +++ b/terracotta/drivers/base_classes.py @@ -38,6 +38,7 @@ class MetaStore(ABC): Defines a common interface for all metadata backends. """ _RESERVED_KEYS = ('limit', 'page') + WRITABLE: bool = True @property @abstractmethod diff --git a/terracotta/drivers/sqlite_remote_meta_store.py b/terracotta/drivers/sqlite_remote_meta_store.py index e40968ad..2084a04a 100644 --- a/terracotta/drivers/sqlite_remote_meta_store.py +++ b/terracotta/drivers/sqlite_remote_meta_store.py @@ -73,6 +73,8 @@ class RemoteSQLiteMetaStore(SQLiteMetaStore): """ + WRITEABLE: bool = False + def __init__(self, remote_path: Union[str, Path]) -> None: """Initialize the RemoteSQLiteDriver. diff --git a/terracotta/drivers/terracotta_driver.py b/terracotta/drivers/terracotta_driver.py index e0ac77d1..5c943474 100644 --- a/terracotta/drivers/terracotta_driver.py +++ b/terracotta/drivers/terracotta_driver.py @@ -5,7 +5,8 @@ import contextlib from collections import OrderedDict -from typing import (Any, Collection, Dict, List, Mapping, Optional, Sequence, Tuple, TypeVar, +import functools +from typing import (Any, Callable, Collection, Dict, List, Mapping, Optional, Sequence, Tuple, TypeVar, Union) import terracotta @@ -19,6 +20,20 @@ T = TypeVar('T') +def requires_writable( + fun: Callable[..., T] = None +) -> Callable[..., T]: + @functools.wraps(fun) + def inner(self: "TerracottaDriver", *args: Any, **kwargs: Any) -> T: + assert fun is not None + if self.meta_store.WRITABLE: + return fun(self, *args, **kwargs) + else: + raise exceptions.DatabaseNotWritable("Database not writable") + + return inner + + def squeeze(iterable: Collection[T]) -> T: assert len(iterable) == 1 return next(iter(iterable)) @@ -58,6 +73,7 @@ def key_names(self) -> Tuple[str, ...]: """ return self.meta_store.key_names + @requires_writable def create(self, keys: Sequence[str], *, key_descriptions: Mapping[str, str] = None) -> None: """Create a new, empty metadata store. @@ -168,15 +184,17 @@ def get_metadata(self, keys: ExtendedKeysType) -> Dict[str, Any]: raise exceptions.DatasetNotFoundError('No dataset found') path = squeeze(dataset.values()) - metadata = self.compute_metadata(path, max_shape=self.LAZY_LOADING_MAX_SHAPE) - self.insert(keys, path, metadata=metadata) + if self.meta_store.WRITABLE: + metadata = self.compute_metadata(path, max_shape=self.LAZY_LOADING_MAX_SHAPE) + self.insert(keys, path, metadata=metadata) - # ensure standardized/consistent output (types and floating point precision) - metadata = self.meta_store.get_metadata(keys) + # ensure standardized/consistent output (types and floating point precision) + metadata = self.meta_store.get_metadata(keys) assert metadata is not None return metadata + @requires_writable @requires_connection def insert( self, keys: ExtendedKeysType, @@ -211,6 +229,7 @@ def insert( metadata=metadata ) + @requires_writable @requires_connection def delete(self, keys: ExtendedKeysType) -> None: """Remove a dataset from the meta store. diff --git a/terracotta/exceptions.py b/terracotta/exceptions.py index b6d904b2..52b2d951 100644 --- a/terracotta/exceptions.py +++ b/terracotta/exceptions.py @@ -24,5 +24,9 @@ class InvalidDatabaseError(Exception): pass +class DatabaseNotWritable(Exception): + pass + + class PerformanceWarning(UserWarning): pass From 22353ca9c3f99dbe4cdb70b7497d93162827713a Mon Sep 17 00:00:00 2001 From: Nicklas Boserup Date: Mon, 28 Feb 2022 14:56:00 +0100 Subject: [PATCH 02/14] Always compute metadata --- terracotta/drivers/terracotta_driver.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/terracotta/drivers/terracotta_driver.py b/terracotta/drivers/terracotta_driver.py index 5c943474..ff486821 100644 --- a/terracotta/drivers/terracotta_driver.py +++ b/terracotta/drivers/terracotta_driver.py @@ -184,13 +184,14 @@ def get_metadata(self, keys: ExtendedKeysType) -> Dict[str, Any]: raise exceptions.DatasetNotFoundError('No dataset found') path = squeeze(dataset.values()) + metadata = self.compute_metadata(path, max_shape=self.LAZY_LOADING_MAX_SHAPE) + if self.meta_store.WRITABLE: - metadata = self.compute_metadata(path, max_shape=self.LAZY_LOADING_MAX_SHAPE) self.insert(keys, path, metadata=metadata) # ensure standardized/consistent output (types and floating point precision) metadata = self.meta_store.get_metadata(keys) - assert metadata is not None + assert metadata is not None return metadata From 8cd1c6a1865fd1ea15ac60ea1057a638ba218eb3 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Mon, 28 Feb 2022 15:54:18 +0100 Subject: [PATCH 03/14] :bug: --- terracotta/drivers/terracotta_driver.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/terracotta/drivers/terracotta_driver.py b/terracotta/drivers/terracotta_driver.py index 5c943474..79eb75d8 100644 --- a/terracotta/drivers/terracotta_driver.py +++ b/terracotta/drivers/terracotta_driver.py @@ -6,8 +6,8 @@ import contextlib from collections import OrderedDict import functools -from typing import (Any, Callable, Collection, Dict, List, Mapping, Optional, Sequence, Tuple, TypeVar, - Union) +from typing import (Any, Callable, Collection, Dict, List, Mapping, + Optional, Sequence, Tuple, TypeVar, Union) import terracotta from terracotta import exceptions @@ -21,7 +21,7 @@ def requires_writable( - fun: Callable[..., T] = None + fun: Callable[..., T] ) -> Callable[..., T]: @functools.wraps(fun) def inner(self: "TerracottaDriver", *args: Any, **kwargs: Any) -> T: From 70d314bf65fe60ad11a69c1db5069625361a1c67 Mon Sep 17 00:00:00 2001 From: Nicklas Boserup Date: Mon, 28 Feb 2022 17:07:27 +0100 Subject: [PATCH 04/14] Don't assert always true statement --- terracotta/drivers/terracotta_driver.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/terracotta/drivers/terracotta_driver.py b/terracotta/drivers/terracotta_driver.py index dfe8dd40..582b7704 100644 --- a/terracotta/drivers/terracotta_driver.py +++ b/terracotta/drivers/terracotta_driver.py @@ -20,12 +20,9 @@ T = TypeVar('T') -def requires_writable( - fun: Callable[..., T] -) -> Callable[..., T]: +def requires_writable(fun: Callable[..., T]) -> Callable[..., T]: @functools.wraps(fun) def inner(self: "TerracottaDriver", *args: Any, **kwargs: Any) -> T: - assert fun is not None if self.meta_store.WRITABLE: return fun(self, *args, **kwargs) else: From 372ee17e6ca645c6075a86673cf1d57069a1aef9 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Mon, 7 Mar 2022 16:08:51 +0100 Subject: [PATCH 05/14] Handle writability in meta stores --- terracotta/drivers/base_classes.py | 15 +++++++++++- terracotta/drivers/relational_meta_store.py | 5 +++- .../drivers/sqlite_remote_meta_store.py | 14 +++++++---- terracotta/drivers/terracotta_driver.py | 23 +++++-------------- tests/drivers/test_sqlite_remote.py | 8 ++++--- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/terracotta/drivers/base_classes.py b/terracotta/drivers/base_classes.py index 5ff36dac..2717e988 100644 --- a/terracotta/drivers/base_classes.py +++ b/terracotta/drivers/base_classes.py @@ -10,12 +10,25 @@ from typing import (Any, Callable, Dict, List, Mapping, Optional, Sequence, Tuple, TypeVar, Union) +from terracotta import exceptions + KeysType = Mapping[str, str] MultiValueKeysType = Mapping[str, Union[str, List[str]]] Number = TypeVar('Number', int, float) T = TypeVar('T') +def requires_writable(fun: Callable[..., T]) -> Callable[..., T]: + @functools.wraps(fun) + def inner(self: MetaStore, *args: Any, **kwargs: Any) -> T: + if self._WRITABLE: + return fun(self, *args, **kwargs) + else: + raise exceptions.DatabaseNotWritable("Database not writable") + + return inner + + def requires_connection( fun: Callable[..., T] = None, *, verify: bool = True @@ -38,7 +51,7 @@ class MetaStore(ABC): Defines a common interface for all metadata backends. """ _RESERVED_KEYS = ('limit', 'page') - WRITABLE: bool = True + _WRITABLE: bool = True @property @abstractmethod diff --git a/terracotta/drivers/relational_meta_store.py b/terracotta/drivers/relational_meta_store.py index 2ccdf45f..3bfb7bed 100644 --- a/terracotta/drivers/relational_meta_store.py +++ b/terracotta/drivers/relational_meta_store.py @@ -21,7 +21,8 @@ from terracotta import exceptions from terracotta.drivers.base_classes import (KeysType, MetaStore, MultiValueKeysType, - requires_connection) + requires_connection, + requires_writable) from terracotta.profile import trace _ERROR_ON_CONNECT = ( @@ -349,6 +350,7 @@ def get_metadata(self, keys: KeysType) -> Optional[Dict[str, Any]]: return self._decode_data(encoded_data) @trace('insert') + @requires_writable @requires_connection @convert_exceptions('Could not write to database') def insert( @@ -383,6 +385,7 @@ def insert( ) @trace('delete') + @requires_writable @requires_connection @convert_exceptions('Could not write to database') def delete(self, keys: KeysType) -> None: diff --git a/terracotta/drivers/sqlite_remote_meta_store.py b/terracotta/drivers/sqlite_remote_meta_store.py index 2084a04a..eac35cc9 100644 --- a/terracotta/drivers/sqlite_remote_meta_store.py +++ b/terracotta/drivers/sqlite_remote_meta_store.py @@ -15,6 +15,7 @@ from terracotta import exceptions, get_settings from terracotta.drivers.sqlite_meta_store import SQLiteMetaStore +from terracotta.drivers.base_classes import requires_writable from terracotta.profile import trace logger = logging.getLogger(__name__) @@ -69,11 +70,11 @@ class RemoteSQLiteMetaStore(SQLiteMetaStore): Warning: This driver is read-only. Any attempts to use the create, insert, or delete methods - will throw a NotImplementedError. + will throw a DatabaseNotWritable. """ - WRITEABLE: bool = False + _WRITABLE: bool = False def __init__(self, remote_path: Union[str, Path]) -> None: """Initialize the RemoteSQLiteDriver. @@ -134,14 +135,17 @@ def _connection_callback(self) -> None: self._update_db(self._remote_path, self._local_path) super()._connection_callback() + @requires_writable def create(self, *args: Any, **kwargs: Any) -> None: - raise NotImplementedError('Remote SQLite databases are read-only') + pass + @requires_writable def insert(self, *args: Any, **kwargs: Any) -> None: - raise NotImplementedError('Remote SQLite databases are read-only') + pass + @requires_writable def delete(self, *args: Any, **kwargs: Any) -> None: - raise NotImplementedError('Remote SQLite databases are read-only') + pass def __del__(self) -> None: """Clean up temporary database upon exit""" diff --git a/terracotta/drivers/terracotta_driver.py b/terracotta/drivers/terracotta_driver.py index 582b7704..b4b5fbae 100644 --- a/terracotta/drivers/terracotta_driver.py +++ b/terracotta/drivers/terracotta_driver.py @@ -5,8 +5,7 @@ import contextlib from collections import OrderedDict -import functools -from typing import (Any, Callable, Collection, Dict, List, Mapping, +from typing import (Any, Collection, Dict, List, Mapping, Optional, Sequence, Tuple, TypeVar, Union) import terracotta @@ -20,17 +19,6 @@ T = TypeVar('T') -def requires_writable(fun: Callable[..., T]) -> Callable[..., T]: - @functools.wraps(fun) - def inner(self: "TerracottaDriver", *args: Any, **kwargs: Any) -> T: - if self.meta_store.WRITABLE: - return fun(self, *args, **kwargs) - else: - raise exceptions.DatabaseNotWritable("Database not writable") - - return inner - - def squeeze(iterable: Collection[T]) -> T: assert len(iterable) == 1 return next(iter(iterable)) @@ -70,7 +58,6 @@ def key_names(self) -> Tuple[str, ...]: """ return self.meta_store.key_names - @requires_writable def create(self, keys: Sequence[str], *, key_descriptions: Mapping[str, str] = None) -> None: """Create a new, empty metadata store. @@ -183,16 +170,19 @@ def get_metadata(self, keys: ExtendedKeysType) -> Dict[str, Any]: path = squeeze(dataset.values()) metadata = self.compute_metadata(path, max_shape=self.LAZY_LOADING_MAX_SHAPE) - if self.meta_store.WRITABLE: + try: self.insert(keys, path, metadata=metadata) # ensure standardized/consistent output (types and floating point precision) metadata = self.meta_store.get_metadata(keys) assert metadata is not None + except exceptions.DatabaseNotWritable as exc: + raise exceptions.DatabaseNotWritable( + "Lazy loading requires a writable database" + ) from exc return metadata - @requires_writable @requires_connection def insert( self, keys: ExtendedKeysType, @@ -227,7 +217,6 @@ def insert( metadata=metadata ) - @requires_writable @requires_connection def delete(self, keys: ExtendedKeysType) -> None: """Remove a dataset from the meta store. diff --git a/tests/drivers/test_sqlite_remote.py b/tests/drivers/test_sqlite_remote.py index d4852617..d928a644 100644 --- a/tests/drivers/test_sqlite_remote.py +++ b/tests/drivers/test_sqlite_remote.py @@ -11,6 +11,8 @@ import pytest +from terracotta import exceptions + boto3 = pytest.importorskip('boto3') moto = pytest.importorskip('moto') @@ -142,13 +144,13 @@ def test_immutability(s3_db_factory, raster_file): driver = get_driver(dbpath) - with pytest.raises(NotImplementedError): + with pytest.raises(exceptions.DatabaseNotWritable): driver.create(keys) - with pytest.raises(NotImplementedError): + with pytest.raises(exceptions.DatabaseNotWritable): driver.insert(('some', 'value'), str(raster_file)) - with pytest.raises(NotImplementedError): + with pytest.raises(exceptions.DatabaseNotWritable): driver.delete(('some', 'value')) From a423d6e9c031078c259b8e05cdbd124f618b67d1 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Tue, 8 Mar 2022 10:51:22 +0100 Subject: [PATCH 06/14] Error naming and specific try-catch --- terracotta/drivers/base_classes.py | 2 +- terracotta/drivers/sqlite_remote_meta_store.py | 3 ++- terracotta/drivers/terracotta_driver.py | 12 ++++++------ terracotta/exceptions.py | 2 +- tests/drivers/test_sqlite_remote.py | 6 +++--- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/terracotta/drivers/base_classes.py b/terracotta/drivers/base_classes.py index 2717e988..a4c99f29 100644 --- a/terracotta/drivers/base_classes.py +++ b/terracotta/drivers/base_classes.py @@ -24,7 +24,7 @@ def inner(self: MetaStore, *args: Any, **kwargs: Any) -> T: if self._WRITABLE: return fun(self, *args, **kwargs) else: - raise exceptions.DatabaseNotWritable("Database not writable") + raise exceptions.DatabaseNotWritableError("Database not writable") return inner diff --git a/terracotta/drivers/sqlite_remote_meta_store.py b/terracotta/drivers/sqlite_remote_meta_store.py index eac35cc9..3f8f2f4f 100644 --- a/terracotta/drivers/sqlite_remote_meta_store.py +++ b/terracotta/drivers/sqlite_remote_meta_store.py @@ -70,7 +70,7 @@ class RemoteSQLiteMetaStore(SQLiteMetaStore): Warning: This driver is read-only. Any attempts to use the create, insert, or delete methods - will throw a DatabaseNotWritable. + will throw a DatabaseNotWritableError. """ @@ -135,6 +135,7 @@ def _connection_callback(self) -> None: self._update_db(self._remote_path, self._local_path) super()._connection_callback() + # Always raises DatabaseNotWritableError @requires_writable def create(self, *args: Any, **kwargs: Any) -> None: pass diff --git a/terracotta/drivers/terracotta_driver.py b/terracotta/drivers/terracotta_driver.py index b4b5fbae..2fac7588 100644 --- a/terracotta/drivers/terracotta_driver.py +++ b/terracotta/drivers/terracotta_driver.py @@ -172,15 +172,15 @@ def get_metadata(self, keys: ExtendedKeysType) -> Dict[str, Any]: try: self.insert(keys, path, metadata=metadata) - - # ensure standardized/consistent output (types and floating point precision) - metadata = self.meta_store.get_metadata(keys) - assert metadata is not None - except exceptions.DatabaseNotWritable as exc: - raise exceptions.DatabaseNotWritable( + except exceptions.DatabaseNotWritableError as exc: + raise exceptions.DatabaseNotWritableError( "Lazy loading requires a writable database" ) from exc + # ensure standardized/consistent output (types and floating point precision) + metadata = self.meta_store.get_metadata(keys) + assert metadata is not None + return metadata @requires_connection diff --git a/terracotta/exceptions.py b/terracotta/exceptions.py index 52b2d951..6ba68e74 100644 --- a/terracotta/exceptions.py +++ b/terracotta/exceptions.py @@ -24,7 +24,7 @@ class InvalidDatabaseError(Exception): pass -class DatabaseNotWritable(Exception): +class DatabaseNotWritableError(Exception): pass diff --git a/tests/drivers/test_sqlite_remote.py b/tests/drivers/test_sqlite_remote.py index d928a644..395ab541 100644 --- a/tests/drivers/test_sqlite_remote.py +++ b/tests/drivers/test_sqlite_remote.py @@ -144,13 +144,13 @@ def test_immutability(s3_db_factory, raster_file): driver = get_driver(dbpath) - with pytest.raises(exceptions.DatabaseNotWritable): + with pytest.raises(exceptions.DatabaseNotWritableError): driver.create(keys) - with pytest.raises(exceptions.DatabaseNotWritable): + with pytest.raises(exceptions.DatabaseNotWritableError): driver.insert(('some', 'value'), str(raster_file)) - with pytest.raises(exceptions.DatabaseNotWritable): + with pytest.raises(exceptions.DatabaseNotWritableError): driver.delete(('some', 'value')) From b5ee523be89dba5a622633751f4181ad7f827246 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Wed, 9 Mar 2022 11:23:17 +0100 Subject: [PATCH 07/14] Add unwritable TC driver test --- tests/drivers/test_raster_drivers.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/drivers/test_raster_drivers.py b/tests/drivers/test_raster_drivers.py index 7954cf07..791d7a7e 100644 --- a/tests/drivers/test_raster_drivers.py +++ b/tests/drivers/test_raster_drivers.py @@ -5,6 +5,8 @@ import numpy as np +from terracotta import exceptions + DRIVERS = ['sqlite', 'mysql'] METADATA_KEYS = ('bounds', 'range', 'mean', 'stdev', 'percentiles', 'metadata') @@ -144,6 +146,22 @@ def test_lazy_loading(driver_path, provider, raster_file): assert all(np.all(data1[k] == data2[k]) for k in data1.keys()) +@pytest.mark.parametrize('provider', DRIVERS) +def test_non_writable_lazy_loading(driver_path, provider, raster_file): + from terracotta import drivers + db = drivers.get_driver(driver_path, provider=provider) + keys = ('some', 'keynames') + + db.create(keys) + db.insert(['some', 'value'], str(raster_file), skip_metadata=True) + + # Manually set the meta store to un-writable + db.meta_store._WRITABLE = False + + with pytest.raises(exceptions.DatabaseNotWritableError): + db.get_metadata(['some', 'value']) + + @pytest.mark.parametrize('provider', DRIVERS) def test_precomputed_metadata(driver_path, provider, raster_file): from terracotta import drivers From 4c884755fee0ae9b7c6e77e1018e0b9adb9b4743 Mon Sep 17 00:00:00 2001 From: Nicklas Boserup Date: Tue, 29 Mar 2022 10:49:01 +0200 Subject: [PATCH 08/14] Also require writable for database creation --- terracotta/drivers/relational_meta_store.py | 1 + 1 file changed, 1 insertion(+) diff --git a/terracotta/drivers/relational_meta_store.py b/terracotta/drivers/relational_meta_store.py index 3bfb7bed..eb247bf3 100644 --- a/terracotta/drivers/relational_meta_store.py +++ b/terracotta/drivers/relational_meta_store.py @@ -182,6 +182,7 @@ def db_version(self) -> str: version = self.connection.execute(stmt).scalar() return version + @requires_writable @convert_exceptions('Could not create database') def create(self, keys: Sequence[str], key_descriptions: Mapping[str, str] = None) -> None: """Create and initialize database with empty tables. From 7c4fb6532a60e911c6142d4cb0da73686fd3b510 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Wed, 30 Mar 2022 14:28:37 +0200 Subject: [PATCH 09/14] Add non-writable error handler --- .../drivers/sqlite_remote_meta_store.py | 16 +----- terracotta/server/flask_api.py | 8 +++ tests/conftest.py | 45 ++++++++++++++++ tests/drivers/test_sqlite_remote.py | 54 ++----------------- tests/server/test_flask_api.py | 16 ++++++ 5 files changed, 75 insertions(+), 64 deletions(-) diff --git a/terracotta/drivers/sqlite_remote_meta_store.py b/terracotta/drivers/sqlite_remote_meta_store.py index 3f8f2f4f..d2c999ce 100644 --- a/terracotta/drivers/sqlite_remote_meta_store.py +++ b/terracotta/drivers/sqlite_remote_meta_store.py @@ -11,11 +11,10 @@ import time import urllib.parse as urlparse from pathlib import Path -from typing import Any, Iterator, Union +from typing import Iterator, Union from terracotta import exceptions, get_settings from terracotta.drivers.sqlite_meta_store import SQLiteMetaStore -from terracotta.drivers.base_classes import requires_writable from terracotta.profile import trace logger = logging.getLogger(__name__) @@ -135,19 +134,6 @@ def _connection_callback(self) -> None: self._update_db(self._remote_path, self._local_path) super()._connection_callback() - # Always raises DatabaseNotWritableError - @requires_writable - def create(self, *args: Any, **kwargs: Any) -> None: - pass - - @requires_writable - def insert(self, *args: Any, **kwargs: Any) -> None: - pass - - @requires_writable - def delete(self, *args: Any, **kwargs: Any) -> None: - pass - def __del__(self) -> None: """Clean up temporary database upon exit""" self.__rm(self._local_path) diff --git a/terracotta/server/flask_api.py b/terracotta/server/flask_api.py index c9976c84..4801925e 100644 --- a/terracotta/server/flask_api.py +++ b/terracotta/server/flask_api.py @@ -63,6 +63,14 @@ def handle_dataset_not_found_error(exc: Exception) -> Any: register_error_handler(exceptions.DatasetNotFoundError, handle_dataset_not_found_error) + def handle_database_not_writable_error(exc: Exception) -> Any: + # database not writable -> 403 + if current_app.debug: + raise exc + return _abort(403, str(exc)) + + register_error_handler(exceptions.DatabaseNotWritableError, handle_database_not_writable_error) + def handle_marshmallow_validation_error(exc: Exception) -> Any: # wrong query arguments -> 400 if current_app.debug: diff --git a/tests/conftest.py b/tests/conftest.py index 44a85ffe..f57ccac7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,9 @@ import shapely.geometry # noqa: F401 import os +from pathlib import Path +import tempfile +import uuid import multiprocessing import time from functools import partial @@ -12,6 +15,8 @@ import numpy as np import rasterio +boto3 = pytest.importorskip('boto3') + def pytest_configure(config): os.environ['TC_TESTING'] = '1' @@ -344,6 +349,46 @@ def use_testdb(testdb, monkeypatch): terracotta.update_settings(DRIVER_PATH=str(testdb)) +@pytest.fixture() +def s3_db_factory(tmpdir): + bucketname = str(uuid.uuid4()) + + def _s3_db_factory(keys, datasets=None, skip_metadata=False): + from terracotta import get_driver + + with tempfile.TemporaryDirectory() as tmpdir: + dbfile = Path(tmpdir) / 'tc.sqlite' + driver = get_driver(dbfile) + driver.create(keys) + + if datasets: + for keys, path in datasets.items(): + driver.insert(keys, path, skip_metadata=skip_metadata) + + with open(dbfile, 'rb') as f: + db_bytes = f.read() + + conn = boto3.resource('s3') + conn.create_bucket(Bucket=bucketname) + + s3 = boto3.client('s3') + s3.put_object(Bucket=bucketname, Key='tc.sqlite', Body=db_bytes) + + return f's3://{bucketname}/tc.sqlite' + + return _s3_db_factory + + +@pytest.fixture() +def mock_aws_env(monkeypatch): + with monkeypatch.context() as m: + m.setenv('AWS_DEFAULT_REGION', 'us-east-1') + m.setenv('AWS_ACCESS_KEY_ID', 'FakeKey') + m.setenv('AWS_SECRET_ACCESS_KEY', 'FakeSecretKey') + m.setenv('AWS_SESSION_TOKEN', 'FakeSessionToken') + yield + + def run_test_server(driver_path, port): from terracotta import update_settings update_settings(DRIVER_PATH=driver_path) diff --git a/tests/drivers/test_sqlite_remote.py b/tests/drivers/test_sqlite_remote.py index 395ab541..3342b44c 100644 --- a/tests/drivers/test_sqlite_remote.py +++ b/tests/drivers/test_sqlite_remote.py @@ -4,29 +4,15 @@ """ import os -import tempfile import time -import uuid -from pathlib import Path import pytest from terracotta import exceptions -boto3 = pytest.importorskip('boto3') moto = pytest.importorskip('moto') -@pytest.fixture(autouse=True) -def mock_aws_env(monkeypatch): - with monkeypatch.context() as m: - m.setenv('AWS_DEFAULT_REGION', 'us-east-1') - m.setenv('AWS_ACCESS_KEY_ID', 'FakeKey') - m.setenv('AWS_SECRET_ACCESS_KEY', 'FakeSecretKey') - m.setenv('AWS_SESSION_TOKEN', 'FakeSessionToken') - yield - - class Timer: def __init__(self, auto=False): self.auto = auto @@ -41,38 +27,8 @@ def tick(self): self.time += 1 -@pytest.fixture() -def s3_db_factory(tmpdir): - bucketname = str(uuid.uuid4()) - - def _s3_db_factory(keys, datasets=None): - from terracotta import get_driver - - with tempfile.TemporaryDirectory() as tmpdir: - dbfile = Path(tmpdir) / 'tc.sqlite' - driver = get_driver(dbfile) - driver.create(keys) - - if datasets: - for keys, path in datasets.items(): - driver.insert(keys, path) - - with open(dbfile, 'rb') as f: - db_bytes = f.read() - - conn = boto3.resource('s3') - conn.create_bucket(Bucket=bucketname) - - s3 = boto3.client('s3') - s3.put_object(Bucket=bucketname, Key='tc.sqlite', Body=db_bytes) - - return f's3://{bucketname}/tc.sqlite' - - return _s3_db_factory - - @moto.mock_s3 -def test_remote_database(s3_db_factory): +def test_remote_database(s3_db_factory, mock_aws_env): keys = ('some', 'keys') dbpath = s3_db_factory(keys) @@ -91,7 +47,7 @@ def test_invalid_url(): @moto.mock_s3 -def test_nonexisting_url(): +def test_nonexisting_url(mock_aws_env): from terracotta import exceptions, get_driver driver = get_driver('s3://foo/db.sqlite') with pytest.raises(exceptions.InvalidDatabaseError): @@ -100,7 +56,7 @@ def test_nonexisting_url(): @moto.mock_s3 -def test_remote_database_cache(s3_db_factory, raster_file, monkeypatch): +def test_remote_database_cache(s3_db_factory, raster_file, mock_aws_env): keys = ('some', 'keys') dbpath = s3_db_factory(keys) @@ -136,7 +92,7 @@ def test_remote_database_cache(s3_db_factory, raster_file, monkeypatch): @moto.mock_s3 -def test_immutability(s3_db_factory, raster_file): +def test_immutability(s3_db_factory, raster_file, mock_aws_env): keys = ('some', 'keys') dbpath = s3_db_factory(keys, datasets={('some', 'value'): str(raster_file)}) @@ -155,7 +111,7 @@ def test_immutability(s3_db_factory, raster_file): @moto.mock_s3 -def test_destructor(s3_db_factory, raster_file, capsys): +def test_destructor(s3_db_factory, raster_file, capsys, mock_aws_env): keys = ('some', 'keys') dbpath = s3_db_factory(keys, datasets={('some', 'value'): str(raster_file)}) diff --git a/tests/server/test_flask_api.py b/tests/server/test_flask_api.py index 0ee0f9b6..59502bbe 100644 --- a/tests/server/test_flask_api.py +++ b/tests/server/test_flask_api.py @@ -8,6 +8,8 @@ import pytest +moto = pytest.importorskip('moto') + @pytest.fixture(scope='module') def flask_app(): @@ -39,6 +41,20 @@ def test_get_metadata(client, use_testdb): assert ['extra_data'] == json.loads(rv.data)['metadata'] +@moto.mock_s3 +def test_get_metadata_lazily_nonwritable_db(client, s3_db_factory, mock_aws_env, raster_file): + import terracotta + + keys = ('some', 'keys') + dbpath = s3_db_factory( + keys, datasets={("some", "value"): str(raster_file)}, skip_metadata=True + ) + terracotta.update_settings(DRIVER_PATH=str(dbpath), DRIVER_PROVIDER="sqlite-remote") + + rv = client.get('/metadata/some/value') + assert rv.status_code == 403 + + def test_get_metadata_nonexisting(client, use_testdb): rv = client.get('/metadata/val11/x/NONEXISTING/') assert rv.status_code == 404 From cad011ed9b1cba76a3af3f40384d08cea73d2231 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Wed, 30 Mar 2022 15:01:53 +0200 Subject: [PATCH 10/14] Add flask debug exception tests --- tests/server/test_flask_api.py | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/server/test_flask_api.py b/tests/server/test_flask_api.py index 59502bbe..5fbcf9a1 100644 --- a/tests/server/test_flask_api.py +++ b/tests/server/test_flask_api.py @@ -23,6 +23,18 @@ def client(flask_app): yield client +@pytest.fixture(scope='module') +def debug_flask_app(): + from terracotta.server import create_app + return create_app(debug=True) + + +@pytest.fixture(scope='module') +def debug_client(debug_flask_app): + with debug_flask_app.test_client() as client: + yield client + + def test_get_keys(client, use_testdb): rv = client.get('/keys') @@ -55,6 +67,43 @@ def test_get_metadata_lazily_nonwritable_db(client, s3_db_factory, mock_aws_env, assert rv.status_code == 403 +@moto.mock_s3 +def test_debug_errors(debug_client, s3_db_factory, mock_aws_env, raster_file, raster_file_xyz): + import terracotta + from terracotta import exceptions + import marshmallow + + keys = ('some', 'keys') + dbpath = s3_db_factory( + keys, datasets={("some", "value"): str(raster_file)}, skip_metadata=True + ) + terracotta.update_settings(DRIVER_PATH=str(dbpath), DRIVER_PROVIDER="sqlite-remote") + + with pytest.raises(exceptions.DatabaseNotWritableError): + debug_client.get('/metadata/some/value') + + with pytest.raises(exceptions.DatasetNotFoundError): + debug_client.get('/metadata/NONEXISTING/KEYS') + + with pytest.raises(exceptions.InvalidKeyError): + debug_client.get('/metadata/ONLYONEKEY') + + x, y, z = raster_file_xyz + + with pytest.raises(exceptions.InvalidArgumentsError): + debug_client.get( + f'/compute/some/value/{z}/{x}/{y}.png' + '?expression=v1*v2&v1=val22&v2=val23' + '&stretch_range=[10000,0]' + ) + + with pytest.raises(marshmallow.ValidationError): + debug_client.get( + f'/compute/some/value/{z}/{x}/{y}.png' + '?stretch_range=[10000,0]' + ) + + def test_get_metadata_nonexisting(client, use_testdb): rv = client.get('/metadata/val11/x/NONEXISTING/') assert rv.status_code == 404 From 1c647d0d1d783840e9f471006030381b37d4b63f Mon Sep 17 00:00:00 2001 From: kiksekage Date: Wed, 30 Mar 2022 15:02:37 +0200 Subject: [PATCH 11/14] :lipstick: --- tests/server/test_flask_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server/test_flask_api.py b/tests/server/test_flask_api.py index 5fbcf9a1..0ee2a346 100644 --- a/tests/server/test_flask_api.py +++ b/tests/server/test_flask_api.py @@ -87,7 +87,7 @@ def test_debug_errors(debug_client, s3_db_factory, mock_aws_env, raster_file, ra with pytest.raises(exceptions.InvalidKeyError): debug_client.get('/metadata/ONLYONEKEY') - + x, y, z = raster_file_xyz with pytest.raises(exceptions.InvalidArgumentsError): From e08135540412aa2f09abd9424c9bdb036e0c0ca6 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Thu, 31 Mar 2022 11:44:55 +0200 Subject: [PATCH 12/14] Monkeypatch meta_store._WRITABLE --- tests/conftest.py | 49 ++++++-------------------- tests/drivers/test_sqlite_remote.py | 54 ++++++++++++++++++++++++++--- tests/server/test_flask_api.py | 33 ++++-------------- 3 files changed, 66 insertions(+), 70 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index f57ccac7..1d9d5df2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,9 +3,6 @@ import shapely.geometry # noqa: F401 import os -from pathlib import Path -import tempfile -import uuid import multiprocessing import time from functools import partial @@ -15,8 +12,6 @@ import numpy as np import rasterio -boto3 = pytest.importorskip('boto3') - def pytest_configure(config): os.environ['TC_TESTING'] = '1' @@ -350,43 +345,19 @@ def use_testdb(testdb, monkeypatch): @pytest.fixture() -def s3_db_factory(tmpdir): - bucketname = str(uuid.uuid4()) - - def _s3_db_factory(keys, datasets=None, skip_metadata=False): - from terracotta import get_driver - - with tempfile.TemporaryDirectory() as tmpdir: - dbfile = Path(tmpdir) / 'tc.sqlite' - driver = get_driver(dbfile) - driver.create(keys) - - if datasets: - for keys, path in datasets.items(): - driver.insert(keys, path, skip_metadata=skip_metadata) - - with open(dbfile, 'rb') as f: - db_bytes = f.read() - - conn = boto3.resource('s3') - conn.create_bucket(Bucket=bucketname) - - s3 = boto3.client('s3') - s3.put_object(Bucket=bucketname, Key='tc.sqlite', Body=db_bytes) - - return f's3://{bucketname}/tc.sqlite' +def use_non_writable_testdb(testdb, monkeypatch, raster_file): + import terracotta + terracotta.update_settings(DRIVER_PATH=str(testdb)) - return _s3_db_factory + driver = terracotta.get_driver(testdb) + with driver.connect(): + driver.insert(('first', 'second', 'third'), str(raster_file), skip_metadata=True) + driver.meta_store._WRITABLE = False + yield + driver.meta_store._WRITABLE = True -@pytest.fixture() -def mock_aws_env(monkeypatch): - with monkeypatch.context() as m: - m.setenv('AWS_DEFAULT_REGION', 'us-east-1') - m.setenv('AWS_ACCESS_KEY_ID', 'FakeKey') - m.setenv('AWS_SECRET_ACCESS_KEY', 'FakeSecretKey') - m.setenv('AWS_SESSION_TOKEN', 'FakeSessionToken') - yield + driver.delete(('first', 'second', 'third')) def run_test_server(driver_path, port): diff --git a/tests/drivers/test_sqlite_remote.py b/tests/drivers/test_sqlite_remote.py index 3342b44c..395ab541 100644 --- a/tests/drivers/test_sqlite_remote.py +++ b/tests/drivers/test_sqlite_remote.py @@ -4,15 +4,29 @@ """ import os +import tempfile import time +import uuid +from pathlib import Path import pytest from terracotta import exceptions +boto3 = pytest.importorskip('boto3') moto = pytest.importorskip('moto') +@pytest.fixture(autouse=True) +def mock_aws_env(monkeypatch): + with monkeypatch.context() as m: + m.setenv('AWS_DEFAULT_REGION', 'us-east-1') + m.setenv('AWS_ACCESS_KEY_ID', 'FakeKey') + m.setenv('AWS_SECRET_ACCESS_KEY', 'FakeSecretKey') + m.setenv('AWS_SESSION_TOKEN', 'FakeSessionToken') + yield + + class Timer: def __init__(self, auto=False): self.auto = auto @@ -27,8 +41,38 @@ def tick(self): self.time += 1 +@pytest.fixture() +def s3_db_factory(tmpdir): + bucketname = str(uuid.uuid4()) + + def _s3_db_factory(keys, datasets=None): + from terracotta import get_driver + + with tempfile.TemporaryDirectory() as tmpdir: + dbfile = Path(tmpdir) / 'tc.sqlite' + driver = get_driver(dbfile) + driver.create(keys) + + if datasets: + for keys, path in datasets.items(): + driver.insert(keys, path) + + with open(dbfile, 'rb') as f: + db_bytes = f.read() + + conn = boto3.resource('s3') + conn.create_bucket(Bucket=bucketname) + + s3 = boto3.client('s3') + s3.put_object(Bucket=bucketname, Key='tc.sqlite', Body=db_bytes) + + return f's3://{bucketname}/tc.sqlite' + + return _s3_db_factory + + @moto.mock_s3 -def test_remote_database(s3_db_factory, mock_aws_env): +def test_remote_database(s3_db_factory): keys = ('some', 'keys') dbpath = s3_db_factory(keys) @@ -47,7 +91,7 @@ def test_invalid_url(): @moto.mock_s3 -def test_nonexisting_url(mock_aws_env): +def test_nonexisting_url(): from terracotta import exceptions, get_driver driver = get_driver('s3://foo/db.sqlite') with pytest.raises(exceptions.InvalidDatabaseError): @@ -56,7 +100,7 @@ def test_nonexisting_url(mock_aws_env): @moto.mock_s3 -def test_remote_database_cache(s3_db_factory, raster_file, mock_aws_env): +def test_remote_database_cache(s3_db_factory, raster_file, monkeypatch): keys = ('some', 'keys') dbpath = s3_db_factory(keys) @@ -92,7 +136,7 @@ def test_remote_database_cache(s3_db_factory, raster_file, mock_aws_env): @moto.mock_s3 -def test_immutability(s3_db_factory, raster_file, mock_aws_env): +def test_immutability(s3_db_factory, raster_file): keys = ('some', 'keys') dbpath = s3_db_factory(keys, datasets={('some', 'value'): str(raster_file)}) @@ -111,7 +155,7 @@ def test_immutability(s3_db_factory, raster_file, mock_aws_env): @moto.mock_s3 -def test_destructor(s3_db_factory, raster_file, capsys, mock_aws_env): +def test_destructor(s3_db_factory, raster_file, capsys): keys = ('some', 'keys') dbpath = s3_db_factory(keys, datasets={('some', 'value'): str(raster_file)}) diff --git a/tests/server/test_flask_api.py b/tests/server/test_flask_api.py index 0ee2a346..430f62e6 100644 --- a/tests/server/test_flask_api.py +++ b/tests/server/test_flask_api.py @@ -8,8 +8,6 @@ import pytest -moto = pytest.importorskip('moto') - @pytest.fixture(scope='module') def flask_app(): @@ -53,37 +51,20 @@ def test_get_metadata(client, use_testdb): assert ['extra_data'] == json.loads(rv.data)['metadata'] -@moto.mock_s3 -def test_get_metadata_lazily_nonwritable_db(client, s3_db_factory, mock_aws_env, raster_file): - import terracotta - - keys = ('some', 'keys') - dbpath = s3_db_factory( - keys, datasets={("some", "value"): str(raster_file)}, skip_metadata=True - ) - terracotta.update_settings(DRIVER_PATH=str(dbpath), DRIVER_PROVIDER="sqlite-remote") - - rv = client.get('/metadata/some/value') +def test_get_metadata_lazily_nonwritable_db(client, use_non_writable_testdb): + rv = client.get('/metadata/first/second/third') assert rv.status_code == 403 -@moto.mock_s3 -def test_debug_errors(debug_client, s3_db_factory, mock_aws_env, raster_file, raster_file_xyz): - import terracotta +def test_debug_errors(debug_client, use_non_writable_testdb, raster_file_xyz): from terracotta import exceptions import marshmallow - keys = ('some', 'keys') - dbpath = s3_db_factory( - keys, datasets={("some", "value"): str(raster_file)}, skip_metadata=True - ) - terracotta.update_settings(DRIVER_PATH=str(dbpath), DRIVER_PROVIDER="sqlite-remote") - with pytest.raises(exceptions.DatabaseNotWritableError): - debug_client.get('/metadata/some/value') + debug_client.get('/metadata/first/second/third') with pytest.raises(exceptions.DatasetNotFoundError): - debug_client.get('/metadata/NONEXISTING/KEYS') + debug_client.get('/metadata/NONEXISTING/KEYS/YO') with pytest.raises(exceptions.InvalidKeyError): debug_client.get('/metadata/ONLYONEKEY') @@ -92,14 +73,14 @@ def test_debug_errors(debug_client, s3_db_factory, mock_aws_env, raster_file, ra with pytest.raises(exceptions.InvalidArgumentsError): debug_client.get( - f'/compute/some/value/{z}/{x}/{y}.png' + f'/compute/first/second/third/{z}/{x}/{y}.png' '?expression=v1*v2&v1=val22&v2=val23' '&stretch_range=[10000,0]' ) with pytest.raises(marshmallow.ValidationError): debug_client.get( - f'/compute/some/value/{z}/{x}/{y}.png' + f'/compute/first/second/third/{z}/{x}/{y}.png' '?stretch_range=[10000,0]' ) From d476ac90de48b422ba11011f4e0ca3574f99b8f1 Mon Sep 17 00:00:00 2001 From: Thomas Nyegaard-Signori Date: Thu, 31 Mar 2022 12:50:35 +0200 Subject: [PATCH 13/14] Actually use the monkeypatch... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dion Häfner --- tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1d9d5df2..950bfa8e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -353,9 +353,9 @@ def use_non_writable_testdb(testdb, monkeypatch, raster_file): with driver.connect(): driver.insert(('first', 'second', 'third'), str(raster_file), skip_metadata=True) - driver.meta_store._WRITABLE = False - yield - driver.meta_store._WRITABLE = True + with monkeypatch.context() as m: + m.setattr(driver.meta_store, "_WRITABLE", False) + yield driver.delete(('first', 'second', 'third')) From f07de5ef47276d88a3bb76f5c6df78dce2e8aa15 Mon Sep 17 00:00:00 2001 From: kiksekage Date: Thu, 31 Mar 2022 13:23:56 +0200 Subject: [PATCH 14/14] Use the contexts --- tests/drivers/test_raster_drivers.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/drivers/test_raster_drivers.py b/tests/drivers/test_raster_drivers.py index 791d7a7e..5eb4d859 100644 --- a/tests/drivers/test_raster_drivers.py +++ b/tests/drivers/test_raster_drivers.py @@ -11,6 +11,22 @@ METADATA_KEYS = ('bounds', 'range', 'mean', 'stdev', 'percentiles', 'metadata') +@pytest.fixture +def dynamic_non_writable_db(monkeypatch, driver_path, provider, raster_file): + from terracotta import drivers + db = drivers.get_driver(driver_path, provider=provider) + keys = ('some', 'keynames') + + db.create(keys) + db.insert(['some', 'value'], str(raster_file), skip_metadata=True) + + with monkeypatch.context() as m: + m.setattr(db.meta_store, "_WRITABLE", False) + yield + + db.delete(['some', 'value']) + + @pytest.mark.parametrize('provider', DRIVERS) def test_insertion_and_retrieval(driver_path, provider, raster_file): from terracotta import drivers @@ -147,16 +163,9 @@ def test_lazy_loading(driver_path, provider, raster_file): @pytest.mark.parametrize('provider', DRIVERS) -def test_non_writable_lazy_loading(driver_path, provider, raster_file): +def test_non_writable_lazy_loading(driver_path, provider, dynamic_non_writable_db): from terracotta import drivers db = drivers.get_driver(driver_path, provider=provider) - keys = ('some', 'keynames') - - db.create(keys) - db.insert(['some', 'value'], str(raster_file), skip_metadata=True) - - # Manually set the meta store to un-writable - db.meta_store._WRITABLE = False with pytest.raises(exceptions.DatabaseNotWritableError): db.get_metadata(['some', 'value'])