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

Use stable MSC3030 /timestamp_to_event endpoints #559

Merged

Conversation

MadLittleMods
Copy link
Contributor

Use stable MSC3030 /timestamp_to_event endpoints

Synapse changes: matrix-org/synapse#14471

@@ -118,7 +118,7 @@ func TestJumpToDateEndpoint(t *testing.T) {
// Make the `/timestamp_to_event` request from Bob's perspective (non room member)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that MSC3030 is merged and stable, do we also need to remove the +build msc3030 build tags from the top here and change these tests to Synapse only instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I've been doing. 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Copied from https://github.com/matrix-org/complement/pull/225/files

Added the Dendrite blacklist tags:

//go:build !dendrite_blacklist
// +build !dendrite_blacklist

Comment on lines +315 to +316
} else if timestampToEventRes.StatusCode != 404 || (timestampToEventRes.StatusCode == 404 && expectedEventId != "") {
t.Fatalf("mustCheckEventisReturnedForTime: /timestamp_to_event request failed with status=%d body=%s", timestampToEventRes.StatusCode, string(timestampToEventResBody))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a small fix to make easier to debug when the endpoint fails.

We now also throw when the endpoint throws a 404 but we were expecting an event.

@@ -1164,42 +1164,6 @@ func includes(needle string, haystack []string) bool {
return false
}

func fetchUntilMessagesResponseHas(t *testing.T, c *client.CSAPI, roomID string, check func(gjson.Result) bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this to tests/room_timestamp_to_event_test.go because we can't reference it from the msc2716 build tag when it's not included.

We don't build with msc2716 in the workers Complement environment (related matrix-org/synapse#14074). So now we can instead reference fetchUntilMessagesResponseHas from the file that isn't put behind a build tag.

@@ -1126,16 +1126,6 @@ func TestImportHistoricalMessages(t *testing.T) {
})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the Dendrite CI job is failing on main as well

@MadLittleMods MadLittleMods marked this pull request as ready for review November 18, 2022 00:03
@MadLittleMods MadLittleMods requested review from a team as code owners November 18, 2022 00:03
Copy link
Contributor

@squahtx squahtx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

tests/room_timestamp_to_event_test.go Outdated Show resolved Hide resolved
MadLittleMods added a commit to matrix-org/synapse that referenced this pull request Nov 28, 2022
…4471)

Fix #14390

 - Client API: `/_matrix/client/unstable/org.matrix.msc3030/rooms/<roomID>/timestamp_to_event?ts=<timestamp>&dir=<direction>` -> `/_matrix/client/v1/rooms/<roomID>/timestamp_to_event?ts=<timestamp>&dir=<direction>`
 - Federation API: `/_matrix/federation/unstable/org.matrix.msc3030/timestamp_to_event/<roomID>?ts=<timestamp>&dir=<direction>` -> `/_matrix/federation/v1/timestamp_to_event/<roomID>?ts=<timestamp>&dir=<direction>`

Complement test changes: matrix-org/complement#559
Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>
@MadLittleMods MadLittleMods merged commit 756d388 into main Nov 28, 2022
@MadLittleMods MadLittleMods deleted the madlittlemods/14390-stable-timestamp-to-event-endpoint branch November 28, 2022 22:59
H-Shay pushed a commit to matrix-org/synapse that referenced this pull request Dec 13, 2022
…4471)

Fix #14390

 - Client API: `/_matrix/client/unstable/org.matrix.msc3030/rooms/<roomID>/timestamp_to_event?ts=<timestamp>&dir=<direction>` -> `/_matrix/client/v1/rooms/<roomID>/timestamp_to_event?ts=<timestamp>&dir=<direction>`
 - Federation API: `/_matrix/federation/unstable/org.matrix.msc3030/timestamp_to_event/<roomID>?ts=<timestamp>&dir=<direction>` -> `/_matrix/federation/v1/timestamp_to_event/<roomID>?ts=<timestamp>&dir=<direction>`

Complement test changes: matrix-org/complement#559
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

Successfully merging this pull request may close these issues.

None yet

3 participants