Author: mhermanto
Date: Wed Apr 27 07:39:49 2011
New Revision: 1097023
URL: http://svn.apache.org/viewvc?rev=1097023&view=rev
Log:
JsCompiler.compile() no longers throws JsException ... in favor of JsResponse
built-in http return code and extensible error messages.
http://codereview.appspot.com/4430068/
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponse.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/JsCompiler.java
shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java?rev=1097023&r1=1097022&r2=1097023&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java
Wed Apr 27 07:39:49 2011
@@ -36,9 +36,10 @@ public class CompilationProcessor implem
* TODO: Re-add support for externs here if they're ever used.
* TODO: Convert JsCompiler to take JsResponseBuilder directly rather than
Iterable<JsContent>
*/
- public boolean process(JsRequest request, JsResponseBuilder builder)
- throws JsException {
- Iterable<JsContent> jsContents = builder.build().getAllJsContent();
+ public boolean process(JsRequest request, JsResponseBuilder builder) throws
JsException {
+ JsResponse responseSoFar = builder.build();
+
+ Iterable<JsContent> jsContents = responseSoFar.getAllJsContent();
for (JsContent jsc : jsContents) {
FeatureBundle bundle = jsc.getFeatureBundle();
if (bundle != null) {
@@ -47,8 +48,11 @@ public class CompilationProcessor implem
}
JsResponse result = compiler.compile(request.getJsUri(), jsContents,
- builder.build().getExterns());
+ responseSoFar.getExterns());
+
builder.clearJs().appendAllJs(result.getAllJsContent());
+ builder.setStatusCode(result.getStatusCode());
+ builder.addErrors(result.getErrors());
return true;
}
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponse.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponse.java?rev=1097023&r1=1097022&r2=1097023&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponse.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsResponse.java
Wed Apr 27 07:39:49 2011
@@ -32,6 +32,7 @@ public class JsResponse {
private final int statusCode;
private final boolean proxyCacheable;
private String codeString;
+ private String errorString;
JsResponse(List<JsContent> jsCode, int statusCode, int cacheTtlSecs,
boolean proxyCacheable, List<String> errors, String externs) {
@@ -100,7 +101,21 @@ public class JsResponse {
public List<String> getErrors() {
return errors;
}
-
+
+ /**
+ * Returns a string of all error messages associated with this response.
+ */
+ public String toErrorString() {
+ if (errorString == null) {
+ StringBuilder sb = new StringBuilder();
+ for (String error : getErrors()) {
+ sb.append(error);
+ }
+ errorString = sb.toString();
+ }
+ return errorString;
+ }
+
/**
* Returns a string of generated externs.
*/
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/JsCompiler.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/JsCompiler.java?rev=1097023&r1=1097022&r2=1097023&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/JsCompiler.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/js/JsCompiler.java
Wed Apr 27 07:39:49 2011
@@ -21,12 +21,9 @@ import com.google.inject.ImplementedBy;
import org.apache.shindig.gadgets.features.FeatureRegistry.FeatureBundle;
import org.apache.shindig.gadgets.js.JsContent;
-import org.apache.shindig.gadgets.js.JsException;
import org.apache.shindig.gadgets.js.JsResponse;
import org.apache.shindig.gadgets.uri.JsUriManager.JsUri;
-import java.util.List;
-
/**
* Compiler to pre-process each feature independently and compile a
* concatenation of pre-processed data.
@@ -49,6 +46,5 @@ public interface JsCompiler {
* @param externs The externs.
* @return A compilation result object.
*/
- JsResponse compile(JsUri jsUri, Iterable<JsContent> content, String externs)
- throws JsException;
+ JsResponse compile(JsUri jsUri, Iterable<JsContent> content, String externs);
}
Modified:
shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java?rev=1097023&r1=1097022&r2=1097023&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
Wed Apr 27 07:39:49 2011
@@ -113,8 +113,7 @@ public class ClosureJsCompiler implement
return new Compiler(errorManager);
}
- public JsResponse compile(JsUri jsUri, Iterable<JsContent> content, String
externs)
- throws JsException {
+ public JsResponse compile(JsUri jsUri, Iterable<JsContent> content, String
externs) {
JsResponse exportResponse = defaultCompiler.compile(jsUri, content,
externs);
content = exportResponse.getAllJsContent();
@@ -147,13 +146,12 @@ public class ClosureJsCompiler implement
if (actualCompiler.hasErrors()) {
ImmutableList.Builder<String> errors = ImmutableList.builder();
for (JSError error : actualCompiler.getErrors()) {
- errors.add(error.toString());
+ errors.add(error.toString());
}
- builder.setStatusCode(HttpResponse.SC_NOT_FOUND)
- .addErrors(errors.build()).build();
- JsResponse errorResult = builder.build();
- cache.addElement(cacheKey, errorResult);
- return errorResult;
+ return cacheAndReturnErrorResult(
+ builder, cacheKey,
+ HttpResponse.SC_NOT_FOUND,
+ errors.build());
}
String compiled = actualCompiler.toSource();
@@ -161,8 +159,15 @@ public class ClosureJsCompiler implement
// Emit code correlated w/ original source.
// This operation is equivalent in final code to bundled-output,
// but is less efficient and should perhaps only be used in code
profiling.
- SourceMappings sm = processSourceMap(result, allContent);
- builder.appendAllJs(sm.mapCompiled(compiled));
+ SourceMappings sm = processSourceMap(result, allContent);
+ if (sm != null) {
+ builder.appendAllJs(sm.mapCompiled(compiled));
+ } else {
+ return cacheAndReturnErrorResult(
+ builder, cacheKey,
+ HttpResponse.SC_INTERNAL_SERVER_ERROR,
+ Lists.newArrayList("Parse error for source map"));
+ }
} else {
builder.appendJs(compiled, "[compiled]");
}
@@ -178,6 +183,16 @@ public class ClosureJsCompiler implement
return lastResult;
}
+ private JsResponse cacheAndReturnErrorResult(
+ JsResponseBuilder builder, String cacheKey,
+ int statusCode, List<String> messages) {
+ builder.setStatusCode(statusCode);
+ builder.addErrors(messages);
+ JsResponse result = builder.build();
+ cache.addElement(cacheKey, result);
+ return result;
+ }
+
// Override this method to return "true" for cases where individual chunks of
// compiled JS should be emitted as JsContent objects, each correlating
output JS
// with the original source file from which they came.
@@ -265,15 +280,15 @@ public class ClosureJsCompiler implement
* @param allInputs All inputs supplied to the compiler, in JsContent form.
* @return SourceMappings object correlating compiled with originating input.
*/
- private SourceMappings processSourceMap(Result result, List<JsContent>
allInputs)
- throws JsException {
+ private SourceMappings processSourceMap(Result result, List<JsContent>
allInputs) {
StringBuilder sb = new StringBuilder();
try {
result.sourceMap.appendTo(sb, "done");
return SourceMappings.parseV1(sb.toString(), allInputs);
- } catch (Exception e) {
- throw new JsException(HttpResponse.SC_INTERNAL_SERVER_ERROR,
- "Parse error for source map: " + e);
+ } catch (IOException e) {
+ return null;
+ } catch (JSONException e) {
+ return null;
}
}
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java?rev=1097023&r1=1097022&r2=1097023&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java
Wed Apr 27 07:39:49 2011
@@ -86,21 +86,6 @@ public class CompilationProcessorTest {
assertFalse(outIterator.hasNext());
}
- @Test(expected = JsException.class)
- public void compilerExceptionThrows() throws Exception {
- JsUri jsUri = control.createMock(JsUri.class);
- JsResponseBuilder builder =
- new JsResponseBuilder().setCacheTtlSecs(1234).setStatusCode(200)
- .appendJs("content1:", "source1").appendJs("content2", "source2");
- JsRequest request = control.createMock(JsRequest.class);
- expect(request.getJsUri()).andReturn(jsUri);
- List<String> emptyList = ImmutableList.of();
- expect(compiler.compile(same(jsUri),
eq(builder.build().getAllJsContent()), eq("")))
- .andThrow(new JsException(400, "foo"));
- control.replay();
- processor.process(request, builder);
- }
-
private FeatureBundle mockBundle(String... externs) {
FeatureBundle result = createMock(FeatureBundle.class);
expect(result.getApis(ApiDirective.Type.JS, false)).andReturn(
