Author: johnh
Date: Wed Mar  3 19:42:59 2010
New Revision: 918661

URL: http://svn.apache.org/viewvc?rev=918661&view=rev
Log:
Restructures ConcatProxyServlet so that it uses ConcatUriManager to process a
request.

This CL also dissociates ConcatProxyServlet from its dependency on ProxyHandler
as well, favoring direct use of RequestPipeline instead. In doing so, it breaks
the need to use RequestWrapper along with its somewhat indirect rendering logic.
Instead, all output is handled by a ConcatServletOutputStream instance, either
JSON or "direct"/verbatim.

One downstream effect in terms of output is that Exception cases and 404s
continue to result in 400-responses but also yield invalid JSON. This behavior
somewhat existed before but is now consistently held.

As well, small tweaks are made to output, in particular start/end markers only
being rendered for successful verbatim output. Error output is now marked
directly w/ the file causing the error.


Modified:
    
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
    
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java

Modified: 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java?rev=918661&r1=918660&r2=918661&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
 Wed Mar  3 19:42:59 2010
@@ -21,24 +21,26 @@
 import com.google.inject.Inject;
 
 import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.shindig.common.servlet.HttpUtil;
 import org.apache.shindig.common.servlet.InjectedServlet;
 import org.apache.shindig.common.uri.Uri;
+import org.apache.shindig.common.uri.UriBuilder;
 import org.apache.shindig.gadgets.GadgetException;
+import org.apache.shindig.gadgets.http.HttpRequest;
+import org.apache.shindig.gadgets.http.HttpResponse;
+import org.apache.shindig.gadgets.http.RequestPipeline;
+import org.apache.shindig.gadgets.uri.ConcatUriManager;
+import org.apache.shindig.gadgets.uri.UriCommon.Param;
 
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
-import java.util.Locale;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 
 import javax.servlet.ServletOutputStream;
-import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
-import javax.servlet.http.HttpServletResponseWrapper;
 
 /**
  * Servlet which concatenates the content of several proxied HTTP responses
@@ -47,16 +49,27 @@
  */
 public class ConcatProxyServlet extends InjectedServlet {
 
-  public static final String JSON_PARAM = "json";
+  public static final String JSON_PARAM = Param.JSON.getKey();
+  private static final Pattern JSON_PARAM_PATTERN = Pattern.compile("^\\w*$");
+  
+  // TODO: parameterize these.
+  static final Integer LONG_LIVED_REFRESH = (365 * 24 * 60 * 60);  // 1 year
+  static final Integer DEFAULT_REFRESH = (60 * 60);                // 1 hour
 
   private static final Logger logger
       = Logger.getLogger(ConcatProxyServlet.class.getName());
 
-  private transient ProxyHandler proxyHandler;
+  private RequestPipeline requestPipeline;
+  private ConcatUriManager concatUriManager;
 
   @Inject
-  public void setProxyHandler(ProxyHandler proxyHandler) {
-    this.proxyHandler = proxyHandler;
+  public void setRequestPipeline(RequestPipeline requestPipeline) {
+    this.requestPipeline = requestPipeline;
+  }
+  
+  @Inject
+  public void setConcatUriManager(ConcatUriManager concatUriManager) {
+    this.concatUriManager = concatUriManager;
   }
 
   @SuppressWarnings("boxing")
@@ -67,360 +80,186 @@
       response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
       return;
     }
-    // Avoid response splitting vulnerability
-    String ct = request.getParameter(ProxyBase.REWRITE_MIME_TYPE_PARAM);
-    if(ct != null && ct.indexOf('\r')<0 && ct.indexOf('\n')<0) {
-      response.setHeader("Content-Type",
-          request.getParameter(ProxyBase.REWRITE_MIME_TYPE_PARAM));
-    }
 
-    boolean ignoreCache = proxyHandler.getIgnoreCache(request);
-    if (!ignoreCache && request.getParameter(ProxyBase.REFRESH_PARAM) != null) 
{
-      try {
-        HttpUtil.setCachingHeaders(response, Integer.parseInt(request
-            .getParameter(ProxyBase.REFRESH_PARAM)));
-      } catch (NumberFormatException e) {
-        // Ignore malform ttl:
-        HttpUtil.setNoCache(response);        
-      }
-    } else {
-      HttpUtil.setNoCache(response);
-    }
-    
-    response.setHeader("Content-Disposition", "attachment;filename=p.txt");
+    Uri uri = new UriBuilder(request).toUri();
+    ConcatUriManager.ConcatUri concatUri = concatUriManager.process(uri);
 
-    // Check for json concat
-    String jsonVar = request.getParameter(JSON_PARAM);
-    if (jsonVar != null && !jsonVar.matches("^\\w*$")) {
-      response.getOutputStream().println(
-          formatHttpError(HttpServletResponse.SC_BAD_REQUEST,
-              "Bad json variable name " + jsonVar));
-      response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+    ConcatUriManager.Type concatType = concatUri.getType();
+    try {
+      if (concatType == null) {
+        throw new GadgetException(GadgetException.Code.MISSING_PARAMETER, 
"Missing type",
+            HttpResponse.SC_BAD_REQUEST);
+      }
+      HttpUtil.setCachingHeaders(response,
+          concatUri.translateStatusRefresh(LONG_LIVED_REFRESH, 
DEFAULT_REFRESH), false);
+    } catch (GadgetException gex) {
+      response.sendError(HttpResponse.SC_BAD_REQUEST, formatError(gex, uri));
       return;
     }
     
-    ResponseWrapper wrapper = new ResponseWrapper(response, jsonVar);
+    // Throughout this class, wherever output is generated it's done as a UTF8 
String.
+    // As such, we affirmatively state that UTF8 is being returned here.
+    response.setHeader("Content-Type", concatType.getMimeType() + "; 
charset=UTF8");
+    response.setHeader("Content-Disposition", "attachment;filename=p.txt");
 
-    for (int i = 1; i < Integer.MAX_VALUE; i++) {
-      String url = request.getParameter(Integer.toString(i));
-      if (url == null) {
-        break;
+    // Check for json concat and set output stream.
+    ConcatOutputStream cos = null;
+    
+    String jsonVar = concatUri.getSplitParam();
+    if (jsonVar != null) {
+      // JSON-concat mode.
+      if (JSON_PARAM_PATTERN.matcher(jsonVar).matches()) {
+        cos = new JsonConcatOutputStream(response.getOutputStream(), jsonVar);
+      } else {
+        response.getOutputStream().println(
+            formatHttpError(HttpServletResponse.SC_BAD_REQUEST,
+                "Bad json variable name " + jsonVar, null));
+        response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+        return;
       }
-      try {
-        try {
-          // Validate the url:
-          Uri uri = Uri.parse(url);
-          url = uri.toString();
-        } catch (Uri.UriException e) {
-          // ServletOutputStream.println support only ascii, so lets use write:
-          response.getOutputStream().write(
-              formatHttpError(HttpServletResponse.SC_BAD_REQUEST, 
e.getMessage())
-                .getBytes("UTF-8"));
-          continue;
-        }
-        wrapper.processUrl(url);
-        proxyHandler.doFetch(new RequestWrapper(request, url), wrapper);
+    } else {
+      // Standard concat output mode.
+      cos = new VerbatimConcatOutputStream(response.getOutputStream());
+    }
 
-        if (wrapper.getStatus() != HttpServletResponse.SC_OK) {
-          response.getOutputStream().println(
-              formatHttpError(wrapper.getStatus(), wrapper.getErrorMessage()));
-        }
-        
+    for (Uri resourceUri : concatUri.getBatch()) {
+      try {
+        HttpRequest httpReq = concatUri.makeHttpRequest(resourceUri);
+        HttpResponse httpResp = requestPipeline.execute(httpReq);
+        cos.output(resourceUri, httpResp);
       } catch (GadgetException ge) {
-        if (ge.getCode() != GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT) {
-          wrapper.done();
-          outputError(ge, url, response);
+        response.setStatus(HttpResponse.SC_BAD_REQUEST);
+        if (cos.outputError(resourceUri, ge)) {
+          // True returned from outputError indicates a terminal error.
           return;
-        } else {
-          response.getOutputStream().println("/* ---- End " + url + " 404 ---- 
*/");
         }
       }
     }
-    wrapper.done();
-    response.setStatus(200);
+    
+    cos.close();
+    response.setStatus(HttpResponse.SC_OK);
   }
 
-  private static String formatHttpError(int status, String errorMessage) {
+  private static String formatHttpError(int status, String errorMessage, Uri 
uri) {
     StringBuilder err = new StringBuilder();
     err.append("/* ---- Error ");
     err.append(status);
-    if (errorMessage != null) {
+    if (!StringUtils.isEmpty(errorMessage)) {
       err.append(", ");
       err.append(errorMessage);
     }
+    if (uri != null) {
+      err.append(" (").append(uri.toString()).append(")");
+    }
 
     err.append(" ---- */");
     return err.toString();
   }
 
-  private static void outputError(GadgetException excep, String url, 
HttpServletResponse resp)
+  private static String formatError(GadgetException excep, Uri uri)
       throws IOException {
     StringBuilder err = new StringBuilder();
     err.append(excep.getCode().toString());
     err.append(" concat(");
-    err.append(url);
+    err.append(uri.toString());
     err.append(") ");
     err.append(excep.getMessage());
 
     // Log the errors here for now. We might want different severity levels
     // for different error codes.
     logger.log(Level.INFO, "Concat proxy request failed", err);
-    resp.sendError(HttpServletResponse.SC_BAD_REQUEST, err.toString());
-  }
-
-  /**
-   * Simple request wrapper to make repeated calls to ProxyHandler
-   */
-  private static class RequestWrapper extends HttpServletRequestWrapper {
-
-    private final String url;
-
-    protected RequestWrapper(HttpServletRequest httpServletRequest, String 
url) {
-      super(httpServletRequest);
-      this.url = url;
-    }
-
-    @Override
-    public String getParameter(String paramName) {
-      if (ProxyBase.URL_PARAM.equals(paramName)) {
-        return url;
-      }
-      return super.getParameter(paramName);
-    }
+    return err.toString();
   }
-
-  /**
-   * Wrap the response to prevent writing through of the status code and to 
hold a reference to the
-   * stream across multiple proxied parts
-   * Handles json concatenation by using the EscapedServletOutputStream class
-   * to escape the data
-   */
-  private static class ResponseWrapper extends HttpServletResponseWrapper {
-
-    private ServletOutputStream outputStream;
-    private EscapedServletOutputStream jsonStream;
-
-    private int errorCode = SC_OK;
-    private String errorMessage;
-    /** Specify hash key for json concat **/ 
-    private String jsonVar = null;
-    private String url = null;
-
-    protected ResponseWrapper(HttpServletResponse httpServletResponse,
-        String jsonVar) throws IOException {
-      super(httpServletResponse);
-      if (jsonVar != null && jsonVar.length() > 0) {
-        this.jsonVar = jsonVar;
-        super.getOutputStream().println(jsonVar + "={");
-      }
-    }
-
+  
+  private static abstract class ConcatOutputStream extends ServletOutputStream 
{
+    private final ServletOutputStream wrapped;
     
-    @Override
-    public ServletOutputStream getOutputStream() throws IOException {
-      // For errors, we don't want the content returned by the remote
-      // server;  we'll just include an HTTP error code to avoid creating
-      // syntactically invalid output overall.
-      if (errorCode != SC_OK) {
-        closeStream();
-        outputStream = new NullServletOutputStream();
-      }
-      
-      if (outputStream == null) {
-        outputStream = super.getOutputStream();
-      }
-      
-      return outputStream;
+    protected ConcatOutputStream(ServletOutputStream wrapped) {
+      this.wrapped = wrapped;
     }
-
-    /**
-     * Restart a new file to concat
-     * Close previous file, and add start comment if not json concat
-     */
-    public void processUrl(String fileUrl) throws IOException {
-      closeStream();
-      errorCode = SC_OK;
-      this.url = fileUrl;
-      if (jsonVar == null) {
-        super.getOutputStream().println("/* ---- Start " + url + " ---- */");
+    
+    protected abstract void outputJs(Uri uri, String data) throws IOException;
+    
+    public void output(Uri uri, HttpResponse resp) throws IOException {
+      if (resp.getHttpStatusCode() != HttpServletResponse.SC_OK) {
+        println(formatHttpError(resp.getHttpStatusCode(), 
resp.getResponseAsString(), uri));
       } else {
-        // Create escaping stream (make sure url variable is defined)
-        jsonStream = new EscapedServletOutputStream();
-        outputStream = jsonStream;
+        outputJs(uri, resp.getResponseAsString());
       }
     }
-
-    /**
-     * Add close of json hash
-     */
-    public void done() throws IOException {
-      closeStream();
-      if (jsonVar != null) {
-        // Close json concat main variable
-        super.getOutputStream().println("};");
-      }
-    }
-
-    private void closeStream() throws IOException {
-      if (jsonVar == null && outputStream != null) {
-        outputStream.println("/* ---- End " + url + " ---- */");
-      } else if (jsonStream != null) {
-        byte[] data = jsonStream.getBytes();
-        ServletOutputStream mainStream = super.getOutputStream();
-        mainStream.print("\"" + url + "\":\"");
-        mainStream.write(data);
-        mainStream.println("\",");
-      }      
-      outputStream = null;
-      jsonStream = null;
-    }
     
-    public int getStatus() {
-      return errorCode;
-    }
-
-    public String getErrorMessage() {
-      return errorMessage;
-    }
-
-    @Override
-    public void addCookie(Cookie cookie) {
-    }
-
-    // Suppress headers
-    @Override
-    public void setDateHeader(String s, long l) {
-    }
-
-    @Override
-    public void addDateHeader(String s, long l) {
-    }
-
-    @Override
-    public void setHeader(String s, String s1) {
-    }
-
-    @Override
-    public void addHeader(String s, String s1) {
-    }
-
-    @Override
-    public void setIntHeader(String s, int i) {
-    }
-
-    @Override
-    public void addIntHeader(String s, int i) {
+    public boolean outputError(Uri uri, GadgetException e)
+        throws IOException {
+      print(formatError(e, uri));
+      return e.getHttpStatusCode() == HttpResponse.SC_INTERNAL_SERVER_ERROR;
     }
 
     @Override
-    public void sendError(int i, String s) throws IOException {
-      errorCode = i;
-      errorMessage = s;
-    }
-
-    @Override
-    public void sendError(int i) throws IOException {
-      errorCode = i;
-    }
-
-    @Override
-    public void sendRedirect(String s) throws IOException {
-    }
-
-    @Override
-    public void setStatus(int i) {
-    }
-
-    @Override
-    public void setStatus(int i, String s) {
-    }
-
-    @Override
-    public void setContentLength(int i) {
-    }
-
-    @Override
-    public void setContentType(String s) {
+    public void write(int b) throws IOException {
+      wrapped.write(b);
     }
 
     @Override
-    public void flushBuffer() throws IOException {
+    public void write(byte b[], int off, int len) throws IOException {
+      wrapped.write(b, off, len);
     }
 
     @Override
-    public void reset() {
+    public void write(byte b[]) throws IOException {
+      wrapped.write(b);
     }
-
+    
     @Override
-    public void resetBuffer() {
+    public void close() throws IOException {
+      wrapped.close();
     }
-
+    
     @Override
-    public void setLocale(Locale locale) {
+    public void print(String data) throws IOException {
+      write(data.getBytes("UTF8"));
     }
-
+    
     @Override
-    public void setCharacterEncoding(String s) {
+    public void println(String data) throws IOException {
+      print(data);
+      write("\r\n".getBytes("UTF8"));
     }
   }
-
-  /**
-   * Small ServletOutputStream class, overriding just enough to ensure
-   * there's no output.
-   */
-  private static class NullServletOutputStream extends ServletOutputStream {
-
-    protected NullServletOutputStream() {
-    }
-
-    @Override
-    public void write(int b) throws IOException {
-    }
-
-    @Override
-    public void write(byte b[], int off, int len) throws IOException {
+  
+  private static class VerbatimConcatOutputStream extends ConcatOutputStream { 
   
+    public VerbatimConcatOutputStream(ServletOutputStream wrapped) {
+      super(wrapped);
     }
 
     @Override
-    public void write(byte b[]) throws IOException {
+    protected void outputJs(Uri uri, String data) throws IOException {
+      println("/* ---- Start " + uri.toString() + " ---- */");
+      print(data);
+      println("/* ---- End " + uri.toString() + " ---- */");
     }
   }
   
-  /**
-   * Override Servlet output stream to support json escaping of the stream data
-   * Use getBytes to get the escaped data. 
-   */
-  private static class EscapedServletOutputStream extends ServletOutputStream {
-
-    private final ByteArrayOutputStream tempStream;
-    protected EscapedServletOutputStream() {
-      tempStream = new ByteArrayOutputStream();
-    }
-    
-    public byte[] getBytes() throws IOException {
-      try {
-        return 
StringEscapeUtils.escapeJavaScript(tempStream.toString("UTF8")).getBytes();
-      } catch (UnsupportedEncodingException e) {
-        // Need to return IOException since that what ServletOutputStream 
constructor do.
-        throw new IOException("Unsuported encoding in data");
-      }
+  private static class JsonConcatOutputStream extends ConcatOutputStream {    
+    public JsonConcatOutputStream(ServletOutputStream wrapped, String tok) 
throws IOException {
+      super(wrapped);
+      this.println(tok + "={");
     }
 
     @Override
-    public void write(int b) throws IOException {
-      tempStream.write(b);
+    protected void outputJs(Uri uri, String data) throws IOException {
+      print("\"");
+      print(uri.toString());
+      print("\":\"");
+      print(StringEscapeUtils.escapeJavaScript(data));
+      println("\",");
     }
-
-    @Override
-    public void write(byte b[], int off, int len) throws IOException {
-      tempStream.write(b, off, len);
-    }
-
+    
     @Override
-    public void write(byte b[]) throws IOException {
-      tempStream.write(b);
+    public void close() throws IOException {
+      println("};");
+      super.close();
     }
+    
   }
-
 }
 

Modified: 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java?rev=918661&r1=918660&r2=918661&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ConcatProxyServletTest.java
 Wed Mar  3 19:42:59 2010
@@ -20,14 +20,24 @@
 
 import static org.easymock.EasyMock.expect;
 
+import java.util.List;
+
 import org.apache.shindig.common.uri.Uri;
+import org.apache.shindig.common.uri.UriBuilder;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.http.HttpRequest;
 import org.apache.shindig.gadgets.http.HttpResponse;
 import org.apache.shindig.gadgets.http.HttpResponseBuilder;
+import org.apache.shindig.gadgets.uri.ConcatUriManager;
+import org.apache.shindig.gadgets.uri.UriStatus;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+import java.util.Map;
+
 public class ConcatProxyServletTest extends ServletTestFixture {
   private static final String REQUEST_DOMAIN = "example.org";
 
@@ -44,20 +54,22 @@
       "var v2 = { \\\"a-b\\\": 1 , c: \\\"hello!,\\\" };";
   private static final String SCRT3_ESCAPED = "var v3 = \\\"world\\\";";
 
-  private final ProxyHandler proxyHandler =
-      new ProxyHandler(pipeline, lockedDomainService, null);
   private final ConcatProxyServlet servlet = new ConcatProxyServlet();
+  private TestConcatUriManager uriManager;
   
   @Before
   public void setUp() throws Exception {
-    servlet.setProxyHandler(proxyHandler);
+    servlet.setRequestPipeline(pipeline);
+    uriManager = new TestConcatUriManager();
+    servlet.setConcatUriManager(uriManager);
+    
     expect(request.getHeader("Host")).andReturn(REQUEST_DOMAIN).anyTimes();
     expect(lockedDomainService.isSafeForOpenProxy(REQUEST_DOMAIN))
         .andReturn(true).anyTimes();
 
-    expectGetAndReturnData(URL1,SCRT1);
-    expectGetAndReturnData(URL2,SCRT2);
-    expectGetAndReturnData(URL3,SCRT3);
+    expectGetAndReturnData(URL1, SCRT1);
+    expectGetAndReturnData(URL2, SCRT2);
+    expectGetAndReturnData(URL3, SCRT3);
   }
 
   private void expectGetAndReturnData(Uri url, String data) throws Exception {
@@ -79,14 +91,7 @@
   }
 
   private String addErrComment(String url, int code) {
-    String res = "/* ---- Start " + url + " ---- */\r\n"
-        + "/* ---- Error " + code + " ---- */\r\n";
-    return res;
-  }
-
-  private String addErrComment(String url, int code, String msg) {
-    String res = "/* ---- Error " + code + ", " + msg + " ---- */";
-    return res;
+    return "/* ---- Error " + code + " (" + url + ") ---- */\r\n";
   }
 
   /**
@@ -106,12 +111,9 @@
    * @param uris - list of uris to concat
    * @throws Exception
    */
-  private void runConcat(String result, Uri... uris) throws Exception {
-    for (int i = 0 ; i < uris.length ; i++) {
-      
expect(request.getParameter(Integer.toString(i+1))).andReturn(uris[i].toString()).once();
-    }
-    
expect(request.getParameter(Integer.toString(uris.length+1))).andReturn(null).once();
-    replay();
+  private void runConcat(String result, String tok, Uri... uris) throws 
Exception {
+    expectRequestWithUris(Lists.newArrayList(uris), tok);
+    
     // Run the servlet
     servlet.doGet(request, recorder);
     verify();
@@ -122,14 +124,14 @@
   @Test
   public void testSimpleConcat() throws Exception {
     String results = addComment(SCRT1, URL1.toString()) + 
addComment(SCRT2,URL2.toString());
-    runConcat(results, URL1,URL2);
+    runConcat(results, null, URL1, URL2);
   }
 
   @Test
   public void testThreeConcat() throws Exception {
     String results = addComment(SCRT1, URL1.toString()) + 
addComment(SCRT2,URL2.toString())
-        + addComment(SCRT3,URL3.toString());
-    runConcat(results, URL1, URL2, URL3);
+        + addComment(SCRT3, URL3.toString());
+    runConcat(results, null, URL1, URL2, URL3);
   }
 
   @Test
@@ -140,17 +142,16 @@
     expect(pipeline.execute(req)).andThrow(
         new GadgetException(GadgetException.Code.HTML_PARSE_ERROR)).anyTimes();
 
-    String results = addComment(SCRT1, URL1.toString())
-        + "/* ---- Start http://example.org/4.js ---- */\r\n"
-        + "HTML_PARSE_ERROR concat(http://example.org/4.js) null";
-
-    
expect(request.getParameter(Integer.toString(1))).andReturn(URL1.toString()).once();
-    
expect(request.getParameter(Integer.toString(2))).andReturn(URL4.toString()).once();
-    replay();
+    expectRequestWithUris(Lists.newArrayList(URL1, URL4));
+    
     // Run the servlet
     servlet.doGet(request, recorder);
     verify();
+
+    String results = addComment(SCRT1, URL1.toString())
+        + "HTML_PARSE_ERROR concat(http://example.org/4.js) null";
     assertEquals(results, recorder.getResponseAsString());
+    
     assertEquals(400, recorder.getHttpStatusCode());
   }
 
@@ -161,9 +162,8 @@
     HttpResponse resp = new 
HttpResponseBuilder().setHttpStatusCode(404).create();
     expect(pipeline.execute(req)).andReturn(resp).anyTimes();
     
-    expect(request.getParameter("1")).andReturn(URL1.toString()).once();
-    expect(request.getParameter("2")).andReturn(url).once();
-    replay();
+    expectRequestWithUris(Lists.newArrayList(URL1, Uri.parse(url)));
+    
     servlet.doGet(request, recorder);
     verify();
     
@@ -173,48 +173,27 @@
   }
 
   @Test
-  public void testConcatBadUrl() throws Exception {
-    String url = "http://\u03C0 1";
-    expect(request.getParameter("1")).andReturn(URL1.toString()).once();
-    expect(request.getParameter("2")).andReturn(url).once();
-    replay();
-    servlet.doGet(request, recorder);
-    verify();
-
-    // Note that the results is a bit out of order. 
-    String results = addComment(SCRT1 + addErrComment(url, 400,
-        "java.lang.IllegalArgumentException: " +
-        "java.net.URISyntaxException: Illegal character in authority at index 
7: " + url),
-        URL1.toString());
-    assertEquals(results, recorder.getResponseAsString());
-    assertEquals(200, recorder.getHttpStatusCode());
-  }
-
-  @Test
   public void testAsJsonConcat() throws Exception {
-    expect(request.getParameter("json")).andReturn("_js").once();
     String results = "_js={\r\n"
         + addVar(URL1.toString(), SCRT1_ESCAPED)
         + addVar(URL2.toString(), SCRT2_ESCAPED)
         + "};\r\n";
-    runConcat(results, URL1, URL2);
+    runConcat(results, "_js", URL1, URL2);
   }
 
   @Test
   public void testThreeAsJsonConcat() throws Exception {
-    expect(request.getParameter("json")).andReturn("testJs").once();
-    String results = "testJs={\r\n"
+    String results = "_js={\r\n"
         + addVar(URL1.toString(), SCRT1_ESCAPED)
         + addVar(URL2.toString(), SCRT2_ESCAPED)
         + addVar(URL3.toString(), SCRT3_ESCAPED)
         + "};\r\n";
-    runConcat(results, URL1, URL2, URL3);
+    runConcat(results, "_js", URL1, URL2, URL3);
   }
   
   @Test
   public void testBadJsonVarConcat() throws Exception {
-    expect(request.getParameter("json")).andReturn("bad code;").once();
-    replay();
+    expectRequestWithUris(Lists.<Uri>newArrayList(), "bad code;");
     servlet.doGet(request, recorder);
     verify();
     String results = "/* ---- Error 400, Bad json variable name bad code; ---- 
*/\r\n";
@@ -230,13 +209,11 @@
     HttpResponse resp = new 
HttpResponseBuilder().setHttpStatusCode(404).create();
     expect(pipeline.execute(req)).andReturn(resp).anyTimes();
 
-    expect(request.getParameter("json")).andReturn("_js").once();
     String results = "_js={\r\n"
         + addVar(URL1.toString(), SCRT1_ESCAPED)
-        + addVar(URL4.toString(),"")
-        + "/* ---- Error 404 ---- */\r\n"
+        + "/* ---- Error 404 (http://example.org/4.js) ---- */\r\n"
         + "};\r\n";
-    runConcat(results, URL1, URL4);
+    runConcat(results, "_js", URL1, URL4);
   }
   
   @Test
@@ -247,13 +224,14 @@
     expect(pipeline.execute(req)).andThrow(
         new 
GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT)).anyTimes();
 
-    expect(request.getParameter("json")).andReturn("_js").once();
+    expectRequestWithUris(Lists.newArrayList(URL1, URL4), "_js");
+    servlet.doGet(request, recorder);
+    verify();
     String results = "_js={\r\n"
-        + addVar(URL1.toString(), SCRT1_ESCAPED)
-        + "/* ---- End http://example.org/4.js 404 ---- */\r\n"
-        + addVar(URL4.toString(),"") 
-        + "};\r\n";
-    runConcat(results, URL1, URL4);
+      + addVar(URL1.toString(), SCRT1_ESCAPED)
+      + "FAILED_TO_RETRIEVE_CONTENT concat(http://example.org/4.js) null";
+    assertEquals(results, recorder.getResponseAsString());
+    assertEquals(400, recorder.getHttpStatusCode());
   }
 
   @Test
@@ -264,21 +242,58 @@
     expect(pipeline.execute(req)).andThrow(
         new GadgetException(GadgetException.Code.HTML_PARSE_ERROR)).anyTimes();
 
-    expect(request.getParameter("json")).andReturn("_js").once();
     String results = "_js={\r\n"
         + addVar(URL1.toString(), SCRT1_ESCAPED)
-        + addVar(URL4.toString(),"")
-        + "};\r\n"
         + "HTML_PARSE_ERROR concat(http://example.org/4.js) null";
 
-    
expect(request.getParameter(Integer.toString(1))).andReturn(URL1.toString()).once();
-    
expect(request.getParameter(Integer.toString(2))).andReturn(URL4.toString()).once();
-    replay();
+    expectRequestWithUris(Lists.newArrayList(URL1, URL4), "_js");
+    
     // Run the servlet
     servlet.doGet(request, recorder);
     verify();
     assertEquals(results, recorder.getResponseAsString());
     assertEquals(400, recorder.getHttpStatusCode());
   }
+    
+  private void expectRequestWithUris(List<Uri> uris) {
+    expectRequestWithUris(uris, null);
+  }
+  
+  private void expectRequestWithUris(List<Uri> uris, String tok) {
+    expect(request.getScheme()).andReturn("http").anyTimes();
+    expect(request.getServerPort()).andReturn(80).anyTimes();
+    expect(request.getServerName()).andReturn("example.com").anyTimes();
+    expect(request.getRequestURI()).andReturn("/path").anyTimes();
+    expect(request.getQueryString()).andReturn("").anyTimes();
+    replay();
+    
+    Uri uri = new UriBuilder(request).toUri();
+    uriManager.expect(uri, uris, tok);
+  }
+
+  private static class TestConcatUriManager implements ConcatUriManager {
+    private final Map<Uri, ConcatUri> uriMap;
+    
+    private TestConcatUriManager() {
+      this.uriMap = Maps.newHashMap();
+    }
+    
+    public List<ConcatData> make(
+        List<ConcatUri> resourceUris, boolean isAdjacent) {
+      // Not used by ConcatProxyServlet
+      throw new UnsupportedOperationException();
+    }
 
+    public ConcatUri process(Uri uri) {
+      return uriMap.get(uri);
+    }
+    
+    private void expect(Uri orig, UriStatus status, Type type, List<Uri> uris, 
String json) {
+      uriMap.put(orig, new ConcatUri(status, uris, json, type, null));
+    }
+    
+    private void expect(Uri orig, List<Uri> uris, String tok) {
+      expect(orig, UriStatus.VALID_UNVERSIONED, Type.JS, uris, tok);
+    }
+  }
 }


Reply via email to