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
API Review: Interception
- Move to .Interception namespace
- No types that belong in the root namespace
- 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
- Should we expose CommandTreeCreating?
- Not really useful - you'd be doing everything we do
- No changes needed
- Rename to DbInterception
- Current name is pretty generic
- Allows us to have the Interception namespace
- Should it go in root?
- 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
- 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
- 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
- Remove this class in favor of Funcs (discussed in code based config API Review)
- Sync up names (add Db prefix)
- 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
- 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>
- 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