Skip to content

Commit 9f43aa2

Browse files
hughnskegsayDMRobertson
authored
Tests for transaction ID semantics (#622)
* Test for transaction ID semantics * Update internal/client/client.go Co-authored-by: kegsay <[email protected]> * Incorporate review feedback * Refactor for legibility * Clarify that conduit doesn't pass idempotency tests * Apply suggestions from code review Co-authored-by: David Robertson <[email protected]> * Incorporate review feedback --------- Co-authored-by: kegsay <[email protected]> Co-authored-by: David Robertson <[email protected]>
1 parent bcc2bce commit 9f43aa2

File tree

2 files changed

+204
-9
lines changed

2 files changed

+204
-9
lines changed

‎internal/client/client.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,15 @@ func (c *CSAPI) SetPushRule(t *testing.T, scope string, kind string, ruleID stri
266266
func (c *CSAPI) SendEventUnsynced(t *testing.T, roomID string, e b.Event) string {
267267
t.Helper()
268268
txnID := int(atomic.AddInt64(&c.txnID, 1))
269-
paths := []string{"_matrix", "client", "v3", "rooms", roomID, "send", e.Type, strconv.Itoa(txnID)}
269+
return c.SendEventUnsyncedWithTxnID(t, roomID, e, strconv.Itoa(txnID))
270+
}
271+
272+
// SendEventUnsyncedWithTxnID sends `e` into the room with a prescribed transaction ID.
273+
// This is useful for writing tests that interrogate transaction semantics.
274+
// Returns the event ID of the sent event.
275+
func (c *CSAPI) SendEventUnsyncedWithTxnID(t *testing.T, roomID string, e b.Event, txnID string) string {
276+
t.Helper()
277+
paths := []string{"_matrix", "client", "v3", "rooms", roomID, "send", e.Type, txnID}
270278
if e.StateKey != nil {
271279
paths = []string{"_matrix", "client", "v3", "rooms", roomID, "state", e.Type, *e.StateKey}
272280
}
@@ -414,8 +422,16 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck
414422
}
415423
}
416424

425+
type LoginOpt func(map[string]interface{})
426+
427+
func WithDeviceID(deviceID string) LoginOpt {
428+
return func(loginBody map[string]interface{}) {
429+
loginBody["device_id"] = deviceID
430+
}
431+
}
432+
417433
// LoginUser will log in to a homeserver and create a new device on an existing user.
418-
func (c *CSAPI) LoginUser(t *testing.T, localpart, password string) (userID, accessToken, deviceID string) {
434+
func (c *CSAPI) LoginUser(t *testing.T, localpart, password string, opts ...LoginOpt) (userID, accessToken, deviceID string) {
419435
t.Helper()
420436
reqBody := map[string]interface{}{
421437
"identifier": map[string]interface{}{
@@ -425,6 +441,11 @@ func (c *CSAPI) LoginUser(t *testing.T, localpart, password string) (userID, acc
425441
"password": password,
426442
"type": "m.login.password",
427443
}
444+
445+
for _, opt := range opts {
446+
opt(reqBody)
447+
}
448+
428449
res := c.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "login"}, WithJSONBody(t, reqBody))
429450

430451
body, err := ioutil.ReadAll(res.Body)
@@ -438,8 +459,8 @@ func (c *CSAPI) LoginUser(t *testing.T, localpart, password string) (userID, acc
438459
return userID, accessToken, deviceID
439460
}
440461

441-
//RegisterUser will register the user with given parameters and
442-
// return user ID & access token, and fail the test on network error
462+
// RegisterUser will register the user with given parameters and
463+
// return user ID, access token and device ID. It fails the test on network error.
443464
func (c *CSAPI) RegisterUser(t *testing.T, localpart, password string) (userID, accessToken, deviceID string) {
444465
t.Helper()
445466
reqBody := map[string]interface{}{

‎tests/csapi/txnid_test.go‎

Lines changed: 179 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package csapi_tests
22

33
import (
4+
"fmt"
5+
"testing"
6+
47
"github.com/matrix-org/complement/internal/b"
58
"github.com/matrix-org/complement/internal/client"
9+
"github.com/matrix-org/complement/internal/must"
610
"github.com/matrix-org/complement/runtime"
711
"github.com/tidwall/gjson"
8-
"testing"
912
)
1013

1114
// TestTxnInEvent checks that the transaction ID is present when getting the event from the /rooms/{roomID}/event/{eventID} endpoint.
@@ -22,20 +25,191 @@ func TestTxnInEvent(t *testing.T) {
2225
// Create a room where we can send events.
2326
roomID := c.CreateRoom(t, map[string]interface{}{})
2427

28+
txnId := "abcdefg"
2529
// Let's send an event, and wait for it to appear in the timeline.
26-
eventID := c.SendEventSynced(t, roomID, b.Event{
30+
eventID := c.SendEventUnsyncedWithTxnID(t, roomID, b.Event{
2731
Type: "m.room.message",
2832
Content: map[string]interface{}{
2933
"msgtype": "m.text",
3034
"body": "first",
3135
},
32-
})
36+
}, txnId)
3337

3438
// The transaction ID should be present on the GET /rooms/{roomID}/event/{eventID} response.
3539
res := c.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "rooms", roomID, "event", eventID})
3640
body := client.ParseJSON(t, res)
3741
result := gjson.ParseBytes(body)
38-
if !result.Get("unsigned.transaction_id").Exists() {
39-
t.Fatalf("Event did not have a 'transaction_id' on the GET /rooms/%s/event/%s response", roomID, eventID)
42+
unsignedTxnId := result.Get("unsigned.transaction_id")
43+
if !unsignedTxnId.Exists() {
44+
t.Fatalf("Event did not have a 'unsigned.transaction_id' on the GET /rooms/%s/event/%s response", roomID, eventID)
45+
}
46+
47+
must.EqualStr(t, unsignedTxnId.Str, txnId, fmt.Sprintf("Event had an incorrect 'unsigned.transaction_id' on GET /rooms/%s/event/%s response", eventID, roomID))
48+
}
49+
50+
51+
func mustHaveTransactionIDForEvent(t *testing.T, roomID, eventID, expectedTxnId string) client.SyncCheckOpt {
52+
return client.SyncTimelineHas(roomID, func(r gjson.Result) bool {
53+
if r.Get("event_id").Str == eventID {
54+
unsignedTxnId := r.Get("unsigned.transaction_id")
55+
if !unsignedTxnId.Exists() {
56+
t.Fatalf("Event %s in room %s should have a 'unsigned.transaction_id', but it did not", eventID, roomID)
57+
}
58+
59+
must.EqualStr(t, unsignedTxnId.Str, expectedTxnId, fmt.Sprintf("Event %s in room %s had an incorrect 'unsigned.transaction_id'", eventID, roomID))
60+
61+
return true
62+
}
63+
64+
return false
65+
})
66+
}
67+
68+
func mustNotHaveTransactionIDForEvent(t *testing.T, roomID, eventID string) client.SyncCheckOpt {
69+
return client.SyncTimelineHas(roomID, func(r gjson.Result) bool {
70+
if r.Get("event_id").Str == eventID {
71+
unsignedTxnId := r.Get("unsigned.transaction_id")
72+
if unsignedTxnId.Exists() {
73+
t.Fatalf("Event %s in room %s should NOT have a 'unsigned.transaction_id', but it did (%s)", eventID, roomID, unsignedTxnId.Str)
74+
}
75+
76+
return true
77+
}
78+
79+
return false
80+
})
81+
}
82+
83+
// TestTxnScopeOnLocalEcho tests that transaction IDs in the sync response are scoped to the "client session", not the device
84+
func TestTxnScopeOnLocalEcho(t *testing.T) {
85+
// Conduit scope transaction IDs to the device ID, not the access token.
86+
runtime.SkipIf(t, runtime.Conduit)
87+
88+
deployment := Deploy(t, b.BlueprintCleanHS)
89+
defer deployment.Destroy(t)
90+
91+
deployment.RegisterUser(t, "hs1", "alice", "password", false)
92+
93+
// Create a first client, which allocates a device ID.
94+
c1 := deployment.Client(t, "hs1", "")
95+
c1.UserID, c1.AccessToken, c1.DeviceID = c1.LoginUser(t, "alice", "password")
96+
97+
// Create a room where we can send events.
98+
roomID := c1.CreateRoom(t, map[string]interface{}{})
99+
100+
txnId := "abdefgh"
101+
// Let's send an event, and wait for it to appear in the timeline.
102+
eventID := c1.SendEventUnsyncedWithTxnID(t, roomID, b.Event{
103+
Type: "m.room.message",
104+
Content: map[string]interface{}{
105+
"msgtype": "m.text",
106+
"body": "first",
107+
},
108+
}, txnId)
109+
110+
// When syncing, we should find the event and it should have a transaction ID on the first client.
111+
c1.MustSyncUntil(t, client.SyncReq{}, mustHaveTransactionIDForEvent(t, roomID, eventID, txnId))
112+
113+
// Create a second client, inheriting the first device ID.
114+
c2 := deployment.Client(t, "hs1", "")
115+
c2.UserID, c2.AccessToken, c2.DeviceID = c2.LoginUser(t, "alice", "password", client.WithDeviceID(c1.DeviceID))
116+
must.EqualStr(t, c1.DeviceID, c2.DeviceID, "Device ID should be the same")
117+
118+
// When syncing, we should find the event and it should *not* have a transaction ID on the second client.
119+
c2.MustSyncUntil(t, client.SyncReq{}, mustNotHaveTransactionIDForEvent(t, roomID, eventID))
120+
}
121+
122+
// TestTxnIdempotencyScopedToClientSession tests that transaction IDs are scoped to a "client session"
123+
// and behave as expected across multiple clients even if they use the same device ID
124+
func TestTxnIdempotencyScopedToClientSession(t *testing.T) {
125+
// Conduit scope transaction IDs to the device ID, not the client session.
126+
runtime.SkipIf(t, runtime.Conduit)
127+
128+
deployment := Deploy(t, b.BlueprintCleanHS)
129+
defer deployment.Destroy(t)
130+
131+
deployment.RegisterUser(t, "hs1", "alice", "password", false)
132+
133+
// Create a first client, which allocates a device ID.
134+
c1 := deployment.Client(t, "hs1", "")
135+
c1.UserID, c1.AccessToken, c1.DeviceID = c1.LoginUser(t, "alice", "password")
136+
137+
// Create a room where we can send events.
138+
roomID := c1.CreateRoom(t, map[string]interface{}{})
139+
140+
txnId := "abcdef"
141+
event := b.Event{
142+
Type: "m.room.message",
143+
Content: map[string]interface{}{
144+
"msgtype": "m.text",
145+
"body": "foo",
146+
},
40147
}
148+
// send an event with set txnId
149+
eventID1 := c1.SendEventUnsyncedWithTxnID(t, roomID, event, txnId)
150+
151+
// Create a second client, inheriting the first device ID.
152+
c2 := deployment.Client(t, "hs1", "")
153+
c2.UserID, c2.AccessToken, c2.DeviceID = c2.LoginUser(t, "alice", "password", client.WithDeviceID(c1.DeviceID))
154+
must.EqualStr(t, c1.DeviceID, c2.DeviceID, "Device ID should be the same")
155+
156+
// send another event with the same txnId via the second client
157+
eventID2 := c2.SendEventUnsyncedWithTxnID(t, roomID, event, txnId)
158+
159+
// the two events should have different event IDs as they came from different clients
160+
must.NotEqualStr(t, eventID2, eventID1, "Expected eventID1 and eventID2 to be different from two clients sharing the same device ID")
161+
}
162+
163+
// TestTxnIdempotency tests that PUT requests idempotency follows required semantics
164+
func TestTxnIdempotency(t *testing.T) {
165+
// Conduit appears to be tracking transaction IDs individually rather than combined with the request URI/room ID
166+
runtime.SkipIf(t, runtime.Conduit)
167+
168+
deployment := Deploy(t, b.BlueprintCleanHS)
169+
defer deployment.Destroy(t)
170+
171+
deployment.RegisterUser(t, "hs1", "alice", "password", false)
172+
173+
// Create a first client, which allocates a device ID.
174+
c1 := deployment.Client(t, "hs1", "")
175+
c1.UserID, c1.AccessToken, c1.DeviceID = c1.LoginUser(t, "alice", "password")
176+
177+
// Create a room where we can send events.
178+
roomID1 := c1.CreateRoom(t, map[string]interface{}{})
179+
roomID2 := c1.CreateRoom(t, map[string]interface{}{})
180+
181+
// choose a transaction ID
182+
txnId := "abc"
183+
event1 := b.Event{
184+
Type: "m.room.message",
185+
Content: map[string]interface{}{
186+
"msgtype": "m.text",
187+
"body": "first",
188+
},
189+
}
190+
event2 := b.Event{
191+
Type: "m.room.message",
192+
Content: map[string]interface{}{
193+
"msgtype": "m.text",
194+
"body": "second",
195+
},
196+
}
197+
198+
// we send the event and get an event ID back
199+
eventID1 := c1.SendEventUnsyncedWithTxnID(t, roomID1, event1, txnId)
200+
201+
// we send the identical event again and should get back the same event ID
202+
eventID2 := c1.SendEventUnsyncedWithTxnID(t, roomID1, event1, txnId)
203+
204+
must.EqualStr(t, eventID2, eventID1, "Expected eventID1 and eventID2 to be the same, but they were not")
205+
206+
// even if we change the content we should still get back the same event ID as transaction ID is the same
207+
eventID3 := c1.SendEventUnsyncedWithTxnID(t, roomID1, event2, txnId)
208+
209+
must.EqualStr(t, eventID3, eventID1, "Expected eventID3 and eventID2 to be the same even with different content, but they were not")
210+
211+
// if we change the room ID we should be able to use the same transaction ID
212+
eventID4 := c1.SendEventUnsyncedWithTxnID(t, roomID2, event1, txnId)
213+
214+
must.NotEqualStr(t, eventID4, eventID3, "Expected eventID4 and eventID3 to be different, but they were not")
41215
}

0 commit comments

Comments
 (0)