diff --git a/app/Http/Controllers/Api/ApiV1Controller.php b/app/Http/Controllers/Api/ApiV1Controller.php index 100028459..f86e582d4 100644 --- a/app/Http/Controllers/Api/ApiV1Controller.php +++ b/app/Http/Controllers/Api/ApiV1Controller.php @@ -3370,7 +3370,7 @@ class ApiV1Controller extends Controller abort_unless($request->user()->tokenCan('write'), 403); $service = app(DirectMessageService::class); - $found = is_numeric($id) ? $service->conversationFor($id, $request->user()->profile_id) : null; + $found = is_numeric($id) ? $service->conversationForMastodonId($id, $request->user()->profile_id) : null; abort_if(! $found, 404); $service->setHidden($found[1], true); @@ -3390,7 +3390,7 @@ class ApiV1Controller extends Controller $payloads = app(DirectMessagePayloadService::class); $pid = $request->user()->profile_id; - $found = is_numeric($id) ? $service->conversationFor($id, $pid) : null; + $found = is_numeric($id) ? $service->conversationForMastodonId($id, $pid) : null; abort_if(! $found, 404); [$conversation, $participant] = $found; @@ -3425,6 +3425,14 @@ class ApiV1Controller extends Controller $res = $request->has(self::PF_API_ENTITY_KEY) ? StatusService::get($id, false) : StatusService::getMastodon($id, false); if (! $res || ! isset($res['visibility'])) { + // Direct messages are no longer statuses, but clients still take + // the id they got from /api/v1/conversations to this endpoint + $direct = app(DirectMessagePayloadService::class)->mastodonStatusById($id, $pid); + + if ($direct) { + return $this->json($direct); + } + abort(404); } @@ -3478,6 +3486,12 @@ class ApiV1Controller extends Controller ); if (! $status || ! isset($status['account'])) { + $direct = app(DirectMessagePayloadService::class)->mastodonContext($id, $pid); + + if ($direct) { + return $this->json($direct); + } + return response('', 404); } @@ -4040,8 +4054,24 @@ class ApiV1Controller extends Controller abort_unless($request->user()->tokenCan('write'), 403); AccountService::setLastActive($request->user()->id); - $status = Status::whereProfileId($request->user()->profile->id) - ->findOrFail($id); + $pid = $request->user()->profile_id; + $status = Status::whereProfileId($pid)->find($id); + + if (! $status) { + $message = DmMessage::where('profile_id', $pid)->find($id); + abort_if(! $message, 404); + + $payloads = app(DirectMessagePayloadService::class); + $res = $payloads->mastodonStatusById($message->id, $pid); + abort_if(! $res, 404); + + app(DirectMessageService::class)->deleteMessage($message); + + $res['text'] = $res['content_text']; + unset($res['content']); + + return $this->json($res); + } $resource = new Fractal\Resource\Item($status, new StatusTransformer); diff --git a/app/Services/DirectMessagePayloadService.php b/app/Services/DirectMessagePayloadService.php index 85e0a1653..aa321769a 100644 --- a/app/Services/DirectMessagePayloadService.php +++ b/app/Services/DirectMessagePayloadService.php @@ -186,10 +186,13 @@ class DirectMessagePayloadService */ public function mastodonConversation(DmConversation $conversation, DmConversationParticipant $viewer, Collection $members, ?DmMessage $last, int $viewerId): ?array { - $accounts = $members - ->filter(fn ($member) => (int) $member->profile_id !== $viewerId) + $everyone = $members ->map(fn ($member) => AccountService::getMastodon($member->profile_id, true)) ->filter(fn ($account) => $account && isset($account['id'])) + ->values(); + + $accounts = $everyone + ->filter(fn ($account) => (int) $account['id'] !== $viewerId) ->values() ->all(); @@ -197,22 +200,108 @@ class DirectMessagePayloadService return null; } + // The id is the viewer's participant row, not the conversation. It + // is an autoincrement, and the old endpoint handed out one of those + // too, so clients that store this as a 32-bit int (Pixelix does) + // keep working. It is only used for the DELETE and read calls, where + // it resolves back to the conversation for this viewer. return [ - 'id' => (string) $conversation->id, + 'id' => (string) $viewer->id, 'unread' => $viewer->unread_count > 0, 'accounts' => $accounts, - 'last_status' => $this->mastodonStatus($last, $accounts, $viewerId), + 'last_status' => $this->mastodonStatus($last, $everyone->all(), $viewerId), ]; } + /** + * The message as a status entity, for a viewer who is in its + * conversation. Mastodon clients take the `last_status` id from + * /api/v1/conversations straight to /api/v1/statuses/{id}. + * + * @return array|null + */ + public function mastodonStatusById(int|string $messageId, int $viewerId): ?array + { + $message = DmMessage::with('media')->find($messageId); + + if (! $message) { + return null; + } + + $found = app(DirectMessageService::class)->conversationFor($message->conversation_id, $viewerId); + + if (! $found || in_array((int) $message->profile_id, $this->blockedIds($viewerId), true)) { + return null; + } + + return $this->mastodonStatus($message, $this->mastodonParticipants($found[0]), $viewerId); + } + + /** + * The rest of the conversation around a message, in the shape of + * /api/v1/statuses/{id}/context: what came before as ancestors and what + * came after as descendants. + * + * @return array{ancestors: array>, descendants: array>}|null + */ + public function mastodonContext(int|string $messageId, int $viewerId, int $limit = 40): ?array + { + $message = DmMessage::find($messageId); + + if (! $message) { + return null; + } + + $found = app(DirectMessageService::class)->conversationFor($message->conversation_id, $viewerId); + + if (! $found) { + return null; + } + + $participants = $this->mastodonParticipants($found[0]); + $blocked = $this->blockedIds($viewerId) ?: [0]; + + $query = fn () => DmMessage::with('media') + ->where('conversation_id', $message->conversation_id) + ->whereNotIn('profile_id', $blocked); + + $ancestors = $query()->where('id', '<', $message->id)->orderByDesc('id')->limit($limit)->get()->reverse(); + $descendants = $query()->where('id', '>', $message->id)->orderBy('id')->limit($limit)->get(); + + $toStatus = fn (DmMessage $m) => $this->mastodonStatus($m, $participants, $viewerId); + + return [ + 'ancestors' => $ancestors->map($toStatus)->values()->all(), + 'descendants' => $descendants->map($toStatus)->values()->all(), + ]; + } + + /** + * Everyone in the conversation, as Mastodon account entities. + * + * @return array> + */ + public function mastodonParticipants(DmConversation $conversation): array + { + return DmConversationParticipant::where('conversation_id', $conversation->id) + ->orderBy('id') + ->pluck('profile_id') + ->map(fn ($id) => AccountService::getMastodon($id, true)) + ->filter(fn ($account) => $account && isset($account['id'])) + ->values() + ->all(); + } + /** * Messages are not statuses any more, but Mastodon clients expect one as - * `last_status`, so this builds the entity from the message. + * `last_status`, so this builds the entity from the message. Everyone in + * the conversation other than the author is a mention, the viewer + * included: that is how a client tells the message was addressed to them. * - * @param array> $accounts + * @param array> $participants Everyone in the conversation, as account entities * @return array */ - public function mastodonStatus(DmMessage $message, array $accounts, int $viewerId): array + public function mastodonStatus(DmMessage $message, array $participants, int $viewerId): array { $media = collect($this->media($message))->map(function (array $item) { $mime = $item['mime'] ?? null; @@ -259,7 +348,7 @@ class DirectMessagePayloadService 'visibility' => 'direct', 'application' => null, 'language' => null, - 'mentions' => collect($accounts) + 'mentions' => collect($participants) ->filter(fn ($account) => (string) $account['id'] !== (string) $message->profile_id) ->map(fn ($account) => [ 'id' => (string) $account['id'], diff --git a/app/Services/DirectMessageService.php b/app/Services/DirectMessageService.php index 24e483e2c..ddccd2dc6 100644 --- a/app/Services/DirectMessageService.php +++ b/app/Services/DirectMessageService.php @@ -232,6 +232,28 @@ class DirectMessageService return $conversation ? [$conversation, $participant] : null; } + /** + * The conversation behind an id from /api/v1/conversations, which is the + * viewer's participant row. A conversation id is accepted too. + * + * @return array{0: DmConversation, 1: DmConversationParticipant}|null + */ + public function conversationForMastodonId(int|string $id, int $profileId): ?array + { + $participant = DmConversationParticipant::where('id', $id) + ->where('profile_id', $profileId) + ->where('state', '!=', DmConversationParticipant::STATE_LEFT) + ->first(); + + if ($participant) { + $conversation = DmConversation::find($participant->conversation_id); + + return $conversation ? [$conversation, $participant] : null; + } + + return $this->conversationFor($id, $profileId); + } + /** * Profile ids of everyone in the conversation. * diff --git a/tests/Feature/DirectMessage/DirectMessageLegacyApiTest.php b/tests/Feature/DirectMessage/DirectMessageLegacyApiTest.php index 3accb761a..102788349 100644 --- a/tests/Feature/DirectMessage/DirectMessageLegacyApiTest.php +++ b/tests/Feature/DirectMessage/DirectMessageLegacyApiTest.php @@ -15,7 +15,7 @@ use Illuminate\Support\Facades\Redis; use Illuminate\Support\Facades\Storage; use Laravel\Passport\Passport; -require_once __DIR__.'/helpers.php'; +require_once __DIR__ . '/helpers.php'; uses(LazilyRefreshDatabase::class); @@ -35,13 +35,19 @@ beforeEach(function () { Queue::fake(); Http::fake(); + // Ids minted in the same millisecond only sort by creation order when the + // worker bits are fixed. Left unset they are random for every id. + config(['snowflake.datacenter_id' => 1, 'snowflake.worker_id' => 1]); + $this->withoutMiddleware(ThrottleRequests::class); + // Ids minted in the same millisecond only sort by creation order when the + // worker bits are fixed. Left unset they are random for every id. + config(['snowflake.datacenter_id' => 1, 'snowflake.worker_id' => 1]); + config([ 'instance.enable_cc' => false, 'federation.activitypub.enabled' => true, - 'snowflake.datacenter_id' => 1, - 'snowflake.worker_id' => 1, ]); }); @@ -68,7 +74,7 @@ describe('thread endpoints', function () { Passport::actingAs($bob, ['read', 'write']); - $thread = $this->getJson('/api/v1.1/direct/thread?pid='.$alice->profile_id); + $thread = $this->getJson('/api/v1.1/direct/thread?pid=' . $alice->profile_id); $thread->assertOk() ->assertJsonPath('id', (string) $alice->profile_id) @@ -86,7 +92,7 @@ describe('thread endpoints', function () { Passport::actingAs($alice, ['read', 'write']); - $this->getJson('/api/v1.1/direct/thread?pid='.$bob->profile_id) + $this->getJson('/api/v1.1/direct/thread?pid=' . $bob->profile_id) ->assertOk() ->assertJsonCount(0, 'messages') ->assertJsonPath('conversation_id', null); @@ -115,12 +121,12 @@ describe('thread endpoints', function () { Passport::actingAs($bob, ['read', 'write']); - $this->getJson('/api/v1.1/direct/thread?pid='.$alice->profile_id) + $this->getJson('/api/v1.1/direct/thread?pid=' . $alice->profile_id) ->assertJsonPath('messages.0.text', 'my cat') ->assertJsonPath('messages.0.type', 'photo') ->assertJsonCount(1, 'messages.0.carousel'); - expect($this->getJson('/api/v1.1/direct/thread?pid='.$alice->profile_id)->json('messages.0.media'))->not->toBeNull(); + expect($this->getJson('/api/v1.1/direct/thread?pid=' . $alice->profile_id)->json('messages.0.media'))->not->toBeNull(); }); it('marks a thread read and reports it as seen to the sender', function () { @@ -138,7 +144,7 @@ describe('thread endpoints', function () { expect(DmConversationParticipant::where('profile_id', $bob->profile_id)->value('unread_count'))->toBe(0); Passport::actingAs($alice, ['read', 'write']); - $this->getJson('/api/v1.1/direct/thread?pid='.$bob->profile_id)->assertJsonPath('messages.0.seen', true); + $this->getJson('/api/v1.1/direct/thread?pid=' . $bob->profile_id)->assertJsonPath('messages.0.seen', true); }); it('deletes by the id the thread handed out', function () { @@ -195,22 +201,50 @@ describe('GET /api/v1/conversations', function () { Passport::actingAs($bob, ['read', 'write']); - $this->getJson('/api/v1/conversations') + $response = $this->getJson('/api/v1/conversations') ->assertOk() ->assertJsonCount(1) - ->assertJsonPath('0.id', (string) $conversation->id) ->assertJsonPath('0.unread', true) ->assertJsonPath('0.accounts.0.id', (string) $alice->profile_id) ->assertJsonPath('0.last_status.id', (string) $message->id) ->assertJsonPath('0.last_status.visibility', 'direct') ->assertJsonPath('0.last_status.content', '

hello

') - ->assertJsonPath('0.last_status.account.id', (string) $alice->profile_id); + ->assertJsonPath('0.last_status.account.id', (string) $alice->profile_id) + ->assertJsonPath('0.last_status.mentions.0.id', (string) $bob->profile_id); + + // Pixelix stores this id as a 32-bit int, as the old endpoint allowed + $id = $response->json('0.id'); + expect((int) $id)->toBeLessThan(2 ** 31) + ->and((int) $id)->toBeGreaterThan(0) + ->and((int) $id)->toBe(DmConversationParticipant::where('profile_id', $bob->profile_id)->value('id')); + + $this->postJson("/api/v1/conversations/{$id}/read")->assertOk()->assertJsonPath('unread', false); - $this->postJson("/api/v1/conversations/{$conversation->id}/read")->assertOk()->assertJsonPath('unread', false); - $this->deleteJson("/api/v1/conversations/{$conversation->id}")->assertOk(); + // The real conversation id still works for clients written against it + $this->postJson("/api/v1/conversations/{$conversation->id}/read")->assertOk(); + + $this->deleteJson("/api/v1/conversations/{$id}")->assertOk(); $this->getJson('/api/v1/conversations')->assertJsonCount(0); }); + it('does not let one person use another persons conversation id', function () { + $alice = dmLocalUser(); + $bob = dmLocalUser(); + $eve = dmLocalUser(); + + $service = app(DirectMessageService::class); + $conversation = $service->findOrCreateDm(dmProfile($alice), dmProfile($bob)); + $service->sendMessage($conversation, dmProfile($alice), ['body' => 'hello']); + + Passport::actingAs($bob, ['read', 'write']); + $id = $this->getJson('/api/v1/conversations')->json('0.id'); + + Passport::actingAs($eve, ['read', 'write']); + $this->postJson("/api/v1/conversations/{$id}/read")->assertNotFound(); + $this->deleteJson("/api/v1/conversations/{$id}")->assertNotFound(); + $this->deleteJson("/api/v1/conversations/{$conversation->id}")->assertNotFound(); + }); + it('only includes groups when asked', function () { $alice = dmLocalUser(); $bob = dmLocalUser(); @@ -243,6 +277,78 @@ describe('GET /api/v1/conversations', function () { }); }); +describe('mastodon status endpoints', function () { + it('serves a message as a status to people in the conversation and nobody else', function () { + $alice = dmLocalUser(); + $bob = dmLocalUser(); + $eve = dmLocalUser(); + + $service = app(DirectMessageService::class); + $conversation = $service->findOrCreateDm(dmProfile($alice), dmProfile($bob)); + $message = $service->sendMessage($conversation, dmProfile($alice), ['body' => 'hello']); + + Passport::actingAs($bob, ['read', 'write']); + + $this->getJson("/api/v1/statuses/{$message->id}") + ->assertOk() + ->assertJsonPath('id', (string) $message->id) + ->assertJsonPath('visibility', 'direct') + ->assertJsonPath('content', '

hello

') + ->assertJsonPath('account.id', (string) $alice->profile_id) + ->assertJsonPath('mentions.0.id', (string) $bob->profile_id); + + Passport::actingAs($eve, ['read', 'write']); + + $this->getJson("/api/v1/statuses/{$message->id}")->assertNotFound(); + }); + + it('returns the rest of the conversation as the context of a message', function () { + $alice = dmLocalUser(); + $bob = dmLocalUser(); + + $service = app(DirectMessageService::class); + $conversation = $service->findOrCreateDm(dmProfile($alice), dmProfile($bob)); + $one = $service->sendMessage($conversation, dmProfile($alice), ['body' => 'one']); + $two = $service->sendMessage($conversation, dmProfile($bob), ['body' => 'two']); + $three = $service->sendMessage($conversation, dmProfile($alice), ['body' => 'three']); + + Passport::actingAs($bob, ['read', 'write']); + + $this->getJson("/api/v1/statuses/{$two->id}/context") + ->assertOk() + ->assertJsonCount(1, 'ancestors') + ->assertJsonPath('ancestors.0.id', (string) $one->id) + ->assertJsonCount(1, 'descendants') + ->assertJsonPath('descendants.0.id', (string) $three->id) + ->assertJsonPath('descendants.0.visibility', 'direct'); + + // The usual client flow: open the conversation from its last status + $this->getJson("/api/v1/statuses/{$three->id}/context") + ->assertOk() + ->assertJsonCount(2, 'ancestors') + ->assertJsonCount(0, 'descendants'); + }); + + it('lets the author delete a message through the status endpoint', function () { + $alice = dmLocalUser(); + $bob = dmLocalUser(); + + $service = app(DirectMessageService::class); + $conversation = $service->findOrCreateDm(dmProfile($alice), dmProfile($bob)); + $message = $service->sendMessage($conversation, dmProfile($alice), ['body' => 'oops']); + + Passport::actingAs($bob, ['read', 'write']); + $this->deleteJson("/api/v1/statuses/{$message->id}")->assertNotFound(); + + Passport::actingAs($alice, ['read', 'write']); + $this->deleteJson("/api/v1/statuses/{$message->id}") + ->assertOk() + ->assertJsonPath('text', 'oops'); + + expect(DmMessage::count())->toBe(0); + }); +}); + describe('media housekeeping', function () { it('does not let direct message media be attached to a post', function () { $alice = dmLocalUser();