Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes #560

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/_apimeta/auth.go → api/apimeta/auth.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package _apimeta
package apimeta

import (
"net/http"
Expand Down
22 changes: 11 additions & 11 deletions api/_auth_cache/auth_cache.go → api/auth_cache/auth_cache.go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert 6fc49c2 - the package naming is deliberate

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package _auth_cache
package auth_cache

import (
"errors"
Expand Down Expand Up @@ -103,25 +103,25 @@ func GetUserId(ctx rcontext.RequestContext, accessToken string, appserviceUserId
return checkTokenWithHomeserver(ctx, accessToken, appserviceUserId, true)
}

for _, r := range ctx.Config.AccessTokens.Appservices {
if r.AppserviceToken != accessToken {
for _, appSrv := range ctx.Config.AccessTokens.Appservices {
if appSrv.AppserviceToken != accessToken {
continue
}

if r.SenderUserId != "" && (r.SenderUserId == appserviceUserId || appserviceUserId == "") {
ctx.Log.Debugf("Access token belongs to appservice (sender user ID): %s", r.Id)
cacheToken(ctx, accessToken, appserviceUserId, r.SenderUserId, nil)
return r.SenderUserId, nil
if appSrv.SenderUserId != "" && (appSrv.SenderUserId == appserviceUserId || appserviceUserId == "") {
ctx.Log.Debugf("Access token belongs to appservice (sender user ID): %s", appSrv.Id)
cacheToken(ctx, accessToken, appserviceUserId, appSrv.SenderUserId, nil)
return appSrv.SenderUserId, nil
}

for _, n := range r.UserNamespaces {
for _, n := range appSrv.UserNamespaces {
regex, ok := regexCache[n.Regex]
if !ok {
regex = regexp.MustCompile(n.Regex)
regexCache[n.Regex] = regex
}
if regex.MatchString(appserviceUserId) {
ctx.Log.Debugf("Access token belongs to appservice: %s", r.Id)
ctx.Log.Debugf("Access token belongs to appservice: %s", appSrv.Id)
cacheToken(ctx, accessToken, appserviceUserId, appserviceUserId, nil)
return appserviceUserId, nil
}
Expand All @@ -133,13 +133,13 @@ func GetUserId(ctx rcontext.RequestContext, accessToken string, appserviceUserId
}

func cacheToken(ctx rcontext.RequestContext, accessToken string, appserviceUserId string, userId string, err error) {
v := cachedToken{
token := cachedToken{
userId: userId,
err: err,
}
t := time.Duration(ctx.Config.AccessTokens.MaxCacheTimeSeconds) * time.Second
rwLock.Lock()
tokenCache.Set(cacheKey(accessToken, appserviceUserId), v, t)
tokenCache.Set(cacheKey(accessToken, appserviceUserId), token, t)
rwLock.Unlock()
}

Expand Down
24 changes: 12 additions & 12 deletions api/branched_route.go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert a6b6598 - the package naming is deliberate

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"net/http"
"strings"

"github.com/t2bot/matrix-media-repo/api/_routers"
"github.com/t2bot/matrix-media-repo/api/routers"
)

type branch struct {
Expand All @@ -19,29 +19,29 @@ type splitBranch struct {

func branchedRoute(branches []branch) http.Handler {
sbranches := make([]splitBranch, len(branches))
for i, b := range branches {
for i, branch := range branches {
sbranches[i] = splitBranch{
segments: strings.Split(b.string, "/"),
handler: b.Handler,
segments: strings.Split(branch.string, "/"),
handler: branch.Handler,
}
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
catchAll := _routers.GetParam("branch", r)
catchAll := routers.GetParam("branch", r)
if catchAll[0] == '/' {
catchAll = catchAll[1:]
}
params := strings.Split(catchAll, "/")
for _, b := range sbranches {
if b.segments[0][0] == ':' || b.segments[0] == params[0] {
if len(b.segments) != len(params) {
for _, branch := range sbranches {
if branch.segments[0][0] == ':' || branch.segments[0] == params[0] {
if len(branch.segments) != len(params) {
continue
}
for i, s := range b.segments {
if s[0] == ':' {
r = _routers.ForceSetParam(s[1:], params[i], r)
for i, segment := range branch.segments {
if segment[0] == ':' {
r = routers.ForceSetParam(segment[1:], params[i], r)
}
}
b.handler.ServeHTTP(w, r)
branch.handler.ServeHTTP(w, r)
return
}
}
Expand Down
68 changes: 35 additions & 33 deletions api/custom/datastores.go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert 87964bb - this is not a change I'm looking to support at the moment, for reasons discussed over chat.

Primarily, time as a package allows mistakes in precision to be made far too easily. We work with a single precision of milliseconds throughout the codebase deliberately - please don't change that.

Original file line number Diff line number Diff line change
@@ -1,58 +1,60 @@
package custom

import (
"errors"
"fmt"
"net/http"
"strconv"
"time"

"github.com/getsentry/sentry-go"
"github.com/t2bot/matrix-media-repo/api/_apimeta"
"github.com/t2bot/matrix-media-repo/api/_responses"
"github.com/t2bot/matrix-media-repo/api/_routers"
"github.com/t2bot/matrix-media-repo/api/apimeta"
"github.com/t2bot/matrix-media-repo/api/responses"
"github.com/t2bot/matrix-media-repo/api/routers"
"github.com/t2bot/matrix-media-repo/common/config"
"github.com/t2bot/matrix-media-repo/datastores"
"github.com/t2bot/matrix-media-repo/tasks"

"net/http"
"strconv"

"github.com/sirupsen/logrus"
"github.com/t2bot/matrix-media-repo/common/rcontext"
"github.com/t2bot/matrix-media-repo/util"
)

type DatastoreMigration struct {
*datastores.SizeEstimate
TaskID int `json:"task_id"`
}

func GetDatastores(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} {
func GetDatastores(r *http.Request, rctx rcontext.RequestContext, user apimeta.UserInfo) interface{} {
response := make(map[string]interface{})
for _, ds := range config.UniqueDatastores() {
uri, err := datastores.GetUri(ds)
for _, store := range config.UniqueDatastores() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's ds for datastore. Please revert.

uri, err := datastores.GetUri(store)
if err != nil {
sentry.CaptureException(err)
rctx.Log.Error("Error getting datastore URI: ", err)
return _responses.InternalServerError("unexpected error getting datastore information")
return responses.InternalServerError(errors.New("unexpected error getting datastore information"))
}
dsMap := make(map[string]interface{})
dsMap["type"] = ds.Type
dsMap["uri"] = uri
response[ds.Id] = dsMap
dataStoreMap := make(map[string]interface{})
dataStoreMap["type"] = store.Type
dataStoreMap["uri"] = uri
response[store.Id] = dataStoreMap
}

return &_responses.DoNotCacheResponse{Payload: response}
return &responses.DoNotCacheResponse{Payload: response}
}

func MigrateBetweenDatastores(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} {
func MigrateBetweenDatastores(r *http.Request, rctx rcontext.RequestContext, user apimeta.UserInfo) interface{} {
beforeTsStr := r.URL.Query().Get("before_ts")
beforeTs := util.NowMillis()
beforeTs := time.Now().UnixMilli()
var err error
if beforeTsStr != "" {
beforeTs, err = strconv.ParseInt(beforeTsStr, 10, 64)
if err != nil {
return _responses.BadRequest("Error parsing before_ts: " + err.Error())
return responses.BadRequest(fmt.Errorf("Error parsing before_ts: %w", err))
}
}

sourceDsId := _routers.GetParam("sourceDsId", r)
targetDsId := _routers.GetParam("targetDsId", r)
sourceDsId := routers.GetParam("sourceDsId", r)
targetDsId := routers.GetParam("targetDsId", r)

rctx = rctx.LogWithFields(logrus.Fields{
"beforeTs": beforeTs,
Expand All @@ -61,50 +63,50 @@ func MigrateBetweenDatastores(r *http.Request, rctx rcontext.RequestContext, use
})

if sourceDsId == targetDsId {
return _responses.BadRequest("Source and target datastore cannot be the same")
return responses.BadRequest(errors.New("Source and target datastore cannot be the same"))
}
if _, ok := datastores.Get(rctx, sourceDsId); !ok {
return _responses.BadRequest("Source datastore does not appear to exist")
return responses.BadRequest(errors.New("Source datastore does not appear to exist"))
}
if _, ok := datastores.Get(rctx, targetDsId); !ok {
return _responses.BadRequest("Target datastore does not appear to exist")
return responses.BadRequest(errors.New("Target datastore does not appear to exist"))
}

estimate, err := datastores.SizeOfDsIdWithAge(rctx, sourceDsId, beforeTs)
if err != nil {
rctx.Log.Error(err)
sentry.CaptureException(err)
return _responses.InternalServerError("Unexpected error getting storage estimate")
return responses.InternalServerError(errors.New("Unexpected error getting storage estimate"))
}

rctx.Log.Infof("User %s has started a datastore media transfer", user.UserId)
task, err := tasks.RunDatastoreMigration(rctx, sourceDsId, targetDsId, beforeTs)
if err != nil {
rctx.Log.Error(err)
sentry.CaptureException(err)
return _responses.InternalServerError("Unexpected error starting migration")
return responses.InternalServerError(errors.New("Unexpected error starting migration"))
}

migration := &DatastoreMigration{
SizeEstimate: estimate,
TaskID: task.TaskId,
}

return &_responses.DoNotCacheResponse{Payload: migration}
return &responses.DoNotCacheResponse{Payload: migration}
}

func GetDatastoreStorageEstimate(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} {
func GetDatastoreStorageEstimate(r *http.Request, rctx rcontext.RequestContext, user apimeta.UserInfo) interface{} {
beforeTsStr := r.URL.Query().Get("before_ts")
beforeTs := util.NowMillis()
beforeTs := time.Now().UnixMilli()
var err error
if beforeTsStr != "" {
beforeTs, err = strconv.ParseInt(beforeTsStr, 10, 64)
if err != nil {
return _responses.BadRequest("Error parsing before_ts: " + err.Error())
return responses.BadRequest(fmt.Errorf("Error parsing before_ts: %w", err))
}
}

datastoreId := _routers.GetParam("datastoreId", r)
datastoreId := routers.GetParam("datastoreId", r)

rctx = rctx.LogWithFields(logrus.Fields{
"beforeTs": beforeTs,
Expand All @@ -115,7 +117,7 @@ func GetDatastoreStorageEstimate(r *http.Request, rctx rcontext.RequestContext,
if err != nil {
rctx.Log.Error(err)
sentry.CaptureException(err)
return _responses.InternalServerError("Unexpected error getting storage estimate")
return responses.InternalServerError(errors.New("Unexpected error getting storage estimate"))
}
return &_responses.DoNotCacheResponse{Payload: result}
return &responses.DoNotCacheResponse{Payload: result}
}
Loading