efcore icon indicating copy to clipboard operation
efcore copied to clipboard

Expose a non-generic DbSet to support operations with shared-type entity types

Open mitselplik opened this issue 4 years ago • 13 comments

Assume a DbContext with two related DBSet<> properties like so:

public MyDbContext : DbContext
{
    public virtual DbSet<Supplier> Account { get; set; }
    public virtual DbSet<Part> Account { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        . . .
        EntityTypeBuilder<Part> partEntity
            .HasOne(d => d.Supplier)
            .WithMany(p => p.Part)
            .HasForeignKey(d => d.SupplierId);
        . . .
    }

No means exists (that I could find) to be able to the following:

public static IEnumerable DoAwesomeStuff<TEntity>(this DbSet<TEntity> dataSource, params object[] otherParameters)
	where TEntity : class
{
	string sql = "...Stuff I did to generate this internally...";
	var dbContext = (dataSource as IInfrastructure<IServiceProvider>).GetService<ICurrentDbContext>().Context;

	// Get the result
	var dbSet = dbContext.Set<TEntity>();
	var parentResults = dbSet.FromSqlRaw(sql);

	// Do other logic to hydrate navigation properties 
	string childSql = "...more stuff I did to create this for a navigation property...";
	string navigationPropertyName = (string)otherParameters[0];
	IEntityType entityType = dbContext.Model.FindEntityType(typeof(TEntity));
	var navigationProperties = entityType.GetDeclaredNavigations();
	var navigationProperty = navigationProperties.Single(p => p.Name == navigationPropertyName);
	var navigationEntityType = navigationProperty.GetTargetType();
			
	// I wrote an extension method to Get the DbSet as an IQueryable.  
        // It was the best I could do...
	var childDbSet = dbContext.GetDbSetForClrType(navigationEntityType.ClrType);

	// But even with this, all the extension methods defined in 
        // RelationalQueryableExtensions extend a strongly typed to a DbSet<>
	var childResults = childDbSet.FromSqlRaw(sql);  // <-- Not currently possible

}

private static IQueryable GetDbSetForClrType(this DbContext dbContext, Type clrType)
{
	Type DbSetType = typeof(DbSet<>);

	var propertyInfo = dbContext.GetType()
			.GetProperties(BindingFlags.Public | BindingFlags.Instance)
			.Select(p => new { PropertyInfo = p, PropertyType = p.PropertyType })
			.First(t => 
				t.PropertyType.IsGenericType && 
				t.PropertyType.GetGenericTypeDefinition() == DbSetType && 
				t.PropertyType.GetGenericArguments()[0] == clrType)
			.PropertyInfo;
	return propertyInfo.GetValue(dbContext) as IQueryable;
}

I want this because I wrote my own Uri query string parser from which I use certain information to decide which parts of a DbContext to hydrate. (Consider for example, the $expand operator that OData uses. Not only is the main entity queried, but the related entities are queried as well.)
If I want more control of the SQL generated, some solution to the above would work best.

Would it be possible to add similar methods in RelationalQueryableExtensions without the strong typing?

For example:

        [StringFormatMethod("sql")]
        public static IQueryable FromSqlRaw(
           [NotNull] this IQueryable source,
           [NotNull] [NotParameterized] string sql,
           [NotNull] params object[] parameters)
        {
            Check.NotNull(source, nameof(source));
            Check.NotEmpty(sql, nameof(sql));
            Check.NotNull(parameters, nameof(parameters));

            var queryableSource = (IQueryable)source;
            return queryableSource.Provider.CreateQuery(
                GenerateFromSqlQueryRoot(
                    queryableSource,
                    sql,
                    parameters));
        }

mitselplik avatar May 28 '20 07:05 mitselplik

I no longer need this. I devised a clever hack to create type-free calls to extension methods like this:

/// <summary>
/// A Type-free version of DbSet.FromSqlRaw
/// </summary>
/// <param name="source">A DbSet disguised as a generic IQueryable.</param>
/// <param name="sql">The sql to execute.</param>
/// <param name="parameters">The sql parameters to use.</param>
/// <returns></returns>
public static IQueryable FromSqlRaw(this IQueryable source, string sql, params object[] parameters)
{
	var methodInfo = MethodInfo
		.GetCurrentMethod().DeclaringType
		.GetMethods(BindingFlags.NonPublic | BindingFlags.Static)
		.First(m => m.Name == "Call_DbSet_FromSqlRaw");
	var genericMethod = methodInfo.MakeGenericMethod(source.ElementType);

	object[] newParameters = (parameters == null)
		? new object[] { source, sql }
		: new object[] { source, sql, parameters };
	
	return genericMethodInfo.Invoke(source, newParameters) as IQueryable;
}

/// <summary>
/// Wrapper for an extension method I can't otherwise call without a specific type.
/// This is *only* called from the above version of FromSqlRaw through use of a MethodInfo
/// </summary>
private static IQueryable Call_DbSet_FromSqlRaw<TEntity>(DbSet<TEntity> source, string sql, params object[] parameters)
	where TEntity : class
	=> RelationalQueryableExtensions.FromSqlRaw(source, sql, parameters);

mitselplik avatar May 29 '20 02:05 mitselplik

If you make GetDbSetForClrType generic on TEntity and invoke it like how you are doing to invoke Call_DbSet_FromSqlRaw you wouldn't need another method.

smitpatel avatar May 29 '20 05:05 smitpatel

Further, GetDbSetForClrType can just be replaced to invoke DbContext.Set<TEntity>() method.

smitpatel avatar May 29 '20 05:05 smitpatel

Because of bugs in either OData or EF Core or Automapper (the latest was in EF Core - the SQL it generates is buggy and doesn't work), I was being roadblocked while developing an n-tier API I have been writing for work. So I wrote my own Uri query string parser and replaced OData entirely. I use it to:

  1. Parse a Uri query string
  2. Generate working SQL statement(s)
  3. Execute the SQL statement(s) against the appropriate DbContext(s) using calls to FromSqlRaw
  4. Iterate the EntityQueryable and attach the objects returned to my DbContext to hydrate it
  5. Iterate over the root DbSet (and any descendent DbSets requested through $expand), transforming the results into a Dictionary<string, object> that is returned to the Controller method.

It all works beautifully! The JSON serializer handles the Dictionary<string, object> and transforms the payload into results that are identical to OData's output. And I did all that with just a couple thousand lines of code. If I ever decide to publish the library, the guys who prefer to use Dapper are going to love me :-)

In any case, to answer your question specifically, Imagine a controller method to handle a Uri like this: ~/odata/Suppliers?$expand=Parts,DistributionCenters($expand=Address,Contacts)

Using a DbContext directly with OData, the method might look something like this:

[EnableQuery]
public IQueryable<Supplier> Get() => dbContext.Supplier;

At design time, the only known type here is Supplier. I have no idea that I am going to need a DbSet for Part, DistributionCenter, Address, or Contact until run-time when I parse the query string and determine that. EF Core allows me to determine the ClrType for navigation properties though, so I use the aforementioned functions to do the following (queryInfo is an internal class I have that holds information determined from parsing the query string):

private static IQueryable ExecuteQueries(DbContext dbContext, 
    QueryInfo queryInfo, params QueryInfo[] ancestors)
{
	List<QueryInfo> queryInfos = new List<QueryInfo>();

	string sql = null;
	if (ancestors != null && ancestors.Length > 0)
	{
		queryInfos.AddRange(ancestors);
		queryInfos.Add(queryInfo);
		sql = BuildSqlQuery(queryInfos.ToArray());
	}
	else
	{
		queryInfos.Add(queryInfo);
		sql = BuildSqlQuery(queryInfo);
	}

	Debug.WriteLine("------------------------------------------------------------");
	Debug.WriteLine(sql);
	Debug.WriteLine("------------------------------------------------------------");
			
	IQueryable entitySet = GetDbSetForClrType(dbContext, queryInfo.ClrType)
		.FromSqlRaw(sql,
			new SqlParameter($"@{queryInfo.TableAlias}_Skip", queryInfo.Skip),
			new SqlParameter($"@{queryInfo.TableAlias}_Top", queryInfo.Top));

	// Eager load descendant data so that navigation properties are correctly hydrated
	// when we finally iterate over the root DbSet.
	// Do not iterate the parent most DbSet (where ancestors is null).  
	// Wait to do that until last or the database gets hit with an extra query 
	// when iterating over the DbSet in WalkTree().
	if (ancestors != null && ancestors.Length > 0)
	{
		List<object> data = new List<object>();
		foreach (var e in entitySet) data.Add(e);
		dbContext.AttachRange(data);
	}
			
	foreach (var child in queryInfo.Expansions)
	{
		IQueryable childQueryable = 
				ExecuteQueries(dbContext, child.Value, queryInfos.ToArray());
	}

	return entitySet;
}

As you can see, there is no TEntity type parameter to work with. I have to be able to retrieve the DbSets and execute the FromSqlRaw function at run-time knowing only the CLR Type of my navigation properties.

mitselplik avatar May 29 '20 16:05 mitselplik

GetDbSetForClrType(dbContext, queryInfo.ClrType)

The ClrType of your navigation is the type of TEntity. DbContext.Set<TEntity> API will return DbSet for the entity type which has backing clr type of TEntity

smitpatel avatar May 29 '20 16:05 smitpatel

No sir, that is only true for the root most DbSet<> - <TEntity> is of Type <Supplier> only in that case. (Notice that there is only place in all that code above that a <TEntity> of a specified type is known and coded?)

The ClrType of the Navigation properties is <Part>, <DistributionCenter>, <Address>, and <Contact>.

How would you propose I obtain the DbSet<> for those entities at run-time when the only specified type for <TEntity> in the code is the <Supplier> parameter type from the Controller's Get() method?

No means exists to obtain a DbSet<> from a DbContext by name or run-time type information, unlike with EF 6. (See DbContext.Set Method). I guess some person somewhere writing the EF Core code decided that this method was of little or no value and removed it from EF Core (or never added it maybe). I'll keep my opinions about that decision to myself :-)

mitselplik avatar May 29 '20 17:05 mitselplik

FWIW, the EF Core bug I mentioned can rather easily be reproduced.

  1. Create a database with two tables
create table AppraisalYear
(
    tax_yr decimal(4,0) not null,
    certification_dt datetime,
    constraint pk_AppraisalYear 
		primary key clustered (tax_yr)
)

create table PropertyYearLayer
(
	owner_tax_yr decimal(4,0) not null,
	sup_num int not null,
	prop_id int not null,
	constraint pk_PropertyYearLayer 
		primary key clustered (owner_tax_yr, sup_num, prop_id),
	constraint fk_PropertyYearLayer_AppraisalYear
		foreign key (owner_tax_yr)
	references AppraisalYear(tax_yr)
)
  1. Add some data
  2. Scaffold the database, then modify the resulting classes and DbContext to make properties more readable and consistent (apologies, I am dealing with an existing legacy database myself - so I know this all seems a little odd. Oh - and I wrote my own scaffolding tool that is visual rather than command line driven, and it has some translation steps used internally to mitigate some of the legacy naming convention issues, So the structure of my code is slightly different than the command-line version.)
public partial class PropertyDbContext : DbContext
{
	#region Constructor(s)

	public PropertyDbContext() { }
	public PropertyDbContext(DbContextOptions<PropertyDbContext> options)
		: base(options)
	{ }

	#endregion

	#region DbSet<> Properties

	public virtual DbSet<AppraisalYear> AppraisalYear { get; set; }
	public virtual DbSet<PropertyYearLayer> PropertyYearLayer { get; set; }

	#endregion

	protected override void OnModelCreating(ModelBuilder modelBuilder)
	{
		modelBuilder.Entity<AppraisalYear>(OnModelCreating_AppraisalYear);
		modelBuilder.Entity<PropertyYearLayer>(OnModelCreating_PropertyYearLayer);
	}

	#endregion

	#region Fluent API Methods

	private void OnModelCreating_AppraisalYear(EntityTypeBuilder<AppraisalYear> entity)
	{
		entity.HasKey(e => new { e.Year });
		entity.ToTable("AppraisalYear");

		entity.Property(e => e.Year).HasColumnName("tax_yr")
			.IsRequired()
			.HasColumnType("numeric(4,0)");

		entity.Property(e => e.CertificationDate).HasColumnName("certification_dt")
			.HasColumnType("datetime");
	}

	private void OnModelCreating_PropertyYearLayer(EntityTypeBuilder<PropertyYearLayer> entity)
	{
		entity.HasKey(e => new { e.Year, e.SupNumber, e.PropertyId });

		entity.ToTable("PropertyYearLayer");

		entity.Property(e => e.Year).HasColumnName("owner_tax_yr")
			.IsRequired()
			.HasColumnType("numeric(4,0)");

		entity.Property(e => e.SupNumber).HasColumnName("sup_num")
			.IsRequired();

		entity.Property(e => e.PropertyId).HasColumnName("prop_id")
			.IsRequired();

		entity.HasOne(d => d.AppraisalYear)
			.WithMany(p => p.PropertyYearLayer)
			.HasForeignKey(d => d.Year);
	}

	#endregion
}
  1. Create an OData based Web API, then using Postman, attempt the following uri to get the top 5 PropertyYearLayers per AppraisalYear: ~/odata/AppraisalYear?$orderby=Year desc&$expand=PropertyYearLayer($top=5)

The result is an ugly error:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> Microsoft.Data.SqlClient.SqlException (0x80131904): Invalid column name 'owner_tax_yr'.
Invalid column name 'sup_num'.
Invalid column name 'prop_id'.
Invalid column name 'prop_id'.
Invalid column name 'sup_num'.
Invalid column name 'owner_tax_yr'.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
. . . etc . . .

That is stemming from a bug in the SQL that EF Core generated (I ran SQL Profiler to determine this):

exec sp_executesql N'SELECT [t].[tax_yr], [t].[certification_dt], [t1].[c], [t1].[owner_tax_yr], [t1].[sup_num], [t1].[prop_id], [t1].[c0]
FROM (
    SELECT TOP(@__TypedProperty_4) [a].[tax_yr], [a].[certification_dt] 
    FROM [PACSNext].[Appraisal_Year] AS [a]
    ORDER BY [a].[tax_yr] DESC
) AS [t]
OUTER APPLY (
    SELECT TOP(@__TypedProperty_2) @__TypedProperty_3 AS [c], [t].[owner_tax_yr], [t].[sup_num], [t].[prop_id], CAST(1 AS bit) AS [c0]
    FROM (
        SELECT TOP(@__TypedProperty_1) [p].[owner_tax_yr], [p].[sup_num], [p].[prop_id]
        FROM [PACSNext].[Property_Year_Layer] AS [p]
        WHERE [t].[tax_yr] = [p].[owner_tax_yr]
        ORDER BY [p].[prop_id], [p].[sup_num], [p].[owner_tax_yr]
    ) AS [t0]
    ORDER BY [t].[prop_id], [t].[sup_num], [t].[owner_tax_yr]
) AS [t1]
ORDER BY [t].[tax_yr] DESC, [t1].[prop_id], [t1].[sup_num], [t1].[owner_tax_yr]',N'@__TypedProperty_4 int,@__TypedProperty_2 int,@__TypedProperty_3 nvarchar(4000),@__TypedProperty_1 int',@__TypedProperty_4=101,@__TypedProperty_2=101,@__TypedProperty_3=N'cd7f27e6-36d1-4190-91de-871916fd4c42',@__TypedProperty_1=5

The problem is the [t0] alias. Change the [t] to [t0] in the select and order by clauses of the inner query of the outer apply and the statement works. I should report the issue I guess...

mitselplik avatar May 29 '20 17:05 mitselplik

How would you propose I obtain the DbSet<> for those entities at run-time when the only specified type for <TEntity> in the code is the <Supplier> parameter type from the Controller's Get() method?

If you extract method info of following method, using MakeGenericMethod with whatever runtime clr type you get and invoking it will give DbSet for it. You are already doing that kind of thing for FromSql. https://github.com/dotnet/efcore/blob/95e779b5b623c71f74bad7bcd7b98723d4892cf6/src/EFCore/DbContext.cs#L283-L284

smitpatel avatar May 29 '20 18:05 smitpatel

That makes sense. I realize now that my GetDbSetForClrType(dbContext, queryInfo.ClrType) function would only provide access to declared properties, while your approach would grant access to shadow properties as well.

In any case, I hope this discussion has demonstrated that there are actually two needs for the current framework.

  1. I type-less generic interface IDbSet : IQueryable, IEnumerable, IListSource { } should exist from which DbSet<> inherits.
  2. Methods like FromSqlRaw that don't actually need the value type should be coded against the proposed IDbSet instead.

mitselplik avatar May 29 '20 19:05 mitselplik

We discussed this again, but came to the same conclusion as before. Since LINQ is inherently generic for anything other than the most trivial case, we don't want to lead people down the path of trying to do things with a non--generic DbSet. If you're willing to handle the dynamic LINQ code, then calling CreateGenericMethod to create a DbSet with the generic API should not be difficult.

All that being said, I'm putting this on the backlog to gather votes and feedback.

ajcvickers avatar Jun 01 '20 21:06 ajcvickers

Just a note on not being able to do much with a non-generic IQueryable: because the T in IQueryable<T> is covariant, in some situations, where you know the IQueryable element type is derived from a certain unmapped base type, you can cast to IQueryable<BaseType>and then do regular linq operations on those base properties without resorting to dynamic linq. Edit: forgot to add backticks for the angle brackets...

stevendarby avatar Oct 28 '20 23:10 stevendarby

I need this feature, in my situation, I have to declare many validators like

public class ValidDepartmentIdAttribute: ValidationAttribute
{
    protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
    {
        if (value is not Guid id)
        {
            return ValidationResult.Success;
        }
        var departmentBusiness = validationContext.GetTypedService<IDepartmentBusiness>();
        if (!departmentBusiness.ExistById(id).Result)
        {
            return new ValidationResult($"Element {id} can't be found");
        }

        return ValidationResult.Success;
    }
}

And I have to repeat this for all entity classes. Since attribute class doesn't support generic yet, I suppose I can write something like this one so it can be reused.

public class ValidEntityIdAttribute: ValidationAttribute
{
    public Type Type { get; set; }
    
    public ValidEntityIdAttribute(Type type)
    {
        Type = type;
    }

    protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
    {
        if (value is not Guid id)
        {
            return ValidationResult.Success;
        }
        
        var dataContext = validationContext.GetTypedService<DataContext>();
        if (dataContext.Set(Type).Find(id) == null)
        {
            return new ValidationResult($"Element {id} can't be found");
        }

        return ValidationResult.Success;
    }
}

But then I found dataContext.Set(Type). is not available on EFCore.

So, I hope this method will be implemented soon.

Thank you

voquanghoa avatar Nov 07 '21 12:11 voquanghoa

a nongeneric dbset instantiation mechanism would help out in savechanges interceptors.

such an api surface would support enforcement of business rules that require querying navigation properties of heterogeneous entity types in the change tracker during savechanges

one workaround i'm canary testing is

var ownerEntry = entry.Collection("Owners").FindEntry(principal);

however even if this works and lets me filter for owners of an arbitrary entity

  • FindEntry does not yield a composable query
  • findentry would not permit elegant sql materialization of a lookup from a list

so this is a vote for at minimum, an extension method that

  • introspects a given dbcontext for a given entity set
  • exposes the dbset as a typed iqueryable to permit Func<IMyEntityInterface, bool> composition
  • alternately exposes the dbset as a nongeneric IQueryable in case the previous bullet point has no solutions

obviously, if the aforementioned extension method were pasted into a comment on this issue the party responsible would instantly attain guru status everywhere on the internet

thanks for leaving the matter open to further consideration

vigouredelaruse avatar Apr 20 '24 20:04 vigouredelaruse