Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions tests/test_validator.py
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)
5 changes: 3 additions & 2 deletions uritemplate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
__author__ = 'Ian Cordasco'
__license__ = 'Modified BSD or Apache License, Version 2.0'
__copyright__ = 'Copyright 2013 Ian Cordasco'
__version__ = '3.0.0'
__version__ = '3.1.0'
__version_info__ = tuple(int(i) for i in __version__.split('.') if i.isdigit())

from uritemplate.api import (
URITemplate, expand, partial, variables # noqa: E402
)
from uritemplate.validator import Validator # noqa: E402

__all__ = ('URITemplate', 'expand', 'partial', 'variables')
__all__ = ('URITemplate', 'Validator', 'expand', 'partial', 'variables')
31 changes: 31 additions & 0 deletions uritemplate/exceptions.py
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)
Copy link

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 you super for an exception will end up on e.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.

58 changes: 58 additions & 0 deletions uritemplate/validator.py
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
Copy link

Choose a reason for hiding this comment

The 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)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 property with a setter will mean that people might try to assign weird values and it breaks chaining.


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
Copy link

Choose a reason for hiding this comment

The 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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could get rid of this if if you wanted by instead having enforce_unbalanced_braces be a function rather than a boolean (and making it do nothing when you disable validation), although whether you'd do that probably depends heavily on your opinion above (about whether someone should want to write if somevalidator.enforcing_unbalanced_braces in external code bases)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(about whether someone should want to write if somevalidator.enforcing_unbalanced_braces in external code bases)

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
Copy link

Choose a reason for hiding this comment

The 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 }{}{}{ valid? Or do the braces need to be properly nested too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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)