-
-
Notifications
You must be signed in to change notification settings - Fork 207
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
$vocabulary tests are incorrectly required #574
Comments
That's how I understand it as well. I agree that the test should be moved to "optional". |
Strong disagree. I will work up a more thorough response. |
Regardless of the outcome here, we should clean up the wording in the spec so it's more clear what the Right Thing is. |
Ha. Quoting someone named @handrews from here :D
But will wait for your more thorough response to clarify :) |
@Julian thanks - it might be a few days before I have time to sort it out and explain (and make sure I'm actually right first). |
@handrews I know you're still doing what you can, but just a heads up, I'd like to clear a bunch more things out of the way (this and |
Determining where these test cases go involves determining two things:
What is
|
It seems like the new information in your comment, to summarize, is that spec authors intended (and agreed) that The second bit seems a bit shakier. There are a few lines in there that no one (well I) at least wasn't making, perhaps just to avoid doubt I'll list them, so we can focus on the other bit:
None of this was intended certainly. Though neither is precisely how you wrote the first option:
It is to me additional in the sense that the rest of the vocabulary is not enabled. It thereby differs in behavior from @gregsdennis / @Relequestual on the Keep in mind the suite's role isn't to decide just how pathological something is, nor whether such a thing is what we want to be true in future specs, just current ones -- in fact to me it was easier to think about this via If the answer is that that was the intention (that |
There's a lot going on this this issue, and I've completely lost track of who's arguing for what. That said, if someome created a meta-schema that didn't include the validation vocab and then included If that meta-schema also required (value of In short, I think the takeaway is that the language could be better. Personally I like the idea that vocabs declare keywords that are available for use, which implies keywords defined by missing vocabs are unavailable. |
There indeed is a lot of back and forth -- to summarize, the final question isn't the scenario you mentioned about a concrete implementation, it's about implementers and ones with some specific need or desire to add extra keywords, not your specific implementation's behavior (or mine or whatever) -- specifically: Is an implementer of JSON Schema allowed by the spec, in their own new implementation they're writing, to make their implementation always understand the Adding extra keywords beyond those given in a metaschema, and beyond
(I assume your line break was acknowledging this was a tangential, but yeah there's I believe no argument that non-vocabulary keywords are allowed today, it's explicitly in the spec, so whether or not I agree with your last line for future specs -- n.b. I do, if |
No, an implementation may not assume a keyword that is not defined in the vocabularies listed in the meta-schema. Section 8.1.2 explicitly forbids it:
A keyword in a vocabulary not listed in |
I don't think even Henry (who doesn't go as far as I do) agrees with that, but it could be I'm just really off. But keywords outside of vocabularies are explicitly allowed by the section both he and I cited previously, from §6.5:
To me the word "unrecognized" in your passage clearly does not apply to keywords the implementation indeed recognizes. What you're citing is that if I present an implementation with |
@Julian what I don't understand is why you are so dead-set on insisting that this particular configuration: an implementation that deliberately circumvents the intent of Is the test suite designed to test reasonable configurations or pathological ones? I'm defining a "pathological" implementation as one that exploits non-interoperable behavior to circumvent more clearly-defined behavior. The disabling of the validation vocabulary by omitting it from That is a choice that an implementation makes to go beyond the spec, into areas that are not interoperable. §6.5 makes it quite clear that this is outside of what is "normal" JSON Schema behavior: "Save for explicit agreement...", etc. The test suite is correctly happy to rely on this elsewhere. The (again, correctly) required test that ensures that There are quite a few configurations or implementation choices that are not explicitly forbidden by the spec but that the test suite. The spec's wording technically allows ignoring certain uses of Because if we go that far, then (as you and I have discussed), the core spec starts falling apart entirely. The normative language just isn't clear enough. Tons of things in the core spec lack normative language entirely. There is no normative language around what it means to "identify" a schema with a URI. None of this has to do with whether only vocabularies are allowed to add keywords. It has to do with how far implementations can go beyond what the spec states into murky, explicitly non-interoperable areas, and still expect to pass the test suite. You keep saying that you don't want the test suite to be making subjective decisions, but there is unquestionably a subjective decision going on here, where you are willing to allow the test suite to assume a reasonable configuration and set of implementation choices for some tests in the required suite, but seem dead-set on forbidding such an assumption for |
An implementation that notices that the validation vocabulary is disabled and then automatically enables a non-vocabulary keyword that takes its place is not interoperable. It is also clearly built to circumvent the normal behavior of Yeah, I forgot to explicitly exclude it because it never occurred to me that someone would try to do that. Previous versions of the spec, of course, did not talk about conditionally automatically enabling extension keywords to circumvent other required behavior, because there wasn't behavior to circumvent. So why is "conditionally automatically enable an extension keyword to circumvent the disabling of the validation vocabulary" the reading of the spec that we have to accommodate? Instead of "Yes it's possible to define |
Such a thing is again ridiculous, and not what I've said -- I again am focused strictly on an implementation where the keyword is always enabled, no matter what.
The absolute only thing I am focused on is what the spec means and intends, which includes indeed some room to pepper over stuff missing from the "letter" of the spec, but which does not mean the test suite is a way to enact new change -- the minute it's clear that's indeed what the spec authors (in this case you cited them) really meant and agree on, that's it. Here (and often) I represent some stubborn implementer who says it's not clear that the spec and its authors agree this isn't allowed. That is literally my most important role maintaining this repo, is to ensure the test suite isn't valueless or devalued because someone thinks it diverges from the specification, and I try to do it uniformly for any test.
Guarding against an implementation adding a keyword called
I haven't made any decision. I pointed out an area of lack of agreement first in the spec text based on my reading, and now between spec folks, starting with Jason, and then now Greg, who you'll forgive me for saying it so explicitly, is saying something very straightforwardly incorrect according to both the spec and the intention behind the spec. I then simply asked that if you all indeed agree this (== banning keywords present in the core vocabularies from behavior otherwise allowed) is the intention of the spec, and if that's indeed the case, that these can stay. |
This is not in any way a new change.
But that is not a thing that is possible with a standard keyword. It is either a standard keyword, and therefore under the control of the vocabulary system that was specifically designed to control such things, or it is not.
Jason wasn't involved in the PR adding
I'm tempted to just say yes to get the outcome that I want, but in terms of Greg's exact wording that would be dishonest. I expected people to be allowed to implement additional keywords outside of the vocabulary system. But the standard validation vocabulary keyword Therefore if we are truly testing standard implementations, then the only relevant On the other hand, if an implementation had an always-on non-vocabulary extension But |
You are asserting things that I've given possible alternate explanations for. I have no way of knowing which explanation is correct, as I myself didn't participate, and the spec doesn't say it. Your opinion is valuable, and it indeed weighs things. It immediately weighed things enough to have me believe
despite not applying to If they are on board, great, then regardless of the language of the spec, this behavior was what was intended, and the tests stay. If they misunderstood or disagree with the outcome, then it's not the test suite's position to make a call there, and it's something that should be resolved in the spec before it's here. This isn't very complicated, all I want to know is others understand and agree. |
That is not what I am saying. There is a difference between a straw man and us not understanding each other's arguments, and it is the latter that is happening here. I am trying to explain why the way you are looking at this is not the way that was intended, and that if we follow the way you are looking at it then we end up in place that does not work. I don't think your point ("can never be considered not part of a vocabulary") is an intentional straw man, but it's not my position.
They just cannot be both a vocabulary and non-vocabulary keyword at the same time, and only the vocabulary keyword is covered by the standard. Do you see how that is different from "can never be considered not part of a vocabulary"? |
So to answer your question directly, if an implementer does this, then they are not implementing the standard validation vocabulary. They are implementing something else that just so happens to look like it.
That differentiation proves that if the keyword still functions after the validation vocabulary is removed from This, I'm guessing, is why you read my arguments as straw men, because we fundamentally disagree on something here. The test suite tests implementations of the standard. There are things that are gray areas because they are not forbidden, but swapping out a standard keyword for a non-standard keyword violates the standard even if the mechanism used to do so is, in general, allowed, and even if the replacement happens to look enough like the standard to slip by the tests. |
This is because I (and I assume @gregsdennis and @Relequestual) never contemplated the possibility of someone substituting non-vocabulary keywords for standard keywords in a way that can't be turned off. In my case, I had no idea that always-on extension keywords were considered valid when it came to assessing conformance in the first place (Greg, Ben, were either of you aware of this?). Don't get me wrong- it's excellent QA thinking! From the perspective of my QA career I respect the thoroughness and reasoning. And I would have loved to have heard it 3 years and 8 months ago when we could have easily fixed the wording to make it beyond a doubt that this was not allowed, whether by exempting the standard vocabularies, or by forbidding always-on extension keywords, or whatever else. But it's difficult to prove now, because how do you prove that you intended to forbid something that you didn't think was possible in the first place? (By which I mean "I did not (and do not) think it is possible to have such a |
Okay... I think I see what's going on here. We have a (currently required) test: // schema
{
"$id": "https://schema/using/no/validation",
"$schema": "http://localhost:1234/draft2020-12/metaschema-no-validation.json",
"properties": {
"badProperty": false,
"numberProperty": {
"minimum": 10
}
}
}
// instance
{
"numberProperty": 1
} The expected outcome from this test is that since the validation vocab is explicitly excluded, @Julian is saying that the specification allows (via not explicitly disallowing) that a JSON Schema implementation may have an internal, always-on implementation of I would say that writing a meta-schema that explicitly excludes a vocab is a very intentional act, and an implementation that doesn't allow an author to do this (as its default behavior) should be considered non-conformant. The test suite should cover only what is required by the spec. So to show that this is in fact a requirement... From the opening statement for section 8.1.2,
it seems clear there exists an implication that keywords defined in a vocab that is not listed in Consider my Similarly, if a meta-schema excludes the validation vocab, it must be expected that an implementation ignore those keywords. The explicit exclusion of a vocabulary implies that its keywords MUST be ignored. This is not to say that an implementation can't be configured so that a keyword from an excluded vocab is "always-on." But it does mean that this cannot be the default behavior. |
Fair enough we can leave this here. Thanks for the input folks. |
Can/should we state in the specification that redefining any keyword that exists in the specification is strictly prohibited? e.g. in the example in the github thread, if someone wants to define new semantics for "minimum" in their implementation, and exclude the validation vocabulary from a metaschema, you need to pick a new name for that keyword, because "minimum" is taken. For that matter, I don't really like the idea of "always on" keywords that aren't defined by a vocabulary. We [that is, great minds not including me] invented the concept of vocabularies for a reason; having a keyword outside of that framework reduces interoperability to zero. ..but that might be a more controversial opinion? |
The vocabulary tests here are not strictly correct according to the spec, unless I'm missing something. They appear to assert that if the validation vocabulary isn't present, that an implementation must mark instances as valid even if the validation vocabulary says they are not -- but that's not the specified (or intended) behavior of
$vocabulary
as far as I know. Quoting §6.5:I.e. a schema author may not depend on support for a keyword or vocabulary they use but do not place in
$vocabulary
, but an implementation may indeed offer support for it and enable it, either by always enabling the vocabulary or because it has chosen to add a keyword called "minimum" whose behavior is precisely the same as the validation vocabulary's, and then enable it by default regardless of what's in$vocabulary
.When the
$vocabulary
keyword does have mandatory effect is in the converse -- where an implementation lacks support for a vocabulary and a schema author requires its use, the implementation may not ignore those keywords:from §8.1.2.
TL;DR, an implementation given this schema:
(with metaschema here) is indeed free to still apply the validation vocabulary, or to similarly define some behavior for the
minimum
keyword which makes instances like20
be invalid.In "today's test layout", the above means that these tests belong in
optional
, though given we have #561 on hold pending restructuring theoptional/
directory, perhaps we instead should remove them and do the same with these?CC @handrews (since I believe you confirmed the above interpretation previously, but just making sure) and @karenetheridge (since you added these looks like, in case you disagree).
The text was updated successfully, but these errors were encountered: