Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Avoid hitting the settings store from TextForEvent #6205

Merged
merged 17 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
76 changes: 45 additions & 31 deletions src/TextForEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,21 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import {MatrixClientPeg} from './MatrixClientPeg';
import { MatrixEvent } from "matrix-js-sdk/src/models/event";

import { MatrixClientPeg } from './MatrixClientPeg';
import { _t } from './languageHandler';
import * as Roles from './Roles';
import {isValid3pidInvite} from "./RoomInvite";
import { isValid3pidInvite } from "./RoomInvite";
import SettingsStore from "./settings/SettingsStore";
import {ALL_RULE_TYPES, ROOM_RULE_TYPES, SERVER_RULE_TYPES, USER_RULE_TYPES} from "./mjolnir/BanList";
import {WIDGET_LAYOUT_EVENT_TYPE} from "./stores/widgets/WidgetLayoutStore";
import { ALL_RULE_TYPES, ROOM_RULE_TYPES, SERVER_RULE_TYPES, USER_RULE_TYPES } from "./mjolnir/BanList";
import { WIDGET_LAYOUT_EVENT_TYPE } from "./stores/widgets/WidgetLayoutStore";

// These functions are frequently used just to check whether an event has
// any text to display at all. For this reason they return deferred values
// to avoid the expense of looking up translations when they're not needed.

function textForMemberEvent(ev): () => string | null {
function textForMemberEvent(ev: MatrixEvent, showHiddenEvents?: boolean): () => string | null {
// XXX: SYJS-16 "sender is sometimes null for join messages"
const senderName = ev.sender ? ev.sender.name : ev.getSender();
const targetName = ev.target ? ev.target.name : ev.getStateKey();
Expand Down Expand Up @@ -75,7 +77,7 @@ function textForMemberEvent(ev): () => string | null {
return () => _t('%(senderName)s changed their profile picture.', {senderName});
} else if (!prevContent.avatar_url && content.avatar_url) {
return () => _t('%(senderName)s set a profile picture.', {senderName});
} else if (SettingsStore.getValue("showHiddenEventsInTimeline")) {
} else if (showHiddenEvents ?? SettingsStore.getValue("showHiddenEventsInTimeline")) {
// This is a null rejoin, it will only be visible if the Labs option is enabled
return () => _t("%(senderName)s made no change.", {senderName});
} else {
Expand Down Expand Up @@ -107,15 +109,15 @@ function textForMemberEvent(ev): () => string | null {
}
}

function textForTopicEvent(ev): () => string | null {
function textForTopicEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
return () => _t('%(senderDisplayName)s changed the topic to "%(topic)s".', {
senderDisplayName,
topic: ev.getContent().topic,
});
}

function textForRoomNameEvent(ev): () => string | null {
function textForRoomNameEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();

if (!ev.getContent().name || ev.getContent().name.trim().length === 0) {
Expand All @@ -134,12 +136,12 @@ function textForRoomNameEvent(ev): () => string | null {
});
}

function textForTombstoneEvent(ev): () => string | null {
function textForTombstoneEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
return () => _t('%(senderDisplayName)s upgraded this room.', {senderDisplayName});
}

function textForJoinRulesEvent(ev): () => string | null {
function textForJoinRulesEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
switch (ev.getContent().join_rule) {
case "public":
Expand All @@ -159,7 +161,7 @@ function textForJoinRulesEvent(ev): () => string | null {
}
}

function textForGuestAccessEvent(ev): () => string | null {
function textForGuestAccessEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
switch (ev.getContent().guest_access) {
case "can_join":
Expand All @@ -175,7 +177,7 @@ function textForGuestAccessEvent(ev): () => string | null {
}
}

function textForRelatedGroupsEvent(ev): () => string | null {
function textForRelatedGroupsEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
const groups = ev.getContent().groups || [];
const prevGroups = ev.getPrevContent().groups || [];
Expand Down Expand Up @@ -205,7 +207,7 @@ function textForRelatedGroupsEvent(ev): () => string | null {
}
}

function textForServerACLEvent(ev): () => string | null {
function textForServerACLEvent(ev: MatrixEvent): () => string | null {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
const prevContent = ev.getPrevContent();
const current = ev.getContent();
Expand Down Expand Up @@ -235,7 +237,7 @@ function textForServerACLEvent(ev): () => string | null {
return getText;
}

function textForMessageEvent(ev): () => string | null {
function textForMessageEvent(ev: MatrixEvent): () => string | null {
return () => {
const senderDisplayName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
let message = senderDisplayName + ': ' + ev.getContent().body;
Expand All @@ -248,7 +250,7 @@ function textForMessageEvent(ev): () => string | null {
};
}

function textForCanonicalAliasEvent(ev): () => string | null {
function textForCanonicalAliasEvent(ev: MatrixEvent): () => string | null {
const senderName = ev.sender && ev.sender.name ? ev.sender.name : ev.getSender();
const oldAlias = ev.getPrevContent().alias;
const oldAltAliases = ev.getPrevContent().alt_aliases || [];
Expand Down Expand Up @@ -299,15 +301,15 @@ function textForCanonicalAliasEvent(ev): () => string | null {
});
}

function textForCallAnswerEvent(event): () => string | null {
function textForCallAnswerEvent(event: MatrixEvent): () => string | null {
return () => {
const senderName = event.sender ? event.sender.name : _t('Someone');
const supported = MatrixClientPeg.get().supportsVoip() ? '' : _t('(not supported by this browser)');
return _t('%(senderName)s answered the call.', {senderName}) + ' ' + supported;
};
}

function textForCallHangupEvent(event): () => string | null {
function textForCallHangupEvent(event: MatrixEvent): () => string | null {
const getSenderName = () => event.sender ? event.sender.name : _t('Someone');
const eventContent = event.getContent();
let getReason = () => "";
Expand Down Expand Up @@ -344,14 +346,14 @@ function textForCallHangupEvent(event): () => string | null {
return () => _t('%(senderName)s ended the call.', {senderName: getSenderName()}) + ' ' + getReason();
}

function textForCallRejectEvent(event): () => string | null {
function textForCallRejectEvent(event: MatrixEvent): () => string | null {
return () => {
const senderName = event.sender ? event.sender.name : _t('Someone');
return _t('%(senderName)s declined the call.', {senderName});
};
}

function textForCallInviteEvent(event): () => string | null {
function textForCallInviteEvent(event: MatrixEvent): () => string | null {
const getSenderName = () => event.sender ? event.sender.name : _t('Someone');
// FIXME: Find a better way to determine this from the event?
let isVoice = true;
Expand Down Expand Up @@ -383,7 +385,7 @@ function textForCallInviteEvent(event): () => string | null {
}
}

function textForThreePidInviteEvent(event): () => string | null {
function textForThreePidInviteEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();

if (!isValid3pidInvite(event)) {
Expand All @@ -399,7 +401,7 @@ function textForThreePidInviteEvent(event): () => string | null {
});
}

function textForHistoryVisibilityEvent(event): () => string | null {
function textForHistoryVisibilityEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
switch (event.getContent().history_visibility) {
case 'invited':
Expand All @@ -421,7 +423,7 @@ function textForHistoryVisibilityEvent(event): () => string | null {
}

// Currently will only display a change if a user's power level is changed
function textForPowerEvent(event): () => string | null {
function textForPowerEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
if (!event.getPrevContent() || !event.getPrevContent().users ||
!event.getContent() || !event.getContent().users) {
Expand Down Expand Up @@ -466,12 +468,12 @@ function textForPowerEvent(event): () => string | null {
});
}

function textForPinnedEvent(event): () => string | null {
function textForPinnedEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender ? event.sender.name : event.getSender();
return () => _t("%(senderName)s changed the pinned messages for the room.", {senderName});
}

function textForWidgetEvent(event): () => string | null {
function textForWidgetEvent(event: MatrixEvent): () => string | null {
const senderName = event.getSender();
const {name: prevName, type: prevType, url: prevUrl} = event.getPrevContent();
const {name, type, url} = event.getContent() || {};
Expand Down Expand Up @@ -501,12 +503,12 @@ function textForWidgetEvent(event): () => string | null {
}
}

function textForWidgetLayoutEvent(event): () => string | null {
function textForWidgetLayoutEvent(event: MatrixEvent): () => string | null {
const senderName = event.sender?.name || event.getSender();
return () => _t("%(senderName)s has updated the widget layout", {senderName});
}

function textForMjolnirEvent(event): () => string | null {
function textForMjolnirEvent(event: MatrixEvent): () => string | null {
const senderName = event.getSender();
const {entity: prevEntity} = event.getPrevContent();
const {entity, recommendation, reason} = event.getContent();
Expand Down Expand Up @@ -594,7 +596,7 @@ function textForMjolnirEvent(event): () => string | null {
}

interface IHandlers {
[type: string]: (ev: any) => (() => string | null);
[type: string]: (ev: MatrixEvent, showHiddenEvents?: boolean) => (() => string | null);
}

const handlers: IHandlers = {
Expand Down Expand Up @@ -630,12 +632,24 @@ for (const evType of ALL_RULE_TYPES) {
stateHandlers[evType] = textForMjolnirEvent;
}

export function hasText(ev): boolean {
/**
* Determines whether the given event has text to display.
* @param ev The event
* @param showHiddenEvents An optional cached setting value for showHiddenEventsInTimeline
* to avoid hitting the settings store
*/
export function hasText(ev: MatrixEvent, showHiddenEvents?: boolean): boolean {
const handler = (ev.isState() ? stateHandlers : handlers)[ev.getType()];
return Boolean(handler?.(ev));
return Boolean(handler?.(ev, showHiddenEvents));
}

export function textForEvent(ev): string {
/**
* Gets the textual content of the given event.
* @param ev The event
* @param showHiddenEvents An optional cached setting value for showHiddenEventsInTimeline
* to avoid hitting the settings store
*/
export function textForEvent(ev: MatrixEvent, showHiddenEvents?: boolean): string {
const handler = (ev.isState() ? stateHandlers : handlers)[ev.getType()];
return handler?.(ev)?.() || '';
return handler?.(ev, showHiddenEvents)?.() || '';
}
20 changes: 11 additions & 9 deletions src/components/structures/MessagePanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const continuedTypes = ['m.sticker', 'm.room.message'];

// check if there is a previous event and it has the same sender as this event
// and the types are the same/is in continuedTypes and the time between them is <= CONTINUATION_MAX_INTERVAL
function shouldFormContinuation(prevEvent, mxEvent) {
function shouldFormContinuation(prevEvent, mxEvent, showHiddenEvents) {
// sanity check inputs
if (!prevEvent || !prevEvent.sender || !mxEvent.sender) return false;
// check if within the max continuation period
Expand All @@ -61,7 +61,7 @@ function shouldFormContinuation(prevEvent, mxEvent) {
mxEvent.sender.getMxcAvatarUrl() !== prevEvent.sender.getMxcAvatarUrl()) return false;

// if we don't have tile for previous event then it was shown by showHiddenEvents and has no SenderProfile
if (!haveTileForEvent(prevEvent)) return false;
if (!haveTileForEvent(prevEvent, showHiddenEvents)) return false;

return true;
}
Expand Down Expand Up @@ -202,7 +202,8 @@ export default class MessagePanel extends React.Component {
this._readReceiptsByUserId = {};

// Cache hidden events setting on mount since Settings is expensive to
// query, and we check this in a hot code path.
// query, and we check this in a hot code path. This is also cached in
// our RoomContext, however we still need a fallback for roomless MessagePanels.
this._showHiddenEventsInTimeline =
SettingsStore.getValue("showHiddenEventsInTimeline");

Expand Down Expand Up @@ -372,11 +373,11 @@ export default class MessagePanel extends React.Component {
return false; // ignored = no show (only happens if the ignore happens after an event was received)
}

if (this._showHiddenEventsInTimeline) {
if (this.context?.showHiddenEventsInTimeline ?? this._showHiddenEventsInTimeline) {
robintown marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

if (!haveTileForEvent(mxEv)) {
if (!haveTileForEvent(mxEv, this.context?.showHiddenEventsInTimeline)) {
robintown marked this conversation as resolved.
Show resolved Hide resolved
return false; // no tile = no show
}

Expand Down Expand Up @@ -536,7 +537,7 @@ export default class MessagePanel extends React.Component {

if (grouper) {
if (grouper.shouldGroup(mxEv)) {
grouper.add(mxEv);
grouper.add(mxEv, this.context?.showHiddenEventsInTimeline);
robintown marked this conversation as resolved.
Show resolved Hide resolved
continue;
} else {
// not part of group, so get the group tiles, close the
Expand Down Expand Up @@ -613,7 +614,8 @@ export default class MessagePanel extends React.Component {
}

// is this a continuation of the previous message?
const continuation = !wantsDateSeparator && shouldFormContinuation(prevEvent, mxEv);
const continuation = !wantsDateSeparator &&
shouldFormContinuation(prevEvent, mxEv, this.context?.showHiddenEventsInTimeline);
robintown marked this conversation as resolved.
Show resolved Hide resolved

const eventId = mxEv.getId();
const highlight = (eventId === this.props.highlightedEventId);
Expand Down Expand Up @@ -1165,10 +1167,10 @@ class MemberGrouper {
return isMembershipChange(ev);
}

add(ev) {
add(ev, showHiddenEvents) {
if (ev.getType() === 'm.room.member') {
// We can ignore any events that don't actually have a message to display
if (!hasText(ev)) return;
if (!hasText(ev, showHiddenEvents)) return;
}
this.readMarker = this.readMarker || this.panel._readMarkerForEvent(
ev.getId(),
Expand Down
7 changes: 6 additions & 1 deletion src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export interface IState {
canReply: boolean;
layout: Layout;
lowBandwidth: boolean;
showHiddenEventsInTimeline: boolean;
showReadReceipts: boolean;
showRedactions: boolean;
showJoinLeaves: boolean;
Expand Down Expand Up @@ -244,6 +245,7 @@ export default class RoomView extends React.Component<IProps, IState> {
canReply: false,
layout: SettingsStore.getValue("layout"),
lowBandwidth: SettingsStore.getValue("lowBandwidth"),
showHiddenEventsInTimeline: SettingsStore.getValue("showHiddenEventsInTimeline"),
showReadReceipts: true,
showRedactions: true,
showJoinLeaves: true,
Expand Down Expand Up @@ -282,6 +284,9 @@ export default class RoomView extends React.Component<IProps, IState> {
SettingsStore.watchSetting("lowBandwidth", null, () =>
this.setState({ lowBandwidth: SettingsStore.getValue("lowBandwidth") }),
),
SettingsStore.watchSetting("showHiddenEventsInTimeline", null, () =>
this.setState({ showHiddenEventsInTimeline: SettingsStore.getValue("showHiddenEventsInTimeline") }),
),
];
}

Expand Down Expand Up @@ -1402,7 +1407,7 @@ export default class RoomView extends React.Component<IProps, IState> {
continue;
}

if (!haveTileForEvent(mxEv)) {
if (!haveTileForEvent(mxEv, this.state.showHiddenEventsInTimeline)) {
// XXX: can this ever happen? It will make the result count
// not match the displayed count.
continue;
Expand Down
3 changes: 2 additions & 1 deletion src/components/structures/TimelinePanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,8 @@ class TimelinePanel extends React.Component {

const shouldIgnore = !!ev.status || // local echo
(ignoreOwn && ev.sender && ev.sender.userId == myUserId); // own message
const isWithoutTile = !haveTileForEvent(ev) || shouldHideEvent(ev, this.context);
const isWithoutTile = !haveTileForEvent(ev, this.context?.showHiddenEventsInTimeline) ||
shouldHideEvent(ev, this.context);

if (isWithoutTile || !node) {
// don't start counting if the event should be ignored,
Expand Down
5 changes: 4 additions & 1 deletion src/components/views/messages/TextualEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.

import React from 'react';
import PropTypes from 'prop-types';
import RoomContext from "../../../contexts/RoomContext";
import * as TextForEvent from "../../../TextForEvent";
import {replaceableComponent} from "../../../utils/replaceableComponent";

Expand All @@ -27,8 +28,10 @@ export default class TextualEvent extends React.Component {
mxEvent: PropTypes.object.isRequired,
};

static contextType = RoomContext;

render() {
const text = TextForEvent.textForEvent(this.props.mxEvent);
const text = TextForEvent.textForEvent(this.props.mxEvent, this.context?.showHiddenEventsInTimeline);
robintown marked this conversation as resolved.
Show resolved Hide resolved
if (text == null || text.length === 0) return null;
return (
<div className="mx_TextualEvent">{ text }</div>
Expand Down
Loading