Breaking/migrate maven to plugin parameters

[buchananwill]

This PR combines the work I've done yesterday and today on both the Gradle and Maven plugins. The first half is a duplicate of the other pull request which is only the Gradle plugin. I've done this so you can view the entire proposal in context. I don't mind if you want to accept the smaller PR first (or only), and address the Maven aspect separately. However I think it makes more sense to see the changes as a whole, which really comprises three stages:

1. Add Gradle plugin
2. Pull the parameters into the core module without breaking the Gradle plugin
3. Replace the matching parameters from the Maven plugin with the shared class now available from the Core module.

Also, I didn't like the idea of pre-empting the version bump as that's totally your prerogative as repo owner, but it seemed like the safest way to ensure that I wasn't testing stale caches in the dependent modules. I won't be offended if you want to take a different tack or increment differently.

[buchananwill]

I did a bit more digging, and the gradle api jar is hosted by Jetbrains, so I replaced the local .env with that, and the build runs now.

[ivangreene]

Hi, thanks for updating this. I kind of have a full week so probably won’t get a chance to dig in and run it until this weekend. Will leave some comments when I can

[buchananwill]

No problem, take your time! :-)

…

On Tue, 1 Oct 2024, 21:15 Ivan Greene, ***@***.***> wrote: Hi, thanks for updating this. I kind of have a full week so probably won’t get a chance to dig in and run it until this weekend. Will leave some comments when I can — Reply to this email directly, view it on GitHub <#55 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BBL2M6IMBOI42CWBFGRX2VLZZL7FZAVCNFSM6AAAAABPDDMY66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBWHE4DGOBTGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ivangreene]

Thanks for your patience. I had some time to review this over the weekend. Here are some changes I'd love to see:

• Build and package the gradle plugin with Maven
• Align the tests of the two plugins to share more example code from a shared submodule
• Convert the configuration class into a class that is extended by both the Maven and Gradle plugins, so that the config can be at the top level and we don't have to break the API. This keeps aligned with typescript-generator

[ivangreene]

We can generate the appropriate getters in this way:

Suggested change

	public class PluginParameters implements Serializable {
	@Getter(onMethod=@__({@Input}))
	public class PluginParameters implements Serializable {

Not sure of the exact syntax. See Lombok docs: https://projectlombok.org/features/GetterSetter

[ivangreene]

Do we actually need to implement Serializable too? I would also like to see this class extended by the Maven Mojo and the Gradle equivalent, so that the config properties are defined at the top level. I think that may eliminate the need for Serializable.

I guess to make this happen, we would need to extend both AbstractMojo and DefaultTask in PluginParameters. I think in that case, it makes more sense to name this BasePlugin or something.

[ivangreene]

For Optional Inputs, we can use a variation of Getter on the specific field declaration:

@Getter(onMethod=@__({@Input, @Optional}))
private String schemaNameSuffix = "Schema";

[ivangreene]

Although this is a Gradle plugin, I would like to see this built with Maven to keep consistency in the project. It looks like typescript-generator has accomplished this. I think this is mostly encapsulated in that sub-module's pom.xml: https://github.com/vojtechhabarta/typescript-generator/blob/main/typescript-generator-gradle-plugin/pom.xml
As well as this META-INF file: https://github.com/vojtechhabarta/typescript-generator/blob/main/typescript-generator-gradle-plugin/src/main/resources/META-INF/gradle-plugins/cz.habarta.typescript-generator.properties

[ivangreene]

This class should go into its own submodule, java-to-zod-plugin-support or something like that. It shouldn't be packaged with the core. Same with the class above

[ivangreene]

Is this the typical way to configure a plugin like this in Gradle? I'm expecting something more like this: https://github.com/vojtechhabarta/typescript-generator/blob/main/sample-gradle/build.gradle#L44-L65

[ivangreene]

I personally think we should delete this test. It looks way too reliant on internal details of how Gradle plugins work. I'm more interested in asserting that the can be used from a regular old Gradle project, and generate a file that meets expectations. This is what we test in the Maven plugin. Thoughts?

[ivangreene]

No need to change the version number. This is all handled automatically when I do the release

[ivangreene]

Suggested name: JavaToZodPluginConverter. Thoughts?

[ivangreene]

Do these comments get added to any kind of Gradle online documentation system? Would love to retain these somehow. Worst case, we can always link to the generated Maven configuration docs. Here's what that looks like: https://java-to-zod.ivan.sh/java-to-zod-maven-plugin/generate-mojo.html

This reminds me, we may want to find a way to publish any generated Gradle documentation to the site along with the Maven version. Site: https://java-to-zod.ivan.sh/

[ivangreene]

Let's not add more tests here. I'd like to maintain exact test parity with the Maven plugin. This can be accomplished in a few steps:

1. Move the POJOs in java-to-zod-maven-plugin-test into their own new submodule named java-to-zod-plugin-test-support
2. Bring that module in as a dependency to both java-to-zod-maven-plugin-test and to the module where we test the Gradle plugin
3. Find a way to share the JS files (symlink a directory that contains them in both projects maybe?): https://github.com/ivangreene/java-to-zod/tree/main/java-to-zod-maven-plugin-test/js - not the schemas file though, that will get generated by each plugin
4. The test of the Gradle plugin should exist as a separate directory. It will be the only Gradle project in the tree. It will configure the Gradle plugin, and then run the same JS tests. I'm assuming that's mostly what this gradle build file does? https://github.com/ivangreene/java-to-zod/blob/6735d0785a0fb59b5e655b49d21997ebed93d6a4/java-to-zod-gradle-plugin/src/test/resources/build.gradle.kts

[buchananwill]

Isn't single parent class extension a fundamental feature of Java? I.e. a class cannot extend two different super/parent classes. That was why I implemented the shared delegate class.

…

On Tue, 15 Oct 2024, 00:46 Ivan Greene, ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks for your patience. I had some time to review this over the weekend. Here are some changes I'd love to see: - Build and package the gradle plugin with Maven - Align the tests of the two plugins to share more example code from a shared submodule - Convert the configuration class into a class that is extended by both the Maven and Gradle plugins, so that the config can be at the top level and we don't have to break the API. This keeps aligned with typescript-generator ------------------------------ In java-to-zod-core/src/main/java/sh/ivan/zod/plugins/PluginParameters.java <#55 (comment)> : > @@ -0,0 +1,320 @@ +package sh.ivan.zod.plugins; + +import cz.habarta.typescript.generator.*; +import java.io.Serializable; +import java.util.Collections; +import java.util.List; +import lombok.Setter; +import org.apache.maven.plugins.annotations.Parameter; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Optional; + ***@***.*** +public class PluginParameters implements Serializable { We can generate the appropriate getters in this way: ⬇️ Suggested change -public class PluginParameters implements Serializable { ***@***.***(onMethod=@***@***.***})) +public class PluginParameters implements Serializable { Not sure of the exact syntax. See Lombok docs: https://projectlombok.org/features/GetterSetter ------------------------------ In java-to-zod-core/src/main/java/sh/ivan/zod/plugins/PluginParameters.java <#55 (comment)> : > + @parameter + private Logger.Level loggingLevel = Logger.Level.Verbose; + + @parameter(property = "java.to.zod.skip") + private boolean skip = false; + + /** + * The presence of any annotation in this list on a JSON property will cause + * the typescript-generator to treat that property as optional when generating + * the corresponding TypeScript interface. + * Example optional annotation: <code>javax.annotation.Nullable</code>. + */ + private List<String> optionalAnnotations = Collections.emptyList(); + + @input + @optional For Optional Inputs, we can use a variation of Getter on the specific field declaration: @Getter(onMethod=@***@***.***, @optional}))private String schemaNameSuffix = "Schema"; ------------------------------ In java-to-zod-gradle-plugin/build.gradle.kts <#55 (comment)> : > @@ -0,0 +1,60 @@ +plugins { Although this is a Gradle plugin, I would like to see this built with Maven to keep consistency in the project. It looks like typescript-generator has accomplished this. I think this is mostly encapsulated in that sub-module's pom.xml: https://github.com/vojtechhabarta/typescript-generator/blob/main/typescript-generator-gradle-plugin/pom.xml As well as this META-INF file: https://github.com/vojtechhabarta/typescript-generator/blob/main/typescript-generator-gradle-plugin/src/main/resources/META-INF/gradle-plugins/cz.habarta.typescript-generator.properties ------------------------------ In java-to-zod-core/src/main/java/sh/ivan/zod/plugins/PluginParameters.java <#55 (comment)> : > @@ -0,0 +1,320 @@ +package sh.ivan.zod.plugins; This class should go into its own submodule, java-to-zod-plugin-support or something like that. It shouldn't be packaged with the core. Same with the class above ------------------------------ In java-to-zod-gradle-plugin/src/test/resources/build.gradle.kts <#55 (comment)> : > +tasks.withType<sh.ivan.zod.GenerateZodSchemas> { + val tempOutputDir = layout.buildDirectory.dir("temp/generated-schemas") + outputFile = tempOutputDir.map { it.file("schemas.ts") }.get().asFile + pluginParameters.jsonLibrary = JsonLibrary.jackson2 + pluginParameters.classes = mutableListOf("sh.ivan.zod.resources.TestPersonClass", "sh.ivan.zod.resources.TestPersonRecord") +} Is this the typical way to configure a plugin like this in Gradle? I'm expecting something more like this: https://github.com/vojtechhabarta/typescript-generator/blob/main/sample-gradle/build.gradle#L44-L65 ------------------------------ In java-to-zod-gradle-plugin/src/test/java/sh/ivan/zod/GenerateZodSchemasTest.java <#55 (comment)> : > +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Arrays; + +import static org.gradle.testkit.runner.TaskOutcome.SUCCESS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GenerateZodSchemasTest { I personally think we should delete this test. It looks way too reliant on internal details of how Gradle plugins work. I'm more interested in asserting that the can be used from a regular old Gradle project, and generate a file that meets expectations. This is what we test in the Maven plugin. Thoughts? ------------------------------ In java-to-zod-core/pom.xml <#55 (comment)> : > @@ -4,7 +4,7 @@ <parent> <groupId>sh.ivan</groupId> <artifactId>java-to-zod</artifactId> - <version>0.7.0-SNAPSHOT</version> + <version>0.8.0-SNAPSHOT</version> No need to change the version number. This is all handled automatically when I do the release ------------------------------ In java-to-zod-core/src/main/java/sh/ivan/zod/JavaToZodConvertorWrapper.java <#55 (comment)> : > @@ -0,0 +1,71 @@ +package sh.ivan.zod; + +import cz.habarta.typescript.generator.*; +import cz.habarta.typescript.generator.parser.Model; +import java.io.File; +import java.io.IOException; +import java.net.URLClassLoader; +import java.util.Map; +import java.util.function.Supplier; +import sh.ivan.zod.plugins.PluginParameters; +import sh.ivan.zod.schema.ObjectSchema; + +public class JavaToZodConvertorWrapper { Suggested name: JavaToZodPluginConverter. Thoughts? ------------------------------ In java-to-zod-core/src/main/java/sh/ivan/zod/plugins/PluginParameters.java <#55 (comment)> : > + +import cz.habarta.typescript.generator.*; +import java.io.Serializable; +import java.util.Collections; +import java.util.List; +import lombok.Setter; +import org.apache.maven.plugins.annotations.Parameter; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Optional; + ***@***.*** +public class PluginParameters implements Serializable { + private static final long serialVersionUID = 1L; + + /** + * Prefix added to generated schema names. Do these comments get added to any kind of Gradle online documentation system? Would love to retain these somehow. Worst case, we can always link to the generated Maven configuration docs. Here's what that looks like: https://java-to-zod.ivan.sh/java-to-zod-maven-plugin/generate-mojo.html This reminds me, we may want to find a way to publish any generated Gradle documentation to the site along with the Maven version. Site: https://java-to-zod.ivan.sh/ ------------------------------ In java-to-zod-gradle-plugin/src/test/java/sh/ivan/zod/resources/TestPersonClass.java <#55 (comment)> : > @@ -0,0 +1,89 @@ +package sh.ivan.zod.resources; + +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; + +import java.util.Date; +import java.util.Objects; + +public final class TestPersonClass { Let's not add more tests here. I'd like to maintain exact test parity with the Maven plugin. This can be accomplished in a few steps: 1. Move the POJOs in java-to-zod-maven-plugin-test <https://github.com/ivangreene/java-to-zod/tree/main/java-to-zod-maven-plugin-test/src/main/java/sh/ivan/pojo> into their own new submodule named java-to-zod-plugin-test-support 2. Bring that module in as a dependency to both java-to-zod-maven-plugin-test and to the module where we test the Gradle plugin 3. Find a way to share the JS files (symlink a directory that contains them in both projects maybe?): https://github.com/ivangreene/java-to-zod/tree/main/java-to-zod-maven-plugin-test/js - not the schemas file though, that will get generated by each plugin 4. The test of the Gradle plugin should exist as a separate directory. It will be the only Gradle project in the tree. It will configure the Gradle plugin, and then run the same JS tests. I'm assuming that's mostly what this gradle build file does? https://github.com/ivangreene/java-to-zod/blob/6735d0785a0fb59b5e655b49d21997ebed93d6a4/java-to-zod-gradle-plugin/src/test/resources/build.gradle.kts ------------------------------ In java-to-zod-core/src/main/java/sh/ivan/zod/plugins/PluginParameters.java <#55 (comment)> : > @@ -0,0 +1,320 @@ +package sh.ivan.zod.plugins; + +import cz.habarta.typescript.generator.*; +import java.io.Serializable; +import java.util.Collections; +import java.util.List; +import lombok.Setter; +import org.apache.maven.plugins.annotations.Parameter; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Optional; + ***@***.*** +public class PluginParameters implements Serializable { Do we actually need to implement Serializable too? I would also like to see this class extended by the Maven Mojo and the Gradle equivalent, so that the config properties are defined at the top level. I think that may eliminate the need for Serializable. I guess to make this happen, we would need to extend both AbstractMojo and DefaultTask in PluginParameters. I think in that case, it makes more sense to name this BasePlugin or something. — Reply to this email directly, view it on GitHub <#55 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BBL2M6LOYXXZWFWP2IH4I2TZ3RJXBAVCNFSM6AAAAABPDDMY66VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNRXG44TSOJYGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ivangreene]

Oh yeah... brain not working lol. Hmm... maybe we can do something else there.

In the end, I'm trying to figure out what is a bigger priority:

1. Share the configuration params in a single class
2. Keep the API the same as typescript-generator with config at the top level

Probably number 1. So let's see if there's a way to "unwrap" the config for Maven or Gradle and do both. But ultimately this seems fine if we can't figure that out

[buchananwill]

I've had a read around, and it looks like it may be possible to hoist/flatten the internal delegates getters/setters using another custom annotation(!). That would result in the final class having the same API surface as the current maven/ Typescript plugins. Need to test this idea tomorrow though!

…

On Tue, 15 Oct 2024, 02:25 Ivan Greene, ***@***.***> wrote: Oh yeah... brain not working lol. Hmm... maybe we can do something else there. In the end, I'm trying to figure out what is a bigger priority: 1. Share the configuration params in a single class 2. Keep the API the same as typescript-generator with config at the top level Probably number 1. So let's see if there's a way to "unwrap" the config for Maven or Gradle and do both. But ultimately this seems fine if we can't figure that out — Reply to this email directly, view it on GitHub <#55 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BBL2M6M3FIK6K6OFUA5YBV3Z3RVHJAVCNFSM6AAAAABPDDMY66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJSGYZTCMBXHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>