This is an automated email from the ASF dual-hosted git repository.
chia7712 pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new de8482a4e24 KAFKA-20779 KIP-1265 review follow-ups from #21337 (#22772)
de8482a4e24 is described below
commit de8482a4e24766043a8e0894192a852131f19242
Author: ashwinpankaj <[email protected]>
AuthorDate: Fri Jul 31 20:12:38 2026 +0530
KAFKA-20779 KIP-1265 review follow-ups from #21337 (#22772)
## Summary
Follow-up nits from @chia7712's review of #21337. None affects checker
behaviour; grouped as a single small PR.
JIRA: [KAFKA-20779](https://issues.apache.org/jira/browse/KAFKA-20779)
## Changes
| # | File | Change | |---|------|--------| | 1 |
`core/…/PublicApiViolation.java` | `equals` uses
`Objects.equals(memberName, …)` instead of the ternary null-check. | | 2
| `core/…/CascadeValidator.java` | `asSuppression` omits `#member` when
the finding has no `memberName`, so class-scope `INVALID_SUPERTYPE`
suppressions no longer render as `…Owner#null`. Also uses ASCII hyphens
in the description separator. | | 3 |
`maven-plugin/…/KafkaInternalApiCheckerMojo.java` | Doc comments +
`@param` tags on the class default ctor + six setters;
`:maven-plugin:javadoc` is now warning-free. | | 4 |
`api-checker/build.gradle` | Adds `testLogging { events "passed",
"skipped", "failed" }` for consistency with the main Kafka build. | | 5
| `api-checker/README.md` | Documents `:maven-plugin:test`; layout tree
shows the test folder. |
Hooking `:api-checker:*:check` into the root `check` was dropped from
this PR per review — Kafka's CI relies on `kafka.test.xml.output.dir` +
`copyTestXml` to collect outputs from composite-included builds, so that
wiring needs a proper follow-up rather than being folded in here.
## Test plan
- [x] `./gradlew :api-checker:core:check
:api-checker:gradle-plugins:check :api-checker:maven-plugin:check` —
clean; all api-checker verification tasks pass.
- [x] `./gradlew :api-checker:maven-plugin:javadoc` — zero warnings.
Reviewers: Chia-Ping Tsai <[email protected]>
---------
Co-authored-by: Claude Opus 4.7 <[email protected]>
---
api-checker/README.md | 19 ++++++----
api-checker/build.gradle | 3 ++
.../apache/kafka/apicheck/CascadeValidator.java | 12 +++++--
.../apache/kafka/apicheck/PublicApiViolation.java | 4 ++-
.../kafka/maven/KafkaInternalApiCheckerMojo.java | 40 +++++++++++++++++++++-
5 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/api-checker/README.md b/api-checker/README.md
index 7db9851dde3..7ea37bd7985 100644
--- a/api-checker/README.md
+++ b/api-checker/README.md
@@ -25,11 +25,16 @@ below cover building, testing, and publishing the plugins
themselves.
## Test
```bash
-./gradlew :api-checker:core:test :api-checker:gradle-plugins:test
+./gradlew :api-checker:core:test :api-checker:gradle-plugins:test
:api-checker:maven-plugin:test
```
-`:gradle-plugins:test` includes a Gradle TestKit end-to-end test that applies
the
-`org.apache.kafka.internal-api-checker` plugin to a synthetic consumer project.
+- `:core:test` — unit tests for the scanner, validators, and reporter, plus
the shared
+ `testFixtures` (`AsmClassFactory`, `TempJarBuilder`) they and the plugin
tests use.
+- `:gradle-plugins:test` — includes a Gradle TestKit end-to-end test that
applies the
+ `org.apache.kafka.internal-api-checker` plugin to a synthetic consumer
project.
+- `:maven-plugin:test` — hosts `PluginXmlParityTest`, which locks the generated
+ `plugin.xml` Mojo descriptor against the fields on
`KafkaInternalApiCheckerMojo` so
+ adding a parameter without exposing it (or vice versa) fails locally.
## Publish
@@ -84,7 +89,9 @@ api-checker/
│ └── src/{main,test}/java/.../gradle/ # Plugin/Task/Extension × 2
└── maven-plugin/
├── build.gradle # Maven deps; templates plugin.xml at
processResources
- └── src/main/
- ├── java/.../maven/KafkaInternalApiCheckerMojo.java
- └── resources/META-INF/maven/plugin.xml
+ └── src/
+ ├── main/
+ │ ├── java/.../maven/KafkaInternalApiCheckerMojo.java
+ │ └── resources/META-INF/maven/plugin.xml
+ └── test/java/.../maven/PluginXmlParityTest.java # locks plugin.xml
↔ Mojo fields
```
diff --git a/api-checker/build.gradle b/api-checker/build.gradle
index f7955cb6d9b..9debba7339d 100644
--- a/api-checker/build.gradle
+++ b/api-checker/build.gradle
@@ -80,6 +80,9 @@ subprojects {
test {
useJUnitPlatform()
+ testLogging {
+ events "passed", "skipped", "failed"
+ }
}
checkstyle {
diff --git
a/api-checker/core/src/main/java/org/apache/kafka/apicheck/CascadeValidator.java
b/api-checker/core/src/main/java/org/apache/kafka/apicheck/CascadeValidator.java
index 40bfd8df7c4..3f572f2c08d 100644
---
a/api-checker/core/src/main/java/org/apache/kafka/apicheck/CascadeValidator.java
+++
b/api-checker/core/src/main/java/org/apache/kafka/apicheck/CascadeValidator.java
@@ -444,10 +444,16 @@ final class CascadeValidator {
private static PublicApiViolation asSuppression(PublicApiViolation
original, String reason) {
boolean noReason = reason.isEmpty();
String prettyReason = noReason ? PublicApiViolation.NO_REASON_MARKER :
reason;
+ // Class-level cascade findings (e.g. INVALID_SUPERTYPE) carry a null
memberName; only
+ // append "#member" when we actually have one, so class-scope
suppressions render as
+ // "Suppressed INVALID_SUPERTYPE in org.apache.kafka…Owner" rather
than "…Owner#null".
+ String location = original.getMemberName() != null
+ ? original.getClassName() + "#" + original.getMemberName()
+ : original.getClassName();
String description = "Suppressed " + original.getViolationType() + "
in "
- + original.getClassName() + "#" + original.getMemberName()
- + " — " + original.getDescription()
- + " — reason: " + prettyReason;
+ + location
+ + " - " + original.getDescription()
+ + " - reason: " + prettyReason;
return new PublicApiViolation(original.getClassName(), "SUPPRESSED",
description, original.getMemberName(), noReason);
}
diff --git
a/api-checker/core/src/main/java/org/apache/kafka/apicheck/PublicApiViolation.java
b/api-checker/core/src/main/java/org/apache/kafka/apicheck/PublicApiViolation.java
index 3a55eecb578..62d76a31d36 100644
---
a/api-checker/core/src/main/java/org/apache/kafka/apicheck/PublicApiViolation.java
+++
b/api-checker/core/src/main/java/org/apache/kafka/apicheck/PublicApiViolation.java
@@ -16,6 +16,8 @@
*/
package org.apache.kafka.apicheck;
+import java.util.Objects;
+
/**
* Represents a violation of the public API rules.
*/
@@ -86,7 +88,7 @@ public class PublicApiViolation {
if (!className.equals(that.className)) return false;
if (!violationType.equals(that.violationType)) return false;
if (!description.equals(that.description)) return false;
- return memberName != null ? memberName.equals(that.memberName) :
that.memberName == null;
+ return Objects.equals(memberName, that.memberName);
}
/**
diff --git
a/api-checker/maven-plugin/src/main/java/org/apache/kafka/maven/KafkaInternalApiCheckerMojo.java
b/api-checker/maven-plugin/src/main/java/org/apache/kafka/maven/KafkaInternalApiCheckerMojo.java
index b4759956bc6..7969b626663 100644
---
a/api-checker/maven-plugin/src/main/java/org/apache/kafka/maven/KafkaInternalApiCheckerMojo.java
+++
b/api-checker/maven-plugin/src/main/java/org/apache/kafka/maven/KafkaInternalApiCheckerMojo.java
@@ -49,6 +49,10 @@ import java.util.List;
threadSafe = true)
public class KafkaInternalApiCheckerMojo extends AbstractMojo {
+ /** Public no-arg constructor invoked by Maven when instantiating the
Mojo. */
+ public KafkaInternalApiCheckerMojo() {
+ }
+
/**
* The Maven project.
*/
@@ -199,27 +203,61 @@ public class KafkaInternalApiCheckerMojo extends
AbstractMojo {
return kafkaJars;
}
- // Getters and setters for testing
+ // Setters used by tests to construct the Mojo without going through the
Maven container.
+
+ /**
+ * Sets the Maven project the Mojo runs against (test-only injection).
+ *
+ * @param project the Maven project to scan
+ */
public void setProject(MavenProject project) {
this.project = project;
}
+ /**
+ * Enables or disables the checker for one invocation.
+ *
+ * @param enabled {@code true} to run the checker, {@code false} to skip
+ */
public void setEnabled(boolean enabled) {
this.enabled = enabled;
}
+ /**
+ * Sets whether reported violations fail the Maven build.
+ *
+ * @param failOnViolation {@code true} to fail the build on any violation
+ */
public void setFailOnViolation(boolean failOnViolation) {
this.failOnViolation = failOnViolation;
}
+ /**
+ * Sets whether the checker fails when no {@code org.apache.kafka:*}
dependency is present.
+ *
+ * @param failOnNoKafkaDependency {@code true} to fail (rather than warn)
when the project has
+ * no Kafka artifact on its classpath
+ */
public void setFailOnNoKafkaDependency(boolean failOnNoKafkaDependency) {
this.failOnNoKafkaDependency = failOnNoKafkaDependency;
}
+ /**
+ * Overrides the classes/jars scanned. Each entry may be a class
directory, an individual
+ * {@code .class} file, or a {@code .jar} archive.
+ *
+ * @param classesDirectories roots to scan; if {@code null} or empty,
defaults to the project's
+ * main compiled output
+ */
public void setClassesDirectories(List<File> classesDirectories) {
this.classesDirectories = classesDirectories;
}
+ /**
+ * Sets the file path where the text report is written.
+ *
+ * @param reportFile destination file for the human-readable report
+ */
public void setReportFile(File reportFile) {
this.reportFile = reportFile;
}
