Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Differentiate between a sequence's database schema and model schema #28943

Merged
merged 1 commit into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,11 @@ protected virtual void GenerateSequence(
.Append("(")
.Append(Code.Literal(sequence.Name));

if (!string.IsNullOrEmpty(sequence.Schema)
&& sequence.Model.GetDefaultSchema() != sequence.Schema)
if (!string.IsNullOrEmpty(sequence.ModelSchema))
{
sequenceBuilderNameBuilder
.Append(", ")
.Append(Code.Literal(sequence.Schema));
.Append(Code.Literal(sequence.ModelSchema));
}

sequenceBuilderNameBuilder.Append(")");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ private static void UpdateSequences(IReadOnlyModel model, string version)
var sequencesDictionary = new SortedDictionary<(string, string?), ISequence>();
foreach (var sequence in sequences)
{
sequencesDictionary[(sequence.Name, sequence.Schema)] = sequence;
sequencesDictionary[(sequence.Name, sequence.ModelSchema)] = sequence;
}

if (sequencesDictionary.Count > 0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Text;
using Microsoft.EntityFrameworkCore.Design.Internal;
using Microsoft.EntityFrameworkCore.Internal;
Expand Down Expand Up @@ -328,7 +329,8 @@ private string CreateModelBuilder(
mainBuilder,
methodBuilder,
namespaces,
variables);
variables,
nullable);

foreach (var typeConfiguration in model.GetTypeMappingConfigurations())
{
Expand Down Expand Up @@ -471,7 +473,7 @@ private string GenerateEntityType(IEntityType entityType, string @namespace, str
CreateSkipNavigation(navigation, navigationNumber++, mainBuilder, methodBuilder, namespaces, className, nullable);
}

CreateAnnotations(entityType, mainBuilder, methodBuilder, namespaces, className);
CreateAnnotations(entityType, mainBuilder, methodBuilder, namespaces, className, nullable);
}

mainBuilder.AppendLine("}");
Expand Down Expand Up @@ -522,7 +524,8 @@ private void CreateEntityType(
mainBuilder,
methodBuilder,
namespaces,
variables);
variables,
nullable);

Create(entityType, parameters);

Expand Down Expand Up @@ -1150,7 +1153,8 @@ private void CreateForeignKey(
mainBuilder,
methodBuilder,
namespaces,
variables);
variables,
nullable);

var navigation = foreignKey.DependentToPrincipal;
if (navigation != null)
Expand Down Expand Up @@ -1249,7 +1253,8 @@ private void CreateSkipNavigation(
mainBuilder,
methodBuilder,
namespaces,
variables);
variables,
nullable);

mainBuilder
.Append("var ").Append(navigationVariable).Append(" = ")
Expand Down Expand Up @@ -1352,7 +1357,8 @@ private void CreateAnnotations(
IndentedStringBuilder mainBuilder,
IndentedStringBuilder methodBuilder,
SortedSet<string> namespaces,
string className)
string className,
bool nullable)
{
mainBuilder.AppendLine()
.Append("public static void CreateAnnotations")
Expand All @@ -1373,7 +1379,8 @@ private void CreateAnnotations(
mainBuilder,
methodBuilder,
namespaces,
variables));
variables,
nullable));

mainBuilder
.AppendLine()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ public override void Generate(IModel model, CSharpRuntimeAnnotationCodeGenerator
{
parameters.Namespaces.Add(typeof(SortedDictionary<,>).Namespace!);
var sequencesVariable = Dependencies.CSharpHelper.Identifier("sequences", parameters.ScopeVariables, capitalize: false);
parameters.MainBuilder
.Append("var ").Append(sequencesVariable).AppendLine(" = new SortedDictionary<(string, string), ISequence>();");
var mainBuilder = parameters.MainBuilder;
mainBuilder.Append("var ").Append(sequencesVariable).Append(" = new SortedDictionary<(string, string");

if (parameters.UseNullableReferenceTypes)
{
mainBuilder.Append("?");
}

mainBuilder.AppendLine("), ISequence>();");

foreach (var sequencePair in sequences)
{
Expand Down Expand Up @@ -279,6 +286,12 @@ private void Create(ISequence sequence, string sequencesVariable, CSharpRuntimeA
.Append("maxValue: ").Append(code.Literal(sequence.MaxValue));
}

if (sequence.ModelSchema is null && sequence.Schema is not null)
{
mainBuilder.AppendLine(",")
.Append("modelSchemaIsNull: ").Append(code.Literal(true));
}

mainBuilder.AppendLine(");").DecrementIndent()
.AppendLine();

Expand All @@ -289,7 +302,7 @@ private void Create(ISequence sequence, string sequencesVariable, CSharpRuntimeA

mainBuilder
.Append(sequencesVariable).Append("[(").Append(code.Literal(sequence.Name)).Append(", ")
.Append(code.Literal(sequence.Schema)).Append(")] = ")
.Append(code.Literal(sequence.ModelSchema)).Append(")] = ")
.Append(sequenceVariable).AppendLine(";")
.AppendLine();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,8 @@ private static RuntimeSequence Create(ISequence sequence, RuntimeModel runtimeMo
sequence.IncrementBy,
sequence.IsCyclic,
sequence.MinValue,
sequence.MaxValue);
sequence.MaxValue,
sequence.ModelSchema is null);

/// <summary>
/// Updates the sequence annotations that will be set on the read-only object.
Expand Down
11 changes: 9 additions & 2 deletions src/EFCore.Relational/Metadata/IReadOnlySequence.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ public interface IReadOnlySequence : IReadOnlyAnnotatable
/// </summary>
string Name { get; }

/// <summary>
/// Gets the model schema of the sequence. This is the one specified in
/// <see cref="RelationalModelBuilderExtensions.HasSequence(ModelBuilder, string, string?)" /> and the one to use
/// with <see cref="RelationalModelExtensions.FindSequence(IConventionModel, string, string?)"/>.
/// </summary>
string? ModelSchema { get; }

/// <summary>
/// Gets the database schema that contains the sequence.
/// </summary>
Expand Down Expand Up @@ -80,10 +87,10 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
.Append(indentString)
.Append("Sequence: ");

if (Schema != null)
if (ModelSchema != null)
{
builder
.Append(Schema)
.Append(ModelSchema)
.Append('.');
}

Expand Down
13 changes: 11 additions & 2 deletions src/EFCore.Relational/Metadata/Internal/Sequence.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public static Sequence AddSequence(
sequence.EnsureMutable();

var sequences = (SortedDictionary<(string, string?), ISequence>?)model[RelationalAnnotationNames.Sequences];
var tuple = (sequence.Name, sequence.Schema);
var tuple = (sequence.Name, sequence.ModelSchema);
if (sequences == null
|| !sequences.ContainsKey(tuple))
{
Expand All @@ -201,7 +201,7 @@ public static Sequence AddSequence(

sequence.Name = name;

sequences.Add((name, sequence.Schema), sequence);
sequences.Add((name, sequence.ModelSchema), sequence);

return sequence;
}
Expand Down Expand Up @@ -280,6 +280,15 @@ public override bool IsReadOnly
/// </summary>
public virtual string Name { get; set; }

/// <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 virtual string? ModelSchema
=> _schema;

/// <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
Expand Down
12 changes: 11 additions & 1 deletion src/EFCore.Relational/Metadata/RuntimeSequence.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class RuntimeSequence : AnnotatableBase, ISequence
private readonly long? _minValue;
private readonly long? _maxValue;
private readonly bool _isCyclic;
private readonly bool _modelSchemaIsNull;

/// <summary>
/// Initializes a new instance of the <see cref="RuntimeSequence" /> class.
Expand All @@ -33,6 +34,7 @@ public class RuntimeSequence : AnnotatableBase, ISequence
/// <param name="cyclic">Whether the sequence is cyclic.</param>
/// <param name="minValue">The minimum value.</param>
/// <param name="maxValue">The maximum value.</param>
/// <param name="modelSchemaIsNull">A value indicating whether <see cref="ModelSchema"/> is null.</param>
public RuntimeSequence(
string name,
RuntimeModel model,
Expand All @@ -42,7 +44,8 @@ public RuntimeSequence(
int incrementBy = Sequence.DefaultIncrementBy,
bool cyclic = false,
long? minValue = null,
long? maxValue = null)
long? maxValue = null,
bool modelSchemaIsNull = false)
{
Model = model;
Name = name;
Expand All @@ -53,6 +56,7 @@ public RuntimeSequence(
_isCyclic = cyclic;
_minValue = minValue;
_maxValue = maxValue;
_modelSchemaIsNull = modelSchemaIsNull;
}

/// <summary>
Expand All @@ -65,6 +69,12 @@ public RuntimeSequence(
/// </summary>
public virtual string Name { get; }

/// <summary>
/// Gets the metadata schema of the sequence.
/// </summary>
public virtual string? ModelSchema
=> _modelSchemaIsNull ? null : _schema;
bricelam marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets the database schema that contains the sequence.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ public CSharpRuntimeAnnotationCodeGeneratorParameters(
IndentedStringBuilder mainBuilder,
IndentedStringBuilder methodBuilder,
ISet<string> namespaces,
ISet<string> scopeVariables)
ISet<string> scopeVariables,
bool nullable)
{
TargetName = targetName;
ClassName = className;
MainBuilder = mainBuilder;
MethodBuilder = methodBuilder;
Namespaces = namespaces;
ScopeVariables = scopeVariables;
UseNullableReferenceTypes = nullable;
}

/// <summary>
Expand Down Expand Up @@ -78,4 +80,10 @@ public CSharpRuntimeAnnotationCodeGeneratorParameters(
/// Indicates whether the given annotations are runtime annotations.
/// </summary>
public bool IsRuntime { get; init; }

/// <summary>
/// Gets or sets a value indicating whther nullable reference types are enabled.
/// </summary>
/// <value>A value indicating whther nullable reference types are enabled.</value>
public bool UseNullableReferenceTypes { get; init; }
}
36 changes: 36 additions & 0 deletions test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1278,6 +1278,42 @@ public virtual void Sequence_is_stored_in_snapshot_as_fluent_api()
Assert.Equal("bar", sequence["foo"]);
});

[ConditionalFact]
public virtual void HiLoSequence_with_default_model_schema()
=> Test(
modelBuilder => modelBuilder
.HasDefaultSchema("dbo")
.Entity("Entity").Property<int>("Id").UseHiLo(schema: "dbo"),
AddBoilerPlate(@"
modelBuilder
.HasDefaultSchema(""dbo"")
.HasAnnotation(""Relational:MaxIdentifierLength"", 128);

SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder);

modelBuilder.HasSequence(""EntityFrameworkHiLoSequence"", ""dbo"")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this change, we wouldn't scaffold the schema here causing two sequences to be added to the model (the other by UseHiLo)

.IncrementsBy(10);

modelBuilder.Entity(""Entity"", b =>
{
b.Property<int>(""Id"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"");

SqlServerPropertyBuilderExtensions.UseHiLo(b.Property<int>(""Id""), ""EntityFrameworkHiLoSequence"", ""dbo"");

b.HasKey(""Id"");

b.ToTable(""Entity"", ""dbo"");
});"),
model =>
{
Assert.Equal("dbo", model.GetDefaultSchema());

var sequence = Assert.Single(model.GetSequences());
Assert.Equal("dbo", sequence.Schema);
});

[ConditionalFact]
public virtual void CheckConstraint_is_stored_in_snapshot_as_fluent_api()
=> Test(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2384,14 +2384,15 @@ partial void Initialize()
PrincipalBaseEntityType.CreateAnnotations(principalBase);
PrincipalDerivedEntityType.CreateAnnotations(principalDerived);

var sequences = new SortedDictionary<(string, string), ISequence>();
var sequences = new SortedDictionary<(string, string?), ISequence>();
var principalBaseSequence = new RuntimeSequence(
""PrincipalBaseSequence"",
this,
typeof(long),
schema: ""TPC"");
schema: ""TPC"",
modelSchemaIsNull: true);

sequences[(""PrincipalBaseSequence"", ""TPC"")] = principalBaseSequence;
sequences[(""PrincipalBaseSequence"", null)] = principalBaseSequence;

AddAnnotation(""Relational:Sequences"", sequences);
AddAnnotation(""Relational:DefaultSchema"", ""TPC"");
Expand Down Expand Up @@ -2936,6 +2937,9 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType)
Assert.Equal(
new[] { dependentBase, principalBase, principalDerived },
model.GetEntityTypes());

var principalBaseSequence = model.FindSequence("PrincipalBaseSequence");
Assert.Equal("TPC", principalBaseSequence.Schema);
},
typeof(SqlServerNetTopologySuiteDesignTimeServices));

Expand Down
3 changes: 3 additions & 0 deletions test/EFCore.Sqlite.Tests/SqliteEventIdTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ private class FakeSequence : Annotatable, IReadOnlySequence
public string Name
=> "SequenceName";

public string ModelSchema
=> throw new NotImplementedException();

public string Schema
=> throw new NotImplementedException();

Expand Down