From 0cf34a46fcf77bc93046f75eabb70aac8ba45749 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Fri, 15 May 2015 13:49:36 +0200
Subject: [PATCH] OO-1552: refactor the password condition to be multi-thread,
 fix some other issues

---
 .../nodes/CourseNodePasswordManagerImpl.java  |  6 ++--
 .../org/olat/course/PersistingCourseImpl.java |  2 +-
 .../AdditionalCondition.java                  | 23 +++++++------
 .../AdditionalConditionManager.java           | 20 ++++-------
 .../PasswordCondition.java                    | 33 +++++++++----------
 .../PasswordVerificationController.java       |  9 +++--
 6 files changed, 44 insertions(+), 49 deletions(-)

diff --git a/src/main/java/de/bps/course/nodes/CourseNodePasswordManagerImpl.java b/src/main/java/de/bps/course/nodes/CourseNodePasswordManagerImpl.java
index da8c97bea22..7929678802f 100644
--- a/src/main/java/de/bps/course/nodes/CourseNodePasswordManagerImpl.java
+++ b/src/main/java/de/bps/course/nodes/CourseNodePasswordManagerImpl.java
@@ -42,7 +42,7 @@ import org.olat.properties.PropertyManager;
  */
 public class CourseNodePasswordManagerImpl implements CourseNodePasswordManager {
 
-	private Map<Long, AdditionalConditionAnswerContainer> cache = new ConcurrentHashMap<Long, AdditionalConditionAnswerContainer>();
+	private final Map<Long, AdditionalConditionAnswerContainer> cache = new ConcurrentHashMap<Long, AdditionalConditionAnswerContainer>();
 
 	private static CourseNodePasswordManagerImpl INSTANCE;
 	static {
@@ -66,7 +66,9 @@ public class CourseNodePasswordManagerImpl implements CourseNodePasswordManager
 	@Override
 	public AdditionalConditionAnswerContainer getAnswerContainer(Identity identity) {
 		AdditionalConditionAnswerContainer acac = new AdditionalConditionAnswerContainer();
-		if (cache.containsKey(identity.getKey())) {
+		if(identity == null) {
+			//do nothing
+		} else if (cache.containsKey(identity.getKey())) {
 			acac = cache.get(identity.getKey());
 		} else {
 			PropertyManager pm = PropertyManager.getInstance();
diff --git a/src/main/java/org/olat/course/PersistingCourseImpl.java b/src/main/java/org/olat/course/PersistingCourseImpl.java
index e711d18d0d5..1156d7db7fd 100644
--- a/src/main/java/org/olat/course/PersistingCourseImpl.java
+++ b/src/main/java/org/olat/course/PersistingCourseImpl.java
@@ -472,7 +472,7 @@ public class PersistingCourseImpl implements ICourse, OLATResourceable, Serializ
 			XStream xstream = CourseXStreamAliases.getReadCourseXStream();
 			return XStreamHelper.readObject(xstream, ((VFSLeaf)vfsItem).getInputStream());
 		} catch (Exception e) {
-			log.error("Cannot read course tree file: " + fileName);
+			log.error("Cannot read course tree file: " + fileName, e);
 			throw new CorruptedCourseException("Cannot resolve file: " + fileName + " course=" + toString(), e);
 		}
 	}
diff --git a/src/main/java/org/olat/course/condition/additionalconditions/AdditionalCondition.java b/src/main/java/org/olat/course/condition/additionalconditions/AdditionalCondition.java
index 5f47eb0b10f..8081ccce4be 100644
--- a/src/main/java/org/olat/course/condition/additionalconditions/AdditionalCondition.java
+++ b/src/main/java/org/olat/course/condition/additionalconditions/AdditionalCondition.java
@@ -33,7 +33,11 @@ public abstract class AdditionalCondition extends Condition implements Cloneable
 	
 	protected AbstractAccessableCourseNode node;
 	protected Long courseId;
+	
+	// The field answers must not be used, it's only there for XStream's compatibility purpose
+	@Deprecated
 	protected AdditionalConditionAnswerContainer answers;
+	
 	/**
 	 * 
 	 * @return the controller that is used to input the condition
@@ -46,8 +50,8 @@ public abstract class AdditionalCondition extends Condition implements Cloneable
 	 * @return NULL if something went wrong, true if the condition was met or false if it hadn't been fulfilled
 	 * 
 	 */
-	public abstract Boolean evaluate();
-	public abstract Boolean pwdEvaluate();
+	public abstract boolean evaluate(Object userObject);
+	
 	/**
 	 * used to get an optional controller if this additionalcondition could be fulfilled with userinteraction
 	 * @param ureq
@@ -56,24 +60,23 @@ public abstract class AdditionalCondition extends Condition implements Cloneable
 	 */
 	public abstract Controller getUserInputController(UserRequest ureq, WindowControl wControl);
 	
-	public String getNodeIdentifier(){
-		return node!=null?node.getIdent():null;
+	public String getNodeIdentifier() {
+		return node != null ? node.getIdent() : null;
 	}
 	
-	public Long getCourseId(){
+	public Long getCourseId() {
 		return courseId;
 	}
 	
-	public void setAnswers(AdditionalConditionAnswerContainer answers){
+	public void setAnswers(AdditionalConditionAnswerContainer answers) {
 		this.answers = answers;
 	}
 	
-	protected void setNode(AbstractAccessableCourseNode node){
+	protected void setNode(AbstractAccessableCourseNode node) {
 		this.node = node;
 	}
 	
-	protected void setCourseId(Long courseId){
+	protected void setCourseId(Long courseId) {
 		this.courseId = courseId;
 	}
-	
-}
+}
\ No newline at end of file
diff --git a/src/main/java/org/olat/course/condition/additionalconditions/AdditionalConditionManager.java b/src/main/java/org/olat/course/condition/additionalconditions/AdditionalConditionManager.java
index cb9b8731cc9..0a4935d45ae 100644
--- a/src/main/java/org/olat/course/condition/additionalconditions/AdditionalConditionManager.java
+++ b/src/main/java/org/olat/course/condition/additionalconditions/AdditionalConditionManager.java
@@ -40,19 +40,17 @@ public class AdditionalConditionManager {
 	}
 	
 	public Boolean evaluateConditions(){
-		Boolean retVal = true;
-		for(AdditionalCondition cond : node.getAdditionalConditions()){
-			cond.setAnswers(answers);
+		boolean retVal = true;
+		for(AdditionalCondition cond : node.getAdditionalConditions()) {
 			cond.setNode(node);
 			cond.setCourseId(courseId);
-			retVal = cond.pwdEvaluate();
+			retVal = cond.evaluate(answers);
 			//otherwise all users on this node can enter the course if one user had known the correct answer 
-			cond.setAnswers(null);
-			if(retVal==null || !retVal){
+			if(!retVal) {
 				break;
 			}
 		}
-		return (retVal!=null?retVal:false);
+		return retVal;
 	}
 	
 	
@@ -64,15 +62,11 @@ public class AdditionalConditionManager {
 	 * @return null if either nothing is wrong or the user is unable to influence the condition in olat (and won't get a more detailed error-message) 
 	 */
 	public Controller nextUserInputController(UserRequest ureq, WindowControl wControl){
-		Boolean retVal;
 		for(AdditionalCondition cond : node.getAdditionalConditions()){
-			cond.setAnswers(answers);
 			cond.setNode(node);
 			cond.setCourseId(courseId);
-			
-			retVal = cond.evaluate();
-			
-			if(retVal != null && !retVal.booleanValue()){
+			boolean retVal = cond.evaluate(answers);
+			if(!retVal) {
 				Controller ctrl = cond.getUserInputController(ureq, wControl);
 				CourseNodeConfiguration config = CourseNodeFactory.getInstance().getCourseNodeConfiguration(node.getType());
 				return TitledWrapperHelper.getWrapper(ureq, wControl, ctrl, node, config.getIconCSSClass());
diff --git a/src/main/java/org/olat/course/condition/additionalconditions/PasswordCondition.java b/src/main/java/org/olat/course/condition/additionalconditions/PasswordCondition.java
index cd2a7a279f8..0e54b5d7293 100644
--- a/src/main/java/org/olat/course/condition/additionalconditions/PasswordCondition.java
+++ b/src/main/java/org/olat/course/condition/additionalconditions/PasswordCondition.java
@@ -35,6 +35,9 @@ public class PasswordCondition extends AdditionalCondition {
 	
 	// <OLATCE-91>
 	private String password;
+	
+	// The field answer must not be used, it's only there for XStream's compatibility purpose
+	@SuppressWarnings("unused") @Deprecated
 	private String answer;
 	
 	public final static String PASSWORD_ENDING = "password";
@@ -46,26 +49,19 @@ public class PasswordCondition extends AdditionalCondition {
 
 	// <OLATCE-91>
 	@Override
-	public Boolean evaluate() {
-		boolean retVal = (password==null?true:password.equals(answer)); 
-		answer=null;
-		return retVal;
-	}
-	
-	public Boolean pwdEvaluate() {
-		PasswordStore store = null;
-		
-		if(answers != null) {
-			Object obj = answers.getAnswers(node.getIdent(), courseId.toString()); 
+	public boolean evaluate(Object userAnswerObj) {
+		String userAnswer = null;
+		if(userAnswerObj instanceof AdditionalConditionAnswerContainer) {
+			AdditionalConditionAnswerContainer answersContainer = (AdditionalConditionAnswerContainer)userAnswerObj;
+			Object obj = answersContainer.getAnswers(node.getIdent(), courseId.toString()); 
 			if(obj instanceof PasswordStore){
-				store = (PasswordStore) obj;
+				userAnswer = ((PasswordStore)obj).getPassword();
 			}
+		} else if(userAnswerObj instanceof String) {
+			userAnswer = (String)userAnswerObj;
 		}
-		
-		if(store !=null) {
-			answer = store.getPassword();
-		}
-		return evaluate();
+
+		return password==null ? true : password.equals(userAnswer); 
 	}
 
 	@Override
@@ -85,7 +81,8 @@ public class PasswordCondition extends AdditionalCondition {
 	public void setPassword(String password){
 		this.password = password;
 	}
-
+	
+	@Deprecated
 	public void setAnswer(String answer) {
 		this.answer = answer;
 	}
diff --git a/src/main/java/org/olat/course/condition/additionalconditions/PasswordVerificationController.java b/src/main/java/org/olat/course/condition/additionalconditions/PasswordVerificationController.java
index 1368cac4b1a..0ce78f44d06 100644
--- a/src/main/java/org/olat/course/condition/additionalconditions/PasswordVerificationController.java
+++ b/src/main/java/org/olat/course/condition/additionalconditions/PasswordVerificationController.java
@@ -81,13 +81,12 @@ public class PasswordVerificationController extends FormBasicController {
 		if(!StringHelper.containsNonWhitespace(pwElement.getValue())) {
 			pwElement.setErrorKey("form.legende.mandatory", new String[] {});
 		} else {
-			condition.setAnswer(pwElement.getValue());
-			valid = condition.evaluate();
-			if (!valid) {
-				pwElement.setErrorKey("password.incorrect", new String[] {});
-			} else {
+			valid = condition.evaluate(pwElement.getValue());
+			if (valid) {
 				CourseNodePasswordManager cnpm = CourseNodePasswordManagerImpl.getInstance();
 				cnpm.updatePwd(ureq.getIdentity(), condition.getNodeIdentifier(), condition.getCourseId().toString(), pwElement.getValue());
+			} else {
+				pwElement.setErrorKey("password.incorrect", new String[0]);
 			}
 		}
 		
-- 
GitLab