Bump to v3.9.0-rc4 and reuse FakeHttpServletRequest/Response Bump to v3.9.0-rc4 and reuse Gerrit's FakeHttpServletRequest/Response from its acceptance framework. Remove the duplicated copy in the plugin and the ad-hoc implementation tailored to the tests. Change-Id: Id86945fcede1ba5beb1ccd08b4e944c2bc621be9
diff --git a/github-oauth/pom.xml b/github-oauth/pom.xml index 256abc8..454e0fe 100644 --- a/github-oauth/pom.xml +++ b/github-oauth/pom.xml
@@ -21,7 +21,7 @@ <parent> <groupId>com.googlesource.gerrit.plugins.github</groupId> <artifactId>github-parent</artifactId> - <version>3.9.0-rc2</version> + <version>3.9.0-rc4</version> </parent> <artifactId>github-oauth</artifactId> <name>Gerrit Code Review - GitHub OAuth login</name>
diff --git a/github-plugin/pom.xml b/github-plugin/pom.xml index 96d3290..79e76a0 100644 --- a/github-plugin/pom.xml +++ b/github-plugin/pom.xml
@@ -20,7 +20,7 @@ <parent> <artifactId>github-parent</artifactId> <groupId>com.googlesource.gerrit.plugins.github</groupId> - <version>3.9.0-rc2</version> + <version>3.9.0-rc4</version> </parent> <artifactId>github-plugin</artifactId> @@ -152,6 +152,12 @@ <scope>provided</scope> </dependency> <dependency> + <groupId>com.google.gerrit</groupId> + <artifactId>gerrit-acceptance-framework</artifactId> + <version>${Gerrit-ApiVersion}</version> + <scope>test</scope> + </dependency> + <dependency> <groupId>${project.groupId}</groupId> <artifactId>github-oauth</artifactId> <version>${project.version}</version>
diff --git a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpServletRequest.java b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpServletRequest.java deleted file mode 100644 index cdb229d..0000000 --- a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpServletRequest.java +++ /dev/null
@@ -1,465 +0,0 @@ -// Copyright (C) 2023 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.github; - -import static com.google.common.base.Strings.nullToEmpty; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Objects.requireNonNull; -import static java.util.stream.Collectors.toList; - -import com.google.common.base.Splitter; -import com.google.common.collect.Iterables; -import com.google.common.collect.LinkedListMultimap; -import com.google.common.collect.ListMultimap; -import com.google.common.collect.Maps; -import com.google.gerrit.common.Nullable; -import java.io.BufferedReader; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; -import java.security.Principal; -import java.time.format.DateTimeFormatter; -import java.util.Collection; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import javax.servlet.AsyncContext; -import javax.servlet.DispatcherType; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletContext; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; -import javax.servlet.http.HttpUpgradeHandler; -import javax.servlet.http.Part; - -/** Simple fake implementation of {@link HttpServletRequest}. */ -public class FakeHttpServletRequest implements HttpServletRequest { - public static final String SERVLET_PATH = "/b"; - public static final DateTimeFormatter rfcDateformatter = - DateTimeFormatter.ofPattern("EEE, dd MMM yyyy HH:mm:ss ZZZ"); - - private final Map<String, Object> attributes; - private final ListMultimap<String, String> headers; - - private ListMultimap<String, String> parameters; - private String queryString; - private String servletPath; - private String path; - private String method; - private HttpSession session; - - public FakeHttpServletRequest() { - this(SERVLET_PATH, null); - } - - public FakeHttpServletRequest(String servletPath, HttpSession existingSession) { - this.servletPath = requireNonNull(servletPath, "servletPath"); - attributes = Maps.newConcurrentMap(); - parameters = LinkedListMultimap.create(); - headers = LinkedListMultimap.create(); - session = existingSession; - } - - @Override - public Object getAttribute(String name) { - return attributes.get(name); - } - - @Override - public Enumeration<String> getAttributeNames() { - return Collections.enumeration(attributes.keySet()); - } - - @Override - public String getCharacterEncoding() { - return UTF_8.name(); - } - - @Override - public int getContentLength() { - return 0; - } - - @Nullable - @Override - public String getContentType() { - return null; - } - - @Override - public ServletInputStream getInputStream() { - throw new UnsupportedOperationException(); - } - - @Override - public String getLocalAddr() { - return "1.2.3.4"; - } - - @Override - public String getLocalName() { - return "localhost"; - } - - @Override - public int getLocalPort() { - return 80; - } - - @Override - public Locale getLocale() { - return Locale.US; - } - - @Override - public Enumeration<Locale> getLocales() { - return Collections.enumeration(Collections.singleton(Locale.US)); - } - - @Override - public String getParameter(String name) { - return Iterables.getFirst(parameters.get(name), null); - } - - @Override - public Map<String, String[]> getParameterMap() { - return Collections.unmodifiableMap( - Maps.transformValues(parameters.asMap(), vs -> vs.toArray(new String[0]))); - } - - @Override - public Enumeration<String> getParameterNames() { - return Collections.enumeration(parameters.keySet()); - } - - @Override - public String[] getParameterValues(String name) { - return parameters.get(name).toArray(new String[0]); - } - - public void setQueryString(String qs) { - this.queryString = qs; - ListMultimap<String, String> params = LinkedListMultimap.create(); - for (String entry : Splitter.on('&').split(qs)) { - List<String> kv = Splitter.on('=').limit(2).splitToList(entry); - try { - params.put( - URLDecoder.decode(kv.get(0), UTF_8.name()), - kv.size() == 2 ? URLDecoder.decode(kv.get(1), UTF_8.name()) : ""); - } catch (UnsupportedEncodingException e) { - throw new IllegalArgumentException(e); - } - } - parameters = params; - } - - @Override - public String getProtocol() { - return "HTTP/1.1"; - } - - @Override - public BufferedReader getReader() { - throw new UnsupportedOperationException(); - } - - @Override - @Deprecated - public String getRealPath(String path) { - throw new UnsupportedOperationException(); - } - - @Override - public String getRemoteAddr() { - return "5.6.7.8"; - } - - @Override - public String getRemoteHost() { - return "remotehost"; - } - - @Override - public int getRemotePort() { - return 1234; - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) { - throw new UnsupportedOperationException(); - } - - @Override - public String getScheme() { - return "http"; - } - - @Override - public String getServerName() { - return "localhost"; - } - - @Override - public int getServerPort() { - return 80; - } - - @Override - public boolean isSecure() { - return false; - } - - @Override - public void removeAttribute(String name) { - attributes.remove(name); - } - - @Override - public void setAttribute(String name, Object value) { - attributes.put(name, value); - } - - @Override - public void setCharacterEncoding(String env) throws UnsupportedOperationException { - throw new UnsupportedOperationException(); - } - - @Override - public String getAuthType() { - return null; - } - - @Override - public String getContextPath() { - return ""; - } - - @Override - public Cookie[] getCookies() { - return Splitter.on(";").splitToList(nullToEmpty(getHeader("Cookie"))).stream() - .filter(s -> !s.isEmpty()) - .map( - (String cookieValue) -> { - List<String> kv = Splitter.on("=").splitToList(cookieValue); - return new Cookie(kv.get(0), kv.get(1)); - }) - .collect(toList()) - .toArray(new Cookie[0]); - } - - @Override - public long getDateHeader(String name) { - return 0L; - } - - @Override - public String getHeader(String name) { - return Iterables.getFirst(headers.get(name), null); - } - - @Override - public Enumeration<String> getHeaderNames() { - return Collections.enumeration(headers.keySet()); - } - - @Override - public Enumeration<String> getHeaders(String name) { - return Collections.enumeration(headers.get(name)); - } - - @Override - public int getIntHeader(String name) { - return Integer.parseInt(getHeader(name)); - } - - @Override - public String getMethod() { - return method; - } - - public void setMethod(String method) { - this.method = method; - } - - @Override - public String getPathInfo() { - return path; - } - - public FakeHttpServletRequest setPathInfo(String path) { - this.path = path; - return this; - } - - @Override - public String getPathTranslated() { - return path; - } - - @Override - public String getQueryString() { - return queryString; - } - - @Override - public String getRemoteUser() { - return null; - } - - @Override - public String getRequestURI() { - return nullToEmpty(servletPath) + nullToEmpty(path); - } - - @Override - public StringBuffer getRequestURL() { - return new StringBuffer("http://localhost" + getRequestURI()); - } - - @Override - public String getRequestedSessionId() { - return null; - } - - @Override - public String getServletPath() { - return servletPath; - } - - @Override - public HttpSession getSession() { - return getSession(true); - } - - @Override - public HttpSession getSession(boolean create) { - if (session == null && create) { - session = new FakeHttpSession(); - } - return session; - } - - @Override - public Principal getUserPrincipal() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isRequestedSessionIdFromCookie() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isRequestedSessionIdFromURL() { - throw new UnsupportedOperationException(); - } - - @Override - @Deprecated - public boolean isRequestedSessionIdFromUrl() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isRequestedSessionIdValid() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isUserInRole(String role) { - throw new UnsupportedOperationException(); - } - - @Override - public AsyncContext getAsyncContext() { - throw new UnsupportedOperationException(); - } - - @Override - public DispatcherType getDispatcherType() { - throw new UnsupportedOperationException(); - } - - @Override - public ServletContext getServletContext() { - throw new UnsupportedOperationException(); - } - - @Override - public boolean isAsyncStarted() { - return false; - } - - @Override - public boolean isAsyncSupported() { - return false; - } - - @Override - public AsyncContext startAsync() { - throw new UnsupportedOperationException(); - } - - @Override - public AsyncContext startAsync(ServletRequest req, ServletResponse res) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean authenticate(HttpServletResponse res) { - throw new UnsupportedOperationException(); - } - - @Override - public Part getPart(String name) { - throw new UnsupportedOperationException(); - } - - @Override - public Collection<Part> getParts() { - throw new UnsupportedOperationException(); - } - - @Override - public void login(String username, String password) { - throw new UnsupportedOperationException(); - } - - @Override - public void logout() { - throw new UnsupportedOperationException(); - } - - @Override - public long getContentLengthLong() { - return getContentLength(); - } - - @Override - public String changeSessionId() { - throw new UnsupportedOperationException(); - } - - @Override - public <T extends HttpUpgradeHandler> T upgrade(Class<T> httpUpgradeHandlerClass) { - throw new UnsupportedOperationException(); - } -}
diff --git a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpServletResponse.java b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpServletResponse.java deleted file mode 100644 index 6824a15..0000000 --- a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpServletResponse.java +++ /dev/null
@@ -1,249 +0,0 @@ -// Copyright (C) 2015 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.github; - -import static com.google.common.base.Preconditions.checkArgument; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Objects.requireNonNull; - -import com.google.common.collect.Iterables; -import com.google.common.collect.LinkedListMultimap; -import com.google.common.collect.ListMultimap; -import com.google.common.net.HttpHeaders; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintWriter; -import java.nio.charset.Charset; -import java.util.Collection; -import java.util.Locale; -import javax.servlet.ServletOutputStream; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletResponse; - -/** Simple fake implementation of {@link HttpServletResponse}. */ -public class FakeHttpServletResponse implements HttpServletResponse { - private final ByteArrayOutputStream actualBody = new ByteArrayOutputStream(); - private final ListMultimap<String, String> headers = LinkedListMultimap.create(); - - private int status = SC_OK; - private boolean committed; - private ServletOutputStream outputStream; - private PrintWriter writer; - - public FakeHttpServletResponse() {} - - @Override - public synchronized void flushBuffer() throws IOException { - if (outputStream != null) { - outputStream.flush(); - } - if (writer != null) { - writer.flush(); - } - } - - @Override - public int getBufferSize() { - throw new UnsupportedOperationException(); - } - - @Override - public String getCharacterEncoding() { - return UTF_8.name(); - } - - @Override - public String getContentType() { - return null; - } - - @Override - public Locale getLocale() { - return Locale.US; - } - - @Override - public synchronized ServletOutputStream getOutputStream() { - throw new UnsupportedOperationException(); - } - - @Override - public synchronized PrintWriter getWriter() { - throw new UnsupportedOperationException(); - } - - @Override - public synchronized boolean isCommitted() { - return committed; - } - - @Override - public void reset() { - throw new UnsupportedOperationException(); - } - - @Override - public void resetBuffer() { - throw new UnsupportedOperationException(); - } - - @Override - public void setBufferSize(int sz) { - throw new UnsupportedOperationException(); - } - - @Override - public void setCharacterEncoding(String name) { - checkArgument(UTF_8.equals(Charset.forName(name)), "unsupported charset: %s", name); - } - - @Override - public void setContentLength(int length) { - setContentLengthLong(length); - } - - @Override - public void setContentLengthLong(long length) { - headers.removeAll(HttpHeaders.CONTENT_LENGTH); - addHeader(HttpHeaders.CONTENT_LENGTH, Long.toString(length)); - } - - @Override - public void setContentType(String type) { - headers.removeAll(HttpHeaders.CONTENT_TYPE); - addHeader(HttpHeaders.CONTENT_TYPE, type); - } - - @Override - public void setLocale(Locale locale) { - throw new UnsupportedOperationException(); - } - - @Override - public void addCookie(Cookie cookie) { - addHeader("Set-Cookie", cookie.getName() + "=" + cookie.getValue()); - } - - @Override - public void addDateHeader(String name, long value) { - throw new UnsupportedOperationException(); - } - - @Override - public void addHeader(String name, String value) { - headers.put(name.toLowerCase(Locale.US), value); - } - - @Override - public void addIntHeader(String name, int value) { - addHeader(name, Integer.toString(value)); - } - - @Override - public boolean containsHeader(String name) { - return headers.containsKey(name.toLowerCase(Locale.US)); - } - - @Override - public String encodeRedirectURL(String url) { - throw new UnsupportedOperationException(); - } - - @Override - @Deprecated - public String encodeRedirectUrl(String url) { - throw new UnsupportedOperationException(); - } - - @Override - public String encodeURL(String url) { - throw new UnsupportedOperationException(); - } - - @Override - @Deprecated - public String encodeUrl(String url) { - throw new UnsupportedOperationException(); - } - - @Override - public synchronized void sendError(int sc) { - status = sc; - committed = true; - } - - @Override - public synchronized void sendError(int sc, String msg) { - status = sc; - committed = true; - } - - @Override - public synchronized void sendRedirect(String loc) { - status = SC_FOUND; - setHeader(HttpHeaders.LOCATION, loc); - committed = true; - } - - @Override - public void setDateHeader(String name, long value) { - setHeader(name, Long.toString(value)); - } - - @Override - public void setHeader(String name, String value) { - headers.removeAll(name.toLowerCase(Locale.US)); - addHeader(name, value); - } - - @Override - public void setIntHeader(String name, int value) { - headers.removeAll(name.toLowerCase(Locale.US)); - addIntHeader(name, value); - } - - @Override - public synchronized void setStatus(int sc) { - status = sc; - committed = true; - } - - @Override - @Deprecated - public synchronized void setStatus(int sc, String msg) { - status = sc; - committed = true; - } - - @Override - public synchronized int getStatus() { - return status; - } - - @Override - public String getHeader(String name) { - return Iterables.getFirst(headers.get(requireNonNull(name.toLowerCase(Locale.US))), null); - } - - @Override - public Collection<String> getHeaderNames() { - return headers.keySet(); - } - - @Override - public Collection<String> getHeaders(String name) { - return headers.get(requireNonNull(name.toLowerCase(Locale.US))); - } -}
diff --git a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java index 59a8e3a..5c1707f 100644 --- a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java +++ b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java
@@ -19,6 +19,8 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.gerrit.util.http.testutil.FakeHttpServletRequest; +import com.google.gerrit.util.http.testutil.FakeHttpServletResponse; import com.googlesource.gerrit.plugins.github.filters.GitHubGroupCacheRefreshFilter; import com.googlesource.gerrit.plugins.github.group.GitHubGroupsCache; import com.googlesource.gerrit.plugins.github.groups.OrganizationStructure; @@ -33,6 +35,8 @@ private static final FilterChain NOOP_FILTER_CHAIN_TEST = (req, res) -> {}; private static final String GITHUB_USERNAME_TEST = "somegithubuser"; private static final OrganizationStructure GITHUB_USER_ORGANIZATION = new OrganizationStructure(); + private static final String TEST_SERVER = "test-server"; + private static final int TEST_PORT = 80; private LoadingCache<String, OrganizationStructure> groupsByUsernameCache; private GitHubGroupCacheRefreshFilter filter; @@ -94,16 +98,14 @@ FakeHttpServletRequest regularRequest = new FakeHttpServletRequest(); filter.doFilter(regularRequest, new FakeHttpServletResponse(), NOOP_FILTER_CHAIN_TEST); filter.doFilter( - newHomepageRequest(regularRequest.getSession()), - new FakeHttpServletResponse(), - NOOP_FILTER_CHAIN_TEST); + newHomepageRequest(null), new FakeHttpServletResponse(), NOOP_FILTER_CHAIN_TEST); assertThat(groupsByUsernameCache.get(GITHUB_USERNAME_TEST)).isEqualTo(GITHUB_USER_ORGANIZATION); assertThat(groupsCacheLoader.getLoadCount()).isEqualTo(initialLoadCount); } private ServletRequest newHomepageRequest(HttpSession session) { - return new FakeHttpServletRequest("/", session); + return new FakeHttpServletRequest(TEST_SERVER, TEST_PORT, "", "/", null, session); } private static HttpServletResponse newFinalLoginRedirectWithCookie() { @@ -113,8 +115,11 @@ } private static FakeHttpServletRequest newFinalLoginRequest() { - FakeHttpServletRequest req = new FakeHttpServletRequest("/login", null); + FakeHttpServletRequest req = + new FakeHttpServletRequest( + TEST_SERVER, TEST_PORT, "", "", () -> new FakeHttpSession(), null); req.setQueryString("final=true"); + req.setPathInfo("/login"); return req; } }
diff --git a/pom.xml b/pom.xml index a0c4c21..e4e4b95 100644 --- a/pom.xml +++ b/pom.xml
@@ -18,7 +18,7 @@ <modelVersion>4.0.0</modelVersion> <groupId>com.googlesource.gerrit.plugins.github</groupId> <artifactId>github-parent</artifactId> - <version>3.9.0-rc2</version> + <version>3.9.0-rc4</version> <name>Gerrit Code Review - GitHub integration</name> <url>http://www.gerritforge.com</url> <packaging>pom</packaging>