From 5f4139e5d4ece601cc0a45dc32bef155c66defaf Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 25 Jan 2024 10:45:33 +0900 Subject: [PATCH] Make the same change to chat.postMessage with #1274 (#1275) --- .../methods/impl/AsyncMethodsRateLimiter.java | 9 +++ .../api/methods/RateLimitedTest.java | 68 ++++++++++++++++--- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/slack-api-client/src/main/java/com/slack/api/methods/impl/AsyncMethodsRateLimiter.java b/slack-api-client/src/main/java/com/slack/api/methods/impl/AsyncMethodsRateLimiter.java index 859ed5e56..5f44f4643 100644 --- a/slack-api-client/src/main/java/com/slack/api/methods/impl/AsyncMethodsRateLimiter.java +++ b/slack-api-client/src/main/java/com/slack/api/methods/impl/AsyncMethodsRateLimiter.java @@ -1,5 +1,6 @@ package com.slack.api.methods.impl; +import com.slack.api.methods.Methods; import com.slack.api.methods.MethodsConfig; import com.slack.api.methods.MethodsCustomRateLimitResolver; import com.slack.api.methods.MethodsRateLimits; @@ -66,6 +67,14 @@ public int getAllowedRequestsForChatPostMessagePerMinute(String teamId, String c @Override public WaitTime acquireWaitTimeForChatPostMessage(String teamId, String channel) { + // See MethodsClientImpl#buildMethodNameAndSuffix() for the consistency of this logic + String methodName = Methods.CHAT_POST_MESSAGE + "_" + channel; + Optional rateLimitedEpochMillis = waitTimeCalculator + .getRateLimitedMethodRetryEpochMillis(executorName, teamId, methodName); + if (rateLimitedEpochMillis.isPresent()) { + long millisToWait = rateLimitedEpochMillis.get() - System.currentTimeMillis(); + return new WaitTime(millisToWait, RequestPace.RateLimited); + } return waitTimeCalculator.calculateWaitTimeForChatPostMessage( teamId, channel, diff --git a/slack-api-client/src/test/java/test_locally/api/methods/RateLimitedTest.java b/slack-api-client/src/test/java/test_locally/api/methods/RateLimitedTest.java index f2533ccff..b2cf8765d 100644 --- a/slack-api-client/src/test/java/test_locally/api/methods/RateLimitedTest.java +++ b/slack-api-client/src/test/java/test_locally/api/methods/RateLimitedTest.java @@ -2,6 +2,7 @@ import com.slack.api.Slack; import com.slack.api.SlackConfig; +import com.slack.api.methods.Methods; import com.slack.api.methods.MethodsClient; import com.slack.api.methods.SlackApiException; import com.slack.api.rate_limits.metrics.MetricsDatastore; @@ -22,17 +23,20 @@ public class RateLimitedTest { MockSlackApiServer server = new MockSlackApiServer(); - SlackConfig config = new SlackConfig(); - Slack slack = Slack.getInstance(config); - - String executorName = RateLimitedTest.class.getCanonicalName(); + String executorName; + SlackConfig config; + Slack slack; @Before public void setup() throws Exception { server.start(); + executorName = RateLimitedTest.class.getCanonicalName() + "_" + System.currentTimeMillis(); + config = new SlackConfig(); config.getMethodsConfig().setExecutorName(executorName); config.synchronizeMetricsDatabases(); config.setMethodsEndpointUrlPrefix(server.getMethodsEndpointPrefix()); + + slack = Slack.getInstance(config); } @After @@ -54,13 +58,13 @@ public void sync() throws Exception { Integer numOfRequests = datastore.getNumberOfLastMinuteRequests( executorName, "T1234567", - "users.list"); + Methods.USERS_LIST); assertThat(numOfRequests, is(1)); Long millisToResume = datastore.getRateLimitedMethodRetryEpochMillis( executorName, "T1234567", - "users.list"); + Methods.USERS_LIST); assertThat(millisToResume, is(greaterThan(0L))); } } @@ -76,15 +80,63 @@ public void async() throws Exception { Integer numOfRequests = datastore.getNumberOfLastMinuteRequests( executorName, "T1234567", - "users.list"); + Methods.USERS_LIST); + assertThat(numOfRequests, is(1)); + + Long millisToResume = datastore.getRateLimitedMethodRetryEpochMillis( + executorName, + "T1234567", + Methods.USERS_LIST); + assertThat(millisToResume, is(greaterThan(0L))); + } + } + + @Test + public void chatPostMessageSync() throws Exception { + MethodsClient client = slack.methods(RateLimitedToken); + try { + client.chatPostMessage(r -> r.text("Hey!").channel("C12345")); + } catch (SlackApiException e) { + assertThat(e.getResponse().code(), is(429)); + assertThat(e.getResponse().header("Retry-After"), is("3")); + MetricsDatastore datastore = config.getMethodsConfig().getMetricsDatastore(); + log.debug("stats: {}", datastore.getAllStats()); + + Integer numOfRequests = datastore.getNumberOfLastMinuteRequests( + executorName, + "T1234567", + Methods.CHAT_POST_MESSAGE + "_C12345"); assertThat(numOfRequests, is(1)); Long millisToResume = datastore.getRateLimitedMethodRetryEpochMillis( executorName, "T1234567", - "users.list"); + Methods.CHAT_POST_MESSAGE + "_C12345"); assertThat(millisToResume, is(greaterThan(0L))); } } + @Test + public void chatPostMessageAsync() throws Exception { + try { + slack.methodsAsync(RateLimitedToken) + .chatPostMessage(r -> r.text("Hey!").channel("C12345")) + .get(2, TimeUnit.SECONDS); + } catch (Exception ee) { + MetricsDatastore datastore = config.getMethodsConfig().getMetricsDatastore(); + log.debug("stats: {}", datastore.getAllStats()); + + Integer numOfRequests = datastore.getNumberOfLastMinuteRequests( + executorName, + "T1234567", + Methods.CHAT_POST_MESSAGE + "_C12345"); + assertThat(numOfRequests, is(1)); + + Long millisToResume = datastore.getRateLimitedMethodRetryEpochMillis( + executorName, + "T1234567", + Methods.CHAT_POST_MESSAGE + "_C12345"); + assertThat(millisToResume, is(greaterThan(0L))); + } + } }