Validator constructor gets called many times #1

Closed
thardy opened this Issue Mar 20, 2013 · 6 comments

Comments

Projects
None yet
2 participants
@thardy

thardy commented Mar 20, 2013

The constructors on the AbstractValidator derived classes get called 5 or more times. I get the same behavior when I duplicate the relevant bits in a project of my own.

Any idea why?

p.s. you have to upgrade your solution to the latest FluentValidation to remove a bug about an "operation destabilizing the runtime".

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Mar 20, 2013

Owner

Is that when using which of the 6 techniques presented on https://github.com/robdmoore/UnobtrusiveMVCTechniques/blob/master/UnobtrusiveMVCTechniques/Controllers/CustomValidationController.cs

What kind of stack trace happens for all the calls?

Owner

robdmoore commented Mar 20, 2013

Is that when using which of the 6 techniques presented on https://github.com/robdmoore/UnobtrusiveMVCTechniques/blob/master/UnobtrusiveMVCTechniques/Controllers/CustomValidationController.cs

What kind of stack trace happens for all the calls?

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Mar 20, 2013

Owner

re: fluentvalidation good call - I've seen that destabilisation error before - pretty scary hey! Easy fix though like you say. Feel free to send me a pull request with the update.

Owner

robdmoore commented Mar 20, 2013

re: fluentvalidation good call - I've seen that destabilisation error before - pretty scary hey! Easy fix though like you say. Feel free to send me a pull request with the update.

@thardy

This comment has been minimized.

Show comment
Hide comment
@thardy

thardy Mar 20, 2013

I get it to happen in my own very simple solution. One model, one validator, etc.

I think I've narrowed it down to the fact that the validator constructor is getting called once for each property on the model. After some research, it looks like we should be using singletons for all validators, not per-instance. This has led me to the issues of calling a per-request resource from a singleton using Autofac.

I didn't have much time to finish (i.e. I don't have it working yet), but I started down the path of using a (brackets altered for comment) Func[ISomeResource] in my validator instead of an ISomeResource. ISomeResource is registered as InstancePerHttpRequest, and my validators are registered as SingeInstance. Autofac supposedly supports this, but I had problems getting it to work. Apparently, Autofac supports Func[ISomeResource] out of the box - you don't have to even create a Factory, but I received the following error when trying to simply replace the ISomeResource constructor parm with Func[ISomeResource]...

"No scope with a Tag matching 'AutofacWebRequest' is visible from the scope in which the instance was requested. This generally indicates that a component registered as per-HTTP request is being requested by a SingleInstance() component (or a similar scenario.)"

The problem is that I need to use the current container/context (for that request), and not whatever the default context is for wiring up Func[whatever]. That led me to the following link - http://stackoverflow.com/questions/2599566/autofac-reference-from-a-singleinstanced-type-to-a-httprequestscoped?rq=1, where the solution was to use the following binding...

builder.Register<Func<IExtensions>>(c => RequestContainer.Resolve<IExtensions>());

Problem is that RequestContainer is gone in the latest version of Autofac, plus I'm concerned that they now wire up Func[whatever] in the framework, so I'm not sure I should be using this construct at all. At this point I had to go to my real job, so I'll try to fix it when I get home. The following link looks promising - http://stackoverflow.com/questions/2888621/autofacs-funct-to-resolve-named-service.

thardy commented Mar 20, 2013

I get it to happen in my own very simple solution. One model, one validator, etc.

I think I've narrowed it down to the fact that the validator constructor is getting called once for each property on the model. After some research, it looks like we should be using singletons for all validators, not per-instance. This has led me to the issues of calling a per-request resource from a singleton using Autofac.

I didn't have much time to finish (i.e. I don't have it working yet), but I started down the path of using a (brackets altered for comment) Func[ISomeResource] in my validator instead of an ISomeResource. ISomeResource is registered as InstancePerHttpRequest, and my validators are registered as SingeInstance. Autofac supposedly supports this, but I had problems getting it to work. Apparently, Autofac supports Func[ISomeResource] out of the box - you don't have to even create a Factory, but I received the following error when trying to simply replace the ISomeResource constructor parm with Func[ISomeResource]...

"No scope with a Tag matching 'AutofacWebRequest' is visible from the scope in which the instance was requested. This generally indicates that a component registered as per-HTTP request is being requested by a SingleInstance() component (or a similar scenario.)"

The problem is that I need to use the current container/context (for that request), and not whatever the default context is for wiring up Func[whatever]. That led me to the following link - http://stackoverflow.com/questions/2599566/autofac-reference-from-a-singleinstanced-type-to-a-httprequestscoped?rq=1, where the solution was to use the following binding...

builder.Register<Func<IExtensions>>(c => RequestContainer.Resolve<IExtensions>());

Problem is that RequestContainer is gone in the latest version of Autofac, plus I'm concerned that they now wire up Func[whatever] in the framework, so I'm not sure I should be using this construct at all. At this point I had to go to my real job, so I'll try to fix it when I get home. The following link looks promising - http://stackoverflow.com/questions/2888621/autofacs-funct-to-resolve-named-service.

@thardy thardy closed this Mar 20, 2013

@thardy thardy reopened this Mar 20, 2013

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Mar 21, 2013

Owner

Have you tried changing the FluentValidatorModule to register the validators as instance per http request rather than instance per dependency? That way it will resolve one per request and passing per-request scoped objects to the ctor will work fine.

Whenever you get "No scope with a Tag matching 'AutofacWebRequest' is visible from the scope in which the instance was requested" you can generally use DependencyResolver.Current instead, although it's more difficult (but not impossible: http://robdmoore.id.au/blog/2012/04/27/testing-code-that-uses-autofac-dependencyresolver-in-asp-net-mvc/) to unit test since it's a service locator rather than dependency injector.

Let me know if that works for you and I'll adjust the code sample in this repository.

Owner

robdmoore commented Mar 21, 2013

Have you tried changing the FluentValidatorModule to register the validators as instance per http request rather than instance per dependency? That way it will resolve one per request and passing per-request scoped objects to the ctor will work fine.

Whenever you get "No scope with a Tag matching 'AutofacWebRequest' is visible from the scope in which the instance was requested" you can generally use DependencyResolver.Current instead, although it's more difficult (but not impossible: http://robdmoore.id.au/blog/2012/04/27/testing-code-that-uses-autofac-dependencyresolver-in-asp-net-mvc/) to unit test since it's a service locator rather than dependency injector.

Let me know if that works for you and I'll adjust the code sample in this repository.

@thardy

This comment has been minimized.

Show comment
Hide comment
@thardy

thardy Mar 22, 2013

I got this to work with a singleton. First, in response to your idea about scoping the validators as perHttpRequest, yes, I think that will work too. I really wanted to get it to work with a singleton though, and I posted the following StackOverflow question to get past the Autofac issues and got a very elegant answer - http://stackoverflow.com/questions/15538665/autofac-how-to-resolve-func-for-isomething-from-singleton-where-isomething-is.

The core of the answer is to use a Func[IWhatever] - brackets altered for comment - in the validator constructor, then create the following method...

public Func<T> PerHttpSafeResolve<T>()
{
    return () => DependencyResolver.Current.GetService<T>();
} 

and the following registrations...

builder.RegisterType<Whatever>().As<IWhatever>().InstancePerHttpRequest();
builder.RegisterInstance(PerHttpSafeResolve<IWhatever>());

Anything asking for a Func[IWhatever] will receive a factory that whenever it's invoked will use the correct, current context to resolve the interface. It works great. Singletons can get perHttpRequest scoped registrations.

This also solves the testing issue you bring up in your post because you can simply use a different registration in your test project than the PerHttpSafeResolve registration. Your constructor just needs something bound that will return a Func[IWhatever].

thardy commented Mar 22, 2013

I got this to work with a singleton. First, in response to your idea about scoping the validators as perHttpRequest, yes, I think that will work too. I really wanted to get it to work with a singleton though, and I posted the following StackOverflow question to get past the Autofac issues and got a very elegant answer - http://stackoverflow.com/questions/15538665/autofac-how-to-resolve-func-for-isomething-from-singleton-where-isomething-is.

The core of the answer is to use a Func[IWhatever] - brackets altered for comment - in the validator constructor, then create the following method...

public Func<T> PerHttpSafeResolve<T>()
{
    return () => DependencyResolver.Current.GetService<T>();
} 

and the following registrations...

builder.RegisterType<Whatever>().As<IWhatever>().InstancePerHttpRequest();
builder.RegisterInstance(PerHttpSafeResolve<IWhatever>());

Anything asking for a Func[IWhatever] will receive a factory that whenever it's invoked will use the correct, current context to resolve the interface. It works great. Singletons can get perHttpRequest scoped registrations.

This also solves the testing issue you bring up in your post because you can simply use a different registration in your test project than the PerHttpSafeResolve registration. Your constructor just needs something bound that will return a Func[IWhatever].

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Mar 23, 2013

Owner

So there are three ways to resolve the scoping for the validators the way I see it:

  1. Instance per dependency as per the current code, but that does mean that there are multiple instances created of the validator in each request (one per property) as per your observation
  2. Instance per HTTP request, which will create only one instance per request and is still safe to directly resolve request-scoped objects in the constructor of the validator
  3. Single instance, which only creates one instance of each validator per application, but requires you to resolve any request-scoped instances as a factory (Func) and to use the codeyou posted above to register a factory instance that resolves using DependencyResolver.Current.

Thanks very much for raising this issue! It's been great to learn a couple of new things :)

Owner

robdmoore commented Mar 23, 2013

So there are three ways to resolve the scoping for the validators the way I see it:

  1. Instance per dependency as per the current code, but that does mean that there are multiple instances created of the validator in each request (one per property) as per your observation
  2. Instance per HTTP request, which will create only one instance per request and is still safe to directly resolve request-scoped objects in the constructor of the validator
  3. Single instance, which only creates one instance of each validator per application, but requires you to resolve any request-scoped instances as a factory (Func) and to use the codeyou posted above to register a factory instance that resolves using DependencyResolver.Current.

Thanks very much for raising this issue! It's been great to learn a couple of new things :)

@robdmoore robdmoore closed this Mar 23, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment