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

Create Kotlin DSL for Spring RestDocs #547

Open
checketts opened this Issue Aug 28, 2018 · 24 comments

Comments

Projects
None yet
6 participants
@checketts
Copy link

checketts commented Aug 28, 2018

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

This comment has been minimized.

Copy link

cezary-butler commented Aug 31, 2018

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

This comment has been minimized.

Copy link
Author

checketts commented Aug 31, 2018

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:

  • It relies on https://github.com/petrbalat/kd4smt, which is being published to an unusual maven repo. I would love to see that code (most of which I contributed) moved into SpringTest.
  • I'll add the kd4smt lib as an optional dep for now while creating the PR
  • Instead converting an existing sample, I'll create a new sample so as not to muddy the existing samples
@wilkinsona

This comment has been minimized.

Copy link
Member

wilkinsona commented Sep 3, 2018

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. #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

This comment has been minimized.

Copy link

sdeleuze commented Sep 3, 2018

Sure, I will have a look asap.

@checketts

This comment has been minimized.

Copy link
Author

checketts commented Sep 6, 2018

I've opened the PR spring-projects/spring-framework#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

This comment has been minimized.

Copy link
Contributor

mduesterhoeft commented Sep 12, 2018

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 #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

This comment has been minimized.

Copy link
Author

checketts commented Sep 12, 2018

@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

This comment has been minimized.

Copy link
Contributor

mduesterhoeft commented Sep 13, 2018

@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

This comment has been minimized.

Copy link
Member

wilkinsona commented Sep 13, 2018

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

This comment has been minimized.

Copy link
Author

checketts commented Sep 13, 2018

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

This comment has been minimized.

Copy link

jnizet commented Feb 18, 2019

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

This comment has been minimized.

Copy link

sdeleuze commented Feb 19, 2019

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

This comment has been minimized.

Copy link
Member

wilkinsona commented Feb 19, 2019

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

This comment has been minimized.

Copy link

jnizet commented Feb 19, 2019

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

  • when the same descriptors/snippets must be used inside several tests,
  • to ease the transition between the Java DSL and the Kotlin DSL, when such reusable snippets are used
  • to make sure the Kotlin DSL allows doing what the Java DSL allows

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

This comment has been minimized.

Copy link

sdeleuze commented Feb 19, 2019

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

This comment has been minimized.

Copy link
Author

checketts commented Feb 19, 2019

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

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

@jnizet

This comment has been minimized.

Copy link

jnizet commented Feb 22, 2019

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:

  • you can open issues on my original repo to suggest and discuss changes, and I can keep updating the code there until we agree on everything, and then make a PR targetting spring-restdocs/master to move the code here.
  • I can make a series of commits and PRs to move the code here, either directly to master or to some kotlin-dsl branch that would then be merged to master when everything is ready. I've already forked spring-restdocs and created 3 commits (see https://github.com/jnizet/spring-restdocs/commits/feat/kotlin-dsl-for-descriptors) to prepare the work, but i won't go further until we agree on how to proceed. The remaining commits would for example add the DSL for snippets. Then add the DocumentationScope interface and implementation, then add the extension functions for the 3 testing frameworks, then polish the build (to generate an aggregated documentation, etc.)
  • I can wait until the Kotlin DSL for MockMvc is ready, and restart working on this later.

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

This comment has been minimized.

Copy link
Member

wilkinsona commented Feb 25, 2019

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.

@wilkinsona wilkinsona added this to the 2.1.0.RC1 milestone Feb 25, 2019

@jnizet

This comment has been minimized.

Copy link

jnizet commented Feb 26, 2019

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

This comment has been minimized.

Copy link

sdeleuze commented Mar 4, 2019

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

@jnizet

This comment has been minimized.

Copy link

jnizet commented Mar 4, 2019

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

This comment has been minimized.

Copy link
Author

checketts commented Mar 4, 2019

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

This comment has been minimized.

Copy link

sdeleuze commented Mar 4, 2019

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

This comment has been minimized.

Copy link

jnizet commented Mar 4, 2019

Thanks Sébastien!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.