Desesperately need cascadeSave like parameter for the Android SDK

[ramiro-ciocca]

When Local Data Store is enabled and you have very complex ParseObjects with pointers to other ParseObjects, save methods insice ParseObject.java go mad and it can take minutes to complete a simple save.
I have tracked down the issue till line 662 of ParseObject.java, where is called collectDirtyChildren that enters and insane almost infinite loop traversing every single attribute of every single nested object pinned in the entire LDS, not just the objects Im saving.
I understand that is a functionallity that many developers would use, but we need a way to say "we dont want to save children objects" to the save method, just like in JS SDK, with the cascadeSave=false.
Because of this LDS issue, I'm forced to write cloud code for every single save, or disallow LDS.
Please, if I could comment that line I think that would be the solution, but it is into an aar file, I dont know how to modify that.
Thanks!

[ramiro-ciocca]

I have implemented the "cascadeSave=false" parameter for the Android SDK in a very hacky way.
I cloned the repos, then edited ParseObject.java, then commented out line 630, "collectDirtyChildren(object.estimatedData, dirtyChildren, dirtyFiles, seen, seenNew);".
Then I generated the aars and replace the build.gradle to get those modified aars, and not to download the repo ones.

That recursive collectDirtyChildren method is really messy and is the culpruit for all the hangs when saving any object.

The ParseObject class should provide a paramter to choose wether or not save the children objects, but most important, it should have that method fixed, it hangs every device when you try to save any complex object, with some nested children. It is because of the LDS. With my modification I can use the LDS and save objects without hangs.

And when I need to save some object with children, I add the object and the children to an array then I use saveAll, no need for that buggy default cascade save true. I'm using LDS, so I'm even checking for internet connection and when there is no connection, I use nested saveEventuallys for the children.

Everything working great, it takes some more lines of coding to save an object, but anything is better than the faulty buggy default save with its cascade save behaviour, or better than shut LDS off, and better than write cloud code to save in the cloud because of the faulty save in the client.

[tutorbear]

Dear ramiro-ciocca ,
Can you please share the modified sdk ? And also how can I use it my android project.
I Have the EXACT same issue.
A user class with a pointer to anthoer parse Object created locally,Whenver i try to save the parse user using saveEventually() my app completely freezes up.

[ramiro-ciocca]

I know, that save issue is a nigthmare, it hangs the app on every device or it takes minutes to complete a save.
And saveEventually doesn't even really save the data in the database, it kind of schedules the save until the device acquires connection, and at that time you will have the same hang issue.
I found out that the culprit was a buggy messy recursive method in ParseObject.java, that runs when you have the local data store acitvated. For every save on objects with some grade of complexity, lilke pointers to other objects (and no matter what kind of object, ParseUser, or generic) that method traverse the entire local data store looking for dirty children, on every single property of every pinned object, in objects not realted at all with your current save, I mean, the entire LDS, in an exagerated inefficient way.
The first fix I found out was to disable the LDS. If you disable LDS, all the hangs magically disappears. But my app is strong dependant on LDS for offline use, so that was not an option.
Another option was to make every save in the cloud. That implies to write a function in the cloud for every kind o save you have in your app.
That is cumbersome for a lot of reason, one of them, parse cloud doesn't admint ParseObjects as parameters, so you have to pass the ids, and the modifications like in maps, then in the cloud function fetch the objects, make the modifications, then save.
For a while I was doing that, is a horrible solution but a solution, but saves in my app started to grow, and it was really painfull to write a cloud function or to modify a cloud function for each of those saves.
Then, with not knowledge at all about aars and libraries, but no more optiones, I decided I should check the code that causes the hangs and try to fix it. And I did! (kind of :))
The method that causes the hangs is a recursive method called collectDirtyChildre into ParseObject.java.
As I said, for every save, that method takes the object to be saved as root, and from it, it traverse all the LDS looking parent object's dirty children, to save them all in a "cascadeSave=true" way.
That causes the hangs, it is a bad written inefficient method. I don't have the time nor the knowledge to fix the entire logic of that recursive save method, so i just COMMENTED OUT THE RECURSIVE CALL, that is the line 630 of ParseObject.java:
//collectDirtyChildren(object.estimatedData, dirtyChildren, dirtyFiles, seen, seenNew);
Savior comment!!!
With just those 2 characters (//), that method executes just once with the root object, it doesn't traverse anything anymore, it doesnt hangs up the app anymore.
Parse developers: this should be the default behaviour of ParseObject.save, or at least have a parameter, cascadeSave, true or false, just like it exists in JS SDK.

From now on, you have to be aware that if you need to save childrens of some object, you have to do it explicity, like Children.save(...), alongside with parent object. It's a more than fair deal compared to write cloud code or disable LDS. In my case and in my app at least!

How I did it:

1. Clone the entire repo to your PC.
2. Clone the liveQuery repo too, if you use liveQueries.
3. Open the project with Android Studio.
4. Let it finish the initial gradle sync.
5. On project view (left side) choose "Android" view of project, and into the package "parse", open ParseObject.java.
6. Comment that bitch line out!!
7. On project click on parse package.
8. From menu, select Build->Select Build Variant, then choose release (just for a matter of size).
9. I see some red bulbe. Ignore it.
10. After choosing release build, click again in parse in project window, then go to menu and select Build->Make module 'parse'
11. That process will generate the modified aar, into the project folder, parse/build/outputs/aar.
    12 ) Do the same with every module you use (I use fcm and facebook).
12. Do the same with LiveQuery repo if you use liveQueries.
13. You should end up with an aar for each one of the parse modules you use plus the livequery one.
14. In your real main project, into the "app" folder, make a folder called libs
15. Move the generated aars to that folder
16. Edit your build.gradle (module level) and replace all your dependencies to parse with:
    implementation 'com.parse:parse-release:1.23.1@aar'
    implementation 'com.parse.fcm:fcm-release:1.23.1@aar'
    implementation 'com.parse.facebook:facebook-release:1.14.0@aar'
    implementation 'com.parse.livequery:ParseLiveQuery-release:1.1.0@aar'
    api "com.squareup.okhttp3:okhttp:3.14.4"
    That last line is a dependency needed for livequery.
17. Edit the other build.gradle file and in the repositories section, where you have i.e. maven(), put:
    flatDir {
    dirs 'libs'
    }
18. Clean and rebuild. No more hangs.

Things to be aware of:

1. No more cascade saves, so save each object explicity, including children you want to save. I found out in some cases it is usefull to stack the objects in an ArrayList of ParseObjects, then use saveall. When you commented out that line you fixed all saves variants, not just plain save().
2. Subscribe to the "watch release" from this repo and from livequery repo if you used it. When there is an update (other than a fix to this very issue), update (with git or redownload) your cloned repo, make the same fix (comment line), generate and copy again aars, and update build.gradle with new version numbers.
   Not as confortable as a line in gradle that downloads the library automaticallyt but not to much difficult either.

All this issue took weeks of my time and frustration till I arrived to this "solution". Sincerely hope someone from this great community with real knowledge really fixes LDS and save issues.
This is no more than a patch (Although it works :))

[tutorbear]

And here I thought I was doing something wrong lol. I posted a similar issue before I saw yours .
I just dont understand why the android sdk team is not even responding to these issues. Its pretty serious and hampers the entire making of an App that depends on LDS. They should "REALLY" look into this. I even ready the previous issue which was posted a few months back where you tagged important people but none of them responded. Are they even aware of whats going on here ???
Anyways I was tinkering around with the issue yesterday and I did find a "work around" of sorts.
Its by nesting saves.
Lets say we have this code.

Parse user = ParseUser.getCurrentUser();
//A profile object
profile = new ParseObject("StudentProfile");
profile.put("user", user);
profile.put("name", "John");
profile.setACL(new ParseACL(user));
//Put profile object into user
user.put("profile",profile);
user.saveEventually();

This will crash. But if i do something like this

Parse user = ParseUser.getCurrentUser();
//A profile object
profile = new ParseObject("StudentProfile");
profile.put("user", user);
profile.put("name", "John");
profile.setACL(new ParseACL(user));
profile.saveEventually(SaveCallback{
if(noError){
user.put("profile",profile);
user.saveEventually();
}
});

So this tells us one thing , the object which the user class is pointing to has to be saved first onto the server and after getting that object on callback can we put in into user and then call saveEventually on user again.
Also , I tried disabling local db , still dosent work. Something is really wrong here.
I will try to implement your solution and see how it goes. Will keep you updated.

[shlusiak]

This is hurting quite badly in some circumstances where I'm unable to save a single, cycle free object never succeeds. The issue clearly is in the collectDirtyChildren, which I believe itself has a recursion because it doesn't implement the tree traveral properly.

The function uses a ParseTraverser, but it wants to handle traversing into ParseObjects manually to detect cycles of new objects, which cannot be saved. The ParseTraverser would visit each object only once, so it is unsuitable to detect such cycles. The collectDirtyChildren would like to explode in your hand with a RuntimeException if a cycle of new objects is found, pointing to each other, like A (new) -> B (new) -> A (new), because it couldn't assign the objectIds properly.

The code in question is unfortunately rather confusing (who names their variables seen, seenNew, alreadySeen and alreadySeenNew all in the same block)? Anyway, the issue as I see it is that it keeps a Set of seen ParseObjects and another Set of seen ParseObjects that are new. When traversing down into a ParseObject it creates a copy of those sets and adds the current object to it and goes in. So when traversing back up and sideways to the next object it has forgotten that it has already seen the object and will do it again. That causes of course extra churn because of visiting the same objects multiple times, it causes lots of garbage. If you have quite some pointers and arrays flying around, this can explode quickly and you end up visiting the same objects over and over again. While I'm not sure if there is actually a recursion in the logic, my code never reaches that point because it ends up GC'ing all the time.

This is the code in question:

Parse-SDK-Android/parse/src/main/java/com/parse/ParseObject.java

Lines 606 to 625 in 797786f

	// Check for cycles of new objects. Any such cycle means it will be
	// impossible to save this collection of objects, so throw an exception.
	if (object.getObjectId() != null) {
	seenNew = new HashSet<>();
	} else {
	if (seenNew.contains(object)) {
	throw new RuntimeException("Found a circular dependency while saving.");
	}
	seenNew = new HashSet<>(seenNew);
	seenNew.add(object);
	}
	// Check for cycles of any object. If this occurs, then there's no
	// problem, but we shouldn't recurse any deeper, because it would be
	// an infinite recursion.
	if (seen.contains(object)) {
	return true;
	}
	seen = new HashSet<>(seen);
	seen.add(object);

If the current object is not new, i.e. has an objectId, all previously seen new objects are chucked away and we traverse in further with an empty set. This would fail to detect cycles like this, but maybe that is not an issue?
A (old) --> B (new) --> C (old) --> B (new)

The inefficiency would come into play in situations like this:

A --> B --> D
  \-> C --> D

When A dives into B and B traverses D, it adds it to seen. However when A continues to C, it is a different set of seen that does not contain D yet and it will be traversed again. This actually means that the same dirty object (e.g. D) would be added twice to the collection, which on the case of saveEventually is a List, not a Set, so that could be bad.

An obvious improvement is to remove the line seen = new HashSet<>(seen);.

A bit sad to see that there barely are any unit tests for the ParseObject class that I could use as a base to verify the behavior. I'll create a quick PR and would be happy about feedback.