Review comments

This commit is contained in:
Neil Alexander 2022-10-13 10:55:31 +01:00
parent a17df91449
commit 4d9acadd72
No known key found for this signature in database
GPG key ID: A02A2019A2BB0944
6 changed files with 81 additions and 120 deletions

View file

@ -45,7 +45,7 @@ func Setup(
lazyLoadCache caching.LazyLoadCache,
fts *fulltext.Search,
) {
v1mux := csMux.PathPrefix("/v1/").Subrouter()
v1unstablemux := csMux.PathPrefix("/{apiversion:(?:v1|unstable)}/").Subrouter()
v3mux := csMux.PathPrefix("/{apiversion:(?:r0|v3)}/").Subrouter()
// TODO: Add AS support for all handlers below.
@ -111,7 +111,7 @@ func Setup(
}),
).Methods(http.MethodGet, http.MethodOptions)
v1mux.Handle("/rooms/{roomId}/relations/{eventId}",
v1unstablemux.Handle("/rooms/{roomId}/relations/{eventId}",
httputil.MakeAuthAPI(gomatrixserverlib.Join, userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
vars, err := httputil.URLDecodeMapValues(mux.Vars(req))
if err != nil {
@ -125,7 +125,7 @@ func Setup(
}),
).Methods(http.MethodGet, http.MethodOptions)
v1mux.Handle("/rooms/{roomId}/relations/{eventId}/{relType}",
v1unstablemux.Handle("/rooms/{roomId}/relations/{eventId}/{relType}",
httputil.MakeAuthAPI(gomatrixserverlib.Join, userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
vars, err := httputil.URLDecodeMapValues(mux.Vars(req))
if err != nil {
@ -139,7 +139,7 @@ func Setup(
}),
).Methods(http.MethodGet, http.MethodOptions)
v1mux.Handle("/rooms/{roomId}/relations/{eventId}/{relType}/{eventType}",
v1unstablemux.Handle("/rooms/{roomId}/relations/{eventId}/{relType}/{eventType}",
httputil.MakeAuthAPI(gomatrixserverlib.Join, userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
vars, err := httputil.URLDecodeMapValues(mux.Vars(req))
if err != nil {

View file

@ -1,5 +1,4 @@
// Copyright 2017-2018 New Vector Ltd
// Copyright 2019-2020 The Matrix.org Foundation C.I.C.
// Copyright 2022 The Matrix.org Foundation C.I.C.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@ -42,34 +41,33 @@ const insertRelationSQL = "" +
"INSERT INTO syncapi_relations (" +
" room_id, event_id, child_event_id, rel_type" +
") VALUES ($1, $2, $3, $4) " +
" ON CONFLICT ON CONSTRAINT syncapi_relations_unique DO UPDATE SET event_id=EXCLUDED.event_id" +
" RETURNING id"
" ON CONFLICT DO NOTHING"
const deleteRelationSQL = "" +
"DELETE FROM syncapi_relations WHERE room_id = $1 AND child_event_id = $2"
const selectRelationsInRangeAscSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND id > $3 AND id <= $4" +
" ORDER BY id ASC LIMIT $5"
const selectRelationsInRangeDescSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND id >= $3 AND id < $4" +
" ORDER BY id DESC LIMIT $5"
const selectRelationsByTypeInRangeAscSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id > $4 AND id <= $5" +
" ORDER BY id ASC LIMIT $6"
const selectRelationsByTypeInRangeDescSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id >= $4 AND id < $5" +
" ORDER BY id DESC LIMIT $6"
const selectMaxRelationIDSQL = "" +
"SELECT MAX(id) FROM syncapi_relations"
"SELECT COALESCE(MAX(id), 0) FROM syncapi_relations"
type relationsStatements struct {
insertRelationStmt *sql.Stmt
@ -87,36 +85,23 @@ func NewPostgresRelationsTable(db *sql.DB) (tables.Relations, error) {
if err != nil {
return nil, err
}
if s.insertRelationStmt, err = db.Prepare(insertRelationSQL); err != nil {
return nil, err
}
if s.selectRelationsInRangeAscStmt, err = db.Prepare(selectRelationsInRangeAscSQL); err != nil {
return nil, err
}
if s.selectRelationsInRangeDescStmt, err = db.Prepare(selectRelationsInRangeDescSQL); err != nil {
return nil, err
}
if s.selectRelationsByTypeInRangeAscStmt, err = db.Prepare(selectRelationsByTypeInRangeAscSQL); err != nil {
return nil, err
}
if s.selectRelationsByTypeInRangeDescStmt, err = db.Prepare(selectRelationsByTypeInRangeDescSQL); err != nil {
return nil, err
}
if s.deleteRelationStmt, err = db.Prepare(deleteRelationSQL); err != nil {
return nil, err
}
if s.selectMaxRelationIDStmt, err = db.Prepare(selectMaxRelationIDSQL); err != nil {
return nil, err
}
return s, nil
return s, sqlutil.StatementList{
{&s.insertRelationStmt, insertRelationSQL},
{&s.selectRelationsInRangeAscStmt, selectRelationsInRangeAscSQL},
{&s.selectRelationsInRangeDescStmt, selectRelationsInRangeDescSQL},
{&s.selectRelationsByTypeInRangeAscStmt, selectRelationsByTypeInRangeAscSQL},
{&s.selectRelationsByTypeInRangeDescStmt, selectRelationsByTypeInRangeDescSQL},
{&s.deleteRelationStmt, deleteRelationSQL},
{&s.selectMaxRelationIDStmt, selectMaxRelationIDSQL},
}.Prepare(db)
}
func (s *relationsStatements) InsertRelation(
ctx context.Context, txn *sql.Tx, roomID, eventID, childEventID, relType string,
) (streamPos types.StreamPosition, err error) {
err = sqlutil.TxStmt(txn, s.insertRelationStmt).QueryRowContext(
) (err error) {
_, err = sqlutil.TxStmt(txn, s.insertRelationStmt).ExecContext(
ctx, roomID, eventID, childEventID, relType,
).Scan(&streamPos)
)
return
}
@ -127,9 +112,6 @@ func (s *relationsStatements) DeleteRelation(
_, err := stmt.ExecContext(
ctx, roomID, childEventID,
)
if err == sql.ErrNoRows {
return nil
}
return err
}
@ -162,20 +144,19 @@ func (s *relationsStatements) SelectRelationsInRange(
}
defer internal.CloseAndLogIfError(ctx, rows, "selectRelationsInRange: rows.close() failed")
result := map[string][]types.RelationEntry{}
var (
id types.StreamPosition
childEventID string
relationType string
)
for rows.Next() {
var (
id types.StreamPosition
roomID string
childEventID string
relType string
)
if err = rows.Scan(&id, &roomID, &childEventID, &relType); err != nil {
if err = rows.Scan(&id, &childEventID, &relationType); err != nil {
return nil, lastPos, err
}
if id > lastPos {
lastPos = id
}
result[relType] = append(result[relType], types.RelationEntry{
result[relationType] = append(result[relationType], types.RelationEntry{
Position: id,
EventID: childEventID,
})
@ -189,11 +170,7 @@ func (s *relationsStatements) SelectRelationsInRange(
func (s *relationsStatements) SelectMaxRelationID(
ctx context.Context, txn *sql.Tx,
) (id int64, err error) {
var nullableID sql.NullInt64
stmt := sqlutil.TxStmt(txn, s.selectMaxRelationIDStmt)
err = stmt.QueryRowContext(ctx).Scan(&nullableID)
if nullableID.Valid {
id = nullableID.Int64
}
err = stmt.QueryRowContext(ctx).Scan(&id)
return
}

View file

@ -604,11 +604,10 @@ func (d *Database) UpdateRelations(ctx context.Context, event *gomatrixserverlib
return nil
default:
return d.Writer.Do(d.DB, nil, func(txn *sql.Tx) error {
_, err := d.Relations.InsertRelation(
return d.Relations.InsertRelation(
ctx, txn, event.RoomID(), content.Relations.EventID,
event.EventID(), content.Relations.RelationType,
)
return err
})
}
}

View file

@ -605,18 +605,23 @@ func (d *DatabaseTransaction) RelationsFor(ctx context.Context, roomID, eventID,
To: to,
Backwards: backwards,
}
if r.Backwards {
if r.From == 0 {
if r.From, err = d.MaxStreamPositionForRelations(ctx); err != nil {
return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err)
}
r.From++
if r.Backwards && r.From == 0 {
// If we're working backwards (dir=b) and there's no ?from= specified then
// we will automatically want to work backwards from the current position,
// so find out what that is.
if r.From, err = d.MaxStreamPositionForRelations(ctx); err != nil {
return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err)
}
} else {
if r.To == 0 {
if r.To, err = d.MaxStreamPositionForRelations(ctx); err != nil {
return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err)
}
// The result normally isn't inclusive of the event *at* the ?from=
// position, so add 1 here so that we include the most recent relation.
r.From++
} else if !r.Backwards && r.To == 0 {
// If we're working forwards (dir=f) and there's no ?to= specified then
// we will automatically want to work forwards towards the current position,
// so find out what that is.
if r.To, err = d.MaxStreamPositionForRelations(ctx); err != nil {
return nil, "", "", fmt.Errorf("d.MaxStreamPositionForRelations: %w", err)
}
}
@ -645,7 +650,9 @@ func (d *DatabaseTransaction) RelationsFor(ctx context.Context, roomID, eventID,
}
// Otherwise, let's try and work out what sensible prev_batch and next_batch values
// could be.
// could be. We've requested an extra event by adding one to the limit already so
// that we can determine whether or not to provide a "next_batch", so trim off that
// event off the end if needs be.
if len(entries) > limit {
entries = entries[:len(entries)-1]
nextBatch = fmt.Sprintf("%d", entries[len(entries)-1].Position)
@ -653,7 +660,7 @@ func (d *DatabaseTransaction) RelationsFor(ctx context.Context, roomID, eventID,
// TODO: set prevBatch? doesn't seem to affect the tests...
// Extract all of the event IDs from the relation entries so that we can pull the
// events out of the database.
// events out of the database. Then go and fetch the events.
for _, entry := range entries {
eventIDs = append(eventIDs, entry.EventID)
}

View file

@ -1,5 +1,4 @@
// Copyright 2017-2018 New Vector Ltd
// Copyright 2019-2020 The Matrix.org Foundation C.I.C.
// Copyright 2022 The Matrix.org Foundation C.I.C.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@ -40,34 +39,33 @@ const insertRelationSQL = "" +
"INSERT INTO syncapi_relations (" +
" id, room_id, event_id, child_event_id, rel_type" +
") VALUES ($1, $2, $3, $4, $5) " +
" ON CONFLICT (room_id, event_id, child_event_id, rel_type) DO UPDATE SET event_id=EXCLUDED.event_id" +
" RETURNING id"
" ON CONFLICT DO NOTHING"
const deleteRelationSQL = "" +
"DELETE FROM syncapi_relations WHERE room_id = $1 AND child_event_id = $2"
const selectRelationsInRangeAscSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND id > $3 AND id <= $4" +
" ORDER BY id ASC LIMIT $5"
const selectRelationsInRangeDescSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND id >= $3 AND id < $4" +
" ORDER BY id DESC LIMIT $5"
const selectRelationsByTypeInRangeAscSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id > $4 AND id <= $5" +
" ORDER BY id ASC LIMIT $6"
const selectRelationsByTypeInRangeDescSQL = "" +
"SELECT id, room_id, child_event_id, rel_type FROM syncapi_relations" +
"SELECT id, child_event_id, rel_type FROM syncapi_relations" +
" WHERE room_id = $1 AND event_id = $2 AND rel_type = $3 AND id >= $4 AND id < $5" +
" ORDER BY id DESC LIMIT $6"
const selectMaxRelationIDSQL = "" +
"SELECT MAX(id) FROM syncapi_relations"
"SELECT COALESCE(MAX(id), 0) FROM syncapi_relations"
type relationsStatements struct {
streamIDStatements *StreamIDStatements
@ -88,39 +86,27 @@ func NewSqliteRelationsTable(db *sql.DB, streamID *StreamIDStatements) (tables.R
if err != nil {
return nil, err
}
if s.insertRelationStmt, err = db.Prepare(insertRelationSQL); err != nil {
return nil, err
}
if s.selectRelationsInRangeAscStmt, err = db.Prepare(selectRelationsInRangeAscSQL); err != nil {
return nil, err
}
if s.selectRelationsInRangeDescStmt, err = db.Prepare(selectRelationsInRangeDescSQL); err != nil {
return nil, err
}
if s.selectRelationsByTypeInRangeAscStmt, err = db.Prepare(selectRelationsByTypeInRangeAscSQL); err != nil {
return nil, err
}
if s.selectRelationsByTypeInRangeDescStmt, err = db.Prepare(selectRelationsByTypeInRangeDescSQL); err != nil {
return nil, err
}
if s.deleteRelationStmt, err = db.Prepare(deleteRelationSQL); err != nil {
return nil, err
}
if s.selectMaxRelationIDStmt, err = db.Prepare(selectMaxRelationIDSQL); err != nil {
return nil, err
}
return s, nil
return s, sqlutil.StatementList{
{&s.insertRelationStmt, insertRelationSQL},
{&s.selectRelationsInRangeAscStmt, selectRelationsInRangeAscSQL},
{&s.selectRelationsInRangeDescStmt, selectRelationsInRangeDescSQL},
{&s.selectRelationsByTypeInRangeAscStmt, selectRelationsByTypeInRangeAscSQL},
{&s.selectRelationsByTypeInRangeDescStmt, selectRelationsByTypeInRangeDescSQL},
{&s.deleteRelationStmt, deleteRelationSQL},
{&s.selectMaxRelationIDStmt, selectMaxRelationIDSQL},
}.Prepare(db)
}
func (s *relationsStatements) InsertRelation(
ctx context.Context, txn *sql.Tx, roomID, eventID, childEventID, relType string,
) (streamPos types.StreamPosition, err error) {
) (err error) {
var streamPos types.StreamPosition
if streamPos, err = s.streamIDStatements.nextRelationID(ctx, txn); err != nil {
return
}
err = sqlutil.TxStmt(txn, s.insertRelationStmt).QueryRowContext(
_, err = sqlutil.TxStmt(txn, s.insertRelationStmt).ExecContext(
ctx, streamPos, roomID, eventID, childEventID, relType,
).Scan(&streamPos)
)
return
}
@ -131,9 +117,6 @@ func (s *relationsStatements) DeleteRelation(
_, err := stmt.ExecContext(
ctx, roomID, childEventID,
)
if err == sql.ErrNoRows {
return nil
}
return err
}
@ -166,20 +149,19 @@ func (s *relationsStatements) SelectRelationsInRange(
}
defer internal.CloseAndLogIfError(ctx, rows, "selectRelationsInRange: rows.close() failed")
result := map[string][]types.RelationEntry{}
var (
id types.StreamPosition
childEventID string
relationType string
)
for rows.Next() {
var (
id types.StreamPosition
roomID string
childEventID string
relType string
)
if err = rows.Scan(&id, &roomID, &childEventID, &relType); err != nil {
if err = rows.Scan(&id, &childEventID, &relationType); err != nil {
return nil, lastPos, err
}
if id > lastPos {
lastPos = id
}
result[relType] = append(result[relType], types.RelationEntry{
result[relationType] = append(result[relationType], types.RelationEntry{
Position: id,
EventID: childEventID,
})
@ -193,11 +175,7 @@ func (s *relationsStatements) SelectRelationsInRange(
func (s *relationsStatements) SelectMaxRelationID(
ctx context.Context, txn *sql.Tx,
) (id int64, err error) {
var nullableID sql.NullInt64
stmt := sqlutil.TxStmt(txn, s.selectMaxRelationIDStmt)
err = stmt.QueryRowContext(ctx).Scan(&nullableID)
if nullableID.Valid {
id = nullableID.Int64
}
err = stmt.QueryRowContext(ctx).Scan(&id)
return
}

View file

@ -208,7 +208,7 @@ type Presence interface {
}
type Relations interface {
InsertRelation(ctx context.Context, txn *sql.Tx, roomID, eventID, childEventID, relType string) (streamPos types.StreamPosition, err error)
InsertRelation(ctx context.Context, txn *sql.Tx, roomID, eventID, childEventID, relType string) (err error)
DeleteRelation(ctx context.Context, txn *sql.Tx, roomID, childEventID string) error
SelectRelationsInRange(ctx context.Context, txn *sql.Tx, roomID, eventID, relType string, r types.Range, limit int) (map[string][]types.RelationEntry, types.StreamPosition, error)
SelectMaxRelationID(ctx context.Context, txn *sql.Tx) (id int64, err error)