Skip to content

Commit

Permalink
Merge pull request #162 from modoboa/fix/slow_perf_manual_learning
Browse files Browse the repository at this point in the history
Enabled asynchronous learning from user using RQ
  • Loading branch information
tonioo authored Oct 20, 2023
2 parents ca35d46 + ac929a6 commit e5ed770
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 33 deletions.
21 changes: 19 additions & 2 deletions .github/workflows/plugin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ jobs:
ports:
- 3306/tcp
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
redis:
image: redis
ports:
- 6379/tcp
options: >-
--health-cmd "redis-cli ping"
--health-interval 10s
--health-timeout 5s
--health-retries 5
strategy:
matrix:
Expand All @@ -51,7 +60,7 @@ jobs:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
sudo apt-get update -y && sudo apt-get install -y librrd-dev rrdtool
sudo apt-get update -y && sudo apt-get install -y librrd-dev rrdtool redis-server
python -m pip install --upgrade pip
#pip install -e git+https://github.com/modoboa/modoboa.git#egg=modoboa
pip install -r requirements.txt
Expand All @@ -62,6 +71,11 @@ jobs:
python setup.py develop
cd ../modoboa-amavis
python setup.py develop
echo "Testing redis connection"
redis-cli -h $REDIS_HOST -p $REDIS_PORT ping
env:
REDIS_HOST: localhost
REDIS_PORT: ${{ job.services.redis.ports[6379] }}
- name: Install postgres requirements
if: ${{ matrix.database == 'postgres' }}
run: |
Expand All @@ -85,7 +99,8 @@ jobs:
MYSQL_HOST: 127.0.0.1
MYSQL_PORT: ${{ job.services.mysql.ports[3306] }} # get randomly assigned published port
MYSQL_USER: root

REDIS_HOST: localhost
REDIS_PORT: ${{ job.services.redis.ports[6379] }}
- name: Test with pytest and coverage
if: ${{ matrix.python-version == '3.10' && matrix.database == 'postgres' }}
run: |
Expand All @@ -100,6 +115,8 @@ jobs:
MYSQL_HOST: 127.0.0.1
MYSQL_PORT: ${{ job.services.mysql.ports[3306] }} # get randomly assigned published port
MYSQL_USER: root
REDIS_HOST: localhost
REDIS_PORT: ${{ job.services.redis.ports[6379] }}
- name: Upload coverage result
if: ${{ matrix.python-version == '3.10' && matrix.database == 'postgres' }}
uses: actions/upload-artifact@v3
Expand Down
37 changes: 37 additions & 0 deletions modoboa_amavis/tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
"""Async tasks."""

from typing import List

from django.utils.translation import ngettext

from modoboa.core import models as core_models

from .lib import SpamassassinClient
from .sql_connector import SQLconnector


def manual_learning(user_pk: int,
mtype: str,
selection: List[str],
recipient_db: str):
"""Task to trigger manual learning for given selection."""
user = core_models.User.objects.get(pk=user_pk)
connector = SQLconnector()
saclient = SpamassassinClient(user, recipient_db)
for item in selection:
rcpt, mail_id = item.split()
content = connector.get_mail_content(mail_id.encode("ascii"))
result = saclient.learn_spam(rcpt, content) if mtype == "spam" \
else saclient.learn_ham(rcpt, content)
if not result:
break
connector.set_msgrcpt_status(
rcpt, mail_id, mtype[0].upper()
)
if saclient.error is None:
saclient.done()
message = ngettext("%(count)d message processed successfully",
"%(count)d messages processed successfully",
len(selection)) % {"count": len(selection)}
else:
message = saclient.error
44 changes: 37 additions & 7 deletions modoboa_amavis/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
import os
from unittest import mock

from rq import SimpleWorker

from django.core import mail
from django.core.management import call_command
from django.test import override_settings
from django.urls import reverse

import django_rq

from modoboa.admin import factories as admin_factories
from modoboa.core import models as core_models
from modoboa.lib.tests import ModoTestCase
Expand Down Expand Up @@ -226,7 +230,10 @@ def _test_mark_message(self, action, status):
data = {"rcpt": smart_str(self.msgrcpt.rid.email)}
response = self.ajax_post(url, data)
self.assertEqual(
response["message"], "1 message processed successfully")
response["message"], "Your request is being processed...")
queue = django_rq.get_queue("default")
worker = SimpleWorker([queue], connection=queue.connection)
worker.work(burst=True)
self.msgrcpt.refresh_from_db()
self.assertEqual(self.msgrcpt.rs, status)

Expand All @@ -235,7 +242,8 @@ def _test_mark_message(self, action, status):
self.set_global_parameter("sa_is_local", False)
response = self.ajax_post(url, data)
self.assertEqual(
response["message"], "1 message processed successfully")
response["message"], "Your request is being processed...")
worker.work(burst=True)
self.msgrcpt.refresh_from_db()
self.assertEqual(self.msgrcpt.rs, status)

Expand Down Expand Up @@ -289,7 +297,7 @@ def test_manual_learning_as_user(self):
self._test_mark_message("ham", "H")

@mock.patch("socket.socket")
def test_process(self, mock_socket):
def test_process_release(self, mock_socket):
"""Test process mode (bulk)."""
# Initiate session
url = reverse("modoboa_amavis:_mail_list")
Expand All @@ -305,8 +313,9 @@ def test_process(self, mock_socket):
smart_str(msgrcpt.rid.email),
smart_str(msgrcpt.mail.mail_id)),
]
mock_socket.return_value.recv.side_effect = (
b"250 1234 Ok\r\n", b"250 1234 Ok\r\n")
mock_socket.return_value.recv.side_effect = [
b"250 1234 Ok\r\n", b"250 1234 Ok\r\n"
]
data = {
"action": "release",
"rcpt": smart_str(self.msgrcpt.rid.email),
Expand All @@ -317,15 +326,36 @@ def test_process(self, mock_socket):
self.assertEqual(
response["message"], "2 messages released successfully")

def test_process_all(self):
"""Test process mode (bulk)."""
# Initiate session
url = reverse("modoboa_amavis:_mail_list")
response = self.ajax_get(url)

msgrcpt = factories.create_spam("[email protected]")
url = reverse("modoboa_amavis:mail_process")
selection = [
"{} {}".format(
smart_str(self.msgrcpt.rid.email),
smart_str(self.msgrcpt.mail.mail_id)),
"{} {}".format(
smart_str(msgrcpt.rid.email),
smart_str(msgrcpt.mail.mail_id)),
]
data = {
"rcpt": smart_str(self.msgrcpt.rid.email),
"selection": ",".join(selection)
}

data["action"] = "mark_as_spam"
response = self.ajax_post(url, data)
self.assertEqual(
response["message"], "2 messages processed successfully")
response["message"], "Your request is being processed...")

data["action"] = "mark_as_ham"
response = self.ajax_post(url, data)
self.assertEqual(
response["message"], "2 messages processed successfully")
response["message"], "Your request is being processed...")

data = {
"action": "delete",
Expand Down
39 changes: 15 additions & 24 deletions modoboa_amavis/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@
from django.utils.translation import gettext as _, ngettext
from django.views.decorators.csrf import csrf_exempt

import django_rq

from modoboa.admin.models import Domain, Mailbox
from modoboa_amavis import tasks
from modoboa.lib.exceptions import BadRequest
from modoboa.lib.paginator import Paginator
from modoboa.lib.web_utils import getctx, render_to_json_response
from modoboa.parameters import tools as param_tools
from . import constants
from .forms import LearningRecipientForm
from .lib import (
AMrelease, QuarantineNavigationParameters, SpamassassinClient,
AMrelease, QuarantineNavigationParameters,
manual_learning_enabled, selfservice
)
from .models import Msgrcpt, Msgs
Expand Down Expand Up @@ -192,7 +195,7 @@ def viewheaders(request, mail_id):
return render(request, "modoboa_amavis/viewheader.html", context)


def check_mail_id(request, mail_id):
def check_mail_id(request, mail_id) -> list:
if isinstance(mail_id, six.string_types):
if "rcpt" in request.POST:
mail_id = ["%s %s" % (request.POST["rcpt"], mail_id)]
Expand Down Expand Up @@ -345,29 +348,17 @@ def mark_messages(request, selection, mtype, recipient_db=None):
"user" if request.user.role == "SimpleUsers" else "global"
)
selection = check_mail_id(request, selection)
connector = SQLconnector()
saclient = SpamassassinClient(request.user, recipient_db)
for item in selection:
rcpt, mail_id = item.split()
content = connector.get_mail_content(mail_id.encode("ascii"))
result = saclient.learn_spam(rcpt, content) if mtype == "spam" \
else saclient.learn_ham(rcpt, content)
if not result:
break
connector.set_msgrcpt_status(
rcpt, mail_id, mtype[0].upper()
)
if saclient.error is None:
saclient.done()
message = ngettext("%(count)d message processed successfully",
"%(count)d messages processed successfully",
len(selection)) % {"count": len(selection)}
else:
message = saclient.error
status = 400 if saclient.error else 200

queue = django_rq.get_queue("default")
queue.enqueue(tasks.manual_learning,
request.user.pk,
mtype,
selection,
recipient_db)

return render_to_json_response({
"message": message, "reload": True
}, status=status)
"message": _("Your request is being processed..."), "reload": True
})


@login_required
Expand Down
15 changes: 15 additions & 0 deletions test_project/test_project/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,21 @@

MODOBOA_API_URL = "https://api.modoboa.org/1/"

# REDIS

REDIS_HOST = os.environ.get('REDIS_HOST', '127.0.0.1')
REDIS_PORT = os.environ.get('REDIS_PORT', 6379)
REDIS_QUOTA_DB = 0
REDIS_URL = 'redis://{}:{}/{}'.format(REDIS_HOST, REDIS_PORT, REDIS_QUOTA_DB)

# RQ

RQ_QUEUES = {
'default': {
'URL': REDIS_URL,
},
}

# Password validation
# https://docs.djangoproject.com/en/1.11/ref/settings/#auth-password-validators

Expand Down

0 comments on commit e5ed770

Please sign in to comment.