MEDIUM FIX: Enable creation of multipart-form with non-string types

[Catalin-Andronie]

Currently when trying to construct a multipart-form for a model which has properties of another type except for string type, the next exception is thrown: Unable to cast object of type 'System.Decimal' to type 'System.String'.

public class Person
{
    [RESTFulStringContent(name: "firstName")]
    public string FirstName { get; set; }

    [RESTFulStringContent(name: "salary")]
    public decimal Salary { get; set; }
}

async Task MakeRequestAsync()
{
    var person = new Person
    {
        FirstName = "Jane Smith",
        Salary = 10_500M
    };

    var httpClient = SetupHttpClient();
    var apiClient = new RESTFulApiFactoryClient(httpClient);

    var response = await apiClient.PostFormAsync<Person, string>("/users", person);
}

Exception thrown:

RESTFulSense.Models.Client.Exceptions.RESTFulApiClientDependencyException : Form coordination dependency error occurred, fix the errors and try again.
---- RESTFulSense.Models.Orchestrations.Forms.Exceptions.FormOrchestrationServiceException : Form orchestration service error occurred, contact support.
-------- RESTFulSense.Models.Orchestrations.Forms.Exceptions.FailedFormOrchestrationServiceException : Failed form orchestration service occurred, please contact support
------------ System.InvalidCastException : Unable to cast object of type 'System.Decimal' to type 'System.String'.

[BrianLParker]

@Catalin-Andronie MultipartFormDataContent only supports limited data types. They are low level and expect "other" types to be stringified ready for transport. Normally this sort of posting would be handled by the other Post methods in RESTFulSense that supports serialization. The intent of this new method is not to bypass the older methods. Am I missing something in your use case?

Form Add Method

Supported Content Types

[Catalin-Andronie]

@Catalin-Andronie MultipartFormDataContent only supports limited data types. They are low level and expect "other" types to be stringified ready for transport. Normally this sort of posting would be handled by the other Post methods in RESTFulSense that supports serialization. The intent of this new method is not to bypass the older methods. Am I missing something in your use case?

Form Add Method

Supported Content Types

@BrianLParker I am with you on the part regarding the supported types of MultipartFormDataContent. The thing is that RESTFulSense multipart-form implementation assumes that all properties are of string type, and that's because we cast the property value to a string.

internal partial class FormOrchestrationService : IFormOrchestrationService
{
    private void AddStringContents(FormModel formModel)
    {
    ...
        string value = (string)this.valueService.RetrievePropertyValue(formModel.Object, property);
    ...
    }
}

What we can do instead is to remove the cast and make use of object.ToString() method to convert the value to string. In this way the object will decide how the value will be converted.

internal partial class FormOrchestrationService : IFormOrchestrationService
{
    private void AddStringContents(FormModel formModel)
    {
    ...
        string value = this.valueService.RetrievePropertyValue(formModel.Object, property).ToString();
    ...
    }
}

The later approach is more robust and allow for complex data types, like for example I may have a type Money which overrides the ToString() method in order to return the money amount and currency.

public class Person
{
    [RESTFulStringContent(name: "firstName")]
    public string FirstName { get; set; }

    [RESTFulStringContent(name: "age")]
    public int Age { get; set; }

    [RESTFulStringContent(name: "salary")]
    public Money Salary { get; set; }
}

public class Money
{
    public Money(decimal amount, string currency)
    {
        ...
    }

    public override string ToString()
        => $"{amount}{currency}";
}

async Task MakeRequestAsync()
{
    var person = new Person
    {
        FirstName = "Jane Smith",
        Age = 20,
        Salary = new Money(200M, "EUR")
    };

    var httpClient = SetupHttpClient();
    var apiClient = new RESTFulApiFactoryClient(httpClient);

    var response = await apiClient.PostFormAsync<Person, string>("/users", person);
}

I will create a test for the above example to make sure there are not corner cases to it.

@BrianLParker what's your input on the use of ToString() instead of casting ?

[glhays]

Please follow the naming conventions of The Standard Team. Naming standards for Issues, Pull Requests, Commits, and Branching help us all follow along in the trail of code.

[Catalin-Andronie]

Please follow the naming conventions of The Standard Team. Naming standards for Issues, Pull Requests, Commits, and Branching help us all follow along in the trail of code.

I've checked the Category List but I do not see anything related to a new FEATURE. I can mark this as a MEDIUM FIX or MAJOR FIX since the implementation is incomplete from my point of view, but on the other hand, this is not necessarily a bug.

@glhays please let me know what that naming should be.

[glhays]

Please follow the naming conventions of The Standard Team. Naming standards for Issues, Pull Requests, Commits, and Branching help us all follow along in the trail of code.

I've checked the Category List but I do not see anything related to a new FEATURE. I can mark this as a MEDIUM FIX or MAJOR FIX since the implementation is incomplete from my point of view, but on the other hand, this is not necessarily a bug.

@glhays please let me know what that naming should be.

Yeah I know sometimes it is hard to put a title on something where it may cross over. I think the MINER FIX is appropriate.

[Catalin-Andronie]

@BrianLParker Please check the next PR which illustrates the problem and also fixes it: #133