-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Create stetho-stub for convenient removal from release builds #16
Comments
My preferred way to use stetho is to only have it in debug flavour src folders |
Leaning toward recommending this solution: http://littlerobots.nl/blog/stetho-for-android-debug-builds-only |
I believe we can solve the network issue with this pattern very nicely by making stetho-okhttp and stetho-urlconnection depend on stetho using a provided/optional dependency, then having them internally test for Class.forName("com.facebook.stetho.Stetho") or something. For instance, you could have:
public class StethoInterceptorInstaller {
public static void installIfPossible(OkHttpClient client) {
if (isStethoPresent(...)) {
client.networkInterceptors().add(new StethoInterceptor());
}
}
}
public interface StethoURLConnectionManager {
public void preConnect();
public void postConnect();
...
}
public class StethoURLConnectionManagerFactory {
public StethoURLConnectionManager newInstance(...) {
if (isStethoPresent(...)) {
return new StethoURLConnectionManagerImpl(...);
} else {
return NoopURLConnectionManagerImpl.getInstance();
}
}
}
class StethoURLConnectionManagerImpl implements StethoURLConnectionManager {
}
class NoopURLConnectionManagerImpl implements StethoURLConnectionManager {
} This will not necessarily be backwards compatible for |
It is pretty lightweight for the project to create a Shim that is a no-op for release builds if that is what the consumer desires. Is it really worth including this work in the debug tool when there are multiple ways for the consumer to isolate itself? |
@brennantaylor the concern is really about asking every engineer to think through the solution. The helpers (stetho-okhttp and -urlconnection) are there to just make life easier for most devs so they might as well make the removal in release builds a convenient choice as well. I'm not longer looking at a stub vs core architecture that would be hard to maintain so I think we have a good compromise with my above proposal. I agree though that if the solution takes something away in terms of convenience or performance it's clearly not worth it. |
Just to chime in with an option (though it might not be the best option for the library, just tossing another idea in for inspiration). We mark Stetho with // StethoHelper interface, AKA "NetworkEventReporterImpl"
public interface StethoHelper {
void init(Context context);
void configureInterceptor(OkHttpClient httpClient);
} public class FakeStethoHelper implements StethoHelper {
@Override
public void init(Context context) {
// Noop
}
@Override
public void configureInterceptor(OkHttpClient httpClient) {
// Noop
}
} public class RealStethoHelper implements StethoHelper {
@Override
public void init(Context context) {
Stetho.initialize(
Stetho.newInitializerBuilder(context)
.enableDumpapp(Stetho.defaultDumperPluginsProvider(context))
.enableWebKitInspector(Stetho.defaultInspectorModulesProvider(context))
.build());
}
@Override
public void configureInterceptor(OkHttpClient httpClient) {
httpClient.networkInterceptors().add(new StethoInterceptor());
}
} We use then these directly directly in a BuildConfig field: debug {
...
minifyEnabled false
signingConfig signingConfigs.debug
buildConfigField 'flipboard.util.StethoHelper', 'STETHO', 'new flipboard.util.RealStethoHelper()'
}
release {
...
minifyEnabled true
shrinkResources false
signingConfig signingConfigs.release
buildConfigField 'flipboard.util.StethoHelper', 'STETHO', 'new flipboard.util.FakeStethoHelper()'
} This way, the packaged APK only knows about the // In your application class's onCreate()
if (BuildConfig.DEBUG) {
BuildConfig.STETHO.init(this);
}
// In your network stack
if (BuildConfig.DEBUG) {
BuildConfig.STETHO.configureInterceptor(this.httpClient);
} You could further abstract this in the library itself by checking if stetho is available at runtime like you suggested, but we find that this is convenient and clever solution that keeps things simple while also allowing the flexibility to easily enable it in other buildTypes |
@hzsweers Why do you need |
You don't, more of just an extra precaution :P. We have other things in
|
@hzsweers Very thanks,it's useful for me.Thanks again. |
I needed a simple way of doing release builds without having two application subclasses, so I've implemented a very basic and minimal no-op library: https://github.com/iGenius-Srl/stetho-no-op |
Thanks @gotev for making that, I was about to do the same. Don't know why FB devs are so against it. The "optimal" app is ridiculous. |
You can create a wrapper class an put it under debug and release root folders. the one under debug will contain the actual stetho implementation and the one under release will have empty stubs. |
This library works brilliantly and is very easy to integrate. |
Hi! @ZacSweers You still have to remove the Debug implementation from the sourceSets in release build right? |
You may be surprised how many popular android apps use stetho in their release builds which exposes a security hole (you can execute SQL statements, monitor network traffic, edit shared preferences, etc). I fully place the blame on app developers carelesness and NOT the stetho team but I also believe you guys could help by providing a no-op version or at least have a disclaimer in README.md. Anyways thank you for this awesome library! |
Support a convenient way to strip stetho from release builds without relying on proguard. This will likely involve producing a simple
stetho-stub
artifact that contains stubbed out versions ofStetho.initialize
andNetworkEventReporterImpl
, allowing production code to unconditionally use these classes but that are backed by no-op versions which have almost no code overhead.The text was updated successfully, but these errors were encountered: