Pull to refresh

Comments 9

Почему был выбран JsonResult?


return success ? new JsonResult($"Update successful {document.Id}") : new JsonResult("Update was not successful")

Т.е результат вроде бы 200 ОК, а вроде и нет.
Иногда сталкиваюсь с преимущественно старыми системами, которые возвращают ошибки с 200-м кодом ответа Http. Столько "радости" с ними интегрироваться.


В этом случае правильнее использовать наследников StatusCodeResult, например BadRequestResult,OkResult.

Тестирование

Что конкретно вы тестируете и зачем?

Не статья, а сборник вредных советов.
  1. Зачем вызывать Context.SaveChanges(); в методах BaseRepository? Как вы откатите Worker и Car, если упал Document.Create?
  2. services.AddTransient<IBaseRepository<*>, BaseRepository<*>>(); Зачем, если DbContext — Scoped по умолчанию?
  3. Зачем использовать JsonResut, если есть Ok(), Created(), BadRequest() и т.д.?
  4. При поиске по первичному ключу желательно использовать Find, а не FirstOrDefault
  5. Очень странное решение инжектить репозитории в сервисе через свойства, а не конструктор
  6. В Delete два раза загружается entity
  7. Возвращать из контроллера entity — плохая идея
  8. В тесте — это что вообще такое? )))
    serviceMock.Object.Work(); 
    
    serviceMock.Verify(x => x.Work());
Вот действительно, не надо так!
1. В статье куча всего (вредного) кроме самого Rest API
2. Rest API реализован с ошибками (ошибка обработки должна возвращать ошибочный статус, почему-то get только на список, а как получить один экземпляр? Post и вовсе ничего не принимает на вход)
3. Да и вообще, реализацию API стоит начинать с описание этого API.
4. Зачем писать тест, который ничего не тестирует?
5-12. пункты из предыдущего комментария.
Уважаемые авторы!

Большая просьба, не писать статьи если вы действительно не обладаете опытом в данной теме. И тем более, не нужно писать статьи «для новичков» ведь именно они-то и не могут отделять зерен от плевел и в результате начинают тупо копировать странные и вредные практики.

Если по существу, запомните золотое правило архитектуры ПО — не усложняйте архитектуру без надобности. Например, зачем необходим

public abstract class BaseModel
{
    public Guid Id { get; set; }
}

Какие задачи он упрощает потом? Что будет, если мне нужна будет сущность с интовым Id? Я понимаю, если бы было сделано что-то типа этого:


protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    foreach (var entityType in modelBuilder.Model.GetEntityTypes())
    {
        if (typeof(ISoftDelete).IsAssignableFrom(entityType.ClrType))
        {
            entityType.AddSoftDeleteQueryFilter();      
        }    
    }
}
public static void AddSoftDeleteQueryFilter(
        this IMutableEntityType entityData)
    {
        var methodToCall = typeof(SoftDeleteQueryExtension)
            .GetMethod(nameof(GetSoftDeleteFilter),
                BindingFlags.NonPublic | BindingFlags.Static)
            .MakeGenericMethod(entityData.ClrType);
        var filter = methodToCall.Invoke(null, new object[] { });
        entityData.SetQueryFilter((LambdaExpression)filter);
    }
 
    private static LambdaExpression GetSoftDeleteFilter<TEntity>()
        where TEntity : class, ISoftDelete
    {
        Expression<Func<TEntity, bool>> filter = x => !x.SoftDeleted;
        return filter;
    }

В чем тут профит — понятно. Наследуя один раз интерфейс, мы под капотом оставляем кучу работы связанной со скрытием «удаленных» сущностей. Аналогично можно сделать тинанты или аудит. Здесь же мы просто не пишем один раз проперти, зато пишем наследование! Фантастика!

Аналогична ситуация и с

public interface IBaseRepository<TDbModel> where TDbModel : BaseModel
{
        public List<TDbModel> GetAll();
        public TDbModel Get(Guid id);
        public TDbModel Create(TDbModel model);
        public TDbModel Update(TDbModel model);
        public void Delete(Guid id);
}

В чем смысл данной сущности? Что она дает такого нового, что отлично от DbSet? В ней нет никакого смысла, просто еще один слой абстракции который еще потом требует вот такого чуда:

services.AddTransient<IBaseRepository<Car>, BaseRepository<Car>>();

А теперь представьте проект с сотнями моделей? Мы просто создали себе дополнительную работу и не решили ни одной проблемы. Например, нам нужно использовать какой-нибудь хитрый фильтр по автомобилям в трех различных местах. Логично вынести этот фильтр в отдельный сервис для доступа к данным тем самым не повторяя код. Но в данных примерах этого нет, там простое повторение DbSet<>.

Так что, если только вам не платят зарплату за строчки кода, не стоит обмазываться абстракциями ради абстракций. Это не приведет к победе коммунизма, а только к засорению проекта и падению перформанса.

image

Ребята, может я слишком стар и не модный. Но я еще помню, что я писал первое приложение, что писал первую статью, что читал первый пост на хабре. И вот, когда я перешел в комментарии к первому посту, я тогда увидел как правильно надо делать, а не что было в статье не плохо ( об этом я смог догадаться сам).
Так вот, ребята, я понимаю, что хейтить модно, и если автор найдет, как это надо делать сам, то запомнить лучше. Но если все будут хейтить, то и искать будет негде ))
Как надо делать описать гораздо сложнее. Потому что жизнь — не лабораторная работа, и там нет универсального правильного ответа. Опыт как раз таки и говорит, что в разных случаях надо бывает разным. За этот опыт как раз деньги и платить должны, а не за парсинг json.

Если будет время, я попробую написать пару статей, как можно было бы это писать в тех или иных случаях.
Проблема в том, что тут, в этой статье, нет ничего от правильного.
Точнее есть только один хороший пункт, соответствующий заголовку — это какой тип проекта создавать. Да и то, стоило бы показать, что создаётся автоматом, и как надо сразу переделывать эту болванку:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;

namespace WebApi1.Controllers
{
    [ApiController]
    [Route("[controller]")]
    public class WeatherForecastController : ControllerBase
    {
        private static readonly string[] Summaries = new[]
        {
            "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
        };

        private readonly ILogger<WeatherForecastController> _logger;

        public WeatherForecastController(ILogger<WeatherForecastController> logger)
        {
            _logger = logger;
        }

        [HttpGet]
        public IEnumerable<WeatherForecast> Get()
        {
            var rng = new Random();
            return Enumerable.Range(1, 5).Select(index => new WeatherForecast
            {
                Date = DateTime.Now.AddDays(index),
                TemperatureC = rng.Next(-20, 55),
                Summary = Summaries[rng.Next(Summaries.Length)]
            })
            .ToArray();
        }
    }
}


Вот сразу прямо тут можно улучшать, чтобы по шагам превратить в то самое «простое rest api».
Первый шаг, не бесспорный:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Logging;

namespace WebApi1.Controllers
{
    [ApiController]
    [Route("[controller]")]
    public class WeatherForecastController : ControllerBase
    {
        private static Random Rng = new Random();
        private WeatherForecast[] forecasts = Enumerable.Range(1, 5).Select(index => new WeatherForecast
        {
            Date = DateTime.Now.AddDays(index),
            TemperatureC = Rng.Next(-20, 55),
            Summary = Summaries[Rng.Next(Summaries.Length)]
        }).ToArray();

        private static readonly string[] Summaries = new[]
        {
            "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
        };

        private readonly ILogger<WeatherForecastController> _logger;

        public WeatherForecastController(ILogger<WeatherForecastController> logger)
        {
            _logger = logger;
        }

        /// <summary>
        /// Get list of forecasts
        /// </summary>
        /// <returns></returns>
        [HttpGet]
        public IEnumerable<WeatherForecast> Get()
        {
            _logger.LogInformation("Get list of forecasts");
            return forecasts;
        }

        /// <summary>
        /// Get one of forecasts or 404 error
        /// </summary>
        /// <param name="num"></param>
        /// <returns></returns>
        [HttpGet]
        public ActionResult<WeatherForecast> Get(int num)
        {
            _logger.LogInformation($"Get forecasts[{num}]");

            if (num >= 0 && num < forecasts.Length)
                return forecasts[num];
            else
                return NotFound();
        }
    }
}


И далее по вкусу…
Если сейчас в универах такие лабораторные — моё почтение.
Sign up to leave a comment.

Articles