Fix #1549 - Support bodyParam attribute without form parameter name too map the whole body to a single parameter

[wilzbach]

As discussed in #1549 and #1422:

@bodyParam("obj")
string postFoo(FooType obj);

Moreover if

• no @bodyParam annotation is provided
• the method is automatically recognized as POST
• there's only one @bodyParam parameter

string postFoo(FooType obj);

then the entire Json body is serialized into this parameter. This might be a breaking change, but imho it's a sane default setting as most APIs want to serialize the entire request body and vibe.d hasn't reached stable yet.

[zunkree]

Any news?

[s-ludwig]

there's only one @bodyParam parameter
string postFoo(FooType obj);
then the entire Json body is serialized into this parameter. This might be a breaking change, but imho it's a sane default setting as most APIs want to serialize the entire request body and vibe.d hasn't reached stable yet.

This may go a bit too far. The library may not be declared stable, but is already used in production all over the place, so silent breaking changes like these are usually a no-go. Generally, I think that keeping things orthogonal here may be a good idea. For example, in the web frontend the equivalent rule would be very surprising and generally undesirable (form with only a single input field).

[s-ludwig]

Apart from the open questions, the implementation looks good and ready to merge!

[s-ludwig]

What about using an empty string here and asserting in the two-argument @bodyParam overload that the field name is always non-empty? That would eliminate any possibility for name clashes. I'd also tend to encapsulate the "whole body" logic into WebParamAttribute (e.g. WebParamAttribute.isAllInOne or similar, which performs the name comparison) - thinking about it, it may also make sense to extend this to query string parameters later on.

[s-ludwig]

The "foo" was a surprise here, was that left over from an earlier approach?

[s-ludwig]

Hm, I can't really adjust to the new review feature. I guess I'll just stick to normal comments.

[wilzbach]

I'd also tend to encapsulate the "whole body" logic into WebParamAttribute (e.g. WebParamAttribute.isAllInOne or similar, which performs the name comparison) - thinking about it, it may also make sense to extend this to query string parameters later on.

I am not sure how I would be able to access the WebParamAttribute object from the jsonMethodHandler

This may go a bit too far. The library may not be declared stable, but is already used in production all over the place, so silent breaking changes like these are usually a no-go. Generally, I think that keeping things orthogonal here may be a good idea. For example, in the web frontend the equivalent rule would be very surprising and generally undesirable (form with only a single input field).

I get your point and I removed the default rule to serialize the entire body into an parameter if only parameter is provided, s.t. this isn't blocking this PR.
However, imho in the context of REST APIs it's very common to accept a custom object - the best example being the GH API itself. Or in Vibe.d "words" a usual API looks like this:

auto postA(FooType1 obj);
auto postB(FooType2 obj);
auto postC(FooType3 obj);
....

Maybe we can use a special name like it's currently done for id?

[s-ludwig]

I am not sure how I would be able to access the WebParamAttribute object from the jsonMethodHandler

Right, I didn't think of that. But looking at it, the cleanest way to pass that information seems to be to create a new ParameterType for full body parameters. Then RestInterface!T would access the WebParamAttribute and the logic could stay close together.

I get your point and I removed the default rule to serialize the entire body into an parameter if only parameter is provided, s.t. this isn't blocking this PR.
However, imho in the context of REST APIs it's very common to accept a custom object - the best example being the GH API itself. Or in Vibe.d "words" a usual API looks like this:

auto postA(FooType1 obj);
auto postB(FooType2 obj);
auto postC(FooType3 obj);
....

Maybe we can use a special name like it's currently done for id?

I was thinking about using a default name, too, but the "id" feature is rather questionable today when the Collection functionality is taken into account (each "id" needs to have a unique name there, the auth module is in a similar boat). But I can definitely see how the difference that this would make cannot be neglected. I'll have to think about this some more before being able to give a meaningful answer.

[s-ludwig]

Suggestions implemented in #1723.