From 1f8e775986e7f75a76756b18ddfceade50ada6cb Mon Sep 17 00:00:00 2001 From: dp Date: Mon, 9 Mar 2026 09:21:39 +0100 Subject: [PATCH 1/4] Issue #20: Revised input validation. --- .../Controllers/ExamplesController.cs | 35 ++++++++++++------- .../Controllers/VocabularyController.cs | 5 +++ .../BusinessUtils.cs | 22 ++++++++++++ .../ExamplesService.cs | 8 +++-- .../VocabularyService.cs | 6 +--- .../Services/IExamplesService.cs | 2 ++ .../DataAccessService.cs | 4 +-- .../ExamplesControllerTests.cs | 2 +- 8 files changed, 61 insertions(+), 23 deletions(-) diff --git a/src/SpanishByExample.Api/Controllers/ExamplesController.cs b/src/SpanishByExample.Api/Controllers/ExamplesController.cs index 8401263..9724ff2 100644 --- a/src/SpanishByExample.Api/Controllers/ExamplesController.cs +++ b/src/SpanishByExample.Api/Controllers/ExamplesController.cs @@ -8,12 +8,14 @@ namespace SpanishByExample.Api.Controllers; /// /// Handles requests for the example sentences. /// -/// Injected logger. +/// Injected logger. /// Injected Examples service. [Route("api/[controller]")] [ApiController] -public class ExamplesController(ILogger log, IExamplesService examplesService) : ControllerBase +public class ExamplesController(ILogger _log, IExamplesService examplesService) : ControllerBase { + private const int MAX_QUERY_LENGTH = 50; + /// /// Searches for a given verb in the knowledge base. /// @@ -23,38 +25,47 @@ public class ExamplesController(ILogger log, IExamplesServic [HttpGet] public async Task> Get([FromQuery(Name = "q")] string query, CancellationToken cancellationToken) { - log.LogDebug($"Query: {query}"); + _log.LogDebug($"Query: {query}"); - // TODO error handling, logging of exceptions - // TODO other checks eg not a word if (string.IsNullOrWhiteSpace(query)) { - log.LogInformation("Empty query."); - return BadRequest("Query parameter 'q' is required."); + _log.LogDebug("Empty query."); + return BadRequest($"Query parameter 'q' is required."); + } + + if (query.Length > MAX_QUERY_LENGTH) + { + _log.LogDebug("Query too long."); + return BadRequest($"Query too long."); } try { - log.LogDebug($"Retrieving vocabulary entry for {query}."); + _log.LogDebug($"Retrieving vocabulary entry for {query}."); var vocab = await examplesService.GetVocabularyEntryAsync(query, cancellationToken); if (vocab == null) { - log.LogDebug($"No vocabulary entry found for {query}."); + _log.LogDebug($"No vocabulary entry found for {query}."); return NotFound(); } - log.LogDebug($"Returning found vocabulary entry found for {query}: ({nameof(vocab.VocabularyId)}: {vocab.VocabularyId}, {nameof(vocab.WordNorm)}: {vocab.WordNorm}"); + _log.LogDebug($"Returning found vocabulary entry found for {query}: ({nameof(vocab.VocabularyId)}: {vocab.VocabularyId}, {nameof(vocab.WordNorm)}: {vocab.WordNorm}"); return Ok(vocab); } + catch (BusinessException ex) + { + _log.LogError(ex, null); + return BadRequest(ex.Message); + } catch (DatabaseException ex) { - log.LogError(ex, null); + _log.LogError(ex, null); return StatusCode(500); } catch (Exception ex) { - log.LogError(ex, null); + _log.LogError(ex, null); return StatusCode(500); } } diff --git a/src/SpanishByExample.Api/Controllers/VocabularyController.cs b/src/SpanishByExample.Api/Controllers/VocabularyController.cs index ac453ac..ef7203b 100644 --- a/src/SpanishByExample.Api/Controllers/VocabularyController.cs +++ b/src/SpanishByExample.Api/Controllers/VocabularyController.cs @@ -61,5 +61,10 @@ public async Task> Post([FromBody] VocabularyEntry _log.LogError(ex, null); return StatusCode(500, "Database error."); } + catch (Exception ex) + { + _log.LogError(ex, null); + return StatusCode(500); + } } } diff --git a/src/SpanishByExample.Business/BusinessUtils.cs b/src/SpanishByExample.Business/BusinessUtils.cs index 0c24e19..865b9e5 100644 --- a/src/SpanishByExample.Business/BusinessUtils.cs +++ b/src/SpanishByExample.Business/BusinessUtils.cs @@ -1,5 +1,6 @@ using SpanishByExample.Core.Commands; using SpanishByExample.Core.Entities; +using System.Text.RegularExpressions; namespace SpanishByExample.Business; @@ -8,6 +9,25 @@ namespace SpanishByExample.Business; /// public static class BusinessUtils { + #region Private Fields + + private static readonly Regex WORDNORM_REGEX = new(@"\w+(ar|er|ir)"); + + #endregion + + #region Public Methods + + /// + /// Checks if provided word is a Spanish verb in infinitive form. + /// + /// Word. + /// Whether provided word is a Spanish verb in infinitive form. + public static bool IsInfinitiveVerb(string word) + { + return !string.IsNullOrWhiteSpace(word) && word.All(c => char.IsLower(c)) && WORDNORM_REGEX.IsMatch(word); + } + + /// /// Converts CreateVocabularyCommand to VocabularyEntry. /// @@ -29,4 +49,6 @@ public static VocabularyEntry ToVocabularyEntry(this CreateVocabularyCommand voc return new VocabularyEntry(0, vocabularyCmd.RawWord, vocabularyCmd.EnglishTranslation, examples); } + + #endregion } diff --git a/src/SpanishByExample.Business/ExamplesService.cs b/src/SpanishByExample.Business/ExamplesService.cs index 30d5093..ce65f14 100644 --- a/src/SpanishByExample.Business/ExamplesService.cs +++ b/src/SpanishByExample.Business/ExamplesService.cs @@ -1,7 +1,7 @@ using Microsoft.Extensions.Logging; - -using SpanishByExample.Core.Services; using SpanishByExample.Core.Entities; +using SpanishByExample.Core.Errors; +using SpanishByExample.Core.Services; namespace SpanishByExample.Business { @@ -17,6 +17,10 @@ public class ExamplesService(ILogger _log, IDataAccessService _ { _log.LogDebug($"{nameof(word)}: {word}"); + // Check that word is in normal form eg comer + if (!BusinessUtils.IsInfinitiveVerb(word)) + throw new BusinessException($"Supplied word is not a verb in infinitive form: {word}"); + // NOTE: Currently a wrapper only. var vocab = await _da.GetVocabularyEntryAsync(word, token); diff --git a/src/SpanishByExample.Business/VocabularyService.cs b/src/SpanishByExample.Business/VocabularyService.cs index aec7002..f4b61fa 100644 --- a/src/SpanishByExample.Business/VocabularyService.cs +++ b/src/SpanishByExample.Business/VocabularyService.cs @@ -3,7 +3,6 @@ using SpanishByExample.Core.Commands; using SpanishByExample.Core.Entities; using SpanishByExample.Core.Services; -using System.Text.RegularExpressions; namespace SpanishByExample.Business; @@ -15,16 +14,13 @@ namespace SpanishByExample.Business; /// Injected Data Access service. public class VocabularyService(ILogger _log, IDataAccessService _da) : IVocabularyService { - private static readonly Regex WORDNORM_REGEX = new(@"\w+(ar|er|ir)"); - /// public async Task AddVocabularyEntryAsync(string userId, CreateVocabularyCommand vocabularyCmd, CancellationToken token = default) { _log.LogDebug($"Adding vocabulary entry for word: {vocabularyCmd.RawWord}"); // Check that word is in normal form eg comer - if (string.IsNullOrWhiteSpace(vocabularyCmd.RawWord) - || !WORDNORM_REGEX.IsMatch(vocabularyCmd.RawWord)) + if (!BusinessUtils.IsInfinitiveVerb(vocabularyCmd.RawWord)) throw new BusinessException($"Supplied word is not a verb in infinitive form: {vocabularyCmd.RawWord}"); // Check business rule: #examples > 0 diff --git a/src/SpanishByExample.Core/Services/IExamplesService.cs b/src/SpanishByExample.Core/Services/IExamplesService.cs index 3905658..8a39113 100644 --- a/src/SpanishByExample.Core/Services/IExamplesService.cs +++ b/src/SpanishByExample.Core/Services/IExamplesService.cs @@ -1,4 +1,5 @@ using SpanishByExample.Core.Entities; +using SpanishByExample.Core.Errors; namespace SpanishByExample.Core.Services; @@ -13,6 +14,7 @@ public interface IExamplesService /// Queried word. /// Cancellation token. /// The vocabulary entry associated with the queried word, if present. + /// /// Task GetVocabularyEntryAsync(string word, CancellationToken token = default); } diff --git a/src/SpanishByExample.DataAccess/DataAccessService.cs b/src/SpanishByExample.DataAccess/DataAccessService.cs index 35d6e83..b0fd3b2 100644 --- a/src/SpanishByExample.DataAccess/DataAccessService.cs +++ b/src/SpanishByExample.DataAccess/DataAccessService.cs @@ -114,8 +114,6 @@ public async Task AddVocabularyEntryAsync(string userId, Vocabu { _log.LogDebug($"{nameof(word)}: {word}"); - // TODO security - const string query = @" SELECT VOCABULARYID, @@ -132,7 +130,7 @@ FROM dbo.EXAMPLES ex using var connection = new SqlConnection(_connectionString); using var command = new SqlCommand(query, connection); - command.Parameters.AddWithValue("@wordNorm", word); + command.Parameters.AddWithValue("@wordNorm", word); // SECURITY NOTE: supplying input as SQL parameter prevents SQL injection await connection.OpenAsync(token); diff --git a/tests/SpanishByExample.Tests/ExamplesControllerTests.cs b/tests/SpanishByExample.Tests/ExamplesControllerTests.cs index 9a28215..673efd0 100644 --- a/tests/SpanishByExample.Tests/ExamplesControllerTests.cs +++ b/tests/SpanishByExample.Tests/ExamplesControllerTests.cs @@ -47,7 +47,7 @@ public async Task Get_Ok_Hit() [Fact] public async Task Get_NotFound() { - var query = "x"; + var query = "manejar"; _daMock.Setup(da => da.GetVocabularyEntryAsync(query)) .ReturnsAsync((VocabularyEntry?)null); From d536b4426577d2188f75d3e59103c36d69e3668b Mon Sep 17 00:00:00 2001 From: dp Date: Mon, 9 Mar 2026 09:38:25 +0100 Subject: [PATCH 2/4] Issue #21: Cleanup of tests of ExamplesController (mocking of the BL). --- .../ExamplesControllerTests.cs | 39 ++++--------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/tests/SpanishByExample.Tests/ExamplesControllerTests.cs b/tests/SpanishByExample.Tests/ExamplesControllerTests.cs index 673efd0..eed8cea 100644 --- a/tests/SpanishByExample.Tests/ExamplesControllerTests.cs +++ b/tests/SpanishByExample.Tests/ExamplesControllerTests.cs @@ -3,7 +3,6 @@ using Microsoft.AspNetCore.Mvc; using SpanishByExample.Api.Controllers; using SpanishByExample.Core.Services; -using SpanishByExample.Business; using SpanishByExample.Core.Entities; using Microsoft.Extensions.Logging.Abstractions; @@ -14,25 +13,24 @@ namespace SpanishByExample.Tests /// public class ExamplesControllerTests { - private readonly Mock _daMock; + private readonly Mock _exServiceMock; private readonly ExamplesController _controller; public ExamplesControllerTests() { - _daMock = new Mock(); + _exServiceMock = new Mock(); var log = NullLogger.Instance; - var log2 = NullLogger.Instance; - _controller = new ExamplesController(log, new ExamplesService(log2, _daMock.Object)); + _controller = new ExamplesController(log, _exServiceMock.Object); } [Fact] - public async Task Get_Ok_Hit() + public async Task Get_Ok() { var query = "hablar"; var oracle = new VocabularyEntry(1, query, null, [ new() { ExampleId = 1, ExampleText = "Hablo español." } ]); - _daMock.Setup(da => da.GetVocabularyEntryAsync(query)) + _exServiceMock.Setup(exService => exService.GetVocabularyEntryAsync(query)) .ReturnsAsync(oracle); var result = await _controller.Get(query, CancellationToken.None); @@ -41,7 +39,7 @@ public async Task Get_Ok_Hit() var vocabularyEntry = ok.Value.Should().BeOfType().Subject; vocabularyEntry.Should().BeEquivalentTo(oracle); - _daMock.Verify(da => da.GetVocabularyEntryAsync(query), Times.Once()); + _exServiceMock.Verify(exService => exService.GetVocabularyEntryAsync(query), Times.Once()); } [Fact] @@ -49,7 +47,7 @@ public async Task Get_NotFound() { var query = "manejar"; - _daMock.Setup(da => da.GetVocabularyEntryAsync(query)) + _exServiceMock.Setup(exService => exService.GetVocabularyEntryAsync(query)) .ReturnsAsync((VocabularyEntry?)null); var result = await _controller.Get(query, CancellationToken.None); @@ -57,18 +55,6 @@ public async Task Get_NotFound() var notFound = result.Result.Should().BeOfType(); } - [Fact (Skip = "TODO")] - public async Task Get_Ok_NoHit() - { - // TODO test: no hit - var query = "sujetar"; - - var result = await _controller.Get(query, CancellationToken.None); - - var ok = result.Result.Should().BeOfType().Subject; - ok.Value.Should().Be(""); // Empty result set - } - [Theory] [InlineData("")] [InlineData(" ")] @@ -80,15 +66,6 @@ public async Task Get_Error_EmptyInput(string query) bad.Value.Should().Be("Query parameter 'q' is required."); } - [Theory (Skip = "TODO")] - [InlineData("x")] - public async Task Get_Error_InvalidInput(string query) - { - // TODO test: invalid word - var result = await _controller.Get(query, CancellationToken.None); - - var bad = result.Result.Should().BeOfType().Subject; - bad.Value.Should().Be($"Query parameter is not a recognized word: {query}"); - } + // TODO other tests ... } } From b1a2115d712d63e95d0340eadc5a25ec458bf015 Mon Sep 17 00:00:00 2001 From: dp Date: Mon, 9 Mar 2026 10:09:09 +0100 Subject: [PATCH 3/4] Issue #21: Added negative tests to VocabularyControllerTests. --- .../VocabularyControllerTests.cs | 137 +++++++++++++++++- .../VocabularyServiceTests.cs | 2 + 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/tests/SpanishByExample.Tests/VocabularyControllerTests.cs b/tests/SpanishByExample.Tests/VocabularyControllerTests.cs index 1e26391..494ae4c 100644 --- a/tests/SpanishByExample.Tests/VocabularyControllerTests.cs +++ b/tests/SpanishByExample.Tests/VocabularyControllerTests.cs @@ -7,6 +7,7 @@ using SpanishByExample.Api.Dtos; using SpanishByExample.Core.Commands; using SpanishByExample.Core.Entities; +using SpanishByExample.Core.Errors; using SpanishByExample.Core.Services; using System.Security.Claims; @@ -17,6 +18,8 @@ namespace SpanishByExample.Tests; /// public class VocabularyControllerTests { + private const string USERID = "f401d78e-3dda-4bce-9f84-55ee158c57f9"; + private readonly Mock _vocServiceMock; private readonly VocabularyController _controller; @@ -30,7 +33,7 @@ public VocabularyControllerTests() [Fact] public async Task Post_Ok() { - var userId = "f401d78e-3dda-4bce-9f84-55ee158c57f9"; + _controller.ControllerContext = CreateAuthenticatedContext(USERID); VocabularyEntryRequestDto vocDto = new() { @@ -46,8 +49,6 @@ public async Task Post_Ok() ] }; - _controller.ControllerContext = CreateAuthenticatedContext(userId); - var newVocEntry = new VocabularyEntry( 1, vocDto.RawWord, @@ -73,6 +74,136 @@ public async Task Post_Ok() vocabularyEntry.Should().BeEquivalentTo(newVocEntry); } + [Fact] + public async Task Post_Error_MissingExamplesException() + { + _controller.ControllerContext = CreateAuthenticatedContext(USERID); + + VocabularyEntryRequestDto vocDto = new() + { + RawWord = "hablar", + EnglishTranslation = "to speak", + Examples = [] // NOTE: Examples are missing. + }; + + _vocServiceMock.Setup(vocService => vocService.AddVocabularyEntryAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new MissingExamplesException("")); + + var result = await _controller.Post(vocDto, CancellationToken.None); + + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task Post_Error_BusinessException() + { + _controller.ControllerContext = CreateAuthenticatedContext(USERID); + + VocabularyEntryRequestDto vocDto = new() + { + RawWord = "comer", + EnglishTranslation = "to eat", + Examples = + [ + new() + { + ExampleText = "Como carne.", + EnglishTranslation = "I eat meat." + } + ] + }; + + _vocServiceMock.Setup(vocService => vocService.AddVocabularyEntryAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new BusinessException("")); + + var result = await _controller.Post(vocDto, CancellationToken.None); + + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task Post_Error_DuplicateVocabularyEntryException() + { + _controller.ControllerContext = CreateAuthenticatedContext(USERID); + + VocabularyEntryRequestDto vocDto = new() + { + RawWord = "comer", + EnglishTranslation = "to eat", + Examples = + [ + new() + { + ExampleText = "Como carne.", + EnglishTranslation = "I eat meat." + } + ] + }; + + _vocServiceMock.Setup(vocService => vocService.AddVocabularyEntryAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new DuplicateVocabularyEntryException(new VocabularyEntry(0, vocDto.RawWord, vocDto.EnglishTranslation, vocDto.Examples.Select(ex => new Example() { ExampleId = 0, ExampleText = ex.ExampleText, EnglishTranslation = ex.EnglishTranslation }).ToList()), new Exception())); + + var result = await _controller.Post(vocDto, CancellationToken.None); + + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task Post_Error_DatabaseException() + { + _controller.ControllerContext = CreateAuthenticatedContext(USERID); + + VocabularyEntryRequestDto vocDto = new() + { + RawWord = "comer", + EnglishTranslation = "to eat", + Examples = + [ + new() + { + ExampleText = "Como carne.", + EnglishTranslation = "I eat meat." + } + ] + }; + + _vocServiceMock.Setup(vocService => vocService.AddVocabularyEntryAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new DatabaseException("", new Exception())); + + var result = await _controller.Post(vocDto, CancellationToken.None); + + var statusCode = result.Result.Should().BeOfType().Subject; + statusCode.StatusCode.Should().Be(500); + } + + [Fact] + public async Task Post_Error_Exception() + { + _controller.ControllerContext = CreateAuthenticatedContext(USERID); + + VocabularyEntryRequestDto vocDto = new() + { + RawWord = "comer", + EnglishTranslation = "to eat", + Examples = + [ + new() + { + ExampleText = "Como carne.", + EnglishTranslation = "I eat meat." + } + ] + }; + + _vocServiceMock.Setup(vocService => vocService.AddVocabularyEntryAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new Exception("")); + + var result = await _controller.Post(vocDto, CancellationToken.None); + + var statusCode = result.Result.Should().BeOfType().Subject; + statusCode.StatusCode.Should().Be(500); + } + #region Helpers /// diff --git a/tests/SpanishByExample.Tests/VocabularyServiceTests.cs b/tests/SpanishByExample.Tests/VocabularyServiceTests.cs index 02c6041..d47f5fc 100644 --- a/tests/SpanishByExample.Tests/VocabularyServiceTests.cs +++ b/tests/SpanishByExample.Tests/VocabularyServiceTests.cs @@ -98,5 +98,7 @@ public async Task AddVocabularyEntryAsync_Error_MissingExamplesException() await act.Should().ThrowAsync().WithMessage($"No examples were provided for word: {vocabularyCmd.RawWord}"); } + // TODO other tests ... + #endregion } From a9dd6f9cc43266518083baf3c9f904c15ee0eb36 Mon Sep 17 00:00:00 2001 From: dp Date: Mon, 9 Mar 2026 10:18:53 +0100 Subject: [PATCH 4/4] Issue #21: Added negative tests to ExamplesControllerTests. --- .../ExamplesControllerTests.cs | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/SpanishByExample.Tests/ExamplesControllerTests.cs b/tests/SpanishByExample.Tests/ExamplesControllerTests.cs index eed8cea..555e1a5 100644 --- a/tests/SpanishByExample.Tests/ExamplesControllerTests.cs +++ b/tests/SpanishByExample.Tests/ExamplesControllerTests.cs @@ -5,6 +5,7 @@ using SpanishByExample.Core.Services; using SpanishByExample.Core.Entities; using Microsoft.Extensions.Logging.Abstractions; +using SpanishByExample.Core.Errors; namespace SpanishByExample.Tests { @@ -66,6 +67,56 @@ public async Task Get_Error_EmptyInput(string query) bad.Value.Should().Be("Query parameter 'q' is required."); } - // TODO other tests ... + [Fact] + public async Task Get_Error_InputTooLong() + { + var query = "123456789012345678901234567890123456789012345678901"; + + var result = await _controller.Get(query, CancellationToken.None); + + var bad = result.Result.Should().BeOfType().Subject; + bad.Value.Should().Be("Query too long."); + } + + [Fact] + public async Task Get_Error_BusinessException() + { + var query = "hablar"; + + _exServiceMock.Setup(exService => exService.GetVocabularyEntryAsync(query)) + .ThrowsAsync(new BusinessException("")); + + var result = await _controller.Get(query, CancellationToken.None); + + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task Get_Error_DatabaseException() + { + var query = "hablar"; + + _exServiceMock.Setup(exService => exService.GetVocabularyEntryAsync(query)) + .ThrowsAsync(new DatabaseException("", new Exception())); + + var result = await _controller.Get(query, CancellationToken.None); + + var statusCode = result.Result.Should().BeOfType().Subject; + statusCode.StatusCode.Should().Be(500); + } + + [Fact] + public async Task Get_Error_Exception() + { + var query = "hablar"; + + _exServiceMock.Setup(exService => exService.GetVocabularyEntryAsync(query)) + .ThrowsAsync(new Exception("")); + + var result = await _controller.Get(query, CancellationToken.None); + + var statusCode = result.Result.Should().BeOfType().Subject; + statusCode.StatusCode.Should().Be(500); + } } }