diff --git a/src/Controller/Kobo/Api/V1/LibraryController.php b/src/Controller/Kobo/Api/V1/LibraryController.php index 1c9ef95a..4769b5ee 100644 --- 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 index 005ef814..4d72e558 100644 --- 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 index 2540df9b..0a5835db 100644 --- 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 index fd9fe092..9cc1ddfd 100644 --- 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 index 3382d920..7333ad3a 100644 --- 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,8 +358,10 @@ 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; } @@ -365,9 +369,13 @@ public function getChangedBooks(KoboDevice $koboDevice, SyncToken $syncToken, in 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 index 6a195acc..2be4ab7d 100644 --- 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 index 33abb1a3..f0403c82 100644 --- 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 index 13f35946..eea4b1e3 100644 --- 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 index a0580d7f..385e9f54 100644 --- 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 $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 index 8f84684f..6dabc9f9 100644 --- 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();