devlooped / moq

The most popular and friendly mocking framework for .NET
Other
5.85k stars 799 forks source link

Ability to specify proxy generator or invalidate it #925

Open bclothier opened 4 years ago

bclothier commented 4 years ago

This was originally opened here.

Basically, the issue is that Moq uses a single ProxyGenerator, which is fine for 99% of the scenarios. However, I have a scenario where I'm dealing with types that get created at runtime and consequently, can cause collisions with the proxy type cache because the type can change.

The ideal solution would be to be able to use a MockRepository and provide a proxy generator or at least specify that it should use its own proxy generator, separate from the rest of proxy generator, so that any mocks based on those volatile types won't get cached for very long and can be easily invalidated.

stakx commented 4 years ago

We'll have to be very careful not to create a leaky abstraction.

The above points lead me to believe that we need to let client code choose a proxy factory. That way, we can expose the DynamicProxy implementation and document its exact semantics, and keep them separate from the more general proxy factory concept.

I also mentioned in the Gitter chat that Moq v4 isn't quite ready to fully expose its proxy factory abstraction. That is mainly because it, as well as other parts of Moq's design, have been heavily influenced by the capabilities of DynamicProxy, e.g. the fact that you cannot mock static or sealed types, and that you cannot set up non-virtual methods. If we open up Moq to other proxy factory implementations (that may be more powerful, e.g. those based on the CLR's profiler API), we'd have to rewrite large parts of Moq so it is able to deal with statics, non-virtuals, etc. For now, that is out of the question due to the very large amount of work involved.

All of the above leads me to the following proposal:

  1. We expose ProxyFactory as an abstract base class which (for now) has no public members, and a constructor that is internal or private protected. This keeps the set of concrete implementations under Moq's control. Once Moq is ready to allow custom implementations, we can open up that type for subclassing.

  2. We expose a single concrete implementation for ProxyFactory, DynamicProxyProxyFactory, and document its behavior (e.g. its limitations, and the fact that it uses a proxy type cache).

  3. We add a read-write property ProxyFactory to MockRepository that by default is set to a default (global) instance of DynamicProxyProxyFactory, but can be set to a new instance. (This is what would enable you to use a fresh ProxyGenerator with an empty proxy type cache.)

  4. We possibly add a static read-write property Default to ProxyFactory that is likewise set to the same default instance of DynamicProxyProxyFactory. This factory gets used for all mocks that are created globally, i.e. not through a MockRepository.

  5. We might have to add an internal ProxyFactory property to Mock such that MockDefaultValueProvider can discover and use the same proxy factory when auto-mocking properties.

Will the last bit be problematic in your scenario? Say you have a mock with mock.DefaultValue == DefaultValue.Mock. This was created from proxy factory instance A. Now you query mock.SomeProperty which is of a mockable type, and MockDefaultValueProvider auto-mocks the value, i.e. creates another mock, also from proxy factory instance A. We now have two mocks created from the same factory. Is it possible that any of those survives a VBA code execution? If so, your problem isn't solved by the above.

Any thoughts or opinions on the above?

bclothier commented 4 years ago

Yes, I agree with your thoughts regarding avoiding leaking the abstraction. Obviously you see that I'm too focused on solving the issue at hand and not taking a step back toward the overall picture. With your description of what needs to be done, I think that custom proxy factory is sufficient without breaking the abstraction.

RE: item 4 That falls into a category "nice-to-have" because in my mind, if I'm customizing the proxy factory, I should be specific about where I want to use it. In my use case, at least, not all types are volatile --- for example, I might obtain a type from Excel interop library. That type won't change between VBA code executions. OTOH, the type from VBA is volatile, and I have no guarantee that it's still the same type next execution.

That said, I'm not sure that is relevant to Moq's considerations but just so you are aware - I also have to maintain a type cache but for different reasons. To avoid runtime errors such as invalid cast or type mismatch, I have to make sure that there is only one instance of type used, and that should be the same between runs in the case of referenced libraries because referenced libraries are persistent and needs to be comparable. I could choose to reset the cache between runs but that would be wasteful because referenced libraries' types won't change anyway but I need to ensure we don't accidentally create new types for same representation. Whether that paragraph is applicable to Moq's ProxyFactory, I'm not sure, since I would be the one who's managing the concern, not Moq, I think.

RE: item 5 -- I actually learnt about the property now that you mentioned it. Doing some reading on it, yes I think that as long the MockDefaultValueProvider uses the same proxy factory, it will be fine, since we can ensure that none of the types created via either routes survive to the next execution run.

All in all, I like the idea of being able to request a new proxy factory in the mock repository and that would make my life easier.

stakx commented 4 years ago

@bclothier, some quick questions:

bclothier commented 4 years ago

Hadn’t considered using AppDomain. I need to verify whether I can create temporary AppDomain and whether I can control what goes in the AppDomain. If bith hold, then, yes I can create one in response to the start of VBA execution.

No, mocks should not survive the execution run. In theory, one could create a public field and store a mock in the field. That would survive execution runs, but frankly I can’t see a good reason to persist mocks in this manner and I won’t support that scenario. Mocks are meant to be transient anyway.

Znurre commented 2 years ago

I have created a PR #1227 with some initial changes meant to initiate a discussion about implementing this.

Please have a look :)