Generate suppressible error when entity saved to Cosmos without PK va…

…lue set (#34250)

* Generate suppressible error when entity saved to Cosmos without PK value set

Fixes #34180

We throw if there is no property in the primary key that is not generated and does not have a value set, excluding partition key properties.

* Review updates

src/EFCore.Cosmos/Diagnostics/CosmosEventId.cs

@@ -36,6 +36,9 @@ private enum Id
         ExecutedReplaceItem,
         ExecutedDeleteItem,
 
+        // Update events
+        PrimaryKeyValueNotSet = CoreEventId.ProviderBaseId + 200,
+
         // Model validation events
         NoPartitionKeyDefined = CoreEventId.ProviderBaseId + 600,
 
@@ -170,4 +173,22 @@ private static EventId MakeValidationId(Id id)
     ///     </para>
     /// </remarks>
     public static readonly EventId NoPartitionKeyDefined = MakeValidationId(Id.NoPartitionKeyDefined);
+
+    private static EventId MakeUpdateId(Id id)
+        => new((int)id, DbLoggerCategory.Update.Name + "." + id);
+
+    /// <summary>
+    ///     A property is not configured to generate values and has the CLR default or sentinel value while saving a new entity
+    ///     to the database. The Azure Cosmos DB database provider for EF Core does not generate key values by default. This means key
+    ///     values must be explicitly set before saving new entities. See https://aka.ms/ef-cosmos-keys for more information.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This event is in the <see cref="DbLoggerCategory.Update" /> category.
+    ///     </para>
+    ///     <para>
+    ///         This event uses the <see cref="PropertyEventData" /> payload when used with a <see cref="DiagnosticSource" />.
+    ///     </para>
+    /// </remarks>
+    public static readonly EventId PrimaryKeyValueNotSet = MakeUpdateId(Id.PrimaryKeyValueNotSet);
 }

src/EFCore.Cosmos/Diagnostics/Internal/CosmosLoggerExtensions.cs

@@ -487,6 +487,33 @@ public static void NoPartitionKeyDefined(
         }
     }
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public static void PrimaryKeyValueNotSet(
+        this IDiagnosticsLogger<DbLoggerCategory.Update> diagnostics,
+        IProperty property)
+    {
+        var definition = CosmosResources.LogPrimaryKeyValueNotSet(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(diagnostics, property.DeclaringType.DisplayName(), property.Name);
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new PropertyEventData(
+                definition,
+                (d, p) => ((EventDefinition<string, string>)d).GenerateMessage(((PropertyEventData)p).Property.DeclaringType.DisplayName(), ((PropertyEventData)p).Property.Name),
+                property);
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
+
     private static string FormatParameters(IReadOnlyList<(string Name, object? Value)> parameters, bool shouldLogParameterValues)
         => FormatParameters(parameters.Select(p => new SqlParameter(p.Name, p.Value)).ToList(), shouldLogParameterValues);
 

src/EFCore.Cosmos/Diagnostics/Internal/CosmosLoggingDefinitions.cs

@@ -82,4 +82,12 @@ public class CosmosLoggingDefinitions : LoggingDefinitions
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public EventDefinitionBase? LogNoPartitionKeyDefined;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public EventDefinitionBase? LogPrimaryKeyValueNotSet;
 }

src/EFCore.Cosmos/Properties/CosmosStrings.resx

@@ -209,6 +209,10 @@
     <value>No partition key has been configured for entity type '{entityType}'. It is highly recommended that an appropriate partition key be defined. See https://aka.ms/efdocs-cosmos-partition-keys for more information.</value>
     <comment>Warning CosmosEventId.NoPartitionKeyDefined string</comment>
   </data>
+  <data name="LogPrimaryKeyValueNotSet" xml:space="preserve">
+    <value>The key property '{entityType}.{property}' is not configured to generate values and has the CLR default or sentinel value while saving a new entity to the database. The Azure Cosmos DB database provider for EF Core does not generate key values by default. This means key values must be explicitly set before saving new entities. See https://aka.ms/ef-cosmos-keys for more information.</value>
+    <comment>Warning CosmosEventId.PrimaryKeyValueNotSet string string</comment>
+  </data>
   <data name="LogSyncNotSupported" xml:space="preserve">
     <value>Azure Cosmos DB does not support synchronous I/O. Make sure to use and correctly await only async methods when using Entity Framework Core to access Azure Cosmos DB. See https://aka.ms/ef-cosmos-nosync for more information.</value>
     <comment>Error CosmosEventId.SyncNotSupported</comment>

src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseWrapper.cs

@@ -3,6 +3,7 @@
 
 using System.Net;
 using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
+using Microsoft.EntityFrameworkCore.Cosmos.Diagnostics.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.Update.Internal;
@@ -283,6 +284,69 @@ private Task<bool> SaveAsync(IUpdateEntry entry, CancellationToken cancellationT
         switch (state)
         {
             case EntityState.Added:
+                var primaryKey = entityType.FindPrimaryKey();
+                if (primaryKey != null)
+                {
+                    // The code below checks for primary key properties that are not configured for value generation but have not
+                    // had a non-sentinel (effectively, non-CLR default) value set. For composite keys, we only check if at least
+                    // one property has value generation or a value set, since it is normal to have non-value generated parts of composite
+                    // keys where one part is the CLR default. However, on Cosmos, we exclude the partition key properties from this
+                    // check to ensure that, even if partition key properties have been set, at least one other primary key property is
+                    // also set.
+                    var partitionPropertyNeedsValue = true;
+                    var propertyNeedsValue = true;
+                    var allPkPropertiesAreFk = true;
+                    IProperty? firstNonPartitionKeyProperty = null;
+
+                    var partitionKeyProperties = entityType.GetPartitionKeyProperties();
+                    foreach (var property in primaryKey.Properties)
+                    {
+                        if (property.IsForeignKey())
+                        {
+                            // FK properties conceptually get their value from the associated principal key, which can be handled
+                            // automatically by the update pipeline in some cases, so exclude from this check.
+                            continue;
+                        }
+
+                        allPkPropertiesAreFk = false;
+
+                        var isPartitionKeyProperty = partitionKeyProperties.Contains(property);
+                        if (!isPartitionKeyProperty)
+                        {
+                            firstNonPartitionKeyProperty = property;
+                        }
+
+                        if (property.ValueGenerated != ValueGenerated.Never
+                            || entry.HasExplicitValue(property))
+                        {
+                            if (!isPartitionKeyProperty)
+                            {
+                                propertyNeedsValue = false;
+                                break;
+                            }
+
+                            partitionPropertyNeedsValue = false;
+                        }
+                    }
+
+                    if (!allPkPropertiesAreFk)
+                    {
+                        if (firstNonPartitionKeyProperty != null
+                            && propertyNeedsValue)
+                        {
+                            // There were non-partition key properties, so only throw if it is one of these that is not set,
+                            // ignoring partition key properties.
+                            Dependencies.Logger.PrimaryKeyValueNotSet(firstNonPartitionKeyProperty!);
+                        }
+                        else if (firstNonPartitionKeyProperty == null
+                                 && partitionPropertyNeedsValue)
+                        {
+                            // There were no non-partition key properties in the primary key, so in this case check if any of these is not set.
+                            Dependencies.Logger.PrimaryKeyValueNotSet(primaryKey.Properties[0]);
+                        }
+                    }
+                }
+
                 var newDocument = documentSource.GetCurrentDocument(entry);
                 if (newDocument != null)
                 {

src/EFCore/Update/IUpdateEntry.cs

@@ -66,6 +66,13 @@ public interface IUpdateEntry
     /// <returns><see langword="true" /> if the property has a temporary value, otherwise <see langword="false" />.</returns>
     bool HasTemporaryValue(IProperty property);
 
+    /// <summary>
+    ///     Gets a value indicating if the specified property has an explicit value set.
+    /// </summary>
+    /// <param name="property">The property to be checked.</param>
+    /// <returns><see langword="true" /> if the property has an explicitly set value, otherwise <see langword="false" />.</returns>
+    bool HasExplicitValue(IProperty property);
+
     /// <summary>
     ///     Gets a value indicating if the specified property has a store-generated value that has not yet been saved to the entity.
     /// </summary>