Lock /createRoom calls (matrix-org#800)

* Lock /createRoom calls

To reduce the chance of multiple rooms being made in the same
millisecond which makes v12 rooms unhappy as it causes the same
room ID to be generated.

* Update client/client.go

Co-authored-by: Eric Eastwood <erice@element.io>

---------

Co-authored-by: Eric Eastwood <erice@element.io>

Author: kegsay

client/client.go

@@ -15,6 +15,7 @@ import (
 	"net/url"
 	"strconv"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"time"
 
@@ -48,6 +49,19 @@ type retryUntilParams struct {
 // See functions starting with `With...` in this package for more info.
 type RequestOpt func(req *http.Request)
 
+type CSAPIOpts struct {
+	UserID      string
+	AccessToken string
+	DeviceID    string
+	Password    string // if provided
+	BaseURL     string
+	Client      *http.Client
+	// how long are we willing to wait for MustSyncUntil.... calls
+	SyncUntilTimeout time.Duration
+	// True to enable verbose logging
+	Debug bool
+}
+
 type CSAPI struct {
 	UserID      string
 	AccessToken string
@@ -60,7 +74,22 @@ type CSAPI struct {
 	// True to enable verbose logging
 	Debug bool
 
-	txnID int64
+	txnID           int64
+	createRoomMutex *sync.Mutex
+}
+
+func NewCSAPI(opts CSAPIOpts) *CSAPI {
+	return &CSAPI{
+		UserID:           opts.UserID,
+		AccessToken:      opts.AccessToken,
+		DeviceID:         opts.DeviceID,
+		Password:         opts.Password,
+		BaseURL:          opts.BaseURL,
+		Client:           opts.Client,
+		SyncUntilTimeout: opts.SyncUntilTimeout,
+		Debug:            opts.Debug,
+		createRoomMutex:  &sync.Mutex{},
+	}
 }
 
 // CreateMedia creates an MXC URI for asynchronous media uploads.
@@ -137,6 +166,10 @@ func (c *CSAPI) MustCreateRoom(t ct.TestLike, reqBody map[string]interface{}) st
 // CreateRoom creates a room with an optional HTTP request body.
 func (c *CSAPI) CreateRoom(t ct.TestLike, body map[string]interface{}) *http.Response {
 	t.Helper()
+	// Ensure we don't call /createRoom from the same user in parallel, else we might try to make
+	// 2 rooms in the same millisecond (same `origin_server_ts`), causing v12 rooms to get the same room ID thus failing the test.
+	c.createRoomMutex.Lock()
+	defer c.createRoomMutex.Unlock()
 	return c.Do(t, "POST", []string{"_matrix", "client", "v3", "createRoom"}, WithJSONBody(t, body))
 }
 

internal/docker/deployment.go

@@ -105,13 +105,13 @@ func (d *Deployment) Register(t ct.TestLike, hsName string, opts helpers.Registr
 		ct.Fatalf(t, "Deployment.Register - HS name '%s' not found", hsName)
 		return nil
 	}
-	client := &client.CSAPI{
+	client := client.NewCSAPI(client.CSAPIOpts{
 		BaseURL:          dep.BaseURL,
 		Client:           client.NewLoggedClient(t, hsName, nil),
 		SyncUntilTimeout: 5 * time.Second,
 		Debug:            d.Deployer.debugLogging,
 		Password:         opts.Password,
-	}
+	})
 	// Appending a slice is not thread-safe. Protect the write with a mutex.
 	dep.CSAPIClientsMutex.Lock()
 	dep.CSAPIClients = append(dep.CSAPIClients, client)
@@ -155,13 +155,13 @@ func (d *Deployment) Login(t ct.TestLike, hsName string, existing *client.CSAPI,
 	if err != nil {
 		ct.Fatalf(t, "Deployment.Login: existing CSAPI client has invalid user ID '%s', cannot login as this user: %s", existing.UserID, err)
 	}
-	c := &client.CSAPI{
+	c := client.NewCSAPI(client.CSAPIOpts{
 		BaseURL:          dep.BaseURL,
 		Client:           client.NewLoggedClient(t, hsName, nil),
 		SyncUntilTimeout: 5 * time.Second,
 		Debug:            d.Deployer.debugLogging,
 		Password:         existing.Password,
-	}
+	})
 	if opts.Password != "" {
 		c.Password = opts.Password
 	}
@@ -197,12 +197,12 @@ func (d *Deployment) UnauthenticatedClient(t ct.TestLike, hsName string) *client
 		ct.Fatalf(t, "Deployment.Client - HS name '%s' not found", hsName)
 		return nil
 	}
-	client := &client.CSAPI{
+	client := client.NewCSAPI(client.CSAPIOpts{
 		BaseURL:          dep.BaseURL,
 		Client:           client.NewLoggedClient(t, hsName, nil),
 		SyncUntilTimeout: 5 * time.Second,
 		Debug:            d.Deployer.debugLogging,
-	}
+	})
 	// Appending a slice is not thread-safe. Protect the write with a mutex.
 	dep.CSAPIClientsMutex.Lock()
 	dep.CSAPIClients = append(dep.CSAPIClients, client)
@@ -230,15 +230,15 @@ func (d *Deployment) AppServiceUser(t ct.TestLike, hsName, appServiceUserID stri
 	if deviceID == "" && appServiceUserID != "" {
 		t.Logf("WARNING: Deployment.Client - HS name '%s' - user ID '%s' - deviceID not found", hsName, appServiceUserID)
 	}
-	client := &client.CSAPI{
+	client := client.NewCSAPI(client.CSAPIOpts{
 		UserID:           appServiceUserID,
 		AccessToken:      token,
 		DeviceID:         deviceID,
 		BaseURL:          dep.BaseURL,
 		Client:           client.NewLoggedClient(t, hsName, nil),
 		SyncUntilTimeout: 5 * time.Second,
 		Debug:            d.Deployer.debugLogging,
-	}
+	})
 	// Appending a slice is not thread-safe. Protect the write with a mutex.
 	dep.CSAPIClientsMutex.Lock()
 	dep.CSAPIClients = append(dep.CSAPIClients, client)