From ac2dbb3513b2e6ff24624f30a2a8b1bd51bdabc5 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Wed, 3 Aug 2022 10:55:21 +0200 Subject: [PATCH 01/11] Add Cache-Control header to media endpoints (#2612) * Add Cache-Control header * Raise rate_limiting threshold to 20 --- dendrite-sample.monolith.yaml | 2 +- dendrite-sample.polylith.yaml | 2 +- mediaapi/routing/routing.go | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dendrite-sample.monolith.yaml b/dendrite-sample.monolith.yaml index a34b8af55..816c4cae9 100644 --- a/dendrite-sample.monolith.yaml +++ b/dendrite-sample.monolith.yaml @@ -192,7 +192,7 @@ client_api: # and appservice users are exempt from rate limiting by default. rate_limiting: enabled: true - threshold: 5 + threshold: 20 cooloff_ms: 500 exempt_user_ids: # - "@user:domain.com" diff --git a/dendrite-sample.polylith.yaml b/dendrite-sample.polylith.yaml index 550611229..4784dbaff 100644 --- a/dendrite-sample.polylith.yaml +++ b/dendrite-sample.polylith.yaml @@ -195,7 +195,7 @@ client_api: # and appservice users are exempt from rate limiting by default. rate_limiting: enabled: true - threshold: 5 + threshold: 20 cooloff_ms: 500 exempt_user_ids: # - "@user:domain.com" diff --git a/mediaapi/routing/routing.go b/mediaapi/routing/routing.go index 196908184..9dcfa955f 100644 --- a/mediaapi/routing/routing.go +++ b/mediaapi/routing/routing.go @@ -149,6 +149,9 @@ func makeDownloadAPI( } } + // Cache media for at least one day. + w.Header().Set("Cache-Control", "public,max-age=86400,s-maxage=86400") + Download( w, req, From f7f2453a859e9a2b2cccec5b0f47d558bc9ca507 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Aug 2022 10:35:57 +0100 Subject: [PATCH 02/11] Test Go 1.19 in CI --- .github/workflows/dendrite.yml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index 4cbfb380f..0d1970efd 100644 --- a/.github/workflows/dendrite.yml +++ b/.github/workflows/dendrite.yml @@ -97,7 +97,7 @@ jobs: strategy: fail-fast: false matrix: - go: ["1.18"] + go: ["1.18", "1.19"] steps: - uses: actions/checkout@v3 - name: Setup go @@ -127,7 +127,7 @@ jobs: strategy: fail-fast: false matrix: - go: ["1.18"] + go: ["1.18", "1.19"] goos: ["linux"] goarch: ["amd64", "386"] steps: @@ -160,7 +160,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: ["1.18"] + go: ["1.18", "1.19"] goos: ["windows"] goarch: ["amd64"] steps: @@ -384,7 +384,14 @@ jobs: integration-tests-done: name: Integration tests passed - needs: [initial-tests-done, upgrade_test, upgrade_test_direct, sytest, complement] + needs: + [ + initial-tests-done, + upgrade_test, + upgrade_test_direct, + sytest, + complement, + ] runs-on: ubuntu-latest if: ${{ !cancelled() }} # Run this even if prior jobs were skipped steps: From 376391d1c7e309e4a09998c0717ec8adc70fe1a4 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Aug 2022 10:38:36 +0100 Subject: [PATCH 03/11] Update Pinecone --- go.mod | 11 ++++++----- go.sum | 25 ++++++++++++++----------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 5b2d8670d..3559c5bb1 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91 github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 github.com/matrix-org/gomatrixserverlib v0.0.0-20220725104114-b6003e522771 - github.com/matrix-org/pinecone v0.0.0-20220708135211-1ce778fcde6a + github.com/matrix-org/pinecone v0.0.0-20220803093810-b7a830c08fb9 github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 github.com/mattn/go-sqlite3 v1.14.13 github.com/nats-io/nats-server/v2 v2.8.5-0.20220731184415-903a06a5b4ee @@ -45,7 +45,7 @@ require ( golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e golang.org/x/image v0.0.0-20220413100746-70e8d0d3baa9 golang.org/x/mobile v0.0.0-20220518205345-8578da9835fd - golang.org/x/net v0.0.0-20220524220425-1d687d428aca + golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e golang.org/x/term v0.0.0-20220526004731-065cf7ba2467 gopkg.in/h2non/bimg.v1 v1.1.9 gopkg.in/yaml.v2 v2.4.0 @@ -73,10 +73,11 @@ require ( github.com/juju/errors v0.0.0-20220203013757-bd733f3c86b9 // indirect github.com/juju/testing v0.0.0-20220203020004-a0ff61f03494 // indirect github.com/klauspost/compress v1.15.9 // indirect - github.com/lucas-clemente/quic-go v0.26.0 // indirect + github.com/lucas-clemente/quic-go v0.28.1 // indirect github.com/marten-seemann/qtls-go1-16 v0.1.5 // indirect - github.com/marten-seemann/qtls-go1-17 v0.1.1 // indirect - github.com/marten-seemann/qtls-go1-18 v0.1.1 // indirect + github.com/marten-seemann/qtls-go1-17 v0.1.2 // indirect + github.com/marten-seemann/qtls-go1-18 v0.1.2 // indirect + github.com/marten-seemann/qtls-go1-19 v0.1.0-beta.1 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect github.com/miekg/dns v1.1.49 // indirect github.com/minio/highwayhash v1.0.2 // indirect diff --git a/go.sum b/go.sum index 6321adada..2c8bb4f18 100644 --- a/go.sum +++ b/go.sum @@ -321,8 +321,8 @@ github.com/leodido/go-urn v1.2.0 h1:hpXL4XnriNwQ/ABnpepYM/1vCLWNDfUNts8dX3xTG6Y= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= github.com/lib/pq v1.10.5 h1:J+gdV2cUmX7ZqL2B0lFcW0m+egaHC2V3lpO8nWxyYiQ= github.com/lib/pq v1.10.5/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/lucas-clemente/quic-go v0.26.0 h1:ALBQXr9UJ8A1LyzvceX4jd9QFsHvlI0RR6BkV16o00A= -github.com/lucas-clemente/quic-go v0.26.0/go.mod h1:AzgQoPda7N+3IqMMMkywBKggIFo2KT6pfnlrQ2QieeI= +github.com/lucas-clemente/quic-go v0.28.1 h1:Uo0lvVxWg5la9gflIF9lwa39ONq85Xq2D91YNEIslzU= +github.com/lucas-clemente/quic-go v0.28.1/go.mod h1:oGz5DKK41cJt5+773+BSO9BXDsREY4HLf7+0odGAPO0= github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm/+2c2E2WMI= github.com/lxn/walk v0.0.0-20210112085537-c389da54e794/go.mod h1:E23UucZGqpuUANJooIbHWCufXvOcT6E7Stq81gU+CSQ= github.com/lxn/win v0.0.0-20210218163916-a377121e959e/go.mod h1:KxxjdtRkfNoYDCUP5ryK7XJJNTnpC8atvtmTheChOtk= @@ -330,10 +330,12 @@ github.com/mailru/easyjson v0.0.0-20190312143242-1de009706dbe/go.mod h1:C1wdFJiN github.com/marten-seemann/qpack v0.2.1/go.mod h1:F7Gl5L1jIgN1D11ucXefiuJS9UMVP2opoCp2jDKb7wc= github.com/marten-seemann/qtls-go1-16 v0.1.5 h1:o9JrYPPco/Nukd/HpOHMHZoBDXQqoNtUCmny98/1uqQ= github.com/marten-seemann/qtls-go1-16 v0.1.5/go.mod h1:gNpI2Ol+lRS3WwSOtIUUtRwZEQMXjYK+dQSBFbethAk= -github.com/marten-seemann/qtls-go1-17 v0.1.1 h1:DQjHPq+aOzUeh9/lixAGunn6rIOQyWChPSI4+hgW7jc= -github.com/marten-seemann/qtls-go1-17 v0.1.1/go.mod h1:C2ekUKcDdz9SDWxec1N/MvcXBpaX9l3Nx67XaR84L5s= -github.com/marten-seemann/qtls-go1-18 v0.1.1 h1:qp7p7XXUFL7fpBvSS1sWD+uSqPvzNQK43DH+/qEkj0Y= -github.com/marten-seemann/qtls-go1-18 v0.1.1/go.mod h1:mJttiymBAByA49mhlNZZGrH5u1uXYZJ+RW28Py7f4m4= +github.com/marten-seemann/qtls-go1-17 v0.1.2 h1:JADBlm0LYiVbuSySCHeY863dNkcpMmDR7s0bLKJeYlQ= +github.com/marten-seemann/qtls-go1-17 v0.1.2/go.mod h1:C2ekUKcDdz9SDWxec1N/MvcXBpaX9l3Nx67XaR84L5s= +github.com/marten-seemann/qtls-go1-18 v0.1.2 h1:JH6jmzbduz0ITVQ7ShevK10Av5+jBEKAHMntXmIV7kM= +github.com/marten-seemann/qtls-go1-18 v0.1.2/go.mod h1:mJttiymBAByA49mhlNZZGrH5u1uXYZJ+RW28Py7f4m4= +github.com/marten-seemann/qtls-go1-19 v0.1.0-beta.1 h1:7m/WlWcSROrcK5NxuXaxYD32BZqe/LEEnBrWcH/cOqQ= +github.com/marten-seemann/qtls-go1-19 v0.1.0-beta.1/go.mod h1:5HTDWtVudo/WFsHKRNuOhWlbdjrfs5JHrYb0wIJqGpI= github.com/matrix-org/dugong v0.0.0-20210921133753-66e6b1c67e2e h1:DP5RC0Z3XdyBEW5dKt8YPeN6vZbm6OzVaGVp7f1BQRM= github.com/matrix-org/dugong v0.0.0-20210921133753-66e6b1c67e2e/go.mod h1:NgPCr+UavRGH6n5jmdX8DuqFZ4JiCWIJoZiuhTRLSUg= github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91 h1:s7fexw2QV3YD/fRrzEDPNGgTlJlvXY0EHHnT87wF3OA= @@ -343,8 +345,8 @@ github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5d github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s= github.com/matrix-org/gomatrixserverlib v0.0.0-20220725104114-b6003e522771 h1:ZIPHFIPNDS9dmEbPEiJbNmyCGJtn9exfpLC7JOcn/bE= github.com/matrix-org/gomatrixserverlib v0.0.0-20220725104114-b6003e522771/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk= -github.com/matrix-org/pinecone v0.0.0-20220708135211-1ce778fcde6a h1:DdG8vXMlZ65EAtc4V+3t7zHZ2Gqs24pSnyXS+4BRHUs= -github.com/matrix-org/pinecone v0.0.0-20220708135211-1ce778fcde6a/go.mod h1:ulJzsVOTssIVp1j/m5eI//4VpAGDkMt5NrRuAVX7wpc= +github.com/matrix-org/pinecone v0.0.0-20220803093810-b7a830c08fb9 h1:ed8yvWhTLk7+sNeK/eOZRTvESFTOHDRevoRoyeqPtvY= +github.com/matrix-org/pinecone v0.0.0-20220803093810-b7a830c08fb9/go.mod h1:P4MqPf+u83OPulPJ+XTbSDbbWrdFYNY4LZ/B1PIduFE= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U= github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 h1:eCEHXWDv9Rm335MSuB49mFUK44bwZPFSDde3ORE3syk= github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U= @@ -549,7 +551,6 @@ go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= -go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go4.org v0.0.0-20180809161055-417644f6feb5/go.mod h1:MkTOUMDaeVYJUOUsaDXIhWPZYa1yOyC1qaOBpL57BhE= @@ -659,8 +660,8 @@ golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20210927181540-4e4d966f7476/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211011170408-caeb26a5c8c0/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211101193420-4a448f8816b3/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20220524220425-1d687d428aca h1:xTaFYiPROfpPhqrfTIDXj0ri1SpfueYT951s4bAuDO8= -golang.org/x/net v0.0.0-20220524220425-1d687d428aca/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= +golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e h1:TsQ7F31D3bUCLeqPT0u+yjp1guoArKaNKmCr22PYgTQ= +golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -750,6 +751,7 @@ golang.org/x/sys v0.0.0-20220405052023-b1e9470b6e64/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a h1:dGzPydgVsqGcTRVwiLJ1jVbufYwmzD3LfVPLKsKg+0k= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.0.0-20220526004731-065cf7ba2467 h1:CBpWXWQpIRjzmkkA+M7q9Fqnwd2mZr3AFqexg8YTfoM= golang.org/x/term v0.0.0-20220526004731-065cf7ba2467/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -758,6 +760,7 @@ golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.3.8-0.20211004125949-5bd84dd9b33b h1:NXqSWXSRUSCaFuvitrWtU169I3876zRTalMRbfd6LL0= golang.org/x/text v0.3.8-0.20211004125949-5bd84dd9b33b/go.mod h1:EFNZuWvGYxIRUEX+K8UmCFwYmZjqcrnq15ZuVldZkZ0= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= From bbff41b44bff2dbc53867cc0fd94ce8f31fd511a Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Aug 2022 10:50:45 +0100 Subject: [PATCH 04/11] Disable stack protector on Linux CI build pipelines for now (to avoid `relocation target __stack_chk_fail_local not defined` errors) --- .github/workflows/dendrite.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index 0d1970efd..19ebd760c 100644 --- a/.github/workflows/dendrite.yml +++ b/.github/workflows/dendrite.yml @@ -151,6 +151,7 @@ jobs: GOOS: ${{ matrix.goos }} GOARCH: ${{ matrix.goarch }} CGO_ENABLED: 1 + CGO_CFLAGS: -fno-stack-protector run: go build -trimpath -v -o "bin/" ./cmd/... # build for Windows 64-bit From 2250768be16bd0e6b3a6a72b5e55eb3e2ad6e3c6 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Aug 2022 17:14:21 +0100 Subject: [PATCH 05/11] Remove roominfo cache (#2615) * Remove roominfo cache It's the source of a number of race conditions which are seemingly causing bugs and CI failures. * Make the linter less sad --- internal/caching/cache_roominfo.go | 33 ---------------------- internal/caching/cache_roomservernids.go | 1 - internal/caching/caches.go | 1 - internal/caching/impl_ristretto.go | 7 ----- roomserver/storage/shared/storage.go | 35 ++++++------------------ 5 files changed, 9 insertions(+), 68 deletions(-) delete mode 100644 internal/caching/cache_roominfo.go diff --git a/internal/caching/cache_roominfo.go b/internal/caching/cache_roominfo.go deleted file mode 100644 index 5dfed3c85..000000000 --- a/internal/caching/cache_roominfo.go +++ /dev/null @@ -1,33 +0,0 @@ -package caching - -import ( - "github.com/matrix-org/dendrite/roomserver/types" -) - -// WARNING: This cache is mutable because it's entirely possible that -// the IsStub or StateSnaphotNID fields can change, even though the -// room version and room NID fields will not. This is only safe because -// the RoomInfoCache is used ONLY within the roomserver and because it -// will be kept up-to-date by the latest events updater. It MUST NOT be -// used from other components as we currently have no way to invalidate -// the cache in downstream components. - -// RoomInfosCache contains the subset of functions needed for -// a room Info cache. It must only be used from the roomserver only -// It is not safe for use from other components. -type RoomInfoCache interface { - GetRoomInfo(roomID string) (roomInfo *types.RoomInfo, ok bool) - StoreRoomInfo(roomID string, roomInfo *types.RoomInfo) -} - -// GetRoomInfo must only be called from the roomserver only. It is not -// safe for use from other components. -func (c Caches) GetRoomInfo(roomID string) (*types.RoomInfo, bool) { - return c.RoomInfos.Get(roomID) -} - -// StoreRoomInfo must only be called from the roomserver only. It is not -// safe for use from other components. -func (c Caches) StoreRoomInfo(roomID string, roomInfo *types.RoomInfo) { - c.RoomInfos.Set(roomID, roomInfo) -} diff --git a/internal/caching/cache_roomservernids.go b/internal/caching/cache_roomservernids.go index f27154f19..88a5b28bc 100644 --- a/internal/caching/cache_roomservernids.go +++ b/internal/caching/cache_roomservernids.go @@ -7,7 +7,6 @@ import ( type RoomServerCaches interface { RoomServerNIDsCache RoomVersionCache - RoomInfoCache RoomServerEventsCache EventStateKeyCache } diff --git a/internal/caching/caches.go b/internal/caching/caches.go index f13f743d3..78c9ab7ee 100644 --- a/internal/caching/caches.go +++ b/internal/caching/caches.go @@ -29,7 +29,6 @@ type Caches struct { RoomServerRoomIDs Cache[types.RoomNID, string] // room NID -> room ID RoomServerEvents Cache[int64, *gomatrixserverlib.Event] // event NID -> event RoomServerStateKeys Cache[types.EventStateKeyNID, string] // event NID -> event state key - RoomInfos Cache[string, *types.RoomInfo] // room ID -> room info FederationPDUs Cache[int64, *gomatrixserverlib.HeaderedEvent] // queue NID -> PDU FederationEDUs Cache[int64, *gomatrixserverlib.EDU] // queue NID -> EDU SpaceSummaryRooms Cache[string, gomatrixserverlib.MSC2946SpacesResponse] // room ID -> space response diff --git a/internal/caching/impl_ristretto.go b/internal/caching/impl_ristretto.go index fdbbb4d72..fc0c8cc0f 100644 --- a/internal/caching/impl_ristretto.go +++ b/internal/caching/impl_ristretto.go @@ -35,7 +35,6 @@ const ( roomNIDsCache roomIDsCache roomEventsCache - roomInfosCache federationPDUsCache federationEDUsCache spaceSummaryRoomsCache @@ -106,12 +105,6 @@ func NewRistrettoCache(maxCost config.DataUnit, maxAge time.Duration, enableProm Prefix: eventStateKeyCache, MaxAge: maxAge, }, - RoomInfos: &RistrettoCachePartition[string, *types.RoomInfo]{ // room ID -> room info - cache: cache, - Prefix: roomInfosCache, - Mutable: true, - MaxAge: maxAge, - }, FederationPDUs: &RistrettoCostedCachePartition[int64, *gomatrixserverlib.HeaderedEvent]{ // queue NID -> PDU &RistrettoCachePartition[int64, *gomatrixserverlib.HeaderedEvent]{ cache: cache, diff --git a/roomserver/storage/shared/storage.go b/roomserver/storage/shared/storage.go index 9e6a4142c..cbf9c8b20 100644 --- a/roomserver/storage/shared/storage.go +++ b/roomserver/storage/shared/storage.go @@ -156,30 +156,15 @@ func (d *Database) RoomInfo(ctx context.Context, roomID string) (*types.RoomInfo } func (d *Database) roomInfo(ctx context.Context, txn *sql.Tx, roomID string) (*types.RoomInfo, error) { - roomInfo, ok := d.Cache.GetRoomInfo(roomID) - if ok && roomInfo != nil && !roomInfo.IsStub() { - // The data that's in the cache is not stubby, so return it. - return roomInfo, nil - } - // At this point we either don't have an entry in the cache, or - // it is stubby, so let's check the roomserver_rooms table again. - roomInfoFromDB, err := d.RoomsTable.SelectRoomInfo(ctx, txn, roomID) + roomInfo, err := d.RoomsTable.SelectRoomInfo(ctx, txn, roomID) if err != nil { return nil, err } - // If we have a stubby cache entry already, update it and return - // the reference to the cache entry. if roomInfo != nil { - roomInfo.CopyFrom(roomInfoFromDB) - return roomInfo, nil + d.Cache.StoreRoomServerRoomID(roomInfo.RoomNID, roomID) + d.Cache.StoreRoomVersion(roomID, roomInfo.RoomVersion) } - // Otherwise, try to admit the data into the cache and return the - // new reference from the database. - if roomInfoFromDB != nil { - d.Cache.StoreRoomServerRoomID(roomInfoFromDB.RoomNID, roomID) - d.Cache.StoreRoomInfo(roomID, roomInfoFromDB) - } - return roomInfoFromDB, err + return roomInfo, err } func (d *Database) AddState( @@ -504,8 +489,8 @@ func (d *Database) events( fetchNIDList := make([]types.RoomNID, 0, len(uniqueRoomNIDs)) for n := range uniqueRoomNIDs { if roomID, ok := d.Cache.GetRoomServerRoomID(n); ok { - if roomInfo, ok := d.Cache.GetRoomInfo(roomID); ok { - roomVersions[n] = roomInfo.RoomVersion + if roomVersion, ok := d.Cache.GetRoomVersion(roomID); ok { + roomVersions[n] = roomVersion continue } } @@ -762,9 +747,6 @@ func (d *Database) MissingAuthPrevEvents( func (d *Database) assignRoomNID( ctx context.Context, roomID string, roomVersion gomatrixserverlib.RoomVersion, ) (types.RoomNID, error) { - if roomInfo, ok := d.Cache.GetRoomInfo(roomID); ok { - return roomInfo.RoomNID, nil - } // Check if we already have a numeric ID in the database. roomNID, err := d.RoomsTable.SelectRoomNID(ctx, nil, roomID) if err == sql.ErrNoRows { @@ -837,8 +819,9 @@ func extractRoomVersionFromCreateEvent(event *gomatrixserverlib.Event) ( // "servers should not apply or send redactions to clients until both the redaction event and original event have been seen, and are valid." // https://matrix.org/docs/spec/rooms/v3#authorization-rules-for-events // These cases are: -// - This is a redaction event, redact the event it references if we know about it. -// - This is a normal event which may have been previously redacted. +// - This is a redaction event, redact the event it references if we know about it. +// - This is a normal event which may have been previously redacted. +// // In the first case, check if we have the referenced event then apply the redaction, else store it // in the redactions table with validated=FALSE. In the second case, check if there is a redaction for it: // if there is then apply the redactions and set validated=TRUE. From 9fe509b18da997e294813fcc5f46a45b7f6e6784 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Wed, 3 Aug 2022 18:35:17 +0200 Subject: [PATCH 06/11] Fix syncapi shared users query & device lists (#2614) * Fix query issue, only add "changed" users if we actually share a room * Avoid log spam if context is done * Undo changes to filterSharedUsers * Add logging again.. * Fix SQLite shared users query * Change query to include invited users --- keyserver/internal/internal.go | 11 +++++ syncapi/internal/keychange.go | 44 +++++++++--------- syncapi/internal/keychange_test.go | 1 + .../postgres/current_room_state_table.go | 4 +- .../sqlite3/current_room_state_table.go | 45 ++++++++++--------- 5 files changed, 62 insertions(+), 43 deletions(-) diff --git a/keyserver/internal/internal.go b/keyserver/internal/internal.go index c146b2aa0..91f011517 100644 --- a/keyserver/internal/internal.go +++ b/keyserver/internal/internal.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "sync" "time" @@ -314,6 +315,11 @@ func (a *KeyInternalAPI) QueryKeys(ctx context.Context, req *api.QueryKeysReques for targetKeyID := range masterKey.Keys { sigMap, err := a.DB.CrossSigningSigsForTarget(ctx, req.UserID, targetUserID, targetKeyID) if err != nil { + // Stop executing the function if the context was canceled/the deadline was exceeded, + // as we can't continue without a valid context. + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return + } logrus.WithError(err).Errorf("a.DB.CrossSigningSigsForTarget failed") continue } @@ -335,6 +341,11 @@ func (a *KeyInternalAPI) QueryKeys(ctx context.Context, req *api.QueryKeysReques for targetKeyID, key := range forUserID { sigMap, err := a.DB.CrossSigningSigsForTarget(ctx, req.UserID, targetUserID, gomatrixserverlib.KeyID(targetKeyID)) if err != nil { + // Stop executing the function if the context was canceled/the deadline was exceeded, + // as we can't continue without a valid context. + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return + } logrus.WithError(err).Errorf("a.DB.CrossSigningSigsForTarget failed") continue } diff --git a/syncapi/internal/keychange.go b/syncapi/internal/keychange.go index 03df9285c..4bf54cae0 100644 --- a/syncapi/internal/keychange.go +++ b/syncapi/internal/keychange.go @@ -25,10 +25,9 @@ import ( "github.com/matrix-org/dendrite/syncapi/types" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" + "github.com/sirupsen/logrus" ) -const DeviceListLogName = "dl" - // DeviceOTKCounts adds one-time key counts to the /sync response func DeviceOTKCounts(ctx context.Context, keyAPI keyapi.SyncKeyAPI, userID, deviceID string, res *types.Response) error { var queryRes keyapi.QueryOneTimeKeysResponse @@ -93,18 +92,13 @@ func DeviceListCatchup( queryRes.UserIDs = append(queryRes.UserIDs, joinUserIDs...) queryRes.UserIDs = append(queryRes.UserIDs, leaveUserIDs...) queryRes.UserIDs = util.UniqueStrings(queryRes.UserIDs) - var sharedUsersMap map[string]int - sharedUsersMap, queryRes.UserIDs = filterSharedUsers(ctx, db, userID, queryRes.UserIDs) - util.GetLogger(ctx).Debugf( - "QueryKeyChanges request off=%d,to=%d response off=%d uids=%v", - offset, toOffset, queryRes.Offset, queryRes.UserIDs, - ) + sharedUsersMap := filterSharedUsers(ctx, db, userID, queryRes.UserIDs) userSet := make(map[string]bool) for _, userID := range res.DeviceLists.Changed { userSet[userID] = true } - for _, userID := range queryRes.UserIDs { - if !userSet[userID] { + for userID, count := range sharedUsersMap { + if !userSet[userID] && count > 0 { res.DeviceLists.Changed = append(res.DeviceLists.Changed, userID) hasNew = true userSet[userID] = true @@ -113,7 +107,7 @@ func DeviceListCatchup( // Finally, add in users who have joined or left. // TODO: This is sub-optimal because we will add users to `changed` even if we already shared a room with them. for _, userID := range joinUserIDs { - if !userSet[userID] { + if !userSet[userID] && sharedUsersMap[userID] > 0 { res.DeviceLists.Changed = append(res.DeviceLists.Changed, userID) hasNew = true userSet[userID] = true @@ -126,6 +120,13 @@ func DeviceListCatchup( } } + util.GetLogger(ctx).WithFields(logrus.Fields{ + "user_id": userID, + "from": offset, + "to": toOffset, + "response_offset": queryRes.Offset, + }).Debugf("QueryKeyChanges request result: %+v", res.DeviceLists) + return types.StreamPosition(queryRes.Offset), hasNew, nil } @@ -220,24 +221,27 @@ func TrackChangedUsers( // it down to include only users who the requesting user shares a room with. func filterSharedUsers( ctx context.Context, db storage.SharedUsers, userID string, usersWithChangedKeys []string, -) (map[string]int, []string) { +) map[string]int { sharedUsersMap := make(map[string]int, len(usersWithChangedKeys)) - for _, userID := range usersWithChangedKeys { - sharedUsersMap[userID] = 0 + for _, changedUserID := range usersWithChangedKeys { + sharedUsersMap[changedUserID] = 0 + if changedUserID == userID { + // We forcibly put ourselves in this list because we should be notified about our own device updates + // and if we are in 0 rooms then we don't technically share any room with ourselves so we wouldn't + // be notified about key changes. + sharedUsersMap[userID] = 1 + } } sharedUsers, err := db.SharedUsers(ctx, userID, usersWithChangedKeys) if err != nil { + util.GetLogger(ctx).WithError(err).Errorf("db.SharedUsers failed: %s", err) // default to all users so we do needless queries rather than miss some important device update - return nil, usersWithChangedKeys + return sharedUsersMap } for _, userID := range sharedUsers { sharedUsersMap[userID]++ } - // We forcibly put ourselves in this list because we should be notified about our own device updates - // and if we are in 0 rooms then we don't technically share any room with ourselves so we wouldn't - // be notified about key changes. - sharedUsersMap[userID] = 1 - return sharedUsersMap, sharedUsers + return sharedUsersMap } func joinedRooms(res *types.Response, userID string) []string { diff --git a/syncapi/internal/keychange_test.go b/syncapi/internal/keychange_test.go index 79ed440e7..6bfc91edd 100644 --- a/syncapi/internal/keychange_test.go +++ b/syncapi/internal/keychange_test.go @@ -129,6 +129,7 @@ type wantCatchup struct { } func assertCatchup(t *testing.T, hasNew bool, syncResponse *types.Response, want wantCatchup) { + t.Helper() if hasNew != want.hasNew { t.Errorf("got hasNew=%v want %v", hasNew, want.hasNew) } diff --git a/syncapi/storage/postgres/current_room_state_table.go b/syncapi/storage/postgres/current_room_state_table.go index d13b7be41..58f404511 100644 --- a/syncapi/storage/postgres/current_room_state_table.go +++ b/syncapi/storage/postgres/current_room_state_table.go @@ -112,7 +112,7 @@ const selectEventsWithEventIDsSQL = "" + const selectSharedUsersSQL = "" + "SELECT state_key FROM syncapi_current_room_state WHERE room_id = ANY(" + " SELECT room_id FROM syncapi_current_room_state WHERE state_key = $1 AND membership='join'" + - ") AND state_key = ANY($2) AND membership='join';" + ") AND state_key = ANY($2) AND membership IN ('join', 'invite');" type currentRoomStateStatements struct { upsertRoomStateStmt *sql.Stmt @@ -407,7 +407,7 @@ func (s *currentRoomStateStatements) SelectSharedUsers( ctx context.Context, txn *sql.Tx, userID string, otherUserIDs []string, ) ([]string, error) { stmt := sqlutil.TxStmt(txn, s.selectSharedUsersStmt) - rows, err := stmt.QueryContext(ctx, userID, otherUserIDs) + rows, err := stmt.QueryContext(ctx, userID, pq.Array(otherUserIDs)) if err != nil { return nil, err } diff --git a/syncapi/storage/sqlite3/current_room_state_table.go b/syncapi/storage/sqlite3/current_room_state_table.go index e19298aee..3a10b2325 100644 --- a/syncapi/storage/sqlite3/current_room_state_table.go +++ b/syncapi/storage/sqlite3/current_room_state_table.go @@ -94,9 +94,9 @@ const selectEventsWithEventIDsSQL = "" + " FROM syncapi_current_room_state WHERE event_id IN ($1)" const selectSharedUsersSQL = "" + - "SELECT state_key FROM syncapi_current_room_state WHERE room_id = ANY(" + + "SELECT state_key FROM syncapi_current_room_state WHERE room_id IN(" + " SELECT room_id FROM syncapi_current_room_state WHERE state_key = $1 AND membership='join'" + - ") AND state_key IN ($2) AND membership='join';" + ") AND state_key IN ($2) AND membership IN ('join', 'invite');" type currentRoomStateStatements struct { db *sql.DB @@ -420,25 +420,28 @@ func (s *currentRoomStateStatements) SelectStateEvent( func (s *currentRoomStateStatements) SelectSharedUsers( ctx context.Context, txn *sql.Tx, userID string, otherUserIDs []string, ) ([]string, error) { - query := strings.Replace(selectSharedUsersSQL, "($2)", sqlutil.QueryVariadicOffset(len(otherUserIDs), 1), 1) - stmt, err := s.db.Prepare(query) - if err != nil { - return nil, fmt.Errorf("SelectSharedUsers s.db.Prepare: %w", err) - } - defer internal.CloseAndLogIfError(ctx, stmt, "SelectSharedUsers: stmt.close() failed") - rows, err := sqlutil.TxStmt(txn, stmt).QueryContext(ctx, userID, otherUserIDs) - if err != nil { - return nil, err - } - defer internal.CloseAndLogIfError(ctx, rows, "selectSharedUsersStmt: rows.close() failed") - var stateKey string - result := make([]string, 0, len(otherUserIDs)) - for rows.Next() { - if err := rows.Scan(&stateKey); err != nil { - return nil, err - } - result = append(result, stateKey) + params := make([]interface{}, len(otherUserIDs)+1) + params[0] = userID + for k, v := range otherUserIDs { + params[k+1] = v } - return result, rows.Err() + + result := make([]string, 0, len(otherUserIDs)) + query := strings.Replace(selectSharedUsersSQL, "($2)", sqlutil.QueryVariadicOffset(len(otherUserIDs), 1), 1) + err := sqlutil.RunLimitedVariablesQuery( + ctx, query, s.db, params, sqlutil.SQLite3MaxVariables, + func(rows *sql.Rows) error { + var stateKey string + for rows.Next() { + if err := rows.Scan(&stateKey); err != nil { + return err + } + result = append(result, stateKey) + } + return nil + }, + ) + + return result, err } From 3bf5ae5ffef0ebc140f55320658d9b07bc58e848 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Aug 2022 17:37:27 +0100 Subject: [PATCH 07/11] Try more servers when calling `/state_ids` (#2610) * Try more servers when calling `/state_ids` * More logging * Maybe fix concurrent map write * Revert "Maybe fix concurrent map write" This reverts commit da0dbb836207a911afe77e6f6d63c4809669693c. * Enforce a limit of 20s per server, 5 mins total --- roomserver/internal/input/input_missing.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/roomserver/internal/input/input_missing.go b/roomserver/internal/input/input_missing.go index c78e5d79a..0dd2b64c0 100644 --- a/roomserver/internal/input/input_missing.go +++ b/roomserver/internal/input/input_missing.go @@ -326,8 +326,10 @@ func (t *missingStateReq) lookupStateAfterEvent(ctx context.Context, roomVersion return respState, true, nil } + logrus.WithContext(ctx).Warnf("State for event %s not available locally, falling back to federation (via %d servers)", eventID, len(t.servers)) respState, err := t.lookupStateBeforeEvent(ctx, roomVersion, roomID, eventID) if err != nil { + logrus.WithContext(ctx).WithError(err).Errorf("Failed to look up state before event %s", eventID) return nil, false, fmt.Errorf("t.lookupStateBeforeEvent: %w", err) } @@ -339,6 +341,7 @@ func (t *missingStateReq) lookupStateAfterEvent(ctx context.Context, roomVersion case nil: // do nothing default: + logrus.WithContext(ctx).WithError(err).Errorf("Failed to look up event %s", eventID) return nil, false, fmt.Errorf("t.lookupEvent: %w", err) } h = t.cacheAndReturn(h) @@ -662,9 +665,22 @@ func (t *missingStateReq) lookupMissingStateViaStateIDs(ctx context.Context, roo util.GetLogger(ctx).WithField("room_id", roomID).Infof("lookupMissingStateViaStateIDs %s", eventID) // fetch the state event IDs at the time of the event - stateIDs, err := t.federation.LookupStateIDs(ctx, t.origin, roomID, eventID) + var stateIDs gomatrixserverlib.RespStateIDs + var err error + count := 0 + totalctx, totalcancel := context.WithTimeout(ctx, time.Minute*5) + for _, serverName := range t.servers { + reqctx, reqcancel := context.WithTimeout(totalctx, time.Second*20) + stateIDs, err = t.federation.LookupStateIDs(reqctx, serverName, roomID, eventID) + reqcancel() + if err == nil { + break + } + count++ + } + totalcancel() if err != nil { - return nil, err + return nil, fmt.Errorf("t.federation.LookupStateIDs tried %d server(s), last error: %w", count, err) } // work out which auth/state IDs are missing wantIDs := append(stateIDs.StateEventIDs, stateIDs.AuthEventIDs...) From a2bed259dd765bd0b9781cd65594343d22c07986 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 3 Aug 2022 17:42:13 +0100 Subject: [PATCH 08/11] Version 0.9.1 (#2616) * Version 0.9.1 * Update CHANGES.md --- CHANGES.md | 14 ++++++++++++++ internal/version.go | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 0ae927446..5dd8da362 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,19 @@ # Changelog +## Dendrite 0.9.1 (2022-08-03) + +### Fixes + +* Upgrades a dependency which caused issues building Dendrite with Go 1.19 +* The roomserver will no longer give up prematurely after failing to call `/state_ids` +* Removes the faulty room info cache, which caused of a number of race conditions and occasional bugs (including when creating and joining rooms) +* The media endpoint now sets the `Cache-Control` header correctly to prevent web-based clients from hitting media endpoints excessively +* The sync API will now advance the PDU stream position correctly in all cases (contributed by [sergekh2](https://github.com/sergekh2)) +* The sync API will now delete the correct range of send-to-device messages when advancing the stream position +* The device list `changed` key in the `/sync` response should now return the correct users +* A data race when looking up missing state has been fixed +* The `/send_join` API is now applying stronger validation to the received membership event + ## Dendrite 0.9.0 (2022-08-01) ### Features diff --git a/internal/version.go b/internal/version.go index 6d29a68ee..38d0864e7 100644 --- a/internal/version.go +++ b/internal/version.go @@ -17,7 +17,7 @@ var build string const ( VersionMajor = 0 VersionMinor = 9 - VersionPatch = 0 + VersionPatch = 1 VersionTag = "" // example: "rc1" ) From 9a655cb5e7f8b96a0b02203e69d866dfb1a184e2 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 5 Aug 2022 07:20:34 +0200 Subject: [PATCH 09/11] Only create a new destinationQueue if we don't have one (#2620) --- federationapi/queue/queue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/federationapi/queue/queue.go b/federationapi/queue/queue.go index 4c25c4ce6..88664fcf9 100644 --- a/federationapi/queue/queue.go +++ b/federationapi/queue/queue.go @@ -158,7 +158,7 @@ func (oqs *OutgoingQueues) getQueue(destination gomatrixserverlib.ServerName) *d oqs.queuesMutex.Lock() defer oqs.queuesMutex.Unlock() oq, ok := oqs.queues[destination] - if !ok || oq != nil { + if !ok || oq == nil { destinationQueueTotal.Inc() oq = &destinationQueue{ queues: oqs, From de78eab63a99653edf68f783e263688ad4b701d8 Mon Sep 17 00:00:00 2001 From: Brian Meek Date: Fri, 5 Aug 2022 01:19:33 -0700 Subject: [PATCH 10/11] Add race testing to tests, and fix a few small race conditions in the tests (#2587) * Add race testing to tests, and fix a few small race conditions in the tests * Enable run-sytest on MacOS * Remove deadlock detecting mutex, per code review feedback * Remove autoformatting related changes and a closure that is not needed * Adjust to importing nats client as 'natsclient' Signed-off-by: Brian Meek * Clarify the use of gooseMutex to proect goose internal state Signed-off-by: Brian Meek * Remove no longer needed mutex for guarding goose Signed-off-by: Brian Meek --- build/scripts/build-test-lint.sh | 2 +- docs/CONTRIBUTING.md | 2 +- federationapi/federationapi_test.go | 10 ++++++++++ keyserver/storage/storage_test.go | 9 ++++++++- run-sytest.sh | 2 +- setup/jetstream/nats.go | 14 +++++++------- syncapi/sync/requestpool_test.go | 7 +++++++ 7 files changed, 35 insertions(+), 11 deletions(-) diff --git a/build/scripts/build-test-lint.sh b/build/scripts/build-test-lint.sh index 8f0b775b1..32f89c076 100755 --- a/build/scripts/build-test-lint.sh +++ b/build/scripts/build-test-lint.sh @@ -13,4 +13,4 @@ go build ./cmd/... ./build/scripts/find-lint.sh echo "Testing..." -go test -v ./... +go test --race -v ./... diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 169224b9e..771af9ecf 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -64,7 +64,7 @@ comment. Please avoid doing this if you can. We also have unit tests which we run via: ```bash -go test ./... +go test --race ./... ``` In general, we like submissions that come with tests. Anything that proves that the diff --git a/federationapi/federationapi_test.go b/federationapi/federationapi_test.go index ae244c566..8884e34c6 100644 --- a/federationapi/federationapi_test.go +++ b/federationapi/federationapi_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "strings" + "sync" "testing" "time" @@ -48,6 +49,7 @@ func (f *fedRoomserverAPI) QueryRoomsForUser(ctx context.Context, req *rsapi.Que // TODO: This struct isn't generic, only works for TestFederationAPIJoinThenKeyUpdate type fedClient struct { + fedClientMutex sync.Mutex api.FederationClient allowJoins []*test.Room keys map[gomatrixserverlib.ServerName]struct { @@ -59,6 +61,8 @@ type fedClient struct { } func (f *fedClient) GetServerKeys(ctx context.Context, matrixServer gomatrixserverlib.ServerName) (gomatrixserverlib.ServerKeys, error) { + f.fedClientMutex.Lock() + defer f.fedClientMutex.Unlock() fmt.Println("GetServerKeys:", matrixServer) var keys gomatrixserverlib.ServerKeys var keyID gomatrixserverlib.KeyID @@ -122,6 +126,8 @@ func (f *fedClient) MakeJoin(ctx context.Context, s gomatrixserverlib.ServerName return } func (f *fedClient) SendJoin(ctx context.Context, s gomatrixserverlib.ServerName, event *gomatrixserverlib.Event) (res gomatrixserverlib.RespSendJoin, err error) { + f.fedClientMutex.Lock() + defer f.fedClientMutex.Unlock() for _, r := range f.allowJoins { if r.ID == event.RoomID() { r.InsertEvent(f.t, event.Headered(r.Version)) @@ -134,6 +140,8 @@ func (f *fedClient) SendJoin(ctx context.Context, s gomatrixserverlib.ServerName } func (f *fedClient) SendTransaction(ctx context.Context, t gomatrixserverlib.Transaction) (res gomatrixserverlib.RespSend, err error) { + f.fedClientMutex.Lock() + defer f.fedClientMutex.Unlock() for _, edu := range t.EDUs { if edu.Type == gomatrixserverlib.MDeviceListUpdate { f.sentTxn = true @@ -242,6 +250,8 @@ func testFederationAPIJoinThenKeyUpdate(t *testing.T, dbType test.DBType) { testrig.MustPublishMsgs(t, jsctx, msg) time.Sleep(500 * time.Millisecond) + fc.fedClientMutex.Lock() + defer fc.fedClientMutex.Unlock() if !fc.sentTxn { t.Fatalf("did not send device list update") } diff --git a/keyserver/storage/storage_test.go b/keyserver/storage/storage_test.go index 44cfb5f2a..e7a2af7c2 100644 --- a/keyserver/storage/storage_test.go +++ b/keyserver/storage/storage_test.go @@ -3,6 +3,7 @@ package storage_test import ( "context" "reflect" + "sync" "testing" "github.com/matrix-org/dendrite/keyserver/api" @@ -103,6 +104,9 @@ func TestKeyChangesUpperLimit(t *testing.T) { }) } +var dbLock sync.Mutex +var deviceArray = []string{"AAA", "another_device"} + // The purpose of this test is to make sure that the storage layer is generating sequential stream IDs per user, // and that they are returned correctly when querying for device keys. func TestDeviceKeysStreamIDGeneration(t *testing.T) { @@ -169,8 +173,11 @@ func TestDeviceKeysStreamIDGeneration(t *testing.T) { t.Fatalf("Expected StoreLocalDeviceKeys to set StreamID=3 (new key same device) but got %d", msgs[0].StreamID) } + dbLock.Lock() + defer dbLock.Unlock() // Querying for device keys returns the latest stream IDs - msgs, err = db.DeviceKeysForUser(ctx, alice, []string{"AAA", "another_device"}, false) + msgs, err = db.DeviceKeysForUser(ctx, alice, deviceArray, false) + if err != nil { t.Fatalf("DeviceKeysForUser returned error: %s", err) } diff --git a/run-sytest.sh b/run-sytest.sh index 47635fd12..e23982397 100755 --- a/run-sytest.sh +++ b/run-sytest.sh @@ -17,7 +17,7 @@ main() { if [ -d ../sytest ]; then local tmpdir - tmpdir="$(mktemp -d --tmpdir run-systest.XXXXXXXXXX)" + tmpdir="$(mktemp -d -t run-systest.XXXXXXXXXX)" trap "rm -r '$tmpdir'" EXIT if [ -z "$DISABLE_BUILDING_SYTEST" ]; then diff --git a/setup/jetstream/nats.go b/setup/jetstream/nats.go index be216a02a..051d55a35 100644 --- a/setup/jetstream/nats.go +++ b/setup/jetstream/nats.go @@ -14,16 +14,16 @@ import ( "github.com/sirupsen/logrus" natsserver "github.com/nats-io/nats-server/v2/server" - "github.com/nats-io/nats.go" natsclient "github.com/nats-io/nats.go" ) type NATSInstance struct { *natsserver.Server - sync.Mutex } -func DeleteAllStreams(js nats.JetStreamContext, cfg *config.JetStream) { +var natsLock sync.Mutex + +func DeleteAllStreams(js natsclient.JetStreamContext, cfg *config.JetStream) { for _, stream := range streams { // streams are defined in streams.go name := cfg.Prefixed(stream.Name) _ = js.DeleteStream(name) @@ -31,11 +31,12 @@ func DeleteAllStreams(js nats.JetStreamContext, cfg *config.JetStream) { } func (s *NATSInstance) Prepare(process *process.ProcessContext, cfg *config.JetStream) (natsclient.JetStreamContext, *natsclient.Conn) { + natsLock.Lock() + defer natsLock.Unlock() // check if we need an in-process NATS Server if len(cfg.Addresses) != 0 { return setupNATS(process, cfg, nil) } - s.Lock() if s.Server == nil { var err error s.Server, err = natsserver.NewServer(&natsserver.Options{ @@ -63,7 +64,6 @@ func (s *NATSInstance) Prepare(process *process.ProcessContext, cfg *config.JetS process.ComponentFinished() }() } - s.Unlock() if !s.ReadyForConnections(time.Second * 10) { logrus.Fatalln("NATS did not start in time") } @@ -77,9 +77,9 @@ func (s *NATSInstance) Prepare(process *process.ProcessContext, cfg *config.JetS func setupNATS(process *process.ProcessContext, cfg *config.JetStream, nc *natsclient.Conn) (natsclient.JetStreamContext, *natsclient.Conn) { if nc == nil { var err error - opts := []nats.Option{} + opts := []natsclient.Option{} if cfg.DisableTLSValidation { - opts = append(opts, nats.Secure(&tls.Config{ + opts = append(opts, natsclient.Secure(&tls.Config{ InsecureSkipVerify: true, })) } diff --git a/syncapi/sync/requestpool_test.go b/syncapi/sync/requestpool_test.go index 48e6c6c7a..3e5769d8c 100644 --- a/syncapi/sync/requestpool_test.go +++ b/syncapi/sync/requestpool_test.go @@ -12,10 +12,13 @@ import ( ) type dummyPublisher struct { + lock sync.Mutex count int } func (d *dummyPublisher) SendPresence(userID string, presence types.Presence, statusMsg *string) error { + d.lock.Lock() + defer d.lock.Unlock() d.count++ return nil } @@ -125,11 +128,15 @@ func TestRequestPool_updatePresence(t *testing.T) { go rp.cleanPresence(db, time.Millisecond*50) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + publisher.lock.Lock() beforeCount := publisher.count + publisher.lock.Unlock() rp.updatePresence(db, tt.args.presence, tt.args.userID) + publisher.lock.Lock() if tt.wantIncrease && publisher.count <= beforeCount { t.Fatalf("expected count to increase: %d <= %d", publisher.count, beforeCount) } + publisher.lock.Unlock() time.Sleep(tt.args.sleep) }) } From 1b7f84250a46b401eccb89acafdef1b379f2dbc0 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 5 Aug 2022 11:12:41 +0200 Subject: [PATCH 11/11] Fix linter issues (#2624) * Try that again * All hail the mighty linter? * And once again * goimport all the things --- clientapi/routing/directory_public.go | 17 ++++++----- clientapi/routing/sendevent.go | 7 +++-- clientapi/routing/threepid.go | 5 ++-- clientapi/routing/voip.go | 8 +++-- cmd/dendrite-upgrade-tests/main.go | 6 ++-- cmd/dendrite-upgrade-tests/tests.go | 6 ++-- federationapi/consumers/roomserver.go | 8 +++-- federationapi/routing/join.go | 21 ++++++------- internal/log.go | 5 ++-- keyserver/internal/device_list_update.go | 7 +++-- keyserver/internal/device_list_update_test.go | 3 +- roomserver/internal/input/input.go | 15 +++++----- .../internal/input/input_latest_events.go | 30 +++++++++---------- .../internal/perform/perform_backfill.go | 10 ++++--- roomserver/storage/tables/interface.go | 5 ++-- setup/mscs/msc2836/msc2836_test.go | 23 +++++++------- syncapi/internal/keychange_test.go | 20 +++++++------ syncapi/routing/filter.go | 11 ++++--- syncapi/storage/tables/interface.go | 17 ++++++----- userapi/storage/sqlite3/stats_table.go | 17 ++++++----- 20 files changed, 136 insertions(+), 105 deletions(-) diff --git a/clientapi/routing/directory_public.go b/clientapi/routing/directory_public.go index c3e6141b2..8ddb3267a 100644 --- a/clientapi/routing/directory_public.go +++ b/clientapi/routing/directory_public.go @@ -23,13 +23,14 @@ import ( "strings" "sync" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/matrix-org/dendrite/clientapi/api" "github.com/matrix-org/dendrite/clientapi/httputil" "github.com/matrix-org/dendrite/clientapi/jsonerror" roomserverAPI "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/setup/config" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" ) var ( @@ -196,14 +197,14 @@ func fillPublicRoomsReq(httpReq *http.Request, request *PublicRoomReq) *util.JSO // sliceInto returns a subslice of `slice` which honours the since/limit values given. // -// 0 1 2 3 4 5 6 index -// [A, B, C, D, E, F, G] slice +// 0 1 2 3 4 5 6 index +// [A, B, C, D, E, F, G] slice // -// limit=3 => A,B,C (prev='', next='3') -// limit=3&since=3 => D,E,F (prev='0', next='6') -// limit=3&since=6 => G (prev='3', next='') +// limit=3 => A,B,C (prev='', next='3') +// limit=3&since=3 => D,E,F (prev='0', next='6') +// limit=3&since=6 => G (prev='3', next='') // -// A value of '-1' for prev/next indicates no position. +// A value of '-1' for prev/next indicates no position. func sliceInto(slice []gomatrixserverlib.PublicRoom, since int64, limit int16) (subset []gomatrixserverlib.PublicRoom, prev, next int) { prev = -1 next = -1 diff --git a/clientapi/routing/sendevent.go b/clientapi/routing/sendevent.go index 2e864adef..85f1053f3 100644 --- a/clientapi/routing/sendevent.go +++ b/clientapi/routing/sendevent.go @@ -63,9 +63,10 @@ var sendEventDuration = prometheus.NewHistogramVec( ) // SendEvent implements: -// /rooms/{roomID}/send/{eventType} -// /rooms/{roomID}/send/{eventType}/{txnID} -// /rooms/{roomID}/state/{eventType}/{stateKey} +// +// /rooms/{roomID}/send/{eventType} +// /rooms/{roomID}/send/{eventType}/{txnID} +// /rooms/{roomID}/state/{eventType}/{stateKey} func SendEvent( req *http.Request, device *userapi.Device, diff --git a/clientapi/routing/threepid.go b/clientapi/routing/threepid.go index 94b658ee3..4b7989ecb 100644 --- a/clientapi/routing/threepid.go +++ b/clientapi/routing/threepid.go @@ -38,8 +38,9 @@ type threePIDsResponse struct { } // RequestEmailToken implements: -// POST /account/3pid/email/requestToken -// POST /register/email/requestToken +// +// POST /account/3pid/email/requestToken +// POST /register/email/requestToken func RequestEmailToken(req *http.Request, threePIDAPI api.ClientUserAPI, cfg *config.ClientAPI) util.JSONResponse { var body threepid.EmailAssociationRequest if reqErr := httputil.UnmarshalJSONRequest(req, &body); reqErr != nil { diff --git a/clientapi/routing/voip.go b/clientapi/routing/voip.go index c7ddaabcf..f0f69ce3c 100644 --- a/clientapi/routing/voip.go +++ b/clientapi/routing/voip.go @@ -22,15 +22,17 @@ import ( "net/http" "time" + "github.com/matrix-org/gomatrix" + "github.com/matrix-org/util" + "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/userapi/api" - "github.com/matrix-org/gomatrix" - "github.com/matrix-org/util" ) // RequestTurnServer implements: -// GET /voip/turnServer +// +// GET /voip/turnServer func RequestTurnServer(req *http.Request, device *api.Device, cfg *config.ClientAPI) util.JSONResponse { turnConfig := cfg.TURN diff --git a/cmd/dendrite-upgrade-tests/main.go b/cmd/dendrite-upgrade-tests/main.go index 39843dccb..ea5f5e2bc 100644 --- a/cmd/dendrite-upgrade-tests/main.go +++ b/cmd/dendrite-upgrade-tests/main.go @@ -47,7 +47,7 @@ const HEAD = "HEAD" // We cannot use the dockerfile associated with the repo with each version sadly due to changes in // Docker versions. Specifically, earlier Dendrite versions are incompatible with newer Docker clients // due to the error: -// When using COPY with more than one source file, the destination must be a directory and end with a / +// When using COPY with more than one source file, the destination must be a directory and end with a / // We need to run a postgres anyway, so use the dockerfile associated with Complement instead. const Dockerfile = `FROM golang:1.18-stretch as build RUN apt-get update && apt-get install -y postgresql @@ -95,7 +95,9 @@ CMD /build/run_dendrite.sh ` const dendriteUpgradeTestLabel = "dendrite_upgrade_test" // downloadArchive downloads an arbitrary github archive of the form: -// https://github.com/matrix-org/dendrite/archive/v0.3.11.tar.gz +// +// https://github.com/matrix-org/dendrite/archive/v0.3.11.tar.gz +// // and re-tarballs it without the top-level directory which contains branch information. It inserts // the contents of `dockerfile` as a root file `Dockerfile` in the re-tarballed directory such that // you can directly feed the retarballed archive to `ImageBuild` to have it run said dockerfile. diff --git a/cmd/dendrite-upgrade-tests/tests.go b/cmd/dendrite-upgrade-tests/tests.go index e02af92a9..ff1e09dda 100644 --- a/cmd/dendrite-upgrade-tests/tests.go +++ b/cmd/dendrite-upgrade-tests/tests.go @@ -18,9 +18,9 @@ type user struct { } // runTests performs the following operations: -// - register alice and bob with branch name muxed into the localpart -// - create a DM room for the 2 users and exchange messages -// - create/join a public #global room and exchange messages +// - register alice and bob with branch name muxed into the localpart +// - create a DM room for the 2 users and exchange messages +// - create/join a public #global room and exchange messages func runTests(baseURL, branchName string) error { // register 2 users users := []user{ diff --git a/federationapi/consumers/roomserver.go b/federationapi/consumers/roomserver.go index e50ec66ad..2622ecb3f 100644 --- a/federationapi/consumers/roomserver.go +++ b/federationapi/consumers/roomserver.go @@ -208,9 +208,11 @@ func (s *OutputRoomEventConsumer) processMessage(ore api.OutputNewRoomEvent, rew // joinedHostsAtEvent works out a list of matrix servers that were joined to // the room at the event (including peeking ones) // It is important to use the state at the event for sending messages because: -// 1) We shouldn't send messages to servers that weren't in the room. -// 2) If a server is kicked from the rooms it should still be told about the -// kick event, +// +// 1. We shouldn't send messages to servers that weren't in the room. +// 2. If a server is kicked from the rooms it should still be told about the +// kick event. +// // Usually the list can be calculated locally, but sometimes it will need fetch // events from the room server. // Returns an error if there was a problem talking to the room server. diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 30406a155..9ad3bd8eb 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -21,13 +21,14 @@ import ( "sort" "time" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/sirupsen/logrus" + "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/internal/eventutil" "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/setup/config" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" - "github.com/sirupsen/logrus" ) // MakeJoin implements the /make_join API @@ -435,13 +436,13 @@ func SendJoin( // a restricted room join. If the room version does not support restricted // joins then this function returns with no side effects. This returns three // values: -// * an optional JSON response body (i.e. M_UNABLE_TO_AUTHORISE_JOIN) which -// should always be sent back to the client if one is specified -// * a user ID of an authorising user, typically a user that has power to -// issue invites in the room, if one has been found -// * an error if there was a problem finding out if this was allowable, -// like if the room version isn't known or a problem happened talking to -// the roomserver +// - an optional JSON response body (i.e. M_UNABLE_TO_AUTHORISE_JOIN) which +// should always be sent back to the client if one is specified +// - a user ID of an authorising user, typically a user that has power to +// issue invites in the room, if one has been found +// - an error if there was a problem finding out if this was allowable, +// like if the room version isn't known or a problem happened talking to +// the roomserver func checkRestrictedJoin( httpReq *http.Request, rsAPI api.FederationRoomserverAPI, diff --git a/internal/log.go b/internal/log.go index bba0ac6e6..a171555ab 100644 --- a/internal/log.go +++ b/internal/log.go @@ -27,9 +27,10 @@ import ( "github.com/matrix-org/util" - "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dugong" "github.com/sirupsen/logrus" + + "github.com/matrix-org/dendrite/setup/config" ) type utcFormatter struct { @@ -145,7 +146,7 @@ func setupFileHook(hook config.LogrusHook, level logrus.Level, componentName str }) } -//CloseAndLogIfError Closes io.Closer and logs the error if any +// CloseAndLogIfError Closes io.Closer and logs the error if any func CloseAndLogIfError(ctx context.Context, closer io.Closer, message string) { if closer == nil { return diff --git a/keyserver/internal/device_list_update.go b/keyserver/internal/device_list_update.go index acbcd5b8f..e317755e4 100644 --- a/keyserver/internal/device_list_update.go +++ b/keyserver/internal/device_list_update.go @@ -22,12 +22,13 @@ import ( "sync" "time" - fedsenderapi "github.com/matrix-org/dendrite/federationapi/api" - "github.com/matrix-org/dendrite/keyserver/api" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" "github.com/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" + + fedsenderapi "github.com/matrix-org/dendrite/federationapi/api" + "github.com/matrix-org/dendrite/keyserver/api" ) var ( @@ -66,12 +67,14 @@ func init() { // - We don't have unbounded growth in proportion to the number of servers (this is more important in a P2P world where // we have many many servers) // - We can adjust concurrency (at the cost of memory usage) by tuning N, to accommodate mobile devices vs servers. +// // The downsides are that: // - Query requests can get queued behind other servers if they hash to the same worker, even if there are other free // workers elsewhere. Whilst suboptimal, provided we cap how long a single request can last (e.g using context timeouts) // we guarantee we will get around to it. Also, more users on a given server does not increase the number of requests // (as /keys/query allows multiple users to be specified) so being stuck behind matrix.org won't materially be any worse // than being stuck behind foo.bar +// // In the event that the query fails, a lock is acquired and the server name along with the time to wait before retrying is // set in a map. A restarter goroutine periodically probes this map and injects servers which are ready to be retried. type DeviceListUpdater struct { diff --git a/keyserver/internal/device_list_update_test.go b/keyserver/internal/device_list_update_test.go index 0033a5086..e65342e2a 100644 --- a/keyserver/internal/device_list_update_test.go +++ b/keyserver/internal/device_list_update_test.go @@ -27,8 +27,9 @@ import ( "testing" "time" - "github.com/matrix-org/dendrite/keyserver/api" "github.com/matrix-org/gomatrixserverlib" + + "github.com/matrix-org/dendrite/keyserver/api" ) var ( diff --git a/roomserver/internal/input/input.go b/roomserver/internal/input/input.go index ecd4ecbb5..339c9796c 100644 --- a/roomserver/internal/input/input.go +++ b/roomserver/internal/input/input.go @@ -25,6 +25,11 @@ import ( "github.com/Arceliar/phony" "github.com/getsentry/sentry-go" + "github.com/matrix-org/gomatrixserverlib" + "github.com/nats-io/nats.go" + "github.com/prometheus/client_golang/prometheus" + "github.com/sirupsen/logrus" + fedapi "github.com/matrix-org/dendrite/federationapi/api" "github.com/matrix-org/dendrite/roomserver/acls" "github.com/matrix-org/dendrite/roomserver/api" @@ -35,10 +40,6 @@ import ( "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/setup/jetstream" "github.com/matrix-org/dendrite/setup/process" - "github.com/matrix-org/gomatrixserverlib" - "github.com/nats-io/nats.go" - "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" ) // Inputer is responsible for consuming from the roomserver input @@ -60,9 +61,9 @@ import ( // per-room durable consumers will only progress through the stream // as events are processed. // -// A BC * -> positions of each consumer (* = ephemeral) -// ⌄ ⌄⌄ ⌄ -// ABAABCAABCAA -> newest (letter = subject for each message) +// A BC * -> positions of each consumer (* = ephemeral) +// ⌄ ⌄⌄ ⌄ +// ABAABCAABCAA -> newest (letter = subject for each message) // // In this example, A is still processing an event but has two // pending events to process afterwards. Both B and C are caught diff --git a/roomserver/internal/input/input_latest_events.go b/roomserver/internal/input/input_latest_events.go index f7d15fdb5..d6efad79d 100644 --- a/roomserver/internal/input/input_latest_events.go +++ b/roomserver/internal/input/input_latest_events.go @@ -20,32 +20,32 @@ import ( "context" "fmt" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/opentracing/opentracing-go" + "github.com/sirupsen/logrus" + "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/roomserver/state" "github.com/matrix-org/dendrite/roomserver/storage/shared" "github.com/matrix-org/dendrite/roomserver/types" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" - "github.com/opentracing/opentracing-go" - "github.com/sirupsen/logrus" ) // updateLatestEvents updates the list of latest events for this room in the database and writes the // event to the output log. // The latest events are the events that aren't referenced by another event in the database: // -// Time goes down the page. 1 is the m.room.create event (root). -// -// 1 After storing 1 the latest events are {1} -// | After storing 2 the latest events are {2} -// 2 After storing 3 the latest events are {3} -// / \ After storing 4 the latest events are {3,4} -// 3 4 After storing 5 the latest events are {5,4} -// | | After storing 6 the latest events are {5,6} -// 5 6 <--- latest After storing 7 the latest events are {6,7} -// | -// 7 <----- latest +// Time goes down the page. 1 is the m.room.create event (root). +// 1 After storing 1 the latest events are {1} +// | After storing 2 the latest events are {2} +// 2 After storing 3 the latest events are {3} +// / \ After storing 4 the latest events are {3,4} +// 3 4 After storing 5 the latest events are {5,4} +// | | After storing 6 the latest events are {5,6} +// 5 6 <--- latest After storing 7 the latest events are {6,7} +// | +// 7 <----- latest // // Can only be called once at a time func (r *Inputer) updateLatestEvents( diff --git a/roomserver/internal/perform/perform_backfill.go b/roomserver/internal/perform/perform_backfill.go index 5b7ed22ee..298ba04f6 100644 --- a/roomserver/internal/perform/perform_backfill.go +++ b/roomserver/internal/perform/perform_backfill.go @@ -19,6 +19,10 @@ import ( "fmt" "github.com/getsentry/sentry-go" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/sirupsen/logrus" + federationAPI "github.com/matrix-org/dendrite/federationapi/api" "github.com/matrix-org/dendrite/internal/eventutil" "github.com/matrix-org/dendrite/roomserver/api" @@ -26,9 +30,6 @@ import ( "github.com/matrix-org/dendrite/roomserver/internal/helpers" "github.com/matrix-org/dendrite/roomserver/storage" "github.com/matrix-org/dendrite/roomserver/types" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" - "github.com/sirupsen/logrus" ) // the max number of servers to backfill from per request. If this is too low we may fail to backfill when @@ -522,8 +523,9 @@ func (b *backfillRequester) ProvideEvents(roomVer gomatrixserverlib.RoomVersion, } // joinEventsFromHistoryVisibility returns all CURRENTLY joined members if our server can read the room history +// // TODO: Long term we probably want a history_visibility table which stores eventNID | visibility_enum so we can just -// pull all events and then filter by that table. +// pull all events and then filter by that table. func joinEventsFromHistoryVisibility( ctx context.Context, db storage.Database, roomID string, stateEntries []types.StateEntry, thisServer gomatrixserverlib.ServerName) ([]types.Event, error) { diff --git a/roomserver/storage/tables/interface.go b/roomserver/storage/tables/interface.go index 58c43ac45..0bc389b80 100644 --- a/roomserver/storage/tables/interface.go +++ b/roomserver/storage/tables/interface.go @@ -5,9 +5,10 @@ import ( "database/sql" "errors" - "github.com/matrix-org/dendrite/roomserver/types" "github.com/matrix-org/gomatrixserverlib" "github.com/tidwall/gjson" + + "github.com/matrix-org/dendrite/roomserver/types" ) var OptimisationNotSupportedError = errors.New("optimisation not supported") @@ -178,7 +179,7 @@ type StrippedEvent struct { } // ExtractContentValue from the given state event. For example, given an m.room.name event with: -// content: { name: "Foo" } +// content: { name: "Foo" } // this returns "Foo". func ExtractContentValue(ev *gomatrixserverlib.HeaderedEvent) string { content := ev.Content() diff --git a/setup/mscs/msc2836/msc2836_test.go b/setup/mscs/msc2836/msc2836_test.go index 9044823af..edb1e77dd 100644 --- a/setup/mscs/msc2836/msc2836_test.go +++ b/setup/mscs/msc2836/msc2836_test.go @@ -15,6 +15,8 @@ import ( "time" "github.com/gorilla/mux" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/dendrite/internal/hooks" "github.com/matrix-org/dendrite/internal/httputil" roomserver "github.com/matrix-org/dendrite/roomserver/api" @@ -22,7 +24,6 @@ import ( "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/setup/mscs/msc2836" userapi "github.com/matrix-org/dendrite/userapi/api" - "github.com/matrix-org/gomatrixserverlib" ) var ( @@ -32,15 +33,17 @@ var ( ) // Basic sanity check of MSC2836 logic. Injects a thread that looks like: -// A -// | -// B -// / \ -// C D -// /|\ -// E F G -// | -// H +// +// A +// | +// B +// / \ +// C D +// /|\ +// E F G +// | +// H +// // And makes sure POST /event_relationships works with various parameters func TestMSC2836(t *testing.T) { alice := "@alice:localhost" diff --git a/syncapi/internal/keychange_test.go b/syncapi/internal/keychange_test.go index 6bfc91edd..c7d8df740 100644 --- a/syncapi/internal/keychange_test.go +++ b/syncapi/internal/keychange_test.go @@ -6,12 +6,13 @@ import ( "sort" "testing" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + keyapi "github.com/matrix-org/dendrite/keyserver/api" "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/syncapi/types" userapi "github.com/matrix-org/dendrite/userapi/api" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" ) var ( @@ -364,13 +365,14 @@ func TestKeyChangeCatchupChangeAndLeft(t *testing.T) { // tests that joining/leaving the SAME room puts users in `left` if the final state is leave. // NB: Consider the case: -// - Alice and Bob are in a room. -// - Alice goes offline, Charlie joins, sends encrypted messages then leaves the room. -// - Alice comes back online. Technically nothing has changed in the set of users between those two points in time, -// it's still just (Alice,Bob) but then we won't be tracking Charlie -- is this okay though? It's device keys -// which are only relevant when actively sending events I think? And if Alice does need the keys she knows -// charlie's (user_id, device_id) so can just hit /keys/query - no need to keep updated about it because she -// doesn't share any rooms with him. +// - Alice and Bob are in a room. +// - Alice goes offline, Charlie joins, sends encrypted messages then leaves the room. +// - Alice comes back online. Technically nothing has changed in the set of users between those two points in time, +// it's still just (Alice,Bob) but then we won't be tracking Charlie -- is this okay though? It's device keys +// which are only relevant when actively sending events I think? And if Alice does need the keys she knows +// charlie's (user_id, device_id) so can just hit /keys/query - no need to keep updated about it because she +// doesn't share any rooms with him. +// // Ergo, we put them in `left` as it is simpler. func TestKeyChangeCatchupChangeAndLeftSameRoom(t *testing.T) { newShareUser := "@berta:localhost" diff --git a/syncapi/routing/filter.go b/syncapi/routing/filter.go index 1a10bd649..b41714dfc 100644 --- a/syncapi/routing/filter.go +++ b/syncapi/routing/filter.go @@ -19,13 +19,14 @@ import ( "io/ioutil" "net/http" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/util" + "github.com/tidwall/gjson" + "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/syncapi/storage" "github.com/matrix-org/dendrite/syncapi/sync" "github.com/matrix-org/dendrite/userapi/api" - "github.com/matrix-org/gomatrixserverlib" - "github.com/matrix-org/util" - "github.com/tidwall/gjson" ) // GetFilter implements GET /_matrix/client/r0/user/{userId}/filter/{filterId} @@ -65,7 +66,9 @@ type filterResponse struct { FilterID string `json:"filter_id"` } -//PutFilter implements POST /_matrix/client/r0/user/{userId}/filter +// PutFilter implements +// +// POST /_matrix/client/r0/user/{userId}/filter func PutFilter( req *http.Request, device *api.Device, syncDB storage.Database, userID string, ) util.JSONResponse { diff --git a/syncapi/storage/tables/interface.go b/syncapi/storage/tables/interface.go index d8fc8f508..d68351d4c 100644 --- a/syncapi/storage/tables/interface.go +++ b/syncapi/storage/tables/interface.go @@ -18,10 +18,11 @@ import ( "context" "database/sql" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/dendrite/internal/eventutil" "github.com/matrix-org/dendrite/roomserver/api" "github.com/matrix-org/dendrite/syncapi/types" - "github.com/matrix-org/gomatrixserverlib" ) type AccountData interface { @@ -122,12 +123,14 @@ type CurrentRoomState interface { // // We persist the previous event IDs as well, one per row, so when we do fetch even // earlier events we can simply delete rows which referenced it. Consider the graph: -// A -// | Event C has 1 prev_event ID: A. -// B C -// |___| Event D has 2 prev_event IDs: B and C. -// | -// D +// +// A +// | Event C has 1 prev_event ID: A. +// B C +// |___| Event D has 2 prev_event IDs: B and C. +// | +// D +// // The earliest known event we have is D, so this table has 2 rows. // A backfill request gives us C but not B. We delete rows where prev_event=C. This // still means that D is a backwards extremity as we do not have event B. However, event diff --git a/userapi/storage/sqlite3/stats_table.go b/userapi/storage/sqlite3/stats_table.go index e00ed417b..8aa1746c5 100644 --- a/userapi/storage/sqlite3/stats_table.go +++ b/userapi/storage/sqlite3/stats_table.go @@ -20,13 +20,14 @@ import ( "strings" "time" + "github.com/matrix-org/gomatrixserverlib" + "github.com/sirupsen/logrus" + "github.com/matrix-org/dendrite/internal" "github.com/matrix-org/dendrite/internal/sqlutil" "github.com/matrix-org/dendrite/userapi/api" "github.com/matrix-org/dendrite/userapi/storage/tables" "github.com/matrix-org/dendrite/userapi/types" - "github.com/matrix-org/gomatrixserverlib" - "github.com/sirupsen/logrus" ) const userDailyVisitsSchema = ` @@ -297,11 +298,10 @@ func (s *statsStatements) monthlyUsers(ctx context.Context, txn *sql.Tx) (result return } -/* R30Users counts the number of 30 day retained users, defined as: -- Users who have created their accounts more than 30 days ago -- Where last seen at most 30 days ago -- Where account creation and last_seen are > 30 days apart -*/ +// R30Users counts the number of 30 day retained users, defined as: +// - Users who have created their accounts more than 30 days ago +// - Where last seen at most 30 days ago +// - Where account creation and last_seen are > 30 days apart func (s *statsStatements) r30Users(ctx context.Context, txn *sql.Tx) (map[string]int64, error) { stmt := sqlutil.TxStmt(txn, s.countR30UsersStmt) lastSeenAfter := time.Now().AddDate(0, 0, -30) @@ -334,7 +334,8 @@ func (s *statsStatements) r30Users(ctx context.Context, txn *sql.Tx) (map[string return result, rows.Err() } -/* R30UsersV2 counts the number of 30 day retained users, defined as users that: +/* +R30UsersV2 counts the number of 30 day retained users, defined as users that: - Appear more than once in the past 60 days - Have more than 30 days between the most and least recent appearances that occurred in the past 60 days. */