From 1ca7cb68e192b5513344f12345cca8b02c897d6a Mon Sep 17 00:00:00 2001 From: Paolo Caleffi Date: Mon, 4 Dec 2017 17:44:17 +0100 Subject: [PATCH 1/5] Parameters are now stripped away when giving relations to the model 'load()' method, to avoid a 'Relation not found' error and keep the underlying Fractal parameter management. --- src/TransformBuilder.php | 25 ++++++++++++++++++++++++- tests/Unit/TransformBuilderTest.php | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/TransformBuilder.php b/src/TransformBuilder.php index c0b7081..1af10a9 100644 --- a/src/TransformBuilder.php +++ b/src/TransformBuilder.php @@ -251,7 +251,7 @@ protected function prepareRelations($data, $transformer) } if ($data instanceof Model || $data instanceof Collection) { - $data->load($this->with); + $data->load($this->relationsWithoutParameters()); } $this->with = $this->stripEagerLoadConstraints($this->with); @@ -287,4 +287,27 @@ protected function stripEagerLoadConstraints(array $relations): array return is_numeric($key) ? $value : $key; })->values()->all(); } + + /** + * Remove parameters from relations that must be loaded. + * + * @return array + */ + protected function relationsWithoutParameters(): array + { + $cleanedRelations = []; + foreach ($this->with as $key => $value) { + // If the key is numeric, value is the relation name: + // we remove parameters from the value and return the relation name + // Otherwise the key is the relation name and the value is a custom scope: + // we remove parameters from the key and return the relation with the value untouched + if(is_numeric($key)) { + $cleanedRelations[$key] = explode(':', $value)[0]; + } else { + $cleanedRelations[explode(':', $key)[0]] = $value; + } + } + + return $cleanedRelations; + } } \ No newline at end of file diff --git a/tests/Unit/TransformBuilderTest.php b/tests/Unit/TransformBuilderTest.php index 9333fff..870e8d6 100644 --- a/tests/Unit/TransformBuilderTest.php +++ b/tests/Unit/TransformBuilderTest.php @@ -315,6 +315,30 @@ public function testTransformMethodExtractsAndEagerLoadsRelations() ])->once(); } + /** + * Assert that the [transform] method extracts default relationships from transformer and + * automatically eager loads all relationships. + */ + public function testTransformMethodExtractsAndEagerLoadsRelationsWhenThereAreRelationParameters() + { + $this->transformFactory->shouldReceive('make')->andReturn([]); + $this->resource->shouldReceive('getData')->andReturn($model = Mockery::mock(Model::class)); + $model->shouldReceive('load')->andReturnSelf(); + $this->resource->shouldReceive('getTransformer')->andReturn($transformer = Mockery::mock(Transformer::class)); + $transformer->shouldReceive('defaultRelations')->andReturn([]); + + $this->builder->resource()->with($relations = ['foo:first(aa|bb)','bar:second(cc|dd)'])->transform(); + + // Model should receive the relations names without parameters, + // while the transformFactory should receive also parameters to let Fractal use them + $model->shouldHaveReceived('load')->with(['foo','bar'])->once(); + $this->transformFactory->shouldHaveReceived('make')->with($this->resource, $this->serializer, [ + 'includes' => $relations, + 'excludes' => [], + 'fieldsets' => [], + ])->once(); + } + /** * Assert that the [only] method sets the filtered fields that are sent to the * [TransformFactory]. From 48319711692616904d0b01bc9f9ff9c968f4a1fa Mon Sep 17 00:00:00 2001 From: Paolo Caleffi Date: Mon, 4 Dec 2017 18:01:44 +0100 Subject: [PATCH 2/5] Updated README to explicitly declare how include parameters are managed. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 49ef057..6dcb0cc 100644 --- a/README.md +++ b/README.md @@ -633,6 +633,10 @@ return $this->resource($product->shipments, new ShipmentTransformer); _You should be careful with executing any new database calls inside the include methods as you might end up with an unexpected amount of hits to the database._ *** +#### Using parameters + +Parameters management is totally delegated to the underlying Fractal library (see it's [documentation](https://fractal.thephpleague.com/transformers/#include-parameters)) except from the fact that parameters are provided directly as an array instead of a `\League\Fractal\ParamBag`. + #### Setting Available Relationships The `$relations` property specifies a list of relations available to be included. When you generate a transformer, the `$relations` property will be equal to a wildcard, allowing all relations on the transformer: From d0222634fcd4b9cee4471faa68df4f1f4c621972 Mon Sep 17 00:00:00 2001 From: Paolo Caleffi Date: Mon, 4 Dec 2017 18:07:59 +0100 Subject: [PATCH 3/5] Fixed description of the new test --- tests/Unit/TransformBuilderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/TransformBuilderTest.php b/tests/Unit/TransformBuilderTest.php index 870e8d6..3004ac7 100644 --- a/tests/Unit/TransformBuilderTest.php +++ b/tests/Unit/TransformBuilderTest.php @@ -317,7 +317,7 @@ public function testTransformMethodExtractsAndEagerLoadsRelations() /** * Assert that the [transform] method extracts default relationships from transformer and - * automatically eager loads all relationships. + * automatically eager loads all relationships even when the relation name contains include parameters. */ public function testTransformMethodExtractsAndEagerLoadsRelationsWhenThereAreRelationParameters() { From 6a3f74f55f97ecd93af4e7d30c9b9ca297d7c19f Mon Sep 17 00:00:00 2001 From: Paolo Caleffi Date: Mon, 4 Dec 2017 23:37:19 +0100 Subject: [PATCH 4/5] Added a check about cases where the key is not numeric: the load method should receive the closure with the key stripped by parameters, while the TransformFactory should receive the relation name with parameters but without closures. --- tests/Unit/TransformBuilderTest.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/Unit/TransformBuilderTest.php b/tests/Unit/TransformBuilderTest.php index 3004ac7..fe33f92 100644 --- a/tests/Unit/TransformBuilderTest.php +++ b/tests/Unit/TransformBuilderTest.php @@ -327,13 +327,19 @@ public function testTransformMethodExtractsAndEagerLoadsRelationsWhenThereAreRel $this->resource->shouldReceive('getTransformer')->andReturn($transformer = Mockery::mock(Transformer::class)); $transformer->shouldReceive('defaultRelations')->andReturn([]); - $this->builder->resource()->with($relations = ['foo:first(aa|bb)','bar:second(cc|dd)'])->transform(); + $this->builder->resource()->with(['foo:first(aa|bb)', 'bar:second(cc|dd)' => function() {}])->transform(); // Model should receive the relations names without parameters, // while the transformFactory should receive also parameters to let Fractal use them - $model->shouldHaveReceived('load')->with(['foo','bar'])->once(); + // We must use the Mockery::on() method because with() method will try to do a strict match + // for the closure resulting in a failure, because it will check + // if it's the same closure reference but no closure are alike, even when they are defined identically. + // Here we just check that 'bar' element contains a closure. + $model->shouldHaveReceived('load')->with(Mockery::on(function (array $relations) { + return ($relations == 'foo') && ($relations['bar'] instanceof \Closure); + }))->once(); $this->transformFactory->shouldHaveReceived('make')->with($this->resource, $this->serializer, [ - 'includes' => $relations, + 'includes' => ['foo:first(aa|bb)', 'bar:second(cc|dd)'], 'excludes' => [], 'fieldsets' => [], ])->once(); From a2bb05fd38f3d7a8e82e284934ba808b813bb689 Mon Sep 17 00:00:00 2001 From: Paolo Caleffi Date: Mon, 4 Dec 2017 23:40:32 +0100 Subject: [PATCH 5/5] Typo in the test --- tests/Unit/TransformBuilderTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/TransformBuilderTest.php b/tests/Unit/TransformBuilderTest.php index fe33f92..b237695 100644 --- a/tests/Unit/TransformBuilderTest.php +++ b/tests/Unit/TransformBuilderTest.php @@ -336,7 +336,7 @@ public function testTransformMethodExtractsAndEagerLoadsRelationsWhenThereAreRel // if it's the same closure reference but no closure are alike, even when they are defined identically. // Here we just check that 'bar' element contains a closure. $model->shouldHaveReceived('load')->with(Mockery::on(function (array $relations) { - return ($relations == 'foo') && ($relations['bar'] instanceof \Closure); + return ($relations[0] == 'foo') && ($relations['bar'] instanceof \Closure); }))->once(); $this->transformFactory->shouldHaveReceived('make')->with($this->resource, $this->serializer, [ 'includes' => ['foo:first(aa|bb)', 'bar:second(cc|dd)'],