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

#1622 - Clarify spec wrt readOnly and writeOnly in referenced schemas #1638

Closed
wants to merge 1 commit into from

Conversation

tedepstein
Copy link
Contributor

Make it clear that readOnly and writeOnly are allowed in any Schema Object, but only effective when applied (directly or by reference) to a property subschema.

Adds similar language to the xml property, addressing Mike's comment.

Fixes #1622.

Make it clear that `readOnly` and `writeOnly` are allowed in any Schema Object, but only effective when applied (directly or by reference) to a property subschema.

Adds similar language to the `xml` property, addressing [Mike's comment](OAI#1622 (comment)).

Fixes OAI#1622.
@MikeRalphson
Copy link
Member

Just reminding myself really that because we're changing language including MAY, my preference was to retarget this for v3.1.0 to avoid having the possibility of different v3.0.x validators behaving differently depending on the version of the spec they were developed against.

@tedepstein
Copy link
Contributor Author

tedepstein commented Sep 20, 2018

@MikeRalphson,

Just reminding myself really that because we're changing language including MAY, my preference was to retarget this for v3.1.0 to avoid having the possibility of different v3.0.x validators behaving differently depending on the version of the spec they were developed against.

I think that possibility already exists because the current language, "relevant only for Schema properties," is ambiguous. Someone writing a validator could reasonably interpret this to mean that readOnly, writeOnly, or xml are disallowed in top-level schemas. But the spec doesn't say that explicitly, so someone else could reasonably think they're allowed.

The problem extends beyond validators. We already have at least two known cases of parsers that discard readOnly and writeOnly from top-level schemas, and at least one parser (KZOP) that preserves them.

If we disambiguate in 3.0.1, it clearly puts some implementations out of compliance and gives those project owners the clarity they need to fix the problem.

If we defer to 3.1, we have divergent 3.0 implementations, with no clear agreement as to which of these is correct. And we leave open the possibility that new implementations will be added, out of compliance with the intended behavior, compounding the problem.

@handrews
Copy link
Member

As I just noted in #1622: Since we seem very likely to move to JSON Schema 2019-09 in OAS 3.1, this problem will go away since (as noted above) the issue is now addressed in the JSON Schema spec. We haven't had anyone complain about how we specified it so it seems to be working out OK.

Of course, if we want this PR to go into 3.0.3 then the JSON Schema 2019-09 stuff is irrelevant.

@MikeRalphson
Copy link
Member

@tedepstein I honestly think we're on a hiding to nothing trying to specify previously ambiguous behaviour within a patch release. In any case this PR is open against an already-released version.

My preferred way forward would be to take only the xml object change and re-raise against v3.1.0-dev due to @handrews observation that readOnly and writeOnly will be better specified in 3.1. I'm happy to do this, just let me know.

@tedepstein
Copy link
Contributor Author

@MikeRalphson , that solution sounds fine to me.

@MikeRalphson MikeRalphson self-assigned this Feb 11, 2020
@tedepstein
Copy link
Contributor Author

@MikeRalphson I will open a new PR to go against v3.0.3-dev branch, and drop the change to readOnly & writeOnly, keeping only the xml property change. Once that's done, I'll close this PR.

@MikeRalphson
Copy link
Member

Because of #1638 (comment) and the fact that the v3.0.3 'merge window' will be closing on Tuesday, my preference would still be to target v3.1 as per #2130 (if the TSC agree its a patch level change, then of course v3.0.4 is also a possibility).

@tedepstein
Copy link
Contributor Author

@MikeRalphson, sorry, just re-read your comment. And I didn't realize that we had set a deadline for Tuesday. I'll open the xml clarification against 3.1, and we can reassess in that scope to see if any further clarification is needed w.r.t. readOnly and writeOnly.

@darrelmiller
Copy link
Member

@tedepstein Do you still want to try and get this into 3.1? We are closing in fast on resolving the issues for 3.1-rc0

@webron
Copy link
Member

webron commented Apr 23, 2020

Closing the PR in order to be able to delete the branch (which has already been published). If needed, please resubmit the PR on the latest branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants