Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optimize Parse object #195

Merged

Conversation

yulingtianxia
Copy link
Contributor

  1. Support dirty and unsavedChanges
  2. Optimize some API.

This PR fixed a part of the issue: #194

@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Jun 14, 2019

I checked that some metadata and variables were renamed. I have not checked whether they are private or public.
Suggestion. Attention to renaming methods.
Create the new method and mark the old one as @deprecated (''), indicating the new method.

@yulingtianxia
Copy link
Contributor Author

@RodrigoSMarques Thank you for checking! I had marked the old API as deprecated.

Copy link
Member

@phillwiggins phillwiggins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work.

Just for clarification, why is 'parseClassName' preferred? Also, what's the logic behind giving the method 'getObjectData' being private? Would it not be more logical to the actual Map objectData private and leaving the method public so that any manipulation or privatizing data within that Map can be done?

@phillwiggins phillwiggins merged commit 35c83e5 into parse-community:release/1.0.22 Jun 15, 2019
@yulingtianxia
Copy link
Contributor Author

yulingtianxia commented Jun 15, 2019

Excellent work.

Just for clarification, why is 'parseClassName' preferred? Also, what's the logic behind giving the method 'getObjectData' being private? Would it not be more logical to the actual Map objectData private and leaving the method public so that any manipulation or privatizing data within that Map can be done?

  1. 'parseClassName' is defined in iOS/macOS and Android SDK. I think it's more explicit from my point of view. (I'm a mobile app developer) So this needs to be discussed.
  2. In order to record unsaved changes, I need observe all actions that would modify the actual Map objectData. If we make getObjectData and setObjectData public, it will lose control. Developers should use method set/get or operator [] of ParseObject.

@phillwiggins
Copy link
Member

phillwiggins commented Jun 15, 2019 via email

@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Jun 16, 2019

HI @phillwiggins .
This version does not work with my testing application.

As you know I have a test application that perform various actions: login, logout, anonymous user, simple query, complex query, upload and download files, query geolocation, creation of objects with all parse data types.
I always run this app to make sure everything is ok with every change.
I did not have time to investigate, because I'm in another project, but I can not save objects. The code is locked.
Here is the code that is running:

  final user = ParseUser(
      "xxxxxxxxx", "xxxxxxx", "xxxxxxx");

  var response = await user.login();

  if (response.success) {
    print("Login sucess");
    print('EmailVerified ${user.get("emailVerified")}');
  } else {
    print("Login error");
  ...
  ParseACL parseACL = ParseACL(owner: user);
  parseACL.setPublicReadAccess(allowed: true);
  parseACL.setPublicWriteAccess(allowed: false);
  parseACL.setReadAccess(userId: 'gjnKj5IeE7', allowed: true);
  parseACL.setWriteAccess(userId: 'gjnKj5IeE7', allowed: false);

  print(parseACL.toJson());

  var parseObject = ParseObject("TestAPI", debug: true);
  parseObject.set<String>("stringName", "Name");
  parseObject.set<double>("doubleNumber", 1.5);
  parseObject.set<int>("intNumber", 0);
  parseObject.set<bool>("boolFound", true);
  parseObject.set<List<String>>("listString", ["a", "b", "c", "d"]);
  parseObject.set<List<int>>("listNumber", [0, 1]);
  parseObject.set<DateTime>("dateTest", DateTime.now());
  parseObject.set<Map<String, dynamic>>(
      "jsonTest", {"field1": "value1", "field2": "value2"});
  parseObject.setIncrement("intNumber2", 2);
  parseObject.setDecrement("intNumber3", 2);
  parseObject.set<ParseGeoPoint>(
      "location", ParseGeoPoint(latitude: -20.2523818, longitude: -40.2665611));
  parseObject.set<ParseUser>("user", user);
  parseObject.set<List<ParseUser>>("users", [user, user]);
  parseObject.set<ParseFile>("fileImage", parseFile);
  parseObject.set<List<ParseFile>>("fileImages", [parseFile, parseFile]);
  parseObject.set<ParseObject>(
      "produto", ParseObject("Produto")..set("objectId", "E7o3gkZD2T"));
  parseObject.setACL(parseACL);

  print(
      'save init *********************************************************************');
  apiResponse = await parseObject.save();
  print(
      'save finish *********************************************************************');

  if (apiResponse.success) {
    print("Object JSON: " + (apiResponse.result as ParseObject).toString());
    var teste = apiResponse.result as ParseObject;
    print(teste.getACL());
    print('ok');
  } else {
    print("Error: " + apiResponse.error.toString());
    return;
  }
``

@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Jun 17, 2019

Hi @phillwiggins.
I did an analysis and noticed that the save method had many changes. If I add pointer to the object it gets stuck and does not process.
If I remove the pointers, the rendering returns successfully, but the results are not filled correctly and with errors.
I do not know which changed the save method, but it is not working as in version 22.

Captura de Tela 2019-06-16 às 23 12 14

@yulingtianxia yulingtianxia mentioned this pull request Jun 17, 2019
@yulingtianxia
Copy link
Contributor Author

Okay cool. I agree with those changes. I've made a few more changes on this branch. Mainly, the issue around some versions of the SDK not working correctly with CoreStore. There was a big relating to async await when initialising Parse lib. Also tests are passing again. Are you free to do some regression testing. Clear build, refresh dependencies and clear device current installation. Working my end but there is a breaking fix, i.e. await added to Parse.initialise. Thanks for your work. It's really valuable.

On Sat, Jun 15, 2019, 10:42 杨萧玉 @.***> wrote: Excellent work. Just for clarification, why is 'parseClassName' preferred? Also, what's the logic behind giving the method 'getObjectData' being private? Would it not be more logical to the actual Map objectData private and leaving the method public so that any manipulation or privatizing data within that Map can be done? 1. 'parseClassName' is defined in iOS/macOS and Android SDK. I think it's more explicit from my point of view. (I'm a mobile app developer) So this needs to be discussed. 2. In order to record unsaved changes, I need observe all actions that would modify the actual Map objectData. If we make 'getObjectData' and 'setObjectData' public, it will lose control. Developers should use method set/get or operator [] of ParseObject. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#195?email_source=notifications&email_token=AB4CPXRRWERPFUVCAT2RXEDP2S2PFA5CNFSM4HYH5DS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYUPYI#issuecomment-502351841>, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4CPXREUASUS3X6DYPQ3ULP2S2PFANCNFSM4HYH5DSQ .

Thank you for your trust.

I spend most of my time on the job (Tencent, 995 in China ,yeah), but I'm try my best.

@yulingtianxia
Copy link
Contributor Author

Hi @phillwiggins.
I did an analysis and noticed that the save method had many changes. If I add pointer to the object it gets stuck and does not process.
If I remove the pointers, the rendering returns successfully, but the results are not filled correctly and with errors.
I do not know which changed the save method, but it is not working as in version 22.

Captura de Tela 2019-06-16 às 23 12 14

Perhaps this PR would fix it: #199

@RodrigoSMarques
Copy link
Contributor

Yes. It's the same analysis I did here.

#195 (comment)

@phillwiggins
Copy link
Member

Hey

I'm aware of a change in the save method. Logical change is that calling .save() on a ParseObject will manipulate the current object so that it's saved and updated. You no longer need to check for the results in the ParseResponse. You just need to check it was successful. You can then check for any dirty children.

It does look like there is a relation saving issue though

@RodrigoSMarques
Copy link
Contributor

Hi @phillwiggins .
Now that you've explained I understand the change and it makes sense.
It's the same as the native library.

I did a test and this was fine, because in version 21 I retrieve the objectID in the results and use it in a relation / pointer.

But, this is not working in version 21 (and earlier) and this is not documented, which may cause it to break other developers' codes (as with my validation / testing application).

In addition, this change is not allowing Pointer to be saved. If you add a Pointer to an object the application hangs and does nothing.

If I add a pointer to the object the request endpoint is changed to https://parse.xxxxxxxxx.com/1/batch and not to target class.

@phillwiggins
Copy link
Member

phillwiggins commented Jun 24, 2019 via email

@yulingtianxia
Copy link
Contributor Author

yulingtianxia commented Jun 24, 2019

When we add/addRelation/addAll, Parse save the whole JSON data in old version. I fixed it in 1.0.22, with saving a pointer. But this change conflicts with existing data. How to convert old data(whole JSON) to new format(pointer)? This needs Parse Server supports.

@RodrigoSMarques
Copy link
Contributor

Hi @yulingtianxia
I checked that you were responsible for the changes in the Save in ParseObject method.
Can you check to see why the method does not work when you add a pointer to the object?
Version 1.0.22 is available with this failure, and everything is OK in 1.0.21
If you find the defect, the branch to be used is v1.0.23.

@RodrigoSMarques
Copy link
Contributor

When we add/addRelation/addAll, Parse save the whole JSON data in old version. I fixed it in 1.0.22, with saving a pointer. But this change conflicts with existing data. How to convert old data(whole JSON) to new format(pointer)? This needs Parse Server supports.

I did not understand. Pointers and Relations are two different types of objects

@yulingtianxia
Copy link
Contributor Author

When we add/addRelation/addAll, Parse save the whole JSON data in old version. I fixed it in 1.0.22, with saving a pointer. But this change conflicts with existing data. How to convert old data(whole JSON) to new format(pointer)? This needs Parse Server supports.

I did not understand. Pointers and Relations are two different types of objects

Relations need Pointers. They use pointers, but didm't copy full JSON data in REST request:
https://docs.parseplatform.org/rest/guide/#relations
image

@yulingtianxia
Copy link
Contributor Author

Hi @yulingtianxia
I checked that you were responsible for the changes in the Save in ParseObject method.
Can you check to see why the method does not work when you add a pointer to the object?
Version 1.0.22 is available with this failure, and everything is OK in 1.0.21
If you find the defect, the branch to be used is v1.0.23.

Could you provide a sample code snippet?

@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Jun 26, 2019

Hi @yulingtianxia
I checked that you were responsible for the changes in the Save in ParseObject method.
Can you check to see why the method does not work when you add a pointer to the object?
Version 1.0.22 is available with this failure, and everything is OK in 1.0.21
If you find the defect, the branch to be used is v1.0.23.

Could you provide a sample code snippet?

I've put code example above. I create a TestAPI Class and write all the data types supported by Parse.
Any operation that involves Pointer (ParseUser, ParseObject) does not work and is ok in version 21.

#195 (comment)

@yulingtianxia
Copy link
Contributor Author

Hi @yulingtianxia
I checked that you were responsible for the changes in the Save in ParseObject method.
Can you check to see why the method does not work when you add a pointer to the object?
Version 1.0.22 is available with this failure, and everything is OK in 1.0.21
If you find the defect, the branch to be used is v1.0.23.

Could you provide a sample code snippet?

I've put code example above. I create a TestAPI Class and write all the data types supported by Parse.
Any operation that involves Pointer (ParseUser, ParseObject) does not work and is ok in version 21.

#195 (comment)

Yeah, I had fixed it in this PR: #199
But I can't produce the issue you meet, I save TestAPI object successfully. Perhaps there is some other factor of influence.

@RodrigoSMarques
Copy link
Contributor

RodrigoSMarques commented Jun 27, 2019

@yulingtianxia
I checked the PR #199 and saw only one simple change, which did not impact the execution of the lib. ParseGeoPoint is no longer an extension of ParseObject.

Release 1.0.22 has changed the save method and the _saveChildren method is looped when a pointer is inserted into the object.

From my analysis the problem is from this point. File parse_object.dart, line 139

while (remaining.isNotEmpty) {

Below I am putting my code snippet, the execution log in version 1.0.21. I made a change to the _saveChildren method, placing a 'step x' print and displaying it with an endless loop.

My code:

final user = ParseUser(
      "xxxxxxxxx", "xxxxxxx", "xxxxxxx");

  var response = await user.login();

  if (response.success) {
    print("Login sucess");
    print('EmailVerified ${user.get("emailVerified")}');
  } else {
    print("Login error");
  ...
  var parseObject = ParseObject("TestAPI", debug: true);
  parseObject.set<String>("stringName", "Name");
  parseObject.set<double>("doubleNumber", 1.5);
  parseObject.set<int>("intNumber", 0);
  parseObject.set<ParseUser>("user", user);  //Pointer ParseUser
  parseObject.set<List<ParseUser>>("users", [user, user]) // Array Pointer ParseUser
  print(
      'save init *********************************************************************');
  apiResponse = await parseObject.save();
  print(
      'save finish *********************************************************************');

Debug log

flutter:-- Parse Request
flutter: curl -X POST -H 'content-type: text/plain; charset=utf-8' -H 'user-agent: Flutter Parse SDK 1.0.21' -H 'X-Parse-Application-Id: xxxxxxxxxxxxx' -H 'X-Parse-Session-Token: r:xxxxxxxx' -H 'X-Parse-Client-Key: xxxxxxx' -d '{"stringName":"Name","doubleNumber":1.5,"intNumber":0,"user":{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"},"users":[{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"},{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"}]}' https://xxx.xxx.xxxx/1/classes/TestAPI
 https://xxx.xxx.xxxx/1/classes/TestAPI
flutter:--
flutter:-- Parse Response
Class: TestAPI
Function: ParseApiRQ.create
Status Code: 201
Payload: {"className":"TestAPI","objectId":"l42A5WypyU","createdAt":"2019-06-27T05:39:47.913Z","stringName":"Name","doubleNumber":1.5,"intNumber":0,"user":{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"},"users":[{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"},{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"}]}
╰--

In version 1.0.22 I get no result.
Below is the code of the _saveChildren method with some 'step x' print to indicate what you are doing.

See the loop.

  Future<ParseResponse> _saveChildren(dynamic object) async {
    print('begin _saveChildren');
    final Set<ParseObject> uniqueObjects = Set<ParseObject>();
    final Set<ParseFile> uniqueFiles = Set<ParseFile>();
    if (!_collectionDirtyChildren(object, uniqueObjects, uniqueFiles,
        Set<ParseObject>(), Set<ParseObject>())) {
      final ParseResponse response = ParseResponse();
      return response;
    }
    if (object is ParseObject) {
      print('remove ParseObject: ${object.toString()}');
      uniqueObjects.remove(object);
    }
    for (ParseFile file in uniqueFiles) {
      print('Save ParseFiles ${file.toString()}');
      final ParseResponse response = await file.save();
      if (!response.success) {
        return response;
      }
    }
    print('Step 1');
    List<ParseObject> remaining = uniqueObjects.toList();
    final List<ParseObject> finished = List<ParseObject>();
    final ParseResponse totalResponse = ParseResponse()
      ..success = true
      ..results = List<dynamic>()
      ..statusCode = 200;
    print('Step 2');
    while (remaining.isNotEmpty) {
      /* Partition the objects into two sets: those that can be save immediately,
      and those that rely on other objects to be created first. */
      final List<ParseObject> current = List<ParseObject>();
      final List<ParseObject> nextBatch = List<ParseObject>();
      for (ParseObject object in remaining) {
        if (object._canbeSerialized(finished)) {
          current.add(object);
        } else {
          nextBatch.add(object);
        }
      }
      print('Step 3');
      remaining = nextBatch;
      // TODO(yulingtianxia): lazy User
      /* Batch requests have currently a limit of 50 packaged requests per single request
      This splitting will split the overall array into segments of upto 50 requests
      and execute them concurrently with a wrapper task for all of them. */
      print('Step 4');
      final List<List<ParseObject>> chunks = <List<ParseObject>>[];
      for (int i = 0; i < current.length; i += 50) {
        chunks.add(current.sublist(i, min(current.length, i + 50)));
      }
      print('Step 5');
      for (List<ParseObject> chunk in chunks) {
        final List<dynamic> requests = chunk.map<dynamic>((ParseObject obj) {
          print('_getRequestJson: ${obj.toString()}');
          return obj._getRequestJson(obj.objectId == null ? 'POST' : 'PUT');
        }).toList();
        chunk.forEach((ParseObject obj) {
          print('_saveChanges');
          obj._saveChanges();
        });
        final ParseResponse response = await batchRequest(requests, chunk);
        totalResponse.success &= response.success;
        if (response.success) {
          totalResponse.results.addAll(response.results);
          totalResponse.count += response.count;
          for (int i = 0; i < response.count; i++) {
            if (response.results[i] is ParseError) {
              // Batch request succeed, but part of batch failed.
              chunk[i]._revertSavingChanges();
            } else {
              chunk[i]._savingChanges.clear();
            }
          }
        } else {
          // If there was an error, we want to roll forward the save changes before rethrowing.
          chunk.forEach((ParseObject obj) {
            obj._revertSavingChanges();
          });
          totalResponse.statusCode = response.statusCode;
          totalResponse.error = response.error;
        }
      }
      print('Step 6');
      finished.addAll(current);
    }
    return totalResponse;
  }

Log execution:

flutter: begin _saveChildren
flutter: remove ParseObject: {"className":"TestAPI","stringName":"Name","doubleNumber":1.5,"intNumber":0,"user":{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"},"users":[{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"},{"__type":"Pointer","className":"_User","objectId":"0C6ptX39c8"}]}
flutter: Step 1
flutter: Step 2
flutter: Step 3
flutter: Step 4
flutter: Step 5
flutter: Step 6
flutter: Step 3
flutter: Step 4
flutter: Step 5
flutter: Step 6
flutter: Step 3
flutter: Step 4
flutter: Step 5
flutter: Step 6
flutter: Step 3
flutter: Step 4
flutter: Step 5
flutter: Step 6
flutter: Step 3
flutter: Step 4
flutter: Step 5
flutter: Step 6
flutter: Step 3

@yulingtianxia
Copy link
Contributor Author

yulingtianxia commented Jun 27, 2019

@RodrigoSMarques I try your demo, it works fine. Here is log:

flutter: ╭-- Parse Request
flutter: curl -X POST -H 'content-type: text/plain; charset=utf-8' -H 'user-agent: Flutter Parse SDK 1.0.22' -H 'X-Parse-Application-Id: xxx' -H 'X-Parse-Master-Key: xxx' -d '{"stringName":"Name","doubleNumber":1.5,"intNumber":0,"user":{"__type":"Pointer","className":"_User","objectId":"3KA79FePQP"},"users":[{"__type":"Pointer","className":"_User","objectId":"3KA79FePQP"},{"__type":"Pointer","className":"_User","objectId":"3KA79FePQP"}]}' http://xxx/myapp/classes/TestAPI
 http://xxx/myapp/classes/TestAPI
flutter: ╰--
flutter: ╭-- Parse Response
Class: TestAPI
Function: ParseApiRQ.create
Status Code: 201
Payload: {"className":"TestAPI","objectId":"zbxZZrhHkM","createdAt":"2019-06-27T12:01:13.718Z","stringName":"Name","doubleNumber":1.5,"intNumber":0,"user":{"__type":"Pointer","className":"_User","objectId":"3KA79FePQP"},"users":[{"__type":"Pointer","className":"_User","objectId":"3KA79FePQP"},{"__type":"Pointer","className":"_User","objectId":"3KA79FePQP"}]}
╰--

image

@RodrigoSMarques
Copy link
Contributor

@yulingtianxia .
Very strange. I will not associate this problem with the version of Parse Server (I am in 2.8.4), because the processing does not trigger Parse Server, it stays forever in the loop on the client.
How's your pubspec.yaml?

Me:

dependencies:
  flutter:
    sdk: flutter

    # The following adds the Cupertino Icons font to your application.
  # Use with the CupertinoIcons class for iOS style icons.
  cupertino_icons: ^0.1.2
  parse_server_sdk: ^1.0.22
  path_provider: ^0.5.0+1
  http: ^0.12.0+2

@yulingtianxia
Copy link
Contributor Author

@yulingtianxia .
Very strange. I will not associate this problem with the version of Parse Server (I am in 2.8.4), because the processing does not trigger Parse Server, it stays forever in the loop on the client.
How's your pubspec.yaml?

Me:

dependencies:
  flutter:
    sdk: flutter

    # The following adds the Cupertino Icons font to your application.
  # Use with the CupertinoIcons class for iOS style icons.
  cupertino_icons: ^0.1.2
  parse_server_sdk: ^1.0.22
  path_provider: ^0.5.0+1
  http: ^0.12.0+2

Sorry, I had found it! It's really a foolish bug!
image
BTW, I had found some other insignificant bugs in login and #.

@RodrigoSMarques
Copy link
Contributor

I do not believe this is because if I remove the Pointer from the Object all the processing is done successfully, I even do tests with login and #.

The problem is inside the _saveChildren method.

What is your pubspec.yaml?

@yulingtianxia
Copy link
Contributor Author

I do not believe this is because if I remove the Pointer from the Object all the processing is done successfully, I even do tests with login and #.

The problem is inside the _saveChildren method.

What is your pubspec.yaml?

I just use flutter_parse_sdk example, and add your code in main.dart.
_saveChildren method works only when there are Pointers inside ParseObject.

@RodrigoSMarques
Copy link
Contributor

OK. But where are you looking for the parse_server_sdk lib?
From your master repository (phillwiggins) or your Fork?
How is your pubspec.yaml?

@yulingtianxia
Copy link
Contributor Author

OK. But where are you looking for the parse_server_sdk lib?
From your master repository (phillwiggins) or your Fork?
How is your pubspec.yaml?

From my Fork, but sync with phillwiggins master.

dependencies:
  flutter:
    sdk: flutter

  # The following adds the Cupertino Icons font to your application.
  # Use with the CupertinoIcons class for iOS style icons.
  cupertino_icons: ^0.1.2
  sembast: ^1.13.3+1
  shared_preferences: ^0.5.0

dev_dependencies:
  parse_server_sdk:
    path: ../
  flutter_test:
    sdk: flutter
  mockito: ^4.0.0

@yulingtianxia
Copy link
Contributor Author

yulingtianxia commented Jun 28, 2019

@phillwiggins I think I had fixed the bug and commit in my fork(yulingtianxia@5b843a8). Which branch should I pull request?

@RodrigoSMarques
Copy link
Contributor

This is very strange. I run from your Fork and I get the same result. The application loops in the _saveChildren function and does not process anything.
This occurs only when I add a Pointer in the ParseObject.
I run this my project for testing in this lib since version 1.0.12 just to identify the errors.
I'm working on a great project using v1.0.21.
I'll see if I can take the time to investigate what's going on.

@yulingtianxia
Copy link
Contributor Author

@phillwiggins I think I had fixed the bug and commit in my fork(yulingtianxia@5b843a8). Which branch should I pull request?

@RodrigoSMarques Try this version.

fischerscode pushed a commit to fischerscode/Parse-SDK-Flutter that referenced this pull request Sep 5, 2020
* 1. Support dirty and unsavedChanges
2. Optimize some API.

* Fix bugs about unsaved changes.

* Mard Deprecated API.

Fixed:parse-community#195 (comment)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants