Remove device DB from clientapi (#1352)

* Remove device DB from clientapi

* Remove device DB from startup configuration

It's all an impl detail now in user API

9 files changed, 57 insertions, 31 deletions

diff --git a/userapi/api/api.go b/userapi/api/api.go
index 84338dbf..e6d05c33 100644
--- a/userapi/api/api.go
+++ b/userapi/api/api.go
@@ -61,7 +61,7 @@ type PerformDeviceUpdateResponse struct {
 
 type PerformDeviceDeletionRequest struct {
 	UserID string
-	// The devices to delete
+	// The devices to delete. An empty slice means delete all devices.
 	DeviceIDs []string
 }
 
@@ -192,8 +192,7 @@ type Device struct {
 	// The unique ID of the session identified by the access token.
 	// Can be used as a secure substitution in places where data needs to be
 	// associated with access tokens.
-	SessionID int64
-	// TODO: display name, last used timestamp, keys, etc
+	SessionID   int64
 	DisplayName string
 }
 
diff --git a/userapi/internal/api.go b/userapi/internal/api.go
index 05cecc1b..b97f148e 100644
--- a/userapi/internal/api.go
+++ b/userapi/internal/api.go
@@ -123,12 +123,21 @@ func (a *UserInternalAPI) PerformDeviceDeletion(ctx context.Context, req *api.Pe
 	if domain != a.ServerName {
 		return fmt.Errorf("cannot PerformDeviceDeletion of remote users: got %s want %s", domain, a.ServerName)
 	}
-	err = a.DeviceDB.RemoveDevices(ctx, local, req.DeviceIDs)
+	deletedDeviceIDs := req.DeviceIDs
+	if len(req.DeviceIDs) == 0 {
+		var devices []api.Device
+		devices, err = a.DeviceDB.RemoveAllDevices(ctx, local)
+		for _, d := range devices {
+			deletedDeviceIDs = append(deletedDeviceIDs, d.ID)
+		}
+	} else {
+		err = a.DeviceDB.RemoveDevices(ctx, local, req.DeviceIDs)
+	}
 	if err != nil {
 		return err
 	}
 	// create empty device keys and upload them to delete what was once there and trigger device list changes
-	return a.deviceListUpdate(req.UserID, req.DeviceIDs)
+	return a.deviceListUpdate(req.UserID, deletedDeviceIDs)
 }
 
 func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) error {
diff --git a/userapi/storage/devices/interface.go b/userapi/storage/devices/interface.go
index 3c9ec934..9b4261c9 100644
--- a/userapi/storage/devices/interface.go
+++ b/userapi/storage/devices/interface.go
@@ -35,5 +35,6 @@ type Database interface {
 	UpdateDevice(ctx context.Context, localpart, deviceID string, displayName *string) error
 	RemoveDevice(ctx context.Context, deviceID, localpart string) error
 	RemoveDevices(ctx context.Context, localpart string, devices []string) error
-	RemoveAllDevices(ctx context.Context, localpart string) error
+	// RemoveAllDevices deleted all devices for this user. Returns the devices deleted.
+	RemoveAllDevices(ctx context.Context, localpart string) (devices []api.Device, err error)
 }
diff --git a/userapi/storage/devices/postgres/devices_table.go b/userapi/storage/devices/postgres/devices_table.go
index 03bf7c72..282466f8 100644
--- a/userapi/storage/devices/postgres/devices_table.go
+++ b/userapi/storage/devices/postgres/devices_table.go
@@ -251,11 +251,10 @@ func (s *devicesStatements) selectDevicesByID(ctx context.Context, deviceIDs []s
 }
 
 func (s *devicesStatements) selectDevicesByLocalpart(
-	ctx context.Context, localpart string,
+	ctx context.Context, txn *sql.Tx, localpart string,
 ) ([]api.Device, error) {
 	devices := []api.Device{}
-
-	rows, err := s.selectDevicesByLocalpartStmt.QueryContext(ctx, localpart)
+	rows, err := sqlutil.TxStmt(txn, s.selectDevicesByLocalpartStmt).QueryContext(ctx, localpart)
 
 	if err != nil {
 		return devices, err
diff --git a/userapi/storage/devices/postgres/storage.go b/userapi/storage/devices/postgres/storage.go
index 4a7c7f97..04dae986 100644
--- a/userapi/storage/devices/postgres/storage.go
+++ b/userapi/storage/devices/postgres/storage.go
@@ -68,7 +68,7 @@ func (d *Database) GetDeviceByID(
 func (d *Database) GetDevicesByLocalpart(
 	ctx context.Context, localpart string,
 ) ([]api.Device, error) {
-	return d.devices.selectDevicesByLocalpart(ctx, localpart)
+	return d.devices.selectDevicesByLocalpart(ctx, nil, localpart)
 }
 
 func (d *Database) GetDevicesByID(ctx context.Context, deviceIDs []string) ([]api.Device, error) {
@@ -176,11 +176,16 @@ func (d *Database) RemoveDevices(
 // If something went wrong during the deletion, it will return the SQL error.
 func (d *Database) RemoveAllDevices(
 	ctx context.Context, localpart string,
-) error {
-	return sqlutil.WithTransaction(d.db, func(txn *sql.Tx) error {
+) (devices []api.Device, err error) {
+	err = sqlutil.WithTransaction(d.db, func(txn *sql.Tx) error {
+		devices, err = d.devices.selectDevicesByLocalpart(ctx, txn, localpart)
+		if err != nil {
+			return err
+		}
 		if err := d.devices.deleteDevicesByLocalpart(ctx, txn, localpart); err != sql.ErrNoRows {
 			return err
 		}
 		return nil
 	})
+	return
 }
diff --git a/userapi/storage/devices/sqlite3/devices_table.go b/userapi/storage/devices/sqlite3/devices_table.go
index c93e8b77..ecf43524 100644
--- a/userapi/storage/devices/sqlite3/devices_table.go
+++ b/userapi/storage/devices/sqlite3/devices_table.go
@@ -231,11 +231,10 @@ func (s *devicesStatements) selectDeviceByID(
 }
 
 func (s *devicesStatements) selectDevicesByLocalpart(
-	ctx context.Context, localpart string,
+	ctx context.Context, txn *sql.Tx, localpart string,
 ) ([]api.Device, error) {
 	devices := []api.Device{}
-
-	rows, err := s.selectDevicesByLocalpartStmt.QueryContext(ctx, localpart)
+	rows, err := sqlutil.TxStmt(txn, s.selectDevicesByLocalpartStmt).QueryContext(ctx, localpart)
 
 	if err != nil {
 		return devices, err
diff --git a/userapi/storage/devices/sqlite3/storage.go b/userapi/storage/devices/sqlite3/storage.go
index 4f426c6e..f775fb66 100644
--- a/userapi/storage/devices/sqlite3/storage.go
+++ b/userapi/storage/devices/sqlite3/storage.go
@@ -72,7 +72,7 @@ func (d *Database) GetDeviceByID(
 func (d *Database) GetDevicesByLocalpart(
 	ctx context.Context, localpart string,
 ) ([]api.Device, error) {
-	return d.devices.selectDevicesByLocalpart(ctx, localpart)
+	return d.devices.selectDevicesByLocalpart(ctx, nil, localpart)
 }
 
 func (d *Database) GetDevicesByID(ctx context.Context, deviceIDs []string) ([]api.Device, error) {
@@ -180,11 +180,16 @@ func (d *Database) RemoveDevices(
 // If something went wrong during the deletion, it will return the SQL error.
 func (d *Database) RemoveAllDevices(
 	ctx context.Context, localpart string,
-) error {
-	return d.writer.Do(d.db, nil, func(txn *sql.Tx) error {
+) (devices []api.Device, err error) {
+	err = d.writer.Do(d.db, nil, func(txn *sql.Tx) error {
+		devices, err = d.devices.selectDevicesByLocalpart(ctx, txn, localpart)
+		if err != nil {
+			return err
+		}
 		if err := d.devices.deleteDevicesByLocalpart(ctx, txn, localpart); err != sql.ErrNoRows {
 			return err
 		}
 		return nil
 	})
+	return
 }
diff --git a/userapi/userapi.go b/userapi/userapi.go
index c4ab90ba..13249142 100644
--- a/userapi/userapi.go
+++ b/userapi/userapi.go
@@ -23,7 +23,7 @@ import (
 	"github.com/matrix-org/dendrite/userapi/inthttp"
 	"github.com/matrix-org/dendrite/userapi/storage/accounts"
 	"github.com/matrix-org/dendrite/userapi/storage/devices"
-	"github.com/matrix-org/gomatrixserverlib"
+	"github.com/sirupsen/logrus"
 )
 
 // AddInternalRoutes registers HTTP handlers for the internal API. Invokes functions
@@ -34,13 +34,19 @@ func AddInternalRoutes(router *mux.Router, intAPI api.UserInternalAPI) {
 
 // NewInternalAPI returns a concerete implementation of the internal API. Callers
 // can call functions directly on the returned API or via an HTTP interface using AddInternalRoutes.
-func NewInternalAPI(accountDB accounts.Database, deviceDB devices.Database,
-	serverName gomatrixserverlib.ServerName, appServices []config.ApplicationService, keyAPI keyapi.KeyInternalAPI) api.UserInternalAPI {
+func NewInternalAPI(
+	accountDB accounts.Database, cfg *config.UserAPI, appServices []config.ApplicationService, keyAPI keyapi.KeyInternalAPI,
+) api.UserInternalAPI {
+
+	deviceDB, err := devices.NewDatabase(&cfg.DeviceDatabase, cfg.Matrix.ServerName)
+	if err != nil {
+		logrus.WithError(err).Panicf("failed to connect to device db")
+	}
 
 	return &internal.UserInternalAPI{
 		AccountDB:   accountDB,
 		DeviceDB:    deviceDB,
-		ServerName:  serverName,
+		ServerName:  cfg.Matrix.ServerName,
 		AppServices: appServices,
 		KeyAPI:      keyAPI,
 	}
diff --git a/userapi/userapi_test.go b/userapi/userapi_test.go
index 548148f2..3fc97d06 100644
--- a/userapi/userapi_test.go
+++ b/userapi/userapi_test.go
@@ -15,7 +15,6 @@ import (
 	"github.com/matrix-org/dendrite/userapi/api"
 	"github.com/matrix-org/dendrite/userapi/inthttp"
 	"github.com/matrix-org/dendrite/userapi/storage/accounts"
-	"github.com/matrix-org/dendrite/userapi/storage/devices"
 	"github.com/matrix-org/gomatrixserverlib"
 )
 
@@ -23,27 +22,31 @@ const (
 	serverName = gomatrixserverlib.ServerName("example.com")
 )
 
-func MustMakeInternalAPI(t *testing.T) (api.UserInternalAPI, accounts.Database, devices.Database) {
+func MustMakeInternalAPI(t *testing.T) (api.UserInternalAPI, accounts.Database) {
 	accountDB, err := accounts.NewDatabase(&config.DatabaseOptions{
 		ConnectionString: "file::memory:",
 	}, serverName)
 	if err != nil {
 		t.Fatalf("failed to create account DB: %s", err)
 	}
-	deviceDB, err := devices.NewDatabase(&config.DatabaseOptions{
-		ConnectionString: "file::memory:",
-	}, serverName)
-	if err != nil {
-		t.Fatalf("failed to create device DB: %s", err)
+	cfg := &config.UserAPI{
+		DeviceDatabase: config.DatabaseOptions{
+			ConnectionString:   "file::memory:",
+			MaxOpenConnections: 1,
+			MaxIdleConnections: 1,
+		},
+		Matrix: &config.Global{
+			ServerName: serverName,
+		},
 	}
 
-	return userapi.NewInternalAPI(accountDB, deviceDB, serverName, nil, nil), accountDB, deviceDB
+	return userapi.NewInternalAPI(accountDB, cfg, nil, nil), accountDB
 }
 
 func TestQueryProfile(t *testing.T) {
 	aliceAvatarURL := "mxc://example.com/alice"
 	aliceDisplayName := "Alice"
-	userAPI, accountDB, _ := MustMakeInternalAPI(t)
+	userAPI, accountDB := MustMakeInternalAPI(t)
 	_, err := accountDB.CreateAccount(context.TODO(), "alice", "foobar", "")
 	if err != nil {
 		t.Fatalf("failed to make account: %s", err)