yahoo / squidb

SquiDB is a SQLite database library for Android and iOS
https://github.com/yahoo/squidb/wiki
Apache License 2.0
1.31k stars 132 forks source link

Passing a RenamingDelegatingContext to an instance of SquidDatabase does not use renamed database names #127

Closed gallal closed 8 years ago

sbosley commented 8 years ago

This appears related to the fact that the SquidDatabase constructor extracts/stores/uses the application context from the context argument, rather than the argument itself. This was a deliberate design choice, as we've found one of the most common sources of memory leaks was accidentally storing a reference to an Activity in a singleton database instance that lives for the duration of the application. We also need access to the real database name being used to support features like attached databases -- therefore it's assumed that the getName() method is providing the real database name being used.

That being said, I'd suggest this easy workaround while we consider possible options:

public class MyDatabase extends SquidDatabase {

    private static final String BASE_DB_NAME = "base_db_name.db"
    private final String mDbName;

    public MyDatabase(Context context) {
        super(context);
        if (context instanceof RenamingDelegatingContext) {
            mDbName = ((RenamingDelegatingContext) context).getDatabasePrefix() + BASE_DB_NAME;
        } else {
            mDbName = BASE_DB_NAME;
        }
    }

    @Override
    public String getName() {
        return mDbName;
    }
}
gallal commented 8 years ago

Thanks, the suggested workaround works

sbosley commented 8 years ago

I'm going to close this issue as a WONTFIX. RenamingDelegatingContext has been deprecated with the new testing support library, and the changes made to SquidDatabase constructor in version 3.0 mean that SquidDatabase instances don't necessarily have knowledge of what context their open helper instances will be constructed with, which would make it much more complicated to manage a distinction between the name returned by getName() and the real DB name being used. Dynamically constructing a database name isn't too uncommon a pattern even in non-testing use cases, so I'm comfortable with this as the official workaround for those still using RenamingDelegatingContext.