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

Feature request: read only entities #7586

Open
Tracked by #22954
Opiumtm opened this issue Feb 10, 2017 · 25 comments
Open
Tracked by #22954

Feature request: read only entities #7586

Opiumtm opened this issue Feb 10, 2017 · 25 comments

Comments

@Opiumtm
Copy link

Opiumtm commented Feb 10, 2017

Note that entities without keys (previously known as query types) are already read only. We keep this issue to track allowing configuring an entity type to be read-only even if it has a key and we could in theory allow CUD operations.


Provide support for read-only entities.
Read-only entities should fail with exception on insert, delete or update at Entity Framework level.
They would be added and updated at database externally or populated on data base initial setup.

Scenario: API contract for another project team with guarantee that code developed by another team would never change some database entities. Also it's additional layer of protection against bugs.
Some critical information should be changed only by specialized code or directly via SQL, but should never be changed from common application code even in case of bug or security vulnerability exploit.

@rowanmiller
Copy link
Contributor

Putting this on the backlog as something we may implement. In the meantime, you can do it pretty easily yourself.

Define some extensions over metadata:

public static class Extensions
{
    private static readonly string READONLY_ANNOTATION = "custom:readonly";

    public static EntityTypeBuilder<TEntity> IsReadOnly<TEntity>(this EntityTypeBuilder<TEntity> builder)
        where TEntity : class
    {
        builder.HasAnnotation(READONLY_ANNOTATION, true);

        return builder;
    }

    public static bool IsReadOnly(this IEntityType entity)
    {
        var annotation = entity.FindAnnotation(READONLY_ANNOTATION);
        if(annotation != null)
        {
            return (bool)annotation.Value;
        }

        return false;
    }
}

And then override SaveChanges in the context:

public override int SaveChanges()
{
    var errors = ChangeTracker
        .Entries()
        .Where(e => e.Metadata.IsReadOnly() && e.State != EntityState.Unchanged)
        .Select(e => e.Metadata.Name)
        .Distinct()
        .ToList();

    if(errors.Any())
    {
        throw new InvalidOperationException(
            $"Attempted to save read-only entities {string.Join(",", errors)}");
    }

    return base.SaveChanges();
}

Here is a console app that wires it all together:

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Sample
{
    class Program
    {
        static void Main(string[] args)
        {
            using (var db = new BloggingContext())
            {
                db.Database.EnsureCreated();

                db.Blogs.Add(new Blog { Url = "http://blogs.msdn.com/adonet" });
                var count = db.SaveChanges();
                Console.WriteLine("{0} records saved to database", count);
            }
        }
    }

    public class BloggingContext : DbContext
    {
        public DbSet<Blog> Blogs { get; set; }
        public DbSet<Post> Posts { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Sample;Trusted_Connection=True;");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Blog>().IsReadOnly();
        }

        public override int SaveChanges()
        {
            var errors = ChangeTracker
                .Entries()
                .Where(e => e.Metadata.IsReadOnly() && e.State != EntityState.Unchanged)
                .Select(e => e.Metadata.Name)
                .Distinct()
                .ToList();

            if(errors.Any())
            {
                throw new InvalidOperationException(
                    $"Attempted to save read-only entities {string.Join(",", errors)}");
            }

            return base.SaveChanges();
        }
    }

    public static class Extensions
    {
        private static readonly string READONLY_ANNOTATION = "custom:readonly";

        public static EntityTypeBuilder<TEntity> IsReadOnly<TEntity>(this EntityTypeBuilder<TEntity> builder)
            where TEntity : class
        {
            builder.HasAnnotation(READONLY_ANNOTATION, true);

            return builder;
        }

        public static bool IsReadOnly(this IEntityType entity)
        {
            var annotation = entity.FindAnnotation(READONLY_ANNOTATION);
            if(annotation != null)
            {
                return (bool)annotation.Value;
            }

            return false;
        }
    }

    public class Blog
    {
        public int BlogId { get; set; }
        public string Url { get; set; }

        public List<Post> Posts { get; set; }
    }

    public class Post
    {
        public int PostId { get; set; }
        public string Title { get; set; }
        public string Content { get; set; }

        public int BlogId { get; set; }
        public Blog Blog { get; set; }
    }
}

@rowanmiller rowanmiller added this to the Backlog milestone Feb 14, 2017
@rowanmiller
Copy link
Contributor

@ajcvickers
Copy link
Member

Also consider immutable types here.

@davisnw
Copy link

davisnw commented Sep 23, 2019

@rowanmiller - Looking at 2.2.6, the "///" documentation on Microsoft.EntityFrameworkCore.DbContext does not indicate that public virtual int SaveChanges(); is automatically called by any of the following:

  • public virtual int SaveChanges(bool acceptAllChangesOnSuccess);
  • public virtual Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default);
  • public virtual Task<int> SaveChangesAsync(CancellationToken cancellationToken = default);

Instead they only mention that "This method will automatically call Microsoft.EntityFrameworkCore.ChangeTracking.ChangeTracker.DetectChanges to discover any changes to entity instances before saving to the underlying database."

I haven't yet tested myself, but perhaps the workaround should instead throw the exception on DetectChanges() rather than on SaveChanges or perhaps all the variants of SaveChanges() need to be overridden?

@davisnw
Copy link

davisnw commented Sep 23, 2019

Testing shows that the given workaround in this issue is incomplete. ShouldThrowOnSaveChanges passes, ShouldThrowOnSaveChangesAsync fails.

        [Fact]
        public void ShouldThrowOnSaveChanges()
        {
            var exception = Assert.Throws<InvalidOperationException>(() =>
            {
                using (var db = new BloggingContext())
                {
                    db.Database.EnsureCreated();

                    db.Blogs.Add(new Blog { Url = "http://blogs.msdn.com/adonet" });
                    var count = db.SaveChanges();
                    Console.WriteLine("{0} records saved to database", count);
                }
            });

            Assert.StartsWith("Attempted to save read-only entities", exception.Message);
        }

        [Fact]
        public async Task ShouldThrowOnSaveChangesAsync()
        {
            var exception = await Assert.ThrowsAsync<InvalidOperationException>(async() =>
            {
                using (var db = new BloggingContext())
                {
                    await db.Database.EnsureCreatedAsync();

                    db.Blogs.Add(new Blog { Url = "http://blogs.msdn.com/adonet" });
                    var count = await db.SaveChangesAsync();
                    Console.WriteLine("{0} records saved to database", count);
                }
            });

            Assert.StartsWith("Attempted to save read-only entities", exception.Message);
        }

@smitpatel
Copy link
Contributor

@davisnw - A work-around posted in Feb 2017 can easily get outdated with newer version of EF as underlying APIs can change.

@ajcvickers
Copy link
Member

ajcvickers commented Dec 9, 2020

Note to implementer: consider how read-only entity types could be used to support updates from bounded contexts--see #23457.

@gokhanabatay
Copy link

gokhanabatay commented Dec 7, 2022

Please consider while implementing, #29756 (comment)

#29756

@wdhenrik
Copy link

I consider some of my entities as "Read-Only" since they are never changed by the app, but they are seeded using HasData during migrations. Would this feature still allow using HasData?

@sliekens
Copy link

sliekens commented Mar 7, 2023

+1 for making sure this will work with HasData. I would absolutely use read only entities for reference data that are seeded by HasData, but may never change in the context of a business transaction.

@bachratyg
Copy link

This should also block batch updates without tracking the entity i.e. ExecuteUpdate and ExecuteDelete

@rjperes

This comment has been minimized.

@ajcvickers

This comment has been minimized.

@pantonis

This comment has been minimized.

@roji

This comment has been minimized.

@rjperes
Copy link

rjperes commented Jun 21, 2024

Perhaps someone can consider creating a PR?

@pantonis
Copy link

The problem with this approach is that it doesn't have any workaround

@rjperes
Copy link

rjperes commented Jun 21, 2024

A question to @ajcvickers @rowanmiller @divega: should the behaviour be to throw an exception when an attempt is made to save (and what does this actually mean?) a read-only entity or should changes be just silently ignored? I’d say if we have a mix of “dirty” entities, some of which read-only and others that aren’t, we could just persist the non-read-only ones and ignore the others.

@ajcvickers
Copy link
Member

@rjperes That has been an open question since this issue was created, and it is still an open question now.

@pantonis
Copy link

pantonis commented Jun 21, 2024

The big issue here is that you cant even find which entries are already being tracked before you call SaveChanges(). It throws the exception even when you are trying to filter them out. The way it works there is not any workaround.

@roji
Copy link
Member

roji commented Jun 21, 2024

The big issue here is that you cant even find which entries are already being tracked before you call SaveChanges().

Note sure what you mean here, have you seen these docs?

@rjperes
Copy link

rjperes commented Jun 21, 2024

This code will effectively mark an entity as read-only (same as a view), causing EF to throw an exception if someone tries to change its properties and persist it:

public static class EntityTypeBuilderExtensions
{
    public static EntityTypeBuilder<T> IsReadOnly<T>(this EntityTypeBuilder<T> builder) where T : class
    {
        var metadata = builder.Metadata;
        var tableName = metadata.GetAnnotation(RelationalAnnotationNames.TableName).Value;
        var tableSchema = metadata.GetAnnotation(RelationalAnnotationNames.Schema).Value;
        metadata.RemoveAnnotation(RelationalAnnotationNames.TableName);
        metadata.RemoveAnnotation(RelationalAnnotationNames.Schema);
        metadata.SetAnnotation(RelationalAnnotationNames.ViewName, tableName);
        metadata.SetAnnotation(RelationalAnnotationNames.ViewSchema, tableSchema);

        return builder;
    }
}

The exception, however, will not say anything meaningful, that is, it will complain about the entity not being mapped to a table, because it thinks it's a view. It's just a simple workaround, not a full-fledged solution, but seems to work.

@rjperes
Copy link

rjperes commented Jun 21, 2024

Or another option:

public static EntityTypeBuilder<T> IsReadOnly<T>(this EntityTypeBuilder<T> builder) where T : class
{
    var metadata = builder.Metadata;
    var props = metadata.GetProperties();

    foreach (var prop in props)
    {
        prop.SetAfterSaveBehavior(PropertySaveBehavior.Throw);
    }

    return builder;
}

Both seem to work, this one only for scalar properties.

@pantonis

This comment has been minimized.

@roji

This comment has been minimized.

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