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

Add support for FindByAlternateKey. #10303

Closed
john-t-white opened this issue Nov 15, 2017 · 5 comments
Closed

Add support for FindByAlternateKey. #10303

john-t-white opened this issue Nov 15, 2017 · 5 comments

Comments

@john-t-white
Copy link

john-t-white commented Nov 15, 2017

Would it be possible to add FindByAlternateKey and FindByAlternateKeyAsync methods to DbSet. While the find by primary key is nice, it would be great to also be able to have the same functionality with alternate keys. Here is some code that works for me, but it has the restriction on only one alternate key.

public static class DbSetExtensions
		{
			private static readonly MethodInfo _efPropertyMethod = typeof( EF ).GetTypeInfo().GetDeclaredMethod( nameof( EF.Property ) );

		public static TEntity FindByAlternateKey<TEntity>( this DbSet<TEntity> dbSet, params object[] keyValues )
			where TEntity : class
		{
			if ( dbSet == null )
			{
				throw new ArgumentNullException( nameof( dbSet ) );
			}

			if ( keyValues == null )
			{
				throw new ArgumentNullException( nameof( keyValues ) );
			}

			IReadOnlyList<IProperty> keyProperties;
			return DbSetExtensions.FindTrackedEntity( dbSet, keyValues, out keyProperties )
				?? dbSet.FirstOrDefault( DbSetExtensions.BuildPredicate<TEntity>( keyProperties, keyValues ) );
		}

		public static Task<TEntity> FindByAlternateKeyAsync<TEntity>( this DbSet<TEntity> dbSet, params object[] keyValues )
			where TEntity : class
		{
			if ( dbSet == null )
			{
				throw new ArgumentNullException( nameof( dbSet ) );
			}

			if ( keyValues == null )
			{
				throw new ArgumentNullException( nameof( keyValues ) );
			}

			IReadOnlyList<IProperty> keyProperties;
			TEntity trackedEntity = DbSetExtensions.FindTrackedEntity( dbSet, keyValues, out keyProperties );

			return trackedEntity != null
				   ? Task.FromResult( trackedEntity )
				   : dbSet.FirstOrDefaultAsync( DbSetExtensions.BuildPredicate<TEntity>( keyProperties, keyValues ) );
		}

		private static TEntity FindTrackedEntity<TEntity>( DbSet<TEntity> dbSet, object[] keyValues, out IReadOnlyList<IProperty> keyProperties )
			where TEntity : class
		{
			Type entityType = typeof( TEntity );

			IServiceProvider serviceProvider = ( ( IInfrastructure<IServiceProvider> )dbSet ).Instance;
			IDbContextServices dbContextServices = serviceProvider.GetService<IDbContextServices>();

			DbContext dbContext = dbContextServices.CurrentContext.Context;
			IEntityType contextEntityType = dbContext.Model.FindEntityType( entityType );
			IEnumerable<IKey> alternateKeys = contextEntityType.GetKeys().Where( x => !x.IsPrimaryKey() );

			int numberOfAlternateKeys = alternateKeys.Count();
			if ( numberOfAlternateKeys == 0 )
			{
				throw new InvalidOperationException( $"There is no alternate key setup for '{ entityType.FullName }'." );
			}

			if ( numberOfAlternateKeys > 1 )
			{
				throw new InvalidOperationException( $"There are more than one alternate key setup for '{ entityType.FullName }'." );
			}

			IKey alternateKey = alternateKeys.First();
			keyProperties = alternateKey.Properties;

			IStateManager stateManager = serviceProvider.GetService<IStateManager>();

			return stateManager.TryGetEntry( alternateKey, keyValues )?.Entity as TEntity;
		}

		private static Expression<Func<TEntity, bool>> BuildPredicate<TEntity>( IReadOnlyList<IProperty> keyProperties, object[] keyValues )
		{
			var entityParameter = Expression.Parameter( typeof( TEntity ), "e" );

			BinaryExpression predicate = null;
			for ( var i = 0; i < keyValues.Length; i++ )
			{
				var property = keyProperties[i];
				var equals =
					Expression.Equal(
						Expression.Call(
							_efPropertyMethod.MakeGenericMethod( property.ClrType ),
							entityParameter,
							Expression.Constant( property.Name, typeof( string ) ) ),
						Expression.Constant(
							keyValues[i], property.ClrType ) );

				predicate = predicate == null ? equals : Expression.AndAlso( predicate, equals );
			}

			return Expression.Lambda<Func<TEntity, bool>>( predicate, entityParameter );
		}
	}
@ajcvickers
Copy link
Member

@john-t-white Can you explain a bit more about how this would be used? Also, when there are multiple alternate keys, how is one chosen over the others?

(Our inclination here is to close this because it doesn't add much value over just creating a normal LINQ query, but we would like to understand if we're missing some scenario.)

@john-t-white
Copy link
Author

john-t-white commented Nov 21, 2017

Sorry it took so long for my response.

The scenario I have is that we send out notifications to our users. When an event happens, multiple notifications can be sent out and we batch those together in one request to our notification service to keep it less chatty. Each notification request in a batch specifies an identifier for the type of notification it needs to send along with data for that specific notification type. There is only a finite number of notification types so a batch usually contains the same notification type multiple times.

The reason for the Find version is to reduce the number of calls to the database. If the notification type is already loaded into the DbContext, it doesn't need to hit the database again during the same session. This reduced a lot of extra calls because the same information about a notification type is always returned. We did the same thing initially with a dictionary, but it felt very much like how Find works in Entity Framework. The difference is it is based on an alternate key, not the primary key. Since this is a separate service that lives outside of our main application, we do not share what the primary key is to outside applications. This also allows us to change the identifier later on to meet our needs without impacting the database relationships.

Since our service only has one alternate key per entity, I didn't need to figure that out at the time. But as a feature in Entity Framework, that would obviously have to be accounted for. Here are a couple of thoughts:

  1. The alternate key must have a name specified during the configuration, which you would pass into the method.
public TEntity FindByAlternateKey<TEntity>( this DbSet<TEntity> dbSet, string alternateKeyName, params object[] values )
  1. Be able to pass an array of property expressions which represent the properties used as an alternate key.
public TEntity FindByAlternateKey<TEntity>( this DbSet<TEntity> dbSet, Expression<Func<TEntity, object>>[] alternateKeyProperties, object[] values )

With something like that in place, I can easily add my own extension methods on top of my specific DbSet that would abstract it away and make it easier to call.

public NotificationType FindByIdentifier( this DbSet<NotificationType> dbSet, string identifier )
{
  return dbSet.FindByAlternateKey( "AlternateKeyName", identifier );

  /* OR */

  return dbSet.FindByAlternateKey( new [] { x => x.Identifier }, identifier );
}

And of course having the async versions as well.

Thoughts?

@ajcvickers
Copy link
Member

@john-t-white Thanks for the detailed response. It looks like what you want would probably be covered by #7391, the features discussed there work with alternate keys as well as primary keys, which was my intention.

@john-t-white
Copy link
Author

Awesome!

Is this something that is on the roadmap at this point?

@ajcvickers
Copy link
Member

@john-t-white #7391 is something I would love to work on, but realistically I don't know if it will make the next release since there are other higher priority items we need to do. I may get a bit of free time to look at it over the holidays.

Closing this for now, since I think #7391 covers it.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants