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

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);
+  }
 }

extensions-core/druid-catalog/src/test/java/org/apache/druid/server/http/catalog/DummyRequest.java

@@ -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<String, Object> 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<String> 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<String> getParameterNames()
-  {
-    return null;
-  }
-
-  @Override
-  public String[] getParameterValues(String name)
-  {
-    return null;
-  }
-
-  @Override
-  public Map<String, String[]> 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<Locale> 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<String> getHeaders(String name)
-  {
-    return null;
-  }
-
-  @Override
-  public Enumeration<String> 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<Part> getParts()
-  {
-    return null;
-  }
-
-  @Override
-  public Part getPart(String name)
-  {
-    return null;
-  }
-
-  @Override
-  public <T extends HttpUpgradeHandler> T upgrade(Class<T> handlerClass)
-  {
-    return null;
-  }
-}

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;

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);

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);
       }