erikbra / grate

grate - the SQL scripts migration runner
MIT License
209 stars 40 forks source link

Grate internal scripts fail when using CreateDatabase scripts #512

Closed JaDuyve closed 6 months ago

JaDuyve commented 6 months ago

Describe the bug The grate internal structure scripts fail when using CreateDatabase scripts. From what I can see, it's caused because the DbMigrator instance is reused from the CreateDatabase scripts. This instance already has the private instance of _tokens initialized. Because it's already initialized, the custom UserTokens for the grate internal scripts will not be added to _tokens causing TokenReplacer not replacing all supposed tokens and intern, the script will fail.

These tokens aren't taken into account. https://github.com/erikbra/grate/blob/a5d3212f8d19e6af031207916a63c0435339cfeb/src/grate.core/Migration/GrateMigrator.cs#L576-L587

Because _tokens is already initialized.

https://github.com/erikbra/grate/blob/a5d3212f8d19e6af031207916a63c0435339cfeb/src/grate.core/Migration/DbMigrator.cs#L249-L254

I think this is issue will occur when anything has run before grate structure.

Could the solution be to create for the grate internal scripts a new instance of GrateMigrator?

To Reproduce Run grate for new database with CreateDatabase scripts.

Expected behavior Grate internal structure scripts run without errors.

Screenshots

Initializing connections.
Running grate v1.0.0 (build date 05/06/2024 23:17:39) against localhost,1435 - TestDatabase.
Looking in D:\Repos\TestDatabase for scripts to run.
================================================================================
Setup, Backup, Create/Restore/Drop
================================================================================
  Running '20240311_01_Config.sql'.
  Running '20240311_02_SetConfig.ENV.LOCAL.sql'.
  Running '20240311_03_CreateLogin.sql'.
  Running '20240311_04_CreateDatabase.sql'.
  Running '20240311_05_Wait.sql'.
================================================================================
Grate Structure
================================================================================
Initializing connections.
Running grate v1.0.0 (build date 05/06/2024 23:17:39) against localhost,1435 - TestDatabase.
Looking in C:\Users\jaduyve\AppData\Local\Temp\tmpd4ubxq.tmp for scripts to run.
================================================================================
Setup, Backup, Create/Restore/Drop
================================================================================
================================================================================
Grate Structure
================================================================================
================================================================================
Versioning
================================================================================
 Migrating TestDatabase from version 0.0.0.0 to 1.0.0.
 Versioning TestDatabase database with version 1.0.0.
================================================================================
Migration Scripts
================================================================================
Skipping 'BeforeMigration', beforeMigration does not exist.
Skipping 'AlterDatabase', alterDatabase does not exist.
Skipping 'Run After Create Database', runAfterCreateDatabase does not exist.
Skipping 'Run Before Update', runBeforeUp does not exist.

Looking for Update scripts in "C:\Users\jaduyve\AppData\Local\Temp\tmpd4ubxq.tmp\up". These should be one time only scripts.
--------------------------------------------------------------------------------
  Running 'grate-internal/01_create_schema_grate.ENV.GrateInternalBoostrap-a61456d0-e00a-4933-b692-c6a5d7d51539.sql'.
  Running 'grate-internal/02_create_scripts_run_table.sql'.
grate-internal/02_create_scripts_run_table.sql: Incorrect syntax near '{'.
Skipping 'Permissions', permissions does not exist.
Skipping 'AfterMigration', afterMigration does not exist.
An error occurred: Invalid object name 'grate.GrateScriptsRun'.

Desktop (please complete the following information):

erikbra commented 6 months ago

Aha! Good catch! I thought I had tests covering this, but apparently not (strange...). I'll try to write some, and fix the issue.

erikbra commented 6 months ago

I think PR #514 should solve this, do you agree? I make sure to set the _tokens to null whenever I clone the DbMigrator

JaDuyve commented 6 months ago

@erikbra This fixes the issue I'm having. Thanks a lot!

erikbra commented 6 months ago

Lovely, @JaDuyve! It is included in release 1.7.2. Thanks for a detailed bug report and solid analysis 😄