-
-
Notifications
You must be signed in to change notification settings - Fork 370
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
TMVCRequestParamsTable to handle TValue #208
Comments
As a side effect the TMVCRequestParamsTable now has an overridden constructor which performs case insensitive comparisons of param keys, so the case matching between Paths params [e.g. /Patient/{$param) ] and controller method params [e.g GetPatient(PARAM: string) ] works regardless of case sensitivity |
Quite nice addition. Good Job! However, the changes has been made over a very old unit compared to the current one in the repo. This makes difficult the merge phase. Could you reapply your changes on the current version of the units? Then your changes and your tests will be integrated. Thank you |
Changes reapplied to the current base. |
I did a deeper investigation on your changes. I'm not sure to have got the point about your intentions. What your code does that |
I'll take another look and provide some detailed examples as to why I have refactored the FillActualParamsForAction code into its own helper class - later though :) |
Any news on this? |
My bad. I have made the change to my own fork and have it in a production app so need to extract some examples from that app to show how it allows me to write less code in controllers. |
I've finally circled back to this request - to try to share the code changes which I've been using on a private fork of the framework for 18 months.. The main justification for this change is that the parameters to Controller Action implementation methods can be richer than strings which are parsed out of the URL, cookies etc. With this change it is possible to write Controller methods that contain parameters typed for an instance of a TObject/IInterface class. The actual instantiation can be handled by middleware in the BeforeControllerAction method *. For example, the controller method below has the following parameters automatically populated:
This ability to add custom middleware that can automatically instantiate and pre-populate parameters before they hit the controller action is really useful where you have a project with many controllers, each with many methods that expect a fully instantiated object. It means you do not have to call a helper function in each action which is a real plus for developer productivity and code maintainability. I acknowledge that this will be a breaking change as the signature for params changes from string to TValue. However these changes are usually limited to a few controller actions and a new property "ParamsAsString" is provided to provide the string representation. As a breaking change I would suggest a major release such as 4.x Now that I have synchronised 3.2.2 with our code base I will prepare and upload a code diff. This has simplified the changes.
|
OK I'm having trouble with the pull request - looks like I would need to fork the project on github. So I've attached a zip of the changed files instead (just two)
|
The code changes I want allow for an object to be instantiated from the Body of a Request (e.g. by some custom middleware) and then be auto injected into the Action as a parameter. I have a project with 17 controllers, most of which have at least 6 actions that take a fully instantiated object that has been created from the body of the request. This saves me having to call a method to parse and create those objects in every single action, which is repetitive and potentially error prone. Here's an example of a controller action ...
This would be called by a client with something like
The middleware has a factory that will create an object from a concrete class that implements the IFHIRAllergyIntolerance interface. The key to getting this to work is having the TMVCRequestParamsTable class look like this
Also the constructor has been overridden so we can support case insensitive param names. I'll see if I can create a fork on github and show you the changes we are currently running with. |
I've just reconciled the Magnesium RC with the fork we made (5 years ago) to support TValue in the automagically bound Controller Action parameters. I will tidy up some of our uses cases and submit a PR as I think the code change I've made significantly simplifies building reasonable complicated applications. E.g. Our main healthcare API application, now has 19 controllers, each with around 6 methods - so 114 methods, that benefit from automatic instantiation of objects via a custom middleware. And we have more planned.
Our middleware automatically creates the Resource object from the request body, after running some cleanup and business rules to ensure the request body meets desired characteristics. The underlying code is very similar to how the MVCInjectAttribute creates an interfaced object.
In our code, we create the Resource object via a factory, where the resource types and implementing classes have been pre-registered. And for this action/method we also populate the LocationId parameter with a value extracted from the JWT authentication token submitted with the request. All of this happens transparently without having to markup the controller code, or run an inherited method inside of the method. In addition our middleware automatically scans for registered Param Names (e.g. LocationID, ConditionalUpdateSearchTerm, SortOrder, SearchQuery, etc), figures out where to get the parameter vale from and sets the associated value. This makes our code a lot faster to write and improves the code quality as the auto populating is centralised, can be tested onec, and is never omitted by a team member in a hurry. |
Very nice. However, did you check the dependency injection container with automatic injection available in modern dmvc? You can register services using something like the following: unit Unit6;
interface
uses
Unit5,
MVCFramework.Container, System.Generics.Collections;
type
IPeopleService = interface
['{F1866213-A1F6-494A-895F-12833BAAB3A0}']
function GetAll: TObjectList<TPerson>;
end;
TPeopleService = class(TInterfacedObject, IPeopleService)
protected
function GetAll: TObjectList<TPerson>;
end;
procedure RegisterServices(Container: IMVCServiceContainer);
implementation
procedure RegisterServices(Container: IMVCServiceContainer);
begin
Container.RegisterType(TPeopleService, IPeopleService, TRegistrationType.SingletonPerRequest);
// Register other services here
end;
function TPeopleService.GetAll: TObjectList<TPerson>;
begin
Result := TObjectList<TPerson>.Create;
Result.AddRange([
TPerson.Create(1, 'Henry', 'Ford', EncodeDate(1863, 7, 30)),
TPerson.Create(2, 'Guglielmo', 'Marconi', EncodeDate(1874, 4, 25)),
TPerson.Create(3, 'Antonio', 'Meucci', EncodeDate(1808, 4, 13)),
TPerson.Create(4, 'Michael', 'Faraday', EncodeDate(1867, 9, 22))
]);
end; And then service can be injected (recursively!) into you actions and services. //Sample CRUD Actions for a "People" entity
[MVCPath('/people')]
[MVCHTTPMethod([httpGET])]
function GetPeople([MVCInject] PeopleService: IPeopleService): IMVCResponse; |
That's my next task :) |
The current version of the Context.Request.Params table is defined as TDictionary<string, string> so cannot contain the full range of types that Delphi is capable of.
Changing this to TDictionary<string, TValue> allows a wider range of types to be created and dispatched to Controller methods.
The implementing class is TMVCRequestParamsTable which has been changed in this Pull Request.
Updated Files are attached (including new unit test suite) along with a sample project, and a Postman test collection to exercise the sample project.
The text was updated successfully, but these errors were encountered: