IValidatableObject on DTO ISSUES #580

mel@quadsoftpa.com created 4 years ago

ABP Framework version: 4RC
UI type: MVC
Tiered (MVC) or Identity Server Seperated (Angular): no

I really don't like doing this validation in the DTO, but I was unable to get it to work any other way. It would be great if the second half of your validation page (https://docs.abp.io/en/abp/latest/Validation) had more detail and specific usage on an alternative method.

Regardless, I'm dealing with a few issues with this approach:

there really should be a ValidateAsync(ValidationContext validationContext) method
when calling the service (IsIdUnique for example) I'm getting
probably a simple fix, but the only way I found around it is removing authorization from the service which is obviously not a real fix.

Thanks

  public class CountryCreateDto : IValidatableObject
    {
        private string _id;

        private string _name;

        [Required]
        [StringLength(CountryConsts.IdMaxLength, MinimumLength = CountryConsts.IdMinLength)]
        [RegularExpression(@"^[A-Z''-'\s]{2,3}$", ErrorMessage = "The Country Code must be between two and three uppercase letters.")]
        [DisplayName("Code")]
        public string Id
        {
            get => _id;
            set => _id = value.Trim()
                              .ToUpper();
        }

        [Required]
        [StringLength(CountryConsts.NameMaxLength, MinimumLength = CountryConsts.NameMinLength)]
        public string Name
        {
            get => _name;
            set => _name = value.Trim().ToUpper();
        }

        public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
        {
            var service = validationContext.GetRequiredService<ICountryAppService>();

            var validationResults = new List<ValidationResult>();

            var isIdUnique = Task.Run(async () => await service.IsIdUnique(Id))
                                 .Result;

            if (!isIdUnique)
            {
                validationResults.Add(new ValidationResult("Country Code already exists", new[]
                {
                    "Id"
                }));
            }

            var isNameUnique = Task.Run(async () => await service.IsNameUnique( Name))
                                   .Result;

            if (!isNameUnique)
            {
                validationResults.Add(new ValidationResult("Country Name already exists", new[]
                {
                    "Name"
                }));
            }

            return validationResults;
        }
    }

liangshiwei created 4 years ago

Support Team Fullstack Developer

Hi,

In fact, it is not a good practice to do business verification in DTO, DTO should check the value is basically valid(Non-empty, length, range...). You can verify business logic in applications or domain services.

By the way , don't use Task.Run(async () => await service.IsIdUnique(Id)).Result, If you have to use asynchronous method in synchronous method, please use AsyncHelper.run()...;

mel@quadsoftpa.com created 4 years ago

I reverted back to my original implementation which does the validation in the application service. However I still need my second question answered because I have the same issue.

when calling the service from the web project I can successfully use Service.UpdateAsync but Service.ValidateAsync needs permissions. How do I configure a proper permission?

liangshiwei created 4 years ago

Support Team Fullstack Developer

Hi,

No way. You can write a private method without permission. like :

private IBookStoreRepository _bookStoreRepository;

.....

public async Task Create(CreateBookDto createDto)
{
     await CheckNameExists(createDto.Name);
     
     .....
}

private async Task CheckNameExists(string bookName)
{
    var book = await  _bookStoreRepository.GetByName(bookName);
    if(book!=null) {
       throw new UserfriendException("the book already exists ")
    }
}

mel@quadsoftpa.com created 4 years ago

I find that hard to believe that the only methods you can call our the base CRUD. Certainly you should be able to add a permission and be able to call a custom method in the Application Service.

Without this flexibility, that means having repetitive code many many times. I want to use the base CRUD but validate. This should be very very basic.

mel@quadsoftpa.com created 4 years ago

I don't want to have to duplicate every base crud like create below just to do validation. There's got to be a better way. Please advise.

    public virtual async Task&lt;TGetOutputDto&gt; CreateAsync(TCreateInput input)
    {
        await CheckCreatePolicyAsync();

*** VALIDATE***

        var entity = await MapToEntityAsync(input);

        TryToSetTenantId(entity);

        await Repository.InsertAsync(entity, autoSave: true);

        return await MapToGetOutputDtoAsync(entity);
    }

liangshiwei created 4 years ago

Support Team Fullstack Developer

Hi,

when calling the service from the web project I can successfully use Service.UpdateAsync but Service.ValidateAsync needs permissions. How do I configure a proper permission?

If ValidateAsync requires permissions, then you must configure this permission for the current user. I think this is correct. If you don't have permission but you can call the method, it will be a bug.

You don't have to completely override the CreateAsync method.

private IBookStoreRepository _bookStoreRepository;

.....

public async override Task CreateAsync(CreateBookDto createDto)
{
     // VALIDATE
     await CheckNameExists(createDto.Name);
     
     // Call base createAsync method
     await base.CreateAsync(createDto);
}

private async Task CheckNameExists(string bookName)
{
    var book = await  _bookStoreRepository.GetByName(bookName);
    if(book!=null) {
       throw new UserfriendException("the book already exists ")
    }
}

mel@quadsoftpa.com created 4 years ago

Thank you for your response. I actually just switched to that. I'd rather be able to call it separately, but this will work.

my goal was to get a List<ValidationResult>. but I'm hoping the following will work instead

            var exceptions = new List<Exception>();
            var isIdUnique = await IsCodeUnique(dto.Id, dto.Code);

            if (!isIdUnique)
            {
                exceptions.Add(new UserFriendlyException("Country Code already exists"));
            }

            var isNameUnique = await IsNameUnique(dto.Id, dto.Name);

            if (!isNameUnique)
            {
                exceptions.Add(new UserFriendlyException("Country Name already exists"));
            }

            if (exceptions.Any())
            {
                throw new AggregateException(exceptions);
            }