Skip to content

Commit

Permalink
Fix #8474
Browse files Browse the repository at this point in the history
This commit does the following :
* List the available constructors while creating a module, when no available constructor is found
* Add a new unit test
* Add a new utility function in ConstructorUtils
  • Loading branch information
fagossa committed Jul 24, 2019
1 parent 72cc787 commit 5ba3a27
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
22 changes: 22 additions & 0 deletions core/play-guice/src/test/scala/play/api/inject/ModulesSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.specs2.matcher.BeEqualTypedValueCheck
import org.specs2.mutable.Specification
import play.api.Configuration
import play.api.Environment
import play.api.PlayException
import play.{ Environment => JavaEnvironment }

class ModulesSpec extends Specification {
Expand Down Expand Up @@ -63,6 +64,22 @@ class ModulesSpec extends Specification {
}
}

"list current constructor parameters when any match" in {
val env = Environment.simple()
val conf = Configuration(
"play.modules.enabled" -> Seq(
classOf[InvalidConfigModule].getName
)
)

Modules.locate(env, conf) must throwA[PlayException].like {
case e =>
val firstConstructorParameters = "(com.typesafe.config.Config, java.lang.String, long)"
val secondConstructorParameters = "(com.typesafe.config.Config, java.lang.String)"
e.getMessage must contain(s"$firstConstructorParameters, $secondConstructorParameters")
}
}

}

}
Expand All @@ -78,3 +95,8 @@ class ScalaGuiceModule(val environment: Environment, val configuration: Configur
class JavaGuiceConfigModule(val environment: JavaEnvironment, val config: Config) extends AbstractModule {
override def configure(): Unit = ()
}

class InvalidConfigModule(val config: Config, val ignored: String) extends AbstractModule {
def this(config: Config, ignored: String, useless: Long) = this(config, ignored)
override def configure(): Unit = ()
}
18 changes: 18 additions & 0 deletions core/play/src/main/java/play/libs/reflect/ConstructorUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.lang.reflect.Constructor;
import java.lang.reflect.Modifier;
import java.util.Arrays;

/** Imported from apache.commons.lang3 3.6 */
public class ConstructorUtils {
Expand Down Expand Up @@ -115,6 +116,23 @@ public static <T> Constructor<T> getMatchingAccessibleConstructor(
return result;
}

/**
* Finds the class names of all accessible constructors.
*
* @param <T> the constructor type
* @return the constructors parameters, empty array if no matching accessible constructor found
*/
public static <T> String[][] getConstructorParameters(final Class<T> type) {
Constructor[] allConstructors = type.getDeclaredConstructors();
return Arrays.stream(allConstructors)
.map(
cons ->
Arrays.stream(cons.getParameterTypes())
.map(Class::getCanonicalName)
.toArray(String[]::new))
.toArray(String[][]::new);
}

/**
* Learn whether the specified class is generally accessible, i.e. is declared in an entirely
* {@code public} manner.
Expand Down
5 changes: 4 additions & 1 deletion core/play/src/main/scala/play/api/inject/Module.scala
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ object Modules {
tryConstruct()
}
.getOrElse {
throw new PlayException("No valid constructors", "Module [" + className + "] cannot be instantiated.")
val parameters: Array[Array[String]] = ConstructorUtils.getConstructorParameters(moduleClass)
throw new PlayException(s"""No valid public constructors for ${moduleClass.getCanonicalName}. Expected one of:
${parameters.map(_.mkString("(", ", ", ")")).mkString(", ")}
""".stripMargin, "Module [" + className + "] cannot be instantiated.")
}
} catch {
case e: PlayException => throw e
Expand Down

0 comments on commit 5ba3a27

Please sign in to comment.