Merge pull request #40 from AVSystem/compilation-npe-caching

 Disable unexpected compilation exceptions caching

scex-core/src/main/scala/com/avsystem/scex/compiler/CachingScexCompiler.scala

@@ -1,14 +1,14 @@
 package com.avsystem.scex
 package compiler
 
-import java.util.concurrent.{ExecutionException, TimeUnit}
-
+import com.avsystem.scex.compiler.ScexCompiler.CompilationFailedException
 import com.avsystem.scex.parsing.PositionMapping
 import com.avsystem.scex.validation.{SymbolValidator, SyntaxValidator}
 import com.google.common.cache.CacheBuilder
 import com.google.common.util.concurrent.ExecutionError
 
-import scala.util.Try
+import java.util.concurrent.{ExecutionException, TimeUnit}
+import scala.util.{Failure, Success, Try}
 
 trait CachingScexCompiler extends ScexCompiler {
 
@@ -42,21 +42,40 @@ trait CachingScexCompiler extends ScexCompiler {
   private val symbolValidatorsCache =
     CacheBuilder.newBuilder.build[String, SymbolValidator]
 
+  // used to avoid unexpected exceptions caching, such as a random NPE thrown during a machine I/O error
+  private def invalidateCacheEntry(result: Try[_], invalidate: () => Unit): Unit =
+    if (!settings.cacheUnexpectedCompilationExceptions.value)
+      result match {
+        case Failure(_: CompilationFailedException) | Success(_) =>
+        case Failure(_) => invalidate()
+      }
+
   override protected def preprocess(expression: String, template: Boolean) =
     unwrapExecutionException(
       preprocessingCache.get((expression, template), callable(super.preprocess(expression, template))))
 
-  override protected def compileExpression(exprDef: ExpressionDef) =
-    unwrapExecutionException(
-      expressionCache.get(exprDef, callable(super.compileExpression(exprDef))))
+  override protected def compileExpression(exprDef: ExpressionDef) = {
+    val result = unwrapExecutionException(expressionCache.get(exprDef, callable(super.compileExpression(exprDef))))
+    invalidateCacheEntry(result, () => expressionCache.invalidate(exprDef))
 
-  override protected def compileProfileObject(profile: ExpressionProfile) =
-    unwrapExecutionException(underLock(
+    result
+  }
+
+  override protected def compileProfileObject(profile: ExpressionProfile) = {
+    val result = unwrapExecutionException(underLock(
       profileCompilationResultsCache.get(profile, callable(super.compileProfileObject(profile)))))
+    invalidateCacheEntry(result, () => profileCompilationResultsCache.invalidate(profile))
 
-  override protected def compileExpressionUtils(source: NamedSource) =
-    unwrapExecutionException(underLock(
+    result
+  }
+
+  override protected def compileExpressionUtils(source: NamedSource) = {
+    val result = unwrapExecutionException(underLock(
       utilsCompilationResultsCache.get(source.name, callable(super.compileExpressionUtils(source)))))
+    invalidateCacheEntry(result, () => utilsCompilationResultsCache.invalidate(source.name))
+
+    result
+  }
 
   override protected def compileJavaGetterAdapters(profile: ExpressionProfile, name: String, classes: Seq[Class[_]], full: Boolean) =
     unwrapExecutionException(underLock(

scex-core/src/main/scala/com/avsystem/scex/compiler/ScexSettings.scala

@@ -54,6 +54,10 @@ class ScexSettings extends Settings {
   final val backwardsCompatCacheVersion = StringSetting("-SCEXbackwards-compat-cache-version", "versionString",
     "Additional version string for controlling invalidation of classfile cache", "0")
 
+  final val cacheUnexpectedCompilationExceptions = BooleanSetting("-SCEXcache-unexpected-compilation-exceptions",
+    "Enables the caching of unexpected exceptions (such as NPE when accessing scex_classes) thrown during the expression compilation. " +
+      "CompilationFailedExceptions are always cached, regardless of this setting. They indicate e.g. syntax errors, which should always be cached to avoid redundant compilations.", default = false)
+
   def resolvedClassfileDir: Option[PlainDirectory] = Option(classfileDirectory.value)
     .filter(_.trim.nonEmpty).map(path => new PlainDirectory(new Directory(new File(path))))
 }

scex-core/src/test/scala/com/avsystem/scex/compiler/ScexCompilationCachingTest.scala

@@ -0,0 +1,109 @@
+package com.avsystem.scex.compiler
+
+import com.avsystem.scex.ExpressionProfile
+import com.avsystem.scex.compiler.ScexCompiler.CompileError
+import com.avsystem.scex.japi.{DefaultJavaScexCompiler, JavaScexCompiler, ScalaTypeTokens}
+import com.avsystem.scex.util.{PredefinedAccessSpecs, SimpleContext}
+import com.google.common.util.concurrent.UncheckedExecutionException
+import org.scalatest.funsuite.AnyFunSuite
+
+final class ScexCompilationCachingTest extends AnyFunSuite with CompilationTest {
+
+  private var compilationCount = 0
+
+  private val settings = new ScexSettings
+  settings.classfileDirectory.value = "testClassfileCache"
+  settings.noGetterAdapters.value = true // to reduce number of compilations in tests
+
+  private val acl = PredefinedAccessSpecs.basicOperations
+  private val defaultProfile = createProfile(acl, utils = "val utilValue = 42")
+
+  private def createFailingCompiler: JavaScexCompiler =
+    new DefaultJavaScexCompiler(settings) {
+      override protected def compile(sourceFile: ScexSourceFile): Either[ScexClassLoader, List[CompileError]] = {
+        compilationCount += 1
+        if (compilationCount == 1) throw new NullPointerException()
+        else super.compile(sourceFile)
+      }
+    }
+
+  private def createCountingCompiler: JavaScexCompiler =
+    new DefaultJavaScexCompiler(settings) {
+      override protected def compile(sourceFile: ScexSourceFile): Either[ScexClassLoader, List[CompileError]] = {
+        compilationCount += 1
+        super.compile(sourceFile)
+      }
+    }
+
+  override def newProfileName(): String = "constant_name"
+
+  private def compileExpression(
+    compiler: JavaScexCompiler,
+    expression: String = s""""value"""",
+    profile: ExpressionProfile = defaultProfile,
+  ): Unit = {
+    compiler.buildExpression
+      .contextType(ScalaTypeTokens.create[SimpleContext[Unit]])
+      .resultType(classOf[String])
+      .expression(expression)
+      .template(false)
+      .profile(profile)
+      .get
+  }
+
+  test("Unexpected exceptions shouldn't be cached by default") {
+    compilationCount = 0
+    val compiler = createFailingCompiler
+
+    assertThrows[UncheckedExecutionException](compileExpression(compiler))
+    assert(compilationCount == 1) // utils compilation ended with NPE
+    compileExpression(compiler)
+    assert(compilationCount == 3) // 2x utils compilation + 1x final expression compilation
+  }
+
+  test("Unexpected exceptions should be cached when enabled using ScexSetting") {
+    compilationCount = 0
+    val compiler = createFailingCompiler
+    compiler.settings.cacheUnexpectedCompilationExceptions.value = true
+
+    assertThrows[UncheckedExecutionException](compileExpression(compiler))
+    assert(compilationCount == 1)
+    assertThrows[UncheckedExecutionException](compileExpression(compiler))
+    assert(compilationCount == 1) // result fetched from cache
+  }
+
+  test("CompilationFailedExceptions should always be cached") {
+    compilationCount = 0
+    val compiler = createCountingCompiler
+    val profile = createProfile(acl, utils = """invalidValue""")
+
+    compiler.settings.cacheUnexpectedCompilationExceptions.value = true
+    assertThrows[UncheckedExecutionException](compileExpression(compiler, profile = profile))
+    assert(compilationCount == 1)
+    assertThrows[UncheckedExecutionException](compileExpression(compiler, profile = profile))
+    assert(compilationCount == 1)
+
+    compiler.settings.cacheUnexpectedCompilationExceptions.value = false
+    assertThrows[UncheckedExecutionException](compileExpression(compiler, profile = profile))
+    assert(compilationCount == 1)
+    assertThrows[UncheckedExecutionException](compileExpression(compiler, profile = profile))
+    assert(compilationCount == 1)
+  }
+
+  test("Successful compilation should always be cached") {
+    compilationCount = 0
+    val compiler = createCountingCompiler
+
+    compiler.settings.cacheUnexpectedCompilationExceptions.value = true
+    compileExpression(compiler)
+    assert(compilationCount == 2) // utils + expression value
+    compileExpression(compiler)
+    assert(compilationCount == 2)
+
+    compiler.settings.cacheUnexpectedCompilationExceptions.value = false
+    compileExpression(compiler)
+    assert(compilationCount == 2)
+    compileExpression(compiler)
+    assert(compilationCount == 2)
+  }
+}