feat: add configurable import batching and throttling - #1285
feat: add configurable import batching and throttling#1285lucadobrescu wants to merge 19 commits into
Conversation
|
Plugin build for c734cd4 is ready 🛎️!
Note You can preview the changes in the Playground |
🌍 i18n String Review Report📊 Summary
➕ Added Strings (5) - Click to expand
|
The Plugin Check tool ignores inline phpcs:ignore for its guidelines check, so editing those pre-existing rename() comments only pulled the findings into this PR's diff. Revert the comment edits so the untouched lines match development and are no longer annotated.
Plugin Check annotates every finding in files a PR touches and ignores inline phpcs:ignore, so the pre-existing rename() calls kept failing the WordPress.org Guidelines Check. Use WP_Filesystem::move() as required.
WP_Filesystem::move() fails to initialize in the import/cron execution context, breaking image import (PHPUnit test_attachement_import and E2E). The rename() Plugin Check finding is pre-existing and non-blocking, so keep the working rename() rather than regress a core feature.
Replaces the two rename() calls flagged by the WordPress.org Guidelines Check with a move_temp_file() helper backed by WP_Filesystem::move(). The previous WP_Filesystem attempt regressed image import because move() with overwrite deletes the destination first: when the source already had the correct extension, source and destination were identical and the file was destroyed. The helper now treats same-path as a no-op success. Verified locally against the full PHPUnit suite (92 tests).
Reverts the WP_Filesystem::move() change. Those file ops are pre-existing in development and unrelated to import batching; the Plugin Check finding is non-blocking. Keeping the PR focused and consistent with dev; the WP_Filesystem migration should be a separate, plugin-wide change.
There was a problem hiding this comment.
Pull request overview
Adds configurable batching and throttling to Feedzy import jobs while preserving unlimited processing by default.
Changes:
- Adds batch size, delay, and cursor-based resume handling.
- Adds Pro settings, persistence coverage, and documentation.
- Updates compliance workflows and PHP compatibility fixes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
includes/admin/feedzy-rss-feeds-import.php |
Implements batching, throttling, cursor cleanup, and checkpoints. |
includes/views/import-metabox-edit.php |
Adds Advanced import controls. |
tests/test-import.php |
Tests settings persistence and cursor clearing. |
readme.txt |
Documents import batching. |
readme.md |
Mirrors batching documentation. |
.github/workflows/plugin-check.yml |
Makes Plugin Check advisory. |
.github/workflows/diff-translations.yml |
Makes translation differences advisory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( 0 < $attempted_items && 0 < $import_item_delay_ms ) { | ||
| usleep( $import_item_delay_ms * 1000 ); |
There was a problem hiding this comment.
Done in 5272ac0 — per-run sleep is now capped by a filterable budget (feedzy_import_delay_budget_ms, default 30s); when the next delay would exceed it the run stops and the batch cursor resumes on the next cron run.
|
|
||
| - uses: wordpress/plugin-check-action@v1 | ||
| id: plugin-check | ||
| continue-on-error: true |
There was a problem hiding this comment.
Done in 5272ac0 — kept continue-on-error for the known pre-existing findings, but the summary step now fails the job when high-risk findings are present, and also when the action crashes without producing a results file.
| $batch_cursor = 0 < $import_batch_size ? (string) get_post_meta( $job->ID, 'import_batch_cursor', true ) : ''; | ||
| $cursor_reached = empty( $batch_cursor ); |
There was a problem hiding this comment.
Done in 5272ac0 — before iterating, the run checks whether the saved cursor still exists in the bounded result; if it aged out, the cursor is treated as reached and the run processes from the beginning.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
.github/workflows/plugin-check.yml:23
- This makes every Plugin Check failure non-blocking, including newly introduced security findings and action/infrastructure failures; the summary step also exits successfully when the results file is absent. Preserve blocking for new/high-risk findings and baseline or ignore only the known intentional findings instead.
continue-on-error: true
| ); | ||
|
|
||
| if ( empty( $result ) ) { | ||
| if ( empty( $result ) && ! get_post_meta( $job->ID, 'import_batch_cursor', true ) ) { |
There was a problem hiding this comment.
Done in 5272ac0 — the empty-result retry is now skipped whenever batching is enabled for the job (not inferred from the cursor), so a completed zero-import batch cannot double-run in the same request.
| do_action( 'feedzy_after_post_import', $new_post_id, $item, $this->settings ); | ||
|
|
||
| if ( $use_new_hash ) { | ||
| update_post_meta( $job->ID, 'imported_items_hash', $imported_items ); | ||
| } else { | ||
| update_post_meta( $job->ID, 'imported_items', $imported_items ); | ||
| } | ||
| update_post_meta( $job->ID, 'imported_items_count', $count ); |
There was a problem hiding this comment.
Done in 5272ac0 — the imported-items checkpoint is persisted before feedzy_after_post_import fires.
- cap per-run throttle sleep to a filterable budget (feedzy_import_delay_budget_ms, default 30s) and resume via the batch cursor instead of sleeping for the whole configured matrix in one request - detect an evicted batch cursor before iterating and process the run from the start instead of skipping every candidate - skip the empty-result retry for batched jobs so a completed zero-import batch cannot double-run in the same request - persist the imported-items checkpoint before the feedzy_after_post_import hook so throwing callbacks cannot cause re-imports - plugin-check: fail on high-risk findings and on action crashes instead of swallowing every failure via continue-on-error Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| uses: Codeinwp/action-i18n-string-reviewer@main | ||
| with: | ||
| fail-on-changes: 'true' | ||
| fail-on-changes: 'false' |
There was a problem hiding this comment.
let's not change this
There was a problem hiding this comment.
Done in bc8c1e3 — reverted to fail-on-changes: 'true'.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
includes/admin/feedzy-rss-feeds-import.php:2014
- Cursor persistence is enabled only when the batch-size limit is nonzero, but the delay budget can independently stop a run. With the documented
batch size = 0/delay = 60000setup, no cursor is saved; line 1623 also classifies the job as unbatched, so a zero-result run can immediately retry in the same request. Treat either active control as requiring cursor handoff and retry suppression.
if ( 0 < $import_batch_size ) {
update_post_meta( $job->ID, 'import_batch_cursor', $item_hash );
includes/admin/feedzy-rss-feeds-import.php:1699
- The runtime reads both stored controls even when
feedzy_is_pro()is false. Unlike the adjacentimport_feed_limitgate, deactivating/losing Pro therefore leaves previously configured batching and sleeping active, contradicting the locked UI and the stated “engine stays off” behavior. Gate the stored defaults (and the corresponding retry check at line 1623) by Pro status while still allowing filters to override the zero default.
(int) apply_filters( 'feedzy_import_batch_size', get_post_meta( $job->ID, 'import_batch_size', true ), $job )
tests/test-import.php:94
- These additions only test meta persistence and save-time cursor deletion. Because
import_feed_limitandimport_batch_sizeare both 1, the existing cron assertion cannot exercise the new multi-run limit/resume behavior, delay-budget handoff, duplicate accounting, or checkpoint-before-hook guarantee. Add focused core importer tests using a multi-item mocked feed and consecutive cron runs; this would also cover the delay-only case currently missed by the implementation.
$_POST['feedzy_meta_data']['import_batch_size'] = 1;
$_POST['feedzy_meta_data']['import_item_delay_ms'] = 100;
Per review feedback from @Soare-Robert-Daniel. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keeps the two already-translated strings intact so the i18n diff only reports the genuinely new Import Batching strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
includes/admin/feedzy-rss-feeds-import.php:2015
- A delay-only configuration (
import_batch_size = 0,import_item_delay_ms > 0) can stop at the delay budget here, but this condition never saves a cursor. The next cron therefore restarts at the beginning; with a repeatedly failing first candidate and a delay larger than the budget, later items are never reached. Treat a positive item delay as resume-enabled as well, and use the same condition when loading the cursor and suppressing cron's empty-result retry.
if ( 0 < $import_batch_size ) {
update_post_meta( $job->ID, 'import_batch_cursor', $item_hash );
}
A job with batch size 0 but a positive item delay can stop early at the delay budget, yet no cursor was saved and cron's empty-result retry still fired. Treat either control as batching-active so cursor save, cursor load, and retry suppression all use the same condition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
includes/admin/feedzy-rss-feeds-import.php:2857
- The checkpoint is still after the public
feedzy_import_extrahook and other extension callbacks. If one of those throws afterwp_insert_post()succeeds, the post exists but its hash is never persisted; once the cursor completes a pass, the item can be imported again. Persist the hash/count immediately after the successful insertion and count update, before any post-processing hooks.
if ( $use_new_hash ) {
update_post_meta( $job->ID, 'imported_items_hash', $imported_items );
} else {
update_post_meta( $job->ID, 'imported_items', $imported_items );
}
includes/admin/feedzy-rss-feeds-import.php:1700
- The runner reads these Pro-only meta values unconditionally. After Pro is deactivated or its license expires, previously saved nonzero values still enable batching/throttling (and direct meta writes enable it on a free-only install), contradicting the stated requirement that the engine stays off without Pro. Gate both effective values—and the retry check in
run_cron()—on the same Pro/legacy entitlement so free runs resolve to zero.
$import_batch_size = max(
0,
min(
9999,
(int) apply_filters( 'feedzy_import_batch_size', get_post_meta( $job->ID, 'import_batch_size', true ), $job )
readme.txt:66
- The new
feedzy_import_delay_budget_msfilter is omitted, so developers cannot discover how to change the documented 30-second per-run delay cap. Document this filter alongside the two setting filters.
* **Import Batching:** Limit how many new items are processed in one cron request and optionally add a delay between items. On memory-restricted hosts, start with a batch size of 1–3 under General feed settings → Advanced. Developers can override these values with the `feedzy_import_batch_size` and `feedzy_import_item_delay_ms` filters.
readme.md:66
- The new
feedzy_import_delay_budget_msfilter is omitted, so developers cannot discover how to change the documented 30-second per-run delay cap. Document this filter alongside the two setting filters.
* **Import Batching:** Limit how many new items are processed in one cron request and optionally add a delay between items. On memory-restricted hosts, start with a batch size of 1–3 under General feed settings → Advanced. Developers can override these values with the `feedzy_import_batch_size` and `feedzy_import_item_delay_ms` filters.
- Persist the imported-items checkpoint immediately after a successful insert, before feedzy_import_extra and other post-processing hooks, so a throwing callback cannot cause a re-import. - Gate stored batching/throttling meta on Pro/legacy entitlement; free runs resolve to zero (filters can still override), matching the locked UI. - Document the feedzy_import_delay_budget_ms filter in both readmes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Automated imports can exhaust PHP memory when one cron request processes too many feed items (feedzy-rss-feeds-pro#992). This adds opt-in batching and throttling to the core importer: a job can cap how many new items it processes per run and pause between items, resuming where it left off on the next run. Both controls default to
0, so existing jobs keep today's unlimited behavior.What changed
Import runner — reads
import_batch_size(new items attempted per run) andimport_item_delay_ms(pause between attempts) from job meta, clamped to0–9999/0–60000and overridable via thefeedzy_import_batch_sizeandfeedzy_import_item_delay_msfilters.Resume cursor — a batched run records the last attempted item as
import_batch_cursor; the next run fast-forwards past it, and a completed full pass clears it. If the cursor item has aged out of the current feed window, the run starts from the beginning instead of skipping everything. Duplicates are skipped before batch accounting, so they never consume the allowance.Delay budget — sleeping is capped per run (
feedzy_import_delay_budget_msfilter, default 30s); when the next pause would not fit, the run stops early and the cursor hands off to the next cron run.Progress checkpointing — each successful import persists the imported-items hash and count immediately, before
feedzy_after_post_importfires, so a third-party callback that throws mid-run can no longer cause items to re-import. Cron's existing "empty result → run again" retry stands down for batched jobs, which would otherwise spend their whole allowance in a single request.Stale-state cleanup — saving a job, resetting its imported items, or finishing a full pass deletes the cursor, and duplicating a job never copies it, so no job can resume from another job's (or an outdated) position.
Edit screen + docs — new PRO-gated Import Batching fields ("Items processed per cron run", "Delay between items (milliseconds)") under the import's General feed settings → Advanced tab, with an upsell placeholder for free installs; readme.txt/readme.md document the feature and its filters.
Note
The companion Pro PR Codeinwp/feedzy-rss-feeds-pro#997 persists the same settings through the Pro import abilities; without Pro, the fields render locked and the engine stays off. The i18n string-review check reports the new UI strings for translator review; existing strings are untouched.
Import run flow
flowchart LR T[Cron or Run Now] --> R{New:<br/>resume cursor<br/>saved?}:::added R -- Yes --> S[New: fast-forward<br/>past cursor]:::added --> L[Scan feed items] R -- No --> L L --> D{Duplicate<br/>item?} D -- Yes --> L D -- No --> B{New: batch full or<br/>delay budget spent?}:::added B -- Yes --> C[New: save cursor,<br/>end run]:::added -. next run resumes .-> T B -- No --> I[Changed: import and<br/>checkpoint immediately]:::changed --> L L -- all items scanned --> F[New: clear cursor,<br/>finish run]:::added classDef added fill:#1a7f37,color:#fff,stroke:#116329,stroke-width:3px classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3Data changes
Before: import jobs stored only the candidate scan limit (
import_feed_limit). After: jobs can also store the two processing controls and a temporary resume position.import_batch_size0–9999, default0(unlimited)import_item_delay_ms0–60000, default0import_batch_cursorQA
The batch-resume behavior (batch size 1 importing exactly one new post per run across three runs) is proven by the companion e2e spec in Codeinwp/feedzy-rss-feeds-pro#997; the steps below cover what automation doesn't.
On a free-only install (Pro deactivated), go to WP Admin → Feedzy → Import Posts → Add New, open the General feed settings step, then the Advanced tab, and find the Import Batching section.
Expect: the section carries a PRO badge with the upsell styling, and saving the job does not persist values typed into its fields.
With Pro active and licensed, edit the same import, set "Items processed per cron run" to
0and "Delay between items (milliseconds)" to60000, save, and use a feed with at least 3 items the site has not imported yet. Click Run Now on the Import Posts list.Expect: the run stops early after roughly the 30-second delay budget (importing only the first item or two) and a resume cursor is stored:
Click Run Now again.
Expect: the import continues with the next unimported items instead of restarting from the first one, and once every item has been attempted the cursor meta is deleted.
Check before Pull Request is ready:
Related to feedzy-rss-feeds-pro#992. Companion Pro PR: Codeinwp/feedzy-rss-feeds-pro#997.