dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.78k stars 3.19k forks source link

Support saving no-setter properties (for example expression bodied properties) #2319

Open MatthewMWR opened 9 years ago

MatthewMWR commented 9 years ago

Note: updated to be a clear feature request based on thread discussion. Thanks!

There are good use cases for storing get-only properties (e.g., for BI tools, JSON based data feeds, and other non-EF consumers), even when those have no role in materialization. Consider this fact oriented entity class describing a signal from a sensor:

public class Blip 
{
    public int OldState {get; set;}
    public int NewState{get;set;}
    // implying enum here but could be any EF compatible type
    public SensorAction SensorActionDetected=> Domain logic expression which resolves meaningful SensorActionDetected from raw sensor values OldState and NewState
}

After much experimentation the only reliable way I've been able to map such properties (in beta 4) is to lie and create a false setter so EF thinks it can materialize this property, such as:

    public SensorAction SensorActionDetected{ get{ return logic here} set{ ignore value } }

This implementation feels like really dirtying the domain class for the sake of EF compatibility. Future users of my data model would likely be baffled but the no-op setter. Another option would be to make SensorActionDetected a simple auto-property without a custom getter, and set the value elsewhere, like in the set method of NewState, but this falls apart as the number of properties used in a calculation grows, you have to special case every other property setter if you can't control what order they will be populated in. Another option would be to not include SensorActionDetected in the class and instead calculate it elsewhere such as inside SQL Server, but then you need parallel domain expert implementations of the SensorActionDetected logic everywhere Blip goes (SQL, JSON, BI Tools, etc. etc.) which is not desirable.

Please support the ability to save no-setter properties. Thanks.

MatthewMWR commented 9 years ago

Note that there are several issues open related to read-only properties (list below). I don't mean to duplicate those, but rather ask what the intended/suggested pattern is here and if this isn't supported today to consider it for the future.

https://github.com/aspnet/EntityFramework/issues/1527 https://github.com/aspnet/EntityFramework/issues/1311 https://github.com/aspnet/EntityFramework/issues/1526 https://github.com/aspnet/EntityFramework/issues/2318

rowanmiller commented 9 years ago

@MatthewMWR we aren't intending to support this scenario in the initial release (though we do think it would be good to support). The closest you could get with EF Core (or EF6) is to have a private setter that doesn't do anything.

Generally when we talk about "read-only" we are referring to something that can be set/changed before inserting the record, but once saved to the database it cannot be changed.

MatthewMWR commented 9 years ago

Updated main post title and text for clarity as a future feature request. Thanks.

MatthewMWR commented 9 years ago

Edit with example for clarity. Would it be reasonable to accomplish this via explicit use of shadow properties in model building... or does this violate some assumptions about shadows? Example:

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            using(var r = new BlogRepo())
            {
                r.Add(new Blog()
                {
                    Name = "ThisName",
                    Visibility = Visibility.Private
                });
                r.SaveChanges();
            }
            using (var r = new BlogRepo())
            {
                Console.WriteLine(r.Blogs.First().VisibilityLabel);
            }
        }
    }

    public enum Visibility
    {
        Hidden = 0,
        Private,
        Public
    }

    public class Blog
    {
        public string Name { get; set; }
        public Visibility Visibility { get; set; }
        public string VisibilityLabel => Visibility.ToString();
    }

    public class BlogRepo : DbContext
    {
        public DbSet<Blog> Blogs { get; set; } 

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer("server=(localdb)\\MsSqlLocalDb;database=ShadowTest");
        }

        protected override void OnModelCreating(ModelBuilder mb)
        {
            mb.Entity<Blog>( eb =>
            {
                eb.Property<int>("Id").UseSqlServerIdentityColumn();
                eb.Key("Id");
                eb.Property<string>("VisibilityLabel").Metadata.IsShadowProperty = true;
            });
        }

        public override int SaveChanges()
        {
            foreach (var entry in ChangeTracker.Entries<Blog>())
            {
                foreach (var pi in typeof (Blog).GetProperties())
                {
                    if (pi.GetSetMethod() == null)
                    {
                        entry.Property(pi.Name).CurrentValue = pi.GetValue(entry.Entity);
                    }
                }
            }
            return base.SaveChanges();
        }
    }
}
MatthewMWR commented 8 years ago

Note that the workaround shown above (explicitly create and manage shadow properties to represent get-only property values in database) no longer works in 1.0.0. Metadata.IsShadowProperty is now read only. Does this mean a caller should not try to create shadow properties for this scenario, or just that making a shadow property needs to happen somewhere else?

AndriySvyryd commented 5 years ago

@ajcvickers Is this fixed now with backing field support and PropertyAccessMode?

ajcvickers commented 5 years ago

@AndriySvyryd Not as described here. The expression bodied property is not backed by a field.

wojtek-viirtue commented 2 years ago

+1 on this, it's not clear how we would persist a property with an expression body or work around it in EFCore 6.

MichielDP commented 3 months ago

@rowanmiller @ajcvickers Any news regarding this feature request? This would be a very valueable feature in my opinion.

We could extract some common pieces of logic into expression-boddied members, which even reference each other. And then write linq queries using these.

Your suggestion of having a private setter that doesn't do anything throws the error "Invalid column {xyz}" on these properties (EF 8).

Here's a stripped down example of how I would like to use this feature:

internal class InvoiceTask : AggregateRoot
{
    public InvoiceTaskType Type { get; private set; }

    public ICollection<InvoiceTaskStatusChange> StatusChanges { get; private set; } = [];

    private InvoiceTaskStatusChange LastStatusChange => StatusChanges.OrderBy(x => x.ChangedOn).Last();
    public InvoiceTaskStatus Status => LastStatusChange.Status;

    public bool IsPending   => Status == InvoiceTaskStatus.Pending;
    public bool IsInProcess => Status == InvoiceTaskStatus.InProcess;
    public bool IsCompleted => Status == InvoiceTaskStatus.Completed;
    public bool IsFailed    => Status == InvoiceTaskStatus.Failed;
    public bool IsProcessed => IsCompleted || IsFailed;

    public DateTime EnqueuedOn => StatusChanges.First(x => x.Status == InvoiceTaskStatus.Pending).ChangedOn;

    [...]
}

With the goal of using any of these expression-boddied members in linq queries, for example:

List<PendingInvoiceTask> pendingTasks = await _dbContext
    .InvoiceTasks
    .Where(x => x.IsPending)
    .OrderByDescending(x => x.EnqueuedOn)
    .Take(query.Take)
    .Select(x => new PendingInvoiceTask(x.Id, x.Type))
    .ToListAsync(cancellationToken);
ajcvickers commented 3 months ago

This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 9.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you. This issue currently has no votes.