Some Small logging cleanup - Use server error reporting in RF - Some documentation fixes - Create inherits module for projects using logging classes but not enabling loggin Review at http://gwt-code-reviews.appspot.com/875802 Review by: amitmanjhi@google.com git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8849 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml b/user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml new file mode 100644 index 0000000..6caf995 --- /dev/null +++ b/user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml
@@ -0,0 +1,23 @@ +<!-- --> +<!-- Copyright 2010 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 --> +<!-- 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. License for the specific language governing permissions and --> +<!-- limitations under the License. --> + + +<!-- Leaving out the inheritance of gwt.logging.Logging is enough to + disable logging, but some projects need the classes to be available to + avoid compile errors, even if they are disabling logging. Inheriting + this module will work for those projects. --> +<module> + <inherits name="com.google.gwt.logging.Logging"/> + <set-property name="gwt.logging.enabled" value="FALSE"/> +</module> \ No newline at end of file
diff --git a/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java b/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java index 61ded7d..0ddfefa 100644 --- a/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java +++ b/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java
@@ -39,7 +39,7 @@ /** * Logs a Log Record which has been serialized using GWT RPC on the server. - * Returns either an error message, or null if logging is successful. + * @return either an error message, or null if logging is successful. */ public final String logOnServer(SerializableLogRecord slr) { String strongName = getPermutationStrongName(); @@ -48,7 +48,7 @@ slr, strongName, deobfuscator, loggerNameOverride); } catch (RemoteLoggingException e) { logger.log(Level.SEVERE, "Remote logging failed", e); - return "Remote logging failed"; + return "Remote logging failed, check stack trace for details."; } return null; }
diff --git a/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml b/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml index ca52699..0c0dfc5 100644 --- a/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml +++ b/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml
@@ -19,8 +19,7 @@ <inherits name='com.google.gwt.core.Core'/> <inherits name='com.google.gwt.editor.Editor'/> <inherits name='com.google.gwt.http.HTTP'/> - <inherits name='com.google.gwt.logging.Logging'/> - <set-property name="gwt.logging.enabled" value="FALSE"/> + <inherits name='com.google.gwt.logging.LoggingDisabled'/> <source path="client"/> <source path="shared"/>
diff --git a/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java b/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java index 13464d7..15c18e3 100644 --- a/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java +++ b/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java
@@ -38,17 +38,6 @@ LoggingRequest getLoggingRequest(); } - private class LoggingReceiver extends Receiver<Long> { - @Override - public void onSuccess(Long response) { - if (response > 0) { - wireLogger.finest("Remote logging successful"); - } else { - wireLogger.severe("Remote logging failed"); - } - } - } - private LoggingRequestProvider requestProvider; /** @@ -76,12 +65,10 @@ new SerializableLogRecord(record); String json = JsonLogRecordClientUtil.serializableLogRecordAsJson(slr); requestProvider.getLoggingRequest().logMessage(json).fire( - new Receiver<String>() { + new Receiver<Void>() { @Override - public void onSuccess(String response) { - if (!response.isEmpty()) { - wireLogger.severe("Remote Logging failed on server: " + response); - } + public void onSuccess(Void response) { + // Do nothing } }); }
diff --git a/user/src/com/google/gwt/requestfactory/server/Logging.java b/user/src/com/google/gwt/requestfactory/server/Logging.java index 603064b..c72bb7f 100644 --- a/user/src/com/google/gwt/requestfactory/server/Logging.java +++ b/user/src/com/google/gwt/requestfactory/server/Logging.java
@@ -21,9 +21,6 @@ import com.google.gwt.logging.server.StackTraceDeobfuscator; import com.google.gwt.user.client.rpc.RpcRequestBuilder; -import java.util.logging.Level; -import java.util.logging.Logger; - /** * Server side object that handles log messages sent by * {@link RequestFactoryLogHandler}. @@ -36,24 +33,15 @@ private static StackTraceDeobfuscator deobfuscator = new StackTraceDeobfuscator(""); - private static Logger logger = Logger.getLogger(Logging.class.getName()); - - public static String logMessage(String serializedLogRecordJson) { + public static void logMessage(String serializedLogRecordJson) throws + RemoteLoggingException { // if the header does not exist, we pass null, which is handled gracefully // by the deobfuscation code. String strongName = RequestFactoryServlet.getThreadLocalRequest().getHeader( RpcRequestBuilder.STRONG_NAME_HEADER); - try { - RemoteLoggingServiceUtil.logOnServer(serializedLogRecordJson, - strongName, deobfuscator, null); - } catch (RemoteLoggingException e) { - // TODO(unnurg): Change this to use server failure reporting when it is - // submitted. - logger.log(Level.SEVERE, "Remote logging failed", e); - return "Remote logging failed"; - } - return ""; + RemoteLoggingServiceUtil.logOnServer(serializedLogRecordJson, + strongName, deobfuscator, null); } /**
diff --git a/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java b/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java index 49cb8e1..5aaf921 100644 --- a/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java +++ b/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java
@@ -28,9 +28,7 @@ // TODO(unnurg): Pass a SerializableLogRecord here rather than it's // serialized string. /** - * Log a message on the server. Will return empty string if there is no error - * or an error message if there is a problem. + * Log a message on the server. */ - Request<String> logMessage(String serializedLogRecordString); - + Request<Void> logMessage(String serializedLogRecordString); }
diff --git a/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java b/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java index ef2f6fb..8b9a681 100644 --- a/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java +++ b/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java
@@ -15,13 +15,8 @@ */ package com.google.gwt.requestfactory.client; -import com.google.gwt.requestfactory.shared.Receiver; import com.google.gwt.requestfactory.shared.Request; -import com.google.gwt.requestfactory.shared.ServerFailure; import com.google.gwt.requestfactory.shared.SimpleFooProxy; -import com.google.gwt.requestfactory.shared.Violation; - -import java.util.Set; /** * Tests that {@code RequestFactoryServlet} when using a custom @@ -35,36 +30,29 @@ } @Override - public void testServerFailure() { + public void testServerFailureCheckedException() { delayTestFinish(5000); - SimpleFooProxy rayFoo = req.create(SimpleFooProxy.class); final Request<SimpleFooProxy> persistRay = req.simpleFooRequest().persistAndReturnSelf( rayFoo); rayFoo = persistRay.edit(rayFoo); // 42 is the crash causing magic number rayFoo.setPleaseCrash(42); - - persistRay.fire(new Receiver<SimpleFooProxy>() { - @Override - public void onFailure(ServerFailure error) { - assertEquals("THIS EXCEPTION IS EXPECTED BY A TEST", error.getMessage()); - assertEquals("java.lang.UnsupportedOperationException", - error.getExceptionType()); - assertFalse(error.getStackTraceString().length() == 0); - finishTestAndReset(); - } - - @Override - public void onViolation(Set<Violation> errors) { - fail("Failure expected but onViolation() was called"); - } - - @Override - public void onSuccess(SimpleFooProxy response) { - fail("Failure expected but onSuccess() was called"); - } - }); + persistRay.fire(new FooReciever(rayFoo, persistRay, + "java.lang.UnsupportedOperationException")); + } + + @Override + public void testServerFailureRuntimeException() { + delayTestFinish(5000); + SimpleFooProxy rayFoo = req.create(SimpleFooProxy.class); + final Request<SimpleFooProxy> persistRay = req.simpleFooRequest().persistAndReturnSelf( + rayFoo); + rayFoo = persistRay.edit(rayFoo); + // 43 is the crash causing magic number + rayFoo.setPleaseCrash(43); + persistRay.fire(new FooReciever(rayFoo, persistRay, + "java.lang.Exception")); } }
diff --git a/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java b/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java index c90fce3..b6ec204 100644 --- a/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java +++ b/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
@@ -1020,48 +1020,76 @@ }); } - public void testServerFailure() { - delayTestFinish(5000); + + class FooReciever extends Receiver<SimpleFooProxy> { + private SimpleFooProxy mutableFoo; + private Request<SimpleFooProxy> persistRequest; + private String expectedException; + + public FooReciever(SimpleFooProxy mutableFoo, + Request<SimpleFooProxy> persistRequest, String exception) { + this.mutableFoo = mutableFoo; + this.persistRequest = persistRequest; + this.expectedException = exception; + } - SimpleFooProxy newFoo = req.create(SimpleFooProxy.class); - final Request<SimpleFooProxy> persistRequest = req.simpleFooRequest().persistAndReturnSelf( - newFoo); - - final SimpleFooProxy mutableFoo = persistRequest.edit(newFoo); - mutableFoo.setPleaseCrash(42); // 42 is the crash causing magic number - - persistRequest.fire(new Receiver<SimpleFooProxy>() { - @Override - public void onFailure(ServerFailure error) { + @Override + public void onFailure(ServerFailure error) { + assertEquals(expectedException, error.getExceptionType()); + if (expectedException.length() > 0) { + assertFalse(error.getStackTraceString().length() == 0); + assertEquals("THIS EXCEPTION IS EXPECTED BY A TEST", + error.getMessage()); + } else { + assertEquals("", error.getStackTraceString()); assertEquals("Server Error: THIS EXCEPTION IS EXPECTED BY A TEST", error.getMessage()); - assertEquals("", error.getExceptionType()); - assertEquals("", error.getStackTraceString()); - - // Now show that we can fix the error and try again with the same - // request - - mutableFoo.setPleaseCrash(24); // Only 42 crashes - persistRequest.fire(new Receiver<SimpleFooProxy>() { - @Override - public void onSuccess(SimpleFooProxy response) { - finishTestAndReset(); - } - }); } - @Override - public void onSuccess(SimpleFooProxy response) { - fail("Failure expected but onSuccess() was called"); - } + // Now show that we can fix the error and try again with the same + // request - @Override - public void onViolation(Set<Violation> errors) { - fail("Failure expected but onViolation() was called"); - } - }); + mutableFoo.setPleaseCrash(24); // Only 42 and 43 crash + persistRequest.fire(new Receiver<SimpleFooProxy>() { + @Override + public void onSuccess(SimpleFooProxy response) { + finishTestAndReset(); + } + }); + } + + @Override + public void onSuccess(SimpleFooProxy response) { + fail("Failure expected but onSuccess() was called"); + } + + @Override + public void onViolation(Set<Violation> errors) { + fail("Failure expected but onViolation() was called"); + } } - + + public void testServerFailureCheckedException() { + SimpleFooProxy newFoo = req.create(SimpleFooProxy.class); + final Request<SimpleFooProxy> persistRequest = + req.simpleFooRequest().persistAndReturnSelf(newFoo); + final SimpleFooProxy mutableFoo = persistRequest.edit(newFoo); + // 43 is the crash causing magic number for a checked exception + mutableFoo.setPleaseCrash(43); + persistRequest.fire(new FooReciever(mutableFoo, persistRequest, "")); + } + + public void testServerFailureRuntimeException() { + delayTestFinish(5000); + SimpleFooProxy newFoo = req.create(SimpleFooProxy.class); + final Request<SimpleFooProxy> persistRequest = + req.simpleFooRequest().persistAndReturnSelf(newFoo); + final SimpleFooProxy mutableFoo = persistRequest.edit(newFoo); + // 42 is the crash causing magic number for a runtime exception + mutableFoo.setPleaseCrash(42); + persistRequest.fire(new FooReciever(mutableFoo, persistRequest, "")); + } + public void testStableId() { delayTestFinish(5000);
diff --git a/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java b/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java index 54ef3fe..bc253a5 100644 --- a/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java +++ b/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
@@ -437,11 +437,14 @@ this.otherBoolField = otherBoolField; } - public void setPleaseCrash(Integer crashIf42) { - if (crashIf42 == 42) { + public void setPleaseCrash(Integer crashIf42or43) throws Exception { + if (crashIf42or43 == 42) { throw new UnsupportedOperationException("THIS EXCEPTION IS EXPECTED BY A TEST"); } - pleaseCrash = crashIf42; + if (crashIf42or43 == 43) { + throw new Exception("THIS EXCEPTION IS EXPECTED BY A TEST"); + } + pleaseCrash = crashIf42or43; } public void setPassword(String password) {