search

spring-cloud / spring-cloud-contract

Support for Consumer Driven Contracts in Spring
https://cloud.spring.io/spring-cloud-contract
Apache License 2.0
720 stars 438 forks source link

Rewrite more Groovy to Java #1470

Open marcingrzejszczak opened 4 years ago

marcingrzejszczak commented 4 years ago

The rationale

We're looking for help in rewriting all possible production code from Groovy to Java. The Spock tests will remain written in Groovy, however ideally we'd prefer not to have Groovy in any library production code except for the DSL as such.

It's been an ongoing process for years now. The main reason is the compatibility. Gradle comes with a bundled version of groovy and we have a Gradle plugin. Also we depend on groovy version in spring boot. That's all problematic.

It's discouraging for some part of the community to write a fix or a new feature in Groovy rather than in Java.

IDE support for groovy is getting worse every year now. Intellij IDEA introduces new bugs and other IDEs either don't work at all or are even worse.

What do we want to rewrite?

I'd suggest going through all the modules outside of the spring-cloud-contract-spec modules and picking production class after class and rewriting it to java from groovy, running the tests and if the tests work make a commit.

As i said we want to leave the Groovy DSL (groovy-spec module) and the spock tests.

How to start?

Pick a class / package / module, notify the others in a comment and start converting the classes! Once you're done file a pull request and link it to this issue.

Checking if code works

Before you file a pull request you can run 

$ ./mvnw clean install -Pintegration,docs

That way you'll also run the standalone samples that simulate end to end tests. It will also build the documentation and check if the docs aren't broken.

Please note that if you rewrite a class that contains such tags as

// tag::foo[]
// end::foo[]

that means that most likely that the code within those comments is used in the documentation.

armsargis commented 4 years ago

Hi, Groovy DSL should be removed also?

santfirax commented 4 years ago

Hello, @marcingrzejszczak I would love to help in this, could it be possible that you provide some guidance for starters?

marcingrzejszczak commented 4 years ago

Groovy DSL remains.

I'd suggest going through all the modules outside of the spring cloud contract spec modules and picking production class after class and rewriting it to java from groovy, running the tests and if the tests work make a commit.

As i said we want to leave the groovy spec and the spock tests.

saket88 commented 4 years ago

I want to contribute

marcingrzejszczak commented 4 years ago

Be my guest. Pick a class or a package and create a pull request where you migrate it to java!

stessy commented 4 years ago

I can help as well.

I'm currently rewriting the module spring-cloud-contract-converters.

marcingrzejszczak commented 4 years ago

Fantastic!

shanman190 commented 4 years ago

I'm still trying to work through the Gradle plugin pieces also.

marcingrzejszczak commented 4 years ago

@ankurongit You can pick a module / class that has not been worked on and put a note here that you're working on it and then link a pull request.

@shanman190 fabulous! I wanted to port whole groovy code from the plugin to java!

zhmaeff commented 4 years ago

@marcingrzejszczak I'd like to work on spring-cloud-contract-verifier module.

marcingrzejszczak commented 4 years ago

To everyone willing to help on this issue!

If you haven't already, pick a class / package / module, notify the others in a comment and start converting the classes! Once you're done file a pull request and link it to this issue. If you've picked a module / class / package, I'll add an :+1: emoji to notify that it's a good idea. In any other case I'll leave a comment.

Good luck and thank you so much for your help! :)

BTW I've updated the issue's description. If you think anything else could be put there that could be helpful to others, leave a comment and I'll update it.

stessy commented 4 years ago

Hi @marcingrzejszczak ,

I cannot run a full clean install. I have multiple modules for which the tests fail. In fact 2 modules so far (spring-cloud-contract-verifier and spring-cloud-contract-stub-runner). On the other hand the spring-cloud-contrat-converters, the one I rewrite, build without any problems. Don't really know if the rewriting (WireMockToDslConverter) could impact other modules tests. Here is the link to the rewritten class: https://github.com/stessy/spring-cloud-contract/commit/eb79ae83a3741fceb957048002f58d7b934dae60

So, what can I do ?

marcingrzejszczak commented 4 years ago

@stessy you need to provide some sort of a stacktrace or sth why the build is failing. It builds fine in Jenkins and CircleCI. If I were you I'd debug if the changes have any impact.

stessy commented 4 years ago

@marcingrzejszczak I think the problem is related to the Temp folder in Windows. For some tests I have an AccessDeniedException when trying to delete recursively a git repo. And some tests fail as well with the AetherStubDownloader. :-( I'm gonna try on my mac.

marcingrzejszczak commented 4 years ago

To everyone willing to help on this issue!

If you haven't already, pick a class / package / module, notify the others in a comment and start converting the classes! Once you're done file a pull request and link it to this issue. If you've picked a module / class / package, I'll add an :+1: emoji to notify that it's a good idea. In any other case I'll leave a comment.

Good luck and thank you so much for your help! :)

BTW I've updated the issue's description. If you think anything else could be put there that could be helpful to others, leave a comment and I'll update it.

stessy commented 4 years ago

@marcingrzejszczak The build fails due to security checks failures. Can you tell me what are the problems ?

stessy commented 4 years ago

Hi,

I have a question.

What's the plan with Closures and GString ? I'm rewriting the spring cloud contract pact module. But some part of the code refers to Closures outside the module. I manage to get rid of local closures (inside the pact module) by converting them to Function, but I'm stuck with the ones implemented outside the module ( i.e: ContentUtils in the spring-cloud-contract-verifier module).

Thanks for your help.

marcingrzejszczak commented 4 years ago

Yeah... so GString is a valid thing inside a Groovy based contract so we need to support it. As for ContentUtils I think I've already started introducing Functions instead of Closures there. So Closures can be rewritten to Functions IMO.

stessy commented 4 years ago

Hi there,

can you tell me what's going on with checkstyle complaining about missing javadoc on line 30 ? Do I have to document each variable in the enum ?

Thanks for your help.

marcingrzejszczak commented 4 years ago

Yeah, a javadoc is necessary over that enum element.

stessy commented 4 years ago

Hi there,

what's the current status of the spring-cloud-contract-verifier module? Is there someone working on it ? @zhmaeff I see that you started the rewrite. Dunno where you arrived until now. Can I help for that module ? If yes, which classes/packages not yet rewritten locally ?

zhmaeff commented 4 years ago

Hi @stessy! You can pick it up) All migrations from my side have been merged

stessy commented 4 years ago

Hi @zhmaeff ,

OK.

Thanks for your prompt reply.

stessy commented 4 years ago

Hi there,

there was a PR to convert SpringCloudContractAssertions from java to groovy: https://github.com/spring-cloud/spring-cloud-contract/pull/1110.

Can we now rewrite the groovy class to java? This means the CollectionAssertSpec groovy class has to be removed as well. But the class CollectionAssertTests covers the same tests.

marcingrzejszczak commented 4 years ago

@stessy hmm there was a reason why it was written in Groovy, however I don't remember what that reason was ;) Can you check that once you rewrite this from groovy to java all the tests including the standalone ones are still passing? :P

stessy commented 4 years ago

@marcingrzejszczak The tests in CollectionAssertSpec groovy class which test the SpringCloudContractAssertions all fail. That's the reason why I deleted that test class too as the CollectionAssertTests covers the same tests for the java class. Now I can rollback my change and keep the groovy class.

santfirax commented 3 years ago

Hello @marcingrzejszczak I would like to work on BodyExtractor.groovy in spring-cloud-contract-verifier. I also noticed that I need ObjectMapperFactory class in spring-cloud-contract-pact module, so, can I add that dependency into spring-cloud-contract-verifier module ?

marcingrzejszczak commented 3 years ago

hey @santfirax , I'd definitely prefer you to copy that class and not rely on the pact module.

orbfish commented 2 years ago

This is sad - I was considering using this project until I saw you were reverting it back to Java. "It's discouraging for some part of the community to write a fix or a new feature in Groovy rather than in Java." Speaking as a Java developer who uses Groovy for testing and builds - people are going to have to move to more modern languages at some point!

shanman190 commented 2 years ago

@orbfish, this is specifically centering around the internal components of Spring Cloud Contract. The available DSLs are being left as they exist today as interfaces for end users to utilize the project.

@marcingrzejszczak's statement was particularly aimed at contributors to Spring Cloud Contract itself. By having a number of pieces that were written in Groovy or Kotlin, it makes it much more difficult for contributions to the Spring Cloud Contract project itself as the contributor very well may have to know how to write, and the idioms therein, in multiple source languages.

There were additionally a lot of bugs that surfaced up via Spring Cloud Contract related specifically to which version of Groovy or Kotlin Spring Cloud Contract provided that then conflicted with what the user had as a dependency. Eg. Groovy 2.5 VS Groovy 3.0 or Kotlin 1.3 VS Kotlin 1.4

The point is that you can absolutely write your contracts in Groovy DSL and Spock as that feature will not be going away anytime soon.