Design Meeting Notes - May 31, 2012

Making EF code more testable

Currently many EF classes are sealed and/or have non-virtual methods. This makes it hard to create mocks for these classes. Options:

  • Use new mocking capabilities in .NET 4.5/Dev11
    • Pros:
      • No need to change existing classes just for testing
      • Well-defined public inheritance is retained as recommended by Framework guidelines
    • Cons:
      • Will tie us to .NET 4.5 which will be problematic for testing against .NET 4.
      • Doesn't act as a forcing function for generally improving design--e.g. introducing seams
  • Create wrapper classes so we can mock internal types
    • Pros:
      • No need to change existing classes just for testing
      • Well-defined public inheritance is retained as recommended by Framework guidelines
    • Cons:
      • Introduces an additional layer of indirection which is potentially not needed
      • Doesn't allow the more mockable types to be used by customers
  • Unseal classes and add virtual methods
    • Pros:
      • Allows customers to mock EF types more effectively as well as making it easier for us
      • Doesn't change the amount of code/indirection we have or tie us to .NET 4
      • Can still add internal classes for factoring where we want to change class responsibilities/design without breaking existing public surface
      • Public inheritance can be used in places we didn't anticipate it
    • Cons:
      • Goes against Framework guidelines in that public inheritance can now be used in places where the code doesn't anticipate it resulting in strange behavior in EF

Open questions:

  • Should we remove wrapper classes that have already been added?
    • Yes
  • Should we go through and make everything virtual in one go?
    • The problem with just making everything virtual and constructible is that it doesn't help introduce seams into the code that would allow appropriate dependency injection for people to use to substitute their own implementations or mocks
    • In the future we need to address this and move to a more open architecture
    • For now we will just use internal constructors and make methods virtual on a class-by-class basis as needed
    • We will look at introducing abstract base classes or interfaces for publicly interesting classes
    • If we need to jump through hoops to mock something then we look at introducing seams and using dependency injection and consider making it public
  • What do we do for mocking of classes that we don't own?
    • We should not use conditional compilation here
    • We can wrap external classes in proxies and use refactoring to ensure that the proxies are used internally while the public surface doesn't change.


 

Code duplication in Async

It is not possible to use the same code for async and sync versions using normal mechanisms of re-use because of the way the compiler re-writes the code. This leads to some code duplication.

Possible solutions:

  • We could use T4 templates or conditional compilation. We decided not to do this because it adds complexity/overhead to the build which doesn't seem like a good tradeoff for the amount of duplicate code that is removed. Also, right now the async and sync methods are quite similar, but they will likely diverge as we implement more features at which point the value of the T4/conditionals decreases.
  • We could call the async method from the sync version and block. We won't do this because it will very likely have a large perf implication.
  • We will factor out non-async parts of the methods where we can and as appropriate and add documentation to the code to make sure people know that there are two versions of the method to be changed.

Supporting MVC scaffolding

Problem

The MVC scaffolding code makes use of types defined in the .NET Framework. This means that it will not work in EF6 when these types are pulled out of into EF.dll.

Details

The MVC scaffolding code uses Dynamic Data. However, it doesn’t use the EF model provider built in to Dynamic Data but rather passes in a new EF model provider contained in the MVC assembly:

    MetaModel metaModel = new MetaModel(false);

    metaModel.RegisterContext(
        (DataModelProvider) new EntityFrameworkDataModelProvider(this._contextType, modelAssemblies));

Given this information it seems to me we have a few options:

  • We could add a method somewhere in EntityFramework.dll that would return a Dynamic Data model provider. We could essentially copy the code for this from MVC for EF5 to reduce the risk associated with adding this code. MVC could then call this method anytime it has a DbContext and this will continue to work when we update to EF6.
    • Pros: Makes MVC4 work with EF6; relatively low risk
    • Cons: Dependency on dynamic data; adds public surface that we probably don’t want
  • The Dynamic Data model provider in MVC could be updated to work entirely by Reflection against .NET or EF6 types
    • Pros: Makes MVC4 work with EF6; doesn’t introduce new dependencies into EF5
    • Cons: From looking at the code in MVC4 this seems like quite a lot of work and it would be easy to get it wrong, especially since we are still in the early days of EF6
  • We could implement a full MVC scaffolder in EntityFramework.dll or a separate assembly in our NuGet package
    • Pros: Makes MVC4 work with EF6; gives us control over the EF scaffolding so we can improve it when needed
    • Cons: Very high risk for EF5; EF takes dependency on MVC which seems wrong
  • We could do nothing now and then release an MVC scaffolder for EF6 with EF6 or as part of the MVC tooling update
    • Pros: Lowest risk for Dev11; gives us time to get the scaffolder right for EF6
    • Cons: MVC4 won’t work with EF6 without pulling in a new scaffolder; I’m not sure how easy it is to integrate a new scaffolder into MVC   

Decision:

  • We will update the EF scaffolder as part of the tooling update which will happen before EF6 goes RTM
  • We can release an EF6 scaffolder for people using pre-release versions of EF6; this will no longer be needed once the tooling updates

Last edited Dec 14, 2012 at 5:30 PM by ajcvickers, version 2

Comments

No comments yet.