[GitHub] [kafka] jlprat commented on a change in pull request #10839: KAFKA-12913: Make case class's final

[GitBox <gi...@apache.org>]

jlprat commented on a change in pull request #10839:
URL: https://github.com/apache/kafka/pull/10839#discussion_r649848669



##########
File path: core/src/test/scala/unit/kafka/security/authorizer/AclAuthorizerWithZkSaslTest.scala
##########
@@ -178,9 +177,3 @@ class TestableDigestLoginModule extends DigestLoginModule {
     }
   }
 }
-
-class TestableJaasSection(contextName: String, modules: Seq[JaasModule]) extends JaasSection(contextName, modules) {

Review comment:
       So you removed this class, and no change on the test was needed?

##########
File path: core/src/main/scala/kafka/tools/TestRaftServer.scala
##########
@@ -466,3 +459,10 @@ object TestRaftServer extends Logging {
   }
 
 }
+
+private[tools] sealed trait RaftEvent
+private[tools] final case class HandleClaim(epoch: Int) extends RaftEvent
+private[tools] case object HandleResign extends RaftEvent
+private[tools] final case class HandleCommit(reader: BatchReader[Array[Byte]]) extends RaftEvent
+private[tools] final case class HandleSnapshot(reader: SnapshotReader[Array[Byte]]) extends RaftEvent
+private[tools] case object Shutdown extends RaftEvent

Review comment:
       Any reason to move it down the file?

##########
File path: core/src/main/scala/kafka/server/ZkAdminManager.scala
##########
@@ -1143,3 +1141,5 @@ class ZkAdminManager(val config: KafkaConfig,
     retval
   }
 }
+
+private[server] final case class RequestStatus(user: String, mechanism: Option[ScramMechanism], legalRequest: Boolean, iterations: Int)

Review comment:
       Nice catch. Wondering why it was creating using lower case on the first place as it doesn't follow any Scala convention.

##########
File path: core/src/test/scala/integration/kafka/server/MultipleListenersWithSameSecurityProtocolBaseTest.scala
##########
@@ -180,7 +180,7 @@ abstract class MultipleListenersWithSameSecurityProtocolBaseTest extends ZooKeep
     props.put(s"${prefix}${KafkaConfig.SaslJaasConfigProp}", jaasConfig)
   }
 
-  case class ClientMetadata(val listenerName: ListenerName, val saslMechanism: String, topic: String) {

Review comment:
       Nice catch.

##########
File path: core/src/test/scala/unit/kafka/security/authorizer/AclAuthorizerWithZkSaslTest.scala
##########
@@ -69,7 +68,7 @@ class AclAuthorizerWithZkSaslTest extends ZooKeeperTestHarness with SaslSetup {
     val jaasSections = JaasTestUtils.zkSections
     val serverJaas = jaasSections.filter(_.contextName == "Server")
     val clientJaas = jaasSections.filter(_.contextName == "Client")
-      .map(section => new TestableJaasSection(section.contextName, section.modules))
+      .map(section => JaasSection(section.contextName, section.modules))

Review comment:
       Nice catch

##########
File path: core/src/main/scala/kafka/server/MetadataCache.scala
##########
@@ -462,10 +462,10 @@ class ZkMetadataCache(brokerId: Int) extends MetadataCache with Logging {
     }
   }
 
-  case class MetadataSnapshot(partitionStates: mutable.AnyRefMap[String, mutable.LongMap[UpdateMetadataPartitionState]],
-                              topicIds: Map[String, Uuid],
-                              controllerId: Option[Int],
-                              aliveBrokers: mutable.LongMap[Broker],
-                              aliveNodes: mutable.LongMap[collection.Map[ListenerName, Node]])
-
 }
+
+private[server] final case class MetadataSnapshot(partitionStates: mutable.AnyRefMap[String, mutable.LongMap[UpdateMetadataPartitionState]],

Review comment:
       I guess if there are no usages of it, it's fine to mark it as package private.

##########
File path: core/src/test/scala/unit/kafka/security/authorizer/AclAuthorizerWithZkSaslTest.scala
##########
@@ -178,9 +177,3 @@ class TestableDigestLoginModule extends DigestLoginModule {
     }
   }
 }
-
-class TestableJaasSection(contextName: String, modules: Seq[JaasModule]) extends JaasSection(contextName, modules) {

Review comment:
       Gotcha




-- 
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.

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