Allow stats to be nested_on resources

[evanrolfe]

Currently, you can only set stats for in the top-level part of the response, but we need to be able to set a meta tag with stats for each individual resource returned in the response i.e.

{
  "data": [
    {
      "id": "135",
      "type": "employee",
      "attributes": {
        "name": "Alice Smith",
        "age": 40,
        "createdAt": "2020-04-14T09:25:07+00:00"
      },
      "relationships": {},
      "meta": {
        "stats": {
          "age": {
            "squared": 1600
          }
        }
      }
    },
    {
      "id": "134",
      "type": "employee",
      "attributes": {
        "name": "Bob Smith",
        "age": 50,
        "createdAt": "2020-04-14T09:25:07+00:00"        
      },
      "relationships": {},
      "meta": {
        "stats": {
          "age": {
            "squared": 2500
          }
        }
      }
    }
  ],
  "meta": {}
}

This is my first time making any changes to the Graphiti codebase, so please let me know if there is a better way of doing this or if I have missed anything important.

Thanks, Evan.

[richmolj]

Thanks for this @evanrolfe, I definitely love to see the premise of per-resource metadata making its way into graphiti.

The code itself looks pretty good, but I'm a little concerned about the interface. The first thing that jumped out to me was that the request is the same, and the server is in charge of rendering at the resource level instead of the overall meta level (via the nested_on flag). Is my understanding correct? If so I worry this is confusing for clients, who should be able to distinguish between overall stats and per-resource stats in the request as well as the response.

One way to do this currently in the request is with extra_fields. What are the advantages of putting these stats on resource meta, instead of extra fields?

The other issue I see is with N+1 queries. The way many users (including myself) currently handle this scenario is by creating a separate StatsResource and sideloading it. The advantage is avoiding N+1 queries by default, and executing concurrently with sibling nodes on the graph. If I'm reading correctly, the PR as implemented would necessitate N+1 queries?

I guess I like the idea of resource-level metadata more than resource-level stats, specifically, for these reasons. But I'm definitely open to it, would like to get your thoughts on the above.

[danleyden]

@richmolj thanks for the thoughts and taking time to consider.

For the confusion on overall vs nested stats for the requester, I would suggest that is handled by the name of the stat. We didn't really want to introduce a new parameter for a "different kind of stat" because that felt a little wrong. I don't think that the extra_fields option is really appropriate here because this isn't a field/attribute - it is metadata... and more specifically, it is a stat. I wonder if the trick here might be more to follow something similar to nested includes like stat[resource_type.stat_name]=stat_type, e.g. stat[employees.age]=squared.

On the N+1, agree that there may be an issue with some adapters and that it would be easy to fall in to a bad place there... In our particular case, we have our own adapter which is using active resource to pull back data from a third service. This third service returns the nested stats as part of the payload of its response so for us the N+1 is little more than reading a variable in the adapter. Splitting out in to a separate resource would mean, for our case, making a second request to the external resource - which is much more expensive (and adding a caching layer behind the adapter isn't a trivial option for our case).

An alternative approach which might address the N+1 in a more comprehensive way could be to do something similar to the assign and assign_each approach to end up with code in the resource like:

Class.new(PORO::EmployeeResource) do
  stat age: [:squared], nested_on: :employees do
    squared do
      calc_all do |scope, attr, context, employees|
        # returns a hash of employees => value
        Hash[employees.map { |employee| [employee, employee.age * employee.age] }]
      end

      calc_each do |scope, attr, context, employee|
        employee.age * employee.age
      end
    end
  end
end

Then the NestedPayload class would call the all block once with all employees and the each block with every individual employee. In the all case, it would then need to assemble the responses and associate them correctly. In the case of both being provided then all would run first and the each would run second, allowing an adapter to alter the individual elements if needed as part of the all block, then associate any additional in the each block.