Skip to content

Commit 272ebfd

Browse files
AndriySvyrydAndriySvyryd
committed
Refactor validation methods into smaller methods
1 parent abba36e commit 272ebfd

File tree

17 files changed

+1305
-1036
lines changed

17 files changed

+1305
-1036
lines changed

‎.github/copilot-instructions.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ If you are not sure, do not guess, just tell that you don't know or ask clarifyi
7676
- Use nullable reference types
7777
- Use proper null-checking patterns
7878
- Use the null-conditional operator (`?.`) and null-coalescing operator (`??`) when appropriate
79+
- Don't disable nullability with a preprocessor directive (`#nullable disable`)
7980

8081
## Architecture and Design Patterns
8182

‎src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs‎

Lines changed: 156 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
54
using Microsoft.EntityFrameworkCore.Cosmos.Diagnostics.Internal;
65
using Microsoft.EntityFrameworkCore.Cosmos.Internal;
76
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
@@ -37,14 +36,9 @@ public override void Validate(IModel model, IDiagnosticsLogger<DbLoggerCategory.
3736
/// </summary>
3837
protected override void ValidateEntityType(
3938
IEntityType entityType,
40-
IConventionModel? conventionModel,
41-
bool requireFullNotifications,
42-
HashSet<IEntityType> validEntityTypes,
43-
Dictionary<IKey, IIdentityMap> identityMaps,
44-
bool sensitiveDataLogged,
4539
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
4640
{
47-
base.ValidateEntityType(entityType, conventionModel, requireFullNotifications, validEntityTypes, identityMaps, sensitiveDataLogged, logger);
41+
base.ValidateEntityType(entityType, logger);
4842

4943
ValidateKeys(entityType, logger);
5044
ValidateDatabaseProperties(entityType, logger);
@@ -466,53 +460,93 @@ protected override void ValidateIndex(
466460
{
467461
base.ValidateIndex(index, logger);
468462

469-
var entityType = index.DeclaringEntityType;
470-
471463
if (index.GetVectorIndexType() != null)
472464
{
473-
if (index.Properties.Count > 1)
474-
{
475-
throw new InvalidOperationException(
476-
CosmosStrings.CompositeVectorIndex(
477-
entityType.DisplayName(),
478-
string.Join(",", index.Properties.Select(e => e.Name))));
479-
}
480-
481-
if (index.Properties[0].GetVectorDistanceFunction() == null
482-
|| index.Properties[0].GetVectorDimensions() == null)
483-
{
484-
throw new InvalidOperationException(
485-
CosmosStrings.VectorIndexOnNonVector(
486-
entityType.DisplayName(),
487-
index.Properties[0].Name));
488-
}
465+
ValidateVectorIndex(index, logger);
489466
}
490467
else if (index.IsFullTextIndex() == true)
491468
{
492-
if (index.Properties.Count > 1)
493-
{
494-
throw new InvalidOperationException(
495-
CosmosStrings.CompositeFullTextIndex(
496-
index.DeclaringEntityType.DisplayName(),
497-
string.Join(",", index.Properties.Select(e => e.Name))));
498-
}
499-
500-
if (index.Properties[0].GetIsFullTextSearchEnabled() != true)
501-
{
502-
throw new InvalidOperationException(
503-
CosmosStrings.FullTextIndexOnNonFullTextProperty(
504-
index.DeclaringEntityType.DisplayName(),
505-
index.Properties[0].Name,
506-
nameof(CosmosPropertyBuilderExtensions.EnableFullTextSearch)));
507-
}
469+
ValidateFullTextIndex(index, logger);
508470
}
509471
else
472+
{
473+
ValidateUnsupportedIndex(index, logger);
474+
}
475+
}
476+
477+
/// <summary>
478+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
479+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
480+
/// any release. You should only use it directly in your code with extreme caution and knowing that
481+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
482+
/// </summary>
483+
protected virtual void ValidateVectorIndex(
484+
IIndex index,
485+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
486+
{
487+
var entityType = index.DeclaringEntityType;
488+
489+
if (index.Properties.Count > 1)
510490
{
511491
throw new InvalidOperationException(
512-
CosmosStrings.IndexesExist(
492+
CosmosStrings.CompositeVectorIndex(
513493
entityType.DisplayName(),
514494
string.Join(",", index.Properties.Select(e => e.Name))));
515495
}
496+
497+
if (index.Properties[0].GetVectorDistanceFunction() == null
498+
|| index.Properties[0].GetVectorDimensions() == null)
499+
{
500+
throw new InvalidOperationException(
501+
CosmosStrings.VectorIndexOnNonVector(
502+
entityType.DisplayName(),
503+
index.Properties[0].Name));
504+
}
505+
}
506+
507+
/// <summary>
508+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
509+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
510+
/// any release. You should only use it directly in your code with extreme caution and knowing that
511+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
512+
/// </summary>
513+
protected virtual void ValidateFullTextIndex(
514+
IIndex index,
515+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
516+
{
517+
if (index.Properties.Count > 1)
518+
{
519+
throw new InvalidOperationException(
520+
CosmosStrings.CompositeFullTextIndex(
521+
index.DeclaringEntityType.DisplayName(),
522+
string.Join(",", index.Properties.Select(e => e.Name))));
523+
}
524+
525+
if (index.Properties[0].GetIsFullTextSearchEnabled() != true)
526+
{
527+
throw new InvalidOperationException(
528+
CosmosStrings.FullTextIndexOnNonFullTextProperty(
529+
index.DeclaringEntityType.DisplayName(),
530+
index.Properties[0].Name,
531+
nameof(CosmosPropertyBuilderExtensions.EnableFullTextSearch)));
532+
}
533+
}
534+
535+
/// <summary>
536+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
537+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
538+
/// any release. You should only use it directly in your code with extreme caution and knowing that
539+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
540+
/// </summary>
541+
protected virtual void ValidateUnsupportedIndex(
542+
IIndex index,
543+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
544+
{
545+
var entityType = index.DeclaringEntityType;
546+
throw new InvalidOperationException(
547+
CosmosStrings.IndexesExist(
548+
entityType.DisplayName(),
549+
string.Join(",", index.Properties.Select(e => e.Name))));
516550
}
517551

518552
/// <summary>
@@ -528,13 +562,41 @@ protected override void ValidateProperty(
528562
{
529563
base.ValidateProperty(property, structuralType, logger);
530564

565+
ValidateVectorProperty(property, structuralType, logger);
566+
ValidateElementConverters(property, structuralType, logger);
567+
ValidateConcurrencyToken(property, structuralType, logger);
568+
}
569+
570+
/// <summary>
571+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
572+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
573+
/// any release. You should only use it directly in your code with extreme caution and knowing that
574+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
575+
/// </summary>
576+
protected virtual void ValidateVectorProperty(
577+
IProperty property,
578+
ITypeBase structuralType,
579+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
580+
{
531581
if (property.GetVectorDistanceFunction() is not null
532582
&& property.GetVectorDimensions() is not null)
533583
{
534584
// Will throw if the data type is not set and cannot be inferred.
535585
CosmosVectorType.CreateDefaultVectorDataType(property.ClrType);
536586
}
587+
}
537588

589+
/// <summary>
590+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
591+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
592+
/// any release. You should only use it directly in your code with extreme caution and knowing that
593+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
594+
/// </summary>
595+
protected virtual void ValidateElementConverters(
596+
IProperty property,
597+
ITypeBase structuralType,
598+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
599+
{
538600
var typeMapping = property.GetElementType()?.GetTypeMapping();
539601
while (typeMapping != null)
540602
{
@@ -550,7 +612,19 @@ protected override void ValidateProperty(
550612

551613
typeMapping = typeMapping.ElementTypeMapping;
552614
}
615+
}
553616

617+
/// <summary>
618+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
619+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
620+
/// any release. You should only use it directly in your code with extreme caution and knowing that
621+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
622+
/// </summary>
623+
protected virtual void ValidateConcurrencyToken(
624+
IProperty property,
625+
ITypeBase structuralType,
626+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
627+
{
554628
if (property.IsConcurrencyToken)
555629
{
556630
var storeName = property.GetJsonPropertyName();
@@ -581,18 +655,58 @@ protected override void ValidateTrigger(
581655
{
582656
base.ValidateTrigger(trigger, entityType, logger);
583657

658+
ValidateTriggerOnRootType(trigger, entityType, logger);
659+
ValidateTriggerType(trigger, entityType, logger);
660+
ValidateTriggerOperation(trigger, entityType, logger);
661+
}
662+
663+
/// <summary>
664+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
665+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
666+
/// any release. You should only use it directly in your code with extreme caution and knowing that
667+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
668+
/// </summary>
669+
protected virtual void ValidateTriggerOnRootType(
670+
ITrigger trigger,
671+
IEntityType entityType,
672+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
673+
{
584674
if (entityType.BaseType != null)
585675
{
586676
throw new InvalidOperationException(
587677
CosmosStrings.TriggerOnDerivedType(trigger.ModelName, entityType.DisplayName(), entityType.BaseType.DisplayName()));
588678
}
679+
}
589680

681+
/// <summary>
682+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
683+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
684+
/// any release. You should only use it directly in your code with extreme caution and knowing that
685+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
686+
/// </summary>
687+
protected virtual void ValidateTriggerType(
688+
ITrigger trigger,
689+
IEntityType entityType,
690+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
691+
{
590692
if (trigger.GetTriggerType() == null)
591693
{
592694
throw new InvalidOperationException(
593695
CosmosStrings.TriggerMissingType(trigger.ModelName, entityType.DisplayName()));
594696
}
697+
}
595698

699+
/// <summary>
700+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
701+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
702+
/// any release. You should only use it directly in your code with extreme caution and knowing that
703+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
704+
/// </summary>
705+
protected virtual void ValidateTriggerOperation(
706+
ITrigger trigger,
707+
IEntityType entityType,
708+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
709+
{
596710
if (trigger.GetTriggerOperation() == null)
597711
{
598712
throw new InvalidOperationException(

‎src/EFCore.Relational/Diagnostics/RelationalEventId.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ private enum Id
9999
// Model validation events
100100
ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600,
101101
BoolWithDefaultWarning,
102-
AllIndexPropertiesNotToMappedToAnyTable,
102+
AllIndexPropertiesNotMappedToAnyTable,
103103
IndexPropertiesBothMappedAndNotMappedToTable,
104104
IndexPropertiesMappedToNonOverlappingTables,
105105
ForeignKeyPropertiesMappedToUnrelatedTables,
@@ -914,8 +914,8 @@ private static EventId MakeValidationId(Id id)
914914
/// This event uses the <see cref="IndexEventData" /> payload when used with a <see cref="DiagnosticSource" />.
915915
/// </para>
916916
/// </remarks>
917-
public static readonly EventId AllIndexPropertiesNotToMappedToAnyTable =
918-
MakeValidationId(Id.AllIndexPropertiesNotToMappedToAnyTable);
917+
public static readonly EventId AllIndexPropertiesNotMappedToAnyTable =
918+
MakeValidationId(Id.AllIndexPropertiesNotMappedToAnyTable);
919919

920920
/// <summary>
921921
/// An index specifies properties some of which are mapped and some of which are not mapped to a column in a table.

‎src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2868,19 +2868,19 @@ private static string BatchSmallerThanMinBatchSize(EventDefinitionBase definitio
28682868
}
28692869

28702870
/// <summary>
2871-
/// Logs the <see cref="RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable" /> event.
2871+
/// Logs the <see cref="RelationalEventId.AllIndexPropertiesNotMappedToAnyTable" /> event.
28722872
/// </summary>
28732873
/// <param name="diagnostics">The diagnostics logger to use.</param>
28742874
/// <param name="entityType">The entity type on which the index is defined.</param>
28752875
/// <param name="index">The index on the entity type.</param>
2876-
public static void AllIndexPropertiesNotToMappedToAnyTable(
2876+
public static void AllIndexPropertiesNotMappedToAnyTable(
28772877
this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
28782878
IEntityType entityType,
28792879
IIndex index)
28802880
{
28812881
if (index.Name == null)
28822882
{
2883-
var definition = RelationalResources.LogUnnamedIndexAllPropertiesNotToMappedToAnyTable(diagnostics);
2883+
var definition = RelationalResources.LogUnnamedIndexAllPropertiesNotMappedToAnyTable(diagnostics);
28842884

28852885
if (diagnostics.ShouldLog(definition))
28862886
{
@@ -2904,7 +2904,7 @@ public static void AllIndexPropertiesNotToMappedToAnyTable(
29042904
}
29052905
else
29062906
{
2907-
var definition = RelationalResources.LogNamedIndexAllPropertiesNotToMappedToAnyTable(diagnostics);
2907+
var definition = RelationalResources.LogNamedIndexAllPropertiesNotMappedToAnyTable(diagnostics);
29082908

29092909
if (diagnostics.ShouldLog(definition))
29102910
{

‎src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
518518
/// doing so can result in application failures when updating to a new Entity Framework Core release.
519519
/// </summary>
520520
[EntityFrameworkInternal]
521-
public EventDefinitionBase? LogNamedIndexAllPropertiesNotToMappedToAnyTable;
521+
public EventDefinitionBase? LogNamedIndexAllPropertiesNotMappedToAnyTable;
522522

523523
/// <summary>
524524
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -527,7 +527,7 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
527527
/// doing so can result in application failures when updating to a new Entity Framework Core release.
528528
/// </summary>
529529
[EntityFrameworkInternal]
530-
public EventDefinitionBase? LogUnnamedIndexAllPropertiesNotToMappedToAnyTable;
530+
public EventDefinitionBase? LogUnnamedIndexAllPropertiesNotMappedToAnyTable;
531531

532532
/// <summary>
533533
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

0 commit comments

Comments
 (0)