Changes to property ModelMetadata provided by a ModelMetadataDetailsProvider are lost for Nullable<T> in Display/Edit templates when DataType is used to change the template

[rwasef1830]

Hello ASP.net team,
This bug is related to #2539, but the fix for that bug doesn't work when the nullable property has a DataType attribute that changes the display / edit templates.

protected ViewDataDictionary(ViewDataDictionary source, object model, Type declaredModelType)
            : this(...)
        {
            // ...

            // This is the core constructor called when Model is known.
            var modelType = GetModelType(model);
            var metadataModelType = source.ModelMetadata.UnderlyingOrModelType;
            if (modelType == metadataModelType && model == source.ModelExplorer.Model)
            {
                // Preserve any customizations made to source.ModelExplorer.ModelMetadata if the Type
                // that will be calculated in SetModel() and source.Model match new instance's values.
                ModelExplorer = source.ModelExplorer;
            }
            else if (model == null)
            {
                // Ensure ModelMetadata is never null though SetModel() isn't called below.
                ModelExplorer = _metadataProvider.GetModelExplorerForType(_declaredModelType, model: null);
            }

            // If we're constructing a ViewDataDictionary<TModel> where TModel is a non-Nullable value type,
            // SetModel() will throw if we try to call it with null. We should not throw in that case.
            if (model != null)
            {
                SetModel(model);
            }
        }

The main part of that fix was this part:

var modelType = GetModelType(model);
var metadataModelType = source.ModelMetadata.UnderlyingOrModelType;
if (modelType == metadataModelType && model == source.ModelExplorer.Model) { ... }

I suspect that the code does not handle nullable types correctly because metadataModelType will never be Nullable<T> whereas modelType can be, so the ModelExplorer is being always discarded in case of nullable properties.

https://github.com/rwasef1830/aspnet-mvc6-modelmetadata-repro

Hope that helps.

[rynowak]

/cc @dougbu

[dougbu]

The problem is only partially about Nullable<T>.

It's true the modelType == metadataModelType && model == source.ModelExplorer.Model check is incorrect when model == null and _declaredModelType (the best guess we have) is Nullable<T>. In this case, metadataModelType will be the underlying type T and not Nullable<T>. E.g. in the original scenario, the code compares DateTime with DateTime? and skips the block maintaining the ModelExplorer and its ModelMetadata.

The chosen ModelMetadata is also incorrect when model != null and happens to have a derived runtime type. The condition mentioned above is correctly false because modelType not exactly metadataModelType. But SetModel() overrides the perfectly-valid ModelMetadata and creates a ModelExplorer for model.GetType(). (That method has special cases to handle Nullable<T> but not a subclass.)

More generally, ViewDataDictionary contains lots of code giving undue preference for the runtime type of a model. This makes sense only when _declaredModelType == typeof(object) && ModelMetadata.MetadataKind == ModelMetadataKind.Type. Time to prefer property metadata if available.

[dougbu]

I take back "This makes sense only when _declaredModelType == typeof(object) && ModelMetadata.MetadataKind == ModelMetadataKind.Type." It never makes sense for ViewData.ModelType to reflect the model's runtime type. ModelExplorer has separate properties for runtime information.

[dougbu]

a0c8834 and merged into dev