From 57151c879376374425adb04fb68dad2cf7930df8 Mon Sep 17 00:00:00 2001 From: 杨宇千 Date: Mon, 5 Aug 2019 18:58:56 +0800 Subject: 3 things. 1. Exchange Models and Entities namespace. 2. Fix the bug that input with missing field leads to 500. 3. Write unit tests. --- Timeline/Controllers/UserController.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'Timeline/Controllers/UserController.cs') diff --git a/Timeline/Controllers/UserController.cs b/Timeline/Controllers/UserController.cs index 2099690c..042a8107 100644 --- a/Timeline/Controllers/UserController.cs +++ b/Timeline/Controllers/UserController.cs @@ -4,13 +4,14 @@ using Microsoft.Extensions.Logging; using System; using System.Threading.Tasks; using Timeline.Authenticate; -using Timeline.Entities; -using Timeline.Entities.Http; +using Timeline.Models; +using Timeline.Models.Http; using Timeline.Services; using static Timeline.Helpers.MyLogHelper; namespace Timeline.Controllers { + [ApiController] public class UserController : Controller { private static class ErrorCodes -- cgit v1.2.3 From a35030d0a379ed7ee7fdd403449f53743209059c Mon Sep 17 00:00:00 2001 From: 杨宇千 Date: Thu, 8 Aug 2019 17:13:14 +0800 Subject: 2 things. 1. Make Administrator in UserPutRequest nullable. 2. Remove default route. --- Timeline/Controllers/UserController.cs | 10 +--------- Timeline/Models/Http/User.cs | 2 +- Timeline/Startup.cs | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) (limited to 'Timeline/Controllers/UserController.cs') diff --git a/Timeline/Controllers/UserController.cs b/Timeline/Controllers/UserController.cs index 0992946c..28d9523a 100644 --- a/Timeline/Controllers/UserController.cs +++ b/Timeline/Controllers/UserController.cs @@ -18,8 +18,6 @@ namespace Timeline.Controllers { public const int Get_NotExists = -1001; - public const int Put_NoPassword = -2001; - public const int Patch_NotExists = -3001; public const int ChangePassword_BadOldPassword = -4001; @@ -55,13 +53,7 @@ namespace Timeline.Controllers [HttpPut("user/{username}"), AdminAuthorize] public async Task Put([FromBody] UserPutRequest request, [FromRoute] string username) { - if (request.Password == null) // This place will be refactored. - { - _logger.LogInformation("Attempt to put a user without a password. Username: {} .", username); - return BadRequest(); - } - - var result = await _userService.PutUser(username, request.Password, request.Administrator); + var result = await _userService.PutUser(username, request.Password, request.Administrator.Value); switch (result) { case PutResult.Created: diff --git a/Timeline/Models/Http/User.cs b/Timeline/Models/Http/User.cs index 3259a448..d45543fb 100644 --- a/Timeline/Models/Http/User.cs +++ b/Timeline/Models/Http/User.cs @@ -7,7 +7,7 @@ namespace Timeline.Models.Http [Required] public string Password { get; set; } [Required] - public bool Administrator { get; set; } + public bool? Administrator { get; set; } } public class UserPatchRequest diff --git a/Timeline/Startup.cs b/Timeline/Startup.cs index a28899f4..414bc705 100644 --- a/Timeline/Startup.cs +++ b/Timeline/Startup.cs @@ -89,7 +89,7 @@ namespace Timeline app.UseAuthentication(); - app.UseMvcWithDefaultRoute(); + app.UseMvc(); } } } -- cgit v1.2.3 From aa7d7d8ce33646dc63afb0c084e7daa14916646a Mon Sep 17 00:00:00 2001 From: 杨宇千 Date: Thu, 8 Aug 2019 17:19:38 +0800 Subject: Rename user yo users in route. --- Timeline/Controllers/UserController.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'Timeline/Controllers/UserController.cs') diff --git a/Timeline/Controllers/UserController.cs b/Timeline/Controllers/UserController.cs index 28d9523a..af4cfb53 100644 --- a/Timeline/Controllers/UserController.cs +++ b/Timeline/Controllers/UserController.cs @@ -16,11 +16,11 @@ namespace Timeline.Controllers { private static class ErrorCodes { - public const int Get_NotExists = -1001; + public const int Get_NotExist = -1001; - public const int Patch_NotExists = -3001; + public const int Patch_NotExist = -2001; - public const int ChangePassword_BadOldPassword = -4001; + public const int ChangePassword_BadOldPassword = -3001; } private readonly ILogger _logger; @@ -38,19 +38,19 @@ namespace Timeline.Controllers return Ok(await _userService.ListUsers()); } - [HttpGet("user/{username}"), AdminAuthorize] + [HttpGet("users/{username}"), AdminAuthorize] public async Task Get([FromRoute] string username) { var user = await _userService.GetUser(username); if (user == null) { _logger.LogInformation(FormatLogMessage("Attempt to get a non-existent user.", Pair("Username", username))); - return NotFound(new CommonResponse(ErrorCodes.Get_NotExists, "The user does not exist.")); + return NotFound(new CommonResponse(ErrorCodes.Get_NotExist, "The user does not exist.")); } return Ok(user); } - [HttpPut("user/{username}"), AdminAuthorize] + [HttpPut("users/{username}"), AdminAuthorize] public async Task Put([FromBody] UserPutRequest request, [FromRoute] string username) { var result = await _userService.PutUser(username, request.Password, request.Administrator.Value); @@ -67,7 +67,7 @@ namespace Timeline.Controllers } } - [HttpPatch("user/{username}"), AdminAuthorize] + [HttpPatch("users/{username}"), AdminAuthorize] public async Task Patch([FromBody] UserPatchRequest request, [FromRoute] string username) { try @@ -78,11 +78,11 @@ namespace Timeline.Controllers catch (UserNotExistException e) { _logger.LogInformation(e, FormatLogMessage("Attempt to patch a non-existent user.", Pair("Username", username))); - return BadRequest(new CommonResponse(ErrorCodes.Patch_NotExists, "The user does not exist.")); + return BadRequest(new CommonResponse(ErrorCodes.Patch_NotExist , "The user does not exist.")); } } - [HttpDelete("user/{username}"), AdminAuthorize] + [HttpDelete("users/{username}"), AdminAuthorize] public async Task Delete([FromRoute] string username) { try -- cgit v1.2.3 From 58986da4a5bfe519af44e5834edfbe8d4651b51c Mon Sep 17 00:00:00 2001 From: 杨宇千 Date: Fri, 9 Aug 2019 15:39:58 +0800 Subject: Add UserController unit tests. --- .../Authentication/AuthenticationExtensions.cs | 1 + Timeline.Tests/Helpers/HttpClientExtensions.cs | 15 ++ Timeline.Tests/Helpers/InvalidModelTestHelpers.cs | 8 + Timeline.Tests/Helpers/ResponseExtensions.cs | 47 ++++++ Timeline.Tests/UserUnitTest.cs | 187 ++++++++++++++++++++- Timeline/Controllers/UserController.cs | 4 +- 6 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 Timeline.Tests/Helpers/HttpClientExtensions.cs (limited to 'Timeline/Controllers/UserController.cs') diff --git a/Timeline.Tests/Helpers/Authentication/AuthenticationExtensions.cs b/Timeline.Tests/Helpers/Authentication/AuthenticationExtensions.cs index e31bd51c..8a44c852 100644 --- a/Timeline.Tests/Helpers/Authentication/AuthenticationExtensions.cs +++ b/Timeline.Tests/Helpers/Authentication/AuthenticationExtensions.cs @@ -14,6 +14,7 @@ namespace Timeline.Tests.Helpers.Authentication public static async Task CreateUserTokenAsync(this HttpClient client, string username, string password, int? expireOffset = null) { var response = await client.PostAsJsonAsync(CreateTokenUrl, new CreateTokenRequest { Username = username, Password = password, ExpireOffset = expireOffset }); + response.AssertOk(); var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); return result; } diff --git a/Timeline.Tests/Helpers/HttpClientExtensions.cs b/Timeline.Tests/Helpers/HttpClientExtensions.cs new file mode 100644 index 00000000..cd40d91e --- /dev/null +++ b/Timeline.Tests/Helpers/HttpClientExtensions.cs @@ -0,0 +1,15 @@ +using Newtonsoft.Json; +using System.Net.Http; +using System.Text; +using System.Threading.Tasks; + +namespace Timeline.Tests.Helpers +{ + public static class HttpClientExtensions + { + public static Task PatchAsJsonAsync(this HttpClient client, string url, T body) + { + return client.PatchAsync(url, new StringContent(JsonConvert.SerializeObject(body), Encoding.UTF8, "application/json")); + } + } +} diff --git a/Timeline.Tests/Helpers/InvalidModelTestHelpers.cs b/Timeline.Tests/Helpers/InvalidModelTestHelpers.cs index 51919021..1c079d0e 100644 --- a/Timeline.Tests/Helpers/InvalidModelTestHelpers.cs +++ b/Timeline.Tests/Helpers/InvalidModelTestHelpers.cs @@ -15,5 +15,13 @@ namespace Timeline.Tests.Helpers var responseBody = await response.ReadBodyAsJson(); Assert.Equal(CommonResponse.ErrorCodes.InvalidModel, responseBody.Code); } + + public static async Task TestPutInvalidModel(HttpClient client, string url, T body) + { + var response = await client.PutAsJsonAsync(url, body); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var responseBody = await response.ReadBodyAsJson(); + Assert.Equal(CommonResponse.ErrorCodes.InvalidModel, responseBody.Code); + } } } diff --git a/Timeline.Tests/Helpers/ResponseExtensions.cs b/Timeline.Tests/Helpers/ResponseExtensions.cs index 155836fb..46c9e81d 100644 --- a/Timeline.Tests/Helpers/ResponseExtensions.cs +++ b/Timeline.Tests/Helpers/ResponseExtensions.cs @@ -1,11 +1,58 @@ using Newtonsoft.Json; +using System.Net; using System.Net.Http; using System.Threading.Tasks; +using Timeline.Models.Http; +using Xunit; namespace Timeline.Tests.Helpers { public static class ResponseExtensions { + public static void AssertOk(this HttpResponseMessage response) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + public static void AssertNotFound(this HttpResponseMessage response) + { + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + public static void AssertBadRequest(this HttpResponseMessage response) + { + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + public static async Task AssertIsPutCreated(this HttpResponseMessage response) + { + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + var body = await response.ReadBodyAsJson(); + Assert.Equal(CommonPutResponse.CreatedCode, body.Code); + } + + public static async Task AssertIsPutModified(this HttpResponseMessage response) + { + response.AssertOk(); + var body = await response.ReadBodyAsJson(); + Assert.Equal(CommonPutResponse.ModifiedCode, body.Code); + } + + + public static async Task AssertIsDeleteDeleted(this HttpResponseMessage response) + { + response.AssertOk(); + var body = await response.ReadBodyAsJson(); + Assert.Equal(CommonDeleteResponse.DeletedCode, body.Code); + } + + public static async Task AssertIsDeleteNotExist(this HttpResponseMessage response) + { + response.AssertOk(); + var body = await response.ReadBodyAsJson(); + Assert.Equal(CommonDeleteResponse.NotExistsCode, body.Code); + } + public static async Task ReadBodyAsJson(this HttpResponseMessage response) { return JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); diff --git a/Timeline.Tests/UserUnitTest.cs b/Timeline.Tests/UserUnitTest.cs index 7935dd9a..c5c91d34 100644 --- a/Timeline.Tests/UserUnitTest.cs +++ b/Timeline.Tests/UserUnitTest.cs @@ -1,9 +1,10 @@ using Microsoft.AspNetCore.Mvc.Testing; -using Newtonsoft.Json; -using System.Linq; using System.Net; +using System.Net.Http; using System.Threading.Tasks; +using Timeline.Controllers; using Timeline.Models; +using Timeline.Models.Http; using Timeline.Tests.Helpers; using Timeline.Tests.Helpers.Authentication; using Timeline.Tests.Mock.Data; @@ -22,15 +23,185 @@ namespace Timeline.Tests } [Fact] - public async Task UserTest() + public async Task Get_Users_List() { using (var client = await _factory.CreateClientAsAdmin()) { - var res1 = await client.GetAsync("users"); - Assert.Equal(HttpStatusCode.OK, res1.StatusCode); - var users = JsonConvert.DeserializeObject(await res1.Content.ReadAsStringAsync()).ToList(); - users.Sort(UserInfoComparers.Comparer); - Assert.Equal(MockUsers.UserInfos, users, UserInfoComparers.EqualityComparer); + var res = await client.GetAsync("users"); + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + + // Because tests are running asyncronized. So database may be modified and + // we can't check the exact user lists at this point. So only check the format. + + // var users = (await res.ReadBodyAsJson()).ToList(); + // users.Sort(UserInfoComparers.Comparer); + // Assert.Equal(MockUsers.UserInfos, users, UserInfoComparers.EqualityComparer); + await res.ReadBodyAsJson(); + } + } + + [Fact] + public async Task Get_Users_User() + { + using (var client = await _factory.CreateClientAsAdmin()) + { + var res = await client.GetAsync("users/" + MockUsers.UserUsername); + res.AssertOk(); + var user = await res.ReadBodyAsJson(); + Assert.Equal(MockUsers.UserUserInfo, user, UserInfoComparers.EqualityComparer); + } + } + + [Fact] + public async Task Get_Users_404() + { + using (var client = await _factory.CreateClientAsAdmin()) + { + var res = await client.GetAsync("users/usernotexist"); + res.AssertNotFound(); + var body = await res.ReadBodyAsJson(); + Assert.Equal(UserController.ErrorCodes.Get_NotExist, body.Code); + } + } + + [Fact] + public async Task Put_Patch_Delete_User() + { + using (var client = await _factory.CreateClientAsAdmin()) + { + const string username = "putpatchdeleteuser"; + const string password = "password"; + const string url = "users/" + username; + + // Put Invalid Model + await InvalidModelTestHelpers.TestPutInvalidModel(client, url, new UserPutRequest { Password = null, Administrator = false }); + await InvalidModelTestHelpers.TestPutInvalidModel(client, url, new UserPutRequest { Password = password, Administrator = null }); + + async Task CheckAdministrator(bool administrator) + { + var res = await client.GetAsync(url); + res.AssertOk(); + var body = await res.ReadBodyAsJson(); + Assert.Equal(administrator, body.Administrator); + } + + { + // Put Created. + var res = await client.PutAsJsonAsync(url, new UserPutRequest + { + Password = password, + Administrator = false + }); + await res.AssertIsPutCreated(); + await CheckAdministrator(false); + } + + { + // Put Modified. + var res = await client.PutAsJsonAsync(url, new UserPutRequest + { + Password = password, + Administrator = true + }); + await res.AssertIsPutModified(); + await CheckAdministrator(true); + } + + // Patch Not Exist + { + var res = await client.PatchAsJsonAsync("users/usernotexist", new UserPatchRequest { }); + res.AssertNotFound(); + var body = await res.ReadBodyAsJson(); + Assert.Equal(UserController.ErrorCodes.Patch_NotExist, body.Code); + } + + // Patch Success + { + var res = await client.PatchAsJsonAsync(url, new UserPatchRequest { Administrator = false }); + res.AssertOk(); + await CheckAdministrator(false); + } + + // Delete Deleted + { + var res = await client.DeleteAsync(url); + await res.AssertIsDeleteDeleted(); + + var res2 = await client.GetAsync(url); + res2.AssertNotFound(); + } + + // Delete Not Exist + { + var res = await client.DeleteAsync(url); + await res.AssertIsDeleteNotExist(); + } + } + } + + + public class ChangePasswordUnitTest : IClassFixture> + { + private const string url = "userop/changepassword"; + + private readonly WebApplicationFactory _factory; + + public ChangePasswordUnitTest(MyWebApplicationFactory factory, ITestOutputHelper outputHelper) + { + _factory = factory.WithTestLogging(outputHelper); + } + + + [Fact] + public async Task InvalidModel_OldPassword() + { + using (var client = await _factory.CreateClientAsUser()) + { + await InvalidModelTestHelpers.TestPostInvalidModel(client, url, new ChangePasswordRequest { OldPassword = null, NewPassword = "???" }); + } + } + + [Fact] + public async Task InvalidModel_NewPassword() + { + using (var client = await _factory.CreateClientAsUser()) + { + await InvalidModelTestHelpers.TestPostInvalidModel(client, url, new ChangePasswordRequest { OldPassword = "???", NewPassword = null }); + } + } + + [Fact] + public async Task BadOldPassword() + { + using (var client = await _factory.CreateClientAsUser()) + { + var res = await client.PostAsJsonAsync(url, new ChangePasswordRequest { OldPassword = "???", NewPassword = "???" }); + res.AssertBadRequest(); + var body = await res.ReadBodyAsJson(); + Assert.Equal(UserController.ErrorCodes.ChangePassword_BadOldPassword, body.Code); + } + } + + [Fact] + public async Task Success() + { + const string username = "changepasswordtest"; + const string password = "password"; + + // create a new user to avoid interference + using (var client = await _factory.CreateClientAsAdmin()) + { + var res = await client.PutAsJsonAsync("users/" + username, new UserPutRequest { Password = password, Administrator = false }); + Assert.Equal(HttpStatusCode.Created, res.StatusCode); + } + + using (var client = await _factory.CreateClientWithCredential(username, password)) + { + const string newPassword = "new"; + var res = await client.PostAsJsonAsync(url, new ChangePasswordRequest { OldPassword = password, NewPassword = newPassword }); + res.AssertOk(); + await client.CreateUserTokenAsync(username, newPassword); + } } } } diff --git a/Timeline/Controllers/UserController.cs b/Timeline/Controllers/UserController.cs index af4cfb53..6f2fe77f 100644 --- a/Timeline/Controllers/UserController.cs +++ b/Timeline/Controllers/UserController.cs @@ -14,7 +14,7 @@ namespace Timeline.Controllers [ApiController] public class UserController : Controller { - private static class ErrorCodes + public static class ErrorCodes { public const int Get_NotExist = -1001; @@ -78,7 +78,7 @@ namespace Timeline.Controllers catch (UserNotExistException e) { _logger.LogInformation(e, FormatLogMessage("Attempt to patch a non-existent user.", Pair("Username", username))); - return BadRequest(new CommonResponse(ErrorCodes.Patch_NotExist , "The user does not exist.")); + return NotFound(new CommonResponse(ErrorCodes.Patch_NotExist, "The user does not exist.")); } } -- cgit v1.2.3