feat(kobo): Paginate sync

ragusa87 · Dec 4, 2024 · b62540d · b62540d
1 parent e257f56
commit b62540d
Show file tree

Hide file tree

Showing 10 changed files with 134 additions and 56 deletions.
diff --git a/src/Controller/Kobo/Api/V1/LibraryController.php b/src/Controller/Kobo/Api/V1/LibraryController.php
@@ -179,8 +179,9 @@ public function apiEndpoint(KoboDevice $koboDevice, SyncToken $syncToken, Reques
             $syncToken->archiveLastModified = null;
         }
 
+        $maxBookPerSync = $request->query->getInt('per_page', self::MAX_BOOKS_PER_SYNC);
         // We fetch a subset of book to sync, based on the SyncToken.
-        $books = $this->bookRepository->getChangedBooks($koboDevice, $syncToken, 0, self::MAX_BOOKS_PER_SYNC);
+        $books = $this->bookRepository->getChangedBooks($koboDevice, $syncToken, 0, $maxBookPerSync);
         $count = $this->bookRepository->getChangedBooksCount($koboDevice, $syncToken);
         $this->koboSyncLogger->debug("Sync for Kobo {id}: {$count} books to sync", ['id' => $koboDevice->getId(), 'count' => $count, 'token' => $syncToken]);
 
@@ -191,17 +192,14 @@ public function apiEndpoint(KoboDevice $koboDevice, SyncToken $syncToken, Reques
         // Fetch the books upstream and merge the answer
         $shouldContinue = $this->upstreamSyncMerger->merge($koboDevice, $response, $request);
 
-        // TODO Pagination based on the sync token and lastSyncDate
         $httpResponse = $response->toJsonResponse();
-        $httpResponse->headers->set('x-kobo-sync-todo', $shouldContinue || count($books) < $count ? 'continue' : 'done');
+        $httpResponse->headers->set('x-kobo-sync', $shouldContinue || count($books) < $count ? 'continue' : 'done');
 
         // Once the response is generated, we update the list of synced books
         // If you do this before, the logic will be broken
-        if (false === $forced) {
-            $this->koboSyncLogger->debug('Set synced date for {count} downloaded books', ['count' => count($books)]);
+        $this->koboSyncLogger->debug('Set synced date for {count} downloaded books', ['count' => count($books)]);
 
-            $this->koboSyncedBookRepository->updateSyncedBooks($koboDevice, $books, $syncToken);
-        }
+        $this->koboSyncedBookRepository->updateSyncedBooks($koboDevice, $books, $syncToken);
 
         return $httpResponse;
     }

diff --git a/src/Entity/KoboDevice.php b/src/Entity/KoboDevice.php
@@ -19,6 +19,8 @@ class KoboDevice
     use RandomGeneratorTrait;
     public const KOBO_DEVICE_ID_HEADER = 'X-Kobo-Deviceid';
     public const KOBO_DEVICE_MODEL_HEADER = 'X-Kobo-Devicemodel';
+    public const KOBO_SYNC_TOKEN_HEADER = 'kobo-synctoken';
+
     #[ORM\Id]
     #[ORM\GeneratedValue]
     #[ORM\Column]

diff --git a/src/Kobo/Response/SyncResponse.php b/src/Kobo/Response/SyncResponse.php
@@ -192,7 +192,7 @@ private function getNewTags(): array
     private function getChangedTag(): array
     {
         $shelves = array_filter($this->shelves, function (Shelf $shelf) {
-            return $shelf->getCreated() < $this->syncToken->lastCreated;
+            return $this->syncToken->lastModified instanceof \DateTimeInterface && $shelf->getUpdated() < $this->syncToken->lastModified;
         });
 
         return array_map(function (Shelf $shelf) {

diff --git a/src/Kobo/Response/SyncResponseHelper.php b/src/Kobo/Response/SyncResponseHelper.php
@@ -27,14 +27,14 @@ public function isChangedEntitlement(Book $book, KoboDevice $koboDevice, SyncTok
             return false;
         }
 
-        return ($book->getUpdated() instanceof \DateTimeInterface
+        return ($syncToken->lastModified instanceof \DateTimeInterface && $book->getUpdated() instanceof \DateTimeInterface
             && $book->getUpdated() >= $syncToken->lastModified)
-            || $book->getCreated() >= $syncToken->lastCreated;
+            || ($syncToken->lastCreated instanceof \DateTimeInterface && $book->getCreated() >= $syncToken->lastCreated);
     }
 
     public function isNewEntitlement(Book $book, SyncToken $syncToken): bool
     {
-        return $book->getKoboSyncedBook()->isEmpty() || $book->getCreated() < $syncToken->lastCreated;
+        return $book->getKoboSyncedBook()->isEmpty(); // $book->getCreated() >= $syncToken->lastCreated;
     }
 
     public function isChangedReadingState(Book $book, KoboDevice $koboDevice, SyncToken $syncToken): bool

diff --git a/src/Repository/BookRepository.php b/src/Repository/BookRepository.php
@@ -20,8 +20,10 @@ class BookRepository extends ServiceEntityRepository
 {
     private Security $security;
 
-    public function __construct(ManagerRegistry $registry, Security $security)
-    {
+    public function __construct(
+        ManagerRegistry $registry,
+        Security $security,
+    ) {
         parent::__construct($registry, Book::class);
         $this->security = $security;
     }
@@ -356,18 +358,24 @@ public function getChangedBooks(KoboDevice $koboDevice, SyncToken $syncToken, in
         $qb->setFirstResult($firstResult)
             ->setMaxResults($maxResults);
         $qb->orderBy('book.updated', 'ASC');
+
+        $query = $qb->getQuery();
         /** @var Book[] $result */
-        $result = $qb->getQuery()->getResult();
+        $result = $query->getResult();
 
         return $result;
     }
 
     public function getChangedBooksCount(KoboDevice $koboDevice, SyncToken $syncToken): int
     {
         $qb = $this->getChangedBooksQueryBuilder($koboDevice, $syncToken);
-        $qb->select('count(book.id) as nb');
+        $qb->select('count(distinct book.id) as nb');
+        $qb->resetDQLPart('groupBy');
+
+        /** @var array{0: int} $result */
+        $result = $qb->getQuery()->getSingleColumnResult();
 
-        return (int) $qb->getQuery()->getSingleColumnResult();
+        return $result[0];
     }
 
     private function getChangedBooksQueryBuilder(KoboDevice $koboDevice, SyncToken $syncToken): QueryBuilder
@@ -382,13 +390,12 @@ private function getChangedBooksQueryBuilder(KoboDevice $koboDevice, SyncToken $
             ->andWhere('book.extension = :extension')
             ->setParameter('id', $koboDevice->getId())
             ->setParameter('koboDevice', $koboDevice)
-            ->setParameter('extension', 'epub'); // Pdf is not supported by kobo sync
-
+            ->setParameter('extension', 'epub') // Pdf is not supported by kobo sync
+            ->groupBy('book.id');
         if ($syncToken->lastCreated instanceof \DateTimeInterface) {
-            $qb->andWhere('book.created > :lastCreated');
-            $qb->orWhere($qb->expr()->orX(
-                $qb->expr()->isNull('koboSyncedBooks.created is null'),
-                $qb->expr()->isNull('koboSyncedBooks.created > :lastCreated'),
+            $qb->andWhere($qb->expr()->orX(
+                $qb->expr()->isNull('koboSyncedBooks.created'),
+                $qb->expr()->gte('book.created', ':lastCreated'),
             ))
             ->setParameter('lastCreated', $syncToken->lastCreated);
         }

diff --git a/src/Repository/KoboSyncedBookRepository.php b/src/Repository/KoboSyncedBookRepository.php
@@ -25,33 +25,14 @@ public function __construct(ManagerRegistry $registry)
         parent::__construct($registry, KoboSyncedBook::class);
     }
 
-    //    /**
-    //     * @return KoboSyncedBook[] Returns an array of KoboSyncedBook objects
-    //     */
-    //    public function findByExampleField($value): array
-    //    {
-    //        return $this->createQueryBuilder('k')
-    //            ->andWhere('k.exampleField = :val')
-    //            ->setParameter('val', $value)
-    //            ->orderBy('k.id', 'ASC')
-    //            ->setMaxResults(10)
-    //            ->getQuery()
-    //            ->getResult()
-    //        ;
-    //    }
-
-    //    public function findOneBySomeField($value): ?KoboSyncedBook
-    //    {
-    //        return $this->createQueryBuilder('k')
-    //            ->andWhere('k.exampleField = :val')
-    //            ->setParameter('val', $value)
-    //            ->getQuery()
-    //            ->getOneOrNullResult()
-    //        ;
-    //    }
     public function updateSyncedBooks(KoboDevice $koboDevice, array $books, SyncToken $syncToken): void
     {
+        if ($books === []) {
+            return;
+        }
+
         $updatedAt = $syncToken->lastModified ?? new \DateTime();
+        $createdAt = $syncToken->lastCreated ?? new \DateTime();
 
         $qb = $this->createQueryBuilder('koboSyncedBook')
              ->select('book.id')
@@ -65,8 +46,8 @@ public function updateSyncedBooks(KoboDevice $koboDevice, array $books, SyncToke
             ->getQuery()->getResult(AbstractQuery::HYDRATE_ARRAY);
 
         $qb->update()
-            ->set('koboSyncedBook.created', ':updatedA')
-            ->setParameter('updatedA', $updatedAt)
+            ->set('koboSyncedBook.updated', ':updatedAt')
+            ->setParameter('updatedAt', $updatedAt)
         ->getQuery()
             ->execute();
 
@@ -95,11 +76,12 @@ public function updateSyncedBooks(KoboDevice $koboDevice, array $books, SyncToke
             $object->setBook($book);
             $object->setKoboDevice($koboDevice);
             $object->setUpdated($updatedAt);
-            $object->setCreated($updatedAt);
+            $object->setCreated($createdAt);
             $book->addKoboSyncedBook($object);
             $koboDevice->addKoboSyncedBook($object);
             $this->_em->persist($object);
         }
+
         $this->_em->flush();
     }
 

diff --git a/src/Service/KoboSyncTokenExtractor.php b/src/Service/KoboSyncTokenExtractor.php
@@ -2,6 +2,7 @@
 
 namespace App\Service;
 
+use App\Entity\KoboDevice;
 use App\Kobo\SyncToken;
 use App\Kobo\SyncTokenParser;
 use Symfony\Component\HttpFoundation\Request;
@@ -24,13 +25,24 @@ public function get(Request $request): SyncToken
     public function set(Response $response, SyncToken $token): Response
     {
         $token = $this->syncTokenParser->encode($token);
-        $response->headers->set('kobo-synctoken', $token);
+        $response->headers->set(KoboDevice::KOBO_SYNC_TOKEN_HEADER, $token);
 
         return $response;
     }
 
+    /**
+     * @return array{'HTTP_kobo-synctoken': string}
+     * @throws \JsonException
+     */
+    public function getTestHeader(SyncToken $token): array
+    {
+        $token = $this->syncTokenParser->encode($token);
+
+        return ['HTTP_'.KoboDevice::KOBO_SYNC_TOKEN_HEADER => $token];
+    }
+
     protected function extract(Request $request): ?string
     {
-        return $request->headers->get('kobo-synctoken');
+        return $request->headers->get(KoboDevice::KOBO_SYNC_TOKEN_HEADER);
     }
 }
diff --git a/tests/Contraints/JSONIsValidSyncResponse.php b/tests/Contraints/JSONIsValidSyncResponse.php
@@ -8,7 +8,7 @@
 class JSONIsValidSyncResponse extends Constraint
 {
 
-    public function __construct(protected array $expectedKeysCount)
+    public function __construct(protected array $expectedKeysCount, protected int $pageNum = 1)
     {
         foreach($this->expectedKeysCount as $key => $count){
             if(false === in_array($key, self::KNOWN_TYPES, true)){
@@ -69,7 +69,7 @@ private function test(mixed $other): void
         asort($count);
         asort($this->expectedKeysCount);
 
-        (new IsIdentical($this->expectedKeysCount))->evaluate($count, 'Sync response doesnt contains the right entries count', false);
+        (new IsIdentical($this->expectedKeysCount))->evaluate($count, 'Sync response doesnt contains the right entries count for page '.$this->pageNum, false);
 
     }
 

diff --git a/tests/Controller/Kobo/AbstractKoboControllerTest.php b/tests/Controller/Kobo/AbstractKoboControllerTest.php
@@ -107,6 +107,22 @@ protected function getKoboProxyConfiguration(): KoboProxyConfiguration
 
         return $service;
     }
+
+    /**
+     * @template T of object
+     * @param class-string<T> $name
+     * @return T
+     */
+    protected function getService(string $name): mixed
+    {
+        $service = self::getContainer()->get($name);
+        if(!$service instanceof $name){
+            throw new \RuntimeException(sprintf('Service %s not found', $name));
+        }
+        assert($service instanceof $name);
+        return $service;
+    }
+
     protected function getMockClient(string $returnValue): ClientInterface
     {
         $mock = new MockHandler([

diff --git a/tests/Controller/Kobo/KoboSyncControllerTest.php b/tests/Controller/Kobo/KoboSyncControllerTest.php
@@ -5,6 +5,9 @@
 use App\DataFixtures\BookFixture;
 use App\Entity\KoboSyncedBook;
 use App\Kobo\Response\MetadataResponseService;
+use App\Kobo\SyncToken;
+use App\Repository\KoboSyncedBookRepository;
+use App\Service\KoboSyncTokenExtractor;
 use App\Tests\Contraints\AssertHasDownloadWithFormat;
 use App\Tests\Contraints\JSONIsValidSyncResponse;
 
@@ -18,10 +21,12 @@ protected function tearDown(): void
 
         parent::tearDown();
     }
-    public function assertPreConditions(): void
+
+    protected function setUp():void
     {
-        $count = $this->getEntityManager()->getRepository(KoboSyncedBook::class)->count(['koboDevice' => 1]);
-        self::assertSame(0, $count, 'There should be no synced books');
+        parent::setUp();
+
+        $this->getService(KoboSyncedBookRepository::class)->deleteAllSyncedBooks(1);
     }
 
     /**
@@ -71,6 +76,62 @@ public function testSyncControllerWithoutForce() : void
 
     }
 
+    /**
+     * @covers pagination
+     * @throws \JsonException
+     */
+    public function testSyncControllerPaginated() : void
+    {
+        $client = static::getClient();
+
+
+        $perPage = 7;
+        $numberOfPages = (int)ceil(BookFixture::NUMBER_OF_YAML_BOOKS / $perPage);
+
+        $syncToken = new SyncToken();
+        $syncToken->lastCreated = new \DateTime('now');
+        $syncToken->lastModified = null;
+
+        foreach(range(1, $numberOfPages) as $pageNum) {
+            // Build the sync-token header
+            $headers = $this->getService(KoboSyncTokenExtractor::class)->getTestHeader($syncToken);
+
+            $client?->request('GET', '/kobo/' . $this->accessKey . '/v1/library/sync?per_page=' . $perPage, [], [], $headers);
+
+            $response = self::getJsonResponse();
+            self::assertResponseIsSuccessful();
+
+            // We have 20 books, with 7 book per page, we do 3 calls that have respectively 7, 7 and 6 books
+            self::assertThat($response, new JSONIsValidSyncResponse(match($pageNum){
+                1 => [
+                    'NewTag' => 1,
+                    'NewEntitlement' => 7,
+                ],
+                2 => [
+                    'NewEntitlement' => 7,
+                ],
+                3 => [
+                    'NewEntitlement' => 6,
+                ],
+                default => [],
+            }, $pageNum), 'Response is not a valid sync response for page '.$pageNum);
+
+            $expectedContinueHeader = $pageNum === $numberOfPages ? 'done' : 'continue';
+            self::assertResponseHeaderSame('x-kobo-sync', $expectedContinueHeader, 'x-kobo-sync is invalid');
+        }
+
+        $count = $this->getEntityManager()->getRepository(KoboSyncedBook::class)->count(['koboDevice' => 1]);
+        self::assertSame(BookFixture::NUMBER_OF_YAML_BOOKS, $count, 'Number of synced book is invalid');
+
+        // Calling one more time should have an empty result
+        $headers = $this->getService(KoboSyncTokenExtractor::class)->getTestHeader($syncToken);
+        $client?->request('GET', '/kobo/' . $this->accessKey . '/v1/library/sync?per_page=' . $perPage, [], [], $headers);
+        self::assertResponseIsSuccessful();
+        self::assertThat(self::getJsonResponse(), new JSONIsValidSyncResponse([], $numberOfPages+1));
+
+
+    }
+
     public function testSyncControllerWithRemote() : void
     {
         $client = static::getClient();