Replace re.search() with re.findall() in MessgeListenerMatches middleware to provide better matching results #386

The current implementation of message_listener_matches.py for regex-enabled message search middleware uses re.search() on the search pattern provided by an app. While this is fine in some use cases, it fails to provide all results in other cases, where re.findall() would be more complete.

For exmple, in the test case of:

@app.message(re.compile("We've received ([0-9]+) messages from (.+)!"))

...a message like We've received 42 messages from Jack! results in a tuple containing all matched groups like so:

context['matches'] = ('42', 'Jack')

However, when attempting to match multiple matches for the same group in a single message, the python implementation of re.search() returns only the last match for that group, not all matches. For example, if the regex criteria is:

@app.message(re.compile("([a-z|A-Z]{3,}-[0-9]+)"))

... and a message like We need to fix JIRA-1234 and SCM-5678 is received, the resulting context look like:

context['matches'] = ('JIRA-1234',)

If instead the re.findall() function is used for the above example, the results would be:

context['matches'] = ['JIRA-1234', 'SCM-5678']

I'm not sure why only the first, not last match is returned in the "bad" case, as the python docs suggest, but the fact that only one is being returned due to both matches being from the same capture group is what's important here.

Reproducible in:

pip freeze | grep slack
python --version
sw_vers && uname -v # or `ver`

The slack_bolt version

slack-bolt==1.6.1
slack-sdk==3.7.0

Python runtime version

Python 3.6.9

OS info

WIndows 10 Ubuntu WSL
#488-Microsoft Mon Sep 01 13:43:00 PST 2020

Steps to reproduce:

Sample code:

import os
import re

from slack_bolt import App
from slack_bolt.adapter.socket_mode import SocketModeHandler

# Install the Slack app and get xoxb- token in advance
app = App(token=os.environ["SLACK_BOT_TOKEN"])

@app.message(re.compile("([a-z|A-Z]{3,}-[0-9]+)"))
def get_jira_issue(say, context):
    print(context)    
    for match in context['matches']:
        jira = match
        say(f"I noticed a JIRA issue named {jira}")

@app.message(re.compile("We've received ([0-9]+) messages from (.+)!"))
def do_something(say, context):
    print(context)
    for match in context['matches']:
        p = match
        say(f"You said {p}")

@app.message("hello")
def get_jira_issue_too(message, say):
    print(message)
    user = message['user']
    say(f"hello, {user}")

if __name__ == "__main__":
    SocketModeHandler(app, os.environ["SLACK_APP_TOKEN"]).start()

Sample messages sent to Slack channel:

We've received 42 messages from your Jack!
We need to fix JIRA-1234 and SCM-5678

Expected result:

Expected context for first message:

context['matches'] = ('42', 'Jack')

Expected slack channel output:

You said 42
You said Jack

Expected context for second message:

context['matches'] = ['JIRA-1234', 'SCM-5678']

Expected slack channel output:

I noticed a JIRA issue named JIRA-1234
I noticed a JIRA issue named SCM-5678

Actual result:

Result for first test case is as expected and matches current behavior. Result for second case is as follows:
Actual context for second message:

context['matches'] = ('JIRA-1234',)

Actual slack channel output:

I noticed a JIRA issue named JIRA-1234

Using current API implementation, the only way to properly retrieve multiple matches for a single capture group in a single message is to manually filter through all messages instead of using the regex capability, shifting the overhead of this search operation to the application instead of the API.

Requirements

I have already built and tested an improvement for this issue which returns the same results as the prior version for the test case currently in the repository, but also provides the desired results when there are multiple matches in the same capture group, i.e. the existing test case still returns the expected tuple, but the new test case returns a list of matches from the same capture group as desired.

I can also propose an alternate solution which always returns a list of individual matches, instead of tuples, but this would modify the current existing behavior that might be out in the wild.