Skip to content
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

Using FEMMapping as NSMapTable key leads to duplicated objects #96

Closed
jcavar opened this issue May 19, 2017 · 13 comments
Closed

Using FEMMapping as NSMapTable key leads to duplicated objects #96

jcavar opened this issue May 19, 2017 · 13 comments

Comments

@jcavar
Copy link
Contributor

jcavar commented May 19, 2017

Hi,
It new version, FEM uses FEMMapping instead entity name as before as NSMapTable key. When using recursive relationships and therefore different FEMMapping in child object and when processing child, it tries to get object from cache but since child mapping is different it can't get it and inserts new object.

Simple solution is I think just to use mapping.entityName as key.

@dimazen
Copy link
Contributor

dimazen commented May 19, 2017

Can you share code of the mapping that you're using as well as some sample JSON, please?

@dimazen
Copy link
Contributor

dimazen commented May 19, 2017

Side note about why I rewrote code to use FEMMapping as a key instead of entityName and why we can't go back: initially I was planning to use the same prefetch code for the Realm extension. However later it turned out that no prefetch required for the Realm. So why I didn't move the code back? In the next version I'm planning to deprecate entityName, and to leave simply objectClass for all of the cases. Using class instead of the entityName gives me ability to cache setter implementations and to speed up deserialization.

@jcavar
Copy link
Contributor Author

jcavar commented May 19, 2017

Here is simplified version:

Chat:

+ (FEMMapping *)recursiveMapping {
    FEMMapping *mapping = [[FEMMapping alloc] initWithEntityName:@"WCTChat"];
    [mapping addAttribute:[WCTChat identifierAttribute]];
    mapping.primaryKey = @"identifier";
    return mapping;
}

+ (FEMMapping *)defaultMapping {
    FEMMapping *mapping = [[FEMMapping alloc] initWithEntityName:@"WCTChat"];
    [mapping addAttribute:[WCTChat identifierAttribute]];
    [mapping addRelationshipMapping:[WCTMessage defaultMapping]
                        forProperty:@"lastMessage"
                            keyPath:@"lastMsg"];
    mapping.primaryKey = @"identifier";
    return mapping;
}

Message:

+ (FEMMapping *)defaultMapping {
    FEMMapping *mapping = [[FEMMapping alloc] initWithEntityName:@"WCTMessage"];
    [mapping addRelationshipMapping:[WCTChat recursiveMapping]
                        forProperty:@"chat"
                            keyPath:@"chat"];
    mapping.primaryKey = @"identifier";
    return mapping;
}

JSON:

"chat" : {
    "idChat" : 1,
    "lastMsg" : {
        "chat" : { idChat" : 1 },
        "idMessage" : 345
    }
}

@jcavar
Copy link
Contributor Author

jcavar commented May 19, 2017

Sure, objectClass should be fine as well.

@jcavar
Copy link
Contributor Author

jcavar commented May 19, 2017

And also, I know that we can't do much about it now, but I think it would be much nicer if there was bug fixes release before Realm support. We wanted to get temporary fix for #80 but had to include whole Realm support as well, which in the end it seems broke some of functionality.

@dimazen
Copy link
Contributor

dimazen commented May 19, 2017

Agree, that to rollout a bugfix release would be a better option. Anyway it is a little bit late.

@dimazen
Copy link
Contributor

dimazen commented May 19, 2017

I'll try to release a hotfix within 1.5 hours

@dimazen
Copy link
Contributor

dimazen commented May 19, 2017

Observation: @jcavar do you have an inverse relationship? It looks like that you're trying to map chat to the message that is already included into the chat property.

dimazen pushed a commit that referenced this issue May 19, 2017
dimazen pushed a commit that referenced this issue May 19, 2017
dimazen pushed a commit that referenced this issue May 19, 2017
dimazen pushed a commit that referenced this issue May 19, 2017
@dimazen dimazen mentioned this issue May 19, 2017
dimazen pushed a commit that referenced this issue May 20, 2017
dimazen pushed a commit that referenced this issue May 20, 2017
dimazen pushed a commit that referenced this issue May 20, 2017
dimazen pushed a commit that referenced this issue May 20, 2017
@dimazen
Copy link
Contributor

dimazen commented May 20, 2017

@jcavar this issue has been fixed in the 1.2.1 release.
Available in both Carthage and Pods (don't forget to update your repo)!

@dimazen dimazen closed this as completed May 20, 2017
@jcavar
Copy link
Contributor Author

jcavar commented May 20, 2017

@dimazen thank you, that was fast 👍

Yes, we have inverse relationship from message to chat.

@dimazen
Copy link
Contributor

dimazen commented May 20, 2017

@jcavar what I meant by my observation is that you probably don't need nested Chat mapping, since Message already included into the Chat and by using inverse relationship CoreData will be able to set a correct chat value on the message object.

In any case thanks for your report!

@jcavar
Copy link
Contributor Author

jcavar commented May 20, 2017

Yes, that is good point, but we have explicitly disabled inverse relationship in CoreData editor for some reason.

@dimazen
Copy link
Contributor

dimazen commented May 20, 2017

Got it. Good luck with your project 👍

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

No branches or pull requests

2 participants