Direct, account and hashtag timelines as two queries

appinfo/info.xml

@@ -31,7 +31,7 @@ What it does:
 - 👋 A short introduction on the first visit: your address, who to follow, and a way to bring the follows you already have
 
 You can pin your own posts to the top of your profile, bookmark any post, and see which hashtags the instance is using most. This is a partial implementation of ActivityPub and of the Mastodon client API. Blocking, muting and reporting (with a moderation panel in the administration settings) are supported, as are locked accounts with approvable follow requests. Polls are fully supported: create your own, and view and vote on federated ones. Profiles carry an avatar, a banner image and up to four profile metadata fields. Posts that link somewhere get a link preview card. Image (JPEG, PNG, GIF, WebP, AVIF, and HEIC from an iPhone where the server can read it), video (MP4, WebM, QuickTime) and audio attachments are supported, up to ten per post, each with alt text and a focal point. It does not offer lists.]]></description>
-	<version>0.19.36</version>
+	<version>0.19.38</version>
 	<licence>agpl</licence>
 	<author mail="benedikt.schaechner@web.de" homepage="https://benedikt.xn--schchner-2za.de">Benedikt Schächner</author>
 	<namespace>Social</namespace>

composer.json

@@ -2,7 +2,7 @@
 	"name": "nextcloud/social",
 	"description": "Social app",
 	"license": "AGPL-3.0-or-later",
-	"version": "0.19.36",
+	"version": "0.19.38",
 	"minimum-stability": "stable",
 	"authors": [
 		{

docs/Architecture.md

@@ -7,7 +7,7 @@ Nextcloud Social is a federated social networking app built on the W3C ActivityP
 **App ID:** `social`  
 **Namespace:** `OCA\Social`  
 **License:** AGPL-3.0-or-later  
-**App version:** 0.19.36  
+**App version:** 0.19.38  
 **Supported Nextcloud versions:** 35 – 36  
 **Supported PHP versions:** 8.3 – 8.5  
 

docs/Performance.md

@@ -95,31 +95,26 @@ any refused statement, so that caught violation took the commit with it and the
 post was lost. Both now use `insertIgnoreConflict()`: the database skips the
 duplicate row, nothing fails, and the raise is left to mean what it says.
 
-Two places still have no transaction and want one:
-
-- `StreamActionService::saveAction()` — update, and insert if no row was
-  affected. Two concurrent likes both see nothing affected and both insert. On
-  MySQL/MariaDB `rowCount()` returns *changed* rows, so setting a flag to the
-  value it already holds takes the same path. `ActorRelationRequest` and
-  `StreamCardsRequest` already use the insert-then-catch-unique-then-update
-  shape that avoids this.
-- `ModerationRequest::save()` — delete then insert, with the insert failure only
-  logged: the old decision is gone and the new one was never applied.
+The two places this section used to name are settled: `StreamActionsRequest::save()`
+inserts first and updates on the unique violation (`StreamActionsFlagsTest` pins
+the race), and `ModerationRequest::save()` does the same rather than delete-then-
+insert, so a decision is replaced or kept, never lost.
 
 ### Wide `SELECT DISTINCT`
 
 `getStreamNidsSelectSql()` exists precisely to avoid deduplicating over every
-column — it selects nids, then hydrates. Seven call sites use it: the home and
-public branches of `getTimeline()`, the marked timelines (favourites and
-bookmarks), the list timeline in `ListsRequest`, and the deprecated direct
-timeline. **Eighteen** call sites in `StreamRequest` still go through
-`getStreamSelectSql()`, which pairs `selectDistinct('s.id')` with the full
-stream column set plus, on some paths, a second `os_*` stream set and two
-cached-actor and cached-document sets. The database sorts or hashes all of it —
-including `content`, `source`, `details`, `cache`, `tags` and `to_array` — to
-deduplicate. Direct messages, account timelines, hashtag timelines,
-notifications, search, `getNoteSince` and `getDescendants` are the ones worth
-moving.
+column — it selects nids, then hydrates. **Every branch of `getTimeline()` now
+goes through it**: home (both halves), public, direct, account, hashtag,
+favourites and bookmarks, notifications, and the list timeline in `ListsRequest`
+— each a `*TimelineNids()` method that decides the page over one indexed
+column, then `streamsByNids()` for exactly those rows
+(`tests/Db/TwoQueryTimelinesTest.php` pins the shape for the three moved last).
+What still pairs `selectDistinct('s.id')` with the full stream column set is
+the single-row lookups (`getStreamById()` and friends, which return one row and
+have nothing to deduplicate), `searchContent()`, `getDescendants()` /
+`getRepliesTo()`, `getAnnouncesAndRepliesTo()`, and the `*_dep()` methods behind
+the uncalled Custom Local API routes. Of those, search and the thread walk are
+the ones worth moving next; the `_dep` ones go with their routes.
 
 ### Lookups that cannot use an index
 
@@ -325,22 +320,16 @@ What is left, in order of how much it would cost to try:
 
 ## What to do next
 
-1. **Move the remaining read paths onto `getStreamNidsSelectSql()`.** The one
-   item left that changes how the app scales. Every timeline that is not home or
-   public still makes the database sort or hash `content`, `source`, `details`,
-   `cache` and `to_array` to deduplicate a page of twenty rows. It is also the
-   most mechanical: the pattern exists, it is proven on seven paths, and each
-   move is independently testable. Do notifications first — it carries two full
-   stream column sets, two cached-actor sets and two cached-document sets, and
-   every client polls it on a timer.
-2. **`StreamActionService::saveAction()`** wants the
-   insert-then-catch-unique-then-update shape its two neighbours already use.
-   Correctness rather than speed: two concurrent likes can both insert today.
-3. **`ModerationRequest::save()`** wants a transaction around its delete and
-   insert, for the same reason.
-4. **The schema items**, next time a migration touches those tables. The
+1. **Move `searchContent()` and the thread walk (`getDescendants()`,
+   `getRepliesTo()`) onto `getStreamNidsSelectSql()`.** Every timeline is on it
+   now; these two are what is left of the wide `SELECT DISTINCT`, and the thread
+   walk is also the one read that is still a query per level.
+2. **Retire the `*_dep()` methods with the Custom Local API routes** that call
+   them (Technical-Debt.md, item 2): that removes the last wide timeline reads
+   without rewriting them.
+3. **The schema items**, next time a migration touches those tables. The
    `social_follow` index order is the one worth doing deliberately: it is why a
    duplicate accepted/pending pair can exist at all.
-5. **An index on `social_actor.preferred_username`**, or a `*_prim` column for
+4. **An index on `social_actor.preferred_username`**, or a `*_prim` column for
    it, if the public actor endpoint and webfinger ever show up in a profile.
    Both still compare `LOWER(column)` against `LOWER(?)`.

lib/Db/StreamRequest.php

@@ -1017,21 +1017,38 @@ private function homeTimelineFilters(
 	 * @return Stream[]
 	 */
 	private function getTimelineDirect(ProbeOptions $options): array {
-		$qb = $this->getStreamSelectSql($options->getFormat());
-
-		$qb->filterType(SocialAppNotification::TYPE);
-		$qb->paginate($options);
-		$this->filterMedia($qb, $options);
+		// two queries, as every other timeline: which posts, decided over one
+		// indexed column, then what they say
+		$nids = $this->directTimelineNids($options);
+		if ($nids === []) {
+			return [];
+		}
 
-		$qb->linkToCacheActors('ca', 's.attributed_to_prim');
+		return $this->streamsByNids($nids, $options);
+	}
 
-		$viewer = $qb->getViewer();
-		$qb->selectDestFollowing('sd', '');
-		$qb->limitToDest($viewer->getId(), 'dm', '', 'sd');
+	/**
+	 * The page of direct messages addressed to the viewer.
+	 *
+	 * The recipient join fixes the viewer and the type `dm`, which the unique
+	 * index on the recipient rows makes at most one row per post, so the page
+	 * needs no `DISTINCT`.
+	 *
+	 * @return int[]
+	 */
+	protected function directTimelineNids(ProbeOptions $options): array {
+		$page = $this->getStreamNidsSelectSql(false);
+		$page->filterType(SocialAppNotification::TYPE);
+		$page->paginate($options);
+		$this->filterMedia($page, $options);
 
-		$qb->filterHiddenActors();
+		// the author is joined for the filters below, not for its columns
+		$page->linkToCacheActors('ca', 's.attributed_to_prim', true, false);
+		$page->selectDestFollowing('sd', '');
+		$page->limitToDest($page->getViewer()->getId(), 'dm', '', 'sd');
+		$page->filterHiddenActors();
 
-		return $this->getStreamsFromRequest($qb);
+		return $this->getNidsFromRequest($page);
 	}
 
 	/**
@@ -1044,30 +1061,50 @@ private function getTimelineDirect(ProbeOptions $options): array {
 	 * @return Stream[]
 	 */
 	private function getTimelineAccount(ProbeOptions $options): array {
-		$qb = $this->getStreamSelectSql($options->getFormat());
-
-		$qb->limitToStatusTypes();
-		$qb->paginate($options);
-		$this->filterMedia($qb, $options);
+		if ($options->getAccountId() === '') {
+			return [];
+		}
 
-		$actorId = $options->getAccountId();
-		if ($actorId === '') {
+		$nids = $this->accountTimelineNids($options);
+		if ($nids === []) {
 			return [];
 		}
 
-		$qb->limitToAttributedTo($actorId, true);
+		return $this->streamsByNids($nids, $options);
+	}
 
-		$qb->selectDestFollowing('sd', '');
-		$qb->innerJoinStreamDest('recipient', 'id_prim', 'sd', 's');
-		$accountIsViewer = ($qb->hasViewer() && $qb->getViewer()->getId() === $actorId);
-		$qb->limitToDest($accountIsViewer ? '' : ACore::CONTEXT_PUBLIC, 'recipient', '', 'sd');
+	/**
+	 * The page of one account's posts the viewer may read: its public ones,
+	 * or -- for the account reading its own profile -- everything it wrote.
+	 *
+	 * The recipient join is one row per post when it names the public
+	 * collection; for the account itself it names no recipient at all and a
+	 * post addressed to several accounts would come back once per row, so
+	 * that page is `DISTINCT` and the other is not.
+	 *
+	 * @return int[]
+	 */
+	protected function accountTimelineNids(ProbeOptions $options): array {
+		$actorId = $options->getAccountId();
+		$page = $this->getStreamNidsSelectSql(false);
+		$accountIsViewer = ($page->hasViewer() && $page->getViewer()->getId() === $actorId);
+		if ($accountIsViewer) {
+			$page = $this->getStreamNidsSelectSql(true);
+		}
 
-		$qb->linkToCacheActors('ca', 's.attributed_to_prim');
-		$qb->leftJoinStreamAction();
+		$page->limitToStatusTypes();
+		$page->paginate($options);
+		$this->filterMedia($page, $options);
+		$page->limitToAttributedTo($actorId, true);
 
-		$qb->filterHiddenActors(SocialCoreQueryBuilder::HIDDEN_DIRECT);
+		$page->selectDestFollowing('sd', '');
+		$page->innerJoinStreamDest('recipient', 'id_prim', 'sd', 's');
+		$page->limitToDest($accountIsViewer ? '' : ACore::CONTEXT_PUBLIC, 'recipient', '', 'sd');
 
-		return $this->getStreamsFromRequest($qb);
+		$page->linkToCacheActors('ca', 's.attributed_to_prim', true, false);
+		$page->filterHiddenActors(SocialCoreQueryBuilder::HIDDEN_DIRECT);
+
+		return $this->getNidsFromRequest($page);
 	}
 
 	/**
@@ -1146,24 +1183,39 @@ private function getTimelineBookmarks(ProbeOptions $options): array {
 	 * @return Stream[]
 	 */
 	private function getTimelineHashtag(ProbeOptions $options): array {
-		$qb = $this->getStreamSelectSql($options->getFormat());
-		$qb->limitToStatusTypes();
-		$qb->paginate($options);
-		$this->filterMedia($qb, $options);
+		$nids = $this->hashtagTimelineNids($options);
+		if ($nids === []) {
+			return [];
+		}
 
-		$expr = $qb->expr();
-		$qb->linkToCacheActors('ca', 's.attributed_to_prim');
-		$qb->linkToStreamTags('st', 's.id_prim');
-		$qb->andWhere($qb->exprLimitToDBField('hashtag', $options->getArgument(), true, false, 'st'));
+		return $this->streamsByNids($nids, $options);
+	}
 
-		$qb->limitToViewer('sd', 'f', true);
-		$qb->andWhere($expr->eq('s.attributed_to_prim', 'ca.id_prim'));
-		// a hashtag timeline is part of the public square a silenced account loses
-		$this->filterSilencedActors($qb);
+	/**
+	 * The page of posts carrying a hashtag that the viewer may read.
+	 *
+	 * The tag join and the viewer's recipient join can each match a post more
+	 * than once, so the page is `DISTINCT` -- over one integer column, which
+	 * is what this is for: it used to be over the whole post.
+	 *
+	 * @return int[]
+	 */
+	protected function hashtagTimelineNids(ProbeOptions $options): array {
+		$page = $this->getStreamNidsSelectSql(true);
+		$page->limitToStatusTypes();
+		$page->paginate($options);
+		$this->filterMedia($page, $options);
 
-		$qb->leftJoinStreamAction('sa');
+		$page->linkToCacheActors('ca', 's.attributed_to_prim', true, false);
+		$page->linkToStreamTags('st', 's.id_prim');
+		$page->andWhere($page->exprLimitToDBField('hashtag', $options->getArgument(), true, false, 'st'));
 
-		return $this->getStreamsFromRequest($qb);
+		$page->limitToViewer('sd', 'f', true);
+		$page->andWhere($page->expr()->eq('s.attributed_to_prim', 'ca.id_prim'));
+		// a hashtag timeline is part of the public square a silenced account loses
+		$this->filterSilencedActors($page);
+
+		return $this->getNidsFromRequest($page);
 	}
 
 	/**

package.json

@@ -1,7 +1,7 @@
 {
 	"name": "social",
 	"description": "Nextcloud becomes part of the federated social networks!",
-	"version": "0.19.36",
+	"version": "0.19.38",
 	"authors": [
 		{
 			"name": "Benedikt Schächner",

tests/Db/TwoQueryTimelinesTest.php

@@ -0,0 +1,90 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Social\Tests\Db;
+
+use OCA\Social\Db\StreamRequest;
+use OCA\Social\Model\ActivityPub\Actor\Person;
+use OCA\Social\Model\ActivityPub\Object\Note;
+use OCA\Social\Model\ActivityPub\Stream;
+use OCA\Social\Model\Client\Options\ProbeOptions;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * The direct, account and hashtag timelines are two queries, like every
+ * other: the page is decided over one indexed column, then exactly those
+ * rows are read. What is pinned here is the shape -- which query decides and
+ * that the wide read is handed the page and nothing else, or is not made at
+ * all. The SQL of each page needs a database and is exercised by the
+ * integration suite (TimelineSeedTest, StreamFilterTest, MediaTypeTimelineTest).
+ */
+class TwoQueryTimelinesTest extends TestCase {
+	private const VIEWER = 'https://cloud.example/users/alice';
+
+	/** @var int[]|null what the wide read was handed, null if it was never made */
+	private ?array $read = null;
+
+	/** @return StreamRequest&\PHPUnit\Framework\MockObject\MockObject */
+	private function request(string $pageMethod, array $page) {
+		$request = $this->getMockBuilder(StreamRequest::class)
+			->disableOriginalConstructor()
+			->onlyMethods(['directTimelineNids', 'accountTimelineNids', 'hashtagTimelineNids', 'streamsByNids'])
+			->getMock();
+		$request->method($pageMethod)->willReturn($page);
+		$request->method('streamsByNids')->willReturnCallback(function (array $nids): array {
+			$this->read = $nids;
+
+			return array_map(static function (int $nid): Stream {
+				$note = new Note();
+				$note->setNid($nid);
+
+				return $note;
+			}, $nids);
+		});
+		$person = new Person();
+		$person->setId(self::VIEWER);
+		$request->setViewer($person);
+
+		return $request;
+	}
+
+	/** @return iterable<string, array{string, string, ProbeOptions}> */
+	public static function timelines(): iterable {
+		yield 'direct' => ['directTimelineNids', ProbeOptions::DIRECT, new ProbeOptions()];
+		yield 'account' => ['accountTimelineNids', ProbeOptions::ACCOUNT, (new ProbeOptions())->setAccountId('https://remote.example/users/bob')];
+		yield 'hashtag' => ['hashtagTimelineNids', ProbeOptions::HASHTAG, (new ProbeOptions())->setArgument('nextcloud')];
+	}
+
+	#[DataProvider('timelines')]
+	public function testThePageDecidesAndTheWideReadGetsExactlyIt(string $pageMethod, string $probe, ProbeOptions $options): void {
+		$request = $this->request($pageMethod, [50, 40, 30]);
+
+		$timeline = $request->getTimeline($options->setProbe($probe)->setLimit(20));
+
+		$this->assertSame([50, 40, 30], array_map(static fn (Stream $s): int => $s->getNid(), $timeline));
+		$this->assertSame([50, 40, 30], $this->read, 'only the page is read back');
+	}
+
+	#[DataProvider('timelines')]
+	public function testAnEmptyPageMakesNoWideReadAtAll(string $pageMethod, string $probe, ProbeOptions $options): void {
+		$request = $this->request($pageMethod, []);
+
+		$this->assertSame([], $request->getTimeline($options->setProbe($probe)->setLimit(20)));
+		$this->assertNull($this->read, 'nothing to read, nothing asked');
+	}
+
+	public function testAnAccountTimelineWithNoAccountAsksNothing(): void {
+		$request = $this->request('accountTimelineNids', [1, 2]);
+		$request->expects($this->never())->method('accountTimelineNids');
+
+		$this->assertSame([], $request->getTimeline((new ProbeOptions())->setProbe(ProbeOptions::ACCOUNT)->setLimit(20)));
+		$this->assertNull($this->read);
+	}
+}