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

Incomplete basic output #403

Open
BrynCooke opened this issue Jan 6, 2023 · 2 comments
Open

Incomplete basic output #403

BrynCooke opened this issue Jan 6, 2023 · 2 comments

Comments

@BrynCooke
Copy link

BrynCooke commented Jan 6, 2023

When creating basic output sometimes the results look incomplete. For example:

 let schema = JSONSchema::compile(&json!({
          "$id": "https://example.com/arrays.schema.json",
          "$schema": "https://json-schema.org/draft/2020-12/schema",
          "type": "object",
          "properties": {
            "vegetables": {
              "type": "array",
              "description": "Desc1",
              "items": { "$ref": "#/$defs/veggie" }
            }
          },
          "$defs": {
            "veggie": {
              "type": "object",
              "required": [ "veggieName", "veggieLike" ],
              "properties": {
                "veggieName": {
                  "description": "Desc2",
                  "type": "string"
                },
                "veggieLike": {
                  "description": "Desc3",
                  "type": "boolean"
                }
              }
            }
          }
        }))
        .unwrap();
        if let BasicOutput::Valid(output) = schema
            .apply(&json!({
                "vegetables":[{"veggieName": "carrot", "veggieLike": true}]
            }))
            .basic()
        {
            for unit in output {
                println!("{}", unit.instance_location());
            }
        }
    

Output:

/vegetables
/vegetables/0

Expected output:

/vegetables
/vegetables/0
/vegetables/0/veggieName
/vegetables/0/veggieLike
@BrynCooke
Copy link
Author

I updated the example and did a little investigation.

I think the issue is that RefValidator doesn't contain a custom implementation of apply, so any PartialApplication information from down stream is discarded.

BrynCooke pushed a commit to apollographql/router that referenced this issue May 2, 2024
We can't use the json schema to redact the config anymore because annotations are not surfaced across schema refs:

Stranger6667/jsonschema-rs#403
BrynCooke added a commit to apollographql/router that referenced this issue May 3, 2024
Fix #5003

This reworks the json schema generation for the config so that it is reduced in size from approx 100k lines to jsut over 7k.

The fix involves three things:

* Enable references on json schema generation. This got disabled in the past because there were issues with the generated references, but by adding a schema visitor we can work around this.
* Adjust the schema generation for Extendable and Conditional. These previously relied on the scheme not using references.
* Modify orbiter metrics to redact only based on the properties in the schema rather than on validation metadata as this is not possible when using schema refs: Incomplete basic output Stranger6667/jsonschema-rs#403
Geal pushed a commit to apollographql/router that referenced this issue May 6, 2024
Fix #5003

This reworks the json schema generation for the config so that it is reduced in size from approx 100k lines to jsut over 7k.

The fix involves three things:

* Enable references on json schema generation. This got disabled in the past because there were issues with the generated references, but by adding a schema visitor we can work around this.
* Adjust the schema generation for Extendable and Conditional. These previously relied on the scheme not using references.
* Modify orbiter metrics to redact only based on the properties in the schema rather than on validation metadata as this is not possible when using schema refs: Incomplete basic output Stranger6667/jsonschema-rs#403
@Stranger6667
Copy link
Owner

Sorry for the delay, with the current approach, it is a bit complicated as annotations are bound to the schema. Not sure if it would be possible to keep the references, but maybe making annotations own their data would be a better way to go anyway.

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

No branches or pull requests

2 participants