Implement buffer_size overrides

openmaptiles/mbtile_tools.py

@@ -2,6 +2,7 @@
 import os
 import sqlite3
 from datetime import datetime
+from os import getenv
 from pathlib import Path
 from sqlite3 import Cursor
 from typing import Dict, List, Optional, Tuple
@@ -315,7 +316,7 @@ def show_tile(self, zoom, x, y, show_names, summary):
 
     def _update_metadata(self, metadata, auto_minmax, reset, file, center_zoom=None):
         def update_from_env(param, env_var):
-            val = os.environ.get(env_var)
+            val = getenv(env_var)
             if val is not None:
                 metadata[param] = val
 
@@ -328,10 +329,10 @@ def update_from_env(param, env_var):
 
         metadata['filesize'] = os.path.getsize(file)
 
-        bbox_str = os.environ.get('BBOX')
+        bbox_str = getenv('BBOX')
         if bbox_str:
             bbox = Bbox(bbox=bbox_str,
-                        center_zoom=os.environ.get('CENTER_ZOOM', center_zoom))
+                        center_zoom=getenv('CENTER_ZOOM', center_zoom))
             metadata['bounds'] = bbox.bounds_str()
             metadata['center'] = bbox.center_str()
 

openmaptiles/tileset.py

@@ -1,6 +1,7 @@
 from dataclasses import dataclass
+import os
 from pathlib import Path
-from typing import List, Union, Dict, Any, Callable
+from typing import List, Union, Dict, Any, Callable, Optional, NewType
 
 import sys
 import warnings
@@ -10,13 +11,35 @@
 from .utils import print_err
 
 
+GetEnv = NewType('GetEnv', Callable[[str, Optional[str]], Optional[str]])
+
+
 def tag_fields_to_sql(fields):
     """Converts a list of fields stored in the tags hstore into a list of SQL fields:
         name:en   =>   NULLIF(tags->'name:en', '') AS name:en
     """
     return [f"NULLIF(tags->'{fld}', '') AS \"{fld}\"" for fld in fields]
 
 
+def assert_int(value, name: str, min_val: Optional[int] = None, max_val: Optional[int] = None, required=False):
+    if value is None:
+        if required:
+            raise ValueError(f'Value {name} does not exist')
+        return None
+    elif isinstance(value, str):
+        try:
+            value = int(value)
+        except ValueError:
+            raise ValueError(f'Unable to parse {name} value "{value}" as an integer')
+    elif not isinstance(value, int):
+        raise ValueError(f'The {name} value was expected to be an integer, but found {type(value).__name__} "{value}"')
+    if min_val is not None and value < min_val:
+        raise ValueError(f'The {name} value {value} is less than the minimum allowed {min_val}')
+    if max_val is not None and value > max_val:
+        raise ValueError(f'The {name} value {value} is more than the maximum allowed {max_val}')
+    return value
+
+
 @dataclass
 class ParsedData:
     data: Union[dict, str]
@@ -70,14 +93,17 @@ def parse(layer_source: Union[str, Path, ParsedData]) -> 'Layer':
     def __init__(self,
                  layer_source: Union[str, Path, ParsedData],
                  tileset: 'Tileset' = None,
-                 index: int = None):
+                 index: int = None,
+                 overrides: dict = None):
         """
         :param layer_source: load layer from this source, e.g. a file
         :param tileset: parent tileset object (optional)
         :param index: layer's position index within the tileset
+        :param overrides: additional override parameters for this layer
         """
         self.tileset = tileset
         self.index = index
+        self.overrides = overrides
 
         if isinstance(layer_source, ParsedData):
             self.filename = layer_source.path
@@ -116,7 +142,7 @@ def __init__(self,
             requires = requires.copy()  # dict will be modified to detect unrecognized properties
 
         err = 'If set, "requires" parameter must be a map with optional "layers", "tables", and "functions" sub-elements. Each sub-element must be a string or a list of strings. If "requires" is a list or a string itself, it is treated as a list of layers. ' + \
-            'Optionally add "helpText" sub-element string to help the user with generating missing tables and functions.'
+              'Optionally add "helpText" sub-element string to help the user with generating missing tables and functions.'
         self.requires_layers = get_requires_prop(
             requires, 'layers',
             err + '"requires.layers" must be an ID of another layer, or a list of layer IDs.')
@@ -147,17 +173,6 @@ def __init__(self,
             # osm_id column twice - once for feature_id, and once as an attribute
             raise ValueError('key_field_as_attribute=yes is not yet implemented')
 
-    @deprecated(version='3.2.0', reason='use named properties instead')
-    def __getitem__(self, attr):
-        if attr in self.definition:
-            return self.definition[attr]
-        elif attr == 'fields':
-            return {}
-        elif attr == 'description':
-            return ''
-        else:
-            raise KeyError
-
     def get_fields(self) -> List[str]:
         """Get a list of field names this layer generates.
            Geometry field is not included."""
@@ -181,7 +196,49 @@ def description(self) -> str:
 
     @property
     def buffer_size(self) -> int:
-        return self.definition['layer']['buffer_size']
+        """Each layer must have a default buffer size, with either `buffer_size` or `min_buffer_size` or both.
+        If both are set, `buffer_size` must be >= `min_buffer_size`.
+        min_buffer_size is only used when there is a global buffer size override,
+        e.g. if global `buffer_size` is set to 0, and layer's `min_buffer_size` is set to 4, the result is 4.
+        Per layer tileset overrides are allowed for both buffer_size and min_buffer_size.
+        Per layer overrides have higher priority than global overrides, but less than ENV var.
+        """
+        size = assert_int(self.definition['layer'].get('buffer_size'), 'buffer_size', min_val=0)
+        min_size = assert_int(self.definition['layer'].get('min_buffer_size'), 'min_buffer_size', min_val=0)
+        if size is None and min_size is None:
+            raise ValueError(f'Layer "{self.id}" is missing an integer buffer_size and/or min_buffer_size')
+        elif size is not None and min_size is not None:
+            if size < min_size:
+                raise ValueError(f'Layer "{self.id}" has buffer_size less than min_buffer_size')
+        elif size is None:
+            # size is not set, use min_size as default
+            size = min_size
+        else:
+            # size is set, min_size is not set
+            min_size = 0
+
+        if self.tileset:
+            val = assert_int(self.tileset.overrides.get('buffer_size'), 'buffer_size global override', min_val=0)
+            if val is not None:
+                size = val
+        if self.overrides:
+            val = assert_int(self.overrides.get('buffer_size'), 'buffer_size layer override', min_val=0)
+            min_val = assert_int(self.overrides.get('min_buffer_size'), 'min_buffer_size layer override', min_val=0)
+            if val is not None and min_val is not None and val < min_val:
+                raise ValueError(f'Layer overrides for "{self.id}" have buffer_size less than min_buffer_size')
+            if val is not None:
+                size = val
+            if min_val is not None:
+                min_size = min_val
+        # Allow empty env var to be the same as unset.
+        getenv = self.tileset.getenv if self.tileset else os.getenv
+        tbs = getenv('TILE_BUFFER_SIZE', '')
+        val = assert_int(tbs if tbs != '' else None, 'TILE_BUFFER_SIZE env var', min_val=0)
+        if val is not None:
+            size = val
+        if size < min_size:
+            size = min_size
+        return size
 
     @property
     def max_size(self) -> int:
@@ -255,24 +312,33 @@ class Tileset:
     filename: Path
     definition: dict
     layers: List[Layer]
+    getenv: GetEnv
 
     @staticmethod
     def parse(tileset_source: Union[str, Path, ParsedData]) -> 'Tileset':
         return Tileset(tileset_source)
 
-    def __init__(self, tileset_source: Union[str, Path, ParsedData]):
+    def __init__(self, tileset_source: Union[str, Path, ParsedData], getenv: GetEnv = None):
+        """Create a new tileset from a file (str|Path), or already parsed.
+        Optionally provide environment variables (used in unit testing)."""
+        self.getenv = getenv or os.getenv
         if isinstance(tileset_source, ParsedData):
             self.filename = tileset_source.path
-            self.definition = tileset_source.data
+            data = tileset_source.data
         else:
             self.filename = Path(tileset_source).resolve()
-            self.definition = parse_file(self.filename)
+            data = parse_file(self.filename)
 
-        self.definition = self.definition['tileset']
+        self.definition = data['tileset']
         self.layers = []
         self.layers_by_id = {}
-        for index, layer_filename in enumerate(self.definition['layers']):
-            layer = Layer(layer_filename, self, index)
+
+        layer_obj: Union[str, Path, ParsedData, dict]
+        for index, layer_obj in enumerate(self.definition['layers']):
+            if isinstance(layer_obj, dict):
+                layer = Layer(layer_obj['file'], self, index, layer_obj)
+            else:
+                layer = Layer(layer_obj, self, index, overrides=None)
             if layer.id in self.layers_by_id:
                 raise ValueError(f"Layer '{layer.id}' is defined more than once")
             self.layers.append(layer)
@@ -301,13 +367,6 @@ def __init__(self, tileset_source: Union[str, Path, ParsedData]):
 
         validate_properties(self, f'Tileset {self.filename}')
 
-    @deprecated(version='3.2.0', reason='use named properties instead')
-    def __getitem__(self, attr):
-        if attr in self.definition:
-            return self.definition[attr]
-        else:
-            raise KeyError
-
     @property
     def attribution(self) -> str:
         return self.definition['attribution']
@@ -360,6 +419,10 @@ def minzoom(self) -> int:
     def name(self) -> str:
         return self.definition['name']
 
+    @property
+    def overrides(self) -> dict:
+        return self.definition.get('overrides', {})
+
     @property
     def pixel_scale(self) -> int:
         return self.definition['pixel_scale']

tests/python/README.md

@@ -0,0 +1,9 @@
+# Python Unit Tests
+
+These tests can be quickly ran using `make bash` (from the root of the tools project):
+
+```bash
+host$  make bash
+
+openmaptiles@...:/tileset$   python -m unittest
+```

tests/python/test_sql.py

@@ -1,7 +1,7 @@
 import warnings
 from dataclasses import dataclass
 from pathlib import Path
-from typing import List, Union, Dict
+from typing import List, Union, Dict, Optional
 from unittest import main, TestCase
 
 from openmaptiles.sql import collect_sql, sql_assert_table, sql_assert_func
@@ -43,16 +43,16 @@ def parsed_data(layers: Union[Case, List[Case]]):
             defaults=dict(srs='test_srs', datasource=dict(srid='test_datasource')),
             id='id1',
             layers=[
-                ParsedData(dict(
+                dict(file=ParsedData(dict(
                     layer=dict(
-                        buffer_size='test_buffer_size',
+                        buffer_size='10',
                         datasource=dict(query='test_query'),
                         id=v.id,
                         fields={},
                         requires=[v.reqs] if isinstance(v.reqs, str) else v.reqs or []
                     ),
                     schema=[ParsedData(v.query, Path(v.id + '_s.yaml'))] if v.query else [],
-                ), Path(f'./{v.id}.yaml')) for v in ([layers] if isinstance(layers, Case) else layers)
+                ), Path(f'./{v.id}.yaml'))) for v in ([layers] if isinstance(layers, Case) else layers)
             ],
             maxzoom='test_maxzoom',
             minzoom='test_minzoom',
@@ -158,6 +158,7 @@ def _ts_parse(self, reqs, expected_layers, expected_tables, expected_funcs, extr
         self.assertEqual(layer.requires_layers, expected_layers)
         self.assertEqual(layer.requires_tables, expected_tables)
         self.assertEqual(layer.requires_functions, expected_funcs)
+        self.assertEqual(layer.buffer_size, 10)
 
         # This test can be deleted once we remove the deprecated property in some future version
         with warnings.catch_warnings():
@@ -181,6 +182,55 @@ def test_ts_parse(self):
         self._ts_parse(dict(layers=['c1'], tables=['a', 'b'], functions=['x', 'y']),
                        ['c1'], ['a', 'b'], ['x', 'y'], extra)
 
+    def _ts_overrides(self, expected_layer: dict,
+                      layer: Optional[dict] = None,
+                      override_ts: Optional[dict] = None,
+                      override_layer: Optional[dict] = None,
+                      env: Optional[dict] = None):
+        data = parsed_data([Case('my_id', 'my_query;')])
+
+        ts_data = data.data['tileset']
+        if override_ts is not None:
+            ts_data['overrides'] = override_ts
+        if layer is not None:
+            ts_data['layers'][0]['file'].data['layer'].update(layer)
+        if override_layer is not None:
+            ts_data['layers'][0].update(override_layer)
+
+        ts = Tileset(data, getenv=env.get if env else None)
+        for k in expected_layer.keys():
+            self.assertEqual(getattr(ts.layers_by_id['my_id'], k), expected_layer[k])
+
+    def test_overrides(self):
+        buf_0 = dict(buffer_size=0)
+        buf_1 = dict(buffer_size=1)
+        buf_2 = dict(buffer_size=2)
+        buf_3 = dict(buffer_size=3)
+        min_1 = dict(min_buffer_size=1)
+        min_2 = dict(min_buffer_size=2)
+        min_3 = dict(min_buffer_size=3)
+
+        self._ts_overrides(buf_2, buf_2)
+        self._ts_overrides(buf_0, buf_2, override_ts=buf_0)
+        self._ts_overrides(buf_1, buf_2, override_ts=buf_1)
+        self._ts_overrides(buf_3, buf_2, override_ts=buf_3)
+        self._ts_overrides(buf_1, min_1 | buf_2, override_ts=buf_0)
+        self._ts_overrides(buf_1, buf_2, override_layer=buf_1)
+        self._ts_overrides(buf_2, min_2 | buf_3, override_layer=buf_1)
+        self._ts_overrides(buf_3, min_1 | buf_2, override_layer=min_3)
+        self._ts_overrides(buf_3, min_1 | buf_2, override_layer=min_2 | buf_3, override_ts=buf_0)
+
+        env_0 = dict(TILE_BUFFER_SIZE='0')
+        env_2 = dict(TILE_BUFFER_SIZE='2')
+        self._ts_overrides(buf_2, min_1 | buf_2, override_layer=min_2 | buf_3, override_ts=buf_0, env=env_0)
+        self._ts_overrides(buf_2, buf_1, env=env_2)
+        self._ts_overrides(buf_2, min_2 | buf_3, env=env_0)
+        self._ts_overrides(buf_2, min_1 | buf_3, override_ts=buf_0, env=env_2)
+        self._ts_overrides(buf_2, buf_2, dict(TILE_BUFFER_SIZE=''))
+
+        # str parsing
+        self._ts_overrides(dict(buffer_size=2), dict(buffer_size='2'))
+
 
 if __name__ == '__main__':
     main()