Skip to content

Commit

Permalink
Generate a new RouteSegment for each player identity, unique within t…
Browse files Browse the repository at this point in the history
…he team #626
  • Loading branch information
sussexrick committed Dec 11, 2022
1 parent 869d1e2 commit b7a7cf8
Show file tree
Hide file tree
Showing 16 changed files with 186 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -564,16 +564,17 @@ public void CreateUmbracoBaseRecords()
public void CreatePlayerIdentity(PlayerIdentity playerIdentity)
{
_connection.Execute($@"INSERT INTO {Tables.PlayerIdentity}
(PlayerIdentityId, PlayerId, TeamId, PlayerIdentityName, ComparableName)
(PlayerIdentityId, PlayerId, TeamId, PlayerIdentityName, ComparableName, RouteSegment)
VALUES
(@PlayerIdentityId, @PlayerId, @TeamId, @PlayerIdentityName, @ComparableName)",
(@PlayerIdentityId, @PlayerId, @TeamId, @PlayerIdentityName, @ComparableName, @RouteSegment)",
new
{
playerIdentity.PlayerIdentityId,
playerIdentity.Player.PlayerId,
playerIdentity.Team.TeamId,
playerIdentity.PlayerIdentityName,
ComparableName = playerIdentity.ComparableName()
ComparableName = playerIdentity.ComparableName(),
playerIdentity.RouteSegment
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ public async Task Read_player_identities_returns_basic_fields()
Assert.NotNull(result);

Assert.Equal(identity.PlayerIdentityName, result!.PlayerIdentityName);
Assert.Equal(identity.RouteSegment, result!.RouteSegment);
Assert.Equal(identity.TotalMatches, result.TotalMatches);
Assert.Equal(identity.FirstPlayed!.Value.AccurateToTheMinute(), result.FirstPlayed!.Value.AccurateToTheMinute());
Assert.Equal(identity.LastPlayed!.Value.AccurateToTheMinute(), result.LastPlayed!.Value.AccurateToTheMinute());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ private class PlayerIdentityResult
public Guid? PlayerId { get; set; }
public string? PlayerIdentityName { get; set; }
public string? ComparableName { get; set; }
public string? RouteSegment { get; set; }
public Guid? TeamId { get; set; }
}

Expand All @@ -183,6 +184,7 @@ public async Task Unmatched_PlayerIdentity_returns_new_player_and_identity()
_copier.Setup(x => x.CreateAuditableCopy(playerIdentity)).Returns(playerIdentity);
_playerNameFormatter.Setup(x => x.CapitaliseName(playerIdentity.PlayerIdentityName)).Returns(playerIdentity.PlayerIdentityName);
_routeGenerator.Setup(x => x.GenerateUniqueRoute("/players", playerIdentity.PlayerIdentityName, NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>())).Returns(Task.FromResult(playerRoute));
_routeGenerator.Setup(x => x.GenerateUniqueRoute(string.Empty, playerIdentity.PlayerIdentityName.Kebaberize(), NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>())).Returns(Task.FromResult(playerIdentity.PlayerIdentityName.Kebaberize()));

var repo = CreateRepository();

Expand All @@ -194,13 +196,14 @@ public async Task Unmatched_PlayerIdentity_returns_new_player_and_identity()
_routeGenerator.Verify(x => x.GenerateUniqueRoute("/players", playerIdentity.PlayerIdentityName, NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>()));

var identityResult = await transaction.Connection.QuerySingleAsync<PlayerIdentityResult>(
$"SELECT PlayerId, PlayerIdentityName, ComparableName, TeamId FROM {Tables.PlayerIdentity} WHERE PlayerIdentityName = @PlayerIdentityName",
$"SELECT PlayerId, PlayerIdentityName, ComparableName, RouteSegment, TeamId FROM {Tables.PlayerIdentity} WHERE PlayerIdentityName = @PlayerIdentityName",
new { playerIdentity.PlayerIdentityName },
transaction);

Assert.NotNull(identityResult);
Assert.Equal(playerIdentity.PlayerIdentityName, identityResult.PlayerIdentityName);
Assert.Equal(playerIdentity.ComparableName(), identityResult.ComparableName);
Assert.Equal(playerIdentity.PlayerIdentityName.Kebaberize(), identityResult.RouteSegment);
Assert.Equal(playerIdentity.Team.TeamId, identityResult.TeamId);

var playerResult = await transaction.Connection.QuerySingleAsync<Player>(
Expand Down Expand Up @@ -228,6 +231,7 @@ public async Task CreateOrMatchPlayerIdentity_audits_and_logs()
_copier.Setup(x => x.CreateAuditableCopy(playerIdentity)).Returns(playerIdentity);
_playerNameFormatter.Setup(x => x.CapitaliseName(playerIdentity.PlayerIdentityName)).Returns(playerIdentity.PlayerIdentityName);
_routeGenerator.Setup(x => x.GenerateUniqueRoute("/players", playerIdentity.PlayerIdentityName, NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>())).Returns(Task.FromResult($"/players/{Guid.NewGuid()}"));
_routeGenerator.Setup(x => x.GenerateUniqueRoute(string.Empty, playerIdentity.PlayerIdentityName.Kebaberize(), NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>())).Returns(Task.FromResult(playerIdentity.PlayerIdentityName.Kebaberize()));
var memberName = "Member name";
var memberKey = Guid.NewGuid();

Expand Down
Binary file not shown.
Binary file not shown.
12 changes: 8 additions & 4 deletions Stoolball.Data.SqlServer/SqlServerPlayerDataSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,18 @@ public async Task<List<PlayerIdentity>> ReadPlayerIdentities(PlayerFilter? filte
{
using (var connection = _databaseConnectionFactory.CreateDatabaseConnection())
{
// Get PlayerIdentity data from the original tables rather than PlayerInMatchStatistics because the original tables
// will be updated when a player identity is renamed, and we need to see the change immediately.
// Updates to PlayerInMatchStatistics are done asynchronously and the data will not be updated by the time this is called again.

const string PROBABILITY_CALCULATION = "COUNT(DISTINCT MatchId)*10-DATEDIFF(DAY,MAX(MatchStartTime),GETDATE())";
var sql = $@"SELECT stats.PlayerIdentityId, stats.PlayerIdentityName, {PROBABILITY_CALCULATION} AS Probability,
var sql = $@"SELECT stats.PlayerIdentityId, pi.PlayerIdentityName, pi.RouteSegment, {PROBABILITY_CALCULATION} AS Probability,
COUNT(DISTINCT MatchId) AS TotalMatches, MIN(MatchStartTime) AS FirstPlayed, MAX(MatchStartTime) AS LastPlayed,
stats.PlayerId, stats.PlayerRoute, stats.TeamId, stats.TeamName
FROM {Tables.PlayerInMatchStatistics} AS stats
FROM {Tables.PlayerIdentity} pi INNER JOIN {Tables.PlayerInMatchStatistics} AS stats ON pi.PlayerIdentityId = stats.PlayerIdentityId
<<WHERE>>
GROUP BY stats.PlayerId, stats.PlayerRoute, stats.PlayerIdentityId, stats.PlayerIdentityName, stats.TeamId, stats.TeamName
ORDER BY stats.TeamId ASC, {PROBABILITY_CALCULATION} DESC, stats.PlayerIdentityName ASC";
GROUP BY stats.PlayerId, stats.PlayerRoute, stats.PlayerIdentityId, pi.PlayerIdentityName, pi.RouteSegment, stats.TeamId, stats.TeamName
ORDER BY stats.TeamId ASC, {PROBABILITY_CALCULATION} DESC, pi.PlayerIdentityName ASC";

var (where, parameters) = BuildWhereClause(filter);
sql = sql.Replace("<<WHERE>>", $"WHERE 1=1 {where}");
Expand Down
15 changes: 10 additions & 5 deletions Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using System.Threading.Tasks;
using Dapper;
using Humanizer;
using Newtonsoft.Json;
using Stoolball.Data.Abstractions;
using Stoolball.Logging;
Expand All @@ -29,7 +30,7 @@ public class SqlServerPlayerRepository : IPlayerRepository
private readonly IPlayerNameFormatter _playerNameFormatter;
private readonly IBestRouteSelector _bestRouteSelector;
private readonly IPlayerCacheInvalidator _playerCacheClearer;
internal const string PROCESS_ASYNC_STORED_PROCEDURE = "usp_Link_Player_To_Member_Async_Update";
internal const string PROCESS_ASYNC_STORED_PROCEDURE = "usp_Player_Async_Update";
internal const string LOG_TEMPLATE_WARN_SQL_TIMEOUT = nameof(ProcessAsyncUpdatesForLinkingAndUnlinkingPlayersToMemberAccounts) + " running. Caught SQL Timeout. {allowedRetries} retries remaining";
internal const string LOG_TEMPLATE_INFO_PLAYERS_AFFECTED = nameof(ProcessAsyncUpdatesForLinkingAndUnlinkingPlayersToMemberAccounts) + " running. Players affected: {affectedRoutes}";
internal const string LOG_TEMPLATE_ERROR_SQL_EXCEPTION = nameof(ProcessAsyncUpdatesForLinkingAndUnlinkingPlayersToMemberAccounts) + " threw SqlException: {message}";
Expand Down Expand Up @@ -119,13 +120,16 @@ public async Task<PlayerIdentity> CreateOrMatchPlayerIdentity(PlayerIdentity pla

auditablePlayerIdentity.PlayerIdentityId = Guid.NewGuid();
auditablePlayerIdentity.PlayerIdentityName = _playerNameFormatter.CapitaliseName(auditablePlayerIdentity.PlayerIdentityName);
auditablePlayerIdentity.RouteSegment = (await _routeGenerator.GenerateUniqueRoute(string.Empty, auditablePlayerIdentity.PlayerIdentityName.Kebaberize(), NoiseWords.PlayerRoute,
async route => await transaction.Connection.ExecuteScalarAsync<int>($"SELECT COUNT(*) FROM {Tables.PlayerIdentity} WHERE RouteSegment = @RouteSegment AND TeamId = @TeamId", new { RouteSegment = route, auditablePlayerIdentity.Team.TeamId }, transaction)
).ConfigureAwait(false))?.TrimStart('/');

var player = new Player { PlayerId = Guid.NewGuid() };
player.PlayerIdentities.Add(auditablePlayerIdentity);

player.PlayerRoute = await _routeGenerator.GenerateUniqueRoute($"/players", auditablePlayerIdentity.PlayerIdentityName, NoiseWords.PlayerRoute,
async route => await transaction.Connection.ExecuteScalarAsync<int>($"SELECT COUNT(*) FROM {Tables.Player} WHERE PlayerRoute = @PlayerRoute", new { PlayerRoute = route }, transaction)
);
).ConfigureAwait(false);

await transaction.Connection.ExecuteAsync(
$@"INSERT INTO {Tables.Player}
Expand All @@ -139,14 +143,15 @@ await transaction.Connection.ExecuteAsync(
}, transaction);

await transaction.Connection.ExecuteAsync($@"INSERT INTO {Tables.PlayerIdentity}
(PlayerIdentityId, PlayerId, PlayerIdentityName, ComparableName, TeamId)
VALUES (@PlayerIdentityId, @PlayerId, @PlayerIdentityName, @ComparableName, @TeamId)",
(PlayerIdentityId, PlayerId, PlayerIdentityName, ComparableName, RouteSegment, TeamId)
VALUES (@PlayerIdentityId, @PlayerId, @PlayerIdentityName, @ComparableName, @RouteSegment, @TeamId)",
new
{
auditablePlayerIdentity.PlayerIdentityId,
player.PlayerId,
auditablePlayerIdentity.PlayerIdentityName,
ComparableName = auditablePlayerIdentity.ComparableName(),
auditablePlayerIdentity.RouteSegment,
auditablePlayerIdentity.Team.TeamId
}, transaction);

Expand Down Expand Up @@ -387,7 +392,7 @@ public async Task ProcessAsyncUpdatesForLinkingAndUnlinkingPlayersToMemberAccoun
try
{
retry = false;
affectedRoutes = await _dapperWrapper.QueryAsync<string>("usp_Link_Player_To_Member_Async_Update", commandType: CommandType.StoredProcedure, connection: connection).ConfigureAwait(false);
affectedRoutes = await _dapperWrapper.QueryAsync<string>(PROCESS_ASYNC_STORED_PROCEDURE, commandType: CommandType.StoredProcedure, connection: connection).ConfigureAwait(false);
foreach (var route in affectedRoutes)
{
_playerCacheClearer.InvalidateCacheForPlayer(new Player { PlayerRoute = route });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using Microsoft.Extensions.Logging;
using Stoolball.Data.SqlServer;
using Umbraco.Cms.Infrastructure.Migrations;

namespace Stoolball.Data.UmbracoMigrations
{
/// <summary>
/// Adds a RouteSegment column to StoolballPlayerIdentity which is used for routes that edit a single identity rather than a player
/// </summary>
public partial class AddRouteSegmentToPlayerIdentity : MigrationBase
{
public AddRouteSegmentToPlayerIdentity(IMigrationContext context) : base(context)
{
}

protected override void Migrate()
{
Logger.LogDebug("Running migration {MigrationStep}", typeof(AddRouteSegmentToPlayerIdentity).Name);

if (!ColumnExists(Tables.PlayerIdentity, "RouteSegment"))
{
Create.Column("RouteSegment").OnTable(Tables.PlayerIdentity).AsString(255).Nullable().Do();
Execute.SqlFromFile("042_Add_PlayerIdentity_RouteSegment.StoolballPlayerIdentity_PopulateRouteSegment.sql").Do();
Execute.SqlFromFile("042_Add_PlayerIdentity_RouteSegment.usp_Player_Async_Update.sql").Do();
}
else
{
Logger.LogDebug("The database column {DbTable}.{DbColumn} already exists, skipping", Tables.PlayerIdentity, "RouteSegment");
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
UPDATE StoolballPlayerIdentity
SET StoolballPlayerIdentity.RouteSegment = Results.RouteSegment
FROM StoolballPlayerIdentity
INNER JOIN (
SELECT PlayerIdentityId, CASE WHEN RouteSegment = '' THEN CAST(PlayerIdentityId AS varchar(255)) ELSE RouteSegment END AS RouteSegment FROM
(
SELECT PlayerIdentityId,
CASE
WHEN RIGHT(RouteSegment,1) = '-' THEN LEFT(RouteSegment, LEN(RouteSegment)-1)
ELSE RouteSegment
END
AS RouteSegment FROM (
SELECT PlayerIdentityId, LOWER(LTRIM(RTRIM(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(PlayerIdentityName,' ','-'),'''',''),'`',''),'&',''),'.',''),'(',''),')',''),'/','-'),'#',''),';',''),':',''),'?',''),'|','')))) AS RouteSegment
FROM StoolballPlayerIdentity
) AS Results
) AS Results
) AS Results
ON StoolballPlayerIdentity.PlayerIdentityId = Results.PlayerIdentityId;

ALTER TABLE StoolballPlayerIdentity
ALTER COLUMN RouteSegment nvarchar(255) NOT NULL
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
DROP PROCEDURE [dbo].[usp_Link_Player_To_Member_Async_Update]
GO

-- =============================================
-- Author: Rick Mason
-- Create date: 17 Sept 2022
-- Description: Checks for outstanding work following linking/unlinking a player to a member or renaming a player identity, and completes a batch of that work.
-- This is called asynchronously rather than at the time of linking/unlinking/renaming the player to avoid a slow update of
-- StoolballPlayerInMatchStatistics causing SQL timeouts in production.
-- =============================================
CREATE OR ALTER PROCEDURE [dbo].[usp_Player_Async_Update]
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;

BEGIN TRAN

DECLARE @DoStatisticsUpdate bit

-- Check for a batch of up to 10 records that need to be updated with a changed details for a PlayerIdentity.
-- The limit of 10 is to mitigate the risk of SQL timeouts updating the StoolballPlayerInMatchStatistics table in production,
-- because the table is heavily indexed and updates can be slow.
-- SELECT into a temp table with a separate UPDATE, rather than an all in one UPDATE...FROM statement, is the only way to limit to 10.
SELECT TOP 10 PlayerInMatchStatisticsId, p.PlayerId, p.PlayerRoute, pi.PlayerIdentityName, s.PlayerRoute AS PlayerRouteToReplace
INTO #LinkPlayerToMemberAsyncUpdate
FROM StoolballPlayerInMatchStatistics s
INNER JOIN StoolballPlayerIdentity pi ON s.PlayerIdentityId = pi.PlayerIdentityId
INNER JOIN StoolballPlayer p ON pi.PlayerId = p.PlayerId
WHERE s.PlayerRoute != p.PlayerRoute OR s.PlayerId != p.PlayerId OR s.PlayerIdentityName != pi.PlayerIdentityName

IF @@ROWCOUNT > 0
SET @DoStatisticsUpdate = 1
ELSE
SET @DoStatisticsUpdate = 0

IF @DoStatisticsUpdate = 1
BEGIN
UPDATE StoolballPlayerInMatchStatistics
SET
PlayerRoute = todo.PlayerRoute,
PlayerId = todo.PlayerId
FROM StoolballPlayerInMatchStatistics s
INNER JOIN #LinkPlayerToMemberAsyncUpdate todo ON s.PlayerInMatchStatisticsId = todo.PlayerInMatchStatisticsId

-- Return affected PlayerRoutes so that the calling code can clear the player cache, and so it knows that work was done
-- and it might need to call this procedure again to process further records.
SELECT PlayerRoute FROM #LinkPlayerToMemberAsyncUpdate
UNION
SELECT PlayerRouteToReplace AS PlayerRoute FROM #LinkPlayerToMemberAsyncUpdate
END

DROP TABLE #LinkPlayerToMemberAsyncUpdate

-- When combining one player identity with another, there's a leftover player to delete that the identity used to belong to.
-- Once the SELECT above returns no rows we know it is safe to delete these players without a blocking foreign key in the
-- StoolballPlayerInMatchStatistics table.
IF @DoStatisticsUpdate = 0
BEGIN
SELECT p.PlayerId, p.PlayerRoute
INTO #LinkPlayerToMemberAsyncDelete
FROM StoolballPlayer p
LEFT JOIN StoolballPlayerInMatchStatistics s ON p.PlayerId = s.PlayerId
WHERE ForAsyncDelete = 1 AND s.PlayerInMatchStatisticsId IS NULL

IF @@ROWCOUNT > 0
BEGIN
DELETE FROM StoolballPlayer WHERE PlayerId IN (SELECT PlayerId FROM #LinkPlayerToMemberAsyncDelete)

-- Return affected PlayerRoutes so that the calling code can clear the player cache, and so it knows that work was done
-- and it might need to call this procedure again to process further records.
SELECT PlayerRoute FROM #LinkPlayerToMemberAsyncDelete
END

DROP TABLE #LinkPlayerToMemberAsyncDelete
END

COMMIT TRAN
END
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
<None Remove="040_LinkPlayerToMember_Async_Update\usp_Link_Player_To_Member_Async_Update.sql" />
<None Remove="041_Index_TeamRoute_OppositionTeamRoute\IX_StoolballPlayerInMatchStatistics_OppositionTeamRoute.sql" />
<None Remove="041_Index_TeamRoute_OppositionTeamRoute\IX_StoolballPlayerInMatchStatistics_TeamRoute.sql" />
<None Remove="042_Add_PlayerIdentity_RouteSegment\StoolballPlayerIdentity_PopulateRouteSegment.sql" />
<None Remove="042_Add_PlayerIdentity_RouteSegment\usp_Player_Async_Update.sql" />
</ItemGroup>

<ItemGroup>
Expand All @@ -19,6 +21,8 @@
<EmbeddedResource Include="041_Index_TeamRoute_OppositionTeamRoute\IX_StoolballPlayerInMatchStatistics_TeamRoute.sql" />
<EmbeddedResource Include="040_LinkPlayerToMember_Async_Update\IX_StoolballPlayerInMatchStatistics_PlayerRoute.sql" />
<EmbeddedResource Include="040_LinkPlayerToMember_Async_Update\usp_Link_Player_To_Member_Async_Update.sql" />
<EmbeddedResource Include="042_Add_PlayerIdentity_RouteSegment\StoolballPlayerIdentity_PopulateRouteSegment.sql" />
<EmbeddedResource Include="042_Add_PlayerIdentity_RouteSegment\usp_Player_Async_Update.sql" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public StoolballDataMigrationPlan() : base("StoolballData")
.To<StatisticsRemoveProbability>(typeof(StatisticsRemoveProbability).ToString())
.To<LinkPlayerToMember>(typeof(LinkPlayerToMember).ToString())
.To<LinkPlayerToMemberAsyncUpdate>(typeof(LinkPlayerToMemberAsyncUpdate).ToString())
.To<IndexTeamRouteAndOppositionTeamRoute>(typeof(IndexTeamRouteAndOppositionTeamRoute).ToString());
.To<IndexTeamRouteAndOppositionTeamRoute>(typeof(IndexTeamRouteAndOppositionTeamRoute).ToString())
.To<AddRouteSegmentToPlayerIdentity>(typeof(AddRouteSegmentToPlayerIdentity).ToString());
}
}
}
Loading

0 comments on commit b7a7cf8

Please sign in to comment.