omarsmak commented on a change in pull request #5121:
URL: https://github.com/apache/camel/pull/5121#discussion_r585588672
##########
File path:
components/camel-slack/src/main/java/org/apache/camel/component/slack/SlackComponentVerifierExtension.java
##########
@@ -118,23 +101,12 @@ private void verifyCredentials(ResultBuilder builder,
Map<String, Object> parame
String token = (String) parameters.get("token");
try {
- HttpClient client =
HttpClientBuilder.create().useSystemProperties().build();
- HttpPost httpPost = new HttpPost(parameters.get("serverUrl") +
"/api/conversations.list");
-
- List<BasicNameValuePair> params = new ArrayList<>();
- params.add(new BasicNameValuePair("token", token));
- httpPost.setEntity(new UrlEncodedFormEntity(params));
+ ConversationsListResponse response = Slack.getInstance(new
CustomSlackHttpClient()).methods(token)
+ .conversationsList(req -> req
+
.types(Collections.singletonList(ConversationType.PUBLIC_CHANNEL))
Review comment:
If we only have private channels, does still return `200`?
##########
File path:
components/camel-slack/src/main/java/org/apache/camel/component/slack/SlackConstants.java
##########
@@ -16,6 +16,7 @@
*/
package org.apache.camel.component.slack;
+@Deprecated
Review comment:
These constants before used to build the Json request to Slack APIs,
isn't? Is there any reason to deprecate them instead of removing this class
entirely?
##########
File path:
components/camel-slack/src/main/java/org/apache/camel/component/slack/SlackConsumer.java
##########
@@ -141,68 +129,40 @@ public int processBatch(Queue<Object> exchanges) throws
Exception {
return total;
}
- private String getChannelId(String channel) throws IOException,
DeserializationException {
- HttpPost httpPost = new HttpPost(slackEndpoint.getServerUrl() +
"/api/conversations.list");
-
- List<BasicNameValuePair> params = new ArrayList<>();
- params.add(new BasicNameValuePair("token", slackEndpoint.getToken()));
- httpPost.setEntity(new UrlEncodedFormEntity(params));
-
- HttpResponse response = client.execute(httpPost);
-
- String jsonString = readResponse(response);
- JsonObject c = (JsonObject) Jsoner.deserialize(jsonString);
-
- checkSlackReply(c);
-
- Collection<JsonObject> channels = c.getCollection("channels");
- if (channels == null) {
- throw new RuntimeCamelException("The response was successful but
no channel list was provided");
- }
-
- for (JsonObject singleChannel : channels) {
- if (singleChannel.get("name") != null) {
- if (singleChannel.get("name").equals(channel)) {
- if (singleChannel.get("id") != null) {
- return (String) singleChannel.get("id");
- }
- }
+ private String getChannelId(final String channel, final String cursor) {
+ try {
+ // Maximum limit is 1000. Slack recommends no more than 200
results at a time.
+ // https://api.slack.com/methods/conversations.list
+ ConversationsListResponse response =
slack.methods(slackEndpoint.getToken()).conversationsList(req -> req
+
.types(Collections.singletonList(slackEndpoint.getConversationType()))
+ .cursor(cursor)
+ .limit(200));
Review comment:
Can we move the limit number into a constant somewhere? Let's say this
class
##########
File path:
components/camel-slack/src/main/java/org/apache/camel/component/slack/SlackConsumer.java
##########
@@ -141,68 +129,40 @@ public int processBatch(Queue<Object> exchanges) throws
Exception {
return total;
}
- private String getChannelId(String channel) throws IOException,
DeserializationException {
- HttpPost httpPost = new HttpPost(slackEndpoint.getServerUrl() +
"/api/conversations.list");
-
- List<BasicNameValuePair> params = new ArrayList<>();
- params.add(new BasicNameValuePair("token", slackEndpoint.getToken()));
- httpPost.setEntity(new UrlEncodedFormEntity(params));
-
- HttpResponse response = client.execute(httpPost);
-
- String jsonString = readResponse(response);
- JsonObject c = (JsonObject) Jsoner.deserialize(jsonString);
-
- checkSlackReply(c);
-
- Collection<JsonObject> channels = c.getCollection("channels");
- if (channels == null) {
- throw new RuntimeCamelException("The response was successful but
no channel list was provided");
- }
-
- for (JsonObject singleChannel : channels) {
- if (singleChannel.get("name") != null) {
- if (singleChannel.get("name").equals(channel)) {
- if (singleChannel.get("id") != null) {
- return (String) singleChannel.get("id");
- }
- }
+ private String getChannelId(final String channel, final String cursor) {
+ try {
+ // Maximum limit is 1000. Slack recommends no more than 200
results at a time.
+ // https://api.slack.com/methods/conversations.list
+ ConversationsListResponse response =
slack.methods(slackEndpoint.getToken()).conversationsList(req -> req
+
.types(Collections.singletonList(slackEndpoint.getConversationType()))
+ .cursor(cursor)
+ .limit(200));
+
+ if (!response.isOk()) {
+ throw new RuntimeCamelException("API request
conversations.list to Slack failed: " + response);
}
- }
- return jsonString;
+ return response.getChannels().stream()
+ .filter(it -> it.getName().equals(channel))
+ .map(Conversation::getId)
+ .findFirst().orElseGet(() -> {
+ if
(isNullOrEmpty(response.getResponseMetadata().getNextCursor())) {
+ throw new
RuntimeCamelException(String.format("Channel %s not found", channel));
+ }
+ return getChannelId(channel,
response.getResponseMetadata().getNextCursor());
+ });
+ } catch (IOException | SlackApiException e) {
+ throw new RuntimeCamelException("API request conversations.list to
Slack failed", e);
+ }
}
- private void checkSlackReply(JsonObject c) {
- boolean okStatus = c.getBoolean("ok");
-
- if (!okStatus) {
- String errorMessage = c.getString("error");
-
- if (errorMessage == null || errorMessage.isEmpty()) {
- errorMessage = "the slack server did not provide error
details";
- }
-
- throw new RuntimeCamelException(String.format("API request to
Slack failed: %s", errorMessage));
- }
+ private static boolean isNullOrEmpty(String str) {
+ return str == null || str.isEmpty();
Review comment:
You can use `ObjectHelper.isNotEmpty()` in `org.apache.camel.util` to
check for empty strings as well nulls
##########
File path:
components/camel-slack/src/main/java/org/apache/camel/component/slack/SlackConsumer.java
##########
@@ -17,102 +17,90 @@
package org.apache.camel.component.slack;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
+import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Queue;
+import com.slack.api.Slack;
+import com.slack.api.methods.SlackApiException;
+import
com.slack.api.methods.response.conversations.ConversationsHistoryResponse;
+import com.slack.api.methods.response.conversations.ConversationsListResponse;
+import com.slack.api.model.Conversation;
+import com.slack.api.model.Message;
import org.apache.camel.Exchange;
-import org.apache.camel.Message;
import org.apache.camel.Processor;
import org.apache.camel.RuntimeCamelException;
-import org.apache.camel.component.slack.helper.SlackMessage;
import org.apache.camel.support.ScheduledBatchPollingConsumer;
import org.apache.camel.util.CastUtils;
import org.apache.camel.util.ObjectHelper;
-import org.apache.camel.util.json.DeserializationException;
-import org.apache.camel.util.json.JsonArray;
-import org.apache.camel.util.json.JsonObject;
-import org.apache.camel.util.json.Jsoner;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.entity.UrlEncodedFormEntity;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.message.BasicNameValuePair;
-
-import static org.apache.camel.component.slack.utils.SlackUtils.readResponse;
public class SlackConsumer extends ScheduledBatchPollingConsumer {
- private SlackEndpoint slackEndpoint;
+ private final SlackEndpoint slackEndpoint;
+ private Slack slack;
private String timestamp;
private String channelId;
- private CloseableHttpClient client;
- public SlackConsumer(SlackEndpoint endpoint, Processor processor) throws
IOException, DeserializationException {
+ public SlackConsumer(SlackEndpoint endpoint, Processor processor) {
super(endpoint, processor);
this.slackEndpoint = endpoint;
}
@Override
protected void doStart() throws Exception {
- this.client = HttpClientBuilder.create().useSystemProperties().build();
+ this.slack = Slack.getInstance();
Review comment:
This instance of the client, differs from the one that was used in the
verifier whereby the `CustomSlackHttpClient`, is this intentional?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
