Skip to content

Commit

Permalink
Fix database id mapping to remove guesswork hacks
Browse files Browse the repository at this point in the history
The SqliteDatabaseDriver had to resort to counter intuitive hacks
to manually locate a database by its file name and didn't
properly handle deduplicating multiple databases with the same name.  As
we expanded flexibility we failed to address the possibility (nay,
likelihood) that this will occur and it would create very confusing
results for users.

This diff doesn't address the confusion in the UI (two database entries
of the same name can still occur) but it internally makes it possible to
treat them as separate databases and track their filenames separately.
It also makes it relatively straightforward to fix the uniqueness of the
user visible naming by introducing a "context" parameter to
DatabaseDescriptor which would be used in the case of name ambiguity.

This diff addresses concerns raised in #366 and #367.
  • Loading branch information
jasta committed Mar 11, 2017
1 parent 5c480f5 commit 1506dd2
Show file tree
Hide file tree
Showing 8 changed files with 355 additions and 136 deletions.
56 changes: 41 additions & 15 deletions stetho/src/main/java/com/facebook/stetho/Stetho.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import android.app.Application;
import android.content.Context;
import android.content.SharedPreferences;
import android.database.sqlite.SQLiteDatabase;
import android.os.Build;
import com.facebook.stetho.common.LogUtil;
import com.facebook.stetho.common.Util;
Expand All @@ -26,7 +25,8 @@
import com.facebook.stetho.dumpapp.plugins.SharedPreferencesDumperPlugin;
import com.facebook.stetho.inspector.DevtoolsSocketHandler;
import com.facebook.stetho.inspector.console.RuntimeReplFactory;
import com.facebook.stetho.inspector.database.DatabaseConnectionProvider;
import com.facebook.stetho.inspector.database.ContentProviderDatabaseDriver;
import com.facebook.stetho.inspector.database.DatabaseDriver2Adapter;
import com.facebook.stetho.inspector.database.DatabaseFilesProvider;
import com.facebook.stetho.inspector.database.DefaultDatabaseConnectionProvider;
import com.facebook.stetho.inspector.database.DefaultDatabaseFilesProvider;
Expand All @@ -44,6 +44,8 @@
import com.facebook.stetho.inspector.protocol.module.DOMStorage;
import com.facebook.stetho.inspector.protocol.module.Database;
import com.facebook.stetho.inspector.protocol.module.DatabaseConstants;
import com.facebook.stetho.inspector.protocol.module.DatabaseDriver;
import com.facebook.stetho.inspector.protocol.module.DatabaseDriver2;
import com.facebook.stetho.inspector.protocol.module.Debugger;
import com.facebook.stetho.inspector.protocol.module.HeapProfiler;
import com.facebook.stetho.inspector.protocol.module.Inspector;
Expand Down Expand Up @@ -241,7 +243,8 @@ public static final class DefaultInspectorModulesBuilder {
@Nullable private DocumentProviderFactory mDocumentProvider;
@Nullable private RuntimeReplFactory mRuntimeRepl;
@Nullable private DatabaseFilesProvider mDatabaseFilesProvider;
@Nullable private List<Database.DatabaseDriver> mDatabaseDrivers;
@Nullable private List<DatabaseDriver2> mDatabaseDrivers;
private boolean mExcludeSqliteDatabaseDriver;

public DefaultInspectorModulesBuilder(Context context) {
mContext = (Application)context.getApplicationContext();
Expand Down Expand Up @@ -285,7 +288,8 @@ public DefaultInspectorModulesBuilder runtimeRepl(RuntimeReplFactory factory) {
* new DefaultDatabaseConnectionProvider(...)))
* </pre>
*
* @deprecated See example alternative above.
* @deprecated Use {@link #provideDatabaseDriver(DatabaseDriver2)} with
* {@link SqliteDatabaseDriver} explicitly.
*/
@Deprecated
public DefaultInspectorModulesBuilder databaseFiles(DatabaseFilesProvider provider) {
Expand All @@ -294,19 +298,45 @@ public DefaultInspectorModulesBuilder databaseFiles(DatabaseFilesProvider provid
}

/**
* Extend and provide additional database drivers. Currently two database drivers are supported
* in this lib: <br>
* 1. <code>SqliteDatabaseDriver</code> - Presents sqlite databases and all tables of the app.<br>
* 2. <code>ContentProviderDatabaseDriver</code> - Configure and present content providers data.
* @deprecated Convert your custom database driver to {@link DatabaseDriver2}.
*/
public DefaultInspectorModulesBuilder provideDatabaseDriver(Database.DatabaseDriver databaseDriver) {
@Deprecated
public DefaultInspectorModulesBuilder provideDatabaseDriver(DatabaseDriver databaseDriver) {
provideDatabaseDriver(new DatabaseDriver2Adapter(databaseDriver));
return this;
}

/**
* Extend and provide additional database drivers. Stetho provides two database
* drivers by default, with the option for developers to provide their own:
* <ol>
* <li>{@link SqliteDatabaseDriver} - Presents SQLite databases.</li>
* <li>{@link ContentProviderDatabaseDriver} - Configure and present content provider
* data.</li>
* </ol>
*
* <p>Stetho assumes the {@link SqliteDatabaseDriver} should be installed if
* no driver of that type is provided and {@link #excludeSqliteDatabaseDriver} is not
* used.</p>
*/
public DefaultInspectorModulesBuilder provideDatabaseDriver(DatabaseDriver2 databaseDriver) {
if (mDatabaseDrivers == null) {
mDatabaseDrivers = new ArrayList<>();
}
mDatabaseDrivers.add(databaseDriver);
return this;
}

/**
* Do not automatically provide the {@link SqliteDatabaseDriver} instance. The instance
* is provided by default for backwards compatibility purposes and simplicity of API, with
* this API provided to disable that functionality if desired.
*/
public DefaultInspectorModulesBuilder excludeSqliteDatabaseDriver(boolean exclude) {
mExcludeSqliteDatabaseDriver = exclude;
return this;
}

/**
* Provide either a new domain module or override an existing one.
*
Expand Down Expand Up @@ -363,18 +393,14 @@ public Iterable<ChromeDevtoolsDomain> finish() {
Database database = new Database();
boolean hasSqliteDatabaseDriver = false;
if (mDatabaseDrivers != null) {
for (Database.DatabaseDriver databaseDriver : mDatabaseDrivers) {
for (DatabaseDriver2 databaseDriver : mDatabaseDrivers) {
database.add(databaseDriver);
if (databaseDriver instanceof SqliteDatabaseDriver) {
hasSqliteDatabaseDriver = true;
}
}
}
if (!hasSqliteDatabaseDriver) {
// Add the SqliteDatabaseDriver by default for convenience. If this isn't desired,
// the caller must install a dummy version of the driver (with an empty files provider).
// Not ideal, but given the current API and the need for backwards compatability we
// don't have much of a choice here...
if (!hasSqliteDatabaseDriver && !mExcludeSqliteDatabaseDriver) {
database.add(
new SqliteDatabaseDriver(mContext,
mDatabaseFilesProvider != null ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,41 @@
import android.content.Context;
import android.database.Cursor;
import android.database.sqlite.SQLiteException;
import com.facebook.stetho.inspector.protocol.module.Database;

import javax.annotation.concurrent.ThreadSafe;
import com.facebook.stetho.inspector.protocol.module.Database;
import com.facebook.stetho.inspector.protocol.module.DatabaseDescriptor;
import com.facebook.stetho.inspector.protocol.module.DatabaseDriver;
import com.facebook.stetho.inspector.protocol.module.DatabaseDriver2;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import javax.annotation.concurrent.ThreadSafe;

@ThreadSafe
public class ContentProviderDatabaseDriver extends Database.DatabaseDriver {
public class ContentProviderDatabaseDriver
extends DatabaseDriver2<ContentProviderDatabaseDriver.ContentProviderDatabaseDescriptor> {

private final static String sDatabaseName = "content-providers";

private final ContentProviderSchema[] mContentProviderSchemas;
private List<String> mDatabaseNames;
private List<String> mTableNames;

public ContentProviderDatabaseDriver(Context context, ContentProviderSchema... contentProviderSchemas) {
public ContentProviderDatabaseDriver(
Context context,
ContentProviderSchema... contentProviderSchemas) {
super(context);
mContentProviderSchemas = contentProviderSchemas;
}

@Override
public List<String> getDatabaseNames() {
if (mDatabaseNames == null && mContentProviderSchemas != null) {
mDatabaseNames = new ArrayList<>();
mDatabaseNames.add(sDatabaseName);
}
return mDatabaseNames;
public List<ContentProviderDatabaseDescriptor> getDatabaseNames() {
return Collections.singletonList(new ContentProviderDatabaseDescriptor());
}

@Override
public List<String> getTableNames(String databaseId) {
public List<String> getTableNames(ContentProviderDatabaseDescriptor databaseDesc) {
if (mTableNames == null) {
mTableNames = new ArrayList<>();
for (ContentProviderSchema schema : mContentProviderSchemas) {
Expand All @@ -55,7 +58,10 @@ public List<String> getTableNames(String databaseId) {
}

@Override
public Database.ExecuteSQLResponse executeSQL(String databaseName, String query, ExecuteResultHandler<Database.ExecuteSQLResponse> handler) throws SQLiteException {
public Database.ExecuteSQLResponse executeSQL(
ContentProviderDatabaseDescriptor databaseDesc,
String query,
ExecuteResultHandler<Database.ExecuteSQLResponse> handler) throws SQLiteException {

// resolve table name from query
String tableName = fetchTableName(query);
Expand Down Expand Up @@ -91,4 +97,15 @@ private String fetchTableName(String query) {
return "";
}

static class ContentProviderDatabaseDescriptor implements DatabaseDescriptor {
public ContentProviderDatabaseDescriptor() {
}

@Override
public String name() {
// Hmm, this probably should be each unique URI or authority instead of treating all
// content provider instances as one.
return sDatabaseName;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.facebook.stetho.inspector.database;

import android.database.sqlite.SQLiteException;

import com.facebook.stetho.inspector.protocol.module.Database;
import com.facebook.stetho.inspector.protocol.module.DatabaseDescriptor;
import com.facebook.stetho.inspector.protocol.module.DatabaseDriver;
import com.facebook.stetho.inspector.protocol.module.DatabaseDriver2;

import java.util.ArrayList;
import java.util.List;

/**
* @deprecated Use {@link DatabaseDriver2} directly. This is provided only for legacy
* drivers to be adapted internally within Stetho.
*/
@Deprecated
public class DatabaseDriver2Adapter
extends DatabaseDriver2<DatabaseDriver2Adapter.StringDatabaseDescriptor> {
private final DatabaseDriver mLegacy;

public DatabaseDriver2Adapter(DatabaseDriver legacy) {
super(legacy.getContext());
mLegacy = legacy;
}

@Override
public List<StringDatabaseDescriptor> getDatabaseNames() {
List<?> names = mLegacy.getDatabaseNames();
List<StringDatabaseDescriptor> descriptors = new ArrayList<>(names.size());
for (Object name : names) {
descriptors.add(new StringDatabaseDescriptor(name.toString()));
}
return descriptors;
}

@SuppressWarnings("unchecked")
public List<String> getTableNames(StringDatabaseDescriptor database) {
return mLegacy.getTableNames(database.name);
}

@SuppressWarnings("unchecked")
public Database.ExecuteSQLResponse executeSQL(
StringDatabaseDescriptor database,
String query,
ExecuteResultHandler handler) throws SQLiteException {
return mLegacy.executeSQL(database.name, query, handler);
}

static class StringDatabaseDescriptor implements DatabaseDescriptor {
public final String name;

public StringDatabaseDescriptor(String name) {
this.name = name;
}

@Override
public String name() {
return name;
}
}
}
Loading

0 comments on commit 1506dd2

Please sign in to comment.