The purpose of API Review meetings is to go over new API surface one class/member at a time to ensure we have a consistent and high quality API. We look at how classes are structured, which members are public as well as the names of classes, members and parameters.

 

API Review: Interception

Namespace

  • Move to .Interception namespace
  • No types that belong in the root namespace

IDbInterceptor

  • Marker interface - do we need it or could we just have overloads of AddInterceptor?
    • Makes it easy for things that implement more than one interface

IDbCommandTreeInterceptor

  • Should we expose CommandTreeCreating?
    • Not really useful - you'd be doing everything we do
  • No changes needed

IDbCommandInterceptor

  • No changes needed

Interception

  • Rename to DbInterception
    • Current name is pretty generic
    • Allows us to have the Interception namespace
  • Should it go in root?
    • No, not common enough.
  • Rename AddInterceptor/RemoveInterceptor to Add/Remove
  • Discussed removing Dispatch property and having a separate class with static properties
    • Decided it is fine to combine registration and dispatch given it is an advanced scenario

Dispatchers

  • Rename to DbDispatchers

 

DbCommandDispatcher

  • Async methods don't end in Async because they are not marked as async
    • They do return a task though so they should end in Async
    • CancellationToken should be last parameter on async methods

DbCommandLogger

  • Rename to DatabaseLogFormatter
    • Clears up confusion around the use of this class (it’s for changing the format of what gets written to Database.Log)
    • Also could handle more than DbCommand in the future (Connection etc.)
    • Make sure we change method name on DbConfiguration and docs on Database.Log
  • Sink property is confusing
    • More like writer (used to be called that)
    • Provide a protected Write(string) method and make this property internal (better known pattern)
    • Ctor params should be called writeAction
  • Make Context and Stopwatch properties protected too
  • Add info to xml docs about how Stopwatch works

DbCommandLoggerFactory

  • Remove this class in favor of Funcs (discussed in code based config API Review)

DispatcherBase<TInterceptor>

  • Sync up names (add Db prefix)

General

  • Should we wire up MessageInfoHandler (non-error messages from the database provider) to the log/interceptors?
    • Not in EF6
    • Need to open a work item

Interception Contexts

  • Collapse the generic and non-generic DbCommandInterceptionContext classes into a single generic class and update places that make use of the non-generic (mostly DatabaseLogFormatter) with generic methods.
    • This allowed renaming of DbCommandBaseInterceptionContext to DbCommandInterceptionContext
  • Remove the DbCommandTree parameter from the IDbCommandTreeInterecptor.TreeCreated method. This parameter is the object that is performing the operation, but in this case the DbCommandTree is not performing the operation, it is instead the result of the operation. Therefore having it in the interception context as Result and OriginalResult makes sense. Also, the parameter would always be the same as OriginalResult, but it is more appropriate for most interceptors to use Result, which is not obvious if the parameter is kept. Therefore it should be removed.
  • Rename IsSuppressed to IsExecutionSuppressed
    • We discussed using “cancelled” but this has overloaded meaning when using async so we kept “suppressed”.

 

 

API Review: Code First Stored Procedure Mapping

Shape of classes/methods etc. is all good

Some general things to fix across all the Fluent API classes:

  • Number of params called 'modificationFunction…', these should change to ‘storedProcedure…’
    • Also update the class names (will need to leave Modification in the class names to distinguish from any query related sproc configuration in the future)
      • Make sure we don’t miss AssociationModificationFunctionConfiguration<TEntityType>
        (different namespace)
  • Association methods should be renamed to Navigation
    • Make sure we update xml docs to talk about navigations/relationships rather than associations
    • Add docs about dotting through to PK parameter in lambda

Last edited Jul 26, 2013 at 7:00 PM by romiller, version 4