Possible data caching issue

[alexblack]

I'm running into a strange issue, and I was wondering if you could offer any initial insights/intuitions while I look into it further. We're using Android Parse SDK 1.11.0.

High level repro steps:

1. Create an invoice on device 1 using our app, this creates an Invoice object and stores it at parse
2. Device 2 picks this up, gets a copy of the object, stores it locally, it looks good
3. On device 1, we now make a change to one of the fields on this object, a JSON field (containing a map or array)
4. Device 2 picks this up, gets a copy of the object, stores it locally, but it doesn't look good. The ParseObject it got does not match what is shown on the Parse.com dashboard, it is out of date and doesn't reflect the changes made by device 1 at parse.com.

More details:
a. We can repro this when Device 2 is Android and Device 1 is Android, we can also repro this when Device 2 is Android and device 1 is iOS. We cannot repro this if device 2 is iOS. I think this tells us its likely a bug in the Android SDK (assuming iOS SDK behaviour is correct)

b. The code used in Step 2 to get the object from parse is like this:

 public Task<List<T>> get() {
    final ParseQuery<T> query = ParseQuery.getQuery(cls);
    query.whereEqualTo("account", account);
    query.setLimit(LIMIT);
    return pageAllResults(query);
  }

  private Task<List<T>> pageAllResults(final ParseQuery<T> query, final List<T> allResults, final int offset, final int page) {
    query.setSkip(offset);
    Log.d(TAG, String.format("pageAllResults (Offset=%d, Page=%d)", offset, page));
    return query.findInBackground().onSuccessTask(new Continuation<List<T>, Task<List<T>>>() {
      @Override
      public Task<List<T>> then(Task<List<T>> task) throws Exception {
        List<T> results = task.getResult();

        allResults.addAll(results);

        if (results.size() == LIMIT)
          return pageAllResults(query, allResults, offset + results.size(), page + 1);
        else
          return Task.forResult(allResults);
      }
    });
  }

c. We can't repro this issue for primitive fields, like a string. So, for example, if on Device 1 you change a JSON field and a primitive field, then, when Device 2 gets the object using the query it will have the updated primitive field value, but not the updated JSON field value.

d. We tried reproing this by making manual changes at parse.com, and we haven't yet been able to repro the problem this way. Summarizing: if we make the changes using the Android SDK or the iOS SDK then we can repro the problem.

We're going to try calling ParseObject.fetchInBackground for each object in the query results to see if that works around the issue.

My expectation was that after calling ParseQuery.findInBackground and then calling ParseQuery.getResult that the resulting ParseObjects would have current data in them matching what is at parse.com

[alexblack]

Update:

I'm attaching some screenshots, one of which shows the ParseObject after the query. The value we changed here is in the field Setting, its a subvalue (of the json map) called taxRate.

1. The value on parse.com is 200 (current)
2. The value in ParseObject.estimatedData is 20 (old)
3. The value in ParseObject.serverData is 200 (current)

The value our code gets is 20 (old). Our code ends up calling ParseObject.getJsonObject which accesses estimatedData:

https://github.com/ParsePlatform/Parse-SDK-Android/blob/master/Parse/src/main/java/com/parse/ParseObject.java#L3233

    final JSONObject remoteSetting = getJSONObject("setting");
    setting.setTaxRate((float) remoteSetting.optDouble("taxRate"));

Note: we tried calling fetch() on the objects individually, that didn't fix the issue, it also didn't change the value in either estimatedData nor serverData.

 

[alexblack]

Looks similar to:

http://stackoverflow.com/questions/32238867/parse-com-android-sdk-estimateddata-and-serverdata-on-parseobject-differ-immedi

and

https://groups.google.com/forum/#!topic/parse-developers/3fIi1TuWwj0

[grantland]

This seems to be due to the way we handle JSONObjects/Maps and JSONArrays/Lists. We treat the JSON representation as the Java construct under the hood, but we auto-convert them to JSON representations with the JSON getters. Due to this and mutable container tracking, there's a behavior where we cache the converted version, but the cached value is stored as a dirty change that gets priority over data received from the server.

This was desired behavior with mutable containers, but shouldn't be necessary anymore and can remove the caching functionality which will resolve any issues with getJSON* in the future.

For now, you can use getMap and getList instead of getJSON* and it should solve your problem. I'd recommend to continue using this in the future as well since it'll be more performant since I'll be changing getJSON* to convert on every call.

[alexblack]

Great, I will try that work around. Looks like the bug has been around for a while. It really surprised me seeing that the object had just been fetched that it would return old data. :(

[grantland]

It's not really a bug, so I've moved it to "enhancement". It's also not returning "old" data, but dirty data.

[alexblack]

Hmm, I'm not sure what you mean. I updated the value, pulled the object,
and it gave me the old value, when I expected it to give me the new value.

How is that not a bug?

On Tue, Dec 15, 2015 at 4:55 PM, Grantland Chew [email protected]
wrote:

It's not really a bug, so I've moved it to "enhancement". It's also not
returning "old" data, but dirty data.

—
Reply to this email directly or view it on GitHub
#282 (comment)
.

[alexblack]

If I update the object a 2nd time, then get it again, then it returns the
expected value.
On Dec 15, 2015 4:57 PM, "Alex Black" [email protected] wrote:

Hmm, I'm not sure what you mean. I updated the value, pulled the object,
and it gave me the old value, when I expected it to give me the new value.

How is that not a bug?

On Tue, Dec 15, 2015 at 4:55 PM, Grantland Chew [email protected]
wrote:

It's not really a bug, so I've moved it to "enhancement". It's also not
returning "old" data, but dirty data.

—
Reply to this email directly or view it on GitHub
#282 (comment)
.

[grantland]

This has been working as expected for years. This functionality has only recently become obsolete with the removal of mutable containers.

[alexblack]

How is it expected that after fetching an object that this method returns
something other than current data?

I haven't read the docs, but it seemed the intention of these methods was
to return current data.
On Dec 15, 2015 5:37 PM, "Grantland Chew" [email protected] wrote:

This has been working as expected for years. This functionality has only
recently become obsolete with the removal of mutable containers.

—
Reply to this email directly or view it on GitHub
#282 (comment)
.

[alexblack]

I took a quick look at the source for getMap that you suggested using, and it looks to me like it gets its data from estimatedData, which I think doesn't have the current data. Thoughts?

  public <V> Map<String, V> getMap(String key) {
    synchronized (mutex) {
      Object value = estimatedData.get(key);

      if (value instanceof JSONObject) {
        ParseDecoder decoder = ParseDecoder.get();
        value = decoder.convertJSONObjectToMap((JSONObject) value);
        put(key, value);
      }

      if (!(value instanceof Map)) {
        return null;
      }
      @SuppressWarnings("unchecked")
      Map<String, V> returnValue = (Map<String, V>) value;
      return returnValue;
    }
  }

[grantland]

If you only used #getMap() it wouldn't need to convert anything since dictionaries get decoded into Map by default. If you've called #getJSONObject(key) before, you'd want to use #isDirty(key) and #revert(key) to clear cached values.

[alexblack]

I haven't got up to speed on how your SDK works here regarding dirty, converting etc.

I'm just saying that in the screenshots above we see that estimatedData doesn't have the current values, so I am wondering if getMap which uses estimatedData will suffer the same problem?

[alexblack]

Here is the source for getJsonObject, it looks similar to getMap?

  public JSONObject getJSONObject(String key) {
    synchronized (mutex) {
      checkGetAccess(key);
      Object value = estimatedData.get(key);

      if (value instanceof Map) {
        value = PointerOrLocalIdEncoder.get().encode(value);
        put(key, value);
      }

      if (!(value instanceof JSONObject)) {
        return null;
      }

      return (JSONObject) value;
    }
  }

[grantland]

Let me explain this again...

• ParseObject Dictionaries and Arrays on the server get decoded into Map and List by default.
• #getJSONObject(key) and #getJSONArray(key) automatically convert Map and List to JSONObject and JSONArray.
• Since we used to track changes to mutable containers, we would need to track and cache the new converted value instead of the original value. To cache it, we used #put(key, jsonValue) internally which caused the key to be dirty and that dirty value would take priority over new values from the server.
• This functionality is no longer required with the removal of mutable container tracking (Remove mutable container tracking #108)
• As a result of your issue, we've removed this functionality (Remove caching of auto-converted values #303)
• To workaround this issue for now, call #revert(key) to remove the dirty cached value to receive the most up-to-date value received from the server.
• Separately, I recommend using #getMap(key) and #getList(key) instead of #getJSON*(key) since it requires extra cycles to convert from Map/List to JSONObject/JSONArray.

If you have more questions, please refer to the implementation since it's the source of truth for most of these things.

[alexblack]

Also, you mentioned dirty data several times. Which step(s) in the repro
cause there to be dirty data?
On Dec 17, 2015 4:59 PM, "Alex Black" [email protected] wrote:

I haven't got up to speed on how your SDK works here regarding dirty,
converting etc.

I'm just saying that in the screenshots above we see that estimatedData
doesn't have the current values, so I am wondering if getMap which uses
estimatedData will suffer the same problem?

On Thu, Dec 17, 2015 at 1:57 PM, Grantland Chew [email protected]
wrote:

If you only used #getMap() it wouldn't need to convert anything since
dictionaries get decoded into Map by default. If you've called
#getJSONObject(key) before, you'd want to use #isDirty(key) and
#revert(key) to clear cached values.

—
Reply to this email directly or view it on GitHub
#282 (comment)
.

[alexblack]

While i appreciate the detailed explanation, I don't have time to learn how
your code works.

I just want to get the current values. You said initially that I should
switch to use getMap instead of getJsonObject.

I'm happy to do that. If I do that, will I get the current values?

I'm only asking for confirmation because a quick look at the screenshot
above and the source code made me think maybe not.

If you just confirm that after switching to getMap I'll get current data
(despite what I wondered about above) that would be super helpful thanks!
On Dec 17, 2015 5:40 PM, "Grantland Chew" [email protected] wrote:

Let me explain this again...

• ParseObject Dictionaries and Arrays on the server get decoded into
  Map and List by default.
• #getJSONObject(key) and #getJSONArray(key) automatically convert Map
  and List to JSONObject and JSONArray.
• Since we used to track changes to mutable containers, we would need
  to track and cache the new converted value instead of the original value.
  To cache it, we used #put(key, jsonValue) internally which caused the
  key to be dirty and that dirty value would take priority over new values
  from the server.
• This functionality is no longer required with the removal of mutable
  container tracking (Remove mutable container tracking #108
  Remove mutable container tracking #108)
• As a result of your issue, we've removed this functionality (Remove caching of auto-converted values #303
  Remove caching of auto-converted values #303)
• To workaround this issue for now, call #revert(key) to remove the
  dirty cached value to receive the most up-to-date value received from the
  server.
• Separately, I recommend using #getMap(key) and #getList(key) instead
  of #getJSON*(key) since it requires extra cycles to convert from Map/
  List to JSONObject/JSONArray.

If you have more questions, please refer to the implementation since it's
the source of truth for most of these things.

—
Reply to this email directly or view it on GitHub
#282 (comment)
.

[alexblack]

I worked around this issue by switching to getMap() and getList() as you suggested.

[grantland]

Awesome, it should also no longer occur with #303