nandi icon indicating copy to clipboard operation
nandi copied to clipboard

Add runtime assertions for safe-in-practice operations

Open tragiclifestories opened this issue 4 years ago • 3 comments

Motivation

Almost all operations that are unsafe in the general case may be safe in practice under certain conditions.

For example: when a new feature has a new table or tables to go with it, they tend to change a lot as we iterate. Until the table is actually being used by the app, all changes are effectively safe (except adding a foreign key constraint that involves some other table maybe). We have one mechanism for dealing with this (.nandiignore). It would be good if it wasn't such a binary choice.

Proposal

Add a new layer of runtime assertions that will be run at the beginning of the migration.

Example

# say we have defined a foo field as a number, but we decide that 
# a string is more appropriate. This is unsafe unless the table is empty
class MakeFooText < Nandi::Migration
  def up
    assert! :table_is_empty, :my_new_table
    
    change_column :foo, type: :text
  end
end

With this feature, the above migration would be valid, but would raise an error at runtime if the the table turned out to have data in it.

tragiclifestories avatar Jun 04 '20 10:06 tragiclifestories

Yes, please! Is this a reasonably easy change to make for someone that doesn't know Nandi? I'd be happy to contribute this

dmagliola avatar Jun 04 '20 11:06 dmagliola

I think this is probably quite a big change, but totally open to PRs.

My own initial idea is:

  1. define the assert! method on Nandi::Migration, which is going to build up a data structure with all the assertions in. Let's say for starters we only implement the :table_is_empty condition.

  2. Come up with a way to mark particular instructions as safe when the assertion is true. Do that for basically everything.

  3. Make the validator respect that.

  4. Implement the assertions in the compiled output.

tragiclifestories avatar Jun 04 '20 12:06 tragiclifestories

Could you make your life slightly easier by going:

change_column :table, :foo, type: :text, assert_empty_table: true

And then the validator already knows it's safe because you're going to assert, and the code generator generates the assert.

It'll be more verbose if you're doing multiple things because you have to write that multiple times, and you'll also end up checking the empty table is empty multiple times, but it sounds much easier to implement that way :)

dmagliola avatar Jun 04 '20 14:06 dmagliola