You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2019/08/01 13:08:22 UTC

[isis] branch v2 updated: ISIS-2156 fixes broken context cleanup when running multiple shiro tests

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch v2
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/v2 by this push:
     new 8763ff7  ISIS-2156 fixes broken context cleanup when running multiple shiro tests
8763ff7 is described below

commit 8763ff78d50ff357ce27469603e325a5b8d4bc4c
Author: Andi Huber <ah...@apache.org>
AuthorDate: Thu Aug 1 15:08:13 2019 +0200

    ISIS-2156 fixes broken context cleanup when running multiple shiro tests
---
 .../apache/isis/testdomain/rest/RestServiceTest.java | 14 +++++++-------
 .../isis/testdomain/shiro/AbstractShiroTest.java     | 20 ++++++++++++++++++--
 .../apache/isis/testdomain/shiro/ShiroLdapTest.java  |  9 +++++----
 .../isis/testdomain/shiro/ShiroSecmanTest.java       |  8 ++++----
 4 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/examples/smoketest/src/test/java/org/apache/isis/testdomain/rest/RestServiceTest.java b/examples/smoketest/src/test/java/org/apache/isis/testdomain/rest/RestServiceTest.java
index 0ade52b..96023ee 100644
--- a/examples/smoketest/src/test/java/org/apache/isis/testdomain/rest/RestServiceTest.java
+++ b/examples/smoketest/src/test/java/org/apache/isis/testdomain/rest/RestServiceTest.java
@@ -47,17 +47,17 @@ import lombok.val;
 })
 class RestServiceTest {
 
-	@LocalServerPort int port;
-	@Inject RestService restServerService;
+	@LocalServerPort int port; // just for reference (not used)
+	@Inject RestService restService;
 
 	@Test
-	void bookOfTheWeek_viaRestService() {
+	void bookOfTheWeek_viaRestEndpoint() {
 		
-		assertNotNull(restServerService.getPort());
-		assertTrue(restServerService.getPort()>0);
+		assertNotNull(restService.getPort());
+		assertTrue(restService.getPort()>0);
 
-		val restfulClient = restServerService.newClient();
-		val request = restServerService.newRecommendedBookOfTheWeekRequest(restfulClient);
+		val restfulClient = restService.newClient();
+		val request = restService.newRecommendedBookOfTheWeekRequest(restfulClient);
 
 		val args = restfulClient.arguments()
 				.build();
diff --git a/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/AbstractShiroTest.java b/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/AbstractShiroTest.java
index 8e5d6dd..b256c00 100644
--- a/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/AbstractShiroTest.java
+++ b/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/AbstractShiroTest.java
@@ -24,6 +24,7 @@ import org.apache.shiro.mgt.SecurityManager;
 import org.apache.shiro.subject.Subject;
 import org.apache.shiro.subject.support.SubjectThreadState;
 import org.apache.shiro.util.LifecycleUtils;
+import org.apache.shiro.util.ThreadContext;
 import org.apache.shiro.util.ThreadState;
 
 class AbstractShiroTest {
@@ -60,6 +61,13 @@ class AbstractShiroTest {
     }
 
     private static void doClearSubject() {
+    	
+    	Subject subject = ThreadContext.getSubject();
+    	if(subject!=null) {
+    		ThreadContext.unbindSubject();
+    	}
+    	
+    	
         if (subjectThreadState != null) {
             subjectThreadState.clear();
             subjectThreadState = null;
@@ -67,7 +75,15 @@ class AbstractShiroTest {
     }
 
     protected static void setSecurityManager(SecurityManager securityManager) {
-        SecurityUtils.setSecurityManager(securityManager);
+    	try {
+    		// guard against SecurityManager already being set
+    		getSecurityManager();
+    		throw new IllegalStateException("It seems a previous test, did not cleanup the its SecurityManager.");
+    	} catch (UnavailableSecurityManagerException e) {
+    		
+    		// happy case
+    		SecurityUtils.setSecurityManager(securityManager);
+		}
     }
 
     protected static SecurityManager getSecurityManager() {
@@ -85,7 +101,7 @@ class AbstractShiroTest {
             // need a SecurityManager instance because it was using only
             // mock Subject instances)
         }
-        setSecurityManager(null);
+        SecurityUtils.setSecurityManager(null);
     }
 	
 }
diff --git a/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroLdapTest.java b/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroLdapTest.java
index 6cbcf4a..9622069 100644
--- a/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroLdapTest.java
+++ b/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroLdapTest.java
@@ -58,9 +58,9 @@ class ShiroLdapTest extends AbstractShiroTest {
 
 	@BeforeAll
 	static void beforeClass() {
-		//0.  Build and set the SecurityManager used to build Subject instances used in your tests
-		//    This typically only needs to be done once per class if your shiro.ini doesn't change,
-		//    otherwise, you'll need to do this logic in each test that is different
+		// Build and set the SecurityManager used to build Subject instances used in your tests
+		// This typically only needs to be done once per class if your shiro.ini doesn't change,
+		// otherwise, you'll need to do this logic in each test that is different
 		val factory = new IniSecurityManagerFactory("classpath:shiro-ldap.ini");
 		setSecurityManager(factory.getInstance());
 	}
@@ -69,7 +69,7 @@ class ShiroLdapTest extends AbstractShiroTest {
 	static void afterClass() {
 		tearDownShiro();
 	}
-
+	
 	@Test
 	void loginLogoutRoundtrip() {
 		
@@ -80,6 +80,7 @@ class ShiroLdapTest extends AbstractShiroTest {
 
 		val subject = SecurityUtils.getSubject(); 
 		assertNotNull(subject);
+		
 		assertFalse(subject.isAuthenticated());
 
 		val token = (AuthenticationToken) new UsernamePasswordToken(
diff --git a/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanTest.java b/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanTest.java
index 0f0d64c..caebbd3 100644
--- a/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanTest.java
+++ b/examples/smoketest/src/test/java/org/apache/isis/testdomain/shiro/ShiroSecmanTest.java
@@ -60,7 +60,7 @@ class ShiroSecmanTest extends AbstractShiroTest {
 
 	@BeforeAll
 	static void beforeClass() {
-		//0.  Build and set the SecurityManager used to build Subject instances used in your tests
+		//    Build and set the SecurityManager used to build Subject instances used in your tests
 		//    This typically only needs to be done once per class if your shiro.ini doesn't change,
 		//    otherwise, you'll need to do this logic in each test that is different
 		val factory = new IniSecurityManagerFactory("classpath:shiro-secman.ini");
@@ -71,13 +71,13 @@ class ShiroSecmanTest extends AbstractShiroTest {
 	static void afterClass() {
 		tearDownShiro();
 	}
-
+	
 	@Test
 	void loginLogoutRoundtrip() {
 
-		val secMan = SecurityUtils.getSecurityManager();
+		val secMan = getSecurityManager();
 		assertNotNull(secMan);
-
+		
 		val subject = SecurityUtils.getSubject(); 
 		assertNotNull(subject);
 		assertFalse(subject.isAuthenticated());