From 2c8d291e8a0f578db05688f22e1da8a631d06ebb Mon Sep 17 00:00:00 2001 From: jared Date: Tue, 24 Oct 2023 00:02:26 -0600 Subject: [PATCH 1/4] Allow Model.beforeFind to execute closure to satisfy injection requirement --- README.md | 17 +++ src/Model/Behavior/FootprintBehavior.php | 7 +- tests/Fixture/ArticlesFixture.php | 25 ++++ .../Model/Behavior/FootprintBehaviorTest.php | 136 +++++++++++++++--- tests/schema.php | 2 + 5 files changed, 163 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 6227fcb..1058a16 100755 --- a/README.md +++ b/README.md @@ -75,6 +75,23 @@ fields when creating a record, on the `modified_by` field again when updating the record and it will use the associated user record's company `id` in the `company_id` field when creating a record. +You can also provide a closure that accepts an EntityInterface and returns a bool: + +```php +$this->addBehavior('Muffin/Footprint.Footprint', [ + 'events' => [ + 'Model.beforeSave' => [ + 'user_id' => 'new', + 'company_id' => 'new', + 'modified_by' => 'always', + 'deleted_by' => function ($entity): bool { + return $entity->deleted !== null; + }, + ] + ], +]); +``` + ### Adding middleware via event In some cases you don't have direct access to the place where the `AuthenticationMiddleware` is added. Then you will have to add this to your `src/Application.php` diff --git a/src/Model/Behavior/FootprintBehavior.php b/src/Model/Behavior/FootprintBehavior.php index 63e3baf..bd8dc37 100644 --- a/src/Model/Behavior/FootprintBehavior.php +++ b/src/Model/Behavior/FootprintBehavior.php @@ -163,9 +163,9 @@ protected function _injectEntity(EntityInterface $entity, ArrayObject $options, $new = $entity->isNew() !== false; foreach ($fields as $field => $when) { - if (!in_array($when, ['always', 'new', 'existing'])) { + if (!in_array($when, ['always', 'new', 'existing']) && !is_callable($when)) { throw new UnexpectedValueException(sprintf( - 'When should be one of "always", "new" or "existing". The passed value "%s" is invalid', + 'When should be one of "always", "new" or "existing", or a closure that takes an EntityInterface and returns a bool. The passed value "%s" is invalid', $when )); } @@ -177,7 +177,8 @@ protected function _injectEntity(EntityInterface $entity, ArrayObject $options, if ( $when === 'always' || ($when === 'new' && $new) || - ($when === 'existing' && !$new) + ($when === 'existing' && !$new) || + (is_callable($when) && $when($entity)) ) { $entity->set( $field, diff --git a/tests/Fixture/ArticlesFixture.php b/tests/Fixture/ArticlesFixture.php index e4474e1..2751d51 100644 --- a/tests/Fixture/ArticlesFixture.php +++ b/tests/Fixture/ArticlesFixture.php @@ -12,16 +12,41 @@ class ArticlesFixture extends TestFixture 'title' => 'article 1', 'created_by' => 1, 'modified_by' => 1, + 'manager_id' => 10, ], [ 'title' => 'article 2', 'created_by' => 1, 'modified_by' => 2, + 'manager_id' => null, ], [ 'title' => 'article 3', 'created_by' => 2, 'modified_by' => 1, + 'company_id' => 2, + 'manager_id' => null, ], + [ + 'title' => 'find article', + 'created_by' => 4, + 'modified_by' => 4, + 'company_id' => 2, + 'manager_id' => null, + ], + [ + 'title' => 'final article', + 'created_by' => 3, + 'modified_by' => 4, + 'company_id' => 4, + 'manager_id' => null, + ], + [ + 'title' => 'penultimate article', + 'created_by' => 4, + 'modified_by' => 4, + 'company_id' => 1, + 'manager_id' => null, + ] ]; } diff --git a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php index 4521a61..3c6e247 100644 --- a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php +++ b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php @@ -28,19 +28,22 @@ public function setUp(): void 'created_by' => 'new', 'modified_by' => 'always', 'company_id' => 'always', + 'manager_id' => function ($entity): bool { + return $entity->company_id == 1; + } ], - 'Model.beforeFind' => 'created_by', + 'Model.beforeFind' => [ + 'created_by', + 'company_id', + ] ], 'propertiesMap' => [ 'company_id' => '_footprint.company.id', + 'manager_id' => '_footprint.manager.id', ], ]); $this->Table = $table; - $this->footprint = new Entity([ - 'id' => 2, - 'company' => new Entity(['id' => 5]), - ]); } public function tearDown(): void @@ -51,58 +54,149 @@ public function tearDown(): void public function testSave() { - $entity = new Entity(['title' => 'new article']); - $entity = $this->Table->save($entity, ['_footprint' => $this->footprint]); + // Properties may still be assigned even if + // closure would be satisfied. + $entity = new Entity(['title' => 'new article', 'manager_id' => 7]); + $footprint = new Entity([ + 'id' => 2, + 'company' => new Entity(['id' => 1]), + 'manager' => new Entity(['id' => 10]), + ]); + + $entity = $this->Table->save($entity, ['_footprint' => $footprint]); $expected = [ 'id' => $entity->id, 'title' => 'new article', 'created_by' => 2, 'modified_by' => 2, - 'company_id' => 5, + 'company_id' => 1, + 'manager_id' => 7, ]; + $this->assertSame( $expected, - $entity->extract(['id', 'title', 'created_by', 'modified_by', 'company_id']) + $entity->extract(['id', 'title', 'created_by', 'modified_by', 'company_id', 'manager_id']) ); + // Closure fields won't set if disallowed + // even if provided. + $entity = new Entity(); + $entity->title = 'new title'; $footprint = new Entity([ 'id' => 3, + 'company' => new Entity(['id' => 5]), + 'manager' => new Entity(['id' => 4]), ]); - $entity->title = 'new title'; + $entity = $this->Table->save($entity, ['_footprint' => $footprint]); - $expected = ['id' => $entity->id, 'title' => 'new title', 'created_by' => 2, 'modified_by' => 3]; - $this->assertSame($expected, $entity->extract(['id', 'title', 'created_by', 'modified_by'])); + $expected = [ + 'id' => $entity->id, + 'title' => 'new title', + 'created_by' => 3, + 'modified_by' => 3, + 'company_id' => 5, + 'manager_id' => null, + ]; + + $this->assertSame($expected, $entity->extract(['id', 'title', 'created_by', 'modified_by', 'company_id', 'manager_id'])); + // Fields won't set if a footprint isn't provided $entity = new Entity(['title' => 'without footprint']); + $entity = $this->Table->save($entity); - $expected = ['id' => $entity->id, 'title' => 'without footprint', 'created_by' => null, 'modified_by' => null]; - $this->assertSame($expected, $entity->extract(['id', 'title', 'created_by', 'modified_by'])); + $expected = [ + 'id' => $entity->id, + 'title' => 'without footprint', + 'created_by' => null, + 'modified_by' => null, + 'manager_id' => null, + ]; + + $this->assertSame($expected, $entity->extract(['id', 'title', 'created_by', 'modified_by', 'manager_id'])); + + // Satisfying closure manually still permits + // explicit field assignments + $entity = new Entity(['title' => 'different manager', 'company_id' => 1]); + $footprint = new Entity([ + 'id' => 3, + 'company' => new Entity(['id' => 5]), + 'manager' => new Entity(['id' => 4]), + ]); + + $entity = $this->Table->save($entity, ['_footprint' => $footprint]); + $expected = [ + 'id' => $entity->id, + 'title' => 'different manager', + 'created_by' => 3, + 'modified_by' => 3, + 'company_id' => 1, + 'manager_id' => 4, + ]; + + $this->assertSame($expected, $entity->extract(['id', 'title', 'created_by', 'modified_by', 'company_id', 'manager_id'])); } public function testFind() { - $result = $this->Table->find('all', _footprint: $this->footprint) + $footprint = new Entity(['id' => 4]); + + $result = $this->Table->find('all', _footprint: $footprint) ->enableHydration(false) ->first(); - $expected = ['id' => 3, 'title' => 'article 3', 'created_by' => 2, 'modified_by' => 1]; + $expected = [ + 'id' => 4, + 'title' => 'find article', + 'created_by' => 4, + 'modified_by' => 4, + 'company_id' => 2, + 'manager_id' => null, + ]; $this->assertSame($expected, $result); // Test to show value of "id" is not used from footprint if // "Articles.created_by" is already set in condition. - $result = $this->Table->find('all', _footprint: $this->footprint) - ->where(['Articles.created_by' => 1]) + $result = $this->Table->find('all', _footprint: $footprint) + ->where(['Articles.created_by' => 3]) ->enableHydration(false) ->first(); - $expected = ['id' => 1, 'title' => 'article 1', 'created_by' => 1, 'modified_by' => 1]; + $expected = [ + 'id' => 5, + 'title' => 'final article', + 'created_by' => 3, + 'modified_by' => 4, + 'company_id' => 4, + 'manager_id' => null, + ]; + $this->assertSame($expected, $result); + + // Test to show value of "id" is not used from footprint even + // "Articles.manager_id" validates the Model.beforeSave closure + $result = $this->Table->find('all', _footprint: $footprint) + ->where(['Articles.company_id' => 1]) + ->enableHydration(false) + ->first(); + + $expected = [ + 'id' => 6, + 'title' => 'penultimate article', + 'created_by' => 4, + 'modified_by' => 4, + 'company_id' => 1, + 'manager_id' => null, + ]; $this->assertSame($expected, $result); } public function testInjectEntityException() { $this->expectException('UnexpectedValueException'); - $this->expectExceptionMessage('When should be one of "always", "new" or "existing". The passed value "invalid" is invalid'); + $this->expectExceptionMessage('When should be one of "always", "new" or "existing", or a closure that takes an EntityInterface and returns a bool. The passed value "invalid" is invalid'); + + $footprint = new Entity([ + 'id' => 2, + ]); $this->Table->behaviors()->Footprint->setConfig( 'events', @@ -113,6 +207,6 @@ public function testInjectEntityException() ] ); $entity = new Entity(['title' => 'new article']); - $entity = $this->Table->save($entity, ['_footprint' => $this->footprint]); + $entity = $this->Table->save($entity, ['_footprint' => $footprint]); } } diff --git a/tests/schema.php b/tests/schema.php index a32612c..2bf56cb 100644 --- a/tests/schema.php +++ b/tests/schema.php @@ -9,6 +9,8 @@ 'title' => ['type' => 'string', 'length' => 255], 'created_by' => ['type' => 'integer'], 'modified_by' => ['type' => 'integer'], + 'company_id' => ['type' => 'integer'], + 'manager_id' => ['type' => 'integer'], ], 'constraints' => [ 'primary' => [ From 64092bc52d213f4a91c7b9f8802f7200465087e9 Mon Sep 17 00:00:00 2001 From: Jared Harder Date: Tue, 24 Oct 2023 23:08:56 -0600 Subject: [PATCH 2/4] Change is_callable to 'instanceof Closure', cs-stan fixes --- src/Model/Behavior/FootprintBehavior.php | 5 +++-- tests/Fixture/ArticlesFixture.php | 2 +- tests/TestCase/Model/Behavior/FootprintBehaviorTest.php | 8 +++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Model/Behavior/FootprintBehavior.php b/src/Model/Behavior/FootprintBehavior.php index bd8dc37..76d60d4 100644 --- a/src/Model/Behavior/FootprintBehavior.php +++ b/src/Model/Behavior/FootprintBehavior.php @@ -11,6 +11,7 @@ use Cake\ORM\Behavior; use Cake\ORM\Query\SelectQuery; use Cake\Utility\Hash; +use Closure; use UnexpectedValueException; class FootprintBehavior extends Behavior @@ -163,7 +164,7 @@ protected function _injectEntity(EntityInterface $entity, ArrayObject $options, $new = $entity->isNew() !== false; foreach ($fields as $field => $when) { - if (!in_array($when, ['always', 'new', 'existing']) && !is_callable($when)) { + if (!in_array($when, ['always', 'new', 'existing']) && !($when instanceof Closure)) { throw new UnexpectedValueException(sprintf( 'When should be one of "always", "new" or "existing", or a closure that takes an EntityInterface and returns a bool. The passed value "%s" is invalid', $when @@ -178,7 +179,7 @@ protected function _injectEntity(EntityInterface $entity, ArrayObject $options, $when === 'always' || ($when === 'new' && $new) || ($when === 'existing' && !$new) || - (is_callable($when) && $when($entity)) + ($when instanceof Closure && $when($entity)) ) { $entity->set( $field, diff --git a/tests/Fixture/ArticlesFixture.php b/tests/Fixture/ArticlesFixture.php index 2751d51..1bee04e 100644 --- a/tests/Fixture/ArticlesFixture.php +++ b/tests/Fixture/ArticlesFixture.php @@ -47,6 +47,6 @@ class ArticlesFixture extends TestFixture 'modified_by' => 4, 'company_id' => 1, 'manager_id' => null, - ] + ], ]; } diff --git a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php index 3c6e247..d1a5394 100644 --- a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php +++ b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php @@ -30,12 +30,12 @@ public function setUp(): void 'company_id' => 'always', 'manager_id' => function ($entity): bool { return $entity->company_id == 1; - } + }, ], 'Model.beforeFind' => [ 'created_by', 'company_id', - ] + ], ], 'propertiesMap' => [ 'company_id' => '_footprint.company.id', @@ -192,7 +192,9 @@ public function testFind() public function testInjectEntityException() { $this->expectException('UnexpectedValueException'); - $this->expectExceptionMessage('When should be one of "always", "new" or "existing", or a closure that takes an EntityInterface and returns a bool. The passed value "invalid" is invalid'); + $this->expectExceptionMessage('When should be one of "always", "new" or "existing", ' . + . 'or a closure that takes an EntityInterface and returns a bool. ' . + . 'The passed value "invalid" is invalid'); $footprint = new Entity([ 'id' => 2, From e8ffbeed9e32aa0009fbe027cf06fa32e861dbde Mon Sep 17 00:00:00 2001 From: Jared Harder Date: Tue, 24 Oct 2023 23:14:26 -0600 Subject: [PATCH 3/4] String concatenation is hard --- tests/TestCase/Model/Behavior/FootprintBehaviorTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php index d1a5394..d8d4eda 100644 --- a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php +++ b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php @@ -193,8 +193,8 @@ public function testInjectEntityException() { $this->expectException('UnexpectedValueException'); $this->expectExceptionMessage('When should be one of "always", "new" or "existing", ' . - . 'or a closure that takes an EntityInterface and returns a bool. ' . - . 'The passed value "invalid" is invalid'); + 'or a closure that takes an EntityInterface and returns a bool. ' . + 'The passed value "invalid" is invalid'); $footprint = new Entity([ 'id' => 2, From 19e5b0884f7a805acc8effafd2de5d7c08b89931 Mon Sep 17 00:00:00 2001 From: Jared Harder Date: Thu, 9 Nov 2023 20:45:30 -0600 Subject: [PATCH 4/4] cd-stan fix --- src/Model/Behavior/FootprintBehavior.php | 4 +++- tests/TestCase/Model/Behavior/FootprintBehaviorTest.php | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Model/Behavior/FootprintBehavior.php b/src/Model/Behavior/FootprintBehavior.php index 76d60d4..1ddd253 100644 --- a/src/Model/Behavior/FootprintBehavior.php +++ b/src/Model/Behavior/FootprintBehavior.php @@ -166,7 +166,9 @@ protected function _injectEntity(EntityInterface $entity, ArrayObject $options, foreach ($fields as $field => $when) { if (!in_array($when, ['always', 'new', 'existing']) && !($when instanceof Closure)) { throw new UnexpectedValueException(sprintf( - 'When should be one of "always", "new" or "existing", or a closure that takes an EntityInterface and returns a bool. The passed value "%s" is invalid', + 'When should be one of "always", "new" or "existing", ' . + 'or a closure that takes an EntityInterface and returns a bool. ' . + 'The passed value "%s" is invalid.', $when )); } diff --git a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php index d8d4eda..833ac54 100644 --- a/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php +++ b/tests/TestCase/Model/Behavior/FootprintBehaviorTest.php @@ -194,7 +194,7 @@ public function testInjectEntityException() $this->expectException('UnexpectedValueException'); $this->expectExceptionMessage('When should be one of "always", "new" or "existing", ' . 'or a closure that takes an EntityInterface and returns a bool. ' . - 'The passed value "invalid" is invalid'); + 'The passed value "invalid" is invalid.'); $footprint = new Entity([ 'id' => 2,