diff --git a/src/Web/WebMVC/Controllers/AccountController.cs b/src/Web/WebMVC/Controllers/AccountController.cs index ab2d75da8..0dee2ccfb 100644 --- a/src/Web/WebMVC/Controllers/AccountController.cs +++ b/src/Web/WebMVC/Controllers/AccountController.cs @@ -11,16 +11,11 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers public class AccountController : Controller { private readonly IIdentityParser _identityParser; - public AccountController(IIdentityParser identityParser) - { + public AccountController(IIdentityParser identityParser) => _identityParser = identityParser; - } - - public ActionResult Index() - { - return View(); - } + public ActionResult Index() => View(); + [Authorize] public IActionResult SignIn(string returnUrl) { diff --git a/src/Web/WebMVC/Controllers/CatalogController.cs b/src/Web/WebMVC/Controllers/CatalogController.cs index 4c649e507..83ea0b4a0 100644 --- a/src/Web/WebMVC/Controllers/CatalogController.cs +++ b/src/Web/WebMVC/Controllers/CatalogController.cs @@ -14,10 +14,8 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers { private ICatalogService _catalogSvc; - public CatalogController(ICatalogService catalogSvc) - { + public CatalogController(ICatalogService catalogSvc) => _catalogSvc = catalogSvc; - } public async Task Index(int? BrandFilterApplied, int? TypesFilterApplied, int? page) { @@ -35,7 +33,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers ActualPage = page ?? 0, ItemsPerPage = catalog.Data.Count, TotalItems = catalog.Count, - TotalPages = int.Parse(Math.Ceiling(((decimal)catalog.Count / itemsPage)).ToString()) + TotalPages = (int)Math.Ceiling(((decimal)catalog.Count / itemsPage)) } }; @@ -45,10 +43,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers return View(vm); } - public IActionResult Error() - { - return View(); - } + public IActionResult Error() => View(); } } diff --git a/src/Web/WebMVC/Extensions/HttpClientExtensions.cs b/src/Web/WebMVC/Extensions/HttpClientExtensions.cs index d1b83dc00..a1d6f215d 100644 --- a/src/Web/WebMVC/Extensions/HttpClientExtensions.cs +++ b/src/Web/WebMVC/Extensions/HttpClientExtensions.cs @@ -11,20 +11,14 @@ namespace Microsoft.eShopOnContainers.WebMVC.Extensions { public static class HttpClientExtensions { - public static void SetBasicAuthentication(this HttpClient client, string userName, string password) - { + public static void SetBasicAuthentication(this HttpClient client, string userName, string password) => client.DefaultRequestHeaders.Authorization = new BasicAuthenticationHeaderValue(userName, password); - } - public static void SetToken(this HttpClient client, string scheme, string token) - { + public static void SetToken(this HttpClient client, string scheme, string token) => client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(scheme, token); - } - public static void SetBearerToken(this HttpClient client, string token) - { + public static void SetBearerToken(this HttpClient client, string token) => client.SetToken(JwtConstants.TokenType, token); - } } public class BasicAuthenticationHeaderValue : AuthenticationHeaderValue diff --git a/src/Web/WebMVC/Extensions/SessionExtensions.cs b/src/Web/WebMVC/Extensions/SessionExtensions.cs index 4b095866d..79bd435f0 100644 --- a/src/Web/WebMVC/Extensions/SessionExtensions.cs +++ b/src/Web/WebMVC/Extensions/SessionExtensions.cs @@ -8,10 +8,8 @@ using System.Threading.Tasks; public static class SessionExtensions { - public static void SetObject(this ISession session, string key, object value) - { + public static void SetObject(this ISession session, string key, object value) => session.SetString(key, JsonConvert.SerializeObject(value)); - } public static T GetObject(this ISession session, string key) { diff --git a/src/Web/WebMVC/Services/BasketService.cs b/src/Web/WebMVC/Services/BasketService.cs index bcb138b87..0e6b13ab5 100644 --- a/src/Web/WebMVC/Services/BasketService.cs +++ b/src/Web/WebMVC/Services/BasketService.cs @@ -33,16 +33,14 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services _apiClient.Inst.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token); - var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id.ToString()}"; + var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id}"; var dataString = await _apiClient.GetStringAsync(basketUrl); - var response = JsonConvert.DeserializeObject(dataString); - if (response == null) - { - response = new Basket() + // Use the ?? Null conditional operator to simplify the initialization of response + var response = JsonConvert.DeserializeObject(dataString) ?? + new Basket() { BuyerId = user.Id }; - } return response; } @@ -67,9 +65,12 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services basket.Items.ForEach(x => { - var quantity = quantities.Where(y => y.Key == x.Id).FirstOrDefault(); - if (quantities.Where(y => y.Key == x.Id).Count() > 0) - x.Quantity = quantity.Value; + // Simplify this logic by using the + // new out variable initializer. + if (quantities.TryGetValue(x.Id, out var quantity)) + { + x.Quantity = quantity; + } }); return basket; @@ -119,7 +120,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services var token = await context.Authentication.GetTokenAsync("access_token"); _apiClient.Inst.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token); - var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id.ToString()}"; + var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id}"; var response = await _apiClient.DeleteAsync(basketUrl); //CCE: response status code... diff --git a/src/Web/WebMVC/Services/CatalogService.cs b/src/Web/WebMVC/Services/CatalogService.cs index 83b139c1f..0e521c637 100644 --- a/src/Web/WebMVC/Services/CatalogService.cs +++ b/src/Web/WebMVC/Services/CatalogService.cs @@ -61,8 +61,11 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services JArray brands = JArray.Parse(dataString); foreach (JObject brand in brands.Children()) { - dynamic item = brand; - items.Add(new SelectListItem() { Value = item.id, Text = item.brand }); + items.Add(new SelectListItem() + { + Value = brand.Value("id"), + Text = brand.Value("brand") + }); } return items; @@ -79,10 +82,12 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services JArray brands = JArray.Parse(dataString); foreach (JObject brand in brands.Children()) { - dynamic item = brand; - items.Add(new SelectListItem() { Value = item.id, Text = item.type }); + items.Add(new SelectListItem() + { + Value = brand.Value("id"), + Text = brand.Value("type") + }); } - return items; } } diff --git a/src/Web/WebMVC/Services/IdentityParser.cs b/src/Web/WebMVC/Services/IdentityParser.cs index 619cfa517..70e82e53b 100644 --- a/src/Web/WebMVC/Services/IdentityParser.cs +++ b/src/Web/WebMVC/Services/IdentityParser.cs @@ -12,26 +12,31 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services { public ApplicationUser Parse(IPrincipal principal) { - var user = new ApplicationUser(); - var claims = (ClaimsPrincipal)principal; + // Pattern matching 'is' expression + // assigns "claims" if "principal" is a "ClaimsPrincipal" + if (principal is ClaimsPrincipal claims) + { + return new ApplicationUser + { - user.CardHolderName = (claims.Claims.Where(x => x.Type == "card_holder").Count() > 0) ? claims.Claims.First(x => x.Type == "card_holder").Value : ""; - user.CardNumber = (claims.Claims.Where(x => x.Type == "card_number").Count() > 0) ? claims.Claims.First(x => x.Type == "card_number").Value : ""; - user.Expiration = (claims.Claims.Where(x => x.Type == "card_expiration").Count() > 0) ? claims.Claims.First(x => x.Type == "card_expiration").Value : ""; - user.CardType = (claims.Claims.Where(x => x.Type == "missing").Count() > 0) ? int.Parse(claims.Claims.First(x => x.Type == "missing").Value) : 0; - user.City = (claims.Claims.Where(x => x.Type == "address_city").Count() > 0) ? claims.Claims.First(x => x.Type == "address_city").Value : ""; - user.Country = (claims.Claims.Where(x => x.Type == "address_country").Count() > 0) ? claims.Claims.First(x => x.Type == "address_country").Value : ""; - user.Email = (claims.Claims.Where(x => x.Type == "email").Count() > 0) ? claims.Claims.First(x => x.Type == "email").Value : ""; - user.Id = (claims.Claims.Where(x => x.Type == "sub").Count() > 0) ? claims.Claims.First(x => x.Type == "sub").Value : ""; - user.LastName = (claims.Claims.Where(x => x.Type == "last_name").Count() > 0) ? claims.Claims.First(x => x.Type == "last_name").Value : ""; - user.Name = (claims.Claims.Where(x => x.Type == "name").Count() > 0) ? claims.Claims.First(x => x.Type == "name").Value : ""; - user.PhoneNumber = (claims.Claims.Where(x => x.Type == "phone_number").Count() > 0) ? claims.Claims.First(x => x.Type == "phone_number").Value : ""; - user.SecurityNumber = (claims.Claims.Where(x => x.Type == "card_security_number").Count() > 0) ? claims.Claims.First(x => x.Type == "card_security_number").Value : ""; - user.State = (claims.Claims.Where(x => x.Type == "address_state").Count() > 0) ? claims.Claims.First(x => x.Type == "address_state").Value : ""; - user.Street = (claims.Claims.Where(x => x.Type == "address_street").Count() > 0) ? claims.Claims.First(x => x.Type == "address_street").Value : ""; - user.ZipCode = (claims.Claims.Where(x => x.Type == "address_zip_code").Count() > 0) ? claims.Claims.First(x => x.Type == "address_zip_code").Value : ""; - - return user; + CardHolderName = claims.Claims.FirstOrDefault(x => x.Type == "card_holder")?.Value ?? "", + CardNumber = claims.Claims.FirstOrDefault(x => x.Type == "card_number")?.Value ?? "", + Expiration = claims.Claims.FirstOrDefault(x => x.Type == "card_expiration")?.Value ?? "", + CardType = int.Parse(claims.Claims.FirstOrDefault(x => x.Type == "missing")?.Value ?? "0"), + City = claims.Claims.FirstOrDefault(x => x.Type == "address_city")?.Value ?? "", + Country = claims.Claims.FirstOrDefault(x => x.Type == "address_country")?.Value ?? "", + Email = claims.Claims.FirstOrDefault(x => x.Type == "email")?.Value ?? "", + Id = claims.Claims.FirstOrDefault(x => x.Type == "sub")?.Value ?? "", + LastName = claims.Claims.FirstOrDefault(x => x.Type == "last_name")?.Value ?? "", + Name = claims.Claims.FirstOrDefault(x => x.Type == "name")?.Value ?? "", + PhoneNumber = claims.Claims.FirstOrDefault(x => x.Type == "phone_number")?.Value ?? "", + SecurityNumber = claims.Claims.FirstOrDefault(x => x.Type == "card_security_number")?.Value ?? "", + State = claims.Claims.FirstOrDefault(x => x.Type == "address_state")?.Value ?? "", + Street = claims.Claims.FirstOrDefault(x => x.Type == "address_street")?.Value ?? "", + ZipCode = claims.Claims.FirstOrDefault(x => x.Type == "address_zip_code")?.Value ?? "" + }; + } + throw new ArgumentException(message: "The principal must be a ClaimsPrincipal", paramName: nameof(principal)); } } } diff --git a/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs b/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs index 8b142f393..30a31ff23 100644 --- a/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs +++ b/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs @@ -16,30 +16,18 @@ namespace WebMVC.Services.Utilities _client = new HttpClient(); _logger = new LoggerFactory().CreateLogger(nameof(HttpApiClientWrapper)); } + + public Task GetStringAsync(string uri) => + _client.GetStringAsync(uri); - public async Task GetStringAsync(string uri) - { - return await HttpInvoker(async () => - await _client.GetStringAsync(uri)); - } - - public async Task PostAsync(string uri, T item) + public Task PostAsync(string uri, T item) { var contentString = new StringContent(JsonConvert.SerializeObject(item), System.Text.Encoding.UTF8, "application/json"); - return await HttpInvoker(async () => - await _client.PostAsync(uri, contentString)); - } - - public async Task DeleteAsync(string uri) - { - return await HttpInvoker(async () => - await _client.DeleteAsync(uri)); + return _client.PostAsync(uri, contentString); } - private async Task HttpInvoker(Func> action) - { - return await action(); - } + public Task DeleteAsync(string uri) => + _client.DeleteAsync(uri); } } diff --git a/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs b/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs index dba0db9df..5dd26a0fb 100644 --- a/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs +++ b/src/Web/WebMVC/Services/Utilities/HttpApiClientWrapper.cs @@ -24,76 +24,74 @@ namespace WebMVC.Services.Utilities CreateRetryPolicy(), CreateCircuitBreakerPolicy() ); - } - - private Policy CreateCircuitBreakerPolicy() - { - return Policy - .Handle() - .CircuitBreakerAsync( - // number of exceptions before breaking circuit - 3, - // time circuit opened before retry - TimeSpan.FromMinutes(1), - (exception, duration) => { - // on circuit opened - _logger.LogTrace("Circuit breaker opened"); - }, - () => { - // on circuit closed - _logger.LogTrace("Circuit breaker reset"); - } - ); } - private Policy CreateRetryPolicy() - { - return Policy - .Handle() - .WaitAndRetryAsync( - // number of retries - 3, - // exponential backofff - retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)), - // on retry - (exception, timeSpan, retryCount, context) => - { - _logger.LogTrace($"Retry {retryCount} " + - $"of {context.PolicyKey} " + - $"at {context.ExecutionKey}, " + - $"due to: {exception}."); - }); - } + private Policy CreateCircuitBreakerPolicy() => + Policy.Handle() + .CircuitBreakerAsync( + // number of exceptions before breaking circuit + 3, + // time circuit opened before retry + TimeSpan.FromMinutes(1), + (exception, duration) => + { + // on circuit opened + _logger.LogTrace("Circuit breaker opened"); + }, + () => + { + // on circuit closed + _logger.LogTrace("Circuit breaker reset"); + } + ); - public async Task GetStringAsync(string uri) - { - return await HttpInvoker(async () => - await _client.GetStringAsync(uri)); - } + private Policy CreateRetryPolicy() => + Policy.Handle() + .WaitAndRetryAsync( + // number of retries + 3, + // exponential backofff + retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)), + // on retry + (exception, timeSpan, retryCount, context) => + { + _logger.LogTrace($"Retry {retryCount} " + + $"of {context.PolicyKey} " + + $"at {context.ExecutionKey}, " + + $"due to: {exception}."); + } + ); - public async Task PostAsync(string uri, T item) - { + // Notice that these (and other methods below) are Task + // returning asynchronous methods. But, they do not + // have the 'async' modifier, and do not contain + // any 'await statements. In each of these methods, + // the only asynchronous call is the last (or only) + // statement of the method. In those instances, + // a Task returning method that does not use the + // async modifier is preferred. The compiler generates + // synchronous code for this method, but returns the + // task from the underlying asynchronous method. The + // generated code does not contain the state machine + // generated for asynchronous methods. + public Task GetStringAsync(string uri) => + HttpInvoker(() => _client.GetStringAsync(uri)); + + public Task PostAsync(string uri, T item) => // a new StringContent must be created for each retry // as it is disposed after each call - return await HttpInvoker(async () => - await _client.PostAsync(uri, - new StringContent(JsonConvert.SerializeObject(item), - System.Text.Encoding.UTF8, "application/json"))); - } + HttpInvoker(() =>_client.PostAsync(uri, + new StringContent(JsonConvert.SerializeObject(item), + System.Text.Encoding.UTF8, "application/json"))); - public async Task DeleteAsync(string uri) - { - return await HttpInvoker(async () => - await _client.DeleteAsync(uri)); - } + public Task DeleteAsync(string uri) => + HttpInvoker(() => _client.DeleteAsync(uri)); - private async Task HttpInvoker(Func> action) - { + + private Task HttpInvoker(Func> action) => // Executes the action applying all // the policies defined in the wrapper - return await _policyWrapper - .ExecuteAsync(async () => await action()); - } + _policyWrapper.ExecuteAsync(() => action()); } } diff --git a/src/Web/WebMVC/ViewComponents/Cart.cs b/src/Web/WebMVC/ViewComponents/Cart.cs index 952c5ed3f..2db0e35b6 100644 --- a/src/Web/WebMVC/ViewComponents/Cart.cs +++ b/src/Web/WebMVC/ViewComponents/Cart.cs @@ -13,10 +13,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewComponents { private readonly IBasketService _cartSvc; - public Cart(IBasketService cartSvc) - { - _cartSvc = cartSvc; - } + public Cart(IBasketService cartSvc) => _cartSvc = cartSvc; public async Task InvokeAsync(ApplicationUser user) { diff --git a/src/Web/WebMVC/ViewComponents/CartList.cs b/src/Web/WebMVC/ViewComponents/CartList.cs index fbb9894fe..099cc8a3d 100644 --- a/src/Web/WebMVC/ViewComponents/CartList.cs +++ b/src/Web/WebMVC/ViewComponents/CartList.cs @@ -12,19 +12,14 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewComponents { private readonly IBasketService _cartSvc; - public CartList(IBasketService cartSvc) - { - _cartSvc = cartSvc; - } + public CartList(IBasketService cartSvc) => _cartSvc = cartSvc; public async Task InvokeAsync(ApplicationUser user) { var item = await GetItemsAsync(user); return View(item); } - private async Task GetItemsAsync(ApplicationUser user) - { - return await _cartSvc.GetBasket(user); - } + + private Task GetItemsAsync(ApplicationUser user) => _cartSvc.GetBasket(user); } } diff --git a/src/Web/WebMVC/ViewModels/Annotations/CardExpiration.cs b/src/Web/WebMVC/ViewModels/Annotations/CardExpiration.cs index 5955ab30f..d60150a44 100644 --- a/src/Web/WebMVC/ViewModels/Annotations/CardExpiration.cs +++ b/src/Web/WebMVC/ViewModels/Annotations/CardExpiration.cs @@ -14,11 +14,20 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels.Annotations if (value == null) return false; - var month = value.ToString().Split('/')[0]; - var year = $"20{value.ToString().Split('/')[1]}"; - DateTime d = new DateTime(int.Parse(year), int.Parse(month), 1); + var monthString = value.ToString().Split('/')[0]; + var yearString = $"20{value.ToString().Split('/')[1]}"; + // Use the 'out' variable initializer to simplify + // the logic of validating the expiration date + if ((int.TryParse(monthString, out var month)) && + (int.TryParse(yearString, out var year))) + { + DateTime d = new DateTime(year, month, 1); - return d > DateTime.UtcNow; + return d > DateTime.UtcNow; + } else + { + return false; + } } } } diff --git a/src/Web/WebMVC/ViewModels/Basket.cs b/src/Web/WebMVC/ViewModels/Basket.cs index aef34e4fd..b95a910a2 100644 --- a/src/Web/WebMVC/ViewModels/Basket.cs +++ b/src/Web/WebMVC/ViewModels/Basket.cs @@ -7,11 +7,11 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels { public class Basket { - public Basket() - { - Items = new List(); - } - public List Items { get; set; } + // Use property initializer syntax. + // While this is often more useful for read only + // auto implemented properties, it can simplify logic + // for read/write properties. + public List Items { get; set; } = new List(); public string BuyerId { get; set; } public decimal Total() diff --git a/src/Web/WebMVC/ViewModels/CartViewModels/IndexViewModel.cs b/src/Web/WebMVC/ViewModels/CartViewModels/IndexViewModel.cs index e46fdd4a3..9a8802542 100644 --- a/src/Web/WebMVC/ViewModels/CartViewModels/IndexViewModel.cs +++ b/src/Web/WebMVC/ViewModels/CartViewModels/IndexViewModel.cs @@ -9,6 +9,6 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels.CartViewModels public class CartComponentViewModel { public int ItemsCount { get; set; } - public string Disabled { get { return (ItemsCount == 0) ? "is-disabled" : ""; } } + public string Disabled => (ItemsCount == 0) ? "is-disabled" : ""; } } diff --git a/src/Web/WebMVC/ViewModels/Order.cs b/src/Web/WebMVC/ViewModels/Order.cs index 8f3acc562..785b29119 100644 --- a/src/Web/WebMVC/ViewModels/Order.cs +++ b/src/Web/WebMVC/ViewModels/Order.cs @@ -11,10 +11,6 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels { public class Order { - public Order() { - OrderItems = new List(); - } - public string OrderNumber {get;set;} public DateTime Date {get;set;} @@ -53,7 +49,10 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels public string Buyer { get; set; } - public List OrderItems { get; } + // See the property initializer syntax below. This + // initializes the compiler generated field for this + // auto-implemented property. + public List OrderItems { get; } = new List(); [Required] public Guid RequestId { get; set; }