Store download, delete and new actions without overwriting play positions

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": [

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()]);
@@ -52,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([
@@ -71,8 +73,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');
-	}
 }

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()
             );
         }
     }

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 {

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);
     }

lib/Migration/Version0008Date20261025120000.php

@@ -0,0 +1,32 @@
+<?php
+declare(strict_types=1);
+
+namespace OCA\GPodderSync\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * Keep one episode action per episode and action type, so that a download, delete or new action
+ * no longer overwrites the play position of the same episode.
+ */
+class Version0008Date20261025120000 extends SimpleMigrationStep {
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->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;
+	}
+}

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()

tests/Integration/EpisodeActionTypesTest.php

@@ -0,0 +1,139 @@
+<?php
+declare(strict_types=1);
+
+namespace tests\Integration;
+
+use OCA\GPodderSync\Controller\EpisodeActionController;
+use OCA\GPodderSync\Core\EpisodeAction\EpisodeActionSaver;
+use OCA\GPodderSync\Db\EpisodeAction\EpisodeActionRepository;
+use OCP\AppFramework\App;
+use OCP\AppFramework\IAppContainer;
+use OCP\IRequest;
+use Test\TestCase;
+
+/**
+ * Saving an action whose episode already has one goes through a failing insert, which would abort a surrounding
+ * transaction on PostgreSQL, so these tests run without one (like EpisodeActionSaverGuidBackwardCompatibilityTest).
+ *
+ * @group DB
+ */
+class EpisodeActionTypesTest extends TestCase
+{
+    private IAppContainer $container;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+        $app = new App("gpoddersync");
+        $this->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"]);
+    }
+
+    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);
+        $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, bool $allActions = true): array
+    {
+        $episodeActionController = new EpisodeActionController(
+            "gpoddersync",
+            $this->createMock(IRequest::class),
+            $userId,
+            $this->container->get(EpisodeActionRepository::class),
+            $this->container->get(EpisodeActionSaver::class)
+        );
+        $actionsByType = [];
+        foreach ($episodeActionController->list(0, $allActions)->getData()["actions"] as $episodeAction) {
+            $actionsByType[$episodeAction["action"]] = $episodeAction;
+        }
+        return $actionsByType;
+    }
+}