Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Copy EmbeddedObject on initializing an unmanaged object#7301

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
pavel-ship-it wants to merge17 commits intomaster
base:master
Choose a base branch
Loading
frompy/copy_embedded_on_add

Conversation

@pavel-ship-it
Copy link
Contributor

When creating an unmanaged copy of a managed object with the embedded object property we should make a copy of this embedded object instead of reusing it.

This isfix#6921

Copy link
Contributor

@dianaafanador3dianaafanador3 left a comment

Choose a reason for hiding this comment

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

Just a minor comments

Copy link
Contributor

@dianaafanador3dianaafanador3 left a comment

Choose a reason for hiding this comment

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

LGTM, only comment It looks like SwiftLint CI Job is failing, because of format issues.

@tgoyne
Copy link
Member

We only want to copy the object if it's managed.

Comment on lines 96 to 97
if (([objisKindOfClass:cls] && ![(id)clsisEmbedded]) ||
([(id)clsisEmbedded] && [objrespondsToSelector:@selector(realm)] && ![objrealm])) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (([objisKindOfClass:cls] && ![(id)clsisEmbedded]) ||
([(id)clsisEmbedded] && [objrespondsToSelector:@selector(realm)] && ![objrealm])) {
if ([objisKindOfClass:cls] && (![(id)clsisEmbedded] || ![objrealm])) {

I think what you have would callisEmbedded on classes that don't implement it and would accept objects of any type as long as they're an unmanaged embedded object. This also points at some missing tests for initializing an unmanaged object with a value that has an object of a different type in an embedded object field (which copies that object into an unmanaged object of the correct type if all of the required fields are present and have the correct values, and throws an exception if it can't).

@stoneyMDBstoneyMDB marked this pull request as draftMay 17, 2022 15:05
@afsmarques
Copy link

so, no more updates on this? 😕

@dianaafanador3dianaafanador3force-pushed thepy/copy_embedded_on_add branch 2 times, most recently from27eee84 to7af1c9cCompareSeptember 20, 2022 15:16
@dianaafanador3dianaafanador3force-pushed thepy/copy_embedded_on_add branch 2 times, most recently fromd793f69 to62a7f79CompareOctober 11, 2022 08:36
@dianaafanador3
Copy link
Contributor

@tgoyne checking this, I think the changes regarding the comments were made, looking at the original issue I think some test are missing what the original intention was, something like the follow.

func testCopyEmbeddedObjectFromManagedObjectInSameRealm() {        let realm = try! Realm()        try! realm.write {            let parent = realm.create(EmbeddedPrimaryParentObject.self, value: [                "object": ["value": 1],                "array": [[2]]            ])            // Copy Managed object            let copyA = EmbeddedPrimaryParentObject(value: parent)            realm.add(copyA, update: .modified)            XCTAssertEqual(realm.objects(EmbeddedPrimaryParentObject.self).count, 1)            XCTAssertEqual(parent, copyA)            XCTAssertEqual(copyA.object!.value, 1)            XCTAssertEqual(copyA.array.count, 1)            // Explicit copy of object            let copyB = EmbeddedPrimaryParentObject()            copyB.object = EmbeddedTreeObject1(value: parent.object!)            copyB.array.append(EmbeddedTreeObject1(value: parent.array[0]))            realm.add(copyB, update: .modified)            XCTAssertEqual(realm.objects(EmbeddedPrimaryParentObject.self).count, 1)            XCTAssertEqual(parent, copyB)            XCTAssertEqual(copyB.object!.value, 1)            XCTAssertEqual(copyB.array.count, 1)            // Assign of EmbeddedObject            let copyC = EmbeddedParentObject()            copyC.object = parent.object            assertThrows(realm.add(copyC), "Cannot set a link to an existing managed embedded object")            let parentUnmanaged = EmbeddedPrimaryParentObject(value: [                "object": ["value": 4],                "array": [[5]]            ])            // Do not copy unmanaged object            let copyD = EmbeddedPrimaryParentObject(value: parentUnmanaged)            XCTAssertTrue(copyD.object === parentUnmanaged.object)            assertThrows(realm.add(parentUnmanaged), "Cannot set a link to an existing managed embedded object")        }    }

Let me know if there are more changes needed on this, or If I should change the current text with the text above.

@realmrealm deleted a comment fromcla-botbotFeb 27, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@tgoynetgoynetgoyne left review comments

+1 more reviewer

@dianaafanador3dianaafanador3dianaafanador3 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Exception on saving of Unmanaged Object with Embedded Object

5 participants

@pavel-ship-it@tgoyne@afsmarques@dianaafanador3
[8]ページ先頭
©2009-2025 Movatter.jp