Merge pull request #15 from overleaf/handler-refactor

Put API handlers into their own namespace

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java

@@ -72,7 +72,7 @@ public class CandidateSnapshot {
     }
 
     public JsonElement getJsonRepresentation(String postbackKey) {
-        String projectURL = Util.getPostbackURL() + projectName;
+        String projectURL = Util.getPostbackURL() + "api/" + projectName;
         JsonObject jsonObject = new JsonObject();
         jsonObject.addProperty("latestVerId", currentVersion);
         jsonObject.add("files", getFilesAsJson(projectURL, postbackKey));

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/FileHandler.java

@@ -0,0 +1,56 @@
+package uk.ac.ic.wlgitbridge.server;
+
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.handler.ResourceHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import uk.ac.ic.wlgitbridge.bridge.BridgeAPI;
+import uk.ac.ic.wlgitbridge.snapshot.push.exception.InvalidPostbackKeyException;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Serve files referenced by the snapshot that we send to the Overleaf API.
+ *
+ * Requests must include the postback key.
+ */
+public class FileHandler extends ResourceHandler {
+    private static final Logger LOG = LoggerFactory.getLogger(FileHandler.class);
+
+    private final BridgeAPI writeLatexDataSource;
+    private final Pattern DOC_KEY_PATTERN = Pattern.compile("^/(\\w+)/.+$");
+
+    public FileHandler(BridgeAPI writeLatexDataSource) {
+        this.writeLatexDataSource = writeLatexDataSource;
+    }
+
+    @Override
+    public void handle(String target,
+                       Request baseRequest,
+                       HttpServletRequest request,
+                       HttpServletResponse response) throws IOException, ServletException {
+        if (!"GET".equals(baseRequest.getMethod())) return;
+        LOG.info("GET <- {}", baseRequest.getRequestURI());
+
+        Matcher docKeyMatcher = DOC_KEY_PATTERN.matcher(target);
+        if (!docKeyMatcher.matches()) return;
+        String docKey = docKeyMatcher.group(1);
+
+        String apiKey = request.getParameter("key");
+        if (apiKey == null) return;
+
+        try {
+            writeLatexDataSource.checkPostbackKey(docKey, apiKey);
+        } catch (InvalidPostbackKeyException e) {
+            LOG.warn("INVALID POST BACK KEY: docKey={} apiKey={}", docKey, apiKey);
+            return;
+        }
+
+        super.handle(target, baseRequest, request, response);
+    }
+}

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/FileServlet.java

@@ -1,50 +0,0 @@
-package uk.ac.ic.wlgitbridge.server;
-
-import org.eclipse.jetty.http.HttpURI;
-import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.server.handler.ResourceHandler;
-import org.eclipse.jetty.util.MultiMap;
-import uk.ac.ic.wlgitbridge.bridge.BridgeAPI;
-import uk.ac.ic.wlgitbridge.snapshot.push.exception.InvalidPostbackKeyException;
-import uk.ac.ic.wlgitbridge.util.Log;
-
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
-
-/**
- * Created by Winston on 04/12/14.
- */
-public class FileServlet extends ResourceHandler {
-
-    private final BridgeAPI writeLatexDataSource;
-
-    public FileServlet(BridgeAPI writeLatexDataSource) {
-        this.writeLatexDataSource = writeLatexDataSource;
-    }
-
-    @Override
-    public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
-        String method = baseRequest.getMethod();
-        if (method.equals("GET")) {
-            HttpURI uri = baseRequest.getHttpURI();
-            Log.info(method + " <- " + uri);
-            MultiMap<String> multimap = new MultiMap<String>();
-            uri.decodeQueryTo(multimap);
-            String[] pathSections = uri.getPath().split("/");
-            String key = multimap.getString("key");
-            if (key == null || pathSections.length < 2) {
-                throw new ServletException();
-            }
-            try {
-                writeLatexDataSource.checkPostbackKey(pathSections[1], key);
-            } catch (InvalidPostbackKeyException e) {
-                e.printStackTrace();
-                throw new ServletException();
-            }
-            super.handle(target, baseRequest, request, response);
-        }
-    }
-
-}

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java

@@ -2,8 +2,7 @@ package uk.ac.ic.wlgitbridge.server;
 
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.server.handler.HandlerCollection;
-import org.eclipse.jetty.server.handler.ResourceHandler;
+import org.eclipse.jetty.server.handler.*;
 import org.eclipse.jetty.servlet.FilterHolder;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
@@ -82,15 +81,25 @@ public class GitBridgeServer {
     }
 
     private void configureJettyServer(Config config) throws ServletException, InvalidRootDirectoryPathException {
-        HandlerCollection handlers = new HandlerCollection();
-        handlers.setHandlers(new Handler[] {
-                initResourceHandler(),
-                new PostbackHandler(bridgeAPI),
-                initGitHandler(config)
-        });
+        HandlerCollection handlers = new HandlerList();
+        handlers.addHandler(initApiHandler());
+        handlers.addHandler(initGitHandler(config));
         jettyServer.setHandler(handlers);
     }
 
+    private Handler initApiHandler() {
+        ContextHandler api = new ContextHandler();
+        api.setContextPath("/api");
+
+        HandlerCollection handlers = new HandlerList();
+        handlers.addHandler(initResourceHandler());
+        handlers.addHandler(new PostbackHandler(bridgeAPI));
+        handlers.addHandler(new DefaultHandler());
+
+        api.setHandler(handlers);
+        return api;
+    }
+
     private Handler initGitHandler(Config config) throws ServletException, InvalidRootDirectoryPathException {
         final ServletContextHandler servletContextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
         if (config.isUsingOauth2()) {
@@ -107,9 +116,8 @@ public class GitBridgeServer {
     }
 
     private Handler initResourceHandler() {
-        ResourceHandler resourceHandler = new FileServlet(bridgeAPI);
+        ResourceHandler resourceHandler = new FileHandler(bridgeAPI);
         resourceHandler.setResourceBase(new File(rootGitDirectoryPath, ".wlgb/atts").getAbsolutePath());
         return resourceHandler;
     }
-
 }

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/PostbackHandler.java

@@ -32,7 +32,7 @@ public class PostbackHandler extends AbstractHandler {
             if (request.getMethod().equals("POST") && target.endsWith("postback")) {
                 response.setContentType("application/json");
                 String contents = Util.getContentsOfReader(request.getReader());
-                String[] parts = request.getRequestURI().split("/");
+                String[] parts = target.split("/");
                 if (parts.length < 4) {
                     throw new ServletException();
                 }

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/push/PostbackManager.java

@@ -78,7 +78,12 @@ public class PostbackManager {
 
     public void checkPostbackKey(String projectName, String postbackKey)
             throws InvalidPostbackKeyException {
-        postbackContentsTable.get(projectName).checkPostbackKey(postbackKey);
+        PostbackPromise postbackPromise = postbackContentsTable.get(projectName);
+        if (postbackPromise == null) {
+            throw new InvalidPostbackKeyException(); // project not found; can't check key
+        } else {
+            postbackPromise.checkPostbackKey(postbackKey);
+        }
     }
 
     private String randomString() {

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest.java

@@ -1,5 +1,7 @@
 package uk.ac.ic.wlgitbridge;
 
+import com.ning.http.client.AsyncHttpClient;
+import com.ning.http.client.Response;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.junit.Rule;
 import org.junit.Test;
@@ -18,6 +20,7 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -96,6 +99,9 @@ public class WLGitBridgeIntegrationTest {
             put("invalidState", new SnapshotAPIStateBuilder(getResourceAsStream("/pushSucceedsAfterRemovingInvalidFiles/invalidState/state.json")).build());
             put("validState", new SnapshotAPIStateBuilder(getResourceAsStream("/pushSucceedsAfterRemovingInvalidFiles/validState/state.json")).build());
         }});
+        put("canServePushedFiles", new HashMap<String, SnapshotAPIState>() {{
+            put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/canServePushedFiles/state/state.json")).build());
+        }});
     }};
 
     @Rule
@@ -534,6 +540,59 @@ public class WLGitBridgeIntegrationTest {
         assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/pushSucceedsAfterRemovingInvalidFiles/validState/testproj"), testprojDir.toPath()));
     }
 
+    @Test
+    public void canServePushedFiles() throws IOException, ExecutionException, InterruptedException {
+        //
+        // I don't think we can test this completely without some changes to the mock server, because we have no way
+        // of pausing the test while the push is in progress. Once the push is over, the file isn't actually there for
+        // us to fetch any more. We can however test the access and error conditions, which comprise most of the logic.
+        //
+        int gitBridgePort = 33873;
+        int mockServerPort = 3873;
+
+        MockSnapshotServer server = new MockSnapshotServer(
+                mockServerPort, getResource("/canServePushedFiles").toFile());
+        server.start();
+        server.setState(states.get("canServePushedFiles").get("state"));
+
+        GitBridgeApp wlgb = new GitBridgeApp(new String[] {
+                makeConfigFile(gitBridgePort, mockServerPort)
+        });
+        wlgb.run();
+
+        File dir = folder.newFolder();
+        File testprojDir = cloneRepository("testproj", gitBridgePort, dir);
+        assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canServePushedFiles/state/testproj"), testprojDir.toPath()));
+        runtime.exec("touch push.tex", null, testprojDir).waitFor();
+        runtime.exec("git add -A", null, testprojDir).waitFor();
+        runtime.exec("git commit -m \"push\"", null, testprojDir).waitFor();
+        Process gitPush = runtime.exec("git push", null, testprojDir);
+        int pushExitCode = gitPush.waitFor();
+        assertEquals(0, pushExitCode);
+
+        // With no key, we should get a 404.
+        String url = "http://127.0.0.1:" + gitBridgePort + "/api/testproj/push.tex";
+        Response response = new AsyncHttpClient().prepareGet(url).execute().get();
+        assertEquals(404, response.getStatusCode());
+
+        // With an invalid project and no key, we should get a 404.
+        url = "http://127.0.0.1:" + gitBridgePort + "/api/notavalidproject/push.tex";
+        response = new AsyncHttpClient().prepareGet(url).execute().get();
+        assertEquals(404, response.getStatusCode());
+
+        // With a bad key for a valid project, we should get a 404.
+        url = "http://127.0.0.1:" + gitBridgePort + "/api/testproj/push.tex?key=notavalidkey";
+        response = new AsyncHttpClient().prepareGet(url).execute().get();
+        assertEquals(404, response.getStatusCode());
+
+        // With a bad key for an invalid project, we should get a 404.
+        url = "http://127.0.0.1:" + gitBridgePort + "/api/notavalidproject/push.tex?key=notavalidkey";
+        response = new AsyncHttpClient().prepareGet(url).execute().get();
+        assertEquals(404, response.getStatusCode());
+
+        wlgb.stop();
+    }
+
     private File cloneRepository(String repositoryName, int port, File dir) throws IOException, InterruptedException {
         String repo = "git clone http://127.0.0.1:" + port + "/" + repositoryName + ".git";
         assertEquals(0, runtime.exec(repo, null, dir).waitFor());

services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canServePushedFiles/state/state.json

@@ -0,0 +1,29 @@
+[
+	{
+		"project": "testproj",
+		"getDoc": {
+			"versionID": 1,
+			"createdAt": "2014-11-30T18:40:58.123Z",
+			"email": "test1@example.com",
+			"name": "John+1"
+		},
+		"getSavedVers": [],
+		"getForVers": [
+			{
+				"versionID": 1,
+				"srcs": [
+                    {
+                    	"content": "content\n",
+                    	"path": "main.tex"
+                    }
+	            ],
+				"atts": []
+	        }
+		],
+		"push": "success",
+		"postback": {
+			"type": "success",
+			"versionID": 1
+		}
+	}
+]

services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canServePushedFiles/state/testproj/main.tex

@@ -0,0 +1 @@
+content