microsoft / kiota-dotnet

Abstractions library for the Kiota generated SDKs in dotnet
https://aka.ms/kiota/docs
MIT License
36 stars 33 forks source link

MulitpartBody does not write out simple string values #300

Closed waynebrantley closed 3 months ago

waynebrantley commented 3 months ago

Given code like this:

        var multipartBody = new MultipartBody();
        multipartBody.AddOrReplacePart("Name", "application/x-www-form-urlencoded", "SomeName");
        multipartBody.AddOrReplacePart("Address", "application/x-www-form-urlencoded", "SomeAddress");

The parts inside MulitpartBody are correctly setup.
Serialize is called and this line of code executes: https://github.com/microsoft/kiota-dotnet/blob/main/src/abstractions/MultipartBody.cs#L148

As you can see it calls partWriter.WriteStringValue(string.Empty, currentString); and the first parameter of WriteStringValue is always empty. When writing out a form, that calls this code: https://github.com/microsoft/kiota-dotnet/blob/main/src/serialization/form/FormSerializationWriter.cs#L220

If you look at this line if(string.IsNullOrEmpty(key) || string.IsNullOrEmpty(value)) return;, it is always true because the key is always empty!

This results in none of the string values actually being written out.

The current tests you have around everything always mock out serialization library - so it does not reveal that it always calls with an empty string. Additionally, the tests for the FormSerializationWriter always pass in a key, so that does not reveal this issue either.

It appears that the Serialize code always passes empty key for all types of content so those are likely wrong too. In addition, this line effectively is a no-op also: https://github.com/microsoft/kiota-dotnet/blob/main/src/abstractions/MultipartBody.cs#L121

andrueastman commented 3 months ago

Thanks for raising this @waynebrantley

The issue here is essentially a duplicate of https://github.com/microsoft/kiota-dotnet/issues/247#issuecomment-2188326343 that needs some investigation.

The thing is that if we pass would you expect the payload to contain both key/values? Or just the values?

multipartBody.AddOrReplacePart("Name", "application/x-www-form-urlencoded", "SomeName");
multipartBody.AddOrReplacePart("Address", "application/x-www-form-urlencoded", "SomeAddress");

If only values, does it make sense to change to this?

multipartBody.AddOrReplacePart("Name", "text/plain", "SomeName");
multipartBody.AddOrReplacePart("Address", "text/plain", "SomeAddress");
waynebrantley commented 3 months ago

Essential duplicate - sorry about that, I saw that ticket and thought it was all about the extra CRLF.

Good question. If the type is x-www-form-urlencoded, then it would be expected to be key=value&key=value, etc. If not then it would just be the value - as the form-data has the key name in it. Sort of weird to pass x-www-form-urlencoded in form-data because the form-data has the name. Not sure what asp.net would do with that? Look for a variable with that name and then populate the properties from the x-www-form-urlencoded?

High level, we we expecting just the data to come out as it was form-data and was not sure how the encoded would work.

We are really struggling getting an open api definition that kiota can call successfully, if that mapPost contains IFile and other data along with the file. Its either an open api generation issue (swashbuckle/nswag) or a kiota issue or both, it is a bit of a mess! :-)

baywet commented 3 months ago

Thanks for the additional context. Out of curiosity, what are you using to generate the description?

waynebrantley commented 3 months ago

currently Swashbuckle. Cannot wait for the built-in openapi support. We did try the text/plain and that seemed to work much better.

baywet commented 3 months ago

Thanks for confirming. Closing.