Catch more changes between builds, and date files by their last change

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.

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
@@ -148,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
@@ -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 (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
+`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
 

docs/guide.html

@@ -135,15 +135,19 @@ <h2 id="building-title">Building</h2>
       <div class="prose">
 <pre><code class="language-sh">bin/rails mapitaine:build     <span class="c"># rewrite the files whose records changed</span>
 bin/rails mapitaine:rebuild   <span class="c"># rewrite everything</span></code></pre>
-        <p><code>Mapitaine.build</code> does the same from Ruby and returns what it did: <code>written</code>, <code>kept</code>, <code>deleted</code>, <code>url_count</code>, <code>submitted</code> and <code>warnings</code>. Run it on a schedule, every night or every hour. With Solid Queue:</p>
+        <p><code>Mapitaine.build</code> does the same from Ruby and returns what it did: <code>full</code>, <code>written</code>, <code>kept</code>, <code>deleted</code>, <code>url_count</code>, <code>submitted</code> and <code>warnings</code>. Run it on a schedule, every night or every hour. With Solid Queue:</p>
 <pre><code class="language-yaml"><span class="c1"># config/recurring.yml</span>
 <span class="na">production</span><span class="pi">:</span>
   <span class="na">sitemap</span><span class="pi">:</span>
     <span class="na">command</span><span class="pi">:</span> <span class="s2">"</span><span class="s">Mapitaine.build"</span>
     <span class="na">schedule</span><span class="pi">:</span> <span class="s">every hour</span></code></pre>
         <h3>How a build decides</h3>
-        <p>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 <code>update_all</code>, imports and deletions are seen too.</p>
-        <p>A build writes the sitemap files first, then the index, then removes the files no range fills any more, then writes <code>sitemap-manifest.json</code>, 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.</p>
+        <p>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 <code>update_all</code>, imports and deletions are seen too.</p>
+        <p>In the index, a file that changed takes the latest <code>lastmod</code> it holds, or the time of the build when a record left it, so that crawlers read it again.</p>
+        <p>Some changes leave those numbers alone: <code>update_columns</code>, a change in another table that the <code>url</code>, <code>locales</code> or <code>images</code> blocks read, a new <code>url</code> block, a file someone removed from the storage. Name the other table in <code>fingerprint</code>, or <code>touch</code> the record from it. For the rest, a full build comes once a week by itself:</p>
+<pre><code class="language-ruby"><span class="n">config</span><span class="p">.</span><span class="nf">rebuild_every</span> <span class="o">=</span> <span class="mi">1</span><span class="p">.</span><span class="nf">day</span>   <span class="c1"># default: a week; nil leaves it to mapitaine:rebuild</span></code></pre>
+        <p>After deploying a change to a <code>url</code> block, run <code>mapitaine:rebuild</code> rather than wait.</p>
+        <p>A build notes the files it is about to add in <code>sitemap-manifest.json</code>, 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.</p>
       </div>
     </section>
 

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,6 +15,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_slug, :shard, :fingerprint, :files, :changed_urls)
+
     Warning = Data.define(:source, :url, :message) do
       def to_s
         "#{source}: #{url} #{message}"
@@ -29,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
 
@@ -37,8 +42,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,13 +55,16 @@ 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)
 
-      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
 
@@ -64,7 +75,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 +85,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 +98,74 @@ 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_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|
+        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
+
+    def same_as_before?(file)
+      @previous_file_by_name.dig(file.name, "digest") == file.digest
+    end
 
-      publish(source, shard, fingerprint, writer.files)
-      @changed_urls.concat(changed_urls)
+    # 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?
+
+      @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) && !later?(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
+
+    # 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)
     end
 
     def publish_index

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
 
@@ -38,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

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

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