Bug: method name used used as provider-key is very error prone.

[Rolf-Smit]

Problem

We use Proguard to obfuscate our code, last week we had a major issue in a production version of our app. After investigating we found an issue that was caused by Proguard, which converted two methods with different names to two methods with equal names (which is exactly what Proguard should do).

Before Proguard:

interface Cache {
    Observable<List<ObjectA>> listObjectA(Observable<List<ObjectA>> observable, DynamicKey dynamicKey, EvictProvider evictProvider);

    Observable<List<ObjectB>> listObjectB(Observable<List<ObjectB>> observable, DynamicKey dynamicKey, EvictProvider evictProvider);

}

After Proguard (simplified for the sake of the example):

interface Cache {
    Observable<List<ObjectA>> a(Observable<List<ObjectA>> observable, DynamicKey dynamicKey, EvictProvider evictProvider);

    Observable<List<ObjectB>> a(Observable<List<ObjectB>> observable, DynamicKey dynamicKey, EvictProvider evictProvider);

}

After Proguard the method names are the same: a (this is totally valid Java code!). This is a problem as RxCache currently uses the method name as provider key. This leads to ugly run-time ClassCastExceptions because two providers use the same provider-key (and thus use the same "space" in Memory).

Dirty solution

To prevent this we can either keep the Cache class using the following Proguard rule:

-keep class org.example.Cache { *; }

Proposed solution

But it would be way nicer if we can manually provide the provider key using an Annotation, as we don't use Proguard for nothing (we want the names to be non-readable). This also enables us to change the method names without writing a migration.

Example:

interface Cache {

    @ProviderKey("some-key-for-list-a")
    Observable<List<ObjectA>> listObjectA(Observable<List<ObjectA>> observable, DynamicKey dynamicKey, EvictProvider evictProvider);

   @ProviderKey("some-key-for-list-b")
    Observable<List<ObjectB>> listObjectB(Observable<List<ObjectB>> observable, DynamicKey dynamicKey, EvictProvider evictProvider);

}

Impact

Modifying the ProxyTranslator.getProviderKey() should be very simple and straightforward, it also wont impact other components of RxCache like migration.

Remarks

• If for some good reason this change is rejected at least make sure the Proguard section of the documentation is complete and includes a warning!
• Another way to fix this is to use the full method signature (not only the name but also the argument types and return types etc.) But this will probably break migration and some other components and is thus not fit for the job.

Let me know what you think, I'm willing to create a pull-request for the proposed changes!

[VictorAlbertos]

Thanks @Rolf-Smit for such detailed explanation of the issue.

You're right, creating an annotation as ProviderKey which expects the value of the key should be an easy fix. I've been thinking about collaterals damages, like breaking the compiler module for generating sources, but as fas as I know, this addition seems to be pretty innocuous.

Please, open a PR and I'll be more than glad to accept it. Just one thing, and proper test coverage for this new feature, as such as documentation about it on the README (adding the Proguard rule would be nice too).

Thanks!