Skip to content

Commit

Permalink
v3.19.1 (#460)
Browse files Browse the repository at this point in the history
* 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).
  • Loading branch information
sei-bstein authored May 28, 2024
1 parent 6df79a6 commit 8e3c051
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 71 deletions.
7 changes: 0 additions & 7 deletions .vscode/extensions.json

This file was deleted.

50 changes: 50 additions & 0 deletions .vscode/gameboard.code-snippets
Original file line number Diff line number Diff line change
@@ -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<GameboardTestContext>",
"{",
"\tprivate readonly GameboardTestContext _testContext;",
"",
"\tpublic ${0:Some}ControllerTests(GameboardTestContext testContext",
"\t\t=> _testContext = testContext;",
"}"
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public GameControllerTests(GameboardTestContext testContext)
public async Task GameController_Create_ReturnsGame()
{
// arrange

var game = new NewGame()
{
Name = "Test game",
Expand All @@ -31,7 +32,7 @@ public async Task GameController_Create_ReturnsGame()
var responseGame = await _testContext
.CreateHttpClientWithAuthRole(UserRole.Designer)
.PostAsync("/api/game", game.ToJsonBody())
.WithContentDeserializedAs<Api.Data.Game>();
.WithContentDeserializedAs<Data.Game>();

// assert
responseGame?.Name.ShouldBe(game.Name);
Expand Down
3 changes: 3 additions & 0 deletions src/Gameboard.Api/Data/GameboardDbContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 7 additions & 11 deletions src/Gameboard.Api/Features/Challenge/Services/ChallengeService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -650,27 +650,23 @@ public ConsoleActor GetConsoleActor(string userId)
public async Task<ChallengeIdUserIdMap> GetChallengeUserMaps(IQueryable<Data.Challenge> 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<Data.Challenge>()
.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<Data.Player>()
.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<string, IEnumerable<string>>();
foreach (var kv in userIdChallengeIds)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -79,7 +81,23 @@ public async Task<ExternalGameHost> 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<ExternalGameHost>()
.Where(h => h.Id == request.Host.Id)
.Select(h => h.HostApiKey)
.SingleAsync(cancellationToken);

retVal.HostApiKey = currentApiKey;
}

retVal = await _store.SaveUpdate(retVal, cancellationToken);
}

return retVal;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Gameboard.Api.Features.Teams;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using ServiceStack;

namespace Gameboard.Api.Features.Games.External;

Expand Down Expand Up @@ -95,7 +94,7 @@ public IQueryable<GetExternalGameHostsResponseHost> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ internal class ExternalGameService : IExternalGameService,
INotificationHandler<GameResourcesDeployStartNotification>,
INotificationHandler<GameResourcesDeployEndNotification>
{
private readonly IGameModeServiceFactory _gameModeServiceFactory;
private readonly IGuidService _guids;
private readonly ILogger<ExternalGameService> _logger;
private readonly INowService _now;
Expand All @@ -43,7 +42,6 @@ internal class ExternalGameService : IExternalGameService,

public ExternalGameService
(
IGameModeServiceFactory gameModeServiceFactory,
IGuidService guids,
ILogger<ExternalGameService> logger,
INowService now,
Expand All @@ -52,7 +50,6 @@ public ExternalGameService
ITeamService teamService
)
{
_gameModeServiceFactory = gameModeServiceFactory;
_guids = guids;
_logger = logger;
_now = now;
Expand Down
2 changes: 1 addition & 1 deletion src/Gameboard.Api/Features/Game/GameController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public Task DeployResources([FromRoute] string gameId, [FromBody] IEnumerable<st
[Authorize(AppConstants.DesignerPolicy)]
public async Task Update([FromBody] ChangedGame model)
{
await Validate(new Entity { Id = model.Id });
await Validate(model);
await GameService.Update(model);
}

Expand Down
6 changes: 6 additions & 0 deletions src/Gameboard.Api/Features/Game/GameExceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ public class GameDoesntAllowReset : GameboardValidationException
public GameDoesntAllowReset(string gameId) : base($"""Game {gameId} has "Allow Reset" set to disabled.""") { }
}

public class InvalidTeamSize : GameboardValidationException
{
public InvalidTeamSize(string id, string name, int min, int max)
: base($"Couldn't set team size {min}-{max} for game {name}. The minimum team size must be less than the max.") { }
}

internal class UserIsntPlayingGame : GameboardValidationException
{
public UserIsntPlayingGame(string userId, string gameId, string whyItMatters = null) : base($"""User {userId} isn't playing game {gameId}.{(string.IsNullOrWhiteSpace(whyItMatters) ? string.Empty : ". " + whyItMatters)} """) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public Task<GamePlayState> GetGamePlayState(string gameId, CancellationToken can
public Task<GamePlayState> GetGamePlayStateForTeam(string teamId, CancellationToken cancellationToken)
=> GetGamePlayStateForGameAndTeam(null, teamId, cancellationToken);

private async Task<GamePlayState> GetGamePlayStateForGameAndTeam(string gameId, string teamId, CancellationToken cancellationToken)
internal async Task<GamePlayState> GetGamePlayStateForGameAndTeam(string gameId, string teamId, CancellationToken cancellationToken)
{
if (teamId.IsNotEmpty())
gameId = await _teamService.GetGameId(teamId, cancellationToken);
Expand Down
22 changes: 14 additions & 8 deletions src/Gameboard.Api/Features/Game/GameService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,24 @@ public async Task<Game> 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<Data.Game>(model);

await _gameStore.Create(entity);

return Mapper.Map<Game>(entity);
}

Expand All @@ -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<Data.Game> BuildQuery(GameSearchFilter model = null, bool sudo = false)
{
Expand Down
86 changes: 51 additions & 35 deletions src/Gameboard.Api/Features/Game/GameValidator.cs
Original file line number Diff line number Diff line change
@@ -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<GameValidator>(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<GameValidator>(model.GetType());
}

private async Task _validate(Entity model)
{
if ((await Exists(model.Id)).Equals(false))
throw new ResourceNotFound<Data.Game>(model.Id);

await Task.CompletedTask;
}

private async Task<bool> 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<Data.Game>(model.Id);

await Task.CompletedTask;
}

private async Task<bool> Exists(string id)
{
return id.IsNotEmpty() && await _store
.WithNoTracking<Data.Game>()
.AnyAsync(g => g.Id == id);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public async Task<PagedEnumerable<SiteUsageReportPlayer>> Handle(GetSiteUsageRep
paging.PageNumber ??= 0;
paging.PageSize ??= 20;


var challengeIdUserIdMaps = await _challengeService.GetChallengeUserMaps(_reportService.GetBaseQuery(request.ReportParameters), cancellationToken);
var challengeData = await _store
.WithNoTracking<Data.Challenge>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public async Task<IEnumerable<SiteUsageReportSponsor>> Handle(GetSiteUsageReport
ParentName = gr.Key.ParentName,
PlayerCount = gr.Select(p => p.UserId).Distinct().Count()
})
.OrderByDescending(s => s.PlayerCount)
.ToArrayAsync(cancellationToken);
}
}

0 comments on commit 8e3c051

Please sign in to comment.