Skip to content

adding specification enhancement for additionalProperties 2020-12 #726

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

Merged
merged 6 commits into from
Apr 6, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions test-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@
"type": "array",
"items": { "$ref": "#/$defs/test" },
"minItems": 1
},
"specification":{
"description": "Location of the test case in the specification",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not just "the" specification, given it can either be the JSON Schema specification or some other specification. Probably we should elaborate here with some guidance for how to use this, specifically:

Suggested change
"description": "Location of the test case in the specification",
"description": "A reference to a specification document which defines the behavior tested by this test case. Typically this should be a JSON Schema specification document, though in cases where the JSON Schema specification points to another RFC it should contain *both* the portion of the JSON Schema specification which indicates what RFC (and section) to follow as *well* as information on where in that specification the behavior is specified.",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got this 👍

"type": "object",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect an array is more correct here actually , which would allow referencing multiple locations, perhaps even within one spec.

(Or if we want to get fancy at the expense of making it harder to parse we could do oneOf: [{items: spec}, spec] and allow either a spec or else an array of specs. But always requiring an array seems fine to me personally.)

Copy link
Contributor Author

@suprith-hub suprith-hub Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concluding this, which of them is better to be chosen:

  1. "specifications": [ { "core": "6.7.1", "quote": "Lorem" }, { "core": "7.4.3" }, { "validation": "12.2.1" } ]
"specification": [ 
          { "core" : [
              {"section": "6.7.1", "quote": "Lorem" },  
              {"section": "7.4.3", "quote": "LoremEpsom" } ] 
          }, 
          { "validation": [ {"section": "12.2.1", "quote": "Example quote" } ] }
          ]
  1. specification is an object:
"specification": {
          { "core" : [
              {"section": "6.7.1", "quote": "Lorem" },  
              {"section": "7.4.3", "quote": "LoremEpsom" } ] 
          }, 
          { "validation": [ {"section": "12.2.1", "quote": "Example quote" } ] }
          }

I felt first one is simpler, but doesn't mention its value is a "section" explicitely

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always having an array is easier to handle in some languages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concluding this, which of them is better to be chosen:

I'd go with number 1 personally.

Always having an array is easier to handle in some languages.

Makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great

"minProperties": 1,
"properties":{
"jsonschema": {"type": "string"},
"rfc": {"type": "string"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these both URIs?

Copy link
Contributor Author

@suprith-hub suprith-hub Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are URL's for a particular section in the JSON Schema Spec. They will redirect to the location where test-case is more relevant.
They are supposed to be, can add regex if pattern matching for scheme is required

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the structure of "specification" property supposed to be something else

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. These are supposed to be informative comments. It's not really anything that needs to be linked to. We really just need some sort of reference. I think just the spec name and section number will suffice.
    "additionalProperties":
    {
      "type": "string",
      "pattern": "^[0-9](\.[0-9])*$"}
    }
    
    which would give something like
    {
      "core": "8.5.2",
      "validation": "6.3"
    }
    
  2. What are "jsonschema" and "rfc" intended to refer to?

Copy link
Contributor Author

@suprith-hub suprith-hub Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.. Actually each section of the draft has many paragraphs, and each paragraph has unique URI's.
like, https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-01#section-10.3.2.3-2 URI will redirect to paragraph 2 of "additionalProperties" section.
So, I thought why not that can be made use. And rfc -> too an URI for any section of RFC the description of the test located in.

  1. Wouldn't it make test cases bigger. And also we can provide more precise location using URLs. So, why not URI's?
  • as we may need to provide multiple uris?
  • usability of
{
 "core": "8.5.2",
 "validation": "6.3"
}
```  this will be greater??

Copy link
Member

@gregsdennis gregsdennis Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-01#section-10.3.2.3-2

If you look at this format it has a common pattern.

  • https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-01#section-
  • <section>-<para>

I think it'd be better just to include the last part of that, the section and paragraph. If desired, tooling can construct the URL. The spec name (core/validation) indicates which document were referring to.

Again, this is just a reference for documentation purposes. It doesn't need to be functional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally got that. We can have spec name: core/validation and for rfc: rfc3456/rfc4567 or ...
Then we can have---> section
---> para
So, mostly 3-4 indicators. Right? ❤️‍🔥

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Julian what is the philosophy of tests for requirements like "behaves per X spec, section Y"? Do we include tests checking behavior against other specs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like why would we include that info? Yeah we definitely have lots of that, see e.g. all the format tests, all of which essentially are testing behavior specified in some other specification and generally RFC.

But def fine to leave it out to start I think.

(I haven't read the rest of the thread yet!)

Copy link
Contributor Author

@suprith-hub suprith-hub Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the example structure for a test-case be:

{
    "specification":{
        "jsonschema-validation":{
              "section": "6.3.1",
              "para": "3"
        },
        "rfc8259":{
            "section": 7
        },
        // "jsonschema-core":{...}
    }
} 

and because there may be more than one place in spec, we can make for example:

        "jsonschema-validation":[{
              "section": "6.3.1",
              "para": "3"
        }, 
        {
            "section": "6.3.1",
             "para": "4"
        }]

can keep it such that "jsonschema-validation" can take both array and object..

And instead of asking can I make as many commits as possible and then take comments?

},
"additionalProperties": false
}
},
"additionalProperties": false
Expand Down
Loading