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.

Sign up for GitHub

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

Jump to bottom

Cosmos: Allow PK with just the partition key and using id as the partition key #21405

Merged
merged 1 commit into from
Jun 24, 2020
Merged

Cosmos: Allow PK with just the partition key and using id as the partition key #21405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ protected override object NextValue(EntityEntry entry)
var partitionKey = entityType.GetPartitionKeyPropertyName();
foreach (var property in primaryKey.Properties)
{
if (property.Name == partitionKey)
if (property.Name == partitionKey
&& primaryKey.Properties.Count > 1)
{
continue;
}
Expand Down
166 changes: 133 additions & 33 deletions test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ private class Customer
public int PartitionKey { get; set; }
}

private class Customer_WithResourceId
private class CustomerWithResourceId
{
public string id { get; set; }
public string Name { get; set; }
Expand All @@ -427,7 +427,7 @@ private class CustomerGuid
public int PartitionKey { get; set; }
}

private class Customer_NoPartitionKey
private class CustomerNoPartitionKey
{
public int Id { get; set; }
public string Name { get; set; }
Expand Down Expand Up @@ -471,20 +471,23 @@ public async Task Can_read_with_find_with_resource_id_async()
const int pk1 = 1;
const int pk2 = 2;

var customer = new Customer_WithResourceId
var customer = new CustomerWithResourceId
{
id = "42",
Name = "Theon",
PartitionKey = pk1
};

await using (var context = new PartitionKeyContext_WithResourceId(options))
await using (var context = new PartitionKeyContextWithResourceId(options))
{
await context.Database.EnsureCreatedAsync();

Assert.Null(context.Model.FindEntityType(typeof(CustomerWithResourceId))
.FindProperty(StoreKeyConvention.DefaultIdPropertyName));

context.Add(customer);
context.Add(
new Customer_WithResourceId
new CustomerWithResourceId
{
id = "42",
Name = "Theon Twin",
Expand All @@ -494,9 +497,9 @@ public async Task Can_read_with_find_with_resource_id_async()
await context.SaveChangesAsync();
}

await using (var context = new PartitionKeyContext_WithResourceId(options))
await using (var context = new PartitionKeyContextWithResourceId(options))
{
var customerFromStore = await context.Set<Customer_WithResourceId>()
var customerFromStore = await context.Set<CustomerWithResourceId>()
.FindAsync(pk1, "42");

Assert.Equal("42", customerFromStore.id);
Expand All @@ -509,9 +512,9 @@ public async Task Can_read_with_find_with_resource_id_async()
await context.SaveChangesAsync();
}

await using (var context = new PartitionKeyContext_WithResourceId(options))
await using (var context = new PartitionKeyContextWithResourceId(options))
{
var customerFromStore = await context.Set<Customer_WithResourceId>()
var customerFromStore = await context.Set<CustomerWithResourceId>()
.WithPartitionKey(partitionKey: pk1.ToString())
.FirstAsync();

Expand All @@ -528,20 +531,20 @@ public void Can_read_with_find_with_resource_id()
const int pk1 = 1;
const int pk2 = 2;

var customer = new Customer_WithResourceId
var customer = new CustomerWithResourceId
{
id = "42",
Name = "Theon",
PartitionKey = pk1
};

using (var context = new PartitionKeyContext_WithResourceId(options))
using (var context = new PartitionKeyContextWithResourceId(options))
{
context.Database.EnsureCreated();

context.Add(customer);
context.Add(
new Customer_WithResourceId
new CustomerWithResourceId
{
id = "42",
Name = "Theon Twin",
Expand All @@ -551,9 +554,9 @@ public void Can_read_with_find_with_resource_id()
context.SaveChanges();
}

using (var context = new PartitionKeyContext_WithResourceId(options))
using (var context = new PartitionKeyContextWithResourceId(options))
{
var customerFromStore = context.Set<Customer_WithResourceId>()
var customerFromStore = context.Set<CustomerWithResourceId>()
.Find(pk1, "42");

Assert.Equal("42", customerFromStore.id);
Expand All @@ -566,9 +569,9 @@ public void Can_read_with_find_with_resource_id()
context.SaveChanges();
}

using (var context = new PartitionKeyContext_WithResourceId(options))
using (var context = new PartitionKeyContextWithResourceId(options))
{
var customerFromStore = context.Set<Customer_WithResourceId>()
var customerFromStore = context.Set<CustomerWithResourceId>()
.WithPartitionKey(partitionKey: pk1.ToString())
.First();

Expand All @@ -582,12 +585,12 @@ public void Can_read_with_find_with_resource_id()
public void Find_with_empty_resource_id_throws()
{
var options = Fixture.CreateOptions();
using (var context = new PartitionKeyContext_WithResourceId(options))
using (var context = new PartitionKeyContextWithResourceId(options))
{
context.Database.EnsureCreated();

Assert.Equal(CosmosStrings.InvalidResourceId,
Assert.Throws<InvalidOperationException>(() => context.Set<Customer_WithResourceId>().Find(1, "")).Message);
Assert.Throws<InvalidOperationException>(() => context.Set<CustomerWithResourceId>().Find(1, "")).Message);
}
}

Expand Down Expand Up @@ -796,13 +799,13 @@ public async Task Can_read_with_find_without_partition_key()
{
var options = Fixture.CreateOptions();

var customer = new Customer_NoPartitionKey
var customer = new CustomerNoPartitionKey
{
Id = 42,
Name = "Theon"
};

await using (var context = new PartitionKeyContext_EntityWithNoPartitionKey(options))
await using (var context = new PartitionKeyContextEntityWithNoPartitionKey(options))
{
await context.Database.EnsureCreatedAsync();

Expand All @@ -811,13 +814,78 @@ public async Task Can_read_with_find_without_partition_key()
await context.SaveChangesAsync();
}

await using (var context = new PartitionKeyContext_EntityWithNoPartitionKey(options))
await using (var context = new PartitionKeyContextEntityWithNoPartitionKey(options))
{
var customerFromStore = context.Set<Customer_NoPartitionKey>().Find(42);
var customerFromStore = context.Set<CustomerNoPartitionKey>().Find(42);

Assert.Equal(42, customerFromStore.Id);
Assert.Equal("Theon", customerFromStore.Name);
AssertSql(context, @"ReadItem(, Customer_NoPartitionKey|42)");
AssertSql(context, @"ReadItem(, CustomerNoPartitionKey|42)");
}
}

[ConditionalFact]
public async Task Can_read_with_find_with_PK_partition_key()
{
var options = Fixture.CreateOptions();

var customer = new Customer
{
Id = 42,
Name = "Theon"
};

await using (var context = new PartitionKeyContextPrimaryKey(options))
{
await context.Database.EnsureCreatedAsync();

context.Add(customer);

await context.SaveChangesAsync();
}

await using (var context = new PartitionKeyContextPrimaryKey(options))
{
var customerFromStore = context.Set<Customer>().Find(42);

Assert.Equal(42, customerFromStore.Id);
Assert.Equal("Theon", customerFromStore.Name);
AssertSql(context, @"ReadItem(42, 42)");
}
}

[ConditionalFact]
public async Task Can_read_with_find_with_PK_resource_id()
{
var options = Fixture.CreateOptions();

var customer = new CustomerWithResourceId
{
id = "42",
Name = "Theon"
};

await using (var context = new PartitionKeyContextWithPrimaryKeyResourceId(options))
{
await context.Database.EnsureCreatedAsync();

context.Add(customer);

await context.SaveChangesAsync();
}

await using (var context = new PartitionKeyContextWithPrimaryKeyResourceId(options))
{
var customerFromStore = context.Set<CustomerWithResourceId>().Find("42");

Assert.Equal("42", customerFromStore.id);
Assert.Equal("Theon", customerFromStore.Name);
AssertSql(context, @"@__p_0='42'

SELECT c
FROM root c
WHERE ((c[""Discriminator""] = ""CustomerWithResourceId"") AND (c[""id""] = @__p_0))
OFFSET 0 LIMIT 1");
}
}

Expand All @@ -840,16 +908,16 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
}
}

private class PartitionKeyContext_EntityWithNoPartitionKey : DbContext
private class PartitionKeyContextEntityWithNoPartitionKey : DbContext
{
public PartitionKeyContext_EntityWithNoPartitionKey(DbContextOptions dbContextOptions)
public PartitionKeyContextEntityWithNoPartitionKey(DbContextOptions dbContextOptions)
: base(dbContextOptions)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Customer_NoPartitionKey>();
modelBuilder.Entity<CustomerNoPartitionKey>();
}
}

Expand All @@ -870,7 +938,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
cb.Property(StoreKeyConvention.DefaultIdPropertyName)
.HasValueGenerator((p, e) => valueGeneratorFactory.Create(p));

cb.Property(c => c.Id);
cb.Property(c => c.PartitionKey).HasConversion<string>();

cb.HasPartitionKey(c => c.PartitionKey);
Expand All @@ -895,7 +962,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)

cb.Property(StoreKeyConvention.DefaultIdPropertyName).HasValueGenerator((Type)null);

cb.Property(c => c.Id);
cb.Property(c => c.PartitionKey).HasConversion<string>();

cb.HasPartitionKey(c => c.PartitionKey);
Expand All @@ -911,29 +977,63 @@ public PartitionKeyContextNonPrimaryKey(DbContextOptions dbContextOptions)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Customer>();
}
}

private class PartitionKeyContextPrimaryKey : DbContext
{
public PartitionKeyContextPrimaryKey(DbContextOptions dbContextOptions)
: base(dbContextOptions)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Customer>(
cb =>
{
var valueGeneratorFactory = new CustomPartitionKeyIdValueGeneratorFactory();

cb.Property(c => c.Id);
cb.HasKey(c => new { c.Id });
cb.HasNoDiscriminator();
cb.Property(c => c.Id).HasConversion<string>();
cb.HasPartitionKey(c => c.Id);
});
}
}

private class PartitionKeyContextWithPrimaryKeyResourceId : DbContext
{
public PartitionKeyContextWithPrimaryKeyResourceId(DbContextOptions dbContextOptions)
: base(dbContextOptions)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<CustomerWithResourceId>(
cb =>
{
cb.HasPartitionKey(c => c.PartitionKey);
cb.Property(c => c.PartitionKey).HasConversion<string>();
cb.Property(c => c.id).HasConversion<string>();
cb.HasKey(c => new { c.id });
});
}
}

private class PartitionKeyContext_WithResourceId : DbContext
private class PartitionKeyContextWithResourceId : DbContext
{
public PartitionKeyContext_WithResourceId(DbContextOptions dbContextOptions)
public PartitionKeyContextWithResourceId(DbContextOptions dbContextOptions)
AndriySvyryd marked this conversation as resolved.
Show resolved Hide resolved
: base(dbContextOptions)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Customer_WithResourceId>(
modelBuilder.Entity<CustomerWithResourceId>(
cb =>
{
cb.HasPartitionKey(c => c.PartitionKey);
Expand Down
21 changes: 17 additions & 4 deletions test/EFCore.Cosmos.Tests/Infrastructure/CosmosModelValidatorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,22 @@ public virtual void Passes_on_valid_partition_keys()
modelBuilder.Entity<Order>().ToContainer("Orders").HasPartitionKey(o => o.PartitionId)
.Property(o => o.PartitionId).HasConversion<string>();

var model = modelBuilder.Model;
Validate(model);
Validate(modelBuilder.Model);
}

[ConditionalFact]
public virtual void Passes_PK_partition_key()
{
var modelBuilder = CreateConventionalModelBuilder();
modelBuilder.Entity<Order>(b =>
{
b.HasKey(o => o.PartitionId);
b.Ignore(o => o.Customer);
b.Ignore(o => o.OrderDetails);
b.Ignore(o => o.Products);
});

Validate(modelBuilder.Model);
}

[ConditionalFact]
Expand All @@ -115,8 +129,7 @@ public virtual void Detects_non_key_partition_key_property()
b.Ignore(o => o.Products);
});

var model = modelBuilder.Model;
VerifyError(CosmosStrings.NoPartitionKeyKey(typeof(Order).Name, nameof(Order.PartitionId), "id"), model);
VerifyError(CosmosStrings.NoPartitionKeyKey(typeof(Order).Name, nameof(Order.PartitionId), "id"), modelBuilder.Model);
}

[ConditionalFact]
Expand Down