Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse common WireMock components to create aggregate MatchResult. #18

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

kyle-winkelman
Copy link
Contributor

@kyle-winkelman kyle-winkelman commented Dec 14, 2023

  • Add operationName parameter
  • Break GraphqlBodyMatcher into 3 parts to be combined in a MatchResult.aggregate() to begin generating a distance
    • EqualToGraphqlQueryPattern - new class to only compare the query
    • EqualToJsonPattern/AbsentPattern - to compare the variables
    • EqualToPattern/AbsentPattern - to compare the operationName
  • Remove org.json:json dependency in favor of reusing com.github.tomakehurst.wiremock.common.Json
  • Add container tests that parse from a mapping json to ensure the parameters are parsed as expected.
  • Deprecate withRequestJson and withRequest in favor of parameters

References

closes #15

Submitter checklist

  • Recommended: Join WireMock Slack to get any help in #help-contributing or a project-specific channel like #wiremock-java
  • The PR request is well described and justified, including the body and the references
  • The PR title represents the desired changelog entry
  • The repository's code style is followed (see the contributing guide)
  • Test coverage that demonstrates that the change works as expected
  • For new features, there's necessary documentation in this pull request or in a subsequent PR to wiremock.org

@kyle-winkelman
Copy link
Contributor Author

In the future I would like to add a distance calculation to EqualToGraphqlQueryPattern similar to that of EqualToJsonPattern where we count the number of correct nodes in the GraphQL Document vs the max number of nodes. This will help give recommendations to the users if their query is only off by a small margin.

@kyle-winkelman
Copy link
Contributor Author

In the attempt to reuse com.github.tomakehurst.wiremock.common.Json, I ran into some rough edges. Everything was working wonderfully in the Java unit tests, but was failing in strange ways for the ContainerTest. This was happening because I was referencing classes from the com.fasterxml.jackson package which ends up being shadowed in the WireMock standalone jar that is used in the WireMockContainer in ContainerTest. I had to remove all references to those classes; hence why I use Map::class.java instead of a TypeReference<Map<String, Any>> and why EqualToGraphqlQueryPattern subclasses ContentPattern instead of StringValuePattern (the later requires an @JsonProperty annotation).

Because of this headache I am wondering if reusing com.github.tomakehurst.wiremock.common.Json is worth it.

@nilwurtz
Copy link
Collaborator

First of all, thank you so much for this great PR!

The GraphqlBodyMatcher.parameters approach looks very good.
Regarding the com.fasterxml.jackson issue, it indeed seems strange. It might be a problem caused by version differences. I believe that using com.github.tomakehurst.wiremock.common.Json for this extension and avoiding the use of external libraries is worthwhile.

Overall, this looks like a perfect!

@nilwurtz nilwurtz merged commit 6279a0c into wiremock:main Dec 14, 2023
@kyle-winkelman kyle-winkelman deleted the aggregate branch December 14, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question on Deprecation of withRequestQueryAndVariables and Java Use
2 participants