diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index a460251a6..a40362093 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -21,10 +21,14 @@ import static feign.Util.valuesOrEmpty; import static java.util.Objects.nonNull; +import java.io.Closeable; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; import java.util.logging.FileHandler; +import java.util.logging.Handler; import java.util.logging.LogRecord; import java.util.logging.SimpleFormatter; @@ -182,9 +186,10 @@ protected void log(String configKey, String format, Object... args) { } /** Logs to the category {@link Logger} at {@link java.util.logging.Level#FINE}, if loggable. */ - public static class JavaLogger extends Logger { + public static class JavaLogger extends Logger implements Closeable { final java.util.logging.Logger logger; + private final List handlers = new ArrayList<>(); /** * @deprecated Use {@link #JavaLogger(String)} or {@link #JavaLogger(Class)} instead. @@ -256,11 +261,21 @@ public String format(LogRecord record) { } }); logger.addHandler(handler); + handlers.add(handler); } catch (IOException e) { throw new IllegalStateException("Could not add file handler.", e); } return this; } + + @Override + public void close() { + for (Handler handler : handlers) { + logger.removeHandler(handler); + handler.close(); + } + handlers.clear(); + } } public static class NoOpLogger extends Logger { diff --git a/core/src/test/java/feign/MultipleLoggerTest.java b/core/src/test/java/feign/MultipleLoggerTest.java index 685b60d77..dbe1ff453 100644 --- a/core/src/test/java/feign/MultipleLoggerTest.java +++ b/core/src/test/java/feign/MultipleLoggerTest.java @@ -15,8 +15,11 @@ */ package feign; +import static org.assertj.core.api.Assertions.assertThat; + import java.io.File; import java.lang.reflect.Field; +import java.util.UUID; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -34,39 +37,53 @@ private static java.util.logging.Logger getInnerLogger(Logger.JavaLogger logger) @SuppressWarnings("deprecation") @Test void appendSeveralFilesToOneJavaLogger() throws Exception { - Logger.JavaLogger logger = + try (Logger.JavaLogger logger = new Logger.JavaLogger() .appendToFile(File.createTempFile("1.log", null, tmp).getAbsolutePath()) - .appendToFile(File.createTempFile("2.log", null, tmp).getAbsolutePath()); - java.util.logging.Logger inner = getInnerLogger(logger); - assert (inner.getHandlers().length == 2); + .appendToFile(File.createTempFile("2.log", null, tmp).getAbsolutePath())) { + java.util.logging.Logger inner = getInnerLogger(logger); + assert (inner.getHandlers().length == 2); + } + } + + @Test + void javaLoggerClosesFileHandlers() throws Exception { + File log = File.createTempFile("close.log", null, tmp); + Logger.JavaLogger logger = + new Logger.JavaLogger("close-" + UUID.randomUUID()).appendToFile(log.getAbsolutePath()); + + logger.close(); + + assertThat(getInnerLogger(logger).getHandlers()).isEmpty(); } @Test void javaLoggerInstantiationWithLoggerName() throws Exception { - Logger.JavaLogger l1 = - new Logger.JavaLogger("First client") - .appendToFile(File.createTempFile("1.log", null, tmp).getAbsolutePath()); - Logger.JavaLogger l2 = - new Logger.JavaLogger("Second client") - .appendToFile(File.createTempFile("2.log", null, tmp).getAbsolutePath()); - java.util.logging.Logger logger1 = getInnerLogger(l1); - assert (logger1.getHandlers().length == 1); - java.util.logging.Logger logger2 = getInnerLogger(l2); - assert (logger2.getHandlers().length == 1); + try (Logger.JavaLogger l1 = + new Logger.JavaLogger("First client") + .appendToFile(File.createTempFile("1.log", null, tmp).getAbsolutePath()); + Logger.JavaLogger l2 = + new Logger.JavaLogger("Second client") + .appendToFile(File.createTempFile("2.log", null, tmp).getAbsolutePath())) { + java.util.logging.Logger logger1 = getInnerLogger(l1); + assert (logger1.getHandlers().length == 1); + java.util.logging.Logger logger2 = getInnerLogger(l2); + assert (logger2.getHandlers().length == 1); + } } @Test void javaLoggerInstantationWithClazz() throws Exception { - Logger.JavaLogger l1 = - new Logger.JavaLogger(String.class) - .appendToFile(File.createTempFile("1.log", null, tmp).getAbsolutePath()); - Logger.JavaLogger l2 = - new Logger.JavaLogger(Integer.class) - .appendToFile(File.createTempFile("2.log", null, tmp).getAbsolutePath()); - java.util.logging.Logger logger1 = getInnerLogger(l1); - assert (logger1.getHandlers().length == 1); - java.util.logging.Logger logger2 = getInnerLogger(l2); - assert (logger2.getHandlers().length == 1); + try (Logger.JavaLogger l1 = + new Logger.JavaLogger(String.class) + .appendToFile(File.createTempFile("1.log", null, tmp).getAbsolutePath()); + Logger.JavaLogger l2 = + new Logger.JavaLogger(Integer.class) + .appendToFile(File.createTempFile("2.log", null, tmp).getAbsolutePath())) { + java.util.logging.Logger logger1 = getInnerLogger(l1); + assert (logger1.getHandlers().length == 1); + java.util.logging.Logger logger2 = getInnerLogger(l2); + assert (logger2.getHandlers().length == 1); + } } }