From 7b92b851684d4bc816ec701bcef8b22ad57ba095 Mon Sep 17 00:00:00 2001 From: imply-cheddar <86940447+imply-cheddar@users.noreply.github.com> Date: Thu, 22 Dec 2022 13:15:08 +0900 Subject: [PATCH] Unify DummyRequest with MockHttpServletRequest (#13602) We had 2 different classes both creating fake instances of an HttpServletRequest, this makes it to that we only have one in a common location --- .../http/catalog/CatalogResourceTest.java | 39 +- .../server/http/catalog/DummyRequest.java | 500 ------------------ .../druid/server/mocks/MockAsyncContext.java | 7 + .../server/mocks/MockHttpServletRequest.java | 17 +- .../server/mocks/MockHttpServletResponse.java | 12 +- 5 files changed, 67 insertions(+), 508 deletions(-) delete mode 100644 extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/DummyRequest.java diff --git a/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java b/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java index 00cced8b975..3b1e26f7ae5 100644 --- a/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java +++ b/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/CatalogResourceTest.java @@ -37,6 +37,9 @@ import org.apache.druid.catalog.model.table.InputFormats; import org.apache.druid.catalog.model.table.TableBuilder; import org.apache.druid.catalog.storage.CatalogTests; import org.apache.druid.metadata.TestDerbyConnector; +import org.apache.druid.server.mocks.MockHttpServletRequest; +import org.apache.druid.server.security.AuthConfig; +import org.apache.druid.server.security.AuthenticationResult; import org.apache.druid.server.security.ForbiddenException; import org.junit.After; import org.junit.Before; @@ -44,16 +47,12 @@ import org.junit.Rule; import org.junit.Test; import javax.ws.rs.core.Response; - import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; -import static org.apache.druid.server.http.catalog.DummyRequest.deleteBy; -import static org.apache.druid.server.http.catalog.DummyRequest.getBy; -import static org.apache.druid.server.http.catalog.DummyRequest.postBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; @@ -64,6 +63,10 @@ import static org.junit.Assert.assertTrue; */ public class CatalogResourceTest { + public static final String GET = "GET"; + public static final String POST = "POST"; + public static final String DELETE = "DELETE"; + @Rule public final TestDerbyConnector.DerbyConnectorRule derbyConnectorRule = new TestDerbyConnector.DerbyConnectorRule(); @@ -694,4 +697,32 @@ public class CatalogResourceTest CatalogUtils.columnNames(read.spec().columns()) ); } + + private static MockHttpServletRequest makeRequest(String method, String user, String contentType) + { + final MockHttpServletRequest retVal = new MockHttpServletRequest(); + retVal.method = method; + retVal.attributes.put( + AuthConfig.DRUID_AUTHENTICATION_RESULT, + new AuthenticationResult(user, CatalogTests.TEST_AUTHORITY, null, null + ) + ); + retVal.contentType = contentType; + return retVal; + } + + private static MockHttpServletRequest postBy(String user) + { + return makeRequest(POST, user, null); + } + + private static MockHttpServletRequest getBy(String user) + { + return makeRequest(GET, user, null); + } + + private static MockHttpServletRequest deleteBy(String user) + { + return makeRequest(DELETE, user, null); + } } diff --git a/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/DummyRequest.java b/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/DummyRequest.java deleted file mode 100644 index 8316392c0eb..00000000000 --- a/extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/DummyRequest.java +++ /dev/null @@ -1,500 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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 org.apache.druid.server.http.catalog; - -import org.apache.druid.catalog.storage.CatalogTests; -import org.apache.druid.server.security.AuthConfig; -import org.apache.druid.server.security.AuthenticationResult; - -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; - -import java.io.BufferedReader; -import java.security.Principal; -import java.util.Collection; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.Locale; -import java.util.Map; - -/** - * Test-only implementation of an HTTP request. Allows us to control - * aspects of the request without resorting to mocks. - */ -public class DummyRequest implements HttpServletRequest -{ - public static final String GET = "GET"; - public static final String POST = "POST"; - public static final String DELETE = "DELETE"; - - private final String method; - private final Map attribs = new HashMap<>(); - private final String contentType; - - public DummyRequest(String method, String userName) - { - this(method, userName, null); - } - - public DummyRequest(String method, String userName, String contentType) - { - this.method = method; - AuthenticationResult authResult = - new AuthenticationResult(userName, CatalogTests.TEST_AUTHORITY, null, null); - attribs.put(AuthConfig.DRUID_AUTHENTICATION_RESULT, authResult); - this.contentType = contentType; - } - - public static HttpServletRequest postBy(String user) - { - return new DummyRequest(DummyRequest.POST, user); - } - - public static HttpServletRequest getBy(String user) - { - return new DummyRequest(DummyRequest.GET, user); - } - - public static HttpServletRequest deleteBy(String user) - { - return new DummyRequest(DummyRequest.DELETE, user); - } - - @Override - public Object getAttribute(String name) - { - return attribs.get(name); - } - - @Override - public Enumeration getAttributeNames() - { - return null; - } - - @Override - public String getCharacterEncoding() - { - return null; - } - - @Override - public void setCharacterEncoding(String env) - { - } - - @Override - public int getContentLength() - { - return 0; - } - - @Override - public long getContentLengthLong() - { - return 0; - } - - @Override - public String getContentType() - { - return contentType; - } - - @Override - public ServletInputStream getInputStream() - { - return null; - } - - @Override - public String getParameter(String name) - { - return null; - } - - @Override - public Enumeration getParameterNames() - { - return null; - } - - @Override - public String[] getParameterValues(String name) - { - return null; - } - - @Override - public Map getParameterMap() - { - return null; - } - - @Override - public String getProtocol() - { - return null; - } - - @Override - public String getScheme() - { - return null; - } - - @Override - public String getServerName() - { - return null; - } - - @Override - public int getServerPort() - { - return 0; - } - - @Override - public BufferedReader getReader() - { - return null; - } - - @Override - public String getRemoteAddr() - { - return null; - } - - @Override - public String getRemoteHost() - { - return null; - } - - @Override - public void setAttribute(String name, Object o) - { - attribs.put(name, o); - } - - @Override - public void removeAttribute(String name) - { - } - - @Override - public Locale getLocale() - { - return null; - } - - @Override - public Enumeration getLocales() - { - return null; - } - - @Override - public boolean isSecure() - { - return false; - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) - { - return null; - } - - @Override - public String getRealPath(String path) - { - return null; - } - - @Override - public int getRemotePort() - { - return 0; - } - - @Override - public String getLocalName() - { - return null; - } - - @Override - public String getLocalAddr() - { - return null; - } - - @Override - public int getLocalPort() - { - return 0; - } - - @Override - public ServletContext getServletContext() - { - return null; - } - - @Override - public AsyncContext startAsync() - { - return null; - } - - @Override - public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) - { - return null; - } - - @Override - public boolean isAsyncStarted() - { - return false; - } - - @Override - public boolean isAsyncSupported() - { - return false; - } - - @Override - public AsyncContext getAsyncContext() - { - return null; - } - - @Override - public DispatcherType getDispatcherType() - { - return null; - } - - @Override - public String getAuthType() - { - return null; - } - - @Override - public Cookie[] getCookies() - { - return null; - } - - @Override - public long getDateHeader(String name) - { - return 0; - } - - @Override - public String getHeader(String name) - { - return null; - } - - @Override - public Enumeration getHeaders(String name) - { - return null; - } - - @Override - public Enumeration getHeaderNames() - { - return null; - } - - @Override - public int getIntHeader(String name) - { - return 0; - } - - @Override - public String getMethod() - { - return method; - } - - @Override - public String getPathInfo() - { - return null; - } - - @Override - public String getPathTranslated() - { - return null; - } - - @Override - public String getContextPath() - { - return null; - } - - @Override - public String getQueryString() - { - return null; - } - - @Override - public String getRemoteUser() - { - return null; - } - - @Override - public boolean isUserInRole(String role) - { - return false; - } - - @Override - public Principal getUserPrincipal() - { - return null; - } - - @Override - public String getRequestedSessionId() - { - return null; - } - - @Override - public String getRequestURI() - { - return null; - } - - @Override - public StringBuffer getRequestURL() - { - return null; - } - - @Override - public String getServletPath() - { - return null; - } - - @Override - public HttpSession getSession(boolean create) - { - return null; - } - - @Override - public HttpSession getSession() - { - return null; - } - - @Override - public String changeSessionId() - { - return null; - } - - @Override - public boolean isRequestedSessionIdValid() - { - return false; - } - - @Override - public boolean isRequestedSessionIdFromCookie() - { - return false; - } - - @Override - public boolean isRequestedSessionIdFromURL() - { - return false; - } - - @Override - public boolean isRequestedSessionIdFromUrl() - { - return false; - } - - @Override - public boolean authenticate(HttpServletResponse response) - { - return false; - } - - @Override - public void login(String username, String password) - { - } - - @Override - public void logout() - { - } - - @Override - public Collection getParts() - { - return null; - } - - @Override - public Part getPart(String name) - { - return null; - } - - @Override - public T upgrade(Class handlerClass) - { - return null; - } -} diff --git a/server/src/test/java/org/apache/druid/server/mocks/MockAsyncContext.java b/server/src/test/java/org/apache/druid/server/mocks/MockAsyncContext.java index 4c74acbcb76..4bdc56747d5 100644 --- a/server/src/test/java/org/apache/druid/server/mocks/MockAsyncContext.java +++ b/server/src/test/java/org/apache/druid/server/mocks/MockAsyncContext.java @@ -26,6 +26,13 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import java.util.concurrent.atomic.AtomicBoolean; +/** + * A fake AsyncContext used in tests. A lot of methods are implemented as + * {@code throw new UnsupportedOperationException}, this is just an indication that nobody has needed to flesh out + * that functionality for the mock yet and is not an indication that calling said method is a problem. If an + * {@code throw new UnsupportedOperationException} gets thrown out from one of these methods in a test, it is expected + * that the developer will implement the necessary methods. + */ public class MockAsyncContext implements AsyncContext { public ServletRequest request; diff --git a/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletRequest.java b/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletRequest.java index 34a425ea88c..85f1333244b 100644 --- a/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletRequest.java +++ b/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletRequest.java @@ -19,6 +19,8 @@ package org.apache.druid.server.mocks; +import org.apache.druid.java.util.common.ISE; + import javax.servlet.AsyncContext; import javax.servlet.DispatcherType; import javax.servlet.RequestDispatcher; @@ -41,8 +43,18 @@ import java.util.Locale; import java.util.Map; import java.util.function.Supplier; +/** + * A fake HttpServletRequest used in tests. A lot of methods are implemented as + * {@code throw new UnsupportedOperationException}, this is just an indication that nobody has needed to flesh out + * that functionality for the mock yet and is not an indication that calling said method is a problem. If an + * {@code throw new UnsupportedOperationException} gets thrown out from one of these methods in a test, it is expected + * that the developer will implement the necessary methods. + * + * Also, there is a mimic method. If you add fields to this class, be certain to adjust the mimic method as well. + */ public class MockHttpServletRequest implements HttpServletRequest { + public String method = null; public String contentType = null; public String remoteAddr = null; @@ -98,7 +110,7 @@ public class MockHttpServletRequest implements HttpServletRequest @Override public String getMethod() { - throw new UnsupportedOperationException(); + return method; } @Override @@ -435,7 +447,7 @@ public class MockHttpServletRequest implements HttpServletRequest public AsyncContext startAsync() { if (asyncContextSupplier == null) { - throw new UnsupportedOperationException(); + throw new ISE("Start Async called, but no context supplier set."); } else { if (currAsyncContext == null) { currAsyncContext = asyncContextSupplier.get(); @@ -493,6 +505,7 @@ public class MockHttpServletRequest implements HttpServletRequest { MockHttpServletRequest retVal = new MockHttpServletRequest(); + retVal.method = method; retVal.asyncContextSupplier = asyncContextSupplier; retVal.attributes.putAll(attributes); retVal.headers.putAll(headers); diff --git a/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletResponse.java b/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletResponse.java index a480c0ef656..b484cb56afd 100644 --- a/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletResponse.java +++ b/server/src/test/java/org/apache/druid/server/mocks/MockHttpServletResponse.java @@ -22,6 +22,7 @@ package org.apache.druid.server.mocks; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.servlet.ServletOutputStream; import javax.servlet.WriteListener; @@ -34,6 +35,13 @@ import java.util.Collection; import java.util.LinkedHashMap; import java.util.Locale; +/** + * A fake HttpServletResponse used in tests. A lot of methods are implemented as + * {@code throw new UnsupportedOperationException}, this is just an indication that nobody has needed to flesh out + * that functionality for the mock yet and is not an indication that calling said method is a problem. If an + * {@code throw new UnsupportedOperationException} gets thrown out from one of these methods in a test, it is expected + * that the developer will implement the necessary methods. + */ public class MockHttpServletResponse implements HttpServletResponse { public static MockHttpServletResponse forRequest(MockHttpServletRequest req) @@ -223,13 +231,13 @@ public class MockHttpServletResponse implements HttpServletResponse } @Override - public void write(byte[] b) + public void write(@Nonnull byte[] b) { baos.write(b, 0, b.length); } @Override - public void write(byte[] b, int off, int len) + public void write(@Nonnull byte[] b, int off, int len) { baos.write(b, off, len); }