Using a version with VersionGeneration.Never does not throw StaleObjectStateException

[Roemer]

When I use a Version field that is managed by nhibernate (VersionGeneration.Never) and modify the value from the version field before saving, nhibernate ignores this value and uses the previous value and happily saves the value. This is an issue in case that the values are coming from a client which might have an old version of the entity. If I use the exact same code and just modify the Version field to VersionGeneration.Always, it works correctly and throws an StaleObjectStateException. Here is a simple example to reproduce the issue:

// Entity
public class Student
{
    public virtual int Id { get; set; }
    public virtual string Name { get; set; }
    public virtual DateTime Timestamp { get; set; }
}

// Mapping
public class StudentMap : ClassMapping<Student>
{
    public StudentMap()
    {
        Lazy(true);
        Table(nameof(Student));
        Id(x => x.Id, map =>
        {
            map.Generator(Generators.Identity);
        });
        Property(p => p.Name);
        Version(x => x.Timestamp, m =>
        {
            m.Generated(VersionGeneration.Always);
            m.Type(new DateTimeType());
            m.UnsavedValue(DateTime.MinValue);
            m.Column(c =>
            {
                c.NotNullable(true);
                c.Default("CURRENT_TIMESTAMP");
            });
        });
    }
}

// Session Factory
public static ISessionFactory GetSessionFactory()
{
    var config = new Configuration();
    config.DataBaseIntegration(db =>
    {
        db.Dialect<SQLiteDialect>();
        db.Driver<SQLite20Driver>();
        db.ConnectionString = @"Data Source=c:\temp\test.db;Version=3;New=True";
        db.LogSqlInConsole = true;
    });

    var mapper = new ModelMapper();
    mapper.AddMapping<StudentMap>();
    mapper.AddMapping<Student2Map>();
    var mappings = mapper.CompileMappingForAllExplicitlyAddedEntities();
    config.AddDeserializedMapping(mappings, null);

    var exporter = new SchemaExport(config);
    exporter.Execute(true, true, false);

    return config.BuildSessionFactory();
}

// Test Fixture
[TestFixture]
public class OptimisticLockingTestFixture
{
    private ISessionFactory _sessionFactory;

    [OneTimeSetUp]
    public void FixtureSetUp()
    {
        // Create the session factory
        _sessionFactory = GetSessionFactory();
    }

    [Test]
    public void TestWithDbGeneratedVersion()
    {
        // Perform the initial insert
        int studentId = 0;
        using (var session = _sessionFactory.OpenSession())
        {
            using (var tx = session.BeginTransaction())
            {
                var student = new Student { Name = "John Doe" };
                session.Persist(student);
                studentId = student.Id;
                tx.Commit();
            }
        }

        // Perform a simple update
        using (var session = _sessionFactory.OpenSession())
        {
            using (var tx = session.BeginTransaction())
            {
                var student = session.Get<Student>(studentId);
                student.Name = "John Doe 2";
                tx.Commit();
            }
        }

        // Modify the timestamp before an update, so it should throw
        Assert.Throws<StaleObjectStateException>(() =>
        {
            using (var session = _sessionFactory.OpenSession())
            {
                using (var tx = session.BeginTransaction())
                {
                    var student = session.Get<Student>(studentId);
                    student.Name = "John Doe 3";
                    student.Timestamp = DateTime.Now.AddYears(-10);
                    tx.Commit();
                }
            }
        });
    }
}

This example works, now if in the mapping the VersionGeneration.Always is changed to VersionGeneration.Never, the exception is not thrown anymore. This seems like a rather nasty bug which can lead to wrong data being saved.

[bahusoid]

When I use a Version field that is managed by nhibernate, and modify the value from the version field before saving, nhibernate ignores this value

When Version is managed by NHibernate you shouldn't change Version field with your custom values - all your custom values will be ignored by design and in check will be used initial value that's been loaded from DB. So current behavior for VersionGeneration.Never looks correct to me - you just load fresh entity from DB and flush updates.

[Roemer]

But look at the following scenario: We have a client/server application. The client can call services on server side to interact with the database. The services use DTOs as input and output. There is mapping in the service between DTO and Entity.

1. Bob loads a student => Student(Id: 1, Name: "Leeroy", Timestamp: 10:00)
2. Alice loads a student => Student(Id: 1, Name: "Leeroy", Timestamp: 10:00)
3. Bob modifies the student = > Student(Id: 1, Name: "Lee-Roy", Timestamp: 10:00)
4. Bob saves the student => Student(Id: 1, Name: "Lee-Roy", Timestamp: 11:00)
5. Alice modifies the student => Student(Id: 1, Name: "LeeRoy", Timestamp: 10:00)
6. Alice saves the student

Now here is the interesting part. When saving in the service is done like:

1. Load the entity from the db
2. Map the values from the dto to the entity (including the timestamp)
3. Save the entity

Then we have exactly that issue, that even thou we also mapped the timestamp (10:00) to the loaded entity (which had 11:00), it is ignored and the data from Alice are happily saved without any warning. This does not seem right to me.

[bahusoid]

Still NHibernate behaves exactly as you configured it. It's incorrect DTO -> entity management I would say. To make it work as you want it seems you should recreate entity from DTO and then call session.SaveOrUpdate. Check the following discussion for details: https://stackoverflow.com/questions/15968861/problems-with-nhibernate-optimistic-concurrency-in-a-web-application-scenario

So it might be considered as feature request to support user provided values in concurrency check but it's definitely not a bug.

[Roemer]

If I detach/reattach it, it also works:

session.Evict(student);
session.Update(student);

or if I create a new object and merge it

var student = new Student2
{
    Id = studentId,
    Name = "John Doe 3",
    Timestamp = DateTime.Now.AddYears(-10)
};
session.Merge(student);

or if I create a new object and SaveOrUpdateit:

var student = new Student2
{
    Id = studentId,
    Name = "John Doe 3",
    Timestamp = DateTime.Now.AddYears(-10)
};
session.SaveOrUpdate(student);

But still, the behavior is different then when I let the DB control the versioning. IMHO if nhibernate takes care of the versioning, it should be cautious and do not save if unsure instead of by default just save it. Or at least should have a configuration to behave that way. I am sure in larger codebases, this gets forgotten and there are many places that data just gets overwritten especially because it is different for db-managed and nhibernate managed versioning fields.

[fredericDelaporte]

At best, I think NHibernate should throw something like an InvalidOperationException if a version property has been changed directly instead of merging/updating the entity. (I mean by merging/updating the process of calling ISession.Merge, ISession.Update, or ISession.SaveOrUpdate on a detached entity.) Version properties are meant to be readonly properties.

Your case seems to me as a misunderstanding of how versioning works. NHibernate should never try to compensate users mistakes: in most cases, this worsens the trouble in the end. NHibernate should instead throw when an user mistake can be detected.

[Roemer]

Let's look at the following example: I have an entity that looks like:

public class PersonEntity {
    public virtual int Id { get; set; }
    public virtual DateTime Timestamp { get; set; }
    public virtual string Firstname { get; set; }
    public virtual string Lastname { get; set; }
}

And Dtos that look like this:

public class PersonWithFirstnameDto {
    public int Id { get; set; }
    public DateTime Timestamp { get; set; }
    public string Firstname { get; set; }
}
public class PersonWithLastnameDto {
    public int Id { get; set; }
    public DateTime Timestamp { get; set; }
    public string Lastname { get; set; }
}

and corresponding services like:

public class PersonService {
    public PersonWithFirstnameDto GetPersonWithFirstname() {
        ...
    }
    public PersonWithFirstnameDto SavePersonWithFirstname(PersonWithFirstnameDto dto) {
        ...
    }
    public PersonWithLastnameDto GetPersonWithLastname() {
        ...
    }
    public PersonWithLastnameDto SavePersonWithLastname(PersonWithLastnameDto dto) {
        ...
    }
}

So as you see, in the services I always only get a part of the Person object and uppon saving, I just want to update some of the values of the PersonEntity and not touch the others. In that scenario, I don't see another way then loading the existing PersonEntity from the database, overwrite the values I want to overwrite and save it back. And here I need to overwrite the Timestamp in order to be able to detect an optimistic locking issue. This is a pretty common scenario in almost all our applications. How else is versioning supposed to work? Also my main complaint is still that this works as I expect when using database-based versioning but not when using nHibernate-based versioning.

[fredericDelaporte]

Updating a version property of an entity attached to the session is an invalid operation, period.

The fact it kinda works for generated version property is unintended, and should not be relied upon.

Your use case matches this paragraph of the NHibernate documentation. When you want to work with DTO without converting them to detached entities for merging/updating, you have to check the version manually as illustrated in the documentation.