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

Schema set in "AdditionalProperties" instead in "Properties" #638

Closed
SParekh opened this issue Mar 18, 2020 · 5 comments
Closed

Schema set in "AdditionalProperties" instead in "Properties" #638

SParekh opened this issue Mar 18, 2020 · 5 comments
Assignees
Labels

Comments

@SParekh
Copy link

SParekh commented Mar 18, 2020

Describe the bug

Schema is in the "additionalProperties" keyword rather than the "properties" keyword and type set to "object" instead of "array".

Serialziation/Deserliazation of data works as expected but

  1. Code generation tools referencing this schema produce types called “Anonymous”, “Anonymous1"....
  2. Just inconsistent intent as other corresponding properties have schema defined as expected. See Other comments

E.g responsible-parties

"responsible-parties" :

has no "properties" but "additionalProperties" has "properties" to schema #/definitions/responsible-party.

"$ref" : "#/definitions/responsible-party" },

Additional definitions with same issue
responsible-parties

"responsible-parties" :

set-parameters

{ "parameter-settings" :

Who is the bug affecting?

Anyone using code generation tools to generate complex type from Json Schema

Expected behavior (i.e. solution)

If intent was to have this contain ONLY additionalProperties of type "responsible-party", this should have been represented as

 "responsible-parties" : 
          { "type" : "array",
            "minItems" : 1,
            "items" : 
            { "$ref" : "#/definitions/responsible-party" } }

Other Comments

Other corresponding definitions have expected schema (i.e. schema defined in "properties" and not "additionalProperties"

roles


alterations*

@SParekh SParekh added the bug label Mar 18, 2020
@david-waltermire david-waltermire self-assigned this Mar 27, 2020
@wendellpiez
Copy link
Contributor

This is very helpful.

Work refactoring the schema production (both XSD and JSON Schema) is happening in usnistgov/metaschema#39.

Also: these specifications call for unit tests, being worked on in usnistgov/metaschema#18.

@david-waltermire
Copy link
Contributor

After looking into this, I don't think there is a bug here. Atleast the JSON Schema productions are correct as they are intended.

In the OSCAL formats, we have cases where some objects with document-unique identifiers are represented as follows based on the (SSP Schema](https://github.com/usnistgov/OSCAL/blob/master/json/schema/oscal_ssp_schema.json):

"responsible-roles": {
  "provider": {
    "party-ids": [
      "logging-server-vendor"
    ]
  },
  "asset-owner": {
    "party-ids": [
      "enterprise-asset-owners"
    ]
  },
  "asset-administrator": {
    "party-ids": [
      "enterprise-asset-administrators"
    ]
  }
}

In this case we can't provide specific property names. since these are based on the identifiers.

The property is defined as follows on line 1524 of the SSP JSON schema:

"responsible-roles" : 
          { "type" : "object",
            "minProperties" : 1,
            "additionalProperties" : 
            { "allOf" : 
              [ 
                { "type" : "object",
                  "$ref" : "#/definitions/responsible-role" },
                
                { "not" : 
                  { "type" : "string" } } ] } },

The property names for the responsible-roles object are the identifiers of each unique role. The type of each associated object is defined by reference to #/definitions/responsible-role.

This does create a binding problem, since each map entry will be an anonymous type.

We can:

  1. Keep it like it is; you can use some application logic to sort this out.
  2. Avoid the use of this property identifier key feature.

In the interest of weighing what to do, I am interested to hear if others are having this problem. Anyone have any insights?

@SParekh
Copy link
Author

SParekh commented Apr 27, 2020

Thanks for your feedback.

Just to get some clarity, is this use case different from "#/definitions/role" where we accept "id" (E.g. "id": "legal-officer") and details around the role ?

Instead of anonymous type, can this object "#/definitions/responsible-party" also not contain a property called "id" which can be of type string with set to "provider", "asset-owner", "asset-administrator" and so forth.

@wendellpiez
Copy link
Contributor

Hi @SParekh cc @david-waltermire-nist,

As modeled, a responsible-party will appear with a key that is interpreted as a value referencing a role by its id value. Another way of looking at it is that the responsible-party/role-id property is implicit, represented as the key of the object. Alternatively, you could say that the key in the JSON is overloaded: it is both a key, and a reference (to the role to which the responsible-party is being bound).

See https://pages.nist.gov/OSCAL/documentation/schema/implementation-layer/component/json-schema/#responsible-party - note that in the current design, a responsible-party belongs to only one role but it can be linked to multiple party objects by party-id.

It could be that this design is confusing, in which case we should consider (a) refactoring to make it more obvious how it works, and/or (b) documenting the relation.

If you think we should consider doing this, would you mind making a new Issue? as it is not a bug in the schema production as much as a modeling/design question.

Meanwhile thanks indeed for your interest and diligence.

@SParekh
Copy link
Author

SParekh commented May 4, 2020

Further reviewing the documentation and usage under "metadata" and "component", I agree that JsonSchema complies with the documentation.

I appreciate team reviewing this issue and adding clarification. For now, we are going to work around this by tweaking generation logic on our end.

@SParekh SParekh closed this as completed May 4, 2020
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

3 participants