Pending drops migration with the same version of the target migration cause trouble

[cleidiano]

Expected behavior

Ebean generate migration successfully.

Actual behavior

Ebean are allowing the user to set the ddl.migration.version and ddl.migration.pendingDropsFor with the same version, this cause trouble since the generated files will have the same vertion that the pending drops is target on.
The current behavior is that Ebean crash with a NullPointException in the next time a migration generation is launched, I inspect the source code and figure out that is happened because Ebean are loading the pending drops model before the migration that is the source of the pending drops for.
Should Ebean fail to prevent this behavior when generate a migration with those property in such stste or fix the order that migration are loaded and continue allowing user to do that?

Steps to reproduce

1. Drop a column from any entity
2. Generate a migration with the version property set, e.g System.setProperty("ddl.migration.version", "1.0")
3. Generate a next migration for pending drops with the version and pending drops property set with the same version that is used before, e.g System.setProperty("ddl.migration.version", "1.0"); System.setProperty("ddl.migration.pendingDropsFor", "1.0");

That step will produce the files and succeed:

1.0.model.xml
1.0__dropsFor_1.0.model.xml
-----
1.0__dropsFor_1.0.sql

4. Make any change on any entity
5. Try to generate a next migration

The step 5 result will be something like:

Exception in thread "main" java.lang.NullPointerException
	at io.ebeaninternal.dbmigration.model.PendingDrops.appliedDropsFor(PendingDrops.java:52)
	at io.ebeaninternal.dbmigration.model.ModelContainer.apply(ModelContainer.java:132)
	at io.ebeaninternal.dbmigration.model.MigrationModel.readMigrations(MigrationModel.java:57)
	at io.ebeaninternal.dbmigration.model.MigrationModel.read(MigrationModel.java:36)
	at io.ebeaninternal.dbmigration.DefaultDbMigration$Request.<init>(DefaultDbMigration.java:526)
	at io.ebeaninternal.dbmigration.DefaultDbMigration$Request.<init>(DefaultDbMigration.java:506)
	at io.ebeaninternal.dbmigration.DefaultDbMigration.createRequest(DefaultDbMigration.java:503)
	at io.ebeaninternal.dbmigration.DefaultDbMigration.generateMigrationFor(DefaultDbMigration.java:329)
	at io.ebeaninternal.dbmigration.DefaultDbMigration.generateMigration(DefaultDbMigration.java:288)

[rbygrave]

Should Ebean fail to prevent this behavior when generate a migration with those property in such

Yes. I think Ebean should throw a nice error here saying that they can't be the same.

[rbygrave]

There is a second case where we generate a migration that is drops only. In this case a model.xml is generated but no migration sql is generated (as it would be empty).

e.g.

1.0.model.xml
1.1.model.xml  // only contains drops

1.0.sql

At this point we maybe want to generate a 1.1 migration (from the 1.1 drops) but we can't do that.

There might be a better way to deal with this case by detecting when a "drops only" migration is generated and not "consuming" a version number for that. Like:

1.0.model.xml
1.1.D__dropsOnly.model.xml  // only contains drops

1.0.sql

So 1.1.D model only contains drops and doesn't "consume" the 1.1 version number. We could generate "1.1" for pendingDropsFor "1.1.D" ... or something like that.

Summary

Yes, we can't use the same version number as pendingDropsFor - so will fix that by detected that and throwing a reasonable exception.

In future we might get an improve for the case where a migration is generated that is for drops only such that it doesn't effectively consume a version number - but this does need some more thinking.

[cleidiano]

Thank you for the quick answer and fix!