From ad8f2abbb41e4e70a8439e2b31b8b87c6462b7b4 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 23 Jan 2018 08:34:07 +0100 Subject: [PATCH] Issue #2075 cleanup MultiException to better use suppressed (#2105) * Issue #2075 cleanup multiexceptio to better use suppressed * Update MultiException.java fixes from review Signed-off-by: Greg Wilkins --- .../eclipse/jetty/util/MultiException.java | 149 +++++++++++------- .../jetty/util/MultiExceptionTest.java | 26 ++- 2 files changed, 113 insertions(+), 62 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java index c51b0e66acd..124bcd57c4e 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.util; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -30,8 +31,6 @@ import java.util.List; @SuppressWarnings("serial") public class MultiException extends Exception { - private List nested; - /* ------------------------------------------------------------ */ public MultiException() { @@ -44,69 +43,105 @@ public class MultiException extends Exception if (e==null) throw new IllegalArgumentException(); - if(nested == null) - { - initCause(e); - nested = new ArrayList<>(); - } - else - addSuppressed(e); - if (e instanceof MultiException) - { - MultiException me = (MultiException)e; - nested.addAll(me.nested); - } + Arrays.stream(MultiException.class.cast(e).getSuppressed()).forEach(this::add); else - nested.add(e); + { + if (getCause()==null) + initCause(e); + else + addSuppressed(e); + } } /* ------------------------------------------------------------ */ public int size() { - return (nested ==null)?0:nested.size(); + if (getCause()==null) + return 0; + return 1+getSuppressed().length; } /* ------------------------------------------------------------ */ public List getThrowables() { - if(nested == null) + if (getCause()==null) return Collections.emptyList(); - return nested; + + Throwable[] suppressed = getSuppressed(); + List list = new ArrayList<>(suppressed.length+1); + list.add(getCause()); + Arrays.stream(suppressed).forEach(list::add); + return list; } /* ------------------------------------------------------------ */ public Throwable getThrowable(int i) { - return nested.get(i); + if (getCause()==null) + throw new ArrayIndexOutOfBoundsException(); + if (i==0) + return getCause(); + return getSuppressed()[i-1]; } /* ------------------------------------------------------------ */ /** Throw a multiexception. * If this multi exception is empty then no action is taken. If it - * contains a single exception that is thrown, otherwise the this + * contains a single exception then that is thrown, otherwise the this * multi exception is thrown. - * @exception Exception the Error or Exception if nested is 1, or the MultiException itself if nested is more than 1. + * @exception Exception the Error or Exception if nested is 1, or + * the MultiException itself if nested is more than 1. */ public void ifExceptionThrow() throws Exception { - if(nested == null) + Throwable cause=getCause(); + if (cause==null) return; - switch (nested.size()) + Throwable[] suppressed = getSuppressed(); + + if (suppressed.length==0) { - case 0: - break; - case 1: - Throwable th=nested.get(0); - if (th instanceof Error) - throw (Error)th; - if (th instanceof Exception) - throw (Exception)th; - default: - throw this; + if (cause instanceof Error) + throw (Error)cause; + if (cause instanceof Exception) + throw (Exception)cause; } + + throw this; + } + + + /* ------------------------------------------------------------ */ + /** Throw an Exception, potentially with suppress. + * If this multi exception is empty then no action is taken. If the first + * exception added is an Error or Exception, then that is throw with + * any additional exceptions added as suppressed. Otherwise this exception + * is thrown. + * @exception Exception the Error or Exception if at least one is added. + */ + public void ifExceptionThrowSuppressed() + throws Exception + { + Throwable cause=getCause(); + if (cause==null) + return; + + if (cause instanceof Error) + { + Arrays.stream(getSuppressed()).forEach(cause::addSuppressed); + throw (Error)cause; + } + + if (cause instanceof Exception) + { + Arrays.stream(getSuppressed()).forEach(cause::addSuppressed); + throw (Exception)cause; + } + + throw this; } /* ------------------------------------------------------------ */ @@ -120,42 +155,38 @@ public class MultiException extends Exception */ public void ifExceptionThrowRuntime() throws Error - { - if(nested == null) + { + Throwable cause = getCause(); + if (cause==null) return; - - switch (nested.size()) + + Throwable[] nested = getSuppressed(); + + if (nested.length==0) { - case 0: - break; - case 1: - Throwable th=nested.get(0); - if (th instanceof Error) - throw (Error)th; - else if (th instanceof RuntimeException) - throw (RuntimeException)th; - else - throw new RuntimeException(th); - default: - throw new RuntimeException(this); + if (cause instanceof Error) + throw (Error)cause; + if (cause instanceof RuntimeException) + throw (RuntimeException)cause; + throw new RuntimeException(cause); } + + throw new RuntimeException(this); } /* ------------------------------------------------------------ */ - /** Throw a multiexception. + /** Throw a MultiException. * If this multi exception is empty then no action is taken. If it - * contains a any exceptions then this - * multi exception is thrown. + * contains a any exceptions then this multi exception is thrown. * @throws MultiException the multiexception if there are nested exception */ public void ifExceptionThrowMulti() throws MultiException { - if(nested == null) + if (getCause()==null) return; - - if (nested.size()>0) - throw this; + + throw this; } /* ------------------------------------------------------------ */ @@ -164,11 +195,7 @@ public class MultiException extends Exception { StringBuilder str = new StringBuilder(); str.append(MultiException.class.getSimpleName()); - if((nested == null) || (nested.size()<=0)) { - str.append("[]"); - } else { - str.append(nested); - } + str.append(Arrays.asList(getSuppressed())); return str.toString(); } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java index 091d4373fcc..5b07906b023 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/MultiExceptionTest.java @@ -37,6 +37,7 @@ public class MultiExceptionTest me.ifExceptionThrow(); me.ifExceptionThrowMulti(); me.ifExceptionThrowRuntime(); + me.ifExceptionThrowSuppressed(); } @Test @@ -77,6 +78,16 @@ public class MultiExceptionTest { assertTrue(e.getCause()==io); } + + try + { + me.ifExceptionThrowSuppressed(); + assertTrue(false); + } + catch(IOException e) + { + assertTrue(e==io); + } me = new MultiException(); RuntimeException run = new RuntimeException("one"); @@ -98,7 +109,7 @@ public class MultiExceptionTest { MultiException me = new MultiException(); IOException io = new IOException("one"); - RuntimeException run = new RuntimeException("one"); + RuntimeException run = new RuntimeException("two"); me.add(io); me.add(run); @@ -134,6 +145,19 @@ public class MultiExceptionTest assertTrue(e.getCause()==me); } + + try + { + me.ifExceptionThrowSuppressed(); + assertTrue(false); + } + catch(IOException e) + { + assertTrue(e==io); + assertEquals(1,e.getSuppressed().length); + assertTrue(e.getSuppressed()[0]==run); + } + me = new MultiException(); me.add(run); me.add(run);