From e2352bc0871e511634b0c66a6179b250aca0f0e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:43:28 +0000 Subject: [PATCH 1/8] Percent-encode URLs, write hreflang codes, check more of each sitemap A URL built by hand could put a space, an accent or a control character in a file, which made it unreadable. Rails locales such as pt_BR went out as hreflang="pt_BR", which search engines ignore. A lastmod string was written whatever it held, and the index took the greatest string rather than the latest time. x_default was missed when the locales block returned strings, and two languages sharing a URL listed it twice. URLs are now percent-encoded outside RFC 3986's characters, locales are written with a hyphen, lastmods are read as W3C dates and written in UTC, and a page lists each of its URLs once. The check reports URLs over 2,048 characters, hreflang values that are not language codes, lastmods that are not dates, and an index with no file or more than 50,000. Assisted-by: Claude Code --- README.md | 20 ++++++++++++---- docs/guide.html | 15 +++++++++--- lib/mapitaine.rb | 2 ++ lib/mapitaine/builder.rb | 37 ++++++++++++++++++++++++++--- lib/mapitaine/page.rb | 12 +++++++--- lib/mapitaine/source.rb | 16 +++++++++++-- lib/mapitaine/tasks.rake | 2 +- lib/mapitaine/xml.rb | 34 +++++++++++++++++++++++---- test/build_test.rb | 50 +++++++++++++++++++++++++++++++++++++++- test/check_test.rb | 32 +++++++++++++++++++++++++ test/source_test.rb | 2 +- 11 files changed, 199 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 231e6b0..aae9588 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,10 @@ Then tell crawlers where the index is, in `public/robots.txt`: Sitemap: https://www.example.com/sitemap.xml ``` +When the sitemaps are served from elsewhere (`config.sitemaps_url`, see [Storage](#storage)), that line names +the index there: a sitemap on another host, or in a folder of the site, may list the pages of the site only +when its `robots.txt` points to it. + ## Sources A source is a class in `app/sitemaps`. Its blocks run in an object that has your route helpers, already set to @@ -94,7 +98,10 @@ end ``` A page in one language gets no `hreflang` link. A page that does not exist in the `x_default` language gets -no `x-default` link. +no `x-default` link. A Rails locale such as `pt_BR` is written `hreflang="pt-BR"`, and two languages that share +a URL list it once. + +A URL your block writes by hand is percent-encoded: spaces, accents and control characters cannot break a file. ### When the date depends on more than one column @@ -205,9 +212,14 @@ cannot be reached is a warning, never a failed build. bin/rails mapitaine:check ``` -It renders every sitemap without publishing or submitting anything, and fails on a URL outside `config.host`, -a URL listed twice in a source, or a `lastmod` in the future. A build reports the same, for the files it -rewrites, in `warnings`. +It renders every sitemap without publishing or submitting anything, and fails on: + +- a URL outside `config.host`, listed twice in a source, or longer than 2,048 characters; +- an `hreflang` that is not a language code (`en`, `pt-BR`, `zh-Hant`, `es-419`); +- a `lastmod` in the future, or one that is not a W3C date, which is then left out; +- an index with no file, or with more than the 50,000 files an index may hold. + +A build reports the same, for the files it rewrites, in `warnings`. ## Instrumentation diff --git a/docs/guide.html b/docs/guide.html index fe0f027..db3c9c3 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -61,6 +61,7 @@

Installation

end

Then tell crawlers where the index is, in public/robots.txt:

Sitemap: https://www.example.com/sitemap.xml
+

When the sitemaps are served from elsewhere (config.sitemaps_url, see Storage), that line names the index there: a sitemap on another host, or in a folder of the site, may list the pages of the site only when its robots.txt points to it.

@@ -98,7 +99,8 @@

Languages

url { |product, locale| product_url(product, locale:) } locales { |product| product.translated_locales } end -

Each <url> then lists every language of its page, itself included, as hreflang links. A page in one language gets no link. A page that does not exist in the x_default language gets no x-default link.

+

Each <url> then lists every language of its page, itself included, as hreflang links. A page in one language gets no link. A page that does not exist in the x_default language gets no x-default link. A Rails locale such as pt_BR is written hreflang="pt-BR", and two languages that share a URL list it once.

+

A URL your block writes by hand is percent-encoded: spaces, accents and control characters cannot break a file.

@@ -163,7 +165,7 @@

Through Active Storage

config.sitemaps_url = "https://cdn.example.com/sitemaps" # where those files are served from end

No blob is created: the files keep their names as keys, and nothing is written to the database. Any service works that serves a key at a stable URL: S3, Google Cloud Storage, Azure, a mirror of them. The Disk service does not, it scatters keys in subfolders: keep the default for files on disk.

-

Sitemaps served from another host than the pages they list are accepted when robots.txt names the index.

+

Sitemaps served from another host than the pages they list are accepted when the site's robots.txt names the index there.

Your own

A storage is any object with read(name), write(name, path) and delete(name).

@@ -183,7 +185,14 @@

IndexNow

Checking

bin/rails mapitaine:check
-

It renders every sitemap without publishing or submitting anything, and fails on a URL outside config.host, a URL listed twice in a source, or a lastmod in the future. A build reports the same in warnings, for the files it rewrites.

+

It renders every sitemap without publishing or submitting anything, and fails on:

+ +

A build reports the same in warnings, for the files it rewrites.

Mapitaine never requests a page of the site. Whether a URL answers 200 or carries a noindex is what Search Console reports, from the visits crawlers make anyway.

diff --git a/lib/mapitaine.rb b/lib/mapitaine.rb index 696b9a5..fec6bda 100644 --- a/lib/mapitaine.rb +++ b/lib/mapitaine.rb @@ -4,10 +4,12 @@ require "active_support/core_ext/object/blank" require "active_support/core_ext/string/inflections" require "active_support/notifications" +require "date" require "digest" require "fileutils" require "json" require "net/http" +require "time" require "tmpdir" require "uri" require "zlib" diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index 4b66dea..75d0e24 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -8,6 +8,12 @@ def changed? end end + # https://www.sitemaps.org/protocol.html + MAX_URL_LENGTH = 2_048 + MAX_FILES_PER_INDEX = 50_000 + # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. + HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i + Warning = Data.define(:source, :url, :message) do def to_s "#{source}: #{url} #{message}" @@ -37,6 +43,7 @@ def build_in(directory, full:) @kept = [] @warnings = [] @changed_urls = [] + @checked_hreflangs = Set.new @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } publish_index @@ -96,10 +103,12 @@ def publish(source, shard, fingerprint, files) end def publish_index + name = "#{@configuration.filename}.xml" entries = @manifest.files.map do |file| Xml.sitemap_entry("#{@configuration.sitemaps_url}/#{file["name"]}", file["lastmod"]) end - upload("#{@configuration.filename}.xml", "#{Xml::INDEX_OPEN}#{entries.join}#{Xml::INDEX_CLOSE}") + inspect_index("#{@configuration.sitemaps_url}/#{name}", entries.size) + upload(name, "#{Xml::INDEX_OPEN}#{entries.join}#{Xml::INDEX_CLOSE}") end def upload(name, content) @@ -116,13 +125,35 @@ def inspect_page(source, page, seen_urls) page.urls.each do |url| warn(source, url, "is outside #{@configuration.host!}") unless url.start_with?("#{@configuration.host!}/") || url == @configuration.host! warn(source, url, "is listed twice") unless seen_urls.add?(url) + warn(source, url, "is longer than #{MAX_URL_LENGTH} characters") if url.length > MAX_URL_LENGTH + end + page.url_by_locale.each do |hreflang, url| + warn(source, url, "has hreflang #{hreflang.inspect}, not a language code") if @checked_hreflangs.add?(hreflang) && !hreflang.match?(HREFLANG) end + inspect_lastmod(source, page) + end + + def inspect_lastmod(source, page) + return if page.lastmod.nil? + modified_at = page.modified_at - warn(source, page.urls.first, "has a lastmod in the future (#{Xml.timestamp(page.lastmod)})") if modified_at && modified_at > @manifest.built_at + if modified_at.nil? + warn(source, page.urls.first, "has a lastmod that is not a W3C date (#{page.lastmod.inspect}), left out") + elsif modified_at > @manifest.built_at + warn(source, page.urls.first, "has a lastmod in the future (#{Xml.timestamp(page.lastmod)})") + end + end + + def inspect_index(url, file_count) + if file_count.zero? + warn("index", url, "lists no file: no source has a page") + elsif file_count > MAX_FILES_PER_INDEX + warn("index", url, "lists #{file_count} files, more than #{MAX_FILES_PER_INDEX}: raise shard_size") + end end def warn(source, url, message) - @warnings << Warning.new(source: source.slug, url:, message:) + @warnings << Warning.new(source: source.is_a?(Source) ? source.slug : source, url:, message:) end # The first build has no "before": submitting a whole site at once is not what IndexNow is for. diff --git a/lib/mapitaine/page.rb b/lib/mapitaine/page.rb index 3e9962c..2d987d6 100644 --- a/lib/mapitaine/page.rb +++ b/lib/mapitaine/page.rb @@ -1,14 +1,20 @@ # frozen_string_literal: true module Mapitaine - # One page of the site: the same content at one URL per language. + # One page of the site: the same content at one URL per language, keyed by hreflang. Page = Data.define(:url_by_locale, :lastmod, :images) do + # Two languages may share a URL; the file lists it once. def urls - url_by_locale.values + url_by_locale.values.uniq end def modified_at - lastmod.to_time if lastmod.respond_to?(:to_time) + return lastmod.getutc if lastmod.respond_to?(:getutc) && !lastmod.is_a?(Date) + + stamp = Xml.timestamp(lastmod) + return unless stamp + + stamp.include?("T") ? Time.iso8601(stamp) : Time.utc(*stamp.split("-").map(&:to_i)) end end end diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index 75dd4b2..b432506 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -16,6 +16,10 @@ class Source SLUG_FORMAT = /\A[a-z0-9][a-z0-9_-]*\z/ # A source that is not an Active Record relation is one shard, rendered at each build. WHOLE = "all" + # What RFC 3986 lets a URL hold as is. Anything else is percent-encoded, byte by byte. + URL_CHARACTERS = "A-Za-z0-9\\-._~:/?#\\[\\]@!$&'()*+,;=%" + UNSAFE_CHARACTER = /[^#{URL_CHARACTERS}]/ + UNSAFE_BYTE = Regexp.new("[^#{URL_CHARACTERS}]", Regexp::NOENCODING) class << self def inherited(subclass) @@ -144,13 +148,21 @@ def page_for(record) locales = definitions[:locales] ? Array(instance_exec(record, &definitions[:locales])) : configuration.locales url_by_locale = locales.filter_map do |locale| location = instance_exec(record, locale, &definitions.fetch(:url)) - [locale, absolute(location)] if location + [Xml.hreflang(locale), absolute(location)] if location end Page.new(url_by_locale: url_by_locale.to_h, lastmod: lastmod_of(record), images: images_of(record)) end def absolute(location) - location.start_with?("/") ? "#{configuration.host!}#{location}" : location + location = location.to_s + encoded(location.start_with?("/") ? "#{configuration.host!}#{location}" : location) + end + + # Route helpers encode their URLs already; a URL written by hand may hold spaces or accents. + def encoded(url) + return url if url.ascii_only? && !url.match?(UNSAFE_CHARACTER) + + url.b.gsub(UNSAFE_BYTE) { |byte| format("%%%02X", byte.ord) }.force_encoding(Encoding::UTF_8) end def lastmod_of(record) diff --git a/lib/mapitaine/tasks.rake b/lib/mapitaine/tasks.rake index 4714222..1fe25cf 100644 --- a/lib/mapitaine/tasks.rake +++ b/lib/mapitaine/tasks.rake @@ -17,7 +17,7 @@ namespace :mapitaine do report.call(Mapitaine.build(full: true)) end - desc "Render every sitemap without publishing it, and fail on duplicate, foreign or future-dated URLs" + desc "Render every sitemap without publishing it, and fail on what looks wrong in it" task check: :environment do warnings = Mapitaine.check abort warnings.join("\n") if warnings.any? diff --git a/lib/mapitaine/xml.rb b/lib/mapitaine/xml.rb index a84b442..d71d3c9 100644 --- a/lib/mapitaine/xml.rb +++ b/lib/mapitaine/xml.rb @@ -11,13 +11,16 @@ module Xml INDEX_CLOSE = "\n" # Google reads no more than 1,000 images per URL. MAX_IMAGES_PER_URL = 1_000 + # https://www.w3.org/TR/NOTE-datetime: a year, a month, a day, or a time with its zone. + W3C_DATE = /\A\d{4}(-\d{2}(-\d{2})?)?\z/ + W3C_TIME = /\A(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})(?::(\d{2})(?:\.\d+)?)?(Z|[+-]\d{2}:\d{2})\z/ module_function # One per language. Each lists every language of the page, itself included: search engines drop # hreflang links that are not returned by the page they point to. def url_entries(page, x_default:) - shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, x_default)}#{images(page.images)}\n" + shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, hreflang(x_default))}#{images(page.images)}\n" page.urls.map { |url| "#{escape(url)}#{shared}" } end @@ -25,11 +28,31 @@ def sitemap_entry(url, lastmod) "#{escape(url)}#{lastmod_tag(lastmod)}\n" end + # The W3C date or time of a lastmod, in UTC, or nil when it is not one. def timestamp(value) - return if value.nil? - return value.utc.iso8601 if value.respond_to?(:utc) + case value + when nil then nil + when DateTime then value.to_time.getutc.iso8601 + when Date then value.iso8601 + when String then w3c_timestamp(value) + else value.getutc.iso8601 if value.respond_to?(:getutc) + end + end + + def w3c_timestamp(text) + return text if text.match?(W3C_DATE) + + year, month, day, hour, minute, second, zone = W3C_TIME.match(text)&.captures + return unless year + + Time.new(year.to_i, month.to_i, day.to_i, hour.to_i, minute.to_i, second.to_i, (zone == "Z") ? "UTC" : zone).getutc.iso8601 + rescue ArgumentError + nil + end - value.respond_to?(:iso8601) ? value.iso8601 : value.to_s + # Rails locales spell regions with an underscore (pt_BR), hreflang with a hyphen (pt-BR). + def hreflang(locale) + locale.to_s.tr("_", "-") end def escape(text) @@ -45,7 +68,8 @@ def alternates(page, x_default) return "" if page.url_by_locale.size < 2 links = page.url_by_locale.map { |locale, url| alternate(locale, url) } - links << alternate("x-default", page.url_by_locale[x_default]) if page.url_by_locale.key?(x_default) + default_url = page.url_by_locale[x_default] + links << alternate("x-default", default_url) if default_url links.join end diff --git a/test/build_test.rb b/test/build_test.rb index 4d1e7a9..03c24ad 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -168,11 +168,59 @@ def test_special_characters_are_escaped_and_images_listed build xml = storage.xml("sitemap-post-00000.xml.gz") - assert_includes xml, "https://example.com/en/posts?title=Tom & "Jerry" <3" + assert_includes xml, "https://example.com/en/posts?title=Tom%20&%20%22Jerry%22%20%3C3" assert_includes xml, "https://example.com/images/1.jpg" \ "https://cdn.example.com/1.jpg?a=1&b=2" end + def test_a_url_written_by_hand_is_percent_encoded + create_post(1) + by_hand = Class.new(PostSitemap) { url { |_post, locale| "/#{locale}/Café\u0001 d" + "\xE9j\xE0?q=100%25" } } + Mapitaine.configuration.sources = [by_hand] + build + + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "https://example.com/en/Caf%C3%A9%01%20d%E9j%E0?q=100%25" + end + + def test_rails_locales_become_hreflang_codes_and_x_default_is_found_whatever_its_type + create_post(1, languages: "en,pt_BR") + strings = Class.new(PostSitemap) { locales { |post| post.languages.split(",") } } + Mapitaine.configuration.sources = [strings] + Mapitaine.configuration.x_default = :en + build + xml = storage.xml("sitemap-post-00000.xml.gz") + + assert_includes xml, %(hreflang="pt-BR" href="https://example.com/pt_BR/posts/1") + assert_includes xml, %(hreflang="x-default" href="https://example.com/en/posts/1") + end + + def test_two_languages_sharing_a_url_list_it_once + create_post(1, languages: "en,fr,de") + shared = Class.new(PostSitemap) { url { |post, locale| post_url(post, locale: (locale == :de) ? :en : locale) } } + Mapitaine.configuration.sources = [shared] + built = build + xml = storage.xml("sitemap-post-00000.xml.gz") + + assert_equal 2, built.url_count + assert_equal 2, xml.scan("").size + assert_includes xml, %(hreflang="de" href="https://example.com/en/posts/1") + end + + def test_lastmods_are_written_in_utc_and_the_index_takes_the_latest + create_post(1) + create_post(2) + tokyo = Time.new(2026, 1, 15, 10, 0, 0, "+09:00") + stamps = {1 => "2026-01-15T10:00+09:00", 2 => "2026-01-15T05:00:00Z"} + zoned = Class.new(PostSitemap) { lastmod { |post| stamps.fetch(post.id) } } + Mapitaine.configuration.sources = [zoned, Class.new(PageSitemap) { lastmod { tokyo } }] + build + + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "2026-01-15T01:00:00Z" + assert_includes storage.xml("sitemap.xml"), "sitemap-post-00000.xml.gz2026-01-15T05:00:00Z" + assert_includes storage.xml("sitemap-page.xml.gz"), "2026-01-15T01:00:00Z" + assert_equal 9 * 3600, tokyo.utc_offset + end + def test_a_lastmod_block_is_fingerprinted_by_the_expression_it_names create_post(1) reviewed = Class.new(PostSitemap) do diff --git a/test/check_test.rb b/test/check_test.rb index 4ba05f4..fb9433f 100644 --- a/test/check_test.rb +++ b/test/check_test.rb @@ -13,6 +13,7 @@ def test_a_sound_site_has_nothing_to_report_and_nothing_is_written def test_a_url_on_another_host_a_duplicate_and_a_future_lastmod_are_reported create_post(1, updated_at: Time.utc(2999, 1, 1)) create_post(12) + create_post(13, languages: "en") wrong = Class.new(PostSitemap) { url { |post, locale| (post.id == 1) ? "https://example.org/#{locale}" : "/same" } } Mapitaine.configuration.sources = [wrong] @@ -24,6 +25,37 @@ def test_a_url_on_another_host_a_duplicate_and_a_future_lastmod_are_reported ], Mapitaine.check.map(&:to_s) end + def test_a_long_url_a_wrong_hreflang_and_a_lastmod_that_is_not_a_date_are_reported + create_post(1, title: "a" * 2_100) + wrong = Class.new(PostSitemap) do + url { |post, locale| "/#{locale}/#{post.title}" } + locales { %i[en english] } + lastmod { "yesterday" } + end + Mapitaine.configuration.sources = [wrong] + + assert_equal [ + "post: https://example.com/en/#{"a" * 2_100} is longer than 2048 characters", + "post: https://example.com/english/#{"a" * 2_100} is longer than 2048 characters", + %(post: https://example.com/english/#{"a" * 2_100} has hreflang "english", not a language code), + %(post: https://example.com/en/#{"a" * 2_100} has a lastmod that is not a W3C date ("yesterday"), left out) + ], Mapitaine.check.map(&:to_s) + end + + def test_an_empty_index_is_reported + Mapitaine.configuration.sources = [PostSitemap] + + assert_equal ["index: https://example.com/sitemap.xml lists no file: no source has a page"], Mapitaine.check.map(&:to_s) + end + + def test_one_wrong_hreflang_is_reported_once + create_post(1, languages: "en_GB,zz_top") + create_post(2, languages: "en_GB,zz_top") + Mapitaine.configuration.sources = [PostSitemap] + + assert_equal [%(post: https://example.com/zz_top/posts/1 has hreflang "zz-top", not a language code)], Mapitaine.check.map(&:to_s) + end + def test_a_build_reports_what_it_renders create_post(1, updated_at: Time.utc(2999, 1, 1)) diff --git a/test/source_test.rb b/test/source_test.rb index 0c0b7bd..7a6d215 100644 --- a/test/source_test.rb +++ b/test/source_test.rb @@ -44,7 +44,7 @@ def test_a_record_without_a_url_in_a_language_is_left_out_of_it english_only = Class.new(PostSitemap) { url { |post, locale| post_url(post, locale:) if locale == :en } } pages = english_only.new(Mapitaine.configuration).each_page("0").to_a - assert_equal [{en: "https://example.com/en/posts/1"}], pages.map(&:url_by_locale) + assert_equal [{"en" => "https://example.com/en/posts/1"}], pages.map(&:url_by_locale) end def test_route_helpers_use_the_configured_host_port_and_scheme From 669c7178976b99a76e1d9d0558af314aa60d6057 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:47:46 +0000 Subject: [PATCH 2/8] Validate dates, catch locale-blind URLs, encode lone % and // URLs An impossible date such as 2026-13-01 crashed the build, and 2026-02-30 was written out. A date alone counted from UTC midnight, so today's date looked like the future east of UTC. A url block that forgot the locale no longer warned once its URLs were listed once per page. A protocol-relative URL got the host in front, and a % not followed by two hex digits stayed. Dates are validated, a date alone starts at UTC+14, a page whose languages share one URL is reported once per source, as is a lastmod that is not a date, and the lastmod is read once per page. An empty site no longer fails the check. Assisted-by: Claude Code --- README.md | 5 +++-- docs/guide.html | 4 ++-- lib/mapitaine/builder.rb | 38 +++++++++++++++++++++----------------- lib/mapitaine/page.rb | 15 +++------------ lib/mapitaine/source.rb | 17 +++++++++++++---- lib/mapitaine/xml.rb | 28 +++++++++++++++++++--------- test/build_test.rb | 13 ++++++++++--- test/check_test.rb | 27 ++++++++++++++++++++++++--- 8 files changed, 95 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index aae9588..d531d4f 100644 --- a/README.md +++ b/README.md @@ -214,10 +214,11 @@ bin/rails mapitaine:check It renders every sitemap without publishing or submitting anything, and fails on: -- a URL outside `config.host`, listed twice in a source, or longer than 2,048 characters; +- a URL outside `config.host`, listed twice in a source, or longer than 2,048 characters, and a page whose + languages all share one URL; - an `hreflang` that is not a language code (`en`, `pt-BR`, `zh-Hant`, `es-419`); - a `lastmod` in the future, or one that is not a W3C date, which is then left out; -- an index with no file, or with more than the 50,000 files an index may hold. +- an index with more than the 50,000 files it may hold. A build reports the same, for the files it rewrites, in `warnings`. diff --git a/docs/guide.html b/docs/guide.html index db3c9c3..b471b88 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -187,10 +187,10 @@

Checking

bin/rails mapitaine:check

It renders every sitemap without publishing or submitting anything, and fails on:

A build reports the same in warnings, for the files it rewrites.

Mapitaine never requests a page of the site. Whether a URL answers 200 or carries a noindex is what Search Console reports, from the visits crawlers make anyway.

diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index 75d0e24..8437549 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -9,7 +9,7 @@ def changed? end # https://www.sitemaps.org/protocol.html - MAX_URL_LENGTH = 2_048 + MAX_URL_CHARACTERS = 2_048 MAX_FILES_PER_INDEX = 50_000 # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i @@ -44,6 +44,8 @@ def build_in(directory, full:) @warnings = [] @changed_urls = [] @checked_hreflangs = Set.new + @reported = Set.new + @x_default = Xml.hreflang(@configuration.x_default) @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } publish_index @@ -78,7 +80,7 @@ def render(source, shard, fingerprint, stored, seen_urls) ActiveSupport::Notifications.instrument("shard.mapitaine", source: source.slug, shard:) do source.each_page(shard) do |page| inspect_page(source, page, seen_urls) - writer.add(Xml.url_entries(page, x_default: @configuration.x_default), Xml.timestamp(page.lastmod)) + writer.add(Xml.url_entries(page, x_default: @x_default), page.lastmod) changed_urls.concat(page.urls) if changed_since_last_build?(page) end writer.close @@ -125,31 +127,34 @@ def inspect_page(source, page, seen_urls) page.urls.each do |url| warn(source, url, "is outside #{@configuration.host!}") unless url.start_with?("#{@configuration.host!}/") || url == @configuration.host! warn(source, url, "is listed twice") unless seen_urls.add?(url) - warn(source, url, "is longer than #{MAX_URL_LENGTH} characters") if url.length > MAX_URL_LENGTH + warn(source, url, "is longer than #{MAX_URL_CHARACTERS} characters") if url.length > MAX_URL_CHARACTERS end page.url_by_locale.each do |hreflang, url| warn(source, url, "has hreflang #{hreflang.inspect}, not a language code") if @checked_hreflangs.add?(hreflang) && !hreflang.match?(HREFLANG) end + if page.url_by_locale.size > 1 && page.urls.one? + warn_once(source, :one_url, page.urls.first, "is the URL of every language of its page: does the url block use the locale?") + end inspect_lastmod(source, page) end def inspect_lastmod(source, page) - return if page.lastmod.nil? - - modified_at = page.modified_at - if modified_at.nil? - warn(source, page.urls.first, "has a lastmod that is not a W3C date (#{page.lastmod.inspect}), left out") - elsif modified_at > @manifest.built_at - warn(source, page.urls.first, "has a lastmod in the future (#{Xml.timestamp(page.lastmod)})") + if page.rejected_lastmod + warn_once(source, :rejected_lastmod, page.urls.first, "has a lastmod that is not a W3C date (#{page.rejected_lastmod.inspect}), left out") + elsif page.modified_at && page.modified_at > @manifest.built_at + warn(source, page.urls.first, "has a lastmod in the future (#{page.lastmod})") end end def inspect_index(url, file_count) - if file_count.zero? - warn("index", url, "lists no file: no source has a page") - elsif file_count > MAX_FILES_PER_INDEX - warn("index", url, "lists #{file_count} files, more than #{MAX_FILES_PER_INDEX}: raise shard_size") - end + return if file_count <= MAX_FILES_PER_INDEX + + warn("index", url, "lists #{file_count} files, more than #{MAX_FILES_PER_INDEX}: raise shard_size") + end + + # For a mistake every page of a source would repeat. + def warn_once(source, kind, url, message) + warn(source, url, message) if @reported.add?([source.slug, kind]) end def warn(source, url, message) @@ -158,8 +163,7 @@ def warn(source, url, message) # The first build has no "before": submitting a whole site at once is not what IndexNow is for. def changed_since_last_build?(page) - modified_at = page.modified_at - @previous.built_at && modified_at && modified_at >= @previous.built_at + @previous.built_at && page.modified_at && page.modified_at >= @previous.built_at end def submit_changed_urls diff --git a/lib/mapitaine/page.rb b/lib/mapitaine/page.rb index 2d987d6..ab62e43 100644 --- a/lib/mapitaine/page.rb +++ b/lib/mapitaine/page.rb @@ -1,20 +1,11 @@ # frozen_string_literal: true module Mapitaine - # One page of the site: the same content at one URL per language, keyed by hreflang. - Page = Data.define(:url_by_locale, :lastmod, :images) do - # Two languages may share a URL; the file lists it once. + # One page of the site: the same content at one URL per language, keyed by hreflang. lastmod is a W3C date; + # rejected_lastmod what a source gave that was not one. + Page = Data.define(:url_by_locale, :lastmod, :modified_at, :images, :rejected_lastmod) do def urls url_by_locale.values.uniq end - - def modified_at - return lastmod.getutc if lastmod.respond_to?(:getutc) && !lastmod.is_a?(Date) - - stamp = Xml.timestamp(lastmod) - return unless stamp - - stamp.include?("T") ? Time.iso8601(stamp) : Time.utc(*stamp.split("-").map(&:to_i)) - end end end diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index b432506..87c3365 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -18,8 +18,9 @@ class Source WHOLE = "all" # What RFC 3986 lets a URL hold as is. Anything else is percent-encoded, byte by byte. URL_CHARACTERS = "A-Za-z0-9\\-._~:/?#\\[\\]@!$&'()*+,;=%" - UNSAFE_CHARACTER = /[^#{URL_CHARACTERS}]/ - UNSAFE_BYTE = Regexp.new("[^#{URL_CHARACTERS}]", Regexp::NOENCODING) + UNSAFE = "[^#{URL_CHARACTERS}]|%(?![0-9A-Fa-f]{2})" + UNSAFE_CHARACTER = Regexp.new(UNSAFE) + UNSAFE_BYTE = Regexp.new(UNSAFE, Regexp::NOENCODING) class << self def inherited(subclass) @@ -150,12 +151,20 @@ def page_for(record) location = instance_exec(record, locale, &definitions.fetch(:url)) [Xml.hreflang(locale), absolute(location)] if location end - Page.new(url_by_locale: url_by_locale.to_h, lastmod: lastmod_of(record), images: images_of(record)) + given_lastmod = lastmod_of(record) + lastmod = Xml.timestamp(given_lastmod) + Page.new(url_by_locale: url_by_locale.to_h, lastmod:, modified_at: lastmod && Xml.time(lastmod), images: images_of(record), + rejected_lastmod: (given_lastmod if lastmod.nil? && given_lastmod.present?)) end def absolute(location) location = location.to_s - encoded(location.start_with?("/") ? "#{configuration.host!}#{location}" : location) + if location.start_with?("//") + location = "#{configuration.host!.split("//", 2).first}#{location}" + elsif location.start_with?("/") + location = "#{configuration.host!}#{location}" + end + encoded(location) end # Route helpers encode their URLs already; a URL written by hand may hold spaces or accents. diff --git a/lib/mapitaine/xml.rb b/lib/mapitaine/xml.rb index d71d3c9..0dc52d3 100644 --- a/lib/mapitaine/xml.rb +++ b/lib/mapitaine/xml.rb @@ -12,7 +12,7 @@ module Xml # Google reads no more than 1,000 images per URL. MAX_IMAGES_PER_URL = 1_000 # https://www.w3.org/TR/NOTE-datetime: a year, a month, a day, or a time with its zone. - W3C_DATE = /\A\d{4}(-\d{2}(-\d{2})?)?\z/ + W3C_DATE = /\A(\d{4})(?:-(\d{2})(?:-(\d{2}))?)?\z/ W3C_TIME = /\A(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})(?::(\d{2})(?:\.\d+)?)?(Z|[+-]\d{2}:\d{2})\z/ module_function @@ -20,7 +20,7 @@ module Xml # One per language. Each lists every language of the page, itself included: search engines drop # hreflang links that are not returned by the page they point to. def url_entries(page, x_default:) - shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, hreflang(x_default))}#{images(page.images)}\n" + shared = "#{lastmod_tag(page.lastmod)}#{alternates(page, x_default)}#{images(page.images)}\n" page.urls.map { |url| "#{escape(url)}#{shared}" } end @@ -40,16 +40,27 @@ def timestamp(value) end def w3c_timestamp(text) - return text if text.match?(W3C_DATE) + if (date = W3C_DATE.match(text)) + year, month, day = date.captures + text if Date.valid_date?(year.to_i, (month || 1).to_i, (day || 1).to_i) + elsif (time = W3C_TIME.match(text)) + year, month, day, hour, minute, second, zone = time.captures + return unless Date.valid_date?(year.to_i, month.to_i, day.to_i) - year, month, day, hour, minute, second, zone = W3C_TIME.match(text)&.captures - return unless year - - Time.new(year.to_i, month.to_i, day.to_i, hour.to_i, minute.to_i, second.to_i, (zone == "Z") ? "UTC" : zone).getutc.iso8601 + Time.new(year.to_i, month.to_i, day.to_i, hour.to_i, minute.to_i, second.to_i, (zone == "Z") ? "UTC" : zone).getutc.iso8601 + end rescue ArgumentError nil end + # A date alone is the day as it starts first, at UTC+14: no time zone has it in the future after that. + def time(stamp) + return Time.iso8601(stamp) if stamp.include?("T") + + year, month, day = stamp.split("-").map(&:to_i) + Time.new(year, month || 1, day || 1, 0, 0, 0, "+14:00") + end + # Rails locales spell regions with an underscore (pt_BR), hreflang with a hyphen (pt-BR). def hreflang(locale) locale.to_s.tr("_", "-") @@ -59,8 +70,7 @@ def escape(text) text.to_s.encode(xml: :text).gsub('"', """) end - def lastmod_tag(value) - stamp = timestamp(value) + def lastmod_tag(stamp) stamp ? "#{escape(stamp)}" : "" end diff --git a/test/build_test.rb b/test/build_test.rb index 03c24ad..ea4f82f 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -175,11 +175,16 @@ def test_special_characters_are_escaped_and_images_listed def test_a_url_written_by_hand_is_percent_encoded create_post(1) - by_hand = Class.new(PostSitemap) { url { |_post, locale| "/#{locale}/Café\u0001 d" + "\xE9j\xE0?q=100%25" } } + by_hand = Class.new(PostSitemap) do + url { |_post, locale| "/#{locale}/Café\u0001 d" + "\xE9j\xE0?q=100%25&off=100%" } + images { ["//cdn.example.com/a b.jpg"] } + end Mapitaine.configuration.sources = [by_hand] build + xml = storage.xml("sitemap-post-00000.xml.gz") - assert_includes storage.xml("sitemap-post-00000.xml.gz"), "https://example.com/en/Caf%C3%A9%01%20d%E9j%E0?q=100%25" + assert_includes xml, "https://example.com/en/Caf%C3%A9%01%20d%E9j%E0?q=100%25&off=100%25" + assert_includes xml, "https://cdn.example.com/a%20b.jpg" end def test_rails_locales_become_hreflang_codes_and_x_default_is_found_whatever_its_type @@ -209,13 +214,15 @@ def test_two_languages_sharing_a_url_list_it_once def test_lastmods_are_written_in_utc_and_the_index_takes_the_latest create_post(1) create_post(2) + create_post(3) tokyo = Time.new(2026, 1, 15, 10, 0, 0, "+09:00") - stamps = {1 => "2026-01-15T10:00+09:00", 2 => "2026-01-15T05:00:00Z"} + stamps = {1 => "2026-01-15T10:00+09:00", 2 => "2026-01-15T05:00:00Z", 3 => "2026-02-30T05:00:00Z"} zoned = Class.new(PostSitemap) { lastmod { |post| stamps.fetch(post.id) } } Mapitaine.configuration.sources = [zoned, Class.new(PageSitemap) { lastmod { tokyo } }] build assert_includes storage.xml("sitemap-post-00000.xml.gz"), "2026-01-15T01:00:00Z" + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "https://example.com/en/posts/32026-01-15T05:00:00Z" assert_includes storage.xml("sitemap-page.xml.gz"), "2026-01-15T01:00:00Z" assert_equal 9 * 3600, tokyo.utc_offset diff --git a/test/check_test.rb b/test/check_test.rb index fb9433f..a4ae9c6 100644 --- a/test/check_test.rb +++ b/test/check_test.rb @@ -21,6 +21,7 @@ def test_a_url_on_another_host_a_duplicate_and_a_future_lastmod_are_reported "post: https://example.org/en is outside https://example.com", "post: https://example.org/fr is outside https://example.com", "post: https://example.org/en has a lastmod in the future (2999-01-01T00:00:00Z)", + "post: https://example.com/same is the URL of every language of its page: does the url block use the locale?", "post: https://example.com/same is listed twice" ], Mapitaine.check.map(&:to_s) end @@ -42,10 +43,30 @@ def test_a_long_url_a_wrong_hreflang_and_a_lastmod_that_is_not_a_date_are_report ], Mapitaine.check.map(&:to_s) end - def test_an_empty_index_is_reported - Mapitaine.configuration.sources = [PostSitemap] + def test_mistakes_every_page_repeats_are_reported_once_and_impossible_dates_left_out + create_post(1) + create_post(2) + careless = Class.new(PostSitemap) do + url { |post| "/posts/#{post.id}" } + lastmod { "2026-13-01" } + end + Mapitaine.configuration.sources = [careless] + + assert_equal [ + "post: https://example.com/posts/1 is the URL of every language of its page: does the url block use the locale?", + %(post: https://example.com/posts/1 has a lastmod that is not a W3C date ("2026-13-01"), left out) + ], Mapitaine.check.map(&:to_s) + end + + def test_a_date_is_in_the_future_only_once_it_has_started_nowhere + dated = ->(date) { Class.new(PageSitemap) { lastmod { date } } } + first_date_on_earth = Time.now.getlocal("+14:00").to_date + Mapitaine.configuration.sources = [dated.call(first_date_on_earth)] + + assert_empty Mapitaine.check + Mapitaine.configuration.sources = [dated.call(first_date_on_earth + 1)] - assert_equal ["index: https://example.com/sitemap.xml lists no file: no source has a page"], Mapitaine.check.map(&:to_s) + assert_equal 1, Mapitaine.check.size end def test_one_wrong_hreflang_is_reported_once From a2a9e488f518473db78721db69284a910b522d05 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:53:32 +0000 Subject: [PATCH 3/8] Catch more changes between builds, and date files by their last change A record deleted and another inserted with an older timestamp left a shard's count and latest change alone, so its file kept the deleted page. A new shard size kept files cut for the old ranges. update_columns, a change in another table or a new url block went unseen until someone ran a rebuild. In the index, a file that lost a record kept its date or went back in time, and a build that died after uploading a new file left it there for good. The fingerprint adds the sum of the ids and the shard size. A file that comes out the same is not uploaded and keeps its date; one that changed without a later lastmod takes the build's time. A full build comes every week by itself (config.rebuild_every). The names a build is about to add are recorded in the manifest first, so the next build can remove them. Assisted-by: Claude Code --- AGENTS.md | 10 +-- README.md | 29 +++++++-- docs/guide.html | 8 ++- lib/mapitaine/builder.rb | 81 +++++++++++++++++++----- lib/mapitaine/configuration.rb | 7 ++- lib/mapitaine/manifest.rb | 21 +++++-- lib/mapitaine/shard_writer.rb | 13 ++-- lib/mapitaine/source.rb | 7 ++- test/build_test.rb | 110 ++++++++++++++++++++++++++++++++- 9 files changed, 242 insertions(+), 44 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dd8ce6d..46b1f4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,10 +25,12 @@ joins CI's `rails` matrix. storage goes through Active Storage only: no SDK of our own. - Mapitaine never requests a page of the site, and never hooks into models: a build reads the database and the manifest of the last build, nothing else. -- A file name is a promise: the same ids land in the same file at every build. Anything that moves them (the shard - size, the slug, the file name) belongs in `Configuration#digest` or rewrites everything by itself. -- A build publishes the sitemap files, then the index, then deletes, then writes the manifest. Keep that order: it - is what lets the next build repair one that died midway. +- A file name is a promise: the same ids land in the same file at every build. Anything that moves them belongs in + `Configuration#digest` (the file name), in each shard's fingerprint (the shard size), or rewrites everything by + itself (the slug). +- A build renders every shard, records the names it is about to add as `pending` in the manifest, publishes the + sitemap files, then the index, then deletes, then writes the manifest. Keep that order: it is what lets the next + build repair one that died midway. - `priority`, `changefreq` and the ping endpoints are left out on purpose. - The README is the user guide and `docs/` the GitHub Pages site: change them with the code. The README's table and `docs/benchmarks.html` come from the same `bench/build.rb` run. diff --git a/README.md b/README.md index d531d4f..d324f08 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,9 @@ on each run is time spent on pages that did not change. It works differently fro - A page is one record in several languages, not a list of unrelated URLs. The `hreflang` links come from that: every language lists all the others and itself, which search engines require before they trust any of them. - Each file holds a fixed range of ids, so a record stays in the same file from one build to the next. A build - asks the database one question per source, the count and the latest change of each range, and rewrites only - the files whose answer moved. No callback in your models: `update_all`, imports and deletions are seen too. + asks the database one question per source, the count, the sum of the ids and the latest change of each range, + and rewrites only the files whose answer moved. No callback in your models: `update_all`, imports and deletions + are seen too, and a full build once a week catches what leaves those numbers alone. - `lastmod` is the only hint search engines still read, so it is the date your content changed, in the files and in the index. `priority` and `changefreq`, which Google ignores, are not written. - Search engines are told through [IndexNow](https://www.indexnow.org) which URLs changed. The ping endpoints @@ -159,9 +160,27 @@ production: schedule: every hour ``` -A build writes the sitemap files first, then the index, then removes the files no range fills any more, then -writes `sitemap-manifest.json`, its memory of what each file held. A build that fails midway is repaired by the -next one. Changing the host, the languages or the version of Mapitaine rewrites everything once. +### How a build decides + +For each source, one query groups the records by range of ids and returns the count, the sum of the ids and the +latest change of each. A range whose answer is the one the last build stored keeps its file. The others are +rendered again, and a file that comes out the same is not uploaded. In the index, a file that changed takes the +latest `lastmod` it holds, or the time of the build when a record left it, so that crawlers read it again. + +Some changes leave those numbers alone: `update_columns`, a change in another table that the `url`, `locales` or +`images` blocks read, a new `url` block, a file someone removed from the storage. Name the other table in +`fingerprint`, or `touch` the record from it. For the rest, a full build comes once a week by itself: + +```ruby +config.rebuild_every = 1.day # default: a week; nil leaves it to mapitaine:rebuild +``` + +After deploying a change to a `url` block, run `mapitaine:rebuild` rather than wait. + +A build notes the files it is about to add in `sitemap-manifest.json`, its memory of what each file held, then +writes the sitemap files, then the index, then removes the files no range fills any more, then writes the +manifest again. A build that fails midway is repaired by the next one. Changing the host, the languages, the +size of a range or the version of Mapitaine rewrites everything it moves once. ## Storage diff --git a/docs/guide.html b/docs/guide.html index b471b88..53731f8 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -142,8 +142,12 @@

Building

command: "Mapitaine.build" schedule: every hour

How a build decides

-

For each source, one query groups the records by range of ids and returns the count and the latest change of each. A range whose answer is the one the last build stored keeps its file. The others are rendered again. Nothing hooks into your models, so update_all, imports and deletions are seen too.

-

A build writes the sitemap files first, then the index, then removes the files no range fills any more, then writes sitemap-manifest.json, its memory of what each file held. A build that fails midway is repaired by the next one. Changing the host, the languages or the version of Mapitaine rewrites everything once.

+

For each source, one query groups the records by range of ids and returns the count, the sum of the ids and the latest change of each. A range whose answer is the one the last build stored keeps its file. The others are rendered again, and a file that comes out the same is not uploaded. Nothing hooks into your models, so update_all, imports and deletions are seen too.

+

In the index, a file that changed takes the latest lastmod it holds, or the time of the build when a record left it, so that crawlers read it again.

+

Some changes leave those numbers alone: update_columns, a change in another table that the url, locales or images blocks read, a new url block, a file someone removed from the storage. Name the other table in fingerprint, or touch the record from it. For the rest, a full build comes once a week by itself:

+
config.rebuild_every = 1.day   # default: a week; nil leaves it to mapitaine:rebuild
+

After deploying a change to a url block, run mapitaine:rebuild rather than wait.

+

A build notes the files it is about to add in sitemap-manifest.json, its memory of what each file held, then writes the sitemap files, then the index, then removes the files no range fills any more, then writes the manifest again. A build that fails midway is repaired by the next one. Changing the host, the languages, the size of a range or the version of Mapitaine rewrites everything it moves once.

diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index 8437549..08fb8bc 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -14,6 +14,8 @@ def changed? # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i + Rendered = Data.define(:source, :shard, :fingerprint, :files, :changed_urls) + Warning = Data.define(:source, :url, :message) do def to_s "#{source}: #{url} #{message}" @@ -37,8 +39,11 @@ def build(full: false) def build_in(directory, full:) @directory = directory @previous = Manifest.read(@storage, manifest_name) - @reusable = !full && @previous.digest == @configuration.digest - @manifest = Manifest.new(digest: @configuration.digest, built_at: Time.now) + @previous_file_by_name = @previous.files.to_h { |file| [file["name"], file] } + built_at = Time.now + @full = full || rebuild_due?(built_at) + @reusable = !@full && @previous.digest == @configuration.digest + @manifest = Manifest.new(digest: @configuration.digest, built_at:, rebuilt_at: @full ? built_at : @previous.rebuilt_at) @written = [] @kept = [] @warnings = [] @@ -47,9 +52,12 @@ def build_in(directory, full:) @reported = Set.new @x_default = Xml.hreflang(@configuration.x_default) + @rendered = [] + @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } + publish_rendered publish_index - deleted = @previous.file_names - @manifest.file_names + deleted = (@previous.file_names | @previous.pending) - @manifest.file_names deleted.each { |name| @storage.delete(name) } upload(manifest_name, @manifest.to_json) @@ -64,7 +72,7 @@ def build_source(source) if fingerprint && stored && stored["fingerprint"] == fingerprint keep(source, shard, stored) else - render(source, shard, fingerprint, stored, seen_urls) + render(source, shard, fingerprint, seen_urls) end end end @@ -74,7 +82,7 @@ def keep(source, shard, stored) @kept.concat(stored["files"].map { |file| file["name"] }) end - def render(source, shard, fingerprint, stored, seen_urls) + def render(source, shard, fingerprint, seen_urls) writer = ShardWriter.new(directory: @directory, basename: source.basename(shard)) changed_urls = [] ActiveSupport::Notifications.instrument("shard.mapitaine", source: source.slug, shard:) do @@ -87,21 +95,64 @@ def render(source, shard, fingerprint, stored, seen_urls) end fingerprint ||= writer.content_digest - return keep(source, shard, stored) if stored && stored["fingerprint"] == fingerprint + # Recorded now, published once every shard is rendered: the index keeps the order of the sources. + @manifest.record(source.slug, shard, fingerprint:, files: []) + @rendered << Rendered.new(source: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:) + end + + def publish_rendered + changed_names = @rendered.flat_map(&:files).reject { |file| same_as_before?(file) }.to_set(&:name) + announce_new_files(changed_names) + @rendered.each do |rendered| + changed = rendered.files.select { |file| changed_names.include?(file.name) } + rendered.files.each { |file| publish(file, changed.include?(file)) } + @manifest.record(rendered.source, rendered.shard, fingerprint: rendered.fingerprint, + files: rendered.files.map { |file| manifest_entry(file, changed.include?(file)) }) + @changed_urls.concat(rendered.changed_urls) if changed.any? + end + end + + def same_as_before?(file) + @previous_file_by_name.dig(file.name, "digest") == file.digest + end + + # Recorded before the first upload, so that a build dying midway leaves no file the next one cannot remove. + def announce_new_files(names) + new_names = names.to_a - @previous_file_by_name.keys - @previous.pending + return if new_names.empty? - publish(source, shard, fingerprint, writer.files) - @changed_urls.concat(changed_urls) + @previous.pending.concat(new_names) + upload(manifest_name, @previous.to_json) end - def publish(source, shard, fingerprint, files) - files.each do |file| + # A full build uploads every file, so that it also repairs a storage that lost one. + def publish(file, changed) + if changed || @full @storage.write(file.name, file.path) - File.delete(file.path) + @written << file.name + else + @kept << file.name + end + File.delete(file.path) + end + + # The index tells crawlers which files to read again: a file that changed gets a later date even when the + # change is a removal, and one that did not keeps its date. + def manifest_entry(file, changed) + previous_lastmod = @previous_file_by_name.dig(file.name, "lastmod") + lastmod = if !changed + previous_lastmod + elsif @previous_file_by_name.key?(file.name) && (file.lastmod.nil? || (previous_lastmod && file.lastmod <= previous_lastmod)) + @manifest.built_at.getutc.iso8601 + else + file.lastmod end - @written.concat(files.map(&:name)) - @manifest.record(source.slug, shard, fingerprint:, files: files.map do |file| - {"name" => file.name, "urls" => file.url_count, "lastmod" => file.lastmod} - end) + {"name" => file.name, "urls" => file.url_count, "lastmod" => lastmod, "digest" => file.digest} + end + + def rebuild_due?(now) + every = @configuration.rebuild_every + every.present? && (@previous.rebuilt_at.nil? || @previous.rebuilt_at + every <= now) end def publish_index diff --git a/lib/mapitaine/configuration.rb b/lib/mapitaine/configuration.rb index 129656f..2c5d83a 100644 --- a/lib/mapitaine/configuration.rb +++ b/lib/mapitaine/configuration.rb @@ -2,13 +2,18 @@ module Mapitaine class Configuration - attr_accessor :filename, :index_now_key, :index_now_key_location, :load_sources, :storage, :x_default + # A full build now and then catches what no fingerprint sees: update_columns, a change in another table, + # a new url block, a file removed from the storage. + DEFAULT_REBUILD_EVERY_SECONDS = 7 * 24 * 60 * 60 + + attr_accessor :filename, :index_now_key, :index_now_key_location, :load_sources, :rebuild_every, :storage, :x_default attr_reader :host attr_writer :locales, :sitemaps_url, :sources def initialize @filename = "sitemap" @load_sources = -> {} + @rebuild_every = DEFAULT_REBUILD_EVERY_SECONDS @storage = Storage::Local.new("public") end diff --git a/lib/mapitaine/manifest.rb b/lib/mapitaine/manifest.rb index d85cf4d..da45644 100644 --- a/lib/mapitaine/manifest.rb +++ b/lib/mapitaine/manifest.rb @@ -1,25 +1,33 @@ # frozen_string_literal: true module Mapitaine - # What the last build wrote: for each shard of each source, its fingerprint and its files. + # What the last build wrote: for each shard of each source, its fingerprint and its files. pending lists the + # files a build started to publish before it could record them, so that the next one can remove them. class Manifest - attr_reader :built_at, :digest, :shards_by_source + attr_reader :built_at, :rebuilt_at, :digest, :shards_by_source, :pending def self.read(storage, name) json = storage.read(name) return new unless json stored = JSON.parse(json) - new(digest: stored["digest"], built_at: Time.iso8601(stored["built_at"]), shards_by_source: stored["sources"]) + new(digest: stored["digest"], built_at: time(stored["built_at"]), rebuilt_at: time(stored["rebuilt_at"]), + shards_by_source: stored["sources"] || {}, pending: stored["pending"] || []) rescue JSON::ParserError, ArgumentError, TypeError # A manifest we cannot read is a manifest we do not have: the build starts over and replaces it. new end - def initialize(digest: nil, built_at: nil, shards_by_source: {}) + def self.time(text) + Time.iso8601(text) if text + end + + def initialize(digest: nil, built_at: nil, rebuilt_at: nil, shards_by_source: {}, pending: []) @digest = digest @built_at = built_at + @rebuilt_at = rebuilt_at @shards_by_source = shards_by_source + @pending = pending end def shard(source, key) @@ -39,7 +47,10 @@ def file_names end def to_json(*) - JSON.pretty_generate("digest" => digest, "built_at" => built_at.utc.iso8601, "sources" => shards_by_source) + # Whole seconds, like the lastmods compared to built_at: a change in the second of a build is sent again + # rather than never. + JSON.pretty_generate("digest" => digest, "built_at" => built_at&.utc&.iso8601, + "rebuilt_at" => rebuilt_at&.utc&.iso8601, "pending" => pending, "sources" => shards_by_source) end end end diff --git a/lib/mapitaine/shard_writer.rb b/lib/mapitaine/shard_writer.rb index da387f4..74558fd 100644 --- a/lib/mapitaine/shard_writer.rb +++ b/lib/mapitaine/shard_writer.rb @@ -7,7 +7,7 @@ class ShardWriter MAX_URLS_PER_FILE = 50_000 MAX_BYTES_PER_FILE = 52_428_800 - Written = Data.define(:name, :path, :url_count, :lastmod) + Written = Data.define(:name, :path, :url_count, :lastmod, :digest) attr_reader :files @@ -17,7 +17,6 @@ def initialize(directory:, basename:, max_urls: MAX_URLS_PER_FILE, max_bytes: MA @max_urls = max_urls @max_bytes = max_bytes @files = [] - @digest = Digest::SHA256.new end # The entries of one page stay in the same file. @@ -36,7 +35,7 @@ def close end def content_digest - @digest.hexdigest + Digest::SHA256.hexdigest(@files.map(&:digest).join) end private @@ -46,6 +45,7 @@ def open_file @name = "#{@basename}#{suffix}.xml.gz" @path = File.join(@directory, @name) @gzip = Zlib::GzipWriter.open(@path) + @file_digest = Digest::SHA256.new @url_count = 0 @bytes = Xml::URLSET_CLOSE.bytesize @lastmod = nil @@ -53,16 +53,15 @@ def open_file end def roll - @gzip.write(Xml::URLSET_CLOSE) - @digest << Xml::URLSET_CLOSE + write(Xml::URLSET_CLOSE) @gzip.close @gzip = nil - @files << Written.new(name: @name, path: @path, url_count: @url_count, lastmod: @lastmod) + @files << Written.new(name: @name, path: @path, url_count: @url_count, lastmod: @lastmod, digest: @file_digest.hexdigest) end def write(text) @gzip.write(text) - @digest << text + @file_digest << text @bytes += text.bytesize end end diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index 87c3365..76854c1 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -102,8 +102,11 @@ def default_url_options def fingerprint_by_shard return {WHOLE => nil} unless relation? - relation.reorder(nil).group(shard_sql).pluck(shard_sql, Arel.sql("COUNT(*)"), fingerprint_sql) - .to_h { |shard, count, latest| [shard.to_i.to_s, "#{count}/#{precise(latest)}"] } + # The sum of the ids moves when a record leaves a range and another enters it; the shard size, when the + # ranges themselves move. + id_sum = Arel.sql("SUM(#{model.quoted_table_name}.#{model.quoted_primary_key})") + relation.reorder(nil).group(shard_sql).pluck(shard_sql, Arel.sql("COUNT(*)"), id_sum, fingerprint_sql) + .to_h { |shard, count, sum, latest| [shard.to_i.to_s, "#{shard_size}/#{count}/#{sum.to_i}/#{precise(latest)}"] } end def each_page(shard) diff --git a/test/build_test.rb b/test/build_test.rb index ea4f82f..575db70 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -3,13 +3,13 @@ require "test_helper" class BuildTest < MapitaineTest - def test_first_build_writes_a_file_per_shard_then_the_index_then_the_manifest + def test_first_build_announces_its_files_writes_one_per_shard_then_the_index_then_the_manifest create_post(1) create_post(12) built = build - assert_equal %w[sitemap-post-00000.xml.gz sitemap-post-00001.xml.gz sitemap-page.xml.gz sitemap.xml sitemap-manifest.json], - storage.writes + assert_equal %w[sitemap-manifest.json sitemap-post-00000.xml.gz sitemap-post-00001.xml.gz sitemap-page.xml.gz sitemap.xml + sitemap-manifest.json], storage.writes assert_equal %w[sitemap-post-00000.xml.gz sitemap-post-00001.xml.gz sitemap-page.xml.gz], built.written assert_equal 6, built.url_count assert_empty built.warnings @@ -108,6 +108,110 @@ def test_a_shard_left_without_records_loses_its_file assert_predicate built, :changed? end + def test_a_record_replaced_by_an_older_one_rewrites_its_shard + create_post(1, updated_at: LONG_AGO + 60) + create_post(2) + build + Post.where(id: 2).delete_all + create_post(3) + built = build + + assert_equal %w[sitemap-post-00000.xml.gz], built.written + assert_includes storage.xml("sitemap-post-00000.xml.gz"), "posts/3" + end + + def test_a_new_shard_size_rewrites_the_source + create_post(5) + create_post(15) + build + Post.where(id: 15).delete_all + create_post(25) + Mapitaine.configuration.sources = [Class.new(PostSitemap) { shard_size 20 }, PageSitemap] + built = build + + assert_equal %w[sitemap-post-00001.xml.gz], built.written + assert_includes built.kept, "sitemap-post-00000.xml.gz" + refute_includes storage.xml("sitemap-post-00001.xml.gz"), "posts/15" + assert_includes storage.xml("sitemap-post-00001.xml.gz"), "posts/25" + end + + def test_a_file_that_changed_moves_forward_in_the_index_even_when_a_record_left_it + create_post(1) + create_post(2) + build + Post.where(id: 2).delete_all + built = build + + assert_equal %w[sitemap-post-00000.xml.gz], built.written + stamp = storage.xml("sitemap.xml")[%r{sitemap-post-00000.xml.gz([^<]+)}, 1] + + assert_operator Time.iso8601(stamp), :>, LONG_AGO + end + + def test_a_shard_rendered_again_without_a_visible_change_is_not_uploaded + create_post(1) + fixed = Class.new(PostSitemap) { lastmod { Date.new(2026, 1, 1) } } + Mapitaine.configuration.sources = [fixed] + build + Post.where(id: 1).update_all(updated_at: LONG_AGO + 60) + built = build + + assert_empty built.written + assert_equal %w[sitemap-post-00000.xml.gz], built.kept + assert_includes storage.xml("sitemap.xml"), "sitemap-post-00000.xml.gz2026-01-01" + end + + def test_a_full_build_uploads_everything_and_keeps_the_dates_of_what_did_not_change + create_post(1) + build + index = storage.xml("sitemap.xml") + storage.delete("sitemap-post-00000.xml.gz") + built = build(full: true) + + assert_equal %w[sitemap-post-00000.xml.gz sitemap-page.xml.gz], built.written + assert_equal index, storage.xml("sitemap.xml") + end + + def test_a_full_build_comes_every_week_by_itself + create_post(1) + build + manifest = JSON.parse(storage.content_by_name["sitemap-manifest.json"]) + manifest["rebuilt_at"] = (Time.now - 8 * 86_400).utc.iso8601 + storage.content_by_name["sitemap-manifest.json"] = manifest.to_json + + assert_equal %w[sitemap-post-00000.xml.gz sitemap-page.xml.gz], build.written + assert_empty build.written + Mapitaine.configuration.rebuild_every = nil + storage.content_by_name["sitemap-manifest.json"] = manifest.to_json + + assert_empty build.written + end + + def test_files_of_a_build_that_died_midway_are_removed_by_the_next + create_post(1) + build + create_post(12) + failing = Class.new(MemoryStorage) do + def write(name, source_path) + raise IOError, "disk full" if name == "sitemap.xml" + + super + end + end.new + failing.content_by_name.merge!(storage.content_by_name) + Mapitaine.configuration.storage = failing + + assert_raises(IOError) { Mapitaine.build } + assert failing.content_by_name.key?("sitemap-post-00001.xml.gz") + Post.where(id: 12).delete_all + @storage = MemoryStorage.new + storage.content_by_name.merge!(failing.content_by_name) + Mapitaine.configuration.storage = storage + built = build + + assert_equal %w[sitemap-post-00001.xml.gz], built.deleted + end + def test_a_full_build_rewrites_everything create_post(1) build From 04c2c5a56b23b5e8aa9c8d01b9a8740f62ef280a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:55:58 +0000 Subject: [PATCH 4/8] Fingerprint large ids on SQLite, compare dates as times, report full builds SQLite's SUM overflows on ids near 2^63, so the fingerprint sums offsets within each range instead. A lastmod from an older manifest is compared as a time, not as text. A build that turned full because rebuild_every had passed says so in the result and in the build.mapitaine payload. Assisted-by: Claude Code --- README.md | 6 +++--- docs/guide.html | 4 ++-- lib/mapitaine/builder.rb | 33 +++++++++++++++++++++++---------- lib/mapitaine/source.rb | 6 +++--- lib/mapitaine/tasks.rake | 2 +- test/build_test.rb | 33 +++++++++++++++++++++++++++++++-- 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index d324f08..56cca6a 100644 --- a/README.md +++ b/README.md @@ -149,8 +149,8 @@ bin/rails mapitaine:build # rewrite the files whose records changed bin/rails mapitaine:rebuild # rewrite everything ``` -`Mapitaine.build` does the same from Ruby and returns what it did (`written`, `kept`, `deleted`, `url_count`, -`submitted`, `warnings`). Run it on a schedule, every night or every hour. With Solid Queue: +`Mapitaine.build` does the same from Ruby and returns what it did (`full`, `written`, `kept`, `deleted`, +`url_count`, `submitted`, `warnings`). Run it on a schedule, every night or every hour. With Solid Queue: ```yaml # config/recurring.yml @@ -164,7 +164,7 @@ production: For each source, one query groups the records by range of ids and returns the count, the sum of the ids and the latest change of each. A range whose answer is the one the last build stored keeps its file. The others are -rendered again, and a file that comes out the same is not uploaded. In the index, a file that changed takes the +rendered again, and a file that comes out the same is not uploaded (a full build uploads every file). In the index, a file that changed takes the latest `lastmod` it holds, or the time of the build when a record left it, so that crawlers read it again. Some changes leave those numbers alone: `update_columns`, a change in another table that the `url`, `locales` or diff --git a/docs/guide.html b/docs/guide.html index 53731f8..34770ff 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -135,14 +135,14 @@

Building

bin/rails mapitaine:build     # rewrite the files whose records changed
 bin/rails mapitaine:rebuild   # rewrite everything
-

Mapitaine.build does the same from Ruby and returns what it did: written, kept, deleted, url_count, submitted and warnings. Run it on a schedule, every night or every hour. With Solid Queue:

+

Mapitaine.build does the same from Ruby and returns what it did: full, written, kept, deleted, url_count, submitted and warnings. Run it on a schedule, every night or every hour. With Solid Queue:

# config/recurring.yml
 production:
   sitemap:
     command: "Mapitaine.build"
     schedule: every hour

How a build decides

-

For each source, one query groups the records by range of ids and returns the count, the sum of the ids and the latest change of each. A range whose answer is the one the last build stored keeps its file. The others are rendered again, and a file that comes out the same is not uploaded. Nothing hooks into your models, so update_all, imports and deletions are seen too.

+

For each source, one query groups the records by range of ids and returns the count, the sum of the ids and the latest change of each. A range whose answer is the one the last build stored keeps its file. The others are rendered again, and a file that comes out the same is not uploaded (a full build uploads every file). Nothing hooks into your models, so update_all, imports and deletions are seen too.

In the index, a file that changed takes the latest lastmod it holds, or the time of the build when a record left it, so that crawlers read it again.

Some changes leave those numbers alone: update_columns, a change in another table that the url, locales or images blocks read, a new url block, a file someone removed from the storage. Name the other table in fingerprint, or touch the record from it. For the rest, a full build comes once a week by itself:

config.rebuild_every = 1.day   # default: a week; nil leaves it to mapitaine:rebuild
diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index 08fb8bc..55c3743 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -2,7 +2,8 @@ module Mapitaine class Builder - Result = Data.define(:written, :kept, :deleted, :url_count, :submitted, :warnings) do + # full: whether this build rewrote everything, asked to or because rebuild_every had passed. + Result = Data.define(:full, :written, :kept, :deleted, :url_count, :submitted, :warnings) do def changed? written.any? || deleted.any? end @@ -14,7 +15,7 @@ def changed? # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i - Rendered = Data.define(:source, :shard, :fingerprint, :files, :changed_urls) + Rendered = Data.define(:source_slug, :shard, :fingerprint, :files, :changed_urls) Warning = Data.define(:source, :url, :message) do def to_s @@ -31,6 +32,8 @@ def initialize(configuration, storage: configuration.storage, submit: true) def build(full: false) ActiveSupport::Notifications.instrument("build.mapitaine", full:) do |payload| payload[:result] = Dir.mktmpdir("mapitaine") { |directory| build_in(directory, full:) } + payload[:full] = payload[:result].full + payload[:result] end end @@ -61,7 +64,7 @@ def build_in(directory, full:) deleted.each { |name| @storage.delete(name) } upload(manifest_name, @manifest.to_json) - Result.new(written: @written, kept: @kept, deleted:, url_count: @manifest.files.sum { |file| file["urls"] }, + Result.new(full: @full, written: @written, kept: @kept, deleted:, url_count: @manifest.files.sum { |file| file["urls"] }, submitted: submit_changed_urls, warnings: @warnings) end @@ -97,18 +100,20 @@ def render(source, shard, fingerprint, seen_urls) fingerprint ||= writer.content_digest # Recorded now, published once every shard is rendered: the index keeps the order of the sources. @manifest.record(source.slug, shard, fingerprint:, files: []) - @rendered << Rendered.new(source: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:) + @rendered << Rendered.new(source_slug: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:) end def publish_rendered changed_names = @rendered.flat_map(&:files).reject { |file| same_as_before?(file) }.to_set(&:name) announce_new_files(changed_names) @rendered.each do |rendered| - changed = rendered.files.select { |file| changed_names.include?(file.name) } - rendered.files.each { |file| publish(file, changed.include?(file)) } - @manifest.record(rendered.source, rendered.shard, fingerprint: rendered.fingerprint, - files: rendered.files.map { |file| manifest_entry(file, changed.include?(file)) }) - @changed_urls.concat(rendered.changed_urls) if changed.any? + entries = rendered.files.map do |file| + changed = changed_names.include?(file.name) + publish(file, changed) + manifest_entry(file, changed) + end + @manifest.record(rendered.source_slug, rendered.shard, fingerprint: rendered.fingerprint, files: entries) + @changed_urls.concat(rendered.changed_urls) if rendered.files.any? { |file| changed_names.include?(file.name) } end end @@ -142,7 +147,7 @@ def manifest_entry(file, changed) previous_lastmod = @previous_file_by_name.dig(file.name, "lastmod") lastmod = if !changed previous_lastmod - elsif @previous_file_by_name.key?(file.name) && (file.lastmod.nil? || (previous_lastmod && file.lastmod <= previous_lastmod)) + elsif @previous_file_by_name.key?(file.name) && !later?(file.lastmod, previous_lastmod) @manifest.built_at.getutc.iso8601 else file.lastmod @@ -150,6 +155,14 @@ def manifest_entry(file, changed) {"name" => file.name, "urls" => file.url_count, "lastmod" => lastmod, "digest" => file.digest} end + # A manifest written by an older version may hold a lastmod in another format: compare times, not text. + def later?(stamp, previous_stamp) + return false if stamp.nil? + + previous_stamp = Xml.timestamp(previous_stamp) + previous_stamp.nil? || Xml.time(stamp) > Xml.time(previous_stamp) + end + def rebuild_due?(now) every = @configuration.rebuild_every every.present? && (@previous.rebuilt_at.nil? || @previous.rebuilt_at + every <= now) diff --git a/lib/mapitaine/source.rb b/lib/mapitaine/source.rb index 76854c1..1ba4cd6 100644 --- a/lib/mapitaine/source.rb +++ b/lib/mapitaine/source.rb @@ -103,9 +103,9 @@ def fingerprint_by_shard return {WHOLE => nil} unless relation? # The sum of the ids moves when a record leaves a range and another enters it; the shard size, when the - # ranges themselves move. - id_sum = Arel.sql("SUM(#{model.quoted_table_name}.#{model.quoted_primary_key})") - relation.reorder(nil).group(shard_sql).pluck(shard_sql, Arel.sql("COUNT(*)"), id_sum, fingerprint_sql) + # ranges themselves move. Summed as offsets within the range: SQLite's SUM overflows on large ids. + offset_sum = Arel.sql("SUM(#{model.quoted_table_name}.#{model.quoted_primary_key} % #{shard_size})") + relation.reorder(nil).group(shard_sql).pluck(shard_sql, Arel.sql("COUNT(*)"), offset_sum, fingerprint_sql) .to_h { |shard, count, sum, latest| [shard.to_i.to_s, "#{shard_size}/#{count}/#{sum.to_i}/#{precise(latest)}"] } end diff --git a/lib/mapitaine/tasks.rake b/lib/mapitaine/tasks.rake index 1fe25cf..8b680b3 100644 --- a/lib/mapitaine/tasks.rake +++ b/lib/mapitaine/tasks.rake @@ -2,7 +2,7 @@ namespace :mapitaine do report = lambda do |result| - puts "#{result.url_count} URLs: #{result.written.size} files written, #{result.kept.size} kept, " \ + puts "#{result.url_count} URLs#{" (full build)" if result.full}: #{result.written.size} files written, #{result.kept.size} kept, " \ "#{result.deleted.size} deleted, #{result.submitted} URLs sent to IndexNow" result.warnings.each { |warning| warn " #{warning}" } end diff --git a/test/build_test.rb b/test/build_test.rb index 575db70..73ccc9d 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -120,6 +120,16 @@ def test_a_record_replaced_by_an_older_one_rewrites_its_shard assert_includes storage.xml("sitemap-post-00000.xml.gz"), "posts/3" end + def test_ids_too_large_for_a_sum_are_fingerprinted + create_post(9_000_000_000_000_000_001) + create_post(9_000_000_000_000_000_002) + build + Post.where(id: 9_000_000_000_000_000_002).delete_all + create_post(9_000_000_000_000_000_003) + + assert_equal %w[sitemap-post-900000000000000000.xml.gz], build.written + end + def test_a_new_shard_size_rewrites_the_source create_post(5) create_post(15) @@ -148,6 +158,18 @@ def test_a_file_that_changed_moves_forward_in_the_index_even_when_a_record_left_ assert_operator Time.iso8601(stamp), :>, LONG_AGO end + def test_a_file_dated_by_an_older_manifest_is_compared_by_time + create_post(1, updated_at: Time.utc(2026, 1, 15, 5)) + build + manifest = JSON.parse(storage.content_by_name["sitemap-manifest.json"]) + manifest["sources"]["post"]["0"]["files"][0]["lastmod"] = "2026-01-15T10:00:00+09:00" + storage.content_by_name["sitemap-manifest.json"] = manifest.to_json + Post.where(id: 1).update_all(title: "Edited", updated_at: Time.utc(2026, 1, 15, 6)) + build + + assert_includes storage.xml("sitemap.xml"), "sitemap-post-00000.xml.gz2026-01-15T06:00:00Z" + end + def test_a_shard_rendered_again_without_a_visible_change_is_not_uploaded create_post(1) fixed = Class.new(PostSitemap) { lastmod { Date.new(2026, 1, 1) } } @@ -179,8 +201,11 @@ def test_a_full_build_comes_every_week_by_itself manifest["rebuilt_at"] = (Time.now - 8 * 86_400).utc.iso8601 storage.content_by_name["sitemap-manifest.json"] = manifest.to_json - assert_equal %w[sitemap-post-00000.xml.gz sitemap-page.xml.gz], build.written - assert_empty build.written + built = build + + assert_predicate built, :full + assert_equal %w[sitemap-post-00000.xml.gz sitemap-page.xml.gz], built.written + refute_predicate build, :full Mapitaine.configuration.rebuild_every = nil storage.content_by_name["sitemap-manifest.json"] = manifest.to_json @@ -354,6 +379,10 @@ def test_a_build_is_instrumented built = build assert_equal ["shard.mapitaine", {source: "post", shard: "0"}], events.first + assert_equal ["build.mapitaine", {full: true, result: built}], events.last + events.clear + built = build + assert_equal ["build.mapitaine", {full: false, result: built}], events.last ensure ActiveSupport::Notifications.unsubscribe(subscriber) From b10e8001948de8720f6bee858dcecf65c7dd52fb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 07:56:58 +0000 Subject: [PATCH 5/8] Encode JSON without Active Support's encoder On ruby-head, Active Support 8.0's encoder passes json an option it no longer takes (quirks_mode), so the configuration digest and the IndexNow body raised. Both hold plain values: JSON.generate writes the same bytes. Assisted-by: Claude Code --- lib/mapitaine/configuration.rb | 2 +- lib/mapitaine/index_now.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mapitaine/configuration.rb b/lib/mapitaine/configuration.rb index 2c5d83a..4703089 100644 --- a/lib/mapitaine/configuration.rb +++ b/lib/mapitaine/configuration.rb @@ -43,7 +43,7 @@ def sources # Whatever changes every file: a build under another digest starts over. def digest - Digest::SHA256.hexdigest([VERSION, host!, sitemaps_url, locales, x_default, filename].to_json) + Digest::SHA256.hexdigest(JSON.generate([VERSION, host!, sitemaps_url, locales, x_default, filename])) end private diff --git a/lib/mapitaine/index_now.rb b/lib/mapitaine/index_now.rb index 55390d8..faba80e 100644 --- a/lib/mapitaine/index_now.rb +++ b/lib/mapitaine/index_now.rb @@ -19,7 +19,7 @@ def initialize(host:, key:, key_location: nil, transport: Net::HTTP.method(:post def submit(urls) urls.each_slice(MAX_URLS_PER_REQUEST).sum do |batch| body = {host: @host, key: @key, keyLocation: @key_location, urlList: batch}.compact - response = @transport.call(ENDPOINT, body.to_json, HEADERS) + response = @transport.call(ENDPOINT, JSON.generate(body), HEADERS) response.code.to_i.between?(200, 299) ? batch.size : 0 end end From 90dccc559146a5ffa395cf7d1f47560ed2c095c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:00:06 +0000 Subject: [PATCH 6/8] Send IndexNow what it refused again, and pages that appear or go away A refusal (429, 403) or an unreachable endpoint lost the URLs for good: the next build compared lastmods with its own start. Pages that went away were never sent, nor new pages whose lastmod predates the last build, nor pages dated by a day that was still today. One URL on another host made the endpoint refuse its whole batch, and with sitemaps on a CDN the key went to the CDN, where IndexNow does not read it. URLs the endpoint did not take stay in the manifest and go out with the next build, with a warning. For each rewritten file, the URLs of the file it replaces are compared with the new ones; removed files count too. A date alone counts as changed while it may still be the day of the last build. Only URLs on the host are sent, and the key is published only where IndexNow reads it, with a warning otherwise. Assisted-by: Claude Code --- README.md | 14 ++++-- docs/guide.html | 5 ++- docs/index.html | 2 +- lib/mapitaine/builder.rb | 88 +++++++++++++++++++++++++++++++------- lib/mapitaine/index_now.rb | 16 +++++-- lib/mapitaine/manifest.rb | 12 +++--- lib/mapitaine/xml.rb | 8 ++++ test/index_now_test.rb | 72 ++++++++++++++++++++++++++++--- 8 files changed, 181 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 56cca6a..1013c66 100644 --- a/README.md +++ b/README.md @@ -220,10 +220,16 @@ A storage is any object with `read(name)`, `write(name, path)` and `delete(name) config.index_now_key = "c1f0e2b4a6d84f3e9b7a5c3d1e0f2a4b" # yours: 8 to 128 characters, a-z, A-Z, 0-9 and - ``` -Each build then publishes the key as `.txt` next to the sitemaps, and sends the URLs of the rewritten files -whose `lastmod` is later than the previous build. The first build sends nothing. When the key cannot live at -the root of the site, host it yourself and set `config.index_now_key_location` to its URL. An endpoint that -cannot be reached is a warning, never a failed build. +Each build then publishes the key as `.txt` at the root of the site, and sends, from the files it rewrote, +the URLs whose `lastmod` is later than the previous build, and those that appeared or went away whatever their +`lastmod`. The first build sends nothing, and only URLs on `config.host` are sent. + +When the sitemaps are served from elsewhere (`config.sitemaps_url`), the key cannot follow them: IndexNow reads +it on the site. Put `.txt` in `public/` and set `config.index_now_key_location` to its URL; a build warns +until you do. + +A refusal or an endpoint that cannot be reached is a warning, never a failed build: the URLs it did not take are +kept in the manifest and sent again with the next build. ## Checking diff --git a/docs/guide.html b/docs/guide.html index 34770ff..60d172e 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -180,8 +180,9 @@

IndexNow

IndexNow tells Bing, Yandex, Seznam and Naver that a URL changed. Google does not take part. Choose a key of 8 to 128 characters among a-z, A-Z, 0-9 and -:

config.index_now_key = "c1f0e2b4a6d84f3e9b7a5c3d1e0f2a4b"
-

Each build then publishes the key as <key>.txt next to the sitemaps, and sends the URLs of the rewritten files whose lastmod is later than the previous build. The first build sends nothing.

-

When the key cannot live at the root of the site, host it yourself and set config.index_now_key_location to its URL. An endpoint that cannot be reached is a warning, never a failed build.

+

Each build then publishes the key as <key>.txt at the root of the site, and sends, from the files it rewrote, the URLs whose lastmod is later than the previous build, and those that appeared or went away whatever their lastmod. The first build sends nothing, and only URLs on config.host are sent.

+

When the sitemaps are served from elsewhere (config.sitemaps_url), the key cannot follow them: IndexNow reads it on the site. Put <key>.txt in public/ and set config.index_now_key_location to its URL; a build warns until you do.

+

A refusal or an endpoint that cannot be reached is a warning, never a failed build: the URLs it did not take are kept in the manifest and sent again with the next build.

diff --git a/docs/index.html b/docs/index.html index 7730555..3cf4883 100644 --- a/docs/index.html +++ b/docs/index.html @@ -108,7 +108,7 @@

Any bucket Active Storage reaches

IndexNow

-

Each build sends Bing, Yandex, Seznam and Naver the URLs that changed since the previous one. The first build sends nothing.

+

Each build sends Bing, Yandex, Seznam and Naver the URLs that changed, appeared or went away since the previous one, and sends again what they refused. The first build sends nothing.

IndexNow

diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index 55c3743..dae2194 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -12,10 +12,14 @@ def changed? # https://www.sitemaps.org/protocol.html MAX_URL_CHARACTERS = 2_048 MAX_FILES_PER_INDEX = 50_000 + LAST_TIME_ZONE_OFFSET_SECONDS = 12 * 60 * 60 + # One request's worth: what a refusing endpoint leaves in the manifest stays small. + MAX_UNSUBMITTED_URLS = IndexNow::MAX_URLS_PER_REQUEST # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i - Rendered = Data.define(:source_slug, :shard, :fingerprint, :files, :changed_urls) + # urls: every URL of the shard, kept only when IndexNow needs to know which ones appeared or went away. + Rendered = Data.define(:source_slug, :shard, :fingerprint, :files, :changed_urls, :urls) Warning = Data.define(:source, :url, :message) do def to_s @@ -56,16 +60,20 @@ def build_in(directory, full:) @x_default = Xml.hreflang(@configuration.x_default) @rendered = [] + # The first build has no "before": submitting a whole site at once is not what IndexNow is for. + @tracking = @submit && @configuration.index_now_key.present? && @previous.built_at.present? @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } publish_rendered publish_index deleted = (@previous.file_names | @previous.pending) - @manifest.file_names + @changed_urls.concat(urls_in(deleted)) if @tracking deleted.each { |name| @storage.delete(name) } + submitted = submit_changed_urls upload(manifest_name, @manifest.to_json) Result.new(full: @full, written: @written, kept: @kept, deleted:, url_count: @manifest.files.sum { |file| file["urls"] }, - submitted: submit_changed_urls, warnings: @warnings) + submitted:, warnings: @warnings) end def build_source(source) @@ -88,11 +96,13 @@ def keep(source, shard, stored) def render(source, shard, fingerprint, seen_urls) writer = ShardWriter.new(directory: @directory, basename: source.basename(shard)) changed_urls = [] + urls = [] if @tracking ActiveSupport::Notifications.instrument("shard.mapitaine", source: source.slug, shard:) do source.each_page(shard) do |page| inspect_page(source, page, seen_urls) writer.add(Xml.url_entries(page, x_default: @x_default), page.lastmod) - changed_urls.concat(page.urls) if changed_since_last_build?(page) + changed_urls.concat(page.urls) if @tracking && changed_since_last_build?(page) + urls&.concat(page.urls) end writer.close end @@ -100,20 +110,39 @@ def render(source, shard, fingerprint, seen_urls) fingerprint ||= writer.content_digest # Recorded now, published once every shard is rendered: the index keeps the order of the sources. @manifest.record(source.slug, shard, fingerprint:, files: []) - @rendered << Rendered.new(source_slug: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:) + @rendered << Rendered.new(source_slug: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:, urls:) end def publish_rendered changed_names = @rendered.flat_map(&:files).reject { |file| same_as_before?(file) }.to_set(&:name) announce_new_files(changed_names) @rendered.each do |rendered| + changed = rendered.files.any? { |file| changed_names.include?(file.name) } + note_changed_urls(rendered) if changed && @tracking entries = rendered.files.map do |file| changed = changed_names.include?(file.name) publish(file, changed) manifest_entry(file, changed) end @manifest.record(rendered.source_slug, rendered.shard, fingerprint: rendered.fingerprint, files: entries) - @changed_urls.concat(rendered.changed_urls) if rendered.files.any? { |file| changed_names.include?(file.name) } + end + end + + # Read before the new files replace the old: a page that appeared or went away is a change too, whatever its + # lastmod says. + def note_changed_urls(rendered) + previous_names = @previous.shard(rendered.source_slug, rendered.shard)&.fetch("files")&.map { |file| file["name"] } || [] + previous_urls = urls_in(previous_names).to_set + urls = rendered.urls.to_set + @changed_urls.concat(rendered.changed_urls, (urls - previous_urls).to_a, (previous_urls - urls).to_a) + end + + def urls_in(names) + names.flat_map do |name| + content = @storage.read(name) + content ? Xml.locations(Zlib.gunzip(content.b)) : [] + rescue Zlib::Error + [] end end @@ -189,7 +218,7 @@ def manifest_name def inspect_page(source, page, seen_urls) page.urls.each do |url| - warn(source, url, "is outside #{@configuration.host!}") unless url.start_with?("#{@configuration.host!}/") || url == @configuration.host! + warn(source, url, "is outside #{@configuration.host!}") unless on_host?(url) warn(source, url, "is listed twice") unless seen_urls.add?(url) warn(source, url, "is longer than #{MAX_URL_CHARACTERS} characters") if url.length > MAX_URL_CHARACTERS end @@ -225,23 +254,52 @@ def warn(source, url, message) @warnings << Warning.new(source: source.is_a?(Source) ? source.slug : source, url:, message:) end - # The first build has no "before": submitting a whole site at once is not what IndexNow is for. + def on_host?(url) + url.start_with?("#{@configuration.host!}/") || url == @configuration.host! + end + + # A date alone may still be the day of the last build somewhere: it changed if it is not older than the date + # at UTC-12, the last time zone to leave a day. def changed_since_last_build?(page) - @previous.built_at && page.modified_at && page.modified_at >= @previous.built_at + return false unless page.lastmod + return page.modified_at >= @previous.built_at if page.lastmod.include?("T") + + last_date = (@previous.built_at - LAST_TIME_ZONE_OFFSET_SECONDS).utc.to_date.iso8601 + page.lastmod >= last_date[0, page.lastmod.length] end + # Returns how many URLs the endpoint took. The ones it did not take, refused or unreachable, are kept in the + # manifest and sent again with the next build: the sitemaps are published, so a refusal never fails the build. def submit_changed_urls key = @configuration.index_now_key return 0 unless @submit && key - upload("#{key}.txt", key) unless @configuration.index_now_key_location - return 0 if @changed_urls.empty? + publish_index_now_key(key) + urls = (@previous.unsubmitted + @changed_urls).uniq.select { |url| on_host?(url) } + return 0 if urls.empty? + + replies = IndexNow.new(host: @configuration.host!, key:, key_location: @configuration.index_now_key_location).submit(urls) + refused = replies.reject(&:accepted?) + refused.group_by(&:status).each do |status, replies_with_status| + count = replies_with_status.sum { |reply| reply.urls.size } + answer = status.is_a?(Integer) ? "answered #{status}" : "could not be reached (#{status})" + warn("indexnow", IndexNow::ENDPOINT, "#{answer} for #{count} URLs, sent again with the next build") + end + @manifest.unsubmitted.replace(refused.flat_map(&:urls).last(MAX_UNSUBMITTED_URLS)) + replies.select(&:accepted?).sum { |reply| reply.urls.size } + end - IndexNow.new(host: @configuration.host!, key:, key_location: @configuration.index_now_key_location).submit(@changed_urls) - rescue *IndexNow::NETWORK_ERRORS => exception - # The sitemaps are published: a search engine that cannot be reached must not fail the build. - @warnings << Warning.new(source: "indexnow", url: IndexNow::ENDPOINT.to_s, message: "could not be reached (#{exception.class})") - 0 + # IndexNow reads the key at the root of the site unless told where it is, and only on the site's host. + def publish_index_now_key(key) + location = @configuration.index_now_key_location + if location + warn("indexnow", location, "is not on #{@configuration.host!}: IndexNow only reads a key there") unless on_host?(location) + elsif @configuration.sitemaps_url == @configuration.host! + upload("#{key}.txt", key) + else + warn("indexnow", "#{@configuration.host!}/#{key}.txt", "must hold the key: sitemaps go to #{@configuration.sitemaps_url}, " \ + "so put #{key}.txt in public/ and set index_now_key_location to this URL") + end end end end diff --git a/lib/mapitaine/index_now.rb b/lib/mapitaine/index_now.rb index faba80e..f61127c 100644 --- a/lib/mapitaine/index_now.rb +++ b/lib/mapitaine/index_now.rb @@ -15,12 +15,20 @@ def initialize(host:, key:, key_location: nil, transport: Net::HTTP.method(:post @transport = transport end - # Returns how many URLs the endpoint took. + # status: the HTTP status, or the class of the network error. + Reply = Data.define(:urls, :status) do + def accepted? + status.is_a?(Integer) && status.between?(200, 299) + end + end + + # One reply per request of up to 10,000 URLs. def submit(urls) - urls.each_slice(MAX_URLS_PER_REQUEST).sum do |batch| + urls.each_slice(MAX_URLS_PER_REQUEST).map do |batch| body = {host: @host, key: @key, keyLocation: @key_location, urlList: batch}.compact - response = @transport.call(ENDPOINT, JSON.generate(body), HEADERS) - response.code.to_i.between?(200, 299) ? batch.size : 0 + Reply.new(urls: batch, status: @transport.call(ENDPOINT, JSON.generate(body), HEADERS).code.to_i) + rescue *NETWORK_ERRORS => exception + Reply.new(urls: batch, status: exception.class.name) end end end diff --git a/lib/mapitaine/manifest.rb b/lib/mapitaine/manifest.rb index da45644..9afca93 100644 --- a/lib/mapitaine/manifest.rb +++ b/lib/mapitaine/manifest.rb @@ -2,9 +2,10 @@ module Mapitaine # What the last build wrote: for each shard of each source, its fingerprint and its files. pending lists the - # files a build started to publish before it could record them, so that the next one can remove them. + # files a build started to publish before it could record them, so that the next one can remove them; + # unsubmitted the URLs IndexNow did not take, for the next one to send again. class Manifest - attr_reader :built_at, :rebuilt_at, :digest, :shards_by_source, :pending + attr_reader :built_at, :rebuilt_at, :digest, :shards_by_source, :pending, :unsubmitted def self.read(storage, name) json = storage.read(name) @@ -12,7 +13,7 @@ def self.read(storage, name) stored = JSON.parse(json) new(digest: stored["digest"], built_at: time(stored["built_at"]), rebuilt_at: time(stored["rebuilt_at"]), - shards_by_source: stored["sources"] || {}, pending: stored["pending"] || []) + shards_by_source: stored["sources"] || {}, pending: stored["pending"] || [], unsubmitted: stored["unsubmitted"] || []) rescue JSON::ParserError, ArgumentError, TypeError # A manifest we cannot read is a manifest we do not have: the build starts over and replaces it. new @@ -22,12 +23,13 @@ def self.time(text) Time.iso8601(text) if text end - def initialize(digest: nil, built_at: nil, rebuilt_at: nil, shards_by_source: {}, pending: []) + def initialize(digest: nil, built_at: nil, rebuilt_at: nil, shards_by_source: {}, pending: [], unsubmitted: []) @digest = digest @built_at = built_at @rebuilt_at = rebuilt_at @shards_by_source = shards_by_source @pending = pending + @unsubmitted = unsubmitted end def shard(source, key) @@ -50,7 +52,7 @@ def to_json(*) # Whole seconds, like the lastmods compared to built_at: a change in the second of a build is sent again # rather than never. JSON.pretty_generate("digest" => digest, "built_at" => built_at&.utc&.iso8601, - "rebuilt_at" => rebuilt_at&.utc&.iso8601, "pending" => pending, "sources" => shards_by_source) + "rebuilt_at" => rebuilt_at&.utc&.iso8601, "pending" => pending, "unsubmitted" => unsubmitted, "sources" => shards_by_source) end end end diff --git a/lib/mapitaine/xml.rb b/lib/mapitaine/xml.rb index 0dc52d3..cdd601e 100644 --- a/lib/mapitaine/xml.rb +++ b/lib/mapitaine/xml.rb @@ -15,6 +15,9 @@ module Xml W3C_DATE = /\A(\d{4})(?:-(\d{2})(?:-(\d{2}))?)?\z/ W3C_TIME = /\A(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})(?::(\d{2})(?:\.\d+)?)?(Z|[+-]\d{2}:\d{2})\z/ + LOCATION = %r{([^<]*)} + UNESCAPED_BY_ENTITY = {"&" => "&", "<" => "<", ">" => ">", """ => '"', "'" => "'"}.freeze + module_function # One per language. Each lists every language of the page, itself included: search engines drop @@ -66,6 +69,11 @@ def hreflang(locale) locale.to_s.tr("_", "-") end + # The of each of a sitemap file. + def locations(xml) + xml.scan(LOCATION).map { |(location)| location.gsub(/&(?:amp|lt|gt|quot|apos);/, UNESCAPED_BY_ENTITY) } + end + def escape(text) text.to_s.encode(xml: :text).gsub('"', """) end diff --git a/test/index_now_test.rb b/test/index_now_test.rb index bab81e5..d1d1672 100644 --- a/test/index_now_test.rb +++ b/test/index_now_test.rb @@ -64,13 +64,71 @@ def test_a_key_hosted_elsewhere_is_named_and_not_published assert_equal "https://example.com/keys/indexnow.txt", @requests.dig(0, 1, "keyLocation") end - def test_a_refusal_counts_no_url + def test_a_refusal_is_reported_and_sent_again_with_the_next_build create_post(1) + create_post(2) build Post.find(1).update!(title: "Edited", updated_at: Time.now) - self.response_code = "403" + self.response_code = "429" + refused = build + + assert_equal 0, refused.submitted + assert_equal ["indexnow: https://api.indexnow.org/indexnow answered 429 for 2 URLs, sent again with the next build"], + refused.warnings.map(&:to_s) + Post.find(2).update!(title: "Edited", updated_at: Time.now) + self.response_code = "202" - assert_equal 0, build.submitted + assert_equal 4, build.submitted + assert_equal %w[https://example.com/en/posts/1 https://example.com/fr/posts/1 https://example.com/en/posts/2 + https://example.com/fr/posts/2], @requests.last.dig(1, "urlList") + end + + def test_pages_that_appear_or_go_away_are_submitted_whatever_their_lastmod + create_post(1) + create_post(2) + create_post(12) + build + Post.find(2).update!(published: false, updated_at: LONG_AGO) + Post.where(id: 12).delete_all + create_post(3) + + assert_equal 6, build.submitted + assert_equal %w[https://example.com/en/posts/3 https://example.com/fr/posts/3 https://example.com/en/posts/2 + https://example.com/fr/posts/2 https://example.com/en/posts/12 https://example.com/fr/posts/12], @requests.last.dig(1, "urlList") + end + + def test_a_date_of_the_day_of_the_last_build_is_a_change + create_post(1) + dated = Class.new(PostSitemap) { lastmod { |post| post.reviewed_at&.to_date || Date.new(2026, 1, 1) } } + Mapitaine.configuration.sources = [dated] + build + Post.find(1).update!(reviewed_at: Time.now) + + assert_equal 2, build.submitted + end + + def test_only_urls_of_the_host_are_submitted + Mapitaine.configuration.sources = [Class.new(PostSitemap) { url { |post, locale| (locale == :fr) ? "https://example.fr/posts/#{post.id}" : "/en/posts/#{post.id}" } }] + create_post(1) + build + Post.find(1).update!(title: "Edited", updated_at: Time.now) + build + + assert_equal %w[https://example.com/en/posts/1], @requests.last.dig(1, "urlList") + end + + def test_a_key_the_site_does_not_serve_is_reported + Mapitaine.configuration.sitemaps_url = "https://cdn.example.com/sitemaps" + create_post(1) + built = build + + refute storage.content_by_name.key?("0123456789abcdef.txt") + assert_equal ["indexnow: https://example.com/0123456789abcdef.txt must hold the key: sitemaps go to https://cdn.example.com/sitemaps, " \ + "so put 0123456789abcdef.txt in public/ and set index_now_key_location to this URL"], built.warnings.map(&:to_s) + Mapitaine.configuration.index_now_key_location = "https://cdn.example.com/sitemaps/0123456789abcdef.txt" + + assert_equal ["indexnow: https://cdn.example.com/sitemaps/0123456789abcdef.txt is not on https://example.com: IndexNow only reads a key there"], + build.warnings.map(&:to_s) end def test_an_unreachable_endpoint_is_a_warning_and_the_sitemaps_are_published @@ -80,7 +138,8 @@ def test_an_unreachable_endpoint_is_a_warning_and_the_sitemaps_are_published self.network_error = Errno::ECONNREFUSED built = build - assert_equal ["indexnow: https://api.indexnow.org/indexnow could not be reached (Errno::ECONNREFUSED)"], built.warnings.map(&:to_s) + assert_equal ["indexnow: https://api.indexnow.org/indexnow could not be reached (Errno::ECONNREFUSED) for 2 URLs, sent again with the next build"], + built.warnings.map(&:to_s) assert_equal %w[sitemap-post-00000.xml.gz], built.written end @@ -92,8 +151,11 @@ def test_urls_go_in_batches_of_ten_thousand end index_now = Mapitaine::IndexNow.new(host: "https://example.com", key: "key", transport:) - assert_equal 10_001, index_now.submit(Array.new(10_001) { |number| "https://example.com/#{number}" }) + replies = index_now.submit(Array.new(10_001) { |number| "https://example.com/#{number}" }) + assert_equal [10_000, 1], batches + assert_equal [10_000, 1], replies.map { |reply| reply.urls.size } + assert replies.all?(&:accepted?) end def test_check_submits_nothing From 0387a58419522f371a3c64cdbfb201d83ef88921 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:03:21 +0000 Subject: [PATCH 7/8] Write the manifest before submitting, and send only what really moved Submitting before the manifest meant an unexpected error or a slow endpoint held back the manifest after files were already published. The manifest is now written with the URLs still to send, then again once IndexNow answered, so a build that dies while submitting loses none. A new configuration or new ranges no longer read every previous file to send the whole site: only the lastmods count then. URLs are compared right after each shard renders, so no list of every URL waits for the others, a month or a year alone no longer counts as a change, the check reports a key the site does not serve, and Storage::Local reads files as bytes. Assisted-by: Claude Code --- AGENTS.md | 5 +- README.md | 3 +- docs/guide.html | 2 +- lib/mapitaine/builder.rb | 86 ++++++++++++++++++++++------------ lib/mapitaine/storage/local.rb | 2 +- test/index_now_test.rb | 60 ++++++++++++++++++++++++ 6 files changed, 124 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 46b1f4e..0d80dc6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,8 +29,9 @@ joins CI's `rails` matrix. `Configuration#digest` (the file name), in each shard's fingerprint (the shard size), or rewrites everything by itself (the slug). - A build renders every shard, records the names it is about to add as `pending` in the manifest, publishes the - sitemap files, then the index, then deletes, then writes the manifest. Keep that order: it is what lets the next - build repair one that died midway. + sitemap files, then the index, then deletes, then writes the manifest with the URLs IndexNow has yet to take, + submits them, and writes the manifest again. Keep that order: it is what lets the next build repair one that died + midway. - `priority`, `changefreq` and the ping endpoints are left out on purpose. - The README is the user guide and `docs/` the GitHub Pages site: change them with the code. The README's table and `docs/benchmarks.html` come from the same `bench/build.rb` run. diff --git a/README.md b/README.md index 1013c66..4982485 100644 --- a/README.md +++ b/README.md @@ -222,7 +222,8 @@ config.index_now_key = "c1f0e2b4a6d84f3e9b7a5c3d1e0f2a4b" # yours: 8 to 128 ch Each build then publishes the key as `.txt` at the root of the site, and sends, from the files it rewrote, the URLs whose `lastmod` is later than the previous build, and those that appeared or went away whatever their -`lastmod`. The first build sends nothing, and only URLs on `config.host` are sent. +`lastmod`. The first build sends nothing, nor does a new configuration beyond the pages whose `lastmod` moved, +and only URLs on `config.host` are sent. When the sitemaps are served from elsewhere (`config.sitemaps_url`), the key cannot follow them: IndexNow reads it on the site. Put `.txt` in `public/` and set `config.index_now_key_location` to its URL; a build warns diff --git a/docs/guide.html b/docs/guide.html index 60d172e..0b6e1de 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -180,7 +180,7 @@

IndexNow

IndexNow tells Bing, Yandex, Seznam and Naver that a URL changed. Google does not take part. Choose a key of 8 to 128 characters among a-z, A-Z, 0-9 and -:

config.index_now_key = "c1f0e2b4a6d84f3e9b7a5c3d1e0f2a4b"
-

Each build then publishes the key as <key>.txt at the root of the site, and sends, from the files it rewrote, the URLs whose lastmod is later than the previous build, and those that appeared or went away whatever their lastmod. The first build sends nothing, and only URLs on config.host are sent.

+

Each build then publishes the key as <key>.txt at the root of the site, and sends, from the files it rewrote, the URLs whose lastmod is later than the previous build, and those that appeared or went away whatever their lastmod. The first build sends nothing, nor does a new configuration beyond the pages whose lastmod moved, and only URLs on config.host are sent.

When the sitemaps are served from elsewhere (config.sitemaps_url), the key cannot follow them: IndexNow reads it on the site. Put <key>.txt in public/ and set config.index_now_key_location to its URL; a build warns until you do.

A refusal or an endpoint that cannot be reached is a warning, never a failed build: the URLs it did not take are kept in the manifest and sent again with the next build.

diff --git a/lib/mapitaine/builder.rb b/lib/mapitaine/builder.rb index dae2194..e5464a4 100644 --- a/lib/mapitaine/builder.rb +++ b/lib/mapitaine/builder.rb @@ -13,13 +13,13 @@ def changed? MAX_URL_CHARACTERS = 2_048 MAX_FILES_PER_INDEX = 50_000 LAST_TIME_ZONE_OFFSET_SECONDS = 12 * 60 * 60 + DAY_LENGTH = "2026-01-01".length # One request's worth: what a refusing endpoint leaves in the manifest stays small. MAX_UNSUBMITTED_URLS = IndexNow::MAX_URLS_PER_REQUEST # A language, then maybe a script and a region: en, pt-BR, zh-Hant-TW, es-419. HREFLANG = /\A[a-z]{2,3}(-[a-z]{4})?(-([a-z]{2}|\d{3}))?\z/i - # urls: every URL of the shard, kept only when IndexNow needs to know which ones appeared or went away. - Rendered = Data.define(:source_slug, :shard, :fingerprint, :files, :changed_urls, :urls) + Rendered = Data.define(:source_slug, :shard, :fingerprint, :files, :changed_urls) Warning = Data.define(:source, :url, :message) do def to_s @@ -60,17 +60,24 @@ def build_in(directory, full:) @x_default = Xml.hreflang(@configuration.x_default) @rendered = [] - # The first build has no "before": submitting a whole site at once is not what IndexNow is for. + # The first build has no "before": submitting a whole site at once is not what IndexNow is for. Nor is a + # new configuration, which rewrites every file: then only the lastmods tell what changed. @tracking = @submit && @configuration.index_now_key.present? && @previous.built_at.present? + @diffing = @tracking && @previous.digest == @configuration.digest + @moved_sources = Set.new @configuration.sources.each { |source_class| build_source(source_class.new(@configuration)) } publish_rendered publish_index deleted = (@previous.file_names | @previous.pending) - @manifest.file_names - @changed_urls.concat(urls_in(deleted)) if @tracking + @changed_urls.concat(urls_in(deleted.reject { |name| @moved_sources.include?(previous_source_of(name)) })) if @diffing deleted.each { |name| @storage.delete(name) } - submitted = submit_changed_urls + inspect_index_now_key + urls = urls_to_submit + # Written before submitting, with the URLs still to send: a build that dies while submitting loses none. + @manifest.unsubmitted.replace(urls.last(MAX_UNSUBMITTED_URLS)) upload(manifest_name, @manifest.to_json) + submitted = submit(urls) Result.new(full: @full, written: @written, kept: @kept, deleted:, url_count: @manifest.files.sum { |file| file["urls"] }, submitted:, warnings: @warnings) @@ -96,7 +103,7 @@ def keep(source, shard, stored) def render(source, shard, fingerprint, seen_urls) writer = ShardWriter.new(directory: @directory, basename: source.basename(shard)) changed_urls = [] - urls = [] if @tracking + urls = [] if @diffing ActiveSupport::Notifications.instrument("shard.mapitaine", source: source.slug, shard:) do source.each_page(shard) do |page| inspect_page(source, page, seen_urls) @@ -107,18 +114,36 @@ def render(source, shard, fingerprint, seen_urls) writer.close end + if writer.files.all? { |file| same_as_before?(file) } + changed_urls.clear + elsif urls + changed_urls.concat(appeared_or_gone(source.slug, shard, fingerprint, urls)) + end fingerprint ||= writer.content_digest # Recorded now, published once every shard is rendered: the index keeps the order of the sources. @manifest.record(source.slug, shard, fingerprint:, files: []) - @rendered << Rendered.new(source_slug: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:, urls:) + @rendered << Rendered.new(source_slug: source.slug, shard:, fingerprint:, files: writer.files, changed_urls:) + end + + # Read while the previous files are still in the storage: a page that appeared or went away is a change too, + # whatever its lastmod says. Not when the ranges moved, which only carries pages from one file to another. + def appeared_or_gone(source_slug, shard, fingerprint, urls) + stored = @previous.shard(source_slug, shard) + if stored && fingerprint && stored["fingerprint"].to_s.split("/").first != fingerprint.split("/").first + @moved_sources << source_slug + return [] + end + + previous_urls = urls_in((stored&.fetch("files") || []).map { |file| file["name"] }).to_set + urls = urls.to_set + (urls - previous_urls).to_a + (previous_urls - urls).to_a end def publish_rendered changed_names = @rendered.flat_map(&:files).reject { |file| same_as_before?(file) }.to_set(&:name) announce_new_files(changed_names) @rendered.each do |rendered| - changed = rendered.files.any? { |file| changed_names.include?(file.name) } - note_changed_urls(rendered) if changed && @tracking + @changed_urls.concat(rendered.changed_urls) entries = rendered.files.map do |file| changed = changed_names.include?(file.name) publish(file, changed) @@ -128,13 +153,11 @@ def publish_rendered end end - # Read before the new files replace the old: a page that appeared or went away is a change too, whatever its - # lastmod says. - def note_changed_urls(rendered) - previous_names = @previous.shard(rendered.source_slug, rendered.shard)&.fetch("files")&.map { |file| file["name"] } || [] - previous_urls = urls_in(previous_names).to_set - urls = rendered.urls.to_set - @changed_urls.concat(rendered.changed_urls, (urls - previous_urls).to_a, (previous_urls - urls).to_a) + def previous_source_of(file_name) + @previous_source_by_file_name ||= @previous.shards_by_source.each_with_object({}) do |(source_slug, shards), source_by_name| + shards.each_value { |stored| stored["files"].each { |file| source_by_name[file["name"]] = source_slug } } + end + @previous_source_by_file_name[file_name] end def urls_in(names) @@ -258,27 +281,28 @@ def on_host?(url) url.start_with?("#{@configuration.host!}/") || url == @configuration.host! end - # A date alone may still be the day of the last build somewhere: it changed if it is not older than the date - # at UTC-12, the last time zone to leave a day. + # A day may still be the day of the last build somewhere: it changed if it is not older than the date at + # UTC-12, the last time zone to leave a day. A month or a year alone says too little to tell. def changed_since_last_build?(page) return false unless page.lastmod return page.modified_at >= @previous.built_at if page.lastmod.include?("T") - last_date = (@previous.built_at - LAST_TIME_ZONE_OFFSET_SECONDS).utc.to_date.iso8601 - page.lastmod >= last_date[0, page.lastmod.length] + page.lastmod.length == DAY_LENGTH && page.lastmod >= (@previous.built_at - LAST_TIME_ZONE_OFFSET_SECONDS).utc.to_date.iso8601 end - # Returns how many URLs the endpoint took. The ones it did not take, refused or unreachable, are kept in the - # manifest and sent again with the next build: the sitemaps are published, so a refusal never fails the build. - def submit_changed_urls - key = @configuration.index_now_key - return 0 unless @submit && key + def urls_to_submit + return [] unless @submit && @configuration.index_now_key + + (@previous.unsubmitted + @changed_urls).uniq.select { |url| on_host?(url) } + end - publish_index_now_key(key) - urls = (@previous.unsubmitted + @changed_urls).uniq.select { |url| on_host?(url) } + # Returns how many URLs the endpoint took. The others, refused or unreachable, stay in the manifest for the + # next build: the sitemaps are published, so a refusal never fails the build. + def submit(urls) return 0 if urls.empty? - replies = IndexNow.new(host: @configuration.host!, key:, key_location: @configuration.index_now_key_location).submit(urls) + replies = IndexNow.new(host: @configuration.host!, key: @configuration.index_now_key, + key_location: @configuration.index_now_key_location).submit(urls) refused = replies.reject(&:accepted?) refused.group_by(&:status).each do |status, replies_with_status| count = replies_with_status.sum { |reply| reply.urls.size } @@ -286,12 +310,16 @@ def submit_changed_urls warn("indexnow", IndexNow::ENDPOINT, "#{answer} for #{count} URLs, sent again with the next build") end @manifest.unsubmitted.replace(refused.flat_map(&:urls).last(MAX_UNSUBMITTED_URLS)) + upload(manifest_name, @manifest.to_json) replies.select(&:accepted?).sum { |reply| reply.urls.size } end # IndexNow reads the key at the root of the site unless told where it is, and only on the site's host. - def publish_index_now_key(key) + def inspect_index_now_key + key = @configuration.index_now_key location = @configuration.index_now_key_location + return unless key + if location warn("indexnow", location, "is not on #{@configuration.host!}: IndexNow only reads a key there") unless on_host?(location) elsif @configuration.sitemaps_url == @configuration.host! diff --git a/lib/mapitaine/storage/local.rb b/lib/mapitaine/storage/local.rb index 29f4051..80ef510 100644 --- a/lib/mapitaine/storage/local.rb +++ b/lib/mapitaine/storage/local.rb @@ -13,7 +13,7 @@ def initialize(directory) def read(name) path = File.join(@directory, name) - File.read(path) if File.exist?(path) + File.binread(path) if File.exist?(path) end # Copied beside the target, then renamed: a crawler never downloads half a file. diff --git a/test/index_now_test.rb b/test/index_now_test.rb index d1d1672..80c9fae 100644 --- a/test/index_now_test.rb +++ b/test/index_now_test.rb @@ -97,6 +97,51 @@ def test_pages_that_appear_or_go_away_are_submitted_whatever_their_lastmod https://example.com/fr/posts/2 https://example.com/en/posts/12 https://example.com/fr/posts/12], @requests.last.dig(1, "urlList") end + def test_a_new_configuration_or_new_ranges_send_only_what_changed + create_post(5) + create_post(15) + build + Mapitaine.configuration.locales = %i[en fr de] + + assert_equal 0, build.submitted + Mapitaine.configuration.sources = [Class.new(PostSitemap) { shard_size 20 }, PageSitemap] + Post.find(5).update!(title: "Edited", updated_at: Time.now) + + build + + assert_equal %w[https://example.com/en/posts/5 https://example.com/fr/posts/5], @requests.last.dig(1, "urlList") + end + + def test_a_build_that_dies_while_submitting_leaves_its_urls_for_the_next + create_post(1) + build + Post.find(1).update!(title: "Edited", updated_at: Time.now) + self.network_error = Net::HTTPBadResponse + + assert_raises(Net::HTTPBadResponse) { build } + self.network_error = nil + + assert_equal 2, build.submitted + end + + def test_a_year_alone_is_not_a_change + create_post(1) + create_post(2) + yearly = Class.new(PostSitemap) { lastmod { "2026" } } + Mapitaine.configuration.sources = [yearly] + build + Post.where(id: 2).delete_all + build + + assert_equal %w[https://example.com/en/posts/2 https://example.com/fr/posts/2], @requests.last.dig(1, "urlList") + end + + def test_check_reports_a_key_the_site_does_not_serve + Mapitaine.configuration.sitemaps_url = "https://cdn.example.com/sitemaps" + + assert_equal 1, Mapitaine.check.size + end + def test_a_date_of_the_day_of_the_last_build_is_a_change create_post(1) dated = Class.new(PostSitemap) { lastmod { |post| post.reviewed_at&.to_date || Date.new(2026, 1, 1) } } @@ -158,6 +203,21 @@ def test_urls_go_in_batches_of_ten_thousand assert replies.all?(&:accepted?) end + def test_each_batch_gets_its_own_reply + statuses = ["202", Errno::ECONNRESET] + transport = lambda do |_uri, _body, _headers| + status = statuses.shift + raise status if status.is_a?(Class) + + Response.new(code: status) + end + replies = Mapitaine::IndexNow.new(host: "https://example.com", key: "key", transport:) + .submit(Array.new(10_001) { |number| "https://example.com/#{number}" }) + + assert_equal [202, "Errno::ECONNRESET"], replies.map(&:status) + assert_equal [true, false], replies.map(&:accepted?) + end + def test_check_submits_nothing create_post(1) build From a572e8fa1f4e324a75c1658eea748fe21a74df48 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 08:03:50 +0000 Subject: [PATCH 8/8] Write test manifests without Active Support's encoder The ruby-head job still failed on the two tests that rewrite a manifest with Hash#to_json, which Active Support 8.0 routes through its encoder. Assisted-by: Claude Code --- test/build_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/build_test.rb b/test/build_test.rb index 73ccc9d..462ba8e 100644 --- a/test/build_test.rb +++ b/test/build_test.rb @@ -163,7 +163,7 @@ def test_a_file_dated_by_an_older_manifest_is_compared_by_time build manifest = JSON.parse(storage.content_by_name["sitemap-manifest.json"]) manifest["sources"]["post"]["0"]["files"][0]["lastmod"] = "2026-01-15T10:00:00+09:00" - storage.content_by_name["sitemap-manifest.json"] = manifest.to_json + storage.content_by_name["sitemap-manifest.json"] = JSON.generate(manifest) Post.where(id: 1).update_all(title: "Edited", updated_at: Time.utc(2026, 1, 15, 6)) build @@ -199,7 +199,7 @@ def test_a_full_build_comes_every_week_by_itself build manifest = JSON.parse(storage.content_by_name["sitemap-manifest.json"]) manifest["rebuilt_at"] = (Time.now - 8 * 86_400).utc.iso8601 - storage.content_by_name["sitemap-manifest.json"] = manifest.to_json + storage.content_by_name["sitemap-manifest.json"] = JSON.generate(manifest) built = build @@ -207,7 +207,7 @@ def test_a_full_build_comes_every_week_by_itself assert_equal %w[sitemap-post-00000.xml.gz sitemap-page.xml.gz], built.written refute_predicate build, :full Mapitaine.configuration.rebuild_every = nil - storage.content_by_name["sitemap-manifest.json"] = manifest.to_json + storage.content_by_name["sitemap-manifest.json"] = JSON.generate(manifest) assert_empty build.written end