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

Feature/gradle plugin 0 7 0 #54

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

buchananwill
Copy link
Contributor

@buchananwill buchananwill commented Sep 29, 2024

  1. Updated to match the latest main.
  2. Merged in my Gradle plugin module.
  3. Added tests for the plugin output.
  4. Added tests that the output functions with Zod and rejects invalid objects.

The main thing missing is that I haven't added the tests to your existing pipeline, as I wasn't sure how to do that part. I.e. there is a manual Gradle test, and manual Jest test, which both pass and cover the core behaviour of the Gradle plugin, matching the Maven plugin.

I used a very small test sample, as I thought it seemed unnecessarily redundant to perform all the battery of tests as the Maven plugin. The JavaToZod core is the same, so there shouldn't be any divergence in the behaviour. This seemed better than bloating the Gradle test with repetition.


// Skip processing
private boolean skip = false;
private List<String> optionalAnnotations = Collections.emptyList();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to find a way that we can share the definition of accepted parameters between this and the Maven plugin. Will Gradle allow these configuration parameters to be public? I think if that is the case, these could both extend an abstract class with all of the public properties, commented for Gradle, and annotated for Maven. See the Maven mojo: https://github.com/ivangreene/java-to-zod/blob/main/java-to-zod-maven-plugin/src/main/java/sh/ivan/zod/GenerateZodSchemasMojo.java

parameters.isClassNameExcluded = settings.getExcludeFilter();
parameters.classLoader = classLoader;
parameters.scanningAcceptedPackages = scanningAcceptedPackages;
parameters.debug = loggingLevel == Logger.Level.Debug;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we can probably share all of this config/setup once we have an abstract class for all the params

Comment on lines +64 to +88
@Override
public int hashCode() {
return Objects.hash(id, dateOfBirth, fName, lName);
}

@Override
public boolean equals(Object obj) {
if (obj == this) return true;
if (obj == null || obj.getClass() != this.getClass()) return false;
var that = (TestPersonClass) obj;
return Objects.equals(this.id, that.id) &&
Objects.equals(this.dateOfBirth, that.dateOfBirth) &&
Objects.equals(this.fName, that.fName) &&
Objects.equals(this.lName, that.lName);
}

@Override
public String toString() {
return getClass().getSimpleName() + "(" +
"party_id = " + id + ", " +
"dateOfBirth = " + dateOfBirth + ", " +
"fName = " + fName + ", " +
"lName = " + lName + ")";
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Lombok for this

@ivangreene
Copy link
Owner

We'll also want to publish the Gradle plugin to Maven central. Here's an answer that has a bit of info about that: https://stackoverflow.com/questions/67393965/gradle-how-to-publish-a-gradle-plugin-to-maven-central

Regarding the JS tests. I think we should create a new submodule that is POJOs to share between the two plugin-testing modules. We run generate on those with the different plugins, and exec the JS tests against that. That way, we have the exact same coverage of both plugins, and no extra code. Thoughts?

@buchananwill
Copy link
Contributor Author

buchananwill commented Sep 29, 2024 via email

@buchananwill
Copy link
Contributor Author

buchananwill commented Sep 29, 2024

I've gone back over the Gradle plugin implementation and stripped it down to just this:

package sh.ivan.zod;

// imports...

@Setter
public class GenerateZodSchemas extends DefaultTask {

    private static final org.slf4j.Logger log = LoggerFactory.getLogger(GenerateZodSchemas.class);

    // Path and name of generated file
    private File outputFile = null;

    @Nested
    public PluginParameters getPluginParameters() {
        return pluginParameters;
    }

    private PluginParameters pluginParameters = new PluginParameters();

    public GenerateZodSchemas() {}

    @TaskAction
    public void runTask() {
        try {
            init();
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }

    public void init() throws IOException {
        TypeScriptGenerator.setLogger(new Logger(pluginParameters.getLoggingLevel()));
        TypeScriptGenerator.printVersion();
        if (pluginParameters.isSkip()) {
            TypeScriptGenerator.getLogger().info("Skipping plugin execution");
            return;
        }

        // class loader
        List<URL> urls = new ArrayList<>();
        // Add the output directory for your compiled classes (this excludes dependencies)
        File classesDir = new File(getBuildDirectoryAsFile(), "classes/java/main");


        // Check if the directory exists before adding it
        if (classesDir.exists()) {
            urls.add(classesDir.toURI().toURL());
        }

        // Create a class loader using URLs from the classpath
        try (URLClassLoader classLoader = new URLClassLoader(
                urls.toArray(new URL[0]), Thread.currentThread().getContextClassLoader())) {

            new JavaToZodConvertorWrapper(classLoader, pluginParameters, this::getOutputFile).run();

        } catch (IOException e) {
            log.error(e.getMessage());
        }
    }

    @OutputFile
    public File getOutputFile() {

        if (outputFile == null) {
            // Set the default output file location based on the project directory
            outputFile =
                    new File(
                            getBuildDirectoryAsFile(), "java-to-zod/" + getProject().getName() + "-schemas.js");

        }
        return outputFile;
    }

    private @NotNull File getBuildDirectoryAsFile() {
        return getProject().getLayout().getBuildDirectory().get().getAsFile();
    }

}

I think this is about the minimum that is feasible, because in order to leverage the Gradle API certain things have to be directly in this class. This still passes all the tests though, so it's functionally equivalent to my previous iteration.

I need to test tomorrow (Monday) whether the same approach could be applied to Maven. I don't see any reason why it wouldn't - it uses different annotations and also supports nested params. The main drawback is that this may be a breaking change in that all the settings have been push one level deeper, so in Gradle:

 generateZodSchemas {
        outputFile = file("D:\\Coding\\fep-dashboard-1.0.0\\src\\api\\generated-schemas\\test_schemas.ts")
        pluginParameters.jsonLibrary = JsonLibrary.jackson2
        pluginParameters.classPatterns = mutableListOf("**.Dto.*")
        // etc.. 
    }

Instead of:

 generateZodSchemas {
        outputFile = file("D:\\Coding\\fep-dashboard-1.0.0\\src\\api\\generated-schemas\\test_schemas.ts")
        jsonLibrary = JsonLibrary.jackson2
        classPatterns = mutableListOf("**.Dto.*")
        // etc.. 
    }

I tried a couple of other approaches, but this looks like the only way we could re-use the same parameters object across both Maven and Gradle. FWIW, the TypescriptGenerator code actually seems to verbosely duplicate the exact code for the Maven/Gradle versions, without any shared intermediary. I don't know whether the author used a generative approach for this, or simply accepted that as the cost of supporting both platforms identically.

IMO, a documented breaking change like this in order to simplify the design, and make it more maintainable long-term, is an acceptable outcome, but I don't know how many people already use your package? Is there a way the Maven plugin could flatten the parameters while still using the shared internal class? (I.e. keep the same API but refactor the code.)

@buchananwill
Copy link
Contributor Author

I've applied the same approach to the Maven plugin and that also works, with the proviso that all the parameters apart from outputFile need wrapping in an additional tag:

       <configuration>
          <outputFile>${generated-schema-file}</outputFile>
          <pluginParameters>
            <jsonLibrary>jackson2</jsonLibrary>
            <classPatterns>
              <classPattern>sh.ivan.pojo.**</classPattern>
            </classPatterns>
          </pluginParameters>
        </configuration>

I don't think there's any other way to use the same code across both plugins, because they both need to extend the abstract class of the plugin's API package. They are happy to co-annotate the same delegate though, so that is the point where they intersect.

The PluginParameters object itself I put in the core module, so that neither plugin is depending on the other - they both already depend on the core module.

The final complication is that the Gradle Api isn't itself on Maven Central, so the simplest way to supply it to the Maven build (for the core module) was to use an environment variable pointed directly at the jar in my local machine's Gradle cache:

    <dependency>
      <groupId>org.gradle</groupId>
      <artifactId>gradle-api</artifactId>
      <version>8.5</version>
      <scope>system</scope>
      <systemPath>${env.GRADLE_API_JAR}</systemPath>
    </dependency>

This was a fiddly solution to put together, but I think it's a good one, because both plugins are now very lean, and all the parameters are shared apart from the outputFile, which depends on the surrounding build tool context.

/**
 * Generates Zod schemas from specified java classes.
 */
@Mojo(
        name = "generate",
        defaultPhase = LifecyclePhase.PROCESS_CLASSES,
        requiresDependencyResolution = ResolutionScope.COMPILE,
        threadSafe = true)
public class GenerateZodSchemasMojo extends AbstractMojo {

    /**
     * Path and name of generated file.
     */
    @Parameter(
            defaultValue = "${project.build.directory}/java-to-zod/${project.artifactId}-schemas.js",
            required = true)
    public File outputFile;

    @Parameter
    private PluginParameters pluginParameters;

    @Parameter(defaultValue = "${project}", readonly = true, required = true)
    public MavenProject project;

    @Parameter(defaultValue = "${project.build.directory}", readonly = true, required = true)
    public String projectBuildDirectory;

    @Override
    public void execute() throws MojoExecutionException {
        TypeScriptGenerator.setLogger(new Logger(pluginParameters.getLoggingLevel()));
        TypeScriptGenerator.printVersion();
        if (pluginParameters.isSkip()) {
            TypeScriptGenerator.getLogger().info("Skipping plugin execution");
            return;
        }

        // class loader
        List<URL> urls = new ArrayList<>();
        try {
            for (String element : project.getCompileClasspathElements()) {
                urls.add(new File(element).toURI().toURL());
            }
        } catch (DependencyResolutionRequiredException | IOException e) {
            throw new MojoExecutionException(e);
        }

        try (URLClassLoader classLoader = Settings.createClassLoader(
                project.getArtifactId(),
                urls.toArray(new URL[0]),
                Thread.currentThread().getContextClassLoader())) {

            new JavaToZodConvertorWrapper(classLoader, pluginParameters, () -> outputFile).run();

        } catch (IOException e) {
            throw new MojoExecutionException(e);
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants