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

Malformed Query when GetAsync() and including a navigation collection with HasManyThrough #884

Closed
rachael-ross opened this issue Nov 19, 2020 · 1 comment
Labels

Comments

@rachael-ross
Copy link

rachael-ross commented Nov 19, 2020

Description

I have three cases where this is happening... I'll demonstrate one of them here.

I have UserProfile which can have many roles. So there is a collection of UserRoles (associative object) and an exposed collection of Roles which is configured to go through UserRoles.

When issuing the following request, it's resulting in a malformed query: /api/v1/userprofiles?include=roles

    public class UserProfile : Identifiable
    {
        [Attr]
        public UserProfileStatus Status { get; set; } = UserProfileStatus.Setup;

       [Attr]
        public string SubjectId { get; set; }

        [Attr]
        [Required, MaxLength(100)]
        public string FirstName { get; set; }

        [Attr]
        [Required, MaxLength(100)]
        public string LastName { get; set; }

        [Attr]
        [Required, MaxLength(100), EmailAddress]
        public string Email { get; set; }

        [Attr]
        public int OrgId { get; set; }

        [HasOne(IdentifiablePropertyName = nameof(OrgId))]
        public virtual Org Org { get; set; }

        [NotMapped]
        [HasManyThrough(nameof(UserRoles))]
        public ICollection<Role> Roles { get; set; } = new HashSet<Role>();

        public ICollection<UserRole> UserRoles { get; set; } = new HashSet<UserRole>();

        [Attr]
        public DateTime CreatedOn { get; set; } = DateTime.UtcNow;

        [Attr]
        public int? CreatedById { get; set; }

        [HasOne(IdentifiablePropertyName = nameof(CreatedById))]
        public virtual UserProfile CreatedBy { get; set; }
    }

    public partial class UserProfileConfig : IEntityTypeConfiguration<UserProfile>
    {
        public UserProfileConfig() { }

        public void Configure(EntityTypeBuilder<UserProfile> builder)
        {
            //builder.HasKey(e => e.Id); //tried with and without thinking EF might be getting confused on PK??
            builder.HasIndex(e => e.SubjectId).IsUnique();
            builder.HasIndex(e => e.Email).IsUnique();
            builder.HasOne(e => e.Org).WithMany().HasForeignKey(e => e.OrgId);
        }
    }

    public class UserRole : Identifiable
    {
        [Attr]
        public int UserProfileId { get; set; }

        [HasOne(IdentifiablePropertyName = nameof(UserProfileId))]
        public virtual UserProfile UserProfile { get; set; }

        [Attr]
        public int RoleId { get; set; }

        [HasOne(IdentifiablePropertyName = nameof(RoleId))]
        public virtual Role Role { get; set; }
    }

    public partial class UserRoleConfig : IEntityTypeConfiguration<UserRole>
    {
        public UserRoleConfig() { }

        public void Configure(EntityTypeBuilder<UserRole> builder)
        {
            //builder.HasKey(e => e.Id);  //tried with and without thinking EF might be getting confused on PK??
            builder.HasOne(e => e.UserProfile).WithMany(d => d.UserRoles).HasForeignKey(e => e.UserProfileId);
            builder.HasOne(e => e.Role).WithMany(d => d.UserRoles).HasForeignKey(e => e.RoleId);
            builder.HasIndex(e => new { e.RoleId, e.UserProfileId }).IsClustered(false);
        }
    }

    public class Role : Identifiable
    {
        [Attr]
       [Required, MaxLength(100)]
        public string Name { get; set; }

        [NotMapped]
        [HasManyThrough(nameof(UserRoles))]
        public ICollection<UserProfile> Users { get; set; } = new HashSet<UserProfile>();
        public ICollection<UserRole> UserRoles { get; set; } = new HashSet<UserRole>();
    }

    public partial class RoleConfig : IEntityTypeConfiguration<Role>
    {
        public RoleConfig() { }

        public void Configure(EntityTypeBuilder<Role> builder)
        {
            //builder.HasKey(e => e.Id);     //tried with and without thinking EF might be getting confused on PK??
        }
    }

If I place a breakpoint on line 60 of the EntityFrameworkCoreRepository, the query.Expression is:

public virtual async Task<IReadOnlyCollection<TResource>> GetAsync(QueryLayer layer)
        {
            _traceWriter.LogMethodStart(new {layer});
            if (layer == null) throw new ArgumentNullException(nameof(layer));

            IQueryable<TResource> query = ApplyQueryLayer(layer);
            return await query.ToListAsync();   // BREAKPOINT HERE
        }

{value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[SomeProject.Data.UserProfile]).Include("UserRoles.Role").OrderBy(userProfile => userProfile.Id).Take(Create(10).Item1).Select(userProfile => new UserProfile() {UserRoles = new HashSet`1(userProfile.UserRoles.OrderBy(userRole => userRole.Role.Id).Take(Create(10).Item1)), Id = userProfile.Id, CreatedById = userProfile.CreatedById, CreatedOn = userProfile.CreatedOn, Email = userProfile.Email, FirstName = userProfile.FirstName, LastName = userProfile.LastName, OrgId = userProfile.OrgId, Status = userProfile.Status, SubjectId = userProfile.SubjectId})}

Which results in the following SQL to be generated:

exec sp_executesql N'SELECT [t].[Id], [t].[CreatedById], [t].[CreatedOn], [t].[Email], [t].[FirstName], [t].[LastName], [t].[OrgId], [t].[Status], [t].[SubjectId], [t1].[Id], [t1].[RoleId], [t1].[UserProfileId], [t1].[Id0], [t1].[Name], [t1].[Id00]
FROM (
SELECT TOP(@__Create_Item1_0) [u].[Id], [u].[CreatedById], [u].[CreatedOn], [u].[Email], [u].[FirstName], [u].[LastName], [u].[OrgId], [u].[Status], [u].[SubjectId]
FROM [UserProfiles] AS [u]
ORDER BY [u].[Id]
) AS [t]
OUTER APPLY (
SELECT [t].[Id], [t].[RoleId], [t].[UserProfileId], [r0].[Id] AS [Id0], [r0].[Name], [t].[Id0] AS [Id00]
FROM (
SELECT TOP(@__Create_Item1_0) [u0].[Id], [u0].[RoleId], [u0].[UserProfileId], [r].[Id] AS [Id0]
FROM [UserRoles] AS [u0]
INNER JOIN [Roles] AS [r] ON [u0].[RoleId] = [r].[Id]
WHERE [t].[Id] = [u0].[UserProfileId]
ORDER BY [r].[Id]
) AS [t0]
INNER JOIN [Roles] AS [r0] ON [t].[RoleId] = [r0].[Id]
) AS [t1]
ORDER BY [t].[Id], [t1].[Id00], [t1].[Id], [t1].[Id0]',N'@__Create_Item1_0 int',@__Create_Item1_0=10

And then 3 Microsoft.Data.SqlClient.SqlException's are thrown:

"Invalid column name 'RoleId'.\r\nInvalid column name 'RoleId'.\r\nInvalid column name 'UserProfileId'.\r\nInvalid column name 'Id0'."

The query is looking for RoleId, UserProfileId and Id0 from the first select statement (table [t]), which doesn't exist.

This only happens when getting all UserProfiles and including Roles. If I request a single UserProfile (GetAsync(id)), it works fine and Roles are returned.

Environment

  • JsonApiDotNetCore Version: Master branch as of 11-15-2020
  • Other Relevant Package Versions: EFCore 3.1.10

EDIT**

Also, I took a look at the Can_include_in_primary_resources() test and changed the above code to use ISet<>, removed virtual and didn't initialize the collections to mirror the test case and I'm still getting the same error after regenerating my migration scripts and rebuilding the database.

Can't get the tests to run. Running the docker command and it reports the server is up and ready to accept connections, but getting a "password authentication failed for user "postgres"" when running the tests and when trying to connect to it via pgAdmin. Tried changing passwords to a single one throughout the system (postgres and the appveyor one), still no dice.

@bart-degreed
Copy link
Contributor

I've taken your code and created a repro in #886. This shows how to apply the right EF Core mappings for your data model. You can simplify a lot!

The error you're observing is the same as reported here, which is caused by a bug in EF Core. Please upvote that issue to draw some attention if getting it fixed is important to you.

One workaround is to disable paging, like this:

GET http://localhost:14141/api/v1/userprofiles?include=roles&page[size]=0 HTTP/1.1

Alternatively, set options.DefaultPageSize = null in Startup.cs (see PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants