ConvertEmptyStringToNull stopped being honored after upgrading from RC1

[rsheptolut]

I had a website working on ASP.NET 5 RC1 and it broke after updating to ASP.NET Core 1.0.
It depended on a property ConvertEmptyStringToNull of class DisplayMetadata. I'm setting it to false for all types and properties using IDisplayMetadataProvider, that was added to ModelMetadataDetailsProviders collection of MvcOptions using AddMvc() in Startup.cs.

My case: I have a link http://localhost/Configuration/Display?ShopId=R505&Name=
I also have controller "Configuration" and action "Display" that takes a Configuration object, which has "ShopId" and "Name" string properties. In method Display() I need ShopId to be "R505" and Name to be "" (string.Empty) in the Configuration object. But after the upgrade the Name property has started to come as null. I can expand on why I need it to be an empty string, but this is probably not relevant. I'll just say that empty-string name is the default configuration for that shop, but there are also configurations with other names of course, I have no problem with those.

I also tried to apply [DisplayFormatAttribute(ConvertEmptyStringToNull = false)] directly to the Configuration.Name property, but it also didn't work. This property just makes no difference now. Seems like a bug.

P.S. To upgrade from RC1 I created a new blank ASP.NET Core app (so all package versions are at a default) and then added my class files from the old RC1 project, reworked Startup.cs and renamed/changed a bunch of stuff that was changed since RC1 (not very much).
I'm trying this locally in Windows 8.1 in IIS Express with Visual Studio 2015 update 3 and latest ASP.NET Core Tools installed.

[dougbu]

Thanks very much for the report.

This block in SimpleTypeModelBinder never leaves model==string.Empty. The ConvertEmptyStringToNull check in that block doesn't help because modelAsString==null when value==string.Empty. ModelMetadata.ConvertEmptyStringToNull is no longer relevant in model binding.

FYI the problem is a regression introduced in 4e97c72.

[rsheptolut]

@dougbu I just stared for 10 minutes flat at that block of code, and still couldn't grasp why it's not working. It's like you were looking at a different patch of code when you were describing the issue. Or maybe I'm just not real bright. From what I see, the bug can only be inside "_typeConverter.ConvertFrom()", or the value variable is null from the start somehow. Because in my case, ConvertEmptyStringToNull is false and will not let "model = null" to execute, so model should stay string.Empty, which means something else is nulling it. My problem is, I need string.Empty, but I get null.

But anyway, this issue is a bug and it will be fixed, right? :)

[dougbu]

@rsheptolut well, I had the advantage of debugging a repro of the problem 😺 In any case, the issue relates to how model is set when value is string.Empty. The first condition in that block is false and there's no else to change model from its initial null.

Yes, this is a bug and it should be fixed.

[DanielLaberge]

@rsheptolut Could you provide an example of the MvcOptions call where you set ConvertEmptyStringToNull?

I couldn't find a single working example anywhere.

Thanks,

[dougbu]

fae0e9a

[rsheptolut]

@DanielLaberge In case you (or someone) still need it, that's how you're supposed to set ConvertEmptyStringToNull through a metadata provider (which is an alternative to applying [DisplayFormatAttribute(ConvertEmptyStringToNull = false)] manually to every property):

public class Startup
{
    // ...

    public void ConfigureServices(IServiceCollection services)
    {
        // ...
        services.AddMvc()
            .AddMvcOptions(o => o.ModelMetadataDetailsProviders.Add(new MyDeclarativeMetadataProvider()));
        // ...
    }
}

public class MyDeclarativeMetadataProvider : IMetadataDetailsProvider, IDisplayMetadataProvider
{
    public void CreateDisplayMetadata(DisplayMetadataProviderContext context)
    {
        // Some logic to determine what metadata we're dealing with here, for example:
        if (context.Key.MetadataKind == ModelMetadataKind.Type)
        {
            // And that's how you set ConvertEmptyStringToNull
            context.DisplayMetadata.ConvertEmptyStringToNull = false;
        }
    }
}

[dougbu]

FYI neither [DisplayFormat] nor @DanielLaberge's global metadata update will work unless you're on the bleeding edge. The fix for this issue is in the dev branch and will be available in 1.1.0.

[kinetiq]

@rsheptolut That is very helpful, even a year later. I'm sure a lot of people are still finding this.

One small tweak on the ModelMetaDataKind... Based on what I'm seeing, it should be Property:

           if (context.Key.MetadataKind == ModelMetadataKind.Property)
            {
                context.DisplayMetadata.ConvertEmptyStringToNull = false;
            }

[rsheptolut]

@kinetiq Yep, you're probably right :)