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

Question on Deprecation of withRequestQueryAndVariables and Java Use #15

Closed
kyle-winkelman opened this issue Dec 11, 2023 · 4 comments · Fixed by #18
Closed

Question on Deprecation of withRequestQueryAndVariables and Java Use #15

kyle-winkelman opened this issue Dec 11, 2023 · 4 comments · Fixed by #18

Comments

@kyle-winkelman
Copy link
Contributor

Summary

I would like to understand why the withRequestQueryAndVariables method is @Deprecated. It feels strange that the recommended method for creating a GraphqlBodyMatcher is to manually create the expectedJson each time.

String expectedQuery = """
        query HeroInfo($id: Int) {
            hero(id: $id) {
                name
            }
        }
        """;
String expectedVariables = """
        {
            "id": 1
        }
        """;
String expectedJson = """
        {
            "query": "%s",
            "variables": %s
        }
        """.formatted(expectedQuery, expectedVariables);
GraphqlBodyMatcher.Companion.withRequestJson(expectedJson);

What I would expect as a user of the library is an easier (and less repetitive) way to create a GraphqlBodyMatcher. Also, it doesn't make sense to me that expectedJson doesn't need to be valid JSON (because newline characters are removed). For example, my IDE will get very mad about this, but it can be used to successfully create a GraphqlBodyMatcher.

@Language("JSON")
var expectedJson = """
        {
            "query": "
                query HeroInfo($id: Int) {
                    hero(id: $id) {
                        name
                    }
                }
            ",
            "variables": {
                "id": 1
            }         
        }
        """;

References

30e722c
3f0dd56

@nilwurtz
Copy link
Collaborator

Hello,

I'd like to provide some context on this deprecation.

Wiremock operates both as a standalone application and as a library, requiring different implementation approaches for extensions. In standalone mode, you need to pass the extension's name and parameters, whereas in library mode, passing a class is possible.

My thought is to unify these experiences, allowing developers to use the same API for extensions without worrying about the mode. The standalone API, which passes JSON via parameters, also works in library mode. Therefore, I'm leaning towards using it. (The deprecated methods are implemented for library mode.)

I agree that manually assembling JSON can be cumbersome (thank you for pointing this out). Providing a Json Builder Class or a similar tool might be a good solution for resolving this issue. I would appreciate your thoughts on this matter.

@kyle-winkelman
Copy link
Contributor Author

kyle-winkelman commented Dec 12, 2023

I definitely agree with your first point. We should unify the experience of using the extension to passing the extensions name and passing parameters. Custom matching document makes it sound like although passing a class is possible, that should really only be used for inline classes. Another example to back this up is wiremock-state-extension which only ever references StateRequestMatcher by name (state-matcher) and with parameters.

@kyle-winkelman
Copy link
Contributor Author

As to your second point, a builder could be nice, but in this situation it may be excessive when a single method might be enough. I will try out some things on my side and post back later with my thoughts.

@nilwurtz
Copy link
Collaborator

In the current implementation, the following code can be used for both Standalone and Library modes, as shown here: Client-Side Test Configuration.

fun registerGraphQLWiremock(json: String) {
    WireMock(8080).register(
        post(urlPathEqualTo(endPoint))
            .andMatching(GraphqlBodyMatcher.extensionName, GraphqlBodyMatcher.withRequest(json))
            .willReturn(
                aResponse()
                    .withStatus(200)
            )
    )
}

This usage is similar to what we have in the wiremock-state-extension, as demonstrated in this example: StateRequestMatcherTest.java.

The andMatching method takes a Name as its first argument and Parameters as the second. I believe that providing a class that takes a query or variables and returns Parameters is a feasible solution at this moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants