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

Property named id not possible when using @Persisted property wrapper #7372

Open
miklselsoe opened this issue Aug 2, 2021 · 10 comments
Open

Comments

@miklselsoe
Copy link

miklselsoe commented Aug 2, 2021

When declaring a class with @Persisted properties, a property named _id cannot exist alongside a property named id.

Code Sample

class DemoClass: Object {
    @Persisted(primaryKey: true) var _id: ObjectId = ObjectId.generate() // Error:  Invalid redeclaration of synthesized property '_id'
    @Persisted var id: String = ""
}

This scenario works when using the old @objc dynamic var declaration of properties:

class DemoClass: Object {
    @objc dynamic var _id: ObjectId = ObjectId.generate()
    @objc dynamic var id: String = ""
	
    override static func primaryKey() -> String? {
        return "_id"
    }
}

Version of Realm and Tooling

Realm framework version: 10.11.0

Xcode version: 12.5.1

iOS/OSX version: 14.5

Dependency manager + version: Carthage 0.38.0

@tgoyne
Copy link
Member

tgoyne commented Aug 2, 2021

This is just how the swift property wrapper feature works, so there probably isn't anything we can do about it.

@drmarkpowell
Copy link

I have this same problem and I have a requirement to use a field named "id" in addition to "_id". Is there nothing that can be done with @persisted to work around this, or are we stuck doing our model classes the "old way"?

And it's all or nothing, right? We can't mix @persisted with objc dynamic var because once we use @persisted once in a class, it will ignore the fields that use objc dynamic according to the docs:

If a class has at least one @persisted property, all other properties will be ignored by Realm. This means that they will not be persisted and will not be usable in queries and other operations such as sorting and aggregates which require a managed property.

@tgoyne
Copy link
Member

tgoyne commented Aug 5, 2021

A dumb workaround would be to define the id property in a base class using the legacy syntax, and then inherit from that and define the rest of your properties in the subclass. You can't mix the two in a single class definition, but you can mix them within a class hierarchy.

Is your _id property just to conform with sync's requirement and something that you aren't actually using directly? Independently of this I've been thinking that we should just synthesize that property if you don't explicitly declare it when using sync and that would sorta sidestep this problem in some cases.

@drmarkpowell
Copy link

drmarkpowell commented Aug 5, 2021

I could deal with the dumb workaround for now! I have ~20 model classes that all have an id property that I could use in a base class and extend that ~20 times into specific model classes with @persisted properties. That sounds pretty good to me for the time being.

Yes, the _id property is just to conform to the Atlas/Realm Sync requirements that use that name automatically...although the part of our system that populates Atlas with data outside of Realm is using this _id name as well as it inserts documents into collection (or perhaps that's Atlas itself?)

I'm trying this out and I'm getting many cases where I encounter this error:

Sync: Connection[1]: Session[1]: Failed to transform received changeset: Schema mismatch: 'mymodelclass' has primary key '_id', which is nullable on one side, but not the other

When this happens, and I have a declaration in Swift that looks like

    @Persisted(primaryKey: true) var _id: ObjectId

Does this mean that _id is nullable on the server side? Or, have I somehow become nullable on the client side?

Also, if I declare a base class like

class IDObject: Object {
    @objc dynamic var id: Int = 0

I get an error that I haven't defined an _id primary key for this class. If I try to put just the _id in the base class, when I extend the class, I get the same error as this original issue (synthesizing _id collides with the other definition of _id.)
I can put both id and _id in the base class, and use @persisted in the extended class...that seems to compile...but now I have this schema mismatch issue going on so Realm Sync just stops with that error.

@tgoyne
Copy link
Member

tgoyne commented Aug 5, 2021

Ah, you'll need to put both _id and id in the base class so that the base class has a primary key. You also could explicitly specify objectTypes in your configuration to exclude the base class so that it doesn't need to be sync-compatible. A more "clever" way to do that while continuing to use automatic type discovery would be something like class BaseClass { override class func shouldIncludeInDefaultSchema() -> Bool { self != BaseClass.self } }.

That error message from sync should mean that the PK is optional on the server. It'd be nice if the error actually said which one was which, but I think the code doing the merging may not actually know that.

@drmarkpowell
Copy link

Thanks @tgoyne !

Here is what worked for me today: (RealmSwift SDK 10.12)

class IDObject: Object {
    @objc dynamic var id: Int = 0
    @objc dynamic var _id: ObjectId = ObjectId.generate()

    override class func primaryKey() -> String? {
        return "_id"
    }

    override class func shouldIncludeInDefaultSchema() -> Bool {
        self != IDObject.self
    }
}

class mymodel: IDObject, ObjectKeyIdentifiable {
    @Persisted var name: String?
}

I also needed to make both "_id" and "id" required properties on the Atlas Realm Schema end, so they would not be nullable.

Data syncs, so we're good to go for now with this implementation.

@rromanchuk
Copy link

All my models have a primary key of @Persisted(primaryKey: true) var id: String, (i consume from JSONAPI) for the last few hours i've been trying to figure out why all of a sudden one of my models is generating a schema, just for this model, with an id of string but not marked as a primary key.

The only thing i did today was integrate/update a few swiftui components we have, and that included adding ObjectKeyIdentifiable

https://github.com/realm/realm-cocoa/blob/v10.5.1/RealmSwift/Combine.swift#L38

Obviously this var id: UInt64 { get } would fail to compile since i have a String id, but this extension https://github.com/realm/realm-cocoa/blob/v10.5.1/RealmSwift/Combine.swift#L47 is squashing the issue.

Still dont understand how schema is able able to get the string id in there, but that extension sure looks dangerous.

@rromanchuk
Copy link

https://github.com/realm/realm-cocoa/blob/master/Realm/RLMObject_Private.h#L83

This is confusing, i don't understand the motivation. Is this MongoDB specifics leaking into the SDK or something? My models already conform to Identifiable. Why does @ObservedRealmObject require ObjectKeyIdentifiable conformity? Shouldn't Identifiable be enough?

/// You can also manually conform to Identifiable if you wish, but note that
/// using the object's memory address does not work for managed objects.

This is confusing. Are you just trying to say if your model doesn't have a primary key, you can't conform to Identifiable, and you obviously cant just randomly hash an instance of Object and expect it to be unique. Or is this something internal related to swiftui

@tgoyne
Copy link
Member

tgoyne commented Aug 12, 2021

ObservedRealmObject shouldn't require ObjectKeyIdentifiable and I think that's just an oversight. ObjectKeyIdentifiable is intended as just a convenience helper for making types without primary keys conform to Identifiable.

@tgoyne
Copy link
Member

tgoyne commented Aug 13, 2021

#7391 relaxes the protocol requirement from ObjectKeyIdentifiable to Identifiable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants