Skip to content

Add Grails 8 database migration guide (v8 prose, snippets, guides.yml)#523

Open
sanjana2505006 wants to merge 1 commit into
apache:masterfrom
sanjana2505006:grails-database-migration-v8-guide
Open

Add Grails 8 database migration guide (v8 prose, snippets, guides.yml)#523
sanjana2505006 wants to merge 1 commit into
apache:masterfrom
sanjana2505006:grails-database-migration-v8-guide

Conversation

@sanjana2505006

Copy link
Copy Markdown
Contributor

@jamesfredley

Copy link
Copy Markdown
Contributor

Thanks @sanjana2505006 - this is a strong contribution. I reviewed the guide prose against the companion sample app in full: built it via CI, cross-checked every prose↔snippet include, verified the plugin commands/config against the Apache Grails source, and did a comprehensiveness pass on the migration coverage. The core arc is solid and everything is factually correct about the plugin.

Setup done on my side: I created the grails8 branch on grails-guides/grails-database-migration (seeded from your sample app, your commits preserved), so sampleRef now resolves and CI is green there (initial + complete unit tests, complete integration tests all pass).

Important

Overlapping changes need a companion PR. Several items below change files the guide include::s as snippets (domains, migrations, tests, config). Those live in the sample app repo, so please mirror any such change in a companion PR against the grails8 branch of grails-guides/grails-database-migration, so the published guide and the runnable sample never drift. Items tagged [overlapping] below belong in that companion PR (and this one).


1. Sample-app correctness - [overlapping] (companion PR on grails-guides repo)

  • [MAJOR] Rollback is advertised but breaks. The guide shows dbm-rollback-count 1, but the redesignTables phase (raw-SQL data copy + three dropColumn changesets) has no rollback {} blocks, so dbm-rollback fails on drop-person-zip-1 and would lose data even if it ran. Either add explicit rollback {} blocks (re-add columns + restore from address) or reframe as expand-migrate-contract and scope the rollback claim to the reversible steps. Good chance to teach designed rollback (see §2).
  • [MAJOR] Address ownership contradiction. create-address-table.groovy makes address.person_id NOT NULL, but AddressSpec asserts new Address().validate() is true (no person). An address validates yet can never persist. Pick one contract - recommended: mandatory ownership (person nullable: false, keep person_id NOT NULL, and change AddressSpec to expect a validation error).
  • [MAJOR] Integration test never proves the headline feature. DatabaseMigrationIntegrationSpec only checks the final DDL on a clean DB. It never seeds a legacy person row before migrate-address-data-1, so it doesn't verify the data-migration SQL actually preserves street_name/city/zip. Add a migration-path test: apply through add-address-fields-to-person, seed legacy data, apply the rest, then assert the address row matches and the legacy columns are gone.
  • [MINOR] initial/src/test/resources/application-test.yml sets updateOnStart: true pointing at a changelog.groovy that doesn't exist in initial/ yet - a trap for anyone adding an @Integration test there. Set updateOnStart: false until the guide generates the baseline.
  • [MINOR] initial/ ApplicationSpec only asserts true; the advertised ./gradlew test proves nothing. Replace with DomainUnitTest<Person> checks (name + age required).

2. Comprehensiveness - the part we care most about - [overlapping]

Current coverage is good (GORM baseline, nullable via dbm-gorm-diff, add columns, table redesign + data SQL, basic rollback, unit + integration tests). To make this the definitive database-migration guide it should be, these are the must-adds (each touches both the prose and the sample app, so → companion PR too):

  • Release runbook: inspect → preview → apply. dbm-status, then dbm-update-sql pending.sql to preview, then dbm-update. Note updateOnStart is fine for local/single-instance, but production should run migrations once before rolling out multiple app instances.
  • Rollback as a designed capability. rollback {} blocks, tag a known-good release with dbm-tag, preview with dbm-rollback-sql <tag>, execute dbm-rollback <tag>. (This also resolves the §1 rollback issue.)
  • Legacy / existing-DB onboarding. Contrast dbm-generate-gorm-changelog (from domains) with dbm-generate-changelog (snapshot a live schema), then dbm-changelog-sync to mark it applied without DDL. Covers the very common "prod DB already exists" case.
  • Changelog discipline / immutability. Append-only includes in release order, one changeset per file with unique author+id, never edit an applied changeset (add a corrective one), and what dbm-clear-checksums is/isn't for.
  • Preconditions. Guard the destructive redesign with preConditions(onFail: 'HALT') { tableExists(...); columnExists(...) } so drift/wrong ordering fails loudly instead of damaging data.

Nice follow-ons if you want to go further: indexes/unique constraints via migrations, contexts for env-specific changesets (--contexts), and multiple-datasource migrations. (All command names above verified against the current plugin reference.)

3. Guide prose / metadata - this PR only

  • [MINOR] Command style is inconsistent: the prose uses ./gradlew runCommand "-Pargs=dbm-*" while the sample README leads with ./grailsw dbm-*. Both are valid - please standardize on one (suggest ./grailsw dbm-* as primary, the Gradle form as the CI/automation fallback).
  • [MINOR] You authored the guide but aren't in guides.yml authors: (only Nirav Assar + Sergio del Amo are listed on the parent). Please add yourself to the parent authors: list.
  • FYI - no change needed: the requirements: What you will need bare-scalar TOC key is valid (YamlTocStrategy has a string overload; grails-spring-security v8 uses the same form).

4. Verified correct - no action (FYI)

Checked against Apache Grails source: plugin coordinate org.apache.grails:grails-data-hibernate5-dbmigration ✅, ./gradlew runCommand "-Pargs=dbm-*" is a real task ✅, all dbm-* commands exist ✅, updateOnStart / updateOnStartFileName / dbCreate: none correct ✅, Groovy changelog DSL (dropNotNullConstraint, addForeignKeyConstraint, etc.) valid ✅. All 12 TOC keys map 1:1 to the guide files, every snippet include resolves, and GET /api/persons is wired up.


Happy to help get the companion sample-app PR up against grails8 if that's easier. Nice work overall - the bones here are very good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants