sapiens / SqlFu

Fast and versatile .net core data mapper/micro-orm
Other
229 stars 50 forks source link

Should allow for a registered converter against a non-value (custom object) property type #16

Closed ventaur closed 11 years ago

ventaur commented 11 years ago

My team often use a Java-like enumerator as opposed to .NET's enum. See below for the main class I use for these property types: https://github.com/HeadspringLabs/Enumeration/

The problem with using a class for a model's property is SqlFu will always treat such "custom object types" as candidates for complex type mapping only. There are cases, like with class enumerations (and other one-offs), where we need such properties to be treated more like a value object. BTW, the above Enumeration class simply does not work as a struct due to the generics use.

It would be nice if SqlFu would use any specifically registered converter, even for a non-value object type property. Pull request is coming shortly.

sapiens commented 11 years ago

I'm reviewing the PR, however I don't understand the test, as it doesn't use the behavior you specified. Perhaps you had this in mind?

 public class TypeWithCustomObject
        {
            public CustomObject Value { get; set; }           
        }

        public class CustomObject
        {
            public string CustomValue { get; private set; }

            public CustomObject(string customValue)
            {
                CustomValue = customValue;
            }
        }

 [Fact]
        public void global_custom_converter()
        {
            PocoFactory.RegisterConverterFor<CustomObject>(o => new CustomObject(o.ToString()));
            var custom = _db.QuerySingle<TypeWithCustomObject>("select 'custom-value' as Value");
            Assert.Equal("custom-value", custom.Value.CustomValue);
        }

The PocoFactory changes are for this case, otherwise that code is never reached.

ventaur commented 11 years ago

You are absolutely correct. That was my intent, but I never fleshed out the test due to the missing db file. Ha! My test is worthless. Yours is perfect. Thanks. On Sep 2, 2013 6:48 AM, "Mihai Mogosanu" notifications@github.com wrote:

I'm reviewing the PR, however I don't understand the test, as it doesn't use the behavior you specified. Perhaps you had this in mind?

public class TypeWithCustomObject { public CustomObject Value { get; set; } }

    public class CustomObject
    {
        public string CustomValue { get; private set; }

        public CustomObject(string customValue)
        {
            CustomValue = customValue;
        }
    }

[Fact] public void global_custom_converter() { PocoFactory.RegisterConverterFor(o => new CustomObject(o.ToString())); var custom = _db.QuerySingle("select 'custom-value' as Value"); Assert.Equal("custom-value", custom.Value.CustomValue); }

The PocoFactory changes are for this case, otherwise that code is never reached.

— Reply to this email directly or view it on GitHubhttps://github.com/sapiens/SqlFu/issues/16#issuecomment-23653176 .

sapiens commented 11 years ago

Great! Can you detaliate more about the problem with sqlite db?

ventaur commented 11 years ago

It's definitely possible I missed something, but when I ran any of the tests against a actual DB (like the QueryTests), they failed due to bad connection to SQL Server (I'm not running it right now). When I switched Tests.Setup to use SQLite instead, it couldn't find testdb.db, but it's there. I have one space in the path to this solution, maybe that's it. I changed the SQLite connection string to quote the data source, but still no success.

I'm sure it's something with my setup. I'll work it out for any future code I may contribute. I'll start with the space in the path. Not sure how that sneaked in there anyway.

On Mon, Sep 2, 2013 at 11:13 AM, Mihai Mogosanu notifications@github.comwrote:

Great! Can you detaliate more about the problem with sqlite db?

— Reply to this email directly or view it on GitHubhttps://github.com/sapiens/SqlFu/issues/16#issuecomment-23665649 .

ventaur commented 11 years ago

I see my issue now. It's the SQL in the tests setup for checking if the Posts table exists and creating it, etc. All of that is targeting SQL Server (which makes sense for the default). When I switched over to SQLite, it couldn't comprehend that stuff. I'll setup SQL Express in the future so the tests work.