jsancio commented on code in PR #21122:
URL: https://github.com/apache/kafka/pull/21122#discussion_r2765594942
##########
server-common/src/main/java/org/apache/kafka/server/common/FinalizedFeatures.java:
##########
@@ -20,24 +20,58 @@
import java.util.Map;
import java.util.Objects;
-public record FinalizedFeatures(
- MetadataVersion metadataVersion,
- Map<String, Short> finalizedFeatures,
- long finalizedFeaturesEpoch
-) {
- public static FinalizedFeatures fromKRaftVersion(MetadataVersion version) {
- return new FinalizedFeatures(version, Map.of(), -1);
- }
+public final class FinalizedFeatures {
Review Comment:
Let's add java doc for the type `FinalizedFeatures` and all of the public
methods and public static methods. Not need to add documentation for inherited
public methods like `toString`, `hashCode` and `equals`.
##########
server-common/src/main/java/org/apache/kafka/server/common/FinalizedFeatures.java:
##########
@@ -20,24 +20,58 @@
import java.util.Map;
import java.util.Objects;
-public record FinalizedFeatures(
- MetadataVersion metadataVersion,
- Map<String, Short> finalizedFeatures,
- long finalizedFeaturesEpoch
-) {
- public static FinalizedFeatures fromKRaftVersion(MetadataVersion version) {
- return new FinalizedFeatures(version, Map.of(), -1);
- }
+public final class FinalizedFeatures {
+ private static final FinalizedFeatures UNKNOWN = new
FinalizedFeatures(null, Map.of(), -1);
+
+ private final MetadataVersion metadataVersion;
+ private final Map<String, Short> finalizedFeatures;
+ private final long finalizedFeaturesEpoch;
- public FinalizedFeatures(
+ private FinalizedFeatures(
MetadataVersion metadataVersion,
Map<String, Short> finalizedFeatures,
long finalizedFeaturesEpoch
) {
- this.metadataVersion = Objects.requireNonNull(metadataVersion);
+ this.metadataVersion = metadataVersion;
this.finalizedFeatures = new HashMap<>(finalizedFeatures);
this.finalizedFeaturesEpoch = finalizedFeaturesEpoch;
- this.finalizedFeatures.put(MetadataVersion.FEATURE_NAME,
metadataVersion.featureLevel());
+ if (metadataVersion != null) {
+ this.finalizedFeatures.put(MetadataVersion.FEATURE_NAME,
metadataVersion.featureLevel());
+ }
+ }
+
+ public static FinalizedFeatures unknown() {
+ return UNKNOWN;
+ }
+
+ public static FinalizedFeatures fromKRaftVersion(MetadataVersion version) {
+ Objects.requireNonNull(version, "version cannot be null");
+ return new FinalizedFeatures(version, Map.of(), -1);
+ }
+
+ public static FinalizedFeatures of(MetadataVersion metadataVersion,
Map<String, Short> finalizedFeatures, long epoch) {
+ Objects.requireNonNull(metadataVersion, "metadataVersion cannot be
null");
+ Objects.requireNonNull(finalizedFeatures, "finalizedFeatures cannot be
null");
+ return new FinalizedFeatures(metadataVersion, finalizedFeatures,
epoch);
+ }
+
+ public boolean isMetadataKnown() {
+ return metadataVersion != null;
+ }
+
+ public MetadataVersion metadataVersionOrThrow() {
+ if (metadataVersion == null) {
+ throw new IllegalStateException("Metadata version is unknown");
+ }
+ return metadataVersion;
+ }
+
+ public Map<String, Short> finalizedFeatures() {
+ return finalizedFeatures;
+ }
+
+ public long finalizedFeaturesEpoch() {
+ return finalizedFeaturesEpoch;
}
public FinalizedFeatures setFinalizedLevel(String key, short level) {
Review Comment:
This method should fail again the unknown variant of this object, no?
##########
server-common/src/test/java/org/apache/kafka/server/common/FinalizedFeaturesTest.java:
##########
@@ -38,20 +80,38 @@ public void testKRaftModeFeatures() {
assertEquals(2, finalizedFeatures.finalizedFeatures().size());
}
+ @Test
+ public void testOfNullMetadataVersionThrows() {
+ assertThrows(NullPointerException.class,
+ () -> FinalizedFeatures.of(null, Map.of(), 0));
+ }
+
+ @Test
+ public void testOfNullFeaturesMapThrows() {
+ assertThrows(NullPointerException.class,
+ () -> FinalizedFeatures.of(MINIMUM_VERSION, null, 0));
+ }
+
@Test
public void testSetFinalizedLevel() {
Review Comment:
Let's add a test that shows that this method fails if the finalized version
is unknown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
