Skip to content

Fix relative references #306

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

Fix relative references #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
37d4f0a
fixed jsonschema.exceptions.RefResolutionError: Unresolvable JSON poi…
mathewmarcus Nov 16, 2016
d9efe69
added tests for modified resolve_fragment
mathewmarcus Nov 18, 2016
aab7523
fixed styling on new tests
mathewmarcus Nov 20, 2016
e74dedc
removed .idea from .gitignore
mathewmarcus Nov 20, 2016
62640dd
removed unecessary code from mocked resolve_fragment
mathewmarcus Nov 20, 2016
e455439
removed unnecessary imports and items from schemas in tests
mathewmarcus Nov 20, 2016
79baa6c
readded import statement for ErrorTree in validators.py
mathewmarcus Nov 20, 2016
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
74 changes: 73 additions & 1 deletion jsonschema/tests/test_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
from jsonschema.tests.compat import mock, unittest
from jsonschema.validators import (
RefResolutionError, UnknownType, Draft3Validator,
Draft4Validator, RefResolver, create, extend, validator_for, validate,
Draft4Validator, RefResolver, create, extend, validator_for, validate
)
from jsonschema.compat import unquote


class TestCreateAndExtend(unittest.TestCase):
Expand Down Expand Up @@ -771,6 +772,20 @@ class TestRefResolver(unittest.TestCase):
stored_uri = "foo://stored"
stored_schema = {"stored" : "schema"}

def prev_resolve_fragment(self, document, fragment):
fragment = fragment.lstrip(u"/")
Copy link
Member

Choose a reason for hiding this comment

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

This seems like quite a lot of not-testing-specific-looking code making an appearance in a test. I don't have a specific recommendation yet on how to remove it, but it doesn't look right to me, especially not in combination with being patched into place (I'd like to sometime soon remove the mock dependency entirely, but alas).

Copy link
Author

Choose a reason for hiding this comment

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

Ok sorry about that, I can remove some of the unnecessary parts of that method. I only put them in there to replicate the original method as closely as possible.

parts = unquote(fragment).split(u"/") if fragment else []

for part in parts:
try:
document = document[part]
except (TypeError, LookupError):
raise RefResolutionError(
"Unresolvable JSON pointer: %r" % fragment
)

return document

def setUp(self):
self.referrer = {}
self.store = {self.stored_uri : self.stored_schema}
Expand Down Expand Up @@ -898,6 +913,63 @@ def test_helpful_error_message_on_failed_pop_scope(self):
resolver.pop_scope()
self.assertIn("Failed to pop the scope", str(exc.exception))

def test_prev_resolve_fragment_raises_exception(self):
schema = {
"definitions": {
Copy link
Member

Choose a reason for hiding this comment

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

This schema seems like it should be easy to simplify.

"Pet": {
"properties": {
"category": {
"type": "array",
"items": {
"$ref": "#/definitions/Category"
}
}
}
},
"Category": {
"properties": {
"name": {
"type": "string"
},
}
}
}
}
resolver = RefResolver.from_schema(schema)
resolver.resolve_fragment = mock.MagicMock(
side_effect=self.prev_resolve_fragment)
self.assertRaises(RefResolutionError,
resolver.resolve_fragment,
schema,
"#/definitions/Category")

def test_resolve_fragment(self):
schema = {
"definitions": {
"Pet": {
"properties": {
"category": {
"type": "array",
"items": {
"$ref": "#/definitions/Category"
}
}
}
},
"Category": {
"properties": {
"name": {
"type": "string"
},
}
}
}
}
resolver = RefResolver.from_schema(schema)
self.assertEqual(resolver.resolve_fragment
(schema, "#/definitions/Category"),
schema['definitions']['Category'])


class UniqueTupleItemsMixin(object):
"""
Expand Down
5 changes: 2 additions & 3 deletions jsonschema/validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
Sequence, urljoin, urlsplit, urldefrag, unquote, urlopen,
str_types, int_types, iteritems, lru_cache,
)
from jsonschema.exceptions import ErrorTree # Backwards compatibility # noqa
from jsonschema.exceptions import RefResolutionError, SchemaError, UnknownType

from jsonschema.exceptions import ErrorTree # Backwards compatibility # noqa

_unset = _utils.Unset()

Expand Down Expand Up @@ -356,7 +355,7 @@ def resolve_fragment(self, document, fragment):

"""

fragment = fragment.lstrip(u"/")
fragment = fragment.lstrip(u"#").lstrip(u"/")
Copy link
Member

Choose a reason for hiding this comment

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

This seems likely to be misplaced overall to me (which maybe is another reason why an integration test would be good).

This method is about resolving fragments -- fragments don't start with #, # delimits a fragment, so whoever called this should have removed the #

Copy link
Author

Choose a reason for hiding this comment

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

Got it, my mistake

Copy link
Author

Choose a reason for hiding this comment

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

Would you want functionality, perhaps in another method, to convert a pointer into a fragment, by stripping the #, etc, or do you want to close the request?

parts = unquote(fragment).split(u"/") if fragment else []

for part in parts:
Expand Down