Skip to content

Commit

Permalink
Cosmos: Allow PK with just the partition key and using id as the part…
Browse files Browse the repository at this point in the history 
…ition key

Fixes #21396
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Jun 24, 2020
1 parent 8cd69e0 commit 4ce52b6
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 38 deletions.
3 changes: 2 additions & 1 deletion src/EFCore.Cosmos/ValueGeneration/Internal/IdValueGenerator.cs
View file
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
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)
: 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
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

0 comments on commit 4ce52b6

Please sign in to comment.