Re: [PR] KAFKA-19119: Move ApiVersionManager/SimpleApiVersionManager to server [kafka]

[via GitHub]

chia7712 commented on code in PR #19426:
URL: https://github.com/apache/kafka/pull/19426#discussion_r2042716391


##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -179,12 +179,8 @@ class KafkaApisTest extends Logging {
     overrideProperties.foreach( p => properties.put(p._1, p._2))
     val config = new KafkaConfig(properties)
 
-    val listenerType = ListenerType.BROKER
-    val enabledApis = ApiKeys.apisForListener(listenerType).asScala
-
     val apiVersionManager = new SimpleApiVersionManager(

Review Comment:
   It can be streamlined to 
   ```java
       val apiVersionManager = new SimpleApiVersionManager(
         ListenerType.BROKER,
         true,
         () => new FinalizedFeatures(MetadataVersion.latestTesting(), 
Collections.emptyMap[String, java.lang.Short], 0))
   ```
   
   and then we can remove the constructor 
   ```java
       public SimpleApiVersionManager(ApiMessageType.ListenerType listenerType,
                                      Features<SupportedVersionRange> 
brokerFeatures,
                                      boolean enableUnstableLastVersion,
                                      Supplier<FinalizedFeatures> 
featuresProvider) {
   ```



##########
core/src/test/scala/unit/kafka/server/DefaultApiVersionManagerTest.scala:
##########
@@ -114,20 +118,21 @@ class ApiVersionManagerTest {
 
   @Test
   def testEnvelopeDisabledForKRaftBroker(): Unit = {
-    val forwardingManager = Mockito.mock(classOf[ForwardingManager])
-    Mockito.when(forwardingManager.controllerApiVersions).thenReturn(None)
+    val nodeApiVersionsSupplier = 
Mockito.mock(classOf[Supplier[Optional[NodeApiVersions]]])
+    Mockito.when(nodeApiVersionsSupplier.get).thenReturn(Optional.empty())
 
     val versionManager = new DefaultApiVersionManager(
-      listenerType = ListenerType.BROKER,
-      forwardingManager = forwardingManager,
-      brokerFeatures = brokerFeatures,
-      metadataCache = metadataCache,
-      enableUnstableLastVersion = true
+      ListenerType.BROKER,
+      nodeApiVersionsSupplier,
+      brokerFeatures,
+      metadataCache,
+      true,
+      Optional.empty
     )
     assertFalse(versionManager.isApiEnabled(ApiKeys.ENVELOPE, 
ApiKeys.ENVELOPE.latestVersion))
-    assertFalse(versionManager.enabledApis.contains(ApiKeys.ENVELOPE))
+    
assertFalse(ApiKeys.apisForListener(ListenerType.BROKER).contains(ApiKeys.ENVELOPE))

Review Comment:
   ```
   
assertFalse(ApiKeys.apisForListener(versionManager.listenerType()).contains(ApiKeys.ENVELOPE))
   ```



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org