Remove PowerMock from the Slack GCF sample. (#3186)

* Remove PowerMock from the Slack GCF sample.

Instead of using Whitebox, use package-private constructor overloads
to supply the values needed by the test.

* fix lint

* Fix the tests

Co-authored-by: Éamonn McManus <[email protected]>

Author: saturnism

functions/slack/pom.xml

@@ -31,7 +31,6 @@
   </parent>
 
   <properties>
-    <powermock.version>2.0.7</powermock.version>
     <maven.compiler.target>11</maven.compiler.target>
     <maven.compiler.source>11</maven.compiler.source>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
@@ -101,24 +100,6 @@
       <version>29.0-jre</version>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.powermock</groupId>
-      <artifactId>powermock-core</artifactId>
-      <version>${powermock.version}</version>
-      <scope>test</scope>
-    </dependency>
-    <dependency>
-      <groupId>org.powermock</groupId>
-      <artifactId>powermock-module-junit4</artifactId>
-      <version>${powermock.version}</version>
-      <scope>test</scope>
-    </dependency>
-    <dependency>
-      <groupId>org.powermock</groupId>
-      <artifactId>powermock-api-mockito2</artifactId>
-      <version>${powermock.version}</version>
-      <scope>test</scope>
-    </dependency>
   </dependencies>
 
   <!-- Disable tests during GCF builds (from parent POM) -->

functions/slack/src/main/java/functions/SlackSlashCommand.java

@@ -30,8 +30,7 @@
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.security.GeneralSecurityException;
-import java.util.HashMap;
-import java.util.List;
+import java.util.Optional;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -43,14 +42,24 @@ public class SlackSlashCommand implements HttpFunction {
   private static final String SLACK_SECRET = getenv("SLACK_SECRET");
   private static final Gson gson = new Gson();
 
-  private Kgsearch kgClient;
-  private SlackSignature.Verifier verifier;
+  private final String apiKey;
+  private final Kgsearch kgClient;
+  private final SlackSignature.Verifier verifier;
 
   public SlackSlashCommand() throws IOException, GeneralSecurityException {
-    kgClient = new Kgsearch.Builder(
-        GoogleNetHttpTransport.newTrustedTransport(), new JacksonFactory(), null).build();
+    this(new SlackSignature.Verifier(new SlackSignature.Generator(SLACK_SECRET)));
+  }
+
+  SlackSlashCommand(SlackSignature.Verifier verifier) throws IOException, GeneralSecurityException {
+    this(verifier, API_KEY);
+  }
 
-    verifier = new SlackSignature.Verifier(new SlackSignature.Generator(SLACK_SECRET));
+  SlackSlashCommand(SlackSignature.Verifier verifier, String apiKey)
+      throws IOException, GeneralSecurityException {
+    this.verifier = verifier;
+    this.apiKey = apiKey;
+    this.kgClient = new Kgsearch.Builder(
+        GoogleNetHttpTransport.newTrustedTransport(), new JacksonFactory(), null).build();
   }
 
   // Avoid ungraceful deployment failures due to unset environment variables.
@@ -73,18 +82,13 @@ private static String getenv(String name) {
    * @return true if the provided request came from Slack, false otherwise
    */
   boolean isValidSlackWebhook(HttpRequest request, String requestBody) {
-
     // Check for headers
-    HashMap<String, List<String>> headers = new HashMap(request.getHeaders());
-    if (!headers.containsKey("X-Slack-Request-Timestamp")
-        || !headers.containsKey("X-Slack-Signature")) {
+    Optional<String> maybeTimestamp = request.getFirstHeader("X-Slack-Request-Timestamp");
+    Optional<String> maybeSignature = request.getFirstHeader("X-Slack-Signature");
+    if (!maybeTimestamp.isPresent() || !maybeSignature.isPresent()) {
       return false;
     }
-    return verifier.isValid(
-        headers.get("X-Slack-Request-Timestamp").get(0),
-        requestBody,
-        headers.get("X-Slack-Signature").get(0),
-        1L);
+    return verifier.isValid(maybeTimestamp.get(), requestBody, maybeSignature.get(), 1L);
   }
   // [END functions_verify_webhook]
 
@@ -158,7 +162,7 @@ String formatSlackMessage(JsonObject kgResponse, String query) {
   JsonObject searchKnowledgeGraph(String query) throws IOException {
     Kgsearch.Entities.Search kgRequest = kgClient.entities().search();
     kgRequest.setQuery(query);
-    kgRequest.setKey(API_KEY);
+    kgRequest.setKey(apiKey);
 
     return gson.fromJson(kgRequest.execute().toString(), JsonObject.class);
   }

functions/slack/src/test/java/functions/SlackSlashCommandTest.java

@@ -17,10 +17,12 @@
 package functions;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyLong;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.powermock.api.mockito.PowerMockito.mock;
-import static org.powermock.api.mockito.PowerMockito.when;
+import static org.mockito.Mockito.when;
 
 import com.github.seratch.jslack.app_backend.SlackSignature;
 import com.google.api.client.googleapis.json.GoogleJsonResponseException;
@@ -34,15 +36,12 @@
 import java.io.StringWriter;
 import java.net.HttpURLConnection;
 import java.security.GeneralSecurityException;
-import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.ArgumentMatchers;
 import org.mockito.Mock;
-import org.powermock.reflect.Whitebox;
+import org.mockito.MockitoAnnotations;
 
 public class SlackSlashCommandTest {
 
@@ -58,37 +57,27 @@ public class SlackSlashCommandTest {
 
   @Before
   public void beforeTest() throws IOException {
-    request = mock(HttpRequest.class);
-    when(request.getReader()).thenReturn(new BufferedReader(new StringReader("")));
+    MockitoAnnotations.initMocks(this);
 
-    response = mock(HttpResponse.class);
+    when(request.getReader()).thenReturn(new BufferedReader(new StringReader("")));
 
     responseOut = new StringWriter();
 
     writerOut = new BufferedWriter(responseOut);
     when(response.getWriter()).thenReturn(writerOut);
 
-    alwaysValidVerifier = mock(SlackSignature.Verifier.class);
-    when(alwaysValidVerifier.isValid(
-          ArgumentMatchers.any(),
-          ArgumentMatchers.any(),
-          ArgumentMatchers.any(),
-          ArgumentMatchers.anyLong())
-    ).thenReturn(true);
+    when(alwaysValidVerifier.isValid(any(), any(), any(), anyLong())).thenReturn(true);
 
     // Construct valid header list
     String validSlackSignature = System.getenv("SLACK_TEST_SIGNATURE");
     String timestamp = "0"; // start of Unix epoch
 
     Map<String, List<String>> validHeaders = Map.of(
-        "X-Slack-Signature", Arrays.asList(validSlackSignature),
-        "X-Slack-Request-Timestamp", Arrays.asList(timestamp)
-    );
+        "X-Slack-Signature", List.of(validSlackSignature),
+        "X-Slack-Request-Timestamp", List.of(timestamp));
 
     when(request.getHeaders()).thenReturn(validHeaders);
-
-    // Reset knowledge graph API key
-    Whitebox.setInternalState(SlackSlashCommand.class, "API_KEY", System.getenv("KG_API_KEY"));
+    when(request.getFirstHeader(any())).thenCallRealMethod();
   }
 
   @Test
@@ -126,20 +115,19 @@ public void recognizesValidSlackTokenTest() throws IOException, GeneralSecurityE
     verify(response, times(1)).setStatusCode(HttpURLConnection.HTTP_BAD_REQUEST);
   }
 
-  @Test(expected = GoogleJsonResponseException.class)
+  @Test
   public void handlesSearchErrorTest() throws IOException, GeneralSecurityException {
     String jsonStr = gson.toJson(Map.of("text", "foo"));
     StringReader requestReadable = new StringReader(jsonStr);
 
     when(request.getReader()).thenReturn(new BufferedReader(requestReadable));
     when(request.getMethod()).thenReturn("POST");
 
-    SlackSlashCommand functionInstance = new SlackSlashCommand();
-    Whitebox.setInternalState(functionInstance, "verifier", alwaysValidVerifier);
-    Whitebox.setInternalState(SlackSlashCommand.class, "API_KEY", "gibberish");
+    SlackSlashCommand functionInstance = new SlackSlashCommand(alwaysValidVerifier, "gibberish");
 
     // Should throw a GoogleJsonResponseException (due to invalid API key)
-    functionInstance.service(request, response);
+    assertThrows(
+        GoogleJsonResponseException.class, () -> functionInstance.service(request, response));
   }
 
   @Test
@@ -150,9 +138,7 @@ public void handlesEmptyKgResultsTest() throws IOException, GeneralSecurityExcep
     when(request.getReader()).thenReturn(new BufferedReader(requestReadable));
     when(request.getMethod()).thenReturn("POST");
 
-    SlackSlashCommand functionInstance = new SlackSlashCommand();
-    Whitebox.setInternalState(functionInstance, "verifier", alwaysValidVerifier);
-
+    SlackSlashCommand functionInstance = new SlackSlashCommand(alwaysValidVerifier);
 
     functionInstance.service(request, response);
 
@@ -168,9 +154,7 @@ public void handlesPopulatedKgResultsTest() throws IOException, GeneralSecurityE
     when(request.getReader()).thenReturn(new BufferedReader(requestReadable));
     when(request.getMethod()).thenReturn("POST");
 
-    SlackSlashCommand functionInstance = new SlackSlashCommand();
-    Whitebox.setInternalState(functionInstance, "verifier", alwaysValidVerifier);
-
+    SlackSlashCommand functionInstance = new SlackSlashCommand(alwaysValidVerifier);
 
     functionInstance.service(request, response);