PR review comments

federationapi/routing/events.go

@@ -20,10 +20,11 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/matrix-org/dendrite/clientapi/jsonerror"
-	"github.com/matrix-org/dendrite/roomserver/api"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
+
+	"github.com/matrix-org/dendrite/clientapi/jsonerror"
+	"github.com/matrix-org/dendrite/roomserver/api"
 )
 
 // GetEvent returns the requested event
@@ -38,6 +39,8 @@ func GetEvent(
 	if err != nil {
 		return *err
 	}
+	// /_matrix/federation/v1/event/{eventId} doesn't have a roomID, we use an empty string,
+	// which results in `QueryEventsByID` to first get the event and use that to determine the roomID.
 	event, err := fetchEvent(ctx, rsAPI, "", eventID)
 	if err != nil {
 		return *err

roomserver/api/api.go

@@ -54,7 +54,8 @@ type QueryBulkStateContentAPI interface {
 }
 
 type QueryEventsAPI interface {
-	// Query a list of events by event ID.
+	// QueryEventsByID queries a list of events by event ID for one room. If no room is specified, it will try to determine
+	// which room to use by querying the first events roomID.
 	QueryEventsByID(
 		ctx context.Context,
 		req *QueryEventsByIDRequest,
@@ -71,7 +72,8 @@ type SyncRoomserverAPI interface {
 	QueryBulkStateContentAPI
 	// QuerySharedUsers returns a list of users who share at least 1 room in common with the given user.
 	QuerySharedUsers(ctx context.Context, req *QuerySharedUsersRequest, res *QuerySharedUsersResponse) error
-	// Query a list of events by event ID.
+	// QueryEventsByID queries a list of events by event ID for one room. If no room is specified, it will try to determine
+	// which room to use by querying the first events roomID.
 	QueryEventsByID(
 		ctx context.Context,
 		req *QueryEventsByIDRequest,
@@ -108,7 +110,8 @@ type SyncRoomserverAPI interface {
 }
 
 type AppserviceRoomserverAPI interface {
-	// Query a list of events by event ID.
+	// QueryEventsByID queries a list of events by event ID for one room. If no room is specified, it will try to determine
+	// which room to use by querying the first events roomID.
 	QueryEventsByID(
 		ctx context.Context,
 		req *QueryEventsByIDRequest,
@@ -182,6 +185,8 @@ type FederationRoomserverAPI interface {
 	QueryMembershipsForRoom(ctx context.Context, req *QueryMembershipsForRoomRequest, res *QueryMembershipsForRoomResponse) error
 	QueryRoomVersionForRoom(ctx context.Context, req *QueryRoomVersionForRoomRequest, res *QueryRoomVersionForRoomResponse) error
 	GetRoomIDForAlias(ctx context.Context, req *GetRoomIDForAliasRequest, res *GetRoomIDForAliasResponse) error
+	// QueryEventsByID queries a list of events by event ID for one room. If no room is specified, it will try to determine
+	// which room to use by querying the first events roomID.
 	QueryEventsByID(ctx context.Context, req *QueryEventsByIDRequest, res *QueryEventsByIDResponse) error
 	// Query to get state and auth chain for a (potentially hypothetical) event.
 	// Takes lists of PrevEventIDs and AuthEventsIDs and uses them to calculate

roomserver/api/query.go

@@ -86,6 +86,8 @@ type QueryStateAfterEventsResponse struct {
 
 // QueryEventsByIDRequest is a request to QueryEventsByID
 type QueryEventsByIDRequest struct {
+	// The roomID to query events for. If this is empty, we first try to fetch the roomID from the database
+	// as this is needed for further processing/parsing events.
 	RoomID string `json:"room_id"`
 	// The event IDs to look up.
 	EventIDs []string `json:"event_ids"`

roomserver/internal/helpers/helpers_test.go

@@ -4,9 +4,10 @@ import (
 	"context"
 	"testing"
 
-	"github.com/matrix-org/dendrite/roomserver/types"
 	"github.com/stretchr/testify/assert"
 
+	"github.com/matrix-org/dendrite/roomserver/types"
+
 	"github.com/matrix-org/dendrite/setup/base"
 
 	"github.com/matrix-org/dendrite/test"
@@ -38,9 +39,9 @@ func TestIsInvitePendingWithoutNID(t *testing.T) {
 		var authNIDs []types.EventNID
 		for _, x := range room.Events() {
 
-			roomNID, roomInfo, err := db.GetOrCreateRoomNID(context.Background(), x.Unwrap())
+			roomInfo, err := db.GetOrCreateRoomInfo(context.Background(), x.Unwrap())
 			assert.NoError(t, err)
-			assert.Greater(t, roomNID, types.RoomNID(0))
+			assert.NotNil(t, roomInfo)
 
 			eventTypeNID, err := db.GetOrCreateEventTypeNID(context.Background(), x.Type())
 			assert.NoError(t, err)

roomserver/internal/perform/perform_backfill.go

@@ -605,7 +605,7 @@ func persistEvents(ctx context.Context, db storage.Database, events []*gomatrixs
 			i++
 		}
 
-		_, roomInfo, err := db.GetOrCreateRoomNID(ctx, ev.Unwrap())
+		roomInfo, err := db.GetOrCreateRoomInfo(ctx, ev.Unwrap())
 		if err != nil {
 			logrus.WithError(err).Error("failed to get or create roomNID")
 			continue

roomserver/internal/query/query.go

@@ -21,11 +21,12 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/matrix-org/dendrite/roomserver/storage/tables"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
 	"github.com/sirupsen/logrus"
 
+	"github.com/matrix-org/dendrite/roomserver/storage/tables"
+
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/roomserver/acls"
@@ -132,7 +133,8 @@ func (r *Queryer) QueryStateAfterEvents(
 	return nil
 }
 
-// QueryEventsByID implements api.RoomserverInternalAPI
+// QueryEventsByID queries a list of events by event ID for one room. If no room is specified, it will try to determine
+// which room to use by querying the first events roomID.
 func (r *Queryer) QueryEventsByID(
 	ctx context.Context,
 	request *api.QueryEventsByIDRequest,

roomserver/storage/interface.go

@@ -42,8 +42,6 @@ type Database interface {
 		ctx context.Context, e *gomatrixserverlib.Event,
 	) (missingAuth, missingPrev []string, err error)
 
-	// GetEventDatabase returns an EventDatabase to work with events only.
-	GetEventDatabase() *shared.EventDatabase
 	// Look up the state of a room at each event for a list of string event IDs.
 	// Returns an error if there is an error talking to the database.
 	// The length of []types.StateAtEvent is guaranteed to equal the length of eventIDs if no error is returned.
@@ -182,7 +180,7 @@ type Database interface {
 	GetMembershipForHistoryVisibility(
 		ctx context.Context, userNID types.EventStateKeyNID, info *types.RoomInfo, eventIDs ...string,
 	) (map[string]*gomatrixserverlib.HeaderedEvent, error)
-	GetOrCreateRoomNID(ctx context.Context, event *gomatrixserverlib.Event) (types.RoomNID, *types.RoomInfo, error)
+	GetOrCreateRoomInfo(ctx context.Context, event *gomatrixserverlib.Event) (*types.RoomInfo, error)
 	GetOrCreateEventTypeNID(ctx context.Context, eventType string) (eventTypeNID types.EventTypeNID, err error)
 	GetOrCreateEventStateKeyNID(ctx context.Context, eventStateKey *string) (types.EventStateKeyNID, error)
 	MaybeRedactEvent(
@@ -207,7 +205,7 @@ type RoomDatabase interface {
 	StateEntriesForTuples(ctx context.Context, stateBlockNIDs []types.StateBlockNID, stateKeyTuples []types.StateKeyTuple) ([]types.StateEntryList, error)
 	AddState(ctx context.Context, roomNID types.RoomNID, stateBlockNIDs []types.StateBlockNID, state []types.StateEntry) (types.StateSnapshotNID, error)
 	LatestEventIDs(ctx context.Context, roomNID types.RoomNID) ([]gomatrixserverlib.EventReference, types.StateSnapshotNID, int64, error)
-	GetOrCreateRoomNID(ctx context.Context, event *gomatrixserverlib.Event) (types.RoomNID, *types.RoomInfo, error)
+	GetOrCreateRoomInfo(ctx context.Context, event *gomatrixserverlib.Event) (*types.RoomInfo, error)
 	GetOrCreateEventTypeNID(ctx context.Context, eventType string) (eventTypeNID types.EventTypeNID, err error)
 	GetOrCreateEventStateKeyNID(ctx context.Context, eventStateKey *string) (types.EventStateKeyNID, error)
 	GetStateEvent(ctx context.Context, roomID, evType, stateKey string) (*gomatrixserverlib.HeaderedEvent, error)

roomserver/storage/shared/storage.go

@@ -55,22 +55,6 @@ type EventDatabase struct {
 	RedactionsTable     tables.Redactions
 }
 
-// GetEventDatabase returns an EventDatabase to work with events only.
-func (d *Database) GetEventDatabase() *EventDatabase {
-	db := &EventDatabase{
-		DB:                  d.DB,
-		Cache:               d.Cache,
-		Writer:              d.Writer,
-		EventsTable:         d.EventsTable,
-		EventJSONTable:      d.EventJSONTable,
-		EventTypesTable:     d.EventTypesTable,
-		EventStateKeysTable: d.EventStateKeysTable,
-		PrevEventsTable:     d.PrevEventsTable,
-		RedactionsTable:     d.RedactionsTable,
-	}
-	return db
-}
-
 func (d *Database) SupportsConcurrentRoomInputs() bool {
 	return true
 }
@@ -666,7 +650,7 @@ func (d *Database) IsEventRejected(ctx context.Context, roomNID types.RoomNID, e
 
 // GetOrCreateRoomNID gets or creates a new roomNID for the given event. Also returns a RoomInfo, which is only safe to use
 // with functions only needing a roomVersion or roomNID.
-func (d *Database) GetOrCreateRoomNID(ctx context.Context, event *gomatrixserverlib.Event) (roomNID types.RoomNID, roomInfo *types.RoomInfo, err error) {
+func (d *Database) GetOrCreateRoomInfo(ctx context.Context, event *gomatrixserverlib.Event) (roomInfo *types.RoomInfo, err error) {
 	// Get the default room version. If the client doesn't supply a room_version
 	// then we will use our configured default to create the room.
 	// https://matrix.org/docs/spec/client_server/r0.6.0#post-matrix-client-r0-createroom
@@ -675,8 +659,9 @@ func (d *Database) GetOrCreateRoomNID(ctx context.Context, event *gomatrixserver
 	// room.
 	var roomVersion gomatrixserverlib.RoomVersion
 	if roomVersion, err = extractRoomVersionFromCreateEvent(event); err != nil {
-		return 0, nil, fmt.Errorf("extractRoomVersionFromCreateEvent: %w", err)
+		return nil, fmt.Errorf("extractRoomVersionFromCreateEvent: %w", err)
 	}
+	var roomNID types.RoomNID
 	err = d.Writer.Do(d.DB, nil, func(txn *sql.Tx) error {
 		roomNID, err = d.assignRoomNID(ctx, txn, event.RoomID(), roomVersion)
 		if err != nil {
@@ -684,7 +669,7 @@ func (d *Database) GetOrCreateRoomNID(ctx context.Context, event *gomatrixserver
 		}
 		return nil
 	})
-	return roomNID, &types.RoomInfo{
+	return &types.RoomInfo{
 		RoomVersion: roomVersion,
 		RoomNID:     roomNID,
 	}, err