Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrations are always triggering a rebuild when using SupportOpenHelperFactory #11

Closed
vitorhugods opened this issue Jan 25, 2023 · 4 comments · Fixed by #12
Closed

Migrations are always triggering a rebuild when using SupportOpenHelperFactory #11

vitorhugods opened this issue Jan 25, 2023 · 4 comments · Fixed by #12

Comments

@vitorhugods
Copy link
Contributor

Pretty much what the title says.

In my setup, specifically, we're using SQLDelight, but from what I see, any attempt to use SupportOpenHelpFactory when the runtime version is newer than the current db persisted version will cause a database recreation.

I traced it down to this line of code, that passes the runtime version to SQLiteOpenHelper as both the version and the minimumSupportedVersion.

@vitorhugods
Copy link
Contributor Author

vitorhugods commented Feb 7, 2023

I've opened [a PR that fixes the issue:

@developernotes
Copy link
Member

Hello @vitorhugods,

Thank you for posting a pull request for the issue, and my apologizes for a delay in response. In order to accept this pull request, we will need you to submit a contributor agreement 1.

Are you able to reproduce the issue in isolation of SQLDelight, specifically with just the SQLCipher for Android API? When making changes like this, we would like to include unit tests to verify the behavior. Would you review the contributor agreement options and get back to us?

Thank you,

Footnotes

  1. https://www.zetetic.net/contributions/

@vitorhugods
Copy link
Contributor Author

Thank you for posting a pull request for the issue, and my apologizes for a delay in response. In order to accept this pull request, we will need you to submit a contributor agreement 1.

I initialised the CLA signing process.

Are you able to reproduce the issue in isolation of SQLDelight, specifically with just the SQLCipher for Android API? When making changes like this, we would like to include unit tests to verify the behavior. Would you review the contributor agreement options and get back to us?

Ooof! I don't know why I looked for the tests in the sqlciphertest module instead of the androidTest source set and assumed there were no tests 😅.

I reproduced without SQLDelight as well, so now I'm giving another shot into building the project so I can run and write tests.

I must say it's a bit annoying to manually setup all needed dependencies at the moment. Looking at the requirements and instructions from this project and how to build the SQLCipher amalgamation and libcrypto, I think I should be able to do it.

If I come out broken, beaten and scarred, without any hope, I'll let you know.

@vitorhugods
Copy link
Contributor Author

vitorhugods commented Feb 8, 2023

@developernotes Current failing test following below.

I've added this test to the PR as well, with some more explanation.

    @Test
    public void shouldRunUpgradeFromVersion1ToVersion2() {
        FakeCallback initialCallback = new FakeCallback(1);

        SupportHelper initialHelper = new SupportHelper(createConfiguration(initialCallback), null, null, true);

        initialHelper.getWritableDatabase();
        initialHelper.close();

        FakeCallback callbackWrapper = new FakeCallback(2);

        SupportHelper helper = new SupportHelper(createConfiguration(callbackWrapper), null, null, true);

        helper.getWritableDatabase();
        helper.close();

        assertEquals(0, callbackWrapper.callbackCount[CREATION_INDEX]); // This will be 1, it recreates the database
        assertEquals(1, callbackWrapper.callbackCount[UPGRADE_INDEX]);
    }

    private SupportSQLiteOpenHelper.Configuration createConfiguration(SupportSQLiteOpenHelper.Callback callback) {
        Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
        return SupportSQLiteOpenHelper.Configuration.builder(context)
                .name(DATABASE_NAME)
                .callback(callback)
                .build();
    }

    private static class FakeCallback extends SupportSQLiteOpenHelper.Callback {
        public final int[] callbackCount = {0, 0};

        public FakeCallback(int version) {
            super(version);
        }

        SupportSQLiteOpenHelper.Callback callback = new SupportSQLiteOpenHelper.Callback(version) {
            @Override
            public void onCreate(@NonNull SupportSQLiteDatabase db) {
                callbackCount[CREATION_INDEX] += 1;
            }

            @Override
            public void onUpgrade(@NonNull SupportSQLiteDatabase db, int oldVersion, int newVersion) {
                callbackCount[UPGRADE_INDEX] += 1;
            }
        };

        @Override
        @CallSuper
        public void onCreate(@NonNull SupportSQLiteDatabase db) {
            callback.onCreate(db);
        }

        @Override
        @CallSuper
        public void onUpgrade(@NonNull SupportSQLiteDatabase db, int oldVersion, int newVersion) {
            callback.onUpgrade(db, oldVersion, newVersion);
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants