Add RPC data to bfbs binary schema format

[jobol]

The sample bfbs2json was ignoring services present in BFBS files generated by flatc. After this change, it outputs the JSON output:

"service": [ { "name": "", "calls": [ { "name": "", "request": , "response": } ... ] } ... ]

(optional attributes not shown)

[mikkelfj]

I don't think that FlatCC generates RPC fields in the bfbs file it exports: https://github.com/dvidelabs/flatcc/blob/master/src/compiler/codegen_schema.c

That would be more relevant to fix. bfbs2json is just a sample application.

[jobol]

@mikkelfj I fully agree and that is the next step.

At the moment I use flatc to generate the bfbs

My next step is to add generation of it in flatcc but i wanted first to check that it is of interest.

As you wrote "That would be more relevant to fix" i presume that it is of interest. I am going to make soon if you agree.

[mikkelfj]

If you provide an acceptable PR for bfbs support I'll merge both but having this alone doesn't make much sense.

FlatCC doesn't really support RPC in that it doesn't generate RPC code, but parses it in the schema, and it ought to support it in the bfbs file. I might add some support at some point, but it would more like be for MQTT 5.0 rather than gRPC which is extremely tool heavy.

[mikkelfj]

Please observe https://github.com/dvidelabs/flatcc/blob/master/test/reflection_test/reflection_test.c

[jobol]

I did it then

my intention is to generate code for RPC using templating (I'm using https://gitlab.com/jobol/mustach) while using flatcc for accessing/composing messages

in my case I use the following pipe to avoid parsing complications: SCHEMA FBS -> SCHEMA BINARY BFBS using flatcc BINARY BFBS -> JSON using sample JSON processing using jq JSON -> code using mustach

I'll check the link...

[jobol]

do you have an idea of how to change reflection_test.c ? I can just enumerate the services and their ccall

but tomorow now...

[mikkelfj]

On workflow: Good idea to use bfbs for this, but you shouldn't rely on the bfbs2json sample since it isn't intended to be production quality. It's (ideally) better to print the bfbs to json using the generated json printer for the bfbs schema. However, that output is not very friendly due shared links in the bfbs buffer. The FB community is discussing a future version of bfbs for code generation that is also more json friendly, but that is long term. So in praxis perhaps your workflow works for you as long you take responsibility for the json generation.

If you are using Javascript, you could also access bfbs directly as a FlatBuffer.

On test:Just a smoke test to see expected fields are present. Not sure if there is a schema file that needs to be updated.

[jobol]

I added a simple print of service and call names. I can still add the print of request and response typename if you want.

Thank you for your advises. I hadn't thought to use json printer to print bfbs files. I will try it now.

[jobol]

FYI replacing the sample bfbs2json by use --jsonprint generated stuff works well and produce a better result. So I am switching to your idea.

[mikkelfj]

I added a simple print of service and call names. I can still add the print of request and response typename if you want.

I'm not sure that I follow: is this referring to the test or the bfbs2json?

Since you have opted to use the JSON printer, I don't really mind if we entirely skip bfbs2json, but if we update it, it should be complete.

As to tests: It appears that you are just printing debug statements. There should be if statements that can fail if the content is unexpected as is done for other tests. This relies on specific buffer content and I suspect that current content does not include RPC, which we should look into then.

[mikkelfj]

https://github.com/dvidelabs/flatcc/blob/b1927fe9cd639b874bf953dc994349f1d8bc102c/test/reflection_test/reflection_test.sh#L12

[mikkelfj]

It seems there is an RPC service in the test data

https://github.com/dvidelabs/flatcc/blob/b1927fe9cd639b874bf953dc994349f1d8bc102c/test/monster_test/monster_test.fbs#L315

[jobol]

(was referring to test, not bfbs2json)

There is an rpc_service content in the monster schema used by test reflection: https://github.com/dvidelabs/flatcc/blob/master/test/monster_test/monster_test.fbs#L315

rpc_service MonsterStorage {
  Store(Monster):Stat (streaming: "none");
  Retrieve(Stat):Monster (streaming: "server", idempotent);
}

Have you an idea of what to test? a name 'Store' ?

[mikkelfj]

I haven't looked at the details, but something like a known service should be present, and it should be possible to retrieve the information available in the schema for that service. We don't need to test everything in the schema, just that the basic RPC concept is functional.

[mikkelfj]

E.g. fine to check that there is a Store call and its attributes, and the there are two calls, but don't bother testing all data on Retrieve.

[mikkelfj]

Very good work. We only need to check the value of the streaming attribute is "none".

[jobol]

testing to none? done

[mikkelfj]

Almost, but I'm not sure that strcmp takes kindly to a potential null string.

[mikkelfj]

The KeyValue attribute vector is not sorted it seems - this is from master branch: https://github.com/dvidelabs/flatcc/blob/b1927fe9cd639b874bf953dc994349f1d8bc102c/src/compiler/codegen_schema.c#L402

And this is from your source: k = reflection_KeyValue_vec_find(reflection_RPCCall_attributes(Call), "streaming");

This only works on sorted vectors. _scan call can be used instead of _find for linear search but it is probably the vector that should be sorted.

I'm not sure if bfbs is properly exporting attributes since it was added late to the reflection schema, but it doesn't sort them, as mentioned above.

There is a broader problem because sorting will loose information, notably if there are duplicate keys, the sort is not stable in flatcc (because heap sort without external memory cannot be stable).

Since the sort_objects function was written, FlatCC has been generating a global sort function that can be called instead. It should also fix sorting attributes. Expect, as I recall, I think an extra attribute "sorted" is needed in the reflection schema for this to work.

All in all this is a separate issue. The simplest solution is to use scan in this commit.

[mikkelfj]

For reference, here is an example of using the generated sort function https://github.com/dvidelabs/flatcc/blob/b1927fe9cd639b874bf953dc994349f1d8bc102c/test/monster_test/monster_test.c#L2021

[jobol]

I replaced _find by _scan. I also checked the not nullity of the string. thank you for your reviews.

[mikkelfj]

Fine, ready to merge?

[jobol]

Ready to merge? Yes on my side. It is up to you.

[mikkelfj]

Thanks for contributing this - it's very useful have RPC in binary schema even if (or especially because) FlatCC does not export RPC logic on its own.