Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support stable identifiers for MSC4115 #3507

Closed
richvdh opened this issue Jun 4, 2024 · 4 comments
Closed

Support stable identifiers for MSC4115 #3507

richvdh opened this issue Jun 4, 2024 · 4 comments

Comments

@richvdh
Copy link
Member

richvdh commented Jun 4, 2024

matrix-org/matrix-spec-proposals#4115 has now completed FCP; servers will soon start exposing the information using stable identifiers (element-hq/synapse#17264) and we should add support for them.

@richvdh
Copy link
Member Author

richvdh commented Jun 13, 2024

I think this might actually be a no-op?

The code in question has:

            if let Ok(Some(unsigned)) = raw_event.get_field::<UnsignedWithMembership>("unsigned") {
                if let Membership::Leave = unsigned.membership {
                    // We were not a member - this is the cause of the UTD
                    return UtdCause::Membership;
                }
            }

where UnsignedWithMembership is defined as:

/// MSC4115 membership info in the unsigned area.
#[derive(Deserialize)]
struct UnsignedWithMembership {
    #[serde(alias = "io.element.msc4115.membership")]
    membership: Membership,
}

Given that UnsignedWithMembership.membership is defined with #[serde(alias)] (rather than #[serde(rename)], it should accept either the unstable name or just membership.

Would be good to test this works once element-hq/synapse#17282 lands.

@richvdh
Copy link
Member Author

richvdh commented Jun 13, 2024

(by the way, @andybalaam: accepting the proposed stable name before the MSC lands is considered bad karma: there's no guarantee that membership actually ends up meaning what you expect it to mean. In this case, we got away with it :) )

@richvdh
Copy link
Member Author

richvdh commented Jun 13, 2024

In fact, #3337 even included tests for the stable name, so I think we can consider this done.

@richvdh richvdh closed this as completed Jun 13, 2024
@andybalaam
Copy link
Contributor

(by the way, @andybalaam: accepting the proposed stable name before the MSC lands is considered bad karma: there's no guarantee that membership actually ends up meaning what you expect it to mean. In this case, we got away with it :) )

Ouch, sorry/thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants