fix: Error when updating ParseObject with relation to another object #805

juriSacchetta · 2022-12-09T13:33:06Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

Closes: #696

If you try to add a relationship to an object and after you do another operation to it you'll face this error:

Unhandled Exception: type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'ParseRelation?' in type cast

By adding a relation, the set method of ParseObject changes the value corresponding to the key. Thus, the next time the relation is attempted, there will be no instance of ParseRelation but a Map.

final PostModel = ...;
final LikeModel like = LikeModel();

like.onPost = post;
await like.save();

post.likeRelation.add(like); // or post.addRelation('likes', [post]);
ParseResponse resp = await post.update();
if (resp.success) {
  post.likeRelation.remove(like); // Bad
}

Approach

To solve this bug, I had to insert an if within the set method, but it is not a clean solution. I could not find a better solution. Honestly, I think using the same map to store both data and operations is a risky decision.

The last change concerns the creation of a relation in the client.
Every time the relation was accessed, a new one was created, thus also losing the cache function that was already implemented.
I tried to always return the same instance.

TODOs before merging

Add tests
Add changes to documentation (guides, repository pages, in-code descriptions)
A changelog entry

parse-github-assistant · 2022-12-09T13:33:09Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

mtrezza · 2022-12-09T16:20:19Z

~~Could you please create an issue, move the explanation to that issue and just reference the issue in this PR?~~

I just noticed you changed some links in the template, I will correct that.

codecov · 2022-12-09T16:24:59Z

Codecov Report

Patch coverage: 57.89% and project coverage change: -4.61 ⚠️

Comparison is base (d7339cd) 26.71% compared to head (7843787) 22.11%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #805      +/-   ##
==========================================
- Coverage   26.71%   22.11%   -4.61%     
==========================================
  Files          47       47              
  Lines        2905     2908       +3     
==========================================
- Hits          776      643     -133     
- Misses       2129     2265     +136

Impacted Files	Coverage Δ
packages/dart/lib/src/utils/parse_decoder.dart	`73.58% <ø> (ø)`
packages/dart/lib/src/objects/parse_relation.dart	`47.61% <11.11%> (-6.55%)`	⬇️
packages/dart/lib/src/objects/parse_object.dart	`48.76% <100.00%> (-41.92%)`	⬇️

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

juriSacchetta · 2022-12-09T17:04:00Z

I was testing my code, it introduces other problems. Please, do not merge this version.

I'll send another version of it. I'm already working on it!

This reverts commit 4e48925

juriSacchetta · 2022-12-09T18:20:59Z

It should work now 😁

Let me know!!

parse-github-assistant · 2022-12-10T01:23:11Z

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

mbfakourii

The code has been tested, it seems good.
@juriSacchetta is it possible to add the test?

juriSacchetta · 2022-12-10T21:27:14Z

At the moment, I didn't write a test but I've tested it inside my project. It seems to work fine.

Maybe in the next few days, I'll write one. I can't guarantee it, my schedule is very busy sorry.

mtrezza · 2022-12-12T00:45:50Z

Thanks for your feedback, we'll keep this open until a test is added.

juriSacchetta · 2023-02-22T09:31:47Z

Hey guys I added a new test relation specific. Let me know if it can be enough :)

mtrezza · 2023-02-22T15:13:22Z

Rebased and started CI

mbfakourii

Tests are not like other tests, please write like other samples
Please write based on this file

packages/dart/test/relation_test.dart

mbfakourii

The rest of the parts are fine, just move the duplicate codes to the setup section

packages/dart/test/relation_test.dart

xanderelsmith · 2023-02-26T21:49:20Z

Hi , please i keep getting this issue "Unhandled Exception: type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'ParseRelation?' in type cast"

how dp i fix it?

mtrezza · 2023-02-26T23:18:11Z

@xanderelsmith is this related to this PR?

mbfakourii · 2023-02-27T10:37:04Z

Hi , please i keep getting this issue "Unhandled Exception: type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'ParseRelation?' in type cast"

how dp i fix it?

This issue is not related to this section. Please open an issue if you have a problem

juriSacchetta · 2023-02-27T10:43:41Z

Hey there!
I noticed that my branch has been merged, so can I close the PR? Thanks again :)

mbfakourii

It was tested and there is no problem

mbfakourii · 2023-02-27T10:49:52Z

Hey there! I noticed that my branch has been merged, so can I close the PR? Thanks again :)

No, it has not been merged yet

xanderelsmith · 2023-02-27T11:55:58Z

@xanderelsmith is this related to this PR?

Yes

mtrezza · 2023-02-27T12:17:00Z

@xanderelsmith How is it related to this PR? Could you give more details?

xanderelsmith · 2023-02-27T12:24:46Z

It throws an error, when removing a parse object from a parse relation

xanderelsmith · 2023-02-27T12:25:53Z

You said something about the data becoming a map once
newobject.remove() is done

xanderelsmith · 2023-02-27T12:28:09Z

New Pull Request Checklist

I am not disclosing a vulnerability.

I am creating this PR in reference to an issue.

Issue Description

Closes: #696

If you try to add a relationship to an object and after you do another operation to it you'll face this error:

Unhandled Exception: type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'ParseRelation?' in type cast

By adding a relation, the set method of ParseObject changes the value corresponding to the key. Thus, the next time the relation is attempted, there will be no instance of ParseRelation but a Map.
final PostModel = ...;
final LikeModel like = LikeModel();

like.onPost = post;
await like.save();

post.likeRelation.add(like); // or post.addRelation('likes', [post]);
ParseResponse resp = await post.update();
if (resp.success) {
  post.likeRelation.remove(like); // Bad
}
Approach

To solve this bug, I had to insert an if within the set method, but it is not a clean solution. I could not find a better solution. Honestly, I think using the same map to store both data and operations is a risky decision.

The last change concerns the creation of a relation in the client. Every time the relation was accessed, a new one was created, thus also losing the cache function that was already implemented. I tried to always return the same instance.

TODOs before merging

Add tests

Add changes to documentation (guides, repository pages, in-code descriptions)

A changelog entry

This🤲

juriSacchetta · 2023-03-24T11:29:04Z

Hey guys, I tested this code version and it seems to work fine for me. @xanderelsmith can you give me more details?

Nidal-Bakir

Thanks for your contribution @juriSacchetta, I'm working to fix this issue and other issues related to using Maps to store the operations, I'm using a more OOP approach to improve and fix all of them using ParseOperations, and the ParseMergeTool class will be removed. I will definitely take a look at your code and see if I use any of it. Thanks again for your work.

Here is my PR: #860

@parse-community/flutter-sdk-review, I'd like to request that we don't merge this pull request just yet. Can we hold off for a bit? I'm working on a more comprehensive solution.

Thanks

mtrezza · 2023-03-25T00:32:25Z

Fine with me, @mbfakourii what do you think?

Do you have an ETA for the other PR you're working on? If it takes longer, another possibility may be to merge this for a quick fix and mere the comprehensive fix later on.

Nidal-Bakir · 2023-03-25T00:44:52Z

I'm currently working on the ParseRelation and it is the last thing to do on my list of TODOs, then I will add tests and documentation for the functions and classes and then I will do any refactoring/cleaning when I look at it again.

I think I need a week or less.

another possibility may be to merge this for a quick fix and merge the comprehensive fix later on.

Sure, that sounds good. Give me some time to review and test this PR.

Nidal-Bakir · 2023-03-25T12:59:04Z

This is a breaking change! I'm sorry, but it's not a quick fix anymore.
We need to be careful with it and open a discussion about it.

Nidal-Bakir · 2023-03-25T13:06:16Z

Can you update your master branch to be in sync with upstream, so you can see the added tests? I just checked out your master and pulled the test files from upstream, and the tests are reporting syntax errors and failing.

mbfakourii · 2023-03-26T07:31:42Z

Fine with me, @mbfakourii what do you think?

Do you have an ETA for the other PR you're working on? If it takes longer, another possibility may be to merge this for a quick fix and mere the comprehensive fix later on.

I think it is good and there is no problem

juriSacchetta · 2023-03-29T11:34:12Z

Fine with me, @mbfakourii what do you think?

Do you have an ETA for the other PR you're working on? If it takes longer, another possibility may be to merge this for a quick fix and mere the comprehensive fix later on.

Hi guys, I really need this fix as soon as possible. This solution would be perfect for me.
Please resolve fast this problem in any way you prefer.

Thank you for your work 😁

Nidal-Bakir · 2023-03-29T11:43:34Z

Hi guys, I really need this fix as soon as possible. This solution would be perfect for me.
Please resolve fast this problem in any way you prefer.

We are working on it!

You can just use the SDK as an embedded package in your project and import it using its path: ./ in the pubspec.yaml
when you have the SDK embedded in your project you can just do and fix it as you wish.

here is a code snippte:

  parse_server_sdk_flutter:
    path: ./Parse-SDK-Flutter/packages/flutter

see (https://docs.flutter.dev/development/packages-and-plugins/using-packages#dependencies-on-unpublished-packages)

Nidal-Bakir · 2023-05-13T21:59:50Z

Completed via #860

juriSacchetta added 2 commits December 9, 2022 14:32

Fixed issue parse-community#696

4e48925

Find problem with custom object

2d52e2d

juriSacchetta added 2 commits December 9, 2022 18:12

Revert "Fixed issue parse-community#696"

9e9ef64

This reverts commit 4e48925

Fixed relation for custom object.

2805339

mtrezza changed the title ~~Fix issue #696, bug in the update handling for relations in ParseObject~~ fix: error when updating ParseObject with relation to another object Dec 10, 2022

parse-github-assistant bot changed the title ~~fix: error when updating ParseObject with relation to another object~~ fix: Error when updating ParseObject with relation to another object Dec 10, 2022

mbfakourii previously approved these changes Dec 10, 2022

View reviewed changes

Added test for relation operations

f56250d

juriSacchetta dismissed mbfakourii’s stale review via f56250d February 22, 2023 09:15

mbfakourii suggested changes Feb 23, 2023

View reviewed changes

mbfakourii reviewed Feb 23, 2023

View reviewed changes

packages/dart/test/relation_test.dart Outdated Show resolved Hide resolved

mbfakourii reviewed Feb 23, 2023

View reviewed changes

packages/dart/test/relation_test.dart Outdated Show resolved Hide resolved

juriSacchetta force-pushed the master branch from 06f9f69 to 92770f8 Compare February 25, 2023 15:58

juriSacchetta and others added 2 commits February 25, 2023 16:58

standardized test

92770f8

Merge branch 'master' into master

319b9ea

mbfakourii suggested changes Feb 25, 2023

View reviewed changes

packages/dart/test/relation_test.dart Show resolved Hide resolved

packages/dart/test/relation_test.dart Outdated Show resolved Hide resolved

juriSacchetta and others added 2 commits February 26, 2023 16:11

Moved Parse initialization into setup phase

6c0a32d

Merge branch 'master' into master

ab1e701

mtrezza requested a review from mbfakourii February 26, 2023 23:18

mbfakourii approved these changes Feb 27, 2023

View reviewed changes

Nidal-Bakir mentioned this pull request Mar 1, 2023

adding or removing values from or into an array and then calling get returns _Map<String, dynamic> #834

Closed

4 tasks

Nidal-Bakir reviewed Mar 24, 2023

View reviewed changes

Nidal-Bakir added the state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message label Mar 25, 2023

Merge branch 'master' into master

7843787

Nidal-Bakir closed this May 13, 2023

fix: Error when updating ParseObject with relation to another object #805

fix: Error when updating ParseObject with relation to another object #805

Conversation

juriSacchetta commented Dec 9, 2022 • edited by mtrezza

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

parse-github-assistant bot commented Dec 9, 2022 • edited

Thanks for opening this pull request!

mtrezza commented Dec 9, 2022 • edited

codecov bot commented Dec 9, 2022 • edited

Codecov Report

juriSacchetta commented Dec 9, 2022

juriSacchetta commented Dec 9, 2022

parse-github-assistant bot commented Dec 10, 2022

mbfakourii left a comment • edited

Choose a reason for hiding this comment

juriSacchetta commented Dec 10, 2022

mtrezza commented Dec 12, 2022 • edited

juriSacchetta commented Feb 22, 2023

mtrezza commented Feb 22, 2023

mbfakourii left a comment • edited

Choose a reason for hiding this comment

mbfakourii left a comment

Choose a reason for hiding this comment

xanderelsmith commented Feb 26, 2023

mtrezza commented Feb 26, 2023

mbfakourii commented Feb 27, 2023

juriSacchetta commented Feb 27, 2023

mbfakourii left a comment

Choose a reason for hiding this comment

mbfakourii commented Feb 27, 2023

xanderelsmith commented Feb 27, 2023

mtrezza commented Feb 27, 2023

xanderelsmith commented Feb 27, 2023

xanderelsmith commented Feb 27, 2023

xanderelsmith commented Feb 27, 2023

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

juriSacchetta commented Mar 24, 2023

Nidal-Bakir left a comment • edited

Choose a reason for hiding this comment

mtrezza commented Mar 25, 2023

Nidal-Bakir commented Mar 25, 2023 • edited

Nidal-Bakir commented Mar 25, 2023 • edited

Nidal-Bakir commented Mar 25, 2023

mbfakourii commented Mar 26, 2023

juriSacchetta commented Mar 29, 2023

Nidal-Bakir commented Mar 29, 2023 • edited

Nidal-Bakir commented May 13, 2023

juriSacchetta commented Dec 9, 2022 •

edited by mtrezza

parse-github-assistant bot commented Dec 9, 2022 •

edited

mtrezza commented Dec 9, 2022 •

edited

codecov bot commented Dec 9, 2022 •

edited

mbfakourii left a comment •

edited

mtrezza commented Dec 12, 2022 •

edited

mbfakourii left a comment •

edited

Nidal-Bakir left a comment •

edited

Nidal-Bakir commented Mar 25, 2023 •

edited

Nidal-Bakir commented Mar 25, 2023 •

edited

Nidal-Bakir commented Mar 29, 2023 •

edited