From f76c8c160b20224fae792869f11a1fd7ef3a2f45 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 18 Jun 2026 21:03:48 -0500 Subject: [PATCH] Refactor to remove `SynapseConfig` from shared code --- rust/src/handlers/mod.rs | 11 +- rust/src/handlers/versions.rs | 312 +++++++++++++++++++--------------- rust/src/storage/store.rs | 28 +-- 3 files changed, 191 insertions(+), 160 deletions(-) diff --git a/rust/src/handlers/mod.rs b/rust/src/handlers/mod.rs index e5243f87e1..1f04b65e10 100644 --- a/rust/src/handlers/mod.rs +++ b/rust/src/handlers/mod.rs @@ -52,15 +52,16 @@ impl RustHandlers { let db_pool = PythonDatabasePoolWrapper::new(db_pool_py, reactor.clone_ref(py)); // Store is shared across all of the handlers so let's use an `Arc` - let store = Arc::new(Store { - config: config.clone(), - db_pool, - }); + let store = Arc::new(Store { db_pool }); + + let global_unstable_feature_map = Arc::new( + versions::synapse_config_to_global_unstable_feature_map(&config), + ); let versions = Py::new( py, versions::VersionsHandler { - config: config.clone(), + global_unstable_feature_map: Arc::clone(&global_unstable_feature_map), store: Arc::clone(&store), reactor: reactor.clone_ref(py), }, diff --git a/rust/src/handlers/versions.rs b/rust/src/handlers/versions.rs index 77faaf2c14..e2d8cd1b7d 100644 --- a/rust/src/handlers/versions.rs +++ b/rust/src/handlers/versions.rs @@ -44,7 +44,7 @@ impl<'py> IntoPyObject<'py> for VersionsResponse { #[pyclass] pub struct VersionsHandler { - pub config: SynapseConfig, + pub global_unstable_feature_map: Arc, pub store: Arc>, /// The Twisted reactor, used to bridge our `async` response back into a /// Twisted deferred that Python can `await`. @@ -62,10 +62,10 @@ impl VersionsHandler { user_id: Option, ) -> PyResult> { let store = Arc::clone(&self.store); - let config = self.config.clone(); + let global_unstable_feature_map = Arc::clone(&self.global_unstable_feature_map); create_deferred(py, self.reactor.bind(py), async move { - build_versions_response(&store, &config, user_id.as_deref()) + build_versions_response(&store, &global_unstable_feature_map, user_id.as_deref()) .await .map_err(|err| { pyo3::exceptions::PyRuntimeError::new_err(format!( @@ -77,148 +77,190 @@ impl VersionsHandler { } /// Assemble a `/versions` response body. +/// +/// Args: +/// * store +/// * global_unstable_feature_map: The global values before any per-user overrides +/// * user_id: The user making the request async fn build_versions_response( store: &Store, - config: &SynapseConfig, + global_unstable_feature_map: &UnstableFeatureMap, user_id: Option<&str>, ) -> Result { - { - let msc3881_enabled = match user_id { - Some(user_id) => { - store - .is_feature_enabled(user_id, PerUserExperimentalFeature::MSC3881) + let msc3881_enabled = match user_id { + Some(user_id) => { + // Don't both looking anything up if it's enabled for everyone + if global_unstable_feature_map.msc3881 { + true + } else { + // Look up whether it's explicitly enabled/disabled for this user + match store + .is_feature_enabled_for_user(user_id, PerUserExperimentalFeature::MSC3881) .await? + { + Some(enabled) => enabled, + // Default to false if there is no entry for this user + None => false, + } } - None => PerUserExperimentalFeature::MSC3881.is_globally_enabled(config), - }; + } + None => global_unstable_feature_map.msc3881, + }; - let msc3575_enabled = match user_id { - Some(user_id) => { - store - .is_feature_enabled(user_id, PerUserExperimentalFeature::MSC3575) + let msc3575_enabled = match user_id { + Some(user_id) => { + // Don't both looking anything up if it's enabled for everyone + if global_unstable_feature_map.msc3575 { + true + } else { + // Look up whether it's explicitly enabled/disabled for this user + match store + .is_feature_enabled_for_user(user_id, PerUserExperimentalFeature::MSC3575) .await? + { + Some(enabled) => enabled, + // Default to false if there is no entry for this user + None => false, + } } - None => PerUserExperimentalFeature::MSC3575.is_globally_enabled(config), - }; + } + None => global_unstable_feature_map.msc3575, + }; - // TODO: Calculate these once since they shouldn't change after start-up. - // e2ee_forced_public = ( - // RoomCreationPreset.PUBLIC_CHAT - // in config.room.encryption_enabled_by_default_for_room_presets - // ); - // e2ee_forced_private = ( - // RoomCreationPreset.PRIVATE_CHAT - // in config.room.encryption_enabled_by_default_for_room_presets - // ); - // e2ee_forced_trusted_private = ( - // RoomCreationPreset.TRUSTED_PRIVATE_CHAT - // in config.room.encryption_enabled_by_default_for_room_presets - // ); + // TODO: Calculate these once since they shouldn't change after start-up. + // e2ee_forced_public = ( + // RoomCreationPreset.PUBLIC_CHAT + // in config.room.encryption_enabled_by_default_for_room_presets + // ); + // e2ee_forced_private = ( + // RoomCreationPreset.PRIVATE_CHAT + // in config.room.encryption_enabled_by_default_for_room_presets + // ); + // e2ee_forced_trusted_private = ( + // RoomCreationPreset.TRUSTED_PRIVATE_CHAT + // in config.room.encryption_enabled_by_default_for_room_presets + // ); - Ok(VersionsResponse { - versions: Vec::from([ - // XXX: at some point we need to decide whether we need to include - // the previous version numbers, given we've defined r0.3.0 to be - // backwards compatible with r0.2.0. But need to check how - // conscientious we've been in compatibility, and decide whether the - // middle number is the major revision when at 0.X.Y (as opposed to - // X.Y.Z). And we need to decide whether it's fair to make clients - // parse the version string to figure out what's going on. - "r0.0.1".to_string(), - "r0.1.0".to_string(), - "r0.2.0".to_string(), - "r0.3.0".to_string(), - "r0.4.0".to_string(), - "r0.5.0".to_string(), - "r0.6.0".to_string(), - "r0.6.1".to_string(), - "v1.1".to_string(), - "v1.2".to_string(), - "v1.3".to_string(), - "v1.4".to_string(), - "v1.5".to_string(), - "v1.6".to_string(), - "v1.7".to_string(), - "v1.8".to_string(), - "v1.9".to_string(), - "v1.10".to_string(), - "v1.11".to_string(), - "v1.12".to_string(), - ]), - unstable_features: std::collections::BTreeMap::from([ - // // Implements support for label-based filtering as described in - // // MSC2326. - // ("org.matrix.label_based_filtering".to_string(), true), - // // Implements support for cross signing as described in MSC1756 - // ("org.matrix.e2e_cross_signing".to_string(), true), - // // Implements additional endpoints as described in MSC2432 - // ("org.matrix.msc2432".to_string(), true), - // // Implements additional endpoints as described in MSC2666 - // ("uk.half-shot.msc2666.query_mutual_rooms.stable".to_string(), true), - // // Whether new rooms will be set to encrypted or not (based on presets). - // ("io.element.e2ee_forced.public".to_string(), e2ee_forced_public), - // ("io.element.e2ee_forced.private".to_string(), e2ee_forced_private), - // ("io.element.e2ee_forced.trusted_private".to_string(), e2ee_forced_trusted_private), - // // Supports the busy presence state described in MSC3026. - // ("org.matrix.msc3026.busy_presence".to_string(), config.experimental.msc3026_enabled), - // // Supports receiving private read receipts as per MSC2285 - // ("org.matrix.msc2285.stable".to_string(), true), // TODO: Remove when MSC2285 becomes a part of the spec - // // Supports filtering of /publicRooms by room type as per MSC3827 - // ("org.matrix.msc3827.stable".to_string(), true), - // // Adds support for thread relations, per MSC3440. - // ("org.matrix.msc3440.stable".to_string(), true), // TODO: remove when "v1.3" is added above - // // Support for thread read receipts & notification counts. - // ("org.matrix.msc3771".to_string(), true), - // ("org.matrix.msc3773".to_string(), config.experimental.msc3773_enabled), - // // Allows moderators to fetch redacted event content as described in MSC2815 - // ("fi.mau.msc2815".to_string(), config.experimental.msc2815_enabled), - // // Adds a ping endpoint for appservices to check HS->AS connection - // ("fi.mau.msc2659.stable".to_string(), true), // TODO: remove when "v1.7" is added above - // // TODO: this is no longer needed once unstable MSC3882 does not need to be supported: - // ("org.matrix.msc3882".to_string(), config.auth.login_via_existing_enabled), - // Adds support for remotely enabling/disabling pushers, as per MSC3881 - ("org.matrix.msc3881".to_string(), msc3881_enabled), - // // Adds support for filtering /messages by event relation. - // ("org.matrix.msc3874".to_string(), config.experimental.msc3874_enabled), - // // Adds support for relation-based redactions as per MSC3912. - // ("org.matrix.msc3912".to_string(), config.experimental.msc3912_enabled), - // // Whether recursively provide relations is supported. - // // TODO This is no longer needed once unstable MSC3981 does not need to be supported. - // ("org.matrix.msc3981".to_string(), true), - // // Adds support for deleting account data. - // ("org.matrix.msc3391".to_string(), config.experimental.msc3391_enabled), - // // Allows clients to inhibit profile update propagation. - // ("org.matrix.msc4069".to_string(), config.experimental.msc4069_profile_inhibit_propagation), - // // Allows clients to handle push for encrypted events. - // ("org.matrix.msc4028".to_string(), config.experimental.msc4028_push_encrypted_events), - // // MSC4108: Mechanism to allow OIDC sign in and E2EE set up via QR code - 2024 version - // ("org.matrix.msc4108".to_string(), ( - // config.experimental.msc4108_enabled - // or ( - // config.experimental.msc4108_delegation_endpoint - // is not None - // ) - // )), - // // MSC4140: Delayed events - // ("org.matrix.msc4140".to_string(), bool(config.server.max_event_delay_ms)), - // Simplified sliding sync - ("org.matrix.simplified_msc3575".to_string(), msc3575_enabled), - // // Arbitrary key-value profile fields. - // ("uk.tcpip.msc4133".to_string(), config.experimental.msc4133_enabled), - // ("uk.tcpip.msc4133.stable".to_string(), true), - // // MSC4155: Invite filtering - // ("org.matrix.msc4155".to_string(), config.experimental.msc4155_enabled), - // // MSC4306: Support for thread subscriptions - // ("org.matrix.msc4306".to_string(), config.experimental.msc4306_enabled), - // // MSC4169: Backwards-compatible redaction sending using `/send` - // ("com.beeper.msc4169".to_string(), config.experimental.msc4169_enabled), - // // MSC4354: Sticky events - // ("org.matrix.msc4354".to_string(), config.experimental.msc4354_enabled), - // // MSC4380: Invite blocking - // ("org.matrix.msc4380.stable".to_string(), true), - // // MSC4445: Sync timeline order - // ("org.matrix.msc4445.initial_sync_timeline_topological_ordering".to_string(), true), - ]), - }) + Ok(VersionsResponse { + versions: Vec::from([ + // XXX: at some point we need to decide whether we need to include + // the previous version numbers, given we've defined r0.3.0 to be + // backwards compatible with r0.2.0. But need to check how + // conscientious we've been in compatibility, and decide whether the + // middle number is the major revision when at 0.X.Y (as opposed to + // X.Y.Z). And we need to decide whether it's fair to make clients + // parse the version string to figure out what's going on. + "r0.0.1".to_string(), + "r0.1.0".to_string(), + "r0.2.0".to_string(), + "r0.3.0".to_string(), + "r0.4.0".to_string(), + "r0.5.0".to_string(), + "r0.6.0".to_string(), + "r0.6.1".to_string(), + "v1.1".to_string(), + "v1.2".to_string(), + "v1.3".to_string(), + "v1.4".to_string(), + "v1.5".to_string(), + "v1.6".to_string(), + "v1.7".to_string(), + "v1.8".to_string(), + "v1.9".to_string(), + "v1.10".to_string(), + "v1.11".to_string(), + "v1.12".to_string(), + ]), + unstable_features: std::collections::BTreeMap::from([ + // // Implements support for label-based filtering as described in + // // MSC2326. + // ("org.matrix.label_based_filtering".to_string(), true), + // // Implements support for cross signing as described in MSC1756 + // ("org.matrix.e2e_cross_signing".to_string(), true), + // // Implements additional endpoints as described in MSC2432 + // ("org.matrix.msc2432".to_string(), true), + // // Implements additional endpoints as described in MSC2666 + // ("uk.half-shot.msc2666.query_mutual_rooms.stable".to_string(), true), + // // Whether new rooms will be set to encrypted or not (based on presets). + // ("io.element.e2ee_forced.public".to_string(), e2ee_forced_public), + // ("io.element.e2ee_forced.private".to_string(), e2ee_forced_private), + // ("io.element.e2ee_forced.trusted_private".to_string(), e2ee_forced_trusted_private), + // // Supports the busy presence state described in MSC3026. + // ("org.matrix.msc3026.busy_presence".to_string(), config.experimental.msc3026_enabled), + // // Supports receiving private read receipts as per MSC2285 + // ("org.matrix.msc2285.stable".to_string(), true), // TODO: Remove when MSC2285 becomes a part of the spec + // // Supports filtering of /publicRooms by room type as per MSC3827 + // ("org.matrix.msc3827.stable".to_string(), true), + // // Adds support for thread relations, per MSC3440. + // ("org.matrix.msc3440.stable".to_string(), true), // TODO: remove when "v1.3" is added above + // // Support for thread read receipts & notification counts. + // ("org.matrix.msc3771".to_string(), true), + // ("org.matrix.msc3773".to_string(), config.experimental.msc3773_enabled), + // // Allows moderators to fetch redacted event content as described in MSC2815 + // ("fi.mau.msc2815".to_string(), config.experimental.msc2815_enabled), + // // Adds a ping endpoint for appservices to check HS->AS connection + // ("fi.mau.msc2659.stable".to_string(), true), // TODO: remove when "v1.7" is added above + // // TODO: this is no longer needed once unstable MSC3882 does not need to be supported: + // ("org.matrix.msc3882".to_string(), config.auth.login_via_existing_enabled), + // Adds support for remotely enabling/disabling pushers, as per MSC3881 + ("org.matrix.msc3881".to_string(), msc3881_enabled), + // // Adds support for filtering /messages by event relation. + // ("org.matrix.msc3874".to_string(), config.experimental.msc3874_enabled), + // // Adds support for relation-based redactions as per MSC3912. + // ("org.matrix.msc3912".to_string(), config.experimental.msc3912_enabled), + // // Whether recursively provide relations is supported. + // // TODO This is no longer needed once unstable MSC3981 does not need to be supported. + // ("org.matrix.msc3981".to_string(), true), + // // Adds support for deleting account data. + // ("org.matrix.msc3391".to_string(), config.experimental.msc3391_enabled), + // // Allows clients to inhibit profile update propagation. + // ("org.matrix.msc4069".to_string(), config.experimental.msc4069_profile_inhibit_propagation), + // // Allows clients to handle push for encrypted events. + // ("org.matrix.msc4028".to_string(), config.experimental.msc4028_push_encrypted_events), + // // MSC4108: Mechanism to allow OIDC sign in and E2EE set up via QR code - 2024 version + // ("org.matrix.msc4108".to_string(), ( + // config.experimental.msc4108_enabled + // or ( + // config.experimental.msc4108_delegation_endpoint + // is not None + // ) + // )), + // // MSC4140: Delayed events + // ("org.matrix.msc4140".to_string(), bool(config.server.max_event_delay_ms)), + // Simplified sliding sync + ("org.matrix.simplified_msc3575".to_string(), msc3575_enabled), + // // Arbitrary key-value profile fields. + // ("uk.tcpip.msc4133".to_string(), config.experimental.msc4133_enabled), + // ("uk.tcpip.msc4133.stable".to_string(), true), + // // MSC4155: Invite filtering + // ("org.matrix.msc4155".to_string(), config.experimental.msc4155_enabled), + // // MSC4306: Support for thread subscriptions + // ("org.matrix.msc4306".to_string(), config.experimental.msc4306_enabled), + // // MSC4169: Backwards-compatible redaction sending using `/send` + // ("com.beeper.msc4169".to_string(), config.experimental.msc4169_enabled), + // // MSC4354: Sticky events + // ("org.matrix.msc4354".to_string(), config.experimental.msc4354_enabled), + // // MSC4380: Invite blocking + // ("org.matrix.msc4380.stable".to_string(), true), + // // MSC4445: Sync timeline order + // ("org.matrix.msc4445.initial_sync_timeline_topological_ordering".to_string(), true), + ]), + }) +} + +/// Experimental features the server supports +pub struct UnstableFeatureMap { + msc3881: bool, + msc3575: bool, + msc4222: bool, +} + +/// Convert from [`SynapseConfig`] to the global defaults for unstable features that the +/// server supports [`UnstableFeatureMap`] +pub fn synapse_config_to_global_unstable_feature_map(config: &SynapseConfig) -> UnstableFeatureMap { + UnstableFeatureMap { + msc3881: config.experimental.msc3881_enabled, + msc3575: config.experimental.msc3575_enabled, + msc4222: config.experimental.msc4222_enabled, } } diff --git a/rust/src/storage/store.rs b/rust/src/storage/store.rs index 1b27b6036d..ca13ff2412 100644 --- a/rust/src/storage/store.rs +++ b/rust/src/storage/store.rs @@ -32,16 +32,6 @@ pub enum PerUserExperimentalFeature { MSC4222, } -impl PerUserExperimentalFeature { - pub fn is_globally_enabled(&self, config: &SynapseConfig) -> bool { - match self { - PerUserExperimentalFeature::MSC3881 => config.experimental.msc3881_enabled, - PerUserExperimentalFeature::MSC3575 => config.experimental.msc3575_enabled, - PerUserExperimentalFeature::MSC4222 => config.experimental.msc4222_enabled, - } - } -} - impl std::fmt::Display for PerUserExperimentalFeature { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!( @@ -60,20 +50,18 @@ impl std::fmt::Display for PerUserExperimentalFeature { } pub struct Store { - pub config: SynapseConfig, pub db_pool: P, } impl Store

{ - pub async fn is_feature_enabled( + /// Checks whether a given feature is enabled/disabled for this user + /// + /// If there is no entry, returns None + pub async fn is_feature_enabled_for_user( &self, user_id: &str, feature: PerUserExperimentalFeature, - ) -> Result { - if feature.is_globally_enabled(&self.config) { - return Ok(true); - } - + ) -> Result, anyhow::Error> { // It's not enabled globally, so check whether it's enabled per-user. // // Owned copies so the callback can be `'static` (it may be moved to @@ -97,10 +85,10 @@ impl Store

{ .await?; let enabled = match &rows[..] { - // If there is no row for this user, default to disabled - [] => false, + // No row for this user + [] => None, // Otherwise, we should only find a single row for this (user, feature) - [row] => row.try_get(0)?, + [row] => Some(row.try_get(0)?), _ => { panic!("Programming error") }