From 8e3c051853ff88a74af9f70adb91b030033b9f1c Mon Sep 17 00:00:00 2001 From: Ben Stein <115497763+sei-bstein@users.noreply.github.com> Date: Tue, 28 May 2024 11:50:11 -0400 Subject: [PATCH] v3.19.1 (#460) * Misc cleanup and add snippets (for fun) * Make site usage report sponsors sort by count descending * minor cleanup * Perf optimizations for site usage report. * Add server-side validation for game start/end, registration start/end, and team size. Resolves #250. * Don't show plaintext host API key in response (and only optionally update during upsert operation). --- .vscode/extensions.json | 7 -- .vscode/gameboard.code-snippets | 50 +++++++++++ .../Features/Games/GameControllerTests.cs | 3 +- src/Gameboard.Api/Data/GameboardDbContext.cs | 3 + .../Challenge/Services/ChallengeService.cs | 18 ++-- .../Game/External/ExternalGamesModels.cs | 2 +- .../Requests/UpsertExternalGameHost.cs | 18 ++++ .../Services/ExternalGameHostService.cs | 3 +- .../External/Services/ExternalGameService.cs | 3 - .../Features/Game/GameController.cs | 2 +- .../Features/Game/GameExceptions.cs | 6 ++ .../Game/GameModes/ExternalGameModeService.cs | 2 +- .../Features/Game/GameService.cs | 22 +++-- .../Features/Game/GameValidator.cs | 86 +++++++++++-------- .../GetSiteUsageReportPlayers.cs | 1 - .../GetSiteUsageReportSponsors.cs | 1 + 16 files changed, 156 insertions(+), 71 deletions(-) delete mode 100644 .vscode/extensions.json create mode 100644 .vscode/gameboard.code-snippets diff --git a/.vscode/extensions.json b/.vscode/extensions.json deleted file mode 100644 index 6b6ac57b..00000000 --- a/.vscode/extensions.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "recommendations": [ - "ms-azuretools.vscode-docker", - "ms-dotnettools.csharp", - "ms-dotnettools.csdevkit" - ] -} diff --git a/.vscode/gameboard.code-snippets b/.vscode/gameboard.code-snippets new file mode 100644 index 00000000..8604fdee --- /dev/null +++ b/.vscode/gameboard.code-snippets @@ -0,0 +1,50 @@ +{ + // see https://code.visualstudio.com/docs/editor/userdefinedsnippets + "Create Gameboard Unit Test Suite": { + "scope": "csharp", + "description": "Create a Gameboard unit test suite", + "prefix": "test-suite-unit", + "body": [ + "namespace Gameboard.Api.Tests.Unit;", + "", + "public class ${TM_FILENAME/\\.cs//g}", + "{", + "\t$0", + "}" + ] + }, + "Create Gameboard Unit Test": { + "scope": "csharp", + "description": "Start a new Gameboard unit test", + "prefix": "test-unit", + "isFileTemplate": true, + "body": [ + "[${0:Theory}, ${1:GameboardAutoData}]", + "public async Task ${TM_FILENAME/Tests\\.cs//g}_$2_$3(IFixture fixture)", + "{", + "\t\/\/ given", + "\t$4", + "\t\/\/ when", + "\t\/\/ var sut = new ${TM_FILENAME/Tests\\.cs//g}(...)", + "", + "\t\/\/ then", + "}" + ] + // }, + "Create Gameboard Integration Test Suite": { + "scope": "csharp", + "description": "Create a Gameboard integration test suite", + "prefix": "test-suite-int", + "body": [ + "namespace Gameboard.Api.Tests.Integration;", + "", + "public class ${0:Some}ControllerTests : IClassFixture", + "{", + "\tprivate readonly GameboardTestContext _testContext;", + "", + "\tpublic ${0:Some}ControllerTests(GameboardTestContext testContext", + "\t\t=> _testContext = testContext;", + "}" + ] + } +} diff --git a/src/Gameboard.Api.Tests.Integration/Tests/Features/Games/GameControllerTests.cs b/src/Gameboard.Api.Tests.Integration/Tests/Features/Games/GameControllerTests.cs index e703569f..d9d774f8 100644 --- a/src/Gameboard.Api.Tests.Integration/Tests/Features/Games/GameControllerTests.cs +++ b/src/Gameboard.Api.Tests.Integration/Tests/Features/Games/GameControllerTests.cs @@ -13,6 +13,7 @@ public GameControllerTests(GameboardTestContext testContext) public async Task GameController_Create_ReturnsGame() { // arrange + var game = new NewGame() { Name = "Test game", @@ -31,7 +32,7 @@ public async Task GameController_Create_ReturnsGame() var responseGame = await _testContext .CreateHttpClientWithAuthRole(UserRole.Designer) .PostAsync("/api/game", game.ToJsonBody()) - .WithContentDeserializedAs(); + .WithContentDeserializedAs(); // assert responseGame?.Name.ShouldBe(game.Name); diff --git a/src/Gameboard.Api/Data/GameboardDbContext.cs b/src/Gameboard.Api/Data/GameboardDbContext.cs index 5e2826dc..2751af1d 100644 --- a/src/Gameboard.Api/Data/GameboardDbContext.cs +++ b/src/Gameboard.Api/Data/GameboardDbContext.cs @@ -271,6 +271,9 @@ protected override void OnModelCreating(ModelBuilder builder) b.Property(p => p.InviteCode).HasMaxLength(40); b.Property(p => p.AdvancedFromTeamId).HasStandardGuidLength(); + // performance-oriented indices + b.HasIndex(p => new { p.UserId, p.TeamId }); + // nav properties b.HasOne(p => p.User).WithMany(u => u.Enrollments).OnDelete(DeleteBehavior.Cascade); b diff --git a/src/Gameboard.Api/Features/Challenge/Services/ChallengeService.cs b/src/Gameboard.Api/Features/Challenge/Services/ChallengeService.cs index ff09ab68..feecc847 100644 --- a/src/Gameboard.Api/Features/Challenge/Services/ChallengeService.cs +++ b/src/Gameboard.Api/Features/Challenge/Services/ChallengeService.cs @@ -650,27 +650,23 @@ public ConsoleActor GetConsoleActor(string userId) public async Task GetChallengeUserMaps(IQueryable query, CancellationToken cancellationToken) { var teamChallengeIds = await query - .Select(c => new - { - c.Id, - c.TeamId - }) + .Select(c => new { c.Id, c.TeamId }) .GroupBy(c => c.TeamId) .ToDictionaryAsync(gr => gr.Key, gr => gr.Select(c => c.Id).ToArray(), cancellationToken); var teamIds = teamChallengeIds.Keys; var userTeamIds = await _store - .WithNoTracking() - .Include(c => c.Player) - .Where(c => c.Player.UserId != null && c.Player.UserId != string.Empty) - .Where(c => teamIds.Contains(c.TeamId)) - .Select(c => new { c.Player.UserId, c.TeamId }) + .WithNoTracking() + .Where(p => p.UserId != null && p.UserId != string.Empty) + .Where(p => teamIds.Contains(p.TeamId)) + .Select(p => new { p.UserId, p.TeamId }) .GroupBy(p => p.UserId) .ToDictionaryAsync(gr => gr.Key, gr => gr.Select(thing => thing.TeamId).Distinct(), cancellationToken); var userIdChallengeIds = userTeamIds - .ToDictionary(gr => gr.Key, gr => gr.Value.SelectMany(tId => teamChallengeIds[tId])); + .ToDictionary(gr => gr.Key, gr => gr.Value + .SelectMany(tId => teamChallengeIds[tId])); var challengeIdUserIds = new Dictionary>(); foreach (var kv in userIdChallengeIds) diff --git a/src/Gameboard.Api/Features/Game/External/ExternalGamesModels.cs b/src/Gameboard.Api/Features/Game/External/ExternalGamesModels.cs index 0e26ef2c..5b001e92 100644 --- a/src/Gameboard.Api/Features/Game/External/ExternalGamesModels.cs +++ b/src/Gameboard.Api/Features/Game/External/ExternalGamesModels.cs @@ -17,7 +17,7 @@ public sealed class GetExternalGameHostsResponseHost public required string ClientUrl { get; set; } public required bool DestroyResourcesOnDeployFailure { get; set; } public required int? GamespaceDeployBatchSize { get; set; } - public required string HostApiKey { get; set; } + public required bool HasApiKey { get; set; } public required string HostUrl { get; set; } public required string PingEndpoint { get; set; } public required string StartupEndpoint { get; set; } diff --git a/src/Gameboard.Api/Features/Game/External/Requests/UpsertExternalGameHost.cs b/src/Gameboard.Api/Features/Game/External/Requests/UpsertExternalGameHost.cs index 064eac01..97f8fb39 100644 --- a/src/Gameboard.Api/Features/Game/External/Requests/UpsertExternalGameHost.cs +++ b/src/Gameboard.Api/Features/Game/External/Requests/UpsertExternalGameHost.cs @@ -1,9 +1,11 @@ +using System.Linq; using System.Threading; using System.Threading.Tasks; using Gameboard.Api.Data; using Gameboard.Api.Structure.MediatR; using Gameboard.Api.Structure.MediatR.Validators; using MediatR; +using Microsoft.EntityFrameworkCore; namespace Gameboard.Api.Features.Games.External; @@ -79,7 +81,23 @@ public async Task Handle(UpsertExternalGameHostCommand request if (request.Host.Id.IsEmpty()) retVal = await _store.Create(retVal); else + { + // we only update the API key if the request explicitly asks us to. if it's empty, we make + // the assumption that the user isn't changing it (to avoid serving the actual API key in the response + // when they edit a host) + if (request.Host.HostApiKey.IsEmpty()) + { + var currentApiKey = await _store + .WithNoTracking() + .Where(h => h.Id == request.Host.Id) + .Select(h => h.HostApiKey) + .SingleAsync(cancellationToken); + + retVal.HostApiKey = currentApiKey; + } + retVal = await _store.SaveUpdate(retVal, cancellationToken); + } return retVal; } diff --git a/src/Gameboard.Api/Features/Game/External/Services/ExternalGameHostService.cs b/src/Gameboard.Api/Features/Game/External/Services/ExternalGameHostService.cs index 6778c288..4cb8197d 100644 --- a/src/Gameboard.Api/Features/Game/External/Services/ExternalGameHostService.cs +++ b/src/Gameboard.Api/Features/Game/External/Services/ExternalGameHostService.cs @@ -11,7 +11,6 @@ using Gameboard.Api.Features.Teams; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; -using ServiceStack; namespace Gameboard.Api.Features.Games.External; @@ -95,7 +94,7 @@ public IQueryable GetHosts() ClientUrl = h.ClientUrl, DestroyResourcesOnDeployFailure = h.DestroyResourcesOnDeployFailure, GamespaceDeployBatchSize = h.GamespaceDeployBatchSize, - HostApiKey = h.HostApiKey, + HasApiKey = h.HostApiKey != null && h.HostApiKey != string.Empty, HostUrl = h.HostUrl, PingEndpoint = h.PingEndpoint, StartupEndpoint = h.StartupEndpoint, diff --git a/src/Gameboard.Api/Features/Game/External/Services/ExternalGameService.cs b/src/Gameboard.Api/Features/Game/External/Services/ExternalGameService.cs index d87b85fd..32081f88 100644 --- a/src/Gameboard.Api/Features/Game/External/Services/ExternalGameService.cs +++ b/src/Gameboard.Api/Features/Game/External/Services/ExternalGameService.cs @@ -33,7 +33,6 @@ internal class ExternalGameService : IExternalGameService, INotificationHandler, INotificationHandler { - private readonly IGameModeServiceFactory _gameModeServiceFactory; private readonly IGuidService _guids; private readonly ILogger _logger; private readonly INowService _now; @@ -43,7 +42,6 @@ internal class ExternalGameService : IExternalGameService, public ExternalGameService ( - IGameModeServiceFactory gameModeServiceFactory, IGuidService guids, ILogger logger, INowService now, @@ -52,7 +50,6 @@ public ExternalGameService ITeamService teamService ) { - _gameModeServiceFactory = gameModeServiceFactory; _guids = guids; _logger = logger; _now = now; diff --git a/src/Gameboard.Api/Features/Game/GameController.cs b/src/Gameboard.Api/Features/Game/GameController.cs index 5cd77362..dcd3046a 100644 --- a/src/Gameboard.Api/Features/Game/GameController.cs +++ b/src/Gameboard.Api/Features/Game/GameController.cs @@ -103,7 +103,7 @@ public Task DeployResources([FromRoute] string gameId, [FromBody] IEnumerable GetGamePlayState(string gameId, CancellationToken can public Task GetGamePlayStateForTeam(string teamId, CancellationToken cancellationToken) => GetGamePlayStateForGameAndTeam(null, teamId, cancellationToken); - private async Task GetGamePlayStateForGameAndTeam(string gameId, string teamId, CancellationToken cancellationToken) + internal async Task GetGamePlayStateForGameAndTeam(string gameId, string teamId, CancellationToken cancellationToken) { if (teamId.IsNotEmpty()) gameId = await _teamService.GetGameId(teamId, cancellationToken); diff --git a/src/Gameboard.Api/Features/Game/GameService.cs b/src/Gameboard.Api/Features/Game/GameService.cs index 1f0722c3..a4a8ec5d 100644 --- a/src/Gameboard.Api/Features/Game/GameService.cs +++ b/src/Gameboard.Api/Features/Game/GameService.cs @@ -78,17 +78,24 @@ public async Task Create(NewGame model) model.CertificateTemplate = _defaults.CertificateTemplate; } - // default to standard-mode challenges + // defaults: standard, 60 minutes, scoreboard access, etc. if (model.Mode.IsEmpty()) model.Mode = GameEngineMode.Standard; - // by default, enable public scoreboard access (after the game has ended) + // default to a session length of 60 minutes + if (model.SessionMinutes == 0) + model.SessionMinutes = 60; + + if (model.MinTeamSize == 0) + model.MinTeamSize = 1; + + if (model.MaxTeamSize == 0) + model.MaxTeamSize = 1; + model.AllowPublicScoreboardAccess = true; var entity = Mapper.Map(model); - await _gameStore.Create(entity); - return Mapper.Map(entity); } @@ -111,10 +118,9 @@ public async Task Update(ChangedGame game) await _gameStore.Update(entity); } - public async Task Delete(string id) - { - await _gameStore.Delete(id); - } + public Task Delete(string id) + => _gameStore.Delete(id); + public IQueryable BuildQuery(GameSearchFilter model = null, bool sudo = false) { diff --git a/src/Gameboard.Api/Features/Game/GameValidator.cs b/src/Gameboard.Api/Features/Game/GameValidator.cs index 300aa87f..a48a74f3 100644 --- a/src/Gameboard.Api/Features/Game/GameValidator.cs +++ b/src/Gameboard.Api/Features/Game/GameValidator.cs @@ -1,45 +1,61 @@ // Copyright 2021 Carnegie Mellon University. All Rights Reserved. // Released under a MIT (SEI)-style license. See LICENSE.md in the project root for license information. +using System; using System.Threading.Tasks; -using Gameboard.Api.Data.Abstractions; +using Gameboard.Api.Data; +using Gameboard.Api.Features.Games; +using Microsoft.EntityFrameworkCore; -namespace Gameboard.Api.Validators +namespace Gameboard.Api.Validators; + +public class GameValidator : IModelValidator { - public class GameValidator : IModelValidator + private readonly IStore _store; + + public GameValidator(IStore store) + { + _store = store; + } + + public Task Validate(object model) + { + if (model is Entity) + return _validate(model as Entity); + + if (model is ChangedGame) + return _validate(model as ChangedGame); + + throw new ValidationTypeFailure(model.GetType()); + } + + private Task _validate(ChangedGame game) { - private readonly IGameStore _store; - - public GameValidator( - IGameStore store - ) - { - _store = store; - } - - public Task Validate(object model) - { - if (model is Entity) - return _validate(model as Entity); - - throw new ValidationTypeFailure(model.GetType()); - } - - private async Task _validate(Entity model) - { - if ((await Exists(model.Id)).Equals(false)) - throw new ResourceNotFound(model.Id); - - await Task.CompletedTask; - } - - private async Task Exists(string id) - { - return - id.NotEmpty() && - (await _store.Retrieve(id)) is not null - ; - } + if (game.MinTeamSize > game.MaxTeamSize) + throw new InvalidTeamSize(game.Id, game.Name, game.MinTeamSize, game.MaxTeamSize); + if (game.GameStart.IsNotEmpty() && game.GameEnd.IsNotEmpty() && game.GameStart > game.GameEnd) + throw new InvalidDateRange(new DateRange(game.GameStart, game.GameEnd)); + + if (game.RegistrationType == GameRegistrationType.Open && game.RegistrationOpen > game.RegistrationClose) + throw new InvalidDateRange(new DateRange(game.RegistrationOpen, game.RegistrationClose)); + + return Task.CompletedTask; + } + + private async Task _validate(Entity model) + { + if ((await Exists(model.Id)).Equals(false)) + throw new ResourceNotFound(model.Id); + + await Task.CompletedTask; } + + private async Task Exists(string id) + { + return id.IsNotEmpty() && await _store + .WithNoTracking() + .AnyAsync(g => g.Id == id); + } + } diff --git a/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportPlayers.cs b/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportPlayers.cs index 4d8781dc..9de9578e 100644 --- a/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportPlayers.cs +++ b/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportPlayers.cs @@ -45,7 +45,6 @@ public async Task> Handle(GetSiteUsageRep paging.PageNumber ??= 0; paging.PageSize ??= 20; - var challengeIdUserIdMaps = await _challengeService.GetChallengeUserMaps(_reportService.GetBaseQuery(request.ReportParameters), cancellationToken); var challengeData = await _store .WithNoTracking() diff --git a/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportSponsors.cs b/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportSponsors.cs index 1aa0d5a3..0b0b990e 100644 --- a/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportSponsors.cs +++ b/src/Gameboard.Api/Features/Reports/Queries/SiteUsageReport/GetSiteUsageReportSponsors.cs @@ -59,6 +59,7 @@ public async Task> Handle(GetSiteUsageReport ParentName = gr.Key.ParentName, PlayerCount = gr.Select(p => p.UserId).Distinct().Count() }) + .OrderByDescending(s => s.PlayerCount) .ToArrayAsync(cancellationToken); } }