spring-projects / spring-restdocs

Test-driven documentation for RESTful services
https://spring.io/projects/spring-restdocs
Apache License 2.0
1.16k stars 733 forks source link

Create Kotlin DSL for Spring RestDocs #547

Open checketts opened 6 years ago

checketts commented 6 years ago

I've been working on a Kotlin DSL for restDocs and have converted a number of samples to demonstrate it.

I would love to contribute the DSL into the spring-restdocs project. Would you be interested in that DSL? (I also wanted to contribute the base MockMVC DSL to spring test, but that is a different project)

Example DSL:

    @Test
    fun indexExample() {
        this.mockMvc.performGet("/") {
            expect { status { isOk } }
            document(documentationHandler) {
                "notes" link { description("The <<resources-notes,Notes resource>>") }
                "tags" link { description("The <<resources-tags,Tags resource>>") }
                response {
                    "Content-Type" header { description("The Content-Type of the payload, e.g. `application/hal+json`") }
                    "_links" subsection { description("<<resources-index-links,Links>> to other resources") }
                }
            }
        }
    }
cezary-butler commented 6 years ago

Great idea. Why not work on it separately and merge it to spring when it's been proven useful and stable?

Lately I've been working with spring rest-docs and kotlin, maybe I could also contribute.

checketts commented 6 years ago

Cool! This issue was to gauge interest and collaboration. At this point. I believe the underlying code is ready for contribution. I played with converting 3 of the samples and they all worked out well. (not to mention my own project's tests)

So I'll move forward with creating a pull request.

Ideas going forward:

wilkinsona commented 6 years ago

Yes, I'm definitely interested in a Kotlin DSL. Thanks for the offer of a contribution. @sdeleuze is our resident Kotlin expert so I'm hoping we can get him involved in this too. https://github.com/spring-projects/spring-restdocs/issues/192 may also be somewhat relevant here.

The use of kd4smt may be a bit problematic, particularly if it's not in Maven Central, but it needn't block things at this early stage.

sdeleuze commented 6 years ago

Sure, I will have a look asap.

checketts commented 6 years ago

I've opened the PR https://github.com/spring-projects/spring-framework/pull/1951 to move the kd4smt DSL into Spring Test itself.

While iterating on that PR, I'll get started on the PR for this repo too

mduesterhoeft commented 6 years ago

This is really interesting. I also started playing with this a little. Mainly because I wanted to learn about writing a kotlin-dsl.

I find the example by @checketts very interesting - but also a little bit too much. Especially because it seems to use extension functions for String to construct e.g. a LinkDescriptor.

I think something like the sample below could be an alternative. It is closer to the proposal in https://github.com/spring-projects/spring-restdocs/issues/192.

.andDo(document("some") {
    request {
        field("some") { description("some") }
        field("more") { description("more")}
        header(CONTENT_TYPE) { description("some") }
    }
    response {
        field("some") { description("some") }
        field("more") { description("more")}
        link("self") { description("self link") }
    }
})

What do you think?

For details see also https://github.com/mduesterhoeft/restdocs-kotlin-dsl

checketts commented 6 years ago

@mduesterhoeft Do you view the extension function on String as a negative?

Keep in mind that extension functions don't leak all over the place (like monkey patching in other languages), it is actually scoped only to the document block.

That approach is pretty common in DSLs. The one negative I could think of is if you didn't know it existed looking for autocomplete in the response block works better with the field() you provided.

mduesterhoeft commented 6 years ago

@checketts your link to the sample code does not work (anymore). Would be interesting to look at the code.

Actually I wanted to discuss here a bit about designing the DSL - I think the string extension function approach is not really contributing to readability. But that might be a question of taste...

wilkinsona commented 6 years ago

Yeah, I think a lot of DSL design is a question of taste. Something that I'd like to explore is having a Kotlin DSL that looks like a more concise version of a Java API rather than being radically different. That may mean that this issue and #192 need to be worked on in tandem. That said, I'm not sufficiently experienced with Kotlin to know if that's likely to yield an idiomatic result. @sdeleuze and anyone else who's more experience with Kotlin, I'd welcome your input here please.

checketts commented 6 years ago

I'd like to explore is having a Kotlin DSL that looks like a more concise version of a Java API rather than being radically different.

Thanks for the direction @wilkinsona

@mduesterhoeft I've fixed the link above.

jnizet commented 5 years ago

I've discovered this issue after I also tried creating a Kotlin DSL for Spring REST Docs: https://github.com/Ninja-Squad/spring-rest-docs-kotlin. See the README for links to examples. I think it's a bit closer to the Java DSL.

Feedbacks welcome :-)

sdeleuze commented 5 years ago

Hey, after a look to the various proposals, I think I would like to take @jnizet proposal as starting point, as @wilkinsona said DSL is a matter of taste, but I think we should try to be consistent with what I have done in beans { } and router { } DSLs which indeed stay pretty close of the Java DSL.

@jnizet The direction taken by https://github.com/Ninja-Squad/spring-rest-docs-kotlin looks great, I like the andDocument principle. I am a little bit concerned by the number of top level functions like pathParametersSnippet that I would like to try to limit, is it possible to limit their scope? Maybe #192 will help, not sure.

Before that, I want to move forward on modifying and merging spring-projects/spring-framework#1951 in Spring Framework master branch, and potentially do the same for WebTestClient. I will add a comment here what that will be done.

wilkinsona commented 5 years ago

Thanks, both.

I am a little bit concerned by the number of top level functions like pathParametersSnippet that I would like to try to limit, is it possible to limit their scope? Maybe #192 will help, not sure.

I share this concern about REST Docs in general. In the current Java API there are lots of static methods that aren't easy to discover. I'm hoping that #192 will help. I have a rough idea of a more AssertJ-like API but haven't yet had time to really explore what that may look like.

jnizet commented 5 years ago

I also agree. Ideally, none of those top-level functions would exist, and that's the goal of the scopes: be able to use, for example:

.andDocument {
    pathParameters {
        add("productId", "the ID of the product to get")
    }
}

In the above example, andDocument is an extension function (for MockMvc, WebTestClient or RestAssured), pathParameters is a member function of DocumentationScope, and add is a member function of PathParametersScope.

I added these top-level functions to create reusable descriptors and reusable snippets, which can be valuable

But I agree that it makes a lot of top-level functions. I'm not sure how to best scope them. My intuition would be to create two objects:

object Snippets {
    fun pathParameters(...) = ...
    fun requestParameters(...) = ...
    fun requestFields(...) = ...
    ...
}
object Descriptors {
    fun parameter(...) = ...
    fun field(...) = ...
    fun requestPart(...) = ...
    ...
}

This would remove all top-level functions while keeping the functionality. Instead of using

val reusablePathParameters = pathParametersSnippet { ... }
val reusableField = field(...)

we would use

val reusablePathParameters = Snippets.pathParameters { ... }
val reusableField = Descriptors.field(...)

which, IMHO, makes it a liitle bit more verbose, but as or even more readable. Those objects could be the target for custom extension functions too.

What do you think?

sdeleuze commented 5 years ago

I think that's a good idea yes. I would have favor extension functions on types like ParametersBuilder but since KT-11968 is still not fixed, I think that's the best option.

checketts commented 5 years ago

I'm also in favor of @jnizet 's direction. So I'll let him lead on his proposal while I focus on https://github.com/spring-projects/spring-framework/pull/1951

I like that the focus is ease of discoverabiliy, readability, and allowing extension points for further custom extension functions

jnizet commented 5 years ago

Here's where I am with the Kotlin DSL.

I have made changes in my original repo to split the source code in multiple smaller source files, add tests to achieve 100% code coverage, and add kdoc comments for everything.

You can use ./gradlew build and ./gradlew dokka to build and test everything and generate the documentation.

I'm thus ready to move the code to spring-restdocs now. Of course, I'm aware that you might want to challenge some parts and to change things, and willing to do it.

How should I proceed?

Here are some options:

For the record, my code uses JUnit 5 (nothing fancy, but nested tests are nice), so the first commit I did in my fork is to add JUnit 5 support in the build. All the existing tests continue using JUnit4, executed by the vintage engine. But I'm in hope that it wouldn't hurt using JUnit 5 for the kotlin tests. I can of course revert to JUnit 4 tests if that's really an issue. I also chose to put the code for the kotlin DSL under the package org.springframework.restdocs.kotlin. Please tell if you'd prefer to put it somewhere else.

wilkinsona commented 5 years ago

Thanks again, @jnizet. I'm not sure how best to proceed, in the short term at least. I need to carve out some time to explore what a revised Java DSL for REST Docs may look like (#192). I also need to educate myself on what is and is not possible with a Kotlin DSL and what's considered to be idiomatic otherwise I'll be reviewing things from a position of relative ignorance.

As things stand, trying to do something in the Spring Framework 5.2, REST Docs 2.1 timeframe is appealing. That would allow us to align with the MockMVC Kotlin DSL that will hopefully make it into Framework 5.2 and to also aim for inclusion in this summer's Spring Boot 2.2 release.

jnizet commented 5 years ago

OK thanks @wilkinsona . Beware when learning more about Kotlin though: you might not want to come back to Java ;-)

Let's wait a little, and also see what @sdeleuze has to say about this. I'll wait for your feedbacks patiently.

sdeleuze commented 5 years ago

FYI, I have merged MockMvc Kotlin DSL in Spring Framework master, see this commit for more details.

jnizet commented 5 years ago

Great @sdeleuze ! Is there a nightly release of spring framework (and its bom) that I could use to compile and experiment against the new MockMvc DSL?

I'm starting to wonder if making the functions in ResultActionsDsl infix were a good idea: My RestDocs MockMvc DSL should provide a fun ResultActionsDsl.andDocument("someId") { } function instead of the current ResultActions.andDocument(...) extension method, but that doesn't seem possible after an infix function call. And since the ID is mandatory, making it a mutable property of the DocumentationScope doesn't look like a good idea to me.

To make what I just said clearer (hopefully), AFAIK it's possible to do

mockMvc.get(...) {
    ...
}.andExpect {
    ...
}.andDo {
    ...
}.andDocument("users/get") {
    ...
}

but if using the infix notation (see below), that wouldn't compile:

mockMvc.get(...) {
    ...
} andExpect {
    ...
} andDo {
    ...
}.andDocument("users/get") { // this wouldn't compile.
    ...
}

So, in short, the infix functions make it impossible to chain additional extension functions (unless we use them as non-infix functions, but then the usages of the API become inconsistent and confusing, IMHO)

checketts commented 5 years ago

The infix option doesn't forbid the first style though. Or do you think it shouldn't be allowed so as to avoid confusion? I would prefer keeping it.

sdeleuze commented 5 years ago

I agree with @jnizet, the inconsistency between infix and non infix form is an issue, which is also present with andReturn(), so I have removed it via this commit.

Snapshot builds of Spring Framework 5.2.0.BUILD-SNAPSHOT are available on https://repo.spring.io./snapshot.

jnizet commented 5 years ago

Thanks Sébastien!

pangyikhei commented 3 years ago

@sdeleuze

In spring-restdocs-mockmvc, the String urlTemplate static functions in RestDocumentationRequestBuilders add a request attribute with the url template value:

Ex.

    public static MockHttpServletRequestBuilder get(String urlTemplate, Object... urlVariables) {
        return MockMvcRequestBuilders.get(urlTemplate, urlVariables)
                .requestAttr(RestDocumentationGenerator.ATTRIBUTE_NAME_URL_TEMPLATE, urlTemplate);
    }

However, the Kotlin DSL implemented in spring-test's MockMvcExtensions.kt does not set this, requiring everyone using the Kotlin DSL to add that requestAttr on every request if they are using a handler that expects it (ex. https://github.com/ePages-de/restdocs-api-spec).

Can we add the urlTemplate request attribute to the Kotlin DSL? Maybe in spring-restdocs?

sdeleuze commented 3 years ago

Not an easy one. We could introduce some MockMvc.restDocsFoo() extensions but that would not be very elegant.

@wilkinsona Do we have a way to set this request attribute as a default in the related MockMvc instance? If so we could maybe add a MockMvc extensions like MockMvc.enableRestDocs().

wilkinsona commented 3 years ago

There's no default for the request attribute, unfortunately, as it needs to be set on a per-request basis. The various methods on RestDocumentationRequestBuilders do that in Java. For Kotlin, I wonder if we could provide a REST Docs-specific equivalent of MockMvcExtensions that calls RestDocumentationRequestBuilders rather than MockMvcRequestBuilders to create the request builder?

jnizet commented 3 years ago

FWIW, in my Kotlin DSL for rest-docs, I chose to indeed implement such specific extensions, and to name them differently, in order to avoid the common pitfall that the Java API has, i.e. using the wrong get()or post() static method. See https://github.com/Ninja-Squad/spring-rest-docs-kotlin/blob/master/mockmvc/src/main/kotlin/com/ninjasquad/springrestdocskotlin/mockmvc/MockMvcDsl.kt, and an example usage: https://github.com/Ninja-Squad/spring-rest-docs-kotlin/blob/master/examples/src/main/kotlin/com/ninjasquad/springrestdocskotlin/examples/MockMvcExampleTest.kt

sdeleuze commented 3 years ago

Indeed it seems to be the only reasonable solution.