From 4ce504a1e16b0416c0a8ad15ec24e1350e433e7d Mon Sep 17 00:00:00 2001 From: "nicolas.dorier" Date: Sat, 19 Nov 2022 23:39:41 +0900 Subject: [PATCH] Add index to WalletObjects + allow additional queries --- BTCPayServer.Data/Data/WalletObjectData.cs | 6 ++ .../Migrations/20220929132704_label.cs | 4 + .../ApplicationDbContextModelSnapshot.cs | 4 +- BTCPayServer.Tests/GreenfieldAPITests.cs | 17 +++- BTCPayServer/Services/WalletRepository.cs | 92 ++++++++++++++----- 5 files changed, 95 insertions(+), 28 deletions(-) diff --git a/BTCPayServer.Data/Data/WalletObjectData.cs b/BTCPayServer.Data/Data/WalletObjectData.cs index 9bbb8af4b..dbbb1ed4b 100644 --- a/BTCPayServer.Data/Data/WalletObjectData.cs +++ b/BTCPayServer.Data/Data/WalletObjectData.cs @@ -69,6 +69,12 @@ namespace BTCPayServer.Data o.Type, o.Id, }); + builder.Entity().HasIndex(o => + new + { + o.Type, + o.Id + }); if (databaseFacade.IsNpgsql()) { diff --git a/BTCPayServer.Data/Migrations/20220929132704_label.cs b/BTCPayServer.Data/Migrations/20220929132704_label.cs index 118028c8b..7d67d16f2 100644 --- a/BTCPayServer.Data/Migrations/20220929132704_label.cs +++ b/BTCPayServer.Data/Migrations/20220929132704_label.cs @@ -30,6 +30,10 @@ namespace BTCPayServer.Migrations { table.PrimaryKey("PK_WalletObjects", x => new { x.WalletId, x.Type, x.Id }); }); + migrationBuilder.CreateIndex( + name: "IX_WalletObjects_Type_Id", + table: "WalletObjects", + columns: new[] { "Type", "Id" }); migrationBuilder.CreateTable( name: "WalletObjectLinks", diff --git a/BTCPayServer.Data/Migrations/ApplicationDbContextModelSnapshot.cs b/BTCPayServer.Data/Migrations/ApplicationDbContextModelSnapshot.cs index 305d08afe..79e3591e1 100644 --- a/BTCPayServer.Data/Migrations/ApplicationDbContextModelSnapshot.cs +++ b/BTCPayServer.Data/Migrations/ApplicationDbContextModelSnapshot.cs @@ -16,7 +16,7 @@ namespace BTCPayServer.Migrations protected override void BuildModel(ModelBuilder modelBuilder) { #pragma warning disable 612, 618 - modelBuilder.HasAnnotation("ProductVersion", "6.0.7"); + modelBuilder.HasAnnotation("ProductVersion", "6.0.9"); modelBuilder.Entity("BTCPayServer.Data.AddressInvoiceData", b => { @@ -862,6 +862,8 @@ namespace BTCPayServer.Migrations b.HasKey("WalletId", "Type", "Id"); + b.HasIndex("Type", "Id"); + b.ToTable("WalletObjects"); }); diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs index 9181c34d3..8fcb4ad21 100644 --- a/BTCPayServer.Tests/GreenfieldAPITests.cs +++ b/BTCPayServer.Tests/GreenfieldAPITests.cs @@ -3075,19 +3075,26 @@ namespace BTCPayServer.Tests // Only the node `test` `test` is connected to `test1` var wid = new WalletId(admin.StoreId, "BTC"); var repo = tester.PayTester.GetService(); - var allObjects = await repo.GetWalletObjects((GetWalletObjectsQuery)(new(wid, null) { UseInefficientPath = useInefficient })); - var allTests = await repo.GetWalletObjects((GetWalletObjectsQuery)(new(wid, "test") { UseInefficientPath = useInefficient })); - var twoTests2 = await repo.GetWalletObjects((GetWalletObjectsQuery)(new(wid, "test", new[] { "test1", "test2", "test-unk" }) { UseInefficientPath = useInefficient })); - var oneTest = await repo.GetWalletObjects((GetWalletObjectsQuery)(new(wid, "test", new[] { "test" }) { UseInefficientPath = useInefficient })); - var oneTestWithoutData = await repo.GetWalletObjects((GetWalletObjectsQuery)(new(wid, "test", new[] { "test" }) { UseInefficientPath = useInefficient, IncludeNeighbours = false })); + var allObjects = await repo.GetWalletObjects((new(wid, null) { UseInefficientPath = useInefficient })); + var allObjectsNoWallet = await repo.GetWalletObjects((new() { UseInefficientPath = useInefficient })); + var allObjectsNoWalletAndType = await repo.GetWalletObjects((new() { Type = "test", UseInefficientPath = useInefficient })); + var allTests = await repo.GetWalletObjects((new(wid, "test") { UseInefficientPath = useInefficient })); + var twoTests2 = await repo.GetWalletObjects((new(wid, "test", new[] { "test1", "test2", "test-unk" }) { UseInefficientPath = useInefficient })); + var oneTest = await repo.GetWalletObjects((new(wid, "test", new[] { "test" }) { UseInefficientPath = useInefficient })); + var oneTestWithoutData = await repo.GetWalletObjects((new(wid, "test", new[] { "test" }) { UseInefficientPath = useInefficient, IncludeNeighbours = false })); + var idsTypes = await repo.GetWalletObjects((new(wid) { TypesIds = new[] { new ObjectTypeId("test", "test1"), new ObjectTypeId("test", "test2") }, UseInefficientPath = useInefficient })); Assert.Equal(4, allObjects.Count); + // We are reusing a db in this test, as such we may have other wallets here. + Assert.True(allObjectsNoWallet.Count >= 4); + Assert.True(allObjectsNoWalletAndType.Count >= 4); Assert.Equal(3, allTests.Count); Assert.Equal(2, twoTests2.Count); Assert.Single(oneTest); Assert.NotNull(oneTest.First().Value.GetNeighbours().Select(n => n.Data).FirstOrDefault()); Assert.Single(oneTestWithoutData); Assert.Null(oneTestWithoutData.First().Value.GetNeighbours().Select(n => n.Data).FirstOrDefault()); + Assert.Equal(2, idsTypes.Count); } await TestWalletRepository(false); await TestWalletRepository(true); diff --git a/BTCPayServer/Services/WalletRepository.cs b/BTCPayServer/Services/WalletRepository.cs index d2d20260b..b71cdb943 100644 --- a/BTCPayServer/Services/WalletRepository.cs +++ b/BTCPayServer/Services/WalletRepository.cs @@ -14,32 +14,44 @@ namespace BTCPayServer.Services { #nullable enable public record WalletObjectId(WalletId WalletId, string Type, string Id); + public record ObjectTypeId(string Type, string Id); public class GetWalletObjectsQuery { - public GetWalletObjectsQuery(WalletId walletId) : this(walletId, null, null) + public GetWalletObjectsQuery() + { + + } + public GetWalletObjectsQuery(WalletId? walletId) : this(walletId, null, null) { } public GetWalletObjectsQuery(WalletObjectId walletObjectId) : this(walletObjectId.WalletId, walletObjectId.Type, new[] { walletObjectId.Id }) { } - public GetWalletObjectsQuery(WalletId walletId, string type) : this (walletId, type, null) + public GetWalletObjectsQuery(WalletId? walletId, string type) : this(walletId, type, null) { } - public GetWalletObjectsQuery(WalletId walletId, string? type, string[]? ids) + public GetWalletObjectsQuery(WalletId? walletId, string? type, string[]? ids) { - ArgumentNullException.ThrowIfNull(walletId); WalletId = walletId; Type = type; Ids = ids; } + public GetWalletObjectsQuery(ObjectTypeId[]? typesIds) + { + TypesIds = typesIds; + } - public WalletId WalletId { get; set; } + public WalletId? WalletId { get; set; } + // Either the user passes a list of Types/Ids + public ObjectTypeId[]? TypesIds { get; set; } + // Or the user passes one type, and a list of Ids public string? Type { get; set; } public string[]? Ids { get; set; } public bool IncludeNeighbours { get; set; } = true; public bool UseInefficientPath { get; set; } } + #nullable restore public class WalletRepository { @@ -60,6 +72,10 @@ namespace BTCPayServer.Services ArgumentNullException.ThrowIfNull(queryObject); if (queryObject.Ids != null && queryObject.Type is null) throw new ArgumentException("If \"Ids\" is not null, \"Type\" is mandatory"); + if (queryObject.Type is not null && queryObject.TypesIds is not null) + throw new ArgumentException("If \"Type\" is not null, \"TypesIds\" should be null"); + + using var ctx = _ContextFactory.CreateContext(); // If we are using postgres, the `transactionIds.Contains(w.ChildId)` result in a long query like `ANY(@txId1, @txId2, @txId3, @txId4)` @@ -73,30 +89,36 @@ namespace BTCPayServer.Services if (connection.State != System.Data.ConnectionState.Open) await connection.OpenAsync(); - string typeFilter = queryObject.Type is not null ? "AND wos.\"Type\"=@type " : ""; + string walletIdFilter = queryObject.WalletId is not null ? " AND wos.\"WalletId\"=@walletId" : ""; + string typeFilter = queryObject.Type is not null ? " AND wos.\"Type\"=@type" : ""; var cmd = connection.CreateCommand(); var selectWalletObjects = + queryObject.TypesIds is not null ? + $"SELECT wos.* FROM unnest(@ids, @types) t(i,t) JOIN \"WalletObjects\" wos ON true{walletIdFilter} AND wos.\"Type\"=t AND wos.\"Id\"=i" : queryObject.Ids is null ? - $"SELECT wos.* FROM \"WalletObjects\" wos WHERE wos.\"WalletId\"=@walletId {typeFilter}" : + $"SELECT wos.* FROM \"WalletObjects\" wos WHERE true{walletIdFilter}{typeFilter} " : queryObject.Ids.Length == 1 ? - "SELECT wos.* FROM \"WalletObjects\" wos WHERE wos.\"WalletId\"=@walletId AND wos.\"Type\"=@type AND wos.\"Id\"=@id" : - "SELECT wos.* FROM unnest(@ids) t JOIN \"WalletObjects\" wos ON wos.\"WalletId\"=@walletId AND wos.\"Type\"=@type AND wos.\"Id\"=t"; + $"SELECT wos.* FROM \"WalletObjects\" wos WHERE true{walletIdFilter} AND wos.\"Type\"=@type AND wos.\"Id\"=@id" : + $"SELECT wos.* FROM unnest(@ids) t JOIN \"WalletObjects\" wos ON true{walletIdFilter} AND wos.\"Type\"=@type AND wos.\"Id\"=t"; var includeNeighbourSelect = queryObject.IncludeNeighbours ? ", wos2.\"Data\" AS \"Data2\"" : ""; var includeNeighbourJoin = queryObject.IncludeNeighbours ? "LEFT JOIN \"WalletObjects\" wos2 ON wos.\"WalletId\"=wos2.\"WalletId\" AND wol.\"Type2\"=wos2.\"Type\" AND wol.\"Id2\"=wos2.\"Id\"" : ""; var query = - $"SELECT wos.\"Id\", wos.\"Type\", wos.\"Data\", wol.\"LinkData\", wol.\"Type2\", wol.\"Id2\"{includeNeighbourSelect} FROM ({selectWalletObjects}) wos " + + $"SELECT wos.\"WalletId\", wos.\"Id\", wos.\"Type\", wos.\"Data\", wol.\"LinkData\", wol.\"Type2\", wol.\"Id2\"{includeNeighbourSelect} FROM ({selectWalletObjects}) wos " + $"LEFT JOIN LATERAL ( " + "SELECT \"ParentType\" AS \"Type2\", \"ParentId\" AS \"Id2\", \"Data\" AS \"LinkData\" FROM \"WalletObjectLinks\" WHERE \"WalletId\"=wos.\"WalletId\" AND \"ChildType\"=wos.\"Type\" AND \"ChildId\"=wos.\"Id\" " + "UNION " + "SELECT \"ChildType\" AS \"Type2\", \"ChildId\" AS \"Id2\", \"Data\" AS \"LinkData\" FROM \"WalletObjectLinks\" WHERE \"WalletId\"=wos.\"WalletId\" AND \"ParentType\"=wos.\"Type\" AND \"ParentId\"=wos.\"Id\"" + $" ) wol ON true " + includeNeighbourJoin; cmd.CommandText = query; - var walletIdParam = cmd.CreateParameter(); - walletIdParam.ParameterName = "walletId"; - walletIdParam.Value = queryObject.WalletId.ToString(); - walletIdParam.DbType = System.Data.DbType.String; - cmd.Parameters.Add(walletIdParam); + if (queryObject.WalletId is not null) + { + var walletIdParam = cmd.CreateParameter(); + walletIdParam.ParameterName = "walletId"; + walletIdParam.Value = queryObject.WalletId.ToString(); + walletIdParam.DbType = System.Data.DbType.String; + cmd.Parameters.Add(walletIdParam); + } if (queryObject.Type != null) { @@ -107,6 +129,20 @@ namespace BTCPayServer.Services cmd.Parameters.Add(typeParam); } + if (queryObject.TypesIds != null) + { + var typesParam = cmd.CreateParameter(); + typesParam.ParameterName = "types"; + typesParam.Value = queryObject.TypesIds.Select(t => t.Type).ToList(); + typesParam.DbType = System.Data.DbType.Object; + cmd.Parameters.Add(typesParam); + var idParam = cmd.CreateParameter(); + idParam.ParameterName = "ids"; + idParam.Value = queryObject.TypesIds.Select(t => t.Id).ToList(); + idParam.DbType = System.Data.DbType.Object; + cmd.Parameters.Add(idParam); + } + if (queryObject.Ids != null) { if (queryObject.Ids.Length == 1) @@ -121,7 +157,7 @@ namespace BTCPayServer.Services { var txIdsParam = cmd.CreateParameter(); txIdsParam.ParameterName = "ids"; - txIdsParam.Value = queryObject.Ids.ToHashSet().ToList(); + txIdsParam.Value = queryObject.Ids.ToList(); txIdsParam.DbType = System.Data.DbType.Object; cmd.Parameters.Add(txIdsParam); } @@ -131,9 +167,10 @@ namespace BTCPayServer.Services while (await reader.ReadAsync()) { WalletObjectData wo = new WalletObjectData(); + wo.WalletId = (string)reader["WalletId"]; wo.Type = (string)reader["Type"]; wo.Id = (string)reader["Id"]; - var id = new WalletObjectId(queryObject.WalletId, wo.Type, wo.Id); + var id = new WalletObjectId(WalletId.Parse(wo.WalletId), wo.Type, wo.Id); wo.Data = reader["Data"] is DBNull ? null : (string)reader["Data"]; if (wosById.TryGetValue(id, out var wo2)) wo = wo2; @@ -163,8 +200,19 @@ namespace BTCPayServer.Services } else // Unefficient path { - var q = ctx.WalletObjects - .Where(w => w.WalletId == queryObject.WalletId.ToString() && (queryObject.Type == null || w.Type == queryObject.Type) && (queryObject.Ids == null || queryObject.Ids.Contains(w.Id))); + IQueryable q; + if (queryObject.TypesIds is not null) + { + // Note this is problematic if the type contains '##', but I don't see how to do it properly with entity framework + var idTypes = queryObject.TypesIds.Select(o => $"{o.Type}##{o.Id}").ToArray(); + q = ctx.WalletObjects + .Where(w => (queryObject.WalletId == null || w.WalletId == queryObject.WalletId.ToString()) && idTypes.Contains(w.Type + "##" + w.Id)); + } + else + { + q = ctx.WalletObjects + .Where(w => (queryObject.WalletId == null || w.WalletId == queryObject.WalletId.ToString()) && (queryObject.Type == null || w.Type == queryObject.Type) && (queryObject.Ids == null || queryObject.Ids.Contains(w.Id))); + } if (queryObject.IncludeNeighbours) { q = q.Include(o => o.ChildLinks).ThenInclude(o => o.Child) @@ -175,7 +223,7 @@ namespace BTCPayServer.Services var wosById = new Dictionary(); foreach (var row in await q.ToListAsync()) { - var id = new WalletObjectId(queryObject.WalletId, row.Type, row.Id); + var id = new WalletObjectId(WalletId.Parse(row.WalletId), row.Type, row.Id); wosById.TryAdd(id, row); } return wosById; @@ -216,7 +264,7 @@ namespace BTCPayServer.Services public async Task<(string Label, string Color)[]> GetWalletLabels(WalletId walletId) { await using var ctx = _ContextFactory.CreateContext(); - return (await + return (await ctx.WalletObjects.AsNoTracking().Where(w => w.WalletId == walletId.ToString() && w.Type == WalletObjectData.Types.Label) .ToArrayAsync()) .Select(o => (o.Id, JObject.Parse(o.Data)["color"]!.Value()!)).ToArray(); @@ -291,7 +339,7 @@ namespace BTCPayServer.Services public async Task SetWalletObjectLink(WalletObjectId a, WalletObjectId b, JObject? data = null) { SortWalletObjectLinks(ref a, ref b); - + await using var ctx = _ContextFactory.CreateContext(); var l = new WalletObjectLinkData()