fix #109 search for episode action by guid only (retry with episode url if nothing found) thus avoid multiple results

2024-12-23 21:53:07 +01:00 · 2023-02-24 16:48:16 +01:00 · 2023-02-24 16:48:16 +01:00 · a1893061ce
commit a1893061ce
parent 4999b96456
8 changed files with 141 additions and 116 deletions
--- a/lib/Core/EpisodeAction/EpisodeActionSaver.php
+++ b/lib/Core/EpisodeAction/EpisodeActionSaver.php
@ -13,107 +13,106 @@ use OCP\DB\Exception;
 class EpisodeActionSaver
 {

-	private EpisodeActionRepository $episodeActionRepository;
-	private EpisodeActionWriter $episodeActionWriter;
-	private EpisodeActionReader $episodeActionReader;
+    private EpisodeActionRepository $episodeActionRepository;
+    private EpisodeActionWriter $episodeActionWriter;
+    private EpisodeActionReader $episodeActionReader;

-	private const DATETIME_FORMAT = 'Y-m-d\TH:i:s';
+    private const DATETIME_FORMAT = 'Y-m-d\TH:i:s';

-	public function __construct(
-		EpisodeActionRepository $episodeActionRepository,
-		EpisodeActionWriter $episodeActionWriter,
-		EpisodeActionReader $episodeActionReader
-	)
-	{
-		$this->episodeActionRepository = $episodeActionRepository;
-		$this->episodeActionWriter = $episodeActionWriter;
-		$this->episodeActionReader = $episodeActionReader;
-	}
+    public function __construct(
+        EpisodeActionRepository $episodeActionRepository,
+        EpisodeActionWriter     $episodeActionWriter,
+        EpisodeActionReader     $episodeActionReader
+    )
+    {
+        $this->episodeActionRepository = $episodeActionRepository;
+        $this->episodeActionWriter = $episodeActionWriter;
+        $this->episodeActionReader = $episodeActionReader;
+    }

-	/**
-	 * @param array $episodeActionsArray
-	 * @param string $userId
-	 * @return EpisodeActionEntity[]
-	 */
-	public function saveEpisodeActions(array $episodeActionsArray, string $userId): array
-	{
-		$episodeActions = $this->episodeActionReader->fromArray($episodeActionsArray);
+    public function saveEpisodeActions(array $episodeActionsArray, string $userId): array
+    {
+        $episodeActions = $this->episodeActionReader->fromArray($episodeActionsArray);

-		$episodeActionEntities = [];
+        $episodeActionEntities = [];

        foreach ($episodeActions as $episodeAction) {
-			$episodeActionEntity = $this->hydrateEpisodeActionEntity($episodeAction, $userId);
+            $episodeActionEntity = $this->hydrateEpisodeActionEntity($episodeAction, $userId);

-			try {
+            try {
                $episodeActionEntities[] = $this->episodeActionWriter->save($episodeActionEntity);
-			} catch (UniqueConstraintViolationException $uniqueConstraintViolationException) {
-					$episodeActionEntities[] = $this->updateEpisodeAction($episodeActionEntity, $userId);
+            } catch (UniqueConstraintViolationException $uniqueConstraintViolationException) {
+                $episodeActionEntities[] = $this->updateEpisodeAction($episodeActionEntity, $userId);
            } catch (Exception $exception) {
                if ($exception->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
-						$episodeActionEntities[] = $this->updateEpisodeAction($episodeActionEntity, $userId);
-				}
+                    $episodeActionEntities[] = $this->updateEpisodeAction($episodeActionEntity, $userId);
+                }
            }
        }
-		return $episodeActionEntities;
-	}
+        return $episodeActionEntities;
+    }

-	private function convertTimestampToUnixEpoch(string $timestamp): string
-	{
-		return DateTime::createFromFormat(self::DATETIME_FORMAT, $timestamp)
-			->format("U");
-	}
+    private function convertTimestampToUnixEpoch(string $timestamp): string
+    {
+        return DateTime::createFromFormat(self::DATETIME_FORMAT, $timestamp)
+            ->format("U");
+    }

-	private function updateEpisodeAction(
-		EpisodeActionEntity $episodeActionEntity,
-		string $userId
-	): EpisodeActionEntity
-	{
-		$identifier = $episodeActionEntity->getGuid() ?? $episodeActionEntity->getEpisode();
-		$episodeActionToUpdate = $this->episodeActionRepository->findByEpisodeIdentifier(
-			$identifier,
-			$userId
-		);
+    private function updateEpisodeAction(
+        EpisodeActionEntity $episodeActionEntity,
+        string              $userId
+    ): EpisodeActionEntity
+    {
+        $episodeActionToUpdate = $this->findEpisodeActionToUpdate($episodeActionEntity, $userId);

-		if ($episodeActionToUpdate === null && $episodeActionEntity->getGuid() !== null) {
-			$episodeActionToUpdate = $this->getOldEpisodeActionByEpisodeUrl($episodeActionEntity->getEpisode(), $userId);
-		}
+        $episodeActionEntity->setId($episodeActionToUpdate->getId());

-		$episodeActionEntity->setId($episodeActionToUpdate->getId());
+        $this->ensureGuidDoesNotGetNulledWithOldData($episodeActionToUpdate, $episodeActionEntity);

-		$this->ensureGuidDoesNotGetNulledWithOldData($episodeActionToUpdate, $episodeActionEntity);
+        return $this->episodeActionWriter->update($episodeActionEntity);
+    }

-		return $this->episodeActionWriter->update($episodeActionEntity);
-	}
+    private function ensureGuidDoesNotGetNulledWithOldData(EpisodeAction $episodeActionToUpdate, EpisodeActionEntity $episodeActionEntity): void
+    {
+        $existingGuid = $episodeActionToUpdate->getGuid();
+        if ($existingGuid !== null && $episodeActionEntity->getGuid() === null) {
+            $episodeActionEntity->setGuid($existingGuid);
+        }
+    }

-	private function getOldEpisodeActionByEpisodeUrl(string $episodeUrl, string $userId): ?EpisodeAction
-	{
-		return $this->episodeActionRepository->findByEpisodeIdentifier(
-			$episodeUrl,
-			$userId
-		);
-	}
+    private function hydrateEpisodeActionEntity(EpisodeAction $episodeAction, string $userId): EpisodeActionEntity
+    {
+        $episodeActionEntity = new EpisodeActionEntity();
+        $episodeActionEntity->setPodcast($episodeAction->getPodcast());
+        $episodeActionEntity->setEpisode($episodeAction->getEpisode());
+        $episodeActionEntity->setGuid($episodeAction->getGuid());
+        $episodeActionEntity->setAction($episodeAction->getAction());
+        $episodeActionEntity->setPosition($episodeAction->getPosition());
+        $episodeActionEntity->setStarted($episodeAction->getStarted());
+        $episodeActionEntity->setTotal($episodeAction->getTotal());
+        $episodeActionEntity->setTimestampEpoch($this->convertTimestampToUnixEpoch($episodeAction->getTimestamp()));
+        $episodeActionEntity->setUserId($userId);

-	private function ensureGuidDoesNotGetNulledWithOldData(EpisodeAction $episodeActionToUpdate, EpisodeActionEntity $episodeActionEntity): void
-	{
-		$existingGuid = $episodeActionToUpdate->getGuid();
-		if ($existingGuid !== null && $episodeActionEntity->getGuid() === null) {
-			$episodeActionEntity->setGuid($existingGuid);
-		}
-	}
+        return $episodeActionEntity;
+    }

-	private function hydrateEpisodeActionEntity(EpisodeAction $episodeAction, string $userId): EpisodeActionEntity
-	{
-		$episodeActionEntity = new EpisodeActionEntity();
-		$episodeActionEntity->setPodcast($episodeAction->getPodcast());
-		$episodeActionEntity->setEpisode($episodeAction->getEpisode());
-		$episodeActionEntity->setGuid($episodeAction->getGuid());
-		$episodeActionEntity->setAction($episodeAction->getAction());
-		$episodeActionEntity->setPosition($episodeAction->getPosition());
-		$episodeActionEntity->setStarted($episodeAction->getStarted());
-		$episodeActionEntity->setTotal($episodeAction->getTotal());
-		$episodeActionEntity->setTimestampEpoch($this->convertTimestampToUnixEpoch($episodeAction->getTimestamp()));
-		$episodeActionEntity->setUserId($userId);
+    private function findEpisodeActionToUpdate(EpisodeActionEntity $episodeActionEntity, string $userId): ?EpisodeAction
+    {
+        $episodeAction = null;
+        if ($episodeActionEntity->getGuid() !== null) {
+            $episodeAction = $this->episodeActionRepository->findByGuid(
+                $episodeActionEntity->getGuid(),
+                $userId
+            );
+        }

-		return $episodeActionEntity;
-	}
+        if ($episodeAction === null) {
+            $episodeAction = $this->episodeActionRepository->findByEpisodeUrl(
+                $episodeActionEntity->getEpisode(),
+                $userId
+            );
+        }
+
+        return $episodeAction;
+    }
 }
--- a/lib/Db/EpisodeAction/EpisodeActionMapper.php
+++ b/lib/Db/EpisodeAction/EpisodeActionMapper.php
@ -43,20 +43,18 @@ class EpisodeActionMapper extends QBMapper
 	 * @param string $userId
 	 * @return EpisodeActionEntity|null
 	 */
-	public function findByEpisodeIdentifier(string $episodeIdentifier, string $userId) : ?EpisodeActionEntity
+	public function findByEpisodeUrl(string $episodeIdentifier, string $userId) : ?EpisodeActionEntity
 	{
 		$qb = $this->db->getQueryBuilder();

 		$qb->select('*')
 			->from($this->getTableName())
 			->where(
-				$qb->expr()->orX(
-					$qb->expr()->eq('episode', $qb->createNamedParameter($episodeIdentifier)),
-					$qb->expr()->eq('guid', $qb->createNamedParameter($episodeIdentifier)))
-			)
+                $qb->expr()->eq('episode', $qb->createNamedParameter($episodeIdentifier))
+            )
 			->andWhere(
-				$qb->expr()->eq('user_id', $qb->createNamedParameter($userId))
-			);
+                $qb->expr()->eq('user_id', $qb->createNamedParameter($userId))
+            );

 		try {
 			/** @var EpisodeActionEntity $episodeActionEntity */
@ -66,5 +64,26 @@ class EpisodeActionMapper extends QBMapper
 		}
 	}

+    public function findByGuid(string $guid, string $userId) : ?EpisodeActionEntity
+    {
+        $qb = $this->db->getQueryBuilder();
+
+        $qb->select('*')
+            ->from($this->getTableName())
+            ->where(
+                $qb->expr()->eq('guid', $qb->createNamedParameter($guid))
+            )
+            ->andWhere(
+                $qb->expr()->eq('user_id', $qb->createNamedParameter($userId))
+            );
+
+        try {
+            /** @var EpisodeActionEntity $episodeActionEntity */
+            return $this->findEntity($qb);
+        } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) {
+            return null;
+        }
+    }
+

 }
--- a/lib/Db/EpisodeAction/EpisodeActionRepository.php
+++ b/lib/Db/EpisodeAction/EpisodeActionRepository.php
@ -28,17 +28,29 @@ class EpisodeActionRepository {
 		return $episodeActions;
 	}

-	public function findByEpisodeIdentifier(string $identifier, string $userId): ?EpisodeAction {
-		$episodeActionEntity = $this->episodeActionMapper->findByEpisodeIdentifier($identifier, $userId);
+    public function findByEpisodeUrl(string $episodeUrl, string $userId): ?EpisodeAction {
+        $episodeActionEntity = $this->episodeActionMapper->findByEpisodeUrl($episodeUrl, $userId);

-		if ($episodeActionEntity === null) {
-			return null;
-		}
+        if ($episodeActionEntity === null) {
+            return null;
+        }

-		return $this->mapEntityToEpisodeAction(
-			$episodeActionEntity
-		);
-	}
+        return $this->mapEntityToEpisodeAction(
+            $episodeActionEntity
+        );
+    }
+
+    public function findByGuid(string $guid, string $userId): ?EpisodeAction {
+        $episodeActionEntity = $this->episodeActionMapper->findByGuid($guid, $userId);
+
+        if ($episodeActionEntity === null) {
+            return null;
+        }
+
+        return $this->mapEntityToEpisodeAction(
+            $episodeActionEntity
+        );
+    }

 	private function mapEntityToEpisodeAction(EpisodeActionEntity $episodeActionEntity): EpisodeAction
 	{
--- a/tests/Integration/Controller/EpisodeActionControllerTest.php
+++ b/tests/Integration/Controller/EpisodeActionControllerTest.php
@ -3,7 +3,9 @@ declare(strict_types=1);

 namespace tests\Integration\Controller;

-use OC\Security\SecureRandom;
+require_once __DIR__ . '/../../Helper/DatabaseTransaction.php';
+require_once __DIR__ . '/../../Helper/Writer/TestWriter.php';
+
 use OCA\GPodderSync\Controller\EpisodeActionController;
 use OCA\GPodderSync\Core\EpisodeAction\EpisodeActionSaver;
 use OCA\GPodderSync\Db\EpisodeAction\EpisodeActionEntity;
@ -12,7 +14,6 @@ use OCA\GPodderSync\Db\EpisodeAction\EpisodeActionRepository;
 use OCA\GPodderSync\Db\EpisodeAction\EpisodeActionWriter;
 use OCP\AppFramework\App;
 use OCP\AppFramework\IAppContainer;
-use OCP\IConfig;
 use OCP\IRequest;
 use Test\TestCase;
 use tests\Helper\DatabaseTransaction;
--- a/tests/Integration/EpisodeActionGuidMigrationTest.php
+++ b/tests/Integration/EpisodeActionGuidMigrationTest.php
@ -22,19 +22,13 @@ class EpisodeActionGuidMigrationTest extends TestCase

 	private IAppContainer $container;

-	/**
-	 * @var EpisodeActionWriter
-	 */
-	private $episodeActionWriter;
+	private EpisodeActionWriter $episodeActionWriter;

-	/**
-	 * @var EpisodeActionRepository
-	 */
-	private $episodeActionRepository;
+	private EpisodeActionRepository $episodeActionRepository;

 	public function setUp(): void {
 		parent::setUp();
-		$app = new App('gpoddersync');
+		$app = new App('nextcloud-gpodder');
 		$this->container = $app->getContainer();
 		$this->episodeActionWriter = $this->container->get(EpisodeActionWriter::class);
 		$this->episodeActionRepository = $this->container->get(EpisodeActionRepository::class);
@ -85,7 +79,7 @@ class EpisodeActionGuidMigrationTest extends TestCase

 		self::assertSame(
 			$savedEpisodeActionEntity->getId(),
-			$this->episodeActionRepository->findByEpisodeIdentifier($episodeActionEntity->getEpisode(), self::USER_ID_0)->getId()
+			$this->episodeActionRepository->findByEpisodeUrl($episodeActionEntity->getEpisode(), self::USER_ID_0)->getId()
 		);

 		//update same episode action again this time with guid
@ -96,12 +90,12 @@ class EpisodeActionGuidMigrationTest extends TestCase

 		self::assertSame(
 			$savedEpisodeActionEntityWithGuid->getId(),
-			$this->episodeActionRepository->findByEpisodeIdentifier($episodeActionEntityWithGuid->getEpisode(), self::USER_ID_0)->getId()
+			$this->episodeActionRepository->findByEpisodeUrl($episodeActionEntityWithGuid->getEpisode(), self::USER_ID_0)->getId()
 		);

 		self::assertSame(
 			$savedEpisodeActionEntityWithGuid->getId(),
-			$this->episodeActionRepository->findByEpisodeIdentifier($episodeActionEntityWithGuid->getGuid(), self::USER_ID_0)->getId()
+			$this->episodeActionRepository->findByGuid($episodeActionEntityWithGuid->getGuid(), self::USER_ID_0)->getId()
 		);

 	}
--- a/tests/Integration/EpisodeActionRepositoryTest.php
+++ b/tests/Integration/EpisodeActionRepositoryTest.php
@ -49,7 +49,7 @@ class EpisodeActionRepositoryTest extends \Test\TestCase
 		/** @var $episodeActionRepository EpisodeActionRepository */
 		$episodeActionRepository = $this->container->get(EpisodeActionRepository::class);

-		$retrievedEpisodeActionEntity = $episodeActionRepository->findByEpisodeIdentifier($guid, self::USER_ID_0);
+		$retrievedEpisodeActionEntity = $episodeActionRepository->findByGuid($guid, self::USER_ID_0);
 		self::assertSame('2021-08-22T23:58:56', $retrievedEpisodeActionEntity->getTimestamp());

 	}
--- a/tests/Integration/EpisodeActionSaverGuidBackwardCompatibilityTest.php
+++ b/tests/Integration/EpisodeActionSaverGuidBackwardCompatibilityTest.php
@ -10,7 +10,7 @@ use Test\TestCase;
 /**
 * @group DB
 */
-class EpisodeActionSaverGuidBackwardCompatbilityTest extends TestCase
+class EpisodeActionSaverGuidBackwardCompatibilityTest extends TestCase
 {

 	private const USER_ID_0 = "testuser0";
--- a/tests/Integration/Migration/TimestampMigrationTest.php
+++ b/tests/Integration/Migration/TimestampMigrationTest.php
@ -79,7 +79,7 @@ class TimestampMigrationTest extends TestCase
 		$trueCrimeEpisodeActionEntity->setGuid(uniqid("self::TEST_GUID_1234"));
 		$this->episodeActionWriter->save($trueCrimeEpisodeActionEntity);

-		$episodeActionBeforeConversion = $this->episodeActionMapper->findByEpisodeIdentifier(
+		$episodeActionBeforeConversion = $this->episodeActionMapper->findByGuid(
 			$scienceEpisodeActionEntity->getGuid(),
 			self::ADMIN
 		);
@ -92,7 +92,7 @@ class TimestampMigrationTest extends TestCase
 		$timestampMigration = new TimestampMigration($this->dbConnection, $this->migrationConfig);
 		$timestampMigration->run($this->createMock(SimpleOutput::class));

-		$scienceEpisodeActionAfterConversion = $this->episodeActionMapper->findByEpisodeIdentifier(
+		$scienceEpisodeActionAfterConversion = $this->episodeActionMapper->findByGuid(
 			$scienceEpisodeActionEntity->getGuid(),
 			self::ADMIN
 		);
@ -101,7 +101,7 @@ class TimestampMigrationTest extends TestCase
 			$scienceEpisodeActionAfterConversion->getTimestampEpoch()
 		);

-		$trueCrimeEpisodeActionAfterConversion = $this->episodeActionMapper->findByEpisodeIdentifier(
+		$trueCrimeEpisodeActionAfterConversion = $this->episodeActionMapper->findByGuid(
 			$trueCrimeEpisodeActionEntity->getGuid(),
 			self::ADMIN
 		);