Geo: Fix generate-blob-replicator script for upload partition FK strategy, indentation, and factory naming

Summary

The scripts/geo/generate-blob-replicator script (introduced in !224164 (merged), #589925) has three bugs that affect upload partition replicators (--upload-partition mode). These were discovered while using the script to add replication for Projects::Topic uploads (#589917 (closed)).

Bug 1: FK to uploads fails — should FK to partition table (RESOLVED)

Severity: Blocks migration execution.

The script generates a foreign key from the *_states table to :uploads:

add_concurrent_foreign_key :project_topic_upload_states, :uploads, column: :project_topic_upload_id, on_delete: :cascade

This fails with:

PG::InvalidForeignKey: ERROR: there is no unique constraint matching given keys for referenced table "uploads"

Because uploads is now a partitioned table with a composite PK (id, model_type) — there's no unique constraint on just id.

Fix: Match the pattern used in !229562 (merged), !229569 (merged), and !229718 (merged):

  1. Add a unique index on the partition table's id column first: 
    add_concurrent_index :project_topic_uploads, :id, unique: true, name: "idx_project_topic_uploads_on_id",
  allow_partition: true
  2. FK to the partition table directly (not :uploads): 
    add_concurrent_foreign_key :project_topic_upload_states, :project_topic_uploads,
  column: :project_topic_upload_id, on_delete: :cascade
  3. Add remove_concurrent_index_by_name in down method.

Affected code: generate_state_migration method — the state_foreign_key_options and fk_after_create logic for the upload_partition branch.

Related: #595702 (closed) (FK strategy discussion for upload partition states tables).

Bug 2: Replicator include indentation (minor)

The upload_model_validation_include method (line ~664) produces:

    include ::Geo::BlobReplicatorStrategy
            include ::Geo::Concerns::UploadReplicatorBehavior

The second include has extra leading whitespace. Should be:

    include ::Geo::BlobReplicatorStrategy
    include ::Geo::Concerns::UploadReplicatorBehavior

Fix: Change the newline prefix in upload_model_validation_include from "\n include" to "\n include".

Bug 3: Factory parent model name derivation may pick wrong factory

The script derives parent_model_factory_name by stripping _upload from the replicable name:

def parent_model_factory_name
  replicable_name.sub(/_upload$/, '')
end

For project_topic_upload, this produces project_topic, but:

  • :project_topic = Projects::ProjectTopic (the join table)
  • :topic = Projects::Topic (the actual model we need)

This also affects the generated model spec's selective sync fixtures which use the same derived name.

Possible fixes:

  • Add a --parent-factory CLI option to explicitly specify the factory name
  • Print a warning after generation reminding the user to verify the factory name
  • Or both

Additional: Index name prefix convention

The merged MRs (!229562 (merged), !229569 (merged), !229718 (merged)) use shortened idx_ prefixed index names in the state migration (e.g. idx_project_topic_upload_states_on_verification_state) rather than the full index_ prefix the script currently generates. Consider aligning with this convention since long table names can exceed PostgreSQL's 63-character identifier limit.

Related

Edited by Scott Murray