From 8761d78cc9da79e796cc1069ecad3500ae032eb9 Mon Sep 17 00:00:00 2001 From: arikisonfire <21263163+arikisonfire@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:04:27 +0100 Subject: [PATCH 1/2] Store download, delete and new actions without overwriting play positions --- lib/Controller/EpisodeActionController.php | 5 - lib/Core/EpisodeAction/EpisodeActionSaver.php | 11 +- lib/Db/EpisodeAction/EpisodeActionMapper.php | 11 +- .../EpisodeAction/EpisodeActionRepository.php | 17 ++- .../Version0008Date20261025120000.php | 32 +++++ .../EpisodeActionControllerTest.php | 12 +- tests/Integration/EpisodeActionTypesTest.php | 117 ++++++++++++++++++ 7 files changed, 186 insertions(+), 19 deletions(-) create mode 100644 lib/Migration/Version0008Date20261025120000.php create mode 100644 tests/Integration/EpisodeActionTypesTest.php diff --git a/lib/Controller/EpisodeActionController.php b/lib/Controller/EpisodeActionController.php index 19ff329..d135643 100644 --- a/lib/Controller/EpisodeActionController.php +++ b/lib/Controller/EpisodeActionController.php @@ -41,7 +41,6 @@ public function __construct( public function create(): JSONResponse { $episodeActionsArray = $this->filterEpisodesFromRequestParams($this->request->getParams()); - $episodeActionsArray = $this->filterOnlyPlays($episodeActionsArray); $this->episodeActionSaver->saveEpisodeActions($episodeActionsArray, $this->userId); return new JSONResponse(["timestamp" => time()]); @@ -71,8 +70,4 @@ public function list(int $since = 0): JSONResponse { public function filterEpisodesFromRequestParams(array $episodesFromRequestParams): array { return array_filter($episodesFromRequestParams, "is_numeric", ARRAY_FILTER_USE_KEY); } - - public function filterOnlyPlays(array $episodesFromRequestParams): array { - return array_filter($episodesFromRequestParams, fn($ep) => isset($ep['action']) && strtolower($ep['action']) === 'play'); - } } diff --git a/lib/Core/EpisodeAction/EpisodeActionSaver.php b/lib/Core/EpisodeAction/EpisodeActionSaver.php index 123ed30..af9512b 100644 --- a/lib/Core/EpisodeAction/EpisodeActionSaver.php +++ b/lib/Core/EpisodeAction/EpisodeActionSaver.php @@ -143,14 +143,16 @@ private function findEpisodeActionToUpdate( if ($episodeActionEntity->getGuid() !== null) { $episodeAction = $this->episodeActionRepository->findByGuid( $episodeActionEntity->getGuid(), - $userId + $userId, + $episodeActionEntity->getAction() ); } if ($episodeAction === null) { $episodeAction = $this->episodeActionRepository->findByEpisodeUrl( $episodeActionEntity->getEpisode(), - $userId + $userId, + $episodeActionEntity->getAction() ); } @@ -167,12 +169,13 @@ private function deleteConflictingEpisodeAction( string $userId ): void { $collidingEpisodeActionId = $this->episodeActionRepository - ->findByEpisodeUrl($episodeActionEntity->getEpisode(), $userId) + ->findByEpisodeUrl($episodeActionEntity->getEpisode(), $userId, $episodeActionEntity->getAction()) ->getId(); if ($collidingEpisodeActionId !== $episodeActionEntity->getId()) { $this->episodeActionRepository->deleteEpisodeActionByEpisodeUrl( $episodeActionEntity->getEpisode(), - $userId + $userId, + $episodeActionEntity->getAction() ); } } diff --git a/lib/Db/EpisodeAction/EpisodeActionMapper.php b/lib/Db/EpisodeAction/EpisodeActionMapper.php index 668f954..41c4779 100644 --- a/lib/Db/EpisodeAction/EpisodeActionMapper.php +++ b/lib/Db/EpisodeAction/EpisodeActionMapper.php @@ -41,9 +41,10 @@ public function findAll(int $sinceTimestamp, string $userId): array /** * @param string $episodeIdentifier * @param string $userId + * @param string $action * @return EpisodeActionEntity|null */ - public function findByEpisodeUrl(string $episodeIdentifier, string $userId) : ?EpisodeActionEntity + public function findByEpisodeUrl(string $episodeIdentifier, string $userId, string $action = 'PLAY') : ?EpisodeActionEntity { $qb = $this->db->getQueryBuilder(); @@ -54,6 +55,9 @@ public function findByEpisodeUrl(string $episodeIdentifier, string $userId) : ?E ) ->andWhere( $qb->expr()->eq('user_id', $qb->createNamedParameter($userId)) + ) + ->andWhere( + $qb->expr()->eq('action', $qb->createNamedParameter($action)) ); try { @@ -64,7 +68,7 @@ public function findByEpisodeUrl(string $episodeIdentifier, string $userId) : ?E } } - public function findByGuid(string $guid, string $userId) : ?EpisodeActionEntity + public function findByGuid(string $guid, string $userId, string $action = 'PLAY') : ?EpisodeActionEntity { $qb = $this->db->getQueryBuilder(); @@ -75,6 +79,9 @@ public function findByGuid(string $guid, string $userId) : ?EpisodeActionEntity ) ->andWhere( $qb->expr()->eq('user_id', $qb->createNamedParameter($userId)) + ) + ->andWhere( + $qb->expr()->eq('action', $qb->createNamedParameter($action)) ); try { diff --git a/lib/Db/EpisodeAction/EpisodeActionRepository.php b/lib/Db/EpisodeAction/EpisodeActionRepository.php index 924e71e..2499dc4 100644 --- a/lib/Db/EpisodeAction/EpisodeActionRepository.php +++ b/lib/Db/EpisodeAction/EpisodeActionRepository.php @@ -35,11 +35,13 @@ public function findAll(int $sinceEpoch, string $userId): array public function findByEpisodeUrl( string $episodeUrl, - string $userId + string $userId, + string $action = 'PLAY' ): ?EpisodeAction { $episodeActionEntity = $this->episodeActionMapper->findByEpisodeUrl( $episodeUrl, - $userId + $userId, + $action ); if ($episodeActionEntity === null) { @@ -49,11 +51,12 @@ public function findByEpisodeUrl( return $this->mapEntityToEpisodeAction($episodeActionEntity); } - public function findByGuid(string $guid, string $userId): ?EpisodeAction + public function findByGuid(string $guid, string $userId, string $action = 'PLAY'): ?EpisodeAction { $episodeActionEntity = $this->episodeActionMapper->findByGuid( $guid, - $userId + $userId, + $action ); if ($episodeActionEntity === null) { @@ -65,11 +68,13 @@ public function findByGuid(string $guid, string $userId): ?EpisodeAction public function deleteEpisodeActionByEpisodeUrl( string $episodeUrl, - string $userId + string $userId, + string $action = 'PLAY' ): void { $episodeAction = $this->episodeActionMapper->findByEpisodeUrl( $episodeUrl, - $userId + $userId, + $action ); $this->episodeActionMapper->delete($episodeAction); } diff --git a/lib/Migration/Version0008Date20261025120000.php b/lib/Migration/Version0008Date20261025120000.php new file mode 100644 index 0000000..870272a --- /dev/null +++ b/lib/Migration/Version0008Date20261025120000.php @@ -0,0 +1,32 @@ +getTable('gpodder_episode_action'); + if ($table->hasIndex('gpodder_episode_user_id')) { + $table->dropIndex('gpodder_episode_user_id'); + } + if ($table->hasIndex('gpodder_guid_user_id')) { + $table->dropIndex('gpodder_guid_user_id'); + } + $table->addUniqueIndex(['episode', 'action', 'user_id'], 'gpodder_episode_action_user'); + $table->addUniqueIndex(['guid', 'action', 'user_id'], 'gpodder_guid_action_user'); + + return $schema; + } +} diff --git a/tests/Integration/Controller/EpisodeActionControllerTest.php b/tests/Integration/Controller/EpisodeActionControllerTest.php index a6cd0f9..72bdbf0 100644 --- a/tests/Integration/Controller/EpisodeActionControllerTest.php +++ b/tests/Integration/Controller/EpisodeActionControllerTest.php @@ -189,9 +189,17 @@ public function testEpisodeActionCreateAction(): void /** @var EpisodeActionMapper $mapper */ $mapper = $this->container->query(EpisodeActionMapper::class); $episodeActionEntities = $mapper->findAll(0, $userId); + $this->assertCount(2, $episodeActionEntities); + $entitiesByAction = []; + foreach ($episodeActionEntities as $entity) { + $entitiesByAction[$entity->getAction()] = $entity; + } /** @var EpisodeActionEntity $firstEntity */ - $firstEntity = $episodeActionEntities[0]; - $this->assertCount(1, $episodeActionEntities); + $firstEntity = $entitiesByAction["PLAY"]; + $this->assertSame( + "https://example.com/files/s01e21.mp3", + $entitiesByAction["DELETE"]->getEpisode() + ); $this->assertSame( "https://example.com/feed.rss", $firstEntity->getPodcast() diff --git a/tests/Integration/EpisodeActionTypesTest.php b/tests/Integration/EpisodeActionTypesTest.php new file mode 100644 index 0000000..4e7a828 --- /dev/null +++ b/tests/Integration/EpisodeActionTypesTest.php @@ -0,0 +1,117 @@ +container = $app->getContainer(); + } + + public function testOtherActionsKeepThePlayPosition(): void + { + $userId = uniqid("test_user", true); + $episode = [ + "podcast" => "https://example.com/feed.rss", + "episode" => "https://example.com/files/s01e20.mp3", + "guid" => "s01e20-example-org", + "started" => -1, + "position" => -1, + "total" => -1, + ]; + $this->createEpisodeActions($userId, [ + ["action" => "PLAY", "timestamp" => "2009-12-12T09:00:00", "started" => 15, "position" => 120, "total" => 500] + $episode, + ["action" => "DOWNLOAD", "timestamp" => "2009-12-12T09:05:00"] + $episode, + ["action" => "DELETE", "timestamp" => "2009-12-12T09:10:00"] + $episode, + ["action" => "NEW", "timestamp" => "2009-12-12T09:15:00"] + $episode, + ]); + $this->createEpisodeActions($userId, [ + ["action" => "PLAY", "timestamp" => "2009-12-12T10:00:00", "started" => 120, "position" => 300, "total" => 500] + $episode, + ]); + + $actions = $this->listEpisodeActionsByAction($userId); + self::assertCount(4, $actions); + self::assertSame(300, $actions["PLAY"]["position"]); + self::assertSame(120, $actions["PLAY"]["started"]); + self::assertSame("2009-12-12T10:00:00+00:00", $actions["PLAY"]["timestamp"]); + self::assertSame("2009-12-12T09:05:00+00:00", $actions["DOWNLOAD"]["timestamp"]); + self::assertSame("2009-12-12T09:10:00+00:00", $actions["DELETE"]["timestamp"]); + self::assertSame("2009-12-12T09:15:00+00:00", $actions["NEW"]["timestamp"]); + + /** @var EpisodeActionRepository $episodeActionRepository */ + $episodeActionRepository = $this->container->get(EpisodeActionRepository::class); + self::assertSame(300, $episodeActionRepository->findByGuid("s01e20-example-org", $userId)->getPosition()); + self::assertSame( + "DOWNLOAD", + $episodeActionRepository->findByGuid("s01e20-example-org", $userId, "DOWNLOAD")->getAction() + ); + } + + public function testOtherActionsKeepThePlayPositionWithoutGuid(): void + { + $userId = uniqid("test_user", true); + $episode = [ + "podcast" => "https://example.com/feed.rss", + "episode" => "https://example.com/files/s01e22.mp3", + ]; + $this->createEpisodeActions($userId, [ + ["action" => "PLAY", "timestamp" => "2009-12-12T09:00:00", "started" => 0, "position" => 60, "total" => 500] + $episode, + ["action" => "DOWNLOAD", "timestamp" => "2009-12-12T09:05:00", "started" => -1, "position" => -1, "total" => -1] + $episode, + ]); + + $actions = $this->listEpisodeActionsByAction($userId); + self::assertCount(2, $actions); + self::assertSame(60, $actions["PLAY"]["position"]); + self::assertSame(-1, $actions["DOWNLOAD"]["position"]); + } + + private function createEpisodeActions(string $userId, array $episodeActions): void + { + $request = $this->createMock(IRequest::class); + $request->method("getParams")->willReturn($episodeActions); + $episodeActionController = new EpisodeActionController( + "gpoddersync", + $request, + $userId, + $this->container->get(EpisodeActionRepository::class), + $this->container->get(EpisodeActionSaver::class) + ); + $episodeActionController->create(); + } + + private function listEpisodeActionsByAction(string $userId): array + { + $episodeActionController = new EpisodeActionController( + "gpoddersync", + $this->createMock(IRequest::class), + $userId, + $this->container->get(EpisodeActionRepository::class), + $this->container->get(EpisodeActionSaver::class) + ); + $actionsByType = []; + foreach ($episodeActionController->list()->getData()["actions"] as $episodeAction) { + $actionsByType[$episodeAction["action"]] = $episodeAction; + } + return $actionsByType; + } +} From 01003bf93209de6db5e34f7d9b45307a13832a93 Mon Sep 17 00:00:00 2001 From: arikisonfire <21263163+arikisonfire@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:07:05 +0100 Subject: [PATCH 2/2] Return only play actions unless a client asks for all of them YourPods and Podcast Merlin do not check the action type of the rows they receive, so download, delete and new rows with position -1 would hide or overwrite the play position there. GET episode_action keeps returning only play actions, and allActions=true also returns the others. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 ++- lib/Controller/EpisodeActionController.php | 7 ++++-- tests/Integration/EpisodeActionTypesTest.php | 26 ++++++++++++++++++-- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index ae1f7b1..6a9532c 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,7 @@ POST /index.php/apps/gpoddersync/subscription_change/create ### episode action * **get episode actions**: `GET /index.php/apps/gpoddersync/episode_action` * *(optional)* GET parameter `since` (UNIX time) + * *(optional)* GET parameter `allActions` (`true` or `false`, default `false`): also return *download*, *delete* and *new* actions, otherwise only *play* actions are returned * fields: *podcast*, *episode*, *guid*, *action*, *timestamp*, *position*, *started*, *total* * **create episode actions**: `POST /index.php/apps/gpoddersync/episode_action/create` * fields: *podcast*, *episode*, *guid*, *action*, *timestamp*, *position*, *started*, *total* @@ -72,7 +73,7 @@ The API replicates this: https://gpoddernet.readthedocs.io/en/latest/api/referen #### Example requests: ```json -GET /index.php/apps/gpoddersync/episode_action?since=1633240761 +GET /index.php/apps/gpoddersync/episode_action?since=1633240761&allActions=true { "actions": [ diff --git a/lib/Controller/EpisodeActionController.php b/lib/Controller/EpisodeActionController.php index d135643..52b946f 100644 --- a/lib/Controller/EpisodeActionController.php +++ b/lib/Controller/EpisodeActionController.php @@ -51,14 +51,17 @@ public function create(): JSONResponse { * @NoCSRFRequired * * @param int $since + * @param bool $allActions also return download, delete and new actions, not only play actions * @return JSONResponse */ - public function list(int $since = 0): JSONResponse { + public function list(int $since = 0, bool $allActions = false): JSONResponse { $episodeActions = $this->episodeActionRepository->findAll($since, $this->userId); $untypedEpisodeActionData = []; foreach ($episodeActions as $episodeAction) { - $untypedEpisodeActionData[] = $episodeAction->toArray(); + if ($allActions || strtolower($episodeAction->getAction()) === 'play') { + $untypedEpisodeActionData[] = $episodeAction->toArray(); + } } return new JSONResponse([ diff --git a/tests/Integration/EpisodeActionTypesTest.php b/tests/Integration/EpisodeActionTypesTest.php index 4e7a828..076e53e 100644 --- a/tests/Integration/EpisodeActionTypesTest.php +++ b/tests/Integration/EpisodeActionTypesTest.php @@ -85,6 +85,28 @@ public function testOtherActionsKeepThePlayPositionWithoutGuid(): void self::assertSame(-1, $actions["DOWNLOAD"]["position"]); } + public function testListReturnsOtherActionsOnlyWhenAskedFor(): void + { + $userId = uniqid("test_user", true); + $episode = [ + "podcast" => "https://example.com/feed.rss", + "episode" => "https://example.com/files/s01e23.mp3", + "guid" => "s01e23-example-org", + ]; + $this->createEpisodeActions($userId, [ + ["action" => "PLAY", "timestamp" => "2009-12-12T09:00:00", "started" => 0, "position" => 60, "total" => 500] + $episode, + ["action" => "DOWNLOAD", "timestamp" => "2009-12-12T09:05:00"] + $episode, + ["action" => "DELETE", "timestamp" => "2009-12-12T09:10:00"] + $episode, + ["action" => "NEW", "timestamp" => "2009-12-12T09:15:00"] + $episode, + ]); + + self::assertSame(["PLAY"], array_keys($this->listEpisodeActionsByAction($userId, false))); + + $allActions = array_keys($this->listEpisodeActionsByAction($userId)); + sort($allActions); + self::assertSame(["DELETE", "DOWNLOAD", "NEW", "PLAY"], $allActions); + } + private function createEpisodeActions(string $userId, array $episodeActions): void { $request = $this->createMock(IRequest::class); @@ -99,7 +121,7 @@ private function createEpisodeActions(string $userId, array $episodeActions): vo $episodeActionController->create(); } - private function listEpisodeActionsByAction(string $userId): array + private function listEpisodeActionsByAction(string $userId, bool $allActions = true): array { $episodeActionController = new EpisodeActionController( "gpoddersync", @@ -109,7 +131,7 @@ private function listEpisodeActionsByAction(string $userId): array $this->container->get(EpisodeActionSaver::class) ); $actionsByType = []; - foreach ($episodeActionController->list()->getData()["actions"] as $episodeAction) { + foreach ($episodeActionController->list(0, $allActions)->getData()["actions"] as $episodeAction) { $actionsByType[$episodeAction["action"]] = $episodeAction; } return $actionsByType;