Fix StackOverflowError when merging ParseObject from JSON

[jhansche]

This fixes #896. See comments there, and at #795.

When LocalDataStore is enabled, the same ParseObject(.State) is shared for the same class+id pair. That allows us to update objects in-place whenever a ParseObject is parsed from JSON. But every time the state is copied, via State.newBuilder(), the availableKeys Set is wrapped (and re-wrapped, etc...) in synchronizedSet().

Eventually, if the object has been refreshed enough times, that results in such a deeply nested hierarchy of wrapped sets, that any collection operation on it results in a StackOverflowError.

This change adds a test to prove the bug, and fixes it by severing making a copy of the set before wrapping it in synchronizedSet().

[codecov]

Codecov Report

Merging #925 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #925      +/-   ##
============================================
- Coverage     64.65%   64.64%   -0.02%     
  Complexity     1898     1898              
============================================
  Files           122      122              
  Lines          9709     9709              
  Branches       1361     1361              
============================================
- Hits           6277     6276       -1     
  Misses         2927     2927              
- Partials        505      506       +1

Impacted Files	Coverage Δ	Complexity Δ
parse/src/main/java/com/parse/ParseObject.java	59.71% <100%> (ø)	235 <0> (+1)	⬆️
...arse/src/main/java/com/parse/ParseCorePlugins.java	66.66% <0%> (-0.56%)	50% <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 5bd91aa...0c3dfd2. Read the comment docs.

[coveralls]

Coverage remained the same at 69.853% when pulling 0c3dfd2 on wondrous-io:pr/fix-896-stackoverflow into 5bd91aa on parse-community:master.

[Jawnnypoo]

Looks good to me, other than some minor changes

[AlejandroHCruz]

Hi, now that this is merged, when could we expect a new version of the library to be released?

[Jawnnypoo]

I'll wait for EOD for a review of #926 and then I'll merge and create 1.19.0

[AlejandroHCruz]

Awesome thanks… well, I guess this is as far as I go and now I need to migrate the project to AndroidX 😅 wish me luck

[Jawnnypoo]

Refactor -> Migrate to Android X works great in Android Studio!

[AlejandroHCruz]

Yeah, just tried it out again and have some problems with the native C++ code 😅

[jhansche]

It might be a good idea to release this change as 1.18.6, and then 1.19 separately with the AndroidX update. Since that change breaks compatibility, but this one didn't.

We can still do a retro tag and release of f2e6475. What do you think?

cc @rogerhu @Jawnnypoo

[AlejandroHCruz]

It would be really amazing if you could do that 👍

[Jawnnypoo]

Sure, it has been released as 1.18.6

[jhansche]

Thanks!

[nisrulz]

Hi,
Thank you for your support and for making the release 1.18.6 available. I was looking through release and noticed that the dependencies on this release weren't updated although they were done so on the 1.19.0 release.

buildscript {
    ext.kotlin_version = '1.2.71'
    repositories {
        google()
        jcenter()
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:3.1.3'
        classpath 'org.kt3k.gradle.plugin:coveralls-gradle-plugin:2.8.2'
        classpath 'com.github.dcendents:android-maven-gradle-plugin:2.1'
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
    }
}

plugins {
    id 'com.github.ben-manes.versions' version '0.20.0'
}

allprojects {
    repositories {
        google()
        jcenter()
    }
}

task clean(type: Delete) {
    delete rootProject.buildDir
}

ext {
    compileSdkVersion = 27

    supportLibVersion = '27.1.1'

    firebaseJobdispatcherVersion = '0.8.5'

    minSdkVersion = 14
    targetSdkVersion = 27
}

Any reason to not do so?

If it is fine, I can open a PR to update those :)

[Jawnnypoo]

1.18.6 is sort of a special case for those who cannot move to AndroidX yet, but want the fixes of this PR. 1.19.0 is the preferred upgrade path for anyone who is targeting API 28 and all future versions will build on this.

[jhansche]

@Jawnnypoo did any of those releases make it to Maven Central? Or did I miss something wrt where artifacts are deployed these days?

https://mvnrepository.com/artifact/com.parse/parse-android shows the latest version is 1.17.3

EDIT: disregard :) I see the notes about jitpack

[rogerhu]

Yeah I kind of wished we had it back to maven central. It's created a lot of confusion..

[Jawnnypoo]

Feel free to move it back if you would like, I think jitpack makes the releases much easier (less configuration) and just requires creating a tag, which is simple and allowed me to create 1.18.6 very easily by just creating a tag of a commit that hadn't even been released previously. Easier access to SNAPSHOTS and versioning by a particular commit (pre release) as well.