Revision: 8805
Author: [email protected]
Date: Thu Sep 16 12:42:20 2010
Log: JUnit ought to just serialize exceptions.

Back in the day, Exceptions couldn't be serialized. So we had to do hacky things to support JUnit, since we needed to transmit exceptions across the wire.

Better days are here, and Exceptions are now Serializable. So I'm pulling out some very archaic code. The tangible benefit is: certain exceptions that could never be deserialized before, should just work now that I was able to remove the wacky custom deserialization logic.

http://gwt-code-reviews.appspot.com/870802/show

http://code.google.com/p/google-web-toolkit/source/detail?r=8805

Deleted:
 /trunk/user/src/com/google/gwt/junit/client/impl/StackTraceWrapper.java
Modified:
 /trunk/tools/api-checker/config/gwt20_21userApi.conf
 /trunk/user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java
 /trunk/user/src/com/google/gwt/junit/client/impl/JUnitResult.java
 /trunk/user/src/com/google/gwt/junit/server/JUnitHostImpl.java
/trunk/user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/GWTTestCase.java /trunk/user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/impl/GWTRunner.java

=======================================
--- /trunk/user/src/com/google/gwt/junit/client/impl/StackTraceWrapper.java Wed Dec 27 14:55:26 2006
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * Copyright 2006 Google Inc.
- *
- * Licensed 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 com.google.gwt.junit.client.impl;
-
-import com.google.gwt.user.client.rpc.IsSerializable;
-
-/**
- * A helper class for converting a generic {...@link StackTraceElement} into an
- * Object that can be serialized for RPC.
- */
-public final class StackTraceWrapper implements IsSerializable {
-
-  /**
-   * Creates a {...@link StackTraceWrapper} array around an existing
-   * {...@link StackTraceElement} array.
-   *
-   * @param stackTrace the {...@link StackTraceElement} array to wrap.
-   */
-  public static StackTraceWrapper[] wrapStackTrace(
-      StackTraceElement[] stackTrace) {
-    int len = stackTrace.length;
-    StackTraceWrapper[] result = new StackTraceWrapper[len];
-    for (int i = 0; i < len; ++i) {
-      result[i] = new StackTraceWrapper(stackTrace[i]);
-    }
-    return result;
-  }
-
-  /**
-   * Corresponds to {...@link StackTraceElement#getClassName()}.
-   */
-  public String className;
-
-  /**
-   * Corresponds to {...@link StackTraceElement#getFileName()}.
-   */
-  public String fileName;
-
-  /**
-   * Corresponds to {...@link StackTraceElement#getLineNumber()}.
-   */
-  public int lineNumber;
-
-  /**
-   * Corresponds to {...@link StackTraceElement#getMethodName()}.
-   */
-  public String methodName;
-
-  /**
-   * Creates an empty {...@link StackTraceWrapper}.
-   */
-  public StackTraceWrapper() {
-  }
-
-  /**
-   * Creates a {...@link StackTraceWrapper} around an existing
-   * {...@link StackTraceElement}.
-   *
-   * @param ste the {...@link StackTraceElement} to wrap.
-   */
-  public StackTraceWrapper(StackTraceElement ste) {
-    className = ste.getClassName();
-    fileName = ste.getFileName();
-    lineNumber = ste.getLineNumber();
-    methodName = ste.getMethodName();
-  }
-}
=======================================
--- /trunk/tools/api-checker/config/gwt20_21userApi.conf Fri Sep 10 05:47:32 2010 +++ /trunk/tools/api-checker/config/gwt20_21userApi.conf Thu Sep 16 12:42:20 2010
@@ -87,10 +87,12 @@

 ##############################################
 #excluded packages
-excludedPackages com.google.gwt.core.client.impl\
+excludedPackages com.google.gwt.benchmarks.client.impl\
+:com.google.gwt.core.client.impl\
 :com.google.gwt.i18n.client.impl\
 :com.google.gwt.i18n.client.impl.cldr\
 :com.google.gwt.i18n.client.impl.plurals\
+:com.google.gwt.junit.client.impl\
 :com.google.gwt.user.client.impl\
 :com.google.gwt.user.client.rpc.impl\
 :com.google.gwt.user.client.ui.impl\
=======================================
--- /trunk/user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java Tue Nov 24 12:23:12 2009 +++ /trunk/user/src/com/google/gwt/junit/client/impl/ExceptionWrapper.java Thu Sep 16 12:42:20 2010
@@ -18,50 +18,62 @@
 import java.io.Serializable;

 /**
- * A helper class for converting a generic {...@link Throwable} into an Object that
- * can be serialized for RPC.
+ * Wraps a {...@link Throwable}, and explicitly serializes cause and stack trace.
  */
-public final class ExceptionWrapper implements Serializable {
+final class ExceptionWrapper implements Serializable {

   /**
-   * Corresponds to {...@link Throwable#getCause()}.
+   * Stand-in for the transient {...@link Throwable#getCause()} in GWT JRE.
    */
-  public ExceptionWrapper cause;
+  ExceptionWrapper causeWrapper;

   /**
-   * Corresponds to {...@link Throwable#getMessage()}.
+   * The wrapped exception.
    */
-  public String message;
+  Throwable exception;

   /**
-   * Corresponds to {...@link Throwable#getStackTrace()}.
+ * Stand-in for the transient {...@link Throwable#getStackTrace()} in GWT JRE.
    */
-  public StackTraceWrapper[] stackTrace;
+  StackTraceElement[] stackTrace;

   /**
-   * The run-time type of the exception.
+ * If true, the exception's inner stack trace and cause have been initialized.
+   * Defaults to false immediate after deserialization.
    */
-  public String typeName;
+  private transient boolean isExceptionInitialized;

   /**
-   * Creates an empty {...@link ExceptionWrapper}.
+ * Creates an {...@link ExceptionWrapper} around an existing {...@link Throwable}.
+   *
+   * @param exception the {...@link Throwable} to wrap.
    */
-  public ExceptionWrapper() {
+  public ExceptionWrapper(Throwable exception) {
+    this.exception = exception;
+    this.stackTrace = exception.getStackTrace();
+    Throwable cause = exception.getCause();
+    if (cause != null) {
+      this.causeWrapper = new ExceptionWrapper(cause);
+    }
+    this.isExceptionInitialized = true;
   }

   /**
- * Creates an {...@link ExceptionWrapper} around an existing {...@link Throwable}.
-   *
-   * @param e the {...@link Throwable} to wrap.
+   * Deserialization constructor.
    */
-  public ExceptionWrapper(Throwable e) {
-    typeName = e.getClass().getName();
-    message = e.getMessage();
-    stackTrace = StackTraceWrapper.wrapStackTrace(e.getStackTrace());
-    Throwable ecause = e.getCause();
-    if (ecause != null) {
-      cause = new ExceptionWrapper(ecause);
-    }
+  ExceptionWrapper() {
+    this.isExceptionInitialized = false;
   }

-}
+  public Throwable getException() {
+    if (!isExceptionInitialized) {
+      exception.setStackTrace(stackTrace);
+      if (causeWrapper != null) {
+        exception.initCause(causeWrapper.getException());
+      }
+      isExceptionInitialized = true;
+    }
+    return exception;
+  }
+}
+
=======================================
--- /trunk/user/src/com/google/gwt/junit/client/impl/JUnitResult.java Tue Nov 24 12:23:12 2009 +++ /trunk/user/src/com/google/gwt/junit/client/impl/JUnitResult.java Thu Sep 16 12:42:20 2010
@@ -33,9 +33,14 @@
   // Computed at the server, via HTTP header.
   private transient String agent;

-  // Deserialized from exceptionWrapper on the server-side
-  private transient Throwable exception;
-
+  /**
+   * If non-null, check points that were encountered during the run.
+   */
+  private String[] checkPoints;
+
+  /**
+   * If non-null, an exception that occurred during the run.
+   */
   private ExceptionWrapper exceptionWrapper;

   // Computed at the server, via HTTP header.
@@ -45,12 +50,12 @@
     return agent;
   }

-  public Throwable getException() {
-    return exception;
+  public String[] getCheckPoints() {
+    return checkPoints;
   }

-  public ExceptionWrapper getExceptionWrapper() {
-    return exceptionWrapper;
+  public Throwable getException() {
+ return (exceptionWrapper == null) ? null : exceptionWrapper.getException();
   }

   public String getHost() {
@@ -61,12 +66,12 @@
     this.agent = agent;
   }

-  public void setException(Throwable exception) {
-    this.exception = exception;
+  public void setCheckPoints(String[] checkPoints) {
+    this.checkPoints = checkPoints;
   }

-  public void setExceptionWrapper(ExceptionWrapper exceptionWrapper) {
-    this.exceptionWrapper = exceptionWrapper;
+  public void setException(Throwable exception) {
+    this.exceptionWrapper = new ExceptionWrapper(exception);
   }

   public void setHost(String host) {
=======================================
--- /trunk/user/src/com/google/gwt/junit/server/JUnitHostImpl.java Mon Aug 2 20:55:43 2010 +++ /trunk/user/src/com/google/gwt/junit/server/JUnitHostImpl.java Thu Sep 16 12:42:20 2010
@@ -22,10 +22,8 @@
 import com.google.gwt.junit.JUnitShell;
 import com.google.gwt.junit.JUnitMessageQueue.ClientInfoExt;
 import com.google.gwt.junit.client.TimeoutException;
-import com.google.gwt.junit.client.impl.ExceptionWrapper;
 import com.google.gwt.junit.client.impl.JUnitHost;
 import com.google.gwt.junit.client.impl.JUnitResult;
-import com.google.gwt.junit.client.impl.StackTraceWrapper;
 import com.google.gwt.user.client.rpc.InvocationException;
 import com.google.gwt.user.server.rpc.HybridServiceServlet;
 import com.google.gwt.user.server.rpc.RPCServletUtils;
@@ -34,8 +32,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -94,17 +90,6 @@
     }
     return sHost;
   }
-
-  /**
-   * Simple helper method to set inaccessible fields via reflection.
-   */
-  private static <T> void setField(Class<T> cls, String fieldName, T obj,
-      Object value) throws SecurityException, NoSuchFieldException,
-      IllegalArgumentException, IllegalAccessException {
-    Field fld = cls.getDeclaredField(fieldName);
-    fld.setAccessible(true);
-    fld.set(obj, value);
-  }

private Map<StrongName, Map<SymbolName, String>> symbolMaps = new HashMap<StrongName, Map<SymbolName, String>>();

@@ -128,8 +113,8 @@
       ClientInfo clientInfo) throws TimeoutException {
     for (JUnitResult result : results.values()) {
       initResult(getThreadLocalRequest(), result);
-      ExceptionWrapper ew = result.getExceptionWrapper();
-      result.setException(deserialize(ew));
+      resymbolize(result.getException());
+ addCheckPointElements(result.getException(), result.getCheckPoints());
     }
     JUnitMessageQueue host = getHost();
     ClientInfoExt clientInfoExt = createClientInfo(clientInfo,
@@ -153,6 +138,29 @@
       super.service(request, response);
     }
   }
+
+  /**
+ * Creates fake stack trace elements for any checkpoints passed. The elements + * are copied in reverse order, so that the last check point is at the top of
+   * the trace.
+   */
+ private void addCheckPointElements(Throwable exception, String[] checkPoints) {
+    if (checkPoints == null) {
+      return;
+    }
+    StackTraceElement[] stackTrace = exception.getStackTrace();
+ StackTraceElement[] newStackTrace = new StackTraceElement[checkPoints.length
+        + stackTrace.length];
+    int pos = checkPoints.length - 1;
+    for (String checkPoint : checkPoints) {
+ newStackTrace[pos] = new StackTraceElement("Check", "point", checkPoint,
+          -1);
+      --pos;
+    }
+    System.arraycopy(stackTrace, 0, newStackTrace, checkPoints.length,
+        stackTrace.length);
+    exception.setStackTrace(newStackTrace);
+  }

   private ClientInfoExt createClientInfo(ClientInfo clientInfo,
       HttpServletRequest request) {
@@ -170,113 +178,6 @@
   private int createSessionId() {
     return uniqueSessionId.getAndIncrement();
   }
-
-  /**
-   * Deserializes an ExceptionWrapper back into a Throwable.
-   */
-  private Throwable deserialize(ExceptionWrapper ew) {
-    if (ew == null) {
-      return null;
-    }
-
-    Throwable ex = null;
-    Throwable cause = deserialize(ew.cause);
-    try {
-      Class<?> exClass = Class.forName(ew.typeName);
-      try {
-        // try ExType(String, Throwable)
-        Constructor<?> ctor = exClass.getDeclaredConstructor(String.class,
-            Throwable.class);
-        ctor.setAccessible(true);
-        ex = (Throwable) ctor.newInstance(ew.message, cause);
-      } catch (Throwable e) {
-        // try ExType(String)
-        try {
- Constructor<?> ctor = exClass.getDeclaredConstructor(String.class);
-          ctor.setAccessible(true);
-          ex = (Throwable) ctor.newInstance(ew.message);
-          ex.initCause(cause);
-        } catch (Throwable e2) {
-          // try ExType(Throwable)
-          try {
- Constructor<?> ctor = exClass.getDeclaredConstructor(Throwable.class);
-            ctor.setAccessible(true);
-            ex = (Throwable) ctor.newInstance(cause);
-            setField(Throwable.class, "detailMessage", ex, ew.message);
-          } catch (Throwable e3) {
-            // try ExType()
-            try {
-              Constructor<?> ctor = exClass.getDeclaredConstructor();
-              ctor.setAccessible(true);
-              ex = (Throwable) ctor.newInstance();
-              ex.initCause(cause);
-              setField(Throwable.class, "detailMessage", ex, ew.message);
-            } catch (Throwable e4) {
-              // we're out of options
-              this.log("Failed to deserialize getException of type '"
-                  + ew.typeName + "'; no available constructor", e4);
-
-              // fall through
-            }
-          }
-        }
-      }
-
-    } catch (Throwable e) {
-      this.log("Failed to deserialize getException of type '" + ew.typeName
-          + "'", e);
-    }
-
-    if (ex == null) {
-      ex = new RuntimeException(ew.typeName + ": " + ew.message, cause);
-    }
-
-    ex.setStackTrace(deserialize(ew.stackTrace));
-    return ex;
-  }
-
-  /**
-   * Deserializes a StackTraceWrapper back into a StackTraceElement.
-   */
-  private StackTraceElement deserialize(StackTraceWrapper stw) {
-    StackTraceElement ste = null;
-
-    Object[] args = resymbolize(stw);
-
-    try {
-      try {
-        // Try the 4-arg ctor (JRE 1.5)
- Constructor<StackTraceElement> ctor = StackTraceElement.class.getDeclaredConstructor(
-            String.class, String.class, String.class, int.class);
-        ctor.setAccessible(true);
-        ste = ctor.newInstance(args);
-      } catch (NoSuchMethodException e) {
- // Okay, see if there's a zero-arg ctor we can use instead (JRE 1.4.2) - Constructor<StackTraceElement> ctor = StackTraceElement.class.getDeclaredConstructor();
-        ctor.setAccessible(true);
-        ste = ctor.newInstance();
-        setField(StackTraceElement.class, "declaringClass", ste, args[0]);
-        setField(StackTraceElement.class, "methodName", ste, args[1]);
-        setField(StackTraceElement.class, "fileName", ste, args[2]);
-        setField(StackTraceElement.class, "lineNumber", ste, args[3]);
-      }
-    } catch (Throwable e) {
-      this.log("Error creating stack trace", e);
-    }
-    return ste;
-  }
-
-  /**
-   * Deserializes a StackTraceWrapper[] back into a StackTraceElement[].
-   */
-  private StackTraceElement[] deserialize(StackTraceWrapper[] stackTrace) {
-    int len = stackTrace.length;
-    StackTraceElement[] result = new StackTraceElement[len];
-    for (int i = 0; i < len; ++i) {
-      result[i] = deserialize(stackTrace[i]);
-    }
-    return result;
-  }

   /**
    * Returns a client description for the current request.
@@ -334,30 +235,33 @@
   }

   /**
-   * @return {className, methodName, fileName, lineNumber}
+   * Resymbolizes a trace from obfuscated symbols to Java names.
    */
-  private Object[] resymbolize(StackTraceWrapper stw) {
-    Object[] toReturn;
+  private void resymbolize(Throwable exception) {
+    if (exception == null) {
+      return;
+    }
+    StackTraceElement[] stackTrace = exception.getStackTrace();
     StrongName strongName = new StrongName(getPermutationStrongName());
     Map<SymbolName, String> map = loadSymbolMap(strongName);
-    String symbolData = map == null ? null : map.get(new SymbolName(
-        stw.methodName));
-
-    if (symbolData != null) {
-      // jsniIdent, className, memberName, sourceUri, sourceLine
-      String[] parts = symbolData.split(",");
-      assert parts.length == 5 : "Expected 5, have " + parts.length;
-
-      JsniRef ref = JsniRef.parse(parts[0].substring(0,
-          parts[0].lastIndexOf(')') + 1));
-      toReturn = new Object[]{
-          ref.className(), ref.memberName(), stw.fileName, stw.lineNumber};
-
-    } else {
-      // Use the raw data from the client
-      toReturn = new Object[]{
-          stw.className, stw.methodName, stw.fileName, stw.lineNumber};
-    }
-    return toReturn;
+    if (map == null) {
+      return;
+    }
+    for (int i = 0; i < stackTrace.length; ++i) {
+      StackTraceElement ste = stackTrace[i];
+      String symbolData = map.get(new SymbolName(ste.getMethodName()));
+      if (symbolData != null) {
+        // jsniIdent, className, memberName, sourceUri, sourceLine
+        String[] parts = symbolData.split(",");
+        assert parts.length == 5 : "Expected 5, have " + parts.length;
+
+        JsniRef ref = JsniRef.parse(parts[0].substring(0,
+            parts[0].lastIndexOf(')') + 1));
+        stackTrace[i] = new StackTraceElement(ref.className(),
+            ref.memberName(), ste.getFileName(), ste.getLineNumber());
+      }
+    }
+    exception.setStackTrace(stackTrace);
+    resymbolize(exception.getCause());
   }
 }
=======================================
--- /trunk/user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/GWTTestCase.java Thu Feb 18 11:23:28 2010 +++ /trunk/user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/GWTTestCase.java Thu Sep 16 12:42:20 2010
@@ -17,7 +17,6 @@

 import com.google.gwt.core.client.GWT;
 import com.google.gwt.core.client.GWT.UncaughtExceptionHandler;
-import com.google.gwt.junit.client.impl.ExceptionWrapper;
 import com.google.gwt.junit.client.impl.GWTRunner;
 import com.google.gwt.junit.client.impl.JUnitResult;
 import com.google.gwt.user.client.Timer;
@@ -305,13 +304,11 @@

     JUnitResult myResult = __getOrCreateTestResult();
     if (ex != null) {
-      ExceptionWrapper ew = new ExceptionWrapper(ex);
+      myResult.setException(ex);
       if (checkPoints != null) {
-        for (int i = 0, c = checkPoints.size(); i < c; ++i) {
-          ew.message += "\n" + checkPoints.get(i);
-        }
-      }
-      myResult.setExceptionWrapper(ew);
+ String[] cpArray = checkPoints.toArray(new String[checkPoints.size()]);
+        myResult.setCheckPoints(cpArray);
+      }
     }

     testIsFinished = true;
=======================================
--- /trunk/user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/impl/GWTRunner.java Tue Nov 24 12:23:12 2009 +++ /trunk/user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/impl/GWTRunner.java Thu Sep 16 12:42:20 2010
@@ -245,7 +245,7 @@
     }
     if (result != null && failureMessage != null) {
       RuntimeException ex = new RuntimeException(failureMessage);
-      result.setExceptionWrapper(new ExceptionWrapper(ex));
+      result.setException(ex);
     }
     TestInfo currentTest = getCurrentTest();
     currentResults.put(currentTest, result);
@@ -357,7 +357,7 @@
       RuntimeException ex = new RuntimeException(currentTest
           + ": could not instantiate the requested class", caught);
       JUnitResult result = new JUnitResult();
-      result.setExceptionWrapper(new ExceptionWrapper(ex));
+      result.setException(ex);
       reportResultsAndGetNextMethod(result);
       return;
     }

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to