OO-3317: break the spring dependency cycle to ensue the access control module...

OO-3317: break the spring dependency cycle to ensue the access control module set the settings on the database once initialized

src/main/java/org/olat/resource/accesscontrol/AccessControlModule.java

@@ -24,6 +24,7 @@ import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.olat.core.commons.persistence.DB;
 import org.olat.core.configuration.AbstractSpringModule;
 import org.olat.core.configuration.ConfigOnOff;
 import org.olat.core.logging.OLog;
@@ -84,18 +85,31 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 	private String vatNumber;
 
 	@Autowired
-	private ACMethodDAO acMethodManager;
+	private DB dbInstance;
+	private final ACMethodDAO acMethodManager;
 	@Autowired
 	private List<AccessMethodHandler> methodHandlers;
 
 	@Autowired
-	public AccessControlModule(CoordinatorManager coordinatorManager) {
+	public AccessControlModule(CoordinatorManager coordinatorManager, ACMethodDAO acMethodManager) {
 		super(coordinatorManager);
+		this.acMethodManager = acMethodManager;
 	}
 
 	@Override
 	public void init() {
 		//module enabled/disabled
+		updateProperties();
+		updateAccessMethods();
+		log.info("Access control module is enabled: " + Boolean.toString(enabled));
+	}
+
+	@Override
+	protected void initFromChangedProperties() {
+		updateProperties();
+	}
+	
+	private void updateProperties() {
 		String enabledObj = getStringPropertyValue(AC_ENABLED, true);
 		if(StringHelper.containsNonWhitespace(enabledObj)) {
 			enabled = "true".equals(enabledObj);
@@ -144,12 +158,14 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 		if(StringHelper.containsNonWhitespace(vatNrObj)) {
 			vatNumber = vatNrObj;
 		}
-		log.info("Access control module is enabled: " + Boolean.toString(enabled));
 	}
-
-	@Override
-	protected void initFromChangedProperties() {
-		init();
+	
+	private void updateAccessMethods() {
+		acMethodManager.enableMethod(TokenAccessMethod.class, isTokenEnabled());
+		acMethodManager.enableMethod(FreeAccessMethod.class, isFreeEnabled());
+		acMethodManager.enableMethod(PaypalAccessMethod.class, isPaypalEnabled());
+		acMethodManager.enableAutoMethods(isAutoEnabled());
+		dbInstance.commitAndCloseSession();
 	}
 
 	@Override
@@ -158,9 +174,8 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 	}
 
 	public void setEnabled(boolean enabled) {
-		if(this.enabled != enabled) {
-			setStringProperty(AC_ENABLED, Boolean.toString(enabled), true);
-		}
+		this.enabled = enabled;
+		setStringProperty(AC_ENABLED, Boolean.toString(enabled), true);
 	}
 
 	public boolean isTokenEnabled() {
@@ -168,12 +183,9 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 	}
 
 	public void setTokenEnabled(boolean tokenEnabled) {
-		if(this.tokenEnabled != tokenEnabled) {
-			setStringProperty(TOKEN_ENABLED, Boolean.toString(tokenEnabled), true);
-		}
-		if(acMethodManager != null) {
-			acMethodManager.enableMethod(TokenAccessMethod.class, tokenEnabled);
-		}
+		this.tokenEnabled = tokenEnabled;
+		setStringProperty(TOKEN_ENABLED, Boolean.toString(tokenEnabled), true);
+		acMethodManager.enableMethod(TokenAccessMethod.class, tokenEnabled);
 	}
 
 	public boolean isFreeEnabled() {
@@ -181,9 +193,8 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 	}
 
 	public void setFreeEnabled(boolean freeEnabled) {
-		if(this.freeEnabled != freeEnabled) {
-			setStringProperty(FREE_ENABLED, Boolean.toString(freeEnabled), true);
-		}
+		this.freeEnabled = freeEnabled;
+		setStringProperty(FREE_ENABLED, Boolean.toString(freeEnabled), true);
 		if(acMethodManager != null) {
 			acMethodManager.enableMethod(FreeAccessMethod.class, freeEnabled);
 		}
@@ -194,12 +205,9 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 	}
 
 	public void setAutoEnabled(boolean autoEnabled) {
-		if(this.autoEnabled != autoEnabled) {
-			setStringProperty(AUTO_ENABLED, Boolean.toString(autoEnabled), true);
-		}
-		if(acMethodManager != null) {
-			acMethodManager.enableAutoMethods(autoEnabled);
-		}
+		this.autoEnabled = autoEnabled;
+		setStringProperty(AUTO_ENABLED, Boolean.toString(autoEnabled), true);
+		acMethodManager.enableAutoMethods(autoEnabled);
 	}
 
 	public boolean isPaypalEnabled() {
@@ -207,12 +215,9 @@ public class AccessControlModule extends AbstractSpringModule implements ConfigO
 	}
 
 	public void setPaypalEnabled(boolean paypalEnabled) {
-		if(this.paypalEnabled != paypalEnabled) {
-			setStringProperty(PAYPAL_ENABLED, Boolean.toString(paypalEnabled), true);
-		}
-		if(acMethodManager != null) {
-			acMethodManager.enableMethod(PaypalAccessMethod.class, paypalEnabled);
-		}
+		this.paypalEnabled = paypalEnabled;
+		setStringProperty(PAYPAL_ENABLED, Boolean.toString(paypalEnabled), true);
+		acMethodManager.enableMethod(PaypalAccessMethod.class, paypalEnabled);
 	}
 
 	public boolean isHomeOverviewEnabled() {

src/main/java/org/olat/resource/accesscontrol/manager/ACFrontendManager.java

@@ -68,6 +68,7 @@ import org.olat.resource.accesscontrol.method.AccessMethodHandler;
 import org.olat.resource.accesscontrol.model.ACResourceInfo;
 import org.olat.resource.accesscontrol.model.ACResourceInfoImpl;
 import org.olat.resource.accesscontrol.model.AccessMethod;
+import org.olat.resource.accesscontrol.model.AccessMethodSecurityCallback;
 import org.olat.resource.accesscontrol.model.AccessTransactionStatus;
 import org.olat.resource.accesscontrol.model.OLATResourceAccess;
 import org.olat.resource.accesscontrol.model.PSPTransactionStatus;
@@ -599,7 +600,18 @@ public class ACFrontendManager implements ACService {
 
 	@Override
 	public List<AccessMethod> getAvailableMethods(Identity identity, Roles roles) {
-		return methodManager.getAvailableMethods(identity, roles);
+		List<AccessMethod> methods = methodManager.getAvailableMethods();
+		
+		List<AccessMethod> allowedMethods = new ArrayList<>();
+		for(AccessMethod method:methods) {
+			AccessMethodHandler handler = accessModule.getAccessMethodHandler(method.getType());
+			AccessMethodSecurityCallback secCallback = handler.getSecurityCallback(identity, roles);
+			if(secCallback.canUse()) {
+				allowedMethods.add(method);
+			}
+		}
+		
+		return methods;
 	}
 
 	@Override

src/main/java/org/olat/resource/accesscontrol/manager/ACMethodDAO.java

@@ -35,28 +35,21 @@ import javax.persistence.TypedQuery;
 
 import org.olat.core.commons.persistence.DB;
 import org.olat.core.commons.persistence.PersistenceHelper;
-import org.olat.core.id.Identity;
-import org.olat.core.id.Roles;
 import org.olat.core.logging.OLog;
 import org.olat.core.logging.Tracing;
 import org.olat.core.util.StringHelper;
 import org.olat.resource.OLATResource;
 import org.olat.resource.OLATResourceImpl;
-import org.olat.resource.accesscontrol.AccessControlModule;
 import org.olat.resource.accesscontrol.Offer;
 import org.olat.resource.accesscontrol.OfferAccess;
 import org.olat.resource.accesscontrol.Price;
-import org.olat.resource.accesscontrol.method.AccessMethodHandler;
 import org.olat.resource.accesscontrol.model.AbstractAccessMethod;
 import org.olat.resource.accesscontrol.model.AccessMethod;
-import org.olat.resource.accesscontrol.model.AccessMethodSecurityCallback;
 import org.olat.resource.accesscontrol.model.FreeAccessMethod;
 import org.olat.resource.accesscontrol.model.OLATResourceAccess;
 import org.olat.resource.accesscontrol.model.OfferAccessImpl;
 import org.olat.resource.accesscontrol.model.TokenAccessMethod;
-import org.olat.resource.accesscontrol.provider.paypal.model.PaypalAccessMethod;
 import org.olat.shibboleth.manager.ShibbolethAutoAccessMethod;
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
@@ -72,24 +65,12 @@ import org.springframework.stereotype.Service;
  * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com
  */
 @Service
-public class ACMethodDAO implements InitializingBean {
+public class ACMethodDAO {
 
 	private static final OLog log = Tracing.createLoggerFor(ACMethodDAO.class);
 
 	@Autowired
 	private DB dbInstance;
-	@Autowired
-	private AccessControlModule acModule;
-
-	@Override
-	public void afterPropertiesSet() throws Exception {
-		enableMethod(TokenAccessMethod.class, acModule.isTokenEnabled());
-		enableMethod(FreeAccessMethod.class, acModule.isFreeEnabled());
-		enableMethod(PaypalAccessMethod.class, acModule.isPaypalEnabled());
-		enableAutoMethods(acModule.isAutoEnabled());
-		dbInstance.commitAndCloseSession();
-	}
-
 
 	public void enableAutoMethods(boolean autoEnabled) {
 		enableMethod(ShibbolethAutoAccessMethod.class, autoEnabled);
@@ -108,7 +89,7 @@ public class ACMethodDAO implements InitializingBean {
 				Date now = new Date();
 				((AbstractAccessMethod)method).setCreationDate(now);
 				((AbstractAccessMethod)method).setLastModified(now);
-				dbInstance.saveObject(method);
+				dbInstance.getCurrentEntityManager().persist(method);
 			} catch (InstantiationException e) {
 				log.error("Failed to instantiate an access method", e);
 			} catch (IllegalAccessException e) {
@@ -119,7 +100,7 @@ public class ACMethodDAO implements InitializingBean {
 				if(method.isEnabled() != enable) {
 					((AbstractAccessMethod)method).setEnabled(enable);
 					((AbstractAccessMethod)method).setLastModified(new Date());
-					dbInstance.updateObject(method);
+					dbInstance.getCurrentEntityManager().merge(method);
 				}
 			}
 		}
@@ -162,26 +143,14 @@ public class ACMethodDAO implements InitializingBean {
 				.getResultList();
 	}
 
-	public List<AccessMethod> getAvailableMethods(Identity identity, Roles roles) {
+	public List<AccessMethod> getAvailableMethods() {
 		StringBuilder sb = new StringBuilder();
 		sb.append("select method from ").append(AbstractAccessMethod.class.getName()).append(" method")
 			.append(" where method.valid=true and method.enabled=true");
 
-		TypedQuery<AccessMethod> query = dbInstance.getCurrentEntityManager().createQuery(sb.toString(), AccessMethod.class);
-		if(identity != null) {
-			//query.setLong("identityKey", identity.getKey());
-		}
-
-		List<AccessMethod> methods = query.getResultList();
-		List<AccessMethod> allowedMethods = new ArrayList<AccessMethod>();
-		for(AccessMethod method:methods) {
-			AccessMethodHandler handler = acModule.getAccessMethodHandler(method.getType());
-			AccessMethodSecurityCallback secCallback = handler.getSecurityCallback(identity, roles);
-			if(secCallback.canUse()) {
-				allowedMethods.add(method);
-			}
-		}
-		return allowedMethods;
+		return dbInstance.getCurrentEntityManager()
+				.createQuery(sb.toString(), AccessMethod.class)
+				.getResultList();
 	}
 
 	public List<AccessMethod> getAvailableMethodsByType(Class<? extends AccessMethod> type) {
@@ -346,7 +315,7 @@ public class ACMethodDAO implements InitializingBean {
 		access.setValid(false);
 
 		if(link.getKey() == null) return;
-		dbInstance.updateObject(access);
+		dbInstance.getCurrentEntityManager().merge(access);
 	}
 
 	/**
@@ -363,13 +332,13 @@ public class ACMethodDAO implements InitializingBean {
 			TokenAccessMethod method = new TokenAccessMethod();
 			method.setCreationDate(new Date());
 			method.setLastModified(method.getCreationDate());
-			dbInstance.saveObject(method);
+			dbInstance.getCurrentEntityManager().persist(method);
 		} else {
 			for(AccessMethod method:methods) {
 				if(method.isEnabled() != enable) {
 					((AbstractAccessMethod)method).setEnabled(enable);
 					((AbstractAccessMethod)method).setLastModified(new Date());
-					dbInstance.updateObject(method);
+					dbInstance.getCurrentEntityManager().merge(method);
 				}
 			}
 		}
@@ -386,13 +355,13 @@ public class ACMethodDAO implements InitializingBean {
 			FreeAccessMethod method = new FreeAccessMethod();
 			method.setCreationDate(new Date());
 			method.setLastModified(method.getCreationDate());
-			dbInstance.saveObject(method);
+			dbInstance.getCurrentEntityManager().persist(method);
 		} else {
 			for(AccessMethod method:methods) {
 				if(method.isEnabled() != enable) {
 					((AbstractAccessMethod)method).setEnabled(enable);
 					((AbstractAccessMethod)method).setLastModified(new Date());
-					dbInstance.updateObject(method);
+					dbInstance.getCurrentEntityManager().merge(method);
 				}
 			}
 		}

src/test/java/org/olat/resource/accesscontrol/ACFrontendManagerTest.java

@@ -24,7 +24,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 
 import org.junit.Assert;
@@ -34,6 +36,7 @@ import org.olat.basesecurity.GroupRoles;
 import org.olat.core.commons.persistence.DB;
 import org.olat.core.id.Identity;
 import org.olat.core.id.OLATResourceable;
+import org.olat.core.id.Roles;
 import org.olat.core.util.CodeHelper;
 import org.olat.group.BusinessGroup;
 import org.olat.group.BusinessGroupService;
@@ -361,6 +364,34 @@ public class ACFrontendManagerTest extends OlatTestCase {
 		dbInstance.commit();
 		CodeHelper.printNanoTime(start, "One click");
 	}
+	
+	@Test
+	public void testStandardMethods() {
+		Identity ident = JunitTestHelper.createAndPersistIdentityAsRndUser("ac-method-mgr");
+		
+		Roles roles = new Roles(false, false, false, true, false, false, false);
+		List<AccessMethod> methods = acService.getAvailableMethods(ident, roles);
+		assertNotNull(methods);
+		assertTrue(methods.size() >= 2);
+
+		Set<String> duplicateTypes = new HashSet<>();
+
+		boolean foundFree = false;
+		boolean foundToken = false;
+		for(AccessMethod method:methods) {
+			Assert.assertFalse(duplicateTypes.contains(method.getType()));
+			if(method instanceof FreeAccessMethod) {
+				foundFree = true;
+			} else if(method instanceof TokenAccessMethod) {
+				foundToken = true;
+			}
+			assertTrue(method.isEnabled());
+			assertTrue(method.isValid());
+			duplicateTypes.add(method.getType());
+		}
+		assertTrue(foundFree);
+		assertTrue(foundToken);
+	}
 
 	private RepositoryEntry createRepositoryEntry() {
 		//create a repository entry

src/test/java/org/olat/resource/accesscontrol/ACMethodManagerTest.java

@@ -35,9 +35,7 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.olat.core.commons.persistence.DB;
-import org.olat.core.id.Identity;
 import org.olat.core.id.OLATResourceable;
-import org.olat.core.id.Roles;
 import org.olat.resource.OLATResource;
 import org.olat.resource.OLATResourceManager;
 import org.olat.resource.accesscontrol.manager.ACMethodDAO;
@@ -46,7 +44,6 @@ import org.olat.resource.accesscontrol.model.AccessMethod;
 import org.olat.resource.accesscontrol.model.FreeAccessMethod;
 import org.olat.resource.accesscontrol.model.TokenAccessMethod;
 import org.olat.shibboleth.manager.ShibbolethAutoAccessMethod;
-import org.olat.test.JunitTestHelper;
 import org.olat.test.OlatTestCase;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -61,9 +58,6 @@ import org.springframework.beans.factory.annotation.Autowired;
  */
 public class ACMethodManagerTest extends OlatTestCase {
 
-	private static Identity ident1;
-	private static boolean isInitialized = false;
-
 	@Autowired
 	private DB dbInstance;
 
@@ -81,9 +75,6 @@ public class ACMethodManagerTest extends OlatTestCase {
 
 	@Before
 	public void setUp() {
-		if(!isInitialized) {
-			ident1 = JunitTestHelper.createAndPersistIdentityAsRndUser("ac-method-mgr");
-		}
 		acMethodManager.enableMethod(ShibbolethAutoAccessMethod.class, true);
 	}
 
@@ -118,8 +109,7 @@ public class ACMethodManagerTest extends OlatTestCase {
 
 	@Test
 	public void testStandardMethods() {
-		Roles roles = new Roles(false, false, false, true, false, false, false);
-		List<AccessMethod> methods = acMethodManager.getAvailableMethods(ident1, roles);
+		List<AccessMethod> methods = acMethodManager.getAvailableMethods();
 		assertNotNull(methods);
 		assertTrue(methods.size() >= 2);