From 5040b4d7f0ccb813db198d51dde6792efde6edb2 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 26 Apr 2019 23:33:12 +0200 Subject: [PATCH 01/50] [WLGitBridgeIntegrationTest] ensure that we stop the started servers Signed-off-by: Jakob Ackermann --- .../servermock/server/MockSnapshotServer.java | 8 + .../WLGitBridgeIntegrationTest.java | 162 ++++++++---------- 2 files changed, 77 insertions(+), 93 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/servermock/server/MockSnapshotServer.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/servermock/server/MockSnapshotServer.java index 15204ba44b..b6bebcb44f 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/servermock/server/MockSnapshotServer.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/servermock/server/MockSnapshotServer.java @@ -47,6 +47,14 @@ public class MockSnapshotServer { port = ((NetworkConnector) server.getConnectors()[0]).getLocalPort(); } + public void stop() { + try { + server.stop(); + } catch (Exception e) { + Log.warn("Exception when trying to stop server", e); + } + } + public void setState(SnapshotAPIState state) { responseBuilder.setState(state); } diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index e0f0ddeffb..f1a2e991dc 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -5,6 +5,7 @@ import org.apache.commons.io.IOUtils; import static org.asynchttpclient.Dsl.*; import org.asynchttpclient.*; import org.eclipse.jgit.api.errors.GitAPIException; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -134,6 +135,8 @@ public class WLGitBridgeIntegrationTest { @Rule public TemporaryFolder folder = new TemporaryFolder(); + private MockSnapshotServer server; + private GitBridgeApp wlgb; private File dir; @Before @@ -141,6 +144,12 @@ public class WLGitBridgeIntegrationTest { dir = folder.newFolder(); } + @After + public void tearDown() { + server.stop(); + wlgb.stop(); + } + private void gitConfig(File dir) throws IOException, InterruptedException { assertEquals(0, runtime.exec( "git config user.name TEST", null, dir @@ -221,40 +230,38 @@ public class WLGitBridgeIntegrationTest { @Test public void canCloneARepository() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3857, getResource("/canCloneARepository").toFile()); + server = new MockSnapshotServer(3857, getResource("/canCloneARepository").toFile()); server.start(); server.setState(states.get("canCloneARepository").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33857, 3857) }); wlgb.run(); File testprojDir = gitClone("testproj", 33857, dir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canCloneARepository/state/testproj"), testprojDir.toPath())); } @Test public void canCloneMultipleRepositories() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3858, getResource("/canCloneMultipleRepositories").toFile()); + server = new MockSnapshotServer(3858, getResource("/canCloneMultipleRepositories").toFile()); server.start(); server.setState(states.get("canCloneMultipleRepositories").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33858, 3858) }); wlgb.run(); File testproj1Dir = gitClone("testproj1", 33858, dir); File testproj2Dir = gitClone("testproj2", 33858, dir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canCloneMultipleRepositories/state/testproj1"), testproj1Dir.toPath())); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canCloneMultipleRepositories/state/testproj2"), testproj2Dir.toPath())); } @Test public void canPullAModifiedTexFile() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3859, getResource("/canPullAModifiedTexFile").toFile()); + server = new MockSnapshotServer(3859, getResource("/canPullAModifiedTexFile").toFile()); server.start(); server.setState(states.get("canPullAModifiedTexFile").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33859, 3859) }); wlgb.run(); @@ -262,16 +269,15 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullAModifiedTexFile/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullAModifiedTexFile").get("withModifiedTexFile")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullAModifiedTexFile/withModifiedTexFile/testproj"), testprojDir.toPath())); } @Test public void canPullADeletedTexFile() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3860, getResource("/canPullADeletedTexFile").toFile()); + server = new MockSnapshotServer(3860, getResource("/canPullADeletedTexFile").toFile()); server.start(); server.setState(states.get("canPullADeletedTexFile").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33860, 3860) }); wlgb.run(); @@ -279,16 +285,15 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullADeletedTexFile/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullADeletedTexFile").get("withDeletedTexFile")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullADeletedTexFile/withDeletedTexFile/testproj"), testprojDir.toPath())); } @Test public void canPullAModifiedBinaryFile() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3862, getResource("/canPullAModifiedBinaryFile").toFile()); + server = new MockSnapshotServer(3862, getResource("/canPullAModifiedBinaryFile").toFile()); server.start(); server.setState(states.get("canPullAModifiedBinaryFile").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33862, 3862) }); wlgb.run(); @@ -296,16 +301,15 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullAModifiedBinaryFile/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullAModifiedBinaryFile").get("withModifiedBinaryFile")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullAModifiedBinaryFile/withModifiedBinaryFile/testproj"), testprojDir.toPath())); } @Test public void canPullADeletedBinaryFile() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3863, getResource("/canPullADeletedBinaryFile").toFile()); + server = new MockSnapshotServer(3863, getResource("/canPullADeletedBinaryFile").toFile()); server.start(); server.setState(states.get("canPullADeletedBinaryFile").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33863, 3863) }); wlgb.run(); @@ -313,16 +317,15 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullADeletedBinaryFile/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullADeletedBinaryFile").get("withDeletedBinaryFile")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullADeletedBinaryFile/withDeletedBinaryFile/testproj"), testprojDir.toPath())); } @Test public void canPullADuplicateBinaryFile() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(4001, getResource("/canPullADuplicateBinaryFile").toFile()); + server = new MockSnapshotServer(4001, getResource("/canPullADuplicateBinaryFile").toFile()); server.start(); server.setState(states.get("canPullADuplicateBinaryFile").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(44001, 4001) }); wlgb.run(); @@ -330,30 +333,28 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullADuplicateBinaryFile/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullADuplicateBinaryFile").get("withDuplicateBinaryFile")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullADuplicateBinaryFile/withDuplicateBinaryFile/testproj"), testprojDir.toPath())); } @Test public void canCloneDuplicateBinaryFiles() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(4002, getResource("/canCloneDuplicateBinaryFiles").toFile()); + server = new MockSnapshotServer(4002, getResource("/canCloneDuplicateBinaryFiles").toFile()); server.start(); server.setState(states.get("canCloneDuplicateBinaryFiles").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(44002, 4002) }); wlgb.run(); File testprojDir = gitClone("testproj", 44002, dir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canCloneDuplicateBinaryFiles/state/testproj"), testprojDir.toPath())); } @Test public void canPullUpdatedBinaryFiles() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(4003, getResource("/canPullUpdatedBinaryFiles").toFile()); + server = new MockSnapshotServer(4003, getResource("/canPullUpdatedBinaryFiles").toFile()); server.start(); server.setState(states.get("canPullUpdatedBinaryFiles").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(44003, 4003) }); wlgb.run(); @@ -361,16 +362,15 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullUpdatedBinaryFiles/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullUpdatedBinaryFiles").get("withUpdatedBinaryFiles")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullUpdatedBinaryFiles/withUpdatedBinaryFiles/testproj"), testprojDir.toPath())); } @Test public void canPullAModifiedNestedFile() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3864, getResource("/canPullAModifiedNestedFile").toFile()); + server = new MockSnapshotServer(3864, getResource("/canPullAModifiedNestedFile").toFile()); server.start(); server.setState(states.get("canPullAModifiedNestedFile").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33864, 3864) }); wlgb.run(); @@ -378,16 +378,15 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullAModifiedNestedFile/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullAModifiedNestedFile").get("withModifiedNestedFile")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullAModifiedNestedFile/withModifiedNestedFile/testproj"), testprojDir.toPath())); } @Test public void canPullDeletedNestedFiles() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3865, getResource("/canPullDeletedNestedFiles").toFile()); + server = new MockSnapshotServer(3865, getResource("/canPullDeletedNestedFiles").toFile()); server.start(); server.setState(states.get("canPullDeletedNestedFiles").get("base")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33865, 3865) }); wlgb.run(); @@ -395,15 +394,14 @@ public class WLGitBridgeIntegrationTest { assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullDeletedNestedFiles/base/testproj"), testprojDir.toPath())); server.setState(states.get("canPullDeletedNestedFiles").get("withDeletedNestedFiles")); gitPull(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canPullDeletedNestedFiles/withDeletedNestedFiles/testproj"), testprojDir.toPath())); } @Test public void canPushFilesSuccessfully() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3866, getResource("/canPushFilesSuccessfully").toFile()); + server = new MockSnapshotServer(3866, getResource("/canPushFilesSuccessfully").toFile()); server.start(); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33866, 3866) }); wlgb.run(); @@ -414,8 +412,7 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); gitPush(testprojDir); - wlgb.stop(); - } + } private static final String EXPECTED_OUT_PUSH_OUT_OF_DATE_FIRST = "error: failed to push some refs to 'http://127.0.0.1:33867/testproj.git'\n" + @@ -426,10 +423,10 @@ public class WLGitBridgeIntegrationTest { @Test public void pushFailsOnFirstStageOutOfDate() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3867, getResource("/pushFailsOnFirstStageOutOfDate").toFile()); + server = new MockSnapshotServer(3867, getResource("/pushFailsOnFirstStageOutOfDate").toFile()); server.start(); server.setState(states.get("pushFailsOnFirstStageOutOfDate").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33867, 3867) }); wlgb.run(); @@ -439,7 +436,6 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); Process push = gitPush(testprojDir, 1); - wlgb.stop(); assertEquals(EXPECTED_OUT_PUSH_OUT_OF_DATE_FIRST, Util.fromStream(push.getErrorStream(), 2)); } @@ -452,10 +448,10 @@ public class WLGitBridgeIntegrationTest { @Test public void pushFailsOnSecondStageOutOfDate() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3868, getResource("/pushFailsOnSecondStageOutOfDate").toFile()); + server = new MockSnapshotServer(3868, getResource("/pushFailsOnSecondStageOutOfDate").toFile()); server.start(); server.setState(states.get("pushFailsOnSecondStageOutOfDate").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33868, 3868) }); wlgb.run(); @@ -465,7 +461,6 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); Process push = gitPush(testprojDir, 1); - wlgb.stop(); assertEquals(EXPECTED_OUT_PUSH_OUT_OF_DATE_SECOND, Util.fromStream(push.getErrorStream(), 2)); } @@ -482,10 +477,10 @@ public class WLGitBridgeIntegrationTest { @Test public void pushFailsOnInvalidFiles() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3869, getResource("/pushFailsOnInvalidFiles").toFile()); + server = new MockSnapshotServer(3869, getResource("/pushFailsOnInvalidFiles").toFile()); server.start(); server.setState(states.get("pushFailsOnInvalidFiles").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33869, 3869) }); wlgb.run(); @@ -495,7 +490,6 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); Process push = gitPush(testprojDir, 1); - wlgb.stop(); List actual = Util.linesFromStream(push.getErrorStream(), 2, "[K"); assertEquals(EXPECTED_OUT_PUSH_INVALID_FILES, actual); } @@ -510,10 +504,10 @@ public class WLGitBridgeIntegrationTest { @Test public void pushFailsOnInvalidProject() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3870, getResource("/pushFailsOnInvalidProject").toFile()); + server = new MockSnapshotServer(3870, getResource("/pushFailsOnInvalidProject").toFile()); server.start(); server.setState(states.get("pushFailsOnInvalidProject").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33870, 3870) }); wlgb.run(); @@ -523,7 +517,6 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); Process push = gitPush(testprojDir, 1); - wlgb.stop(); List actual = Util.linesFromStream(push.getErrorStream(), 2, "[K"); assertEquals(EXPECTED_OUT_PUSH_INVALID_PROJECT, actual); } @@ -539,10 +532,10 @@ public class WLGitBridgeIntegrationTest { /* this one prints a stack trace */ @Test public void pushFailsOnUnexpectedError() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3871, getResource("/pushFailsOnUnexpectedError").toFile()); + server = new MockSnapshotServer(3871, getResource("/pushFailsOnUnexpectedError").toFile()); server.start(); server.setState(states.get("pushFailsOnUnexpectedError").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33871, 3871) }); wlgb.run(); @@ -552,7 +545,6 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); Process push = gitPush(testprojDir, 1); - wlgb.stop(); List actual = Util.linesFromStream(push.getErrorStream(), 2, "[K"); assertEquals(EXPECTED_OUT_PUSH_UNEXPECTED_ERROR, actual); } @@ -569,10 +561,10 @@ public class WLGitBridgeIntegrationTest { @Test public void pushSucceedsAfterRemovingInvalidFiles() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3872, getResource("/pushSucceedsAfterRemovingInvalidFiles").toFile()); + server = new MockSnapshotServer(3872, getResource("/pushSucceedsAfterRemovingInvalidFiles").toFile()); server.start(); server.setState(states.get("pushSucceedsAfterRemovingInvalidFiles").get("invalidState")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33872, 3872) }); wlgb.run(); @@ -592,7 +584,6 @@ public class WLGitBridgeIntegrationTest { gitCommit(testprojDir, "remove_invalid_file"); server.setState(states.get("pushSucceedsAfterRemovingInvalidFiles").get("validState")); gitPush(testprojDir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/pushSucceedsAfterRemovingInvalidFiles/validState/testproj"), testprojDir.toPath())); } @@ -606,12 +597,12 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33873; int mockServerPort = 3873; - MockSnapshotServer server = new MockSnapshotServer( + server = new MockSnapshotServer( mockServerPort, getResource("/canServePushedFiles").toFile()); server.start(); server.setState(states.get("canServePushedFiles").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); @@ -643,19 +634,18 @@ public class WLGitBridgeIntegrationTest { response = asyncHttpClient().prepareGet(url).execute().get(); assertEquals(404, response.getStatusCode()); - wlgb.stop(); - } + } @Test public void wlgbCanSwapProjects( ) throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer( + server = new MockSnapshotServer( 3874, getResource("/wlgbCanSwapProjects").toFile() ); server.start(); server.setState(states.get("wlgbCanSwapProjects").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33874, 3874, new SwapJobConfig(1, 0, 0, 250)) }); wlgb.run(); @@ -674,7 +664,6 @@ public class WLGitBridgeIntegrationTest { while (testProj2ServerDir.exists()); assertTrue(testProj1ServerDir.exists()); assertFalse(testProj2ServerDir.exists()); - wlgb.stop(); } private static final List EXPECTED_OUT_PUSH_SUBMODULE = Arrays.asList( @@ -688,10 +677,10 @@ public class WLGitBridgeIntegrationTest { @Test public void pushSubmoduleFailsWithInvalidGitRepo() throws IOException, GitAPIException, InterruptedException { - MockSnapshotServer server = new MockSnapshotServer(3875, getResource("/pushSubmoduleFailsWithInvalidGitRepo").toFile()); + server = new MockSnapshotServer(3875, getResource("/pushSubmoduleFailsWithInvalidGitRepo").toFile()); server.start(); server.setState(states.get("pushSubmoduleFailsWithInvalidGitRepo").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(33875, 3875) }); wlgb.run(); @@ -705,10 +694,8 @@ public class WLGitBridgeIntegrationTest { gitAdd(testprojDir); gitCommit(testprojDir, "push"); Process push = gitPush(testprojDir, 1); - wlgb.stop(); List actual = Util.linesFromStream(push.getErrorStream(), 2, "[K"); assertEquals(EXPECTED_OUT_PUSH_SUBMODULE, actual); - wlgb.stop(); } @Test @@ -718,12 +705,12 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33873; int mockServerPort = 3873; - MockSnapshotServer server = new MockSnapshotServer( + server = new MockSnapshotServer( mockServerPort, getResource("/canServePushedFiles").toFile()); server.start(); server.setState(states.get("canServePushedFiles").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); @@ -741,8 +728,6 @@ public class WLGitBridgeIntegrationTest { response = asyncHttpClient().prepareGet(url).execute().get(); assertEquals(500, response.getStatusCode()); assertEquals("{\"message\":\"HTTP error 500\"}", response.getResponseBody()); - - wlgb.stop(); } @Test @@ -750,16 +735,15 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33883; int mockServerPort = 3883; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAProtectedProjectWithoutAuthentication").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAProtectedProjectWithoutAuthentication").toFile()); server.start(); server.setState(states.get("cannotCloneAProtectedProjectWithoutAuthentication").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); Process gitProcess = runtime.exec("git clone http://127.0.0.1:" + gitBridgePort + "/testproj.git", null, dir); - wlgb.stop(); assertNotEquals(0, gitProcess.waitFor()); } @@ -768,16 +752,15 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33879; int mockServerPort = 3879; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneA4xxProject").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneA4xxProject").toFile()); server.start(); server.setState(states.get("cannotCloneA4xxProject").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); Process gitProcess = runtime.exec("git clone http://127.0.0.1:" + gitBridgePort + "/testproj.git", null, dir); - wlgb.stop(); assertNotEquals(0, gitProcess.waitFor()); } @@ -786,16 +769,15 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33880; int mockServerPort = 3880; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAMissingProject").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAMissingProject").toFile()); server.start(); server.setState(states.get("cannotCloneAMissingProject").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); Process gitProcess = runtime.exec("git clone http://127.0.0.1:" + gitBridgePort + "/testproj.git", null, dir); - wlgb.stop(); assertNotEquals(0, gitProcess.waitFor()); } @@ -803,17 +785,15 @@ public class WLGitBridgeIntegrationTest { public void canMigrateRepository() throws IOException, GitAPIException, InterruptedException { int gitBridgePort = 33881; int mockServerPort = 3881; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/canMigrateRepository").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/canMigrateRepository").toFile()); server.start(); server.setState(states.get("canMigrateRepository").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); File testprojDir = gitClone("testproj", gitBridgePort, dir); File testprojDir2 = gitClone("testproj2", gitBridgePort, dir); - wlgb.stop(); - // Second project content is equal to content of the first assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canMigrateRepository/state/testproj"), testprojDir2.toPath())); } @@ -822,17 +802,15 @@ public class WLGitBridgeIntegrationTest { public void skipMigrationWhenMigratedFromMissing() throws IOException, GitAPIException, InterruptedException { int gitBridgePort = 33882; int mockServerPort = 3882; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/skipMigrationWhenMigratedFromMissing").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/skipMigrationWhenMigratedFromMissing").toFile()); server.start(); server.setState(states.get("skipMigrationWhenMigratedFromMissing").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); // don't clone the source project first File testprojDir2 = gitClone("testproj2", gitBridgePort, dir); - wlgb.stop(); - assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/skipMigrationWhenMigratedFromMissing/state/testproj2"), testprojDir2.toPath())); } @@ -840,15 +818,14 @@ public class WLGitBridgeIntegrationTest { public void canCloneAMigratedRepositoryWithoutChanges() throws IOException, GitAPIException, InterruptedException { int gitBridgePort = 33883; int mockServerPort = 3883; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/canCloneAMigratedRepositoryWithoutChanges").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/canCloneAMigratedRepositoryWithoutChanges").toFile()); server.start(); server.setState(states.get("canCloneAMigratedRepositoryWithoutChanges").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); File testprojDir = gitClone("testproj_no_change", gitBridgePort, dir); - wlgb.stop(); assertTrue(FileUtil.gitDirectoriesAreEqual(getResource("/canCloneAMigratedRepositoryWithoutChanges/state/testproj_no_change"), testprojDir.toPath())); } @@ -856,15 +833,14 @@ public class WLGitBridgeIntegrationTest { public void rejectV1Repository() throws IOException, GitAPIException, InterruptedException { int gitBridgePort = 33884; int mockServerPort = 3884; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/rejectV1Repository").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/rejectV1Repository").toFile()); server.start(); server.setState(states.get("rejectV1Repository").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); wlgb.run(); Process gitProcess = runtime.exec("git clone http://127.0.0.1:" + gitBridgePort + "/1234bbccddff.git", null, dir); - wlgb.stop(); assertNotEquals(0, gitProcess.waitFor()); } From a333aabfa997bb99e1b61c856ad25fdad2061adc Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 26 Apr 2019 23:49:26 +0200 Subject: [PATCH 02/50] [WLGitBridgeIntegrationTest] adjust the urls for the individual tests Signed-off-by: Jakob Ackermann --- .../canMigrateRepository/state/state.json | 4 ++-- .../rejectV1Repository/state/state.json | 2 +- .../skipMigrationWhenMigratedFromMissing/state/state.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canMigrateRepository/state/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canMigrateRepository/state/state.json index 895cd6ea76..b2352496d0 100644 --- a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canMigrateRepository/state/state.json +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canMigrateRepository/state/state.json @@ -24,7 +24,7 @@ ], "atts": [ { - "url": "http://127.0.0.1:3857/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", + "url": "http://127.0.0.1:3881/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", "path": "min_mean_wait_evm_7_eps_150dpi.png" } ] @@ -68,7 +68,7 @@ ], "atts": [ { - "url": "http://127.0.0.1:3857/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", + "url": "http://127.0.0.1:3881/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", "path": "min_mean_wait_evm_7_eps_150dpi.png" } ] diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/rejectV1Repository/state/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/rejectV1Repository/state/state.json index 7486bf3898..f273818712 100644 --- a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/rejectV1Repository/state/state.json +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/rejectV1Repository/state/state.json @@ -31,7 +31,7 @@ ], "atts": [ { - "url": "http://127.0.0.1:3857/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", + "url": "http://127.0.0.1:3884/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", "path": "min_mean_wait_evm_7_eps_150dpi.png" } ] diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/skipMigrationWhenMigratedFromMissing/state/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/skipMigrationWhenMigratedFromMissing/state/state.json index 5e1f5efe80..72fc40fdda 100644 --- a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/skipMigrationWhenMigratedFromMissing/state/state.json +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/skipMigrationWhenMigratedFromMissing/state/state.json @@ -24,7 +24,7 @@ ], "atts": [ { - "url": "http://127.0.0.1:3857/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", + "url": "http://127.0.0.1:3882/state/testproj2/min_mean_wait_evm_7_eps_150dpi.png", "path": "min_mean_wait_evm_7_eps_150dpi.png" } ] From d51f2fecfcc625636ac4ccfbd6d596ee6a5c40f1 Mon Sep 17 00:00:00 2001 From: Simon Detheridge Date: Thu, 16 May 2019 10:03:27 +0100 Subject: [PATCH 03/50] Allow colons in passwords bug: overleaf/issues#1393 --- .../src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index 0a553be2cc..dfcfdf21be 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -131,7 +131,7 @@ public class Oauth2Filter implements Filter { Base64.decodeBase64(st.nextToken()), "UTF-8" ); - String[] split = credentials.split(":"); + String[] split = credentials.split(":",2); if (split.length == 2) { String username = split[0]; String password = split[1]; From c7d8c1c6b544051c7b588f5ca0aa4d7781f3ffea Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 9 May 2019 15:01:29 +0100 Subject: [PATCH 04/50] Run GC on a project at start of swap job. When there is an exception during swap, add project name to log --- .../uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java | 7 +++++++ .../java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStore.java | 2 ++ .../uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java | 6 ++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java index 47a7eede67..72eb44a5b4 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/FSGitRepoStore.java @@ -139,6 +139,13 @@ public class FSGitRepoStore implements RepoStore { return Tar.bz2.zip(getDotGitForProject(projectName), sizePtr); } + @Override + public void gcProject(String projectName) throws IOException { + Project.checkValidProjectName(projectName); + ProjectRepo repo = getExistingRepo(projectName); + repo.runGC(); + } + @Override public void remove(String projectName) throws IOException { Project.checkValidProjectName(projectName); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStore.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStore.java index 18bea89cd5..737d065f34 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStore.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStore.java @@ -50,6 +50,8 @@ public interface RepoStore { return bzip2Project(projectName, null); } + void gcProject(String projectName) throws IOException; + /** * Called after {@link #bzip2Project(String, long[])}'s has been safely * uploaded to the swap store. Removes all traces of the project from disk, diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java index ddf309786a..6be11626b5 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java @@ -118,10 +118,11 @@ public class SwapJobImpl implements SwapJob { (totalSize = repoStore.totalSize()) > lowWatermarkBytes && (numProjects = dbStore.getNumUnswappedProjects()) > minProjects ) { + String projectName = dbStore.getOldestUnswappedProject(); try { - evict(dbStore.getOldestUnswappedProject()); + evict(projectName); } catch (IOException e) { - Log.warn("Exception while swapping, giving up", e); + Log.warn("[{}] Exception while swapping, giving up", projectName, e); } } if (totalSize > lowWatermarkBytes) { @@ -161,6 +162,7 @@ public class SwapJobImpl implements SwapJob { Preconditions.checkNotNull(projName, "projName was null"); Log.info("Evicting project: {}", projName); try (LockGuard __ = lock.lockGuard(projName)) { + repoStore.gcProject(projName); long[] sizePtr = new long[1]; try (InputStream blob = repoStore.bzip2Project(projName, sizePtr)) { swapStore.upload(projName, blob, sizePtr[0]); From f237efa6d59e84ea033cb24391974fbd004c862f Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 23 May 2019 11:14:02 +0100 Subject: [PATCH 05/50] Log and trap/ignore gc errors during swap --- .../uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java index 6be11626b5..a094b6428d 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java @@ -162,7 +162,11 @@ public class SwapJobImpl implements SwapJob { Preconditions.checkNotNull(projName, "projName was null"); Log.info("Evicting project: {}", projName); try (LockGuard __ = lock.lockGuard(projName)) { - repoStore.gcProject(projName); + try { + repoStore.gcProject(projName); + } catch (Exception e) { + Log.error("[{}] Exception while running gc on project: {}", projName, e); + } long[] sizePtr = new long[1]; try (InputStream blob = repoStore.bzip2Project(projName, sizePtr)) { swapStore.upload(projName, blob, sizePtr[0]); From 2492c95c0b75b4d1a90874875c36d1e9a91fd395 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 24 May 2019 16:31:52 +0100 Subject: [PATCH 06/50] Handle a 409 response with code=projectHasDotGit --- .../ic/wlgitbridge/snapshot/base/Request.java | 26 +++++++++++++++++-- .../WLGitBridgeIntegrationTest.java | 21 +++++++++++++++ .../state/state.json | 19 ++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/cannotCloneAHasDotGitProject/state/state.json diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java index eeb40c8f35..648e24a0ba 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java @@ -11,6 +11,7 @@ import uk.ac.ic.wlgitbridge.util.Log; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Arrays; import java.util.concurrent.*; /** @@ -73,9 +74,30 @@ public abstract class Request { if (cause instanceof HttpResponseException) { HttpResponseException httpCause = (HttpResponseException) cause; int sc = httpCause.getStatusCode(); - if (sc == HttpServletResponse.SC_UNAUTHORIZED || sc == HttpServletResponse.SC_FORBIDDEN) { + if (sc == HttpServletResponse.SC_UNAUTHORIZED || sc == HttpServletResponse.SC_FORBIDDEN) { // 401, 403 throw new ForbiddenException(); - } else if (sc == HttpServletResponse.SC_NOT_FOUND) { + } else if (sc == HttpServletResponse.SC_CONFLICT) { // 409 + try { + JsonObject json = Instance.gson.fromJson(httpCause.getContent(), JsonObject.class); + String code = json.get("code").getAsString(); + if ("projectHasDotGit".equals(code)) { + throw new MissingRepositoryException(Arrays.asList( + "This project contains a '.git' entity at the top level, indicating that it is", + "already a git repository. The Overleaf git-bridge cannot work with this project", + "due to a known problem with handling these '.git' folders.", + "", + "If this is unexpected, please contact us at support@overleaf.com, or", + "see https://www.overleaf.com/help/342 for more information." + )); + } else { + throw new MissingRepositoryException(Arrays.asList("Conflict: 409")); + } + } catch (IllegalStateException + | ClassCastException + | NullPointerException _e) { // json parse errors + throw new MissingRepositoryException(Arrays.asList("Conflict: 409")); + } + } else if (sc == HttpServletResponse.SC_NOT_FOUND) { // 404 try { JsonObject json = Instance.gson.fromJson(httpCause.getContent(), JsonObject.class); String message = json.get("message").getAsString(); diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index e0f0ddeffb..4ffa9dda67 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -129,6 +129,9 @@ public class WLGitBridgeIntegrationTest { put("rejectV1Repository", new HashMap() {{ put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/rejectV1Repository/state/state.json")).build()); }}); + put("cannotCloneAHasDotGitProject", new HashMap() {{ + put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/cannotCloneAHasDotGitProject/state/state.json")).build()); + }}); }}; @Rule @@ -868,6 +871,24 @@ public class WLGitBridgeIntegrationTest { assertNotEquals(0, gitProcess.waitFor()); } + @Test + public void cannotCloneAHasDotGitProject() throws IOException, GitAPIException, InterruptedException { + int gitBridgePort = 33885; + int mockServerPort = 3885; + + MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAHasDotGitProject").toFile()); + server.start(); + server.setState(states.get("cannotCloneAHasDotGitProject").get("state")); + GitBridgeApp wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + + wlgb.run(); + Process gitProcess = runtime.exec("git clone http://127.0.0.1:" + gitBridgePort + "/conflict.git", null, dir); + assertNotEquals(0, gitProcess.waitFor()); + wlgb.stop(); + } + private String makeConfigFile( int port, int apiPort diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/cannotCloneAHasDotGitProject/state/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/cannotCloneAHasDotGitProject/state/state.json new file mode 100644 index 0000000000..23bf520b29 --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/cannotCloneAHasDotGitProject/state/state.json @@ -0,0 +1,19 @@ +[ + { + "project": "conflict", + "getDoc": { + "error": 409, + "code": "projectHasDotGit", + "versionID": 1, + "createdAt": "2018-02-05T15:30:00Z", + "email": "michael.walker@overleaf.com", + "name": "msw" + }, + "getSavedVers": [], + "getForVers": [], + "push": "success", + "postback": { + "type": "outOfDate" + } + } +] From 719f0c3661dc18bb75d3585e56efff8774939c68 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 24 May 2019 10:53:54 +0100 Subject: [PATCH 07/50] Give up on projects that cannot be swapped, rather than spinning the loop forever --- .../bridge/swap/job/SwapJobImpl.java | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java index 6be11626b5..7ed0748c7e 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java @@ -14,6 +14,7 @@ import java.io.InputStream; import java.sql.Timestamp; import java.time.Duration; import java.time.LocalDateTime; +import java.util.ArrayList; import java.util.Timer; import java.util.concurrent.atomic.AtomicInteger; @@ -105,6 +106,8 @@ public class SwapJobImpl implements SwapJob { } private void doSwap_() { + ArrayList exceptionProjectNames = new ArrayList(); + Log.info("Running swap number {}", swaps.get() + 1); long totalSize = repoStore.totalSize(); Log.info("Size is {}/{} (high)", totalSize, highWatermarkBytes); @@ -114,15 +117,40 @@ public class SwapJobImpl implements SwapJob { return; } int numProjects = dbStore.getNumProjects(); + // while we have too many projects on disk while ( (totalSize = repoStore.totalSize()) > lowWatermarkBytes && (numProjects = dbStore.getNumUnswappedProjects()) > minProjects ) { + // check if we've had too many exceptions so far + if (exceptionProjectNames.size() >= 20) { + StringBuilder sb = new StringBuilder(); + for (String s: exceptionProjectNames) { + sb.append(s); + sb.append(' '); + } + Log.error( + "Too many exceptions while running swap, giving up on this run: {}", + sb.toString() + ); + break; + } + // get the oldest project and try to swap it String projectName = dbStore.getOldestUnswappedProject(); try { evict(projectName); - } catch (IOException e) { - Log.warn("[{}] Exception while swapping, giving up", projectName, e); + } catch (Throwable t) { + Log.warn("[{}] Exception while swapping, mark project and move on", projectName, t); + // NOTE: this is something of a hack. If a project fails to swap we get stuck in a + // loop where `dbStore.getOldestUnswappedProject()` gives the same failing project over and over again, + // which fills up the disk with errors. By touching the access time we can mark the project as a + // non-candidate for swapping. Ideally we should be checking the logs for these log events and fixing + // whatever is wrong with the project + dbStore.setLastAccessedTime( + projectName, + Timestamp.valueOf(LocalDateTime.now()) + ); + exceptionProjectNames.add(projectName); } } if (totalSize > lowWatermarkBytes) { From 552e0955da2c9815eda4cf0d26a3335a540693ee Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 29 May 2019 11:05:53 +0100 Subject: [PATCH 08/50] Tell user to remove .git folder --- .../main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java index 648e24a0ba..3cb3e51816 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/Request.java @@ -86,6 +86,8 @@ public abstract class Request { "already a git repository. The Overleaf git-bridge cannot work with this project", "due to a known problem with handling these '.git' folders.", "", + "We recommend removing the .git folder before trying again.", + "", "If this is unexpected, please contact us at support@overleaf.com, or", "see https://www.overleaf.com/help/342 for more information." )); From 28865e2956300d5700fd98cbc718b321a525001a Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 7 Jun 2019 10:26:55 +0100 Subject: [PATCH 09/50] Don't catch `Throwable` in swap-job, catch Exception --- .../uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java index ce82420ea2..f989fe1341 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/swap/job/SwapJobImpl.java @@ -139,8 +139,8 @@ public class SwapJobImpl implements SwapJob { String projectName = dbStore.getOldestUnswappedProject(); try { evict(projectName); - } catch (Throwable t) { - Log.warn("[{}] Exception while swapping, mark project and move on", projectName, t); + } catch (Exception e) { + Log.warn("[{}] Exception while swapping, mark project and move on", projectName, e); // NOTE: this is something of a hack. If a project fails to swap we get stuck in a // loop where `dbStore.getOldestUnswappedProject()` gives the same failing project over and over again, // which fills up the disk with errors. By touching the access time we can mark the project as a From 71df1b0a3178f3f4801430eff0741cc4391b73ce Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 7 Jun 2019 14:17:33 +0100 Subject: [PATCH 10/50] Encode file path when building url, allows unicode --- .../ic/wlgitbridge/data/CandidateSnapshot.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java index f27601227b..29aa917011 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java @@ -5,10 +5,13 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory; +import uk.ac.ic.wlgitbridge.util.Log; import uk.ac.ic.wlgitbridge.util.Util; import java.io.File; import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -115,10 +118,17 @@ public class CandidateSnapshot implements AutoCloseable { JsonObject jsonFile = new JsonObject(); jsonFile.addProperty("name", file.getPath()); if (file.isChanged()) { - jsonFile.addProperty( - "url", - projectURL + "/" + file.getPath() + "?key=" + postbackKey - ); + String path = file.getPath(); + String encodedPath; + try { + encodedPath = URLEncoder.encode(path, "UTF-8"); + } catch (UnsupportedEncodingException e) { + // This should never happen + Log.error("Error while encoding file path: projectUrl={}, path={}", projectURL, path, e); + encodedPath = path; + } + String url = projectURL + "/" + encodedPath + "?key=" + postbackKey; + jsonFile.addProperty("url", url); } return jsonFile; } From 11e42ecb6f5c296b0cb300e2c1a2528e38ee799d Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 3 Jul 2019 09:37:11 +0100 Subject: [PATCH 11/50] Point local config to v2 --- services/git-bridge/conf/local.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/conf/local.json b/services/git-bridge/conf/local.json index 3c280ad726..c119c85075 100644 --- a/services/git-bridge/conf/local.json +++ b/services/git-bridge/conf/local.json @@ -9,7 +9,7 @@ "oauth2": { "oauth2ClientID": "264c723c925c13590880751f861f13084934030c13b4452901e73bdfab226edc", "oauth2ClientSecret": "e6b2e9eee7ae2bb653823250bb69594a91db0547fe3790a7135acb497108e62d", - "oauth2Server": "http://www.overleaf.test:5000" + "oauth2Server": "http://v2.overleaf.test:4000" }, "repoStore": { "maxFileSize": 52428800 From 88adce3a0299106d3d5ac1038e4b96928175e6e0 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 4 Jul 2019 14:14:01 +0100 Subject: [PATCH 12/50] Use UUID in file url, not (encoded) file path. This fixes a bunch of issues where funny characters in the file path (spaces, unicode, etc) would cause the file server in this process to respond with a 404 when asked for the file. The 404 would then cause the push to fail. Now we just use a UUID as an opaque and unambiguous identifier for each file. --- .../ic/wlgitbridge/data/CandidateSnapshot.java | 17 +++-------------- .../uk/ac/ic/wlgitbridge/data/ServletFile.java | 6 +++++- .../ic/wlgitbridge/data/filestore/RawFile.java | 6 +++++- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java index 29aa917011..4c3aef49f8 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/CandidateSnapshot.java @@ -5,13 +5,10 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory; -import uk.ac.ic.wlgitbridge.util.Log; import uk.ac.ic.wlgitbridge.util.Util; import java.io.File; import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.net.URLEncoder; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -80,7 +77,7 @@ public class CandidateSnapshot implements AutoCloseable { ); for (ServletFile file : files) { if (file.isChanged()) { - file.writeToDisk(attsDirectory); + file.writeToDiskWithName(attsDirectory, file.getUniqueIdentifier()); } } } @@ -118,16 +115,8 @@ public class CandidateSnapshot implements AutoCloseable { JsonObject jsonFile = new JsonObject(); jsonFile.addProperty("name", file.getPath()); if (file.isChanged()) { - String path = file.getPath(); - String encodedPath; - try { - encodedPath = URLEncoder.encode(path, "UTF-8"); - } catch (UnsupportedEncodingException e) { - // This should never happen - Log.error("Error while encoding file path: projectUrl={}, path={}", projectURL, path, e); - encodedPath = path; - } - String url = projectURL + "/" + encodedPath + "?key=" + postbackKey; + String identifier = file.getUniqueIdentifier(); + String url = projectURL + "/" + identifier + "?key=" + postbackKey; jsonFile.addProperty("url", url); } return jsonFile; diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ServletFile.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ServletFile.java index ed3216ae4d..cabd761a98 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ServletFile.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/ServletFile.java @@ -1,6 +1,7 @@ package uk.ac.ic.wlgitbridge.data; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; +import java.util.UUID; /** * Created by Winston on 21/02/15. @@ -9,12 +10,16 @@ public class ServletFile extends RawFile { private final RawFile file; private final boolean changed; + private String uuid; public ServletFile(RawFile file, RawFile oldFile) { this.file = file; + this.uuid = UUID.randomUUID().toString(); changed = !equals(oldFile); } + public String getUniqueIdentifier() { return uuid; } + @Override public String getPath() { return file.getPath(); @@ -38,5 +43,4 @@ public class ServletFile extends RawFile { public String toString() { return getPath(); } - } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/filestore/RawFile.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/filestore/RawFile.java index d799843c6a..8c74f4de20 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/filestore/RawFile.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/data/filestore/RawFile.java @@ -20,7 +20,11 @@ public abstract class RawFile { public abstract long size(); public final void writeToDisk(File directory) throws IOException { - File file = new File(directory, getPath()); + writeToDiskWithName(directory, getPath()); + } + + public final void writeToDiskWithName(File directory, String name) throws IOException { + File file = new File(directory, name); file.getParentFile().mkdirs(); file.createNewFile(); OutputStream out = new FileOutputStream(file); From abf525f43a5b200f261defdb0e5838bd7379e55d Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 1 Aug 2019 16:34:49 +0100 Subject: [PATCH 13/50] Update test to match new setup/teardown pattern --- .../wlgitbridge/application/WLGitBridgeIntegrationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 7f66eadbbc..b45b27be7a 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -852,10 +852,10 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33885; int mockServerPort = 3885; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAHasDotGitProject").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/cannotCloneAHasDotGitProject").toFile()); server.start(); server.setState(states.get("cannotCloneAHasDotGitProject").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); From b0ab2e07c5c119a268487e2194f07f400a416668 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 1 Aug 2019 11:01:39 +0100 Subject: [PATCH 14/50] Reject requests when the project uri begins with '/project' --- .../ic/wlgitbridge/server/Oauth2Filter.java | 22 +++++++++++++++- .../WLGitBridgeIntegrationTest.java | 26 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index dfcfdf21be..bfbf0d6eea 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -40,6 +40,18 @@ public class Oauth2Filter implements Filter { @Override public void init(FilterConfig filterConfig) {} + private void sendResponse(ServletResponse servletResponse, int code, List lines) throws IOException { + HttpServletResponse response = ((HttpServletResponse) servletResponse); + response.setContentType("text/plain"); + response.setStatus(code); + PrintWriter w = response.getWriter(); + for (String line : lines) { + w.println(line); + } + w.close(); + return; + } + /** * The original request from git will not contain the Authorization header. * @@ -57,8 +69,16 @@ public class Oauth2Filter implements Filter { ServletResponse servletResponse, FilterChain filterChain ) throws IOException, ServletException { + String requestUri = ((Request) servletRequest).getRequestURI(); + if (requestUri.startsWith("/project")) { + Log.info("[{}] Invalid request URI", requestUri); + sendResponse(servletResponse,400, Arrays.asList( + "Invalid Project ID (must not have a '/project' prefix)" + )); + return; + } String project = Util.removeAllSuffixes( - ((Request) servletRequest).getRequestURI().split("/")[1], + requestUri.split("/")[1], ".git" ); // Reject v1 ids, the request will be rejected by v1 anyway diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index b45b27be7a..f150585f0f 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -865,6 +865,32 @@ public class WLGitBridgeIntegrationTest { wlgb.stop(); } + @Test + public void cannotCloneProjectWithSlash() throws IOException, GitAPIException, InterruptedException { + int gitBridgePort = 33886; + int mockServerPort = 3886; + + MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); + server.start(); + server.setState(states.get("canCloneARepository").get("state")); + GitBridgeApp wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + + wlgb.run(); + Process gitProcess = runtime.exec("git clone http://127.0.0.1:" + gitBridgePort + "/project/1234abcd", null, dir); + assertNotEquals(0, gitProcess.waitFor()); + + List actual = Util.linesFromStream(gitProcess.getErrorStream(), 0, ""); + assertEquals(Arrays.asList( + "Cloning into '1234abcd'...", + "remote: Invalid Project ID (must not have a '/project' prefix)", + "fatal: unable to access 'http://127.0.0.1:33886/project/1234abcd/': The requested URL returned error: 400" + ), actual); + + wlgb.stop(); + } + private String makeConfigFile( int port, int apiPort From 76b349591c7877d99342e60e1c6b5c5f01341be2 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 1 Aug 2019 11:08:41 +0100 Subject: [PATCH 15/50] Refactor to use new helper to send error response --- .../uk/ac/ic/wlgitbridge/server/Oauth2Filter.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index bfbf0d6eea..9eb8442086 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -84,11 +84,7 @@ public class Oauth2Filter implements Filter { // Reject v1 ids, the request will be rejected by v1 anyway if (project.matches("^[0-9]+[bcdfghjklmnpqrstvwxyz]{6,12}$") && !project.matches("^[0-9a-f]{24}$")) { Log.info("[{}] Request for v1 project, refusing", project); - HttpServletResponse response = ((HttpServletResponse) servletResponse); - response.setContentType("text/plain"); - response.setStatus(404); - PrintWriter w = response.getWriter(); - List l = Arrays.asList( + sendResponse(servletResponse, 404, Arrays.asList( "This project has not yet been moved into the new version", "of Overleaf. You will need to move it in order to continue working on it.", "Please visit this project online on www.overleaf.com to do this.", @@ -98,11 +94,7 @@ public class Oauth2Filter implements Filter { "", "If this is unexpected, please contact us at support@overleaf.com, or", "see https://www.overleaf.com/help/342 for more information." - ); - for (String line : l) { - w.println(line); - } - w.close(); + )); return; } Log.info("[{}] Checking if auth needed", project); From ac4f4082c8c8e3f67dbaff5e6d71ebb78648b3aa Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 2 Aug 2019 10:46:15 +0100 Subject: [PATCH 16/50] Use 404 code when rejecting invalid project id --- .../src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java | 2 +- .../ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index 9eb8442086..631401c759 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -72,7 +72,7 @@ public class Oauth2Filter implements Filter { String requestUri = ((Request) servletRequest).getRequestURI(); if (requestUri.startsWith("/project")) { Log.info("[{}] Invalid request URI", requestUri); - sendResponse(servletResponse,400, Arrays.asList( + sendResponse(servletResponse,404, Arrays.asList( "Invalid Project ID (must not have a '/project' prefix)" )); return; diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index f150585f0f..1a6cffbe6a 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -885,7 +885,7 @@ public class WLGitBridgeIntegrationTest { assertEquals(Arrays.asList( "Cloning into '1234abcd'...", "remote: Invalid Project ID (must not have a '/project' prefix)", - "fatal: unable to access 'http://127.0.0.1:33886/project/1234abcd/': The requested URL returned error: 400" + "fatal: repository 'http://127.0.0.1:33886/project/1234abcd/' not found" ), actual); wlgb.stop(); From ffcb382f0c34f1b83e365bdc8f04830a6f4d1b6a Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 2 Aug 2019 13:34:43 +0100 Subject: [PATCH 17/50] Update test to match new setup/teardown pattern --- .../wlgitbridge/application/WLGitBridgeIntegrationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 1a6cffbe6a..2aeedde531 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -870,10 +870,10 @@ public class WLGitBridgeIntegrationTest { int gitBridgePort = 33886; int mockServerPort = 3886; - MockSnapshotServer server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); + server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); server.start(); server.setState(states.get("canCloneARepository").get("state")); - GitBridgeApp wlgb = new GitBridgeApp(new String[] { + wlgb = new GitBridgeApp(new String[] { makeConfigFile(gitBridgePort, mockServerPort) }); From 5188e7c06aefb687e2b02c6ce7454fbe4060797f Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Tue, 12 Nov 2019 11:16:54 -0400 Subject: [PATCH 18/50] add file limit error --- services/git-bridge/README.md | 33 ++++++++---------- services/git-bridge/conf/example_config.json | 1 + services/git-bridge/conf/local.json | 1 + .../uk/ac/ic/wlgitbridge/bridge/Bridge.java | 18 ++++++++-- .../bridge/repo/RepoStoreConfig.java | 10 +++++- .../exception/FileLimitExceededException.java | 34 +++++++++++++++++++ 6 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/FileLimitExceededException.java diff --git a/services/git-bridge/README.md b/services/git-bridge/README.md index 1b01ddc49e..4b43c1b4bf 100644 --- a/services/git-bridge/README.md +++ b/services/git-bridge/README.md @@ -97,23 +97,20 @@ You have to restart the server for configuration changes to take effect. ## Creating OAuth app -In dev-env, run `bin/run rails_v1 rake db:seed`, or, if using this solo, run the following in the v1 -database: +In dev-env, run the following command in mongo to create the oauth application +for git-bridge. -```sql -INSERT INTO public.oauth_applications ( - "name", uid, secret, redirect_uri, scopes, skip_authorization, - created_at, updated_at, partner, confidential -) VALUES ( - 'gitbridge', - '264c723c925c13590880751f861f13084934030c13b4452901e73bdfab226edc', - 'e6b2e9eee7ae2bb653823250bb69594a91db0547fe3790a7135acb497108e62d', - 'http://www.overleaf.test:5000/no-callback-required', - 'git_bridge', - true, - now(), - now(), - null, - true -); +``` +db.oauthApplications.insert({ + "clientSecret" : "e6b2e9eee7ae2bb653823250bb69594a91db0547fe3790a7135acb497108e62d", + "grants" : [ + "password" + ], + "id" : "264c723c925c13590880751f861f13084934030c13b4452901e73bdfab226edc", + "name" : "Overleaf Git Bridge", + "redirectUris" : [], + "scopes" : [ + "git_bridge" + ] +}) ``` diff --git a/services/git-bridge/conf/example_config.json b/services/git-bridge/conf/example_config.json index 3b844ef099..7965ce9570 100644 --- a/services/git-bridge/conf/example_config.json +++ b/services/git-bridge/conf/example_config.json @@ -12,6 +12,7 @@ "oauth2Server": "https://localhost" }, "repoStore": { + "maxFileNum": 2000, "maxFileSize": 52428800 }, "swapStore": { diff --git a/services/git-bridge/conf/local.json b/services/git-bridge/conf/local.json index c119c85075..0acb24ac0a 100644 --- a/services/git-bridge/conf/local.json +++ b/services/git-bridge/conf/local.json @@ -12,6 +12,7 @@ "oauth2Server": "http://v2.overleaf.test:4000" }, "repoStore": { + "maxFileNum": 2000, "maxFileSize": 52428800 }, "swapStore": { diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index 3c543f50f5..9246eb988f 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -28,6 +28,7 @@ import uk.ac.ic.wlgitbridge.data.filestore.RawFile; import uk.ac.ic.wlgitbridge.data.model.Snapshot; import uk.ac.ic.wlgitbridge.git.exception.GitUserException; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; +import uk.ac.ic.wlgitbridge.git.exception.FileLimitExceededException; import uk.ac.ic.wlgitbridge.git.handler.WLReceivePackFactory; import uk.ac.ic.wlgitbridge.git.handler.WLRepositoryResolver; import uk.ac.ic.wlgitbridge.git.handler.WLUploadPackFactory; @@ -426,6 +427,7 @@ public class Bridge { * @throws IOException * @throws MissingRepositoryException * @throws ForbiddenException + * @throws GitUserException */ public void push( Optional oauth2, @@ -433,7 +435,7 @@ public class Bridge { RawDirectory directoryContents, RawDirectory oldDirectoryContents, String hostname - ) throws SnapshotPostException, IOException, MissingRepositoryException, ForbiddenException { + ) throws SnapshotPostException, IOException, MissingRepositoryException, ForbiddenException, GitUserException { try (LockGuard __ = lock.lockGuard(projectName)) { pushCritical( oauth2, @@ -507,13 +509,23 @@ public class Bridge { * @throws MissingRepositoryException * @throws ForbiddenException * @throws SnapshotPostException + * @throws GitUserException */ private void pushCritical( Optional oauth2, String projectName, RawDirectory directoryContents, RawDirectory oldDirectoryContents - ) throws IOException, MissingRepositoryException, ForbiddenException, SnapshotPostException { + ) throws IOException, MissingRepositoryException, ForbiddenException, SnapshotPostException, GitUserException { + Optional maxFileNum = config + .getRepoStore() + .flatMap(RepoStoreConfig::getMaxFileNum); + if (maxFileNum.isPresent()) { + long maxFileNum_ = maxFileNum.get(); + if (directoryContents.getFileTable().size() > maxFileNum_) { + throw new FileLimitExceededException(directoryContents.getFileTable().size(), maxFileNum_); + } + } Log.info("[{}] Pushing", projectName); String postbackKey = postbackManager.makeKeyForProject(projectName); Log.info( @@ -529,7 +541,7 @@ public class Bridge { ); ) { Log.info( - "[{}] Candindate snapshot created: {}", + "[{}] Candidate snapshot created: {}", projectName, candidate ); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStoreConfig.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStoreConfig.java index 3bfd308dcd..52d41104a1 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStoreConfig.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/RepoStoreConfig.java @@ -11,11 +11,19 @@ public class RepoStoreConfig { @Nullable private final Long maxFileSize; - public RepoStoreConfig(Long maxFileSize) { + @Nullable + private final Long maxFileNum; + + public RepoStoreConfig(Long maxFileSize, Long maxFileNum) { this.maxFileSize = maxFileSize; + this.maxFileNum = maxFileNum; } public Optional getMaxFileSize() { return Optional.ofNullable(maxFileSize); } + + public Optional getMaxFileNum() { + return Optional.ofNullable(maxFileNum); + } } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/FileLimitExceededException.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/FileLimitExceededException.java new file mode 100644 index 0000000000..f25767a4db --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/FileLimitExceededException.java @@ -0,0 +1,34 @@ +package uk.ac.ic.wlgitbridge.git.exception; + +import uk.ac.ic.wlgitbridge.util.Util; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; + +public class FileLimitExceededException extends GitUserException { + + private final long numFiles; + + private final long maxFiles; + + public FileLimitExceededException(long numFiles, long maxFiles) { + this.numFiles = numFiles; + this.maxFiles = maxFiles; + } + + @Override + public String getMessage() { + return "too many files"; + } + + @Override + public List getDescriptionLines() { + return Arrays.asList( + "repository contains " + + numFiles + " files, which exceeds the limit of " + + maxFiles + " files" + ); + } + +} From b1262ff06ef6ee57fd13c1ed6640bd3b7e334bec Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Tue, 19 Nov 2019 12:08:35 -0400 Subject: [PATCH 19/50] pass client ip as url param --- .../java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index 631401c759..bbf2696cd3 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -133,7 +133,11 @@ public class Oauth2Filter implements Filter { String authHeader = request.getHeader("Authorization"); if (authHeader != null) { - Log.info("[{}] Authorization header present"); + String clientIp = request.getHeader("X-Forwarded-For"); + if (clientIp == null) { + clientIp = request.getRemoteAddr(); + } + Log.info("[{}] Authorization header present", clientIp); StringTokenizer st = new StringTokenizer(authHeader); if (st.hasMoreTokens()) { String basic = st.nextToken(); @@ -157,7 +161,8 @@ public class Oauth2Filter implements Filter { Instance.jsonFactory, new GenericUrl( oauth2.getOauth2Server() - + "/oauth/token" + + "/oauth/token?client_ip=" + + clientIp ), username, password From b6812462d6b0cbe7c5e56a798716a2052121c1a8 Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Mon, 25 Nov 2019 10:20:43 -0400 Subject: [PATCH 20/50] add debug --- .../main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index bbf2696cd3..72efe42982 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -138,6 +138,12 @@ public class Oauth2Filter implements Filter { clientIp = request.getRemoteAddr(); } Log.info("[{}] Authorization header present", clientIp); + // TEMPORARY DEBUG CODE + Enumeration headerNames = request.getHeaderNames(); + while(headerNames.hasMoreElements()) { + String headerName = (String)headerNames.nextElement(); + Log.info("HEADER {}: {}", headerName, request.getHeader(headerName)); + } StringTokenizer st = new StringTokenizer(authHeader); if (st.hasMoreTokens()) { String basic = st.nextToken(); From d89dbb7ff8d08990fd7cc098a460b9cdf4a55cdd Mon Sep 17 00:00:00 2001 From: Ersun Warncke Date: Wed, 27 Nov 2019 09:05:18 -0400 Subject: [PATCH 21/50] Revert "add debug" This reverts commit 0d28d39563ad5de0db666c9e67493f5c814fe930. --- .../main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index 72efe42982..bbf2696cd3 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -138,12 +138,6 @@ public class Oauth2Filter implements Filter { clientIp = request.getRemoteAddr(); } Log.info("[{}] Authorization header present", clientIp); - // TEMPORARY DEBUG CODE - Enumeration headerNames = request.getHeaderNames(); - while(headerNames.hasMoreElements()) { - String headerName = (String)headerNames.nextElement(); - Log.info("HEADER {}: {}", headerName, request.getHeader(headerName)); - } StringTokenizer st = new StringTokenizer(authHeader); if (st.hasMoreTokens()) { String basic = st.nextToken(); From f98212e96b269606eaa928a3e71eed33b250ae59 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Tue, 10 Mar 2020 15:35:50 +0100 Subject: [PATCH 22/50] [misc] make: skip tests for the build and package target http://maven.apache.org/plugins-archives/maven-surefire-plugin-2.12.4/examples/skipping-test.html --- services/git-bridge/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/Makefile b/services/git-bridge/Makefile index 599d096cee..4fc3df28d1 100644 --- a/services/git-bridge/Makefile +++ b/services/git-bridge/Makefile @@ -6,7 +6,7 @@ run: package build: - mvn package + mvn package -DskipTests test: @@ -18,7 +18,7 @@ clean: package: clean - mvn package + mvn package -DskipTests .PHONY: run package build clean test From 090e58a95391e4c53943dc1ad3861c3de28bb399 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 11 Mar 2020 11:46:46 +0100 Subject: [PATCH 23/50] [misc] add documentation on how to run commands from the dev-environment --- services/git-bridge/README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/services/git-bridge/README.md b/services/git-bridge/README.md index 4b43c1b4bf..038d1b8b3f 100644 --- a/services/git-bridge/README.md +++ b/services/git-bridge/README.md @@ -20,6 +20,17 @@ To be run from the base directory: **Clean**: `mvn clean` +To be run from the dev-environment: + +**Build jar**: +`bin/run git-bridge make package` + +**Run tests**: +`bin/run git-bridge make test` + +**Clean**: +`bin/run git-bridge make clean` + Installation ------------ From eba6c907e96b37f5c30f75a88ce12cef517eb6a1 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 17 Jun 2020 16:05:53 +0100 Subject: [PATCH 24/50] wip: status handler --- .../wlgitbridge/server/GitBridgeServer.java | 1 + .../ic/wlgitbridge/server/StatusHandler.java | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java index 3008ed1028..e39dbb3a41 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java @@ -128,6 +128,7 @@ public class GitBridgeServer { api.setContextPath("/api"); HandlerCollection handlers = new HandlerList(); + handlers.addHandler(new StatusHandler(bridge)); handlers.addHandler(initResourceHandler()); handlers.addHandler(new PostbackHandler(bridge)); handlers.addHandler(new DefaultHandler()); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java new file mode 100644 index 0000000000..fc60aa8057 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java @@ -0,0 +1,40 @@ +package uk.ac.ic.wlgitbridge.server; + +import org.eclipse.jetty.server.HttpConnection; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.ic.wlgitbridge.bridge.Bridge; +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; + +public class StatusHandler extends AbstractHandler { + + private final Bridge bridge; + + public StatusHandler(Bridge bridge) { + this.bridge = bridge; + } + + @Override + public void handle( + String target, + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response + ) throws IOException { + if ("GET".equals(baseRequest.getMethod()) && "/status".equals(target)) { + Log.info("GET <- /api/status"); + baseRequest.setHandled(true); + response.setContentType("text/plain"); + response.setStatus(200); + response.getWriter().println("ok"); + } + } + +} From 1befc3582b759d44f6b46e0ad597431f53a73b7e Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 17 Jun 2020 16:15:40 +0100 Subject: [PATCH 25/50] wip: add skeleton health-check handler --- .../wlgitbridge/server/GitBridgeServer.java | 1 + .../server/HealthCheckHandler.java | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java index e39dbb3a41..86c7e193c0 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java @@ -129,6 +129,7 @@ public class GitBridgeServer { HandlerCollection handlers = new HandlerList(); handlers.addHandler(new StatusHandler(bridge)); + handlers.addHandler(new HealthCheckHandler(bridge)); handlers.addHandler(initResourceHandler()); handlers.addHandler(new PostbackHandler(bridge)); handlers.addHandler(new DefaultHandler()); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java new file mode 100644 index 0000000000..c7f30b3497 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java @@ -0,0 +1,40 @@ +package uk.ac.ic.wlgitbridge.server; + +import org.eclipse.jetty.server.HttpConnection; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.ic.wlgitbridge.bridge.Bridge; +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; + +public class HealthCheckHandler extends AbstractHandler { + + private final Bridge bridge; + + public HealthCheckHandler(Bridge bridge) { + this.bridge = bridge; + } + + @Override + public void handle( + String target, + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response + ) throws IOException { + if ("GET".equals(baseRequest.getMethod()) && "/health_check".equals(target)) { + Log.info("GET <- /api/health_check"); + baseRequest.setHandled(true); + response.setContentType("text/plain"); + response.setStatus(200); + response.getWriter().println("ok"); + } + } + +} From 86769eedea22fc8136e48ec7cafecab1877faccc Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 18 Jun 2020 11:45:04 +0100 Subject: [PATCH 26/50] Add a healthCheck method to the Bridge, check db --- .../main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index 9246eb988f..3e54cd1920 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -263,6 +263,17 @@ public class Bridge { gcJob.start(); } + public boolean healthCheck() { + try { + dbStore.getNumProjects(); + Log.error("[HealthCheck] passed"); + return true; + } catch (Exception e) { + Log.error("[HealthCheck] FAILED!", e); + return false; + } + } + /** * Performs a check of inconsistencies in the DB. This was used to upgrade * the schema. From 689362b24f98126c482f2d2632ba0cc2bd594d06 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 18 Jun 2020 11:45:21 +0100 Subject: [PATCH 27/50] Use the bridge.healthCheck method --- .../uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java index c7f30b3497..74f962fb65 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java @@ -32,8 +32,13 @@ public class HealthCheckHandler extends AbstractHandler { Log.info("GET <- /api/health_check"); baseRequest.setHandled(true); response.setContentType("text/plain"); - response.setStatus(200); - response.getWriter().println("ok"); + if (bridge.healthCheck()) { + response.setStatus(200); + response.getWriter().println("ok"); + } else { + response.setStatus(500); + response.getWriter().println("failed"); + } } } From badeea3e0ba73e5eabc13f6d44ec61f4be2054a1 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 18 Jun 2020 13:54:10 +0100 Subject: [PATCH 28/50] Add test for status and health-check endpoints --- .../WLGitBridgeIntegrationTest.java | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 2aeedde531..4a1ab04adb 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -3,6 +3,13 @@ package uk.ac.ic.wlgitbridge.application; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import static org.asynchttpclient.Dsl.*; + +import org.apache.http.HttpResponse; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.HttpClients; import org.asynchttpclient.*; import org.eclipse.jgit.api.errors.GitAPIException; import org.junit.After; @@ -891,7 +898,29 @@ public class WLGitBridgeIntegrationTest { wlgb.stop(); } - private String makeConfigFile( + @Test + public void testStatusAndHealthCheckEndpoints() throws ClientProtocolException, IOException { + int gitBridgePort = 33887; + int mockServerPort = 3887; + server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); + server.start(); + server.setState(states.get("canCloneARepository").get("state")); + wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + wlgb.run(); + HttpClient client = HttpClients.createDefault(); + // Status + HttpGet statusRequest = new HttpGet("http://127.0.0.1:"+gitBridgePort+"/api/status"); + HttpResponse statusResponse = client.execute(statusRequest); + assertEquals(statusResponse.getStatusLine().getStatusCode(), 200); + // Health Check + HttpGet healthCheckRequest = new HttpGet("http://127.0.0.1:"+gitBridgePort+"/api/health_check"); + HttpResponse healthCheckResponse = client.execute(healthCheckRequest); + assertEquals(healthCheckResponse.getStatusLine().getStatusCode(), 200); + } + + private String makeConfigFile( int port, int apiPort ) throws IOException { From 973a18b1b843717acebd70062370661edbcc554a Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 18 Jun 2020 14:56:38 +0100 Subject: [PATCH 29/50] Clean up new healthcheck test --- .../wlgitbridge/application/WLGitBridgeIntegrationTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 4a1ab04adb..b2fb4e08a5 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -910,12 +910,13 @@ public class WLGitBridgeIntegrationTest { }); wlgb.run(); HttpClient client = HttpClients.createDefault(); + String urlBase = "http://127.0.0.1:" + gitBridgePort; // Status - HttpGet statusRequest = new HttpGet("http://127.0.0.1:"+gitBridgePort+"/api/status"); + HttpGet statusRequest = new HttpGet(urlBase+"/api/status"); HttpResponse statusResponse = client.execute(statusRequest); assertEquals(statusResponse.getStatusLine().getStatusCode(), 200); // Health Check - HttpGet healthCheckRequest = new HttpGet("http://127.0.0.1:"+gitBridgePort+"/api/health_check"); + HttpGet healthCheckRequest = new HttpGet(urlBase+"/api/health_check"); HttpResponse healthCheckResponse = client.execute(healthCheckRequest); assertEquals(healthCheckResponse.getStatusLine().getStatusCode(), 200); } From 40a171d44a153a5f35e8ba48dfd3bc35fb43729c Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 18 Jun 2020 14:56:50 +0100 Subject: [PATCH 30/50] Also check we can touch the filesystem in healthcheck --- .../src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index 3e54cd1920..6bd5df2a59 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -266,6 +266,10 @@ public class Bridge { public boolean healthCheck() { try { dbStore.getNumProjects(); + File rootDirectory = new File("/"); + if (!rootDirectory.exists()) { + throw new Exception("bad filesystem state, root directory does not exist"); + } Log.error("[HealthCheck] passed"); return true; } catch (Exception e) { From 3c6ef380364cc8f63056f322a19087918b34bcaa Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 18 Jun 2020 15:00:17 +0100 Subject: [PATCH 31/50] Fix alignment of function --- .../ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index b2fb4e08a5..07373b40a4 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -921,7 +921,7 @@ public class WLGitBridgeIntegrationTest { assertEquals(healthCheckResponse.getStatusLine().getStatusCode(), 200); } - private String makeConfigFile( + private String makeConfigFile( int port, int apiPort ) throws IOException { From 8def058d85a29bedf5946524f139761bbd24c817 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 19 Jun 2020 15:50:16 +0100 Subject: [PATCH 32/50] Fix log level --- .../src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index 6bd5df2a59..c5b82cb142 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -270,7 +270,7 @@ public class Bridge { if (!rootDirectory.exists()) { throw new Exception("bad filesystem state, root directory does not exist"); } - Log.error("[HealthCheck] passed"); + Log.info("[HealthCheck] passed"); return true; } catch (Exception e) { Log.error("[HealthCheck] FAILED!", e); From ed778639a87acdd258638d12c4262a1b5888fc38 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 19 Jun 2020 15:50:27 +0100 Subject: [PATCH 33/50] Move the status and healthcheck to root level --- .../ac/ic/wlgitbridge/server/GitBridgeServer.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java index 86c7e193c0..7424c98056 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java @@ -119,17 +119,26 @@ public class GitBridgeServer { ) throws ServletException { HandlerCollection handlers = new HandlerList(); handlers.addHandler(initApiHandler()); + handlers.addHandler(initBaseHandler()); handlers.addHandler(initGitHandler(config, repoStore, snapshotApi)); jettyServer.setHandler(handlers); } + private Handler initBaseHandler() { + ContextHandler base = new ContextHandler(); + base.setContextPath("/"); + HandlerCollection handlers = new HandlerList(); + handlers.addHandler(new StatusHandler(bridge)); + handlers.addHandler(new HealthCheckHandler(bridge)); + base.setHandler(handlers); + return base; + } + private Handler initApiHandler() { ContextHandler api = new ContextHandler(); api.setContextPath("/api"); HandlerCollection handlers = new HandlerList(); - handlers.addHandler(new StatusHandler(bridge)); - handlers.addHandler(new HealthCheckHandler(bridge)); handlers.addHandler(initResourceHandler()); handlers.addHandler(new PostbackHandler(bridge)); handlers.addHandler(new DefaultHandler()); From ec278ffe0acbaa2cf3a7dd77324fb927e9953945 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 19 Jun 2020 15:50:42 +0100 Subject: [PATCH 34/50] Fix log lines --- .../java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java | 2 +- .../main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java index 74f962fb65..120b2d6163 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java @@ -29,7 +29,7 @@ public class HealthCheckHandler extends AbstractHandler { HttpServletResponse response ) throws IOException { if ("GET".equals(baseRequest.getMethod()) && "/health_check".equals(target)) { - Log.info("GET <- /api/health_check"); + Log.info("GET <- /health_check"); baseRequest.setHandled(true); response.setContentType("text/plain"); if (bridge.healthCheck()) { diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java index fc60aa8057..802db35bd1 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java @@ -29,7 +29,7 @@ public class StatusHandler extends AbstractHandler { HttpServletResponse response ) throws IOException { if ("GET".equals(baseRequest.getMethod()) && "/status".equals(target)) { - Log.info("GET <- /api/status"); + Log.info("GET <- /status"); baseRequest.setHandled(true); response.setContentType("text/plain"); response.setStatus(200); From 6ff3877dd1f0d2dee2c91b8c06875eb7b943f0b4 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 19 Jun 2020 15:50:51 +0100 Subject: [PATCH 35/50] Fix status and healthcheck tests --- .../application/WLGitBridgeIntegrationTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 07373b40a4..482fe8de8d 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -912,13 +912,13 @@ public class WLGitBridgeIntegrationTest { HttpClient client = HttpClients.createDefault(); String urlBase = "http://127.0.0.1:" + gitBridgePort; // Status - HttpGet statusRequest = new HttpGet(urlBase+"/api/status"); + HttpGet statusRequest = new HttpGet(urlBase+"/status"); HttpResponse statusResponse = client.execute(statusRequest); - assertEquals(statusResponse.getStatusLine().getStatusCode(), 200); + assertEquals(200, statusResponse.getStatusLine().getStatusCode()); // Health Check - HttpGet healthCheckRequest = new HttpGet(urlBase+"/api/health_check"); + HttpGet healthCheckRequest = new HttpGet(urlBase+"/health_check"); HttpResponse healthCheckResponse = client.execute(healthCheckRequest); - assertEquals(healthCheckResponse.getStatusLine().getStatusCode(), 200); + assertEquals(200, healthCheckResponse.getStatusLine().getStatusCode()); } private String makeConfigFile( From a9a7f54a96d12c0be6c6b819c5d41f4dcb7a95af Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 25 Jun 2020 16:43:57 +0100 Subject: [PATCH 36/50] Handle trailing slash on status and health_check --- .../server/HealthCheckHandler.java | 6 ++++- .../ic/wlgitbridge/server/StatusHandler.java | 6 ++++- .../WLGitBridgeIntegrationTest.java | 23 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java index 120b2d6163..d251bd9ae5 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java @@ -28,7 +28,11 @@ public class HealthCheckHandler extends AbstractHandler { HttpServletRequest request, HttpServletResponse response ) throws IOException { - if ("GET".equals(baseRequest.getMethod()) && "/health_check".equals(target)) { + if ( + "GET".equals(baseRequest.getMethod()) + && target != null + && target.matches("^\\/health_check\\/?$") + ) { Log.info("GET <- /health_check"); baseRequest.setHandled(true); response.setContentType("text/plain"); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java index 802db35bd1..b205fa46ca 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java @@ -28,7 +28,11 @@ public class StatusHandler extends AbstractHandler { HttpServletRequest request, HttpServletResponse response ) throws IOException { - if ("GET".equals(baseRequest.getMethod()) && "/status".equals(target)) { + if ( + "GET".equals(baseRequest.getMethod()) + && target != null + && target.matches("^\\/status\\/?$") + ) { Log.info("GET <- /status"); baseRequest.setHandled(true); response.setContentType("text/plain"); diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 482fe8de8d..29356de856 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -921,6 +921,29 @@ public class WLGitBridgeIntegrationTest { assertEquals(200, healthCheckResponse.getStatusLine().getStatusCode()); } + @Test + public void testStatusAndHealthCheckEndpointsWithTrailingSlash() throws ClientProtocolException, IOException { + int gitBridgePort = 33888; + int mockServerPort = 3888; + server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); + server.start(); + server.setState(states.get("canCloneARepository").get("state")); + wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + wlgb.run(); + HttpClient client = HttpClients.createDefault(); + String urlBase = "http://127.0.0.1:" + gitBridgePort; + // Status + HttpGet statusRequest = new HttpGet(urlBase+"/status/"); + HttpResponse statusResponse = client.execute(statusRequest); + assertEquals(200, statusResponse.getStatusLine().getStatusCode()); + // Health Check + HttpGet healthCheckRequest = new HttpGet(urlBase+"/health_check/"); + HttpResponse healthCheckResponse = client.execute(healthCheckRequest); + assertEquals(200, healthCheckResponse.getStatusLine().getStatusCode()); + } + private String makeConfigFile( int port, int apiPort From 5e31a11938205543de1503bace7594dc85c8cd0d Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 26 Jun 2020 11:54:18 +0100 Subject: [PATCH 37/50] Respond to HEAD request in /status, /health_check --- .../server/HealthCheckHandler.java | 5 ++-- .../ic/wlgitbridge/server/StatusHandler.java | 6 +++-- .../WLGitBridgeIntegrationTest.java | 24 +++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java index d251bd9ae5..819217befa 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java @@ -28,12 +28,13 @@ public class HealthCheckHandler extends AbstractHandler { HttpServletRequest request, HttpServletResponse response ) throws IOException { + String method = baseRequest.getMethod(); if ( - "GET".equals(baseRequest.getMethod()) + ("GET".equals(method) || "HEAD".equals(method)) && target != null && target.matches("^\\/health_check\\/?$") ) { - Log.info("GET <- /health_check"); + Log.info(method + " <- /health_check"); baseRequest.setHandled(true); response.setContentType("text/plain"); if (bridge.healthCheck()) { diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java index b205fa46ca..50f44724d0 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java @@ -12,6 +12,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Arrays; public class StatusHandler extends AbstractHandler { @@ -28,12 +29,13 @@ public class StatusHandler extends AbstractHandler { HttpServletRequest request, HttpServletResponse response ) throws IOException { + String method = baseRequest.getMethod(); if ( - "GET".equals(baseRequest.getMethod()) + ("GET".equals(method) || "HEAD".equals(method)) && target != null && target.matches("^\\/status\\/?$") ) { - Log.info("GET <- /status"); + Log.info(method + " <- /status"); baseRequest.setHandled(true); response.setContentType("text/plain"); response.setStatus(200); diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 29356de856..d60e5df995 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -9,6 +9,7 @@ import org.apache.http.client.ClientProtocolException; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpHead; import org.apache.http.impl.client.HttpClients; import org.asynchttpclient.*; import org.eclipse.jgit.api.errors.GitAPIException; @@ -944,6 +945,29 @@ public class WLGitBridgeIntegrationTest { assertEquals(200, healthCheckResponse.getStatusLine().getStatusCode()); } + @Test + public void testStatusAndHealthCheckEndpointsWithHead() throws ClientProtocolException, IOException { + int gitBridgePort = 33889; + int mockServerPort = 3889; + server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); + server.start(); + server.setState(states.get("canCloneARepository").get("state")); + wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + wlgb.run(); + HttpClient client = HttpClients.createDefault(); + String urlBase = "http://127.0.0.1:" + gitBridgePort; + // Status + HttpHead statusRequest = new HttpHead(urlBase+"/status"); + HttpResponse statusResponse = client.execute(statusRequest); + assertEquals(200, statusResponse.getStatusLine().getStatusCode()); + // Health Check + HttpHead healthCheckRequest = new HttpHead(urlBase+"/health_check"); + HttpResponse healthCheckResponse = client.execute(healthCheckRequest); + assertEquals(200, healthCheckResponse.getStatusLine().getStatusCode()); + } + private String makeConfigFile( int port, int apiPort From 41614166598da411a4fe4bf7edd23cf4bccfba46 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 26 Jun 2020 13:37:09 +0100 Subject: [PATCH 38/50] Update src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java Co-authored-by: John Lees-Miller --- .../java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java index 819217befa..226fb259fe 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/HealthCheckHandler.java @@ -32,7 +32,7 @@ public class HealthCheckHandler extends AbstractHandler { if ( ("GET".equals(method) || "HEAD".equals(method)) && target != null - && target.matches("^\\/health_check\\/?$") + && target.matches("^/health_check/?$") ) { Log.info(method + " <- /health_check"); baseRequest.setHandled(true); From e7b19ea4a04ca48ae01344c4662a971129e28cf3 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 26 Jun 2020 13:37:18 +0100 Subject: [PATCH 39/50] Update src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java Co-authored-by: John Lees-Miller --- .../main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java index 50f44724d0..45acfe12f0 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/StatusHandler.java @@ -33,7 +33,7 @@ public class StatusHandler extends AbstractHandler { if ( ("GET".equals(method) || "HEAD".equals(method)) && target != null - && target.matches("^\\/status\\/?$") + && target.matches("^/status/?$") ) { Log.info(method + " <- /status"); baseRequest.setHandled(true); From f7125b91596b61b04d11aa4a94b9e2842f062323 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 5 Aug 2020 10:08:40 +0100 Subject: [PATCH 40/50] Handle Git LFS requests, with error message We don't support Git LFS. This change adds a handler for POST requests to ".git/info/lfs/objects/batch", and sends back a 406 response, with json data that the client can use to print a nice error message. --- .../wlgitbridge/server/GitBridgeServer.java | 1 + .../ic/wlgitbridge/server/GitLfsHandler.java | 46 +++++++++++++++++++ .../WLGitBridgeIntegrationTest.java | 27 +++++++++++ 3 files changed, 74 insertions(+) create mode 100644 services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java index 7424c98056..0b25bf99a7 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java @@ -130,6 +130,7 @@ public class GitBridgeServer { HandlerCollection handlers = new HandlerList(); handlers.addHandler(new StatusHandler(bridge)); handlers.addHandler(new HealthCheckHandler(bridge)); + handlers.addHandler(new GitLfsHandler(bridge)); base.setHandler(handlers); return base; } diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java new file mode 100644 index 0000000000..5f88e880b1 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java @@ -0,0 +1,46 @@ +package uk.ac.ic.wlgitbridge.server; + +import org.eclipse.jetty.server.HttpConnection; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.ic.wlgitbridge.bridge.Bridge; +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; +import java.util.Arrays; + +public class GitLfsHandler extends AbstractHandler { + + private final Bridge bridge; + + public GitLfsHandler(Bridge bridge) { + this.bridge = bridge; + } + + @Override + public void handle( + String target, + Request baseRequest, + HttpServletRequest request, + HttpServletResponse response + ) throws IOException { + String method = baseRequest.getMethod(); + if ( + ("POST".equals(method)) + && target != null + && target.matches("^/[0-9a-z]+\\.git/info/lfs/objects/batch/?$") + ) { + Log.info(method + " <- /.git/info/lfs/objects/batch"); + response.setContentType("application/vnd.git-lfs+json"); + response.setStatus(406); + response.getWriter().println("{\"message\": \"ERROR: Git LFS is not supported on Overleaf\"}"); + baseRequest.setHandled(true); + } + } + +} diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index d60e5df995..b91f75e323 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -5,12 +5,18 @@ import org.apache.commons.io.IOUtils; import static org.asynchttpclient.Dsl.*; import org.apache.http.HttpResponse; +import org.apache.http.ParseException; import org.apache.http.client.ClientProtocolException; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpHead; import org.apache.http.impl.client.HttpClients; +import org.apache.http.HttpEntity; +import org.apache.http.util.EntityUtils; +import org.apache.http.ParseException; + import org.asynchttpclient.*; import org.eclipse.jgit.api.errors.GitAPIException; import org.junit.After; @@ -968,6 +974,27 @@ public class WLGitBridgeIntegrationTest { assertEquals(200, healthCheckResponse.getStatusLine().getStatusCode()); } + @Test + public void gitLfsBatchEndpoint() throws ClientProtocolException, IOException, ParseException { + int gitBridgePort = 33890; + int mockServerPort = 3890; + server = new MockSnapshotServer(mockServerPort, getResource("/canCloneARepository").toFile()); + server.start(); + server.setState(states.get("canCloneARepository").get("state")); + wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + wlgb.run(); + HttpClient client = HttpClients.createDefault(); + String urlBase = "http://127.0.0.1:" + gitBridgePort; + HttpPost gitLfsRequest = new HttpPost(urlBase+"/5f2419407929eb0026641967.git/info/lfs/objects/batch"); + HttpResponse gitLfsResponse = client.execute(gitLfsRequest); + assertEquals(406, gitLfsResponse.getStatusLine().getStatusCode()); + HttpEntity entity = gitLfsResponse.getEntity(); + String responseString = EntityUtils.toString(entity, "UTF-8"); + assertTrue(responseString.contains("Git LFS is not supported on Overleaf")); + } + private String makeConfigFile( int port, int apiPort From 2b8f3f4de8e872b97cd385513fc2710b8179addf Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 5 Aug 2020 13:00:20 +0100 Subject: [PATCH 41/50] Use 422 status when rejecting Git LFS request --- .../main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java | 2 +- .../ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java index 5f88e880b1..2fde1e6460 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitLfsHandler.java @@ -37,7 +37,7 @@ public class GitLfsHandler extends AbstractHandler { ) { Log.info(method + " <- /.git/info/lfs/objects/batch"); response.setContentType("application/vnd.git-lfs+json"); - response.setStatus(406); + response.setStatus(422); response.getWriter().println("{\"message\": \"ERROR: Git LFS is not supported on Overleaf\"}"); baseRequest.setHandled(true); } diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index b91f75e323..9fcef7ccf3 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -989,7 +989,7 @@ public class WLGitBridgeIntegrationTest { String urlBase = "http://127.0.0.1:" + gitBridgePort; HttpPost gitLfsRequest = new HttpPost(urlBase+"/5f2419407929eb0026641967.git/info/lfs/objects/batch"); HttpResponse gitLfsResponse = client.execute(gitLfsRequest); - assertEquals(406, gitLfsResponse.getStatusLine().getStatusCode()); + assertEquals(422, gitLfsResponse.getStatusLine().getStatusCode()); HttpEntity entity = gitLfsResponse.getEntity(); String responseString = EntityUtils.toString(entity, "UTF-8"); assertTrue(responseString.contains("Git LFS is not supported on Overleaf")); From bb7831012bbff4770145477f9ecd0ef729af70ef Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 1 Sep 2020 13:28:35 +0100 Subject: [PATCH 42/50] When logging IOException for put, include error --- .../src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index c5b82cb142..64906a6b46 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -473,7 +473,7 @@ public class Bridge { ); throw e; } catch (IOException e) { - Log.warn("[{}] IOException on put", projectName); + Log.warn("[{}] IOException on put: {}", projectName, e); throw e; } From d07ecd2a1b0d1ae12397bce1dfb2fe1355e6ff10 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 23 Sep 2020 11:38:22 +0100 Subject: [PATCH 43/50] During pull: reset repo before writing data --- .../java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java | 1 + 1 file changed, 1 insertion(+) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java index 6f020d79ad..5c7ac2cb07 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java @@ -240,6 +240,7 @@ public class GitProjectRepo implements ProjectRepo { ) throws IOException, GitAPIException { Preconditions.checkState(repository.isPresent()); Repository repo = getJGitRepository(); + resetHard(); String name = getProjectName(); Log.info("[{}] Writing commit", name); contents.write(); From 935770e2ab0db3bcda0ee60fbd6b26ed0b7e76a3 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 24 Sep 2020 11:24:47 +0100 Subject: [PATCH 44/50] Add test for reset before pull --- .../WLGitBridgeIntegrationTest.java | 32 +++++++++++++ .../base/state.json | 41 +++++++++++++++++ .../base/testproj/main.tex | 1 + .../base/testproj/sub/.gitignore | 1 + .../withUpdatedMainFile/state.json | 45 +++++++++++++++++++ .../withUpdatedMainFile/testproj/main.tex | 2 + .../testproj/sub/.gitignore | 1 + 7 files changed, 123 insertions(+) create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/state.json create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/main.tex create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/sub/.gitignore create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/state.json create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/main.tex create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/.gitignore diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 9fcef7ccf3..5aa554aab8 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -147,6 +147,10 @@ public class WLGitBridgeIntegrationTest { put("cannotCloneAHasDotGitProject", new HashMap() {{ put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/cannotCloneAHasDotGitProject/state/state.json")).build()); }}); + put("canPullIgnoredForceAddedFile", new HashMap() {{ + put("base", new SnapshotAPIStateBuilder(getResourceAsStream("/canPullIgnoredForceAddedFile/base/state.json")).build()); + put("withUpdatedMainFile", new SnapshotAPIStateBuilder(getResourceAsStream("/canPullIgnoredForceAddedFile/withUpdatedMainFile/state.json")).build()); + }}); }}; @Rule @@ -995,6 +999,34 @@ public class WLGitBridgeIntegrationTest { assertTrue(responseString.contains("Git LFS is not supported on Overleaf")); } + @Test + public void canPullIgnoredForceAddedFile() throws IOException, InterruptedException { + int gitBridgePort = 33891; + int mockServerPort = 3891; + server = new MockSnapshotServer(mockServerPort, getResource("/canPullIgnoredForceAddedFile").toFile()); + server.start(); + server.setState(states.get("canPullIgnoredForceAddedFile").get("base")); + wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + wlgb.run(); + File testProjDir = gitClone("testproj", gitBridgePort, dir); + File one = new File(testProjDir, "sub/one.txt"); + one.createNewFile(); + FileWriter fw = new FileWriter(one.getPath()); + fw.write("1"); + fw.close(); + assertEquals(0, runtime.exec( + "git add -A -f", null, testProjDir + ).waitFor()); + gitCommit(testProjDir, "push"); + gitPush(testProjDir); + server.setState(states.get("canPullIgnoredForceAddedFile").get("withUpdatedMainFile")); + gitPull(testProjDir); + File f = new File(testProjDir.getPath() + "/sub/one.txt"); + assertTrue(f.exists()); + } + private String makeConfigFile( int port, int apiPort diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/state.json new file mode 100644 index 0000000000..3a099a5fc0 --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/state.json @@ -0,0 +1,41 @@ +[ + { + "project": "testproj", + "getDoc": { + "versionID": 1, + "createdAt": "2014-11-30T18:40:58.123Z", + "email": "jdleesmiller+1@gmail.com", + "name": "John+1" + }, + "getSavedVers": [ + { + "versionID": 1, + "comment": "init", + "email": "jdleesmiller+1@gmail.com", + "name": "John+1", + "createdAt": "2014-11-30T18:47:01.456Z" + } + ], + "getForVers": [ + { + "versionID": 1, + "srcs": [ + { + "content": "content\n", + "path": "main.tex" + }, + { + "content": "*.txt", + "path": "sub/.gitignore" + } + ], + "atts": [] + } + ], + "push": "success", + "postback": { + "type": "success", + "versionID": 2 + } + } +] diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/main.tex b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/main.tex new file mode 100644 index 0000000000..d95f3ad14d --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/main.tex @@ -0,0 +1 @@ +content diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/sub/.gitignore b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/sub/.gitignore new file mode 100644 index 0000000000..2211df63dd --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/base/testproj/sub/.gitignore @@ -0,0 +1 @@ +*.txt diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/state.json new file mode 100644 index 0000000000..393f5a253a --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/state.json @@ -0,0 +1,45 @@ +[ + { + "project": "testproj", + "getDoc": { + "versionID": 5, + "createdAt": "2014-11-30T18:40:58.123Z", + "email": "jdleesmiller+1@gmail.com", + "name": "John+1" + }, + "getSavedVers": [ + { + "versionID": 5, + "comment": "init", + "email": "jdleesmiller+1@gmail.com", + "name": "John+1", + "createdAt": "2014-11-30T18:47:01.456Z" + } + ], + "getForVers": [ + { + "versionID": 5, + "srcs": [ + { + "content": "content\nupdated\n", + "path": "main.tex" + }, + { + "content": "*.txt", + "path": "sub/.gitignore" + }, + { + "content": "1", + "path": "sub/one.txt" + } + ], + "atts": [] + } + ], + "push": "success", + "postback": { + "type": "success", + "versionID": 5 + } + } +] diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/main.tex b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/main.tex new file mode 100644 index 0000000000..3aa9d51a9f --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/main.tex @@ -0,0 +1,2 @@ +content +updated diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/.gitignore b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/.gitignore new file mode 100644 index 0000000000..2211df63dd --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/.gitignore @@ -0,0 +1 @@ +*.txt From 03af20113fe2bf940b330896009085d868c75056 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 24 Sep 2020 13:29:39 +0100 Subject: [PATCH 45/50] Force add the test file, sigh. --- .../withUpdatedMainFile/testproj/sub/one.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/one.txt diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/one.txt b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/one.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/canPullIgnoredForceAddedFile/withUpdatedMainFile/testproj/sub/one.txt @@ -0,0 +1 @@ +1 From c6cfd51fb4f048576c275077d96cdb5d1a361545 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 29 Sep 2020 12:44:33 +0100 Subject: [PATCH 46/50] Use the NoGitignoreIterator recursively --- .../wlgitbridge/bridge/repo/NoGitignoreIterator.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/NoGitignoreIterator.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/NoGitignoreIterator.java index 065aa6d024..64103c075e 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/NoGitignoreIterator.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/NoGitignoreIterator.java @@ -1,6 +1,7 @@ package uk.ac.ic.wlgitbridge.bridge.repo; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.WorkingTreeIterator; import org.eclipse.jgit.treewalk.WorkingTreeOptions; @@ -64,6 +65,8 @@ public class NoGitignoreIterator extends FileTreeIterator { super(p, root, fs, fileModeStrategy); } + // Note: the `list` is a list of top-level entities in this directory, + // not a full list of files in the tree. @Override protected void init(Entry[] list) { super.init(list); @@ -74,4 +77,11 @@ public class NoGitignoreIterator extends FileTreeIterator { } } + // When entering a sub-directory, create a new instance of this class, + // so we can also ignore gitignore specifications in sub-directories + @Override + protected AbstractTreeIterator enterSubtree() { + String fullPath = getDirectory().getAbsolutePath() + "/" + current().getName(); + return new NoGitignoreIterator(this, new File(fullPath), fs, fileModeStrategy); + } } From de23035e809d3e4fa681232fdfecc3943da2e16f Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 29 Sep 2020 13:00:36 +0100 Subject: [PATCH 47/50] Add test for pulling ignored file --- .../WLGitBridgeIntegrationTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 5aa554aab8..5ee773bede 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -1027,6 +1027,24 @@ public class WLGitBridgeIntegrationTest { assertTrue(f.exists()); } + @Test + public void canPullIgnoredFileFromOverleaf() throws IOException, InterruptedException { + int gitBridgePort = 33892; + int mockServerPort = 3892; + server = new MockSnapshotServer(mockServerPort, getResource("/canPullIgnoredForceAddedFile").toFile()); + server.start(); + server.setState(states.get("canPullIgnoredForceAddedFile").get("base")); + wlgb = new GitBridgeApp(new String[] { + makeConfigFile(gitBridgePort, mockServerPort) + }); + wlgb.run(); + File testProjDir = gitClone("testproj", gitBridgePort, dir); + server.setState(states.get("canPullIgnoredForceAddedFile").get("withUpdatedMainFile")); + gitPull(testProjDir); + File f = new File(testProjDir.getPath() + "/sub/one.txt"); + assertTrue(f.exists()); + } + private String makeConfigFile( int port, int apiPort From f89fab5abbb4fd0ff0830de2dea50b79cb92b20b Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 22 Sep 2020 14:46:16 +0100 Subject: [PATCH 48/50] Upgrade to JGit 5.9.0 --- services/git-bridge/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/pom.xml b/services/git-bridge/pom.xml index a63bab6106..3cfe0454b7 100644 --- a/services/git-bridge/pom.xml +++ b/services/git-bridge/pom.xml @@ -100,13 +100,13 @@ org.eclipse.jgit org.eclipse.jgit - 5.2.1.201812262042-r + 5.9.0.202009080501-r org.eclipse.jgit org.eclipse.jgit.http.server - 5.2.1.201812262042-r + 5.9.0.202009080501-r From 2430d5fe1de3f51641a6c76a157ca3a5be083893 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Fri, 4 Dec 2020 11:05:47 -0500 Subject: [PATCH 49/50] Handle errors from the history service If the history service returns a non-success status code when we request a blob, chances are the payload is not the expected blob contents. We throw an exception in that case, which will abort the git operation. --- .../ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java index 41c4bc73f3..e9d7f5f789 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/io/http/ning/NingHttpClient.java @@ -52,11 +52,16 @@ public class NingHttpClient implements NingHttpClientFacade { @Override public byte[] onCompleted( Response response - ) throws IOException { + ) throws Exception { + int statusCode = response.getStatusCode(); + if (statusCode >= 400) { + throw new Exception("got status " + statusCode + + " fetching " + url); + } byte[] ret = bytes.toByteArray(); bytes.close(); log.info( - response.getStatusCode() + statusCode + " " + response.getStatusText() + " (" From 8bbeee0f8d57a7246e51c13bb411fabad7e4eff9 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Fri, 4 Dec 2020 14:42:21 -0500 Subject: [PATCH 50/50] Strip token from blob URLs when using cache Blob URLs coming from web may now contain a token for authentication with history v1. This token will change every request, which makes the URL not suitable as a cache key. Removing the token fixes that. --- .../bridge/resource/UrlResourceCache.java | 21 ++++++++++++++++-- .../bridge/resource/UrlResourceCacheTest.java | 22 ++++++++++++++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java index e3c7911b6e..29a0cbea57 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java @@ -43,7 +43,7 @@ public class UrlResourceCache implements ResourceCache { Map fetchedUrls, Optional maxFileSize ) throws IOException, SizeLimitExceededException { - String path = dbStore.getPathForURLInProject(projectName, url); + String path = dbStore.getPathForURLInProject(projectName, getCacheKeyFromUrl(url)); byte[] contents; if (path == null) { path = newPath; @@ -118,8 +118,25 @@ public class UrlResourceCache implements ResourceCache { throw new SizeLimitExceededException( Optional.of(path), contents.length, maxFileSize.get()); } - dbStore.addURLIndexForProject(projectName, url, path); + dbStore.addURLIndexForProject(projectName, getCacheKeyFromUrl(url), path); return contents; } + /** + * Construct a suitable cache key from the given file URL. + * + * The file URL returned by the web service may contain a token parameter + * used for authentication. This token changes for every request, so we + * need to strip it from the query string before using the URL as a cache + * key. + */ + private String getCacheKeyFromUrl(String url) { + // We're not doing proper URL parsing here, but it should be enough to + // remove the token without touching the important parts of the URL. + // + // The URL looks like: + // + // https://history.overleaf.com/api/projects/:project_id/blobs/:hash?token=:token&_path=:path + return url.replaceAll("token=[^&]*", "token=REMOVED"); + } } diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java index e6a93d8a30..1c814770cd 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java @@ -3,7 +3,7 @@ package uk.ac.ic.wlgitbridge.bridge.resource; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.DefaultHttpHeaders; import org.junit.Test; -import uk.ac.ic.wlgitbridge.bridge.db.noop.NoopDbStore; +import uk.ac.ic.wlgitbridge.bridge.db.DBStore; import uk.ac.ic.wlgitbridge.bridge.util.CastUtil; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClientFacade; @@ -17,6 +17,7 @@ import java.util.concurrent.ExecutionException; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; public class UrlResourceCacheTest { @@ -28,8 +29,9 @@ public class UrlResourceCacheTest { private final NingHttpClientFacade http = mock(NingHttpClientFacade.class); - private final UrlResourceCache cache - = new UrlResourceCache(new NoopDbStore(), http); + private final DBStore dbStore = mock(DBStore.class); + + private final UrlResourceCache cache = new UrlResourceCache(dbStore, http); private static HttpHeaders withContentLength(long cl) { return new DefaultHttpHeaders().add("Content-Length", String.valueOf(cl)); @@ -57,6 +59,10 @@ public class UrlResourceCacheTest { PROJ, URL, NEW_PATH, new HashMap<>(), new HashMap<>(), max); } + private void getUrl(String url) throws IOException, SizeLimitExceededException { + cache.get(PROJ, url, NEW_PATH, new HashMap<>(), new HashMap<>(), Optional.empty()); + } + private void getWithMaxLength(long max) throws IOException, SizeLimitExceededException { getWithMaxLength(Optional.of(max)); @@ -104,4 +110,14 @@ public class UrlResourceCacheTest { getWithMaxLength(5); } + @Test + public void tokenIsRemovedFromCacheKey() throws Exception { + String url = "http://history.overleaf.com/projects/1234/blobs/abdef?token=secretencryptedstuff&_path=test.tex"; + String cacheKey = "http://history.overleaf.com/projects/1234/blobs/abdef?token=REMOVED&_path=test.tex"; + respondWithContentLength(123); + getUrl(url); + verify(dbStore).getPathForURLInProject(PROJ, cacheKey); + verify(dbStore).addURLIndexForProject(PROJ, cacheKey, NEW_PATH); + } + }