From bf4bab2e75f8df6331dc667606cdaab29b55569b Mon Sep 17 00:00:00 2001 From: Grantland Chew Date: Fri, 4 Sep 2015 14:55:35 -0700 Subject: [PATCH] Remove mutable container tracking Resolves #58 --- .../java/com/parse/ParseJSONCacheItem.java | 40 -------- .../src/main/java/com/parse/ParseObject.java | 96 ------------------- Parse/src/main/java/com/parse/ParseUser.java | 12 --- .../test/java/com/parse/ParseUserTest.java | 4 - 4 files changed, 152 deletions(-) delete mode 100644 Parse/src/main/java/com/parse/ParseJSONCacheItem.java diff --git a/Parse/src/main/java/com/parse/ParseJSONCacheItem.java b/Parse/src/main/java/com/parse/ParseJSONCacheItem.java deleted file mode 100644 index c12a4c84b..000000000 --- a/Parse/src/main/java/com/parse/ParseJSONCacheItem.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (c) 2015-present, Parse, LLC. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ -package com.parse; - -import org.json.JSONException; -import org.json.JSONObject; - -// A ParseJSONCacheItem is a pairing of a json string with its hash value. -/** package */ class ParseJSONCacheItem { - private JSONObject json; - private String hashValue; - - public ParseJSONCacheItem(Object object) throws JSONException { - json = new JSONObject(); - json.put("object", PointerOrLocalIdEncoder.get().encode(object)); - hashValue = ParseDigestUtils.md5(json.toString()); - } - - public boolean equals(ParseJSONCacheItem other) { - return hashValue.equals(other.getHashValue()); - } - - public String getHashValue() { - return hashValue; - } - - public Object getJSONObject() { - try { - return json.get("object"); - } catch (JSONException e) { - return null; - } - } -} diff --git a/Parse/src/main/java/com/parse/ParseObject.java b/Parse/src/main/java/com/parse/ParseObject.java index 0ca102c0a..a8d586003 100644 --- a/Parse/src/main/java/com/parse/ParseObject.java +++ b/Parse/src/main/java/com/parse/ParseObject.java @@ -21,7 +21,6 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.Iterator; import java.util.LinkedList; import java.util.List; @@ -306,7 +305,6 @@ public String toString() { // Cached State private final Map estimatedData; - private final Map hashedObjects; // For mutable containers private String localId; private final ParseMulticastDelegate saveEvent = new ParseMulticastDelegate<>(); @@ -383,7 +381,6 @@ public ParseObject(String theClassName) { operationSetQueue = new LinkedList<>(); operationSetQueue.add(new ParseOperationSet()); estimatedData = new HashMap<>(); - hashedObjects = new IdentityHashMap<>(); State.Init builder = newStateBuilder(theClassName); // When called from new, assume hasData for the whole object is true. @@ -615,16 +612,6 @@ public Void then(Task task) throws Exception { } } - private void addToHashedObjects(Object object) { - synchronized (mutex) { - try { - hashedObjects.put(object, new ParseJSONCacheItem(object)); - } catch (JSONException e) { - throw new IllegalArgumentException("Couldn't serialize container value to JSON."); - } - } - } - /** * Converts a {@code ParseObject.State} to a {@code ParseObject}. * @@ -752,7 +739,6 @@ private void setState(State newState, boolean notifyIfObjectIdChanges) { } rebuildEstimatedData(); - checkpointAllMutableContainers(); } } @@ -847,7 +833,6 @@ public void revert(String key) { if (isDirty(key)) { currentOperations().remove(key); rebuildEstimatedData(); - checkpointAllMutableContainers(); } } } @@ -861,7 +846,6 @@ public void revert() { if (isDirty()) { currentOperations().clear(); rebuildEstimatedData(); - checkpointAllMutableContainers(); } } } @@ -1036,8 +1020,6 @@ protected boolean visit(Object object) { /* package */ JSONObject toRest( State state, List operationSetQueue, ParseEncoder objectEncoder) { - checkForChangesToMutableContainers(); - // Public data goes in dataJSON; special fields go in objectJSON. JSONObject json = new JSONObject(); @@ -1187,7 +1169,6 @@ public boolean isDirty() { /* package */ boolean isDirty(boolean considerChildren) { synchronized (mutex) { - checkForChangesToMutableContainers(); return (isDeleted || getObjectId() == null || hasChanges() || (considerChildren && hasDirtyChildren())); } } @@ -1222,80 +1203,6 @@ public boolean isDirty(String key) { } } - //region Mutable Containers - - /* package */ boolean isContainerObject(String key, Object object) { - return (object instanceof JSONObject || object instanceof JSONArray - || object instanceof Map || object instanceof List - || object instanceof ParseACL || object instanceof ParseGeoPoint); - } - - /** - * Updates the JSON cache value for all of the values in estimatedData. - */ - private void checkpointAllMutableContainers() { - synchronized (mutex) { - for (Map.Entry entry : estimatedData.entrySet()) { - checkpointMutableContainer(entry.getKey(), entry.getValue()); - } - } - } - - /** - * Updates the JSON cache value for the given object. - */ - private void checkpointMutableContainer(String key, Object object) { - synchronized (mutex) { - if (isContainerObject(key, object)) { - addToHashedObjects(object); - } - } - } - - /** - * Inspects to see if a given mutable container owned by this object has been mutated, and treats - * any mutation as a new {@link #put(String, Object)} call. - */ - private void checkForChangesToMutableContainer(String key, Object object) { - synchronized (mutex) { - if (isContainerObject(key, object)) { - ParseJSONCacheItem oldCacheItem = hashedObjects.get(object); - if (oldCacheItem == null) { - throw new IllegalArgumentException( - "ParseObject contains container item that isn't cached."); - } else { - ParseJSONCacheItem newCacheItem; - try { - newCacheItem = new ParseJSONCacheItem(object); - } catch (JSONException e) { - throw new RuntimeException(e); - } - if (!oldCacheItem.equals(newCacheItem)) { - // A mutable container changed out from under us. Treat it as a set operation. - performOperation(key, new ParseSetOperation(object)); - } - } - } else { - hashedObjects.remove(object); - } - } - } - - /** - * Inspects to see if any mutable container owned by this object has been mutated, and treats any - * mutation as a new {@link #put(String, Object)} call. - */ - /* package */ void checkForChangesToMutableContainers() { - synchronized (mutex) { - for (String key : estimatedData.keySet()) { - checkForChangesToMutableContainer(key, estimatedData.get(key)); - } - hashedObjects.keySet().retainAll(estimatedData.values()); - } - } - - //endregion - /** * Accessor to the object id. An object id is assigned as soon as an object is saved to the * server. The combination of a className and an objectId uniquely identifies an object in your @@ -2983,8 +2890,6 @@ private void rebuildEstimatedData() { ParseFieldOperation oldOperation = currentOperations().get(key); ParseFieldOperation newOperation = operation.mergeWithPrevious(oldOperation); currentOperations().put(key, newOperation); - - checkpointMutableContainer(key, newValue); } } @@ -3512,7 +3417,6 @@ private ParseACL getACL(boolean mayCopy) { if (mayCopy && ((ParseACL) acl).isShared()) { ParseACL copy = ((ParseACL) acl).copy(); estimatedData.put(KEY_ACL, copy); - addToHashedObjects(copy); return copy; } return (ParseACL) acl; diff --git a/Parse/src/main/java/com/parse/ParseUser.java b/Parse/src/main/java/com/parse/ParseUser.java index 8315d5985..ca6a9676e 100644 --- a/Parse/src/main/java/com/parse/ParseUser.java +++ b/Parse/src/main/java/com/parse/ParseUser.java @@ -194,15 +194,6 @@ boolean isKeyMutable(String key) { } } - @Override - boolean isContainerObject(String key, Object object) { - if (KEY_AUTH_DATA.equals(key)) { - // We're tracking dirtiness of `authData` ourselves. - return false; - } - return super.isContainerObject(key, object); - } - /** * Whether the ParseUser has been authenticated on this device. This will be true if the ParseUser * was obtained via a logIn or signUp method. Only an authenticated ParseUser can be saved (with @@ -637,9 +628,6 @@ public Task then(Task task) throws Exception { new IllegalArgumentException("Attempt to merge currentUser with itself.")); } - checkForChangesToMutableContainers(); - user.checkForChangesToMutableContainers(); - boolean isLazy = user.isLazy(); final String oldUsername = user.getUsername(); final String oldPassword = user.getPassword(); diff --git a/Parse/src/test/java/com/parse/ParseUserTest.java b/Parse/src/test/java/com/parse/ParseUserTest.java index 2af2dab4c..0db427134 100644 --- a/Parse/src/test/java/com/parse/ParseUserTest.java +++ b/Parse/src/test/java/com/parse/ParseUserTest.java @@ -242,8 +242,6 @@ public void testSignUpAsyncWithMergeInDiskAnonymousUser() throws Exception { Task signUpTask = user.signUpAsync(Task.forResult(null)); signUpTask.waitForCompletion(); - // Make sure we checkForChangesToMutableContainers - verify(currentUser, times(1)).checkForChangesToMutableContainers(); // Make sure currentUser copy changes from user verify(currentUser, times(1)).copyChangesFrom(user); // Make sure we update currentUser username and password @@ -289,8 +287,6 @@ public void testSignUpAsyncWithMergeInDiskAnonymousUserSaveFailure() throws Exce Task signUpTask = user.signUpAsync(Task.forResult(null)); signUpTask.waitForCompletion(); - // Make sure we checkForChangesToMutableContainers - verify(partialMockCurrentUser, times(1)).checkForChangesToMutableContainers(); // Make sure we update currentUser username and password verify(partialMockCurrentUser, times(1)).setUsername("userName"); verify(partialMockCurrentUser, times(1)).setPassword("password");