Json schema #4369

schoetbi · 2017-06-28T05:12:34Z

New command line option jsonschema
--jsonschema Generate Json schema.

to generate a json schema.

google#4360

current problems: - typenames are not correct - array types need to be deduced google#4360

Current problems: enum types not written correctly google#4360

google#4360

aardappel

Looks great! just a few nits.

aardappel · 2017-06-28T15:47:00Z

src/flatc_main.cpp

@@ -96,7 +96,12 @@ int main(int argc, const char *argv[]) {
      flatbuffers::IDLOptions::kPhp,
      "Generate PHP files for tables/structs",
      flatbuffers::GeneralMakeRule },
-    };
+   { flatbuffers::GenerateJsonSchema, "-S", "--jsonschema", "JsonSchema", true,


lets leave out the -S ?

aardappel · 2017-06-28T15:48:18Z

src/idl_gen_json_schema.cpp

+      return "number";
+    case BASE_TYPE_STRING:
+      return "string";
+    default: { return "?"; }


is ? a correct way to do this, or are we missing types?

aardappel · 2017-06-28T15:49:01Z

src/idl_gen_json_schema.cpp

+}
+
+template <class T>
+std::string GenFullName(const T *enum_def) {


Try to follow the style of the rest of the code-base, thus template on same line.

aardappel · 2017-06-28T15:49:12Z

src/idl_gen_json_schema.cpp

+  for (auto const &ns : nameSpaces) {
+    fullName << ns << "_";
+  }
+


no need for blank line, here and elsewhere

aardappel · 2017-06-28T15:50:53Z

src/idl_gen_json_schema.cpp

+std::string GenType(const Type &type) {
+  switch (type.base_type) {
+    case BASE_TYPE_VECTOR: {
+      std::stringstream typeline;


not that I mind too much, but doesn't seem using stringstream has any advantages here over using stting

I thought it would but you are right it gives no benefits. See here: http://quick-bench.com/Xh4YAery7pHuO_AIg_ElnqTc3RU

I will remove them

aardappel · 2017-06-28T15:52:19Z

src/idl_gen_json_schema.cpp

+        typeLine << "  \"" + prop->name + "\" : { " +
+                        GenType(prop->value.type) + " }";
+
+        if (&prop != &s->fields.vec.back()) {


this if-else can be simplified :)

aardappel · 2017-06-28T15:53:07Z

src/idl_gen_json_schema.cpp

+
+      auto props = s->fields.vec;
+      std::vector<flatbuffers::FieldDef *> requiredProperties;
+      std::copy_if(props.begin(), props.end(),


ooh fancy :)

aardappel · 2017-06-28T15:53:39Z

src/idl_gen_json_schema.cpp

+        code_ += "  }";  // close properties
+      }
+
+      if (&s != &parser_.structs_.vec.back()) {


aardappel · 2017-06-28T15:54:48Z

tests/monster_test.schema.json

+"definitions": {
+
+"MyGame_OtherNameSpace_FromInclude" : {
+"type" : "string",


maybe nice to add some indentation, for readability?

aardappel · 2017-06-28T15:55:41Z

todo.md

@@ -0,0 +1,11 @@
+# TODOs for json schema generation


I don't think we want a todo in the root. Can you instead add this to a github issue?

I planned to remove as soon as everything is done. Any thoughts on the enum with bit_flag problem? What is expected to be in the json. The name of the enum (=string) or a number? String is only possilbe when one flag is true so I assume that a number should be the right thing?

bit_flags should be handled just like regular enums (they are numbers). all bit_flags does is assign those numbers differently

aardappel · 2017-06-30T15:43:56Z

Note you have some CI errors to address.

- removed command line arg -S - removed new lines - fixed codestyle in template functions - removed usage of stringstream - idented json schema google#4360

aardappel · 2017-07-03T17:08:40Z

src/idl_gen_json_schema.cpp

@@ -53,8 +53,9 @@ std::string GenNativeType(BaseType type) {

 template <class T> std::string GenFullName(const T *enum_def) {
  std::string fullName;
-  auto nameSpaces = enum_def->defined_namespace->components;
-  for (auto const &ns : nameSpaces) {
+  std::vector<std::string> nameSpaces = enum_def->defined_namespace->components;


Why is this making a copy? you can still use auto &

Also, use snake_case instead of camelCase

aardappel · 2017-07-03T17:09:12Z

src/idl_gen_json_schema.cpp

-  auto nameSpaces = enum_def->defined_namespace->components;
-  for (auto const &ns : nameSpaces) {
+  std::vector<std::string> nameSpaces = enum_def->defined_namespace->components;
+  for (int nsindex = 0; nsindex < nameSpaces.size(); nsindex++) {


Why not for (auto it = namespaces.begin(); ... ?

Was auto in vs2010? I do not have a copy installed.

aardappel · 2017-07-03T17:09:43Z

src/idl_gen_json_schema.cpp

-      for (auto &enumval : e->vals.vec) {
+
+      for (int valindex = 0; valindex < e->vals.vec.size(); ++valindex) {
+        EnumVal *enumval = e->vals.vec[valindex];


use auto where possible

google#4360

aardappel · 2017-07-05T17:38:52Z

The CI runs through VS2010, so if it is happy, you know it works there. And yes, VS2010 supports auto and a partial set of other C++11 features.

…n compile

…m-members

aardappel

Looking good, some minor comments

aardappel · 2017-07-07T16:53:11Z

src/idl_gen_json_schema.cpp

+    case BASE_TYPE_STRING:
+      return "string";
+    default: 
+      std::cerr << "GenNativeType: Invalid base type " << type << "\n";


There is no code anywhere else that outputs to cerr, so this is not a good error handling mechanism. It be better to simply handle all types in some way. What happens with e.g. BASE_TYPE_STRUCT, which is equivalent to a JSON object?

It should never go there. So I removed the cerr part and return ""

aardappel · 2017-07-07T16:53:38Z

src/idl_gen_json_schema.cpp

+
+template <class T> std::string GenFullName(const T *enum_def) {
+  std::string full_name;
+  std::vector<std::string> name_spaces = enum_def->defined_namespace->components;


Why is this vector copied?

aardappel · 2017-07-07T16:55:22Z

tests/monster_test.schema.json

@@ -0,0 +1,120 @@
+{
+"$schema": "http://json-schema.org/draft-04/schema#",


indent these 2 lines?

aardappel · 2017-07-07T16:56:06Z

tests/monster_test.schema.json

+
+"MyGame_Example_Test" : {
+  "type" : "object",
+  "description" : "",


if description is empty, should it maybe be left out to reduce clutter? Same of any other fields that are optional?

aardappel · 2017-07-10T15:05:54Z

Thanks!

schoetbi · 2017-07-10T16:15:24Z

Was a pleasure.

schoetbi added 11 commits June 23, 2017 09:35

Added empty generator for json schema (idl_gen_json_schema.cpp)

2b79e11

google#4360

JsonSchemaGenerator: output of tables implemented

5a4e0f8

current problems: - typenames are not correct - array types need to be deduced google#4360

JsonSchemaGenerator: Corrected generation of typenames

85edf5b

Current problems: enum types not written correctly google#4360

JsonSchemaGenerator: Added generation of enum types

7a73168

google#4360

idl_gen_json_schema.cpp: Write required properties to schema

352afa9

google#4360

idl_gen_json_schema.cpp: Export Types including namespace

16e199e

google#4360

idl_gen_json_schema.cpp: Fixed Json format

146107f

google#4360

idl_gen_json_schema.cpp: Formatted according to google code style

8b6ce58

google#4360

Checked in monster_test.bfbs with changes from master

323294e

Added idl_gen_json_schema.cpp in CMakeLists.txt

8ea50de

generate_code.bat: Added generation of json schema

453e730

schoetbi force-pushed the jsonSchema branch from f93ee8c to bff2451 Compare June 28, 2017 06:06

Added todo.md

7d868a8

schoetbi force-pushed the jsonSchema branch from bff2451 to 7d868a8 Compare June 28, 2017 06:09

generate_code.sh: Added generation of json schema

05aee5e

aardappel requested changes Jun 28, 2017

View reviewed changes

schoetbi added 2 commits July 3, 2017 14:04

Addressed some review issues

00100e3

- removed command line arg -S - removed new lines - fixed codestyle in template functions - removed usage of stringstream - idented json schema google#4360

removed auto in idl_gen_json_schema.cpp

c1ded2e

aardappel reviewed Jul 3, 2017

View reviewed changes

schoetbi added 2 commits July 4, 2017 07:17

idl_gen_json_schema.cpp: changed iterator declarations to auto

915e277

google#4360

deleted todo.md

61bc633

schoetbi added 2 commits July 6, 2017 06:25

idl_gen_json_schema.cpp: Removed keyword "override" so that vs2010 ca…

4f3a461

…n compile

idl_gen_json_schema.cpp: switch statement in GenType handeles all enu…

1ac0859

…m-members

aardappel reviewed Jul 7, 2017

View reviewed changes

schoetbi added 4 commits July 10, 2017 07:08

idl_gen_json_schema.cpp: Removed cerr output

4e600de

idl_gen_json_schema.cpp: Avoid vector copying

4875f3b

idl_gen_json_schema.cpp: Fixed identation of json schema output

90e976e

idl_gen_json_schema.cpp: Do not output empty descriptions

2d2fb87

aardappel approved these changes Jul 10, 2017

View reviewed changes

aardappel merged commit f202041 into google:master Jul 10, 2017

schoetbi deleted the jsonSchema branch July 11, 2017 06:14

mpe85 mentioned this pull request Feb 26, 2019

[FEATURE REQUEST] Support flatbuffers schema files asyncapi/spec#163

Closed

W1M0R mentioned this pull request Aug 7, 2020

JSON schema output does not implement integer type correctly #6066

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Json schema #4369

Json schema #4369

schoetbi commented Jun 28, 2017 •

edited

Loading

aardappel left a comment

aardappel Jun 28, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

schoetbi Jun 29, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

aardappel Jun 28, 2017

schoetbi Jun 29, 2017

aardappel Jun 30, 2017

aardappel commented Jun 30, 2017

aardappel Jul 3, 2017

aardappel Jul 3, 2017

aardappel Jul 3, 2017

schoetbi Jul 3, 2017

aardappel Jul 3, 2017

aardappel commented Jul 5, 2017

aardappel left a comment

aardappel Jul 7, 2017

schoetbi Jul 10, 2017

aardappel Jul 7, 2017

aardappel Jul 7, 2017

aardappel Jul 7, 2017

aardappel commented Jul 10, 2017

schoetbi commented Jul 10, 2017

		@@ -0,0 +1,120 @@
		{
		"$schema": "http://json-schema.org/draft-04/schema#",

Json schema #4369

Json schema #4369

Conversation

schoetbi commented Jun 28, 2017 • edited Loading

aardappel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aardappel commented Jun 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aardappel commented Jul 5, 2017

aardappel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aardappel commented Jul 10, 2017

schoetbi commented Jul 10, 2017

schoetbi commented Jun 28, 2017 •

edited

Loading