revoltchat / backend

Monorepo for Revolt backend services.
https://developers.revolt.chat/api/
Other
1.09k stars 123 forks source link

bug: GET /servers/{target}?include_channels=true creates JSON with duplicate keys #286

Closed QuantumToasted closed 3 months ago

QuantumToasted commented 7 months ago

What happened?

As it says on the tin: GET /servers/{target}?include_channels=true will produce JSON similar to the following.

{
    "_id": "01H2SK15KZ914SN7MXDA5SFE4A",
    "channels": [
        "01H2SK15KZ05FZXV9XQ3QN1QBA",
        "01HEW15S36TG3XFK576GJKBF30",
        "01HF28N58Z8JZ896T1BHBJQ2A3",
        "01HF2BMN2Q64DP3TP43X3FA2G1"
    ],
    ...,
    "channels": [
        {
            "channel_type": "TextChannel",
            "_id": "01H2SK15KZ05FZXV9XQ3QN1QBA",
            "server": "01H2SK15KZ914SN7MXDA5SFE4A",
            "name": "General",
            "last_message_id": "01HF2BMFGBJ30TSNYFRVZDGHH9"
        },
        {
            "channel_type": "TextChannel",
            "_id": "01HEW15S36TG3XFK576GJKBF30",
            "server": "01H2SK15KZ914SN7MXDA5SFE4A",
            "name": "test2"
        },
        {
            "channel_type": "VoiceChannel",
            "_id": "01HF28N58Z8JZ896T1BHBJQ2A3",
            "server": "01H2SK15KZ914SN7MXDA5SFE4A",
            "name": "yea"
        },
        {
            "channel_type": "TextChannel",
            "_id": "01HF2BMN2Q64DP3TP43X3FA2G1",
            "server": "01H2SK15KZ914SN7MXDA5SFE4A",
            "name": "balls"
        }
    ]
}

IMO, this should not be a valid response JSON. While a little research has told me that the initial JSON spec did not mention "duplicate keys" by name as to whether they're valid, many people tend to agree that it's bad practice at best and completely invalid JSON at worst. I would kindly ask that ?include_channels=true either replace the original channels ID array with the "full" channels object array, instead of this weird duplicate key response. Alternatively, you could introduce the field with a different name if consistency with other "server" JSON models is desired.

Rexogamer commented 7 months ago

Can reproduce. The relevant code is here - it seems to me that the first channels property isn't being filtered out?

insertish commented 3 months ago

Can't reproduce this anymore, assuming fixed.