robotoworks / mechanoid

Eclipse plugin providing a set of DSL's for the rapid development of Android apps
58 stars 26 forks source link

no migration executed #197

Closed hannesa2 closed 11 years ago

hannesa2 commented 11 years ago

I made a file with several migrations, but no one will be done. I found this in MechanoidSQLiteOpenHelper.java but it will never be called

@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
    for(int i = (oldVersion + 1); i <= newVersion; i++) {
        createMigration(i).up(db);
    }
}

this is my mech file

migration {
    create table tab1 (
        _id integer primary key autoincrement,
        …
    }
    migration {
        create index ix_2 on tab1 (field_id);
    }
    migration {
        create index ix_3 on tab1 (field2_id);
    }   
}

any idea what's wrong here ?

fluxtah commented 11 years ago

Hi @hannesa2

I tried this and the migrations are generated fine, the following generates the migrations to src-gen/qux/migrations

package qux

database Quxxy {
    migration {
        create table tab1 (
            _id integer primary key autoincrement
        );

    }

    migration {
        create index ix_2 on tab1 (_id);
    }
    migration {
        create index ix_3 on tab1 (_id);
    }   
}

Can you paste the full mechdb file that is causing the problem?

hannesa2 commented 11 years ago

Yes, also in my project the files are generated in src-gen, but never called during start/runtime. At least my breakpoint does no stop.

package qux

database Quxxy {
    migration {
        create table tab1 (
            _id integer primary key autoincrement,
            name text
        );
    }

    migration {
        create index ix_2 on tab1 (_id);
    }
    migration {
        create index ix_3 on tab1 (_id);
    }   
    migration {
        alter table tab1 add column abc text;
    }   
    migration {
        create table tab2 (
            _id integer primary key autoincrement,
            name text
        );
    }
}

The result is confusing: Tab1 contains field abc, but without index Tab2 exists But no records are stored ! (to check if just my breakpoints run into troubles)

public class QuxxyOpenHelper extends AbstractQuxxyOpenHelper {
    public QuxxyOpenHelper(Context context) {
        super(context);
    }

    @Override
    protected SQLiteMigration createQuxxyMigrationV2() {
        return new DefaultQuxxyMigrationV2() {
            @Override
            public void up(SQLiteDatabase db) {
                super.up(db);
                Tab1.newBuilder().setName("2").insert();
            }
        };
    }

    @Override
    protected SQLiteMigration createQuxxyMigrationV3() {
        return new DefaultQuxxyMigrationV3() {
            @Override
            public void up(SQLiteDatabase db) {
                super.up(db);
                Tab1.newBuilder().setName("3").insert();
            }
        };
    }

    @Override
    protected SQLiteMigration createQuxxyMigrationV4() {
        return new DefaultQuxxyMigrationV4() {
            @Override
            public void up(SQLiteDatabase db) {
                super.up(db);
                Tab1.newBuilder().setName("4").insert();
            }
        };
    }

    @Override
    protected SQLiteMigration createQuxxyMigrationV5() {
        return new DefaultQuxxyMigrationV5() {
            @Override
            public void up(SQLiteDatabase db) {
                super.up(db);
                Tab1.newBuilder().setName("5").insert();
            }
        };
    }
}
fluxtah commented 11 years ago

Migrations will only ever be applied on subsequent installs so if you are fresh installing then only onCreate() will be called since this has a snapshot of the entire database model (it builds this from all migrations).

When upgrading from a non-fresh install, it will apply the migrations, for instance if you installed with 2 migrations, and run the app, but later add 3 more migrations, then the onUpgrade will be called from version 2 to 5.

hannesa2 commented 11 years ago

Believe me , I did several tests, starting from v3 to v5, and later I deleted the app completely from my phone to see what's happen during new install, means V1 to v5. No success at all

fluxtah commented 11 years ago

RIght, not sure what is happening here but for new installs you would not see any migrations since its all in onCreate(...), I am not sure if this is the problem but thats how it currently works. I will see if I can replicate the issue.

hannesa2 commented 11 years ago

Here is an example project https://github.com/hannesa2/mechanoid.migration , I expect 5 records inserted during upgrade, but there are no records inserted

fluxtah commented 11 years ago

Hi @hannesa2

I checked the example app and got the migration routine executing.

Migrations will only ever be executed for subsequent upgrades, for instance for V1:

database Quxxy {
    migration {
        create table tab1 (
            _id integer primary key autoincrement,
            name text
        );
    }
}

Then run the app, making sure the db is accessed.

Then add a new migration for V2:

database Quxxy {
    migration {
        create table tab1 (
            _id integer primary key autoincrement,
            name text
        );
    }

    migration {
        create index ix_2 on tab1 (_id);
    }
}

Then run the app, making sure the db is accessed, this will trigger the migration routine (onUpgrade) from 1 to 2.

etc. For all migrations.

The issue here is that for fresh installs, if you have for example 10 migrations, then the code generator will optimize them into one schema snapshots and generate the database schema into the onCreate(...) method of the open helper.

There are a few other things I noticed and that is indexes seem to be broken for fresh installs as no index statements are generated into onCreate(...), will log an issue for this.

Also using the record builders in migrations causes a runtime exception I have not seen before.

If you are wanting to create data based off a migration it might be best to initialize that data in the SqliteOpenHelper onOpen(...) method as a workaround.

hannesa2 commented 11 years ago

Thank you ! As I understand now, it's not guaranteed the call of each individual migration-event, am I right ?

protected SQLiteMigration createQuxxyMigrationV3() {...
protected SQLiteMigration createQuxxyMigrationV4() {...
protected SQLiteMigration createQuxxyMigrationV5() {...

My wish is a guaranteed call of each of them, if it's possible and sense full !

As workaround I understand this, am I right ?

public class QuxxyOpenHelper extends AbstractQuxxyOpenHelper {
    public QuxxyOpenHelper(Context context) {
        super(context);
    }

    @Override
    public void onOpen(SQLiteDatabase db) {
        super.onOpen(db);
        switch (QuxxyOpenHelper.VERSION) {
        case 1: 
            //TODO
            break;
        case 2:
            //TODO
            break;
        ...
        }
    }
fluxtah commented 11 years ago

Yes only guaranteed for upgrading users. Your workaround looks reasonable and should work.

Sent from Samsung Mobile

-------- Original message -------- From: hannesa2 notifications@github.com Date:
To: robotoworks/mechanoid mechanoid@noreply.github.com Cc: Ian Warwick fluxtah@hotmail.com Subject: Re: [mechanoid] no migration executed (#197)

Thank you ! As I understand now, it's not guaranteed the call of each individual migration-event, am I right ?

protected SQLiteMigration createQuxxyMigrationV3() {... protected SQLiteMigration createQuxxyMigrationV4() {... protected SQLiteMigration createQuxxyMigrationV5() {... My wish is a guaranteed call of each of them, if it's possible and sense full !

As workaround I understand this, am I right ?

public class QuxxyOpenHelper extends AbstractQuxxyOpenHelper { public QuxxyOpenHelper(Context context) { super(context); }

@Override
public void onOpen(SQLiteDatabase db) {
    super.onOpen(db);
    switch (QuxxyOpenHelper.VERSION) {
    case 1: 
        //TODO
        break;
    case 2:
        //TODO
        break;
    ...
    }
}

— Reply to this email directly or view it on GitHub.

hannesa2 commented 11 years ago

I guess this is the new target version

    switch (QuxxyOpenHelper.VERSION) {

Is there a way to ask for the old-current version ? Otherwise my code is senseless, because I need to know from where to where...

Best would be to a call of each individual migration-event, as already wished

fluxtah commented 11 years ago

I do not know a way to get current version as Android tracks this but there might be a way i am not aware of. 

Been thinking about this and it could be possible to do a migration pass without actually executing the statements in super.up() for new installs which would give the desired effect. This might mean introducing an onBeforeUp() and onAfterUp () method to Migration.

Sent from Samsung Mobile

-------- Original message -------- From: hannesa2 notifications@github.com Date:
To: robotoworks/mechanoid mechanoid@noreply.github.com Cc: Ian Warwick fluxtah@hotmail.com Subject: Re: [mechanoid] no migration executed (#197)

I guess this is the new target version

switch (QuxxyOpenHelper.VERSION) {

Is there a way to ask for the old-current version ? Otherwise my code is senseless, because I need to know from where to where...

Best would be to a call of each individual migration-event, as already wished

— Reply to this email directly or view it on GitHub.

fluxtah commented 11 years ago

Hi @hannesa2,

I made the change and new version is available, to do stuff in the migration you need to override onBeforeUp(...) and onAfterUp(...).

The issue you will have is you can only use the (SqliteDatabase db) to do any changes, using the generated builders will throw an exception because it does all its work through content providers and android does not allow the use of this during an upgrade.

here is an example:

@Override
    protected SQLiteMigration createRecipesDBMigrationV2() {
        return new DefaultRecipesDBMigrationV2(){
            @Override
            public void onAfterUp(SQLiteDatabase db) {
                ContentValues cv = Recipes.newBuilder().setTitle("qux").getValues();
                db.insert(Sources.RECIPES, null, cv);
            }
        };
    }

Hope that makes sense, there should be enough constants and help to do the raw db.insert() within the generated contract, for instance Sources is generated and should help you with the table names, and the builders getValues() will help you construct ContentValues.

hannesa2 commented 11 years ago

Thank you !