7.0 API Reviews

[bricelam]

As a team, we periodically review any changes made to the public API of the product. This issue keeps track of any follow-up items that come out of those reviews for the 7.0 release.

March 7

Done

• @roji Add default implementation for ISaveChangesInterceptor.SaveChangesCanceled (and Async) (done in #27635)
• @roji See if HasIndex().IsDescending() (no args) can work for single-column indexes (done in #28489).
• @maumar Is OwnedNavigationBuilder.Property that takes setTypeConfigurationSource still needed? If not, is it generally useful and worth keeping?
• @maumar Should INavigationExpansionExtensibilityHelper.ValidateQueryRootCreation provide a default implementation?
• @maumar Will mutating the annotations on TableExpressionBase cause problems with caching, etc.?
• @roji IBatchExecutor and ICommandBatchPreparer should use a nominal type instead of ValueTuple.
  • Proposing to integrate the "are more batches coming" into ModificationCommandBatch instead.
• @roji Follow up on ReaderModificationCommandBatch.IsCachedCommandTextEmpty and UpdateCachedCommandText if they don't get removed. (they got removed)
• @AndriySvyryd Use IEnumerable instead of ICollection in SqlServerRetryingExecutionStrategy and SqlServerDbContextOptionsBuilder.EnableRetryOnFailure.

March 25

Done

• @AndriySvyryd Was there a binary breaking change made to record?
• @ajcvickers Remove params, Obsolete, and add a new ctor to IndexAttribute
• @bricelam Make ITextTemplating and ITextTemplatingCallback pubternal (or remove if they're not actually needed)
• @AndriySvyryd Rearrange parameters of RelationalModelValidator.ValidateMappingStrategy to put the metadata object first
• @roji Should TableBuilder.HasTrigger move to SQL Server for now? Was moved to Core instead.
• @roji Change the return type of ReaderModificationCommandBatch.ParameterValues to IDictionary<,>
  • The problem here is that ParameterValues needs to be passed to RawSqlCommand (code), which currently accepts IReadOnlyDictionary; and IDictionary doesn't extend IReadOnlyDictionary (discussion. We could have a private backing field that's Dictionary, but subclasses who wish to override Complete would bump against the same problem.
  • Design decision: leave this as a Dictionary.
• @roji Can you explain the additionalReadValues parameter of UpdateSqlGenerator.AppendDeleteCommand in a design meeting?

May 13

Done

• @smitpatel Revert breaking change to GroupByShaperExpression ctor
• @roji Rename RelationalAnnotationNames.GetReaderFieldValue to FieldValueGetter
• @smitpatel Move EnumerableExpression out of the SqlExpressions namespace
• @smitpatel Should EnumerableExpression be immutable?
• @roji Is there a better place for ReaderModificationCommandBatch.MaxBatchSize since it has the same value for every batch?
• @roji Rename SelectingUpdateSqlGenerator to UpdateAndSelectSqlGenerator

June 24

Done

• @ajcvickers Use entity instead of instance in IIdentityResolutionInterceptor and IMaterializationInterceptor
• @ajcvickers Use parameter objects in IIdentityResolutionInterceptor and IInstantiationBindingInterceptor
• @ajcvickers Remove entityInstanceName parameter in IInstantiationBindingInterceptor.ModifyBinding
• @ajcvickers Rename SkippingIdentityResolutionInterceptor to IgnoringIdentityResolutionInterceptor
• @ajcvickers Why is IEntityType.ServiceOnlyConstructorBinding new?
  • There was a refactoring to InstanceFactory code from EntityType to a common location that could be called with the materializer. This required that ServiceOnlyConstructorBinding be available outside of EntityType.

July 11

Done

• @ajcvickers Remove IQueryExpressionInterceptor.CompilingQuery and CompiledQuery and re-purpose issue for intercepting the SQL tree
• @ajcvickers Rename IQueryExpressionInterceptor.ProcessingQuery to QueryCompilationStarting
• @bricelam Rename IndentedStringBuilder.CurrentIndent to IndentCount and return int
• @bricelam Document CSharpSnapshotGenerator (at least new members) as internal
• @AndriySvyryd Rename RelationalEventId.KeyUnmappedProperties to something better
• @AndriySvyryd Add Is prefix to IReadOnlyRelationalPropertyOverrides.ColumnNameOverridden

August 8

Done

• @smitpatel Rename ExpressionExtensions.BuildEqualsExpression to CreateEqualsExpression
• @AndriySvyryd Rename ConventionsBuilder to ConventionSetBuilder
• @maumar Rename JsonColumnName to ContainerColumnName and JsonColumnTypeMapping to ContainerColumnTypeMapping in RelationalEntityTypeExtensions and RelationalAnnotationNames
• @maumar Move RelationalNavigationBuilderExtensions to the entity type builder instead
• @smitpatel Update CommandSource.CommandSource value to match obleted BulkUpdate value (of 8)
• @maumar Change RelationalModelValidator.ValidateJsonEntityProperties to protected
• @AndriySvyryd Remove are prefix from parameter name inside IConventionStoredProcedure.SetAreTransactionsSuppressed

August 17

• @AndriySvyryd Reorder EntityTypeBuilder.HasTrigger parameters; put entityType first.
• @smitpatel Obsolete IDictionary overloads in RuntimeModelConvention; replace with Dictionary
• @bricelam Should we add ScaffoldingModelExtensions.IsHandledByDataAnnotations for IKey?
• @bricelam Rename ScaffoldingModelExtensions.HasDataAnnotation and FluentApiCodeFragment.HasDataAnnotation to IsHandledByDataAnnotations
• @bricelam Don't pluralize ScaffoldingModelExtensions.IsHandledByConventions
• @smitpatel Rename SetPropertyStatements to SetPropertyCalls (parameters too)
• @AndriySvyryd Rename RelationalTriggerBuilderExtensions.HasName to HasDatabaseName (in RelationalTriggerExtensions too)
• @AndriySvyryd Remove BlankTriggerAddingConvention. Document how to do it in breaking changes. We may add sugar later based on feedback.
• @maumar Rename JsonQueryExpression.JsonPath to just Path (JsonScalarExpression too)
• @maumar Make JsonQueryExpression.MakeNullable overload that takes a dictonary internal
• @smitpatel Make parameterless ctor on SetPropertyStatements private
• @maumar Remove parameter 'nullable' on JsonScalarExpression.Update (replace with a MakeNullable method)
• @smitpatel Rename SetColumnValue to ColumnValueSetter (UpdateExpression property too)
• @roji Make DatabaseTable.Triggers return a class so we can add additional metadata in the future. #28801