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

Consider some cleanup on MvcOptions.ApplicationModelConventions #1445

Closed
rynowak opened this issue Oct 23, 2014 · 2 comments
Closed

Consider some cleanup on MvcOptions.ApplicationModelConventions #1445

rynowak opened this issue Oct 23, 2014 · 2 comments

Comments

@rynowak
Copy link
Member

rynowak commented Oct 23, 2014

1) Consider shortening the property name:

services.Configure<MvcOptions>(options =>
{
    options.ApplicationModelConventions.Add(new CoolRoutingConvention());
});

VS

services.Configure<MvcOptions>(options =>
{
    options.Conventions.Add(new CoolRoutingConvention());
});

2) Consider extension methods for adding any type of convention to the options

Right now options only supports adding IApplicationModelConvention - if you want to operate controllers/actions/parameters, you have have to write a bunch of for-loops. We could provide convenience overloads for you.

From functional tests:

public class ApiExplorerVisibilityEnabledConvention : IApplicationModelConvention
{
        public void Apply(ApplicationModel application)
        {
            foreach (var controller in application.Controllers)
            {
                if (controller.ApiExplorerIsVisible == null)
                {
                    controller.ApiExplorerIsVisible = true;
                    controller.ApiExplorerGroupName = controller.ControllerName;
                }
            }
        }
}

VS

public class ApiExplorerVisibilityEnabledConvention : IControllerConvention
{
        public void Apply(ControllerModel controller)
        {
                if (controller.ApiExplorerIsVisible == null)
                {
                    controller.ApiExplorerIsVisible = true;
                    controller.ApiExplorerGroupName = controller.ControllerName;
                }
        }
}
@danroth27 danroth27 added this to the 6.0.0-rc1 milestone Oct 24, 2014
@yishaigalatzer yishaigalatzer modified the milestones: 6.0.0-beta3, 6.0.0-rc1 Jan 11, 2015
@yishaigalatzer
Copy link
Contributor

Specifically for item number 2, you need only to support a controller convention for every controller

Conventions.Add(new ControllerModelConvention());  // apply to all controllers, and check for the controller type in the convention if you care.

and not

Conventions.Add<HomeController>(new ControllerModelConvention()); // apply to a specific controller

Similarly should we have the same thing for all actions? I think yes. e.g. make all parameters with [FromHeader] required

@sornaks
Copy link

sornaks commented Feb 10, 2015

d2c3985

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

4 participants