with MVC5.
This change removes the behavior in model binding to validate values 'on
the wire' for requiredness instead of the looking at the model. This
restores the behavior of [Required] for model binding to the MVC5
semantics.
Also ensures that when a type is marked as skipped, any sub property which is model bound (and hence a modelstate un validated entry),
is marked as skipped (otherwise it would cause the ModelState to be invalid).
Also fixing a bug in model state dictionary FindKeyWithPrefix was not considering [0] & [0][0] as a valid prefix.
The assumption is ModelState should have entries if
1. An error is explicitly added by a model binder.
2. There is validation error reported while validating the model.
3. There is value bound by the model binder.
With this change there should be no extra entry other than for the cases mentioned above.
Also enabling the integration test cases.
This part of the change removes default value support from ModelBinding.
Updated some unit tests to verify that it does nothing in that case.
Deleted a functional test as it was pure duplication of another
(supported) case where the property has a pre-initialized value.
This change simplifies InputFormatterContext/OutputFormatterContext by
swapping ActionContext for HttpContext.
This change is important especially for InputFormatterContext as it
decouples ModelState from ActionContext - allowing us to fix a
related bug where the _wrong_ ModelState can be passed in for a
TryUpdateModel operation.
Add SerializerSettings to MvcOptions and pass those options to the JsonInputFormatter and JsonOutputFormatter.
Remove custom contract resolver.
PR feedback
Pass JsonSerializerSettings to JsonPatchInputFormatter
PR feedback
Make DI JsonOutputFormatter formatter use MvcOptions SerializerSettings
Fix JsonPatchInputFormatter using null ContractResolver
Fix tests
methods
This change applies the 'override' semantic to attribute routes on virtual
methods.
A method override can either inherit attribute routes from a base
definition, or can replace them. It's not possible to inherit routes and
also add to them.
The change here is to make an attribute route defined on a base class
inherited only if no other routes were defined 'closer' to the controller
class.
To put another way, attribute routes can either be inherited or
overridden, you can't inherit + add your own.
This change dramatically simplifies the parameter discovery logic in
DefaultApiDescriptionProvider. Instead of surfacing POCO objects as
parameters, we now fully expand every model.
The rationale is that we want to show every key/value that can be set by
the user and not force consumers of ApiDescription to do that themselves.
Tests are cleaned up to match the new behavior.
- Mvc is currently broken on CoreCLR because it is
inheriting dependencies from Microsoft.AspNet.Mvc.Common but the resulting
dependency to Microsoft.AspNet.Mvc.Common is then erased at pack time.
This change moves the dependencies down and makes the shared package
only depend on System.Runtime.
#2507
Added `UseMvcWithDefaultRoute` extension method to `BuilderExtensions`,
which adds Mvc to the request pipeline with a default route to the
`Index` action on the `HomeController`.
#1885
Creates a new package 'Microsoft.AspNet.Mvc.Abstractions' which defines
APIs and contracts for core concepts and extensibility points in MVC.
Includes:
- ModelBinding
- Validation
- Model State
- Model Metadata
- Action Descriptors
- IActionResult
- Filters
- IActionConstraint
- part 1/2 of #2294
- handle readonly non-`null` collections in relevant binders
- `CollectionModelBinder.CopyToModel()` and `MutableObjectModelBinder.AddToProperty()` methods
- handle read-only controller properties in `DefaultControllerActionArgumentBinder`
- do not copy into arrays e.g. add `CopyToModel()` override in `ArrayModelBinder`
- remove ability to set a private controller property
- confirm `SetMethod.IsPublic` in `DefaultControllerActionArgumentBinder`
- avoid NREs in `GetModel()` overrides
Test handling of readonly collections
- previous tests barely touched this scenario
- also add more tests setting controller properties
nits:
- add missing `[NotNull]` attributes
- add missing doc comments
- consolidate a few `[Fact]`s into `[Theory]`s
- simplify some wrapping; shorten a few lines
- remove dead code in `DefaultControllerActionArgumentBinder` and `ControllerActionArgumentBinderTests`
Consider
public class Person
{
[FromBody]
public Address Address { get; set; }
}
public class Address
{
[Required]
public string Street { get; set; }
public int Zip { get; set; }
}
Request body { "Zip" : 12345 }
In this case the error key would be "prefix.Address.Street" (assuming there is a prefix because of additional metadata/positioning for/of the Person model).
public class Person
{
[Required]
public string Name { get; set; }
}
public void Action([FromBody]Person p)
{
}
Request body { }
In this case the prefix gets ignored and the error key is Name.
Please note this is so that we are compatible with MVC 5.0
public class Person
{
[Required]
public string Name { get; set; }
}
public void Action([FromBody][ModelBinder(Name = "prefix")] Person p)
{
}
public void Action2([FromBody][Bind(Name = "prefix")] Person p)
{
}
Request body { }
In both these cases (Action and Action2) the prefix gets ignored and the error key is Name.
This is a slight improvement from mvc, as in MVC the action parameter would be null.
The followup for this would be to fix#2416 -
This PR ignores the validation assuming that #2416 will address the issues and update the test.
NOTE: previous versions of mvc did not have property binding and hence there is no precedence in this case. For MVC and Web API it was possible to body bind an action parameter which used an empty prefix instead of a parameter name for adding errors to model state (In case of MVC if a custom prefix was provided, it failed binding from body i.e the parameter was null).
This change moves [BindingBehavior(...)] and friends into the model
metadata layer, and removes the reflection code from
MutableObjectModelBinder that was looking for them previously.
Throw a meaningful exception if the attribute to copy does not exists
in the attributes of the TagHelperContext rather than:
`System.InvalidOperationException : Sequence contains no matching
element`.
Added test too.
This change adds more information to ModelAttributes, so that metadata
providers can look at the attributes on the property and type separately
if so desired
This change introduces a new property to ModelMetadata called
IsBindingRequired, which specifies whether or not a model value must be
present on the wire during model binding.
[DataMember(IsRequired = true)] is currently the only thing that will set
this property.
Updated tests and documentation for clarity on the difference in meaning
between MM.IsRequired and MM.IsBindingRequired. Moved setting for
IsRequired to ValidationMetadata which is a better fit.
Also added functional tests for [BindingBehavior] and [DataMember] in
model binding because they were totally missing.
The fix splits client validation and model validation into two separate hierarchies.
Introduced ClientModelValidatorProvider in MvcOptions, which can be iterated to produce IClientModelValidators.
As a result of this, HtmlGenerator code can be free of ActionBindingContext and directly consumes options.
This also means that we do not modify the client validations during resource filters.
- Also modified existing tests to account for aspnet/Razor#332 which fixed models formatting after newline.
- Updated tests for inject to understand new formatting parsing.
aspnet/Razor#332
- Added unit tests to validate that the properties were rendered correctly.
- Modified functional tests to utilize PreElement and PostElement.
aspnet/Razor#341
- #2261
- include `short`, `ushort`, `float`, `double`
- correct typo in `<input/>` tag helper; ignored calculated format
- only one test for `<input/>`'s calculated format 😦
- fill some `Editor*()` and `<input/>` tag helper test gaps
nit: clean up some trailing whitespace
This change caches the actual model metadata instances. Some profiling
showed we didn't go far enough, we were allocating a lot of ModelMetadata
+ ModelPropertyCollection instances.
- set correct `MarkAsHtmlEncodedMethodName` value in `MvcRazorHost`
- handle `HtmlString` values in `TagHelperOutput.Attributes` in `RazorPage`
- special-case double-quotes in `HtmlString` values
- add `static WriteTo()` method for use in tag helpers
- handle non-`string` `output.Attributes` values in tag helpers
- make `TagHelperContentWrapperTextWriter` a `public` class
- provide a `TagHelperContent.Append(object, ...)` extension method
- add `LinkTagHelper.Href` and `ScriptTagHelper.Src` properties
- avoid Razor HTML-encoding these attribute values before their use
- add `JavaScriptEncoder` properties in `LinkTagHelper` and `ScriptTagHelper`
- allow encoding testing without unit testing the default encoder
- handle MVC and Razor changes for this bug in existing tests
- add functional tests of encodings
- add test encoders to TestCommon project
nits:
- correct `InputTagHelper` to pass `type=""` through unchanged
- set correct `TagHelperContentTypeName` value in `MvcRazorHost`
- remove unnecessary `FormTagHelper.Method` and `OptionTagHelper.Selected` properties
- remove complex ternaries and `ShouldAddFileVersion()` methods
- add a few debug assertions
- fix some odd wrapping
- remove or `#if`-out unused `using`s
- remove trailing whitespace
Adding Equals and GetHashCode implementations - the lack of these results
in a lot of boxing.
Removing dead code, not possible to create a model metadata for a
parameter anymore.
- This solves tooling trying to resolve ChunkInheritanceUtility bits for chunks: Aka can't inherit `@using`, `@inject` etc. from _GlobalImport.cshtml
- Added test to validate paths are normalized for DecorateCodeBuilder.
#2271
- remove useless `configuration` variables and `Configuration` instances
- remove "Review" code comment
- unintentionally included in commit 4b5dd19
- reduce repeated code in `TestHelper` for functional tests
- `CreateServer()` methods had duplicate code, an ambiguous match, and an odd order
- rename `GetTestConfiguration()` to `UseCultureReplacer()` in functional tests
- add `IHtmlGenerator.GetCurrentValues()` method
- bring together bits of `GenerateSelect()` and `UpdateSelectListItemsWithDefaultValue()`
- get rid of ugly `out` parameter
- also allows `<option/>` tag helpers to run before `<select/>` helper generation
- match `null` values and `SelectListItem`s with empty values
- match `enum` names correctly
- add doc comments for `IHtmlGenerator.GenerateSelect()` methods
The DataAnnotationsMetadataProvider was setting the bool? IsRequired, all of the
time instead of only setting it to true when we found a RequiredAttribute.
So we never actually executed the fallback logic here. Found
this while working on removing some reflection code from the validator,
and wanted to split it out because it's simple.
- Tooling passes in rooted paths when asking for a RazorParser for a file. This was problematic when resolving inherited code trees and ultimately this commit tests that.
#2213
- #438 part 1/2 and #2027 part 1/3
slight oddity in the XML docs
- unfortunately Roslyn seems to ignore `TypeInfo` being a subclass of `Type`
nit: use correct `warningsAsErrors` datatype in test project.json
This change removes reflection from validator providers, and instead
relies on cached metadata in in the modelmetadata.
In general this means that our MVPs don't need to cache anything, they
just look at the metadata and create what they need.
In the case of data-annotations, we update the model details provider to
add validation attributes to the modelmetadata. This would allow someone
to replace the DataAnnotationsValidatorProvider, but still use the
metadata in these attributes.
The change to the IModelValidatorProvider api (to use a context) is
intended to minimize allocations. Currently each validator provider needs
to return a list so you end up with N+1 lists (N validators + a final list
to compine them all). This change will let us just create the final list
(and a small context object). This is a very very high traffic API so it
seemed worth doing.
There's also some general massaging of namespaces and file locations.
- #1865
- change `MutableObjectModelBinder` to ignore exact match in value providers
- had an incorrect assumption: don't want exact model name to match since
this binder supports only complex objects
- also ignored `BinderModelName`, value provider filtering, et cetera
- reduces over-binding e.g. `[Required]` validation within missing properties
also add more tests of #2129 scenarios
- #2129
- do not propagate results with `!IsModelSet`, allowing empty prefix fallback
- adjust `ComplexModelDtoModelBinder` to at least fake-bind all properties
- default values not consistently picked up otherwise
nit: correct 2 test names in `KeyValuePairModelBinderTest`
- use named parameters more often
- add more comments about returned `ModelBindingResult`
- clean up `ModelBindingResult` doc comments
- cleanup `using`s
Nits:
- cleanup trailing whitespace
- change `retVal` -> `result` in `KeyValuePairModelBinderTest`
- Updated some naming bits that were outdated (referring to ViewStart).
- Added a TagHelperDirective merging mechanic to ensure user file descriptors and inherited descriptors come together correctly.
#2110
These model binders don't have any per-request state so there is no need
for them to be type activated. In one case the model binder actually does
its own caching which is being defeated by the fact that we register it as
type activated.
More changes to come in this area.
- Any Action<ITagHelper, ViewContext> can be registered in DI to allow setting of tag helper properties after any [Activate] properties are set but before their bound from the source document and the tag helper is executed
- Removed previous tag helper options as initializers is the way to do this now
- #1689
- #2162
- see issues aspnet/DependencyInjection#193 and #1612
- use `TryAdd()` in `MvcServices` for the `IAssemblyProvider`
Move `ApplicationBasePath` handling into `RazorPreCompileModule`
- restores view precompilation in functional tests of sample sites
- if we need this, others will as well
- override `IApplicationEnvironment` but get folder from `ProjectContext`
- also remove remaining `IServiceProvider` implementations in this code
test fixes:
- simplify `IApplicationEnvironment` override
- move override logic into `TestHelper.CreateServer()` methods
- remove `LoggerFactory` setup
- does not seem to require special-casing
- remove unused `ITestConfigurationProvider` / `TestConfigurationProvider`
- remove `TestAssemblyProvider` classes
- no longer required
- remove `ReplaceCallContextServiceLocationService` test helper
- change MVC sample to fully-qualify config.json path
- avoids `ReplaceCallContextServiceLocationService` need in `MvcSampleTests`
- no longer required for other tests
nit: remove unused `_serviceProvider` and constructor overload in `RazorPreCompiler`
- compiler given `ApplicationName`, not `ApplicationBasePath`
- @pranavkm wrote the tests prior to 64ddbe0160
nit: rewrap long line introduced in prior commit
Separates the MMP into two phases:
1). Creation of the ModelMetadata, discovery of properties and attributes
(reflection) is part of the MMP
2). Lookup of details based on attributes is now part of another phase,
and has its results cached.
Users can now implements and register an IFooMetadataProvider to customize
a single aspect of metadata (see how data annotations does it).
- #1468
- Always use `ModelExplorer` in `<select/>`, `DropDownListFor()` and `ListBoxFor()` cases
- allows evaluation of more-complex expressions
- Use `ViewData.Model` in `DropDownList()` and `ListBox()` template cases
- `ViewData` was previously ignored in these cases
nit: change `ViewDataDictionary.Eval()` to return `Model` if `expression` is `null` or empty
- now `throw` on `null` or empty `expression` name in `ViewDataEvaluator.Eval()`
- simplifies some of the higher-level code
- no change to `selectList` fallback; `Model` incorrect for that case
- no change to `GenerateRadioButton()`; would change behaviour unrelated to #1468
- this helper uses incorrect `ViewData` lookup text, see #1487
- see commit 9d5364c
- never correct to pass a `Func<object>` to `GetExplorerForExpression()`
- `<label/>` tests succeeded because that tag helper doesn't use expression result
- `<select/>` tests succeeded because that tag helper gets result from `ViewData`
- does not use `ModelExplorer` due to #1468
nit: update variable names `metadata` -> `modelExplorer`
This change provides caching for the results of searching for view
components and discovering their names.
In our ViewComponent perf test, selecting the component type was about 25%
of the execution of MVC (due to uncached reflection). This change brings
that down to about 1%.
This is a small change to remove uncached reflection. There are some other
code paths where use of view components results in a hotspot. Leaving
those issues to be tracked as part of a larger design issue.
Ryan Nowak
Ajay Bhargav Baaskaran
Harsh Gupta
N. Taylor Mullen
Kiran Challa
Hao Kung
Kirthi Krishnamraju
Pranav K
James Newton-King
David Paquette
Matthew Bonner
Doug Bunting
Chris R
Troy Dai
David Fowler
BrennanConroy
Henk Mollema
Youngjune Hong
sornaks
Victor Hurdugaci
Edward Kim
anurse
KevinDockx
ianhong
damianedwards
Praburaj
Wei Wang
Levi B
Kiran Challa
Yishai Galatzer