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

Users are forced to implement an interface to define composition handlers and are constrained to create multiple classes instead of defining multiple methods in the same composition handler class #247

Open
mauroservienti opened this issue Mar 5, 2021 · 5 comments
Labels
feature New feature or request

Comments

@mauroservienti
Copy link
Member

mauroservienti commented Mar 5, 2021

Depends on:

Add support for conventions, if a class:

  • is defined in a CompositionHandlers namespace
  • has a method, whose name doesn't really matter, that takes the correct argument and returns the correct type
  • has the expected Http*Attribute on the identified method

Then it's a composition handler. And stop making an interface a requirement. This could enable having handlers with multiple methods to group composition actions by use case for example.

@mauroservienti mauroservienti changed the title Implement support for conventions based composition handlers Users are forced to implement an interface to define composition handlers and are constrained to create multiple classes instead of defining multiple methods in the same composition handler class Mar 12, 2021
@markphillips100
Copy link

Will an interface still be an option too do you think?

In the convention case, when defining the method, does the "correct argument" imply a convention also? Just curious how to introduce model bound parameters to a method.

@mauroservienti
Copy link
Member Author

Will an interface still be an option too do you think?

Not necessarily. I was thinking about keeping the current design and adding the new option.

In the convention case, when defining the method, does the "correct argument" imply a convention also? Just curious how to introduce model bound parameters to a method.

The thing I was brainstorming goes something like this:

  • when scanning assemblies scan for all types in a specific namespace (say for example *.Handlers)
  • look for all the public methods that returns Task and are decorated with one of the routing attributes (e.g. HttpGet)
  • for each them and compile an expression tree for each one to invoke them using a so called fast delegate approach
  • as we're doing today build the endpoint using the template defined in attribute, by storing in the custom endpoint also the list of parameters (using something like a parameter descriptor)

At invocation time:

  • bind the parameters stored in the endpoint
    • throw of some titles badly
  • use the compiled expression tree to invoke the user defined method passing the arguments returned by the bind operation

@markphillips100
Copy link

I like it @mauroservienti . So in theory you could get as near to controller action parameter behaviour. For example, not limit a method to just a model object but actual individual parameters with their own model binding attributes:

[HttpPost("cart/{orderId:guid}")]
Task PlaceOrder(HttpRequest httpRequest, [FromRoute] Guid orderId, [FromBody] PlaceOrderCommand command)

I think MVC defaults to FromBody semantics if you only specify a single object parameter and requires an explicit attribute if you include other parameters with attributes.

Always happy to help out if you need anything.

@mauroservienti
Copy link
Member Author

So in theory you could get as near to controller action parameter behaviour.

Correct, that's the idea

I think MVC defaults to FromBody semantics if you only specify a single object parameter and requires an explicit attribute if you include other parameters with attributes.

That's tricky; for example, take a look at this Stack Overflow discussion.

@markphillips100
Copy link

Ah right, so the optional FromBody only applies when using ApiController attributes on controllers. Good catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
Status: To do
Development

No branches or pull requests

2 participants