You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/03/02 14:36:43 UTC
[GitHub] [camel] omarsmak commented on a change in pull request #5121: CAMEL-16255 Use of the slack-api-model to create rich content
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:
users@infra.apache.org