Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Model binding to Controller Properties #1902

Closed
yishaigalatzer opened this issue Jan 28, 2015 · 2 comments
Closed

Model binding to Controller Properties #1902

yishaigalatzer opened this issue Jan 28, 2015 · 2 comments

Comments

@yishaigalatzer
Copy link
Contributor

@harshgMSFT please update this comment with the suggested design.

Including how we deal with [FromBody] appearing twice, is there a TryUpdateModel for the controller? What we do about validation?

Initial thoughts:

Representation :

Controller properties are treated as additional action parameters which are common for all actions.
This gives us a single place where we can bind all controller properties ( which are interesting, more on this later). along with actual action parameter. This will ensure things like validation and handling of multiple [FromBody] attributes is uniform and is at one place ( model binding phase).
Also since the information about the controller properties (of interest) will be part of Action descriptor it will also be available to api explorer.

Since action parameters are passed as a dictionary to any action filters we will have a custom dictionary with source as the controller object. This will ensure any modifications to the dictionary reflect on the controller instance.
This would also mean any conflicts in name would be disastrous as they will override each other's value. I would recommend this dictionary to be case sensitive ( which is the case currently) to avoid name clashes.

Discovery of Controller properties:

The discovery happens when action descriptor is created, viz. in ControllerActionDescriptorBuilder

There are four categories of public properties which can be defined on the controller

  1. Those without any Model binding related attributes (eg From*, ModelBinder etc). and without any Activate attribute.
  2. Those with only activate.
  3. Those with only model binding related attribute.
  4. Those with both activate and model binding attributes.

Case 1 and 3 should be considered as action parameters while case 2 and 4 should not.
Case 4 is weird and we should consider it as an invalid scenario and throw.

Also we should provide an opt out mechanism for controller properties which need to be excluded from model binding. We can consider this to be similar to [BindNever].

@yishaigalatzer yishaigalatzer added this to the 6.0.0-rc1 milestone Jan 28, 2015
@yishaigalatzer yishaigalatzer changed the title Model binding to Model Properties Model binding to Controller Properties Jan 29, 2015
@harshgMSFT
Copy link
Contributor

Decisions :

  1. The controller properties are not present in the actionArguments dictionary. If a filter needs it, he can get it via controller object.
  2. Activate attribute will be no longer applicable for Controller ( will still be used for view components etc). The activate concept for controllers will be replaced by a new attribute like FromBinding (or ModelBinderAttribute.. the jury is still out on that.).
  3. Controller properties are opt in.
  4. Controller Properties will be added to actionDescriptor.Parameters collection.
  5. Validation, ApiExplorer, Errors for multiple FromBody, work as they look at actionDecriptor for parameters.

@harshgMSFT
Copy link
Contributor

adeb1ba

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants