Some questions and problems regarding pagination and marshaling of link objects

[mark-hartmann]

First of all, I would like to say that I really like this package, it is exactly what I was looking for. But here and there I see some problems, especially in two fairly critical areas:

JSON:API is agnostic regarding pagination strategies, this library is not

The problem arises especially if you do not use page- or offset-based pagination, but for example cursor-based pagination, which is often used for large data sets and requires either an additional page parameter (e.g. page[dir]=next) or a prefix for the cursor (e.g. page[cursor]=next__200). But since this package only accepts page[number]=(uint) and page[size]=(uint), this strategy is completely off the table, which definitely should not be.

Unfortunately I don't have a really satisfying solution how one could implement this, because, to stay with the example of cursor-based pagination, an attribute to specify the direction of the cursor (next and prev) would have to be added to SimpleURL and Params. But since this would not be used in page- and offset-based pagination, this could be somewhat confusing.

MarshalDocument ignores links

If you want to marshal a document with additional links (e.g. pagination links), these are completely ignored, because the MarshalDocument function simply overwrites the corresponding entry of the plMap with "self":

if url != nil {
    plMap["links"] = map[string]string{
        "self": doc.PrePath + url.String(),
    }
}

Links should be treated like Meta in my opinion, The Meta object is added to the plMap if len(doc.Meta) > 0, for example like this:

links := map[string]Link{}
if len(doc.Links) > 0 {
    links = doc.Links
}

if url != nil {
    links["self"] = Link{
        HRef: doc.PrePath + url.String(),
    }
}

if len(links) > 0 {
    plMap["links"] = links
}

Please let me know what you think of this and how it would be best approached, assuming of course these were not conscious design decisions (which would be a bummer).

[mark-hartmann]

So I've been playing around a bit, and at least made some progress on the link issue. The marshaled content produced by my modifications is tested against the JSON:API schema, so It conforms to the specification.

Document links

Until now links of the document were completely ignored, now they are appended if len(doc.Links) > 0

Resource links

Currently, resources only have a generated "self" link, but the specification does not limit the links that can be placed. Therefore, similar to the MetaHolder interface, I created another interface LinkHolder, which, like the meta object, takes effect when the resource is marshaled. The generated "self" link is not overwritten.

Relationships

Currently, with relationships, you only have the ability to return the ids of related resources without the ability to contextualize them:

{
    "data": [
        {"id": "id1", "type": "someType"},
        {"id": "id2", "type": "someType"},
    ]
}

This issue has been somewhat challenging and I'm not sure if this is the best way to approach it. I have created two structs which can be returned from resources via Get should an relationship be requested:

// RelData contains information about a to-one relationship, including links and metadata.
type RelData struct {
    Res   Identifier
    Links map[string]Link
    Meta  Meta
}

// RelDataMany contains information about a to-many relationship, including links and metadata.
type RelDataMany struct {
    Res   Identifiers
    Links map[string]Link
    Meta  Meta
}

By returning one of these two structs instead of just ids, the information it contains is then used to represent much more complex relationships.

{
    "data": [
        {"id": "id2", "type": "someType"},
        {"id": "id2", "type": "someType", "meta": {"k1": "v1"}}
    ],
    "links": {
        "self": "/someType/id1/relationships/to-x",
        "related": "/someType/id1/to-x",
        "example": "https://example.org"
    }
}

Link metadata

For links, they are not only added as strings (map[string]string / only HRef), but as a whole. Since Link structs can already be marshaled, the Meta object of links is also output this way, if set. This also applies to other links, such as relationships links.

{
    "links": {
        "self": "https://example.org/some-self-link",
        "example": {
            "href": "https://example.org/foo",
            "meta": {
                "foo": "bar",
                "bar": "baz"
            }
        }
    }
}

[mfcochauxlaberge]

@mark-hartmann

I made an attempt to fix the pagination issue. You are right, there's no reason to enforce a strategy here. It seemed like a good idea according to young me years ago haha!

Here's the PR: #125

Let me know what you think.

For the links issue, I'll think about it more and look at what you have done in your PR.

Thanks a lot.

[mark-hartmann]

Uncomplicated and flexible, I like it. Definitely better than any of my attempts, since I tried to include some kind of enforcement/validation, but that would be up to the end-users now.

I did notice though that URL.String (as well as URL.UnescapedString) still insists on number and size, is that intentional? One way to flexibly represent the page map as page parameters would be the following:

// Maps have no reliable order
var pageParams []string
for k, v := range u.Params.Page {
    pageParams = append(pageParams, "page%5B"+url.QueryEscape(k)+"%5D="+fmt.Sprint(v))
}
sort.Strings(pageParams)
urlParams = append(urlParams, pageParams...)

Of course one could discuss about the sorting, if only the keys should count etc., but I think you get the idea.

[mark-hartmann]

Hi @mfcochauxlaberge, any news regarding the links and the number/size issue?

By the way, after playing with this package for a while, I stumbled upon some more things where I see room for improvement. I know these would be major changes compared to the current implementation, so I can implement this in my fork if you don't want to give up your original vision.

Generalize the opinionated filter

The base specification is agnostic about filtering strategies supported by a server. The filter query parameter family is reserved to be used as the basis for any filtering strategy.

I think filters should be handled similarly to how pagination is handled in your PR. This way, the agnostic nature of the JSON:API specification can be met, and users can implement their own strategies. The existing filter implementation could still be provided in a slightly modified way.

Allow unknown query parameters

Currently, no query parameters are allowed that are not explicitly defined in the JSON:API specification, which makes common parameters like ?language or ?apiKey unusable:

switch {
    default:
        return sURL, NewErrUnknownParameter(name)
    }
}

Make the supplementing of fields in the URLs optional

If a client does not specify the set of fields for a given resource type, the server MAY send all fields, a subset of fields, or no fields for that resource type.

This issue is most definitely debatable as it is not necessarily wrong in the current implementation, however it does make it problematic to be able to return no fields at all or a subset of fields if no explicit sparse fieldsets have been specified in the request.

[mfcochauxlaberge]

Hi, I merged a fix to allow more links at the document level. There is more you mentioned here that might need to be fixed, so we can continue the discussion here.

Let's open new issues for the rest.