diff --git a/pom.xml b/pom.xml index 5a496a7..6dfbbd2 100644 --- a/pom.xml +++ b/pom.xml @@ -391,6 +391,9 @@ under the License. ${failsafe.runorder} + + org/apache/commons/logging/serviceloader/** + **/*TestCase.java @@ -408,6 +411,24 @@ under the License. + + + serviceLoader-test + + integration-test + + + + ${project.build.testOutputDirectory}/serviceloader + + + org/apache/commons/logging/serviceloader/*TestCase.java + + + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f1326d6..545f946 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -65,7 +65,10 @@ The type attribute can be add,update,fix,remove. [StepSecurity] ci: Harden GitHub Actions #145. - + + + Replace custom code with `ServiceLoader` call. + Bump Java from 6 to 8. diff --git a/src/main/java/org/apache/commons/logging/LogFactory.java b/src/main/java/org/apache/commons/logging/LogFactory.java index 150f689..46c2d23 100644 --- a/src/main/java/org/apache/commons/logging/LogFactory.java +++ b/src/main/java/org/apache/commons/logging/LogFactory.java @@ -30,7 +30,10 @@ import java.security.AccessController; import java.security.PrivilegedAction; import java.util.Enumeration; import java.util.Hashtable; +import java.util.Iterator; import java.util.Properties; +import java.util.ServiceConfigurationError; +import java.util.ServiceLoader; /** * Factory for creating {@link Log} instances, with discovery and @@ -189,6 +192,12 @@ public abstract class LogFactory { */ private static final WeakReference thisClassLoaderRef; + /** + * Maximum number of {@link ServiceLoader} errors to ignore, while + * looking for an implementation. + */ + private static final int MAX_BROKEN_SERVICES = 3; + // ----------------------------------------------------------- Constructors /** @@ -507,42 +516,25 @@ public abstract class LogFactory { if (factory == null) { if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] Looking for a resource file of name [" + SERVICE_ID + - "] to define the LogFactory subclass to use..."); + logDiagnostic("[LOOKUP] Using ServiceLoader to define the LogFactory subclass to use..."); } try { - final InputStream is = getResourceAsStream(contextClassLoader, SERVICE_ID); + final ServiceLoader serviceLoader = ServiceLoader.load(LogFactory.class); + final Iterator iterator = serviceLoader.iterator(); - if ( is != null ) { - // This code is needed by EBCDIC and other strange systems. - // It's a fix for bugs reported in xerces - BufferedReader rd; + int i = MAX_BROKEN_SERVICES; + while (factory == null && i-- > 0) { try { - rd = new BufferedReader(new InputStreamReader(is, "UTF-8")); - } catch (final java.io.UnsupportedEncodingException e) { - rd = new BufferedReader(new InputStreamReader(is)); - } - - String factoryClassName; - try { - factoryClassName = rd.readLine(); - } finally { - rd.close(); - } - - if (factoryClassName != null && ! factoryClassName.isEmpty()) { - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] Creating an instance of LogFactory class " + - factoryClassName + - " as specified by file '" + SERVICE_ID + - "' which was present in the path of the context classloader."); + if (iterator.hasNext()) { + factory = iterator.next(); + } + } catch (final ServiceConfigurationError | LinkageError ex) { + if (isDiagnosticsEnabled()) { + logDiagnostic("[LOOKUP] An exception occurred while trying to find an" + + " instance of LogFactory" + + ": [" + trim(ex.getMessage()) + + "]. Trying alternative implementations..."); } - factory = newFactory(factoryClassName, baseClassLoader, contextClassLoader ); - } - } else { - // is == null - if (isDiagnosticsEnabled()) { - logDiagnostic("[LOOKUP] No resource file with name '" + SERVICE_ID + "' found."); } } } catch (final Exception ex) { diff --git a/src/test/java/org/apache/commons/logging/serviceloader/ServiceLoaderTestCase.java b/src/test/java/org/apache/commons/logging/serviceloader/ServiceLoaderTestCase.java new file mode 100644 index 0000000..415bc50 --- /dev/null +++ b/src/test/java/org/apache/commons/logging/serviceloader/ServiceLoaderTestCase.java @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.logging.serviceloader; + +import junit.framework.TestCase; +import org.apache.commons.logging.LogFactory; +import org.apache.commons.logging.serviceloader.internal.DummyLogFactory; + +public class ServiceLoaderTestCase extends TestCase { + + public void testServiceLoader() { + final LogFactory factory = LogFactory.getFactory(); + assertTrue("Wrong factory retrieved through ServiceLoader: " + factory.getClass().getName(), + factory instanceof DummyLogFactory); + } +} diff --git a/src/test/java/org/apache/commons/logging/serviceloader/internal/DummyLogFactory.java b/src/test/java/org/apache/commons/logging/serviceloader/internal/DummyLogFactory.java new file mode 100644 index 0000000..ce1779f --- /dev/null +++ b/src/test/java/org/apache/commons/logging/serviceloader/internal/DummyLogFactory.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.logging.serviceloader.internal; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogConfigurationException; +import org.apache.commons.logging.LogFactory; + +public class DummyLogFactory extends LogFactory { + @Override + public Object getAttribute(String name) { + return null; + } + + @Override + public String[] getAttributeNames() { + return new String[0]; + } + + @Override + public Log getInstance(Class clazz) throws LogConfigurationException { + return null; + } + + @Override + public Log getInstance(String name) throws LogConfigurationException { + return null; + } + + @Override + public void release() { + // empty + } + + @Override + public void removeAttribute(String name) { + // empty + } + + @Override + public void setAttribute(String name, Object value) { + // empty + } +} diff --git a/src/test/java/org/apache/commons/logging/serviceloader/internal/ThrowingLogFactory.java b/src/test/java/org/apache/commons/logging/serviceloader/internal/ThrowingLogFactory.java new file mode 100644 index 0000000..650d031 --- /dev/null +++ b/src/test/java/org/apache/commons/logging/serviceloader/internal/ThrowingLogFactory.java @@ -0,0 +1,25 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.logging.serviceloader.internal; + +/** + * A common ServiceLoader error is finding a class that implements LogFactory from a different classloader. + * This class should emulate that behavior. + */ +public class ThrowingLogFactory { + // empty +} diff --git a/src/test/resources/serviceloader/META-INF/services/org.apache.commons.logging.LogFactory b/src/test/resources/serviceloader/META-INF/services/org.apache.commons.logging.LogFactory new file mode 100644 index 0000000..db99af0 --- /dev/null +++ b/src/test/resources/serviceloader/META-INF/services/org.apache.commons.logging.LogFactory @@ -0,0 +1,25 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +## +# Version 1.2 of Apache Commons Logging did not support comments in the service file. +# +# A class that does not implement LogFactory to emulate services the implement +# LogFactory for another classloader +org.apache.commons.logging.serviceloader.internal.ThrowingLogFactory +## +# A proper LogFactory +org.apache.commons.logging.serviceloader.internal.DummyLogFactory diff --git a/src/test/resources/serviceloader/README.txt b/src/test/resources/serviceloader/README.txt new file mode 100644 index 0000000..f604285 --- /dev/null +++ b/src/test/resources/serviceloader/README.txt @@ -0,0 +1 @@ +Contains resources used exclusively by `ServiceLoaderTestCase`