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 5dd73d5 commit 20ff770
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 21 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
137 changes: 121 additions & 16 deletions test/EFCore.Cosmos.FunctionalTests/EndToEndCosmosTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,10 +478,13 @@ public async Task Can_read_with_find_with_resource_id_async()
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(Customer_WithResourceId))
.FindProperty(StoreKeyConvention.DefaultIdPropertyName));

context.Add(customer);
context.Add(
new Customer_WithResourceId
Expand All @@ -494,7 +497,7 @@ 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>()
.FindAsync(pk1, "42");
Expand All @@ -509,7 +512,7 @@ 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>()
.WithPartitionKey(partitionKey: pk1.ToString())
Expand All @@ -535,7 +538,7 @@ public void Can_read_with_find_with_resource_id()
PartitionKey = pk1
};

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

Expand All @@ -551,7 +554,7 @@ 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>()
.Find(pk1, "42");
Expand All @@ -566,7 +569,7 @@ 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>()
.WithPartitionKey(partitionKey: pk1.ToString())
Expand All @@ -582,7 +585,7 @@ 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();

Expand Down Expand Up @@ -802,7 +805,7 @@ public async Task Can_read_with_find_without_partition_key()
Name = "Theon"
};

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

Expand All @@ -811,7 +814,7 @@ 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);

Expand All @@ -821,6 +824,71 @@ public async Task Can_read_with_find_without_partition_key()
}
}

[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 Customer_WithResourceId
{
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<Customer_WithResourceId>().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""] = ""Customer_WithResourceId"") AND (c[""id""] = @__p_0))
OFFSET 0 LIMIT 1");
}
}

private class PartitionKeyContext : DbContext
{
public PartitionKeyContext(DbContextOptions dbContextOptions)
Expand All @@ -840,9 +908,9 @@ 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)
{
}
Expand Down Expand Up @@ -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,22 +977,61 @@ public PartitionKeyContextNonPrimaryKey(DbContextOptions dbContextOptions)
{
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Customer>(
cb =>
{
cb.HasKey(c => new { c.Id });
});
}
}

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.HasNoDiscriminator();
cb.Property(c => c.Id).HasConversion<string>();
cb.HasPartitionKey(c => c.Id);
cb.HasKey(c => new { c.Id });
});
}
}

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

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Customer_WithResourceId>(
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 PartitionKeyContextWithResourceId : DbContext
{
public PartitionKeyContext_WithResourceId(DbContextOptions dbContextOptions)
public PartitionKeyContextWithResourceId(DbContextOptions dbContextOptions)
: base(dbContextOptions)
{
}
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 20ff770

Please sign in to comment.