- Changing HtmlHelper and HelperResult to implement IHtmlContent.
- Introducing BufferedHtmlContent.
- Making RazorPage handle only IHtmlContent and clearing out other types.
- Making StringCollectionTextWriter use BufferedHtmlContent so that it can be returned where necessary.
- Updating places which involve Write/Copy to pass in encoders.
- The encoders are currently not being used during write. But when HtmlString is modified to carry encode metadata, the encoder can be used for writing. This is a perf optimization and hence not a part of this change.
- Making TagHelperContent implement IHtmlContent.
This change removes the validation that forces an OutputFormatter to set
an encoding, so that you can use the OutputFormatter base class for
non-text.
The check that we had would pretty much only be hit when you
didn't have any SupportedEncodings. If you have anything in
SupportedEncodings, then one of them will be picked as a default. So
removing the check is to do, because for a text-based formatter you'd
never run into this issue in the first place.
- Tested with a page containing 33% of invalid Ids which go through TagBuilder.CreateSanitizedId.
- Ran 10,000 requests with 20 in parallel and measured perf.
- Data before the change:
System.String allocated by CreateSanitizedId: 1.84 mb
System.Text.StringBuilder allocated by CreateSanitizedId: 1.68 mb
- Data after the change:
System.String allocated by CreateSanitizedId: 0.720 mb
System.Text.StringBuilder allocated by CreateSanitizedId: 0.560 mb
Around 60% improvement from the original case.
- Previously `ModelBindingResult.IsModelSet` would be set to true if type conversions resulted in empty => `null` values for ValueTypes. This resulted in improper usage of `ModelBindingResult`s creating null ref exceptions in certain cases.
- Updated existing functional test to account for new behavior. Previously it was handling the null ref exception by stating that errors were simply invalid; now we can provide a more distinct error.
- Added unit test to validate `TypeConverterModelBinder` does what it's supposed to when conversions result in null values.
- Added additional integration tests for `TypeConverterModelBinder`.
#2720
- Disabling tests which have corresponding bugs in mono.
- Fixing a few tests which do not handle *nix file system.
- Updating Travis configuration to use mono's alpha bits.
- Introducing PlatformNormalizer to normalize content across multiple platforms.
- Removed TaskHelper and refactored with ClosedGenericMatcher
- Removed TypeHelper
- Moved custom encodings to InputFormatter
- Moved ObjectToDictionary to PropertyHelper
- Removed respective tests and test projects
This is some low hanging fruit for reducing the number of resolves we have
per request.
DefaultHtmlGenerator: Lots of these are created by RazorPage. It needs
IUrlHelper, so scoped is the best we can do for now. For an example, on
the front page of our sample, 48 of these are created for each request.
48! This takes it down to 1-per-request.
JsonResult: Again, multiple created per request (12 for the sample). This
class is totally stateless, so we can get down to 0-per-request.
DefaultViewComponentInvokerFactory: Same story as JsonResult.
DefaultObjectValidator/MvcMarkerService/DefaultFilterProvider:
these are stateless and pretty much guaranteed to be used by every request.
Getting them off the table.
- #1514
- refactor `RazorCompilationService` to allow a test subclass that normalizes Razor file line endings
- add `TestRazorCompilationService` to `RazorPageExecutionInstrumentationWebSite`
- adjust line endings to match in `RazorPageExecutionInstrumentationTest`
- add `ignoreLineEndingDifferences: true` to `Assert.Equal()` calls
- responses on Windows can have a mix of line endings
- `git config` setting affects line endings in .cshtml (and baseline) files
- however MVC and Razor mix `Environment.NewLine`s into HTTP responses
- update `PrecompilationTest` to split response regardless of line endings
- update `ResourceFile` to normalize all source file streams to LF only
- ensures consistent checksums and line mappings
- change `MvcRazorHostTest` to expect new line mappings
- recreate baseline files to expect new checksums and literal line endings
- use verbatim strings in affected tests
- careful use of `Environment.NewLine` in expectations is now just noise
nits:
- add doc comments in `RazorCompilationService`
- correct `TagHelpersTest` name to match containing file
- avoid incorrect `using` removal when `GENERATE_BASELINES` is not defined
- remove unnecessary `ResourceFile` normalization of output files
- cleanup duplicate code now that #2445 is fixed
- update unit tests using old `ModelBindingContext` setups
- fix (just) one integration test where `MutableObjectModelBinder` incorrectly calculated
`isTopLevelObject` and returned a non-`null` model
- undo temporary changes in `BodyModelBinderTests` due to increased reliance on incorrect
`isTopLevelObject` in #2445 fix
nits:
- combine tests that are now duplicates
- beef up coverage of some `MutableObjectModelBinderTest` cases
- remove unused `using`s
- part II of II for #2445
- `FormCollectionModelBinder` is an exception because container is not user-provided
- no `ModelState` entry added
- enable tests that #2445 was blocking
- fix these and other tests expecting different `ModelState` entries
- simplify logic in `FormFileModelBinder`
`ValueProviderResult`
- remove `protected` setters and parameterless constructor
- no scenario for their use in subclasses; however `ConvertTo()` remains `virtual`
- add single-parameter constructor
- use in most of the greedy and type-matching model binders
- add doc comments throughout class
nits:
- use new `ValueProviderResult` constructor in many existing tests
- `""` -> `string.Empty` and `vpr` -> `valueProviderResult` in `ValueProviderResultTest`
- improve some test names in `BodyValidationIntegrationTests`
- do not check `Message` of a Json.NET `Exception`
- part I of II for #2445 (with a duplicate code PR to follow)
- needed for #2445 because new `ModelState` entries for values will make inconsisteny worse
- change `BodyModelBinder` to use same keys for all `ModelBindingResult`s and `ModelState` entries
- return fatal error result if formatter adds an error to `ModelState`
- update potential callers to avoid avoid ignoring `IsFatalError`
- fix test attempting to serialize all of `ModelState`
- will be borked with additional `RawValue`s in state
- fix two other tests that serialized `ModelState` but checked only `IsValid`
nits:
- address minor inconsistencies in `ModelBindingContext`
- use `System.Reflection.Extensions` package a bit more, where it's already referenced
- remove some unused resources
- #1485, #1487
- handle `TemplateInfo.HtmlFieldPrefix` in `ViewDataEvaluator.Eval()`
- attempt lookup in the `ViewDataDictionary` using full name then evaluate
relative `expression` against `viewData.Model`
- handle `null` or empty `expression` special case in this method (remove `throw`s)
- always pass relative `expression` name into `Eval()`
- remove `null` or empty `expression` handling from higher-level code
- in a couple of cases, special-case returned `ViewDataInfo`
- #2662
- remove incorrect guard from `DefaultHtmlGenerator.GenerateRadioButtion()`
- add doc comments for the core methods that have changed
- enable unit tests skipped due to one of above bugs
- fix one (yeah, just one) other test with incorrect expectations
- remove functional test comments about the above bugs and update expectations
nits:
- move some comments describing `ViewDataEvaluator` methods above the methods
This is some cleanup of how we add multi-registration services to avoid
duplication when calling AddMvcServices multiple times. Also improved
tests to be more clear, and to verify all of our special cases
explicitly.
- #2664
- use new property to correctly determine `isTargetEnum` in `GetCurrentValues()`
- avoid `ArgumentNullException` in all cases where raw values are `enum` but target is not
- stop skipping tests blocked by #2664, exposing a couple more #1487 issues
- use new property instead of private `GetElementType()` methods where possible
- cleans up some duplicate code
- also remove redundant use of `IsCollectionType` and `ElementMetadata`
nits:
- move properties above methods in `ModelMetadata`
- avoid accidentally-incorrect "Remove Unnecessary Usings"
- test additional cases _close_ to these bugs as well
for #1485
- show odd `@Html.CheckBox()`, `@Html.Hidden()` behaviour in unit tests
- show odd `@Html.TextBox()` behaviour in functional tests (templates)
for #1487
- show odd `@Html.Value()` behaviour in unit tests
- show odd `@Html.RadioButton()`, `@Html.TextArea()` behaviour in functional tests
- show lack of validation attributes for `@Html.RadioButton()`, `<select>` tag helper
for #2662
- show odd `@Html.RadioButton(string.Empty)` behaviour in functional tests
for #2664
- show failures with `@Html.ListBox()` in unit tests
nits:
- test `IHtmlHelper` methods, not extensions
- use `ViewData`, not `ViewBag` in `HtmlGeneration_FormController`
- name test methods a bit more consistently
- rename `HtmlHelperValueExtensionsTest` to `HtmlHelperValueTest`
This is the first step is some more refactorings to come in the future
with the goal of making MVC less monolythic. This makes the core of MVC
more reusable and more in line with the design of other vNext platform
components.
With this change, Mvc.Core contains just the minimal guts needed to build
a working app.
- Action Discovery
- Action Invoker
- Filters
- ObjectResult
- Model Metadata
- Model Binding
- Formatters
- Validation System
And yes, we are aware of the irony of 'minimal MVC' not including the view
system. The idea is that this is the kernel of an MVC app, and anything
real is layered on top.
The most noticable impact of this change is that MvcOptions has been blown
apart into more managable chunks. See the various ConfigureMvc*** methods.
The new Mvc.Extensions package is a placeholder while we evaluate and tune
the new definitions. Expect more changes as features are move to their own
packages, and in some case their own repositories.
For now there is no experience to bootstrap an Mvc.Core app. That's coming
next.
Combining IControllerModelBuilder and IActionModelBuilder into a pipeline
of IApplicationModelBuilders. Extensibility for framework features (Auth,
Cors, etc) should implement an IApplicationModelBuilder to add data to
models before IApplicationModelConventions have a chance to run.
Also deleting IGlobalFilterProvider while touching this code, this should
have been removed a while ago when we removed other options facades.
- only use MVC error message when `[BindRequired]` is violated
- update that error message to more clearly describe the problem
- enable all tests skipped due to dupe bug #2493
- update expectations of a few tests using the old messages
nits:
- rename `ModelBinding_MissingRequiredMember` to `ModelBinding_MissingBindRequiredMember`
- remove `<param>` description of removed `requiredValidator` parameter
- remove unused `MutableObjectModelBinderTest.GetRequiredValidator()`
- `CultureReplacerMiddleware` no longer helping `RazorWebSite`
- regressed with @30d11a8
- `[ReplaceCulture]` sets up the correct culture early enough for consistent formatting
- remove `CultureReplacerMiddleware` from `RazorWebSite`
- only `Startup` class we have that used this with `RequestLocalizationMiddleware`
- testing `RequestLocalizationMiddleware`; so remove the confusing extra middleware
- baselines include both expected output and design-time line mappings
- controlled by GENERATE_BASELINES
- assertions related to file content are not checked in this mode
- add design-time test of Basic.cshtml to `MvcRazorHostTest`
- regenerate all files to avoid BOM and blank line noise in future PRs
- update out-of-date design-time Basic.cs file
nits:
- make a few variable names more consistent
- make `Assembly` fields `static`
- remove unused `_resourcesAssembly` field from `ErrorPageTests`
- remove `ResourceHelpers` which was specific to functional tests
- #2456
- visible behaviours now match MVC 5
- prevent `CollectionModelBinder` from converting no value to `byte[0]`
- prevent `TypeConverterModelBinder` from converting empty value to `byte[] { '\0' }`
nit:
- remove additional delegates `ModelBindingTestHelper.GetOperationBindingContext()` calls
- a few left despite #2625 cleanup
- use valid `multipart/form-data` content type; include a `boundary`
- correct expectations of `FormCollection` model binder's operation
- restore tests actually skipped for either of the above reasons
- add more tests with `ModelBindingResult.Model==null` and both `IsModelSet` values
#### Remove test references to Won't Fix bug #2473
- restore #2473 tests; update test expectations
- expect `null` composite results whenever binding fails
#### Restore test skipped due to "#2646"
- that issue does not exist; was likely #2466 or similar fixed bug
#### Rename model binding tests that still mention ReturnsFalse or ReturnsTrue
#### Minor src changes
- remove unused variable and unnecessary nesting in `DefaultControllerActionArgumentBinder`
- remove dangling mention of `[DefaultValue]` in `ComplexModelDtoModelBinder`
#### nits:
- remove empty delegates from some `GetOperationBindingContext` calls; `null` fine
- do some `using` cleanup
- combine two test methods in `KeyValuePairModelBinderTest`
- name a few more arguments
- ICodeTreeCache => IChunkTreeCache
- ModelCodeGenerator => ModelChunkGenerator
- MvcCSharpCodeBuilder => MvcCSharpCodeGenerator
- Updated files that used Razor resources that are now in different namespaces.
- Updated variable names to account for Razor renames.
aspnet/Razor#140
Updated the ResponseCacheFilter Duration, Location, NoStore, and VaryByHeader properties to return one of 3 values: the specific setting applied to the instance, the setting supplied by the CachePolicy (from the constructor), or a default value. The emphasis being not to change the outward function of these properties to consumers, but to defer the evaluation of these properties until the OnActionExecuting method.
Updated the ResponseCacheFilter to check whether a cache duration has been supplied when the NoStore property is false and throw an InvalidOperationException when a duration has not been provided.
Updated ResponseCacheFilterTest to reflect the changes in behavior in the ResponseCacheFilter
Updated the ResponseCacheFilterAttributeTest to reflect the change in the behavior of the ResponseCacheFilter
Added unit tests to ResponseCacheFilterTest.cs to verify that the CachePolicy properties are properly overriden with the properties set directly on the ResponseCacheFilter.
Added functional tests to test the ability to override CacheProfile settings with properties on the ResponseCacheAttribute
FileVersion property renamed to AppendVersion in ImageTagHelper,
LinkTagHelper and ScriptTagHelper.
asp-file-version attribute renamed to asp-append-version.
Resolves issue #2540
This change completely removes [Activate]. In a controller, you should
constructor injection or [FromServices] to access services.
To access context items (ActionContext, ActionBindingContext, root
ViewDataDictionary) you should use the respected attribute class.
We'd like to consider streamlining this further in the future by getting
down to a single injectable context for controllers, but for now this will
have to do.
This change removes [Activate] support from TagHelpers. TagHelpers which
need access to context should use [ViewContext] to have it injected. To
access services, use constructor injection.
This change removes [Activate] from ViewComponents. Accessing context
should be done through [ViewComponentContext]. Accessing services should
be done though constructor injection.
This change treats 'top-level' collection-type models similarly to
top-level POCO model - namely that they will always be instantiated even
if there's no data to put inside.
This change adds a [Required] client validator when
ModelMetadata.IsRequired == true. The bulk of the changes here are
mechanical updates to test files.
- This involved also adding required attributes with wildcards.
- With this change AnchorTagHelpers and FormTagHelpers should no longer light up on every `<form>` or `<a>` tag.
#2581
- also rename files and directories with "GlobalImport" in name
- nearly blind but avoid "ViewImportss" in new names
- public API change: `ViewHierarchyUtility.GetGlobalImportLocations()` -> `GetViewImportsLocations()`
- primary source updates were comments, tests, and implementation details
nit:
- rename NestedGlobalImports.cs file to NestedViewImportsController.cs, matching class
- use `IDictionary<string, TValue>` support in `<a/>` and `<form/>` tag helpers
- make `RouteValues` dictionaries `IDictionary<string, string>` for ease of use
- remove `TagHelperOutputExtensions.FindPrefixedAttributes()`
- set all `GeneratedTagHelperContext` properties
- add error for tag helper dictionary properties where `TValue` is `ModelExpression`
- add new `RazorPage.InvalidTagHelperIndexerAssignment()` method and resource
tests
- use new `isIndexer` argument when creating `TagHelperAttributeDescriptor`
- arrange `AnchorTagHelper` and `FormTagHelper` correctly
- also expect `routeValues != null` in calls to `IHtmlGenerator`
nits:
- get rid of some `foo` and `bar` gunk in tests
- remove unused variable to cleanup a test compilation warning
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
- in .kproj files, let VS mark a couple more test projects as such
- remove duplicate `ModelExpression` class
- not necessary now that test project references MVC Core project
- follow up to 9fded74
- avoid `resources` warning in project.json files
Covers simple scenario for each model binder.
Covers scenarios mixing a POCO model binder -> Simple Model binder.
This contains tests for
FormCollectionModelBinder
BinderTypeBasedModelBinder
TypeConverterModelBinder
Remainging:
TypeMatchModelBinder
This also adds missing unit test for TypeMatchModelBinder as well.
Kirthi Krishnamraju
Ajay Bhargav Baaskaran
Mugdha Kulkarni
Ryan Nowak
Kiran Challa
Hao Kung
sornaks
KevinDockx
N. Taylor Mullen
Doug Bunting
Harsh Gupta
Chris R
Hisham Abdullah Bin Ateya
CodingGorilla
David Paquette
James Newton-King
Pranav K
Troy Dai
David Fowler
Brennan