Skip to content

Commit 3120eb7

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 934fdd9 commit 3120eb7

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.stream.Collectors;
3637

3738
public class SlackSlashCommand implements HttpFunction {
@@ -41,14 +42,24 @@ public class SlackSlashCommand implements HttpFunction {
4142
private static final String SLACK_SECRET = System.getenv("SLACK_SECRET");
4243
private static final Gson gson = new Gson();
4344

44-
private Kgsearch kgClient;
45-
private SlackSignature.Verifier verifier;
45+
private final String apiKey;
46+
private final Kgsearch kgClient;
47+
private final SlackSignature.Verifier verifier;
4648

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

51-
verifier = new SlackSignature.Verifier(new SlackSignature.Generator(SLACK_SECRET));
57+
SlackSlashCommand(SlackSignature.Verifier verifier, String apiKey)
58+
throws IOException, GeneralSecurityException {
59+
this.verifier = verifier;
60+
this.apiKey = apiKey;
61+
this.kgClient = new Kgsearch.Builder(
62+
GoogleNetHttpTransport.newTrustedTransport(), new JacksonFactory(), null).build();
5263
}
5364
// [END functions_slack_setup]
5465

@@ -60,18 +71,13 @@ public SlackSlashCommand() throws IOException, GeneralSecurityException {
6071
* @return true if the provided request came from Slack, false otherwise
6172
*/
6273
boolean isValidSlackWebhook(HttpRequest request, String requestBody) {
63-
6474
// Check for headers
65-
HashMap<String, List<String>> headers = new HashMap(request.getHeaders());
66-
if (!headers.containsKey("X-Slack-Request-Timestamp")
67-
|| !headers.containsKey("X-Slack-Signature")) {
75+
Optional<String> maybeTimestamp = request.getFirstHeader("X-Slack-Request-Timestamp");
76+
Optional<String> maybeSignature = request.getFirstHeader("X-Slack-Signature");
77+
if (!maybeTimestamp.isPresent() || !maybeSignature.isPresent()) {
6878
return false;
6979
}
70-
return verifier.isValid(
71-
headers.get("X-Slack-Request-Timestamp").get(0),
72-
requestBody,
73-
headers.get("X-Slack-Signature").get(0),
74-
1L);
80+
return verifier.isValid(maybeTimestamp.get(), requestBody, maybeSignature.get(), 1L);
7581
}
7682
// [END functions_verify_webhook]
7783

@@ -145,7 +151,7 @@ String formatSlackMessage(JsonObject kgResponse, String query) {
145151
JsonObject searchKnowledgeGraph(String query) throws IOException {
146152
Kgsearch.Entities.Search kgRequest = kgClient.entities().search();
147153
kgRequest.setQuery(query);
148-
kgRequest.setKey(API_KEY);
154+
kgRequest.setKey(apiKey);
149155

150156
return gson.fromJson(kgRequest.execute().toString(), JsonObject.class);
151157
}

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