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

Attach top-level meta in index response #225

Open
tsterker opened this issue Sep 14, 2018 · 10 comments
Open

Attach top-level meta in index response #225

tsterker opened this issue Sep 14, 2018 · 10 comments
Milestone

Comments

@tsterker
Copy link

tsterker commented Sep 14, 2018

It feels like I'm missing something obvious, but I was not able to find a straight-forward way for attaching custom top-level meta to the response (just like the page meta for pagination). I also didn't find any tickets that seemed related to this.

I have different contexts in mind, for which one might want to attach top-level metadata, in case it could inform any design decisions:

  • global - can always be blindly applied globally;
    e.g. requested_at
  • resource-specific - specific metadata/logic for different resources;
    e.g. sortable_fields differs for users and posts
  • query (result)-specific - specific data only available once the query result (in my case a Solr query) is available;
    e.g. facet_fields counts depend on the current query and are only available with the query result

e.g.

{
  "meta": {
      "requested_at": "2018-09-14T00:00:00Z",  // global

      "sortable_fields": ["foo", "bar"],       // resource-specific

      "facet_fields": {                        // query response-specific
          "sizes": {
              "small": 1,
              "medium": 0,
              "large": 3,
          },      
      },
  },
  "links": {
  },
  "data": [
  ]
}

Apologies if there already is a way of achieving this in a clean way. If not, are there any plans of adding support for this?

@lindyhopchris
Copy link
Member

Hi! Thanks for creating this issue. There is a way to include meta in a response at the moment, but it definitely could be a lot better supported. Your summary of the three different ways to add meta is a great list, and I definitely think it would be good to support each of those ways.

At the moment if you wanted to add meta you would have to use the controller hooks that are described in this chapter:
https://laravel-json-api.readthedocs.io/en/latest/basics/controllers/

Basically if you return a response from any hook it will use that. Most of the methods on the JSON API responses factory take meta as their argument. You can see those methods here:
https://github.com/cloudcreativity/laravel-json-api/blob/develop/src/Http/Responses/Responses.php

As an example, when reading a resource, the controller does this:

return $this->reply()->content($post);

You could change this by adding a reading method that returned:

protected function reading($post)
{
  // ... work out $meta
  return $this->reply()->content($post, [], $meta);
}

I'd like to do this in a much easier way but that's how you'd need to implement it at the moment.

@lindyhopchris lindyhopchris added this to the 1.1.0 milestone Sep 18, 2018
@aaron-romanick
Copy link

Also, it's difficult to add top-level, query-independant meta data to index results as the query isn't invoked until after the searching hook (which doesn't get passed the $store parameter, only the $request)

@lindyhopchris
Copy link
Member

@dapperdanman1400 yes that's definitely a fair comment. It would be worth me adding a searched hook to make that easier in the interim before I add a full solution for the top-level meta-data problem.

@tsterker
Copy link
Author

tsterker commented Jan 10, 2019

@lindyhopchris A searched hook would be a great start!

My current quick-and-dirty workaround is using the searching hook like below. But I'm not even sure if I'm doing this correctly or if this comes with some issues I'm currently not aware of.

public function searching(ValidatedRequest $request)
{
    $store = app(\CloudCreativity\LaravelJsonApi\Contracts\Store\StoreInterface::class);
    $result = $store->queryRecords($request->getResourceType(), $request->getParameters());

    $data = [];
    try {
        $data = get_object_vars($result->getData());  // need to do this to e.g. not loose pagination meta?
    } catch {
        // ignore
    }

    return $this->reply()
        ->content(
            $result,
            [],  // links
            array_merge($data, [
                'globalMeta' => 'Hello from searching hook',
            ])
        );
}

edit: Just realized that the StoreAwareTrait might be a better way to get the store instance?

@lindyhopchris
Copy link
Member

Searched hook has been added on develop, will be tagged as 1.0.0-rc.1.

@lindyhopchris
Copy link
Member

FYI controller takes care of calling getData() before passing it to the searched hook.

@andreas83
Copy link

i have a very similar question in mind, i like to add the amount of results in the very top meta field and did not found any solution for this so far.

@lindyhopchris
Copy link
Member

Yes, that's correct I need to add in support for meta in different places.

@tsterker
Copy link
Author

@lindyhopchris Are there any concrete plans on tackling this in the foreseeable future?

I'm currently struggling using the searched hook to add/update a top-level meta field when having paginated results.

e.g. I currently have some response with top-level pagination meta "magically" (to my current knowledge) added:

{
  "meta": {
    "page": {
      "current-page": 1,
      "per-page": 100,
      "from": 1,
      "to": 100,
      "total": 21781,
      "last-page": 218
    }
  },
 "links": {},
 "data": {}
}

Without finding the time yet to delve into the code, it seems that data in a LengthAwarePaginator somehow result in pagination information being included in the top level meta.

Now my problem is, that I didn't find an obvious way to get my hands on that pagination meta data within the searched hook to extend it with some custom data.

My current implementation looks something like this and loses the page metadata:

public function searched($paginatedData)
{
    $meta = ['some' => 'data'];

    return $this->reply()
        ->content(
            $paginatedData,
            [],  // links
           $meta  // <-- how can I add my own meta without overriding the pagination meta?
        );
}

What I would like to have is something like this (note the "some": "data" next to page):

{
  "meta": {
    "some": "data",
    "page": {
      "current-page": 1,
      "per-page": 100,
      "from": 1,
      "to": 100,
      "total": 21781,
      "last-page": 218
    }
  },
 "links": {},
 "data": {}
}

@lindyhopchris
Copy link
Member

@tsterker this looks like a bug... if you're passing $meta into the reply()->content() method as well as paginated data, it should merge them rather than overriding... i.e. it has been given meta in the paginated data plus other meta, so the two should be combined.

I'll create an issue for that separately because the bug is not the same as the feature that this issue is requesting (and which probably won't land until 2.0 as it'll be breaking to add in top-level meta support in a nice way!).

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

No branches or pull requests

4 participants