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

Find by primary key using named/typed parameters #30587

Closed
dvoreckyaa opened this issue Mar 27, 2023 · 4 comments
Closed

Find by primary key using named/typed parameters #30587

dvoreckyaa opened this issue Mar 27, 2023 · 4 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@dvoreckyaa
Copy link

dvoreckyaa commented Mar 27, 2023

Hi all,
we have possibility to find an entity usig Find methdod

public virtual TEntity? Find(params object?[]? keyValues)

where we have to pass all paramters as object array. For me it seems a bit inconvenient and can lead to mistakes.
I would suggest improving this by adding FindByPrimaryKey methods that will accept named and typed parameters.
Maybe this functionality is already implemented but I could not find.

public static Db.Models.ItsDetail FindByPrimaryKey(this DbSet<Models.ItsDetail> entities, Int32 detWstagNo, Int32 detNo)
{
    return entities.Find(detWstagNo, detNo);
}

Actually, a few years ago I implemented a generator that creates an extension for a DbSet to support FindByPK with typed parameters.

https://github.com/dvoreckyaa/FindByPKGenerator

@ajcvickers
Copy link
Member

@dvoreckyaa Do you have a general purpose API proposal here? The examples above all seem specific to certain entity type with known key configurations.

@dvoreckyaa
Copy link
Author

dvoreckyaa commented Mar 28, 2023

@ajcvickers , not it is not specific to certain entity type. The mentioned generator takes a DbContext (should be inherited fom DbContext) then

  1. Calls GetEntityTypes (current implementations uses UseInMemoryDatabase to create the instance of the DbContext)
  2. Lists all selected object types and calls FindPrimaryKey() for each type.
  3. Based on the found PK, creates FindByPK and FindByPKAsync using tt.
public void GenerateFileFromType<T>(string outputFolder) where T : DbContext

The PK is obtained from the model definition:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
   ...
   #region Localization
   modelBuilder.Entity<Localization>()
                     .HasKey(b => new { b.ApplicationID, b.Culture, b.Key });
   #endregion
  ...
}

@roji
Copy link
Member

roji commented Mar 28, 2023

@dvoreckyaa if I understand your proposal correctly, this would be a source generator provided by EF Core, which generates Find extension methods over concrete DbSet<T>, accepting the specific primary key property (or property types). For example, if I have a Blog entity type with a Guid key, the source generator would generate an extension method over DbSet<Blog> which accepts a single Guid.

A source generator like this would need to access the user's EF model, in order to know about entities and their primary key properties. There are unfortunately various problems that make it unfeasible for a source generator to know about the model, see #25009 (comment) for more details.

In any case, we've rarely seen any actual user complaints about bugs because of this, and it's fairly trivial for users to add their own extensions manually, if they so wish. There's also the option to just use a LINQ query to load an entity type from the database (i.e. ctx.Blogs.Single(b => b.Id == id)), which is fully strongly-typed and safe. This doesn't first check the change tracker like Find, though that can also be done by accessing ChangeTracker.Entries.

To summarize, in an ideal world something like this would indeed exist; but the significant issues in actually doing this, coupled with the relative low value and alternative methods for achieving almost the same, make this pretty low value in my opinion.

@dvoreckyaa
Copy link
Author

@roji, it sounds reasonable, thanks for the clarification.

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Mar 29, 2023
@ajcvickers ajcvickers added closed-no-further-action The issue is closed and no further action is planned. and removed type-enhancement labels Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants