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

Stream is lacking outer { } if streamed object is an array #60

Open
pljakobs opened this issue Jan 7, 2025 · 8 comments
Open

Stream is lacking outer { } if streamed object is an array #60

pljakobs opened this issue Jan 7, 2025 · 8 comments

Comments

@pljakobs
Copy link
Contributor

pljakobs commented Jan 7, 2025

I use the schema below to store application data.
The controllers array is filled by the firmware with controllers found using mDNS, I use the streaming method to pass it to the http API, but it turns out that the stream just sends the "naked" array, to be a correct json structure, I believe that it would have to be wrapped in {}

code

AppData::Controllers controllers(*app.data);
	auto controllersStream = controllers.createExportStream(ConfigDB::Json::format);
	response.sendDataStream(controllersStream.release(), MIME_JSON);

result:

[{"cid":10966439,"last-seen":213,"ttl":120,"ip":"192.168.29.41","name":"Test1","model":"esp8266"},{"cid":15603867,"last-seen":213,"ttl":120,"ip":"192.168.29.113","name":"LED_TvU","model":"esp8266"},{"cid":-1204852831,"last-seen":3,"ttl":-1,"ip":"dhcp","name":"32c3 Test","model":"esp32c3"},{"cid":12742997,"last-seen":221,"ttl":120,"ip":"192.168.29.115","name":"LED_So2","model":"esp8266"}]

I think it would make sense to add the outer curlies, and the actual array name, so in my case the full

{
  "controllers" : [
    {...},
    {...}
  ]
}
{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "store":true,
  "properties": {
    "last-color":{
      "$ref":"defs/$defs/hsvct"
    },
    "presets": {
      "type": "array",
      "store":true,
      "items": {
        "type": "object",
        "properties": {
          "name": {
            "type": "string"
          },
          "color": {
            "$ref":"defs/$defs/color"
          },
          "ts": {
            "type": "integer"
          },
          "id":{
            "type":"integer"
          },
          "favorite":{
            "type":"boolean",
            "default":false
          }
        }
      }
    },
    "controllers": {
      "type": "array",
      "store":true,
      "items": {
        "type": "object",
        "properties": {
          "cid":{
            "type":"integer"
          },
          "last-seen": {
            "type": "integer"
          },
          "ttl":{
            "type":"integer"
          },
          "ip": {
            "type": "string"
          },
          "name": {
            "type": "string"
          },
          "model":{
            "type":"string"
          }
        }
      }
    },
    "scenes":{
      "type":"array",
      "store":true,
      "items":{
        "type":"object",
        "properties":{
          "name":{
            "type":"string"
          },
          "settings":{
            "type":"array",
            "items":{
              "type":"object",
              "properties":{
                "controller_id":{
                  "type":"integer"
                },
                "color": {
                  "$ref":"defs/$defs/color"
                }
              }
            }
          }
        }
      }
    }
  }
}
@pljakobs
Copy link
Contributor Author

pljakobs commented Jan 7, 2025

I can make this work in the frontend, it just doesn't seem right, not quite sure what the json standards say here, but I would prefer the full array.

@mikee47
Copy link
Owner

mikee47 commented Jan 7, 2025

@mikee47
Copy link
Owner

mikee47 commented Jan 7, 2025

The import/export are complimentary so the name of an object isn't included, the data is just imported into the pre-existing object. We could add a parameter to the export methods to control output since the underlying JSON Printer can do this anyway via it's style option. However, there are several use cases here:

  1. Just the object content (as we have at present)
  2. Object name plus content (just a JSON fragment, not valid by itself)
  3. Object name plus content, all wrapped in braces (i.e. a JSON object)
  4. All embedded in a larger JSON dataset being streamed

I'll have a think.

@pljakobs
Copy link
Contributor Author

pljakobs commented Jan 7, 2025

it's probably a result of what I'm using it for, which is a json over http API, in which case having the full json object is easiest to handle for the front end.
I can see the current implementation to have it's merits, I also can see point 4 be something we could want, most useful for me would be point2. Maybe extending ConfigDB::Json::format would be a way to pass the requested format to ConfigDB?

@mikee47
Copy link
Owner

mikee47 commented Jan 7, 2025

A more general approach, especially for (4), is to use a StreamChain. For your use case, add a MemoryDataStream containing the header text {"controllers":, then the export stream (as above), then a final MemoryDataStream containing the footer text }.

@mikee47
Copy link
Owner

mikee47 commented Jan 7, 2025

Maybe extending ConfigDB::Json::format would be a way to pass the requested format to ConfigDB?

I think the simplest option here is to extend ExportStream to allow post-constructor customisation. For example:

AppData::Controllers controllers(*app.data);
auto controllersStream = controllers.createExportStream(ConfigDB::Json::format);
controllersStream->setOption(Option::RootStyleNormal);
response.sendDataStream(controllersStream.release(), MIME_JSON);

The ExportStream is a virtual interface and should be format-agnostic, but a general set of option flags with varying support between formats is OK I think.

We could also add an additional method to allow header/footer data to be attached. Probably leave that until we actually have a need for it.

@mikee47
Copy link
Owner

mikee47 commented Jan 8, 2025

I've pushed a branch feature/export-options for you to try out. This should produce what you're after:

AppData::Controllers controllers(*app.data);
auto controllersStream = controllers.createExportStream(ConfigDB::Json::format);
auto options = controllersStream->getOptions();
options.rootStyle = ConfigDB::RootStyle::object;
controllersStream->setOptions(options);
response.sendDataStream(controllersStream.release(), MIME_JSON);

@pljakobs
Copy link
Contributor Author

only noticed this today, will test it asap

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

No branches or pull requests

2 participants