-
Notifications
You must be signed in to change notification settings - Fork 0
3614: WIP: AVRO-4223 Gradle plugin for generating Java code #32
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
base: main
Are you sure you want to change the base?
Conversation
Fix Intellij not finding generated sources Add generating BigDecimal property
Add dynamic output directory
WalkthroughThis pull request introduces a new Apache Avro Gradle plugin module at ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @martin-augment, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Gradle plugin for Apache Avro, enabling the generation of Java code from Avro schemas and protocols within Gradle-based projects. This significantly expands Avro's build tool support beyond Maven, providing a native Gradle experience for schema compilation and code generation. The plugin is implemented using Kotlin DSL and includes comprehensive configuration options, integration with the existing Maven build for its own lifecycle, and thorough testing to ensure robust functionality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new Gradle plugin for generating Java code from Avro schemas, which is a great addition. The implementation is comprehensive, including plugin logic, tasks, extensions, and tests.
My review focuses on a few key areas. The most critical issue is the use of a non-Apache group ID and package name (eu.eventloopsoftware) throughout the plugin's code and configuration. For an official Apache Avro subproject, this must be changed to org.apache.avro. I've also identified a bug in the main compilation task where it incorrectly uses the test source directory for protocol files. Additionally, there are opportunities to improve dependency management, error handling, and code clarity. The tests are well-structured using GradleRunner, which is excellent.
Overall, this is a solid foundation for the Gradle plugin, and addressing these points will bring it in line with Apache project standards and improve its robustness.
| group = "eu.eventloopsoftware" | ||
|
|
||
| version = "0.1.0" | ||
|
|
||
| repositories { | ||
| mavenCentral() | ||
| mavenLocal() | ||
| } | ||
|
|
||
| dependencies { | ||
| // TODO: for release use ${version} | ||
| implementation("org.apache.avro:avro-compiler:1.12.1") | ||
| implementation("org.jetbrains.kotlin:kotlin-gradle-plugin-api:2.3.0") | ||
| testImplementation(kotlin("test")) | ||
| } | ||
|
|
||
| tasks.test { useJUnitPlatform() } | ||
|
|
||
| kotlin { jvmToolchain(21) } | ||
|
|
||
| gradlePlugin { | ||
| plugins { | ||
| website = "https://avro.apache.org/" | ||
| vcsUrl = "https://github.com/apache/avro.git" | ||
| register("gradlePlugin") { | ||
| id = "eu.eventloopsoftware.avro-gradle-plugin" | ||
| displayName = "Avro Gradle Plugin" | ||
| description = "Avro Gradle plugin for generating Java code" | ||
| tags = listOf("avro", "kotlin", "java", "apache") | ||
| implementationClass = "eu.eventloopsoftware.avro.gradle.plugin.AvroGradlePlugin" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The group, plugin id, and implementationClass are all using eu.eventloopsoftware. As this plugin is being contributed to the Apache Avro project, these should be updated to use the org.apache.avro namespace to align with the project's conventions and ownership. This is a critical change for integration into the Apache ecosystem.
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package eu.eventloopsoftware.avro.gradle.plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| dependencies { | ||
| // TODO: for release use ${version} | ||
| implementation("org.apache.avro:avro-compiler:1.12.1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The avro-compiler version is hardcoded. The comment on the preceding line suggests this is a known issue. For better maintainability and to ensure version consistency within the Avro project, this should be replaced with a property that references the main project version (e.g., ${project.version} or a similar mechanism if possible in a mixed Maven/Gradle build). The hardcoded version 1.12.1 also differs from the parent project's version 1.13.0-SNAPSHOT.
| project, | ||
| extension, | ||
| includesProtocol, | ||
| extension.testSourceDirectory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The registerSchemaTask for main sources is incorrectly configured to look for protocol files (.avpr) in the testSourceDirectory. It should be using the main sourceDirectory for both schemas and protocols. This will cause protocol files in the main source set to be ignored.
| extension.testSourceDirectory, | |
| extension.sourceDirectory, |
| import org.gradle.api.tasks.InputFiles | ||
| import org.gradle.api.tasks.OutputDirectory | ||
|
|
||
| abstract class AbstractCompileTask : DefaultTask() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an inconsistency in how classloaders are used for loading custom classes.
loadLogicalTypesFactoriescreates and uses aURLClassLoaderwith the project's runtime classpath.doCompileusesThread.currentThread().getContextClassLoader()to load custom conversions.instantiateAdditionalVelocityToolsusesClass.forName(), which relies on the current class's loader.
This can lead to ClassNotFoundException if dependencies are not available on all of these different classpaths. For robustness, all custom class loading should consistently use a URLClassLoader configured with the project's runtime classpath. Consider refactoring to create a single project classloader instance that can be reused for all three purposes.
|
|
||
| ```kotlin | ||
| plugins { | ||
| id("eu.eventloopsoftware.avro-gradle-plugin") version "0.0.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plugin version in this usage example is 0.0.8, but the version defined in build.gradle.kts is 0.1.0. To avoid confusion for users, it's best to keep the documentation examples aligned with the actual version being developed.
| id("eu.eventloopsoftware.avro-gradle-plugin") version "0.0.8" | |
| id("eu.eventloopsoftware.avro-gradle-plugin") version "0.1.0" |
| * @parameter property="outputDirectory" default-value="${project.layout.buildDirectory}/generated-test-sources/avro" | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This @parameter comment appears to be copied from a Maven plugin's Javadoc and is not applicable here. It's also misleading, as it refers to outputDirectory while documenting testOutputDirectory, and the default value syntax is for Maven properties, not Gradle. It would be clearer to remove this comment, as the property's purpose is already well-documented in the KDoc above it.
|
|
||
| private fun compileSchemas(schemaFileTree: ConfigurableFileCollection, outputDirectory: File) { | ||
| val sourceFileForModificationDetection: File? = | ||
| schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxBy { it.lastModified() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maxBy function is deprecated in favor of maxByOrNull to handle cases where the collection is empty, preventing a NoSuchElementException. Using maxByOrNull would make the code more robust.
| schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxBy { it.lastModified() } | |
| schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxByOrNull { it.lastModified() } |
| } catch (ex: IOException) { | ||
| throw RuntimeException( | ||
| "IO ex: Error compiling a file in " + schemaFileTree.asPath + " to " + outputDirectory, | ||
| ex, | ||
| ) | ||
| } catch (ex: SchemaParseException) { | ||
| throw RuntimeException( | ||
| "SchemaParse ex Error compiling a file in " + schemaFileTree.asPath + " to " + outputDirectory, | ||
| ex, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better integration with Gradle's build lifecycle and error reporting, it's recommended to throw GradleException instead of RuntimeException. This provides more context to the user and allows Gradle to handle the failure more gracefully. This is already done in AbstractCompileTask when handling logical type factories.
| } catch (ex: IOException) { | |
| throw RuntimeException( | |
| "IO ex: Error compiling a file in " + schemaFileTree.asPath + " to " + outputDirectory, | |
| ex, | |
| ) | |
| } catch (ex: SchemaParseException) { | |
| throw RuntimeException( | |
| "SchemaParse ex Error compiling a file in " + schemaFileTree.asPath + " to " + outputDirectory, | |
| ex, | |
| ) | |
| } catch (ex: IOException) { | |
| throw org.gradle.api.GradleException( | |
| "Error compiling a file in " + schemaFileTree.asPath + " to " + outputDirectory, | |
| ex, | |
| ) | |
| } catch (ex: SchemaParseException) { | |
| throw org.gradle.api.GradleException( | |
| "Error compiling a file in " + schemaFileTree.asPath + " to " + outputDirectory, | |
| ex, | |
| ) |
| import org.gradle.testkit.runner.TaskOutcome | ||
| import org.junit.jupiter.api.io.TempDir | ||
|
|
||
| @ExperimentalPathApi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Augment PR SummarySummary: Introduces a new (WIP) Gradle plugin module to generate Java sources from Avro schemas/protocols. Changes:
Technical Notes: The generator delegates to Avro 🤖 Was this summary useful? React with 👍 or 👎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| project, | ||
| extension, | ||
| includesProtocol, | ||
| extension.testSourceDirectory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ) | ||
|
|
||
| compileSchemaTask.runtimeClassPathFileCollection.from( | ||
| project.configurations.getByName("runtimeClasspath").files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using project.configurations.getByName("runtimeClasspath").files will resolve the configuration during configuration-time task wiring, which can unexpectedly force dependency resolution and interfere with configuration caching.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ) { | ||
| compileTask.outputDirectory.set(project.layout.buildDirectory.dir(outputDirectory)) | ||
| compileTask.fieldVisibility.set(extension.fieldVisibility) | ||
| compileTask.testExcludes.set(extension.testExcludes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testExcludes is set on the task, but file selection currently always uses extension.excludes, so testExcludes has no effect on which test schemas/protocols are compiled.
Other Locations
lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/AvroGradlePlugin.kt:153lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/AvroGradlePlugin.kt:171
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| private fun instantiateAdditionalVelocityTools(velocityToolsClassesNames: List<String>): List<Any> { | ||
| return velocityToolsClassesNames.map { velocityToolClassName -> | ||
| try { | ||
| Class.forName(velocityToolClassName).getDeclaredConstructor().newInstance() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| </execution> | ||
| <execution> | ||
| <id>run-gradle-task-publish</id> | ||
| <phase>deploy</phase> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| dependencies { | ||
| // TODO: for release use ${version} | ||
| implementation("org.apache.avro:avro-compiler:1.12.1") | ||
| implementation("org.jetbrains.kotlin:kotlin-gradle-plugin-api:2.3.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Review: AVRO-4223 Gradle PluginI've completed a thorough review of this PR that adds a new Gradle plugin for generating Java code from Avro schemas. Overall, this is a well-structured and valuable addition to the Avro project. StrengthsCode Quality
Test Coverage
Documentation
Issues & RecommendationsCritical Issues1. Package Naming Inconsistency (build.gradle.kts:25) 2. Hardcoded Version Dependency (build.gradle.kts:36) 3. README Typo (README.md:29) 4. Potential Bug in Protocol Files (AvroGradlePlugin.kt:79) Security & Performance5. Reflection-based Class Loading (AbstractCompileTask.kt:96-100) 6. Exception Handling 7. Missing Task Caching Documentation Issues8. Version Documentation Confusion 9. Outdated Examples Recommendations for MergeMust address before merging:
Nice to have:
SummaryThis is a solid contribution that adds valuable Gradle support to Apache Avro. The code is well-written, follows Gradle best practices, and includes good test coverage. Main concerns are package naming consistency, a potential copy-paste bug, and documentation cleanup. Once the critical issues are addressed, this will be ready to merge. Great work! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@lang/java/gradle-plugin/build.gradle.kts`:
- Around line 19-23: The Kotlin JVM plugin version declared in the plugins block
(kotlin("jvm") version "2.2.10") is mismatched with the kotlin-gradle-plugin-api
dependency version ("kotlin-gradle-plugin-api:2.3.0"); update one so both match
(either bump the plugin version to 2.3.0 or change the dependency to 2.2.10) to
ensure consistent Kotlin API/runtime; modify the version string in the plugins
block or the dependency declaration accordingly (look for kotlin("jvm") version
and the kotlin-gradle-plugin-api dependency).
In `@lang/java/gradle-plugin/pom.xml`:
- Around line 94-105: The publishPlugins exec execution (id
"run-gradle-task-publish", goal "exec", executable "./gradlew", arguments
"publishPlugins") must be made opt-in: move that execution into a Maven profile
activated only when a property like "publish.plugins" is true (so it does not
run by default), or add an activation/property guard around it; require callers
to pass -Dpublish.plugins=true to enable publishing. Ensure the profile wraps
the same exec configuration and keep the id "run-gradle-task-publish" so it’s
easy to locate and verify.
In `@lang/java/gradle-plugin/README.md`:
- Line 20: Replace bare URLs in the README lines that contain "Now released on
Gradle plugin portal:
https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin" and
the other occurrence (line 68) with inline Markdown links; e.g., convert the
plain URL to a descriptive link like [Gradle Plugin
Portal](https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin)
so the text uses Markdown link syntax instead of a bare URL.
- Around line 72-78: The FAQ section in the README jumps heading levels (H2 to
H4); normalize them so markdownlint passes by changing the FAQ subheadings to
H3. Specifically update the headings "How can I benefit from Kotlin's null
safety?" and "I get my Avro schemas from a Maven dependency, how can I add JAR
files that contain schemas?" from H4 to H3 (or make the parent FAQ an H2 and
these H3s) so the hierarchy is consistent.
- Around line 28-30: Remove the stray fragment "pu" that was accidentally added
after the sentence "Add `sourceZipFiles` property to add zip files with schemas
in them" in the README; edit the README.md to delete that lone "pu" line so the
documentation reads cleanly without the accidental fragment.
- Line 24: Replace the two-word phrase "any more" with the adverb "anymore" in
the README sentence that mentions tasks.named("compileKotlin") {
dependsOn(tasks.named("avroGenerateJavaClasses")) }, i.e., update the wording so
it reads "It is not needed to add ... anymore" to correct the wording flagged by
LanguageTool.
In
`@lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/AvroGradlePlugin.kt`:
- Around line 74-80: The call that wires protocol files for the main schema task
is passing extension.testSourceDirectory to addProtocolFiles; update the call
made when creating/registering the main compile task (the
registerSchemaTask/compileSchemaTask invocation) to pass
extension.sourceDirectory (the main sources) instead of
extension.testSourceDirectory so the main compilation sources come from
src/main/avro; locate the addProtocolFiles(...) call that includes
compileSchemaTask and replace the testSourceDirectory argument with the main
sourceDirectory symbol.
In
`@lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/extension/AvroGradlePluginExtension.kt`:
- Around line 47-54: The KDoc for testSourceDirectory is incorrect: it describes
an output directory but testSourceDirectory is an input/source directory; update
the comment above the Property<String> testSourceDirectory to state that it is
the input/source directory for generated test Java code (e.g., "The input/source
directory containing Avro IDL/JSON files for generating test Java code") so it
no longer matches the output wording used for testOutputDirectory.
In
`@lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/tasks/CompileAvroSchemaTask.kt`:
- Around line 46-48: In compileSchemas, the use of maxBy on the filtered file
list can throw NoSuchElementException when the collection is empty; change the
call to maxByOrNull when computing sourceFileForModificationDetection and add a
null-safe handling path (e.g., treat sourceFileForModificationDetection as
nullable and only use it if non-null or provide a fallback) so the task no
longer crashes when no files match the filter; update references to
sourceFileForModificationDetection accordingly in the CompileAvroSchemaTask.
In `@lang/java/gradle-plugin/src/test/avro/multipleSchemas/ApplicationEvent.avsc`:
- Around line 26-41: The inline Avro record definition for MyResponse inside the
"response" field should be removed and replaced with a reference to the external
schema: update the "response" field (name "response") to use the named type
"model.MyResponse" instead of the inline record named "MyResponse", and delete
the inline record block so the code relies on the standalone MyResponse.avsc
schema.
In
`@lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt`:
- Around line 278-285: The helper assertFilesExist currently uses
directory.listDirectoryEntries(), which only returns immediate children and
therefore misses Java files generated into package subdirectories (e.g.,
namespaces like "model" produce files under subfolders); update assertFilesExist
to perform a recursive walk (e.g., using directory.walk() or Files.walk) and
collect package-relative paths (or include subdirectory names in the file names)
before comparing to expectedFiles, or alternatively expand expectedFiles to
include package-relative paths; reference the assertFilesExist function and the
use of listDirectoryEntries() to locate and change the logic to a recursive file
collection.
🧹 Nitpick comments (5)
lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/AvroGradlePlugin.kt (1)
82-84: Eager configuration resolution may cause issues.Calling
.fileson a configuration resolves it eagerly during task configuration. This can cause problems with configuration resolution order and may trigger deprecation warnings in Gradle 9.Consider using the configuration directly or wrapping in a
Provider:♻️ Suggested approach
- compileSchemaTask.runtimeClassPathFileCollection.from( - project.configurations.getByName("runtimeClasspath").files - ) + compileSchemaTask.runtimeClassPathFileCollection.from( + project.configurations.named("runtimeClasspath") + )lang/java/gradle-plugin/build.gradle.kts (1)
29-39: Consider removingmavenLocal()before release.Using
mavenLocal()(Line 31) can cause build reproducibility issues since local Maven cache content varies between machines. Consider removing it for the release version, or document when it's needed (e.g., for local avro-compiler development).The TODO on Line 35 indicates the avro-compiler version should be parameterized for release.
lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/tasks/AbstractCompileTask.kt (3)
94-100: Inconsistent exception wrapping and classloader usage.
Line 99: Wrapping
ClassNotFoundExceptioninIOExceptionis semantically misleading. Consider usingGradleExceptionfor consistency with Line 155.Lines 96 vs 149: Custom conversions use
Thread.currentThread().getContextClassLoader()while logical type factories use the customURLClassLoader. This inconsistency may cause class loading failures if custom conversions are in the runtime classpath but not the context classloader.♻️ Suggested fix for consistency
try { - for (customConversion in customConversions.get()) { - compiler.addCustomConversion(Thread.currentThread().getContextClassLoader().loadClass(customConversion)) - } - } catch (e: ClassNotFoundException) { - throw IOException(e) + createClassLoader().use { classLoader -> + for (customConversion in customConversions.get()) { + compiler.addCustomConversion(classLoader.loadClass(customConversion)) + } + } + } catch (e: ClassNotFoundException) { + throw GradleException("Failed to load custom conversion class", e) }
134-142: Velocity tools may fail to load from runtime classpath.
Class.forName(velocityToolClassName)uses the default classloader, which doesn't include the project's runtime classpath. If users specify velocity tool classes from their project dependencies, loading will fail.Consider using
createClassLoader()likeloadLogicalTypesFactoriesdoes.♻️ Suggested fix
- private fun instantiateAdditionalVelocityTools(velocityToolsClassesNames: List<String>): List<Any> { - return velocityToolsClassesNames.map { velocityToolClassName -> - try { - Class.forName(velocityToolClassName).getDeclaredConstructor().newInstance() - } catch (e: Exception) { - throw RuntimeException(e) - } + private fun instantiateAdditionalVelocityTools(velocityToolsClassesNames: List<String>): List<Any> { + if (velocityToolsClassesNames.isEmpty()) return emptyList() + return createClassLoader().use { classLoader -> + velocityToolsClassesNames.map { velocityToolClassName -> + try { + classLoader.loadClass(velocityToolClassName).getDeclaredConstructor().newInstance() + } catch (e: Exception) { + throw GradleException("Failed to instantiate velocity tool class: $velocityToolClassName", e) + } + } } }
80-86: Non-null assertion onoutputDirectorycould cause unclear error.Line 85 uses
outputDirectory!!which will throw a genericNullPointerExceptionif the output directory isn't configured. Consider adding explicit validation with a helpful error message.♻️ Suggested improvement
protected fun doCompile( sourceFileForModificationDetection: File?, protocol: Protocol, outputDirectory: File?, ) { - doCompile(sourceFileForModificationDetection, SpecificCompiler(protocol), outputDirectory!!) + requireNotNull(outputDirectory) { "outputDirectory must be configured" } + doCompile(sourceFileForModificationDetection, SpecificCompiler(protocol), outputDirectory) }
| plugins { | ||
| kotlin("jvm") version "2.2.10" | ||
| `java-gradle-plugin` | ||
| id("com.gradle.plugin-publish") version "2.0.0" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n lang/java/gradle-plugin/build.gradle.ktsRepository: martin-augment/avro
Length of output: 2239
🌐 Web query:
Kotlin gradle plugin version compatibility 2.2 2.3 2024 2025 2026
💡 Result:
Kotlin Gradle Plugin (KGP) 2.2 vs 2.3 compatibility (2025–2026)
| KGP version | Release date | Supported Gradle versions (per Kotlin docs) | Android Gradle Plugin (AGP) notes |
|---|---|---|---|
| 2.2.0 | Jun 23, 2025 | 7.6.3 – 8.14 | (AGP-specific requirements vary by AGP line; see AGP 9.0 note below.) (plugins.gradle.org) |
| 2.3.0 | Dec 16, 2025 | 7.6.3 – 9.0.0 | Min AGP 8.2.2, max AGP 8.13.0 (per Kotlin 2.3.0 release notes). (kotlinlang.org) |
Key 2026 detail (AGP 9.0)
- AGP 9.0 (Jan 2026) includes built-in Kotlin support and has a runtime dependency on KGP 2.2.10; if you declare a lower KGP, Gradle/AGP will upgrade it to 2.2.10. (developer.android.com)
Early 2026 (preview/EAP)
- Kotlin 2.3.20 Beta2 (Jan 29, 2026) notes compatibility with Gradle 9.3.0 (EAP, not necessarily “stable support”). (kotlinlang.org)
- Kotlin’s release page lists 2.3.20 planned for Jan–Mar 2026. (kotlinlang.org)
Citations:
- 1: https://plugins.gradle.org/plugin/org.jetbrains.kotlin.android.extensions/2.2.0?utm_source=openai
- 2: https://kotlinlang.org/docs/whatsnew23.html?utm_source=openai
- 3: https://developer.android.com/build/releases/gradle-plugin?utm_source=openai
- 4: https://kotlinlang.org/docs/whatsnew-eap.html?utm_source=openai
- 5: https://kotlinlang.org/docs/releases.html?utm_source=openai
Align Kotlin plugin and API versions for consistency.
The build declares Kotlin JVM plugin 2.2.10 (line 20) but depends on kotlin-gradle-plugin-api:2.3.0 (line 37). While both versions are stable and have overlapping compatibility ranges, aligning them prevents potential subtle compatibility issues and improves build reproducibility.
Suggested alignment options
Update the plugin to match the API:
plugins {
- kotlin("jvm") version "2.2.10"
+ kotlin("jvm") version "2.3.0"
`java-gradle-plugin`
id("com.gradle.plugin-publish") version "2.0.0"
}Or update the API to match the plugin:
- implementation("org.jetbrains.kotlin:kotlin-gradle-plugin-api:2.3.0")
+ implementation("org.jetbrains.kotlin:kotlin-gradle-plugin-api:2.2.10")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| plugins { | |
| kotlin("jvm") version "2.2.10" | |
| `java-gradle-plugin` | |
| id("com.gradle.plugin-publish") version "2.0.0" | |
| } | |
| plugins { | |
| kotlin("jvm") version "2.3.0" | |
| `java-gradle-plugin` | |
| id("com.gradle.plugin-publish") version "2.0.0" | |
| } |
🤖 Prompt for AI Agents
In `@lang/java/gradle-plugin/build.gradle.kts` around lines 19 - 23, The Kotlin
JVM plugin version declared in the plugins block (kotlin("jvm") version
"2.2.10") is mismatched with the kotlin-gradle-plugin-api dependency version
("kotlin-gradle-plugin-api:2.3.0"); update one so both match (either bump the
plugin version to 2.3.0 or change the dependency to 2.2.10) to ensure consistent
Kotlin API/runtime; modify the version string in the plugins block or the
dependency declaration accordingly (look for kotlin("jvm") version and the
kotlin-gradle-plugin-api dependency).
| <id>run-gradle-task-publish</id> | ||
| <phase>deploy</phase> | ||
| <goals> | ||
| <goal>exec</goal> | ||
| </goals> | ||
| <configuration> | ||
| <executable>./gradlew</executable> | ||
| <arguments> | ||
| <argument>publishPlugins</argument> | ||
| <argument>-i</argument> | ||
| </arguments> | ||
| </configuration> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the full pom.xml file
find . -name "pom.xml" -path "*/lang/java/gradle-plugin/*" | head -5Repository: martin-augment/avro
Length of output: 96
🏁 Script executed:
# Search for any CI/build configuration files
find . -type f \( -name ".github" -o -name ".gitlab-ci.yml" -o -name ".travis.yml" -o -name "Jenkinsfile" -o -name "*.yml" -o -name "*.yaml" \) | grep -E "(\\.github|ci|CI)" | head -20Repository: martin-augment/avro
Length of output: 797
🏁 Script executed:
# Check for any mvn deploy invocations in the codebase
rg -nP '\bmvn\b.*\bdeploy\b' -SRepository: martin-augment/avro
Length of output: 45
🏁 Script executed:
# Read the full pom.xml to verify current state and check for existing gating
cat -n ./lang/java/gradle-plugin/pom.xmlRepository: martin-augment/avro
Length of output: 9229
🏁 Script executed:
# Check the java-publish-snapshot workflow
cat -n ./.github/workflows/java-publish-snapshot.ymlRepository: martin-augment/avro
Length of output: 2904
Guard Gradle plugin publishing behind an explicit opt‑in.
The publishPlugins execution is currently unconditional in the deploy phase, with no skip mechanism. While the CI workflow limits deployment to the main branch, local developer runs of mvn deploy or future CI changes could unexpectedly trigger publishing without explicit authorization. Consider gating this execution behind a property defaulted to skip, requiring explicit opt-in when publishing is intended.
💡 One way to make publishing opt‑in
@@
<properties>
<main.basedir>${project.parent.parent.basedir}</main.basedir>
<pluginTestingVersion>3.3.0</pluginTestingVersion>
+ <gradlePublishSkip>true</gradlePublishSkip>
</properties>
@@
<execution>
<id>run-gradle-task-publish</id>
<phase>deploy</phase>
<goals>
<goal>exec</goal>
</goals>
<configuration>
+ <skip>${gradlePublishSkip}</skip>
<executable>./gradlew</executable>
<arguments>
<argument>publishPlugins</argument>
<argument>-i</argument>
</arguments>
</configuration>
</execution>
@@
<execution>
<id>run-gradle-task-publish</id>
<phase>deploy</phase>
<goals>
<goal>exec</goal>
</goals>
<configuration>
+ <skip>${gradlePublishSkip}</skip>
<executable>gradlew.bat</executable>
<arguments>
<argument>publishPlugins</argument>
<argument>-i</argument>
</arguments>
</configuration>
</execution>🤖 Prompt for AI Agents
In `@lang/java/gradle-plugin/pom.xml` around lines 94 - 105, The publishPlugins
exec execution (id "run-gradle-task-publish", goal "exec", executable
"./gradlew", arguments "publishPlugins") must be made opt-in: move that
execution into a Maven profile activated only when a property like
"publish.plugins" is true (so it does not run by default), or add an
activation/property guard around it; require callers to pass
-Dpublish.plugins=true to enable publishing. Ensure the profile wraps the same
exec configuration and keep the id "run-gradle-task-publish" so it’s easy to
locate and verify.
|
|
||
| Add logical type factories | ||
|
|
||
| Now released on Gradle plugin portal: https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace bare URLs with Markdown links.
Markdownlint flags bare URLs; use inline links instead.
🔗 Suggested edit
-Now released on Gradle plugin portal: https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin
+Now released on Gradle plugin portal: [Gradle Plugin Portal](https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin)
-https://codeberg.org/frevib/use-gradle-plugin-test
+[use-gradle-plugin-test](https://codeberg.org/frevib/use-gradle-plugin-test)Also applies to: 68-68
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 20-20: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In `@lang/java/gradle-plugin/README.md` at line 20, Replace bare URLs in the
README lines that contain "Now released on Gradle plugin portal:
https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin" and
the other occurrence (line 68) with inline Markdown links; e.g., convert the
plain URL to a descriptive link like [Gradle Plugin
Portal](https://plugins.gradle.org/plugin/eu.eventloopsoftware.avro-gradle-plugin)
so the text uses Markdown link syntax instead of a bare URL.
|
|
||
| `0.0.7` | ||
|
|
||
| It is not needed to add `tasks.named("compileKotlin") { dependsOn(tasks.named("avroGenerateJavaClasses")) }` any more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use “anymore” (adverb) here.
Minor wording fix flagged by LanguageTool.
✏️ Suggested edit
-It is not needed to add `tasks.named("compileKotlin") { dependsOn(tasks.named("avroGenerateJavaClasses")) }` any more
+It is not needed to add `tasks.named("compileKotlin") { dependsOn(tasks.named("avroGenerateJavaClasses")) }` anymore📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| It is not needed to add `tasks.named("compileKotlin") { dependsOn(tasks.named("avroGenerateJavaClasses")) }` any more | |
| It is not needed to add `tasks.named("compileKotlin") { dependsOn(tasks.named("avroGenerateJavaClasses")) }` anymore |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~24-~24: Did you mean the adverb “anymore”?
Context: ...ks.named("avroGenerateJavaClasses")) }any more 0.0.8 AddsourceZipFiles` property...
(ANY_MORE)
🤖 Prompt for AI Agents
In `@lang/java/gradle-plugin/README.md` at line 24, Replace the two-word phrase
"any more" with the adverb "anymore" in the README sentence that mentions
tasks.named("compileKotlin") { dependsOn(tasks.named("avroGenerateJavaClasses"))
}, i.e., update the wording so it reads "It is not needed to add ... anymore" to
correct the wording flagged by LanguageTool.
| Add `sourceZipFiles` property to add zip files with schemas in them | ||
| pu | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove stray “pu” line.
This looks like an accidental fragment.
🧹 Suggested edit
-Add `sourceZipFiles` property to add zip files with schemas in them
-pu
+Add `sourceZipFiles` property to add zip files with schemas in them📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Add `sourceZipFiles` property to add zip files with schemas in them | |
| pu | |
| Add `sourceZipFiles` property to add zip files with schemas in them |
🤖 Prompt for AI Agents
In `@lang/java/gradle-plugin/README.md` around lines 28 - 30, Remove the stray
fragment "pu" that was accidentally added after the sentence "Add
`sourceZipFiles` property to add zip files with schemas in them" in the README;
edit the README.md to delete that lone "pu" line so the documentation reads
cleanly without the accidental fragment.
| addProtocolFiles( | ||
| compileSchemaTask, | ||
| project, | ||
| extension, | ||
| includesProtocol, | ||
| extension.testSourceDirectory, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Protocol files for main task incorrectly use testSourceDirectory.
Line 79 passes extension.testSourceDirectory to addProtocolFiles, but this is the registerSchemaTask (main sources), not the test task. This causes protocol files to be sourced from src/test/avro instead of src/main/avro for the main compilation task.
🐛 Proposed fix
addProtocolFiles(
compileSchemaTask,
project,
extension,
includesProtocol,
- extension.testSourceDirectory,
+ extension.sourceDirectory,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| addProtocolFiles( | |
| compileSchemaTask, | |
| project, | |
| extension, | |
| includesProtocol, | |
| extension.testSourceDirectory, | |
| ) | |
| addProtocolFiles( | |
| compileSchemaTask, | |
| project, | |
| extension, | |
| includesProtocol, | |
| extension.sourceDirectory, | |
| ) |
🤖 Prompt for AI Agents
In
`@lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/AvroGradlePlugin.kt`
around lines 74 - 80, The call that wires protocol files for the main schema
task is passing extension.testSourceDirectory to addProtocolFiles; update the
call made when creating/registering the main compile task (the
registerSchemaTask/compileSchemaTask invocation) to pass
extension.sourceDirectory (the main sources) instead of
extension.testSourceDirectory so the main compilation sources come from
src/main/avro; locate the addProtocolFiles(...) call that includes
compileSchemaTask and replace the testSourceDirectory argument with the main
sourceDirectory symbol.
| /** The output directory for the generated test Java code. */ | ||
| val testSourceDirectory: Property<String> = objects.property(String::class.java).convention("src/test/avro") | ||
|
|
||
| /** | ||
| * @parameter property="outputDirectory" default-value="${project.layout.buildDirectory}/generated-test-sources/avro" | ||
| */ | ||
| val testOutputDirectory: Property<String> = | ||
| objects.property(String::class.java).convention("generated-test-sources-avro") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation inconsistency for testSourceDirectory.
Line 47 says "The output directory for the generated test Java code" but testSourceDirectory is actually an input source directory. This appears to be a copy-paste error from testOutputDirectory.
📝 Suggested documentation fix
- /** The output directory for the generated test Java code. */
+ /** The source directory containing Avro test schema files. Defaults to {`@code` src/test/avro}. */
val testSourceDirectory: Property<String> = objects.property(String::class.java).convention("src/test/avro")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** The output directory for the generated test Java code. */ | |
| val testSourceDirectory: Property<String> = objects.property(String::class.java).convention("src/test/avro") | |
| /** | |
| * @parameter property="outputDirectory" default-value="${project.layout.buildDirectory}/generated-test-sources/avro" | |
| */ | |
| val testOutputDirectory: Property<String> = | |
| objects.property(String::class.java).convention("generated-test-sources-avro") | |
| /** The source directory containing Avro test schema files. Defaults to {`@code` src/test/avro}. */ | |
| val testSourceDirectory: Property<String> = objects.property(String::class.java).convention("src/test/avro") | |
| /** | |
| * `@parameter` property="outputDirectory" default-value="${project.layout.buildDirectory}/generated-test-sources/avro" | |
| */ | |
| val testOutputDirectory: Property<String> = | |
| objects.property(String::class.java).convention("generated-test-sources-avro") |
🤖 Prompt for AI Agents
In
`@lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/extension/AvroGradlePluginExtension.kt`
around lines 47 - 54, The KDoc for testSourceDirectory is incorrect: it
describes an output directory but testSourceDirectory is an input/source
directory; update the comment above the Property<String> testSourceDirectory to
state that it is the input/source directory for generated test Java code (e.g.,
"The input/source directory containing Avro IDL/JSON files for generating test
Java code") so it no longer matches the output wording used for
testOutputDirectory.
| private fun compileSchemas(schemaFileTree: ConfigurableFileCollection, outputDirectory: File) { | ||
| val sourceFileForModificationDetection: File? = | ||
| schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxBy { it.lastModified() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxBy on empty collection throws NoSuchElementException.
If schemaFileTree contains no files matching the filter (e.g., all files have lastModified() == 0), maxBy will throw NoSuchElementException. Use maxByOrNull for safe handling.
🐛 Proposed fix
val sourceFileForModificationDetection: File? =
- schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxBy { it.lastModified() }
+ schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxByOrNull { it.lastModified() }🤖 Prompt for AI Agents
In
`@lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/tasks/CompileAvroSchemaTask.kt`
around lines 46 - 48, In compileSchemas, the use of maxBy on the filtered file
list can throw NoSuchElementException when the collection is empty; change the
call to maxByOrNull when computing sourceFileForModificationDetection and add a
null-safe handling path (e.g., treat sourceFileForModificationDetection as
nullable and only use it if non-null or provide a fallback) so the task no
longer crashes when no files match the filter; update references to
sourceFileForModificationDetection accordingly in the CompileAvroSchemaTask.
| { | ||
| "name": "response", | ||
| "type": { | ||
| "namespace": "model", | ||
| "type": "record", | ||
| "doc": "", | ||
| "name": "MyResponse", | ||
| "fields": [ | ||
| { | ||
| "name": "isSuccessful", | ||
| "type": "boolean", | ||
| "doc": "Indicator for successful or unsuccessful call" | ||
| } | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -e avsc MyResponse lang/java/gradle-plugin/src/test/avro -x cat {}Repository: martin-augment/avro
Length of output: 294
🏁 Script executed:
find lang/java/gradle-plugin/src/test/avro -name "MyResponse.avsc" -type fRepository: martin-augment/avro
Length of output: 132
🏁 Script executed:
cat -n lang/java/gradle-plugin/src/test/avro/multipleSchemas/ApplicationEvent.avscRepository: martin-augment/avro
Length of output: 1205
Remove the inline MyResponse definition and reference the external schema instead.
The response field contains an inline definition of MyResponse (lines 28-40) that is identical to the standalone MyResponse.avsc file in the same directory. This duplication can cause issues with Avro's schema registration.
Replace the inline definition with a reference to the external schema:
{
"name": "response",
"type": "model.MyResponse"
}Then remove the inline record definition and rely on the standalone MyResponse.avsc schema.
🤖 Prompt for AI Agents
In `@lang/java/gradle-plugin/src/test/avro/multipleSchemas/ApplicationEvent.avsc`
around lines 26 - 41, The inline Avro record definition for MyResponse inside
the "response" field should be removed and replaced with a reference to the
external schema: update the "response" field (name "response") to use the named
type "model.MyResponse" instead of the inline record named "MyResponse", and
delete the inline record block so the code relies on the standalone
MyResponse.avsc schema.
| private fun assertFilesExist(directory: Path, expectedFiles: Set<String>) { | ||
| assertTrue(directory.exists(), "Directory $directory does not exist") | ||
| assertTrue(expectedFiles.isNotEmpty()) | ||
|
|
||
| val filesInDirectory: Set<String> = directory.listDirectoryEntries().map { it.fileName.toString() }.toSet() | ||
|
|
||
| assertEquals(expectedFiles, filesInDirectory) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cd lang/java/gradle-plugin && find . -name "CompileAvroSchemaTaskTest.kt" -type fRepository: martin-augment/avro
Length of output: 149
🏁 Script executed:
cat -n lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | head -300 | tail -100Repository: martin-augment/avro
Length of output: 4283
🏁 Script executed:
# Check the test directory structure and Avro schemas
find lang/java/gradle-plugin/src/test/avro -type f | head -20Repository: martin-augment/avro
Length of output: 686
🏁 Script executed:
# Look at what Avro schema files exist and check for namespace definitions
find lang/java/gradle-plugin/src/test/avro -name "*.avsc" -exec echo "=== {} ===" \; -exec cat {} \;Repository: martin-augment/avro
Length of output: 3742
🏁 Script executed:
cat -n lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | grep -A 20 "multipleSchemas" | head -30Repository: martin-augment/avro
Length of output: 45
🏁 Script executed:
# Check if there are other tests that verify subdirectory structures
rg -n "listDirectoryEntries|recursive|walk" lang/java/gradle-plugin/src/test/kotlin/Repository: martin-augment/avro
Length of output: 287
🏁 Script executed:
# Look for how the Avro Gradle plugin actually outputs generated files
find lang/java/gradle-plugin/src/main -name "*.kt" | xargs grep -l "outputDirectory\|copyTo\|Files.write" | head -5Repository: martin-augment/avro
Length of output: 501
🏁 Script executed:
cat -n lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/tasks/CompileAvroSchemaTask.ktRepository: martin-augment/avro
Length of output: 3721
🏁 Script executed:
# Also check AbstractCompileTask for common logic
cat -n lang/java/gradle-plugin/src/main/kotlin/eu/eventloopsoftware/avro/gradle/plugin/tasks/AbstractCompileTask.ktRepository: martin-augment/avro
Length of output: 7976
🏁 Script executed:
# Let's see the full test to understand what's being tested
grep -n "multipleSchemas\|model\." lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.ktRepository: martin-augment/avro
Length of output: 45
🏁 Script executed:
# Check what the actual test directory structure looks like and if there are multiple test methods
cat -n lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | head -50Repository: martin-augment/avro
Length of output: 2290
🏁 Script executed:
# Search for all test methods in the file
grep -n "@Test\|fun \`" lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | head -20Repository: martin-augment/avro
Length of output: 493
🏁 Script executed:
# Check if ApplicationEvent/multipleSchemas files are tested anywhere
grep -r "ApplicationEvent\|DocumentInfo\|MyResponse" lang/java/gradle-plugin/src/test/kotlin/ 2>/dev/nullRepository: martin-augment/avro
Length of output: 45
🏁 Script executed:
# Let's understand Avro compiler behavior - check if it creates package subdirectories
# by looking for any documentation or comments about output structure
grep -r "namespace\|package\|subdirector" lang/java/gradle-plugin/src/main --include="*.kt" | head -20Repository: martin-augment/avro
Length of output: 715
🏁 Script executed:
# Let's verify Apache Avro's standard behavior - does it generate files in package subdirectories?
# Check if there's any documentation or test setup that shows the actual generated structure
cat -n lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | sed -n '35,95p'Repository: martin-augment/avro
Length of output: 2528
🏁 Script executed:
# Check the second test to see what it expects
cat -n lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | sed -n '96,156p'Repository: martin-augment/avro
Length of output: 2572
🏁 Script executed:
# Let's check if there are other assertions that might give us clues about file structure
# Look at the third test (lines 158+)
cat -n lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt | sed -n '158,224p'Repository: martin-augment/avro
Length of output: 2926
🏁 Script executed:
# Let's search for any documentation or comments about Avro output structure
# Also check if there are any integration tests or documentation files
find lang/java/gradle-plugin -name "README*" -o -name "*.md" | xargs cat 2>/dev/null | head -50Repository: martin-augment/avro
Length of output: 973
🏁 Script executed:
# Check Apache Avro's standard behavior - look for any reference implementation
# or comments about how compiler.compileToDestination works
grep -r "compileToDestination\|SpecificCompiler" lang/java/gradle-plugin --include="*.kt" -B 2 -A 2Repository: martin-augment/avro
Length of output: 5924
assertFilesExist misses generated files in package subdirectories.
The Apache Avro compiler generates Java files in package subdirectories based on schema namespaces (e.g., test/, model/). The helper uses listDirectoryEntries() which only lists immediate children, so it cannot verify files placed in these subdirectories.
The test schemas include namespaces like "test" and "model", yet the expectedFiles set contains only flat file names. Files from schemas with the "model" namespace (ApplicationEvent, DocumentInfo, MyResponse) are never verified at all.
Consider using a recursive file walk to verify all generated Java files regardless of package structure, or update the expected files set to include package-relative paths.
🤖 Prompt for AI Agents
In
`@lang/java/gradle-plugin/src/test/kotlin/eu/eventloopsoftware/avro/gradle/plugin/CompileAvroSchemaTaskTest.kt`
around lines 278 - 285, The helper assertFilesExist currently uses
directory.listDirectoryEntries(), which only returns immediate children and
therefore misses Java files generated into package subdirectories (e.g.,
namespaces like "model" produce files under subfolders); update assertFilesExist
to perform a recursive walk (e.g., using directory.walk() or Files.walk) and
collect package-relative paths (or include subdirectory names in the file names)
before comparing to expectedFiles, or alternatively expand expectedFiles to
include package-relative paths; reference the assertFilesExist function and the
use of listDirectoryEntries() to locate and change the logic to a recursive file
collection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| extension, | ||
| includesProtocol, | ||
| extension.testSourceDirectory, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Protocol files read from wrong source directory
High Severity
The registerSchemaTask function incorrectly uses extension.testSourceDirectory when adding protocol files for the main source task. This causes the main code generation task to look for .avpr protocol files in the test directory (src/test/avro by default) instead of the main directory (src/main/avro by default), while schema files are correctly read from the main directory. Protocol files in the main source directory will be ignored.
| `0.0.8` | ||
|
|
||
| Add `sourceZipFiles` property to add zip files with schemas in them | ||
| pu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| setCompilerProperties(compiler) | ||
| try { | ||
| for (customConversion in customConversions.get()) { | ||
| compiler.addCustomConversion(Thread.currentThread().getContextClassLoader().loadClass(customConversion)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent class loader for custom conversions
Medium Severity
Custom conversions use Thread.currentThread().getContextClassLoader() to load classes, while custom logical type factories create a URLClassLoader from runtimeClassPathFileCollection. This inconsistency can cause custom conversions to fail to load even when they're available on the runtime classpath, since the thread context class loader may not have access to the same classes.
Additional Locations (1)
|
|
||
| private fun compileSchemas(schemaFileTree: ConfigurableFileCollection, outputDirectory: File) { | ||
| val sourceFileForModificationDetection: File? = | ||
| schemaFileTree.asFileTree.files.filter { file: File -> file.lastModified() > 0 }.maxBy { it.lastModified() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crash when schema files have zero modification time
High Severity
The maxBy function on line 48 throws NoSuchElementException when the filtered collection is empty. This happens when all schema files have lastModified() == 0 (which can occur with synthetic files or certain build systems) or when only protocol files exist without schema files. The variable is declared nullable, indicating null handling was intended, but maxByOrNull should be used instead of maxBy.


3614: To review by AI