Add POST example to README.md and example-github

[eiselems]

Fixes: #978

[wulftone]

Looks like a good example! I think it might help people to let them know explicitly that the parameter order matters, or they'll just get confused about it.. e.g. in development they might make a GET method with some params, then change it to a POST and add a third body parameter, which is wrong, then read the readme again and maybe notice the subtlety. I prefer spelling it out in sentences that this ordering is a hard requirement.

[velo]

Change looks good

I just wanna see a green build from jenkins.

[eiselems]

Good point. Since I am a bit spoiled since I use SpringBoot together with spring-feign ...
how would I do the following with just openFeign?

@RequestMapping(method = RequestMethod.POST, value = "/stores/{storeId}", consumes = "application/json")
Store update(@PathVariable("storeId") Long storeId, Store store);

@kdavisk6 mentioned the @Body annotation, but that requires a templated body which is not what I want here 😕 ? Anybody could shine some light on it?

Or did I get it correct that it must be EITHER the first parameter OR a templated body using the @Body annotation?

Also: Just took this from the spring cloud example page, there it seems that the order is not important - which I find slightly interesting.

[kdavisk6]

@Body can be used two ways, one on the method with a template, the other is on a parameter. To take your example and convert it to vanilla feign:

@RequestLine("POST /stores/{storeId}")
@Header("Content-Type: application/json")
Store update(@Param("storeId") Long storeId, @Body Store store);

The information missing from the documentation is that @Body can be omitted if the last parameter in the method signature is the request body. @eiselems, your intuition about the parameter location is on point.

Here is an example where the @Body is required.

@RequestLine("POST /stores/{storeId}/item/{item}")
@Header("Content-Type: application/json")
Store update(@Param("storeId") Long storeId, @Body Store store, @Param("item") Long item);

Hope this helps.

[kdavisk6]

@velo @eiselems

Not sure why Travis is not running the build. It's breaking on the environment set up, however there are no environment changes in this PR. Either of you have any insight on what we should look for to fix? I kicked Master just to see if it something in the repo, but that build is working just fine.

[eiselems]

Don't stress the build, not in a hurry to merge this asap. Take your time for figuring it out.

@kdavisk6 what is confusing me is that the interface approach doesn't work on my machine.
It is complaining that @Body requires a parameter and is not applicable to a parameter, is there another @Body for parameters which is not on the buildPath of the example? Really confused right now:

feign.Body:

@Target({ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
public @interface Body {
    String value();
}

[velo]

@velo @eiselems

Not sure why Travis is not running the build. It's breaking on the environment set up, however there are no environment changes in this PR. Either of you have any insight on what we should look for to fix? I kicked Master just to see if it something in the repo, but that build is working just fine.

I think is travis failing to install java... let's retry in a day or 2.

[eiselems]

@kdavisk6 Sorry for probably needing too much hand holding for this one ...

I discovered in the example that all of:

@RequestLine("POST /repos/{owner}/{repo}/issues")

1) void createIssue(Issue issue, @Param("owner") String owner, @Param("repo") String repo);
2) void createIssue(@Param("owner") String owner, Issue issue, @Param("repo") String repo);
3) void createIssue(@Param("owner") String owner, @Param("repo") String repo, Issue issue);

work well. I tested it against a local node application which was just printing the body for all requests.

I think I would then not add the notice in the text but definitely add an example for the "POJO as body"

[eiselems]

So I think in the current state it should be good to merge

[wulftone]

Huh. I definitely encountered issues if the pojo wasn't first in the list. I'll have to dig a little to find out why that is...

[kdavisk6]

I found the issue with the build, so I'm going to fix that in another PR before the end of the week. I'm ok with passing on that check for now.

@wulftone If you want, please create an issue asking for clarification on how to use the @Body parameter so we can track it.