Add support to export flat json schemas

[Dogacel]

Motivation:

Nested JSON schemas might be hard to read. Especially if we want to introduce OpenAPI spec generation, the ability to generate the $defs block independently can be super useful.

Modifications:

• Add flat option to the json schema generation for method information.
• Add a method to generate $defs block for a service specification based on all structs.
• Slight refactoring to extract visited out-of-function signatures to the class body.
• Omit description if empty. (Gist is not up-to-date, but should be fine because unit tests verify).

Result:

• Closes Option to generate JSON schemas flattened #4743
• See generated schemas for grpc service here: https://gist.github.com/Dogacel/bfa94713eb3202d4a1f02f057ad1a066

[Dogacel]

The recursion was a little bit tough to write clearly on my end, I wonder how does it look to others.

[minwoox]

Hi!, I left a question. 😄

[minwoox]

So now, it seems like we add pathName which might contain a def with $ref.
However, is it not allowed to do that because of the recursion?

however, that a $ref referring to another $ref could cause an infinite loop in the resolver, and is explicitly disallowed.

https://json-schema.org/understanding-json-schema/structuring.html#id20

[Dogacel]

Ah good catch, let me think about this for some time.

[trustin]

Any updates, @Dogacel ?

[Dogacel]

I am still not sure how to implement this without using recursion. The explanation did not make sense to me. As there can be recursive definitions such as a family tree structure, a $ref should eventually reference itself or another ref.

[Dogacel]

This is a shower thought,

I think what they mean is the following in the example:

{
  "$defs": {
    "alice": { "$ref": "#/$defs/bob" },
    "bob": { "$ref": "#/$defs/alice" }
  }
}

In this case a ref refers to another ref. It doesn't mean it can't refer to another ref that has a ref. The rule is that body can't be a ref. I.e. following would be allowed:

{
  "$defs": {
    "alice": { "$ref": "#/$defs/bob" },
    "bob": { "properties": {"foo": { "$ref": "#/$defs/notAlice" } } } 
  }
}

alice can ref to bob, but bob can't ref to alice, because alice is basically a pointer, it doesn't have any info.

Does that make sense? @minwoox 😆

[minwoox]

The rule is that body can't be a ref. I.e. following would be allowed:

What if the schema is something like:

{
  "$defs": {
    "alice": { "$ref": "#/$defs/bob" },
    "bob": { "properties": {"foo": { "$ref": "#/$defs/alice" } } } 
  }
}

It doesn't mean it can't refer to another ref that has a ref.

I'm not sure about this. The child property of the referenced property might have a reference to the referencing property which causes recursion.
How about allowing referencing to a property that doesn't have any reference in its hierarchy or only has a reference to itself?

[Dogacel]

I'm not sure about this. The child property of the referenced property might have a reference to the referencing property which causes recursion.

This should be fine in my opinion. Let me try to prove it because validation should be done lazily. For example,

Let's use https://www.jsonschemavalidator.net to validate our schemas,

{
  "properties": {
    "foo": {
      "$ref": "#/$defs/alice"
    }
  },
  "$defs": {
    "alice": {
      "$ref": "#/$defs/bob"
    },
    "bob": {
      "$ref": "#/$defs/alice"
    }
  }
}

this is not working because it can't lazily evaluate a ref referring to another ref. But if we refer to a ref with a properties block such as,

{
  "properties": {
    "foo": {
      "$ref": "#/$defs/alice",
    }
  },
  "additionalProperties": false,
  "$defs": {
    "alice": {
      "$ref": "#/$defs/bob",
    },
    "bob": {
      "type": "object",
      "properties": {
        "bar": {
          "type": "object",
          "$ref": "#/$defs/alice"
        }
      },
      "additionalProperties": false
    }
  }
}

it is working. So, does this make sense in that context?

[Dogacel]

How about allowing referencing to a property that doesn't have any reference in its hierarchy or only has a reference to itself?

This is roughly the current approach, so it won't make sense to flatten schemas that way 😆

[minwoox]

But if we refer to a ref with a properties block such as,

I didn't know that it was acceptable. Then, let's do this in this way. Thanks! 😆

[Dogacel]

I know it has been a year however I had a second look into this PR and it LGTM 🙂

I would like to work on fixing the JSON suggestions for Well known types as described in the official documentation: https://protobuf.dev/programming-guides/proto3/#json

Please LMK if it is OK to merge this and for me to continue working on fixing the JSON schema problems.

@minwoox

[github-actions]

🔍 Build Scan® (commit: 9319c0c)

Job name	Status	Build Scan®
build-windows-latest-jdk-21	✅	https://ge.armeria.dev/s/ieoqg4575aadk
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/u2ji7mf425hxq
build-self-hosted-unsafe-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/32myz3y5mf2bs
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/xphzlfqnqqrjo
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/zwm6lady6mxnq
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/d5m2kwmnjodiu
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/z44cwfoiciclm
build-macos-12-jdk-21	✅	https://ge.armeria.dev/s/f7aepgyju7vbo

[trustin]

LGTM and sorry for taking so long until I review this PR, @Dogacel 😓

[Dogacel]

LGTM and sorry for taking so long until I review this PR, @Dogacel 😓

No problem, I kinda left it as it is last year because I wasn't planning to work on DocService related stuff anytime soon 🙂

[ikhoon]

Based on the generated gist, repeated and map are defined as defs which is awkward. What do you think of embedding repeated and map and referencing their elements if they are non-primitive types?

- "strings" : {
-  "description" : "",
-  "$ref" : "#/$defs/repeated<string>"
- }
+ "strings" : {
+  "description" : "",
+  "type": "array",
+  "items" : {
+    "description" : "",
+    "type" : "string"
+  }
+ }

Otherwise, is $defs/repeated<string> necessary to create an OpenAPI specification?

[Dogacel]

Let me check, I think what you suggested makes sense.

[trustin]

@Dogacel Did you have a chance to check it?

[Dogacel]

Coming back to this after another month (I don't understand how time flies by).

Concern is valid, I agree.

I have updated the code to include a new method called canReferenceAsDef. This method returns true if the type is another object but not array or map. So we are not creating weird references to primitives or at least trivial types.

Please take another look 😄