-
-
Notifications
You must be signed in to change notification settings - Fork 317
RFC5: apply distance limiting #2229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1332,3 +1332,53 @@ def evaluate_limit(requested: Union[None, int], server_limits: dict, | |
| else: | ||
| LOGGER.debug('limit requested') | ||
| return min(requested2, max_) | ||
|
|
||
|
|
||
| def evaluate_limit_distance(server_limits: dict, collection_limits: dict, | ||
| request_bbox: list = []) -> bool: | ||
| """ | ||
| Helper function to evaluate limit parameter | ||
|
|
||
| :param server_limits: `dict` of server limits | ||
| :param collection_limits: `dict` of collection limits | ||
| :param request_bbox: `list` bbox of request | ||
|
|
||
| :returns: `bool` successful distance check | ||
| """ | ||
|
|
||
| exceed_msg = None | ||
|
|
||
| effective_limits = ChainMap(collection_limits, server_limits) | ||
|
|
||
| on_exceed = effective_limits.get('on_exceed', 'throttle') | ||
| max_distance_x = effective_limits.get('max_distance_x') | ||
| max_distance_y = effective_limits.get('max_distance_y') | ||
| max_distance_units = effective_limits.get('max_distance_units') | ||
|
|
||
| LOGGER.debug(f'On exceed: {on_exceed}') | ||
| LOGGER.debug(f'Maximum distance x: {max_distance_x}') | ||
| LOGGER.debug(f'Maximum distance y: {max_distance_y}') | ||
| LOGGER.debug(f'Maximum distance units: {max_distance_units}') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a bit worried about what we can get inside the variable max_distance_units. Does it make sense to create an enum here and validate it against it? |
||
|
|
||
| # TODO: assess distance units | ||
|
|
||
| if not request_bbox: | ||
| LOGGER.debug('no bbox requested') | ||
| return True | ||
|
|
||
| if max_distance_x is not None: | ||
| requested_distance_x = abs(request_bbox[2] - request_bbox[0]) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth establishing the unit for distance. This as a simple comparison makes some sense when |
||
| if requested_distance_x > max_distance_x: | ||
| exceed_msg = 'Maximum distance x exceeded' | ||
|
|
||
| if max_distance_y is not None: | ||
| requested_distance_y = abs(request_bbox[3] - request_bbox[1]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the bbox uses degrees (e.g: CRS84/WGS84) would it be correct to apply this type of cartesian math? Not an issue for small distances, since the distortion is small; but if we you have larger distances, we could be looking at differences in the requested distance according to the location on earth. The safe solution would be to always project the bbox to a projected coordinated system (e.g.: 3857). |
||
| if requested_distance_y > max_distance_y: | ||
| exceed_msg = 'Maximum distance y exceeded' | ||
|
|
||
| if exceed_msg is not None: | ||
| LOGGER.warning(exceed_msg) | ||
| if on_exceed == 'error': | ||
| raise ValueError(exceed_msg) | ||
|
|
||
| return True | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,8 +51,8 @@ | |
| from pygeoapi.util import filter_dict_by_key_value, to_json | ||
|
|
||
| from . import ( | ||
| APIRequest, API, SYSTEM_LOCALE, validate_bbox, validate_datetime, | ||
| validate_subset | ||
| APIRequest, API, SYSTEM_LOCALE, evaluate_limit_distance, validate_bbox, | ||
| validate_datetime, validate_subset | ||
| ) | ||
|
|
||
| LOGGER = logging.getLogger(__name__) | ||
|
|
@@ -112,6 +112,11 @@ def get_collection_coverage( | |
| else: | ||
| try: | ||
| bbox = validate_bbox(bbox) | ||
| server_limits = api.config['server'].get('limits', {}) | ||
| collection_limits = api.config['resources'][dataset].get('limits', {}) # noqa | ||
|
|
||
| _ = evaluate_limit_distance(request.params.get('bbox', []), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to previous comment, might be worth transforming the bbox to WGS84 here until the unit is applied? |
||
| server_limits, collection_limits) | ||
| except ValueError as err: | ||
| msg = str(err) | ||
| return api.get_exception( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may be worth casting max_distance_x and mast_distance_y to floats, and wrap this into a try/catch block