From 735ecc1cca8c96acae7d7ecef772a30500e43fbf Mon Sep 17 00:00:00 2001 From: Hampus Date: Mon, 24 Aug 2026 12:39:11 +0200 Subject: [PATCH] fix(api): distinguish a deleted reply target from no reply (#1884) --- fluxer_admin/openapi-admin.json | 2 +- fluxer_api/src/api/openapi/openapi.json | 5 +- .../channel/components/ReplyPreview.tsx | 48 +++-- .../messaging/state/MessageReferences.ts | 203 ++++++------------ .../features/voice/utils/VoiceTtsUtils.tsx | 4 +- fluxer_messages/src/shard_impl.rs | 74 +++++-- fluxer_messages/src/types.rs | 93 +++++++- .../domains/message/MessageResponseSchemas.ts | 2 +- 8 files changed, 250 insertions(+), 181 deletions(-) diff --git a/fluxer_admin/openapi-admin.json b/fluxer_admin/openapi-admin.json index a35a9ec9e..51eeaedee 100644 --- a/fluxer_admin/openapi-admin.json +++ b/fluxer_admin/openapi-admin.json @@ -13669,7 +13669,7 @@ "required": ["participants"] }, "referenced_message": { - "description": "The message that this message is replying to or forwarding", + "description": "The reply target. Present and populated when the target resolved, present and null when the target is gone, absent when this message carries no default reference. Clients must tell null apart from absent by key presence.", "nullable": true, "type": "object", "properties": { diff --git a/fluxer_api/src/api/openapi/openapi.json b/fluxer_api/src/api/openapi/openapi.json index f3df90c93..7ce50d0d9 100644 --- a/fluxer_api/src/api/openapi/openapi.json +++ b/fluxer_api/src/api/openapi/openapi.json @@ -26851,7 +26851,7 @@ }, {"type": "null"} ], - "description": "The message that this message is replying to or forwarding" + "description": "The reply target. Present and populated when the target resolved, present and null when the target is gone, absent when this message carries no default reference. Clients must tell null apart from absent by key presence." } }, "required": [ @@ -26885,8 +26885,7 @@ "value": "4096", "description": "This message will not trigger push or desktop notifications" }, - {"name": "VOICE_MESSAGE", "value": "8192", "description": "This message is a voice message"}, - {"name": "COMPACT_ATTACHMENTS", "value": "131072", "description": "Display attachments in a compact format"} + {"name": "VOICE_MESSAGE", "value": "8192", "description": "This message is a voice message"} ], "description": "Message flags bitfield" }, diff --git a/fluxer_app/src/features/channel/components/ReplyPreview.tsx b/fluxer_app/src/features/channel/components/ReplyPreview.tsx index a7808ca38..dc75dbee2 100644 --- a/fluxer_app/src/features/channel/components/ReplyPreview.tsx +++ b/fluxer_app/src/features/channel/components/ReplyPreview.tsx @@ -57,7 +57,7 @@ export const ReplyPreview = observer( messageDisplayCompact: boolean; }) => { const {i18n} = useLingui(); - const {message: referencedMessage, state: messageState} = MessageReferences.getMessageReference( + const resolution = MessageReferences.getMessageReference( message.messageReference?.channel_id ?? '', message.messageReference?.message_id ?? '', ); @@ -72,12 +72,14 @@ export const ReplyPreview = observer( } }, [message.channelId, message.id, message.messageReference]); if (!message.messageReference) return null; - if (messageState !== MessageReferenceState.LOADED || !referencedMessage) { + if (resolution.state !== MessageReferenceState.LOADED) { + const isDeleted = resolution.state === MessageReferenceState.DELETED; return (
- +
+ ) : ( + + + + )} ); } + const referencedMessage = resolution.message; const isSpammerReply = referencedMessage.author.id !== Authentication.currentUserId && LocalUserSpamOverride.isUserMarkedAsSpammer(referencedMessage.author.id, referencedMessage.author.flags); diff --git a/fluxer_app/src/features/messaging/state/MessageReferences.ts b/fluxer_app/src/features/messaging/state/MessageReferences.ts index 440d5790e..a49300bb1 100644 --- a/fluxer_app/src/features/messaging/state/MessageReferences.ts +++ b/fluxer_app/src/features/messaging/state/MessageReferences.ts @@ -1,18 +1,12 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -import {Endpoints} from '@app/features/app/constants/Endpoints'; -import Channels from '@app/features/channel/state/Channels'; -import GuildMatureContentAgree from '@app/features/guild/state/GuildMatureContentAgree'; import {Message as MessageRecord} from '@app/features/messaging/models/MessagingMessage'; import Messages from '@app/features/messaging/state/MessagingMessages'; -import {http} from '@app/features/platform/transport/RestTransport'; -import {HttpError} from '@app/features/platform/types/EndpointError'; -import {Logger} from '@app/features/platform/utils/AppLogger'; +import {MessageReferenceTypes} from '@fluxer/constants/src/ChannelConstants'; import type {ValueOf} from '@fluxer/constants/src/ValueOf'; import type {Message as WireMessage} from '@fluxer/schema/src/domains/message/MessageResponseSchemas'; import {makeAutoObservable} from 'mobx'; -const logger = new Logger('MessageReferences'); export const MessageReferenceState = { LOADED: 'LOADED', NOT_LOADED: 'NOT_LOADED', @@ -20,6 +14,20 @@ export const MessageReferenceState = { } as const; export type MessageReferenceState = ValueOf; + +export type MessageReferenceResolution = + | {readonly state: typeof MessageReferenceState.LOADED; readonly message: MessageRecord} + | {readonly state: typeof MessageReferenceState.NOT_LOADED} + | {readonly state: typeof MessageReferenceState.DELETED}; + +const NOT_LOADED_RESOLUTION: MessageReferenceResolution = Object.freeze({ + state: MessageReferenceState.NOT_LOADED, +}); + +const DELETED_RESOLUTION: MessageReferenceResolution = Object.freeze({ + state: MessageReferenceState.DELETED, +}); + type MessageInput = WireMessage | MessageRecord; const toWireMessage = (message: MessageInput): WireMessage => @@ -59,7 +67,9 @@ class MessageReferences { const key = this.getKey(refChannelId, refMessageId); const nextMessage = new MessageRecord(toWireMessage(message), {missingReactions: 'preserve'}); const currentMessage = this.cachedMessages.get(key); - if (currentMessage?.equals(nextMessage)) { + if (this.deletedMessageIds.has(key)) { + this.deletedMessageIds.delete(key); + } else if (currentMessage?.equals(nextMessage)) { return false; } this.cachedMessages.set(key, nextMessage); @@ -67,6 +77,17 @@ class MessageReferences { return true; } + private markReferenceDeleted(refChannelId: string, refMessageId: string): boolean { + const key = this.getKey(refChannelId, refMessageId); + if (this.deletedMessageIds.has(key) && !this.cachedMessages.has(key)) { + return false; + } + this.deletedMessageIds.add(key); + this.cachedMessages.delete(key); + this.bumpReferenceVersion(refChannelId, refMessageId); + return true; + } + private updateCachedMessage(refChannelId: string, refMessageId: string, updates: Partial): boolean { const key = this.getKey(refChannelId, refMessageId); const currentMessage = this.cachedMessages.get(key); @@ -116,13 +137,26 @@ class MessageReferences { this.referencingMessages.delete(referencingMessageId); } - handleMessageCreate(message: WireMessage, _optimistic: boolean): void { - if (message.referenced_message) { - const refChannelId = message.message_reference?.channel_id ?? message.channel_id; - const refMessageId = message.referenced_message.id; - this.setCachedMessage(refChannelId, refMessageId, message.referenced_message); - this.addReference(refChannelId, refMessageId, message.id); + private resolveReferenceTarget(message: WireMessage, fallbackChannelId: string): boolean { + const reference = message.message_reference; + if (!reference || reference.type !== MessageReferenceTypes.DEFAULT) { + return false; } + const refChannelId = reference.channel_id ?? fallbackChannelId; + const refMessageId = reference.message_id; + this.addReference(refChannelId, refMessageId, message.id); + if (!('referenced_message' in message)) { + return false; + } + const referenced = message.referenced_message; + if (referenced == null) { + return this.markReferenceDeleted(refChannelId, refMessageId); + } + return this.setCachedMessage(refChannelId, refMessageId, referenced); + } + + handleMessageCreate(message: WireMessage, _optimistic: boolean): void { + this.resolveReferenceTarget(message, message.channel_id); } handleMessageDelete(channelId: string, messageId: string): void { @@ -139,45 +173,13 @@ class MessageReferences { handleMessageDeleteBulk(channelId: string, messageIds: Array): void { for (const messageId of messageIds) { - const key = this.getKey(channelId, messageId); - this.deletedMessageIds.add(key); - this.cachedMessages.delete(key); - this.referenceVersions.delete(key); - this.referenceCount.delete(key); - const referencedBy = this.referencingMessages.get(messageId); - if (referencedBy) { - this.removeReference(referencedBy.channelId, referencedBy.messageId, messageId); - } + this.handleMessageDelete(channelId, messageId); } } handleMessagesFetchSuccess(channelId: string, messages: Array): void { for (const message of messages) { - if (message.referenced_message) { - const refChannelId = message.message_reference?.channel_id ?? channelId; - const refMessageId = message.referenced_message.id; - this.setCachedMessage(refChannelId, refMessageId, message.referenced_message); - this.addReference(refChannelId, refMessageId, message.id); - } - } - const potentiallyMissingMessageIds = messages - .filter((message) => message.message_reference && !message.referenced_message) - .map((message) => ({ - channelId: message.message_reference!.channel_id ?? channelId, - messageId: message.message_reference!.message_id, - referencingMessageId: message.id, - })) - .filter( - ({channelId: refChannelId, messageId}) => - !Messages.getMessage(refChannelId, messageId) && - !this.deletedMessageIds.has(this.getKey(refChannelId, messageId)) && - !this.cachedMessages.has(this.getKey(refChannelId, messageId)), - ); - for (const {channelId: refChannelId, messageId, referencingMessageId} of potentiallyMissingMessageIds) { - this.addReference(refChannelId, messageId, referencingMessageId); - } - if (potentiallyMissingMessageIds.length > 0) { - this.fetchMissingMessages(potentiallyMissingMessageIds.map(({channelId, messageId}) => ({channelId, messageId}))); + this.resolveReferenceTarget(message, channelId); } } @@ -198,9 +200,11 @@ class MessageReferences { if (!('message_reference' in message) && !('referenced_message' in message)) { return; } + const reference = message.message_reference; + const isReferenceBearing = reference != null && reference.type === MessageReferenceTypes.DEFAULT; const previousRef = this.referencingMessages.get(message.id); - const newRefChannelId = message.message_reference?.channel_id ?? message.channel_id; - const newRefMessageId = message.referenced_message?.id ?? message.message_reference?.message_id; + const newRefChannelId = reference?.channel_id ?? message.channel_id; + const newRefMessageId = isReferenceBearing ? reference.message_id : undefined; if (previousRef) { const previousKey = this.getKey(previousRef.channelId, previousRef.messageId); const newKey = newRefMessageId ? this.getKey(newRefChannelId, newRefMessageId) : null; @@ -209,61 +213,7 @@ class MessageReferences { } } if (newRefMessageId) { - if (message.referenced_message) { - this.setCachedMessage(newRefChannelId, newRefMessageId, message.referenced_message); - } - this.addReference(newRefChannelId, newRefMessageId, message.id); - } - } - - private fetchMissingMessages( - refs: Array<{ - channelId: string; - messageId: string; - }>, - ): void { - const allowedRefs = refs.filter(({channelId}) => { - const channel = Channels.getChannel(channelId); - if (!channel) { - return false; - } - if (channel.isPrivate()) { - return true; - } - return !GuildMatureContentAgree.shouldShowGate({channelId: channel.id, guildId: channel.guildId ?? null}); - }); - if (allowedRefs.length === 0) { - return; - } - Promise.allSettled( - allowedRefs.map(({channelId, messageId}) => - http - .get(Endpoints.CHANNEL_MESSAGE(channelId, messageId)) - .then((response) => { - if (response.body) { - this.handleMessageFetchSuccess(channelId, messageId, response.body); - } - }) - .catch((error) => this.handleMessageFetchError(channelId, messageId, error)), - ), - ); - } - - private handleMessageFetchSuccess(channelId: string, messageId: string, message: WireMessage): void { - const messageRecord = new MessageRecord(message); - const key = this.getKey(channelId, messageId); - this.cachedMessages.set(key, messageRecord); - this.bumpReferenceVersion(channelId, messageId); - } - - private handleMessageFetchError(channelId: string, messageId: string, error: unknown): void { - const key = this.getKey(channelId, messageId); - if (error instanceof HttpError && error.status === 404) { - this.deletedMessageIds.add(key); - this.cachedMessages.delete(key); - this.referenceVersions.delete(key); - } else { - logger.error(`Failed to fetch message ${messageId}`, error); + this.resolveReferenceTarget(message, message.channel_id); } } @@ -296,48 +246,21 @@ class MessageReferences { } } - getMessage(channelId: string, messageId: string): MessageRecord | null { + getMessageReference(channelId: string, messageId: string): MessageReferenceResolution { const key = this.getKey(channelId, messageId); this.readReferenceVersion(channelId, messageId); if (this.deletedMessageIds.has(key)) { - return null; - } - return Messages.getMessage(channelId, messageId) || this.cachedMessages.get(key) || null; - } - - getMessageReference( - channelId: string, - messageId: string, - ): { - message: MessageRecord | null; - state: MessageReferenceState; - } { - const key = this.getKey(channelId, messageId); - this.readReferenceVersion(channelId, messageId); - if (this.deletedMessageIds.has(key)) { - return { - message: null, - state: MessageReferenceState.DELETED, - }; - } - const message = Messages.getMessage(channelId, messageId); - if (message) { - return { - message, - state: MessageReferenceState.LOADED, - }; + return DELETED_RESOLUTION; } const cachedMessage = this.cachedMessages.get(key); if (cachedMessage) { - return { - message: cachedMessage, - state: MessageReferenceState.LOADED, - }; + return {state: MessageReferenceState.LOADED, message: cachedMessage}; } - return { - message: null, - state: MessageReferenceState.NOT_LOADED, - }; + const message = Messages.getMessage(channelId, messageId); + if (message) { + return {state: MessageReferenceState.LOADED, message}; + } + return NOT_LOADED_RESOLUTION; } } diff --git a/fluxer_app/src/features/voice/utils/VoiceTtsUtils.tsx b/fluxer_app/src/features/voice/utils/VoiceTtsUtils.tsx index 41bda5fb5..917f72598 100644 --- a/fluxer_app/src/features/voice/utils/VoiceTtsUtils.tsx +++ b/fluxer_app/src/features/voice/utils/VoiceTtsUtils.tsx @@ -3,7 +3,7 @@ import Accessibility from '@app/features/accessibility/state/Accessibility'; import Authentication from '@app/features/auth/state/Authentication'; import Channels from '@app/features/channel/state/Channels'; -import MessageReferences from '@app/features/messaging/state/MessageReferences'; +import MessageReferences, {MessageReferenceState} from '@app/features/messaging/state/MessageReferences'; import {SystemMessageUtils} from '@app/features/messaging/utils/SystemMessageUtils'; import SelectedChannel from '@app/features/navigation/state/SelectedChannel'; import Relationships from '@app/features/relationship/state/Relationships'; @@ -454,7 +454,7 @@ function handleIncomingTtsMessage(message: Message): void { const refChannelId = message.message_reference.channel_id ?? message.channel_id; const refMessageId = message.message_reference.message_id; const ref = MessageReferences.getMessageReference(refChannelId, refMessageId); - if (ref.message) { + if (ref.state === MessageReferenceState.LOADED) { const replyAuthor = Users.getUser(ref.message.author.id); if (replyAuthor) { replyAuthorName = NicknameUtils.getNickname(replyAuthor, channel.guildId ?? null); diff --git a/fluxer_messages/src/shard_impl.rs b/fluxer_messages/src/shard_impl.rs index 5d63f4a04..29d4cd068 100644 --- a/fluxer_messages/src/shard_impl.rs +++ b/fluxer_messages/src/shard_impl.rs @@ -39,6 +39,12 @@ const BUCKET_DURATION_MS: i64 = 864_000_000; const FLUXER_EPOCH_MS: i64 = 1_420_070_400_000; const SERVICE_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); const MESSAGE_REFERENCE_TYPE_DEFAULT: i32 = 0; + +fn effective_reference_type(reference: &MessageReference) -> i32 { + reference + .reference_type + .unwrap_or(MESSAGE_REFERENCE_TYPE_DEFAULT) +} const MESSAGE_FLAG_SUPPRESS_EMBEDS: i64 = 1 << 2; const PUBLIC_USER_FLAGS: i64 = (1 << 0) | (1 << 1) | (1 << 2) | (1 << 3) | (1 << 4) | (1 << 5) | (1 << 6); @@ -787,7 +793,7 @@ impl MessagesShard { let Some(reference) = &message.message_reference else { continue; }; - if reference.reference_type != Some(MESSAGE_REFERENCE_TYPE_DEFAULT) { + if effective_reference_type(reference) != MESSAGE_REFERENCE_TYPE_DEFAULT { continue; } let (Some(channel_id), Some(message_id)) = (reference.channel_id, reference.message_id) @@ -1099,23 +1105,25 @@ impl MessagesShard { Some(( reference.channel_id?, reference.message_id?, - reference.reference_type?, + effective_reference_type(reference), )) }) .filter(|(_, _, reference_type)| *reference_type == MESSAGE_REFERENCE_TYPE_DEFAULT) - .and_then(|(channel_id, message_id, _)| { - context.referenced_messages.get(&(channel_id, message_id)) - }) - .map(|referenced| { - let mut referenced_options = options.clone(); - referenced_options.nonce = None; - referenced_options.tts = false; - Box::new(self.map_message_response( - referenced, - &referenced_options, - context, - false, - )) + .map(|(channel_id, message_id, _)| { + context + .referenced_messages + .get(&(channel_id, message_id)) + .map(|referenced| { + let mut referenced_options = options.clone(); + referenced_options.nonce = None; + referenced_options.tts = false; + Box::new(self.map_message_response( + referenced, + &referenced_options, + context, + false, + )) + }) }) } else { None @@ -2743,7 +2751,7 @@ fn map_message_reference(reference: &MessageReference) -> Option(deserializer: D) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, + T: Deserialize<'de>, +{ + Option::::deserialize(deserializer).map(Some) +} + mod serde_id { use serde::Deserialize; use serde::de::{self, Deserializer}; @@ -461,8 +469,12 @@ pub struct ApiMessageResponse { pub nonce: Option, #[serde(skip_serializing_if = "Option::is_none")] pub call: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub referenced_message: Option>, + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "deserialize_double_option" + )] + pub referenced_message: Option>>, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -740,4 +752,81 @@ mod tests { vec![1_472_426_752_046_002_208] ); } + + fn minimal_api_message( + referenced: Option>>, + ) -> ApiMessageResponse { + let author = ApiUserPartialResponse { + id: "1".to_string(), + username: "user".to_string(), + discriminator: "0001".to_string(), + global_name: None, + avatar: None, + avatar_color: None, + bot: None, + system: None, + flags: 0, + mention_flags: None, + }; + ApiMessageResponse { + id: "2".to_string(), + channel_id: "3".to_string(), + author, + webhook_id: None, + message_type: 0, + flags: 0, + content: String::new(), + timestamp: "2026-01-01T00:00:00Z".to_string(), + edited_timestamp: None, + pinned: false, + mention_everyone: false, + tts: false, + mentions: Vec::new(), + mention_roles: Vec::new(), + mention_channels: None, + users: None, + embeds: Vec::new(), + attachments: Vec::new(), + stickers: Vec::new(), + nsfw_emojis: None, + reactions: None, + message_reference: None, + message_snapshots: None, + nonce: None, + call: None, + referenced_message: referenced, + } + } + + fn msgpack_round_trip( + referenced: Option>>, + ) -> Option>> { + let encoded = + rmp_serde::to_vec_named(&minimal_api_message(referenced)).expect("encodes to msgpack"); + rmp_serde::from_slice::(&encoded) + .expect("decodes from msgpack") + .referenced_message + } + + #[test] + fn referenced_message_null_survives_the_msgpack_round_trip() { + assert!(msgpack_round_trip(None).is_none()); + assert!(matches!(msgpack_round_trip(Some(None)), Some(None))); + assert!(matches!( + msgpack_round_trip(Some(Some(Box::new(minimal_api_message(None))))), + Some(Some(_)) + )); + } + + #[test] + fn referenced_message_null_serialises_as_json_null_not_an_absent_key() { + let deleted = serde_json::to_value(minimal_api_message(Some(None))).expect("serialises"); + assert_eq!( + deleted.get("referenced_message"), + Some(&serde_json::Value::Null) + ); + + let absent = serde_json::to_value(minimal_api_message(None)).expect("serialises"); + assert!(absent.get("referenced_message").is_none()); + } } diff --git a/packages/schema/src/domains/message/MessageResponseSchemas.ts b/packages/schema/src/domains/message/MessageResponseSchemas.ts index 9610052c9..96d72b49b 100644 --- a/packages/schema/src/domains/message/MessageResponseSchemas.ts +++ b/packages/schema/src/domains/message/MessageResponseSchemas.ts @@ -169,7 +169,7 @@ export interface MessageResponse extends MessageBaseResponse { export const MessageResponseSchema = MessageBaseResponseSchema.extend({ referenced_message: MessageBaseResponseSchema.nullish().describe( - 'The message that this message is replying to or forwarding', + 'The reply target. Present and populated when the target resolved, present and null when the target is gone, absent when this message carries no default reference. Clients must tell null apart from absent by key presence.', ), }); const ChannelPinMessageResponse = MessageResponseSchema.omit({