fractal icon indicating copy to clipboard operation
fractal copied to clipboard

defaultInclude introduces circular reference

Open Suven opened this issue 8 years ago • 11 comments

Hey!

thanks for your great plugin. One problem though:

When using a defaultInclude in both directions for an inverse relationship, the script ends in a endless-loop: Maximum function nesting level of '256' reached, aborting!

Here is a quick example of one location having many floors and one floor belonging to one location

class LocationTransformer extends Fractal\TransformerAbstract
{
    protected $defaultIncludes = ['floors'];

    public function includeFloors(Location $location)
    {
        $floors = $location->floors;
        return $this->collection($floors, new FloorTransformer, 'floors');
    }
class FloorTransformer extends Fractal\TransformerAbstract
{
    protected $defaultIncludes = ['location'];

    public function includeLocation(Floor $floor) {
        $location = $floor->location;
        return $this->item($location, new LocationTransformer, 'locations');
    }

Removing the defaultIncludes on either side of the relation prevents the problem.

Suven avatar Mar 10 '16 07:03 Suven

Is this really a bug?

What should the expected behaviour be in your opinion?

ababkov avatar Mar 22 '16 01:03 ababkov

Take this example

{
  "data": [{
    "type": "articles",
    "id": "1",
    "attributes": {
      "title": "JSON API paints my bikeshed!",
      "body": "The shortest article. Ever."
    },
    "relationships": {
      "author": {
        "data": {"id": "42", "type": "people"}
      }
    }
  }],
  "included": [
    {
      "type": "people",
      "id": "42",
      "attributes": {
        "name": "John"
      },
      "relationships": {
        "articles": {
          "data": [
              {"id": "1", "type": "articles"}
          ]
        }
    }
    }
  ]
}

I don't see any reason why this should end in an infinite regress when fetched via /api/v1/articles. It does also not lead to that when articles would not have the defaulInclude but it is fetched via /api/v1/articles?include=author

Suven avatar Mar 22 '16 09:03 Suven

So is your suggested solution to ignore default includes unless the resource being transformed is at the top of the stack?

ababkov avatar Mar 22 '16 21:03 ababkov

Nope. The included data also includes it's relationship.. But listing the relations (i.e their id's) for one entity is always recursion-free. The only thing that needs to be considered is that one needs to have a list of entities that have already been fetched (in the data-block or includes-block) and don't fetch them again.

Suven avatar Mar 23 '16 07:03 Suven

This is a very common use case IMO.

Any movement on this concept?

andrewmclagan avatar Aug 27 '16 02:08 andrewmclagan

I'm having the same issue, did anyone create a fix before i start looking at making one?

morgano86 avatar Dec 13 '16 00:12 morgano86

Never-mind, as a quick fix, I used a constructor on the transformer. I then passed a variable to the included transformers to indicate they should not set any default includes when nested. This solves the issue for now.

morgano86 avatar Dec 13 '16 00:12 morgano86

I remove the model from the relationship model's transformer class default include in the include method as shown below:

class LocationTransformer extends Fractal\TransformerAbstract
{
    protected $defaultIncludes = ['floors'];
    ...
    public function includeFloors(Location $location)
    {
        $floors = $location->floors;
        $transformer = new FloorTransformer();
        $defaultIncludes = $transformer->getDefaultIncludes();
        $transformer->setDefaultIncludes(array_diff($defaultIncludes, ['location']));

        return $this->collection($floors, $transformer, 'floors');
    }
}

class FloorTransformer extends Fractal\TransformerAbstract
{
    protected $defaultIncludes = ['location'];
    ...
    public function includeLocation(Floor $floor) {
        $location = $floor->location;
        $transformer = new LocationTransformer();
        $defaultIncludes = $transformer->getDefaultIncludes();
        $transformer->setDefaultIncludes(array_diff($defaultIncludes, ['floors']));

        return $this->item($location, $transformer, 'locations');
    }
}

allentcm avatar Apr 03 '17 14:04 allentcm

This seems like an ok workaround, I just don't like that each transformer has to have knowledge of the related transformer's defaultIncludes values.

akoebbe avatar Apr 03 '17 14:04 akoebbe

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 4 weeks if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 16 '22 07:04 stale[bot]

It seems to me that this could be fixed by adding support for a defaultExcludes?

Then you could say:

class LocationTransformer extends Fractal\TransformerAbstract
{
    protected $defaultIncludes = ['floors'];
    protected $defaultExcludes = ['floors.location'];

    public function includeFloors(Location $location)
    {
        $floors = $location->floors;
        return $this->collection($floors, new FloorTransformer, 'floors');
    }
class FloorTransformer extends Fractal\TransformerAbstract
{
    protected $defaultIncludes = ['location'];
    protected $defaultExcludes = ['location.floors'];

    public function includeLocation(Floor $floor) {
        $location = $floor->location;
        return $this->item($location, new LocationTransformer, 'locations');
    }

This would complement the paradigm for explicit (non-default) includes that are modified by explicit excludes.

GPHemsley-RELX avatar Jul 22 '22 14:07 GPHemsley-RELX