search

pwall567 / json-kotlin-schema

Kotlin implementation of JSON Schema (Draft 07)
MIT License
90 stars 13 forks source link

Support JAR and opaque URIs #16

Closed dhealio closed 10 months ago

dhealio commented 11 months ago

Description

This allows parsing of opaque URIs when the parser includes a functioning resolver. Paths can also be obtained from JAR URIs without the need of an extended resolver.

Changes

Previous Behavior

Previously, attempting to use any opaque URI would produce an error:

Caused by: java.lang.NullPointerException: uri.path must not be null
    at net.pwall.json.schema.parser.JSONReader.readByResolver(JSONReader.kt:169)
    at net.pwall.json.schema.parser.JSONReader.readJSON(JSONReader.kt:142)
    ... 42 common frames omitted

This occurs because the path value for opaque URIs will always be null.

New Behavior

JAR URIs now function without causing a NullPointerException, and do not require a resolver.

Opaque URIs will now function according to the resolver being used instead of causing a NullPointerException.

YAML parsing for opaque URIs require an extended resolver that is capable of correctly setting the content type, otherwise JSON is assumed.

pwall567 commented 11 months ago

Hi @dhealio , thanks for this. I won't have time to review it until the weekend, but I will get back to you as soon as possible.

Thanks again, Peter

dhealio commented 10 months ago

@pwall567 Have you been able to make any progress on this? This will benefit our use case where we need to reference resources within a JAR file, which is currently hard blocked by the error mentioned in the description. Thanks in advance for your time!

pwall567 commented 10 months ago

Hi @dhealio , sorry I have been so slow in responding to this. I don't know what country you are in, but in Australia the Christmas / New Year period is the main summer holiday time, and many companies (including my day job) are under pressure to get projects completed by mid-December, and then everyone goes on leave. All this is just a complicated excuse for not reviewing your PR before now :smile: .

When I saw the reference to opaque URIs in the heading, I thought you were referring to something like UUID URNs, which are truly opaque. But jar: URIs aren't really opaque, they are just non-standard (at least as far as the java.net.URI class is concerned).

I sympathise with your wish to use URI/URL references to entries in a JAR file – it's a situation I have encountered myself many times. You may have seen me refer (in responses to previous questions and issues) to a new generation of the schema libraries that have been under development for some time now – they use a new library created for exactly this purpose (resource-loader, if you're interested), but the new JSON Schema libraries are still some way off complete.

In the meantime, to get a similar form of functionality into json-kotlin-schema, I think I prefer the following to your approach:

In the companion object of JSONReader, add an extension function:

        fun URI.extendedPath(): String = when (scheme) {
            "file", "http", "https" -> path
            "jar" -> schemeSpecificPart.substringBefore('#').substringBefore("?").substringAfter("!")
            else -> "UNKNOWN"
        }

Then, in the calls to looksLikeYAML() in readByResolver(), replace uri.path with uri.extendedPath(). I think this should achieve your goal, and it keeps the YAML / JSON determination in one place.

I'm explaining this to you rather than implementing it myself in the hope that I can persuade you to modify your PR along these lines, including the tests (I appreciate the effort you have gone to in writing tests that conform to my own style). Would that be possible?

Regards, Peter Wall

dhealio commented 10 months ago

... everyone goes on leave

No worries. That is true here as well.

... reference to opaque URIs in the heading

I believe this heading is mostly accurate, as the changes in this branch technically allow any URIs to now work when an extended resolver is present. That being said, the JAR support was my primary motivation here.

Then, in the calls to looksLikeYAML() in readByResolver(), replace uri.path with uri.extendedPath().

Done. I like the simplification, and it removes the requirement to use an extended resolver. There's a small concern that manual path parsing of the JAR URI is being done vs. relying on the underlying implementation, but I believe it's being done correctly.

Is it possible to get a release published once this is merged?

pwall567 commented 10 months ago

This is now deployed to Maven Central as version 0.44.

I agree that manual parsing of the URI is not ideal, but there really isn't any alternative since the java.net.URI class doesn't do it for us (and I managed to simplify it a little since schemeSpecificPart already removes the fragment).

Thank you for your work on this – I hope the new version meet your requirements,

Regards, Peter