TMVCRequestParamsTable to handle TValue

fastbike commented 5 years ago

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.

A helper record TMVCRequestParam handles the management of state of Params from string (when parsed from the URL) to TValues when bound to the controller method's parameters.
A variety of constructors allows text or rich types to be added as request params
Current types handled presently are Int, Int64, Float, String, TDateTime, Boolean, Interface, Object.
A utility class TMVCParamConverter handles the actual conversions from string representations to TValue

Updated Files are attached (including new unit test suite) along with a sample project, and a Postman test collection to exercise the sample project.

fastbike commented 5 years ago

GitHub_PR_208.zip

fastbike commented 5 years ago

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

danieleteti commented 5 years ago

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

fastbike commented 5 years ago

Changes reapplied to the current base.

Github_PR_208_Take2.zip

danieleteti commented 5 years ago

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 procedure TMVCEngine.FillActualParamsForAction doesn't do already? It is interesting the possibility to change the parameters before inject them into the action, however considering that we are taking about only url_mapped_params, having the possibility to pass complex object on the url is not a priority for the most application, and where it is needed it is far simple to just create the object into the action code. Did I miss something?

fastbike commented 5 years ago

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 :)

danieleteti commented 5 years ago

Any news on this?

fastbike commented 5 years ago

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.

fastbike commented 3 years ago

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:

ResourceId is populated via the normal parameter mapping - in this case from the position in the request URL path,
LocationId is auto populated from the OAuth token passed on the request header (via some middleware),
Resource instance is auto created from the XML body of the request (via some other middleware),
ConditionalUpdateSearchTerm is auto populated/validated from the URL (via a third piece of middleware) - in this case by parsing the request query string [MVCPath('/($ResourceId)')] [MVCHTTPMethod([httpPUT])] procedure AddMedicationDispense(ResourceID, LocationId: string; Resource: IFHIRMedicationDispense; ConditionalUpdateSearchTerm: string);

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.
In the large project we are working on, we currently have 15 controllers each with 5 methods using this parameter mapping. Our roadmap has another 15 controllers to add, so that is 150 calls to a helper class we do not have to write and maintain.

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.

I could share some sample code showing how the middleware maps, and instantiates objects.

fastbike commented 3 years ago

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)

I have simplified and refactored my original approach so that Params remain as strings but they are also available via a new property as a TValue. This removes most of the breaking changes (a good thing) while allowing the additional functionality of the TValue type to be accessed.
Another change is that the TDictionary used to match the params is no longer case sensitive. This fits with the nature of Pascal (identifiers are NOT case sensitive) so prevents annoying problems from the Delphi code formatter changing the case of params !
The third change is to extract the code (TMVCEngine.GetActualParam) that does the conversion of a string value to a typed value into it's own helper class (TMVCParamConverter). This makes it available for middleware to use, so the auto instantiation of complex types can be handled by custom middleware (see the example in my previous post). In future I would like to extend this so it can also handle more types such as enumerated types (just booleans are handled currently).
The last change is to get TMVCEngine.FillActualParamsForAction looking in the ParamsTable to see if a non empty TValue can be matched before reverting to the URL mapped parameters

DMVC_208.zip

fastbike commented 1 year ago

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 procedure TMVCEngine.FillActualParamsForAction doesn't do already? It is interesting the possibility to change the parameters before inject them into the action, however considering that we are taking about only url_mapped_params, having the possibility to pass complex object on the url is not a priority for the most application, and where it is needed it is far simple to just create the object into the action code. Did I miss something?

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 ...

  [MVCPath('/FHIR/AllergyIntolerance')]
  TAllergyIntoleranceController = class(TClinicalController)
  public
    [MVCPath(''), MVCPath('/'), MVCHTTPMethod([httpPOST])]
    [MVCConsumes(TFHIRConstants.XMLContentType), MVCProduces(TFHIRConstants.XMLContentType)]
    procedure AddAllergyIntolerance(LocationId: string; Resource: IFHIRAllergyIntolerance; ConditionalCreateSearchTerm: string);

This would be called by a client with something like

POST http://example.com/FHIR/AllergyIntolerance?identifier=EHRKey|F91AEFB6-6DC6-4DEE-90E0-2C73964BC9F0

<?xml version="1.0" encoding="UTF-8"?>
<AllergyIntolerance xmlns="http://hl7.org/fhir">
  <id value="example"/>
  <identifier>
    <system value="http://acme.com/ids/patients/risks"/>
    <value value="49476534"/>
  </identifier>
  <clinicalStatus>
    <coding>
      <system value="http://terminology.hl7.org/CodeSystem/allergyintolerance-clinical"/>
      <code value="active"/>
      <display value="Active"/>
    </coding>
  </clinicalStatus>
  <type value="allergy"/>
  <category value="food"/>
  <criticality value="high"/>
  <code>
    <coding>
      <system value="http://snomed.info/sct"/>
      <code value="227493005"/>
      <display value="Cashew nuts"/>
    </coding>
  </code>
  <patient>
    <reference value="Patient/example"/>
  </patient>
  <onsetDateTime value="2004"/>
</AllergyIntolerance>

The middleware has a factory that will create an object from a concrete class that implements the IFHIRAllergyIntolerance interface. Incidentally it also populates the LocationId from a value passed in the authentication token, and ConditionalCreateSearchTerm param is auto populated from the query on the URL.

The key to getting this to work is having the TMVCRequestParamsTable class look like this

  TMVCRequestParamsTable = class(TDictionary<string, TValue>)
  public
    constructor Create;
  end;

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.

danieleteti / delphimvcframework

TMVCRequestParamsTable to handle TValue #208