airdcpp-web / airdcpp-webclient

Communal peer-to-peer file sharing application for file servers/NAS devices
https://airdcpp-web.github.io
175 stars 32 forks source link

Definition of /extensions/{extension_id}/settings endpoint #337

Closed doobnet closed 4 years ago

doobnet commented 4 years ago

The GET /extensions/{extension_id}/settings endpoint documented here [1] looks a bit weird when it comes to the attributes. In the documentation it has an Attributes section defined, but it's empty. As far as I can see, other endpoints which don't return any attributes don't have this section defined. Looking at this endpoint in the APIB file, it's defined as:

### Get setting values [GET /extensions/{extension_id}/settings]

Required permission: *settings_view*

+ Response 200 (application/json)
    + Attributes (object)

        Key-value listing of setting keys and their current values

        + spam_on_startup: false (boolean)
        + banner_text: Test

Obviously there are some attributes defined but I think the parser gets tripped by the text before the attributes. I've been using the Drafter parser to convert the APIB file to JSON. The structure of the JSON is a bit unexpected compared to any other endpoints I've seen so far. If I remove the text above the attributes the generated JSON contains two attributes for this endpoint.

Are there supposed to be attributes defined for this endpoint or is it more of a free form where it can return any set of key-values and the above is only an example?

[1] https://airdcpp.docs.apiary.io/#reference/extension-entities/methods/get-setting-values

maksis commented 4 years ago

I've corrected the output.

Are there supposed to be attributes defined for this endpoint or is it more of a free form where it can return any set of key-values and the above is only an example?

Those are just examples, the actual keys and values are extension-specific. Extensions can freely define their own settings schema with https://airdcpp.docs.apiary.io/#reference/extension-entities/methods/post-setting-definitions

doobnet commented 4 years ago

I've corrected the output.

Hmm, that doesn't look correct. I'm working on a tool that parses the APIB file (via JSON) and generates code for communicating with the API. This includes generating static types for the request and response. If the response is defined to return an object with attributes spam_on_startup and banner_text, it won't work if other attributes are returned. Wouldn't it be better to type the result as object?

maksis commented 4 years ago

Which tool are you using and what kind of integration are you going to implement with it...?

https://medium.com/@jessica.ulyate/why-optional-attributes-will-ruin-your-api-blueprint-documentation-1629b95546e2

According to this, additional attributes are allowed in the response, unless the response is marked as fixed-type (which isn't currently being used anywhere as I wasn't aware of it earlier). I'd also like to keep some example data there...

doobnet commented 4 years ago

Which tool are you using and what kind of integration are you going to implement with it...?

I'm using the Drafter parser [1] to generate JSON out of the APIB file. I've written my own tool to parse the JSON and generate Swift code from it. The idea is to implement a native GUI for macOS.

According to this, additional attributes are allowed in the response, unless the response is marked as fixed-type (which isn't currently being used anywhere as I wasn't aware of it earlier)

Hmm, before this I was only aware of optional and required. According to the Drafter parser required is the default for the attributes section [2]. The parser outputs a type attribute regardless if one is specified or not. If it's not specified, the value will be required (I don't know if this is a bug in the parser or in the documentation). For members of the attributes section, the parse will not output a type attribute if no one is specified.

If the whole response is optional, I would assume it means that the response can be present or not. Not that it can contain arbitrary unspecified attributes.

I'd also like to keep some example data there...

Perhaps this is more appropriate [3]. If Sample is added between Attributes and the attributes, so it looks like this instead:

+ Response 200 (application/json)
    + Attributes (object)
        + Sample
            + spam_on_startup: false (boolean)
            + banner_text: Test

I checked with the parser, it properly recognizes the above.

[1] https://github.com/apiaryio/drafter (which I understand is the official API Blueprint parser) [2] https://github.com/apiaryio/api-blueprint/blob/master/API%20Blueprint%20Specification.md#def-attributes-section [3] https://github.com/apiaryio/mson/blob/master/MSON%20Specification.md#44-sample

maksis commented 4 years ago

Perhaps this is more appropriate [3]. If Sample is added between Attributes and the attributes, so it looks like this instead:

+ Response 200 (application/json)
    + Attributes (object)
        + Sample
            + spam_on_startup: false (boolean)
            + banner_text: Test

I checked with the parser, it properly recognizes the above.

Thanks, that looks like a proper solution and I've updated the docs.

I've never done any schema validations against the actual API, so there are most likely other issues as well. Pull requests are welcome, in case you find something.

doobnet commented 4 years ago

Thanks, that looks like a proper solution and I've updated the docs.

Thanks.

I've never done any schema validations against the actual API, so there are most likely other issues as well.

Fair enough. The parser manage to parse it at least.

Pull requests are welcome, in case you find something.

I haven't found the APIB file in any of the git repositories. But I can definitely do pull requests if you point me to it and there's a need.

maksis commented 4 years ago

I haven't found the APIB file in any of the git repositories. But I can definitely do pull requests if you point me to it and there's a need.

https://github.com/airdcpp-web/airdcpp-apidocs