From 76afc0cfa2feb087697bae4bc138e4956873dd62 Mon Sep 17 00:00:00 2001 From: Yassine Doghri Date: Mon, 31 May 2021 13:32:33 +0000 Subject: [PATCH] perf(cache): use deleteMatching method to prevent forgetting cached elements in models --- app/Controllers/Admin/EpisodeController.php | 50 ++++++------ app/Controllers/EpisodeController.php | 2 +- .../Seeds/FakePodcastsAnalyticsSeeder.php | 8 +- .../Seeds/FakeWebsiteAnalyticsSeeder.php | 6 +- .../Controllers/EpisodeController.php | 2 +- app/Models/EpisodeModel.php | 81 ++++--------------- app/Models/PersonModel.php | 22 +++-- .../podcast/_partials/note_authenticated.php | 6 +- .../podcast/_partials/reply_authenticated.php | 6 +- 9 files changed, 68 insertions(+), 115 deletions(-) diff --git a/app/Controllers/Admin/EpisodeController.php b/app/Controllers/Admin/EpisodeController.php index b7c50042..385a94dc 100644 --- a/app/Controllers/Admin/EpisodeController.php +++ b/app/Controllers/Admin/EpisodeController.php @@ -26,7 +26,7 @@ class EpisodeController extends BaseController { protected Podcast $podcast; - protected ?Episode $episode; + protected Episode $episode; public function _remap(string $method, string ...$params): mixed { @@ -253,7 +253,7 @@ class EpisodeController extends BaseController $this->episode->is_blocked = $this->request->getPost('block') === 'yes'; $this->episode->custom_rss_string = $this->request->getPost('custom_rss',); - $this->episode->updated_by = user_id(); + $this->episode->updated_by = (int) user_id(); $audioFile = $this->request->getFile('audio_file'); if ($audioFile !== null && $audioFile->isValid()) { @@ -268,7 +268,7 @@ class EpisodeController extends BaseController $transcriptChoice = $this->request->getPost('transcript-choice'); if ($transcriptChoice === 'upload-file') { $transcriptFile = $this->request->getFile('transcript_file'); - if ($transcriptFile->isValid()) { + if ($transcriptFile !== null && $transcriptFile->isValid()) { $this->episode->transcript_file = $transcriptFile; $this->episode->transcript_file_remote_url = null; } @@ -287,7 +287,7 @@ class EpisodeController extends BaseController $chaptersChoice = $this->request->getPost('chapters-choice'); if ($chaptersChoice === 'upload-file') { $chaptersFile = $this->request->getFile('chapters_file'); - if ($chaptersFile->isValid()) { + if ($chaptersFile !== null && $chaptersFile->isValid()) { $this->episode->chapters_file = $chaptersFile; $this->episode->chapters_file_remote_url = null; } @@ -411,13 +411,11 @@ class EpisodeController extends BaseController if ($publishMethod === 'schedule') { $scheduledPublicationDate = $this->request->getPost('scheduled_publication_date',); if ($scheduledPublicationDate) { - $scheduledDateUTC = Time::createFromFormat( + $this->episode->published_at = Time::createFromFormat( 'Y-m-d H:i', $scheduledPublicationDate, $this->request->getPost('client_timezone'), )->setTimezone('UTC'); - $this->episode->published_at = $scheduledDateUTC; - $newNote->published_at = $scheduledDateUTC; } else { $db->transRollback(); return redirect() @@ -426,11 +424,11 @@ class EpisodeController extends BaseController ->with('error', 'Schedule date must be set!'); } } else { - $dateNow = Time::now(); - $this->episode->published_at = $dateNow; - $newNote->published_at = $dateNow; + $this->episode->published_at = Time::now(); } + $newNote->published_at = $this->episode->published_at; + $noteModel = new NoteModel(); if (! $noteModel->addNote($newNote)) { $db->transRollback(); @@ -498,20 +496,15 @@ class EpisodeController extends BaseController $db = Database::connect(); $db->transStart(); - $note = (new NoteModel())->getNoteById($this->request->getPost('note_id'),); - $note->message = $this->request->getPost('message'); - $publishMethod = $this->request->getPost('publication_method'); if ($publishMethod === 'schedule') { $scheduledPublicationDate = $this->request->getPost('scheduled_publication_date',); if ($scheduledPublicationDate) { - $scheduledDateUTC = Time::createFromFormat( + $this->episode->published_at = Time::createFromFormat( 'Y-m-d H:i', $scheduledPublicationDate, $this->request->getPost('client_timezone'), )->setTimezone('UTC'); - $this->episode->published_at = $scheduledDateUTC; - $note->published_at = $scheduledDateUTC; } else { $db->transRollback(); return redirect() @@ -520,18 +513,23 @@ class EpisodeController extends BaseController ->with('error', 'Schedule date must be set!'); } } else { - $dateNow = Time::now(); - $this->episode->published_at = $dateNow; - $note->published_at = $dateNow; + $this->episode->published_at = Time::now(); } - $noteModel = new NoteModel(); - if (! $noteModel->editNote($note)) { - $db->transRollback(); - return redirect() - ->back() - ->withInput() - ->with('errors', $noteModel->errors()); + $note = (new NoteModel())->getNoteById($this->request->getPost('note_id'),); + + if ($note !== null) { + $note->message = $this->request->getPost('message'); + $note->published_at = $this->episode->published_at; + + $noteModel = new NoteModel(); + if (! $noteModel->editNote($note)) { + $db->transRollback(); + return redirect() + ->back() + ->withInput() + ->with('errors', $noteModel->errors()); + } } $episodeModel = new EpisodeModel(); diff --git a/app/Controllers/EpisodeController.php b/app/Controllers/EpisodeController.php index 6e8cf7ff..96c12894 100644 --- a/app/Controllers/EpisodeController.php +++ b/app/Controllers/EpisodeController.php @@ -39,7 +39,7 @@ class EpisodeController extends BaseController } if ( - ($this->episode = (new EpisodeModel())->getEpisodeBySlug($this->podcast->id, $params[1],)) !== null + ($this->episode = (new EpisodeModel())->getEpisodeBySlug($params[0], $params[1],)) !== null ) { unset($params[1]); unset($params[0]); diff --git a/app/Database/Seeds/FakePodcastsAnalyticsSeeder.php b/app/Database/Seeds/FakePodcastsAnalyticsSeeder.php index 0b15bda7..15c71043 100644 --- a/app/Database/Seeds/FakePodcastsAnalyticsSeeder.php +++ b/app/Database/Seeds/FakePodcastsAnalyticsSeeder.php @@ -57,10 +57,8 @@ class FakePodcastsAnalyticsSeeder extends Seeder $analyticsPodcastsByRegion = []; $episodes = (new EpisodeModel()) - ->where([ - 'podcast_id' => $podcast->id, - 'DATE(published_at) <=' => date('Y-m-d', $date), - ]) + ->where('podcast_id', $podcast->id) + ->where('`published_at` <= NOW()', null, false) ->findAll(); foreach ($episodes as $episode) { $age = floor(($date - strtotime($episode->published_at)) / 86400,); @@ -110,7 +108,7 @@ class FakePodcastsAnalyticsSeeder extends Seeder ? 'N/A' : $city->country->isoCode; - $regionCode = $city->subdivisions[0]->isoCode === null + $regionCode = $city->subdivisions === [] ? 'N/A' : $city->subdivisions[0]->isoCode; $latitude = round($city->location->latitude, 3); diff --git a/app/Database/Seeds/FakeWebsiteAnalyticsSeeder.php b/app/Database/Seeds/FakeWebsiteAnalyticsSeeder.php index aead2496..ac105498 100644 --- a/app/Database/Seeds/FakeWebsiteAnalyticsSeeder.php +++ b/app/Database/Seeds/FakeWebsiteAnalyticsSeeder.php @@ -196,10 +196,8 @@ class FakeWebsiteAnalyticsSeeder extends Seeder $websiteByReferer = []; $episodes = (new EpisodeModel()) - ->where([ - 'podcast_id' => $podcast->id, - 'DATE(published_at) <=' => date('Y-m-d', $date), - ]) + ->where('podcast_id', $podcast->id) + ->where('`published_at` <= NOW()', null, false) ->findAll(); foreach ($episodes as $episode) { $age = floor(($date - strtotime($episode->published_at)) / 86400,); diff --git a/app/Libraries/Analytics/Controllers/EpisodeController.php b/app/Libraries/Analytics/Controllers/EpisodeController.php index 6e8cf7ff..96c12894 100644 --- a/app/Libraries/Analytics/Controllers/EpisodeController.php +++ b/app/Libraries/Analytics/Controllers/EpisodeController.php @@ -39,7 +39,7 @@ class EpisodeController extends BaseController } if ( - ($this->episode = (new EpisodeModel())->getEpisodeBySlug($this->podcast->id, $params[1],)) !== null + ($this->episode = (new EpisodeModel())->getEpisodeBySlug($params[0], $params[1],)) !== null ) { unset($params[1]); unset($params[0]); diff --git a/app/Models/EpisodeModel.php b/app/Models/EpisodeModel.php index 8b0a2828..225c1889 100644 --- a/app/Models/EpisodeModel.php +++ b/app/Models/EpisodeModel.php @@ -13,9 +13,9 @@ use CodeIgniter\Model; class EpisodeModel extends Model { - // TODO: remove - /** + * TODO: remove, shouldn't be here + * * @var array> */ public static $themes = [ @@ -135,45 +135,26 @@ class EpisodeModel extends Model */ protected $afterInsert = ['writeEnclosureMetadata', 'clearCache']; - // clear cache beforeUpdate because if slug changes, so will the episode link - /** * @var string[] */ - protected $beforeUpdate = ['clearCache']; - - /** - * @var string[] - */ - protected $afterUpdate = ['writeEnclosureMetadata']; + protected $afterUpdate = ['clearCache', 'writeEnclosureMetadata']; /** * @var string[] */ protected $beforeDelete = ['clearCache']; - /** - * @param int|string $podcastId may be the id or podcast name - */ - public function getEpisodeBySlug(int | string $podcastId, string $episodeSlug): ?Episode + public function getEpisodeBySlug(string $podcastName, string $episodeSlug): ?Episode { - $cacheName = "podcast#{$podcastId}_episode-{$episodeSlug}"; + $cacheName = "podcast-{$podcastName}_episode-{$episodeSlug}"; if (! ($found = cache($cacheName))) { - $builder = $this->select('episodes.*') + $found = $this->select('episodes.*') + ->join('podcasts', 'podcasts.id = episodes.podcast_id') ->where('slug', $episodeSlug) - ->where('`published_at` <= NOW()', null, false); - - if (is_numeric($podcastId)) { - // passed argument is the podcast id - $builder->where('podcast_id', $podcastId); - } else { - // passed argument is the podcast name, must perform join - $builder - ->join('podcasts', 'podcasts.id = episodes.podcast_id') - ->where('podcasts.name', $podcastId); - } - - $found = $builder->first(); + ->where('podcasts.name', $podcastName) + ->where('`published_at` <= NOW()', null, false) + ->first(); cache() ->save($cacheName, $found, DECADE); @@ -278,9 +259,9 @@ class EpisodeModel extends Model * Returns the timestamp difference in seconds between the next episode to publish and the current timestamp Returns * false if there's no episode to publish * - * @return int|bool seconds + * @return int|false seconds */ - public function getSecondsToNextUnpublishedEpisode(int $podcastId): int | bool + public function getSecondsToNextUnpublishedEpisode(int $podcastId): int | false { $result = $this->select('TIMESTAMPDIFF(SECOND, NOW(), `published_at`) as timestamp_diff',) ->where([ @@ -291,7 +272,7 @@ class EpisodeModel extends Model ->get() ->getResultArray(); - return count($result) !== 0 + return $result !== [] ? (int) $result[0]['timestamp_diff'] : false; } @@ -305,46 +286,18 @@ class EpisodeModel extends Model { $episode = (new self())->find(is_array($data['id']) ? $data['id'][0] : $data['id'],); - // delete cache for rss feed + // delete podcast cache cache() - ->deleteMatching("podcast#{$episode->podcast_id}_feed*"); - - // delete model requests cache + ->deleteMatching("podcast#{$episode->podcast_id}*"); cache() - ->delete("podcast#{$episode->podcast_id}_episodes"); - + ->deleteMatching("podcast-{$episode->podcast->name}*"); cache() ->delete("podcast_episode#{$episode->id}"); cache() - ->deleteMatching("podcast#{$episode->podcast_id}_episode#{$episode->id}*"); - cache() - ->delete("podcast#{$episode->podcast_id}_episode-{$episode->slug}"); - - cache() - ->deleteMatching("page_podcast#{$episode->podcast_id}_activity*"); - cache() - ->deleteMatching("page_podcast#{$episode->podcast_id}_episode#{$episode->id}_*",); + ->deleteMatching("page_podcast#{$episode->podcast_id}*"); cache() ->deleteMatching('page_credits_*'); - if ($episode->season_number) { - cache()->deleteMatching("podcast#{$episode->podcast_id}_season*"); - cache() - ->deleteMatching("page_podcast#{$episode->podcast_id}_episodes_season*",); - } else { - cache()->deleteMatching("podcast#{$episode->podcast_id}_year*"); - cache() - ->deleteMatching("page_podcast#{$episode->podcast_id}_episodes_year*",); - } - - // delete query cache - cache() - ->delete("podcast#{$episode->podcast_id}_defaultQuery"); - cache() - ->delete("podcast#{$episode->podcast_id}_years"); - cache() - ->delete("podcast#{$episode->podcast_id}_seasons"); - return $data; } diff --git a/app/Models/PersonModel.php b/app/Models/PersonModel.php index 3101acda..07b2cea5 100644 --- a/app/Models/PersonModel.php +++ b/app/Models/PersonModel.php @@ -171,7 +171,7 @@ class PersonModel extends Model ->getLocale(); $cacheName = "taxonomy_options_{$locale}"; - /** @var array>>> $personsTaxonomy */ + /** @var array $personsTaxonomy */ $personsTaxonomy = lang('PersonsTaxonomy.persons'); if (! ($options = cache($cacheName))) { @@ -285,14 +285,14 @@ class PersonModel extends Model /** * Add persons to podcast * - * @param array $persons + * @param array $personIds * @param array $roles * * @return bool|int Number of rows inserted or FALSE on failure */ - public function addPodcastPersons(int $podcastId, array $persons = [], array $roles = []): int | bool + public function addPodcastPersons(int $podcastId, array $personIds = [], array $roles = []): int | bool { - if ($persons === []) { + if ($personIds === []) { return 0; } @@ -303,11 +303,11 @@ class PersonModel extends Model ]); $data = []; - foreach ($persons as $person) { + foreach ($personIds as $personId) { if ($roles === []) { $data[] = [ 'podcast_id' => $podcastId, - 'person_id' => $person, + 'person_id' => $personId, ]; } @@ -315,7 +315,7 @@ class PersonModel extends Model $groupRole = explode(',', $role); $data[] = [ 'podcast_id' => $podcastId, - 'person_id' => $person, + 'person_id' => $personId, 'person_group' => $groupRole[0], 'person_role' => $groupRole[1], ]; @@ -337,6 +337,9 @@ class PersonModel extends Model cache()->deleteMatching("podcast#{$podcastId}_person#{$personId}*"); cache() ->delete("podcast#{$podcastId}_persons"); + (new PodcastModel())->clearCache([ + 'id' => $podcastId, + ]); return $this->db->table('podcasts_persons') ->delete([ @@ -399,6 +402,9 @@ class PersonModel extends Model cache()->deleteMatching("podcast#{$podcastId}_episode#{$episodeId}_person#{$personId}*"); cache() ->delete("podcast#{$podcastId}_episode#{$episodeId}_persons"); + (new EpisodeModel())->clearCache([ + 'id' => $episodeId, + ]); return $this->db->table('episodes_persons') ->delete([ @@ -424,7 +430,7 @@ class PersonModel extends Model // clear cache for every credits page cache() - ->deleteMatching('page_credits_*'); + ->deleteMatching('page_credits*'); return $data; } diff --git a/app/Views/podcast/_partials/note_authenticated.php b/app/Views/podcast/_partials/note_authenticated.php index c60f5942..14973c7e 100644 --- a/app/Views/podcast/_partials/note_authenticated.php +++ b/app/Views/podcast/_partials/note_authenticated.php @@ -20,9 +20,9 @@ class="text-xs text-gray-500"> + datetime="published_at->format(DateTime::ATOM) ?>" + title="published_at ?>" + >published_at]) ?> diff --git a/app/Views/podcast/_partials/reply_authenticated.php b/app/Views/podcast/_partials/reply_authenticated.php index 62eab745..72d2b635 100644 --- a/app/Views/podcast/_partials/reply_authenticated.php +++ b/app/Views/podcast/_partials/reply_authenticated.php @@ -14,9 +14,9 @@ + datetime="published_at->format(DateTime::ATOM) ?>" + title="published_at ?>" + >published_at]) ?>

message_html ?>

has_preview_card): ?>