| @@ -2162,16 +2162,20 @@ | ||
| 2162 | 2162 | * auto_generate setting: the user deliberately changed inclusion rules and |
| 2163 | 2163 | * expects the served file to reflect them even if content-triggered |
| 2164 | 2164 | * auto-generation is turned off. Still respects the master `enabled` flag. |
| 2165 | 2165 | * |
| 2166 | - * @return void | |
| 2166 | + * @since 2.10.0 Reports whether the served sitemap was actually rebuilt, so | |
| 2167 | + * a caller can say so rather than assume it (#764). Existing | |
| 2168 | + * callers that ignore the return are unaffected. | |
| 2169 | + * | |
| 2170 | + * @return bool True when the served sitemap now reflects the settings. | |
| 2167 | 2171 | */ |
| 2168 | - public function regenerate_sitemap_from_settings(): void { | |
| 2172 | + public function regenerate_sitemap_from_settings(): bool { | |
| 2169 | 2173 | if (!$this->acquire_generation_lock()) { |
| 2170 | 2174 | // A manual generation (or another request's takeover) is already |
| 2171 | 2175 | // writing the files; the pending marker survives so this rebuild is |
| 2172 | 2176 | // retried rather than lost. |
| 2173 | - return; | |
| 2177 | + return false; | |
| 2174 | 2178 | } |
| 2175 | 2179 | |
| 2176 | 2180 | try { |
| 2177 | 2181 | $settings = $this->get_settings('site'); |
| @@ -2178,30 +2182,56 @@ | ||
| 2178 | 2182 | if (empty($settings['enabled'])) { |
| 2179 | 2183 | // The sitemap was disabled: remove the previously generated static |
| 2180 | 2184 | // files so the web server stops serving a stale sitemap that |
| 2181 | 2185 | // crawlers would otherwise keep fetching. |
| 2182 | - $this->delete_published_sitemaps(); | |
| 2186 | + // | |
| 2187 | + // A file that could not be removed is still being served, so | |
| 2188 | + // this is not a success. Reporting one here would tell a caller | |
| 2189 | + // the sitemap was gone while the web server kept answering with | |
| 2190 | + // it, which is the failure this return value exists to prevent | |
| 2191 | + // (#764). | |
| 2192 | + $removal = $this->delete_published_sitemaps($settings); | |
| 2193 | + $stuck = is_array($removal['failed'] ?? null) ? $removal['failed'] : []; | |
| 2194 | + | |
| 2195 | + if (!empty($stuck)) { | |
| 2196 | + $this->record_regeneration_failure( | |
| 2197 | + $this->stuck_files_message($stuck, true), | |
| 2198 | + 'settings' | |
| 2199 | + ); | |
| 2200 | + | |
| 2201 | + return false; | |
| 2202 | + } | |
| 2203 | + | |
| 2183 | 2204 | $this->mark_regeneration_complete(); |
| 2184 | - return; | |
| 2205 | + | |
| 2206 | + return true; | |
| 2185 | 2207 | } |
| 2186 | 2208 | |
| 2187 | 2209 | $revision = $this->current_regeneration_revision(); |
| 2188 | 2210 | |
| 2189 | 2211 | if ('dynamic' === $this->resolve_delivery_mode($settings)) { |
| 2190 | - $this->switch_to_dynamic_delivery($settings, $revision, 'settings'); | |
| 2191 | - return; | |
| 2212 | + // Returns false when a static file is stuck in the web root: | |
| 2213 | + // the server keeps serving that file in preference to WordPress, | |
| 2214 | + // so the switch has not taken effect (#764). | |
| 2215 | + return $this->switch_to_dynamic_delivery($settings, $revision, 'settings'); | |
| 2192 | 2216 | } |
| 2193 | 2217 | |
| 2194 | 2218 | if ($this->generate_and_save($settings)) { |
| 2195 | 2219 | $this->mark_regeneration_complete($revision); |
| 2196 | - } else { | |
| 2197 | - $this->record_regeneration_failure( | |
| 2198 | - $this->write_failure_message(), | |
| 2199 | - 'settings' | |
| 2200 | - ); | |
| 2220 | + | |
| 2221 | + return true; | |
| 2201 | 2222 | } |
| 2223 | + | |
| 2224 | + $this->record_regeneration_failure( | |
| 2225 | + $this->write_failure_message(), | |
| 2226 | + 'settings' | |
| 2227 | + ); | |
| 2228 | + | |
| 2229 | + return false; | |
| 2202 | 2230 | } catch (\Throwable $e) { |
| 2203 | 2231 | $this->record_regeneration_failure($e->getMessage(), 'settings'); |
| 2232 | + | |
| 2233 | + return false; | |
| 2204 | 2234 | } finally { |
| 2205 | 2235 | $this->release_generation_lock(); |
| 2206 | 2236 | } |
| 2207 | 2237 | } |
| @@ -2206,8 +2236,27 @@ | ||
| 2206 | 2236 | } |
| 2207 | 2237 | } |
| 2208 | 2238 | |
| 2209 | 2239 | /** |
| 2240 | + * When a rebuild has been outstanding since, or 0 when none is. | |
| 2241 | + * | |
| 2242 | + * Lets a caller report an honest "saved, but the served file has not caught | |
| 2243 | + * up yet" instead of a bare success (#764). | |
| 2244 | + * | |
| 2245 | + * @since 2.10.0 | |
| 2246 | + * @return int Unix timestamp, or 0 when nothing is pending. | |
| 2247 | + */ | |
| 2248 | + public static function regeneration_pending_since(): int { | |
| 2249 | + $pending = get_option(self::REGENERATION_PENDING_OPTION, []); | |
| 2250 | + | |
| 2251 | + if (!is_array($pending) || empty($pending['since'])) { | |
| 2252 | + return 0; | |
| 2253 | + } | |
| 2254 | + | |
| 2255 | + return (int) $pending['since']; | |
| 2256 | + } | |
| 2257 | + | |
| 2258 | + /** | |
| 2210 | 2259 | * Remove every static sitemap file ThinkRank publishes to the web root. |
| 2211 | 2260 | * |
| 2212 | 2261 | * Called when the sitemap feature is disabled, by the cleanup route, and by |
| 2213 | 2262 | * both removal paths, so /sitemap.xml, /sitemap_index.xml, the segmented |
| @@ -2444,9 +2493,9 @@ | ||
| 2444 | 2493 | * @param int $revision Revision this rebuild is completing. |
| 2445 | 2494 | * @param string $source 'settings' or 'content', for the failure record. |
| 2446 | 2495 | * @return void |
| 2447 | 2496 | */ |
| 2448 | - private function switch_to_dynamic_delivery(array $settings, int $revision, string $source): void { | |
| 2497 | + private function switch_to_dynamic_delivery(array $settings, int $revision, string $source): bool { | |
| 2449 | 2498 | $this->flush_dynamic_cache(); |
| 2450 | 2499 | |
| 2451 | 2500 | $removal = $this->delete_published_sitemaps($settings); |
| 2452 | 2501 | $stuck = is_array($removal['failed'] ?? null) ? $removal['failed'] : []; |
| @@ -2451,22 +2500,57 @@ | ||
| 2451 | 2500 | $removal = $this->delete_published_sitemaps($settings); |
| 2452 | 2501 | $stuck = is_array($removal['failed'] ?? null) ? $removal['failed'] : []; |
| 2453 | 2502 | |
| 2454 | 2503 | if (!empty($stuck)) { |
| 2455 | - $this->record_regeneration_failure( | |
| 2456 | - sprintf( | |
| 2457 | - /* translators: 1: comma-separated file names, 2: absolute path to the WordPress root. */ | |
| 2458 | - __('The sitemap is being served from WordPress, but these files are still in the site root and your web server will keep serving them instead: %1$s. They could not be removed because %2$s is not writable. Delete them, or ask your host to make the WordPress root writable.', 'thinkrank'), | |
| 2459 | - implode(', ', $stuck), | |
| 2460 | - untrailingslashit(ABSPATH) | |
| 2461 | - ), | |
| 2462 | - $source | |
| 2463 | - ); | |
| 2504 | + $this->record_regeneration_failure($this->stuck_files_message($stuck), $source); | |
| 2464 | 2505 | |
| 2465 | - return; | |
| 2506 | + return false; | |
| 2466 | 2507 | } |
| 2467 | 2508 | |
| 2468 | 2509 | $this->mark_regeneration_complete($revision); |
| 2510 | + | |
| 2511 | + return true; | |
| 2512 | + } | |
| 2513 | + | |
| 2514 | + /** | |
| 2515 | + * Why a stale file left in the web root means the change has not landed. | |
| 2516 | + * | |
| 2517 | + * Shared by every path that removes published files, so they cannot | |
| 2518 | + * describe the same situation differently (#764). | |
| 2519 | + * | |
| 2520 | + * The two situations that reach it differ in what WordPress is doing, and | |
| 2521 | + * the message has to say which. After a switch to dynamic delivery | |
| 2522 | + * WordPress IS serving the sitemap and the files shadow it. After the | |
| 2523 | + * sitemap is switched off WordPress serves nothing, so the one message | |
| 2524 | + * used to tell a site owner who had just disabled the sitemap that it was | |
| 2525 | + * "being served from WordPress", which is the opposite of what they did. | |
| 2526 | + * | |
| 2527 | + * @since 2.10.0 | |
| 2528 | + * @since 2.10.0 Public, so the REST endpoint uses it rather than a copy; | |
| 2529 | + * takes $sitemap_disabled for the disabled path. | |
| 2530 | + * | |
| 2531 | + * @param string[] $stuck Basenames that could not be removed. | |
| 2532 | + * @param bool $sitemap_disabled True when the files outlived disabling | |
| 2533 | + * the sitemap rather than a switch to | |
| 2534 | + * dynamic delivery. | |
| 2535 | + * @return string | |
| 2536 | + */ | |
| 2537 | + public function stuck_files_message(array $stuck, bool $sitemap_disabled = false): string { | |
| 2538 | + if ($sitemap_disabled) { | |
| 2539 | + return sprintf( | |
| 2540 | + /* translators: 1: comma-separated file names, 2: absolute path to the WordPress root. */ | |
| 2541 | + __('The sitemap is disabled, but these files are still in the site root and your web server is still serving them: %1$s. They could not be removed because %2$s is not writable. Delete them, or ask your host to make the WordPress root writable.', 'thinkrank'), | |
| 2542 | + implode(', ', $stuck), | |
| 2543 | + untrailingslashit(ABSPATH) | |
| 2544 | + ); | |
| 2545 | + } | |
| 2546 | + | |
| 2547 | + return sprintf( | |
| 2548 | + /* translators: 1: comma-separated file names, 2: absolute path to the WordPress root. */ | |
| 2549 | + __('The sitemap is being served from WordPress, but these files are still in the site root and your web server will keep serving them instead: %1$s. They could not be removed because %2$s is not writable. Delete them, or ask your host to make the WordPress root writable.', 'thinkrank'), | |
| 2550 | + implode(', ', $stuck), | |
| 2551 | + untrailingslashit(ABSPATH) | |
| 2552 | + ); | |
| 2469 | 2553 | } |
| 2470 | 2554 | |
| 2471 | 2555 | /** |
| 2472 | 2556 | * What to tell the site owner when publishing the files failed. |
