This is an automated email from the ASF dual-hosted git repository.
He-Pin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/main by this push:
new cc37ca0e4 Replace Reflections with ClassGraph in tests (#1132)
cc37ca0e4 is described below
commit cc37ca0e455f4cde4161c36df8dc3f7482a71141
Author: He-Pin(kerr) <[email protected]>
AuthorDate: Tue Jul 7 00:28:20 2026 +0800
Replace Reflections with ClassGraph in tests (#1132)
* refactor: replace java.lang.reflect with MethodHandle/VarHandle for JIT
optimization
Motivation:
java.lang.reflect Method.invoke and Constructor.newInstance bypass JIT
inlining, adding overhead to build-time aggregation calls and test code.
Modification:
- SbtInternalAccess: replace Method.invoke with MethodHandle.invoke via
unreflect for Aggregation.showRun calls
- Http2ServerSpec: replace Field.get with VarHandle.get via
unreflectVarHandle for ManualProbe.probe field access
- TurkishISpec: replace Constructor.newInstance with MethodHandle.invoke
via unreflectConstructor for HttpCharsets$ instantiation
Result:
MethodHandle and VarHandle enable JIT inlining, eliminating reflection
overhead in both build-time and test code paths.
Tests:
- sbt "http2-tests / Test / compile" - success
- sbt "http-core / Test / compile" - success
References:
None - internal refactoring
* refactor: use method handles in tests and sbt access
Motivation:
Reduce direct reflective invocation in HTTP tests and build helper code
while keeping API consistency checks maintainable.
Modification:
Use bounded ClassGraph scans only for test API metadata, cache MethodHandle
and VarHandle access where private or dynamic access is needed, and update test
descriptors away from Class.forName.
Result:
HTTP reflective invocation sites are replaced with cached handles where
practical, with metadata scanning limited to Pekko HTTP test packages.
* fix: use invokeWithArguments() for no-arg calls and fix array class
resolution
- TurkishISpec: replace invoke() with invokeWithArguments() for Scala 3
polymorphic signature compatibility
- SerializabilitySpec: use Class.forName for array type descriptors in
resolveClass, since MethodType.fromMethodDescriptorString can fail on
some array descriptors during deserialization
* refactor: scope reflection replacement to ClassGraph scans
Motivation:
PR review pointed out that replacing ordinary reflection with
MethodHandle/VarHandle code added complexity without a demonstrated performance
win. The intended change is to avoid Reflections' deep reflection during test
classpath scans.
Modification:
Keep the ClassGraph migration for the documentation and directive
consistency scanners, close ScanResult resources after each suite, and restore
the unrelated MethodHandle/VarHandle changes to the simpler existing
implementations.
Result:
The PR diff is focused on replacing org.reflections with ClassGraph 4.8.184
in the relevant test scanners.
Tests:
- scalafmt --mode diff-ref=b0d3009b375b536397a776880301cc1943ef4645 -
success
- scalafmt --list --mode diff-ref=b0d3009b375b536397a776880301cc1943ef4645
- success
- git diff --check - success
- JDK 17 sbt "docs / Test / testOnly docs.ApiMayChangeDocCheckerSpec"
"http-tests / Test / testOnly
org.apache.pekko.http.javadsl.DirectivesConsistencySpec" - success
- qodercli -p --output-format stream-json --cwd
"/Users/hepin/IdeaProjects/pekkos/pekko-http" --attachment
/tmp/project-review.diff review - No must-fix findings
References:
Refs #1132
---
.../scala/docs/ApiMayChangeDocCheckerSpec.scala | 56 +++++---
.../http/javadsl/DirectivesConsistencySpec.scala | 151 ++++++++++++++-------
project/Dependencies.scala | 8 +-
3 files changed, 143 insertions(+), 72 deletions(-)
diff --git a/docs/src/test/scala/docs/ApiMayChangeDocCheckerSpec.scala
b/docs/src/test/scala/docs/ApiMayChangeDocCheckerSpec.scala
index d2be06ea7..19d917c3f 100644
--- a/docs/src/test/scala/docs/ApiMayChangeDocCheckerSpec.scala
+++ b/docs/src/test/scala/docs/ApiMayChangeDocCheckerSpec.scala
@@ -13,29 +13,41 @@
package docs
-import java.lang.reflect.Method
-
import org.apache.pekko.annotation.ApiMayChange
-import org.reflections.Reflections
-import org.reflections.scanners.{ MethodAnnotationsScanner, Scanners,
TypeAnnotationsScanner }
-import org.reflections.util.{ ClasspathHelper, ConfigurationBuilder }
+import io.github.classgraph.{ ClassGraph, MethodInfo }
import org.scalatest.Assertion
-import scala.collection.mutable
import scala.io.Source
import scala.jdk.CollectionConverters._
+import org.scalatest.BeforeAndAfterAll
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec
-class ApiMayChangeDocCheckerSpec extends AnyWordSpec with Matchers {
+class ApiMayChangeDocCheckerSpec extends AnyWordSpec with Matchers with
BeforeAndAfterAll {
+
+ private val apiMayChangeAnnotation = classOf[ApiMayChange].getName
+ private val httpPackagePrefix = "org.apache.pekko.http."
+
+ private lazy val scanResult =
+ new ClassGraph()
+ .acceptPackages("org.apache.pekko.http")
+ .enableClassInfo()
+ .enableMethodInfo()
+ .enableAnnotationInfo()
+ .ignoreClassVisibility()
+ .ignoreMethodVisibility()
+ .scan()
- def prettifyName(clazz: Class[?]): String = {
- clazz.getCanonicalName.replaceAll("\\$minus", "-").split("\\$")(0)
+ def prettifyName(className: String): String = {
+ className.replaceAll("\\$minus", "-").split("\\$")(0)
}
+ private def isHttpClass(className: String): Boolean =
+ className.startsWith(httpPackagePrefix)
+
// As Specs, Directives and HttpApp inherit get all directives methods, we
skip those as they are not really bringing any extra info
- def removeClassesToIgnore(method: Method): Boolean = {
- Seq("Spec", ".Directives",
".HttpApp").exists(method.getDeclaringClass.getCanonicalName.contains)
+ def removeClassesToIgnore(method: MethodInfo): Boolean = {
+ Seq("Spec", ".Directives", ".HttpApp").exists(method.getClassName.contains)
}
def collectMissing(docPage: Seq[String])(set: Set[String], name: String):
Set[String] = {
@@ -56,30 +68,34 @@ class ApiMayChangeDocCheckerSpec extends AnyWordSpec with
Matchers {
}
"compatibility-guidelines.md doc page" should {
- val reflections = new Reflections(new ConfigurationBuilder()
- .setUrls(ClasspathHelper.forPackage("org.apache.pekko.http"))
- .setScanners(
- Scanners.TypesAnnotated,
- Scanners.MethodsAnnotated))
val source =
Source.fromFile("docs/src/main/paradox/compatibility-guidelines.md")
try {
val docPage = source.getLines().toList
"contain all ApiMayChange references in classes" in {
- val classes: mutable.Set[Class[?]] =
reflections.getTypesAnnotatedWith(classOf[ApiMayChange], true).asScala
+ val classes =
scanResult.getClassesWithAnnotation(apiMayChangeAnnotation).asScala.filter(classInfo
=>
+ isHttpClass(classInfo.getName))
val missing = classes
- .map(prettifyName)
+ .map(classInfo => prettifyName(classInfo.getName))
.foldLeft(Set.empty[String])(collectMissing(docPage))
checkNoMissingCases(missing, "Types")
}
"contain all ApiMayChange references in methods" in {
- val methods =
reflections.getMethodsAnnotatedWith(classOf[ApiMayChange]).asScala
+ val methods =
+
scanResult.getClassesWithMethodAnnotation(apiMayChangeAnnotation).asScala.flatMap
{ classInfo =>
+ classInfo.getDeclaredMethodInfo.asScala.filter(method =>
+ isHttpClass(method.getClassName) &&
method.hasAnnotation(apiMayChangeAnnotation))
+ }
val missing = methods
.filterNot(removeClassesToIgnore)
- .map(method => prettifyName(method.getDeclaringClass) + "#" +
method.getName)
+ .map(method => prettifyName(method.getClassName) + "#" +
method.getName)
.foldLeft(Set.empty[String])(collectMissing(docPage))
checkNoMissingCases(missing, "Methods")
}
} finally source.close()
}
+
+ override protected def afterAll(): Unit =
+ try scanResult.close()
+ finally super.afterAll()
}
diff --git
a/http-tests/src/test/scala/org/apache/pekko/http/javadsl/DirectivesConsistencySpec.scala
b/http-tests/src/test/scala/org/apache/pekko/http/javadsl/DirectivesConsistencySpec.scala
index 5c289773d..78a514184 100644
---
a/http-tests/src/test/scala/org/apache/pekko/http/javadsl/DirectivesConsistencySpec.scala
+++
b/http-tests/src/test/scala/org/apache/pekko/http/javadsl/DirectivesConsistencySpec.scala
@@ -13,18 +13,80 @@
package org.apache.pekko.http.javadsl
-import java.lang.reflect.{ Method, Modifier }
-
+import scala.jdk.CollectionConverters._
import scala.util.control.NoStackTrace
+import io.github.classgraph.{ ClassGraph, MethodInfo => ClassGraphMethodInfo }
import org.apache.pekko
-import pekko.http.javadsl.server.directives.CorrespondsTo
+import org.scalatest.BeforeAndAfterAll
import org.scalatest.exceptions.TestPendingException
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec
-class DirectivesConsistencySpec extends AnyWordSpec with Matchers {
+object ClassGraphMembers {
+ final case class MethodInfo(name: String, isStatic: Boolean, correspondsTo:
Option[String])
+
+ private val correspondsToAnnotation =
"org.apache.pekko.http.javadsl.server.directives.CorrespondsTo"
+
+ private lazy val scanResult =
+ new ClassGraph()
+ .acceptPackages("org.apache.pekko.http")
+ .enableClassInfo()
+ .enableMethodInfo()
+ .enableAnnotationInfo()
+ .ignoreClassVisibility()
+ .ignoreMethodVisibility()
+ .scan()
+
+ private lazy val publicMethodsCache =
+ collection.concurrent.TrieMap.empty[String, Vector[MethodInfo]]
+ private lazy val declaredMethodsCache =
+ collection.concurrent.TrieMap.empty[String, Vector[MethodInfo]]
+ private lazy val interfacesCache =
+ collection.concurrent.TrieMap.empty[String, Vector[String]]
+ private lazy val superclassesCache =
+ collection.concurrent.TrieMap.empty[String, Vector[String]]
+
+ def publicMethods(clazz: Class[?]): Array[MethodInfo] =
+ publicMethodsCache
+ .getOrElseUpdate(clazz.getName,
+
classInfo(clazz.getName).getMethodInfo.asScala.filter(_.isPublic).map(toMethod).toVector)
+ .toArray
+
+ def declaredMethods(className: String): Vector[MethodInfo] =
+ declaredMethodsCache.getOrElseUpdate(
+ className,
+
classInfo(className).getDeclaredMethodInfo.asScala.map(toMethod).toVector)
+
+ def allInterfaces(clazz: Class[?]): Vector[String] =
+ interfacesCache.getOrElseUpdate(clazz.getName,
+
classInfo(clazz.getName).getInterfaces.directOnly.asScala.map(_.getName).toVector)
+
+ def superclasses(clazz: Class[?]): Vector[String] =
+ superclassesCache.getOrElseUpdate(
+ clazz.getName,
+ (classInfo(clazz.getName) +:
classInfo(clazz.getName).getSuperclasses.asScala.toVector)
+ .map(_.getName)
+ .filterNot(_ == "java.lang.Object"))
+
+ private def classInfo(className: String) =
+ Option(scanResult.getClassInfo(className))
+ .getOrElse(throw new IllegalArgumentException(s"Could not find class
metadata for [$className]"))
+
+ def close(): Unit =
+ scanResult.close()
+
+ private def toMethod(method: ClassGraphMethodInfo): MethodInfo =
+ MethodInfo(
+ method.getName,
+ method.isStatic,
+ Option(method.getAnnotationInfo(correspondsToAnnotation))
+ .flatMap(annotation =>
Option(annotation.getParameterValues.getValue("value")).map(_.toString)))
+}
+
+class DirectivesConsistencySpec extends AnyWordSpec with Matchers with
BeforeAndAfterAll {
+ import ClassGraphMembers.MethodInfo
val scalaDirectivesClazz = classOf[pekko.http.scaladsl.server.Directives]
val javaDirectivesClazz = classOf[pekko.http.javadsl.server.AllDirectives]
@@ -37,30 +99,30 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
Set("not", "DoubleNumber", "HexIntNumber", "HexLongNumber", "IntNumber",
"JavaUUID", "LongNumber",
"Neutral", "PathEnd", "Remaining", "Segment", "Segments", "Slash",
"RemainingPath") // TODO do we cover these?
- def prepareDirectivesList(in: Array[Method]): List[Method] = {
- in.toSet[Method]
+ def prepareDirectivesList(in: Array[MethodInfo]): List[MethodInfo] = {
+ in.toSet
.toList
- .foldLeft[List[Method]](Nil) {
+ .foldLeft[List[MethodInfo]](Nil) {
(l, s) =>
{
- val test = l.find { _.getName.toLowerCase == s.getName.toLowerCase
}
+ val test = l.find { _.name.toLowerCase == s.name.toLowerCase }
if (test.isEmpty) s :: l else l
}
}
- .sortBy(_.getName)
+ .sortBy(_.name)
.iterator
- .filterNot(m => Modifier.isStatic(m.getModifiers))
- .filterNot(m => ignore(m.getName))
- .filterNot(m => m.getName.contains("$"))
- .filterNot(m => m.getName.startsWith("_"))
+ .filterNot(_.isStatic)
+ .filterNot(m => ignore(m.name))
+ .filterNot(m => m.name.contains("$"))
+ .filterNot(m => m.name.startsWith("_"))
.toList
}
val scalaDirectives = {
- prepareDirectivesList(scalaDirectivesClazz.getMethods)
+
prepareDirectivesList(ClassGraphMembers.publicMethods(scalaDirectivesClazz))
}
val javaDirectives = {
- prepareDirectivesList(javaDirectivesClazz.getMethods)
+ prepareDirectivesList(ClassGraphMembers.publicMethods(javaDirectivesClazz))
}
val correspondingScalaMethods = {
@@ -68,9 +130,8 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
for {
// using Scala annotations - Java annotations were magically not
present in certain places...
d <- javaDirectives
- if d.isAnnotationPresent(classOf[CorrespondsTo])
- annot = d.getAnnotation(classOf[CorrespondsTo])
- } yield d.getName -> annot.value()
+ correspondent <- d.correspondsTo
+ } yield d.name -> correspondent
Map(javaToScalaMappings.toList: _*)
}
@@ -78,17 +139,17 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
val correspondingJavaMethods = Map() ++ correspondingScalaMethods.map(_.swap)
/** Left(@CorrespondsTo(...) or Right(normal name) */
- def correspondingScalaMethodName(m: Method): Either[String, String] =
- correspondingScalaMethods.get(m.getName) match {
+ def correspondingScalaMethodName(m: MethodInfo): Either[String, String] =
+ correspondingScalaMethods.get(m.name) match {
case Some(correspondent) => Left(correspondent)
- case _ => Right(m.getName)
+ case _ => Right(m.name)
}
/** Left(@CorrespondsTo(...) or Right(normal name) */
- def correspondingJavaMethodName(m: Method): Either[String, String] =
- correspondingJavaMethods.get(m.getName) match {
+ def correspondingJavaMethodName(m: MethodInfo): Either[String, String] =
+ correspondingJavaMethods.get(m.name) match {
case Some(correspondent) => Left(correspondent)
- case _ => Right(m.getName)
+ case _ => Right(m.name)
}
val allowMissing: Map[Class[?], Set[String]] = Map(
@@ -122,7 +183,8 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
def assertHasMethod(c: Class[?], name: String, alternativeName: String):
Unit = {
// include class name to get better error message
if (!allowMissing.getOrElse(c, Set.empty).exists(n => n == name || n ==
alternativeName)) {
- val methods = c.getMethods.collect { case m if !ignore(m.getName) =>
c.getName + "." + m.getName }
+ val methods =
+ ClassGraphMembers.publicMethods(c).collect { case m if !ignore(m.name)
=> c.getName + "." + m.name }
def originClazz = {
// look in the "opposite" class
@@ -130,32 +192,20 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
// in hava we have a huge inheritance chain so we unfold it
c match {
case `javaDirectivesClazz` =>
- val all = scalaDirectivesClazz
(for {
- i <- all.getInterfaces
- m <- i.getDeclaredMethods
- if m.getName == name || m.getName == alternativeName
+ i <- ClassGraphMembers.allInterfaces(scalaDirectivesClazz)
+ m <- ClassGraphMembers.declaredMethods(i)
+ if m.name == name || m.name == alternativeName
} yield i).headOption
- .map(_.getName)
- .getOrElse(throw new Exception(s"Unable to locate method [$name]
on source class $all"))
+ .getOrElse(throw new Exception(s"Unable to locate method [$name]
on source class $scalaDirectivesClazz"))
case `scalaDirectivesClazz` =>
- val all = javaDirectivesClazz
-
- var is = List.empty[Class[?]]
- var c: Class[?] = all
- while (c != classOf[java.lang.Object]) {
- is = c :: is
- c = c.getSuperclass
- }
-
(for {
- i <- is
- m <- i.getDeclaredMethods
- if m.getName == name || m.getName == alternativeName
+ i <- ClassGraphMembers.superclasses(javaDirectivesClazz)
+ m <- ClassGraphMembers.declaredMethods(i)
+ if m.name == name || m.name == alternativeName
} yield i).headOption
- .map(_.getName)
- .getOrElse(throw new Exception(s"Unable to locate method [$name]
on source class $all"))
+ .getOrElse(throw new Exception(s"Unable to locate method [$name]
on source class $javaDirectivesClazz"))
}
}
@@ -170,8 +220,8 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
}
"DSL Stats" should {
- info("Scala Directives: ~" +
scalaDirectives.map(_.getName).filterNot(ignore).size)
- info("Java Directives: ~" +
javaDirectives.map(_.getName).filterNot(ignore).size)
+ info("Scala Directives: ~" +
scalaDirectives.map(_.name).filterNot(ignore).size)
+ info("Java Directives: ~" +
javaDirectives.map(_.name).filterNot(ignore).size)
}
"Directive aliases" should {
@@ -182,7 +232,7 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
"Consistency scaladsl -> javadsl" should {
for {
m <- scalaDirectives
- name = m.getName
+ name = m.name
targetName = correspondingJavaMethodName(m) match {
case Left(l) => l
case Right(r) => r
@@ -196,7 +246,7 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
"Consistency javadsl -> scaladsl" should {
for {
m <- javaDirectives
- name = m.getName
+ name = m.name
targetName = correspondingScalaMethodName(m) match {
case Left(l) => l
case Right(r) => r
@@ -207,4 +257,7 @@ class DirectivesConsistencySpec extends AnyWordSpec with
Matchers {
}
}
+ override protected def afterAll(): Unit =
+ try ClassGraphMembers.close()
+ finally super.afterAll()
}
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index daf2cbbe6..2a513523d 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -29,6 +29,7 @@ object Dependencies {
val h2specArtifactExtension = if (h2specExe.endsWith("exe")) "zip" else
"tar.gz"
val h2specUrl =
s"https://github.com/summerwind/h2spec/releases/download/v$h2specVersion/$h2specName.$h2specArtifactExtension";
+ val classGraphVersion = "4.8.184"
val scalaTestVersion = "3.2.20"
val scalaCheckVersion = "1.19.0"
@@ -74,7 +75,7 @@ object Dependencies {
val sprayJson = Compile.sprayJson % "test"
val gson = "com.google.code.gson" % "gson" % "2.14.0" % "test"
val jacksonXml = "com.fasterxml.jackson.dataformat" %
"jackson-dataformat-xml" % jacksonXmlVersion % "test"
- val reflections = "org.reflections" % "reflections" % "0.10.2" % "test"
+ val classGraph = "io.github.classgraph" % "classgraph" %
classGraphVersion % "test"
}
object Test {
@@ -90,6 +91,7 @@ object Dependencies {
val scalatest = "org.scalatest" %% "scalatest" % scalaTestVersion %
"test"
val scalatestplusScalacheck = "org.scalatestplus" %% "scalacheck-1-19" %
(scalaTestVersion + ".0") % "test"
val scalatestplusJUnit = "org.scalatestplus" %% "junit-4-13" %
(scalaTestVersion + ".0") % "test"
+ val classGraph = "io.github.classgraph" % "classgraph" %
classGraphVersion % "test"
// HTTP/2
@@ -149,7 +151,7 @@ object Dependencies {
l ++= Seq(Test.munit % "provided; test")
lazy val httpTests = Seq(
- l ++= junitJupiterTestDeps.value ++ Seq(Test.scalatest))
+ l ++= junitJupiterTestDeps.value ++ Seq(Test.scalatest, Test.classGraph))
lazy val httpXml = Seq(
versionDependentDeps(scalaXml),
@@ -170,7 +172,7 @@ object Dependencies {
Docs.sprayJson,
Docs.gson,
Docs.jacksonXml,
- Docs.reflections) ++ junitJupiterTestDeps.value)
+ Docs.classGraph) ++ junitJupiterTestDeps.value)
}
object DependencyHelpers {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
