wiremock / wiremock-graphql-extension

GraphQL extension for WireMock
MIT License
22 stars 3 forks source link

Question on Deprecation of withRequestQueryAndVariables and Java Use #15

Closed kyle-winkelman closed 11 months ago

kyle-winkelman commented 11 months ago

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

https://github.com/wiremock/wiremock-graphql-extension/commit/30e722c513f125e2230813fca3ee6b48796b3a2f https://github.com/wiremock/wiremock-graphql-extension/commit/3f0dd5664acd8e2f017a0a920064d2d8cafb75fa

nilwurtz commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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.