Skip to content

Commit

Permalink
Adds message de-duplication cache to work around Slack library bug.
Browse files Browse the repository at this point in the history
  • Loading branch information
outofcoffee committed Jul 20, 2017
1 parent d227371 commit 575d731
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package com.gatehill.corebot.chat

import com.gatehill.corebot.action.model.TriggerContext
import com.gatehill.corebot.config.ChatSettings
import com.google.common.cache.CacheBuilder
import com.ullink.slack.simpleslackapi.SlackSession
import com.ullink.slack.simpleslackapi.events.SlackMessagePosted
import com.ullink.slack.simpleslackapi.listeners.SlackMessagePostedListener
import org.apache.logging.log4j.LogManager
import org.apache.logging.log4j.Logger
import java.util.concurrent.TimeUnit
import java.util.regex.Pattern
import javax.inject.Inject

Expand All @@ -20,6 +23,14 @@ open class SlackChatServiceImpl @Inject constructor(private val sessionService:
private val logger: Logger = LogManager.getLogger(SlackChatServiceImpl::class.java)
private val messageMatcher = Pattern.compile("<@(?<botUser>[a-zA-Z0-9]+)>:?(\\s(?<command>.+))?")

/**
* Caches message IDs for de-duplication purposes.
* This is a work-around for a Slack library bug: [https://github.com/Ullink/simple-slack-api/issues/180]
*/
private val messageIdCache = CacheBuilder.newBuilder()
.expireAfterAccess(ChatSettings.chat.messageIdCache, TimeUnit.SECONDS)
.build<String, String>()

override fun listenForEvents() {
messagePostedListeners.forEach { sessionService.session.addMessagePostedListener(it) }
}
Expand All @@ -40,6 +51,9 @@ open class SlackChatServiceImpl @Inject constructor(private val sessionService:
// ignore own messages
if (session.sessionPersona().id == event.sender.id) return@SlackMessagePostedListener

// filter duplicates
if (isDuplicate(event, session)) return@SlackMessagePostedListener

val trigger = TriggerContext(event.channel.id, event.sender.id, event.sender.userName, event.timestamp, event.threadTimestamp)

try {
Expand All @@ -64,6 +78,26 @@ open class SlackChatServiceImpl @Inject constructor(private val sessionService:
}
})

/**
* Determine whether this message has been processed already.
*/
private fun isDuplicate(event: SlackMessagePosted, session: SlackSession): Boolean {
// According to https://api.slack.com/events-api
// 'The combination of event_ts, team_id, user_id, or channel_id is
// intended to be unique. This field is included with every inner event type'.
val uniqueId = "${event.timeStamp}_${session.team.id}_${event.sender?.id}_${event.channel?.id}"

synchronized(messageIdCache) {
val found = (uniqueId == messageIdCache.getIfPresent(uniqueId))
if (found) {
logger.debug("Duplicate message received: $event")
} else {
messageIdCache.put(uniqueId, uniqueId)
}
return found
}
}

/***
* Execute the `block` if the message is addressed to the bot and return its result.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ object ChatSettings {
(System.getenv("SLACK_CHANNELS") ?: "corebot").split(",").map(String::trim)
}
val postJoinMessage by lazy { System.getenv("SLACK_ENABLE_JOIN_MESSAGE")?.toBoolean() ?: true }

/**
* The time in seconds to cache message IDs for de-duplication purposes.
*/
val messageIdCache by lazy { System.getenv("SLACK_MESSAGE_ID_CACHE")?.toLong() ?: 120 }
}

class Threads {
Expand Down

0 comments on commit 575d731

Please sign in to comment.