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

Future Investments: Model Binding #4839

Closed
rynowak opened this issue Jul 14, 2017 · 7 comments
Closed

Future Investments: Model Binding #4839

rynowak opened this issue Jul 14, 2017 · 7 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Milestone

Comments

@rynowak
Copy link
Member

rynowak commented Jul 14, 2017

This is a place for tracking feedback and related bugs for scoping future investments in Model Binding.

Related Issues

#6109 Question: call ModelBinding manually?
#4437 Using validation without model state and controller
#6014 Using DI as a factory and then model binding the object
#4718 Add a secure binding option to model binders
#1786 Support binding immutable types by binding to the constructor

@mrahhal
Copy link
Contributor

mrahhal commented Aug 18, 2017

I'd very much like to see manual model binding possible. I asked a long time go at #4811 and it seems there's still no way to accomplish such a thing without letting go of the advantages that model binding/validation brings.

Making model binding/validation not directly coupled to mvc will also be a super nice thing to have.

@fairking
Copy link

fairking commented Aug 12, 2018

My temporary solution:

public class BaseController : Controller
{
        public bool IsAjaxRequest()
        {
            if (Request == null)
            {
                throw new ArgumentNullException("request");
            }

            return Request.Headers["X-Requested-With"] == "XMLHttpRequest";
        }

        public override async Task<bool> TryUpdateModelAsync(object model, Type modelType, string prefix)
        {
            if (model == null)
                model = Activator.CreateInstance(modelType);
      
            var result = await base.TryUpdateModelAsync(model, modelType, prefix);
      
            if (IsAjaxRequest())
            {
                try
                {
                    using (var reader = new StreamReader(Request.Body))
                        JsonConvert.PopulateObject(reader.ReadToEnd(), model);

                    return true && result;
                }
                catch (Exception exc)
                {
                    var logger = HttpContext.RequestServices.GetService<ILogger<BaseController>>();
                    if (logger != null)
                        logger.LogWarning(exc, "Error occurred while parsing '{0}' model.", modelType.Name);
                }

                return false;
            }
            else
            {
                return result;
            }
        }

        public virtual Task<bool> TryUpdateModelAsync(object model, Type modelType)
        {
            return TryUpdateModelAsync(model, modelType, null);
        }

        public override Task<bool> TryUpdateModelAsync<TModel>(TModel model, string prefix)
        {
            return TryUpdateModelAsync(model, typeof(TModel), prefix);
        }

        public override Task<bool> TryUpdateModelAsync<TModel>(TModel model)
        {
            return TryUpdateModelAsync(model, typeof(TModel));
        }
}

Please correct me if I am doing something weird in the code or there is some better solution.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Mvc Dec 13, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 13, 2018
@aspnet-hello aspnet-hello added 0 - Backlog area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels Dec 13, 2018
@KrzysztofBranicki
Copy link

@rynowak I think that your link to issue about support of binding immutable types is pointing to something unrelated. Anyway this feature would be highly appreciated especially when you design CQRS style API where commands and queries by definition should be immutable. BTW even EntityFramework supports deserializing entity via constructor nowadays so why MVC model binders still require public setters and by doing so discourage good practices like immutable messages/DTOs.

@msschl
Copy link
Contributor

msschl commented Aug 5, 2020

#1786 Support binding immutable types by binding to the constructor

@rynowak, I would very much appreciate this feature. 😍 Now that System.Text.Json supports JsonConstructor in .net 5.0, it feels incomplete not having the ability for immutable complex types in asp net core.

@msschl
Copy link
Contributor

msschl commented Aug 30, 2020

@rynowak It looks like #1786 Support binding immutable types by binding to the constructor links to the wrong issue

@pranavkm pranavkm modified the milestones: Backlog, Discussions Oct 14, 2020
@ghost
Copy link

ghost commented Dec 13, 2020

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Dec 13, 2020
@msschl
Copy link
Contributor

msschl commented Dec 13, 2020

Isn’t this issue relevant for .net 6 planning, as areas like mvc are planned to be revisited?

@ghost ghost locked as resolved and limited conversation to collaborators Jan 12, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

7 participants