-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add basic Validation for URITemplates #36
base: main
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 |
---|---|---|
@@ -0,0 +1,43 @@ | ||
"""Tests for the uritemplate.Validator class.""" | ||
import uritemplate | ||
from uritemplate import exceptions | ||
|
||
import pytest | ||
|
||
|
||
@pytest.mark.parametrize('template', [ | ||
'https://github.com{/user}', | ||
'https://github.com/sigmavirus24{/repository}', | ||
'{foo}', | ||
'?{bar}', | ||
'https://example.com', | ||
]) | ||
def test_valid_uris(template): | ||
"""Verify we don't raise an exception.""" | ||
urit = uritemplate.URITemplate(template) | ||
uritemplate.Validator().validate(urit) | ||
|
||
|
||
@pytest.mark.parametrize('template', [ | ||
'https://github.com{/user', | ||
'https://github.com/sigmavirus24/repository}', | ||
'{foo}}', | ||
'?{{bar}', | ||
]) | ||
def test_invalid_uris(template): | ||
"""Verify we catch invalid URITemplates.""" | ||
urit = uritemplate.URITemplate(template) | ||
with pytest.raises(exceptions.InvalidTemplate): | ||
uritemplate.Validator().validate(urit) | ||
|
||
|
||
@pytest.mark.parametrize('template', [ | ||
'https://github.com{/user', | ||
'https://github.com/sigmavirus24/repository}', | ||
'{foo}}', | ||
'?{{bar}', | ||
]) | ||
def test_allow_invalid_uris(template): | ||
"""Verify we allow invalid URITemplates.""" | ||
urit = uritemplate.URITemplate(template) | ||
uritemplate.Validator().allow_unbalanced_braces().validate(urit) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
"""Module containing all exceptions for the uritemplate module.""" | ||
|
||
|
||
class TemplateException(Exception): | ||
"""Base Exception class for all uritemplate exceptions.""" | ||
|
||
pass | ||
|
||
|
||
class InvalidTemplate(TemplateException): | ||
"""Base class for template validation.""" | ||
|
||
message = "The URI template ({0}) is invalid." | ||
|
||
def __init__(self, uri, *args): | ||
"""Initialize our exception.""" | ||
super(InvalidTemplate, self).__init__(self.message.format(uri, *args)) | ||
self.uri = uri | ||
|
||
|
||
class UnbalancedBraces(InvalidTemplate): | ||
"""The template has unbalanced braces.""" | ||
|
||
message = "The URI template ({0}) has more {1} braces than {2} braces." | ||
|
||
def __init__(self, uri, left_braces_count, right_braces_count): | ||
"""Initialize our exception.""" | ||
more, less = 'left', 'right' | ||
if left_braces_count < right_braces_count: | ||
more, less = less, more | ||
super(UnbalancedBraces, self).__init__(uri, more, less) | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
"""Module containing all the validation logic for uritemplate.""" | ||
from uritemplate import exceptions as exc | ||
|
||
|
||
class Validator(object): | ||
"""Provide configurable validation of URITemplate objects. | ||
|
||
.. versionadded:: 3.1.0 | ||
|
||
.. code-block:: | ||
|
||
from uritemplate import URITemplate, Validator | ||
|
||
|
||
""" | ||
|
||
def __init__(self): | ||
"""Initialize our validator.""" | ||
self.enforcing_unbalanced_braces = True | ||
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. Are you intentionally leaving this public? Will someone want to query one of these to find out whether it's enforcing things or not? (Also could be a class attribute and ditch the method if you'd like) 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. We don't document it, so instead of adding an underscore, I just left it public. If people want to try to mutate it, then oh well. Turning this into a |
||
|
||
def allow_unbalanced_braces(self): | ||
"""Allow a template to have unbalanced braces. | ||
|
||
.. versionadded:: 3.1.0 | ||
|
||
Returns the validator instance. | ||
""" | ||
self.enforcing_unbalanced_braces = False | ||
return self | ||
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 take it you intentionally like chaining, even in cases where the instance itself has been mutated? 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 want to encourage chaining, yes. I've waffled back on having the instance be mutable or not, but in this case, I don't think it needs to be treated as immutable. I'm open to being convinced otherwise. |
||
|
||
def force_balanced_braces(self): | ||
"""Force a template to have balanced braces. | ||
|
||
.. versionadded:: 3.1.0 | ||
|
||
Returns the validator instance. | ||
""" | ||
self.enforcing_unbalanced_braces = True | ||
return self | ||
|
||
def validate(self, template): | ||
"""Validate that a template meets the parameters. | ||
|
||
.. versionadded:: 3.1.0 | ||
|
||
:raises: uritemplate.exceptions.InvalidTemplate | ||
:raises: uritemplate.exceptions.UnbalancedBraces | ||
""" | ||
if self.enforcing_unbalanced_braces: | ||
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. Could get rid of this 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 don't think it would hurt if someone wanted to print out the configuration in a log message for themselves, especially if their validation logic is somewhat dynamic. |
||
_enforce_balanced_braces(template) | ||
|
||
|
||
def _enforce_balanced_braces(template): | ||
uri = template.uri | ||
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 still haven't read the spec unfortunately, so apologies, but is this actually enough? E.g., is 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. That is not valid, no. I'll have to work on some better validation for that situation. |
||
left_braces = uri.count('{') | ||
right_braces = uri.count('}') | ||
if left_braces != right_braces: | ||
raise exc.UnbalancedBraces(uri, left_braces, right_braces) |
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.
Do you mean
uri, more, less
? The stuff yousuper
for an exception will end up one.args
as the stuff that was passed to__init__
, but the "user" of the exception here didn't pass those, they were intermediate values. It looks like you're using that to handle formatting them into your message, could always just define__str__
instead though obviously if you wanted to.