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

[WIP] MLIBZ-2513 - Locally created entity being pushed as a PUT rather than a POST. #169

Merged

Conversation

@vinaygahlawat
Copy link
Contributor

commented May 23, 2018

Description

Locally-created entities, unless they have explicitly had an ID set on it, should have the ID removed prior to pushing to the backend, and this should be done via a POST request. Currently, the ID field is being populated by a GUID from Realm, which is causing the push to be done via a PUT request. This is unexpected, and although the Kinvey backend is more forgiving for the PUT vs POST, things such as Flex services may not handle this.

Changes

Detect when there is no ID or when the entity has a Realm-generated ID, and remove it prior to performing the push operation. Ensure that this request is sent as a POST. Properly delete the entity in Realm when the response is successful, and add the entity returned, which includes the permanent ID, into Realm.

Tests

Instrumented

yuliya-guseva and others added 10 commits Apr 5, 2018
Merge branch 'indev' into feature/MLIBZ-2389_Server-side_Delta_Set_im…
…plementation

# Conflicts:
#	java-api-core/src/com/kinvey/java/Constants.java
Merge branch 'indev' into feature/MLIBZ-2389_Server-side_Delta_Set_im…
…plementation

# Conflicts:
#	android-lib/src/androidTest/java/com/kinvey/androidTest/TestManager.java
#	android-lib/src/androidTest/java/com/kinvey/androidTest/store/DataStoreTest.java
#	android-lib/src/androidTest/java/com/kinvey/androidTest/store/DeltaCacheTest.java
#	android-lib/src/main/java/com/kinvey/android/AsyncPullRequest.java
#	java-api-core/src/com/kinvey/java/network/NetworkManager.java
#	java-api-core/src/com/kinvey/java/store/BaseDataStore.java
#	java-api-core/src/com/kinvey/java/store/requests/data/read/ReadAllRequest.java
feat: Update test
MLIBZ-2389
MLIBZ-2513 Initial fix for replacing temporary entity ID with permane…
…nt ID. Works for SYNC store, but not yet CACHE store.

@vinaygahlawat vinaygahlawat requested a review from yuliya-guseva May 23, 2018

if (syncRequest.getHttpVerb() == SyncRequest.HttpVerb.POST) {
String tempID = syncRequest.getEntityID().id;
GenericJson result = manager.executeRequest(client, syncRequest);
ICache<T> cache = client.getCacheManager().getCache(syncRequest.getCollectionName(), this.storeItemType, Long.MAX_VALUE);

This comment has been minimized.

Copy link
@yuliya-guseva

yuliya-guseva May 28, 2018

Contributor

When we call client.getCacheManager().getCache, cache.get, cache.delete, cache.save the SDK opens and closes realm instance, so for creating 1 item we will open and close realm instance additionally 4 times. Opening/closing realm instance aren't quick operations. How will it affect performance?

This comment has been minimized.

Copy link
@vinaygahlawat

vinaygahlawat Jun 1, 2018

Author Contributor

This will most likely affect performance, but this should be limited to the cases where we are creating entities locally, and not in general. Based on current design, this is necessary for correctness, but we can revisit the performance implications once we take a deeper look at the overall design.

@yuliya-guseva
Copy link
Contributor

left a comment

The previous tests were failed by timeout, I've restarted them. Some of the tests are failed, probably they are not bugs, _id field could be used there. Could you update failed tests?

@yuliya-guseva yuliya-guseva changed the base branch from feature/MLIBZ-2389_Server-side_Delta_Set_implementation to indev Jun 28, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Jun 29, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (indev@ab80428). Click here to learn what that means.
The diff coverage is 93.33%.

Impacted file tree graph

@@           Coverage Diff            @@
##             indev     #169   +/-   ##
========================================
  Coverage         ?   57.97%           
  Complexity       ?      478           
========================================
  Files            ?       42           
  Lines            ?     3172           
  Branches         ?      480           
========================================
  Hits             ?     1839           
  Misses           ?     1183           
  Partials         ?      150
Impacted Files Coverage Δ Complexity Δ
.../main/java/com/kinvey/android/cache/ClassHash.java 64.08% <100%> (ø) 120 <0> (?)
.../android/async/CallableAsyncPushRequestHelper.java 95.65% <100%> (ø) 4 <4> (?)
...ava/com/kinvey/android/async/AsyncPushRequest.java 67.64% <50%> (ø) 10 <0> (?)

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 ab80428...2d2eaf9. Read the comment docs.

@yuliya-guseva yuliya-guseva merged commit b52f790 into indev Jun 29, 2018

1 check passed

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

@yuliya-guseva yuliya-guseva deleted the feature/MLIBZ-2513-Push_of_locally_created_items_sent_as_PUT branch Jun 29, 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.