diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e3cff75..4975c48 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -14,7 +14,7 @@ jobs: with: command: install only_args: -q --no-ansi --no-interaction --no-scripts --no-progress --prefer-dist --ignore-platform-reqs - php_version: 8.1 + php_version: 8.3 - name: Run PHP-CS-Fixer run: vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.php --diff --dry-run @@ -30,7 +30,7 @@ jobs: with: command: install only_args: -q --no-ansi --no-interaction --no-scripts --no-progress --prefer-dist --ignore-platform-reqs - php_version: 8.1 + php_version: 8.3 - name: Run PHP CodeSniffer run: vendor/bin/phpcs --extensions=php diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml new file mode 100644 index 0000000..df395ca --- /dev/null +++ b/.github/workflows/static-analysis.yml @@ -0,0 +1,66 @@ +name: Static Analysis + +on: [ push ] + +jobs: + PHPUnit: + strategy: + matrix: + include: + # Laravel 10.* + - php: 8.1 + laravel: 10.* + composer-flag: '--prefer-stable' + - php: 8.2 + laravel: 10.* + composer-flag: '--prefer-stable' + - php: 8.3 + laravel: 10.* + composer-flag: '--prefer-stable' + - php: 8.1 + laravel: 10.* + composer-flag: '--prefer-lowest' + - php: 8.2 + laravel: 10.* + composer-flag: '--prefer-lowest' + - php: 8.3 + laravel: 10.* + composer-flag: '--prefer-lowest' + # Laravel 11.* + - php: 8.2 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-stable' + - php: 8.3 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-stable' + - php: 8.2 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-lowest' + - php: 8.2 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-lowest' + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + extensions: curl, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, iconv + coverage: none + + - name: Install dependencies + run: composer require "laravel/framework:${{ matrix.laravel }}" --no-interaction --no-update + + - name: Update dependencies + run: composer update ${{ matrix.composer-flag }} --prefer-dist --no-interaction + + - name: Run PhpStan + run: composer analyse diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index 9d99594..04a399f 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -16,6 +16,10 @@ jobs: laravel: 10.* testbench: 8.* composer-flag: '--prefer-stable' + - php: 8.3 + laravel: 10.* + testbench: 8.* + composer-flag: '--prefer-stable' - php: 8.1 laravel: 10.* testbench: 8.* @@ -24,6 +28,27 @@ jobs: laravel: 10.* testbench: 8.* composer-flag: '--prefer-lowest' + - php: 8.3 + laravel: 10.* + testbench: 8.* + composer-flag: '--prefer-lowest' + # Laravel 11.* + - php: 8.2 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-stable' + - php: 8.3 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-stable' + - php: 8.2 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-lowest' + - php: 8.2 + laravel: 11.* + testbench: 9.* + composer-flag: '--prefer-lowest' runs-on: ubuntu-latest @@ -47,4 +72,6 @@ jobs: run: XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-text --coverage-clover=coverage.xml - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/README.md b/README.md index 00f5e05..83319e8 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ composer require korridor/laravel-has-many-merged "^1" This package is tested for the following Laravel and PHP versions: - - 10.* (PHP 8.1, 8.2) + - 10.* (PHP 8.1, 8.2, 8.3) + - 11.* (PHP 8.2, 8.3) ## Usage diff --git a/composer.json b/composer.json index e8f46d7..94209a4 100644 --- a/composer.json +++ b/composer.json @@ -17,16 +17,16 @@ ], "require": { "php": ">=8.1", - "illuminate/database": "^10", - "illuminate/support": "^10" + "illuminate/database": "^10|^11", + "illuminate/support": "^10|^11" }, "require-dev": { - "orchestra/testbench": "^8", - "phpunit/phpunit": "^10.0", "friendsofphp/php-cs-fixer": "^3", + "larastan/larastan": "^2.0", + "orchestra/testbench": "^8|^9", + "phpunit/phpunit": "^10.0", "squizlabs/php_codesniffer": "^3.5" }, - "minimum-stability": "stable", "autoload": { "psr-4": { "Korridor\\LaravelHasManySync\\": "src" @@ -51,9 +51,14 @@ "@php vendor/bin/phpunit --coverage-html coverage" ], "fix": "@php ./vendor/bin/php-cs-fixer fix", - "lint": "@php ./vendor/bin/phpcs --extensions=php" + "lint": "@php ./vendor/bin/phpcs --extensions=php", + "analyse": [ + "@php ./vendor/bin/phpstan analyse --memory-limit=2G" + ] }, "config": { "sort-packages": true - } + }, + "minimum-stability": "dev", + "prefer-stable": true } diff --git a/docker/Dockerfile b/docker/Dockerfile index a5d71e1..86e2c84 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,4 +1,4 @@ -FROM php:8.1-cli +FROM php:8.3-cli ADD https://github.com/mlocati/docker-php-extension-installer/releases/latest/download/install-php-extensions /usr/local/bin/ diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..4037cf0 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,11 @@ +includes: + - ./vendor/larastan/larastan/extension.neon + +parameters: + + paths: + - src + - tests + + # Level 9 is the highest level + level: 9 diff --git a/src/ServiceProvider.php b/src/ServiceProvider.php index 4c550b6..4598523 100644 --- a/src/ServiceProvider.php +++ b/src/ServiceProvider.php @@ -4,6 +4,8 @@ namespace Korridor\LaravelHasManySync; +use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Support\Arr; use Illuminate\Support\ServiceProvider as BaseServiceProvider; @@ -17,37 +19,51 @@ class ServiceProvider extends BaseServiceProvider */ public function boot(): void { - HasMany::macro('sync', function (array $data, bool $deleting = true): array { + + HasMany::macro('sync', function (array $data, bool $deleting = true, bool $throwOnIdNotInScope = true): array { $changes = [ 'created' => [], 'deleted' => [], 'updated' => [], ]; - /** @var HasMany $this */ + /** @var HasMany $this */ // Get the primary key. $relatedKeyName = $this->getRelated()->getKeyName(); // Get the current key values. - $current = $this->newQuery()->pluck($relatedKeyName)->all(); + $currentIds = $this->newQuery()->pluck($relatedKeyName)->all(); // Cast the given key to an integer if it is numeric. $castKey = function ($value) { if (is_null($value)) { - return $value; + return null; } return is_numeric($value) ? (int) $value : (string) $value; }; // Cast the given keys to integers if they are numeric and string otherwise. - $castKeys = function ($keys) use ($castKey) { + $castKeys = function ($keys) use ($castKey): array { return (array) array_map(function ($key) use ($castKey) { return $castKey($key); }, $keys); }; + // The new ids, without null values + $dataIds = Arr::where($castKeys(Arr::pluck($data, $relatedKeyName)), function ($value) { + return !is_null($value); + }); + + $problemKeys = array_diff($dataIds, $currentIds); + if ($throwOnIdNotInScope && count($problemKeys) > 0) { + throw (new ModelNotFoundException())->setModel( + get_class($this->getRelated()), + $problemKeys + ); + } + // Get any non-matching rows. - $deletedKeys = array_diff($current, $castKeys(Arr::pluck($data, $relatedKeyName))); + $deletedKeys = array_diff($currentIds, $dataIds); if ($deleting && count($deletedKeys) > 0) { $this->getRelated()->destroy($deletedKeys); @@ -56,13 +72,15 @@ public function boot(): void // Separate the submitted data into "update" and "new" // We determine "newRows" as those whose $relatedKeyName (usually 'id') is null. - $newRows = Arr::where($data, function ($row) use ($relatedKeyName) { - return null === Arr::get($row, $relatedKeyName); + $newRows = Arr::where($data, function (array $row) use ($relatedKeyName) { + $id = Arr::get($row, $relatedKeyName); + return $id === null; }); // We determine "updateRows" as those whose $relatedKeyName (usually 'id') is set, not null. - $updatedRows = Arr::where($data, function ($row) use ($relatedKeyName) { - return null !== Arr::get($row, $relatedKeyName); + $updateRows = Arr::where($data, function (array $row) use ($relatedKeyName, $problemKeys) { + $id = Arr::get($row, $relatedKeyName); + return $id !== null && !in_array($id, $problemKeys, true); }); if (count($newRows) > 0) { @@ -72,14 +90,16 @@ public function boot(): void ); } - foreach ($updatedRows as $row) { - $this->getRelated() + foreach ($updateRows as $row) { + $updateModel = $this->getRelated() + ->newQuery() ->where($relatedKeyName, $castKey(Arr::get($row, $relatedKeyName))) - ->first() - ->update($row); + ->firstOrFail(); + + $updateModel->update($row); } - $changes['updated'] = $castKeys(Arr::pluck($updatedRows, $relatedKeyName)); + $changes['updated'] = $castKeys(Arr::pluck($updateRows, $relatedKeyName)); return $changes; }); diff --git a/tests/HasManySyncTest.php b/tests/HasManySyncTest.php index 26d9207..2bac3c9 100644 --- a/tests/HasManySyncTest.php +++ b/tests/HasManySyncTest.php @@ -5,14 +5,13 @@ namespace Korridor\LaravelHasManySync\Tests; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Testing\RefreshDatabase; use Korridor\LaravelHasManySync\Tests\TestEnvironment\Models\Task; use Korridor\LaravelHasManySync\Tests\TestEnvironment\Models\User; class HasManySyncTest extends TestCase { - use RefreshDatabase; - private function createTwoUsersWithTwoTasksEach(): void { Model::unguard(); @@ -75,29 +74,29 @@ public function testHasManySyncCreatesUpdatesAndDeletesIfDeletingIsActivated(): // Assert $this->assertEquals(4, Task::query()->count()); $this->assertEquals(5, Task::withTrashed()->count()); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'content' => 'Tasks 3 of Tester 1', 'user_id' => 1, 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 1, 'user_id' => 1, 'content' => 'Task 1 of Tester 1', ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 2, 'user_id' => 1, 'content' => 'Updated Task 2 of Tester 1', 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 3, 'user_id' => 2, 'content' => 'Task 1 of Tester 2', 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 4, 'user_id' => 2, 'content' => 'Task 2 of Tester 2', @@ -132,34 +131,168 @@ public function testHasManySyncCreatesUpdatesAndButDoesNotDeleteIfDeletingIsDeac // Assert $this->assertEquals(5, Task::query()->count()); $this->assertEquals(5, Task::withTrashed()->count()); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'content' => 'Tasks 3 of Tester 1', 'user_id' => 1, 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 1, 'user_id' => 1, 'content' => 'Task 1 of Tester 1', 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 2, 'user_id' => 1, 'content' => 'Updated Task 2 of Tester 1', 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 3, 'user_id' => 2, 'content' => 'Task 1 of Tester 2', 'deleted_at' => null, ]); - $this->assertDatabaseHas('tasks', [ + $this->assertDatabaseHas(Task::class, [ 'id' => 4, 'user_id' => 2, 'content' => 'Task 2 of Tester 2', 'deleted_at' => null, ]); } + + public function testUpdatedElementWithNotExistingIdIsFails(): void + { + // Arrange + $this->createTwoUsersWithTwoTasksEach(); + /** @var User $user1 */ + $user1 = User::query()->find(1); + /** @var User $user2 */ + $user2 = User::query()->find(2); + + // Act + try { + $user1->tasks()->sync([ + // Create + [ + 'id' => null, + 'content' => 'Tasks 3 of Tester 1', + ], + // Update + [ + 'id' => 100000, + 'content' => 'Updated Task 2 of Tester 1', + ], + // Delete, because task with id=1 is missing + ], false); + } catch (\Throwable $throwable) { + // Assert + $this->assertInstanceOf(ModelNotFoundException::class, $throwable); + return; + } + $this->fail(); + } + + public function testUpdateOnIdThatDoesNotBelongToRelationFails(): void + { + // Arrange + $this->createTwoUsersWithTwoTasksEach(); + /** @var User $user1 */ + $user1 = User::query()->find(1); + /** @var User $user2 */ + $user2 = User::query()->find(2); + + // Act + try { + $user1->tasks()->sync([ + // Create + [ + 'id' => null, + 'content' => 'Tasks 3 of Tester 1', + ], + // Update + [ + 'id' => 2, + 'content' => 'Updated Task 2 of Tester 1', + ], + [ + 'id' => 3, + 'content' => 'Updated Task 3 of Tester 2', + ] + // Delete, because task with id=1 is missing + ], true); + } catch (\Throwable $throwable) { + // Assert + $this->assertInstanceOf(ModelNotFoundException::class, $throwable); + } + + // Assert + $this->assertDatabaseMissing(Task::class, [ + 'content' => 'Tasks 3 of Tester 1', + 'user_id' => 1, + 'deleted_at' => null, + ]); + $this->assertDatabaseMissing(Task::class, [ + 'id' => 2, + 'content' => 'Updated Task 2 of Tester 1', + 'deleted_at' => null, + ]); + $this->assertDatabaseHas(Task::class, [ + 'id' => 3, + 'content' => 'Task 1 of Tester 2', + 'deleted_at' => null, + ]); + } + + public function testUpdateOnIdThatDoesNotBelongToRelationIgnoresTheEntryWithProblemIdIfOptionThrowOnIdNotInScopeIsDeactivated(): void + { + // Arrange + $this->createTwoUsersWithTwoTasksEach(); + /** @var User $user1 */ + $user1 = User::query()->find(1); + /** @var User $user2 */ + $user2 = User::query()->find(2); + + // Act + try { + $user1->tasks()->sync([ + // Create + [ + 'id' => null, + 'content' => 'Tasks 3 of Tester 1', + ], + // Update + [ + 'id' => 2, + 'content' => 'Updated Task 2 of Tester 1', + ], + [ + 'id' => 3, + 'content' => 'Updated Task 3 of Tester 2', + ] + // Delete, because task with id=1 is missing + ], true, false); + } catch (\Throwable $throwable) { + // Assert + $this->fail(); + } + + // Assert + $this->assertDatabaseHas(Task::class, [ + 'content' => 'Tasks 3 of Tester 1', + 'user_id' => 1, + 'deleted_at' => null, + ]); + $this->assertDatabaseHas(Task::class, [ + 'id' => 2, + 'content' => 'Updated Task 2 of Tester 1', + 'deleted_at' => null, + ]); + $this->assertDatabaseHas(Task::class, [ + 'id' => 3, + 'content' => 'Task 1 of Tester 2', + 'deleted_at' => null, + ]); + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index ff17225..d285f0c 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -4,6 +4,7 @@ namespace Korridor\LaravelHasManySync\Tests; +use Illuminate\Database\Capsule\Manager; use Korridor\LaravelHasManySync\ServiceProvider; use Illuminate\Foundation\Application; use Orchestra\Testbench\TestCase as Orchestra; diff --git a/tests/TestEnvironment/Models/Task.php b/tests/TestEnvironment/Models/Task.php index aefab63..31aa926 100644 --- a/tests/TestEnvironment/Models/Task.php +++ b/tests/TestEnvironment/Models/Task.php @@ -15,7 +15,7 @@ class Task extends Model /** * The attributes that are mass assignable. * - * @var string[] + * @var array */ protected $fillable = [ 'id', @@ -24,7 +24,7 @@ class Task extends Model ]; /** - * @return BelongsTo + * @return BelongsTo */ public function user(): BelongsTo { diff --git a/tests/TestEnvironment/Models/User.php b/tests/TestEnvironment/Models/User.php index a7ae030..7cb05da 100644 --- a/tests/TestEnvironment/Models/User.php +++ b/tests/TestEnvironment/Models/User.php @@ -15,7 +15,7 @@ class User extends Model /** * The attributes that are mass assignable. * - * @var string[] + * @var array */ protected $fillable = [ 'name',