API - Some things regarding the JSON:API spec

simon_m

Hi everyone! I have been a silent reader for a while and will now for the first time chime into the discussion. First though, thanks for all your work, flarum is turning into a fantastic piece of software! 🚀

I have been playing with the API and have noticed a few quirks, maybe you can help to clarify..

default include

Looking at the code there are several default includes. To keep my requests as small as possible I would like to disable those. However, I can only add more but cannot disable the default ones. According to the JSON:API specification, this must not happen.

If an endpoint supports the include parameter and a client supplies it, the server MUST NOT include unrequested resource objects in the included section of the compound document.

selecting fields

If I use the fields parameter to select, which attributes I want included, this will omit the relationships. See example below.

Example

Let's say I want to fetch the three top discussions with only their titles, nothing else.
https://discuss.flarum.org/api/discussions?page[size]=3&fields[discussions]=title

The default include adds lots of data. However through selecting the title the relationships are gone and there is no way to manually select them (e.g. ?fields[discussions]=title,relationships won't work, since relationships is not an attribute).

{
    "links": {
        "first": "https://discuss.flarum.org/api/discussions?page%5Bsize%5D=3&fields%5Bdiscussions%5D=title",
        "next": "https://discuss.flarum.org/api/discussions?page%5Bsize%5D=3&page%5Boffset%5D=3&fields%5Bdiscussions%5D=title"
    },
    "data": [
        {
            "type": "discussions",
            "id": "22974",
            "attributes": {
                "title": "Dev Diary: Beta 13"
            },
            // there should be a "relationships": section here
        },
        {
            "type": "discussions",
            "id": "22973",
            "attributes": {
                "title": "Flarum 0.1.0-beta.12 Released"
            },
            // there should be a "relationships": section here
        },
        {
            "type": "discussions",
            "id": "4593",
            "attributes": {
                "title": "Guidelines for Posting Your Extension"
            },
            // there should be a "relationships": section here
        }
    ],
    "included": [
        {
            "type": "users",
            "id": "1376",
            "attributes": {
                "username": "luceos",
                "displayName": "luceos",
                "avatarUrl": "https://discuss.flarum.org/assets/avatars/ZwzFapPty4TcGdYw.png",
                "blocksPd": false,
                "cannotBeDirectMessaged": false
            }
        },
        {
            "type": "users",
            "id": "32000",
            "attributes": {
                "username": "cubixel",
                "displayName": "cubixel",
                "avatarUrl": "https://discuss.flarum.org/assets/avatars/gIX8PnjWeWhjW4NQ.png",
                "blocksPd": false,
                "cannotBeDirectMessaged": false
            }
        },
       ... // shortened
        {
            "type": "posts",
            "id": "122372",
            "attributes": {
                "number": 1,
                "createdAt": "2020-03-04T10:12:06+00:00",
                "contentType": "comment",
                "contentHtml": "<p>This is a placeholder, usually you would see some information about our kick off meeting. That meeting is planned for coming Friday, so there's nothing to say to you right now except that Beta 12 just got released.</p>\n\n<p>Feel free to use the Follow button to get notified whenever we talk about development of the upcoming Beta 13 release.</p>\n\n<p>Seeing how well we did with our last release, let's be naughty again by mentioning we target the release of beta 13 for early May 2020 🥳</p> "
            }
        },
       ... // shortened
        {
            "type": "tags",
            "id": "16",
            "attributes": {
                "name": "Meta",
                "description": "Discussion about this community: its organisation and how we can improve it.",
                "slug": "meta",
                "color": "#EF564F",
                "backgroundUrl": null,
                "backgroundMode": null,
                "icon": "fas fa-bullhorn",
                "discussionCount": 359,
                "position": 5,
                "defaultSort": null,
                "isChild": false,
                "isHidden": false,
                "lastPostedAt": "2020-04-28T12:12:24+00:00",
                "canStartDiscussion": false,
                "canAddToDiscussion": false,
                "subscription": null
            }
        },
       ... // shortened
    ]
}

I guess the default inclusions was a conscious decision?
I guess the omitted relationships when specifying fields is a bug?

If you would rather discuss this on GitHub, let me know, I will try to find time to open an issue there..

Best, Simon

clarkwinkelmann

simon_m seems like we indeed have a few bugs in there.

If you specify an include parameter, for example include=firstPost, only that relationship is loaded. The problem seems to be when specifying an empty include=, where Flarum goes back to the default includes instead of no includes.

As for the fields part, it definitely looks like unintended behavior as well. I see in the source code of the json-api-php package that fields are used to filter relationships as well, but I can't get it to work https://github.com/tobyzerner/json-api-php/blob/355b213927a1412ede81b67ac2a5605753382379/src/Resource.php#L244

We need to identify which issues are related to Flarum and which ones are related to json-api-php. The second one looks like a json-api-php issue. The first one I'm not sure where the responsible code lives.

Sounds like json-api-php could benefit from a few more unit tests based on this.

simon_m

Hi @clarkwinkelmann, thanks for getting back so soon!

Indeed, adding include=firstPost does work for the include section (but the relationship section is omitted when combined with fields). Also, in ShowDiscussionController.php these are the default and optionalDefault includes:

    public $include = [
        'posts',
        'posts.discussion',
        'posts.user',
        'posts.user.groups',
        'posts.editedUser',
        'posts.hiddenUser'
    ];
    
    public $optionalInclude = [
        'user',
        'lastPostedUser',
        'firstPost',
        'lastPost'
    ];

The annoying thing is that it's not possible to just chose one of the default ones, it will result in a 400 Bad Request (example).

simon_m

I just noticed something else. The includes/relationships are different when you query one discussion vs many discussions. This may be useful but it's certainly confusing.

https://discuss.flarum.org/api/discussions/1662
---
{
  "data": {
    "type": "discussions",
    "id": "1662",
    "attributes": {…},
    "relationships": {
      "posts": {…},
      "tags": {…},
      "recipientUsers": {…},
      "oldRecipientUsers": {…},
      "recipientGroups": {…},
      "oldRecipientGroups": {…}
    }
  },
  "included": […]
}

https://discuss.flarum.org/api/discussions?page[size]=1
---
{
  "links": {…},
  "data": [
    {
      "type": "discussions",
      "id": "1662",
      "attributes": {…},
      "relationships": {
        "user": {…},
        "lastPostedUser": {…},
        "firstPost": {…},
        "tags": {…},
        "recipientUsers": {…},
        "oldRecipientUsers": {…},
        "recipientGroups": {…},
        "oldRecipientGroups": {…}
      }
    }
  ],
  "included": […]
}

clarkwinkelmann

simon_m yes there's a different set of default includes depending on which controller answers the request.

Usually the default includes are those useful to the single page app, and usually less things are included when you ask a list of results, as not everything will be needed or shown.

It would be highly advisable to always specify the include parameter in third-party apps as the default set of includes could change in subsequent Flarum versions.

Thanks for your research, this is very valuable information. We'll need to turn that into GitHub issues so we can address it in future updates.

simon_m

clarkwinkelmann Maybe (can't promise) I will find the time during the weekend to open some issues.. Do you think separate or one combined issue is the way to go here?

clarkwinkelmann

simon_m in this case one GitHub issue per problem would be the best. That way it makes for very small issues someone can tackle when there's time. There are two issues we discussed so far I think?

But if you prefer you could make one large detailed issue with everything and we can reference sub-issues when we create them at a later time.

simon_m

Hey @clarkwinkelmann, I finally found the time to create issues. See #2147 and #2148.

eddiewebb

It looks like Flarum/Sticky is also doing things against the letter of API , but project does not have issues enabled in github so raising it here..

https://discuss.flarum.org/d/27250-why-sticky-are-included-in-api-calls-when-not-asked-for/3

clarkwinkelmann

eddiewebb all issues for core and core extensions are managed at https://github.com/flarum/core/issues That's why the issues aren't enabled on the repositories on GitHub.

You'll see the title of some issues specifically references some of our core extensions.