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.

Sign up for GitHub

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

Jump to bottom

Use stable MSC3030 /timestamp_to_event endpoints #559

Merged
merged 6 commits into from
Nov 28, 2022
Merged

Use stable MSC3030 /timestamp_to_event endpoints #559

merged 6 commits into from
Nov 28, 2022

Conversation

MadLittleMods
Copy link
Contributor

Use stable MSC3030 /timestamp_to_event endpoints

Synapse changes: matrix-org/synapse#14471

MadLittleMods
MadLittleMods commented Nov 16, 2022
View reviewed changes
tests/msc3030_test.go
@@ -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

@MadLittleMods MadLittleMods Nov 16, 2022 •
edited
Loading

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
Member

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:

complement/tests/room_timestamp_to_event_test.go

Lines 1 to 2 in 9b2ba1a

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

@MadLittleMods MadLittleMods mentioned this pull request Nov 16, 2022
Merged
5 tasks
MadLittleMods
MadLittleMods commented Nov 17, 2022
View reviewed changes
tests/room_timestamp_to_event_test.go
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.

MadLittleMods
MadLittleMods commented Nov 17, 2022
View reviewed changes
tests/msc2716_test.go
@@ -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

@MadLittleMods MadLittleMods Nov 17, 2022 •
edited
Loading

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.

MadLittleMods
MadLittleMods commented Nov 18, 2022
View reviewed changes
tests/msc2716_test.go
@@ -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
squahtx
squahtx approved these changes Nov 23, 2022
View reviewed changes
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
@MadLittleMods
Move MSC3030 /timestamp_to_event endpoint to stable v1 location (#1…
8f10c8b 
…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
@MadLittleMods @squahtx
Better wording
69cf9f5 
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
@MadLittleMods @H-Shay
Move MSC3030 /timestamp_to_event endpoint to stable v1 location (#1…
f68b38c 
…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
@MadLittleMods MadLittleMods mentioned this pull request Apr 5, 2023
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clokep clokep clokep left review comments

@squahtx squahtx squahtx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MadLittleMods @clokep @squahtx