Merge pull request #428 from leepeuker/improve-cli-commands

Improve tmdb sync logging and cli command
leepeuker · Jul 7, 2023 · 8b1cf6c · 8b1cf6c
2 parents 3adc53d + c057918
commit 8b1cf6c
Show file tree

Hide file tree

Showing 13 changed files with 131 additions and 40 deletions.
diff --git a/src/Api/Imdb/ImdbWebScrapper.php b/src/Api/Imdb/ImdbWebScrapper.php
@@ -49,7 +49,7 @@ public function findRating(string $imdbId) : ?ImdbRating
 
         $imdbRating = ImdbRating::create($ratingAverage, $ratingVoteCount);
 
-        $this->logger->debug('IMDb: Found movie rating.', [
+        $this->logger->debug('IMDb: Found movie rating', [
             'url' => $this->urlGenerator->buildMovieUrl($imdbId),
             'average' => $imdbRating->getRating(),
             'voteCount' => $imdbRating->getVotesCount(),

diff --git a/src/Command/ImdbSync.php b/src/Command/ImdbSync.php
@@ -2,6 +2,7 @@
 
 namespace Movary\Command;
 
+use Movary\Command\Mapper\InputMapper;
 use Movary\JobQueue\JobQueueApi;
 use Movary\Service\Imdb\ImdbMovieRatingSync;
 use Movary\ValueObject\JobStatus;
@@ -24,6 +25,7 @@ class ImdbSync extends Command
     public function __construct(
         private readonly ImdbMovieRatingSync $imdbMovieRatingSync,
         private readonly JobQueueApi $jobQueueApi,
+        private readonly InputMapper $inputMapper,
         private readonly LoggerInterface $logger,
     ) {
         parent::__construct();
@@ -40,14 +42,9 @@ protected function configure() : void
 
     protected function execute(InputInterface $input, OutputInterface $output) : int
     {
-        $hoursOption = $input->getOption(self::OPTION_NAME_FORCE_HOURS);
-        $maxAgeInHours = $hoursOption !== null ? (int)$hoursOption : null;
-
-        $thresholdOption = $input->getOption(self::OPTION_NAME_FORCE_THRESHOLD);
-        $movieCountSyncThreshold = (int)$thresholdOption !== 0 ? (int)$thresholdOption : null;
-
-        $movieIdsOption = $input->getOption(self::OPTION_NAME_MOVIE_IDS);
-        $movieIds = (string)$movieIdsOption !== '' ? array_map('intval', explode(',', $movieIdsOption)) : null;
+        $movieCountSyncThreshold = $this->inputMapper->mapOptionToInteger($input, self::OPTION_NAME_FORCE_THRESHOLD);
+        $maxAgeInHours = $this->inputMapper->mapOptionToInteger($input, self::OPTION_NAME_FORCE_HOURS);
+        $movieIds = $this->inputMapper->mapOptionToIds($input, self::OPTION_NAME_MOVIE_IDS);
 
         $jobId = $this->jobQueueApi->addImdbSyncJob(JobStatus::createInProgress());
 

diff --git a/src/Command/Mapper/InputMapper.php b/src/Command/Mapper/InputMapper.php
@@ -0,0 +1,44 @@
+<?php declare(strict_types=1);
+
+namespace Movary\Command\Mapper;
+
+use Symfony\Component\Console\Input\InputInterface;
+
+class InputMapper
+{
+    public function mapOptionToIds(InputInterface $input, string $optionName) : ?array
+    {
+        $optionValue = $input->getOption($optionName);
+
+        if ($optionValue === null) {
+            return null;
+        }
+
+        $ids = [];
+
+        foreach (explode(',', $optionValue) as $idValue) {
+            if (ctype_digit($idValue) === false) {
+                throw new \RuntimeException('Option must be a comma separated string of only numbers: ' . $optionName);
+            }
+
+            $ids[] = (int)$idValue;
+        }
+
+        return $ids;
+    }
+
+    public function mapOptionToInteger(InputInterface $input, string $optionName) : ?int
+    {
+        $optionValue = $input->getOption($optionName);
+
+        if ($optionValue === null) {
+            return null;
+        }
+
+        if (ctype_digit($optionValue) === false) {
+            throw new \RuntimeException('Option must be a number: ' . $optionName);
+        }
+
+        return (int)$optionValue;
+    }
+}
diff --git a/src/Command/TmdbMovieSync.php b/src/Command/TmdbMovieSync.php
@@ -2,6 +2,7 @@
 
 namespace Movary\Command;
 
+use Movary\Command\Mapper\InputMapper;
 use Movary\JobQueue\JobQueueApi;
 use Movary\Service\Tmdb\SyncMovies;
 use Movary\ValueObject\JobStatus;
@@ -17,11 +18,14 @@ class TmdbMovieSync extends Command
 
     private const OPTION_NAME_FORCE_THRESHOLD = 'threshold';
 
+    private const OPTION_NAME_MOVIE_IDS = 'movieIds';
+
     protected static $defaultName = 'tmdb:movie:sync';
 
     public function __construct(
         private readonly SyncMovies $syncMovieDetails,
         private readonly JobQueueApi $jobQueueApi,
+        private readonly InputMapper $inputMapper,
         private readonly LoggerInterface $logger,
     ) {
         parent::__construct();
@@ -32,23 +36,22 @@ protected function configure() : void
         $this
             ->setDescription('Sync themoviedb.org meta data for local movies.')
             ->addOption(self::OPTION_NAME_FORCE_THRESHOLD, 'threshold', InputOption::VALUE_REQUIRED, 'Max number of movies to sync.')
-            ->addOption(self::OPTION_NAME_FORCE_HOURS, 'hours', InputOption::VALUE_REQUIRED, 'Hours since last updated.');
+            ->addOption(self::OPTION_NAME_FORCE_HOURS, 'hours', InputOption::VALUE_REQUIRED, 'Hours since last updated.')
+            ->addOption(self::OPTION_NAME_MOVIE_IDS, 'movieIds', InputOption::VALUE_REQUIRED, 'Comma seperated ids of movies to sync.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output) : int
     {
-        $hoursOption = $input->getOption(self::OPTION_NAME_FORCE_HOURS);
-        $maxAgeInHours = $hoursOption !== null ? (int)$hoursOption : null;
-
-        $thresholdOption = $input->getOption(self::OPTION_NAME_FORCE_THRESHOLD);
-        $movieCountSyncThreshold = $thresholdOption !== null ? (int)$thresholdOption : null;
+        $maxAgeInHours = $this->inputMapper->mapOptionToInteger($input, self::OPTION_NAME_FORCE_HOURS);
+        $maxSyncsThreshold = $this->inputMapper->mapOptionToInteger($input, self::OPTION_NAME_FORCE_THRESHOLD);
+        $movieIds = $this->inputMapper->mapOptionToIds($input, self::OPTION_NAME_MOVIE_IDS);
 
         $jobId = $this->jobQueueApi->addTmdbMovieSyncJob(JobStatus::createInProgress());
 
         try {
             $this->generateOutput($output, 'Syncing movie meta data...');
 
-            $this->syncMovieDetails->syncMovies($maxAgeInHours, $movieCountSyncThreshold);
+            $this->syncMovieDetails->syncMovies($maxAgeInHours, $maxSyncsThreshold, $movieIds);
 
             $this->jobQueueApi->updateJobStatus($jobId, JobStatus::createDone());
 

diff --git a/src/Command/TmdbPersonSync.php b/src/Command/TmdbPersonSync.php
@@ -2,6 +2,7 @@
 
 namespace Movary\Command;
 
+use Movary\Command\Mapper\InputMapper;
 use Movary\JobQueue\JobQueueApi;
 use Movary\Service\Tmdb\SyncPersons;
 use Movary\ValueObject\JobStatus;
@@ -17,11 +18,14 @@ class TmdbPersonSync extends Command
 
     private const OPTION_NAME_FORCE_THRESHOLD = 'threshold';
 
+    private const OPTION_NAME_PERSON_IDS = 'personIds';
+
     protected static $defaultName = 'tmdb:person:sync';
 
     public function __construct(
         private readonly SyncPersons $syncPersons,
         private readonly JobQueueApi $jobQueueApi,
+        private readonly InputMapper $inputMapper,
         private readonly LoggerInterface $logger,
     ) {
         parent::__construct();
@@ -32,23 +36,22 @@ protected function configure() : void
         $this
             ->setDescription('Sync themoviedb.org meta data for local persons.')
             ->addOption(self::OPTION_NAME_FORCE_THRESHOLD, 'threshold', InputOption::VALUE_REQUIRED, 'Max number of persons to sync.')
-            ->addOption(self::OPTION_NAME_FORCE_HOURS, 'hours', InputOption::VALUE_REQUIRED, 'Hours since last updated.');
+            ->addOption(self::OPTION_NAME_FORCE_HOURS, 'hours', InputOption::VALUE_REQUIRED, 'Hours since last updated.')
+            ->addOption(self::OPTION_NAME_PERSON_IDS, 'personIds', InputOption::VALUE_REQUIRED, 'Comma seperated ids of persons to sync.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output) : int
     {
-        $hoursOption = $input->getOption(self::OPTION_NAME_FORCE_HOURS);
-        $maxAgeInHours = $hoursOption !== null ? (int)$hoursOption : null;
-
-        $thresholdOption = $input->getOption(self::OPTION_NAME_FORCE_THRESHOLD);
-        $personCountSyncThreshold = $thresholdOption !== null ? (int)$thresholdOption : null;
+        $maxAgeInHours = $this->inputMapper->mapOptionToInteger($input, self::OPTION_NAME_FORCE_HOURS);
+        $maxSyncsThreshold = $this->inputMapper->mapOptionToInteger($input, self::OPTION_NAME_FORCE_THRESHOLD);
+        $personIds = $this->inputMapper->mapOptionToIds($input, self::OPTION_NAME_PERSON_IDS);
 
         $jobId = $this->jobQueueApi->addTmdbPersonSyncJob(JobStatus::createInProgress());
 
         try {
             $this->generateOutput($output, 'Syncing person meta data...');
 
-            $this->syncPersons->syncPersons($maxAgeInHours, $personCountSyncThreshold);
+            $this->syncPersons->syncPersons($maxAgeInHours, $maxSyncsThreshold, $personIds);
 
             $this->jobQueueApi->updateJobStatus($jobId, JobStatus::createDone());
 

diff --git a/src/Domain/Movie/MovieApi.php b/src/Domain/Movie/MovieApi.php
@@ -110,9 +110,9 @@ public function fetchAll() : MovieEntityList
         return $this->repository->fetchAll();
     }
 
-    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null) : \Traversable
+    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null, ?array $ids = null) : \Traversable
     {
-        return $this->movieRepository->fetchAllOrderedByLastUpdatedAtTmdbAsc($limit);
+        return $this->movieRepository->fetchAllOrderedByLastUpdatedAtTmdbAsc($limit, $ids);
     }
 
     public function fetchById(int $movieId) : MovieEntity

diff --git a/src/Domain/Movie/MovieRepository.php b/src/Domain/Movie/MovieRepository.php
@@ -151,15 +151,21 @@ public function fetchAll() : MovieEntityList
         return MovieEntityList::createFromArray($data);
     }
 
-    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null) : \Traversable
+    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null, ?array $ids = null) : \Traversable
     {
-        $query = 'SELECT * FROM `movie` ORDER BY updated_at_tmdb ASC';
+        $whereQuery = '';
+        if ($ids !== null && count($ids) > 0) {
+            $placeholders = str_repeat('?, ', count($ids));
+            $whereQuery = ' WHERE id IN (' . trim($placeholders, ', ') . ')';
+        }
+
+        $query = "SELECT * FROM `movie` $whereQuery ORDER BY updated_at_tmdb, created_at";
 
         if ($limit !== null) {
             $query .= ' LIMIT ' . $limit;
         }
 
-        return $this->dbConnection->prepare($query)->executeQuery()->iterateAssociative();
+        return $this->dbConnection->prepare($query)->executeQuery($ids ?? [])->iterateAssociative();
     }
 
     public function fetchAveragePersonalRating(int $userId) : float
@@ -437,6 +443,8 @@ public function fetchMovieIdsHavingImdbIdOrderedByLastImdbUpdatedAt(?int $maxAge
         }
 
         if ($this->dbConnection->getDatabasePlatform() instanceof SqlitePlatform) {
+            $maxAgeInHours = $maxAgeInHours ?? 0;
+
             return $this->dbConnection->fetchFirstColumn(
                 'SELECT movie.id
                 FROM `movie` 

diff --git a/src/Domain/Person/PersonApi.php b/src/Domain/Person/PersonApi.php
@@ -84,9 +84,9 @@ public function deleteById(int $id) : void
         $this->repository->deleteById($id);
     }
 
-    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null) : \Traversable
+    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null, ?array $ids = null) : \Traversable
     {
-        return $this->repository->fetchAllOrderedByLastUpdatedAtTmdbAsc($limit);
+        return $this->repository->fetchAllOrderedByLastUpdatedAtTmdbAsc($limit, $ids);
     }
 
     public function findById(int $personId) : ?PersonEntity

diff --git a/src/Domain/Person/PersonRepository.php b/src/Domain/Person/PersonRepository.php
@@ -64,15 +64,21 @@ public function deleteById(int $id) : void
         $this->dbConnection->delete('person', ['id' => $id]);
     }
 
-    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null) : \Traversable
+    public function fetchAllOrderedByLastUpdatedAtTmdbAsc(?int $limit = null, ?array $ids = null) : \Traversable
     {
-        $query = 'SELECT * FROM `person` ORDER BY updated_at_tmdb ASC';
+        $whereQuery = '';
+        if ($ids !== null && count($ids) > 0) {
+            $placeholders = str_repeat('?, ', count($ids));
+            $whereQuery = ' WHERE id IN (' . trim($placeholders, ', ') . ')';
+        }
+
+        $query = "SELECT * FROM `person` $whereQuery ORDER BY updated_at_tmdb, created_at";
 
         if ($limit !== null) {
             $query .= ' LIMIT ' . $limit;
         }
 
-        return $this->dbConnection->prepare($query)->executeQuery()->iterateAssociative();
+        return $this->dbConnection->prepare($query)->executeQuery($ids ?? [])->iterateAssociative();
     }
 
     public function findByPersonId(int $personId) : ?PersonEntity

diff --git a/src/Service/Tmdb/SyncMovie.php b/src/Service/Tmdb/SyncMovie.php
@@ -8,6 +8,7 @@
 use Movary\Domain\Movie\MovieEntity;
 use Movary\JobQueue\JobQueueScheduler;
 use Movary\ValueObject\Date;
+use Psr\Log\LoggerInterface;
 use Throwable;
 
 class SyncMovie
@@ -21,6 +22,7 @@ public function __construct(
         private readonly ProductionCompanyConverter $productionCompanyConverter,
         private readonly Connection $dbConnection,
         private readonly JobQueueScheduler $jobScheduler,
+        private readonly LoggerInterface $logger,
     ) {
     }
 
@@ -39,6 +41,8 @@ public function syncMovie(int $tmdbId) : MovieEntity
 
         $this->dbConnection->beginTransaction();
 
+        $createdMovie = false;
+
         try {
             if ($movie === null) {
                 $movie = $this->movieApi->create(
@@ -57,6 +61,8 @@ public function syncMovie(int $tmdbId) : MovieEntity
                 );
 
                 $this->jobScheduler->storeMovieIdForTmdbImageCacheJob($movie->getId());
+
+                $createdMovie = true;
             } else {
                 $originalTmdbPosterPath = $movie->getTmdbPosterPath();
 
@@ -91,6 +97,12 @@ public function syncMovie(int $tmdbId) : MovieEntity
             throw $e;
         }
 
+        if ($createdMovie === true) {
+            $this->logger->debug('TMDB: Created movie meta data', ['movieId' => $movie->getId(), 'tmdbId' => $movie->getTmdbId()]);
+        } else {
+            $this->logger->debug('TMDB: Updated movie meta data', ['movieId' => $movie->getId(), 'tmdbId' => $movie->getTmdbId()]);
+        }
+
         return $movie;
     }
 

diff --git a/src/Service/Tmdb/SyncMovies.php b/src/Service/Tmdb/SyncMovies.php
@@ -17,9 +17,9 @@ public function __construct(
     ) {
     }
 
-    public function syncMovies(?int $maxAgeInHours = null, ?int $movieCountSyncThreshold = null) : void
+    public function syncMovies(?int $maxAgeInHours = null, ?int $movieCountSyncThreshold = null, ?array $ids = []) : void
     {
-        $movies = $this->movieApi->fetchAllOrderedByLastUpdatedAtTmdbAsc($movieCountSyncThreshold);
+        $movies = $this->movieApi->fetchAllOrderedByLastUpdatedAtTmdbAsc($movieCountSyncThreshold, $ids);
 
         foreach ($movies as $movie) {
             $movie = MovieEntity::createFromArray($movie);
@@ -34,7 +34,14 @@ public function syncMovies(?int $maxAgeInHours = null, ?int $movieCountSyncThres
             try {
                 $this->syncMovieService->syncMovie($movie->getTmdbId());
             } catch (Throwable $t) {
-                $this->logger->warning('Could not sync movie with id "' . $movie->getId() . '". Error: ' . $t->getMessage(), ['exception' => $t]);
+                $this->logger->warning(
+                    'TMDB: Could not update movie.',
+                    [
+                        'exception' => $t,
+                        'movieId' => $movie->getId(),
+                        'tmdbId' => $movie->getTmdbId(),
+                    ],
+                );
             }
         }
     }

diff --git a/src/Service/Tmdb/SyncPerson.php b/src/Service/Tmdb/SyncPerson.php
@@ -30,7 +30,7 @@ public function syncPerson(int $tmdbId) : void
                 $this->personApi->deleteById($person->getId());
             }
 
-            $this->logger->debug('No person existing on tmdb with id: ' . $tmdbId);
+            $this->logger->debug('TMDB: Could not update person, tmdb id not found', ['tmdbId' => $tmdbId]);
 
             return;
         }
@@ -50,6 +50,8 @@ public function syncPerson(int $tmdbId) : void
                 updatedAtTmdb: DateTime::create(),
             );
 
+            $this->logger->debug('TMDB: Created person meta data', ['personId' => $person->getId(), 'tmdbId' => $person->getTmdbId()]);
+
             $this->jobScheduler->storePersonIdForTmdbImageCacheJob($person->getId());
 
             return;
@@ -70,6 +72,8 @@ public function syncPerson(int $tmdbId) : void
             DateTime::create(),
         );
 
+        $this->logger->debug('TMDB: Updated person meta data', ['personId' => $person->getId(), 'tmdbId' => $person->getTmdbId()]);
+
         if ($originalTmdbPosterPath !== $person->getTmdbPosterPath()) {
             $this->jobScheduler->storePersonIdForTmdbImageCacheJob($person->getId());
         }