Skip to content

Commit 4939155

Browse files
eamonnmcmanuseamonnmcmanus
committed
Remove PowerMock from the Slack GCF sample.
Instead of using Whitebox, use package-private constructor overloads to supply the values needed by the test.
1 parent d8df305 commit 4939155

File tree

3 files changed

+39
-67
lines changed

3 files changed

+39
-67
lines changed

‎functions/slack/pom.xml

-19
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
</parent>
3232

3333
<properties>
34-
<powermock.version>2.0.7</powermock.version>
3534
<maven.compiler.target>11</maven.compiler.target>
3635
<maven.compiler.source>11</maven.compiler.source>
3736
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
@@ -101,24 +100,6 @@
101100
<version>29.0-jre</version>
102101
<scope>test</scope>
103102
</dependency>
104-
<dependency>
105-
<groupId>org.powermock</groupId>
106-
<artifactId>powermock-core</artifactId>
107-
<version>${powermock.version}</version>
108-
<scope>test</scope>
109-
</dependency>
110-
<dependency>
111-
<groupId>org.powermock</groupId>
112-
<artifactId>powermock-module-junit4</artifactId>
113-
<version>${powermock.version}</version>
114-
<scope>test</scope>
115-
</dependency>
116-
<dependency>
117-
<groupId>org.powermock</groupId>
118-
<artifactId>powermock-api-mockito2</artifactId>
119-
<version>${powermock.version}</version>
120-
<scope>test</scope>
121-
</dependency>
122103
</dependencies>
123104

124105
<!-- Disable tests during GCF builds (from parent POM) -->

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

+21-15
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.security.GeneralSecurityException;
3333
import java.util.HashMap;
3434
import java.util.List;
35+
import java.util.Optional;
3536
import java.util.logging.Logger;
3637
import java.util.stream.Collectors;
3738

@@ -43,14 +44,24 @@ public class SlackSlashCommand implements HttpFunction {
4344
private static final String SLACK_SECRET = getenv("SLACK_SECRET");
4445
private static final Gson gson = new Gson();
4546

46-
private Kgsearch kgClient;
47-
private SlackSignature.Verifier verifier;
47+
private final String apiKey;
48+
private final Kgsearch kgClient;
49+
private final SlackSignature.Verifier verifier;
4850

4951
public SlackSlashCommand() throws IOException, GeneralSecurityException {
50-
kgClient = new Kgsearch.Builder(
51-
GoogleNetHttpTransport.newTrustedTransport(), new JacksonFactory(), null).build();
52+
this(new SlackSignature.Verifier(new SlackSignature.Generator(SLACK_SECRET)));
53+
}
54+
55+
SlackSlashCommand(SlackSignature.Verifier verifier) throws IOException, GeneralSecurityException {
56+
this(verifier, API_KEY);
57+
}
5258

53-
verifier = new SlackSignature.Verifier(new SlackSignature.Generator(SLACK_SECRET));
59+
SlackSlashCommand(SlackSignature.Verifier verifier, String apiKey)
60+
throws IOException, GeneralSecurityException {
61+
this.verifier = verifier;
62+
this.apiKey = apiKey;
63+
this.kgClient = new Kgsearch.Builder(
64+
GoogleNetHttpTransport.newTrustedTransport(), new JacksonFactory(), null).build();
5465
}
5566

5667
// Avoid ungraceful deployment failures due to unset environment variables.
@@ -73,18 +84,13 @@ private static String getenv(String name) {
7384
* @return true if the provided request came from Slack, false otherwise
7485
*/
7586
boolean isValidSlackWebhook(HttpRequest request, String requestBody) {
76-
7787
// Check for headers
78-
HashMap<String, List<String>> headers = new HashMap(request.getHeaders());
79-
if (!headers.containsKey("X-Slack-Request-Timestamp")
80-
|| !headers.containsKey("X-Slack-Signature")) {
88+
Optional<String> maybeTimestamp = request.getFirstHeader("X-Slack-Request-Timestamp");
89+
Optional<String> maybeSignature = request.getFirstHeader("X-Slack-Signature");
90+
if (!maybeTimestamp.isPresent() || !maybeSignature.isPresent()) {
8191
return false;
8292
}
83-
return verifier.isValid(
84-
headers.get("X-Slack-Request-Timestamp").get(0),
85-
requestBody,
86-
headers.get("X-Slack-Signature").get(0),
87-
1L);
93+
return verifier.isValid(maybeTimestamp.get(), requestBody, maybeSignature.get(), 1L);
8894
}
8995
// [END functions_verify_webhook]
9096

@@ -158,7 +164,7 @@ String formatSlackMessage(JsonObject kgResponse, String query) {
158164
JsonObject searchKnowledgeGraph(String query) throws IOException {
159165
Kgsearch.Entities.Search kgRequest = kgClient.entities().search();
160166
kgRequest.setQuery(query);
161-
kgRequest.setKey(API_KEY);
167+
kgRequest.setKey(apiKey);
162168

163169
return gson.fromJson(kgRequest.execute().toString(), JsonObject.class);
164170
}

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

+18-33
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
package functions;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static org.junit.Assert.assertThrows;
21+
import static org.mockito.Mockito.any;
22+
import static org.mockito.Mockito.anyLong;
2023
import static org.mockito.Mockito.times;
2124
import static org.mockito.Mockito.verify;
22-
import static org.powermock.api.mockito.PowerMockito.mock;
23-
import static org.powermock.api.mockito.PowerMockito.when;
25+
import static org.mockito.Mockito.when;
2426

2527
import com.github.seratch.jslack.app_backend.SlackSignature;
2628
import com.google.api.client.googleapis.json.GoogleJsonResponseException;
@@ -33,14 +35,12 @@
3335
import java.io.StringWriter;
3436
import java.net.HttpURLConnection;
3537
import java.security.GeneralSecurityException;
36-
import java.util.Arrays;
37-
import java.util.HashMap;
3838
import java.util.List;
39+
import java.util.Map;
3940
import org.junit.Before;
4041
import org.junit.Test;
41-
import org.mockito.ArgumentMatchers;
4242
import org.mockito.Mock;
43-
import org.powermock.reflect.Whitebox;
43+
import org.mockito.MockitoAnnotations;
4444

4545
public class SlackSlashCommandTest {
4646

@@ -54,36 +54,26 @@ public class SlackSlashCommandTest {
5454

5555
@Before
5656
public void beforeTest() throws IOException {
57-
request = mock(HttpRequest.class);
58-
when(request.getReader()).thenReturn(new BufferedReader(new StringReader("")));
57+
MockitoAnnotations.initMocks(this);
5958

60-
response = mock(HttpResponse.class);
59+
when(request.getReader()).thenReturn(new BufferedReader(new StringReader("")));
6160

6261
responseOut = new StringWriter();
6362

6463
writerOut = new BufferedWriter(responseOut);
6564
when(response.getWriter()).thenReturn(writerOut);
6665

67-
alwaysValidVerifier = mock(SlackSignature.Verifier.class);
68-
when(alwaysValidVerifier.isValid(
69-
ArgumentMatchers.any(),
70-
ArgumentMatchers.any(),
71-
ArgumentMatchers.any(),
72-
ArgumentMatchers.anyLong())
73-
).thenReturn(true);
66+
when(alwaysValidVerifier.isValid(any(), any(), any(), anyLong())).thenReturn(true);
7467

7568
// Construct valid header list
76-
HashMap<String, List<String>> validHeaders = new HashMap<String, List<String>>();
7769
String validSlackSignature = System.getenv("SLACK_TEST_SIGNATURE");
7870
String timestamp = "0"; // start of Unix epoch
7971

80-
validHeaders.put("X-Slack-Signature", Arrays.asList(validSlackSignature));
81-
validHeaders.put("X-Slack-Request-Timestamp", Arrays.asList(timestamp));
72+
Map<String, List<String>> validHeaders = Map.of(
73+
"X-Slack-Signature", List.of(validSlackSignature),
74+
"X-Slack-Request-Timestamp", List.of(timestamp));
8275

8376
when(request.getHeaders()).thenReturn(validHeaders);
84-
85-
// Reset knowledge graph API key
86-
Whitebox.setInternalState(SlackSlashCommand.class, "API_KEY", System.getenv("KG_API_KEY"));
8777
}
8878

8979
@Test
@@ -120,19 +110,18 @@ public void recognizesValidSlackTokenTest() throws IOException, GeneralSecurityE
120110
verify(response, times(1)).setStatusCode(HttpURLConnection.HTTP_BAD_REQUEST);
121111
}
122112

123-
@Test(expected = GoogleJsonResponseException.class)
113+
@Test
124114
public void handlesSearchErrorTest() throws IOException, GeneralSecurityException {
125115
StringReader requestReadable = new StringReader("{ \"text\": \"foo\" }\n");
126116

127117
when(request.getReader()).thenReturn(new BufferedReader(requestReadable));
128118
when(request.getMethod()).thenReturn("POST");
129119

130-
SlackSlashCommand functionInstance = new SlackSlashCommand();
131-
Whitebox.setInternalState(functionInstance, "verifier", alwaysValidVerifier);
132-
Whitebox.setInternalState(SlackSlashCommand.class, "API_KEY", "gibberish");
120+
SlackSlashCommand functionInstance = new SlackSlashCommand(alwaysValidVerifier, "gibberish");
133121

134122
// Should throw a GoogleJsonResponseException (due to invalid API key)
135-
functionInstance.service(request, response);
123+
assertThrows(
124+
GoogleJsonResponseException.class, () -> functionInstance.service(request, response));
136125
}
137126

138127
@Test
@@ -142,9 +131,7 @@ public void handlesEmptyKgResultsTest() throws IOException, GeneralSecurityExcep
142131
when(request.getReader()).thenReturn(new BufferedReader(requestReadable));
143132
when(request.getMethod()).thenReturn("POST");
144133

145-
SlackSlashCommand functionInstance = new SlackSlashCommand();
146-
Whitebox.setInternalState(functionInstance, "verifier", alwaysValidVerifier);
147-
134+
SlackSlashCommand functionInstance = new SlackSlashCommand(alwaysValidVerifier);
148135

149136
functionInstance.service(request, response);
150137

@@ -159,9 +146,7 @@ public void handlesPopulatedKgResultsTest() throws IOException, GeneralSecurityE
159146
when(request.getReader()).thenReturn(new BufferedReader(requestReadable));
160147
when(request.getMethod()).thenReturn("POST");
161148

162-
SlackSlashCommand functionInstance = new SlackSlashCommand();
163-
Whitebox.setInternalState(functionInstance, "verifier", alwaysValidVerifier);
164-
149+
SlackSlashCommand functionInstance = new SlackSlashCommand(alwaysValidVerifier);
165150

166151
functionInstance.service(request, response);
167152

0 commit comments

Comments
 (0)