FluentNHibernate 1.1 automapper doesn’t accept read-only properties anymore

Sandor Drieënhuizen's Avatar

Sandor Drieënhuizen

07 Aug, 2010 11:50 PM via web

Today I upgraded my project to FluentNHibernate 1.1. Ever since, when running my project I get the following exception:

"Could not find a setter for property 'FullName' in class 'MyNamespace.Employee'"

The code for the FullName property is as follows:

public virtual string FullName
{

get
{
    return FirstName + " " + LastName;
}

}

Well yes, there is no setter indeed. But it used to work prior to version 1.1.

Now I know there are probably a few good arguments in the spritit of separation of concerns, for not implementing such calculated (non-persistent) property but let's just assume I'm doing the right thing here.

Why does FluentNHibernate suddenly require a setter for each property? I don't want to add one, it really wouldn't make sense in this case.

Perhaps there's something I can change to the conventions but I'm a bit puzzled here...

  1. Support Staff 2 Posted by James Gregory on 07 Aug, 2010 11:55 PM

    James Gregory's Avatar

    Hanlon's razor :)

    A bug, by the sounds of it.

    See if you can work around it by overriding the ShouldMap(Member) method in IAutomappingConfiguration. See this post for details on it, if you're not using an external configuration yet.

  2. 3 Posted by Sandor Drieënhuizen on 08 Aug, 2010 12:09 AM

    Sandor Drieënhuizen's Avatar

    I had to look that up. So basically you're saying "Murphy's law" is at work here :)

    I'm already using the new external configuration approach, so I can use the ShouldMap override. However, I'm wondering in what direction you're thinking actually because as far as I can tell, I can only decide whether to map a type or not in ShouldMap, while I'm looking for a way to change/ignore the mapping of a specific property (that is, a property without a setter).

  3. 4 Posted by Sandor Drieënhuizen on 08 Aug, 2010 12:12 AM

    Sandor Drieënhuizen's Avatar

    To continue my reply, I was looking into altering an existing implementation of the IPropertyConvention interface. I was thinking to detect whether the passed property is read-only and subsequentially ignore the mapping or something. Is that an OK direction?

  4. Support Staff 5 Posted by James Gregory on 08 Aug, 2010 12:57 AM

    James Gregory's Avatar

    Hanlon's razor is:

    Never attribute to malice that which is adequately explained by stupidity.

    What I'm saying is, It wasn't an intentional change. It's a bug.

    As for the work around: There are two ShouldMap methods in IAutomappingConfiguration. One with a Type parameter, which you have described, and another with a Member parameter; it's that second method you want to override, as it gives you control over which individual properties are mapped (exactly what you've described as wanting to do). You can see them both in the api docs under ShouldMap.

  5. 6 Posted by Sandor Drieënhuizen on 08 Aug, 2010 10:36 AM

    Sandor Drieënhuizen's Avatar

    Thanks, that overload was exactly what I needed. Here's what I did:

    public override bool ShouldMap(Member member)
    {
        if (member.IsProperty && !member.CanWrite)
        {
            return false;
        }
    
        return base.ShouldMap(member);
    }

    Shouldn't this be default behavior? I mean, mapping read only properties doesn't make a lot of sense.

  6. 7 Posted by abel.online on 26 Sep, 2010 12:00 PM

    abel.online's Avatar

    I mean, mapping read only properties doesn't make a lot of sense.

    Often it does, actually: most primary key identifiers are read-only by default, for instance. Also, suppose a scenario where tables or fields are filled externally (CreationDate field, or a user table that's maintained by another system). In these cases, a private setter is still needed to fill these fields with data, but these fields are conceptually read-only and CanWrite will return false for them.

  7. Support Staff 8 Posted by Paul Batum on 26 Sep, 2010 03:29 PM

    Paul Batum's Avatar

    But I think we all agree that the behavior is it currently stands is less
    than ideal. I had to make the same override for ShouldMap in my most recent
    FNH project. This broke at some point. I remember doing some work on CanRead
    and CanWrite to fix some other bug, maybe I inadvertently broke this?

    On Sun, Sep 26, 2010 at 7:02 AM, abel.online <
    ***@tenderapp.com<tender%***@tenderapp.com>
    > wrote:

  8. Support Staff 9 Posted by Paul Batum on 26 Sep, 2010 03:43 PM

    Paul Batum's Avatar

    Okay, I did a quick blame and now I understand. Alot of people, including
    myself, do this:

    private IList<Order> _orders;

    public IEnumerable Orders
    {
      get { return _orders; }
    }

    James made it so that the automapper can handle this. I think this is
    probably the only case where you do want to map a read only property, so
    maybe we should just make the code more specific. I am thinking that if a
    readonly property returns an IEnumerable, we map it, if not, we don't.

    What do you think James? I am happy to make this change if it makes sense to
    you.

    Paul.

  9. Support Staff 10 Posted by James Gregory on 26 Sep, 2010 05:11 PM

    James Gregory's Avatar

    That makes sense to me. I had someone complain to me the other day about
    mapping read-only properties too, so it's probably a good idea.

    On Sun, Sep 26, 2010 at 4:45 PM, Paul Batum <
    ***@tenderapp.com<tender%***@tenderapp.com>
    > wrote:

  10. 11 Posted by abel.online on 26 Sep, 2010 06:03 PM

    abel.online's Avatar

    While I think it's a good idea, it wouldn't even be that much of a problem currently, if you could simply override the default behavior. But that doesn't always work as one would expect with these readonly properties: see this post where using OverrideAll fails. Here's a similar scenario, where ignoring a readonly IEnumerable property doesn't work.

    In a way, a readonly property can never be set (well, not without ugly hacks) when it's holding a value type. If it is any reference type apart from a string, it would make sense to map it, even though they may need special mapping treatment.

    I'd say, ignore all value types, re-enable OverrideAll plus IgnoreProperties to override this behavior and include reference types.

  11. Support Staff 12 Posted by Paul Batum on 26 Sep, 2010 08:58 PM

    Paul Batum's Avatar

    I don't follow - you can override the default behavior. Inherit from
    DefaultAutomappingConfiguration, override ShouldMap, done!

    I looked at the two issues you linked to, they all look like they would be
    solved with a change to ShouldMap to allow readonly IEnumerables and to
    ignore everything else.

    On Sun, Sep 26, 2010 at 1:05 PM, abel.online <
    ***@tenderapp.com<tender%***@tenderapp.com>
    > wrote:

  12. 13 Posted by abel.online on 27 Sep, 2010 05:54 AM

    abel.online's Avatar

    Maybe I shouldn't have put it in this thread. I understand that overriding ShouldMap should work. But AutoMappings.OverrideAll(map => map.IgnoreProperties("XProp", "YProp") should also work, as a shorthand. But it doesn't.

    The other part of my comment was about whether or not choosing IEnumerable as the default to include, even when it's readonly, in the next release. It's perhaps too restrictive. Sorry if I chose my words poorly.

  13. Support Staff 14 Posted by Paul Batum on 28 Sep, 2010 03:14 AM

    Paul Batum's Avatar

    Ahh, right, yes there seems to be some other bug with OverrideAll. I haven't
    looked at that yet.

    As for the IEnumerable, I expect it will come from some virtual method that
    people will be able to override.

    On Mon, Sep 27, 2010 at 12:56 AM, abel.online <
    ***@tenderapp.com<tender%***@tenderapp.com>
    > wrote:

  14. 15 Posted by Steve Kellaway on 27 Oct, 2010 09:23 AM

    Steve Kellaway's Avatar

    Has this issue been fixed yet?

    I have tried to use the automapper twice before but always run into issues with readonly properties.

    I always use conventions to ensure properties (id, hasmany, references etc) are read using their fields, regardless of whether they are readonly or not e.g.:

    instance.Access.PascalCaseField(PascalCasePrefix.Underscore);

    The debugger hits the break points I put in to check they are being applied but I still get errors such as field 'xyz' cannot be found when the field is called '_Xyz'.

    Build 685 appears never got past trying to find 'id' when it was '_Id', now Build 695 appears to be ignoring access conventions on properties.

    At this rate I will have to kick the automapper out of the major project my department is starting and take on the overhead of explicit mappings :(

  15. Support Staff 16 Posted by James Gregory on 27 Oct, 2010 10:11 AM

    James Gregory's Avatar

    This was answered above. Read-only properties are supported if you customise your automapping configuration: see here, specifically you'll want to override ShouldMap(Member) to not exclude read-only properties.

  16. 17 Posted by Steve Kellaway on 27 Oct, 2010 11:40 AM

    Steve Kellaway's Avatar

    Some progress :) I've included the read-only properties after customising the automapping configuration. I hadn't understood these properties were being skipped altogether. I thought the issue was conventions were not being applied properly.

    The Id's now work with the convention applying 'instance.Access.PascalCaseField(PascalCasePrefix.Underscore)' but the following property has a similar issue:

        private IList<SupportRequestStateItem> _StateLog;
        public IList<SupportRequestStateItem> StateLog
        {
            get { return _StateLog; }
        }

    In the debugger, I step over the Id convention and see the change from the default {nosetter.camelcase} to {field.pascalcase-underscore} but stepping over the similar line of the HasMany convention, the change does not occur and eventually I get PropertyNotFoundException 'Could not find field 'stateLog' ...'

    Using build 695.
    '

  17. Support Staff 18 Posted by James Gregory on 05 Nov, 2010 02:01 PM

    James Gregory's Avatar

    Hmm, that sounds like a bug there! I've created an issue and we'll get onto it when we can. Until then, you might try an IAutomappingOverride for those entities with the collections.

  18. 19 Posted by Steve Kellaway on 05 Nov, 2010 06:39 PM

    Steve Kellaway's Avatar

    I was afraid it might be. I've gone back to mapping files for now as the number of entities will be quite low for the time being.

Reply to this discussion

Preview Comments are parsed with Markdown. Help with syntax

Attached Files

    You can attach files up to 10MB

    What is the last month of the year?