diff --git a/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/GitHubConnectionTest.java b/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/GitHubConnectionTest.java new file mode 100644 index 0000000..69d522a --- /dev/null +++ b/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/GitHubConnectionTest.java @@ -0,0 +1,238 @@ +/******************************************************************************* + * Copyright (C) 2026, Philipp0205 and others + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.egit.pullrequest.internal.github; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; + +import java.io.IOException; +import java.util.List; + +import org.eclipse.egit.pullrequest.internal.client.ConnectionDiagnostics; +import org.eclipse.egit.pullrequest.internal.client.ConnectionDiagnostics.Outcome; +import org.eclipse.egit.pullrequest.internal.client.ConnectionDiagnostics.Step; +import org.eclipse.egit.pullrequest.internal.github.StubGitHubServer.Response; +import org.eclipse.egit.pullrequest.internal.model.PullRequest; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Tests how {@link GitHubClient} lists pull requests across every accessible + * owner scope when GitHub refuses to search some of them. + */ +public class GitHubConnectionTest { + + private static final String VALIDATION_FAILED = "{\"message\":" //$NON-NLS-1$ + + "\"Validation Failed\",\"errors\":[{\"message\":\"The listed " //$NON-NLS-1$ + + "users and repositories cannot be searched either because " //$NON-NLS-1$ + + "the resources do not exist or you do not have permission to " //$NON-NLS-1$ + + "view them.\",\"resource\":\"Search\",\"field\":\"q\"," //$NON-NLS-1$ + + "\"code\":\"invalid\"}],\"status\":\"422\"}"; //$NON-NLS-1$ + + private StubGitHubServer server; + + @Before + public void startServer() throws IOException { + server = new StubGitHubServer(); + server.on("/user", ok("{\"login\":\"Philipp0205\"}")); //$NON-NLS-1$ //$NON-NLS-2$ + server.on("/user/orgs", ok("[{\"login\":\"acme\"}]")); //$NON-NLS-1$ //$NON-NLS-2$ + server.on("/user/repos", ok("[]")); //$NON-NLS-1$ //$NON-NLS-2$ + server.on("/repos/Philipp0205/notes/pulls/1", //$NON-NLS-1$ + ok(pullRequest(1, "Fix the parser"))); //$NON-NLS-1$ + server.on("/repos/acme/app/pulls/4", //$NON-NLS-1$ + ok(pullRequest(4, "Bump the target platform"))); //$NON-NLS-1$ + } + + @After + public void stopServer() throws IOException { + server.close(); + } + + @Test + public void testScopeGitHubRefusesToSearchIsSkipped() throws Exception { + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/Philipp0205/notes/pulls/1"); //$NON-NLS-1$ + searchFailsWithValidationError("org%3Aacme"); //$NON-NLS-1$ + + List pullRequests = client() + .getPullRequests(null, null, null, 25, 0); + + assertThat(pullRequests, hasSize(1)); + assertThat(pullRequests.get(0).getTitle(), + equalTo("Fix the parser")); //$NON-NLS-1$ + } + + @Test + public void testDiagnosticsReportSkippedScopeAsWarningNotFailure() + throws Exception { + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/Philipp0205/notes/pulls/1"); //$NON-NLS-1$ + searchFailsWithValidationError("org%3Aacme"); //$NON-NLS-1$ + + ConnectionDiagnostics report = client().diagnoseConnection(); + + assertThat(report.toReport(), report.isSuccessful(), is(true)); + assertThat(step(report, "Read pull requests").getDetail(), //$NON-NLS-1$ + equalTo("Returned 1 pull request(s)")); //$NON-NLS-1$ + Step scopes = step(report, "Search scopes"); //$NON-NLS-1$ + assertThat(scopes.getOutcome(), equalTo(Outcome.WARNING)); + assertThat(scopes.getDetail(), containsString("org:acme")); //$NON-NLS-1$ + assertThat(scopes.getDetail(), containsString("read:org")); //$NON-NLS-1$ + } + + @Test + public void testDiagnosticsStaySilentWhenEveryScopeIsSearchable() + throws Exception { + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/Philipp0205/notes/pulls/1"); //$NON-NLS-1$ + searchReturns("org%3Aacme", "/repos/acme/app/pulls/4"); //$NON-NLS-1$ //$NON-NLS-2$ + + ConnectionDiagnostics report = client().diagnoseConnection(); + + assertThat(report.toReport(), report.isSuccessful(), is(true)); + assertThat(report.toReport(), not(containsString("Search scopes"))); //$NON-NLS-1$ + } + + @Test + public void testEveryScopeRefusedIsReportedWithTokenAdvice() { + searchFailsWithValidationError("user%3APhilipp0205"); //$NON-NLS-1$ + searchFailsWithValidationError("org%3Aacme"); //$NON-NLS-1$ + + ConnectionDiagnostics report = client().diagnoseConnection(); + + assertThat(report.isSuccessful(), is(false)); + Step read = step(report, "Read pull requests"); //$NON-NLS-1$ + assertThat(read.getOutcome(), equalTo(Outcome.FAILED)); + assertThat(read.getDetail(), containsString("user:Philipp0205")); //$NON-NLS-1$ + assertThat(read.getDetail(), containsString("org:acme")); //$NON-NLS-1$ + assertThat(read.getDetail(), containsString("read:org")); //$NON-NLS-1$ + } + + @Test + public void testOrganizationsThatCannotBeListedDoNotHideOwnPullRequests() + throws Exception { + server.on("/user/orgs", new Response(403, "Forbidden", //$NON-NLS-1$ //$NON-NLS-2$ + "{\"message\":\"Resource not accessible by integration\"}")); //$NON-NLS-1$ + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/Philipp0205/notes/pulls/1"); //$NON-NLS-1$ + + List pullRequests = client() + .getPullRequests(null, null, null, 25, 0); + + assertThat(pullRequests, hasSize(1)); + assertThat(pullRequests.get(0).getTitle(), + equalTo("Fix the parser")); //$NON-NLS-1$ + } + + @Test + public void testSearchFailuresOtherThanRefusedScopesAreReported() { + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/Philipp0205/notes/pulls/1"); //$NON-NLS-1$ + server.onQueryContaining("org%3Aacme", //$NON-NLS-1$ + new Response(503, "Service Unavailable", //$NON-NLS-1$ + "{\"message\":\"No server is currently available\"}")); //$NON-NLS-1$ + + ConnectionDiagnostics report = client().diagnoseConnection(); + + Step read = step(report, "Read pull requests"); //$NON-NLS-1$ + assertThat(read.getOutcome(), equalTo(Outcome.FAILED)); + assertThat(read.getDetail(), containsString("HTTP 503")); //$NON-NLS-1$ + } + + @Test + public void testEveryOwnerScopeIsSearchedSeparately() throws Exception { + server.on("/user/repos", //$NON-NLS-1$ + ok("[{\"full_name\":\"bob/tool\"}]")); //$NON-NLS-1$ + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/Philipp0205/notes/pulls/1"); //$NON-NLS-1$ + searchReturns("org%3Aacme", "/repos/acme/app/pulls/4"); //$NON-NLS-1$ //$NON-NLS-2$ + searchFailsWithValidationError("repo%3Abob%2Ftool"); //$NON-NLS-1$ + + List pullRequests = client() + .getPullRequests(null, null, null, 25, 0); + + assertThat(pullRequests, hasSize(2)); + assertThat(searchTargets(), hasSize(3)); + } + + @Test + public void testPullRequestFoundInTwoScopesIsListedOnce() + throws Exception { + server.on("/user/repos", //$NON-NLS-1$ + ok("[{\"full_name\":\"acme/app\"}]")); //$NON-NLS-1$ + searchReturns("user%3APhilipp0205", //$NON-NLS-1$ + "/repos/acme/app/pulls/4"); //$NON-NLS-1$ + searchReturns("org%3Aacme", "/repos/acme/app/pulls/4"); //$NON-NLS-1$ //$NON-NLS-2$ + + List pullRequests = client() + .getPullRequests(null, null, null, 25, 0); + + assertThat(pullRequests, hasSize(1)); + assertThat(pullRequests.get(0).getTitle(), + equalTo("Bump the target platform")); //$NON-NLS-1$ + } + + private GitHubClient client() { + return new GitHubClient(null, null, "token", server.url()); //$NON-NLS-1$ + } + + private void searchReturns(String encodedScope, String... pullPaths) { + StringBuilder items = new StringBuilder(); + for (String pullPath : pullPaths) { + if (items.length() > 0) { + items.append(','); + } + items.append("{\"pull_request\":{\"url\":\"") //$NON-NLS-1$ + .append(server.url()).append(pullPath).append("\"}}"); //$NON-NLS-1$ + } + server.onQueryContaining(encodedScope, + ok("{\"total_count\":" + pullPaths.length + ",\"items\":[" //$NON-NLS-1$ //$NON-NLS-2$ + + items + "]}")); //$NON-NLS-1$ + } + + private void searchFailsWithValidationError(String encodedScope) { + server.onQueryContaining(encodedScope, new Response(422, + "Unprocessable Entity", VALIDATION_FAILED)); //$NON-NLS-1$ + } + + private List searchTargets() { + return server.requestedTargets().stream() + .filter(target -> target.startsWith("/search/issues")) //$NON-NLS-1$ + .toList(); + } + + private static Response ok(String body) { + return new Response(200, "OK", body); //$NON-NLS-1$ + } + + private static String pullRequest(int number, String title) { + return "{\"number\":" + number + ",\"title\":\"" + title //$NON-NLS-1$ //$NON-NLS-2$ + + "\",\"state\":\"open\",\"body\":\"\"," //$NON-NLS-1$ + + "\"created_at\":\"2026-09-01T10:00:00Z\"," //$NON-NLS-1$ + + "\"updated_at\":\"2026-09-0" + number //$NON-NLS-1$ + + "T10:00:00Z\",\"user\":{\"login\":\"Philipp0205\"}}"; //$NON-NLS-1$ + } + + private static Step step(ConnectionDiagnostics report, String name) { + for (Step step : report.getSteps()) { + if (step.getName().equals(name)) { + return step; + } + } + throw new AssertionError( + "No step " + name + " in\n" + report.toReport()); //$NON-NLS-1$ //$NON-NLS-2$ + } +} diff --git a/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/GitHubPullRequestSearchTest.java b/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/GitHubPullRequestSearchTest.java new file mode 100644 index 0000000..57e5979 --- /dev/null +++ b/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/GitHubPullRequestSearchTest.java @@ -0,0 +1,216 @@ +/******************************************************************************* + * Copyright (C) 2026, Philipp0205 and others + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.egit.pullrequest.internal.github; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.eclipse.egit.pullrequest.internal.github.GitHubPullRequestSearch.PageReader; +import org.eclipse.egit.pullrequest.internal.github.GitHubPullRequestSearch.Result; +import org.junit.Test; + +/** + * Tests for {@link GitHubPullRequestSearch} + */ +public class GitHubPullRequestSearchTest { + + private static final String VALIDATION_FAILED = "GitHub API request " //$NON-NLS-1$ + + "failed: HTTP 422 - {\"message\":\"Validation Failed\"," //$NON-NLS-1$ + + "\"errors\":[{\"message\":\"The listed users and " //$NON-NLS-1$ + + "repositories cannot be searched either because the " //$NON-NLS-1$ + + "resources do not exist or you do not have permission to " //$NON-NLS-1$ + + "view them.\"}]}"; //$NON-NLS-1$ + + private final List requestedQueries = new ArrayList<>(); + + /** + * A reader that answers each query from a canned list of paths and fails + * every query that was registered as rejected. + */ + private class StubReader implements PageReader { + + private final Map> results = new HashMap<>(); + + private final Map failures = new HashMap<>(); + + StubReader returning(String query, String... paths) { + results.put(query, Arrays.asList(paths)); + return this; + } + + StubReader failingWith(String query, String message) { + failures.put(query, new IOException(message)); + return this; + } + + @Override + public List read(String query, int page, int pageSize) + throws IOException { + requestedQueries.add(query); + IOException failure = failures.get(query); + if (failure != null) { + throw failure; + } + if (page > 1) { + return Collections.emptyList(); + } + return results.getOrDefault(query, Collections.emptyList()); + } + } + + @Test + public void testRejectedScopeIsSkippedAndOthersStillReturnResults() + throws Exception { + StubReader reader = new StubReader() + .returning("is:pr user:alice is:open", //$NON-NLS-1$ + "/repos/alice/notes/pulls/1") //$NON-NLS-1$ + .failingWith("is:pr org:acme is:open", VALIDATION_FAILED) //$NON-NLS-1$ + .returning("is:pr repo:bob/tool is:open", //$NON-NLS-1$ + "/repos/bob/tool/pulls/9"); //$NON-NLS-1$ + + Result result = GitHubPullRequestSearch.run( + Arrays.asList("is:pr user:alice is:open", //$NON-NLS-1$ + "is:pr org:acme is:open", //$NON-NLS-1$ + "is:pr repo:bob/tool is:open"), //$NON-NLS-1$ + 1, 1, reader); + + assertThat(result.getPullRequestPaths(), + contains("/repos/alice/notes/pulls/1", //$NON-NLS-1$ + "/repos/bob/tool/pulls/9")); //$NON-NLS-1$ + assertThat(result.getUnsearchableScopes(), contains("org:acme")); //$NON-NLS-1$ + } + + @Test + public void testRejectedScopeDoesNotStopLaterQueries() throws Exception { + StubReader reader = new StubReader() + .failingWith("is:pr user:alice is:open", VALIDATION_FAILED) //$NON-NLS-1$ + .returning("is:pr org:acme is:open", //$NON-NLS-1$ + "/repos/acme/app/pulls/3"); //$NON-NLS-1$ + + GitHubPullRequestSearch.run( + Arrays.asList("is:pr user:alice is:open", //$NON-NLS-1$ + "is:pr org:acme is:open"), //$NON-NLS-1$ + 1, 1, reader); + + assertThat(requestedQueries, hasSize(2)); + } + + @Test + public void testAllScopesRejectedFailsWithActionableMessage() { + StubReader reader = new StubReader() + .failingWith("is:pr user:alice is:open", VALIDATION_FAILED) //$NON-NLS-1$ + .failingWith("is:pr org:acme is:open", VALIDATION_FAILED); //$NON-NLS-1$ + + try { + GitHubPullRequestSearch.run( + Arrays.asList("is:pr user:alice is:open", //$NON-NLS-1$ + "is:pr org:acme is:open"), //$NON-NLS-1$ + 1, 1, reader); + fail("Expected the search to fail"); //$NON-NLS-1$ + } catch (IOException e) { + assertThat(e.getMessage(), containsString("user:alice")); //$NON-NLS-1$ + assertThat(e.getMessage(), containsString("org:acme")); //$NON-NLS-1$ + assertThat(e.getMessage(), containsString("read:org")); //$NON-NLS-1$ + assertThat(e.getCause().getMessage(), + containsString("HTTP 422")); //$NON-NLS-1$ + } + } + + @Test + public void testFailuresOtherThanRejectedScopesArePropagated() { + StubReader reader = new StubReader() + .returning("is:pr user:alice is:open", //$NON-NLS-1$ + "/repos/alice/notes/pulls/1") //$NON-NLS-1$ + .failingWith("is:pr org:acme is:open", //$NON-NLS-1$ + "GitHub API request failed: HTTP 503 - unavailable"); //$NON-NLS-1$ + + try { + GitHubPullRequestSearch.run( + Arrays.asList("is:pr user:alice is:open", //$NON-NLS-1$ + "is:pr org:acme is:open"), //$NON-NLS-1$ + 1, 1, reader); + fail("Expected the search to fail"); //$NON-NLS-1$ + } catch (IOException e) { + assertThat(e.getMessage(), containsString("HTTP 503")); //$NON-NLS-1$ + } + } + + @Test + public void testPullRequestFoundByTwoScopesIsReportedOnce() + throws Exception { + StubReader reader = new StubReader() + .returning("is:pr user:alice is:open", //$NON-NLS-1$ + "/repos/alice/notes/pulls/1") //$NON-NLS-1$ + .returning("is:pr repo:alice/notes is:open", //$NON-NLS-1$ + "/repos/alice/notes/pulls/1"); //$NON-NLS-1$ + + Result result = GitHubPullRequestSearch.run( + Arrays.asList("is:pr user:alice is:open", //$NON-NLS-1$ + "is:pr repo:alice/notes is:open"), //$NON-NLS-1$ + 1, 1, reader); + + assertThat(result.getPullRequestPaths(), + contains("/repos/alice/notes/pulls/1")); //$NON-NLS-1$ + } + + @Test + public void testSuccessfulSearchReportsNoUnsearchableScopes() + throws Exception { + StubReader reader = new StubReader().returning( + "is:pr user:alice is:open", "/repos/alice/notes/pulls/1"); //$NON-NLS-1$ //$NON-NLS-2$ + + Result result = GitHubPullRequestSearch.run( + Arrays.asList("is:pr user:alice is:open"), 1, 1, reader); //$NON-NLS-1$ + + assertThat(result.getUnsearchableScopes(), empty()); + } + + @Test + public void testValidationFailureIsRecognizedAsUnsearchableScope() { + assertThat( + GitHubPullRequestSearch.isUnsearchableScope( + new IOException(VALIDATION_FAILED)), + equalTo(true)); + assertThat( + GitHubPullRequestSearch.isUnsearchableScope(new IOException( + "GitHub API request failed: HTTP 401 - Bad token")), //$NON-NLS-1$ + equalTo(false)); + assertThat(GitHubPullRequestSearch + .isUnsearchableScope(new IOException()), equalTo(false)); + } + + @Test + public void testScopeOfExtractsTheOwnerQualifier() { + assertThat(GitHubSearchQueries.scopeOf("is:pr user:alice is:open"), //$NON-NLS-1$ + equalTo("user:alice")); //$NON-NLS-1$ + assertThat( + GitHubSearchQueries.scopeOf( + "is:pr org:acme author:carol is:merged"), //$NON-NLS-1$ + equalTo("org:acme")); //$NON-NLS-1$ + assertThat(GitHubSearchQueries + .scopeOf("is:pr repo:bob/tool is:closed is:unmerged"), //$NON-NLS-1$ + equalTo("repo:bob/tool")); //$NON-NLS-1$ + assertThat(GitHubSearchQueries.scopeOf("is:pr"), equalTo("is:pr")); //$NON-NLS-1$ //$NON-NLS-2$ + } +} diff --git a/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/StubGitHubServer.java b/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/StubGitHubServer.java new file mode 100644 index 0000000..441fc4f --- /dev/null +++ b/org.eclipse.egit.pullrequest.test/src/org/eclipse/egit/pullrequest/internal/github/StubGitHubServer.java @@ -0,0 +1,172 @@ +/******************************************************************************* + * Copyright (C) 2026, Philipp0205 and others + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.egit.pullrequest.internal.github; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * Minimal HTTP server on the loopback interface that answers canned responses, + * used to exercise {@link GitHubClient} without talking to github.com. + *

+ * Responses are registered either for a request path or for a fragment of the + * query string, so that the different searches sent to + * {@code /search/issues} can be answered individually. + */ +class StubGitHubServer implements AutoCloseable { + + /** A canned response. */ + static class Response { + + private final int status; + + private final String reason; + + private final String body; + + Response(int status, String reason, String body) { + this.status = status; + this.reason = reason; + this.body = body; + } + } + + private final ServerSocket serverSocket; + + private final Map byPath = new ConcurrentHashMap<>(); + + private final Map byQuery = new ConcurrentHashMap<>(); + + private final List requestedTargets = new CopyOnWriteArrayList<>(); + + private volatile boolean running = true; + + StubGitHubServer() throws IOException { + serverSocket = new ServerSocket(0, 0, InetAddress.getLoopbackAddress()); + Thread thread = new Thread(this::serve, "stub-github"); //$NON-NLS-1$ + thread.setDaemon(true); + thread.start(); + } + + /** + * @return the API base URL of this server + */ + String url() { + return "http://" //$NON-NLS-1$ + + serverSocket.getInetAddress().getHostAddress() + ':' + + serverSocket.getLocalPort(); + } + + /** + * Registers the response for a path, ignoring any query string. + * + * @param path + * the request path + * @param response + * the response to send + */ + void on(String path, Response response) { + byPath.put(path, response); + } + + /** + * Registers the response for every request whose target contains the given + * fragment. Checked before the path responses. + * + * @param fragment + * a fragment of the encoded request target + * @param response + * the response to send + */ + void onQueryContaining(String fragment, Response response) { + byQuery.put(fragment, response); + } + + /** + * @return the request targets that were received, in order + */ + List requestedTargets() { + return requestedTargets; + } + + @Override + public void close() throws IOException { + running = false; + serverSocket.close(); + } + + private void serve() { + while (running) { + try (Socket client = serverSocket.accept()) { + handle(client); + } catch (IOException e) { + return; + } + } + } + + private void handle(Socket client) throws IOException { + BufferedReader in = new BufferedReader(new InputStreamReader( + client.getInputStream(), StandardCharsets.UTF_8)); + String requestLine = in.readLine(); + if (requestLine == null) { + return; + } + String line; + while ((line = in.readLine()) != null && !line.isEmpty()) { + // Headers are not inspected, but must be drained. + } + String[] parts = requestLine.split(" "); //$NON-NLS-1$ + String target = parts.length > 1 ? parts[1] : ""; //$NON-NLS-1$ + requestedTargets.add(target); + write(client.getOutputStream(), responseFor(target)); + } + + private Response responseFor(String target) { + for (Map.Entry entry : byQuery.entrySet()) { + if (target.contains(entry.getKey())) { + return entry.getValue(); + } + } + int query = target.indexOf('?'); + String path = query < 0 ? target : target.substring(0, query); + Response response = byPath.get(path); + if (response != null) { + return response; + } + return new Response(404, "Not Found", //$NON-NLS-1$ + "{\"message\":\"Not Found\"}"); //$NON-NLS-1$ + } + + private void write(OutputStream out, Response response) + throws IOException { + byte[] body = response.body.getBytes(StandardCharsets.UTF_8); + StringBuilder head = new StringBuilder(); + head.append("HTTP/1.1 ").append(response.status).append(' ') //$NON-NLS-1$ + .append(response.reason).append("\r\n"); //$NON-NLS-1$ + head.append("Content-Type: application/json\r\n"); //$NON-NLS-1$ + head.append("Content-Length: ").append(body.length).append("\r\n"); //$NON-NLS-1$ //$NON-NLS-2$ + head.append("Connection: close\r\n\r\n"); //$NON-NLS-1$ + out.write(head.toString().getBytes(StandardCharsets.UTF_8)); + out.write(body); + out.flush(); + } +} diff --git a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubClient.java b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubClient.java index 3280fea..66837c4 100644 --- a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubClient.java +++ b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubClient.java @@ -11,7 +11,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Comparator; -import java.util.LinkedHashSet; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -20,6 +19,8 @@ import org.eclipse.egit.pullrequest.internal.model.PullRequest; import org.eclipse.egit.pullrequest.internal.model.PullRequestComment; import org.eclipse.egit.pullrequest.internal.model.PullRequestCommit; +import org.eclipse.egit.pullrequest.internal.client.ConnectionDiagnostics; +import org.eclipse.egit.pullrequest.internal.client.ConnectionDiagnostics.Outcome; import org.eclipse.egit.pullrequest.internal.client.IPullRequestClient; import org.eclipse.egit.pullrequest.internal.client.PullRequestProviderCapabilities; import org.eclipse.egit.pullrequest.Activator; @@ -34,8 +35,6 @@ public class GitHubClient implements IPullRequestClient { private static final String API_BASE_URL = "https://api.github.com"; //$NON-NLS-1$ - private static final String GRAPHQL_URL = "https://api.github.com/graphql"; //$NON-NLS-1$ - private static final int DEFAULT_TIMEOUT = 30000; // 30 seconds private String owner; @@ -44,8 +43,16 @@ public class GitHubClient implements IPullRequestClient { private final String token; + private final String apiBaseUrl; + private final PullRequestProviderCapabilities capabilities; + /** + * Owner scopes the last token-scoped listing had to skip because GitHub + * refused to search them. Reported by {@link #diagnoseConnection()}. + */ + private List unsearchableScopes = List.of(); + /** * Creates a new GitHub client * @@ -58,11 +65,7 @@ public class GitHubClient implements IPullRequestClient { */ public GitHubClient(@NonNull String owner, @NonNull String repo, @NonNull String token) { - this.owner = owner; - this.repo = repo; - this.token = token; - this.capabilities = PullRequestProviderCapabilities - .forProvider(PullRequestProviderType.GITHUB); + this(owner, repo, token, API_BASE_URL); } /** @@ -73,9 +76,28 @@ public GitHubClient(@NonNull String owner, @NonNull String repo, * the GitHub access token */ public GitHubClient(@NonNull String token) { - this.owner = null; - this.repo = null; + this(null, null, token, API_BASE_URL); + } + + /** + * Creates a client against a specific GitHub REST API endpoint. + * + * @param owner + * the repository owner, or {@code null} to list pull requests + * from every accessible repository + * @param repo + * the repository name, or {@code null} + * @param token + * the GitHub access token + * @param apiBaseUrl + * the REST API base URL, without a trailing slash + */ + GitHubClient(String owner, String repo, @NonNull String token, + @NonNull String apiBaseUrl) { + this.owner = owner; + this.repo = repo; this.token = token; + this.apiBaseUrl = apiBaseUrl; this.capabilities = PullRequestProviderCapabilities .forProvider(PullRequestProviderType.GITHUB); } @@ -205,32 +227,15 @@ private List getUserPullRequests(String state, int needed = Math.max(1, start + Math.max(1, limit)); int pageSize = Math.min(100, needed); - LinkedHashSet pullRequestPaths = new LinkedHashSet<>(); - for (String query : queries) { - int collected = 0; - int page = 1; - while (collected < needed) { - String path = "/search/issues?q=" //$NON-NLS-1$ - + URLEncoder.encode(query, StandardCharsets.UTF_8) - + "&per_page=" + pageSize //$NON-NLS-1$ - + "&page=" + page //$NON-NLS-1$ - + "&sort=updated&order=desc"; //$NON-NLS-1$ - List pagePaths = GitHubJsonParser - .parseSearchPullRequestPaths(doGet(path)); - if (pagePaths.isEmpty()) { - break; - } - pullRequestPaths.addAll(pagePaths); - collected += pagePaths.size(); - if (pagePaths.size() < pageSize) { - break; - } - page++; - } - } + GitHubPullRequestSearch.Result search = GitHubPullRequestSearch.run( + queries, needed, pageSize, + (query, page, size) -> GitHubJsonParser + .parseSearchPullRequestPaths( + doGet(searchPath(query, page, size)))); + unsearchableScopes = search.getUnsearchableScopes(); List result = new ArrayList<>(); - for (String pullRequestPath : pullRequestPaths) { + for (String pullRequestPath : search.getPullRequestPaths()) { PullRequest pullRequest = GitHubJsonParser .parseSinglePullRequest(doGet(pullRequestPath)); if (pullRequest != null) { @@ -244,19 +249,56 @@ private List getUserPullRequests(String state, return new ArrayList<>(result.subList(from, to)); } - private List listOrganizationLogins() throws IOException { + private static String searchPath(String query, int page, int pageSize) { + return "/search/issues?q=" //$NON-NLS-1$ + + URLEncoder.encode(query, StandardCharsets.UTF_8) + + "&per_page=" + pageSize //$NON-NLS-1$ + + "&page=" + page //$NON-NLS-1$ + + "&sort=updated&order=desc"; //$NON-NLS-1$ + } + + /** + * Lists the organizations of the authenticated user. + *

+ * A token without the {@code read:org} scope cannot read them. That only + * narrows the set of searched scopes, so it is reported as a warning + * instead of failing the whole listing. + * + * @return the organization logins, empty if they cannot be read + */ + private List listOrganizationLogins() { List logins = new ArrayList<>(); - for (String page : doGetAllPages("/user/orgs?per_page=100")) { //$NON-NLS-1$ - logins.addAll(GitHubJsonParser.parseLogins(page)); + try { + for (String page : doGetAllPages("/user/orgs?per_page=100")) { //$NON-NLS-1$ + logins.addAll(GitHubJsonParser.parseLogins(page)); + } + } catch (IOException e) { + Activator.logWarning( + "Cannot list GitHub organizations; their pull requests " //$NON-NLS-1$ + + "are not listed. " + e.getMessage()); //$NON-NLS-1$ } return logins; } - private List listCollaboratorRepositories() throws IOException { + /** + * Lists the repositories the authenticated user collaborates on. + * + * @return the {@code owner/name} repositories, empty if they cannot be + * read + */ + private List listCollaboratorRepositories() { List fullNames = new ArrayList<>(); - for (String page : doGetAllPages( - "/user/repos?affiliation=collaborator&per_page=100")) { //$NON-NLS-1$ - fullNames.addAll(GitHubJsonParser.parseRepositoryFullNames(page)); + try { + for (String page : doGetAllPages( + "/user/repos?affiliation=collaborator&per_page=100")) { //$NON-NLS-1$ + fullNames.addAll( + GitHubJsonParser.parseRepositoryFullNames(page)); + } + } catch (IOException e) { + Activator.logWarning( + "Cannot list GitHub collaborator repositories; their " //$NON-NLS-1$ + + "pull requests are not listed. " //$NON-NLS-1$ + + e.getMessage()); } return fullNames; } @@ -816,6 +858,20 @@ public boolean testConnection() { return GitHubJsonParser.parseCurrentUser(json); } + @Override + public @NonNull ConnectionDiagnostics diagnoseConnection() { + ConnectionDiagnostics report = IPullRequestClient.super + .diagnoseConnection(); + if (!unsearchableScopes.isEmpty()) { + report.add("Search scopes", Outcome.WARNING, //$NON-NLS-1$ + "GitHub refused to search " //$NON-NLS-1$ + + String.join(", ", unsearchableScopes) //$NON-NLS-1$ + + ".\n" //$NON-NLS-1$ + + GitHubPullRequestSearch.TOKEN_SCOPE_HINT); + } + return report; + } + @Override public @NonNull List getPullRequestCommits( long pullRequestId) throws IOException { @@ -881,7 +937,7 @@ private String doGet(String path) throws IOException { */ private List doGetAllPages(String path) throws IOException { List pages = new ArrayList<>(); - String nextUrl = API_BASE_URL + path; + String nextUrl = apiBaseUrl + path; while (nextUrl != null) { HttpURLConnection conn = null; @@ -1134,7 +1190,7 @@ private String executeRequest(String path, String method, private String executeGraphQL(String query) throws IOException { HttpURLConnection conn = null; try { - URL url = new URL(GRAPHQL_URL); + URL url = new URL(apiBaseUrl + "/graphql"); //$NON-NLS-1$ conn = (HttpURLConnection) url.openConnection(); conn.setRequestMethod("POST"); //$NON-NLS-1$ conn.setConnectTimeout(DEFAULT_TIMEOUT); @@ -1184,7 +1240,7 @@ private String executeGraphQL(String query) throws IOException { */ private HttpURLConnection createConnection(String path, String method) throws IOException { - URL url = new URL(API_BASE_URL + path); + URL url = new URL(apiBaseUrl + path); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); // HttpURLConnection doesn't support PATCH by default in Java diff --git a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubJsonParser.java b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubJsonParser.java index fbd8305..e00e653 100644 --- a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubJsonParser.java +++ b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubJsonParser.java @@ -26,7 +26,7 @@ class GitHubJsonParser { private static final Pattern SEARCH_PULL_URL = Pattern.compile( "\"pull_request\"\\s*:\\s*\\{[^}]*\"url\"\\s*:\\s*" //$NON-NLS-1$ - + "\"https://api\\.github\\.com([^\"?]+)\"", //$NON-NLS-1$ + + "\"https?://[^\"]+?(/repos/[^\"?]+)\"", //$NON-NLS-1$ Pattern.DOTALL); /** diff --git a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubPullRequestSearch.java b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubPullRequestSearch.java new file mode 100644 index 0000000..4e01e50 --- /dev/null +++ b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubPullRequestSearch.java @@ -0,0 +1,186 @@ +/******************************************************************************* + * Copyright (C) 2026, Philipp0205 and others + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.egit.pullrequest.internal.github; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; + +import org.eclipse.egit.pullrequest.Activator; + +/** + * Runs one GitHub issue search per owner scope and merges the results. + *

+ * GitHub answers {@code GET /search/issues} with HTTP 422 as soon as a single + * qualifier in the query names a user, organization or repository the access + * token may not search. That happens for organizations the token was never + * SSO-authorized for, for private repositories when the token lacks the + * {@code repo} scope, and for repositories that were renamed or deleted after + * they were listed. Because every scope is searched with its own query, such a + * scope is skipped instead of failing the whole listing; only when GitHub + * rejects every scope is the failure reported to the caller. + */ +final class GitHubPullRequestSearch { + + /** Advice shown whenever GitHub refuses to search an owner scope. */ + static final String TOKEN_SCOPE_HINT = "Use a classic personal access " //$NON-NLS-1$ + + "token with the 'repo' and 'read:org' scopes, and authorize " //$NON-NLS-1$ + + "it for every organization that enforces SAML single " //$NON-NLS-1$ + + "sign-on. Fine-grained tokens can only search the " //$NON-NLS-1$ + + "repositories they were granted."; //$NON-NLS-1$ + + private GitHubPullRequestSearch() { + // No instantiation + } + + /** + * Reads one page of pull request paths for a single search query. + */ + @FunctionalInterface + interface PageReader { + + /** + * Runs one search request. + * + * @param query + * the search query + * @param page + * the one-based page number + * @param pageSize + * the number of results per page + * @return REST pull request paths in result order + * @throws IOException + * if the request fails + */ + List read(String query, int page, int pageSize) + throws IOException; + } + + /** Merged outcome of searching every owner scope. */ + static final class Result { + + private final List pullRequestPaths; + + private final List unsearchableScopes; + + Result(List pullRequestPaths, + List unsearchableScopes) { + this.pullRequestPaths = pullRequestPaths; + this.unsearchableScopes = unsearchableScopes; + } + + /** + * @return REST pull request paths without duplicates + */ + List getPullRequestPaths() { + return pullRequestPaths; + } + + /** + * @return the owner qualifiers GitHub refused to search + */ + List getUnsearchableScopes() { + return unsearchableScopes; + } + } + + /** + * Searches every query and merges the pull requests they return. + * + * @param queries + * one search query per owner scope + * @param needed + * how many results are needed per scope + * @param pageSize + * how many results to request per page + * @param reader + * runs the individual search requests + * @return the merged result + * @throws IOException + * if a search fails for any reason other than an unsearchable + * scope, or if GitHub refuses to search every scope + */ + static Result run(List queries, int needed, int pageSize, + PageReader reader) throws IOException { + LinkedHashSet paths = new LinkedHashSet<>(); + List unsearchable = new ArrayList<>(); + IOException lastRejection = null; + for (String query : queries) { + try { + collect(query, needed, pageSize, reader, paths); + } catch (IOException e) { + if (!isUnsearchableScope(e)) { + throw e; + } + String scope = GitHubSearchQueries.scopeOf(query); + unsearchable.add(scope); + lastRejection = e; + Activator.logWarning( + "GitHub cannot search " + scope //$NON-NLS-1$ + + "; its pull requests are not listed. " //$NON-NLS-1$ + + e.getMessage()); + } + } + if (lastRejection != null && unsearchable.size() == queries.size()) { + throw new IOException(allScopesRejected(unsearchable), + lastRejection); + } + return new Result(new ArrayList<>(paths), + Collections.unmodifiableList(unsearchable)); + } + + private static void collect(String query, int needed, int pageSize, + PageReader reader, LinkedHashSet paths) + throws IOException { + int collected = 0; + int page = 1; + while (collected < needed) { + List pagePaths = reader.read(query, page, pageSize); + if (pagePaths.isEmpty()) { + return; + } + paths.addAll(pagePaths); + collected += pagePaths.size(); + if (pagePaths.size() < pageSize) { + return; + } + page++; + } + } + + /** + * Tells whether GitHub rejected the query because one of its qualifiers + * names something the token may not search. + * + * @param e + * the failure reported by the search request + * @return true if the scope should be skipped rather than propagated + */ + static boolean isUnsearchableScope(IOException e) { + String message = e.getMessage(); + return message != null && message.contains("HTTP 422"); //$NON-NLS-1$ + } + + /** + * Builds the message for the case where no scope could be searched at all. + * + * @param unsearchableScopes + * the owner qualifiers GitHub refused to search + * @return an actionable error message + */ + static String allScopesRejected(List unsearchableScopes) { + return "GitHub refused to search " //$NON-NLS-1$ + + String.join(", ", unsearchableScopes) //$NON-NLS-1$ + + ". " + TOKEN_SCOPE_HINT; //$NON-NLS-1$ + } +} diff --git a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubSearchQueries.java b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubSearchQueries.java index 8708de8..55bdcdc 100644 --- a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubSearchQueries.java +++ b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/github/GitHubSearchQueries.java @@ -96,6 +96,29 @@ static String pullRequestQuery(String ownerQualifier, String state, return query.toString(); } + /** + * Returns the owner qualifier a query was built for. + * + * @param query + * a query produced by + * {@link #pullRequestQuery(String, String, String)} + * @return the {@code user:}, {@code org:} or {@code repo:} qualifier, or + * the whole query if it carries none + */ + static String scopeOf(String query) { + if (query == null) { + return ""; //$NON-NLS-1$ + } + for (String token : query.split(" ")) { //$NON-NLS-1$ + if (token.startsWith("user:") //$NON-NLS-1$ + || token.startsWith("org:") //$NON-NLS-1$ + || token.startsWith("repo:")) { //$NON-NLS-1$ + return token; + } + } + return query; + } + static boolean isCoveredByUserOrOrg(String fullName, String login, Set organizations) { if (fullName == null || fullName.isBlank()) { diff --git a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/ui/PullRequestPreferencePage.java b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/ui/PullRequestPreferencePage.java index 2c0c404..6e4739b 100644 --- a/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/ui/PullRequestPreferencePage.java +++ b/org.eclipse.egit.pullrequest/src/org/eclipse/egit/pullrequest/internal/ui/PullRequestPreferencePage.java @@ -230,10 +230,13 @@ private Composite createGitHubConfiguration(Composite parent) { Label infoLabel = new Label(group, SWT.WRAP); infoLabel.setText( "Authenticate as a GitHub user with a personal access token " //$NON-NLS-1$ - + "(classic) that has the 'repo' scope:\n" //$NON-NLS-1$ + + "(classic) that has the 'repo' and 'read:org' " //$NON-NLS-1$ + + "scopes:\n" //$NON-NLS-1$ + "https://github.com/settings/tokens\n\n" //$NON-NLS-1$ + "The plugin lists pull requests from every repository " //$NON-NLS-1$ - + "you own or belong to."); //$NON-NLS-1$ + + "you own or belong to. Organizations that enforce " //$NON-NLS-1$ + + "SAML single sign-on are only searched once the token " //$NON-NLS-1$ + + "is authorized for them."); //$NON-NLS-1$ GridDataFactory.fillDefaults().span(2, 1).hint(400, SWT.DEFAULT) .indent(0, 5).applyTo(infoLabel);