Skip to content

Deduplicate and optimize the collection classes#3527

Draft
pfefferle wants to merge 9 commits into
trunkfrom
refactor/optimize-collections
Draft

Deduplicate and optimize the collection classes#3527
pfefferle wants to merge 9 commits into
trunkfrom
refactor/optimize-collections

Conversation

@pfefferle

Copy link
Copy Markdown
Member

Proposed changes:

Refactors includes/collection/ to remove duplication and cut complexity, keeping every public call site identical (composable traits via late static binding, static shape retained).

  • Bug fix: Interactions::persist() attached an anonymous akismet_comment_nonce filter it never removed, leaking a closure per federated comment and forcing Akismet's nonce check to inactive for every later comment in the same request. Now a named method plus try/finally restores all four temporary filters on every path.
  • Six new traits de-duplicate the copy-pasted patterns across the CPT-backed collections: recipient meta CRUD, paginated query() + get_by_authority(), object-title derivation, guid lookup, KSES-safe insert, and the batched purge loop.
  • Followers::get_inboxes collapses a two-step query into a single JOIN (runs on every federation send).
  • Remote_Actors actor serialization now suspends and restores only the mention/hashtag/link filters that were actually registered, at their original priority (fixes an unconditional re-add edge).
  • Actors::get_id_by_resource replaces a sloppy scheme regex with wp_parse_url().
  • Remote_Posts::get_by_remote_actor_id swaps deprecated meta_key/meta_value for a meta_query (drops two PHPCS suppressions).

Net roughly −435 lines across the collection classes, plus six small trait files. With_Guid_Lookup also drops a latent double-escape in Remote_Actors::get_by_uri (it ran esc_sql() inside prepare()), standardizing on the same esc_url_raw normalization used at write time.

Other information:

  • Have you written new tests for your changes, if applicable?

New regression test asserts the comment filters/actions are fully restored after persist().

Testing instructions:

  • Ensure the full PHP suite passes: npm run env-test (2760 tests green).
  • Ensure PHP standards pass: composer lint (clean).
  • Functional spot checks against trunk:
    • Receive a comment from the fediverse on a post, then submit a normal local comment, and confirm spam protection still applies to the local comment.
    • Follow and unfollow a remote actor; view followers and following lists with pagination.
    • Confirm inbox/outbox/remote-post cleanup (purge) still trims old items and preserves remote posts that have local comments.

Changelog entry

Changelog entry for the Akismet fix is included in the branch (.github/changelog/restore-akismet-nonce-after-federated-comment); the rest is internal refactor.

pfefferle added 2 commits July 8, 2026 19:04
Persist() attached an anonymous akismet_comment_nonce filter that was never
removed, leaking a closure per federated comment and forcing Akismet's nonce
check to 'inactive' for every later comment in the request. Use a named method
and a try/finally so all four temporary filters are restored on every path.

Claude-Session: https://claude.ai/code/session_01HDA9QR3bJoqM7NhnhT1whg
Extract the copy-pasted patterns across the CPT-backed collections into six
composable traits (recipients, guid lookup, KSES-safe insert, object title,
paginated query, purge) using late static binding, keeping the static call
sites unchanged. Collapse Followers::get_inboxes into a single JOIN, replace a
sloppy scheme regex with wp_parse_url, harden the actor serialization filter
toggle to restore only the filters it removed, and drop deprecated meta_key
query args.

Claude-Session: https://claude.ai/code/session_01HDA9QR3bJoqM7NhnhT1whg
Split Interactions::activity_to_comment into prepare_local_comment_data,
prepare_remote_comment_data, and prepare_remote_comment_meta helpers, and
funnel Remote_Actors create/update through a single persist() method so the
KSES-safe insert/update path lives in one place.

Claude-Session: https://claude.ai/code/session_01HDA9QR3bJoqM7NhnhT1whg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant