Attached/Detached state is a nightmare

[ghost]

Hi,

I used EF because I was told to. Was it a great experience? ... no not at all. After 6 major versions I would like to submit a complaint.

I have used: Dapper (fastest and best, because it tries to be the least clever and is the easiest to understand and to teach). NHibernate (used with FluentNHibernate is a simili for "code first") Castle Active Record (think hammett got his nailed, least invasive "code first" model) ADO.NET(along with RDO before that ad infinitum) None of these API's impose state management on the developer as a consumer of these libraries in the way EF does. We always had "session per view" or "session per call" with NHibernate, and IDbConnection with ADO however you guys managed to take this further and impose state management at a POCO level when using DTO's with MVC/WebAPI.

Consider the following scenario: We use IoC (Castle Windsor to be precise) We have base libraries (heaps of them) We have services (heaps of them) We use internal NuGet packages Services in our dependency hierarchy can be consumed "in process" or "out of process" via a service boundary. * THINK ABOUT WHAT THIS MEANS I.T.O. EF ATTACHED VS DETACHED STATE *. I think we are all well versed in the theoretical conundrum which is the "Impedance Mismatch" which ORM's give us. All I had to worry about with NHibernate was my session. With ADO it was my IDbConnection. However with EF I have to worry about whether the object is attached or not? Most peculiar. I am guessing you are going to say this has something to do with state tracking. The problem is other frameworks seem to do this via method/property interception transiently without making it part of the external API.

The fact that I have to do reflection(ObjectContext) to get my "unit of work" pattern to work while using DTO's has left me feeling rather frustrated. I launched an agile project where our estimates 48% out of kilter and most of it was dealing with persistence while using EF 6.

The second thing that grates my cheese is the fact that that EF is heavily tied to version of sql you use. So when I blend services in the same process and they depend on different version(ie. SQL 2005 vs SQL 2012 for sake of argument) then EF barfs up exceptions because the process manifest token has a different versions for different context's. Kind of invalidates things like using NuGet with services that require data from different versions of sql in the same process.

This is not scalable at all. It has actually cost us more time than what it has solved problems.

Questions:

• If you how to identify an instance correctly(primary keys etc), surely you can infer what the state is right?
• If the answer to the previous question is yes, then surely the "state" conundrum can be removed all together?
• Doesn't using this approach set you up an easy life when it comes to any kind of caching?

[rowanmiller]

Hey,

It sounds like most of what you are asking for is covered by the new AttachGraph method in EF7 (it calculates state based on PKs etc.). You can read more about it in our design meeting notes.

I wasn't quite sure what you meant by this comment. Could you elaborate on the scenario that you need to use reflection for?

The fact that I have to do reflection(ObjectContext) to get my "unit of work" pattern to work while using DTO's has left me feeling rather frustrated.

~Rowan

[ghost]

So to reiterate, are you saying (Attached/Detached) state WILL be inferred for the purposes of submitting both "in process" and "out of process" graphs (detached and serialised) all the way down to the child level?

• The background

We use AngularJs for our front ends(CORS enabled with WebAPI) and very quickly found out that we could not simply submit entire instance graphs back to WebAPI because of exceptions from Entity Framework. This hurt us big time. Instead we opted for rolling plenty more boiler plate code to deal with the instance graphs in pieces. This has also contributed to confusion because we violated REST in doing so(bye bye DDD). Opting for a more RPC style interface. This has not only chewed into our delivery time lines but caused confusion in our code base because everything tends to look like the database schema. Hence the agro(apologies). Again I stress the fact that I was told to use EF.

In cases where we used stored procedures(for performance) we very quickly realised that EF introspection from the tooling in Visual Studio simply did not map stored procedures that took advantage of return codes(which tells me something is lacking in way of defining custom error codes in stored procs while using EF). It some how got them confused with return types. Simply not mapping the stored proc producing an error. This has restricted us for when things go wrong because things we know things "we should know about" cannot simply be bubbled up to the surface without abstract intervention. We opted for custom db logging. Adding friction because now we have to query hand rolled logging in the database and always have to consider that maybe it failed there ... adding weight to our support of production problems. Absolutely would knock anyone out learning EF for the first time yet having a modicum of an understanding of how SQL+ADO works.

We also tried to simplify queries with views again the tooling would not pull this into the model unless you selected a primary key. What made it peculiar and the feedback was minimal to none. Stack overflow yielded what I think is a "hack" for making it work. There are just cases where you don't care or have the luxury of primary keys. Consider legacy, migrated or ETL'ed databases here. Painful.

Trying to fall back on to ADO, you have trouble with the IDbConnection hanging off the Database Context. It is not open! Even after you have thrown your unit-of-work out the door and settled for the classical IDisposable way of newing it up. Surely this should be a signal to the framework that "Yes, I would like an open db connection please". Sql hides passwords in the connection string, so if you opt for going "Old School" then you have to "twiddle" your EF connection string(to get the ADO one out) or if you are a junior programmer bloat your web.config with another connection string. Abismal.

If you are freshly adopting EF and you have a modicum of understanding databases there is something about your API that punches one right in the face. When it comes to SQL we understand 3 basic things that should happen(because let's face it we all write sql, no matter how dirty it is). The key things we always think about is INSERT, UPDATE or DELETE. Why is this not available from the public API as methods? Why forgo something that is well understood in favour of ctx.Entry(...).State = ... something ... ? You are trading your view of the world for a well understood concept that is discoverable by most developers. This also provides you a tidy hook for inferring state and dealing with graphs. DbContext would be my choice(your design notes mention ChangeTracker). AttachGraph is very far removed from this concept in the way of aesthetics and something which is easy or familiar to adopt.

Suggestions

You are going to need a bigger boat.

Your design meeting notes do not consider performance. Serialisation is expensive already. What "design goals" are you going to set when it comes to implementation in this area? Defer them to the async api after consumer developers spent hours googling the problem and found the solution on stack overflow?

Secondly, you guys seem to missing the fact that your proxies are imposing state management on us via your public API when other frameworks do not. They infer, and if they can't they establish intent and then execute that intent.

Harmonisation of all the paths into your framework using a single Attach is a good start. So 1 point for that. I invite you to explore a code sample I have prepared. Not harsh but honest none the less.

• The Code

using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; using System.Data.Entity; using System.Data.Entity.Infrastructure; using System.Linq; using FluentNHibernate.Cfg; using FluentNHibernate.Cfg.Db; using FluentNHibernate.Mapping; using Newtonsoft.Json;

namespace ConsoleApplication_Nhibernate { //SQL Script //CREATE TABLE [dbo].[MyEntity](// [MyId] [int] IDENTITY%281,1%29 NOT NULL, // [MyText] [nvarchar]%281024%29 NOT NULL, // CONSTRAINT [PK_MyEntity] PRIMARY KEY CLUSTERED //%28 // [MyId] ASC //%29WITH %28PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON%29 ON [PRIMARY] //) ON [PRIMARY]

//App.config
//<?xml version="1.0" encoding="utf-8"?>
//<configuration>
//    <configSections>
//        <section name="entityFramework" type="System.Data.Entity.Internal.ConfigFile.EntityFrameworkSection, EntityFramework, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" requirePermission="false" />
//    </configSections>
//    <startup>
//        <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5" />
//    </startup>
//    <connectionStrings>
//    <add name="MyEntities" connectionString="data source=.;initial catalog=MyEntities;integrated security=True;MultipleActiveResultSets=True;App=EntityFramework" providerName="System.Data.SqlClient" /></connectionStrings>
//    <entityFramework>
//        <providers>
//            <provider invariantName="System.Data.SqlClient" type="System.Data.Entity.SqlServer.SqlProviderServices, EntityFramework.SqlServer" />
//        </providers>
//    </entityFramework>
//</configuration>

[Table("MyEntities")] // Required by Entity Framework v6.1.2
public class MyEntity
{
    [Key] // Required by Entity Framework v6.1.2
    public virtual int MyId { get; set; }
    [Required] // Required by Entity Framework v6.1.2
    public virtual string MyText { get; set; }
}

/* FluentNHibernate v2.0.1.0/NHibernate v4.0.0.4000 */

public class MyEntityMap : ClassMap<MyEntity>
{
    public MyEntityMap()
    {
        Id(x => x.MyId);
        Map(x => x.MyText);
        Table("MyEntities");
    }
}

/* Entity Framework v6.1.2 */

public partial class MyEntityContext : DbContext
{
    public MyEntityContext()
        : base("name=MyEntities")
    {
    }

    public virtual DbSet<MyEntity> MyEntities { get; set; }

    protected override void OnModelCreating(DbModelBuilder modelBuilder)
    {
    }
}

internal class Program
{
    private static void Main(string[] args)
    {
        /* THE HAPPY PATH - JUST WORKS NO EXCEPTIONS */

        /* FluentNHibernate v2.0.1.0/NHibernate v4.0.0.4000 */

        var sessionFactory =
            Fluently
                .Configure()
                .Database(MsSqlConfiguration.MsSql2008.ConnectionString("server=.;database=MyEntities;user id=t;password=p"))
                .Mappings(m => m.FluentMappings.AddFromAssemblyOf<MyEntity>())
                .BuildSessionFactory();

        using (var session = sessionFactory.OpenSession())
        {
            var myEntity = new MyEntity
            {
                MyText = @"From NHibernate"
            };

            session.SaveOrUpdate(myEntity); // Wow, Upsert ... 

            /* In Process/Out of Process Update Serialization Test - What happens when you have services */

            var j = JsonConvert.SerializeObject(myEntity);
            var mySerializedEntityWithResult = JsonConvert.DeserializeObject<MyEntity>(j);

            mySerializedEntityWithResult.MyText = "From NHibernate - Changed";

            session.SaveOrUpdate(myEntity); // Wow, Upsert ... This API subscribes to "tell don't ask". 
        }

        /* THE NOT SO HAPPY PATH - MANY EXCEPTIONS, HARD TO FOLLOW, INEFFICIENT, LOADED WITH FRICTION */

        /* Entity Framework v6.1.2 */

        using (var db = new MyEntityContext())
        {

            // Method 1
            var myEntity = new MyEntity()
            {
                MyText = "From Entity Framework (Method 1)"
            };
            db.MyEntities.Add(myEntity);

            db.SaveChanges();

            /* In Process/Out of Process Update Serialization Test - What happens when you have SOA or services */

            // PROBLEM 1 -> 
            var j = JsonConvert.SerializeObject(myEntity);
            var mySerializedEntityWithResult = JsonConvert.DeserializeObject<MyEntity>(j);
            mySerializedEntityWithResult.MyText = "From Entity Framework (Method 1) - Updated";
            db.Entry(mySerializedEntityWithResult).State = EntityState.Modified; // <- HERE IN FACT

            // EXCEPTION: 
            //
            // Attaching an entity of type 'ConsoleApplication_Nhibernate.MyEntity' failed because another entity of the 
            // same type already has the same primary key value. This can happen when using the 'Attach' method or setting 
            // the state of an entity to 'Unchanged' or 'Modified' if any entities in the graph have conflicting key values. 
            // This may be because some entities are new and have not yet received database-generated key values. In this 
            // case use the 'Add' method or the 'Added' entity state to track the graph and then set the state of non-new 
            // entities to 'Unchanged' or 'Modified' as appropriate.

            // THE PROBLEM:
            //
            // After googling each and every exception until this point would probably cost me in the excess of half an hour(caveat 
            // this with NOT using the designer and consider the adopting mind of a programmer who is crossing over to C#), while 
            // getting going with FluentNHibernate would take 5 minutes if that, no exceptions, just works(had bury a connection 
            // string in a config file for EF which could be tauted as "friction"). 
            // 
            // Not like this, a horrible long windy road of exceptions where things are so abstracted that they become counter 
            // intuitive to learn and implement and where every few seconds you find yourself copying a link from an exception, 
            // and pasting it into a browser ultimately pointing to MSDN or a google yielding yet another Stack Overflow.
            //
            // When did idea's like DbContext.Insert<T>, DbContext.Update<T> and DbContext.Delete<T> become evil(you know like in 
            // the good 'ol days)? And if they are graphs, you guys use data annotations to discover the state? Don't forget we are 
            // telling your API to do it in code. 

            bool saveFailed;
            do
            {
                saveFailed = false;
                try
                {
                    db.SaveChanges();
                }
                // PROBLEM 2 -> 
                //
                // Why do you guys not only thrust state down our necks(see PROBLEM 1) but then create a pecualiar API which
                // forces us to deal with Concurrency in an exception block by way of merging dictionaries or using a ToObject 
                // extension? 
                //
                // Other frameworks deal with this by defaulting their configurations for concurrency. I found on this MSDN
                // after getting an exception. Secondly this is incredibly in-efficient. I dont always want an exception raised
                // and I would love to see something in the way DbContext.Concurrency = ConcurrencyType.Optimistic if you feel
                // aversed to defaulting it.
                //
                catch (DbUpdateConcurrencyException ex)
                {
                    saveFailed = true;
                    var entry = ex.Entries.Single();
                    entry.GetDatabaseValues().SetValues(entry.OriginalValues);
                }

            } while (saveFailed); 

            // PROBLEM 3 ->
            //
            // "Many roads(with exceptions all the way) lead to Rome"
            // 
            // It can be proven from any open source project. The more paths you create into your public API, the more complex it 
            // is to understand or learn. If it is a complex API then establish familiarity through "intent", most of us understand 
            // SQL, so why not bring it home for the bulk of your consumers by shaping your public API that way?
            // 
            // Credit due. Your design notes reflect you accept this is a problem. Make this a first class design goal!

            // Method 2
            var instance = db.MyEntities.Create();
            instance.MyText = "From Entity Framework (Method 2)";
            db.MyEntities.Add(instance); // If we comment this out it does not save. API Tautology perhaps? Intent has already been declared via 'db.MyEntities.Create()'.

            // Method 3 
            db.Entry<MyEntity>(myEntity).State = EntityState.Added; // Insert<T>, Update<T>, Delete<T> invalidates this, hell you could even Upsert<T>!

            // Method 4
            db.Set<MyEntity>().Add(new MyEntity()
            {
                MyText = "From Entity Framework (Method 3)"
            });

            db.SaveChanges();
        }
    }
}

}

[rowanmiller]

@fir3pho3nixx - Sounds like most of your complaints are things that we are addressing in EF7 (or are at least laying ground work to improve in the future).

A few comments...

Regarding graphs, yes when you tell EF about the root of a graph (via the AttachGraph API) it will infer state based on whether the PK is set or not. Of course, this is simplistic and won't work in all cases, so you can also provide an action to set the state based on your own business rules. We want to extent this in the future with ETag support that can provide better state calculation.

DbContext doesn't open the DbConnection it has until it actually needs it, and this isn't changing (by default EF will open and close as needed). You can always open it yourself though and then EF will just leave it open.

The EF7 code base is currently lacking SPROC support. We know EF6 has some limitations and we want to do a better job of it this time around.

Proxies are optional in EF6, typically they are only created for Lazy Loading (but you can have them created for change tracking too). EF7 currently doesn't use proxying at all. It may be available at some point, but we'll likely favor a Roslyn based compile time solution rather than dynamically inheriting classes at runtime.

[ghost]

Will wait and see what the future brings. Until then we are going to opt for something else while you guys build v7.0.

If everything I am complaining about is in EF 7.0 than what we should be seeing is:

• Improved discoverability to new EF users by way of introducing Insert, Update, Delete and Upsert on db context which accepts both graphs and single objects.
• Dropping the state based callback for attach graph. Because you understand that you should be dealing with this. Otherwise you are merely writing a fancy pants tree walking callback.
  • Being able to set concurrency strategy by way of enum on ctor for DbContext ie. var db = new DbContext(Concurrency.Optimistic). The handling of this should be an event not an exception for performance.
  • EF passing the serialization test without throwing exception to better support SOA paradigms. "Tell dont ask".
  • EF wont open the database connection. Silly but ultimately up to you. One extra line of code for us.
  • Stored procs require continued work. Perhaps NOT in 7.0.
  • Tooling problems requires continued work. Perhaps NOT in 7.0. ie. Views just not mapping without PK's and not giving feedback.
  • Context with clashing manifest tokens in same process NOT in EF 7.0. Sorry EF users, you are hard tied to specific version of sql if you try blend 2005, 2008 or 2012 context's via internal NuGet.

So to summarize, you are NOT addressing all the issues but looking at some and then caveating that with maybe only foundation. Ergo might not actually solve the problem but increment the major version and release anyway. Great!

Good luck! You can close this issue if you blindly believe you have happy users out there.

On Tue Jan 13 2015 at 11:09:22 PM Rowan Miller notifications@github.com wrote:

@fir3pho3nixx https://github.com/fir3pho3nixx - Sounds like most of your complaints are things that we are addressing in EF7 (or are at least laying ground work to improve in the future).

A few comments...

Regarding graphs, yes when you tell EF about the root of a graph (via the AttachGraph API) it will infer state based on whether the PK is set or not. Of course, this is simplistic and won't work in all cases, so you can also provide an action to set the state based on your own business rules. We want to extent this in the future with ETag support that can provide better state calculation.

DbContext doesn't open the DbConnection it has until it actually needs it, and this isn't changing (by default EF will open and close as needed). You can always open it yourself though and then EF will just leave it open.

The EF7 code base is currently lacking SPROC support. We know EF6 has some limitations and we want to do a better job of it this time around.

Proxies are optional in EF6, typically they are only created for Lazy Loading (but you can have them created for change tracking too). EF7 currently doesn't use proxying at all. It may be available at some point, but we'll likely favor a Roslyn based compile time solution rather than dynamically inheriting classes at runtime.

— Reply to this email directly or view it on GitHub https://github.com/aspnet/EntityFramework/issues/1351#issuecomment-69839704 .

[rowanmiller]

Hey,

A few closing comments,

Insert/Delete vs Add/Remove is hard because EF is bridging the gap between database and .NET code. Insert/Update are database concepts and Add/Remove are the equivalent .NET concepts (think ICollection). I can see merit in both sides of the argument but we'll be sticking with Add/Remove in EF7 since there doesn't seem to be enough of a distinction to merit breaking form the EF6 API.

AttachGraph has two overload, one where you can pass a call back and the other where we just work it out based on key values.

BTW if EF isn't the tool for you then I'm definitely not going to try and talk you into it. A lot of folks find EF useful but it's definitely not the tool for everyone and every application. We are trying to make EF great for the subset of folks that like this style of data access rather than making a tool that can be used by everyone (as that always ends up with a tool that can be used everywhere but is mediocre).

~Rowan

[ghost]

Not doing graphs properly after 6 major versions reeks of mediocrity. Will look somewhere else, perhaps if not always from now on(think "Customer", our business runs on your software).

Thanks.

On Thu Jan 15 2015 at 6:24:16 PM Rowan Miller notifications@github.com wrote:

Closed #1351 https://github.com/aspnet/EntityFramework/issues/1351.

— Reply to this email directly or view it on GitHub https://github.com/aspnet/EntityFramework/issues/1351#event-219591813.