This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-logging.git
The following commit(s) were added to refs/heads/master by this push:
new 1642a1b No need to nest in else.
1642a1b is described below
commit 1642a1bd98850ec64c3af3c73e3dcde45571403d
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Fri Mar 5 14:43:24 2021 -0500
No need to nest in else.
---
.../org/apache/commons/logging/LogFactory.java | 32 +++----
.../commons/logging/impl/LogFactoryImpl.java | 50 +++++-----
.../org/apache/commons/logging/impl/SimpleLog.java | 7 +-
.../commons/logging/PathableClassLoader.java | 105 +++++++++------------
.../logging/impl/WeakHashtableTestCase.java | 7 +-
.../logging/security/MockSecurityManager.java | 10 +-
6 files changed, 94 insertions(+), 117 deletions(-)
diff --git a/src/main/java/org/apache/commons/logging/LogFactory.java
b/src/main/java/org/apache/commons/logging/LogFactory.java
index b1963e8..fc6c514 100644
--- a/src/main/java/org/apache/commons/logging/LogFactory.java
+++ b/src/main/java/org/apache/commons/logging/LogFactory.java
@@ -878,9 +878,8 @@ public abstract class LogFactory {
//
// nb: nullClassLoaderFactory might be null. That's ok.
return nullClassLoaderFactory;
- } else {
- return (LogFactory) factories.get(contextClassLoader);
}
+ return (LogFactory) factories.get(contextClassLoader);
}
/**
@@ -1239,9 +1238,8 @@ public abstract class LogFactory {
public Object run() {
if (loader != null) {
return loader.getResourceAsStream(name);
- } else {
- return ClassLoader.getSystemResourceAsStream(name);
}
+ return ClassLoader.getSystemResourceAsStream(name);
}
});
}
@@ -1267,9 +1265,8 @@ public abstract class LogFactory {
try {
if (loader != null) {
return loader.getResources(name);
- } else {
- return ClassLoader.getSystemResources(name);
}
+ return ClassLoader.getSystemResources(name);
} catch (final IOException e) {
if (isDiagnosticsEnabled()) {
logDiagnostic("Exception while trying to find
configuration file " +
@@ -1475,17 +1472,17 @@ public abstract class LogFactory {
if (dest.equals("STDOUT")) {
return System.out;
- } else if (dest.equals("STDERR")) {
+ }
+ if (dest.equals("STDERR")) {
return System.err;
- } else {
- try {
- // open the file in append mode
- final FileOutputStream fos = new FileOutputStream(dest, true);
- return new PrintStream(fos);
- } catch (final IOException ex) {
- // We should report this to the user - but how?
- return null;
- }
+ }
+ try {
+ // open the file in append mode
+ final FileOutputStream fos = new FileOutputStream(dest, true);
+ return new PrintStream(fos);
+ } catch (final IOException ex) {
+ // We should report this to the user - but how?
+ return null;
}
}
@@ -1651,9 +1648,8 @@ public abstract class LogFactory {
public static String objectId(final Object o) {
if (o == null) {
return "null";
- } else {
- return o.getClass().getName() + "@" + System.identityHashCode(o);
}
+ return o.getClass().getName() + "@" + System.identityHashCode(o);
}
// ----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
b/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
index 00d0f7b..54ce37b 100644
--- a/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
+++ b/src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java
@@ -682,12 +682,11 @@ public class LogFactoryImpl extends LogFactory {
logDiagnostic("Did not find '" + name + "'.");
}
return false;
- } else {
- if (isDiagnosticsEnabled()) {
- logDiagnostic("Found '" + name + "'.");
- }
- return true;
}
+ if (isDiagnosticsEnabled()) {
+ logDiagnostic("Found '" + name + "'.");
+ }
+ return true;
} catch (final LogConfigurationException e) {
if (isDiagnosticsEnabled()) {
logDiagnostic("Logging system '" + name + "' is available but
not useable.");
@@ -1166,22 +1165,20 @@ public class LogFactoryImpl extends LogFactory {
// In some classloading setups (e.g. JBoss with its
// UnifiedLoaderRepository) this can still work, so if user hasn't
// forbidden it, just return the contextClassLoader.
- if (allowFlawedContext) {
- if (isDiagnosticsEnabled()) {
- logDiagnostic("[WARNING] the context classloader is not
part of a" +
- " parent-child relationship with the
classloader that" +
- " loaded LogFactoryImpl.");
- }
- // If contextClassLoader were null, getLowestClassLoader() would
- // have returned thisClassLoader. The fact we are here means
- // contextClassLoader is not null, so we can just return it.
- return contextClassLoader;
- }
- else {
+ if (!allowFlawedContext) {
throw new LogConfigurationException("Bad classloader hierarchy;
LogFactoryImpl was loaded via" +
" a classloader that is not
related to the current context" +
" classloader.");
}
+ if (isDiagnosticsEnabled()) {
+ logDiagnostic("[WARNING] the context classloader is not part of
a" +
+ " parent-child relationship with the classloader
that" +
+ " loaded LogFactoryImpl.");
+ }
+ // If contextClassLoader were null, getLowestClassLoader() would
+ // have returned thisClassLoader. The fact we are here means
+ // contextClassLoader is not null, so we can just return it.
+ return contextClassLoader;
}
if (baseClassLoader != contextClassLoader) {
@@ -1190,21 +1187,20 @@ public class LogFactoryImpl extends LogFactory {
// that there are a number of broken systems out there which create
// custom classloaders but fail to set the context classloader so
// we handle those flawed systems anyway.
- if (allowFlawedContext) {
- if (isDiagnosticsEnabled()) {
- logDiagnostic(
- "Warning: the context classloader is an ancestor
of the" +
- " classloader that loaded LogFactoryImpl; it
should be" +
- " the same or a descendant. The application using"
+
- " commons-logging should ensure the context
classloader" +
- " is used correctly.");
- }
- } else {
+ if (!allowFlawedContext) {
throw new LogConfigurationException(
"Bad classloader hierarchy; LogFactoryImpl was loaded
via" +
" a classloader that is not related to the current
context" +
" classloader.");
}
+ if (isDiagnosticsEnabled()) {
+ logDiagnostic(
+ "Warning: the context classloader is an ancestor of
the" +
+ " classloader that loaded LogFactoryImpl; it should
be" +
+ " the same or a descendant. The application using" +
+ " commons-logging should ensure the context
classloader" +
+ " is used correctly.");
+ }
}
return baseClassLoader;
diff --git a/src/main/java/org/apache/commons/logging/impl/SimpleLog.java
b/src/main/java/org/apache/commons/logging/impl/SimpleLog.java
index ae24c49..561c4fa 100644
--- a/src/main/java/org/apache/commons/logging/impl/SimpleLog.java
+++ b/src/main/java/org/apache/commons/logging/impl/SimpleLog.java
@@ -633,9 +633,7 @@ public class SimpleLog implements Log, Serializable {
* obtain a class loader) will trigger this exception where
* we can make a distinction.
*/
- if (e.getTargetException() instanceof SecurityException) {
- // ignore
- } else {
+ if (!(e.getTargetException() instanceof SecurityException)) {
// Capture 'e.getTargetException()' exception for details
// alternate: log 'e.getTargetException()', and pass back
'e'.
throw new LogConfigurationException
@@ -664,9 +662,8 @@ public class SimpleLog implements Log, Serializable {
if (threadCL != null) {
return threadCL.getResourceAsStream(name);
- } else {
- return ClassLoader.getSystemResourceAsStream(name);
}
+ return ClassLoader.getSystemResourceAsStream(name);
}
});
}
diff --git a/src/test/java/org/apache/commons/logging/PathableClassLoader.java
b/src/test/java/org/apache/commons/logging/PathableClassLoader.java
index dc9a03a..0efd2e1 100644
--- a/src/test/java/org/apache/commons/logging/PathableClassLoader.java
+++ b/src/test/java/org/apache/commons/logging/PathableClassLoader.java
@@ -312,26 +312,18 @@ public class PathableClassLoader extends URLClassLoader {
if (parentFirst) {
return super.loadClass(name, resolve);
- } else {
- // Implement child-first.
- //
- // It appears that the findClass method doesn't check whether the
- // class has already been loaded. This seems odd to me, but without
- // first checking via findLoadedClass we can get
java.lang.LinkageError
- // with message "duplicate class definition" which isn't good.
-
- try {
- Class clazz = findLoadedClass(name);
- if (clazz == null) {
- clazz = super.findClass(name);
- }
- if (resolve) {
- resolveClass(clazz);
- }
- return clazz;
- } catch(final ClassNotFoundException e) {
- return super.loadClass(name, resolve);
+ }
+ try {
+ Class clazz = findLoadedClass(name);
+ if (clazz == null) {
+ clazz = super.findClass(name);
+ }
+ if (resolve) {
+ resolveClass(clazz);
}
+ return clazz;
+ } catch(final ClassNotFoundException e) {
+ return super.loadClass(name, resolve);
}
}
@@ -344,13 +336,12 @@ public class PathableClassLoader extends URLClassLoader {
public URL getResource(final String name) {
if (parentFirst) {
return super.getResource(name);
- } else {
- final URL local = super.findResource(name);
- if (local != null) {
- return local;
- }
- return super.getResource(name);
}
+ final URL local = super.findResource(name);
+ if (local != null) {
+ return local;
+ }
+ return super.getResource(name);
}
/**
@@ -364,30 +355,29 @@ public class PathableClassLoader extends URLClassLoader {
public Enumeration getResourcesInOrder(final String name) throws
IOException {
if (parentFirst) {
return super.getResources(name);
- } else {
- final Enumeration localUrls = super.findResources(name);
-
- final ClassLoader parent = getParent();
- if (parent == null) {
- // Alas, there is no method to get matching resources
- // from a null (BOOT) parent classloader. Calling
- // ClassLoader.getSystemClassLoader isn't right. Maybe
- // calling Class.class.getResources(name) would do?
- //
- // However for the purposes of unit tests, we can
- // simply assume that no relevant resources are
- // loadable from the parent; unit tests will never be
- // putting any of their resources in a "boot" classloader
- // path!
- return localUrls;
- }
- final Enumeration parentUrls = parent.getResources(name);
+ }
+ final Enumeration localUrls = super.findResources(name);
- final ArrayList localItems = toList(localUrls);
- final ArrayList parentItems = toList(parentUrls);
- localItems.addAll(parentItems);
- return Collections.enumeration(localItems);
+ final ClassLoader parent = getParent();
+ if (parent == null) {
+ // Alas, there is no method to get matching resources
+ // from a null (BOOT) parent classloader. Calling
+ // ClassLoader.getSystemClassLoader isn't right. Maybe
+ // calling Class.class.getResources(name) would do?
+ //
+ // However for the purposes of unit tests, we can
+ // simply assume that no relevant resources are
+ // loadable from the parent; unit tests will never be
+ // putting any of their resources in a "boot" classloader
+ // path!
+ return localUrls;
}
+ final Enumeration parentUrls = parent.getResources(name);
+
+ final ArrayList localItems = toList(localUrls);
+ final ArrayList parentItems = toList(parentUrls);
+ localItems.addAll(parentItems);
+ return Collections.enumeration(localItems);
}
/**
@@ -418,18 +408,17 @@ public class PathableClassLoader extends URLClassLoader {
public InputStream getResourceAsStream(final String name) {
if (parentFirst) {
return super.getResourceAsStream(name);
- } else {
- final URL local = super.findResource(name);
- if (local != null) {
- try {
- return local.openStream();
- } catch(final IOException e) {
- // TODO: check if this is right or whether we should
- // fall back to trying parent. The javadoc doesn't say...
- return null;
- }
+ }
+ final URL local = super.findResource(name);
+ if (local != null) {
+ try {
+ return local.openStream();
+ } catch(final IOException e) {
+ // TODO: check if this is right or whether we should
+ // fall back to trying parent. The javadoc doesn't say...
+ return null;
}
- return super.getResourceAsStream(name);
}
+ return super.getResourceAsStream(name);
}
}
diff --git
a/src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java
b/src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java
index 5a126f6..708ac22 100644
--- a/src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java
+++ b/src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java
@@ -251,11 +251,10 @@ public class WeakHashtableTestCase extends TestCase {
if(weakHashtable.get(new Long(1)) == null) {
break;
- } else {
- // create garbage:
- final byte[] b = new byte[bytz];
- bytz = bytz * 2;
}
+ // create garbage:
+ final byte[] b = new byte[bytz];
+ bytz = bytz * 2;
}
// some JVMs seem to take a little time to put references on
diff --git
a/src/test/java/org/apache/commons/logging/security/MockSecurityManager.java
b/src/test/java/org/apache/commons/logging/security/MockSecurityManager.java
index 53db44c..1a595fa 100644
--- a/src/test/java/org/apache/commons/logging/security/MockSecurityManager.java
+++ b/src/test/java/org/apache/commons/logging/security/MockSecurityManager.java
@@ -116,7 +116,8 @@ public class MockSecurityManager extends SecurityManager {
// the call stack.
System.out.println("Access controller found: returning");
return;
- } else if (cname.startsWith("java.")
+ }
+ if (cname.startsWith("java.")
|| cname.startsWith("javax.")
|| cname.startsWith("junit.")
|| cname.startsWith("org.apache.tools.ant.")
@@ -133,13 +134,12 @@ public class MockSecurityManager extends SecurityManager {
System.out.println("Untrusted code [testcase] found");
throw new SecurityException("Untrusted code [testcase] found");
} else if (cname.startsWith("org.apache.commons.logging.")) {
- if (permissions.implies(p)) {
- // Code here is trusted if the caller is trusted
- System.out.println("Permission in allowed set for JCL
class");
- } else {
+ if (!permissions.implies(p)) {
System.out.println("Permission refused:" + p.getClass() +
":" + p);
throw new SecurityException("Permission refused:" +
p.getClass() + ":" + p);
}
+ // Code here is trusted if the caller is trusted
+ System.out.println("Permission in allowed set for JCL class");
} else {
// we found some code that is not trusted to perform this
operation.
System.out.println("Unexpected code: permission refused:" +
p.getClass() + ":" + p);
