From afe6202460c76a6e1e55e27f4d9d441c56d4e50b Mon Sep 17 00:00:00 2001 From: DariusIII Date: Fri, 26 Jun 2026 23:19:30 +0200 Subject: [PATCH] Fix multiple user id queries --- app/Http/Controllers/CartController.php | 31 ++++-- app/Models/UsersRelease.php | 51 +++++++-- tests/Feature/CartQueryCountTest.php | 137 ++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 22 deletions(-) create mode 100644 tests/Feature/CartQueryCountTest.php diff --git a/app/Http/Controllers/CartController.php b/app/Http/Controllers/CartController.php index f49bf15b6..cde34bf26 100644 --- a/app/Http/Controllers/CartController.php +++ b/app/Http/Controllers/CartController.php @@ -36,24 +36,33 @@ class CartController extends BasePageController */ public function store(Request $request): RedirectResponse|JsonResponse { - $guids = explode(',', $request->input('id')); + $guids = collect(explode(',', (string) $request->input('id'))) + ->map(static fn (string $guid): string => trim($guid)) + ->filter() + ->unique() + ->values(); - $data = Release::query()->whereIn('guid', $guids)->select(['id'])->get(); + $releaseIds = Release::query() + ->whereIn('guid', $guids) + ->pluck('id') + ->map(static fn ($id): int => (int) $id) + ->all(); - if ($data->isEmpty()) { + if ($releaseIds === []) { return $request->ajax() || $request->wantsJson() ? response()->json(['success' => false, 'message' => 'No releases found'], 404) : redirect()->to('/cart/index'); } - $addedCount = 0; - foreach ($data as $d) { - $check = UsersRelease::whereReleasesId($d['id'])->first(); - if (empty($check)) { - UsersRelease::addCart($this->userdata->id, $d['id']); - $addedCount++; - } - } + $existingReleaseIds = UsersRelease::query() + ->where('users_id', $this->userdata->id) + ->whereIn('releases_id', $releaseIds) + ->pluck('releases_id') + ->map(static fn ($id): int => (int) $id) + ->all(); + + $missingReleaseIds = array_values(array_diff($releaseIds, $existingReleaseIds)); + $addedCount = UsersRelease::addCartForReleases((int) $this->userdata->id, $missingReleaseIds); if ($request->ajax() || $request->wantsJson()) { return response()->json([ diff --git a/app/Models/UsersRelease.php b/app/Models/UsersRelease.php index 6fa598b95..9292778f3 100644 --- a/app/Models/UsersRelease.php +++ b/app/Models/UsersRelease.php @@ -77,6 +77,31 @@ class UsersRelease extends Model ); } + /** + * Add multiple releases to a user's cart in one insert statement. + * + * @param list $releaseIds + */ + public static function addCartForReleases(int $uid, array $releaseIds): int + { + if ($releaseIds === []) { + return 0; + } + + $now = now(); + $rows = array_map( + static fn (int $releaseId): array => [ + 'users_id' => $uid, + 'releases_id' => $releaseId, + 'created_at' => $now, + 'updated_at' => $now, + ], + array_values(array_unique($releaseIds)) + ); + + return self::query()->insertOrIgnore($rows); + } + public static function getCart(mixed $uid): mixed { return self::query() @@ -95,23 +120,27 @@ class UsersRelease extends Model return false; } - $del = []; - foreach ($guids as $guid) { - $rel = Release::query()->where('guid', $guid)->first(['id']); - if ($rel !== null) { - $del[] = $rel['id']; - } + $guids = array_values(array_unique(array_filter(array_map( + static fn ($guid): string => trim((string) $guid), + $guids + )))); + + if ($guids === []) { + return false; } - return self::query()->whereIn('releases_id', $del)->where('users_id', $userID)->delete() === 1; + return self::query() + ->where('users_id', $userID) + ->whereIn('releases_id', Release::query()->select('id')->whereIn('guid', $guids)) + ->delete() > 0; } public static function delCartByUserAndRelease(mixed $guid, mixed $uid): void { - $rel = Release::query()->where('guid', $guid)->first(['id']); - if ($rel) { - self::query()->where(['users_id' => $uid, 'releases_id' => $rel['id']])->delete(); - } + self::query() + ->where('users_id', $uid) + ->whereIn('releases_id', Release::query()->select('id')->where('guid', $guid)) + ->delete(); } /** diff --git a/tests/Feature/CartQueryCountTest.php b/tests/Feature/CartQueryCountTest.php new file mode 100644 index 000000000..cfc3b9c66 --- /dev/null +++ b/tests/Feature/CartQueryCountTest.php @@ -0,0 +1,137 @@ + 'sqlite', 'database.connections.sqlite.database' => ':memory:']); + DB::purge(); + DB::reconnect(); + + Schema::create('settings', static function (Blueprint $table): void { + $table->string('section')->nullable(); + $table->string('subsection')->nullable(); + $table->string('name')->primary(); + $table->text('value')->nullable(); + $table->text('hint')->nullable(); + $table->text('setting')->nullable(); + }); + + Schema::create('releases', static function (Blueprint $table): void { + $table->id(); + $table->string('guid')->unique(); + }); + + Schema::create('users_releases', static function (Blueprint $table): void { + $table->id(); + $table->unsignedInteger('users_id'); + $table->unsignedInteger('releases_id'); + $table->timestamps(); + $table->unique(['users_id', 'releases_id']); + }); + } + + public function test_store_batches_cart_add_queries_and_scopes_existing_rows_to_current_user(): void + { + $user = new User; + $user->forceFill(['id' => 1]); + + $otherUserId = 2; + $releaseIdsByGuid = $this->createReleases(12); + $guids = array_keys($releaseIdsByGuid); + + UsersRelease::addCart($user->id, $releaseIdsByGuid[$guids[0]]); + UsersRelease::addCart($otherUserId, $releaseIdsByGuid[$guids[1]]); + + $controller = new CartController; + $controller->userdata = $user; + + $request = Request::create('/cart/add', 'POST', ['id' => implode(',', $guids)]); + $request->headers->set('Accept', 'application/json'); + + $counts = $this->countQueriesPerTable(function () use ($controller, $request, &$response): void { + $response = $controller->store($request); + }); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame(12, $response->getData(true)['cartCount']); + $this->assertSame(12, UsersRelease::query()->where('users_id', $user->id)->count()); + $this->assertSame(1, UsersRelease::query()->where('users_id', $otherUserId)->count()); + + $this->assertLessThanOrEqual(1, $counts['releases'] ?? 0); + $this->assertLessThanOrEqual(3, $counts['users_releases'] ?? 0); + } + + public function test_delete_by_guid_uses_one_set_based_delete_for_multiple_cart_items(): void + { + $userId = 1; + $otherUserId = 2; + $releaseIdsByGuid = $this->createReleases(5); + $guids = array_keys($releaseIdsByGuid); + + foreach ($releaseIdsByGuid as $releaseId) { + UsersRelease::addCart($userId, $releaseId); + UsersRelease::addCart($otherUserId, $releaseId); + } + + $counts = $this->countQueriesPerTable(function () use ($guids, $userId, &$deleted): void { + $deleted = UsersRelease::delCartByGuid($guids, $userId); + }); + + $this->assertTrue($deleted); + $this->assertSame(0, UsersRelease::query()->where('users_id', $userId)->count()); + $this->assertSame(5, UsersRelease::query()->where('users_id', $otherUserId)->count()); + $this->assertSame(1, array_sum($counts)); + $this->assertSame(1, $counts['users_releases'] ?? 0); + } + + /** + * @return array + */ + private function createReleases(int $count): array + { + $releaseIdsByGuid = []; + + for ($i = 1; $i <= $count; $i++) { + $guid = 'cart-query-count-guid-'.$i; + $release = Release::query()->create(['guid' => $guid]); + $releaseIdsByGuid[$guid] = (int) $release->id; + } + + return $releaseIdsByGuid; + } + + /** + * @return array + */ + private function countQueriesPerTable(\Closure $callable): array + { + $counts = []; + $listener = static function (QueryExecuted $event) use (&$counts): void { + if (preg_match('/\b(?:from|into|update|delete\s+from)\s+["`]?([a-zA-Z_][a-zA-Z0-9_]*)["`]?/i', $event->sql, $m)) { + $table = strtolower($m[1]); + $counts[$table] = ($counts[$table] ?? 0) + 1; + } + }; + + DB::listen($listener); + $callable(); + + return $counts; + } +}