diff --git a/android/guava-tests/test/com/google/common/io/ByteSourceTest.java b/android/guava-tests/test/com/google/common/io/ByteSourceTest.java index 078b0f98b9e7..3b63728df36a 100644 --- a/android/guava-tests/test/com/google/common/io/ByteSourceTest.java +++ b/android/guava-tests/test/com/google/common/io/ByteSourceTest.java @@ -23,6 +23,7 @@ import static com.google.common.io.TestOption.READ_THROWS; import static com.google.common.io.TestOption.SKIP_THROWS; import static com.google.common.io.TestOption.WRITE_THROWS; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertThrows; @@ -31,9 +32,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.hash.Hashing; -import com.google.common.io.Closer.LoggingSuppressor; import com.google.common.primitives.UnsignedBytes; -import com.google.common.testing.TestLogHandler; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -378,61 +377,28 @@ public void testConcat_infiniteIterable() throws IOException { ImmutableSet.of(BROKEN_CLOSE_SINK, BROKEN_OPEN_SINK, BROKEN_WRITE_SINK); public void testCopyExceptions() { - if (Closer.create().suppressor instanceof LoggingSuppressor) { - // test that exceptions are logged - - TestLogHandler logHandler = new TestLogHandler(); - Closeables.logger.addHandler(logHandler); - try { - for (ByteSource in : BROKEN_SOURCES) { - runFailureTest(in, newNormalByteSink()); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - runFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, getAndResetRecords(logHandler)); - } - - for (ByteSink out : BROKEN_SINKS) { - runFailureTest(newNormalByteSource(), out); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); + // test that exceptions are suppressed - runFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, getAndResetRecords(logHandler)); - } + for (ByteSource in : BROKEN_SOURCES) { + int suppressed = runSuppressionFailureTest(in, newNormalByteSink()); + assertEquals(0, suppressed); - for (ByteSource in : BROKEN_SOURCES) { - for (ByteSink out : BROKEN_SINKS) { - runFailureTest(in, out); - assertTrue(getAndResetRecords(logHandler) <= 1); - } - } - } finally { - Closeables.logger.removeHandler(logHandler); - } - } else { - // test that exceptions are suppressed + suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); + assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); + } - for (ByteSource in : BROKEN_SOURCES) { - int suppressed = runSuppressionFailureTest(in, newNormalByteSink()); - assertEquals(0, suppressed); + for (ByteSink out : BROKEN_SINKS) { + int suppressed = runSuppressionFailureTest(newNormalByteSource(), out); + assertEquals(0, suppressed); - suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); - } + suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); + assertEquals(1, suppressed); + } + for (ByteSource in : BROKEN_SOURCES) { for (ByteSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(newNormalByteSource(), out); - assertEquals(0, suppressed); - - suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, suppressed); - } - - for (ByteSource in : BROKEN_SOURCES) { - for (ByteSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(in, out); - assertTrue(suppressed <= 1); - } + int suppressed = runSuppressionFailureTest(in, out); + assertThat(suppressed).isAtMost(1); } } } @@ -441,20 +407,6 @@ public void testSlice_returnEmptySource() { assertEquals(ByteSource.empty(), source.slice(0, 3).slice(4, 3)); } - private static int getAndResetRecords(TestLogHandler logHandler) { - int records = logHandler.getStoredLogRecords().size(); - logHandler.clear(); - return records; - } - - private static void runFailureTest(ByteSource in, ByteSink out) { - try { - in.copyTo(out); - fail(); - } catch (IOException expected) { - } - } - /** * @return the number of exceptions that were suppressed on the expected thrown exception */ @@ -463,7 +415,7 @@ private static int runSuppressionFailureTest(ByteSource in, ByteSink out) { in.copyTo(out); fail(); } catch (IOException expected) { - return CloserTest.getSuppressed(expected).length; + return expected.getSuppressed().length; } throw new AssertionError(); // can't happen } diff --git a/android/guava-tests/test/com/google/common/io/CharSourceTest.java b/android/guava-tests/test/com/google/common/io/CharSourceTest.java index a55ff4751ce0..2eb9706da670 100644 --- a/android/guava-tests/test/com/google/common/io/CharSourceTest.java +++ b/android/guava-tests/test/com/google/common/io/CharSourceTest.java @@ -20,14 +20,13 @@ import static com.google.common.io.TestOption.OPEN_THROWS; import static com.google.common.io.TestOption.READ_THROWS; import static com.google.common.io.TestOption.WRITE_THROWS; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import com.google.common.io.Closer.LoggingSuppressor; -import com.google.common.testing.TestLogHandler; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; @@ -248,76 +247,29 @@ public void testConcat_infiniteIterable() throws IOException { ImmutableSet.of(BROKEN_CLOSE_SINK, BROKEN_OPEN_SINK, BROKEN_WRITE_SINK); public void testCopyExceptions() { - if (Closer.create().suppressor instanceof LoggingSuppressor) { - // test that exceptions are logged - - TestLogHandler logHandler = new TestLogHandler(); - Closeables.logger.addHandler(logHandler); - try { - for (CharSource in : BROKEN_SOURCES) { - runFailureTest(in, newNormalCharSink()); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - runFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, getAndResetRecords(logHandler)); - } - - for (CharSink out : BROKEN_SINKS) { - runFailureTest(newNormalCharSource(), out); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - runFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, getAndResetRecords(logHandler)); - } - - for (CharSource in : BROKEN_SOURCES) { - for (CharSink out : BROKEN_SINKS) { - runFailureTest(in, out); - assertTrue(getAndResetRecords(logHandler) <= 1); - } - } - } finally { - Closeables.logger.removeHandler(logHandler); - } - } else { - // test that exceptions are suppressed - - for (CharSource in : BROKEN_SOURCES) { - int suppressed = runSuppressionFailureTest(in, newNormalCharSink()); - assertEquals(0, suppressed); + // test that exceptions are suppressed - suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); - } + for (CharSource in : BROKEN_SOURCES) { + int suppressed = runSuppressionFailureTest(in, newNormalCharSink()); + assertEquals(0, suppressed); - for (CharSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(newNormalCharSource(), out); - assertEquals(0, suppressed); + suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); + assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); + } - suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, suppressed); - } + for (CharSink out : BROKEN_SINKS) { + int suppressed = runSuppressionFailureTest(newNormalCharSource(), out); + assertEquals(0, suppressed); - for (CharSource in : BROKEN_SOURCES) { - for (CharSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(in, out); - assertTrue(suppressed <= 1); - } - } + suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); + assertEquals(1, suppressed); } - } - private static int getAndResetRecords(TestLogHandler logHandler) { - int records = logHandler.getStoredLogRecords().size(); - logHandler.clear(); - return records; - } - - private static void runFailureTest(CharSource in, CharSink out) { - try { - in.copyTo(out); - fail(); - } catch (IOException expected) { + for (CharSource in : BROKEN_SOURCES) { + for (CharSink out : BROKEN_SINKS) { + int suppressed = runSuppressionFailureTest(in, out); + assertThat(suppressed).isAtMost(1); + } } } @@ -329,7 +281,7 @@ private static int runSuppressionFailureTest(CharSource in, CharSink out) { in.copyTo(out); fail(); } catch (IOException expected) { - return CloserTest.getSuppressed(expected).length; + return expected.getSuppressed().length; } throw new AssertionError(); // can't happen } diff --git a/android/guava-tests/test/com/google/common/io/CloserTest.java b/android/guava-tests/test/com/google/common/io/CloserTest.java index 90c4eefe88af..6dce40fda390 100644 --- a/android/guava-tests/test/com/google/common/io/CloserTest.java +++ b/android/guava-tests/test/com/google/common/io/CloserTest.java @@ -25,13 +25,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; -import com.google.common.io.Closer.LoggingSuppressor; -import com.google.common.testing.TestLogHandler; import java.io.Closeable; import java.io.IOException; -import java.lang.reflect.Method; import java.util.List; -import java.util.logging.LogRecord; import junit.framework.TestCase; import org.checkerframework.checker.nullness.qual.Nullable; @@ -49,11 +45,6 @@ protected void setUp() throws Exception { suppressor = new TestSuppressor(); } - @AndroidIncompatible // TODO(cpovirk): Look up Build.VERSION.SDK_INT reflectively. - public void testCreate() { - assertThat(Closer.create().suppressor).isInstanceOf(Closer.SuppressingSuppressor.class); - } - public void testNoExceptionsThrown() throws IOException { Closer closer = new Closer(suppressor); @@ -282,42 +273,8 @@ public void testErrors() throws IOException { new Suppression(c1, c3Exception, c1Exception)); } - public static void testLoggingSuppressor() throws IOException { - TestLogHandler logHandler = new TestLogHandler(); - - Closeables.logger.addHandler(logHandler); - try { - Closer closer = new Closer(new Closer.LoggingSuppressor()); - - TestCloseable c1 = closer.register(TestCloseable.throwsOnClose(new IOException())); - TestCloseable c2 = closer.register(TestCloseable.throwsOnClose(new RuntimeException())); - try { - throw closer.rethrow(new IOException("thrown"), IOException.class); - } catch (IOException expected) { - } - - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - closer.close(); - - assertEquals(2, logHandler.getStoredLogRecords().size()); - - LogRecord record = logHandler.getStoredLogRecords().get(0); - assertEquals("Suppressing exception thrown when closing " + c2, record.getMessage()); - - record = logHandler.getStoredLogRecords().get(1); - assertEquals("Suppressing exception thrown when closing " + c1, record.getMessage()); - } finally { - Closeables.logger.removeHandler(logHandler); - } - } - - public static void testSuppressingSuppressorIfPossible() throws IOException { + public static void testSuppressingSuppressor() throws IOException { Closer closer = Closer.create(); - // can't test the JDK7 suppressor when not running on JDK7 - if (closer.suppressor instanceof LoggingSuppressor) { - return; - } IOException thrownException = new IOException(); IOException c1Exception = new IOException(); @@ -331,7 +288,7 @@ public static void testSuppressingSuppressorIfPossible() throws IOException { } catch (Throwable e) { throw closer.rethrow(thrownException, IOException.class); } finally { - assertThat(getSuppressed(thrownException)).isEmpty(); + assertThat(thrownException.getSuppressed()).isEmpty(); closer.close(); } } catch (IOException expected) { @@ -341,7 +298,7 @@ public static void testSuppressingSuppressorIfPossible() throws IOException { assertTrue(c1.isClosed()); assertTrue(c2.isClosed()); - ImmutableSet suppressed = ImmutableSet.copyOf(getSuppressed(thrownException)); + ImmutableSet suppressed = ImmutableSet.copyOf(thrownException.getSuppressed()); assertEquals(2, suppressed.size()); assertEquals(ImmutableSet.of(c1Exception, c2Exception), suppressed); @@ -353,15 +310,6 @@ public void testNullCloseable() throws IOException { closer.close(); } - static Throwable[] getSuppressed(Throwable throwable) { - try { - Method getSuppressed = Throwable.class.getDeclaredMethod("getSuppressed"); - return (Throwable[]) getSuppressed.invoke(throwable); - } catch (Exception e) { - throw new AssertionError(e); // only called if running on JDK7 - } - } - /** * Asserts that an exception was thrown when trying to close each of the given throwables and that * each such exception was suppressed because of the given thrown exception. @@ -370,6 +318,7 @@ private void assertSuppressed(Suppression... expected) { assertEquals(ImmutableList.copyOf(expected), suppressor.suppressions); } + // TODO(cpovirk): Just use addSuppressed+getSuppressed now that we can rely on it. /** Suppressor that records suppressions. */ private static class TestSuppressor implements Closer.Suppressor { diff --git a/android/guava/src/com/google/common/io/Closer.java b/android/guava/src/com/google/common/io/Closer.java index b1512ae14ca7..b947cd0994f8 100644 --- a/android/guava/src/com/google/common/io/Closer.java +++ b/android/guava/src/com/google/common/io/Closer.java @@ -24,7 +24,6 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.Closeable; import java.io.IOException; -import java.lang.reflect.Method; import java.util.ArrayDeque; import java.util.Deque; import java.util.logging.Level; @@ -33,12 +32,10 @@ /** * A {@link Closeable} that collects {@code Closeable} resources and closes them all when it is - * {@linkplain #close closed}. This is intended to approximately emulate the behavior of Java 7's try-with-resources statement in JDK6-compatible code. Running on Java 7, code using this - * should be approximately equivalent in behavior to the same code written with try-with-resources. - * Running on Java 6, exceptions that cannot be thrown must be logged rather than being added to the - * thrown exception as a suppressed exception. + * {@linkplain #close closed}. This was intended to approximately emulate the behavior of Java 7's + * try-with-resources statement in JDK6-compatible code. Code using this should be + * approximately equivalent in behavior to the same code written with try-with-resources. * *

This class is intended to be used in the following pattern: * @@ -75,14 +72,8 @@ * another exception is already being thrown) is suppressed. * * - *

An exception that is suppressed is not thrown. The method of suppression used depends on the - * version of Java the code is running on: - * - *

+ *

An exception that is suppressed is added to the exception that will be thrown using + * {@code Throwable.addSuppressed(Throwable)}. * * @author Colin Decker * @since 14.0 @@ -92,18 +83,9 @@ @GwtIncompatible @ElementTypesAreNonnullByDefault public final class Closer implements Closeable { - - /** The suppressor implementation to use for the current Java version. */ - private static final Suppressor SUPPRESSOR; - - static { - SuppressingSuppressor suppressingSuppressor = SuppressingSuppressor.tryCreate(); - SUPPRESSOR = suppressingSuppressor == null ? LoggingSuppressor.INSTANCE : suppressingSuppressor; - } - /** Creates a new {@link Closer}. */ public static Closer create() { - return new Closer(SUPPRESSOR); + return new Closer(SUPPRESSING_SUPPRESSOR); } @VisibleForTesting final Suppressor suppressor; @@ -248,55 +230,27 @@ interface Suppressor { void suppress(Closeable closeable, Throwable thrown, Throwable suppressed); } - /** Suppresses exceptions by logging them. */ - @VisibleForTesting - static final class LoggingSuppressor implements Suppressor { - - static final LoggingSuppressor INSTANCE = new LoggingSuppressor(); - - @Override - public void suppress(Closeable closeable, Throwable thrown, Throwable suppressed) { - // log to the same place as Closeables - Closeables.logger.log( - Level.WARNING, "Suppressing exception thrown when closing " + closeable, suppressed); - } - } - /** - * Suppresses exceptions by adding them to the exception that will be thrown using JDK7's + * Suppresses exceptions by adding them to the exception that will be thrown using the * addSuppressed(Throwable) mechanism. */ - @VisibleForTesting - static final class SuppressingSuppressor implements Suppressor { - @CheckForNull - static SuppressingSuppressor tryCreate() { - Method addSuppressed; - try { - addSuppressed = Throwable.class.getMethod("addSuppressed", Throwable.class); - } catch (Throwable e) { - return null; - } - return new SuppressingSuppressor(addSuppressed); - } - - private final Method addSuppressed; - - private SuppressingSuppressor(Method addSuppressed) { - this.addSuppressed = addSuppressed; - } - - @Override - public void suppress(Closeable closeable, Throwable thrown, Throwable suppressed) { - // ensure no exceptions from addSuppressed - if (thrown == suppressed) { - return; - } - try { - addSuppressed.invoke(thrown, suppressed); - } catch (Throwable e) { - // if, somehow, IllegalAccessException or another exception is thrown, fall back to logging - LoggingSuppressor.INSTANCE.suppress(closeable, thrown, suppressed); - } - } - } + private static final Suppressor SUPPRESSING_SUPPRESSOR = + (closeable, thrown, suppressed) -> { + // ensure no exceptions from addSuppressed + if (thrown == suppressed) { + return; + } + try { + thrown.addSuppressed(suppressed); + } catch (Throwable e) { + /* + * A Throwable is very unlikely, but we really don't want to throw from a Suppressor, so + * we catch everything. (Any Exception is either a RuntimeException or + * sneaky checked exception.) With no better options, we log anything to the same + * place as Closeables logs. + */ + Closeables.logger.log( + Level.WARNING, "Suppressing exception thrown when closing " + closeable, suppressed); + } + }; } diff --git a/guava-tests/test/com/google/common/io/ByteSourceTest.java b/guava-tests/test/com/google/common/io/ByteSourceTest.java index 078b0f98b9e7..3b63728df36a 100644 --- a/guava-tests/test/com/google/common/io/ByteSourceTest.java +++ b/guava-tests/test/com/google/common/io/ByteSourceTest.java @@ -23,6 +23,7 @@ import static com.google.common.io.TestOption.READ_THROWS; import static com.google.common.io.TestOption.SKIP_THROWS; import static com.google.common.io.TestOption.WRITE_THROWS; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertThrows; @@ -31,9 +32,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.hash.Hashing; -import com.google.common.io.Closer.LoggingSuppressor; import com.google.common.primitives.UnsignedBytes; -import com.google.common.testing.TestLogHandler; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -378,61 +377,28 @@ public void testConcat_infiniteIterable() throws IOException { ImmutableSet.of(BROKEN_CLOSE_SINK, BROKEN_OPEN_SINK, BROKEN_WRITE_SINK); public void testCopyExceptions() { - if (Closer.create().suppressor instanceof LoggingSuppressor) { - // test that exceptions are logged - - TestLogHandler logHandler = new TestLogHandler(); - Closeables.logger.addHandler(logHandler); - try { - for (ByteSource in : BROKEN_SOURCES) { - runFailureTest(in, newNormalByteSink()); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - runFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, getAndResetRecords(logHandler)); - } - - for (ByteSink out : BROKEN_SINKS) { - runFailureTest(newNormalByteSource(), out); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); + // test that exceptions are suppressed - runFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, getAndResetRecords(logHandler)); - } + for (ByteSource in : BROKEN_SOURCES) { + int suppressed = runSuppressionFailureTest(in, newNormalByteSink()); + assertEquals(0, suppressed); - for (ByteSource in : BROKEN_SOURCES) { - for (ByteSink out : BROKEN_SINKS) { - runFailureTest(in, out); - assertTrue(getAndResetRecords(logHandler) <= 1); - } - } - } finally { - Closeables.logger.removeHandler(logHandler); - } - } else { - // test that exceptions are suppressed + suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); + assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); + } - for (ByteSource in : BROKEN_SOURCES) { - int suppressed = runSuppressionFailureTest(in, newNormalByteSink()); - assertEquals(0, suppressed); + for (ByteSink out : BROKEN_SINKS) { + int suppressed = runSuppressionFailureTest(newNormalByteSource(), out); + assertEquals(0, suppressed); - suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); - } + suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); + assertEquals(1, suppressed); + } + for (ByteSource in : BROKEN_SOURCES) { for (ByteSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(newNormalByteSource(), out); - assertEquals(0, suppressed); - - suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, suppressed); - } - - for (ByteSource in : BROKEN_SOURCES) { - for (ByteSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(in, out); - assertTrue(suppressed <= 1); - } + int suppressed = runSuppressionFailureTest(in, out); + assertThat(suppressed).isAtMost(1); } } } @@ -441,20 +407,6 @@ public void testSlice_returnEmptySource() { assertEquals(ByteSource.empty(), source.slice(0, 3).slice(4, 3)); } - private static int getAndResetRecords(TestLogHandler logHandler) { - int records = logHandler.getStoredLogRecords().size(); - logHandler.clear(); - return records; - } - - private static void runFailureTest(ByteSource in, ByteSink out) { - try { - in.copyTo(out); - fail(); - } catch (IOException expected) { - } - } - /** * @return the number of exceptions that were suppressed on the expected thrown exception */ @@ -463,7 +415,7 @@ private static int runSuppressionFailureTest(ByteSource in, ByteSink out) { in.copyTo(out); fail(); } catch (IOException expected) { - return CloserTest.getSuppressed(expected).length; + return expected.getSuppressed().length; } throw new AssertionError(); // can't happen } diff --git a/guava-tests/test/com/google/common/io/CharSourceTest.java b/guava-tests/test/com/google/common/io/CharSourceTest.java index cf305fd61e95..e59bfcb6448c 100644 --- a/guava-tests/test/com/google/common/io/CharSourceTest.java +++ b/guava-tests/test/com/google/common/io/CharSourceTest.java @@ -21,14 +21,13 @@ import static com.google.common.io.TestOption.OPEN_THROWS; import static com.google.common.io.TestOption.READ_THROWS; import static com.google.common.io.TestOption.WRITE_THROWS; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import com.google.common.io.Closer.LoggingSuppressor; -import com.google.common.testing.TestLogHandler; import java.io.BufferedReader; import java.io.IOException; import java.io.Reader; @@ -278,76 +277,29 @@ public void testConcat_infiniteIterable() throws IOException { ImmutableSet.of(BROKEN_CLOSE_SINK, BROKEN_OPEN_SINK, BROKEN_WRITE_SINK); public void testCopyExceptions() { - if (Closer.create().suppressor instanceof LoggingSuppressor) { - // test that exceptions are logged - - TestLogHandler logHandler = new TestLogHandler(); - Closeables.logger.addHandler(logHandler); - try { - for (CharSource in : BROKEN_SOURCES) { - runFailureTest(in, newNormalCharSink()); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - runFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, getAndResetRecords(logHandler)); - } - - for (CharSink out : BROKEN_SINKS) { - runFailureTest(newNormalCharSource(), out); - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - runFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, getAndResetRecords(logHandler)); - } - - for (CharSource in : BROKEN_SOURCES) { - for (CharSink out : BROKEN_SINKS) { - runFailureTest(in, out); - assertTrue(getAndResetRecords(logHandler) <= 1); - } - } - } finally { - Closeables.logger.removeHandler(logHandler); - } - } else { - // test that exceptions are suppressed - - for (CharSource in : BROKEN_SOURCES) { - int suppressed = runSuppressionFailureTest(in, newNormalCharSink()); - assertEquals(0, suppressed); + // test that exceptions are suppressed - suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); - assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); - } + for (CharSource in : BROKEN_SOURCES) { + int suppressed = runSuppressionFailureTest(in, newNormalCharSink()); + assertEquals(0, suppressed); - for (CharSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(newNormalCharSource(), out); - assertEquals(0, suppressed); + suppressed = runSuppressionFailureTest(in, BROKEN_CLOSE_SINK); + assertEquals((in == BROKEN_OPEN_SOURCE) ? 0 : 1, suppressed); + } - suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); - assertEquals(1, suppressed); - } + for (CharSink out : BROKEN_SINKS) { + int suppressed = runSuppressionFailureTest(newNormalCharSource(), out); + assertEquals(0, suppressed); - for (CharSource in : BROKEN_SOURCES) { - for (CharSink out : BROKEN_SINKS) { - int suppressed = runSuppressionFailureTest(in, out); - assertTrue(suppressed <= 1); - } - } + suppressed = runSuppressionFailureTest(BROKEN_CLOSE_SOURCE, out); + assertEquals(1, suppressed); } - } - private static int getAndResetRecords(TestLogHandler logHandler) { - int records = logHandler.getStoredLogRecords().size(); - logHandler.clear(); - return records; - } - - private static void runFailureTest(CharSource in, CharSink out) { - try { - in.copyTo(out); - fail(); - } catch (IOException expected) { + for (CharSource in : BROKEN_SOURCES) { + for (CharSink out : BROKEN_SINKS) { + int suppressed = runSuppressionFailureTest(in, out); + assertThat(suppressed).isAtMost(1); + } } } @@ -359,7 +311,7 @@ private static int runSuppressionFailureTest(CharSource in, CharSink out) { in.copyTo(out); fail(); } catch (IOException expected) { - return CloserTest.getSuppressed(expected).length; + return expected.getSuppressed().length; } throw new AssertionError(); // can't happen } diff --git a/guava-tests/test/com/google/common/io/CloserTest.java b/guava-tests/test/com/google/common/io/CloserTest.java index 90c4eefe88af..6dce40fda390 100644 --- a/guava-tests/test/com/google/common/io/CloserTest.java +++ b/guava-tests/test/com/google/common/io/CloserTest.java @@ -25,13 +25,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; -import com.google.common.io.Closer.LoggingSuppressor; -import com.google.common.testing.TestLogHandler; import java.io.Closeable; import java.io.IOException; -import java.lang.reflect.Method; import java.util.List; -import java.util.logging.LogRecord; import junit.framework.TestCase; import org.checkerframework.checker.nullness.qual.Nullable; @@ -49,11 +45,6 @@ protected void setUp() throws Exception { suppressor = new TestSuppressor(); } - @AndroidIncompatible // TODO(cpovirk): Look up Build.VERSION.SDK_INT reflectively. - public void testCreate() { - assertThat(Closer.create().suppressor).isInstanceOf(Closer.SuppressingSuppressor.class); - } - public void testNoExceptionsThrown() throws IOException { Closer closer = new Closer(suppressor); @@ -282,42 +273,8 @@ public void testErrors() throws IOException { new Suppression(c1, c3Exception, c1Exception)); } - public static void testLoggingSuppressor() throws IOException { - TestLogHandler logHandler = new TestLogHandler(); - - Closeables.logger.addHandler(logHandler); - try { - Closer closer = new Closer(new Closer.LoggingSuppressor()); - - TestCloseable c1 = closer.register(TestCloseable.throwsOnClose(new IOException())); - TestCloseable c2 = closer.register(TestCloseable.throwsOnClose(new RuntimeException())); - try { - throw closer.rethrow(new IOException("thrown"), IOException.class); - } catch (IOException expected) { - } - - assertTrue(logHandler.getStoredLogRecords().isEmpty()); - - closer.close(); - - assertEquals(2, logHandler.getStoredLogRecords().size()); - - LogRecord record = logHandler.getStoredLogRecords().get(0); - assertEquals("Suppressing exception thrown when closing " + c2, record.getMessage()); - - record = logHandler.getStoredLogRecords().get(1); - assertEquals("Suppressing exception thrown when closing " + c1, record.getMessage()); - } finally { - Closeables.logger.removeHandler(logHandler); - } - } - - public static void testSuppressingSuppressorIfPossible() throws IOException { + public static void testSuppressingSuppressor() throws IOException { Closer closer = Closer.create(); - // can't test the JDK7 suppressor when not running on JDK7 - if (closer.suppressor instanceof LoggingSuppressor) { - return; - } IOException thrownException = new IOException(); IOException c1Exception = new IOException(); @@ -331,7 +288,7 @@ public static void testSuppressingSuppressorIfPossible() throws IOException { } catch (Throwable e) { throw closer.rethrow(thrownException, IOException.class); } finally { - assertThat(getSuppressed(thrownException)).isEmpty(); + assertThat(thrownException.getSuppressed()).isEmpty(); closer.close(); } } catch (IOException expected) { @@ -341,7 +298,7 @@ public static void testSuppressingSuppressorIfPossible() throws IOException { assertTrue(c1.isClosed()); assertTrue(c2.isClosed()); - ImmutableSet suppressed = ImmutableSet.copyOf(getSuppressed(thrownException)); + ImmutableSet suppressed = ImmutableSet.copyOf(thrownException.getSuppressed()); assertEquals(2, suppressed.size()); assertEquals(ImmutableSet.of(c1Exception, c2Exception), suppressed); @@ -353,15 +310,6 @@ public void testNullCloseable() throws IOException { closer.close(); } - static Throwable[] getSuppressed(Throwable throwable) { - try { - Method getSuppressed = Throwable.class.getDeclaredMethod("getSuppressed"); - return (Throwable[]) getSuppressed.invoke(throwable); - } catch (Exception e) { - throw new AssertionError(e); // only called if running on JDK7 - } - } - /** * Asserts that an exception was thrown when trying to close each of the given throwables and that * each such exception was suppressed because of the given thrown exception. @@ -370,6 +318,7 @@ private void assertSuppressed(Suppression... expected) { assertEquals(ImmutableList.copyOf(expected), suppressor.suppressions); } + // TODO(cpovirk): Just use addSuppressed+getSuppressed now that we can rely on it. /** Suppressor that records suppressions. */ private static class TestSuppressor implements Closer.Suppressor { diff --git a/guava/src/com/google/common/io/Closer.java b/guava/src/com/google/common/io/Closer.java index b1512ae14ca7..b947cd0994f8 100644 --- a/guava/src/com/google/common/io/Closer.java +++ b/guava/src/com/google/common/io/Closer.java @@ -24,7 +24,6 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.Closeable; import java.io.IOException; -import java.lang.reflect.Method; import java.util.ArrayDeque; import java.util.Deque; import java.util.logging.Level; @@ -33,12 +32,10 @@ /** * A {@link Closeable} that collects {@code Closeable} resources and closes them all when it is - * {@linkplain #close closed}. This is intended to approximately emulate the behavior of Java 7's try-with-resources statement in JDK6-compatible code. Running on Java 7, code using this - * should be approximately equivalent in behavior to the same code written with try-with-resources. - * Running on Java 6, exceptions that cannot be thrown must be logged rather than being added to the - * thrown exception as a suppressed exception. + * {@linkplain #close closed}. This was intended to approximately emulate the behavior of Java 7's + * try-with-resources statement in JDK6-compatible code. Code using this should be + * approximately equivalent in behavior to the same code written with try-with-resources. * *

This class is intended to be used in the following pattern: * @@ -75,14 +72,8 @@ * another exception is already being thrown) is suppressed. * * - *

An exception that is suppressed is not thrown. The method of suppression used depends on the - * version of Java the code is running on: - * - *

+ *

An exception that is suppressed is added to the exception that will be thrown using + * {@code Throwable.addSuppressed(Throwable)}. * * @author Colin Decker * @since 14.0 @@ -92,18 +83,9 @@ @GwtIncompatible @ElementTypesAreNonnullByDefault public final class Closer implements Closeable { - - /** The suppressor implementation to use for the current Java version. */ - private static final Suppressor SUPPRESSOR; - - static { - SuppressingSuppressor suppressingSuppressor = SuppressingSuppressor.tryCreate(); - SUPPRESSOR = suppressingSuppressor == null ? LoggingSuppressor.INSTANCE : suppressingSuppressor; - } - /** Creates a new {@link Closer}. */ public static Closer create() { - return new Closer(SUPPRESSOR); + return new Closer(SUPPRESSING_SUPPRESSOR); } @VisibleForTesting final Suppressor suppressor; @@ -248,55 +230,27 @@ interface Suppressor { void suppress(Closeable closeable, Throwable thrown, Throwable suppressed); } - /** Suppresses exceptions by logging them. */ - @VisibleForTesting - static final class LoggingSuppressor implements Suppressor { - - static final LoggingSuppressor INSTANCE = new LoggingSuppressor(); - - @Override - public void suppress(Closeable closeable, Throwable thrown, Throwable suppressed) { - // log to the same place as Closeables - Closeables.logger.log( - Level.WARNING, "Suppressing exception thrown when closing " + closeable, suppressed); - } - } - /** - * Suppresses exceptions by adding them to the exception that will be thrown using JDK7's + * Suppresses exceptions by adding them to the exception that will be thrown using the * addSuppressed(Throwable) mechanism. */ - @VisibleForTesting - static final class SuppressingSuppressor implements Suppressor { - @CheckForNull - static SuppressingSuppressor tryCreate() { - Method addSuppressed; - try { - addSuppressed = Throwable.class.getMethod("addSuppressed", Throwable.class); - } catch (Throwable e) { - return null; - } - return new SuppressingSuppressor(addSuppressed); - } - - private final Method addSuppressed; - - private SuppressingSuppressor(Method addSuppressed) { - this.addSuppressed = addSuppressed; - } - - @Override - public void suppress(Closeable closeable, Throwable thrown, Throwable suppressed) { - // ensure no exceptions from addSuppressed - if (thrown == suppressed) { - return; - } - try { - addSuppressed.invoke(thrown, suppressed); - } catch (Throwable e) { - // if, somehow, IllegalAccessException or another exception is thrown, fall back to logging - LoggingSuppressor.INSTANCE.suppress(closeable, thrown, suppressed); - } - } - } + private static final Suppressor SUPPRESSING_SUPPRESSOR = + (closeable, thrown, suppressed) -> { + // ensure no exceptions from addSuppressed + if (thrown == suppressed) { + return; + } + try { + thrown.addSuppressed(suppressed); + } catch (Throwable e) { + /* + * A Throwable is very unlikely, but we really don't want to throw from a Suppressor, so + * we catch everything. (Any Exception is either a RuntimeException or + * sneaky checked exception.) With no better options, we log anything to the same + * place as Closeables logs. + */ + Closeables.logger.log( + Level.WARNING, "Suppressing exception thrown when closing " + closeable, suppressed); + } + }; }