Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate attributes on SaveChanges not working #9662

Closed
Jonatthu opened this issue Aug 31, 2017 · 13 comments
Closed

Validate attributes on SaveChanges not working #9662

Jonatthu opened this issue Aug 31, 2017 · 13 comments
Labels
closed-no-further-action The issue is closed and no further action is planned.

Comments

@Jonatthu
Copy link

Jonatthu commented Aug 31, 2017

I want to check some validation rules first when I call SaveChanges();

My method currently looks like this trying to apply one of this solutions:
http://www.bricelam.net/2016/12/13/validation-in-efcore.html
or
#4434

public override Task<int> SaveChangesAsync(CancellationToken cancellationToken = default(CancellationToken))
        {
            var entities = (from entry in ChangeTracker.Entries()
                            where entry.State == EntityState.Modified || entry.State == EntityState.Added
                            select entry.Entity);

            var validationResults = new List<ValidationResult>();
            foreach (var entity in entities)
            {
                if (!Validator.TryValidateObject(entity, new ValidationContext(entity), validationResults))
                {
                    throw new ValidationException();
                }
            }

            var entities2 = from e in ChangeTracker.Entries()
                           where e.State == EntityState.Added
                               || e.State == EntityState.Modified
                           select e.Entity;
            foreach (var entity in entities2)
            {
                var validationContext = new ValidationContext(entity);
                Validator.ValidateObject(entity, validationContext);
            }

            return base.SaveChangesAsync(cancellationToken);


        }

And my model is:

    public class User : EntityBase
    {
        [StringLength(60, MinimumLength = 10)]
        [RegularExpression(@"^[A-Z]+[a-zA-Z''-'\s]*$")]
        public string UserName { get; set; }
     }

In the controller ModelState has the error count

        [HttpPost]
        public async Task<RG<string>> Post([FromBody] User model)
        {
            try
            {
                if (ModelState.IsValid || model != null)
                {

So this part it is working but since I am going to use view models instead the models from the database I don't want to repeat myself and use attributes in both models, so that's why I want to put the validation on save changes (following DRY).

Any solution for this?
Right now my method SaveChangesAsync() is not detecting any validation errors when Model State does.

I am using EF Core 2

@ajcvickers
Copy link
Member

@Jonatthu We're trying to understand the issue here. Are you saying that with your example ModelState is reporting the entity as invalid, but if you call SaveChangesAsync with your override, then it doesn't detect any validation errors?

@Jonatthu
Copy link
Author

Jonatthu commented Sep 2, 2017

Exactly! SaveAsync is not detecting nothing! @ajcvickers

@Jonatthu Jonatthu changed the title Validate attributes on SaveChanges notworking Validate attributes on SaveChanges not working Sep 5, 2017
@ajcvickers
Copy link
Member

Note for triage: I was able to reproduce this. The code validates [Required] but seems to ignore StringLength and RegularExpression.

@Jonatthu
Copy link
Author

Jonatthu commented Sep 5, 2017

@ajcvickers Is this going to be fix? or Is it supposed to do that?

@ajcvickers
Copy link
Member

@Jonatthu It's not an EF issue. It may be an issue with data annotations/validation, but I'm not sure about that since I don't have enough context on how that stuff is supposed to work. I'm leaving this issue open to discuss with others.

@Jonatthu
Copy link
Author

Jonatthu commented Sep 5, 2017

@ajcvickers Should I open this issue on the asp.net repository ?

@ajcvickers
Copy link
Member

@Jonatthu We have a triage scheduled for tomorrow and I'll get back to you after that.

@ajcvickers
Copy link
Member

@Jonatthu Looks like you need to call the TryValidateObject method with an additional bool parameter:

if (!Validator.TryValidateObject(entity, new ValidationContext(entity), validationResults, true))

It works for me when I do this.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Sep 7, 2017
@Jonatthu
Copy link
Author

Jonatthu commented Sep 8, 2017

@ajcvickers Thanks! good catch :D

@Chris-ZA
Copy link

Just some additional info... When using the TryValidateObject method in EF 6.1.3, I also need to add the bool parameter for the validation to evaluate everything except [Required].

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
@iamrahul127
Copy link

@ajcvickers Could you please help me understand why SaveChanges() and SaveChangesAsync() of Dbcontext class doesn't call Validator.TryValidateObject() itself? Is it because of performance?

if (!Validator.TryValidateObject(entity, new ValidationContext(entity), validationResults, true))

@ajcvickers
Copy link
Member

@iamrahul127 Because a typical application needs to do validation before getting to the data access code. Therefore, doing validation again automatically is not appropriate. This was a mistake we made in legacy EF6.

@roji
Copy link
Member

roji commented Mar 20, 2023

In addition to what @ajcvickers said, if you do want validation at the data layer, you typically want it in the database rather than in your program. For example, you can define check constraints which provide a 100% guarantee that bad data will never make it in. If you're interested in that, you may want to look at https://github.com/efcore/EFCore.CheckConstraints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned.
Projects
None yet
Development

No branches or pull requests

5 participants