Support for entity-maps (dictionaries with entity keys) of entities, composite-elements and/or elements.

[Morten Maxild]

Hi,

 

I (and others) have been struggling with mapping a dictionary with entity keys using FNH. Only when using HasManyToMany are ternary associations supported at the moment. But I guess most people would more often use HasMany to define an entity-map (i.e. dictionary with entity keys) when having a collection of children. I need this mapping, and would like to see it in the official master branch.

 

I have relatively little experience with the FNH codebase but have managed to come up with the update for supporting HasMany-entity-maps. The update can be seen from the entity-map branch of my fork that has one commit.

 

I have also fixed a bug that happens when having an indexed collection (map) inside a component. See the commit for more info

 

Kind regards

Maxild

[maxild]

I hate to reply to my own message, but are the project interested in
supporting this NH mapping? I have send a pull request to James, but
should I create a ticket?

On Sep 7, 11:21 pm, "Morten Maxild" <mmax...@gmail.com> wrote:
> Hi,
>
> I (and others <http://groups.google.com/group/fluent-nhibernate/browse_thread/thread...> ) have been struggling with mapping a dictionary with entity keys using FNH. Only when using HasManyToMany are ternary associations supported at the moment. But I guess most people would more often use HasMany to define an entity-map (i.e. dictionary with entity keys) when having a collection of children. I need this mapping, and would like to see it in the official master branch.
>
> I have relatively little experience with the FNH codebase but have managed to come up with the update for supporting HasMany-entity-maps. The update can be seen from the entity-map branch of my fork that has one commit <http://github.com/maxild/fluent-nhibernate/commit/f485d12f20b120e19e2...> .

[Mikael Henriksson]

James is on vacation for a couple of weeks more so development is on hold for now.

[maxild]

I see. Do anybody know when planning/development for vNext starts?

Many people have forked the project. What people/group should receive
a pull request?

On Sep 14, 11:59 am, Mikael Henriksson <mik...@zoolutions.se> wrote:
> James is on vacation for a couple of weeks more so development is on hold
> for now.
>

[Paul Batum]

Hi Morten,

Work is progressing in my repository while James is away. I expect that the changes I've made including the patches I've applied will be merged into the official trunk when James returns.

Thanks for your contribution,  I'll take a look at your changes and get back to you soon.

Paul Batum

[Morten Maxild]

Thanks.

 

One thing to look at is that my API on OneToManyPart is a bit smaller than the corresponding API on ManyToMamyPart. Also I have introduced a member called IsEntityMap(). I guess the API’s should be more consistent.

 

Give me a sign if/when something have been committed to master, or let me know if you need help with anything.

 

/Morten

---

[Paul Batum]

Hi Morten,

I finally had a chance to look at your patch today, and I was wondering if there is any reason why you didn't provide an overload for AsTernaryAssociation that takes the custom column names much like the ManyToManyPart implementation does? Is this the inconsistency you were referring to in your previous message?

If there is no particular reason, then I'll implement the overload and include it with your changes.

Cheers,

Paul Batum

[Morten Maxild]

Hi Paul,

 

Yes this was the inconsistency, I was referring to. Off course a OneToMany only has an overload with a single ‘string indexColumnName’ parameter, to pass an arg (e.g. ‘MyNameFoTheKeyOrIndexColumn’) to get this xml

 

<index-many-to-many class="...">

  <column name="MyNameFoTheKeyOrIndexColumn" />

</index-many-to-many>

 

Also the AsEntityMap method (that isn’t found on ManyToManyPart, another inconsistency) should have the same overloaded as well, or be dropped from the API. IMO it reads well.

 

Finally I believe that the (new) ‘string indexColumnName’ parameter on AsTernaryAssociation/AsEntityMap shouldn’t be confused with the parameter on the AsMap() method, as shown by this test the former controls the column-name on the index element (as opposed to the index-many-to-many element). This is a confusing, I guess.

 

[Test]

        public void CanSpecifyCollectionTypeAsMapWithStringColumnName()

        {

            new MappingTester<OneToManyTarget>()

                .ForMapping(map => map

                    .HasMany(x => x.MapOfChildren)

                        .AsMap("Name")

                        .KeyColumns.Add("ParentId"))

                .Element("class/map/key/column").HasAttribute("name", "ParentId")

                .Element("class/map/index/column").HasAttribute("name", "Name")

                .Element("class/map/one-to-many").HasAttribute("class", typeof(ChildObject).AssemblyQualifiedName);

[Paul Batum]

Hi Morten,

My apologies for the severe delay, but I've made some additional changes to the entity map stuff based on our discussion in this thread and I was wondering if you would mind sanity checking it?

http://github.com/paulbatum/fluent-nhibernate/commit/c6cbb16042b86e7a79839a6dbbd9fd2630eadbd8

Cheers!

Paul Batum

[Morten Maxild]

I’ll take a look tonight, and provide you some feedback:-Morten

[Morten Maxild]

Hi

 

First, Do you now when James will pull, or do you have access to push to the ’official’ master?

 

Then, my feedback….first all tests passJ

 

Your new specs do not verify the rendered xml through xpath queries, but test the internal semantic model (didn’t know about ModelShouldMatch to verify internal semantic model). And you left my tests that specify the expected rendered xml. I guess you can have too many tests, but in this case I like both, and leaving mine is nice documentation trying to learn FNH with a NH background (easier to read tests specifying the hbm/xml output according to the NH XSD grammar)?  

 

I guess you could delete the ManyToManySubPartModelGenerationTests.cs, as it is empty. 

 

Also the fix to the XmlClasslikeNodeSorter is kind of hidden in the commit (no issue, my mistake), but I hope you guys can live with that (some of the tests, EntityMapOfComplexValuesXXXX, are failing tests without the fix, because they verify the correct position of the child elements ‘key’, ‘index-many-to-many’ and ‘composite-element’)

 

Nice work, looking forward to delete my entity-map-branch

[Paul Batum]

Hi Morten,

Yeah I didn't really want to rewrite your tests. We already have this situation where FNH has a variety of tests, some that are more like integration tests in that they are fluent interface -> xml while others are more like unit tests, fluent interface -> model, and model -> xml. I'm happy to accept that we will have this mix of both styles for quite some time, and they both have their uses. That said, to me having unit style tests is more important so I choose to write those when given the chance. In my mind the best blend is probably comprehensive unit test coverage and then the occasional integration style test to help, as you mentioned, the NH reader trying to learn FNH.

As for the pull into official, I can't say. James is the gatekeeper. I sent him a pull request a few days ago but I'm not sure if he's caught up on his mountains of email that were awaiting his return from holiday. I'll continue to push development ahead on my dev branch with changes from both myself and others. The entity map work has been merged into my dev branch, so if you want to get rid of your entity map branch you can just switch to using my dev branch while we wait for James to catch up. Its at:

http://github.com/paulbatum/fluent-nhibernate/tree/dev

Paul Batum