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

Opaque namespaces (at least "jar" URIs) do not work properly #20

Closed
fge opened this issue Sep 13, 2012 · 1 comment
Closed

Opaque namespaces (at least "jar" URIs) do not work properly #20

fge opened this issue Sep 13, 2012 · 1 comment
Labels

Comments

@fge
Copy link
Collaborator

fge commented Sep 13, 2012

Test file: https://github.com/fge/json-schema-validator/blob/master/src/test/java/org/eel/kitchen/jsonschema/other/JarNamespaceValidationTest.java

Failing test: callingSchemaViaJarURINamespaceWorks() (except, it doesn't)

The test which breaks is due to a difference in behavior between:

factory = JsonSchemaFactory.defaultFactory();
schema = factory.fromURI("jar:/foo.jar!/a/b.json");

and:

factory = new JsonSchemaFactory.Builder()
    .setNamespace("jar:/foo.jar!/a/").build();
schema = factory.fromURI("b.json");

While the first solution will use the base URIDownloader and succeed, the second
one will fail since SchemaRegistry does not use JsonRef, which has a hack for
jar relative resolution, but URI resolution -- and resolving any URI against an
opaque URI, which "jar" URIs are, leads to the URI given as an argument.

First highlighted in issue #19.

fge added a commit that referenced this issue Sep 13, 2012
Ugly aka the namespace is now a JsonRef and not a URI anymore, which means the
test works.

This is ugly for the reason that there is a lot of back-and-forth between URI
and JsonRef, and it is getting really messy. Time for a refactoring of some
sort.

Signed-off-by: Francis Galiegue <fgaliegue@gmail.com>
@fge
Copy link
Collaborator Author

fge commented Sep 14, 2012

Done. Right now, only jar URIs are really supported, but due to JsonRef refactoring, this can easily be extended to other opaque URI schemes should the need arise.

@fge fge closed this as completed Sep 14, 2012
fge added a commit that referenced this issue Sep 14, 2012
Probably the last 1.1.x release.

* Rework JsonRef. This fixes bugs #19, #20.
* Rename FormatSpecifier to FormatAttribute.
* Javadoc updates. In particular, mention the "id" conundrum, and why this
  implementation chooses to ignore section 5.27.

Francis Galiegue (16):
      M{in,ax}PropertiesKeywordValidator: fix javadoc
      Large renaming: format specifier --> format attribute
      Add skeleton test file for opaque namespace testing
      JarNamespaceValidationTest: test that one scenario works
      JarNamespaceValidationTest: improve code
      JarNamespaceValidationTest: devise test which does not work, but should
      "Fix" issue #20, the ugly way
      Introduce JsonLocator class and implementations, plus tests
      JsonRef: rename .asURI() to .toURI()
      More tests to catch malformed "jar" URLs
      JsonRef: method rename
      Rework JsonRef
      Remove JsonLocator, now unneeded
      JsonRef: simplify code a little
      Inspections pass, some nitpicks fixed
      Various Javadoc updates

Signed-off-by: Francis Galiegue <fgaliegue@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant