Skip to content
This repository was archived by the owner on Oct 15, 2024. It is now read-only.

Minor improvements#7

Open
ecolabardini wants to merge 7 commits into
masterfrom
improvements
Open

Minor improvements#7
ecolabardini wants to merge 7 commits into
masterfrom
improvements

Conversation

@ecolabardini

Copy link
Copy Markdown
Contributor
  • Separate XML and JSON body resolvers so anyone can write new ones just by extending the VariableResolver interface
  • Resolve variables per conditional instead of pre-processing all of them first

@ecolabardini ecolabardini self-assigned this Apr 15, 2018
@ecolabardini
ecolabardini requested a review from febpetroni April 15, 2018 21:09
public static String extractValueFromJson(String name, HttpServletRequest request) {
@Override
public boolean handles(String variable) {
return variable.startsWith("body.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Faria sentido verificar se é JSON para retornar true aqui?

public static String extractValueFromXml(String name, HttpServletRequest request) {
@Override
public boolean handles(String variable) {
return variable.startsWith("body.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mesma coisa, só que pra XML?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sim... Quando estava refatorando pensei em fazer isso. Melhoraria a performance pois evitaria o extract. Mas se o extract retorna null a gente também tenta avaliar os próximos Resolvers.

Teríamos que ou mudar o nome da variável para body.json ou body.xml, ou passar o Request e escolher baseado no Content-Type. Pessoalmente eu não gostaria de mudar o nome da variável... O que vc acha?

@codecov-io

codecov-io commented Apr 17, 2018

Copy link
Copy Markdown

Codecov Report

Merging #7 into master will not change coverage.
The diff coverage is 52.94%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master      #7   +/-   ##
======================================
  Coverage    47.5%   47.5%           
======================================
  Files          33      32    -1     
  Lines         381     381           
  Branches       36      37    +1     
======================================
  Hits          181     181           
  Misses        192     192           
  Partials        8       8
Impacted Files Coverage Δ
...le/resolver/URLQueryParameterVariableResolver.java 100% <ø> (ø) ⬆️
...ckkid/variable/resolver/RegexVariableResolver.java 85.29% <ø> (ø) ⬆️
...kid/variable/resolver/RawBodyVariableResolver.java 42.85% <ø> (ø) ⬆️
...kkid/variable/resolver/HeaderVariableResolver.java 100% <ø> (ø) ⬆️
...br/com/moip/mockkid/service/ConditionalSolver.java 0% <0%> (ø) ⬆️
...id/variable/resolver/JSONBodyVariableResolver.java 75% <80%> (ø)
...kid/variable/resolver/XMLBodyVariableResolver.java 76.47% <83.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f57cafb...ae7693d. Read the comment docs.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants