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

Cosmos: EF Core 9 fails to find document with '|' character in it's id #35224

Open
mvaisanen opened this issue Nov 27, 2024 · 5 comments · May be fixed by #35252
Open

Cosmos: EF Core 9 fails to find document with '|' character in it's id #35224

mvaisanen opened this issue Nov 27, 2024 · 5 comments · May be fixed by #35252
Assignees
Labels
area-cosmos area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported needs-design type-bug

Comments

@mvaisanen
Copy link

I'm working on a project which for legacy reasons have lots of documents with the '|' character in their id. To be precise, these used to be '/' in another database and since cosmos doesnt support those, they were converted to '|'. There needs to be some special character in the id as parts of business logic relies on that (and converting the id to base64 would make other matters difficult).

Up until EF Core 9 they worked fine (EF Core 8.0.11 works). However, when upgrading to EF Core 9, these documents cannot be found by the framework. Here is a small .NET 8 console app demonstrating the issue. Example run against Cosmos DB Emulator database "TestDatabase", Autoscale max 4000 RU/s, having 1 container "TestContainer" with partition key "/id".

Program.cs

using Microsoft.EntityFrameworkCore;

namespace CosmosTestingApp
{
    internal class Program
    {
        static void Main(string[] args)
        {
            DoWork().GetAwaiter().GetResult();

            Console.ReadKey();
        }

        private static async Task<int> DoWork()
        {
            var endpoint = "https://localhost:8081";
            var cosmosKey = "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==";
            var DbName = "TestDatabase";

            using (var ctx = new PetContext(new DbContextOptionsBuilder<PetContext>().LogTo(Console.WriteLine).UseCosmos(endpoint, cosmosKey, DbName).Options))
            {
                ctx.Cats.Add(new Cat() { Id = "Cat|1", Name = "First Cat" });
                ctx.Cats.Add(new Cat() { Id = "Cat2", Name = "Second Cat" });
                await ctx.SaveChangesAsync();
            }

            using (var ctx2 = new PetContext(new DbContextOptionsBuilder<PetContext>().LogTo(Console.WriteLine).UseCosmos(endpoint, cosmosKey, DbName).Options))
            {
                var cat2 = await ctx2.Cats.Where(c => c.Id == "Cat2").SingleAsync();
                Console.WriteLine($"Cat name: {cat2.Name}");
                var cat1 = await ctx2.Cats.Where(c => c.Id == "Cat|1").SingleAsync();
                Console.WriteLine($"Cat name: {cat1.Name}");
            }

            return 0;
        }
    }
}

PetContext.cs

using Microsoft.EntityFrameworkCore;

namespace CosmosTestingApp
{
    public class PetContext: DbContext
    {
        public PetContext(DbContextOptions<PetContext> options) : base(options)
        {
        }

        public DbSet<Cat> Cats { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.EnableSensitiveDataLogging();
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            base.OnModelCreating(modelBuilder);

            modelBuilder.Entity<Cat>(e =>
            {
                e.ToContainer("TestContainer");
                e.HasPartitionKey(s => s.Id);
                e.HasDiscriminator<string>("Discriminator");
            });
        }
    }
}

Pet.cs

namespace CosmosTestingApp
{
    public class Cat
    {
        public string Id { get; set; }
        public string Name { get; set; }
        public string Discriminator { get; set; }
    }
}

.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.Cosmos" Version="9.0.0" />
  </ItemGroup>

</Project>

When running the app, second cat is found as expected, but query against the first fails with "Sequence contains no elements". Doesnt work with FindAsync() etc either. Examining the logs, it seems EF Core inserts an extra '^' character to the id of the first cat query, which explains why it fails on db level.

dbug: 27.11.2024 18:19:45.154 CoreEventId.QueryCompilationStarting[10111] (Microsoft.EntityFrameworkCore.Query)
      Compiling query expression:
      'DbSet<Cat>()
          .Where(c => c.Id == "Cat2")
          .Single()'
dbug: 27.11.2024 18:19:45.390 CoreEventId.QueryExecutionPlanned[10107] (Microsoft.EntityFrameworkCore.Query)
      Generated query execution expression:
      'queryContext => ShapedQueryCompilingExpressionVisitor.SingleAsync<Cat>(
          asyncEnumerable: new ReadItemQueryingEnumerable<Cat>(
              (CosmosQueryContext)queryContext,
              EntityType: Cat,
              List<Expression> { [Cosmos.Query.Internal.SqlConstantExpression] },
              ReadItemInfo,
              Func<QueryContext, JToken, Cat>,
              CosmosTestingApp.PetContext,
              False,
              True
          ),
          cancellationToken: queryContext.CancellationToken)'
info: 27.11.2024 18:19:45.408 CosmosEventId.ExecutingReadItem[30101] (Microsoft.EntityFrameworkCore.Database.Command)
      Reading resource 'Cat2' item from container 'TestContainer' in partition '["Cat2"]'.

<SNIP>

dbug: 27.11.2024 18:19:47.904 CoreEventId.QueryCompilationStarting[10111] (Microsoft.EntityFrameworkCore.Query)
      Compiling query expression:
      'DbSet<Cat>()
          .Where(c => c.Id == "Cat|1")
          .Single()'
dbug: 27.11.2024 18:19:47.915 CoreEventId.QueryExecutionPlanned[10107] (Microsoft.EntityFrameworkCore.Query)
      Generated query execution expression:
      'queryContext => ShapedQueryCompilingExpressionVisitor.SingleAsync<Cat>(
          asyncEnumerable: new ReadItemQueryingEnumerable<Cat>(
              (CosmosQueryContext)queryContext,
              EntityType: Cat,
              List<Expression> { [Cosmos.Query.Internal.SqlConstantExpression] },
              ReadItemInfo,
              Func<QueryContext, JToken, Cat>,
              CosmosTestingApp.PetContext,
              False,
              True
          ),
          cancellationToken: queryContext.CancellationToken)'
info: 27.11.2024 18:19:47.919 CosmosEventId.ExecutingReadItem[30101] (Microsoft.EntityFrameworkCore.Database.Command)
      Reading resource 'Cat^|1' item from container 'TestContainer' in partition '["Cat|1"]'.

I'm aware of the recommendation to use only alphanumeric ids (https://learn.microsoft.com/en-us/azure/cosmos-db/concepts-limits), however this used to work ok on earlier versions, and still works just fine when running manual queries against cosmos. The issue seems to be just that EF Core corrupts the id intended to be queried. However, as the example shows, even saving works just fine.

Provider and version information

EF Core version: 9.0.0
Database provider: Microsoft.EntityFrameworkCore.Cosmos
Target framework: NET 8.0
Operating system: Win10 Enterprise
IDE: Visual Studio 2022 17.9.4

@mvaisanen mvaisanen changed the title Cosmos: EF Core 9 fails to load documents with '|' character in it's id Cosmos: EF Core 9 fails to find documents with '|' character in it's id Nov 27, 2024
@mvaisanen mvaisanen changed the title Cosmos: EF Core 9 fails to find documents with '|' character in it's id Cosmos: EF Core 9 fails to find document with '|' character in it's id Nov 27, 2024
@ajcvickers
Copy link
Contributor

@mvaisanen In EF9, we changed the way we generate key values. To get back to the old behavior, you can use:

modelBuilder.HasDiscriminatorInJsonIds();

See The id property now contains only the EF key property by default for more information.

Notes for team: EF appears to be doing the correct thing when creating the documents:

{
    "id": "Cat|1",
    "Discriminator": "Cat",
    "Name": "First Cat",
    "_rid": "59QFAK-3p3YBAAAAAAAAAA==",
    "_self": "dbs/59QFAA==/colls/59QFAK-3p3Y=/docs/59QFAK-3p3YBAAAAAAAAAA==/",
    "_etag": "\"00000000-0000-0000-4275-0430696601db\"",
    "_attachments": "attachments/",
    "_ts": 1732894828
}

{
    "id": "Cat2",
    "Discriminator": "Cat",
    "Name": "Second Cat",
    "_rid": "59QFAK-3p3YCAAAAAAAAAA==",
    "_self": "dbs/59QFAA==/colls/59QFAK-3p3Y=/docs/59QFAK-3p3YCAAAAAAAAAA==/",
    "_etag": "\"00000000-0000-0000-4275-0436508d01db\"",
    "_attachments": "attachments/",
    "_ts": 1732894828
}

Notice that there is a single id property, and the values supplied by the application have not been modified.

However, when we attempt to perform a ReadItem, we end up with the wrong key values:

info: 11/29/2024 16:01:40.327 CosmosEventId.ExecutedReadItem[30103] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed ReadItem (477 ms, 1 RU) ActivityId='d696a6b0-65be-4cd0-91e0-792285c4d0e3', Container='TestContainer', Id='Cat^|1', Partition='["Cat|1"]'
Unhandled exception. System.InvalidOperationException: Sequence contains no elements.

Both the Cosmos ID and the Cosmos partition key should be Cat|1, but the ID is Cat^|1 instead.

@ajcvickers
Copy link
Contributor

Also, note for team: I'm not marking this as a regression, since the breaking change was intentional, and when the API to revert back to the old behavior is used, then everything works still.

@roji
Copy link
Member

roji commented Nov 30, 2024

@ajcvickers but if we generate the wrong id for ReadItem, doesn't it mean that there are queries which worked before and which don't now (returning no results)?

@ajcvickers
Copy link
Contributor

@roji Yes, that's what this issue is. But we knew this was going to happen because we create key values differently--this is the breaking change. So I don't think we should treat it as a regression because of that. But honestly, I expect the fix will not be high risk, and I'm fine with shipping any low-risk patch. Tactics might not be.

@roji
Copy link
Member

roji commented Nov 30, 2024

OK, understood!

ajcvickers added a commit that referenced this issue Dec 2, 2024
Fixes #35224

When we generate a key containing multiple values, then we escape the separator character if it is found in any of the values. However, we should not do this if the key is made up of a single value and so will have no separators.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 2, 2024
toiyabe added a commit to MoonriseSoftwareCalifornia/AspNetCore.Identity.CosmosDb that referenced this issue Dec 5, 2024
…t/efcore#35264. Changed to all Async methods, added support for json delimiters. Unit tests passing. Updated readme.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-cosmos area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported needs-design type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants