Validate auto_update_query when a custom list is saved (PP-4506)

[dbernstein]

Description

Note

Stacked on #3550 — its base is bugfix/custom-list-sweep-query-parse-error, so the diff shown here is only this commit. Held in draft until #3550 merges; the base will then be retargeted to main.

A custom list's auto_update_query was only checked for JSON serializability before being stored — never for whether the search layer could actually parse it. CustomListsController._create_or_update_list now builds a JSONQuery from the submitted query and returns INVALID_INPUT (400) if it can't be parsed, passing along the parser's own explanation of what's wrong.

Also corrects the auto_update_query parameter annotation, which claimed dict[str, str] even though a query's value is a nested dict. That's why the existing tests were able to pass placeholders like {"query": "foo"} without mypy complaining.

Motivation and Context

Follow-up to #3550, which fixed the symptom: a list whose stored query contained a published value of 2025>01>01 failed its Celery task every sweep and (before that PR) wedged the sweep lock for every library.

This PR fixes how the bad value got in. Two gaps combined:

• Creating a list incidentally exercised its query, via the populate_query_pages call on the is_new path — but a QueryParseException there surfaced as an unhandled 500, not a validation error.
• Editing a list never parsed the query at all. So an invalid query could only ever be introduced — and never surfaced — through an edit.

Either way the result was a list that silently stopped updating, since the entry-update task can do nothing with an unparseable query but log it and skip. Rejecting the query at save time turns a silent, indefinitely-stalled list into an immediate 400 for the librarian who fat-fingered the date.

Behavior change worth flagging in review: a librarian editing an existing list whose stored query is already invalid will now get a 400 rather than a successful save, even if they only meant to change the list's name — the admin UI resubmits auto_update_query unchanged. That seems right to me (it forces the broken query to be fixed), but it is a real change, and it means any already-corrupted rows in production should be corrected before this ships.

How Has This Been Tested?

tox -e py312-docker -- tests/manager/api/admin/controller/test_custom_lists.py tests/manager/core/test_customlist_queries.py tests/manager/celery/tasks/test_custom_lists.py (64 passed).

• test_auto_update_query_must_be_a_valid_search_query is parametrized over four ways to be invalid — the 2025>01>01 value from production, an unknown key, an unknown operator, and a missing query root — and asserts the parser's reason is surfaced in the problem detail and that nothing is persisted.
• test_auto_update_query_validated_on_edit covers the gap specifically: an existing list rejects a bad query on edit and keeps its previous good one.
• Three existing tests had to be updated. They passed placeholder queries ({"query": "foo"}, {"query": "...changed"}) that the new validation correctly rejects. They test facets serialization, entry handling, and the repopulate-on-query-change path, so they now pass a valid query. That they needed changing at all is a decent illustration of how little the old code cared what was in this column.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

🤖 Generated with Claude Code

[claude]

Claude finished @dbernstein's task in 4m 59s —— View job

---

Code review — PR #3551

• Read the diff and changed files
• Trace _create_or_update_list control flow and JSONQuery.search_query behavior
• Check test coverage
• Post review

Summary

The approach is sound — parsing the query at save time (for both create and edit) is the right place to turn a silently-stalled list into an immediate error, and the parametrized tests are a good illustration of the gap being closed. The annotation fix (dict[str, str] → dict[str, Any]) is correct and lets the now-unnecessary type: ignore on the serialization test go away. My one concern is that the new validation only catches QueryParseException, so a different class of malformed query still escapes as a 500 rather than the intended 400.

Details

src/palace/manager/api/admin/controller/custom_lists.py:179-187

The new block only catches QueryParseException, but JSONQuery(...).search_query raises a plain AttributeError/TypeError for a malformed-yet-JSON-serializable query whenever a clause isn't a dict. {"query": "foo"} reaches _parse_json_query("foo"), which calls "foo".keys() → AttributeError; {"query": {"key": "published", "value": 20250101}} calls value.split("-") on an int → AttributeError; {"and": 5} iterates 5 → TypeError. Since auto_update_query is only validated as a top-level dict, these shapes can be submitted, and they escape both this handler and the outer except ProblemDetailException, surfacing as an unhandled 500 — the exact outcome this PR set out to convert into a 400. It's also why the description's claim that the old {"query": "foo"} placeholder is "correctly rejected" doesn't hold: that input 500s. Worth widening the catch (and adding a parametrized case with a non-dict clause so a test pins the behavior):

except QueryParseException as exc:
    raise ProblemDetailException(
        INVALID_INPUT.detailed(
            f"auto_update_query is not a valid search query: {exc.detail}"
        )
    )
except (AttributeError, TypeError):
    raise ProblemDetailException(
        INVALID_INPUT.detailed("auto_update_query is not a valid search query")
    )

circulation/src/palace/manager/api/admin/controller/custom_lists.py

Lines 179 to 187 in 46aa7f0

	try:
	# Building the query is what validates it; the result is discarded.
	_ = JSONQuery(auto_update_query).search_query
	except QueryParseException as exc:
	raise ProblemDetailException(
	INVALID_INPUT.detailed(
	f"auto_update_query is not a valid search query: {exc.detail}"
	)
	)

[greptile-apps]

Greptile Summary

This PR validates custom list auto-update queries before saving them. The main changes are:

• Added JSONQuery parsing in the custom list save path.
• Returned invalid-input errors for parser-rejected auto-update queries.
• Widened the auto_update_query annotation for nested query values.
• Updated custom list controller tests to use valid query shapes and cover invalid saves.

Confidence Score: 4/5

The changed save path needs a contained fix for malformed nested query values.

• Normal parser errors now return controlled invalid-input responses.
• JSON-serializable payloads with unexpected nested value types can still escape as server errors.
• The import and storage paths look consistent with the inspected controller and sweep behavior.

src/palace/manager/api/admin/controller/custom_lists.py

Important Files Changed

Filename	Overview
src/palace/manager/api/admin/controller/custom_lists.py	Adds save-time JSONQuery validation, but the new parser call only handles parser-owned exceptions.
tests/manager/api/admin/controller/test_custom_lists.py	Updates query fixtures and adds coverage for invalid parser cases and edit-time validation.

_{Reviews (1): Last reviewed commit: "Validate auto_update_query when a custom..." | Re-trigger Greptile}

[greptile-apps]

Malformed Values Escape Validation

When a librarian submits a JSON-serializable query with the right keys but a wrong nested value type, such as a numeric title value, this new validation can raise a plain parser runtime error instead of QueryParseException. The admin save path then returns a 500 instead of the intended invalid-input response, so malformed auto_update_query payloads are still not handled safely at save time.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.46%. Comparing base (ad810db) to head (46aa7f0).

Additional details and impacted files

@@                              Coverage Diff                               @@
##           bugfix/custom-list-sweep-query-parse-error    #3551      +/-   ##
==============================================================================
- Coverage                                       93.46%   93.46%   -0.01%     
==============================================================================
  Files                                             512      512              
  Lines                                           46614    46623       +9     
  Branches                                         6352     6352              
==============================================================================
+ Hits                                            43570    43578       +8     
- Misses                                           1968     1969       +1     
  Partials                                         1076     1076              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.