Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tec: Improve the performance of news refreshing #702

Merged
merged 1 commit into from
Oct 3, 2024
Merged

tec: Improve the performance of news refreshing #702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/migrations/Migration202410020002AddCreatedAtIndexOnLinksToCollections.php
View file
Open in desktop
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
Open in desktop
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_exclude.id
AND lc_exclude.collection_id = c_exclude.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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading