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-2392 Remove metadata storage on embedded objects #147

Conversation

@yuliya-guseva
Copy link
Contributor

commented Mar 2, 2018

Description

Currently, metadata such as _kmd and _acl are being added to objects that are embedded in an entity. This needs to be removed to match the behavior of the other SDKs.

Changes

_kmd and _acl fields were removed from objects that are embedded in an entity.
If an entity has _kmd and _acl fields in embedded objects they will be ignored and won't be sent to the server in the next push and won't be saved from the server in the pull.

Tests

Instrumented, Manual

@yuliya-guseva yuliya-guseva requested a review from vinaygahlawat Mar 2, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Mar 2, 2018

Codecov Report

Merging #147 into indev will decrease coverage by 0.64%.
The diff coverage is 47.51%.

Impacted file tree graph

@@             Coverage Diff              @@
##              indev     #147      +/-   ##
============================================
- Coverage     56.71%   56.06%   -0.65%     
+ Complexity      438      433       -5     
============================================
  Files            41       41              
  Lines          2932     2982      +50     
  Branches        457      470      +13     
============================================
+ Hits           1663     1672       +9     
- Misses         1128     1160      +32     
- Partials        141      150       +9
Impacted Files Coverage Δ Complexity Δ
...va/com/kinvey/android/cache/RealmCacheManager.java 72.72% <26.19%> (-15.91%) 30 <0> (ø)
.../main/java/com/kinvey/android/cache/ClassHash.java 62.44% <56.56%> (-0.57%) 106 <7> (-5)
...ava/com/kinvey/android/async/AsyncPushRequest.java 61.53% <0%> (-3.85%) 7% <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 6fc6114...1669554. Read the comment docs.

@vinaygahlawat
Copy link
Contributor

left a comment

A few style comments, but other than that LGTM.

cache.checkAclKmdFields(mRealm);
}
mRealm.commitTransaction();
}

mRealm.beginTransaction();
// Check that ACL and KMD fields don't exist in embedded objects (like sync_meta_kmd)
boolean isNeedDeleteEbbeddedSupportTables = mRealm.getSchema().contains(TableNameManager.getShortName(TableNameManager.getShortName(TableNameManager.getShortName(SYNC_COLLECTION, mRealm) + KinveyMetaData.META, mRealm) + Constants.UNDERSCORE + KinveyMetaData.KMD, mRealm));

This comment has been minimized.

Copy link
@vinaygahlawat

vinaygahlawat Mar 8, 2018

Contributor

Typo on variable name. Could also make name shorter if you want, something like shouldRemoveMetadata

This comment has been minimized.

Copy link
@yuliya-guseva

yuliya-guseva Mar 12, 2018

Author Contributor

Updated

boolean isNeedDeleteEbbeddedSupportTables = mRealm.getSchema().contains(TableNameManager.getShortName(TableNameManager.getShortName(TableNameManager.getShortName(SYNC_COLLECTION, mRealm) + KinveyMetaData.META, mRealm) + Constants.UNDERSCORE + KinveyMetaData.KMD, mRealm));
mRealm.commitTransaction();
if (isNeedDeleteEbbeddedSupportTables) {
mRealm.beginTransaction();

This comment has been minimized.

Copy link
@vinaygahlawat

vinaygahlawat Mar 8, 2018

Contributor

Is this code block only for internal table metadata, such as sync_meta_kmd, or is it checking for metadata against any table representing embedded objects?

This comment has been minimized.

Copy link
@yuliya-guseva

yuliya-guseva Mar 12, 2018

Author Contributor

This code block is only for checking sync_meta_kmd. If SDK contains sync_meta_kmd then all other tables which have embedded objects have to have support tables _kmd, _acl for embedded objects. If it's true, then we will delete this support tables _kmd, _acl from embedded objects.

* @param isEmbedded true if it's embedded object
* @return list of schemas to removing
*/
private List<String> prepareACLAndKMDSchemasFromEmbeddedObjectToRemoving(String tableName, DynamicRealm realm, boolean isEmbedded) {

This comment has been minimized.

Copy link
@vinaygahlawat

vinaygahlawat Mar 8, 2018

Contributor

Please rename method to prepareToRemoveMetadataSchemasFromEmbeddedObjects

This comment has been minimized.

Copy link
@yuliya-guseva

yuliya-guseva Mar 12, 2018

Author Contributor

Updated

@yuliya-guseva yuliya-guseva merged commit 6357da2 into indev Mar 14, 2018

1 check passed

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

@yuliya-guseva yuliya-guseva deleted the feature/MLIBZ-2392_Remove_metadata_storage_on_embedded_objects branch Mar 14, 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.