From cd7ec988f28c03af05cd44a17a0d5ecf0eb9ecb6 Mon Sep 17 00:00:00 2001 From: Linus Kardell Date: Mon, 8 Jan 2024 21:54:07 +0100 Subject: [PATCH] Fix getId() on null in deleteConflictingEpisodeAction Previously, it would take the guid and try to search for that in the episode URL column, which may not find a match (or possibly even find the wrong match). testDoNotFailToUpdateEpisodeActionByGuidIfThereIsAnotherWithTheSameValueForEpisodeUrl didn't catch this issue because it used the same value for episode and guid when updating at line 84, so fix that as well. And for good measure, give the save actions different position values, so the asserts actually check that the saves have succeeded and they found the right episode. --- lib/Core/EpisodeAction/EpisodeActionSaver.php | 4 ++-- ...odeActionSaverGuidBackwardCompatibilityTest.php | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/Core/EpisodeAction/EpisodeActionSaver.php b/lib/Core/EpisodeAction/EpisodeActionSaver.php index adb2d58..2cea7ea 100644 --- a/lib/Core/EpisodeAction/EpisodeActionSaver.php +++ b/lib/Core/EpisodeAction/EpisodeActionSaver.php @@ -128,9 +128,9 @@ class EpisodeActionSaver */ private function deleteConflictingEpisodeAction(EpisodeActionEntity $episodeActionEntity, string $userId): void { - $collidingEpisodeActionId = $this->episodeActionRepository->findByEpisodeUrl($episodeActionEntity->getGuid(), $userId)->getId(); + $collidingEpisodeActionId = $this->episodeActionRepository->findByEpisodeUrl($episodeActionEntity->getEpisode(), $userId)->getId(); if ($collidingEpisodeActionId !== $episodeActionEntity->getId()) { - $this->episodeActionRepository->deleteEpisodeActionByEpisodeUrl($episodeActionEntity->getGuid(), $userId); + $this->episodeActionRepository->deleteEpisodeActionByEpisodeUrl($episodeActionEntity->getEpisode(), $userId); } } } diff --git a/tests/Integration/EpisodeActionSaverGuidBackwardCompatibilityTest.php b/tests/Integration/EpisodeActionSaverGuidBackwardCompatibilityTest.php index 47ad971..80f1a4e 100644 --- a/tests/Integration/EpisodeActionSaverGuidBackwardCompatibilityTest.php +++ b/tests/Integration/EpisodeActionSaverGuidBackwardCompatibilityTest.php @@ -55,39 +55,41 @@ class EpisodeActionSaverGuidBackwardCompatibilityTest extends TestCase $url = uniqid("https://podcast-mp3.dradio.de/"); $urlWithParameter = $url . "?ref=never_know_if_ill_be_removed"; + $guid1 = uniqid("quid1"); + $guid2 = uniqid("quid2"); $podcastUrl = uniqid("https://podcast"); $episodeActionSaver->saveEpisodeActions( - [["podcast" => $podcastUrl, "episode" => $url, "guid" => $urlWithParameter, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 100, "total" => 2252]], + [["podcast" => $podcastUrl, "episode" => $url, "guid" => $guid2, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 100, "total" => 2252]], self::USER_ID_0 )[0]; $episodeActionSaver->saveEpisodeActions( - [["podcast" => $podcastUrl, "episode" => $urlWithParameter, "guid" => $url, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 100, "total" => 2252]], + [["podcast" => $podcastUrl, "episode" => $urlWithParameter, "guid" => $guid1, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 101, "total" => 2252]], self::USER_ID_0 )[0]; //act $episodeActionSaver->saveEpisodeActions( - [["podcast" => $podcastUrl, "episode" => $urlWithParameter, "guid" => $url, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 100, "total" => 2252]], + [["podcast" => $podcastUrl, "episode" => $urlWithParameter, "guid" => $guid1, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 102, "total" => 2252]], self::USER_ID_0 )[0]; //assert /** @var EpisodeActionRepository $episodeActionRepository */ $episodeActionRepository = $this->container->get(EpisodeActionRepository::class); - $this->assertSame(100, $episodeActionRepository->findByGuid($urlWithParameter, self::USER_ID_0)->getPosition()); + $this->assertSame(100, $episodeActionRepository->findByGuid($guid2, self::USER_ID_0)->getPosition()); //act $episodeActionSaver->saveEpisodeActions( - [["podcast" => $podcastUrl, "episode" => $urlWithParameter, "guid" => $urlWithParameter, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 100, "total" => 2252]], + [["podcast" => $podcastUrl, "episode" => $urlWithParameter, "guid" => $guid2, "action" => "PLAY", "timestamp" => "2021-08-22T23:58:56", "started" => 35, "position" => 103, "total" => 2252]], self::USER_ID_0 )[0]; //assert /** @var EpisodeActionRepository $episodeActionRepository */ $episodeActionRepository = $this->container->get(EpisodeActionRepository::class); - $this->assertSame(100, $episodeActionRepository->findByGuid($urlWithParameter, self::USER_ID_0)->getPosition()); + $this->assertSame(103, $episodeActionRepository->findByGuid($guid2, self::USER_ID_0)->getPosition()); } }