NH-3527 - UnionSubclassMapper should mark an abstract type as abstract in the generated HbmMapping

[mahara]

Fix for NH-3527 (https://nhibernate.jira.com/browse/NH-3527).

[hazzik]

Could you please add a test case?

[mahara]

I don't know how to test it using NHibernate's internal way, but here's what I do.

I define, at least, three classes: EntityBase, Item, and InventoryItem. Both EntityBase and Item are abstract.
I use table-per-concrete-class mapping inheritance, and define the mapping using ModelMapper.

// Model definition.
public abstract class EntityBase { }
public abstract class Item : EntityBase { }
public class InventoryItem : Item { }

// Mapping definition.
var modelMapper = new ModelMapper();
modelMapper.Class<EntityBase>(cm => { });
modelMapper.UnionSubclass<Item>(cm => { });
modelMapper.UnionSubclass<InventoryItem>(cm => { });
var mapping = modelMapper.CompileForAllExplicitlyAddedEntities();
var xmlMapping = mapping.AsString();

Beftore the fix, the xmlMapping would basically look like this:

<?xml version="1.0" encoding="utf-8"?>
<hibernate-mapping xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" namespace="MyApp.Models" assembly="MyApp" xmlns="urn:nhibernate-mapping-2.2">
  <class name="EntityBase" abstract="true">
  </class>
  <union-subclass name="Item" extends="EntityBase">
  </union-subclass>
  <union-subclass name="InventoryItem" extends="Item">
  </union-subclass>
</hibernate-mapping>

After the fix, the Item mapping would have abstract attribute defined as true:

<?xml version="1.0" encoding="utf-8"?>
<hibernate-mapping xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" namespace="MyApp.Models" assembly="MyApp" xmlns="urn:nhibernate-mapping-2.2">
  <class name="EntityBase" abstract="true">
  </class>
  <union-subclass name="Item" extends="EntityBase" abstract="true">
  </union-subclass>
  <union-subclass name="InventoryItem" extends="Item">
  </union-subclass>
</hibernate-mapping>

To check how the generated SQL would look like, I use SchemaExport.
Before the fix, it would create two tables: Item and InventoryItem. Item table here is the unwanted/unexpected.
After the fix, it would create only one table as expected: InventoryItem contains all columns defined from EntityBase, Item, and InventoryItem.

That's how I figure out the bug and the fix.

[hazzik]

Here is an explanation how to write tests for NH https://github.com/nhibernate/nhibernate-core/blob/master/Contributor%20Guide.md#creating-a-test-case-to-verify-the-issue

[hazzik]

But probably you need to check how is UnionSubclassMapper tested.

[mahara]

@hazzik
No, that's not what I meant. I can create a test case if it's only a simple CRUD operation using session etc. But in this case, I cannot express my test case in NH internal way.

What I've explained above is pretty much a manual integration test case (since it involves UnionSubclassMapper, ModelMapper, XML mapping, and SchemaExport), but it seems you're suggesting to create an automated unit test case (only the UnionSubclassMapper).
Furthermore, even with such unit test case, it wont be enough since it involves other components as well. Which in other words, the integration test case would have to be written anyway.
The fact it's just. in either way, I'm not sure how to write both tests... :D

Could you provide a lead on how to design and write such integration test case?

[mahara]

OK, the unit test case would pretty much look like this:

namespace NHibernate.Test.MappingByCode.MappersTests.UnionSubclassMapperTests
{
    [TestFixture]
    public class AbstractAttributeTests
    {
        private abstract class EntityBase
        {
        }

        private abstract class Item : EntityBase
        {
        }

        private class InventoryItem : Item
        {
        }

        [Test]
        public void CanSetAbstractAttributeOnAbstractClass()
        {
            var mapdoc = new HbmMapping();
            var mapper = new UnionSubclassMapper(typeof(Item), mapdoc);

            mapdoc.UnionSubclasses[0].@abstract.Should().Be.True();
            mapdoc.UnionSubclasses[0].abstractSpecified.Should().Be.True();
        }

        [Test]
        public void CanSetAbstractAttributeOnConcreteClass()
        {
            var mapdoc = new HbmMapping();
            var mapper = new UnionSubclassMapper(typeof(InventoryItem), mapdoc);

            mapdoc.UnionSubclasses[0].@abstract.Should().Be.False();
            mapdoc.UnionSubclasses[0].abstractSpecified.Should().Be.False();
        }
    }
}

[hazzik]

I meant integration test but unit test for UnionSubclassMapper will be also good.

Please see src/NHibernate.Test/NHSpecificTest/NH0000 test (you will need FixtureByCode)

Also you could check Mapping By Code related integration test (src/NHibernate.Test/MappingByCode/IntegrationTests)

The test will run SchemaExport automatically, so you need to check only your expectations (probably make dialect-specific test and check that table does not exist)

[hazzik]

OK, the unit test case would pretty much look like this:

looks good.

[mahara]

OK, that's my question: how do I check whether a mapped table exists in the test?

[hazzik]

I think you need just query DB-specific query, like this http://stackoverflow.com/a/167680/259946 (MS SQL) or this http://stackoverflow.com/a/8829109/259946 (MySQL)

[mahara]

Ah... IC. I meant if such an NHibernate built-in function like TableExists() existed, I'd rather to use it.
But OK, I'll use SQL query directly.
Thanks.

[mahara]

OK, here's the integration test:

namespace NHibernate.Test.MappingByCode.IntegrationTests.NH3527
{
    public abstract class EntityBase
    {
        public virtual int Id { get; set; }
    }

    public abstract class Item : EntityBase
    {
    }

    public class InventoryItem : Item
    {
    }

    [TestFixture]
    public class FixtureByCode : TestCaseMappingByCode
    {
        [Test]
        public void VerifyMapping()
        {
            var mapping = this.GetMappings();

            // Item mapping.
            mapping.UnionSubclasses[0].abstractSpecified.Should().Be.True();
            mapping.UnionSubclasses[0].@abstract.Should().Be.True();
            // InventoryItem mapping.
            mapping.UnionSubclasses[1].abstractSpecified.Should().Be.False();
            mapping.UnionSubclasses[1].@abstract.Should().Be.False();
        }

        [Test]
        public void VerifyTables()
        {
            using (var session = this.OpenSession())
            {
                var tableNameColumnName = "TABLE_NAME";
                var itemTableName = "Item";
                var inventoryItemTableName = "InventoryItem";

                var schema = this.Dialect.GetDataBaseSchema((DbConnection) session.Connection);
                var tables = schema.GetTables(null, null, null, null).AsEnumerable();
                var itemTable = tables.SingleOrDefault(
                    x => string.Equals(x.Field<string>(tableNameColumnName),
                                       itemTableName,
                                       StringComparison.InvariantCultureIgnoreCase));
                var inventoryItemTable = tables.SingleOrDefault(
                    x => string.Equals(x.Field<string>(tableNameColumnName),
                                       inventoryItemTableName,
                                       StringComparison.InvariantCultureIgnoreCase));

                itemTable.Should().Be.Null();
                inventoryItemTable.Should().Not.Be.Null();
            }
        }

        protected override bool AppliesTo(Dialect.Dialect dialect)
        {
            return dialect is MsSql2008Dialect;
        }

        protected override HbmMapping GetMappings()
        {
            var mapper = new ModelMapper();
            mapper.Class<EntityBase>(
                m => m.Id(x => x.Id, im => im.Generator(Generators.HighLow)));
            mapper.UnionSubclass<Item>(m => { });
            mapper.UnionSubclass<InventoryItem>(m => { });

            return mapper.CompileMappingForAllExplicitlyAddedEntities();
        }
    }
}

[mahara]

@hazzik Are these tests OK? If they are, I'll commit and push them. I've verified them all against MSSQL 2012.

[hazzik]

yes, tests are good

[hazzik]

We have second PR to solve this issue #328