Skip to content

Commit

Permalink
Update linters formatters and supported versions (#29)
Browse files Browse the repository at this point in the history
* Bump dev container image to python:3.8-bookworm
* Removed the black config to skip string normalization and then ran black against all the things
* removed old yapf config
* Clean up docker-compose.yml a bit
* Added ruff for linting
* Update github actions to run ruff, drop testing django 3.2, add testing django 5.0, 5.1, 5.2
* drop testing on Django 4.1, add testing on Django 5.2 and Python 3.13
  • Loading branch information
jmichalicek committed Aug 11, 2024
1 parent 8b10330 commit 7d58e87
Show file tree
Hide file tree
Showing 28 changed files with 369 additions and 344 deletions.
14 changes: 9 additions & 5 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13.0-rc.1"]
django-version: ["django==4.2.*", "django==5.0.*", "django==5.1.*", "django==5.2.*"]
exclude:
- python-version: "3.12"
django-version: "django<4.2.8"
- python-version: "3.13.0-rc.1"
django-version: "django<5.2"
steps:
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.python-version }}
Expand All @@ -24,11 +30,9 @@ jobs:
run: |
python -m pip install --upgrade pip
pip install -r requirements_test.txt
- name: Lint with flake8
- name: Lint with ruff
run: |
# stop the build if there are Python syntax errors or undefined names
flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
make ruff-check
- name: Run Tox
run: tox
9 changes: 0 additions & 9 deletions .style.yapf

This file was deleted.

3 changes: 1 addition & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM python:3.7-buster AS dev
FROM python:3.8-bookworm AS dev
# Dockerfile for running a test django installation
LABEL maintainer="Justin Michalicek <jmichalicek@gmail.com>"
ENV PYTHONUNBUFFERED 1
Expand All @@ -8,7 +8,6 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get update && DEBIAN_FRONTEND=noninteract
software-properties-common \
sudo \
vim \
telnet \
postgresql-client \
&& apt-get autoremove && apt-get clean

Expand Down
14 changes: 14 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ setup-and-run: setup migrate run

venv:
python -m venv .venv
pip install --upgrade pip wheel setuptools

run:
python manage.py runserver 0.0.0.0:8000
Expand All @@ -81,8 +82,21 @@ migrate:

dev:
docker compose run --service-ports django /bin/bash

shell:
docker compose exec django /bin/bash

install-mailviewer:
pip install -e /django/mailviewer --no-binary :all:

black:
black django_mail_viewer tests test_project

black-check:
black --check --diff django_mail_viewer tests test_project

ruff:
ruff check django_mail_viewer --fix

ruff-check:
ruff check django_mail_viewer
2 changes: 1 addition & 1 deletion django_mail_viewer/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@


class DjangoMailViewerConfig(AppConfig):
name = 'django_mail_viewer'
name = "django_mail_viewer"
verbose_name = "Django Mail Viewer"
9 changes: 5 additions & 4 deletions django_mail_viewer/backends/cache.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Backend for test environment.
"""

from contextlib import contextmanager
from os import getpid
from time import monotonic, sleep
Expand All @@ -27,14 +28,14 @@ def __init__(self, *args, **kwargs):
# This is for get_outbox() so that the system knows which cache keys are there
# to retrieve them. Django does not have a built in way to get the keys
# which exist in the cache.
self.cache_keys_key = 'message_keys'
self.cache_keys_lock_key = 'message_keys_lock'
self.cache_keys_key = "message_keys"
self.cache_keys_lock_key = "message_keys_lock"

def send_messages(self, messages):
msg_count = 0
for message in messages:
m = message.message()
message_id = m.get('message-id')
message_id = m.get("message-id")
self.cache.set(message_id, m)

# Use a lock key and spinlock
Expand All @@ -57,7 +58,7 @@ def send_messages(self, messages):
msg_count += 1
is_stored = True
else:
sleep(.01)
sleep(0.01)
return msg_count

def get_message(self, lookup_id):
Expand Down
6 changes: 3 additions & 3 deletions django_mail_viewer/backends/database/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@


class EmailMessageAdmin(admin.ModelAdmin):
list_display = ('pk', 'parent', 'message_id', 'created_at', 'updated_at')
search_fields = ('pk', 'message_id', 'message_headers')
readonly_fields = ('created_at', 'updated_at')
list_display = ("pk", "parent", "message_id", "created_at", "updated_at")
search_fields = ("pk", "message_id", "message_headers")
readonly_fields = ("created_at", "updated_at")


admin.site.register(EmailMessage, EmailMessageAdmin)
4 changes: 2 additions & 2 deletions django_mail_viewer/backends/database/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@


class DatabaseBackendConfig(AppConfig):
label = 'mail_viewer_database_backend'
name = 'django_mail_viewer.backends.database'
label = "mail_viewer_database_backend"
name = "django_mail_viewer.backends.database"
# May want to change this verbose_name
verbose_name = _("Database Backend")
28 changes: 14 additions & 14 deletions django_mail_viewer/backends/database/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ def _parse_email_attachment(self, message, decode_file=True):
elif name == "read-date":
attachment.read_date = value # TODO: datetime
return {
'filename': Path(message.get_filename()).name,
'content_type': message.get_content_type(),
'file': attachment,
"filename": Path(message.get_filename()).name,
"content_type": message.get_content_type(),
"file": attachment,
}
return None

Expand All @@ -76,30 +76,30 @@ def send_messages(self, messages):
if message.is_multipart():
# TODO: Should this really be done recursively? I believe forwarded emails may
# have multiple layers of parts/dispositions
message_id = message.get('message-id')
message_id = message.get("message-id")
main_message = None
for i, part in enumerate(message.walk()):
content_type = part.get_content_type()
charset = part.get_param('charset')
charset = part.get_param("charset")
# handle attachments - probably need to look at SingleEmailMixin._parse_email_attachment()
# and make that more reusable
content_disposition = part.get("Content-Disposition", None)
if content_disposition:
# attachment_data = part.get_payload(decode=True)
attachment_data = self._parse_email_attachment(part)
file_attachment = ContentFile(
attachment_data.get('file').read(), name=attachment_data.get('filename', 'attachment')
attachment_data.get("file").read(), name=attachment_data.get("filename", "attachment")
)
content = ''
elif content_type in ['text/plain', 'text/html']:
content = part.get_payload(decode=True).decode(charset, errors='replace')
file_attachment = ''
content = ""
elif content_type in ["text/plain", "text/html"]:
content = part.get_payload(decode=True).decode(charset, errors="replace")
file_attachment = ""
else:
# the main multipart/alternative message for multipart messages has no content/payload
# TODO: handle file attachments
content = ''
file_attachment = ''
message_id = part.get('message-id', '') # do sub-parts have a message-id?
content = ""
file_attachment = ""
message_id = part.get("message-id", "") # do sub-parts have a message-id?
p = self._backend_model(
message_id=message_id,
content=content,
Expand All @@ -111,7 +111,7 @@ def send_messages(self, messages):
if i == 0:
main_message = p
else:
message_id = message.get('message-id')
message_id = message.get("message-id")
main_message = self._backend_model(
message_id=message_id,
content=message.get_payload(),
Expand Down
52 changes: 26 additions & 26 deletions django_mail_viewer/backends/database/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ class AbstractBaseEmailMessage(models.Model):
# Technically optional, but really should be there according to RFC 5322 section 3.6.4
# and Django always creates the message_id on the main part of the message so we know
# it will be there, but not for all sub-parts of a multi-part message
message_id = models.CharField(max_length=250, blank=True, default='')
message_id = models.CharField(max_length=250, blank=True, default="")
# Would love to make message_headers be a JSONField, but do not want to tie this to
# postgres only.
message_headers = models.TextField()
content = models.TextField(blank=True, default='')
content = models.TextField(blank=True, default="")
parent = models.ForeignKey(
'self', blank=True, null=True, default=None, related_name='parts', on_delete=models.CASCADE
"self", blank=True, null=True, default=None, related_name="parts", on_delete=models.CASCADE
)
created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True)
Expand All @@ -38,8 +38,8 @@ def __init__(self, *args, **kwargs):

# methods here expect the concrete subclasses to implement the file_attachment field
# should only be necessary until django 2.2 support is dropped... I hope
if TYPE_CHECKING and not hasattr(self, 'file_attachment'):
self.file_attachment = models.FileField(blank=True, default='', upload_to='mailviewer_attachments')
if TYPE_CHECKING and not hasattr(self, "file_attachment"):
self.file_attachment = models.FileField(blank=True, default="", upload_to="mailviewer_attachments")

# I really only need/use get_filename(), get_content_type(), get_payload(), walk()
# returns Any due to failobj
Expand All @@ -59,14 +59,14 @@ def get(self, attr: str, failobj: Any = None) -> Any:
return failobj

def date(self) -> str:
return self.get('date')
return self.get("date")

def is_multipart(self) -> bool:
"""
Returns True if the message is multipart
"""
# Not certain the self.parts.all() is accurate
return self.get_content_type() == 'rfc/822' or self.parts.exists() # type: ignore
return self.get_content_type() == "rfc/822" or self.parts.exists() # type: ignore

def headers(self) -> Dict[str, str]:
"""
Expand All @@ -81,13 +81,13 @@ def values(self) -> Dict[str, str]:
# not sure this is right...
return self.headers()

def walk(self) -> 'Union[models.QuerySet[AbstractBaseEmailMessage], List[AbstractBaseEmailMessage]]':
def walk(self) -> "Union[models.QuerySet[AbstractBaseEmailMessage], List[AbstractBaseEmailMessage]]":
if not self.parts.all().exists(): # type: ignore
# Or should I be saving a main message all the time and even just a plaintext has a child part, hmm
return [self]
return self.parts.all().order_by('-created_at', 'id') # type: ignore
return self.parts.all().order_by("-created_at", "id") # type: ignore

def get_param(self, param: str, failobj=None, header: str = 'content-type', unquote: bool = True) -> str:
def get_param(self, param: str, failobj=None, header: str = "content-type", unquote: bool = True) -> str:
"""
Return the value of the Content-Type header’s parameter param as a string. If the message has no Content-Type header or if there is no such parameter, then failobj is returned (defaults to None).
Expand All @@ -97,24 +97,24 @@ def get_param(self, param: str, failobj=None, header: str = 'content-type', unqu
# TODO: error handling skipped for sure here... need to see what the real email message does
# Should also consider using cgi.parse_header
h = self.get(header)
params = h.split(';')
params = h.split(";")
for part in params[1:]:
part_name, part_val = part.split('=')
part_name, part_val = part.split("=")
part_name = part_name.strip()
part_val = part_val.strip()
if part_name == param:
return part_val
return ''
return ""

def get_payload(
self, i: Union[int, None] = None, decode: bool = False
) -> 'Union[bytes, AbstractBaseEmailMessage, models.QuerySet[AbstractBaseEmailMessage]]':
) -> "Union[bytes, AbstractBaseEmailMessage, models.QuerySet[AbstractBaseEmailMessage]]":
"""
Temporary backwards compatibility with email.message.Message
"""
# TODO: sort out type hint for return value here. Maybe use monkeytype to figure this out.
if not self.is_multipart():
charset = self.get_param('charset')
charset = self.get_param("charset")
if self.file_attachment:
self.file_attachment.seek(0)
try:
Expand All @@ -134,18 +134,18 @@ def get_content_type(self) -> str:
"""
Return's the message's content-type or mime type.
"""
h = self.get('content-type')
params = h.split(';')
h = self.get("content-type")
params = h.split(";")
return params[0]

def get_filename(self, failobj=None) -> str:
content_disposition = self.headers().get('Content-Disposition', '')
parts = content_disposition.split(';')
content_disposition = self.headers().get("Content-Disposition", "")
parts = content_disposition.split(";")
for part in parts:
if part.strip().startswith('filename'):
filename = part.split('=')[1].strip('"').strip()
if part.strip().startswith("filename"):
filename = part.split("=")[1].strip('"').strip()
return email.utils.unquote(filename)
return ''
return ""


class EmailMessage(AbstractBaseEmailMessage):
Expand All @@ -160,9 +160,9 @@ class EmailMessage(AbstractBaseEmailMessage):
it just needs to be stored elsewhere, such as locally, or a different s3 bucket than the default storage.
"""

file_attachment = models.FileField(blank=True, default='', upload_to='mailviewer_attachments')
file_attachment = models.FileField(blank=True, default="", upload_to="mailviewer_attachments")

class Meta:
db_table = 'mail_viewer_emailmessage'
ordering = ('id',)
indexes = [models.Index(fields=['message_id'])]
db_table = "mail_viewer_emailmessage"
ordering = ("id",)
indexes = [models.Index(fields=["message_id"])]
11 changes: 6 additions & 5 deletions django_mail_viewer/backends/locmem.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Backend for test environment.
"""

from django.core import mail
from django.core.mail.backends.base import BaseEmailBackend

Expand All @@ -18,7 +19,7 @@ class EmailBackend(BaseEmailBackend):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if not hasattr(mail, 'outbox'):
if not hasattr(mail, "outbox"):
mail.outbox = []

def send_messages(self, messages):
Expand All @@ -38,7 +39,7 @@ def get_message(self, lookup_id):
# differently than the expected Message-ID, which is suppored by
# EmailMessage.message(), then we can't just access the key directly. Instead iterate
# over the keys and vls
if message.get('message-id') == lookup_id:
if message.get("message-id") == lookup_id:
return message
return None

Expand All @@ -47,16 +48,16 @@ def get_outbox(self, *args, **kwargs):
Get the outbox used by this backend. This backend returns a copy of mail.outbox.
May add pagination args/kwargs.
"""
return getattr(mail, 'outbox', [])[:]
return getattr(mail, "outbox", [])[:]

def delete_message(self, message_id: str):
"""
Remove the message with the given id from the mailbox
"""
outbox = getattr(mail, 'outbox', [])
outbox = getattr(mail, "outbox", [])
index_to_remove = None
for idx, message in enumerate(outbox):
if message.get('message-id') == message_id:
if message.get("message-id") == message_id:
index_to_remove = idx
break
if index_to_remove is not None:
Expand Down
6 changes: 4 additions & 2 deletions django_mail_viewer/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@

# The cache config from django.core.cache.caches to use for backends.cache.CacheBackend
# default to django.core.cache.caches['default']
MAILVIEWER_CACHE = getattr(settings, 'MAILVIEWER_CACHE', 'default')
MAILVIEWER_DATABASE_BACKEND_MODEL = getattr(settings, 'MAILVIEWER_DATABASE_BACKEND_MODEL', 'mail_viewer_database_backend.EmailMessage')
MAILVIEWER_CACHE = getattr(settings, "MAILVIEWER_CACHE", "default")
MAILVIEWER_DATABASE_BACKEND_MODEL = getattr(
settings, "MAILVIEWER_DATABASE_BACKEND_MODEL", "mail_viewer_database_backend.EmailMessage"
)
Loading

0 comments on commit 7d58e87

Please sign in to comment.