Skip to content
This repository was archived by the owner on Aug 9, 2020. It is now read-only.

Conversation

Rolf-Smit
Copy link
Contributor

Added @ProviderKey annotation that can be used to supply the provider-key. This fixes #96.

Changes in this PR:

  • Added @ProviderKey annotation including a very simple test case to test whether the annotation is detected and used;
  • Updated README. Proguard section to warn Proguard users about problems that are very likely to occur when not using this annotation.
  • Updated README, Usage section to inform users about the annotation and when it should/can be used.

Please let me know what you think of it.

@Rolf-Smit Rolf-Smit changed the title Added ProviderKey annotation that can be used to provide the provider… Added @ProviderKey annotation to fix issues with Proguard. Jul 19, 2017
@Rolf-Smit
Copy link
Contributor Author

Rolf-Smit commented Jul 19, 2017

@VictorAlbertos it looks like the build server build failed to to a exception in a test case, however locally this test works fine. So you might want to look into this.

Another thing to note here is that some test cases failed out-of-the-box (current master branch) without any changes applied.

The following test cases fail on the current master branch on my machine:

  • ProvidersRxCacheEvictExpirableRecordsTest.When_Expirable_Records_Evict (expected 4, was 7)
    This one is probably caused because the evict method fails to delete the file on Windows because it's in use (delete returns false):
  @Override public void evict(String key) {
    key = safetyKey(key);
    final File file = new File(cacheDirectory, key);
    file.delete();
  }

I've investigated the issue and it seems that the MoshSpeaker (which the tests use) does not close the cache file after reading it, if created an issue for this here: VictorAlbertos/Jolyglot#7

After changing the MoshSpeaker to GsonSpeaker this test does not fail anymore.

  • ProvidersRxCacheEvictExpiredRecordsTest._2_Perform_Evicting_Task_And_Check_Results (expected 0, was 50)
    This test does not fail when using the GsonSpeaker.

  • ProvidersRxCacheEvictExpiredRecordsTest._3_Populate_Disk_With_No_Expired_Records (expected 0, was 9)
    This test does not fail when using the GsonSpeaker.

  • ProvidersRxCacheEvictExpiredRecordsTest._5_Populate_Disk_With_Expired_Encrypted_Records (expected 0, was 24)
    This test does not fail when using the GsonSpeaker.

  • ProvidersRxCacheEvictExpiredRecordsTest._6_Perform_Evicting_Task_And_Check_Results (expected 0, was 24)
    This test does not fail when using the GsonSpeaker.

  • ProvidersRxCacheTest._14_When_0_Is_The_Value_For_Life_Time_Not_Cached_Ad_Infinitum

This test fails when using the GsonSpeaker, with the MoshiSpeaker it does not fail. This is weird as the speaker implementation should not affect anything. This needs a look.

  • ProvidersRxCacheTest._15_When_Evict_All_Evict_All (expected 100, was 106)
  • RxCacheBuilderValidationTest.Cache_Directory_Not_Writable (unexpected exception: java.lang.IllegalStateException: Cannot modify permissions

@VictorAlbertos
Copy link
Owner

I cloned your repo and run test locally and everyone of them just passes. I'll investigate the issue.

In order to accept the PR, please move the annotation @ProviderKey from core module to runtime module. corehas no knowledge whatsoever about runtime api, actually, core is the same lib which ReactiveCache uses internally. And please, remove this comment too ;)

Thanks!

@Rolf-Smit
Copy link
Contributor Author

@VictorAlbertos done!

@VictorAlbertos VictorAlbertos merged commit 3492663 into VictorAlbertos:2.x Jul 21, 2017
@VictorAlbertos
Copy link
Owner

Thanks @Rolf-Smit ;)

New version available: compile "com.github.VictorAlbertos.RxCache:runtime:1.8.1-2.x"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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