Skip to content

Commit

Permalink
tec: Improve the performance of news refreshing
Browse files Browse the repository at this point in the history
  • Loading branch information
@marienfressinaud
marienfressinaud committed Oct 3, 2024
1 parent 7d550fa commit f0d563a
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 66 deletions.
28 changes: 28 additions & 0 deletions src/migrations/Migration202410020002AddCreatedAtIndexOnLinksToCollections.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

namespace App\migrations;

class Migration202410020002AddCreatedAtIndexOnLinksToCollections
{
public function migrate(): bool
{
$database = \Minz\Database::get();

$database->exec(<<<'SQL'
CREATE INDEX idx_links_to_collections_created_at ON links_to_collections(created_at);
SQL);

return true;
}

public function rollback(): bool
{
$database = \Minz\Database::get();

$database->exec(<<<'SQL'
DROP INDEX idx_links_to_collections_created_at;
SQL);

return true;
}
}
70 changes: 34 additions & 36 deletions src/models/dao/links/NewsQueries.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,23 @@ trait NewsQueries
*
* @return self[]
*/
public static function listFromFollowedCollections(string $user_id): array
public static function listFromFollowedCollections(string $user_id, int $max): array
{
$values = [
':user_id' => $user_id,
':until_hard_limit' => \Minz\Time::ago(1, 'year')->format(Database\Column::DATETIME_FORMAT),
':until_strict' => \Minz\Time::ago(1, 'day')->format(Database\Column::DATETIME_FORMAT),
':until_normal' => \Minz\Time::ago(1, 'week')->format(Database\Column::DATETIME_FORMAT),
':limit' => $max,
];

$sql = <<<SQL
SELECT l.*, lc.created_at AS published_at, 'collection' AS source_news_type, c.id AS source_news_resource_id
SELECT
l.url_hash,
l.*,
lc.created_at AS published_at,
'collection' AS source_news_type,
c.id AS source_news_resource_id
FROM collections c, links_to_collections lc, followed_collections fc, links l
WHERE fc.user_id = :user_id
Expand All @@ -45,56 +52,47 @@ public static function listFromFollowedCollections(string $user_id): array
)
)
AND NOT EXISTS (
SELECT 1
FROM links l_exclude, collections c_exclude, links_to_collections lc_exclude
WHERE c_exclude.user_id = :user_id
AND l_exclude.user_id = :user_id
AND l_exclude.url_hash = l.url_hash
AND (
c_exclude.type = 'news'
OR c_exclude.type = 'bookmarks'
OR c_exclude.type = 'read'
OR c_exclude.type = 'never'
)
AND lc_exclude.link_id = l.id
AND lc_exclude.collection_id = c.id
)
AND l.user_id != :user_id
AND lc.created_at >= :until_hard_limit
AND (
(fc.time_filter = 'strict' AND lc.created_at >= :until_strict) OR
(fc.time_filter = 'normal' AND lc.created_at >= :until_normal) OR
(fc.time_filter = 'all' AND lc.created_at >= fc.created_at - INTERVAL '1 week')
)
ORDER BY published_at DESC, l.id
SQL;
$database = Database::get();
$statement = $database->prepare($sql);
$statement->execute($values);

return self::fromDatabaseRows($statement->fetchAll());
}

/**
* Return hashes of links that are in news, bookmarks, never or read lists.
*
* @return array<string, bool>
*/
public static function listHashesExcludedFromNews(string $user_id): array
{
$values = [
':user_id' => $user_id,
];

$sql = <<<SQL
SELECT l.url_hash, true
FROM links l, collections c, links_to_collections lc
WHERE c.user_id = :user_id
AND (
c.type = 'news'
OR c.type = 'bookmarks'
OR c.type = 'read'
OR c.type = 'never'
)
AND lc.link_id = l.id
AND lc.collection_id = c.id
LIMIT :limit
SQL;

$database = Database::get();
$statement = $database->prepare($sql);
$statement->execute($values);

return $statement->fetchAll(\PDO::FETCH_KEY_PAIR);
// Get the results indexed by the url_hash (i.e. the first column)
$results = $statement->fetchAll(\PDO::FETCH_UNIQUE);

return self::fromDatabaseRows($results);
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/schema.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ CREATE TABLE links_to_collections (

CREATE UNIQUE INDEX idx_links_to_collections ON links_to_collections(link_id, collection_id);
CREATE INDEX idx_links_to_collections_collection_id ON links_to_collections(collection_id);
CREATE INDEX idx_links_to_collections_created_at ON links_to_collections(created_at);

CREATE TABLE followed_collections (
id BIGSERIAL PRIMARY KEY,
Expand Down
21 changes: 1 addition & 20 deletions src/services/NewsPicker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,6 @@ public function __construct(models\User $user)
*/
public function pick(int $max = 25): array
{
$excluded_hashes = models\Link::listHashesExcludedFromNews($this->user->id);
$links_from_followed = models\Link::listFromFollowedCollections($this->user->id);

$links = [];

foreach ($links_from_followed as $link) {
$hash = $link->url_hash;

if (isset($excluded_hashes[$hash])) {
continue;
}

$links[$hash] = $link;

if (count($links) >= $max) {
break;
}
}

return array_values($links);
return models\Link::listFromFollowedCollections($this->user->id, $max);
}
}
31 changes: 21 additions & 10 deletions tests/services/NewsPickerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,14 @@ public function testPickSelectsFromFollowed(): void
/** @var \DateTimeImmutable */
$now = $this->fake('dateTime');
$this->freeze($now);
/** @var int */
$days_ago = $this->fake('numberBetween', 0, 7);
$published_at = \Minz\Time::ago($days_ago, 'days');
$published_at1 = \Minz\Time::ago(3, 'days');
$published_at2 = \Minz\Time::ago(1, 'days');
$news_picker = new NewsPicker($this->user);
$link = LinkFactory::create([
$link1 = LinkFactory::create([
'user_id' => $this->other_user->id,
'is_hidden' => false,
]);
$link2 = LinkFactory::create([
'user_id' => $this->other_user->id,
'is_hidden' => false,
]);
Expand All @@ -45,9 +48,14 @@ public function testPickSelectsFromFollowed(): void
'is_public' => true,
]);
LinkToCollectionFactory::create([
'created_at' => $published_at,
'created_at' => $published_at1,
'collection_id' => $collection->id,
'link_id' => $link->id,
'link_id' => $link1->id,
]);
LinkToCollectionFactory::create([
'created_at' => $published_at2,
'collection_id' => $collection->id,
'link_id' => $link2->id,
]);
FollowedCollectionFactory::create([
'user_id' => $this->user->id,
Expand All @@ -56,10 +64,13 @@ public function testPickSelectsFromFollowed(): void

$links = $news_picker->pick();

$this->assertSame(1, count($links));
$this->assertSame($link->id, $links[0]->id);
$this->assertSame(2, count($links));
$this->assertSame($link2->id, $links[0]->id);
$this->assertSame('collection', $links[0]->source_news_type);
$this->assertSame($collection->id, $links[0]->source_news_resource_id);
$this->assertSame($link1->id, $links[1]->id);
$this->assertSame('collection', $links[1]->source_news_type);
$this->assertSame($collection->id, $links[1]->source_news_resource_id);
}

public function testPickSelectsHiddenLinkIfCollectionIsShared(): void
Expand Down Expand Up @@ -148,7 +159,7 @@ public function testPickRespectsFromFollowedIfOldLinksButWithTimeFilterAll(): vo
$now = $this->fake('dateTime');
$this->freeze($now);
/** @var int */
$days_ago = $this->fake('numberBetween', 8, 9999);
$days_ago = $this->fake('numberBetween', 8, 180);
$published_at = \Minz\Time::ago($days_ago, 'days');
// time_filter 'all' will search links until 7 days before the date
// when the user started to follow the collection
Expand Down Expand Up @@ -190,7 +201,7 @@ public function testPickDoesNotPickFromFollowedIfTooOld(): void
$now = $this->fake('dateTime');
$this->freeze($now);
/** @var int */
$days_ago = $this->fake('numberBetween', 8, 9999);
$days_ago = $this->fake('numberBetween', 8, 180);
$published_at = \Minz\Time::ago($days_ago, 'days');
$news_picker = new NewsPicker($this->user);
$link = LinkFactory::create([
Expand Down

0 comments on commit f0d563a

Please sign in to comment.