Compare commits

..
Author SHA1 Message Date
Trenton HolmesandClaude Sonnet 5 ff0ce4d123 Fix: register the filter_selection_data e2e stub after routeFromHAR, not before
Per Playwright's own docs, routeFromHAR's notFound: 'fallback' sends
unmatched requests straight to the network -- it does not chain to
other, earlier-registered page.route() handlers via route.fallback()
the way I'd assumed. Since Playwright checks routes in
reverse-registration order, a handler registered in beforeEach (i.e.
before the test body's routeFromHAR call) is checked AFTER routeFromHAR,
whose catch-all pattern intercepts everything first and sends any miss
straight to the (nonexistent, in e2e) network -- my stub never got a
chance to run. Confirmed by a second CI failure with the same "Failed
to fetch" symptom even after the CORS-header fix.

Moved the mockFilterSelectionData(page) call to immediately after each
test's own routeFromHAR(...) call instead, so it's registered later and
checked first for that specific URL, with routeFromHAR's broader
pattern still handling everything else as before.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-20 14:54:58 -07:00
Trenton HolmesandClaude Sonnet 5 8b3665b32d Fix: add CORS header to the e2e filter_selection_data stub response
The e2e app (localhost:4200) calls the backend at localhost:8000, a
cross-origin request from the browser's perspective. Recorded HAR
responses carry the real backend's Access-Control-Allow-Origin header,
which is why they work; the stub added in the previous commit didn't
set one, so the browser rejected the fulfilled response as a CORS
violation -- same symptom as the original bug (TypeError: Failed to
fetch), confirmed via a fresh CI run after the first e2e fix attempt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-20 14:32:59 -07:00
Trenton HolmesandClaude Sonnet 5 9cce6d1b68 Fix: mock the new filter_selection_data request in Playwright e2e tests
CI (https://github.com/paperless-ngx/paperless-ngx/actions/runs/29777331059)
failed every e2e test that reloads the document list, with the browser
throwing "TypeError: Failed to fetch". Root cause: the e2e harness only
starts the Angular dev server (no real backend), and relies entirely on
page.routeFromHAR(..., { notFound: 'fallback' }) to mock API responses,
falling through to the real network for anything not in a recorded HAR.
The new GET /api/documents/filter_selection_data/ request added by this
branch fires on every non-search reload() and isn't in any pre-recorded
HAR fixture, so it fell through to a real network call with nothing
listening on the other end.

Added a shared mockFilterSelectionData() helper that stubs an empty
response for that endpoint, registered via test.beforeEach() before each
affected test's own routeFromHAR() call in every spec file that
exercises document-list reload (document-list, dashboard,
global-permissions, settings, document-detail). Playwright resolves
routes in reverse-registration order, so the later-registered HAR route
checks first, calls route.fallback() on a miss, and defers back to this
earlier-registered stub instead of hitting the network.

Not verified locally (Playwright browser binaries aren't installed in
this environment and the user preferred not to install them); relying
on CI to confirm.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-20 14:21:51 -07:00
Trenton HolmesandClaude Sonnet 5 bb77e65d52 Simplify: dedup HttpParams building, fire selection-data concurrently
Cleanups from a code-simplification pass over the prior commit:

- Extract the repeated "build HttpParams from a plain object, skipping
  null/undefined" loop (previously duplicated in list(), getFew(), and the
  new getFilterSelectionData()) into a single shared
  AbstractPaperlessService.withParams() helper.

- document-list-view.service.ts: fire the filter_selection_data request
  concurrently with the primary list request instead of nesting it inside
  the list request's success callback. They're independent (the follow-up
  only needs filterRules, not the list response), so there's no reason to
  wait on one before starting the other. Extracted into
  loadFilterSelectionData() for readability, and guarded the error path so
  a failed list request cancels the concurrent follow-up too rather than
  letting it resolve afterward and clobber the error-state reset.

- views.py: give filter_selection_data a lean base queryset
  (_base_document_queryset) instead of the full get_queryset(), which
  carries select_related/prefetch_related meant for serializing full
  Document objects. Benchmarked this specifically: no measured query-time
  impact in this path, since those calls were never iterating the
  queryset directly and were already inert here -- keeping it for
  clarity, not as a performance claim.

- Test fallout: since filter_selection_data now fires unconditionally on
  every non-search reload() rather than only after a successful primary
  flush, added a generic drain step to both spec files' afterEach so
  individual tests don't each need to know about this follow-up request
  unless they specifically assert on its response.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-20 10:00:08 -07:00
Trenton HolmesandClaude Sonnet 5 eb5bf53476 Fix (beta): stop blocking the document list on selection-data aggregation
The document overview sent include_selection_data=true on every list
request (page/filter/sort change), computing 5 correlated Count(DISTINCT)
aggregations over the full matching queryset inline before the list could
render. At scale (hundreds of thousands of documents, unfiltered) this is
the confirmed root cause of the document overview "eternally loading" in
paperless-ngx/paperless-ngx#13161.

Split the aggregation into its own endpoint, GET
/api/documents/filter_selection_data/, filter-scoped the same way the
list endpoint already resolves matches (no document ID enumeration
needed). The frontend now fetches it as a separate, non-blocking request
after the list has already rendered, for plain (non-search) browsing.
Full-text search keeps computing it inline since results are already
narrowed by the search backend first.

include_selection_data never shipped in a stable release (introduced
this beta cycle, not present on main), so the plain list endpoint simply
stops acting on it rather than needing a deprecation path.

Also closes a latent localStorage leak between
document-list-view.service.spec.ts tests (filterRules persisted via
localStorage were never cleared, only sessionStorage was) that this
change's new URL-dependent follow-up request exposed, and fixes two
tests that were unknowingly relying on that leaked state to pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-20 10:00:08 -07:00
17 changed files with 331 additions and 1222 deletions
@@ -1,662 +0,0 @@
# Mail Fetch Re-download Fix Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Stop `paperless_mail` from re-downloading full IMAP message bodies for mail it has already handled, and make sure attachment-less mail under attachments-only rules is recorded so it stops matching the search forever.
**Architecture:** In `_handle_mail_rule`, replace the single "fetch every matching message's full body" call with a cheap `UID SEARCH` first, subtract UIDs already in `ProcessedMail`, and fetch bodies only for the remainder (batched). Separately, extract the existing "record `PROCESSED_WO_CONSUMPTION`" block into a shared helper and call it from the no-attachments-under-attachments-only-scope early return in `_handle_message`, which currently skips it entirely.
**Tech Stack:** Django (pytest-django), `imap_tools` (`MailBox.uids()`, `AND` query builder), existing `ProcessedMail` model — no new dependencies, no migrations.
## Global Constraints
- No new required settings and no DB migrations for existing installs (spec: "No new required settings/migrations for existing installs").
- Batch size for body fetches is a hardcoded module constant (`MAIL_FETCH_BATCH_SIZE = 500`), not user-configurable.
- No mailbox action (mark-read/flag/tag/move/delete) is applied for the no-attachment/no-consumption case — only a `ProcessedMail` row is written, matching the existing precedent for other no-consumption outcomes.
- Every path that currently produces a document or applies a mail action must behave exactly as before (no change to `_process_attachments`'/`_process_eml`'s consuming behavior, action application, or the existing per-message `already_processed` dedup check, which stays in place as a safety net).
- Follow the existing per-message `uid_validity` matching semantics exactly: when `self._current_uid_validity is not None`, only match `ProcessedMail` rows with the same `uid_validity` or `uid_validity IS NULL`; when it is `None` (server didn't report UIDVALIDITY), fall back to matching on `(rule, uid, folder)` alone, ignoring `uid_validity`. (This exact conditional already exists at `mail.py:722-726` for the per-message check — the new UID-diff query must replicate it or several existing uidvalidity tests will break.)
---
### Task 1: Add UID search support to the `BogusMailBox` test double
**Files:**
- Modify: `src/paperless_mail/tests/test_mail.py:137-171` (`BogusMailBox.fetch`)
- Test: `src/paperless_mail/tests/test_mail.py` (new test in `class TestMail`)
**Interfaces:**
- Produces: `BogusMailBox.uids(self, criteria, charset="") -> list[str]`, filtering with the same rules as `fetch()`, returning only UIDs (no message bodies). `BogusMailBox.fetch()` gains support for a `UID <comma-list>` criteria token (used by real code's `AND(uid=[...])` queries).
- Consumes: nothing new — this is test infrastructure only, no production code changes in this task.
- [ ] **Step 1: Write the failing test**
Add to `class TestMail` in `src/paperless_mail/tests/test_mail.py` (near the other `BogusMailBox`-adjacent tests, e.g. after `test_handle_message`):
```python
def test_bogus_mailbox_uids_and_uid_criteria(self) -> None:
mailbox = self.mailMocker.bogus_mailbox
all_messages = list(mailbox.messages)
# uids() returns the UIDs of unseen messages, no bodies needed to call it
unseen_uids = mailbox.uids("(UNSEEN)")
self.assertEqual(
set(unseen_uids),
{m.uid for m in all_messages if not m.seen},
)
# fetch() with an explicit UID criteria returns only the matching messages
target_uid = all_messages[0].uid
from imap_tools import AND
fetched = mailbox.fetch(AND(uid=[target_uid]), mark_seen=False)
self.assertEqual([m.uid for m in fetched], [target_uid])
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_bogus_mailbox_uids_and_uid_criteria -v`
Expected: FAIL with `AttributeError: 'BogusMailBox' object has no attribute 'uids'` (and/or the UID-criteria fetch returning all 3 default messages instead of 1, since `fetch()` doesn't understand a `UID` token yet).
- [ ] **Step 3: Implement `uids()` and `UID` criteria support**
Replace `src/paperless_mail/tests/test_mail.py:137-171`:
```python
def fetch(self, criteria, mark_seen, charset="", *, bulk=True):
return self._filter_messages(criteria)
def uids(self, criteria, charset="") -> list[str]:
return [m.uid for m in self._filter_messages(criteria)]
def _filter_messages(self, criteria):
msg = self.messages
criteria = str(criteria).strip("()").split(" ")
if "UNSEEN" in criteria:
msg = filter(lambda m: not m.seen, msg)
if "SUBJECT" in criteria:
subject = criteria[criteria.index("SUBJECT") + 1].strip('"')
msg = filter(lambda m: subject in m.subject, msg)
if "BODY" in criteria:
body = criteria[criteria.index("BODY") + 1].strip('"')
msg = filter(lambda m: body in m.text, msg)
if "FROM" in criteria:
from_ = criteria[criteria.index("FROM") + 1].strip('"')
msg = filter(lambda m: from_ in m.from_, msg)
if "TO" in criteria:
to_ = criteria[criteria.index("TO") + 1].strip('"')
msg = filter(lambda m: any(to_ in to_addr for to_addr in m.to), msg)
if "UNFLAGGED" in criteria:
msg = filter(lambda m: not m.flagged, msg)
if "UNKEYWORD" in criteria:
tag = criteria[criteria.index("UNKEYWORD") + 1].strip("'")
msg = filter(lambda m: tag not in m.flags, msg)
if "(X-GM-LABELS" in criteria: # ['NOT', '(X-GM-LABELS', '"processed"']
msg = filter(lambda m: "processed" not in m.flags, msg)
if "UID" in criteria:
uid_list = criteria[criteria.index("UID") + 1].split(",")
msg = filter(lambda m: m.uid in uid_list, msg)
return list(msg)
```
This is a pure refactor of the existing filtering logic into `_filter_messages`, reused by both `fetch()` (unchanged behavior) and the new `uids()`, plus one new `if "UID" in criteria` branch.
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_bogus_mailbox_uids_and_uid_criteria -v`
Expected: PASS
- [ ] **Step 5: Run the full existing `test_mail.py` suite to confirm the refactor didn't change `fetch()` behavior**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
Expected: all tests PASS (same pass count as before this task)
- [ ] **Step 6: Commit**
```bash
git add src/paperless_mail/tests/test_mail.py
git commit -m "test: add BogusMailBox.uids() and UID criteria support"
```
---
### Task 2: Record `ProcessedMail` for attachment-less mail under attachments-only rules
**Files:**
- Modify: `src/paperless_mail/mail.py:749-793` (`_handle_message`)
- Modify: `src/paperless_mail/mail.py:952-979` (`_process_attachments`, tail)
- Test: `src/paperless_mail/tests/test_mail.py:540-548` (`test_handle_empty_message`, rewritten)
**Interfaces:**
- Produces: `MailAccountHandler._record_processed_without_consumption(self, message: MailMessage, rule: MailRule) -> None` — idempotently writes a `ProcessedMail(status="PROCESSED_WO_CONSUMPTION")` row for `(rule, message.uid, rule.folder)` if one doesn't already exist for the current `self._current_uid_validity`. No mailbox action is applied.
- Consumes: `self._current_uid_validity` (already set by `_handle_mail_rule` before `_handle_message` is called; `None` when called directly, e.g. in unit tests).
- [ ] **Step 1: Write the failing test**
Replace `src/paperless_mail/tests/test_mail.py:540-548` (`test_handle_empty_message`):
```python
def test_handle_empty_message(self) -> None:
message = self.mailMocker.messageBuilder.create_message(
subject="No attachments here",
attachments=[],
)
account = MailAccount.objects.create()
rule = MailRule.objects.create(
account=account,
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
)
result = self.mail_account_handler._handle_message(message, rule)
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
self.assertEqual(result, 0)
processed = ProcessedMail.objects.get(
rule=rule,
uid=message.uid,
folder=rule.folder,
)
self.assertEqual(processed.status, "PROCESSED_WO_CONSUMPTION")
# Calling it again must not create a second row
self.mail_account_handler._handle_message(message, rule)
self.assertEqual(
ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
).count(),
1,
)
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_empty_message -v`
Expected: FAIL — `ProcessedMail.DoesNotExist` (no row is created by the current early return).
- [ ] **Step 3: Add the shared helper and call it from both places**
Insert a new method right after `_handle_message` ends, i.e. after `src/paperless_mail/mail.py:793` (`return processed_elements`), before `def filename_inclusion_matches`:
```python
def _record_processed_without_consumption(
self,
message: MailMessage,
rule: MailRule,
) -> None:
if not ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
uid_validity=self._current_uid_validity,
).exists():
ProcessedMail.objects.create(
rule=rule,
folder=rule.folder,
uid=message.uid,
uid_validity=self._current_uid_validity,
subject=message.subject,
received=make_aware(message.date)
if is_naive(message.date)
else message.date,
status="PROCESSED_WO_CONSUMPTION",
)
```
Then modify `_handle_message`'s early return at `src/paperless_mail/mail.py:756-760`:
```python
# Skip Message handling when only attachments are to be processed but
# message doesn't have any.
if (
not message.attachments
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
):
return processed_elements
```
becomes:
```python
# Skip Message handling when only attachments are to be processed but
# message doesn't have any.
if (
not message.attachments
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
):
self._record_processed_without_consumption(message, rule)
return processed_elements
```
Then replace the tail of `_process_attachments` at `src/paperless_mail/mail.py:952-979`:
```python
if len(consume_tasks) > 0:
queue_consumption_tasks(
consume_tasks=consume_tasks,
rule=rule,
message=message,
uid_validity=self._current_uid_validity,
)
else:
# No files to consume, just mark as processed if it wasn't by .eml processing
if not ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
uid_validity=self._current_uid_validity,
).exists():
ProcessedMail.objects.create(
rule=rule,
folder=rule.folder,
uid=message.uid,
uid_validity=self._current_uid_validity,
subject=message.subject,
received=make_aware(message.date)
if is_naive(message.date)
else message.date,
status="PROCESSED_WO_CONSUMPTION",
)
return processed_attachments
```
with:
```python
if len(consume_tasks) > 0:
queue_consumption_tasks(
consume_tasks=consume_tasks,
rule=rule,
message=message,
uid_validity=self._current_uid_validity,
)
else:
# No files to consume, just mark as processed if it wasn't by .eml processing
self._record_processed_without_consumption(message, rule)
return processed_attachments
```
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_empty_message -v`
Expected: PASS
- [ ] **Step 5: Run the full `test_mail.py` suite (regression check for the `_process_attachments` refactor)**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
Expected: all PASS
- [ ] **Step 6: Commit**
```bash
git add src/paperless_mail/mail.py src/paperless_mail/tests/test_mail.py
git commit -m "fix: record ProcessedMail for attachment-less mail under attachments-only rules"
```
---
### Task 3: Diff UIDs against `ProcessedMail` before fetching message bodies
**Files:**
- Modify: `src/paperless_mail/mail.py:641-693` (`_handle_mail_rule`, the fetch section)
- Test: `src/paperless_mail/tests/test_mail.py` (new test in `class TestMail`)
**Interfaces:**
- Consumes: `MailAccountHandler._record_processed_without_consumption` (Task 2), `ProcessedMail` model, `imap_tools.AND`, `M.uids()` (Task 1's `BogusMailBox.uids()` in tests; `imap_tools.MailBox.uids()` in production).
- Produces: no new public interface — `_handle_mail_rule`'s external behavior (return value, exceptions raised) is unchanged; only its internal fetch strategy changes. `_handle_mail_rule` now returns `0` immediately, without calling `M.fetch()` at all, when every UID matching the search criteria already has a `ProcessedMail` row.
- [ ] **Step 1: Write the failing test**
Add to `class TestMail` in `src/paperless_mail/tests/test_mail.py`:
```python
def test_handle_mail_account_skips_body_fetch_for_already_processed_mail(
self,
) -> None:
"""
GIVEN:
- An attachment-less mail under an attachments-only mark-read rule,
already recorded as PROCESSED_WO_CONSUMPTION
WHEN:
- The mail account is processed again and the mail still matches the
search criteria (it was never marked read, since no mail action is
applied for the no-consumption case)
THEN:
- No IMAP body fetch happens for that mail; only the cheap UID search runs.
"""
account = MailAccount.objects.create(
name="test",
imap_server="",
username="admin",
password="secret",
)
rule = MailRule.objects.create(
name="testrule",
account=account,
action=MailRule.MailAction.MARK_READ,
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
)
message = self.mailMocker.messageBuilder.create_message(
subject="No attachment",
attachments=[],
)
self.mailMocker.bogus_mailbox.messages = [message]
self.mailMocker.bogus_mailbox.updateClient()
# First run: records ProcessedMail without consuming anything.
self.mail_account_handler.handle_mail_account(account)
self.assertTrue(
ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
).exists(),
)
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
# Second run: message still matches UNSEEN (mark-read action never ran),
# but its body must not be downloaded again.
with mock.patch.object(
self.mailMocker.bogus_mailbox,
"fetch",
wraps=self.mailMocker.bogus_mailbox.fetch,
) as fetch_spy:
self.mail_account_handler.handle_mail_account(account)
fetch_spy.assert_not_called()
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_skips_body_fetch_for_already_processed_mail -v`
Expected: FAIL — `fetch_spy.assert_not_called()` fails because the current code always calls `M.fetch()` for every message matching the search criteria, regardless of `ProcessedMail`.
- [ ] **Step 3: Implement the UID diff**
Replace `src/paperless_mail/mail.py:683-693`:
```python
try:
messages = M.fetch(
criteria=criterias,
mark_seen=False,
charset=rule.account.character_set,
bulk=True,
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while fetching folder {rule.folder}",
) from err
```
with:
```python
try:
all_uids = set(
M.uids(criteria=criterias, charset=rule.account.character_set),
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while searching folder {rule.folder}",
) from err
processed_uids_qs = ProcessedMail.objects.filter(
rule=rule,
folder=rule.folder,
uid__in=all_uids,
)
if self._current_uid_validity is not None:
processed_uids_qs = processed_uids_qs.filter(
Q(uid_validity=self._current_uid_validity)
| Q(uid_validity__isnull=True),
)
processed_uids = set(processed_uids_qs.values_list("uid", flat=True))
new_uids = all_uids - processed_uids
if not new_uids:
self.log.debug(
f"Rule {rule}: No new mail matching criteria {criterias}",
)
return 0
try:
messages = M.fetch(
criteria=AND(uid=list(new_uids)),
mark_seen=False,
charset=rule.account.character_set,
bulk=True,
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while fetching folder {rule.folder}",
) from err
```
Note the `uid_validity` handling here deliberately mirrors the existing per-message check at `mail.py:722-726` exactly (see Global Constraints) — the extra `Q(...)` filter is applied only when `self._current_uid_validity is not None`.
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_skips_body_fetch_for_already_processed_mail -v`
Expected: PASS
- [ ] **Step 5: Run the full `test_mail.py` suite (regression check, especially the uidvalidity tests)**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
Expected: all PASS, including:
- `test_handle_mail_account_skip_duplicate_uids_from_fetch`
- `test_handle_mail_account_skips_mail_already_processed_in_same_uidvalidity`
- `test_handle_mail_account_processes_mail_after_uidvalidity_change`
- `test_handle_mail_account_skips_mail_processed_before_uidvalidity_tracking`
- `test_handle_mail_account_processes_mail_when_uidvalidity_unavailable`
- `test_handle_mail_account_skips_mail_when_uidvalidity_unavailable_but_prior_record_exists`
- `test_handle_mail_account_overlapping_rules_only_first_consumes`
If any of these fail, the `uid_validity` branching in Step 3 does not match `mail.py:722-726` closely enough — re-check against the Global Constraints note before changing test expectations.
- [ ] **Step 6: Commit**
```bash
git add src/paperless_mail/mail.py src/paperless_mail/tests/test_mail.py
git commit -m "fix: diff UIDs against ProcessedMail before fetching message bodies"
```
---
### Task 4: Batch the body fetch for large backlogs
**Files:**
- Modify: `src/paperless_mail/mail.py:74-76` (module-level constant)
- Modify: `src/paperless_mail/mail.py` (the `M.fetch(...)` block added in Task 3)
- Test: `src/paperless_mail/tests/test_mail.py` (new test in `class TestMail`)
**Interfaces:**
- Produces: module-level constant `paperless_mail.mail.MAIL_FETCH_BATCH_SIZE: int = 500`.
- Consumes: `itertools` (already imported at `mail.py:2`), `sorted_new_uids` derived from Task 3's `new_uids` set.
- [ ] **Step 1: Write the failing test**
Add to `class TestMail` in `src/paperless_mail/tests/test_mail.py`. Add `from paperless_mail.mail import MAIL_FETCH_BATCH_SIZE` to the imports at the top of the file (alongside the other `from paperless_mail.mail import ...` lines at `test_mail.py:35-38`).
```python
@mock.patch("paperless_mail.mail.MAIL_FETCH_BATCH_SIZE", 5)
def test_handle_mail_account_batches_body_fetch_for_large_backlog(self) -> None:
"""
GIVEN:
- More new/unprocessed mail than MAIL_FETCH_BATCH_SIZE
WHEN:
- The mail account is processed
THEN:
- The body fetch is issued in multiple batches
- Every message is still processed (none dropped at a batch boundary)
"""
account = MailAccount.objects.create(
name="test",
imap_server="",
username="admin",
password="secret",
)
rule = MailRule.objects.create(
name="testrule",
account=account,
action=MailRule.MailAction.MARK_READ,
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
)
message_count = 12 # more than the patched batch size of 5
self.mailMocker.bogus_mailbox.messages = [
self.mailMocker.messageBuilder.create_message(
subject=f"No attachment {i}",
attachments=[],
)
for i in range(message_count)
]
self.mailMocker.bogus_mailbox.updateClient()
with mock.patch.object(
self.mailMocker.bogus_mailbox,
"fetch",
wraps=self.mailMocker.bogus_mailbox.fetch,
) as fetch_spy:
self.mail_account_handler.handle_mail_account(account)
# ceil(12 / 5) == 3 batches
self.assertEqual(fetch_spy.call_count, 3)
self.assertEqual(
ProcessedMail.objects.filter(rule=rule).count(),
message_count,
)
```
- [ ] **Step 2: Run test to verify it fails**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_batches_body_fetch_for_large_backlog -v`
Expected: FAIL — either an `ImportError` for `MAIL_FETCH_BATCH_SIZE` (doesn't exist yet) or, once that import is stubbed out, `fetch_spy.call_count == 1` instead of `3` (Task 3's implementation fetches all new UIDs in one call).
- [ ] **Step 3: Add the constant and batch the fetch**
Insert after `src/paperless_mail/mail.py:74` (right after the `APPLE_MAIL_TAG_COLORS` dict closes, before `class MailError`):
```python
MAIL_FETCH_BATCH_SIZE = 500
```
Replace the `M.fetch(...)` block added in Task 3 (Task 3 Step 3's final `try/except`):
```python
try:
messages = M.fetch(
criteria=AND(uid=list(new_uids)),
mark_seen=False,
charset=rule.account.character_set,
bulk=True,
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while fetching folder {rule.folder}",
) from err
```
with:
```python
sorted_new_uids = sorted(new_uids, key=int)
message_batches = []
for batch_start in range(0, len(sorted_new_uids), MAIL_FETCH_BATCH_SIZE):
batch = sorted_new_uids[batch_start : batch_start + MAIL_FETCH_BATCH_SIZE]
try:
message_batches.append(
M.fetch(
criteria=AND(uid=batch),
mark_seen=False,
charset=rule.account.character_set,
bulk=True,
),
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while fetching folder {rule.folder}",
) from err
messages = itertools.chain(*message_batches)
```
- [ ] **Step 4: Run test to verify it passes**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py::TestMail::test_handle_mail_account_batches_body_fetch_for_large_backlog -v`
Expected: PASS
- [ ] **Step 5: Run the full `test_mail.py` suite**
Run: `uv run pytest --override-ini="addopts=" src/paperless_mail/tests/test_mail.py -v`
Expected: all PASS
- [ ] **Step 6: Commit**
```bash
git add src/paperless_mail/mail.py src/paperless_mail/tests/test_mail.py
git commit -m "perf: batch body fetches when many new UIDs are pending"
```
---
### Task 5: Full regression pass and lint
**Files:** none (verification only, plus any fixups this step surfaces)
- [ ] **Step 1: Run the full backend test suite with coverage/parallelism as configured**
Run: `uv run pytest src/paperless_mail/`
Expected: all `paperless_mail` tests PASS
- [ ] **Step 2: Run the full project test suite**
Run: `uv run pytest`
Expected: all tests PASS (no regressions outside `paperless_mail`)
- [ ] **Step 3: Lint and format**
Run: `uv run ruff check src/paperless_mail/` and `uv run ruff format --check src/paperless_mail/`
Expected: no errors. If `ruff format --check` reports files needing formatting, run `uv run ruff format src/paperless_mail/` and re-check.
- [ ] **Step 4: Type check against the frozen baselines**
Run: `uv run mypy src/paperless_mail/mail.py` (or the project's standard mypy invocation) and confirm no new violations beyond `.mypy-baseline.txt`.
Expected: no new errors introduced by this change.
- [ ] **Step 5: Commit any fixups**
If Steps 1-4 required changes:
```bash
git add -u
git commit -m "chore: fix lint/type fallout from mail fetch fix"
```
If no changes were needed, skip this step — nothing to commit.
@@ -1,216 +0,0 @@
# Mail fetch fix: avoid re-downloading already-handled IMAP mail
## Problem
`paperless_mail`'s `_handle_mail_rule` (`src/paperless_mail/mail.py:641`) fetches
the full RFC822 body of every message an IMAP `SEARCH` matches (via
`M.fetch(criteria=criterias, ..., bulk=True)`), then does de-duplication in
Python afterward. Two things compound into a real bug (reported upstream in
[paperless-ngx#13175](https://github.com/paperless-ngx/paperless-ngx/issues/13175)):
1. **Full-body fetch happens before dedup.** The de-dup check against
`ProcessedMail` (`mail.py:717-731`) happens per-message, after the body has
already been downloaded for every matching message. Steady-state cost is
_O(everything the search matches)_, not _O(new since last run)_.
2. **Attachment-less mail under an attachments-only rule is never recorded.**
`_handle_message` (`mail.py:749`) returns early, before any attachment
processing, when a message has no attachments and
`rule.consumption_scope == ATTACHMENTS_ONLY`:
```python
if (
not message.attachments
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
):
return processed_elements # 0, no ProcessedMail row, no mail action applied
```
No `ProcessedMail` row is written and no mail action (mark-read/flag/tag)
is applied, since both only happen via `queue_consumption_tasks`, which is
only reached from the attachment/eml processing paths. The message keeps
matching the search (e.g. `UNSEEN`) forever.
Combined: an inbox where most mail has no attachments gets fully re-downloaded
on every scheduled run (default every 10 minutes), indefinitely, for large
mailboxes -- confirmed against the current codebase, not user
misconfiguration.
## Goals
- Stop re-downloading full message bodies for mail that has already been
handled (processed into a document, or determined to produce nothing).
- Record `ProcessedMail` for the no-attachment / attachments-only early-return
case, so it participates in dedup like every other terminal outcome.
- No new required settings or DB migrations for existing installs.
- Graceful behavior on first run against a large existing mailbox (no single
giant IMAP command, no giant in-memory batch).
- Preserve existing action/consumption semantics exactly for every path that
currently produces a document or applies a mail action.
## Non-goals
- Changing what mail actions (mark-read/flag/tag/move/delete) get applied, or
when.
- Applying a mail action to the no-attachment early-return case (explicitly
out of scope -- see Decisions).
- Making batch size user-configurable.
- Touching EML_ONLY / EVERYTHING consumption scopes' semantics (they already
don't hit the early-return branch described above).
## Decisions
These were settled during brainstorming and are load-bearing for the design
below:
- **Fix both halves together.** Recording `ProcessedMail` alone does not fix
the bandwidth problem: without a mail action applied, an attachment-less
message stays unseen and keeps matching the search criteria, so its body
gets re-downloaded every run regardless of whether it gets reprocessed.
Fixing only the bandwidth side without recording `ProcessedMail` would mean
the UID-diff step never has anything to exclude for these messages. They
must ship together.
- **No mailbox action for the no-attachment case.** Only a `ProcessedMail`
row is written; the message is not marked read/flagged/moved/deleted. This
matches the existing precedent for other no-consumption outcomes
(`mail.py:960-977`, e.g. an attachment present but filtered out or an
unsupported mime type) and avoids changing the user's mailbox state for
mail paperless previously never touched.
- **UID-diff-before-fetch, not a BODYSTRUCTURE probe or SEARCH-side
exclusion.** Considered and rejected:
- BODYSTRUCTURE probing (fetch structure only, decide whether to fetch full
body) only helps the attachments-only scope, not the general
already-processed case, and `imap_tools` doesn't cleanly expose a
structure-only fetch.
- Excluding already-processed UIDs directly in the IMAP `SEARCH` criteria
(`NOT UID (...)`) was rejected because for tens of thousands of
already-processed UIDs the excluded-UID list itself blows up the command
size -- worse than the two-step approach.
- **Batch size is a hardcoded constant**, not a new setting, per the
"no new required settings" goal.
## Design
### `_handle_mail_rule` (`mail.py:641`)
Replace the single `M.fetch(criteria=criterias, ...)` call with:
1. `self._current_uid_validity` computed as today (unchanged, already first).
2. `criterias = make_criterias(...)` (unchanged).
3. `all_uids = set(M.uids(criteria=criterias, charset=rule.account.character_set))`
-- a `UID SEARCH`, no bodies.
4. Query already-processed UIDs in one DB round trip, reusing the same
uid_validity matching already used per-message:
```python
processed_uids = set(
ProcessedMail.objects.filter(
rule=rule, folder=rule.folder, uid__in=all_uids,
).filter(
Q(uid_validity=self._current_uid_validity) | Q(uid_validity__isnull=True),
).values_list("uid", flat=True)
)
```
5. `new_uids = all_uids - processed_uids`. If empty: log at debug level and
`return 0` -- no body fetch at all. This is the steady-state case.
6. Otherwise, iterate `new_uids` in fixed-size batches of
`MAIL_FETCH_BATCH_SIZE = 500` (module-level constant), calling
`M.fetch(criteria=AND(uid=batch), mark_seen=False, charset=rule.account.character_set, bulk=True)`
per batch. Chain the resulting message iterators into the existing
per-message loop (`mail.py:699+`) unchanged.
7. The existing per-message `already_processed` DB check
(`mail.py:717-731`) stays in place unchanged, as a safety net against
races (e.g. concurrent rule runs against the same folder in this pass) and
to keep behavior identical if steps 3/4 ever disagree with it.
### New helper: `_record_processed_without_consumption`
Extract the existing dedup-and-create block at `mail.py:960-977` into a
method on the same class:
```python
def _record_processed_without_consumption(self, message, rule) -> None:
if not ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
uid_validity=self._current_uid_validity,
).exists():
ProcessedMail.objects.create(
rule=rule,
folder=rule.folder,
uid=message.uid,
uid_validity=self._current_uid_validity,
subject=message.subject,
received=make_aware(message.date) if is_naive(message.date) else message.date,
status="PROCESSED_WO_CONSUMPTION",
)
```
Call sites:
- `_process_attachments`'s existing no-consumables branch (replaces the
inline block, same behavior).
- `_handle_message`'s early return (`mail.py:756-760`), newly, for the
no-attachments-under-attachments-only-scope case.
### Data flow (steady state, nothing new)
search criteria (unchanged) -> `UID SEARCH` for matching UIDs (cheap) ->
subtract UIDs already in `ProcessedMail` (cheap, DB-only) -> zero new UIDs ->
no fetch, no body download.
### Data flow (new mail present)
... same as above -> some UIDs remain -> fetch bodies only for those,
batched -> existing per-message processing, action application, and
`ProcessedMail` recording, unchanged except the no-attachment early return
now also calls `_record_processed_without_consumption`.
## Error handling
- `M.uids(...)` raising is wrapped in the same `try/except` that currently
wraps `M.fetch(...)` (`mail.py:683-693`), surfacing as `MailError`
identically to today's search-failure behavior.
- A batch's `M.fetch(...)` raising mid-loop is caught by the same
`try/except`, applied per-batch. One failing batch fails the whole rule
run for this pass -- matching today's all-or-nothing semantics (currently
a single failed fetch already fails the whole rule).
- Empty `new_uids` short-circuits before any fetch, so it can't hit fetch
error paths at all for the common case.
- `_record_processed_without_consumption` reuses the existing
`ProcessedMail.objects.create` call already in production use; no new
failure mode introduced.
## Testing
In `src/paperless_mail/tests/test_mail.py`:
- Extend the `BogusMailBox` test double with a `uids()` method that mirrors
its existing `fetch()` criteria matching but returns only UIDs.
- New test: an attachment-less message under an attachments-only mark-read
rule is recorded as `PROCESSED_WO_CONSUMPTION` on the first run. On a
second run, with the same message still unseen server-side, assert no
body fetch happens (only `M.uids()` is called) -- i.e. steady state costs
one search and zero body downloads.
- New test: a folder with more new UIDs than `MAIL_FETCH_BATCH_SIZE` results
in multiple batched `fetch` calls, and all messages are still processed
(no messages dropped at a batch boundary).
- Update existing `test_handle_empty_message` to assert a
`PROCESSED_WO_CONSUMPTION` row now exists for the no-attachment case.
- Regression: existing tests for `EML_ONLY`/`EVERYTHING` scopes and the
multi-rule-same-folder dedup (`consumed_messages` set) should pass
unchanged, since neither the per-message `already_processed` check nor the
`consumed_messages` set logic is touched by this design.
## Open questions / risks
- `imap_tools`'s `AND(uid=batch)` criteria builder needs to be confirmed to
produce a valid `UID FETCH <list>` command at the batch sizes used here;
covered by the batching test above.
- If a mail server doesn't support `UID SEARCH` the same way it supports the
existing `fetch`'s implicit search, `M.uids()` could behave differently
from today's `M.fetch()` on some edge-case server. Existing project test
coverage (via `BogusMailBox`) won't catch server-specific quirks; this is
the same class of risk as any IMAP-behavior assumption already baked into
this module.
+4
View File
@@ -1,5 +1,6 @@
import { expect, test } from '@playwright/test'
import path from 'node:path'
import { mockFilterSelectionData } from '../mock-filter-selection-data'
const REQUESTS_HAR = path.join(__dirname, 'requests/api-settings.har')
@@ -7,6 +8,7 @@ test('should activate / deactivate save button when settings change', async ({
page,
}) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/settings')
await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled()
await page.getByLabel('Use system setting').click()
@@ -16,6 +18,7 @@ test('should activate / deactivate save button when settings change', async ({
test('should warn on unsaved changes', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/settings')
await page.getByLabel('Use system setting').click()
await page.getByRole('link', { name: 'Dashboard' }).click()
@@ -28,6 +31,7 @@ test('should warn on unsaved changes', async ({ page }) => {
test('should apply appearance changes when set', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/settings')
await expect(page.locator('html')).toHaveAttribute('data-bs-theme', /auto/)
await page.getByLabel('Use system setting').click()
+6
View File
@@ -1,5 +1,6 @@
import { expect, test } from '@playwright/test'
import path from 'node:path'
import { mockFilterSelectionData } from '../mock-filter-selection-data'
const REQUESTS_HAR1 = path.join(__dirname, 'requests/api-dashboard1.har')
const REQUESTS_HAR2 = path.join(__dirname, 'requests/api-dashboard2.har')
@@ -8,6 +9,7 @@ const REQUESTS_HAR4 = path.join(__dirname, 'requests/api-dashboard4.har')
test('dashboard inbox link', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR1, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await page.getByRole('link', { name: 'Documents in inbox' }).click()
await expect(page).toHaveURL(/tags__id__in=9/)
@@ -16,6 +18,7 @@ test('dashboard inbox link', async ({ page }) => {
test('dashboard total documents link', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR2, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await page.getByRole('link').filter({ hasText: 'Total documents' }).click()
await expect(page).toHaveURL(/documents/)
@@ -25,6 +28,7 @@ test('dashboard total documents link', async ({ page }) => {
test('dashboard saved view show all', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR3, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await page
.locator('pngx-widget-frame')
@@ -38,6 +42,7 @@ test('dashboard saved view show all', async ({ page }) => {
test('dashboard saved view document links', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR4, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await page
.locator('pngx-widget-frame')
@@ -51,6 +56,7 @@ test('dashboard saved view document links', async ({ page }) => {
test('test slim sidebar', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR1, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await page.locator('.sidebar-slim-toggler').click()
await expect(
@@ -1,5 +1,6 @@
import { expect, test } from '@playwright/test'
import path from 'node:path'
import { mockFilterSelectionData } from '../mock-filter-selection-data'
const REQUESTS_HAR = path.join(__dirname, 'requests/api-document-detail.har')
const REQUESTS_HAR2 = path.join(__dirname, 'requests/api-document-detail2.har')
@@ -8,6 +9,7 @@ test('should activate / deactivate save button when changes are saved', async ({
page,
}) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents/175/')
await page.waitForSelector('pngx-document-detail pngx-input-text:first-child')
await expect(page.getByTitle('Storage path', { exact: true })).toHaveText(
@@ -20,6 +22,7 @@ test('should activate / deactivate save button when changes are saved', async ({
test('should warn on unsaved changes', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents/175/')
await expect(page.getByTitle('Correspondent', { exact: true })).toHaveText(
/\w+/
@@ -39,6 +42,7 @@ test('should warn on unsaved changes', async ({ page }) => {
test('should support tab direct navigation', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents/175/details')
await expect(page.getByRole('tab', { name: 'Details' })).toHaveAttribute(
'aria-selected',
@@ -68,6 +72,7 @@ test('should support tab direct navigation', async ({ page }) => {
test('should show a mobile preview', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents/175/')
await page.setViewportSize({ width: 400, height: 1000 })
await expect(page.getByRole('tab', { name: 'Preview' })).toBeVisible()
@@ -77,6 +82,7 @@ test('should show a mobile preview', async ({ page }) => {
test('should show a list of notes', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents/175/notes')
await expect(page.locator('pngx-document-notes')).toBeVisible()
await expect(
@@ -89,6 +95,7 @@ test('should show a list of notes', async ({ page }) => {
test('should support quick filters', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR2, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents/175/details')
await page
.getByRole('button', { name: 'Filter documents with these Tags' })
@@ -1,5 +1,6 @@
import { expect, test } from '@playwright/test'
import path from 'node:path'
import { mockFilterSelectionData } from '../mock-filter-selection-data'
const REQUESTS_HAR1 = path.join(__dirname, 'requests/api-document-list1.har')
const REQUESTS_HAR2 = path.join(__dirname, 'requests/api-document-list2.har')
@@ -10,6 +11,7 @@ const REQUESTS_HAR6 = path.join(__dirname, 'requests/api-document-list6.har')
test('basic filtering', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR1, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents')
await page.getByRole('button', { name: 'Tags' }).click()
await page.getByRole('menuitem', { name: 'Inbox' }).click()
@@ -45,6 +47,7 @@ test('basic filtering', async ({ page }) => {
test('text filtering', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR2, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents')
await page.getByRole('main').getByRole('combobox').click()
await page.getByRole('main').getByRole('combobox').fill('test')
@@ -81,6 +84,7 @@ test('text filtering', async ({ page }) => {
test('date filtering', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR3, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents')
await page.getByRole('button', { name: 'Dates' }).click()
await page.locator('.ng-arrow-wrapper').first().click()
@@ -103,6 +107,7 @@ test('date filtering', async ({ page }) => {
test('sorting', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR4, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents')
await page.getByRole('button', { name: 'Sort' }).click()
await page.getByRole('button', { name: 'ASN' }).click()
@@ -141,6 +146,7 @@ test('sorting', async ({ page }) => {
test('change views', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR5, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents')
await page.locator('.btn-group > label').first().click()
await expect(page.locator('pngx-document-list table')).toBeVisible()
@@ -152,6 +158,7 @@ test('change views', async ({ page }) => {
test('bulk edit', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR6, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/documents')
await page.locator('pngx-document-card-small').nth(0).click()
+35
View File
@@ -0,0 +1,35 @@
import { Page } from '@playwright/test'
const EMPTY_SELECTION_DATA = {
selected_correspondents: [],
selected_tags: [],
selected_document_types: [],
selected_storage_paths: [],
selected_custom_fields: [],
}
/**
* The document list now fires a GET to filter_selection_data on every
* non-search reload(), independent of and concurrent with the main list
* request. It's not present in any of the recorded HAR fixtures, so with
* `notFound: 'fallback'` it would otherwise fall through to the real
* network (nothing listens there in e2e, since only the frontend dev
* server is started) and fail every test that reloads the list.
*
* Playwright checks routes in reverse-registration order, so this must be
* registered before a test's own page.routeFromHAR() call for the HAR
* route's `notFound: 'fallback'` to defer back to this one.
*/
export async function mockFilterSelectionData(page: Page) {
await page.route('**/api/documents/filter_selection_data/**', (route) =>
route.fulfill({
json: EMPTY_SELECTION_DATA,
// The app calls the (cross-origin, from the e2e app's perspective)
// backend at http://localhost:8000 while served from :4200, so a
// fulfilled response needs the same CORS header the real backend
// sends (and that recorded HAR responses already carry) or the
// browser rejects it as a cross-origin failure.
headers: { 'Access-Control-Allow-Origin': 'http://localhost:4200' },
})
)
}
@@ -1,10 +1,12 @@
import { expect, test } from '@playwright/test'
import path from 'node:path'
import { mockFilterSelectionData } from '../mock-filter-selection-data'
const REQUESTS_HAR = path.join(__dirname, 'requests/api-global-permissions.har')
test('should not allow user to edit settings', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(page.getByRole('link', { name: 'Settings' })).not.toBeAttached()
await page.goto('/settings')
@@ -15,6 +17,7 @@ test('should not allow user to edit settings', async ({ page }) => {
test('should not allow user to view documents', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(
page.locator('nav').getByRole('link', { name: 'Documents' })
@@ -31,6 +34,7 @@ test('should not allow user to view documents', async ({ page }) => {
test('should not allow user to view correspondents', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(
page.getByRole('link', { name: 'Attributes' })
@@ -43,6 +47,7 @@ test('should not allow user to view correspondents', async ({ page }) => {
test('should not allow user to view tags', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(
page.getByRole('link', { name: 'Attributes' })
@@ -55,6 +60,7 @@ test('should not allow user to view tags', async ({ page }) => {
test('should not allow user to view document types', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(
page.getByRole('link', { name: 'Attributes' })
@@ -67,6 +73,7 @@ test('should not allow user to view document types', async ({ page }) => {
test('should not allow user to view storage paths', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(
page.getByRole('link', { name: 'Attributes' })
@@ -79,6 +86,7 @@ test('should not allow user to view storage paths', async ({ page }) => {
test('should not allow user to view logs', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(page.getByRole('link', { name: 'Logs' })).not.toBeAttached()
await page.goto('/logs')
@@ -89,6 +97,7 @@ test('should not allow user to view logs', async ({ page }) => {
test('should not allow user to view tasks', async ({ page }) => {
await page.routeFromHAR(REQUESTS_HAR, { notFound: 'fallback' })
await mockFilterSelectionData(page)
await page.goto('/dashboard')
await expect(page.getByRole('link', { name: 'Tasks' })).not.toBeAttached()
await page.goto('/tasks')
@@ -191,6 +191,14 @@ describe('BulkEditorComponent', () => {
})
afterEach(async () => {
// A filter_selection_data request now fires concurrently with every
// non-search reload(), independent of whether a given test flushes or
// even inspects the primary list response. Drain any left unclaimed.
httpTestingController.match(
(request) =>
request.url ===
`${environment.apiBaseUrl}documents/filter_selection_data/`
)
httpTestingController.verify()
})
@@ -386,7 +394,7 @@ describe('BulkEditorComponent', () => {
parameters: { add_tags: [101], remove_tags: [] },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -432,7 +440,7 @@ describe('BulkEditorComponent', () => {
parameters: { add_tags: [101], remove_tags: [] },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
})
@@ -461,7 +469,7 @@ describe('BulkEditorComponent', () => {
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
.flush(true)
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -552,7 +560,7 @@ describe('BulkEditorComponent', () => {
parameters: { correspondent: 101 },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -584,7 +592,7 @@ describe('BulkEditorComponent', () => {
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
.flush(true)
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -650,7 +658,7 @@ describe('BulkEditorComponent', () => {
parameters: { document_type: 101 },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -682,7 +690,7 @@ describe('BulkEditorComponent', () => {
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
.flush(true)
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -748,7 +756,7 @@ describe('BulkEditorComponent', () => {
parameters: { storage_path: 101 },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -780,7 +788,7 @@ describe('BulkEditorComponent', () => {
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
.flush(true)
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -846,7 +854,7 @@ describe('BulkEditorComponent', () => {
parameters: { add_custom_fields: [101], remove_custom_fields: [102] },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -878,7 +886,7 @@ describe('BulkEditorComponent', () => {
.expectOne(`${environment.apiBaseUrl}documents/bulk_edit/`)
.flush(true)
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -987,7 +995,7 @@ describe('BulkEditorComponent', () => {
documents: [3, 4],
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1080,7 +1088,7 @@ describe('BulkEditorComponent', () => {
documents: [3, 4],
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1115,7 +1123,7 @@ describe('BulkEditorComponent', () => {
source_mode: 'latest_version',
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1156,7 +1164,7 @@ describe('BulkEditorComponent', () => {
metadata_document_id: 3,
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1175,7 +1183,7 @@ describe('BulkEditorComponent', () => {
delete_originals: true,
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1196,7 +1204,7 @@ describe('BulkEditorComponent', () => {
archive_fallback: true,
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1299,7 +1307,7 @@ describe('BulkEditorComponent', () => {
},
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -1607,7 +1615,7 @@ describe('BulkEditorComponent', () => {
expect(toastServiceShowInfoSpy).toHaveBeenCalled()
expect(listReloadSpy).toHaveBeenCalled()
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
@@ -84,6 +84,28 @@ const view: SavedView = {
filter_rules: filterRules,
}
const emptySelectionData = {
selected_correspondents: [],
selected_tags: [],
selected_document_types: [],
selected_storage_paths: [],
selected_custom_fields: [],
}
// A successful (non-search) list response now triggers a separate,
// non-blocking request for filter dropdown counts. Tests that flush a
// successful list response need to also flush this follow-up request.
function flushSelectionDataRequest(
httpTestingController: HttpTestingController,
querySuffix: string = ''
) {
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/filter_selection_data/${querySuffix}`
)
expect(req.request.method).toEqual('GET')
req.flush(emptySelectionData)
}
describe('DocumentListViewService', () => {
let httpTestingController: HttpTestingController
let documentListViewService: DocumentListViewService
@@ -105,6 +127,7 @@ describe('DocumentListViewService', () => {
})
sessionStorage.clear()
localStorage.clear()
httpTestingController = TestBed.inject(HttpTestingController)
documentListViewService = TestBed.inject(DocumentListViewService)
settingsService = TestBed.inject(SettingsService)
@@ -114,8 +137,19 @@ describe('DocumentListViewService', () => {
afterEach(() => {
documentListViewService.cancelPending()
// A filter_selection_data request now fires concurrently with every
// non-search reload(), independent of whether the test cares about or
// flushes the primary list response. Drain any that a test didn't
// explicitly claim via flushSelectionDataRequest, so unrelated tests
// don't have to know about this follow-up request to pass verify().
httpTestingController.match(
(request) =>
request.url ===
`${environment.apiBaseUrl}documents/filter_selection_data/`
)
httpTestingController.verify()
sessionStorage.clear()
localStorage.clear()
})
afterAll(() => {
@@ -128,10 +162,11 @@ describe('DocumentListViewService', () => {
expect(documentListViewService.currentPage).toEqual(1)
documentListViewService.reload()
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
expect(req.request.method).toEqual('GET')
expect(documentListViewService.isReloading).toBeFalsy()
expect(documentListViewService.activeSavedViewId).toBeNull()
@@ -143,12 +178,12 @@ describe('DocumentListViewService', () => {
it('should handle error on page request out of range', () => {
documentListViewService.currentPage = 50
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=50&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=50&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush([], { status: 404, statusText: 'Unexpected error' })
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
expect(documentListViewService.currentPage).toEqual(1)
@@ -165,21 +200,20 @@ describe('DocumentListViewService', () => {
]
documentListViewService.setFilterRules(filterRulesAny)
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__in=${tags__id__in}`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__in=${tags__id__in}`
)
expect(req.request.method).toEqual('GET')
req.flush(
{ archive_serial_number: 'hello' },
{ status: 404, statusText: 'Unexpected error' }
)
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
)
expect(req.request.method).toEqual('GET')
// the error is a plain field error (not a page-out-of-range or deleted
// custom-field-sort case), so no automatic retry request is sent here
expect(documentListViewService.error).toBeTruthy()
// reset the list
documentListViewService.setFilterRules([])
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
})
@@ -187,7 +221,7 @@ describe('DocumentListViewService', () => {
documentListViewService.currentPage = 1
documentListViewService.sortField = 'custom_field_999'
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-custom_field_999&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-custom_field_999&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush(
@@ -196,7 +230,7 @@ describe('DocumentListViewService', () => {
)
// resets itself
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
})
@@ -211,7 +245,7 @@ describe('DocumentListViewService', () => {
]
documentListViewService.setFilterRules(filterRulesAny)
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__in=${tags__id__in}`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__in=${tags__id__in}`
)
expect(req.request.method).toEqual('GET')
req.flush('Generic error', { status: 404, statusText: 'Unexpected error' })
@@ -219,7 +253,7 @@ describe('DocumentListViewService', () => {
// reset the list
documentListViewService.setFilterRules([])
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
})
@@ -228,7 +262,7 @@ describe('DocumentListViewService', () => {
expect(documentListViewService.sortReverse).toBeTruthy()
documentListViewService.setSort('added', false)
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=added&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=added&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
expect(documentListViewService.sortField).toEqual('added')
@@ -236,12 +270,12 @@ describe('DocumentListViewService', () => {
documentListViewService.sortField = 'created'
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=created&truncate_content=true&include_selection_data=false`
)
expect(documentListViewService.sortField).toEqual('created')
documentListViewService.sortReverse = true
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
expect(documentListViewService.sortReverse).toBeTruthy()
@@ -284,7 +318,7 @@ describe('DocumentListViewService', () => {
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=${page}&page_size=${
documentListViewService.pageSize
}&ordering=${reverse ? '-' : ''}${sort}&truncate_content=true&include_selection_data=true`
}&ordering=${reverse ? '-' : ''}${sort}&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
expect(documentListViewService.currentPage).toEqual(page)
@@ -301,7 +335,7 @@ describe('DocumentListViewService', () => {
}
documentListViewService.loadFromQueryParams(convertToParamMap(params))
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
)
expect(req.request.method).toEqual('GET')
expect(documentListViewService.filterRules).toEqual([
@@ -311,12 +345,16 @@ describe('DocumentListViewService', () => {
},
])
req.flush(full_results)
flushSelectionDataRequest(
httpTestingController,
`?tags__id__all=${tags__id__all}`
)
})
it('should use filter rules to update query params', () => {
documentListViewService.setFilterRules(filterRules)
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
)
expect(req.request.method).toEqual('GET')
})
@@ -325,26 +363,31 @@ describe('DocumentListViewService', () => {
documentListViewService.currentPage = 2
let req = httpTestingController.expectOne((request) =>
request.urlWithParams.startsWith(
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
)
expect(req.request.method).toEqual('GET')
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.setFilterRules(filterRules, true)
const filteredReqs = httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
)
expect(filteredReqs).toHaveLength(1)
filteredReqs[0].flush(full_results)
flushSelectionDataRequest(
httpTestingController,
`?tags__id__all=${tags__id__all}`
)
expect(documentListViewService.currentPage).toEqual(1)
})
it('should support quick filter', () => {
documentListViewService.quickFilter(filterRules)
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
`${environment.apiBaseUrl}documents/?page=${documentListViewService.currentPage}&page_size=${documentListViewService.pageSize}&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
)
expect(req.request.method).toEqual('GET')
})
@@ -367,21 +410,21 @@ describe('DocumentListViewService', () => {
convertToParamMap(params)
)
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=${page}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=${tags__id__all}`
`${environment.apiBaseUrl}documents/?page=${page}&page_size=${documentListViewService.pageSize}&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=${tags__id__all}`
)
expect(req.request.method).toEqual('GET')
// reset the list
documentListViewService.currentPage = 1
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=true&tags__id__all=9`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=false&tags__id__all=9`
)
documentListViewService.setFilterRules([])
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-added&truncate_content=true&include_selection_data=false`
)
documentListViewService.sortField = 'created'
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
documentListViewService.activateSavedView(null)
})
@@ -389,18 +432,22 @@ describe('DocumentListViewService', () => {
it('should support navigating next / previous', () => {
documentListViewService.setFilterRules([])
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(documentListViewService.currentPage).toEqual(1)
documentListViewService.pageSize = 3
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush({
count: 3,
results: documents.slice(0, 3),
})
// two reload()s ran above (setFilterRules, then pageSize), each firing
// its own concurrent filter_selection_data request with an identical
// (unfiltered) URL; this test doesn't assert on selectionData, so let
// afterEach's drain step clean both up rather than disambiguating here.
expect(documentListViewService.hasNext(documents[0].id)).toBeTruthy()
expect(documentListViewService.hasPrevious(documents[0].id)).toBeFalsy()
documentListViewService.getNext(documents[0].id).subscribe((docId) => {
@@ -447,7 +494,7 @@ describe('DocumentListViewService', () => {
expect(documentListViewService.currentPage).toEqual(1)
documentListViewService.pageSize = 3
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
)
jest
.spyOn(documentListViewService, 'getLastPage')
@@ -462,7 +509,7 @@ describe('DocumentListViewService', () => {
expect(reloadSpy).toHaveBeenCalled()
expect(documentListViewService.currentPage).toEqual(2)
const reqs = httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(reqs.length).toBeGreaterThan(0)
})
@@ -497,11 +544,11 @@ describe('DocumentListViewService', () => {
.mockReturnValue(documents)
documentListViewService.currentPage = 2
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=2&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
documentListViewService.pageSize = 3
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=2&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
)
const reloadSpy = jest.spyOn(documentListViewService, 'reload')
documentListViewService.getPrevious(1).subscribe({
@@ -511,7 +558,7 @@ describe('DocumentListViewService', () => {
expect(reloadSpy).toHaveBeenCalled()
expect(documentListViewService.currentPage).toEqual(1)
const reqs = httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(reqs.length).toBeGreaterThan(0)
})
@@ -524,10 +571,11 @@ describe('DocumentListViewService', () => {
it('should support select a document', () => {
documentListViewService.reload()
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.toggleSelected(documents[0])
expect(documentListViewService.isSelected(documents[0])).toBeTruthy()
documentListViewService.toggleSelected(documents[0])
@@ -537,10 +585,11 @@ describe('DocumentListViewService', () => {
it('should support select all', () => {
documentListViewService.reload()
const reloadReq = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(reloadReq.request.method).toEqual('GET')
reloadReq.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.selectAll()
expect(documentListViewService.allSelected).toBeTruthy()
@@ -553,13 +602,14 @@ describe('DocumentListViewService', () => {
it('should support select page', () => {
documentListViewService.pageSize = 3
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=3&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush({
count: 3,
results: documents.slice(0, 3),
})
flushSelectionDataRequest(httpTestingController)
documentListViewService.selectPage()
expect(documentListViewService.selected.size).toEqual(3)
expect(documentListViewService.isSelected(documents[5])).toBeFalsy()
@@ -568,10 +618,11 @@ describe('DocumentListViewService', () => {
it('should support select range', () => {
documentListViewService.reload()
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.toggleSelected(documents[0])
expect(documentListViewService.isSelected(documents[0])).toBeTruthy()
documentListViewService.selectRangeTo(documents[2])
@@ -583,9 +634,10 @@ describe('DocumentListViewService', () => {
it('should clear all-selected mode when toggling a single document', () => {
documentListViewService.reload()
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.selectAll()
expect(documentListViewService.allSelected).toBeTruthy()
@@ -599,9 +651,10 @@ describe('DocumentListViewService', () => {
it('should clear all-selected mode when selecting a range', () => {
documentListViewService.reload()
const req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.selectAll()
documentListViewService.toggleSelected(documents[1])
@@ -619,22 +672,24 @@ describe('DocumentListViewService', () => {
it('should support selection range reduction', () => {
documentListViewService.reload()
let req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(req.request.method).toEqual('GET')
req.flush(full_results)
flushSelectionDataRequest(httpTestingController)
documentListViewService.selectAll()
expect(documentListViewService.selected.size).toEqual(6)
documentListViewService.setFilterRules(filterRules)
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=9`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false&tags__id__all=9`
)
req.flush({
count: 3,
results: documents.slice(0, 3),
})
flushSelectionDataRequest(httpTestingController, '?tags__id__all=9')
expect(documentListViewService.allSelected).toBeTruthy()
expect(documentListViewService.selected.size).toEqual(3)
})
@@ -643,7 +698,7 @@ describe('DocumentListViewService', () => {
const cancelSpy = jest.spyOn(documentListViewService, 'cancelPending')
documentListViewService.reload()
httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true&tags__id__all=9`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(cancelSpy).toHaveBeenCalled()
})
@@ -662,7 +717,7 @@ describe('DocumentListViewService', () => {
documentListViewService.setFilterRules([])
expect(documentListViewService.sortField).toEqual('created')
httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
})
@@ -689,11 +744,11 @@ describe('DocumentListViewService', () => {
expect(localStorageSpy).toHaveBeenCalled()
// reload triggered
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
documentListViewService.displayFields = null
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
expect(documentListViewService.displayFields).toEqual(
DEFAULT_DISPLAY_FIELDS.filter((f) => f.id !== DisplayField.ADDED).map(
@@ -738,7 +793,7 @@ describe('DocumentListViewService', () => {
it('should generate quick filter URL preserving default state', () => {
documentListViewService.reload()
httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true`
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=false`
)
const urlTree = documentListViewService.getQuickFilterUrl(filterRules)
expect(urlTree).toBeDefined()
@@ -314,12 +314,39 @@ export class DocumentListViewService {
}
}
private loadFilterSelectionData(filterRules: FilterRule[]) {
this.documentService
.getFilterSelectionData(filterRules)
.pipe(takeUntil(this.unsubscribeNotifier))
.subscribe({
next: (selectionData) => {
this.selectionData = selectionData
this.markChanged()
},
error: () => {
this.selectionData = null
this.markChanged()
},
})
}
reload(onFinish?, updateQueryParams: boolean = true) {
this.cancelPending()
this.isReloading = true
this.error = null
this.markChanged()
let activeListViewState = this.activeListViewState
// Full-text search results are already narrowed by the search backend, so
// computing selection data inline there is cheap. A plain (unfiltered or
// ORM-filtered) browse can span the entire document set, so its selection
// data is fetched separately -- concurrently with the list itself, rather
// than blocking or waiting on it.
const isFullTextSearch = isFullTextFilterRule(
activeListViewState.filterRules
)
if (!isFullTextSearch) {
this.loadFilterSelectionData(activeListViewState.filterRules)
}
this.documentService
.listFiltered(
activeListViewState.currentPage,
@@ -327,17 +354,22 @@ export class DocumentListViewService {
activeListViewState.sortField,
activeListViewState.sortReverse,
activeListViewState.filterRules,
{ truncate_content: true, include_selection_data: true }
{
truncate_content: true,
include_selection_data: isFullTextSearch,
}
)
.pipe(takeUntil(this.unsubscribeNotifier))
.subscribe({
next: (result) => {
const resultWithSelectionData = result as DocumentResults
this.initialized = true
this.isReloading = false
activeListViewState.collectionSize = result.count
activeListViewState.documents = result.results
this.selectionData = resultWithSelectionData.selection_data ?? null
if (isFullTextSearch) {
this.selectionData =
(result as DocumentResults).selection_data ?? null
}
this.syncSelectedToCurrentPage()
this.markChanged()
@@ -376,6 +408,9 @@ export class DocumentListViewService {
// e.g. field was deleted
this.sortField = 'created'
} else {
// cancel the concurrently-fired selection-data request too, so it
// can't resolve afterward and clobber this reset with stale data
this.cancelPending()
this.selectionData = null
let errorMessage
if (
@@ -41,6 +41,24 @@ export abstract class AbstractPaperlessService<T extends ObjectWithId> {
}
}
/**
* Merges a plain params object into an HttpParams instance, skipping
* null/undefined values so they aren't serialized as literal "null" /
* "undefined" query string entries.
*/
protected withParams(
params,
base: HttpParams = new HttpParams()
): HttpParams {
let httpParams = base
for (let key in params) {
if (params[key] != null) {
httpParams = httpParams.set(key, params[key])
}
}
return httpParams
}
list(
page?: number,
pageSize?: number,
@@ -60,11 +78,7 @@ export abstract class AbstractPaperlessService<T extends ObjectWithId> {
if (ordering) {
httpParams = httpParams.set('ordering', ordering)
}
for (let extraParamKey in extraParams) {
if (extraParams[extraParamKey] != null) {
httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey])
}
}
httpParams = this.withParams(extraParams, httpParams)
return this.http
.get<Results<T>>(this.getResourceUrl(), {
params: httpParams,
@@ -113,11 +127,7 @@ export abstract class AbstractPaperlessService<T extends ObjectWithId> {
httpParams = httpParams.set('id__in', ids.join(','))
httpParams = httpParams.set('ordering', '-id')
httpParams = httpParams.set('page_size', 1000)
for (let extraParamKey in extraParams) {
if (extraParams[extraParamKey] != null) {
httpParams = httpParams.set(extraParamKey, extraParams[extraParamKey])
}
}
httpParams = this.withParams(extraParams, httpParams)
return this.http
.get<Results<T>>(this.getResourceUrl(), {
params: httpParams,
@@ -398,6 +398,13 @@ export class DocumentService extends AbstractPaperlessService<Document> {
)
}
getFilterSelectionData(filterRules: FilterRule[]): Observable<SelectionData> {
return this.http.get<SelectionData>(
this.getResourceUrl(null, 'filter_selection_data'),
{ params: this.withParams(queryParamsFromFilterRules(filterRules)) }
)
}
getSuggestions(id: number): Observable<DocumentSuggestions> {
return this.http.get<DocumentSuggestions>(
this.getResourceUrl(id, 'suggestions')
+18 -9
View File
@@ -1241,7 +1241,7 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
],
)
def test_list_with_include_selection_data(self) -> None:
def test_selection_data_endpoint(self) -> None:
correspondent = Correspondent.objects.create(name="c1")
doc_type = DocumentType.objects.create(name="dt1")
storage_path = StoragePath.objects.create(name="sp1")
@@ -1259,30 +1259,28 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
non_matching_doc.tags.add(Tag.objects.create(name="other"))
response = self.client.get(
f"/api/documents/?tags__id__in={tag.id}&include_selection_data=true",
f"/api/documents/filter_selection_data/?tags__id__in={tag.id}",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertIn("selection_data", response.data)
self.assertNotIn("results", response.data)
selected_correspondent = next(
item
for item in response.data["selection_data"]["selected_correspondents"]
for item in response.data["selected_correspondents"]
if item["id"] == correspondent.id
)
selected_tag = next(
item
for item in response.data["selection_data"]["selected_tags"]
if item["id"] == tag.id
item for item in response.data["selected_tags"] if item["id"] == tag.id
)
selected_type = next(
item
for item in response.data["selection_data"]["selected_document_types"]
for item in response.data["selected_document_types"]
if item["id"] == doc_type.id
)
selected_storage_path = next(
item
for item in response.data["selection_data"]["selected_storage_paths"]
for item in response.data["selected_storage_paths"]
if item["id"] == storage_path.id
)
@@ -1291,6 +1289,17 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
self.assertEqual(selected_type["document_count"], 1)
self.assertEqual(selected_storage_path["document_count"], 1)
def test_list_no_longer_supports_include_selection_data(self) -> None:
"""
include_selection_data was never part of a stable release (beta-only,
introduced and removed within the 3.0.0-beta cycle) -- the plain list
endpoint should just ignore the param now rather than compute it inline.
"""
response = self.client.get("/api/documents/?include_selection_data=true")
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertNotIn("selection_data", response.data)
def test_statistics(self) -> None:
doc1 = Document.objects.create(
title="none1",
+26 -31
View File
@@ -1034,29 +1034,27 @@ class DocumentViewSet(
],
}
def get_queryset(self):
def _base_document_queryset(self):
# Root documents only, with the annotations that filter_backends rely
# on (effective_content for SearchFilter, num_notes for ordering) --
# but no select_related/prefetch_related, since those only matter for
# serializing documents, not for filtering, ordering, or aggregating.
latest_version_content = Subquery(
Document.objects.filter(root_document=OuterRef("pk"))
.order_by("-id")
.values("content")[:1],
)
# A correlated subquery avoids the LEFT JOIN + Count() this used to
# be, which forced a GROUP BY aggregate over every matching document
# before the query could even be sorted or limited.
note_count = Subquery(
Note.objects.filter(document=OuterRef("pk"))
.order_by()
.values("document")
.annotate(count=Count("pk"))
.values("count"),
output_field=IntegerField(),
)
return (
Document.objects.filter(root_document__isnull=True)
.distinct()
.order_by("-created", "-id")
.annotate(effective_content=Coalesce(latest_version_content, F("content")))
.annotate(num_notes=Coalesce(note_count, 0))
.annotate(num_notes=Count("notes"))
)
def get_queryset(self):
return (
self._base_document_queryset()
.select_related("correspondent", "storage_path", "document_type", "owner")
.prefetch_related(
Prefetch(
@@ -1195,24 +1193,21 @@ class DocumentViewSet(
return response
def list(self, request, *args, **kwargs):
if not get_boolean(
str(request.query_params.get("include_selection_data", "false")),
):
return super().list(request, *args, **kwargs)
queryset = self.filter_queryset(self.get_queryset())
selection_data = self._get_selection_data_for_queryset(queryset)
page = self.paginate_queryset(queryset)
if page is not None:
serializer = self.get_serializer(page, many=True)
response = self.get_paginated_response(serializer.data)
response.data["selection_data"] = selection_data
return response
serializer = self.get_serializer(queryset, many=True)
return Response({"results": serializer.data, "selection_data": selection_data})
@extend_schema(
operation_id="documents_filter_selection_data",
description=(
"Returns per-tag/correspondent/document-type/storage-path/custom-field "
"document counts for the current filter, without paginating or "
"serializing the matching documents themselves. Split out from the "
"plain document list so that browsing the (potentially huge) unfiltered "
"document list doesn't pay for this aggregation on every request."
),
responses={200: inline_serializer(name="SelectionData", fields={})},
)
@action(detail=False, methods=["get"], url_path="filter_selection_data")
def filter_selection_data(self, request, *args, **kwargs):
queryset = self.filter_queryset(self._base_document_queryset())
return Response(self._get_selection_data_for_queryset(queryset))
def destroy(self, request, *args, **kwargs):
from documents.search import get_backend
+23 -67
View File
@@ -73,8 +73,6 @@ APPLE_MAIL_TAG_COLORS = {
"grey": ["$MailFlagBit1", "$MailFlagBit2"],
}
MAIL_FETCH_BATCH_SIZE = 500
class MailError(Exception):
pass
@@ -683,55 +681,17 @@ class MailAccountHandler(LoggingMixin):
)
try:
all_uids = set(
M.uids(criteria=criterias, charset=rule.account.character_set),
messages = M.fetch(
criteria=criterias,
mark_seen=False,
charset=rule.account.character_set,
bulk=True,
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while searching folder {rule.folder}",
f"Rule {rule}: Error while fetching folder {rule.folder}",
) from err
processed_uids_qs = ProcessedMail.objects.filter(
rule=rule,
folder=rule.folder,
uid__in=all_uids,
)
if self._current_uid_validity is not None:
processed_uids_qs = processed_uids_qs.filter(
Q(uid_validity=self._current_uid_validity)
| Q(uid_validity__isnull=True),
)
processed_uids = set(processed_uids_qs.values_list("uid", flat=True))
new_uids = all_uids - processed_uids
if not new_uids:
self.log.debug(
f"Rule {rule}: No new mail matching criteria {criterias}",
)
return 0
sorted_new_uids = sorted(new_uids, key=int)
message_batches = []
# ikvk/imap_tools#268 requests a direct UID-list fetch that would let us drop this manual batching loop
for batch_start in range(0, len(sorted_new_uids), MAIL_FETCH_BATCH_SIZE):
batch = sorted_new_uids[batch_start : batch_start + MAIL_FETCH_BATCH_SIZE]
try:
message_batches.append(
M.fetch(
criteria=AND(uid=batch),
mark_seen=False,
charset=rule.account.character_set,
bulk=True,
),
)
except Exception as err:
raise MailError(
f"Rule {rule}: Error while fetching folder {rule.folder}",
) from err
messages = itertools.chain(*message_batches)
mails_processed = 0
total_processed_files = 0
rule_seen_messages: set[tuple[str, str | None]] = set()
@@ -797,7 +757,6 @@ class MailAccountHandler(LoggingMixin):
not message.attachments
and rule.consumption_scope == MailRule.ConsumptionScope.ATTACHMENTS_ONLY
):
self._record_processed_without_consumption(message, rule)
return processed_elements
self.log.debug(
@@ -833,25 +792,6 @@ class MailAccountHandler(LoggingMixin):
return processed_elements
def _record_processed_without_consumption(
self,
message: MailMessage,
rule: MailRule,
) -> None:
ProcessedMail.objects.get_or_create(
rule=rule,
uid=message.uid,
folder=rule.folder,
uid_validity=self._current_uid_validity,
defaults={
"subject": message.subject,
"received": make_aware(message.date)
if is_naive(message.date)
else message.date,
"status": "PROCESSED_WO_CONSUMPTION",
},
)
def filename_inclusion_matches(
self,
filter_attachment_filename_include: str | None,
@@ -1018,7 +958,23 @@ class MailAccountHandler(LoggingMixin):
)
else:
# No files to consume, just mark as processed if it wasn't by .eml processing
self._record_processed_without_consumption(message, rule)
if not ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
uid_validity=self._current_uid_validity,
).exists():
ProcessedMail.objects.create(
rule=rule,
folder=rule.folder,
uid=message.uid,
uid_validity=self._current_uid_validity,
subject=message.subject,
received=make_aware(message.date)
if is_naive(message.date)
else message.date,
status="PROCESSED_WO_CONSUMPTION",
)
return processed_attachments
+3 -159
View File
@@ -135,12 +135,6 @@ class BogusMailBox(AbstractContextManager):
raise MailboxLoginError("BAD", "OK")
def fetch(self, criteria, mark_seen, charset="", *, bulk=True):
return self._filter_messages(criteria)
def uids(self, criteria, charset="") -> list[str]:
return [m.uid for m in self._filter_messages(criteria)]
def _filter_messages(self, criteria):
msg = self.messages
criteria = str(criteria).strip("()").split(" ")
@@ -174,10 +168,6 @@ class BogusMailBox(AbstractContextManager):
if "(X-GM-LABELS" in criteria: # ['NOT', '(X-GM-LABELS', '"processed"']
msg = filter(lambda m: "processed" not in m.flags, msg)
if "UID" in criteria:
uid_list = criteria[criteria.index("UID") + 1].split(",")
msg = filter(lambda m: m.uid in uid_list, msg)
return list(msg)
def delete(self, uid_list) -> None:
@@ -435,54 +425,6 @@ class TestMail(
super().setUp()
@mock.patch("paperless_mail.mail.MAIL_FETCH_BATCH_SIZE", 5)
def test_handle_mail_account_batches_body_fetch_for_large_backlog(self) -> None:
"""
GIVEN:
- More new/unprocessed mail than MAIL_FETCH_BATCH_SIZE
WHEN:
- The mail account is processed
THEN:
- The body fetch is issued in multiple batches
- Every message is still processed (none dropped at a batch boundary)
"""
account = MailAccount.objects.create(
name="test",
imap_server="",
username="admin",
password="secret",
)
rule = MailRule.objects.create(
name="testrule",
account=account,
action=MailRule.MailAction.MARK_READ,
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
)
message_count = 12 # more than the patched batch size of 5
self.mailMocker.bogus_mailbox.messages = [
self.mailMocker.messageBuilder.create_message(
subject=f"No attachment {i}",
attachments=[],
)
for i in range(message_count)
]
self.mailMocker.bogus_mailbox.updateClient()
with mock.patch.object(
self.mailMocker.bogus_mailbox,
"fetch",
wraps=self.mailMocker.bogus_mailbox.fetch,
) as fetch_spy:
self.mail_account_handler.handle_mail_account(account)
# ceil(12 / 5) == 3 batches
self.assertEqual(fetch_spy.call_count, 3)
self.assertEqual(
ProcessedMail.objects.filter(rule=rule).count(),
message_count,
)
def test_get_correspondent(self) -> None:
message = namedtuple("MailMessage", [])
message.from_ = "someone@somewhere.com"
@@ -595,59 +537,17 @@ class TestMail(
],
)
def test_bogus_mailbox_uids_and_uid_criteria(self) -> None:
mailbox = self.mailMocker.bogus_mailbox
all_messages = list(mailbox.messages)
# uids() returns the UIDs of unseen messages, no bodies needed to call it
unseen_uids = mailbox.uids("(UNSEEN)")
self.assertEqual(
set(unseen_uids),
{m.uid for m in all_messages if not m.seen},
)
# fetch() with an explicit UID criteria returns only the matching messages
target_uid = all_messages[0].uid
from imap_tools import AND
fetched = mailbox.fetch(AND(uid=[target_uid]), mark_seen=False)
self.assertEqual([m.uid for m in fetched], [target_uid])
def test_handle_empty_message(self) -> None:
message = self.mailMocker.messageBuilder.create_message(
subject="No attachments here",
attachments=[],
)
message = namedtuple("MailMessage", [])
account = MailAccount.objects.create()
rule = MailRule.objects.create(
account=account,
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
)
message.attachments = []
rule = MailRule()
result = self.mail_account_handler._handle_message(message, rule)
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
self.assertEqual(result, 0)
processed = ProcessedMail.objects.get(
rule=rule,
uid=message.uid,
folder=rule.folder,
)
self.assertEqual(processed.status, "PROCESSED_WO_CONSUMPTION")
# Calling it again must not create a second row
self.mail_account_handler._handle_message(message, rule)
self.assertEqual(
ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
).count(),
1,
)
def test_handle_unknown_mime_type(self) -> None:
message = self.mailMocker.messageBuilder.create_message(
attachments=[
@@ -1012,62 +912,6 @@ class TestMail(
]
self.assertEqual(queued_rule.id, first_rule.id)
def test_handle_mail_account_skips_body_fetch_for_already_processed_mail(
self,
) -> None:
"""
GIVEN:
- An attachment-less mail under an attachments-only mark-read rule,
already recorded as PROCESSED_WO_CONSUMPTION
WHEN:
- The mail account is processed again and the mail still matches the
search criteria (it was never marked read, since no mail action is
applied for the no-consumption case)
THEN:
- No IMAP body fetch happens for that mail; only the cheap UID search runs.
"""
account = MailAccount.objects.create(
name="test",
imap_server="",
username="admin",
password="secret",
)
rule = MailRule.objects.create(
name="testrule",
account=account,
action=MailRule.MailAction.MARK_READ,
consumption_scope=MailRule.ConsumptionScope.ATTACHMENTS_ONLY,
)
message = self.mailMocker.messageBuilder.create_message(
subject="No attachment",
attachments=[],
)
self.mailMocker.bogus_mailbox.messages = [message]
self.mailMocker.bogus_mailbox.updateClient()
# First run: records ProcessedMail without consuming anything.
self.mail_account_handler.handle_mail_account(account)
self.assertTrue(
ProcessedMail.objects.filter(
rule=rule,
uid=message.uid,
folder=rule.folder,
).exists(),
)
self.mailMocker._queue_consumption_tasks_mock.assert_not_called()
# Second run: message still matches UNSEEN (mark-read action never ran),
# but its body must not be downloaded again.
with mock.patch.object(
self.mailMocker.bogus_mailbox,
"fetch",
wraps=self.mailMocker.bogus_mailbox.fetch,
) as fetch_spy:
self.mail_account_handler.handle_mail_account(account)
fetch_spy.assert_not_called()
def test_handle_mail_account_skip_duplicate_uids_from_fetch(self) -> None:
"""
GIVEN: