Bsh/GroovyScriptFactory reset script cache in case of compilation error

Issue: SPR-14007 (cherry picked from commit 0597ff1)
spring-projects · Mar 24, 2016 · 05ab769 · 05ab769
1 parent a19be75
commit 05ab769
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 38 deletions.
diff --git a/spring-context/src/main/java/org/springframework/scripting/bsh/BshScriptFactory.java b/spring-context/src/main/java/org/springframework/scripting/bsh/BshScriptFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -119,9 +119,9 @@ public boolean requiresConfigInterface() {
 	public Object getScriptedObject(ScriptSource scriptSource, Class<?>... actualInterfaces)
 			throws IOException, ScriptCompilationException {
 
-		try {
-			Class<?> clazz;
+		Class<?> clazz;
 
+		try {
 			synchronized (this.scriptClassMonitor) {
 				boolean requiresScriptEvaluation = (this.wasModifiedForTypeCheck && this.scriptClass == null);
 				this.wasModifiedForTypeCheck = false;
@@ -145,34 +145,40 @@ public Object getScriptedObject(ScriptSource scriptSource, Class<?>... actualInt
 				}
 				clazz = this.scriptClass;
 			}
+		}
+		catch (EvalError ex) {
+			this.scriptClass = null;
+			throw new ScriptCompilationException(scriptSource, ex);
+		}
 
-			if (clazz != null) {
-				// A Class: We need to create an instance for every call.
-				try {
-					return clazz.newInstance();
-				}
-				catch (Throwable ex) {
-					throw new ScriptCompilationException(
-							scriptSource, "Could not instantiate script class: " + clazz.getName(), ex);
-				}
+		if (clazz != null) {
+			// A Class: We need to create an instance for every call.
+			try {
+				return clazz.newInstance();
+			}
+			catch (Throwable ex) {
+				throw new ScriptCompilationException(
+						scriptSource, "Could not instantiate script class: " + clazz.getName(), ex);
 			}
-			else {
-				// Not a Class: We need to evaluate the script for every call.
+		}
+		else {
+			// Not a Class: We need to evaluate the script for every call.
+			try {
 				return BshScriptUtils.createBshObject(
 						scriptSource.getScriptAsString(), actualInterfaces, this.beanClassLoader);
 			}
-		}
-		catch (EvalError ex) {
-			throw new ScriptCompilationException(scriptSource, ex);
+			catch (EvalError ex) {
+				throw new ScriptCompilationException(scriptSource, ex);
+			}
 		}
 	}
 
 	@Override
 	public Class<?> getScriptedObjectType(ScriptSource scriptSource)
 			throws IOException, ScriptCompilationException {
 
-		try {
-			synchronized (this.scriptClassMonitor) {
+		synchronized (this.scriptClassMonitor) {
+			try {
 				if (scriptSource.isModified()) {
 					// New script content: Let's check whether it evaluates to a Class.
 					this.wasModifiedForTypeCheck = true;
@@ -181,9 +187,10 @@ public Class<?> getScriptedObjectType(ScriptSource scriptSource)
 				}
 				return this.scriptClass;
 			}
-		}
-		catch (EvalError ex) {
-			throw new ScriptCompilationException(scriptSource, ex);
+			catch (EvalError ex) {
+				this.scriptClass = null;
+				throw new ScriptCompilationException(scriptSource, ex);
+			}
 		}
 	}
 

diff --git a/spring-context/src/main/java/org/springframework/scripting/groovy/GroovyScriptFactory.java b/spring-context/src/main/java/org/springframework/scripting/groovy/GroovyScriptFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -158,10 +158,9 @@ public boolean requiresConfigInterface() {
 	public Object getScriptedObject(ScriptSource scriptSource, Class<?>... actualInterfaces)
 			throws IOException, ScriptCompilationException {
 
-		try {
-			Class<?> scriptClassToExecute;
-
-			synchronized (this.scriptClassMonitor) {
+		synchronized (this.scriptClassMonitor) {
+			try {
+				Class<?> scriptClassToExecute;
 				this.wasModifiedForTypeCheck = false;
 
 				if (this.cachedResult != null) {
@@ -186,22 +185,24 @@ public Object getScriptedObject(ScriptSource scriptSource, Class<?>... actualInt
 					}
 				}
 				scriptClassToExecute = this.scriptClass;
-			}
 
-			// Process re-execution outside of the synchronized block.
-			return executeScript(scriptSource, scriptClassToExecute);
-		}
-		catch (CompilationFailedException ex) {
-			throw new ScriptCompilationException(scriptSource, ex);
+				// Process re-execution outside of the synchronized block.
+				return executeScript(scriptSource, scriptClassToExecute);
+			}
+			catch (CompilationFailedException ex) {
+				this.scriptClass = null;
+				this.scriptResultClass = null;
+				throw new ScriptCompilationException(scriptSource, ex);
+			}
 		}
 	}
 
 	@Override
 	public Class<?> getScriptedObjectType(ScriptSource scriptSource)
 			throws IOException, ScriptCompilationException {
 
-		try {
-			synchronized (this.scriptClassMonitor) {
+		synchronized (this.scriptClassMonitor) {
+			try {
 				if (this.scriptClass == null || scriptSource.isModified()) {
 					// New script content...
 					this.wasModifiedForTypeCheck = true;
@@ -220,9 +221,12 @@ public Class<?> getScriptedObjectType(ScriptSource scriptSource)
 				}
 				return this.scriptResultClass;
 			}
-		}
-		catch (CompilationFailedException ex) {
-			throw new ScriptCompilationException(scriptSource, ex);
+			catch (CompilationFailedException ex) {
+				this.scriptClass = null;
+				this.scriptResultClass = null;
+				this.cachedResult = null;
+				throw new ScriptCompilationException(scriptSource, ex);
+			}
 		}
 	}