Skip to content

Commit 3fb77f3

Browse files
author
Rob Winch
committed
Polish SecurityJacksonModules
Issue gh-3736 * ClassLoader argument - this is required because we do not want to assume the ClassLoader that should be used * Clean up logging - logging is now at debug level because we don't expect all of the modules are loaded (they are quite possibly off the ClassPath) * Remove ObjectUtils as it was being used on methods that expect a Collection or Array with non collection based objects * Polish Javadoc warnings
1 parent c2d8ea9 commit 3fb77f3

File tree

8 files changed

+30
-24
lines changed

8 files changed

+30
-24
lines changed

cas/src/test/java/org/springframework/security/cas/jackson2/CasAuthenticationTokenMixinTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ private CasAuthenticationToken createCasAuthenticationToken() {
7171
}
7272

7373
ObjectMapper buildObjectMapper() {
74+
ClassLoader loader = getClass().getClassLoader();
7475
ObjectMapper mapper = new ObjectMapper();
75-
mapper.registerModules(SecurityJacksonModules.getModules());
76+
mapper.registerModules(SecurityJacksonModules.getModules(loader));
7677
return mapper;
7778
}
7879

core/src/main/java/org/springframework/security/jackson2/SecurityJacksonModules.java

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.apache.commons.logging.Log;
2424
import org.apache.commons.logging.LogFactory;
2525
import org.springframework.util.ClassUtils;
26-
import org.springframework.util.ObjectUtils;
2726

2827
import java.util.ArrayList;
2928
import java.util.Arrays;
@@ -63,41 +62,42 @@ private SecurityJacksonModules() {
6362
}
6463

6564
public static void enableDefaultTyping(ObjectMapper mapper) {
66-
if(!ObjectUtils.isEmpty(mapper)) {
65+
if(mapper != null) {
6766
TypeResolverBuilder<?> typeBuilder = mapper.getDeserializationConfig().getDefaultTyper(null);
68-
if (ObjectUtils.isEmpty(typeBuilder)) {
67+
if (typeBuilder == null) {
6968
mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY);
7069
}
7170
}
7271
}
7372

74-
private static Module loadAndGetInstance(String className) {
73+
@SuppressWarnings("unchecked")
74+
private static Module loadAndGetInstance(String className, ClassLoader loader) {
7575
Module instance = null;
7676
try {
77-
logger.debug("Loading module " + className);
78-
Class<? extends Module> securityModule = (Class<? extends Module>) ClassUtils.forName(className, ClassUtils.getDefaultClassLoader());
79-
if (!ObjectUtils.isEmpty(securityModule)) {
80-
logger.debug("Loaded module " + className + ", now registering");
77+
Class<? extends Module> securityModule = (Class<? extends Module>) ClassUtils.forName(className, loader);
78+
if (securityModule != null) {
79+
if(logger.isDebugEnabled()) {
80+
logger.debug("Loaded module " + className + ", now registering");
81+
}
8182
instance = securityModule.newInstance();
8283
}
83-
} catch (ClassNotFoundException e) {
84-
logger.warn("Module class not found : " + e.getMessage());
85-
} catch (InstantiationException e) {
86-
logger.error(e.getMessage());
87-
} catch (IllegalAccessException e) {
88-
logger.error(e.getMessage());
84+
} catch (Exception e) {
85+
if(logger.isDebugEnabled()) {
86+
logger.debug("Cannot load module " + className, e);
87+
}
8988
}
9089
return instance;
9190
}
9291

9392
/**
93+
* @param loader the ClassLoader to use
9494
* @return List of available security modules in classpath.
9595
*/
96-
public static List<Module> getModules() {
96+
public static List<Module> getModules(ClassLoader loader) {
9797
List<Module> modules = new ArrayList<Module>();
9898
for (String className : securityJackson2ModuleClasses) {
99-
Module module = loadAndGetInstance(className);
100-
if (!ObjectUtils.isEmpty(module)) {
99+
Module module = loadAndGetInstance(className, loader);
100+
if (module != null) {
101101
modules.add(module);
102102
}
103103
}

core/src/test/java/org/springframework/security/jackson2/AbstractMixinTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ public abstract class AbstractMixinTests {
3737
protected ObjectMapper buildObjectMapper() {
3838
if (ObjectUtils.isEmpty(mapper)) {
3939
mapper = new ObjectMapper();
40-
mapper.registerModules(SecurityJacksonModules.getModules());
40+
ClassLoader loader = getClass().getClassLoader();
41+
mapper.registerModules(SecurityJacksonModules.getModules(loader));
4142
}
4243
return mapper;
4344
}

web/src/main/java/org/springframework/security/web/jackson2/WebJackson2Module.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
* ObjectMapper mapper = new ObjectMapper();
3838
* mapper.registerModule(new WebJackson2Module());
3939
* </pre>
40-
* <b>Note: use {@link SecurityJacksonModules#getModules()} to get list of all security modules.</b>
40+
* <b>Note: use {@link SecurityJacksonModules#getModules(ClassLoader)} to get list of all security modules.</b>
4141
*
4242
* @author Jitendra Singh
4343
* @see SecurityJacksonModules

web/src/test/java/org/springframework/security/web/jackson2/AbstractMixinTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ public abstract class AbstractMixinTests {
3434
protected ObjectMapper buildObjectMapper() {
3535
if (ObjectUtils.isEmpty(mapper)) {
3636
mapper = new ObjectMapper();
37-
mapper.registerModules(SecurityJacksonModules.getModules());
37+
ClassLoader loader = getClass().getClassLoader();
38+
mapper.registerModules(SecurityJacksonModules.getModules(loader));
3839
}
3940
return mapper;
4041
}

web/src/test/java/org/springframework/security/web/jackson2/CookieMixinTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ public class CookieMixinTests {
3939

4040
ObjectMapper buildObjectMapper() {
4141
ObjectMapper mapper = new ObjectMapper();
42-
mapper.registerModules(SecurityJacksonModules.getModules());
42+
ClassLoader loader = getClass().getClassLoader();
43+
mapper.registerModules(SecurityJacksonModules.getModules(loader));
4344
return mapper;
4445
}
4546

web/src/test/java/org/springframework/security/web/jackson2/DefaultCsrfTokenMixinTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ public class DefaultCsrfTokenMixinTests {
4545
@Before
4646
public void setup() {
4747
objectMapper = new ObjectMapper();
48-
objectMapper.registerModules(SecurityJacksonModules.getModules());
48+
ClassLoader loader = getClass().getClassLoader();
49+
objectMapper.registerModules(SecurityJacksonModules.getModules(loader));
4950
defaultCsrfTokenJson = "{\"@class\": \"org.springframework.security.web.csrf.DefaultCsrfToken\", " +
5051
"\"headerName\": \"csrf-header\", \"parameterName\": \"_csrf\", \"token\": \"1\"}";
5152
}

web/src/test/java/org/springframework/security/web/jackson2/WebAuthenticationDetailsMixinTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ public class WebAuthenticationDetailsMixinTests {
4848
@Before
4949
public void setup() {
5050
this.mapper = new ObjectMapper();
51-
this.mapper.registerModules(SecurityJacksonModules.getModules());
51+
ClassLoader loader = getClass().getClassLoader();
52+
this.mapper.registerModules(SecurityJacksonModules.getModules(loader));
5253
}
5354

5455
@Test

0 commit comments

Comments
 (0)