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

Remove scheduleExtraUpdate calls #464

Merged
merged 6 commits into from
Dec 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ It currently handles:

* [Blameable](/docs/blameable.md)
* [Geocodable](/docs/geocodable.md)
* joinable
* [Loggable](/docs/loggable.md)
* [Sluggable](/docs/sluggable.md)
* [SoftDeletable](/docs/soft-deletable.md)
* sortable
* [Sortable](/docs/sortable.md)
* [Uuidable](/docs/uuidable.md)
* [Timestampable](/docs/timestampable.md)
* [Translatable](/docs/translatable.md)
Expand Down
10 changes: 5 additions & 5 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"jakub-onderka/php-parallel-lint": "^1.0",
"phpstan/phpstan": "^0.12.2",
"phpunit/phpunit": "^8.5",
"rector/rector": "^0.6.1",
"rector/rector": "^0.6.2",
"symplify/easy-coding-standard": "^7.1",
"symplify/phpstan-extensions": "^7.1",
"phpstan/phpstan-doctrine": "^0.12.4",
Expand All @@ -52,9 +52,9 @@
}
},
"scripts": {
"check-cs": "ecs check src tests --ansi",
"fix-cs": "ecs check src tests --fix --ansi",
"phpstan": "phpstan analyse src tests --ansi --error-format symplify",
"rector": "rector process --config rector-ci.yaml"
"check-cs": "vendor/bin/ecs check src tests --ansi",
"fix-cs": "vendor/bin/ecs check src tests --fix --ansi",
"phpstan": "vendor/bin/phpstan analyse src tests --ansi --error-format symplify",
"rector": "vendor/bin/rector process --config rector-ci.yaml"
}
}
38 changes: 38 additions & 0 deletions docs/sortable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Sortable

@todo

## Entity

```php
<?php

declare(strict_types=1);

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Knp\DoctrineBehaviors\Contract\Entity\SortableInterface;
use Knp\DoctrineBehaviors\Model\Sortable\SortableTrait;

/**
* @ORM\Entity
*/
class Category implements SortableInterface
{
use SortableTrait;
}
```

## Usage

@todo

```php
$category = new Category;

$entityManager->persist($category);
$entityManager->flush();

$category->getSort(); // 1
```
3 changes: 3 additions & 0 deletions ecs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ parameters:
- "*Repository.php"

services:
Symplify\CodingStandard\Sniffs\CleanCode\CognitiveComplexitySniff:
max_cognitive_complexity: 8

PhpCsFixer\Fixer\ClassNotation\FinalClassFixer: ~

# see single LICENSE.txt file in the root directory
Expand Down
12 changes: 0 additions & 12 deletions src/ORM/Blameable/BlameableSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@ public function prePersist(LifecycleEventArgs $lifecycleEventArgs): void
$entity->setCreatedBy($user);

$this->unitOfWork->propertyChanged($entity, self::CREATED_BY, null, $user);
$this->unitOfWork->scheduleExtraUpdate($entity, [
self::CREATED_BY => [null, $user],
]);
}
}

Expand All @@ -89,9 +86,6 @@ public function prePersist(LifecycleEventArgs $lifecycleEventArgs): void
$entity->setUpdatedBy($user);

$this->unitOfWork->propertyChanged($entity, self::UPDATED_BY, null, $user);
$this->unitOfWork->scheduleExtraUpdate($entity, [
self::UPDATED_BY => [null, $user],
]);
}
}
}
Expand All @@ -115,9 +109,6 @@ public function preUpdate(LifecycleEventArgs $lifecycleEventArgs): void
$entity->setUpdatedBy($user);

$this->unitOfWork->propertyChanged($entity, self::UPDATED_BY, $oldValue, $user);
$this->unitOfWork->scheduleExtraUpdate($entity, [
self::UPDATED_BY => [$oldValue, $user],
]);
}

/**
Expand All @@ -136,9 +127,6 @@ public function preRemove(LifecycleEventArgs $lifecycleEventArgs): void
$entity->setDeletedBy($user);

$this->unitOfWork->propertyChanged($entity, self::DELETED_BY, $oldValue, $user);
$this->unitOfWork->scheduleExtraUpdate($entity, [
self::DELETED_BY => [$oldValue, $user],
]);
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/ORM/Geocodable/GeocodableSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ private function updateLocation(LifecycleEventArgs $lifecycleEventArgs, $overrid
}

$unitOfWork->propertyChanged($entity, 'location', $oldValue, $entity->getLocation());

$unitOfWork->scheduleExtraUpdate($entity, [
'location' => [$oldValue, $entity->getLocation()],
]);
}
}
}
15 changes: 15 additions & 0 deletions tests/AbstractBehaviorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Knp\DoctrineBehaviors\Tests;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Logging\DebugStack;
use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
use Doctrine\ORM\EntityManagerInterface;
use Knp\DoctrineBehaviors\Tests\HttpKernel\DoctrineBehaviorsKernel;
Expand All @@ -17,6 +18,11 @@ abstract class AbstractBehaviorTestCase extends AbstractKernelTestCase
*/
protected $entityManager;

/**
* @var DebugStack
*/
protected $debugStack;

protected function setUp(): void
{
$customConfig = $this->provideCustomConfig();
Expand Down Expand Up @@ -49,4 +55,13 @@ protected function provideCustomConfig(): ?string
{
return null;
}

protected function enableDebugStackLogger(): void
{
$this->debugStack = new DebugStack();

$this->entityManager->getConnection()
->getConfiguration()
->setSQLLogger($this->debugStack);
}
}
41 changes: 41 additions & 0 deletions tests/ORM/BlameableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,20 @@ public function testUpdate(): void
/** @var BlameableEntity $entity */
$entity = $this->blameableRepository->find($id);

$this->enableDebugStackLogger();

$entity->setTitle('test'); // need to modify at least one column to trigger onUpdate
$this->entityManager->flush();
$this->entityManager->clear();

$this->assertCount(3, $this->debugStack->queries);
$this->assertSame('"START TRANSACTION"', $this->debugStack->queries[1]['sql']);
$this->assertSame(
'UPDATE BlameableEntity SET title = ?, updatedBy = ? WHERE id = ?',
$this->debugStack->queries[2]['sql']
);
$this->assertSame('"COMMIT"', $this->debugStack->queries[3]['sql']);

/** @var BlameableEntity $entity */
$entity = $this->blameableRepository->find($id);

Expand Down Expand Up @@ -94,4 +104,35 @@ public function testRemove(): void

$this->assertSame('user3', $entity->getDeletedBy());
}

public function testExtraSqlCalls(): void
{
$entity = new BlameableEntity();

$this->enableDebugStackLogger();

$this->entityManager->persist($entity);
$this->entityManager->flush();

$expectedCount = $this->isPostgreSql() ? 4 : 3;
$startKey = $this->isPostgreSql() ? 2 : 1;

$this->assertCount($expectedCount, $this->debugStack->queries);
$this->assertSame('"START TRANSACTION"', $this->debugStack->queries[$startKey]['sql']);

$sql2 = $this->debugStack->queries[$startKey + 1]['sql'];
if ($this->isPostgreSql()) {
$this->assertSame(
'INSERT INTO BlameableEntity (id, title, createdBy, updatedBy, deletedBy) VALUES (?, ?, ?, ?, ?)',
$sql2
);
} else {
$this->assertSame(
'INSERT INTO BlameableEntity (title, createdBy, updatedBy, deletedBy) VALUES (?, ?, ?, ?)',
$sql2
);
}

$this->assertSame('"COMMIT"', $this->debugStack->queries[$startKey + 2]['sql']);
}
}
24 changes: 24 additions & 0 deletions tests/ORM/SoftDeletableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,28 @@ public function testRestore(): void

$this->assertFalse($entity->isDeleted());
}

public function testExtraSqlCalls(): void
{
$entity = new SoftDeletableEntity();
$this->entityManager->persist($entity);
$this->entityManager->flush();

$id = $entity->getId();
$this->assertNotNull($id);
$this->assertFalse($entity->isDeleted());

$this->enableDebugStackLogger();

$this->entityManager->remove($entity);
$this->entityManager->flush();

$this->assertCount(3, $this->debugStack->queries);
$this->assertSame('"START TRANSACTION"', $this->debugStack->queries[1]['sql']);
$this->assertSame(
'UPDATE SoftDeletableEntity SET deletedAt = ? WHERE id = ?',
$this->debugStack->queries[2]['sql']
);
$this->assertSame('"COMMIT"', $this->debugStack->queries[3]['sql']);
}
}