Skip to content

Conversation

@Hronom
Copy link
Contributor

@Hronom Hronom commented Sep 1, 2018

Proposed solution for #62

Notes

The solution is based on

 <dependency> <groupId>io.github.openfeign.form</groupId> <artifactId>feign-form-spring</artifactId> <version>3.3.0</version> </dependency> 

Class feign.form.spring.SpringFormEncoder was added inside org.springframework.cloud.openfeign.support.SpringEncoder because it's doesn't has some checks:

  1. Check of bodyType on null.
    org.springframework.cloud.openfeign.support.SpringEncoder#encode don't handle situation when bodyType is null.

  2. No check for content type multipart/form-data in header.
    feign.form.FormEncoder requires multipart/form-data in header for proper processing.

All this checks added and tests are passed.

spencergibb and others added 4 commits August 3, 2018 15:04
* Add factory for Param.Expander using ConversionService. Instances use ConversionService and passes annotations (through TypeDescriptor) - ConversionService can now pick up @DateTimeFormat and @numberformat and convert the params applying those.
@pivotal-issuemaster
Copy link

@Hronom Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@Hronom Thank you for signing the Contributor License Agreement!

@codecov-io
Copy link

codecov-io commented Sep 1, 2018

Codecov Report

Merging #65 into master will increase coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #65 +/- ## ============================================ + Coverage 75.87% 76.02% +0.15%  - Complexity 283 286 +3  ============================================ Files 36 36 Lines 1206 1214 +8 Branches 186 188 +2 ============================================ + Hits 915 923 +8  Misses 212 212 Partials 79 79
Impacted Files Coverage Δ Complexity Δ
...amework/cloud/openfeign/support/SpringEncoder.java 74.54% <100%> (+4.33%) 11 <1> (+3) ⬆️
@ryanjbaxter
Copy link
Contributor

Can you submit this PR against the 2.0.x branch?

@Hronom Hronom changed the base branch from master to 2.0.x September 5, 2018 07:51
@Hronom
Copy link
Contributor Author

Hronom commented Sep 5, 2018

@ryanjbaxter changed

@Hronom
Copy link
Contributor Author

Hronom commented Sep 5, 2018

@ryanjbaxter Oh I think I need to apply changes on that branch since it will push some changes from master...

@Hronom Hronom closed this Sep 5, 2018
@Hronom
Copy link
Contributor Author

Hronom commented Sep 5, 2018

Closed.
New here #66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants