Remove SkinnyScalatrabase, use LoggerProvider instead of Logging

common/src/main/scala/skinny/I18n.scala

@@ -2,7 +2,7 @@ package skinny
 
 import java.text.MessageFormat
 import java.util.Locale
-import skinny.logging.Logging
+import skinny.logging.LoggerProvider
 import skinny.util.TypesafeConfigReader
 import scala.collection.concurrent.TrieMap
 
@@ -17,7 +17,7 @@ object I18n {
  *
  * @param locale locale
  */
-case class I18n(locale: Locale = null) extends Logging {
+case class I18n(locale: Locale = null) extends LoggerProvider {
 
   /**
    * Messages loaded from "src/main/resources/messages_{locale}.conf".

common/src/main/scala/skinny/logging/LoggerProvider.scala

@@ -16,4 +16,5 @@ trait LoggerProvider {
    * Get the name associated with this logger.
    */
   protected def loggerName = logger.name
-}
+
+}

common/src/main/scala/skinny/nlp/KuromojiJapaneseAnalyzer.scala

@@ -4,13 +4,13 @@ import org.apache.lucene.analysis.ja.JapaneseAnalyzer
 import org.apache.lucene.analysis.ja.tokenattributes.ReadingAttribute
 import org.apache.lucene.analysis.ja.util.ToStringUtil
 import org.apache.lucene.analysis.tokenattributes.CharTermAttribute
-import skinny.logging.Logging
+import skinny.logging.LoggerProvider
 import skinny.util.LoanPattern._
 
 import scala.collection.mutable.ListBuffer
 import scala.util.Try
 
-case class KuromojiJapaneseAnalyzer(kuromojiAnalyzer: JapaneseAnalyzer) extends SkinnyJapaneseAnalyzer with Logging {
+case class KuromojiJapaneseAnalyzer(kuromojiAnalyzer: JapaneseAnalyzer) extends SkinnyJapaneseAnalyzer with LoggerProvider {
 
   private[this] val KATAKANA_CHARS_TO_BE_AS_IS = Seq('゠', '・', 'ー', 'ヽ', 'ヾ', 'ヿ')
 

common/src/main/scala/skinny/util/TimeLogging.scala

@@ -1,7 +1,7 @@
 package skinny.util
 
-import skinny.logging.Logging
 import skinny.SkinnyEnv
+import skinny.logging.LoggerProvider
 
 object TimeLogging extends TimeLogging
 
@@ -23,7 +23,7 @@ object TimeLogging extends TimeLogging
  * }
  * }}}
  */
-trait TimeLogging extends Logging {
+trait TimeLogging extends LoggerProvider {
 
   protected def stackTraceDepthForTimeLogging: Int = 5
 

engine/src/main/scala/skinny/engine/SkinnyEngineBase.scala

@@ -1,5 +1,19 @@
 package skinny.engine
 
+import scala.language.implicitConversions
+import scala.language.reflectiveCalls
+
+import scala.annotation.tailrec
+import scala.util.control.Exception._
+import scala.util.matching.Regex
+import scala.util.{ Failure, Success, Try }
+
+import java.io.{ File, FileInputStream }
+import java.util.concurrent.atomic.AtomicInteger
+import javax.servlet.http.{ HttpServlet, HttpServletRequest, HttpServletResponse }
+import javax.servlet.{ ServletRegistration, Filter, ServletContext }
+
+import skinny.SkinnyEnv
 import skinny.engine.async.FutureSupport
 import skinny.engine.constant._
 import skinny.engine.context.SkinnyEngineContext
@@ -10,22 +24,9 @@ import skinny.engine.multipart.FileCharset
 import skinny.engine.response.{ ResponseStatus, ActionResult, Found }
 import skinny.engine.routing._
 import skinny.engine.util.UriDecoder
-
-import scala.language.implicitConversions
-import scala.language.reflectiveCalls
-
-import java.io.{ File, FileInputStream }
-import javax.servlet.http.{ HttpServlet, HttpServletRequest, HttpServletResponse }
-import javax.servlet.{ ServletRegistration, Filter, ServletContext }
-
-import SkinnyEngineBase._
+import skinny.logging.LoggerProvider
 import skinny.util.LoanPattern._
 
-import scala.annotation.tailrec
-import scala.util.control.Exception._
-import scala.util.matching.Regex
-import scala.util.{ Failure, Success, Try }
-
 object SkinnyEngineBase {
 
   import ServletApiImplicits._
@@ -100,14 +101,14 @@ trait SkinnyEngineBase
     with CoreDsl
     with DynamicScope
     with Initializable
+    with LoggerProvider
     with ServletApiImplicits
     with EngineParamsImplicits
     with DefaultImplicits
     with RicherStringImplicits
     with SessionImplicits {
 
-  @deprecated("Use servletContext instead", "1.4")
-  def applicationContext: ServletContext = servletContext
+  import SkinnyEngineBase._
 
   /**
    * The routes registered in this kernel.
@@ -161,22 +162,23 @@ trait SkinnyEngineBase
    * $ 4. Executes the after filters with `runFilters`.
    * $ 5. The action result is passed to `renderResponse`.
    */
-  protected def executeRoutes(): Unit = {
+  protected def executeRoutes() {
     var result: Any = null
     var rendered = true
 
     def runActions = {
-      val prehandleException = request.get(PrehandleExceptionKey)
+      val prehandleException = request.get(SkinnyEngineBase.PrehandleExceptionKey)
       if (prehandleException.isEmpty) {
         val (rq, rs) = (request, response)
-        onCompleted { _ =>
+        SkinnyEngineBase.onCompleted { _ =>
           withRequestResponse(rq, rs) {
+            val className = this.getClass.toString
             this match {
-              case f: Filter if !rq.contains("skinny.engine.SkinnyEngineFilter.afterFilters.Run") =>
-                rq("skinny.engine.SkinnyEngineFilter.afterFilters.Run") = new {}
+              case f: Filter if !rq.contains(s"skinny.engine.SkinnyEngineFilter.afterFilters.Run (${className})") =>
+                rq(s"skinny.engine.SkinnyEngineFilter.afterFilters.Run (${className})") = new {}
                 runFilters(routes.afterFilters)
-              case f: HttpServlet if !rq.contains("skinny.engine.SkinnyEngineServlet.afterFilters.Run") =>
-                rq("skinny.engine.SkinnyEngineServlet.afterFilters.Run") = new {}
+              case f: HttpServlet if !rq.contains("org.scalatra.ScalatraServlet.afterFilters.Run") =>
+                rq("org.scalatra.ScalatraServlet.afterFilters.Run") = new {}
                 runFilters(routes.afterFilters)
               case _ =>
             }
@@ -200,11 +202,11 @@ trait SkinnyEngineBase
         result = errorHandler(e)
         rendered = false
       }, e => {
-        runCallbacks(Failure(e))
+        SkinnyEngineBase.runCallbacks(Failure(e))
         try {
           renderUncaughtException(e)
         } finally {
-          runRenderCallbacks(Failure(e))
+          SkinnyEngineBase.runRenderCallbacks(Failure(e))
         }
       })
     })
@@ -356,7 +358,24 @@ trait SkinnyEngineBase
     case t => throw t
   }
 
+  /**
+   * Count execution of error filter registration.
+   */
+  private[this] lazy val errorMethodCallCountAtSkinnyScalatraBase: AtomicInteger = new AtomicInteger(0)
+
+  /**
+   * Detects error filter leak issue as an error.
+   */
+  protected def detectTooManyErrorFilterRegistrationnAsAnErrorAtSkinnyScalatraBase: Boolean = false
+
+  // https://github.com/scalatra/scalatra/blob/v2.3.1/core/src/main/scala/org/scalatra/ScalatraBase.scala#L333-L335
   def error(handler: ErrorHandler): Unit = {
+    val count = errorMethodCallCountAtSkinnyScalatraBase.incrementAndGet()
+    if (count > 500) {
+      val message = s"skinny's error filter registration for this controller has been evaluated $count times, this behavior will cause memory leak."
+      if (detectTooManyErrorFilterRegistrationnAsAnErrorAtSkinnyScalatraBase) throw new RuntimeException(message)
+      else logger.warn(message)
+    }
     errorHandler = handler orElse errorHandler
   }
 
@@ -655,6 +674,7 @@ trait SkinnyEngineBase
    * @return the path plus the query string, if any.  The path is run through
    *         `response.encodeURL` to add any necessary session tracking parameters.
    */
+  // TODO: Scalatra 2.3.1 still has this issue. Remove this override when it will be fixed in the future
   def url(
     path: String,
     params: Iterable[(String, Any)] = Iterable.empty,
@@ -664,25 +684,32 @@ trait SkinnyEngineBase
     withSessionId: Boolean = true)(
       implicit request: HttpServletRequest, response: HttpServletResponse): String = {
 
-    val newPath = path match {
-      case x if x.startsWith("/") && includeContextPath && includeServletPath =>
-        ensureSlash(routeBasePath) + ensureContextPathsStripped(ensureSlash(path))
-      case x if x.startsWith("/") && includeContextPath =>
-        ensureSlash(contextPath) + ensureContextPathStripped(ensureSlash(path))
-      case x if x.startsWith("/") && includeServletPath => request.getServletPath.blankOption map {
-        ensureSlash(_) + ensureServletPathStripped(ensureSlash(path))
-      } getOrElse "/"
-      case _ if absolutize => ensureContextPathsStripped(ensureSlash(path))
-      case _ => path
-    }
+    try {
+      val newPath = path match {
+        case x if x.startsWith("/") && includeContextPath && includeServletPath =>
+          ensureSlash(routeBasePath) + ensureContextPathsStripped(ensureSlash(path))
+        case x if x.startsWith("/") && includeContextPath =>
+          ensureSlash(contextPath) + ensureContextPathStripped(ensureSlash(path))
+        case x if x.startsWith("/") && includeServletPath => request.getServletPath.blankOption map {
+          ensureSlash(_) + ensureServletPathStripped(ensureSlash(path))
+        } getOrElse "/"
+        case _ if absolutize => ensureContextPathsStripped(ensureSlash(path))
+        case _ => path
+      }
 
-    val pairs = params map {
-      case (key, None) => key.urlEncode + "="
-      case (key, Some(value)) => key.urlEncode + "=" + value.toString.urlEncode
-      case (key, value) => key.urlEncode + "=" + value.toString.urlEncode
+      val pairs = params map {
+        case (key, None) => key.urlEncode + "="
+        case (key, Some(value)) => key.urlEncode + "=" + value.toString.urlEncode
+        case (key, value) => key.urlEncode + "=" + value.toString.urlEncode
+      }
+      val queryString = if (pairs.isEmpty) "" else pairs.mkString("?", "&", "")
+      if (withSessionId) addSessionId(newPath + queryString) else newPath + queryString
+    } catch {
+      case e: NullPointerException =>
+        // work around for Scalatra issue
+        if (SkinnyEnv.isTest()) "[work around] see https://github.com/scalatra/scalatra/issues/368"
+        else throw e
     }
-    val queryString = if (pairs.isEmpty) "" else pairs.mkString("?", "&", "")
-    if (withSessionId) addSessionId(newPath + queryString) else newPath + queryString
   }
 
   private[this] def ensureContextPathsStripped(path: String)(implicit request: HttpServletRequest): String = {

engine/src/main/scala/skinny/engine/SkinnyEngineListener.scala

@@ -3,10 +3,10 @@ package skinny.engine
 import javax.servlet.{ ServletContext, ServletContextEvent, ServletContextListener }
 
 import skinny.engine.implicits.RicherStringImplicits
-import skinny.logging.Logging
+import skinny.logging.LoggerProvider
 import SkinnyEngineListener._
 
-class SkinnyEngineListener extends ServletContextListener with Logging {
+class SkinnyEngineListener extends ServletContextListener with LoggerProvider {
 
   import RicherStringImplicits._
 

factory-girl/src/main/scala/skinny/test/FactoryGirl.scala

@@ -1,12 +1,12 @@
 package skinny.test
 
 import com.typesafe.config.ConfigFactory
+import skinny.logging.LoggerProvider
 import scala.collection.JavaConverters._
 import scalikejdbc._
 import skinny.orm.feature.CRUDFeatureWithId
 import skinny.exception.FactoryGirlException
 import skinny.util.{ DateTimeUtil, JavaReflectAPI }
-import skinny.logging.Logging
 
 import scala.util.Try
 
@@ -15,7 +15,7 @@ import scala.util.Try
  *
  * @see "https://github.com/thoughtbot/factory_girl"
  */
-case class FactoryGirl[Id, Entity](mapper: CRUDFeatureWithId[Id, Entity], name: Symbol = null) extends Logging {
+case class FactoryGirl[Id, Entity](mapper: CRUDFeatureWithId[Id, Entity], name: Symbol = null) extends LoggerProvider {
 
   private[this] val c = mapper.column
 

framework/src/main/scala/skinny/controller/SkinnyControllerBase.scala

@@ -4,10 +4,11 @@ import javax.servlet.http.HttpServletRequest
 
 import skinny._
 import skinny.controller.feature._
-import skinny.engine.{ SkinnyScalatraBase, ApiFormats, UrlGeneratorSupport }
+import skinny.engine.{ SkinnyEngineBase, ApiFormats, UrlGeneratorSupport }
 import skinny.engine.response.ResponseStatus
 import skinny.engine.routing.Route
 import skinny.filter.SkinnyFilterActivation
+import skinny.logging.LoggerProvider
 import skinny.validator.implicits.ParametersGetAsImplicits
 import skinny.controller.implicits.ParamsPermitImplicits
 import skinny.routing.implicits.RoutesAsImplicits
@@ -21,7 +22,7 @@ import org.json4s.JsonAST.JInt
 import org.json4s.JDecimal
 
 trait SkinnyControllerBase
-    extends SkinnyScalatraBase
+    extends SkinnyEngineBase
     with ApiFormats
     with EnvFeature
     with QueryParamsFeature
@@ -45,7 +46,7 @@ trait SkinnyControllerBase
     with ParametersGetAsImplicits
     with ParamsPermitImplicits
     with SkinnyFilterActivation
-    with Logging {
+    with LoggerProvider {
 
   /**
    * Default charset.

framework/src/main/scala/skinny/controller/feature/AngularXHRServerFeature.scala

@@ -1,7 +1,7 @@
 package skinny.controller.feature
 
-import skinny.Logging
-import skinny.engine.SkinnyScalatraBase
+import skinny.engine.SkinnyEngineBase
+import skinny.logging.LoggerProvider
 
 /**
  * Server side implementation for Angular apps.
@@ -11,6 +11,6 @@ trait AngularXHRServerFeature
     with JSONParamsAutoBinderFeature
     with AngularXSRFProtectionFeature {
 
-  self: SkinnyScalatraBase with ActionDefinitionFeature with BeforeAfterActionFeature with RequestScopeFeature with Logging =>
+  self: SkinnyEngineBase with ActionDefinitionFeature with BeforeAfterActionFeature with RequestScopeFeature with LoggerProvider =>
 
 }

framework/src/main/scala/skinny/controller/feature/AngularXSRFProtectionFeature.scala

@@ -1,7 +1,7 @@
 package skinny.controller.feature
 
-import skinny.engine.SkinnyScalatraBase
-import skinny.logging.Logging
+import skinny.engine.SkinnyEngineBase
+import skinny.logging.LoggerProvider
 
 /**
  * Angular.js Cross Site Request Forgery (XSRF) Protection support.
@@ -10,7 +10,7 @@ import skinny.logging.Logging
  */
 trait AngularXSRFProtectionFeature extends AngularXSRFCookieProviderFeature {
 
-  self: SkinnyScalatraBase with ActionDefinitionFeature with BeforeAfterActionFeature with RequestScopeFeature with Logging =>
+  self: SkinnyEngineBase with ActionDefinitionFeature with BeforeAfterActionFeature with RequestScopeFeature with LoggerProvider =>
 
   /**
    * Enabled if true.

framework/src/main/scala/skinny/controller/feature/CORSFeature.scala

@@ -1,14 +1,14 @@
 package skinny.controller.feature
 
-import skinny.engine.SkinnyScalatraBase
+import skinny.engine.SkinnyEngineBase
 
 /**
  * CORS(Cross-Origin Resource Sharing) support.
  *
  * http://www.w3.org/TR/cors/
  * http://enable-cors.org/
  */
-trait CORSFeature { self: SkinnyScalatraBase with BeforeAfterActionFeature =>
+trait CORSFeature { self: SkinnyEngineBase with BeforeAfterActionFeature =>
 
   beforeAction() {
     response.setHeader("Access-Control-Allow-Origin", "*")

framework/src/main/scala/skinny/controller/feature/CSRFProtectionFeature.scala

@@ -1,8 +1,8 @@
 package skinny.controller.feature
 
-import skinny.engine.SkinnyScalatraBase
+import skinny.engine.SkinnyEngineBase
 import skinny.engine.csrf.CsrfTokenSupport
-import skinny.logging.Logging
+import skinny.logging.LoggerProvider
 
 object CSRFProtectionFeature {
 
@@ -16,7 +16,7 @@ object CSRFProtectionFeature {
  */
 trait CSRFProtectionFeature extends CsrfTokenSupport {
 
-  self: SkinnyScalatraBase with ActionDefinitionFeature with BeforeAfterActionFeature with RequestScopeFeature with Logging =>
+  self: SkinnyEngineBase with ActionDefinitionFeature with BeforeAfterActionFeature with RequestScopeFeature with LoggerProvider =>
 
   /**
    * Overrides Scalatra's default key name.