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

[gradle-plugin] Initial implementation #162

Merged
merged 11 commits into from
May 31, 2018

Conversation

jimschubert
Copy link
Member

@jimschubert jimschubert commented May 27, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

See #141

This is an initial implementation of a gradle-plugin for OpenAPI Generator. I've opened this PR to solicit feedback. Please see the initial commit of README.adoc for details and examples.

To test this out, you'll first need to build the generator:

mvn clean package install

Then, you'll want to build and publish the gradle plugin locally (update inputSpec references to your own locally available specs):

cd gradle
./gradlew build
./gradlew publishToMavenLocal

From here, you can create a simple build.gradle and execute tasks:

buildscript {
  repositories {
   mavenLocal()
   mavenCentral()
    maven {
        url "https://plugins.gradle.org/m2/"
    }
  }
  dependencies {
    classpath "org.openapitools:openapi-generator-gradle-plugin:3.0.0-SNAPSHOT"
  }
}

apply plugin: 'org.openapi.generator'

openApiMeta {
   generatorName = "Jim"
   packageName = "us.jimschubert.example"
}

openApiValidate {
   inputSpec = "/Users/jim/projects/openapi-generator/modules/openapi-generator/src/test/resources/3_0/petstore.yaml"
}

openApiGenerate {
    generatorName = "kotlin"
    inputSpec = "$rootDir/specs/petstore-v3.0.yaml".toString()
    outputDir = "$buildDir/generated".toString()
    apiPackage = "org.openapi.example.api"
    invokerPackage = "org.openapi.example.invoker"
    modelPackage = "org.openapi.example.model"
    modelFilesConstrainedTo = [
            "Error"
    ]
    configOptions = [
        dateLibrary: "java8"
    ]
}

Details around how to configure this to build and published via CI will be worked out after getting feedback on the plugin.

@jmini
Copy link
Member

jmini commented May 28, 2018

I am not a gradle user myself yet (we may like to switch sometime), but I will try to give your plugin a try.

From a code point of view I do not understand why you have separated this into a /gradle/ folder. I would have created this next to the other modules:

  • modules/openapi-generator
  • modules/openapi-generator-maven-plugin
  • modules/openapi-generator-gradle-plugin <= here.

In the future, we might move our complete build system to gradle, then the split modules/ vs gradle/ will make no sense.

@zwenza
Copy link

zwenza commented May 28, 2018

Will give this a try in our project the next days!

@jimschubert
Copy link
Member Author

@jmini modules is a concept that all projects listed as modules are built by a single build system. This isn't buildable by Maven as far as I know. Moving it isn't out of the question, I just wanted there to be clear separation between the two build steps as I gathered feedback.

If anyone knows how to trigger a gradle build from Maven, please let me know. I've only done a cursory search, but found nothing too promising. Specifically, an issue I've found is that we wouldn't have a way to refer to the artifacts built by Maven. I also didn't find a way to refer to the current version (3.0.0-SNAPSHOT) in a shared way.

@jmini
Copy link
Member

jmini commented May 28, 2018

I guess one way is to fall back to the exec-maven-plugin:

<plugin>
  <groupId>org.codehaus.mojo</groupId>
  <artifactId>exec-maven-plugin</artifactId>
  <version>1.2.1</version>
  <executions>
    <execution>
      <id>run-gradle</id>
      <goals>
        <goal>exec</goal>
      </goals>
      <configuration>
      <executable>gradle</executable>
...

or something similar.

I have not tested it completely, but I think that you could create a pom.xml with packaging pom in modules/openapi-generator-gradle-plugin you need also declare openapi-generator as maven dependency (redundant with your graddle setting) in order to have a correct the maven reactor.
Then you will need to run your maven build with mvn (clean) install because gradle will fetch the dependency in the local maven repo.
When you call gradle from maven you can call the task that publish to local maven repo.
After that you can tell maven to get the dependencies from the local maven build and attach and publish it (in case you want to push it somewhere in a remote repo).


An other idea is to move the complete build of our 4 modules to gradle... but we will need to ensure that building the maven plugin still works.

@jimschubert
Copy link
Member Author

@jmini another option I considered is to move the gradle plugin to another repo. I don't see a reason to roll major/minor/revision versions of the generator for changes to the plugin or vice versa.

@jmini
Copy link
Member

jmini commented May 28, 2018

I had the discussion with @wing328 about moving the maven-plugin to an other repository.

Separate repo make sense if you have different release cycle.

At core level it implies also:

  • the need to be more API aware (define what is API and what is not)
  • versioning policy (simrel is great, should be ensured with tools)
  • deprecation/removal policy.

In my opinion the project has not this kind of maturity yet. We have some of this in place, but we are not at the state of the art level.

If you use the policy of having only released version (no snapshot) as dependency in the dependent modules, this means doing more frequent releases of the core

@jimschubert I know that you have started to work on this, you have shared some architecture principle in the past.


Having everything in one repo make also sense and bring some benefit.

  • Have a look at the arguments of google (several talks / papers / article) to explain the benefit of a single repo.
  • The OpenJDK did just reunify several repo into one.

In our project there is no real logic in the "cli", "online-generator", "maven plugin" or "gradle plugin". Each of this dependent projects just contains some glue to call the "core" engine.


Take the --generator-name next big refactoring (#137): if you have multiple repos (core, maven-plugin, gradle-plugin), the change will be spitted into several interdependent PRs. This is also doable but it is hard to manage, harder to test, harder to investigate later on.

With multiple repo, the difficulty is to keep everything in sync. You start to have people looking only at core + maven plugin or at core + cli. And it is much more difficult to see in one PR (and one CI job) that a change in core breaks something in "maven-plugin" for example.


Why just splitting gradle plugin? If you follow this idea, you can split "core", "maven-plugin", "gradle-plugin" and "cli"...

Splitting repo require more release engineering resources. I am not convinced that a project of this size needs it.

@jimschubert
Copy link
Member Author

I don't feel strongly one way or the other about it. My concern is with the build times and level of complexity of builds as we add more tools to the portfolio. My current machine builds everything with tests in about 20 seconds. My old machine, however, took nearly 10 minutes per build. I'm sure that not all contributors have high end machines, so we need to be aware of this.

As some examples of my concerns, can the usage of the gradle exec plugin cause Maven repository conflicts if Maven were configured for parallel builds? Is build flakiness worth having everything in a single repository? How do we verify that the whole process of

  • Maven build/install
  • Gradle build/install
  • Maven publish
  • Gradle publish

occurs in sequence, and that Gradle plugin artifacts don't compile against old/cached and potentially buggy snapshot versions? This scenario is complicated in a multi-build system repository.

This problem isn't new to the introduction of the Gradle plugin, either. We have some rudimentary integration testing in place, which run as part of the test phase. However, for these tests to run correctly, we have in the past had to install locally without tests, then run mvn install with tests. Without following this process, the integrations tests would use a previously cached SNAPSHOT artifact rather than that of the current build.

But, like I said, I don't feel strongly one way or the other. I just think the discussion needs to be pragmatic and represent the effect on the community.

You've referenced Google and OpenJDK as examples of single-repo best practices. Google's MonoRepo is all internal code, and I've read that it takes hours to build and build failures result in immediate revert which sometimes leads to individual small changes taking a day or more to become releases. This works for them because of their amount of testing and integration testing. But, there's a reason why their open source code isn't under a single repository, and I would argue that it would be a barrier of entry to contributors.

As a personal example... last year, I contributed support for 128-bit trace ids to Twitter's Finagle (a mono repo for the finagle stack). Even with all code in a single repository, and knowing the source of individual artifacts pretty well, the sheer amount of code in the repo was overwhelming. My code was accepted and integrated. But having a mono repo didn't solve the problem that I had missed TraceId propagation to downstream http clients, and it didn't solve the problem that multiple people at Twitter approved the PR and missed this as well.

The example of the --generator-name option is a great example. When I implemented this, I did it such that --langs was deprecated and presented a warning. This is done for consumers, not just CLI users. A user who may have integrated CodegenConfigurator into some custom tooling would now know that the option in deprecated and that they'd need to update to take advantage of the new option. The gradle plugin would be an example of such a consumer.

Anyway, my comment was just a suggestion of an alternate approach, and not a comment about a personal preference.

@jimschubert
Copy link
Member Author

I'd be happy to discuss code organization more in depth. If you'd like, can we open another ticket for the discussion, and focus discussions in this PR to the gradle plugin implementation?

When a user sets the models, apis, or supportingFiles environment
variables, any one of these being set disables generation for the other
two.  This could be confusing to users, so I've added some clarification
text in the comments for these properties.

In addition, I've cleaned up the extension on
Property.ifNotEmpty, to avoid using Suppress annotations where it's not
necessary. The change creates a local variable of type T?, allowing
Kotlin to track the variable's nullable state at compile time.
@jimschubert
Copy link
Member Author

I've made a few small changes in 3621a5d. It's somewhat confusing how the generator handles system properties defining a csv of supporting files, apis, and models to generate. Defining any one of these completely disables generation of the other two. The comments should help. I'll work on adding task options to the doc next, followed by maven integration.

@jimschubert
Copy link
Member Author

I have hooked this plugin into mvn clean install. This uses the same version of the overall project.

I'll need to read the docs for publishing and see if there's something that would need to be updated here, but I think this is ready and can make it to the 3.0.0 release. There is a manual testing project under ./samples of the new module. I'll work on adding automated tests on top of the validate task tests I've added.

@jimschubert jimschubert changed the title WIP (feedback requested): [gradle-plugin] Initial commit [gradle-plugin] Initial implementation May 31, 2018
@wing328 wing328 merged commit b6b8c0d into OpenAPITools:master May 31, 2018
@jimschubert jimschubert deleted the gradle-plugin branch June 1, 2018 04:31
@jmini jmini added this to the 3.0.0 milestone Jun 1, 2018
@FatCash
Copy link

FatCash commented Aug 17, 2018

hi @jimschubert, How do you configure tasks for multiple generations ?
I remember with swagger-codegen-cli it was possible to generate from two specs in same project by:

swaggerSources {
    mySpec1 {
        inputFile = file(pathToMySpec1)
        code {
            language = 'spring'
            outputDir = file(Spec1GenerationTargetPath)
       }
    }
    mySpec2 {
        inputFile = file(pathToMySpec2)
        code {
            language = 'spring'
            components = ['models']
            outputDir = file(Spec2GenerationTargetPath)
        }
    }
}

It then created tasks generateSwaggerCodeMySpec1 and generateSwaggerCodeMySpec2
Do you have any gradle tips on how to do similar with the task openApiGenerate

@jimschubert
Copy link
Member Author

@FatCash

swagger-codegen-cli doesn't have a Gradle plugin, so I'm not sure what the DSL of that plugin would have done. It's odd to me that the extension is called swaggerSources, and then lists input spec, output directory, and codegen configurations.

openapi-generator's gradle plugin offers a declarative DSL via extensions (these are Gradle project extensions). The extensions are documented in the plugin's README.md, and map almost fully 1:1 with the options you'd pass to the CLI or Maven plugin. The plugin maps the extensions to a task of the same name to provide a clean API. If you're interested in the extension/task mapping concept from a high-level, you can check out Gradle's docs.

If you want to perform multiple tasks, you'd want to create a task that inherits from the GenerateTask. I have a few examples in openapi-generator-gradle-plugin/samples/local-spec/build.gradle.

To match the example you are using for that old swagger based plugin:

task buildSpec1(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTask){
    generatorName = "spring"
    inputSpec = file(pathToMySpec1).toString()
    additionalProperties = [
            packageName: "petstore"
    ]
    outputDir = file(Spec1GenerationTargetPath).toString()
    configOptions = [
            dateLibrary: "threetenp"
    ]
}

task buildSpec2(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTask){
    generatorName = "spring"
    inputSpec = file(pathToMySpec2).toString()
    additionalProperties = [
            models: "true",
            modelDocs: "true",
            apis: "false",
            apiDocs: "false",
            apiTests: "false"
    ]
    outputDir = file(Spec1GenerationTargetPath).toString()
}

To execute your specs, you'd then do ./gradlew buildSpec1 buildSpec2.

If you want to simplify the execution, you could create a task with dependsOn.

task codegen(dependsOn: ['buildSpec1', 'buildSpec2'])

Or, if you're generating the code on compile, maybe this:

compileJava.dependsOn buildSpec1, buildSpec2

If you have a single spec, and you want to add it as a dependency to compileJava, you need to gain a reference to the task. One way to do this is:

compileJava.dependsOn tasks.openApiGenerate

@FatCash
Copy link

FatCash commented Aug 20, 2018

Hi, Thank you for the thorough explanation.
It works perfectly like you said! I was stuck on the gradle task syntax, not aware of possibility to do task buildSpec1(type: ....GenerateTask).
Also, thank you for explaining the syntax for dependsOn.
I think you should put this to readme, it can not hurt and will most certainly help someone!

@jmini
Copy link
Member

jmini commented Aug 20, 2018

=> See #847

@Place1 Place1 mentioned this pull request Sep 28, 2018
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants