Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: MLIBZ-2563 Add updating query cache item if it exists instead of creating new #179

Conversation

@yuliya-guseva
Copy link
Contributor

commented Jun 20, 2018

Description

When a deltaset specific error occurs, deltaset request is not retried on next attempts to pull/sync
Steps to reproduce:

  1. Disable deltaset at the backend
  2. Pull
  3. Pull
  4. Pull

Expected result: The first pull makes a regular GET and enter a value in the _QueryCache table, the next one attempts _deltaset request, but it fails for an error, so a regular GET is again made and its value overrides the lastRequestAt value in the _QueryCache table. The third request does exactly the same as the second one

Actual result: The first pull does what is expected, the second uses deltaset and then regular GET, but records a new entry for the query in _QueryCache table, so now it has 2 entries. The third pull directly makes a regular GET instead of first using deltaset. These also make their own entries on the _QueryCache, so after a few pulls, there are a few entries with the same query but with different lastRunAt values.

Changes

Added updating the query cache item if it exists instead of creating new.
Added checking that the query cache has only 1 item for the query, else all items(with the same query) is deleted except the latest one.

Tests

Instrumented

@yuliya-guseva yuliya-guseva requested a review from vinaygahlawat Jun 20, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Jun 20, 2018

Codecov Report

Merging #179 into indev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              indev     #179   +/-   ##
=========================================
  Coverage     58.13%   58.13%           
  Complexity      473      473           
=========================================
  Files            41       41           
  Lines          3134     3134           
  Branches        479      479           
=========================================
  Hits           1822     1822           
  Misses         1163     1163           
  Partials        149      149
Impacted Files Coverage Δ Complexity Δ
...in/java/com/kinvey/android/AsyncClientRequest.java 63.63% <0%> (-3.04%) 10% <0%> (-1%)
...n/java/com/kinvey/android/KinveyHandlerThread.java 86.36% <0%> (+4.54%) 8% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e883743...234eb28. Read the comment docs.

@vinaygahlawat
Copy link
Contributor

left a comment

In the scenario where a deltaset-specific error occurs, the first action that should happen is that the corresponding QueryCacheItem should be deleted. In this way, no matter what the next step should be, there is no risk that there is a duplicate entry in the _queryCache table. Is the corresponding QueryCacheItem always removed when a deltaset error is encountered?

@yuliya-guseva

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2018

In the previous implementation, the QueryCacheItem isn't updated in some cases, so if user has done a lot of pull with disabled delta set at the backend, he had an item in QueryCache table for each of this pull, because for each pull new query cache item was created, therefore I've added logic to clear those duplicates. In the current implementation, QueryCacheTable shouldn't have items with the same query, and this logic won't be used for new items.

@yuliya-guseva yuliya-guseva merged commit ab80428 into indev Jun 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yuliya-guseva yuliya-guseva deleted the feature/MLIBZ-2563_When_a_deltaset_specific_error_occurs,_deltaset_request_is_not_retried_on_next_attempts_to_pull-sync branch Jun 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.