Skip to content

Commit

Permalink
Handle story PK conflict when adding characters (#718)
Browse files Browse the repository at this point in the history 
When adding a character, if it is not already owned by the player a
story will always be added. This is not a problem for normal gameplay
logic, but several players have run into issues after removing
characters from their imported saves and leaving their stories behind.

Fix this by adding an extra check for a story in the UnitRepository.
Also misc. tidying up since that code is oold
  • Loading branch information
@SapiensAnatis
SapiensAnatis authored Mar 13, 2024
1 parent df781a7 commit ff2a0c9
Show file tree
Hide file tree
Showing 15 changed files with 254 additions and 186 deletions.
2 changes: 2 additions & 0 deletions DragaliaAPI/DragaliaAPI.Database.Test/DbTestFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public async Task AddToDatabase<TEntity>(TEntity data)

await this.ApiContext.AddAsync(data);
await this.ApiContext.SaveChangesAsync();
this.ApiContext.ChangeTracker.Clear();
}

public async Task AddRangeToDatabase<TEntity>(IEnumerable<TEntity> data)
Expand All @@ -63,6 +64,7 @@ public async Task AddRangeToDatabase<TEntity>(IEnumerable<TEntity> data)

await this.ApiContext.AddRangeAsync((IEnumerable<object>)data);
await this.ApiContext.SaveChangesAsync();
this.ApiContext.ChangeTracker.Clear();
}

public void Dispose()
Expand Down
156 changes: 107 additions & 49 deletions DragaliaAPI/DragaliaAPI.Database.Test/Repositories/UnitRepositoryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using DragaliaAPI.Shared.PlayerDetails;
using DragaliaAPI.Test.Utils;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using static DragaliaAPI.Database.Test.DbTestFixture;

namespace DragaliaAPI.Database.Test.Repositories;
Expand All @@ -20,6 +21,9 @@ public class UnitRepositoryTest : IClassFixture<DbTestFixture>
public UnitRepositoryTest(DbTestFixture fixture)
{
this.fixture = fixture;
this.fixture.ApiContext.Database.EnsureCreated();
this.fixture.ApiContext.Database.EnsureDeleted();

this.mockPlayerIdentityService = new(MockBehavior.Strict);
this.mockPlayerIdentityService.Setup(x => x.ViewerId).Returns(ViewerId);

Expand All @@ -28,11 +32,15 @@ public UnitRepositoryTest(DbTestFixture fixture)
this.mockPlayerIdentityService.Object,
LoggerTestUtils.Create<UnitRepository>()
);

this.fixture.ApiContext.ChangeTracker.Clear();
}

[Fact]
public async Task GetAllCharaData_ValidId_ReturnsData()
{
await this.fixture.AddToDatabase(new DbPlayerCharaData(1, Charas.Akasha));

(await this.unitRepository.Charas.ToListAsync()).Should().NotBeEmpty();
}

Expand All @@ -41,6 +49,8 @@ public async Task GetAllCharaData_InvalidId_ReturnsEmpty()
{
this.mockPlayerIdentityService.SetupGet(x => x.ViewerId).Returns(400);

await this.fixture.AddToDatabase(DbPlayerDragonDataFactory.Create(1, Dragons.Nyarlathotep));

(await this.unitRepository.Charas.ToListAsync()).Should().BeEmpty();
}

Expand All @@ -55,7 +65,7 @@ public async Task GetAllCharaData_ReturnsOnlyDataForGivenId()
}

[Fact]
public async Task GetAllDragonata_ValidId_ReturnsData()
public async Task GetAllDragonData_ValidId_ReturnsData()
{
await this.fixture.AddToDatabase(DbPlayerDragonDataFactory.Create(ViewerId, Dragons.Agni));
(await this.unitRepository.Dragons.ToListAsync()).Should().NotBeEmpty();
Expand All @@ -72,56 +82,14 @@ public async Task GetAllDragonData_InvalidId_ReturnsEmpty()
[Fact]
public async Task GetAllDragonData_ReturnsOnlyDataForGivenId()
{
await this.fixture.AddToDatabase(new DbPlayerCharaData(ViewerId, Charas.Ilia));
await this.fixture.AddToDatabase(new DbPlayerCharaData(244, Charas.Ilia));

(await this.unitRepository.Charas.ToListAsync())
.Should()
.AllSatisfy(x => x.ViewerId.Should().Be(ViewerId));
}

[Fact]
public async Task CheckHasCharas_OwnedList_ReturnsTrue()
{
IEnumerable<Charas> idList = await fixture
.ApiContext.PlayerCharaData.Where(x => x.ViewerId == ViewerId)
.Select(x => x.CharaId)
.ToListAsync();

(await this.unitRepository.CheckHasCharas(idList)).Should().BeTrue();
}

[Fact]
public async Task CheckHasCharas_NotAllOwnedList_ReturnsFalse()
{
IEnumerable<Charas> idList = (
await fixture
.ApiContext.PlayerCharaData.Where(x => x.ViewerId == ViewerId)
.Select(x => x.CharaId)
.ToListAsync()
).Append(Charas.BondforgedZethia);

(await this.unitRepository.CheckHasCharas(idList)).Should().BeFalse();
}

[Fact]
public async Task CheckHasDragons_OwnedList_ReturnsTrue()
{
await fixture.AddToDatabase(
DbPlayerDragonDataFactory.Create(ViewerId, Dragons.AC011Garland)
);
await fixture.AddToDatabase(DbPlayerDragonDataFactory.Create(ViewerId, Dragons.Ariel));

List<Dragons> idList = new() { Dragons.AC011Garland, Dragons.Ariel };

(await this.unitRepository.CheckHasDragons(idList)).Should().BeTrue();
}

[Fact]
public async Task CheckHasDragons_NotAllOwnedList_ReturnsFalse()
{
(await this.unitRepository.CheckHasDragons(new List<Dragons>() { Dragons.BronzeFafnir }))
.Should()
.BeFalse();
}

[Fact]
public async Task AddCharas_CorrectlyMarksDuplicates()
{
Expand Down Expand Up @@ -162,22 +130,112 @@ await this
);
}

[Fact]
public async Task AddCharas_HandlesExistingStory()
{
int izumoStoryId = MasterAsset.CharaStories[(int)Charas.Izumo].storyIds[0];
int mitsuhideStoryId = MasterAsset.CharaStories[(int)Charas.Mitsuhide].storyIds[0];
await this.fixture.AddRangeToDatabase(
[
new DbPlayerStoryState()
{
ViewerId = ViewerId,
StoryType = StoryTypes.Chara,
StoryId = izumoStoryId,
State = 0,
},
new DbPlayerStoryState()
{
ViewerId = ViewerId,
StoryType = StoryTypes.Dragon,
StoryId = mitsuhideStoryId,
State = 0,
},
new DbPlayerStoryState()
{
ViewerId = ViewerId + 1,
StoryType = StoryTypes.Chara,
StoryId = mitsuhideStoryId,
State = 0,
}
]
);

List<Charas> idList = [Charas.Izumo, Charas.Mitsuhide];

await this.unitRepository.AddCharas(idList);

await this.fixture.ApiContext.SaveChangesAsync();

this.fixture.ApiContext.PlayerStoryState.Should()
.ContainEquivalentOf(
new DbPlayerStoryState()
{
ViewerId = ViewerId,
StoryType = StoryTypes.Chara,
StoryId = izumoStoryId,
State = 0,
},
opts => opts.Excluding(x => x.Owner)
)
.And.ContainEquivalentOf(
new DbPlayerStoryState()
{
ViewerId = ViewerId,
StoryType = StoryTypes.Chara,
StoryId = mitsuhideStoryId,
State = 0,
},
opts => opts.Excluding(x => x.Owner)
);
}

[Fact]
public async Task AddDragons_CorrectlyMarksDuplicates()
{
await fixture.AddToDatabase(DbPlayerDragonDataFactory.Create(ViewerId, Dragons.Barbatos));

List<Dragons> idList = new() { Dragons.Marishiten, Dragons.Barbatos, Dragons.Marishiten };

IEnumerable<(Dragons id, bool isNew)> result = await this.unitRepository.AddDragons(idList);
IEnumerable<(Dragons Id, bool IsNew)> result = await this.unitRepository.AddDragons(idList);

result
.Where(x => x.isNew)
.Select(x => x.id)
.Where(x => x.IsNew)
.Select(x => x.Id)
.Should()
.BeEquivalentTo(new List<Dragons>() { Dragons.Marishiten });
}

[Fact]
public async Task AddDragons_HandlesExistingReliability()
{
await this.fixture.AddRangeToDatabase(
[
new DbPlayerDragonReliability()
{
ViewerId = ViewerId,
DragonId = Dragons.AC011Garland,
},
new DbPlayerDragonReliability()
{
ViewerId = ViewerId + 1,
DragonId = Dragons.Agni,
},
]
);

List<Dragons> idList = [Dragons.AC011Garland, Dragons.Agni];

await this.unitRepository.AddDragons(idList);
await this.fixture.ApiContext.SaveChangesAsync();

this.fixture.ApiContext.PlayerDragonReliability.Should()
.ContainEquivalentOf(
new DbPlayerDragonReliability() { ViewerId = ViewerId, DragonId = Dragons.Agni, },
opts => opts.Including(x => x.ViewerId).Including(x => x.DragonId)
);
}

[Fact]
public async Task AddDragons_UpdatesDatabase()
{
Expand Down
16 changes: 8 additions & 8 deletions DragaliaAPI/DragaliaAPI.Database.Test/packages.lock.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1556,7 +1556,7 @@
"Microsoft.IdentityModel.Tokens": "[7.3.1, )",
"Microsoft.VisualStudio.Azure.Containers.Tools.Targets": "[1.19.6, )",
"Microsoft.VisualStudio.Web.CodeGeneration.Design": "[8.0.1, )",
"MudBlazor": "[6.16.0, )",
"MudBlazor": "[6.17.0, )",
"Riok.Mapperly": "[3.5.0-next.1, )",
"Serilog": "[3.1.1, )",
"Serilog.AspNetCore": "[8.0.1, )",
Expand All @@ -1568,7 +1568,7 @@
"Serilog.Sinks.File": "[5.0.0, )",
"Serilog.Sinks.Seq": "[6.0.0, )",
"System.IdentityModel.Tokens.Jwt": "[7.2.0, )",
"System.Text.Json": "[8.0.1, )"
"System.Text.Json": "[8.0.2, )"
}
},
"dragaliaapi.database": {
Expand Down Expand Up @@ -1856,9 +1856,9 @@
},
"MudBlazor": {
"type": "CentralTransitive",
"requested": "[6.16.0, )",
"resolved": "6.16.0",
"contentHash": "xOgh6oPWXVcjJ+8tzPniXLlz64skRpPgh5x/jY5s54Xy0fy7/WVWI5YwKirvJgJAl8CvJzikvEpNXVXTAStCqA==",
"requested": "[6.17.0, )",
"resolved": "6.17.0",
"contentHash": "f5WmJxdqvKK9zdk/3s80OLxeA80qWzzXlTasy0NKZ1eUxH6SdRDvfURDKj9H1pJ6Simc78pMDxpZXdfxoOVy9A==",
"dependencies": {
"Microsoft.AspNetCore.Components": "8.0.2",
"Microsoft.AspNetCore.Components.Web": "8.0.2",
Expand Down Expand Up @@ -2001,9 +2001,9 @@
},
"System.Text.Json": {
"type": "CentralTransitive",
"requested": "[8.0.1, )",
"resolved": "8.0.1",
"contentHash": "7AWk2za1hSEJBppe/Lg+uDcam2TrDqwIKa9XcPssSwyjC2xa39EKEGul3CO5RWNF+hMuZG4zlBDrvhBdDTg4lg==",
"requested": "[8.0.2, )",
"resolved": "8.0.2",
"contentHash": "29KA2StjWDYp32FvREifawRtNpTziLE1xyuDV9pVQ+MsaE9bIcIieP0io/eZZeLMxR+Nx9zI55RtUtpVpEIdeg==",
"dependencies": {
"System.Text.Encodings.Web": "8.0.0"
}
Expand Down
4 changes: 4 additions & 0 deletions DragaliaAPI/DragaliaAPI.Database/ApiContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
modelBuilder
.Entity<DbSummonTicket>()
.HasQueryFilter(x => x.ViewerId == this.playerIdentityService.ViewerId);

modelBuilder
.Entity<DbPlayerStoryState>()
.HasQueryFilter(x => x.ViewerId == this.playerIdentityService.ViewerId);
}
}
6 changes: 1 addition & 5 deletions DragaliaAPI/DragaliaAPI.Database/Repositories/IUnitRepository.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,11 @@ public interface IUnitRepository
IQueryable<DbAbilityCrest> AbilityCrests { get; }
IQueryable<DbTalisman> Talismans { get; }

Task<bool> CheckHasCharas(IEnumerable<Charas> idList);

Task<bool> CheckHasDragons(IEnumerable<Dragons> idList);

Task<IEnumerable<(Charas id, bool isNew)>> AddCharas(IEnumerable<Charas> idList);

Task<bool> AddCharas(Charas id);

Task<IEnumerable<(Dragons id, bool isNew)>> AddDragons(IEnumerable<Dragons> idList);
Task<IEnumerable<(Dragons Id, bool IsNew)>> AddDragons(IEnumerable<Dragons> idList);

Task<bool> AddDragons(Dragons id);

Expand Down
Loading

0 comments on commit ff2a0c9

Please sign in to comment.