spring-cloud-contract icon indicating copy to clipboard operation
spring-cloud-contract copied to clipboard

Rewrite more Groovy to Java

Open marcingrzejszczak opened this issue 3 years ago • 30 comments

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.

marcingrzejszczak avatar Aug 13 '20 13:08 marcingrzejszczak

Hi, Groovy DSL should be removed also?

armsargis avatar Aug 13 '20 21:08 armsargis

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

santfirax avatar Aug 14 '20 00:08 santfirax

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.

marcingrzejszczak avatar Aug 14 '20 04:08 marcingrzejszczak

I want to contribute

saket88 avatar Aug 14 '20 05:08 saket88

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

marcingrzejszczak avatar Aug 14 '20 06:08 marcingrzejszczak

I can help as well.

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

stessy avatar Aug 14 '20 12:08 stessy

Fantastic!

marcingrzejszczak avatar Aug 14 '20 15:08 marcingrzejszczak

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

shanman190 avatar Aug 14 '20 17:08 shanman190

@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!

marcingrzejszczak avatar Aug 14 '20 18:08 marcingrzejszczak

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

zhmaeff avatar Aug 16 '20 06:08 zhmaeff

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.

marcingrzejszczak avatar Aug 16 '20 07:08 marcingrzejszczak

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 ?

stessy avatar Aug 16 '20 15:08 stessy

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

marcingrzejszczak avatar Aug 16 '20 16:08 marcingrzejszczak

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

stessy avatar Aug 16 '20 17:08 stessy

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.

marcingrzejszczak avatar Aug 17 '20 07:08 marcingrzejszczak

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

stessy avatar Aug 18 '20 07:08 stessy

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.

stessy avatar Aug 23 '20 16:08 stessy

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.

marcingrzejszczak avatar Aug 23 '20 18:08 marcingrzejszczak

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.

stessy avatar Sep 28 '20 06:09 stessy

Yeah, a javadoc is necessary over that enum element.

marcingrzejszczak avatar Sep 28 '20 07:09 marcingrzejszczak

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 ?

stessy avatar Oct 03 '20 10:10 stessy

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

zhmaeff avatar Oct 03 '20 10:10 zhmaeff

Hi @zhmaeff ,

OK.

Thanks for your prompt reply.

stessy avatar Oct 03 '20 11:10 stessy

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.

stessy avatar Oct 26 '20 16:10 stessy

@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

marcingrzejszczak avatar Oct 27 '20 08:10 marcingrzejszczak

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

stessy avatar Oct 27 '20 08:10 stessy

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 ?

santfirax avatar Apr 11 '21 17:04 santfirax

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

marcingrzejszczak avatar Apr 12 '21 06:04 marcingrzejszczak

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!

orbfish avatar Nov 15 '21 00:11 orbfish

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

shanman190 avatar Nov 15 '21 00:11 shanman190