Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: FluentMapper is not working with Abstract Properties #666

Closed
mikependon opened this issue Dec 7, 2020 · 11 comments
Closed

Bug: FluentMapper is not working with Abstract Properties #666

mikependon opened this issue Dec 7, 2020 · 11 comments
Assignees
Labels
bug Something isn't working todo Things to be done in the future working-as-expected The reported issue is working as expected.

Comments

@mikependon
Copy link
Owner

Just tried the FluentMapper and found a problem with abstract properties. RepoDb.FluentMapper.Entity().DbType(e => e.CustomerTypeId, DbType.Int32);

Created by @SpaceOgre from #633.

My classes look like this

    public abstract record Customer
    {
        public int Id { get; init; }
        abstract public CustomerType CustomerTypeId { get; }
        public string CustomerNumber { get; init; }
    }

    public record PrivateCustomer : Customer
    {
        public override CustomerType CustomerTypeId => CustomerType.Private;
        public string SocialSecurityNumber { get; init; }
        public string FirstName { get; init; }
        public string LastName { get; init; }
        public string ChangedBy { get; init; }
    }

If I use the Attribute it works, but not the FluentMapper. If I remove abstract and change override to new then the FluentMapper works.

@mikependon mikependon added bug Something isn't working todo Things to be done in the future labels Dec 7, 2020
@mikependon mikependon self-assigned this Dec 7, 2020
@mikependon mikependon pinned this issue Dec 8, 2020
@mikependon
Copy link
Owner Author

@SpaceOgre, same findings, this issue is fixed at the latest beta version RepoDb v1.12.5-beta4. Would you be able to install such beta version and test your solution there?

@mikependon mikependon added wontfix This will not be worked on working-as-expected The reported issue is working as expected. labels Dec 8, 2020
@SpaceOgre
Copy link
Contributor

Tested with v1.12.5-beta4

With the classes looking like above and using

RepoDb.FluentMapper.Entity<PrivateCustomer>().DbType(e => e.CustomerTypeId, DbType.Int32);

and calling with ExecuteScalarAsync I still get this error:
Microsoft.Data.SqlClient.SqlException (0x80131904): Conversion failed when converting the nvarchar value 'Private' to data type int.

@mikependon
Copy link
Owner Author

I am pretty sure I had tested the abstract properties from the abstract classes using the latest beta version, but that is using the .NET Core 3.1.

But yes, please leave this one open, I may be revoking the "labels" in placed and do the fix for this (if needed). If I can't replicate it as it is also working on our DEV ENV (and the latest beta as we tested), then I might be needing a small project from you that replicate this.

But while we are investigating it, you do not have any action items for now. Hope that is okay.

@mikependon mikependon removed working-as-expected The reported issue is working as expected. wontfix This will not be worked on labels Dec 11, 2020
@mikependon
Copy link
Owner Author

@SpaceOgre - I again retested this in our Integration Tests and even recreated the project solution, the issue is rectified in v1.12.5-betaX. Please see attached project for your reference.

Project: RawSqlExecutionForEnum.zip

Or, would you be able to replicate it using the attached project and attached it back from here? Please note that I am using the .NET Core 3.1 version.

@mikependon mikependon unpinned this issue Dec 11, 2020
@SpaceOgre
Copy link
Contributor

I will check it out and test on Monday.

@mikependon
Copy link
Owner Author

Thanks. Even though I cannot replicate this problem using the latest build, but when I am fixing the #668, I have made some direct fix in relation to the abstract properties when being configured via FluentMapper. Therefore, it is very important that this issue must be replicated by you and revert back to me so it will surely be covered by the next beta. Please do let me know ASAP. Thanks a lot.

@SpaceOgre
Copy link
Contributor

SpaceOgre commented Dec 15, 2020

@mikependon I have tested with your project now and have these findings:
Works

FluentMapper.Type<CustomerType>().DbType(System.Data.DbType.Int32);

Don't Work

FluentMapper.Entity<Customer>().DbType(x => x.CustomerTypeId, System.Data.DbType.Int32);

Project: RawSqlExecutionForEnum.zip

@mikependon
Copy link
Owner Author

@SpaceOgre - great, and I can replicate this one now, but it seems that this kind of problem is already solved by the updates I made on the issue #654

Anyway, I will give you an update later and will probably issue a beta later night or tomorrow morning. :)

@mikependon
Copy link
Owner Author

mikependon commented Dec 17, 2020

@SpaceOgre - I had investigated and analyzed this one further, only to realize that the library is just doing it as expected.

It is correct that it should throw an exception if you are to use the code below.

FluentMapper.Entity<Customer>().DbType(x => x.CustomerTypeId, System.Data.DbType.Int32);

Due to the fact that the ExecuteScalar is a non-model based call. I could not find a way to link the new { .. } into the model Customer at all, and I could not pre-assume that, therefore this property-level will never work (as well).

The reason why the type-level mapping is working, is due to the fact that it ignores the binding in the model, but instead, it is inspecting the type of the property instead.

I would say this one as "not a bug". Hope this explains well.

[EDIT]

P.S: I will close this one and will issue a beta.

@mikependon mikependon added the working-as-expected The reported issue is working as expected. label Dec 17, 2020
mikependon added a commit that referenced this issue Dec 17, 2020
#666 - Fixed the related issues found at model-based operation Execut…
@SpaceOgre
Copy link
Contributor

@mikependon Ah that explains it well :)

@mikependon
Copy link
Owner Author

Yeah, but TBH, I also saw a bug on the model-based operation ExecuteQuery, so I fixed it and just reference it to this report. Almost related ~ please make sure to use the v1.12.5-beta6, not the beta5 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working todo Things to be done in the future working-as-expected The reported issue is working as expected.
Projects
None yet
Development

No branches or pull requests

2 participants