From 00491910a26755a846c254a9d3e41dcf6d581217 Mon Sep 17 00:00:00 2001 From: Bill Wagner Date: Mon, 20 Mar 2017 14:18:20 -0400 Subject: [PATCH 1/4] C# 7 language feature updates Controllers/AccountController: Replace single line methods with expression bodied members. Includes constructor, C# 7 feature. Controllers/CatalogController: Remove unnecessary ToString() call. Replace single line methods with expression bodied members. Extensions/HttpClientExtensions: Replace single line methods with expression bodied members. Extensions/SessionExtensions: Replace single line methods with expression bodied members. Services/BasketService: Remove unnecessary ToString() calls. Add ?? to simplify conditional initialization Use TryGetValue and out variable initialization to simplify Quantity calculation Services/CatalogService: Use Value generic method instead of dynamic types. There is a lot of overhead for dynamic and it seems overkill here. Services/IdentityParser: Use the pattern matching is expression. Refactor the LINQ queries to enumerate the collection (and create an enumerator) only once. The previous code had 3 enumerations. Services/Utilities/HttpApiClient: Remove the 'async' modifier and 'await' statements from methods where the only asynchronous statement is the final statement of the method, and the task from the underlying method can be returned. Services/Utilities/HttpApiClientWrapper: Use expression bodied members where applicable. Remove the 'async' modifier and 'await' statements from methods where the only asynchronous statement is the final statement of the method, and the task from the underlying method can be returned. ViewComponents/Cart: Use expression bodied members where applicable. ViewComponents/CartList: Use expression bodied members where applicable. Remove the 'async' modifier and 'await' statements from methods where the only asynchronous statement is the final statement of the method, and the task from the underlying method can be returned. ViewModels/Annotations/CardExpiration: Use the out variable initializer to simplify the validation of the card expiration date. ViewModels/Basket: Use property initializer syntax instead of constructor body ViewModels/CatalogViewModels/IndexViewModel: Use expression bodied property to return the calculated 'Disabled' property ViewModels/Order: Use property initializer syntax. --- .../WebMVC/Controllers/AccountController.cs | 11 +- .../WebMVC/Controllers/CatalogController.cs | 11 +- .../WebMVC/Extensions/HttpClientExtensions.cs | 12 +- .../WebMVC/Extensions/SessionExtensions.cs | 4 +- src/Web/WebMVC/Services/BasketService.cs | 21 ++-- src/Web/WebMVC/Services/CatalogService.cs | 15 ++- src/Web/WebMVC/Services/IdentityParser.cs | 43 ++++--- .../Services/Utilities/HttpApiClient.cs | 36 +++--- .../Utilities/HttpApiClientWrapper.cs | 118 +++++++++--------- src/Web/WebMVC/ViewComponents/Cart.cs | 5 +- src/Web/WebMVC/ViewComponents/CartList.cs | 26 ++-- .../ViewModels/Annotations/CardExpiration.cs | 17 ++- src/Web/WebMVC/ViewModels/Basket.cs | 10 +- .../CartViewModels/IndexViewModel.cs | 2 +- src/Web/WebMVC/ViewModels/Order.cs | 9 +- 15 files changed, 173 insertions(+), 167 deletions(-) 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..87c885b82 100644 --- a/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs +++ b/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs @@ -17,29 +17,29 @@ namespace WebMVC.Services.Utilities _logger = new LoggerFactory().CreateLogger(nameof(HttpApiClientWrapper)); } - public async Task GetStringAsync(string uri) - { - return await HttpInvoker(async () => - await _client.GetStringAsync(uri)); - } + // 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) => + _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)); + return _client.PostAsync(uri, contentString); } - public async Task DeleteAsync(string uri) - { - return await HttpInvoker(async () => - await _client.DeleteAsync(uri)); - } - - 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..055a79a25 100644 --- a/src/Web/WebMVC/ViewComponents/CartList.cs +++ b/src/Web/WebMVC/ViewComponents/CartList.cs @@ -12,19 +12,29 @@ 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); - } + + // Notice that this method is a Task + // returning asynchronous method. But, it does not + // have the 'async' modifier, and does not contain + // any 'await statements. + // 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. + // Contrast that with the method above, which calls + // and awaits an asynchronous method, and then processes + // it further. + 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; } From 8c6d880f1850e1ed5ca7c51ce6815100f8041150 Mon Sep 17 00:00:00 2001 From: RamonTC Date: Tue, 21 Mar 2017 12:51:25 +0100 Subject: [PATCH 2/4] Remove async await issue comments Thanks for the review! --- src/Web/WebMVC/Services/Utilities/HttpApiClient.cs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs b/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs index 87c885b82..30a31ff23 100644 --- a/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs +++ b/src/Web/WebMVC/Services/Utilities/HttpApiClient.cs @@ -16,19 +16,7 @@ namespace WebMVC.Services.Utilities _client = new HttpClient(); _logger = new LoggerFactory().CreateLogger(nameof(HttpApiClientWrapper)); } - - // 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) => _client.GetStringAsync(uri); From ec71fbb5ae4136818848eca2218d044b24d67bbc Mon Sep 17 00:00:00 2001 From: RamonTC Date: Tue, 21 Mar 2017 12:52:59 +0100 Subject: [PATCH 3/4] Remove async await issue comments --- src/Web/WebMVC/ViewComponents/CartList.cs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/Web/WebMVC/ViewComponents/CartList.cs b/src/Web/WebMVC/ViewComponents/CartList.cs index 055a79a25..099cc8a3d 100644 --- a/src/Web/WebMVC/ViewComponents/CartList.cs +++ b/src/Web/WebMVC/ViewComponents/CartList.cs @@ -19,22 +19,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewComponents var item = await GetItemsAsync(user); return View(item); } - - // Notice that this method is a Task - // returning asynchronous method. But, it does not - // have the 'async' modifier, and does not contain - // any 'await statements. - // 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. - // Contrast that with the method above, which calls - // and awaits an asynchronous method, and then processes - // it further. + private Task GetItemsAsync(ApplicationUser user) => _cartSvc.GetBasket(user); } } From 8a689c45f251e0c8d4498cddad01c5c4d418b42d Mon Sep 17 00:00:00 2001 From: dsanz Date: Tue, 21 Mar 2017 13:10:40 +0100 Subject: [PATCH 4/4] Add Delete and Create actions to the CatalogController. --- .../Controllers/CatalogController.cs | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/src/Services/Catalog/Catalog.API/Controllers/CatalogController.cs b/src/Services/Catalog/Catalog.API/Controllers/CatalogController.cs index 431928e0b..4add23b35 100644 --- a/src/Services/Catalog/Catalog.API/Controllers/CatalogController.cs +++ b/src/Services/Catalog/Catalog.API/Controllers/CatalogController.cs @@ -130,20 +130,21 @@ namespace Microsoft.eShopOnContainers.Services.Catalog.API.Controllers return Ok(items); } + [Route("edit")] [HttpPost] - public async Task Post([FromBody]CatalogItem value) + public async Task EditProduct([FromBody]CatalogItem product) { - var item = await _context.CatalogItems.SingleOrDefaultAsync(i => i.Id == value.Id); + var item = await _context.CatalogItems.SingleOrDefaultAsync(i => i.Id == product.Id); if (item == null) { return NotFound(); } - if (item.Price != value.Price) + if (item.Price != product.Price) { var oldPrice = item.Price; - item.Price = value.Price; + item.Price = product.Price; _context.CatalogItems.Update(item); var @event = new ProductPriceChangedIntegrationEvent(item.Id, item.Price, oldPrice); @@ -158,10 +159,47 @@ namespace Microsoft.eShopOnContainers.Services.Catalog.API.Controllers eventLogEntry.State = EventStateEnum.Published; _context.IntegrationEventLog.Update(eventLogEntry); await _context.SaveChangesAsync(); - } + } return Ok(); - } + } + + [Route("create")] + [HttpPost] + public async Task CreateProduct([FromBody]CatalogItem product) + { + _context.CatalogItems.Add( + new CatalogItem + { + CatalogBrandId = product.CatalogBrandId, + CatalogTypeId = product.CatalogTypeId, + Description = product.Description, + Name = product.Name, + PictureUri = product.PictureUri, + Price = product.Price + }); + + await _context.SaveChangesAsync(); + + return Ok(); + } + + [Route("{id}")] + [HttpDelete] + public async Task DeleteProduct(int id) + { + var product = _context.CatalogItems.SingleOrDefault(x => x.Id == id); + + if (product == null) + { + return NotFound(); + } + + _context.CatalogItems.Remove(product); + await _context.SaveChangesAsync(); + + return Ok(); + } private List ComposePicUri(List items) { var baseUri = _settings.Value.ExternalCatalogBaseUrl;