Refactor user name generation

Expedience.Api/Consumers/DutyCompletionResultConsumer.cs

@@ -1,5 +1,6 @@
 using System;
 using Expedience.Infrastructure;
+using Expedience.Infrastructure.Concurrency;
 using Expedience.Infrastructure.Models;
 using MassTransit;
 
@@ -9,12 +10,14 @@ namespace Expedience.Api.Consumers
     {
         private readonly ILogger<DutyCompletionResultConsumer> _logger;
         private readonly IServiceScopeFactory _serviceScopeFactory;
-
+        private readonly IDistributedLock _distributedLock;
         public DutyCompletionResultConsumer(ILogger<DutyCompletionResultConsumer> logger,
+            IDistributedLock distributedLock,
             IServiceScopeFactory serviceScopeFactory)
         {
             _logger = logger;
             _serviceScopeFactory = serviceScopeFactory;
+            _distributedLock = distributedLock;
         }
 
         public async Task Consume(ConsumeContext<Models.DutyCompletionResult> context)
@@ -33,12 +36,11 @@ namespace Expedience.Api.Consumers
 
             using var scope = _serviceScopeFactory.CreateScope();
             using var dbContext = scope.ServiceProvider.GetRequiredService<ExpedienceContext>();
-            var user = dbContext.Users.FirstOrDefault(x => x.UserHash == userHash && x.WorldId == worldId);
+            var user = dbContext.Users.FirstOrDefault(x => x.UserHash == userHash && x.WorldId == worldId) ??
+                       await CreateUser(dbContext, worldId, userHash, CancellationToken.None);
+
             if (user == null)
-            {
-                _logger.LogError("No user found for World {worldId} and Hash {userHash}", worldId, userHash);
                 return;
-            }
 
             var completionResult = new DutyCompletionResult
             {
@@ -89,5 +91,58 @@ namespace Expedience.Api.Consumers
 
             _logger.LogInformation("Consumed message {uploadId} from user {userHash}", message.UploadId, userHash);
         }
+
+        private async Task<User?> CreateUser(ExpedienceContext dbContext, int worldId, string userHash, CancellationToken cancellationToken)
+        {
+            var lockKey = $"{worldId}-{userHash}";
+            if (_distributedLock.AcquireLock(lockKey, TimeSpan.FromSeconds(10), TimeSpan.FromMinutes(4)))
+            {
+                try
+                {
+                    var user = dbContext.Users.FirstOrDefault(x => x.UserHash == userHash && x.WorldId == worldId);
+                    if (user == null)
+                    {
+                        string userName;
+                        var isDuplicate = false;
+                        do
+                        {
+                            userName = UserNameGenerator.Generate();
+                            isDuplicate = dbContext.Users.Any(x => x.WorldId == worldId && x.UserName == userName);
+                            await Task.Delay(20, cancellationToken); // Don't hog the CPU
+                        }
+                        while (isDuplicate == true);
+
+                        user = new User
+                        {
+                            UserHash = userHash,
+                            WorldId = worldId,
+                            UserName = userName,
+                            CreatedAt = DateTime.UtcNow,
+                        };
+
+                        dbContext.Users.Add(user);
+                        await dbContext.SaveChangesAsync(cancellationToken);
+                        _logger.LogInformation("Created user for World {worldId} and Hash {userHash}: {userName}", worldId, userHash, user.UserName);
+                    }
+
+                    return user;
+
+                }
+                catch (Exception ex)
+                {
+                    _logger.LogError(ex, "Error obtaining user name for World Id {worldId} and hash {userHash}: {errorMessage}", worldId, userHash, ex.Message);
+                }
+                finally
+                {
+                    _distributedLock.ReleaseLock(lockKey);
+                }
+            }
+            else
+            {
+                _logger.LogError("Could not acquire lock for {lockKey}", lockKey);
+            }
+
+            return null;
+        }
     }
 }

Expedience.Api/Controllers/ExpedienceController.cs

@@ -1,12 +1,11 @@
 using System;
-using System.Net;
 using System.Text.Json;
 using Expedience.Api.Encryption;
 using Expedience.Infrastructure;
 using Expedience.Infrastructure.Concurrency;
-using Expedience.Infrastructure.Models;
 using MassTransit;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.EntityFrameworkCore;
 
 namespace Expedience.Api.Controllers
 {
@@ -20,8 +19,8 @@ namespace Expedience.Api.Controllers
         private readonly IDistributedLock _distributedLock;
         private readonly IServiceScopeFactory _serviceScopeFactory;
 
-        public ExpedienceController(ILogger<ExpedienceController> logger, 
+        public ExpedienceController(ILogger<ExpedienceController> logger,
-            IPublishEndpoint publisher, 
+            IPublishEndpoint publisher,
             IDecryptor decrypytor,
             IDistributedLock distributedLock,
             IServiceScopeFactory serviceScopeFactory)
@@ -58,54 +57,20 @@ namespace Expedience.Api.Controllers
         [HttpGet("UserName/{worldId}/{userHash}")]
         public async Task<ActionResult> GetUserName(int worldId, string userHash, CancellationToken cancellationToken)
         {
-            using var scope = _serviceScopeFactory.CreateScope();
+            try
-            using var dbContext = scope.ServiceProvider.GetRequiredService<ExpedienceContext>();
-
-            var lockKey = $"{worldId}-{userHash}";
-            if (_distributedLock.AcquireLock(lockKey, TimeSpan.FromSeconds(10), TimeSpan.FromMinutes(4)))
             {
-                try
+                using var scope = _serviceScopeFactory.CreateScope();
-                {
+                using var dbContext = scope.ServiceProvider.GetRequiredService<ExpedienceContext>();
-                    var user = dbContext.Users.FirstOrDefault(x => x.UserHash == userHash && x.WorldId == worldId);
-                    if (user == null)
-                    {
-                        string userName;
-                        var isDuplicate = false;
-                        do
-                        {
-                            userName = UserNameGenerator.Generate();
-                            isDuplicate = dbContext.Users.Any(x => x.WorldId == worldId && x.UserName == userName);
-                            await Task.Delay(20, cancellationToken); // Don't hog the CPU
-                        }
-                        while (isDuplicate == true);
-
-                        user = new User
-                        {
-                            UserHash = userHash,
-                            WorldId = worldId,
-                            UserName = userName,
-                            CreatedAt = DateTime.UtcNow,
-                        };
-
-                        dbContext.Users.Add(user);
-                        await dbContext.SaveChangesAsync(cancellationToken);
-                    }
 
+                var user = await dbContext.Users.FirstOrDefaultAsync(x => x.UserHash == userHash && x.WorldId == worldId, cancellationToken);
+                if (user != null)
                     return Ok(user.UserName);
-
+                else
-                }
+                    return NotFound();
-                catch (Exception ex)
-                {
-                    _logger.LogError(ex, "Error obtaining user name for World Id {worldId} and hash {userHash}: {errorMessage}", worldId, userHash, ex.Message);
-                }
-                finally
-                {
-                    _distributedLock.ReleaseLock(lockKey);
-                }
             }
-            else
+            catch (Exception ex)
             {
-                _logger.LogError("Could not acquire lock for {lockKey}", lockKey);
+                _logger.LogError(ex, "Error obtaining user name for World Id {worldId} and hash {userHash}: {errorMessage}", worldId, userHash, ex.Message);
             }
 
             return StatusCode(500);

Expedience.Api/UserNameGenerator.cs

@@ -15,12 +15,18 @@ namespace Expedience.Api
         {
             var firstIndex = _random.Next(_adjectives.Count);
             var lastIndex = _random.Next(_names.Count);
+            var suffixIndex = _random.Next(_romanNumerals.Count);
             var firstName = _adjectives[firstIndex];
             var lastName = _names[lastIndex];
-            return $"{firstName} {lastName}";
+            var suffix = _romanNumerals[suffixIndex];
+
+            var name = $"{firstName} {lastName}";
+            if (suffix != string.Empty) name += " " + suffix;
+            
+            return name;
         }
 
-        public static readonly List<string> _names = new List<string>()
+        private static readonly List<string> _names = new List<string>()
         {
             "Strife",
             "Valentine",
@@ -224,7 +230,7 @@ namespace Expedience.Api
             "Cidolfus"
         };
 
-        public static List<string> _adjectives = new List<string>
+        private static List<string> _adjectives = new List<string>
         {
             "Adventurous",
             "Affectionate",
@@ -476,5 +482,7 @@ namespace Expedience.Api
             "Zealous",
             "Zest"
         };
+
+        private static List<string> _romanNumerals = new List<string> { "", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX", "X", "XI", "XII", "XIII", "XIV", "XV", "XVI", "XVII", "XVIII", "XIX", "XX" };
     }
 }