Refactor ref schema URI construction. #177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This refactors JSON::Validator#load_ref_schema to not implement a lot of logic that Pathname and URI are already capable of, and splits some of it into a separate method for clarity. Some JSON::Validator class methods are also added to hide away the specific implementation of how already loaded schemas are kept around. * Refactor JSON::Validator#load_schema_ref to move absolute URI construction to its own method, and to bail ASAP if the schema is already loaded. * Add JSON::Validator#absolutize_ref_uri to handle logic around constructing an absolute URI to a schema based on the ref URI and its parent schema URI. * Add JSON::Validator.schema_for_uri to fetch an already loaded schema for a given URI. * Add JSON::Validator.schema_loaded? predicate method for testing whether a schema is already loaded. * Move some existing code to use the new class methods.
|
Well, it'll probably conflict. But I think we should merge this because it breaks up loading a schema into single blocks of dedicated logic. So 👍 |
RST-J
added a commit
that referenced
this pull request
Nov 7, 2014
…ction Refactor ref schema URI construction.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm fairly certain I've found a bug in handling of the
idproperty, but in the process of tracking it down I found some opportunities for refactoring while trying to reason about how schema URIs were constructed. Rather than lump a bug fix and a refactor into the one PR, I'm starting with this PR that just includes the refactoring. Ran all tests and they are passing locally.This refactors
JSON::Validator#load_ref_schemato not implement a lot of logic thatPathnameandURIare already capable of, and splits some of it into a separate method for clarity. SomeJSON::Validatorclass methods are also added to hide away the specific implementation of how already loaded schemas are kept around.JSON::Validator#load_schema_refto move absolute URI construction to its own method, and to bail ASAP if the schema is already loaded.JSON::Validator#absolutize_ref_urito handle logic around constructing an absolute URI to a schema based on the ref URI and its parent schema URI.JSON::Validator.schema_for_urito fetch an already loaded schema for a given URI.JSON::Validator.schema_loaded?predicate method for testing whether a schema is already loaded.