From 4b95ebdcf4c40cbcd17ca9528a2e341f24369dda Mon Sep 17 00:00:00 2001 From: Laurent Constantin Date: Wed, 4 Dec 2024 17:39:16 +0100 Subject: [PATCH] feat(kobo): Test ChangedEntitlement --- config/services.yaml | 8 ++- .../Kobo/Api/V1/LibraryController.php | 2 +- src/Entity/KoboDevice.php | 1 + src/Kobo/Response/SyncResponse.php | 6 +-- src/Kobo/Response/SyncResponseHelper.php | 30 ++++++++--- src/Kobo/UpstreamSyncMerger.php | 2 +- src/Repository/BookRepository.php | 14 ++++-- .../Kobo/KoboSyncControllerTest.php | 50 ++++++++++++++++++- tests/TestClock.php | 21 ++++++++ 9 files changed, 115 insertions(+), 19 deletions(-) create mode 100644 tests/TestClock.php diff --git a/config/services.yaml b/config/services.yaml index fbd340c1..920ffe94 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -59,6 +59,7 @@ services: - { name: doctrine.event_listener, event: 'loadClassMetadata' } calls: - [ setAnnotationReader, [ "@gedmo.mapping.driver.attribute" ] ] + - [ setClock, ['@Psr\Clock\ClockInterface'] ] gedmo.listener.sluggable: class: Gedmo\Sluggable\SluggableListener @@ -135,4 +136,9 @@ when@test: bind: $publicDirectory: '%kernel.project_dir%/tests/Resources' App\Service\BookFileSystemManagerInterface: - alias: 'App\Tests\FileSystemManagerForTests' \ No newline at end of file + alias: 'App\Tests\FileSystemManagerForTests' + + App\Tests\TestClock: + public: true + + Psr\Clock\ClockInterface: '@App\Tests\TestClock' diff --git a/src/Controller/Kobo/Api/V1/LibraryController.php b/src/Controller/Kobo/Api/V1/LibraryController.php index 4769b5ee..13517b7a 100644 --- a/src/Controller/Kobo/Api/V1/LibraryController.php +++ b/src/Controller/Kobo/Api/V1/LibraryController.php @@ -193,7 +193,7 @@ public function apiEndpoint(KoboDevice $koboDevice, SyncToken $syncToken, Reques $shouldContinue = $this->upstreamSyncMerger->merge($koboDevice, $response, $request); $httpResponse = $response->toJsonResponse(); - $httpResponse->headers->set('x-kobo-sync', $shouldContinue || count($books) < $count ? 'continue' : 'done'); + $httpResponse->headers->set(KoboDevice::KOBO_SYNC_SHOULD_CONTINUE_HEADER, $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 diff --git a/src/Entity/KoboDevice.php b/src/Entity/KoboDevice.php index 4d72e558..2beda821 100644 --- a/src/Entity/KoboDevice.php +++ b/src/Entity/KoboDevice.php @@ -20,6 +20,7 @@ class KoboDevice 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'; + public const KOBO_SYNC_SHOULD_CONTINUE_HEADER = 'x-kobo-sync'; #[ORM\Id] #[ORM\GeneratedValue] diff --git a/src/Kobo/Response/SyncResponse.php b/src/Kobo/Response/SyncResponse.php index 0a5835db..7d5091a0 100644 --- a/src/Kobo/Response/SyncResponse.php +++ b/src/Kobo/Response/SyncResponse.php @@ -138,7 +138,7 @@ public function createReadingState(Book $book): array private function getChangedEntitlement(): array { $books = array_filter($this->books, function (Book $book) { - return $this->helper->isChangedEntitlement($book, $this->koboDevice, $this->syncToken); + return $this->helper->isChangedEntitlement($book, $this->syncToken); }); return array_map(function (Book $book) { @@ -174,7 +174,7 @@ private function getNewEntitlement(): array private function getNewTags(): array { $shelves = array_filter($this->shelves, function (Shelf $shelf) { - return $shelf->getCreated() >= $this->syncToken->lastCreated; + return $this->helper->isNewTag($shelf, $this->syncToken); }); return array_map(function (Shelf $shelf) { @@ -192,7 +192,7 @@ private function getNewTags(): array private function getChangedTag(): array { $shelves = array_filter($this->shelves, function (Shelf $shelf) { - return $this->syncToken->lastModified instanceof \DateTimeInterface && $shelf->getUpdated() < $this->syncToken->lastModified; + return $this->helper->isChangedTag($shelf, $this->syncToken); }); return array_map(function (Shelf $shelf) { diff --git a/src/Kobo/Response/SyncResponseHelper.php b/src/Kobo/Response/SyncResponseHelper.php index 9cc1ddfd..9e40d9d1 100644 --- a/src/Kobo/Response/SyncResponseHelper.php +++ b/src/Kobo/Response/SyncResponseHelper.php @@ -5,6 +5,7 @@ use App\Entity\Book; use App\Entity\BookInteraction; use App\Entity\KoboDevice; +use App\Entity\Shelf; use App\Kobo\SyncToken; // Inspired by https://github.com/janeczku/calibre-web/blob/master/cps/kobo.py @@ -17,19 +18,17 @@ */ class SyncResponseHelper { - public function isChangedEntitlement(Book $book, KoboDevice $koboDevice, SyncToken $syncToken): bool + public function isChangedEntitlement(Book $book, SyncToken $syncToken): bool { if ($this->isNewEntitlement($book, $syncToken)) { return false; } - if ($this->isChangedReadingState($book, $koboDevice, $syncToken)) { + if (!$syncToken->lastModified instanceof \DateTimeInterface) { return false; } - return ($syncToken->lastModified instanceof \DateTimeInterface && $book->getUpdated() instanceof \DateTimeInterface - && $book->getUpdated() >= $syncToken->lastModified) - || ($syncToken->lastCreated instanceof \DateTimeInterface && $book->getCreated() >= $syncToken->lastCreated); + return $book->getUpdated() >= $syncToken->lastModified; } public function isNewEntitlement(Book $book, SyncToken $syncToken): bool @@ -39,11 +38,30 @@ public function isNewEntitlement(Book $book, SyncToken $syncToken): bool public function isChangedReadingState(Book $book, KoboDevice $koboDevice, SyncToken $syncToken): bool { - if ($this->isNewEntitlement($book, $syncToken)) { + if ($this->isChangedEntitlement($book, $syncToken)) { + return false; + } + + if (!$syncToken->readingStateLastModified instanceof \DateTimeInterface) { return false; } + $lastInteraction = $book->getLastInteraction($koboDevice->getUser()); return ($lastInteraction instanceof BookInteraction) && $lastInteraction->getUpdated() >= $syncToken->readingStateLastModified; } + + public function isNewTag(Shelf $shelf, SyncToken $syncToken): bool + { + if (!$syncToken->lastCreated instanceof \DateTimeInterface) { + return true; + } + + return $shelf->getCreated() >= $syncToken->lastCreated; + } + + public function isChangedTag(Shelf $shelf, SyncToken $syncToken): bool + { + return $syncToken->tagLastModified instanceof \DateTimeInterface && $shelf->getUpdated() >= $syncToken->tagLastModified; + } } diff --git a/src/Kobo/UpstreamSyncMerger.php b/src/Kobo/UpstreamSyncMerger.php index 325235d9..0ea95f2c 100644 --- a/src/Kobo/UpstreamSyncMerger.php +++ b/src/Kobo/UpstreamSyncMerger.php @@ -80,6 +80,6 @@ private function parseJson(Response $response): array private function shouldContinue(Response $response): bool { - return $response->headers->get('x-kobo-sync') === 'continue'; + return $response->headers->get(KoboDevice::KOBO_SYNC_SHOULD_CONTINUE_HEADER) === 'continue'; } } diff --git a/src/Repository/BookRepository.php b/src/Repository/BookRepository.php index 7333ad3a..a6becb0b 100644 --- a/src/Repository/BookRepository.php +++ b/src/Repository/BookRepository.php @@ -392,24 +392,28 @@ private function getChangedBooksQueryBuilder(KoboDevice $koboDevice, SyncToken $ ->setParameter('koboDevice', $koboDevice) ->setParameter('extension', 'epub') // Pdf is not supported by kobo sync ->groupBy('book.id'); + $bigOr = $qb->expr()->orX(); + if ($syncToken->lastCreated instanceof \DateTimeInterface) { - $qb->andWhere($qb->expr()->orX( + $bigOr->addMultiple([ $qb->expr()->isNull('koboSyncedBooks.created'), $qb->expr()->gte('book.created', ':lastCreated'), - )) - ->setParameter('lastCreated', $syncToken->lastCreated); + ]); + $qb->setParameter('lastCreated', $syncToken->lastCreated); } if ($syncToken->lastModified instanceof \DateTimeInterface) { - $qb->andWhere($qb->expr()->orX( + $bigOr->addMultiple([ 'book.updated > :lastModified', 'book.created > :lastModified', 'koboSyncedBooks.updated > :lastModified', $qb->expr()->isNull('koboSyncedBooks.updated'), - )); + ]); $qb->setParameter('lastModified', $syncToken->lastModified); } + $qb->andWhere($bigOr); + $qb->orderBy('book.updated'); if ($syncToken->filters['PrioritizeRecentReads'] ?? false) { $qb->orderBy('bookInteractions.updated', 'ASC'); diff --git a/tests/Controller/Kobo/KoboSyncControllerTest.php b/tests/Controller/Kobo/KoboSyncControllerTest.php index 6dabc9f9..973a6a7c 100644 --- a/tests/Controller/Kobo/KoboSyncControllerTest.php +++ b/tests/Controller/Kobo/KoboSyncControllerTest.php @@ -3,6 +3,7 @@ namespace App\Tests\Controller\Kobo; use App\DataFixtures\BookFixture; +use App\Entity\KoboDevice; use App\Entity\KoboSyncedBook; use App\Kobo\Response\MetadataResponseService; use App\Kobo\SyncToken; @@ -10,6 +11,7 @@ use App\Service\KoboSyncTokenExtractor; use App\Tests\Contraints\AssertHasDownloadWithFormat; use App\Tests\Contraints\JSONIsValidSyncResponse; +use App\Tests\TestClock; class KoboSyncControllerTest extends AbstractKoboControllerTest { @@ -18,7 +20,7 @@ protected function tearDown(): void $this->getKoboStoreProxy()->setClient(null); $this->getKoboProxyConfiguration()->setEnabled(false); $this->getEntityManager()->getRepository(KoboSyncedBook::class)->deleteAllSyncedBooks(1); - + $this->getService(TestClock::class)->setTime(null); parent::tearDown(); } @@ -117,7 +119,7 @@ public function testSyncControllerPaginated() : void }, $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'); + self::assertResponseHeaderSame(KoboDevice::KOBO_SYNC_SHOULD_CONTINUE_HEADER, $expectedContinueHeader, 'x-kobo-sync is invalid'); } $count = $this->getEntityManager()->getRepository(KoboSyncedBook::class)->count(['koboDevice' => 1]); @@ -132,6 +134,50 @@ public function testSyncControllerPaginated() : void } + /** + * @covers ChangedEntitlement + * @throws \JsonException + * @throws \DateMalformedStringException + */ + public function testSyncControllerEdited() : void{ + $client = static::getClient(); + + // Create an old sync token + $clock = $this->getService(TestClock::class) + ->setTime(new \DateTimeImmutable('now')); + $syncToken = new SyncToken(); + $syncToken->lastCreated = $clock->now(); + $syncToken->lastModified = $clock->now(); + $syncToken->tagLastModified = $clock->now(); + + // Sync all the books + $headers = $this->getService(KoboSyncTokenExtractor::class)->getTestHeader($syncToken); + $client?->request('GET', '/kobo/' . $this->accessKey . '/v1/library/sync', [], [], $headers); + self::assertResponseIsSuccessful(); + + // Edit the book detail + $clock->setTime($clock->now()->modify('+ 1 hour')); + $book = $this->getBook(); + $slug = $book->getSlug(); + $book->setSlug($slug.".."); + $this->getEntityManager()->flush(); + $book->setSlug($slug); + $this->getEntityManager()->flush(); + + // Restore the real time + $clock->setTime(null); + + // Make sure the book has changed. + $client?->request('GET', '/kobo/' . $this->accessKey . '/v1/library/sync', [], [], $headers); + self::assertResponseIsSuccessful(); + self::assertThat(self::getJsonResponse(), new JSONIsValidSyncResponse([ + 'ChangedEntitlement' => 1, + ])); + + self::assertResponseHeaderSame(KoboDevice::KOBO_SYNC_SHOULD_CONTINUE_HEADER, 'done', 'x-kobo-sync is invalid'); + } + + public function testSyncControllerWithRemote() : void { $client = static::getClient(); diff --git a/tests/TestClock.php b/tests/TestClock.php new file mode 100644 index 00000000..243eb0bf --- /dev/null +++ b/tests/TestClock.php @@ -0,0 +1,21 @@ +