Better javadoc, improve handling of submodules

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

@@ -678,7 +678,7 @@ public class Bridge {
     private void makeCommitsFromSnapshots(
             ProjectRepo repo,
             Collection<Snapshot> snapshots
-    ) throws IOException, SizeLimitExceededException {
+    ) throws IOException, GitUserException {
         String name = repo.getProjectName();
         for (Snapshot snapshot : snapshots) {
             Map<String, RawFile> fileTable = repo.getFiles();

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java

@@ -9,6 +9,7 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
 import uk.ac.ic.wlgitbridge.data.filestore.GitDirectoryContents;
 import uk.ac.ic.wlgitbridge.data.filestore.RawFile;
+import uk.ac.ic.wlgitbridge.git.exception.GitUserException;
 import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException;
 import uk.ac.ic.wlgitbridge.git.util.RepositoryObjectTreeWalker;
 import uk.ac.ic.wlgitbridge.util.Log;
@@ -62,7 +63,7 @@ public class GitProjectRepo implements ProjectRepo {
 
     @Override
     public Map<String, RawFile> getFiles()
-            throws IOException, SizeLimitExceededException {
+            throws IOException, GitUserException {
         Preconditions.checkState(repository.isPresent());
         return new RepositoryObjectTreeWalker(
                 repository.get()

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/ProjectRepo.java

@@ -2,6 +2,7 @@ package uk.ac.ic.wlgitbridge.bridge.repo;
 
 import uk.ac.ic.wlgitbridge.data.filestore.GitDirectoryContents;
 import uk.ac.ic.wlgitbridge.data.filestore.RawFile;
+import uk.ac.ic.wlgitbridge.git.exception.GitUserException;
 import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException;
 
 import java.io.IOException;
@@ -24,7 +25,7 @@ public interface ProjectRepo {
     ) throws IOException;
 
     Map<String, RawFile> getFiles(
-    ) throws IOException, SizeLimitExceededException;
+    ) throws IOException, GitUserException;
 
     Collection<String> commitAndGetMissing(
             GitDirectoryContents gitDirectoryContents

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/InvalidGitRepository.java

@@ -0,0 +1,22 @@
+package uk.ac.ic.wlgitbridge.git.exception;
+
+import java.util.Arrays;
+import java.util.List;
+
+public class InvalidGitRepository extends GitUserException {
+
+    @Override
+    public String getMessage() {
+        return "invalid git repo";
+    }
+
+    @Override
+    public List<String> getDescriptionLines() {
+        return Arrays.asList(
+                "Your Git repository is invalid.",
+                "If your project contains a Git submodule,",
+                "please remove it and try again."
+        );
+    }
+
+}

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java

@@ -74,7 +74,7 @@ public class WriteLatexPutHook implements PreReceiveHook {
                 );
             } catch (OutOfDateException e) {
                 receiveCommand.setResult(Result.REJECTED_NONFASTFORWARD);
-            } catch (SnapshotPostException e) {
+            } catch (GitUserException e) {
                 handleSnapshotPostException(receivePack, receiveCommand, e);
             } catch (Throwable t) {
                 Log.warn("Throwable on pre receive: ", t);
@@ -90,7 +90,7 @@ public class WriteLatexPutHook implements PreReceiveHook {
     private void handleSnapshotPostException(
             ReceivePack receivePack,
             ReceiveCommand receiveCommand,
-            SnapshotPostException e
+            GitUserException e
     ) {
         String message = e.getMessage();
         receivePack.sendError(message);

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/util/RepositoryObjectTreeWalker.java

@@ -8,6 +8,7 @@ import org.eclipse.jgit.treewalk.TreeWalk;
 import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory;
 import uk.ac.ic.wlgitbridge.data.filestore.RawFile;
 import uk.ac.ic.wlgitbridge.data.filestore.RepositoryFile;
+import uk.ac.ic.wlgitbridge.git.exception.InvalidGitRepository;
 import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException;
 
 import java.io.IOException;
@@ -44,7 +45,7 @@ public class RepositoryObjectTreeWalker {
     }
 
     public RawDirectory getDirectoryContents(
-    ) throws IOException, SizeLimitExceededException {
+    ) throws IOException, SizeLimitExceededException, InvalidGitRepository {
         return new RawDirectory(walkGitObjectTree());
     }
 
@@ -63,7 +64,7 @@ public class RepositoryObjectTreeWalker {
     }
 
     private Map<String, RawFile> walkGitObjectTree(
-    ) throws IOException, SizeLimitExceededException {
+    ) throws IOException, SizeLimitExceededException, InvalidGitRepository {
         Map<String, RawFile> fileContentsTable = new HashMap<>();
         if (treeWalk == null) {
             return fileContentsTable;
@@ -71,9 +72,13 @@ public class RepositoryObjectTreeWalker {
         while (treeWalk.next()) {
             String path = treeWalk.getPathString();
 
+            ObjectId objectId = treeWalk.getObjectId(0);
+            if (!repository.hasObject(objectId)) {
+                throw new InvalidGitRepository();
+            }
             try {
                 byte[] content = repository.open(
-                        treeWalk.getObjectId(0)
+                        objectId
                 ).getBytes();
                 fileContentsTable.put(path, new RepositoryFile(path, content));
             } catch (LargeObjectException e) {

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

@@ -8,6 +8,7 @@ import uk.ac.ic.wlgitbridge.application.config.Oauth2;
 import uk.ac.ic.wlgitbridge.snapshot.base.ForbiddenException;
 import uk.ac.ic.wlgitbridge.snapshot.getdoc.GetDocRequest;
 import uk.ac.ic.wlgitbridge.util.Instance;
+import uk.ac.ic.wlgitbridge.util.Log;
 import uk.ac.ic.wlgitbridge.util.Util;
 
 import javax.servlet.*;
@@ -34,6 +35,17 @@ public class Oauth2Filter implements Filter {
     @Override
     public void init(FilterConfig filterConfig) {}
 
+    /**
+     * The original request from git will not contain the Authorization header.
+     *
+     * So, for projects that need auth, we return 401. Git will swallow this
+     * and prompt the user for user/pass, and then make a brand new request.
+     * @param servletRequest
+     * @param servletResponse
+     * @param filterChain
+     * @throws IOException
+     * @throws ServletException
+     */
     @Override
     public void doFilter(
             ServletRequest servletRequest,
@@ -44,24 +56,29 @@ public class Oauth2Filter implements Filter {
                 ((Request) servletRequest).getRequestURI().split("/")[1],
                 ".git"
         );
+        Log.info("[{}] Checking if auth needed", project);
         GetDocRequest doc = new GetDocRequest(project);
         doc.request();
         try {
             doc.getResult();
         } catch (ForbiddenException e) {
+            Log.info("[{}] Auth needed", project);
             getAndInjectCredentials(
+                    project,
                     servletRequest,
                     servletResponse,
                     filterChain
             );
             return;
         }
+        Log.info("[{}] Auth not needed");
         filterChain.doFilter(servletRequest, servletResponse);
     }
 
     // TODO: this is ridiculous. Check for error cases first, then return/throw
     // TODO: also, use an Optional credential, since we treat it as optional
     private void getAndInjectCredentials(
+            String projectName,
             ServletRequest servletRequest,
             ServletResponse servletResponse,
             FilterChain filterChain
@@ -71,6 +88,7 @@ public class Oauth2Filter implements Filter {
 
         String authHeader = request.getHeader("Authorization");
         if (authHeader != null) {
+            Log.info("[{}] Authorization header present");
             StringTokenizer st = new StringTokenizer(authHeader);
             if (st.hasMoreTokens()) {
                 String basic = st.nextToken();
@@ -100,10 +118,9 @@ public class Oauth2Filter implements Filter {
                                                 oauth2.getOauth2ClientID(),
                                                 oauth2.getOauth2ClientSecret()
                                         )
-                                )
+                                ).execute().getAccessToken();
-                                        .execute().getAccessToken();
                             } catch (TokenResponseException e) {
-                                unauthorized(response);
+                                unauthorized(projectName, response);
                                 return;
                             }
                             final Credential cred = new Credential.Builder(
@@ -118,7 +135,7 @@ public class Oauth2Filter implements Filter {
                                     servletResponse
                             );
                         } else {
-                            unauthorized(response);
+                            unauthorized(projectName, response);
                         }
                     } catch (UnsupportedEncodingException e) {
                         throw new Error("Couldn't retrieve authentication", e);
@@ -126,7 +143,7 @@ public class Oauth2Filter implements Filter {
                 }
             }
         } else {
-            unauthorized(response);
+            unauthorized(projectName, response);
         }
     }
 
@@ -134,8 +151,10 @@ public class Oauth2Filter implements Filter {
     public void destroy() {}
 
     private void unauthorized(
+            String projectName,
             ServletResponse servletResponse
     ) throws IOException {
+        Log.info("[{}] Unauthorized", projectName);
         HttpServletResponse response = (HttpServletResponse) servletResponse;
         response.setContentType("text/plain");
         response.setHeader("WWW-Authenticate", "Basic realm=\"Git Bridge\"");

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/SnapshotAPIRequest.java

@@ -39,6 +39,13 @@ public abstract class SnapshotAPIRequest<T extends Result> extends Request<T> {
                 ).intercept(request1);
                 oauth2.intercept(request1);
             });
+        } else {
+            request.setInterceptor(request1 -> {
+                new BasicAuthentication(
+                        USERNAME,
+                        PASSWORD
+                ).intercept(request1);
+            });
         }
     }
 

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

@@ -107,6 +107,9 @@ public class WLGitBridgeIntegrationTest {
         put("wlgbCanSwapProjects", new HashMap<String, SnapshotAPIState>() {{
             put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/wlgbCanSwapProjects/state/state.json")).build());
         }});
+        put("pushSubmoduleFailsWithInvalidGitRepo", new HashMap<String, SnapshotAPIState>() {{
+            put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/pushSubmoduleFailsWithInvalidGitRepo/state/state.json")).build());
+        }});
     }};
 
     @Rule
@@ -624,6 +627,43 @@ public class WLGitBridgeIntegrationTest {
         wlgb.stop();
     }
 
+    private static final List<String> EXPECTED_OUT_PUSH_SUBMODULE = Arrays.asList(
+            "remote: hint: Your Git repository is invalid.",
+            "remote: hint: If your project contains a Git submodule,",
+            "remote: hint: please remove it and try again.",
+            "To http://127.0.0.1:33875/testproj.git",
+            "! [remote rejected] master -> master (invalid git repo)",
+            "error: failed to push some refs to 'http://127.0.0.1:33875/testproj.git'"
+    );
+
+    @Test
+    public void pushSubmoduleFailsWithInvalidGitRepo() throws IOException, GitAPIException, InterruptedException {
+        MockSnapshotServer server = new MockSnapshotServer(3875, getResource("/pushSubmoduleFailsWithInvalidGitRepo").toFile());
+        server.start();
+        server.setState(states.get("pushSubmoduleFailsWithInvalidGitRepo").get("state"));
+        GitBridgeApp wlgb = new GitBridgeApp(new String[] {
+                makeConfigFile(33875, 3875)
+        });
+        wlgb.run();
+        File dir = folder.newFolder();
+        File testprojDir = cloneRepository("testproj", 33875, dir);
+        runtime.exec("mkdir sub", null, testprojDir).waitFor();
+        File sub = new File(testprojDir, "sub");
+        runtime.exec("touch sub.txt", null, sub).waitFor();
+        runtime.exec("git init", null, sub).waitFor();
+        runtime.exec("git add -A", null, sub).waitFor();
+        runtime.exec("git commit -m \"sub\"", null, sub).waitFor();
+        runtime.exec("git add -A", null, testprojDir).waitFor();
+        runtime.exec("git commit -m \"push\"", null, testprojDir).waitFor();
+        Process gitPush = runtime.exec("git push", null, testprojDir);
+        int pushExitCode = gitPush.waitFor();
+        wlgb.stop();
+        assertEquals(1, pushExitCode);
+        List<String> actual = Util.linesFromStream(gitPush.getErrorStream(), 2, "[K");
+        assertEquals(EXPECTED_OUT_PUSH_SUBMODULE, actual);
+        wlgb.stop();
+    }
+
     private File cloneRepository(String repositoryName, int port, File dir) throws IOException, InterruptedException {
         String repo = "git clone http://127.0.0.1:" + port + "/" + repositoryName + ".git";
         Process gitProcess = runtime.exec(repo, null, dir);

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

@@ -0,0 +1,46 @@
+[
+	{
+		"project": "testproj",
+		"getDoc": {
+			"versionID": 1,
+			"createdAt": "2014-11-30T18:40:58.123Z",
+			"email": "jdleesmiller+1@gmail.com",
+			"name": "John+1"
+		},
+		"getSavedVers": [
+        	{
+        		"versionID": 1,
+        		"comment": "added more info on doc GET and error details",
+        		"email": "jdleesmiller+1@gmail.com",
+        		"name": "John+1",
+        		"createdAt": "2014-11-30T18:47:01.333Z"
+        	}
+		],
+		"getForVers": [
+			{
+				"versionID": 1,
+				"srcs": [
+                    {
+                    	"content": "content\n",
+                    	"path": "main.tex"
+                    },
+                    {
+                    	"content": "This text is from another file.",
+                    	"path": "foo/bar/test.tex"
+                    }
+	            ],
+	            "atts": [
+	            	{
+	            		"url": "http://127.0.0.1:3875/state/testproj/min_mean_wait_evm_7_eps_150dpi.png",
+	            		"path": "min_mean_wait_evm_7_eps_150dpi.png"
+	            	}
+	            ]
+	        }
+		],
+		"push": "success",
+		"postback": {
+			"type": "success",
+			"versionID": 2
+		}
+	}
+]

services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/foo/bar/test.tex

@@ -0,0 +1 @@
+This text is from another file.

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

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