diff --git a/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethodsSupport.java b/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethodsSupport.java index a81f2920ab7..e3ef111203a 100644 --- a/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethodsSupport.java +++ b/src/main/org/codehaus/groovy/runtime/DefaultGroovyMethodsSupport.java @@ -83,16 +83,33 @@ protected static int normaliseIndex(int i, int size) { /** * Close the Closeable. Logging a warning if any problems occur. * - * @param c the thing to close + * @param closeable the thing to close */ - public static void closeWithWarning(Closeable c) { - if (c != null) { + public static void closeWithWarning(Closeable closeable) { + tryClose(closeable, true); // ignore result + } + + /** + * Attempts to close the closeable returning rather than throwing + * any Exception that may occur. + * + * @param closeable the thing to close + * @param logWarning if true will log a warning if an exception occurs + * @return throwable Exception from the close method, else null + */ + static Throwable tryClose(AutoCloseable closeable, boolean logWarning) { + Throwable thrown = null; + if (closeable != null) { try { - c.close(); - } catch (IOException e) { - LOG.warning("Caught exception during close(): " + e); + closeable.close(); + } catch (Exception e) { + thrown = e; + if (logWarning) { + LOG.warning("Caught exception during close(): " + e); + } } } + return thrown; } /** diff --git a/src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java b/src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java index e7c26fdee29..f33d3771140 100644 --- a/src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java +++ b/src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java @@ -1589,6 +1589,10 @@ public static void filterLine(InputStream self, Writer writer, String charset, @ /** * Allows this closeable to be used within the closure, ensuring that it * is closed once the closure has been executed and before this method returns. + *

+ * As with the try-with-resources statement, if multiple exceptions are thrown + * the exception from the closure will be returned and the exception from closing + * will be added as a suppressed exception. * * @param self the Closeable * @param action the closure taking the Closeable as parameter @@ -1597,16 +1601,54 @@ public static void filterLine(InputStream self, Writer writer, String charset, @ * @since 2.4.0 */ public static T withCloseable(U self, @ClosureParams(value=FirstParam.class) Closure action) throws IOException { + Throwable thrown = null; try { - T result = action.call(self); - - Closeable temp = self; - self = null; - temp.close(); + return action.call(self); + } catch (Throwable e) { + thrown = e; + throw e; + } finally { + if (thrown != null) { + Throwable suppressed = tryClose(self, true); + if (suppressed != null) { + thrown.addSuppressed(suppressed); + } + } else { + self.close(); + } + } + } - return result; + /** + * Allows this AutoCloseable to be used within the closure, ensuring that it + * is closed once the closure has been executed and before this method returns. + *

+ * As with the try-with-resources statement, if multiple exceptions are thrown + * the exception from the closure will be returned and the exception from closing + * will be added as a suppressed exception. + * + * @param self the AutoCloseable + * @param action the closure taking the AutoCloseable as parameter + * @return the value returned by the closure + * @throws Exception if an Exception occurs. + * @since 2.5.0 + */ + public static T withCloseable(U self, @ClosureParams(value=FirstParam.class) Closure action) throws Exception { + Throwable thrown = null; + try { + return action.call(self); + } catch (Throwable e) { + thrown = e; + throw e; } finally { - DefaultGroovyMethodsSupport.closeWithWarning(self); + if (thrown != null) { + Throwable suppressed = tryClose(self, true); + if (suppressed != null) { + thrown.addSuppressed(suppressed); + } + } else { + self.close(); + } } } diff --git a/src/test/org/codehaus/groovy/runtime/IOGroovyMethodsTest.groovy b/src/test/org/codehaus/groovy/runtime/IOGroovyMethodsTest.groovy new file mode 100644 index 00000000000..e7baa43365e --- /dev/null +++ b/src/test/org/codehaus/groovy/runtime/IOGroovyMethodsTest.groovy @@ -0,0 +1,130 @@ +/* + * 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.codehaus.groovy.runtime + +import groovy.test.GroovyAssert + +class IOGroovyMethodsTest extends GroovyTestCase { + + void testWithAutoCloseable() { + def closeable = new DummyAutoCloseable() + def closeableParam = null + def result = closeable.withCloseable { + closeableParam = it + 123 + } + assert closeableParam == closeable + assert result == 123 + assert closeable.closed + } + + void testWithAutoCloseableDoesNotSuppressException() { + def closeable = new DummyAutoCloseable(new Exception('close exception')) + def throwable = GroovyAssert.shouldFail(UnsupportedOperationException) { + closeable.withCloseable { + throw new UnsupportedOperationException('not a close exception') + } + } + assert closeable.closed + assert throwable.message == 'not a close exception' + assert throwable.suppressed.find { it.message == 'close exception' } + } + + void testWithAutoCloseableAndException() { + def closeable = new DummyAutoCloseable(new Exception('close exception')) + def result = null + def message = shouldFail(Exception) { + closeable.withCloseable { + result = 123 + } + } + assert result == 123 + assert message == 'close exception' + } + + void testWithCloseable() { + def closeable = new DummyCloseable() + def closeableParam = null + def result = closeable.withCloseable { + closeableParam = it + 123 + } + assert closeableParam == closeable + assert result == 123 + assert closeable.closed + } + + void testWithCloseableDoesNotSuppressException() { + def closeable = new DummyCloseable(new IOException('close ioexception')) + def throwable = GroovyAssert.shouldFail(Exception) { + closeable.withCloseable { + throw new Exception('not a close ioexception') + } + } + assert closeable.closed + assert throwable.message == 'not a close ioexception' + assert throwable.suppressed.find { it.message == 'close ioexception' } + } + + void testWithCloseableAndException() { + def closeable = new DummyCloseable(new IOException('close ioexception')) + def result = null + def message = shouldFail(IOException) { + closeable.withCloseable { + result = 123 + } + } + assert result == 123 + assert message == 'close ioexception' + } + + // -------------------------------------------------------------------- + // Helper Classes + + static class DummyAutoCloseable implements AutoCloseable { + Exception throwOnClose + boolean closed + DummyAutoCloseable(Exception throwOnClose=null) { + this.throwOnClose = throwOnClose + } + @Override + void close() throws Exception { + closed = true + if (throwOnClose) { + throw throwOnClose + } + } + } + + static class DummyCloseable implements Closeable { + Exception throwOnClose + boolean closed + DummyCloseable(Exception throwOnClose=null) { + this.throwOnClose = throwOnClose + } + @Override + void close() throws IOException { + closed = true + if (throwOnClose) { + throw throwOnClose + } + } + } + +} diff --git a/subprojects/groovy-nio/src/main/java/org/codehaus/groovy/runtime/NioGroovyMethods.java b/subprojects/groovy-nio/src/main/java/org/codehaus/groovy/runtime/NioGroovyMethods.java index 6ac4837effe..385bcaaeb9a 100644 --- a/subprojects/groovy-nio/src/main/java/org/codehaus/groovy/runtime/NioGroovyMethods.java +++ b/subprojects/groovy-nio/src/main/java/org/codehaus/groovy/runtime/NioGroovyMethods.java @@ -50,7 +50,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.logging.Logger; import java.util.regex.Pattern; import groovy.io.FileType; @@ -60,7 +59,6 @@ import groovy.lang.MetaClass; import groovy.lang.Writable; import groovy.transform.stc.ClosureParams; -import groovy.transform.stc.FirstParam; import groovy.transform.stc.FromString; import groovy.transform.stc.PickFirstResolver; import groovy.transform.stc.SimpleType; @@ -113,8 +111,6 @@ public class NioGroovyMethods extends DefaultGroovyMethodsSupport { - private static final Logger LOG = Logger.getLogger(NioGroovyMethods.class.getName()); - /** * Provide the standard Groovy size() method for Path. * @@ -1949,44 +1945,4 @@ public static T withCloseable(Closeable self, @ClosureParams(value = SimpleT return IOGroovyMethods.withCloseable(self, action); } - /** - * Allows this autocloseable to be used within the closure, ensuring that it - * is closed once the closure has been executed and before this method returns. - * - * @param self the AutoCloseable - * @param action the closure taking the AutoCloseable as parameter - * @return the value returned by the closure - * @throws Exception if an Exception occurs. - * @since 2.5.0 - */ - public static T withAutoCloseable(U self, @ClosureParams(value=FirstParam.class) Closure action) throws Exception { - try { - T result = action.call(self); - - AutoCloseable temp = self; - self = null; - temp.close(); - - return result; - } finally { - closeWithWarning(self); - } - } - - /** - * Close the AutoCloseable. Logging a warning if any problems occur. - * - * @param c the thing to close - */ - public static void closeWithWarning(AutoCloseable c) { - if (c != null) { - try { - c.close(); - } catch (Exception e) { - LOG.warning("Caught exception during close(): " + e); - } - } - } - - } diff --git a/subprojects/groovy-nio/src/test/groovy/org/codehaus/groovy/runtime/NioGroovyMethodsTest.groovy b/subprojects/groovy-nio/src/test/groovy/org/codehaus/groovy/runtime/NioGroovyMethodsTest.groovy index 86bc596212e..01fc88e2a94 100644 --- a/subprojects/groovy-nio/src/test/groovy/org/codehaus/groovy/runtime/NioGroovyMethodsTest.groovy +++ b/subprojects/groovy-nio/src/test/groovy/org/codehaus/groovy/runtime/NioGroovyMethodsTest.groovy @@ -543,73 +543,4 @@ class NioGroovyMethodsTest extends Specification { assert NioGroovyMethods.getBytes(path) == [-2, -1, 0, 72, 0, 101, 0, 108, 0, 108, 0, 111, 0, 32, 0, 119, 0, 111, 0, 114, 0, 108, 0, 100, 0, 33] as byte[] } - def testWithCloseable() { - setup: - def closeable = new DummyCloseable() - - when: - def closeableParam = null - def result = closeable.withCloseable { - closeableParam = it - 123 - } - - then: - closeableParam == closeable - result == 123 - closeable.closed - } - - def testWithAutoCloseable() { - setup: - def closeable = new DummyAutoCloseable() - - when: - def closeableParam = null - def result = closeable.withAutoCloseable { - closeableParam = it - 123 - } - - then: - closeableParam == closeable - result == 123 - closeable.closed - } - - def testWithCloseableAndException() { - setup: - def closeable = new ExceptionDummyCloseable() - - when: - closeable.close() - - then: - thrown IOException - } -} - -class DummyCloseable implements Closeable { - boolean closed = false - - @Override - void close() throws IOException { - closed = true - } -} - -class DummyAutoCloseable implements AutoCloseable { - boolean closed = false - - @Override - void close() throws IOException { - closed = true - } -} - -class ExceptionDummyCloseable implements Closeable { - @Override - void close() throws IOException { - throw new IOException('boom badaboom') - } }