Skip to content

feat: Add saveEventually and deleteEventually in ParseObject #911

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 34 commits into from
Oct 18, 2023
Merged

feat: Add saveEventually and deleteEventually in ParseObject #911

merged 34 commits into from
Oct 18, 2023

Conversation

mbfakourii
Copy link
Member

@mbfakourii mbfakourii commented May 19, 2023

Pull Request

Issue

Closes: #149 and #194

Approach

In this feature, Functions saveEventually and deleteEventually in ParseObject were added

---> in saveEventually
At first, normal save command is entered in parseObject, if a NetworkError is encountered, this object is saved in a list with a key with the help of ParseCoreData.
In our collection, we have a list of objects in ParseCoreData that are in NetworkError.

---> in deleteEventually
At first, normal delete command is entered in parseObject, if a NetworkError is encountered, this object is saved in a list with a key with the help of ParseCoreData.
In our collection, we have a list of objects in ParseCoreData that are in NetworkError.

---> in await ParseObject.submitEventually()
With the help of the function await ParseObject.submitEventually() (which is debatable how to use it) we checks the error objcets

In this function, we first get the error parseObjects from ParseCoreData, then we send this list to _saveChildren. In this function, these objects are sent to the server in the form of chunks, and finally, we empty the ParseCoreData list.

In the following, we also take the list of parseObjects that must be deleted from ParseCoreData and try to delete them, and then we delete the successfully deleted parseObjects from the ParseCoreData list.

@parse-community/flutter-sdk-review
Please check these two functions thoroughly and give suggestions, this PR needs discussion.
and please check the algorithm, is this method good?

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)

Sorry, something went wrong.

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title feat: add saveEventually and deleteEventually in ParseObject feat: Add saveEventually and deleteEventually in ParseObject May 19, 2023
@parse-github-assistant
Copy link

Thanks for opening this pull request!

@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (21ce56f) 39.61% compared to head (638440b) 40.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #911      +/-   ##
==========================================
+ Coverage   39.61%   40.60%   +0.98%     
==========================================
  Files          60       60              
  Lines        3337     3401      +64     
==========================================
+ Hits         1322     1381      +59     
- Misses       2015     2020       +5     
Files Coverage Δ
packages/dart/lib/parse_server_sdk.dart 31.25% <50.00%> (+2.67%) ⬆️
...ackages/dart/lib/src/network/parse_dio_client.dart 0.00% <0.00%> (ø)
...ckages/dart/lib/src/network/parse_http_client.dart 5.06% <0.00%> (-0.07%) ⬇️
packages/dart/lib/src/utils/parse_utils.dart 76.27% <75.00%> (-0.20%) ⬇️
packages/dart/lib/src/objects/parse_object.dart 94.49% <92.85%> (-0.42%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mtrezza mtrezza marked this pull request as draft May 20, 2023 16:13
@mtrezza
Copy link
Member

mtrezza commented May 20, 2023

Please check these two functions thoroughly and give suggestions, this PR needs discussion.

Changed to draft

@mbfakourii mbfakourii changed the title feat: Add saveEventually and deleteEventually in ParseObject feat: Added saveEventually and deleteEventually in ParseObject May 21, 2023
@mbfakourii
Copy link
Member Author

@mtrezza @Nidal-Bakir
Do you think the implemented method is appropriate?

@mtrezza mtrezza requested a review from a team May 21, 2023 15:54
@Nidal-Bakir
Copy link
Member

I suggest looking at what other SDKs do and doing the same. There is the concept of a Queue so the objects get saved in order.

Do other SDKs save the unsaved Objects even when the app is closed(destroyed) using native services (android and iOS) when the connection is restored for example?

@mbfakourii
Copy link
Member Author

mbfakourii commented May 22, 2023

I suggest looking at what other SDKs do and doing the same. There is the concept of a Queue so the objects get saved in order.

Do other SDKs save the unsaved Objects even when the app is closed(destroyed) using native services (android and iOS) when the connection is restored for example?

I think the queue issue happens in dio or Http and we don't need to create this queue.

According to the ios documentation

saveEventually

Saves this object to the server at some unspecified time in the future, even if Parse is currently inaccessible.

Use this when you may not have a solid network connection, and don’t need to know when the save completes. If there is some problem with the object such that it can’t be saved, it will be silently discarded.

Objects saved with this method will be stored locally in an on-disk cache until they can be delivered to Parse. They will be sent immediately if possible. Otherwise, they will be sent the next time a network connection is available. Objects saved this way will persist even after the app is closed, in which case they will be sent the next time the app is opened. If more than 10MB of data is waiting to be sent, subsequent calls to -saveEventually will cause old saves to be silently discarded until the connection can be re-established, and the queued objects can be saved.

Based on what I understand, the internal structure of Android is implemented with this concept. If possible, take a look at the Android structure.

The concept that I didn't understand was where to double check and send the saved it objects?

I wrote the codes of this section in the await ParseObject.submitEventually() function! And I allow the user to use it wherever they need. But not sure about that !

Copy link
Member

@Nidal-Bakir Nidal-Bakir left a comment

Choose a reason for hiding this comment

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

Overall everything is good.

I have some thoughts:

  • What will happen if the current user logs out?
    • should we clear the cache? or could you give the end user the option to decide what should happen?
  • should we give the end user control over the cached objects that need to be saved/deleted?

@@ -677,4 +722,89 @@ class ParseObject extends ParseBase implements ParseCloneable {
return this;
}
}

static Future<ParseResponse?> submitEventually(
Copy link
Member

Choose a reason for hiding this comment

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

who should call this faction the end-user or the SDK itself when initialized?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly my problem is here, I don't know where this function should be used, whether the user should call or the SDK and where in the SDK

@mbfakourii
Copy link
Member Author

mbfakourii commented May 26, 2023

Overall everything is good.

I have some thoughts:

  • What will happen if the current user logs out?

    • should we clear the cache? or could you give the end user the option to decide what should happen?
  • should we give the end user control over the cached objects that need to be saved/deleted?

At the moment, the caches are not deleted, but I think a function should be written to delete the cache

Regarding the second question, I did not see anything in other SDKs. Do you have any suggestions for this? What control do you mean?

@Nidal-Bakir
Copy link
Member

Nidal-Bakir commented May 26, 2023

At the moment, the caches are not deleted, but I think a function should be written to delete the cache

Got it.

Regarding the second question, I did not see anything in other SDKs. Do you have any suggestions for this? What control do you mean?

I mean what if the user wants to clear the currently cached objects for saving/deleting and start over?
Should we expose such a feature?

Or in general, should we expose the current cache state so the end user can alter it?

If yes we should consider a saving mechanism like functions to be invoked. for example:

  • onBeforStartSaving()
  • shouldSave()
  • shouldDelete()
  • onBeforSave()
  • onBeforDelete()
  • onAfterSave()
  • onAfterDelete()
  • onAllSaved()
  • onAllDeleted()

something like that

@mbfakourii
Copy link
Member Author

I mean what if the user wants to clear the currently cached objects for saving/deleting and start over? Should we expose such a feature?

Or in general, should we expose the current cache state so the end user can alter it?

If yes we should consider a saving mechanism like functions to be invoked. for example:

  • onBeforStartSaving()
  • shouldSave()
  • shouldDelete()
  • onBeforSave()
  • onBeforDelete()
  • onAfterSave()
  • onAfterDelete()
  • onAllSaved()
  • onAllDeleted()

something like that

I don't think this is necessary. I haven't seen anything about this in other SDKs. Do you have a reason why it needs to be added?

@Nidal-Bakir
Copy link
Member

Nidal-Bakir commented May 26, 2023

Do you have a reason why it needs to be added?

No, I'm just wounding if we need to give the end user finer control over the eventual save in general. If the other SDKs do not do that we are good.

So currently, we need to discuss how the submitEventually function should be called and what will happen when the user logs out when there are some unsaved objects.

@mbfakourii mbfakourii mentioned this pull request Aug 9, 2023
@mtrezza
Copy link
Member

mtrezza commented Aug 9, 2023

What's missing in this PR to get it merged?

@mbfakourii
Copy link
Member Author

mbfakourii commented Aug 10, 2023

What's missing in this PR to get it merged?

The code is finished, the tests must be written

@mbfakourii mbfakourii marked this pull request as ready for review October 3, 2023 19:16
@mbfakourii mbfakourii requested a review from a team October 3, 2023 19:16
@mbfakourii
Copy link
Member Author

The error related to Test Flutter beta has been resolved in this PR

@mbfakourii mbfakourii requested a review from a team October 7, 2023 18:43
@mbfakourii
Copy link
Member Author

@mtrezza

Done 😃

@mtrezza
Copy link
Member

mtrezza commented Oct 9, 2023

Note for review: The Flutter test is failing, hence we need to merge #969 first before merging this PR.

@mbfakourii mbfakourii changed the title feat: Added saveEventually and deleteEventually in ParseObject feat: Add saveEventually and deleteEventually in ParseObject Oct 13, 2023
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Could you check whether I've resolved the conflicts correctly? Then I believe this can be merged.

@mbfakourii
Copy link
Member Author

mbfakourii commented Oct 17, 2023

Could you check whether I've resolved the conflicts correctly? Then I believe this can be merged.

I checked, and there was no problem.

@mtrezza mtrezza merged commit 9431086 into parse-community:master Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

offline capabilities and auto-syncing
3 participants