rexmas / RealmCrust

Simple Crust Extension for Mapping Realm Objects From JSON
MIT License
25 stars 1 forks source link

Another Primary Key issue #7

Open minuscorp opened 8 years ago

minuscorp commented 8 years ago

Hello!

I'm working with latest version of both Realm, RealmCrust and Crust libraries. I'm getting new issues. In the following model with custom transformations:

public class User: Object {

    public dynamic var identifier: String = ""
    public dynamic var name: String? = nil
    public dynamic var surname: String? = nil
    public dynamic var height: Int = 170
    public dynamic var weight: Int = 70
    public dynamic var birthDate: NSDate? = nil
    public dynamic var sex: Int = 2
    public dynamic var photoPath: String? = nil

    override public static func primaryKey() -> String? {
        return "identifier"
    }

}

public class UserMapping: RealmMapping {

    public var adaptor: RealmAdaptor
    public var primaryKeys: Dictionary<String, CRMappingKey>? {
        return ["identifier": "id_hash"]
    }

    public required init(adaptor: RealmAdaptor) {
        self.adaptor = adaptor
    }

    public func mapping(inout tomap: User, context: MappingContext) {
        let birthdateMapping = DateMapping(dateFormatter: NSDateFormatter.birthdateFormatter())
        let primaryKeyMapping = PrimaryKeyMapping()

        tomap.birthDate     <- (KeyExtensions.Mapping("birthdate", birthdateMapping), context)
        tomap.identifier    <- (KeyExtensions.Mapping("id_hash", primaryKeyMapping), context)
        tomap.name          <- ("user_name", context)
        tomap.surname       <- ("user_surname", context)
        tomap.height        <- ("height", context)
        tomap.weight        <- ("weight", context)
        tomap.sex           <- ("sex", context)
        tomap.photoPath     <- ("photo_path", context)
    }
}

extension NSDate: AnyMappable { }

public class DateMapping: Transform {

    public typealias MappedObject = NSDate

    let dateFormatter: NSDateFormatter

    init(dateFormatter: NSDateFormatter){
        self.dateFormatter = dateFormatter
    }

    public func fromJSON(json: JSONValue) throws -> MappedObject {

        switch json {
        case .JSONString(let date):
            return self.dateFormatter.dateFromString(date) ?? NSDate()
        default:
            throw NSError(domain: "", code: 0, userInfo: nil)
        }
    }

    public func toJSON(obj: MappedObject) -> JSONValue {
        return .JSONString(self.dateFormatter.stringFromDate(obj))
    }
}

extension String: AnyMappable { }

public class PrimaryKeyMapping: Transform {

    public typealias MappedObject = String

    public func fromJSON(json: JSONValue) throws -> MappedObject {
        switch json{
        case .JSONNumber(let number):
            return "\(NSNumber(double: number).integerValue)"
        default:
            throw NSError(domain: "", code: -1, userInfo: nil)
        }
    }

    public func toJSON(obj: MappedObject) -> JSONValue {
        return JSONValue.JSONNumber(Double(obj)!)
    }
}

I apply the following tests:

it("Should decode JSON User objects"){

                let json: Dictionary<String, AnyObject> = ["data": ["id_hash": 170, "user_name": "Jorge", "user_surname": "Revuelta", "birthdate": "1991-03-31", "height": 175, "weight": 60, "sex": 2, "photo_path": "http://somwhere-over-the-internet.com/"]]

                let mapping = CRMapper<User, UserMapping>()
                let jsonValue = try! JSONValue(object: json)
                let realmUser = try! mapping.mapFromJSONToExistingObject(jsonValue["data"]!, mapping: UserMapping(adaptor: adaptor))

                expect(realm.objects(User).count) == 1
            }

            it("Should encode JSON User objects"){

                let user = realm.objects(User).first!
                let mapping = CRMapper<User, UserMapping>()
                let json = try! mapping.mapFromObjectToJSON(user, mapping: UserMapping(adaptor: adaptor))

                let id_hash = json["id_hash"]!
                expect(json).notTo(beNil())
                expect(id_hash.values() as? Int) == 170
            }

The second one crashes with Primary key cannot be modified after object is inserted, but I'm not modifying anything from that object, just encoding it to JSON... Just for let you know it.

Also, is there any way to use the side of Crust of PrimaryKeys without using the Realm side? An example: I generate a model that has not primary key until it's synchronized with a server which returns me the primary key for that object, but that synchronization could happen when the model is already saved on the Realm so primaryKey() isn't what I'm looking for. But using Crust primaryKeys should fit using that supposed primary key to update an already saved Model or even update it with information taking in mind that although the model has no primary key itself from Realm, Crust is trying to find the object in the database before creating a new one. I don't know if I'm explaining well.

rexmas commented 8 years ago

Thanks for providing that code, I'll try running that in a test myself and figure out what's going on.

With your second question, are you stating that you:

  1. Optimistically create a model object on the client without a primary key. Insert it into Realm.
  2. PUT (or equivalent) the model object on the server, the returned response includes a primary key.
  3. Attempt to update the object on the Realm ?

I'm not sure what the issue is here exactly, but I will mention that since you cannot update a primaryKey to a Realm Object after it's been inserted, you shouldn't actually have a primaryKey on your model object at all in this case.

However, if you set primaryKeys on a Crust Mapping it should search for an object with the provided keys before it does a mapping. You wouldn't have to make an explicit primaryKey on your Realm Object to do this, it comes for free with the Mapping for that object. (And if that's broken for some reason let me know :)).

An additional thought, instead of creating an object on the client and getting the primary key from the server, have you thought of generating a UUID on the client for that object and then sending that UUID to the server? UUID's were made to avoid collisions in cases like this. You could use NSUUID to do this.

minuscorp commented 8 years ago

That's exactly I ment. I got what you explained in mind but the tests didn't pass I couldn't get it to work any further, erasing the primaryKey from the Model failed the tests, so when we get this working I'll dig deeper.

Tomorrow I'll bring you more news on this!!

rexmas commented 8 years ago

Awesome! Looking forward to hearing more info!

minuscorp commented 8 years ago

Update:

User_spec__Should_decode_JSON_User_objects

/Users/buddybuild/workspace/EbikemotionTests/UserSpec.swift:38
failed: caught "Invalid value", "Expected object of type string for property 'identifier' on object of type 'User', but received: 170"

When running tests without the Realm primaryKey method overriden.

rexmas commented 8 years ago

Thanks for the info, looking into that now.

Btw, in the case with a primaryKey it looks like since the <- operator uses an inout parameter on the field, realm immediately assumes the field will be mutated, and errors with Primary key cannot be modified after object is inserted. I think we'll have to come up with an entirely different method of mapping primary keys to JSON to circumvent this...

In the meantime, I'll try to figure out why this still throws an exception when not even using a primaryKey. That's definitely no good. And you can still map from JSON to Realm even if you can't map in the other direction for now.

rexmas commented 8 years ago

Oh this is why

public dynamic var identifier: String = ""

identifier is a String, yet we're forming a predicate with an Int. Realm doesn't perform automatic type coercion.

Changing 170 to "170" like so: ["data": ["id_hash": "170", fixes the problem. You'll have to change your transform to the following too

public func fromJSON(json: JSONValue) throws -> MappedObject {
        switch json{
        case .JSONString(let string):
            return string
        default:
            throw NSError(domain: "", code: -1, userInfo: nil)
        }
    }
minuscorp commented 8 years ago

But... I'm using a custom transform to cast the Integer returned from the JSON to the String of the Model, Am I wrong?

public class PrimaryKeyMapping: Transform {

    public typealias MappedObject = String

    public func fromJSON(json: JSONValue) throws -> MappedObject {
        switch json{
        case .JSONNumber(let number):
            return "\(NSNumber(double: number).integerValue)"
        default:
            throw NSError(domain: "", code: -1, userInfo: nil)
        }
    }

    public func toJSON(obj: MappedObject) -> JSONValue {
        return JSONValue.JSONNumber(Double(obj)!)
    }
}
rexmas commented 8 years ago

Huh, you bring up a good point. I hadn't written the Transform to map the data before a search query. Sounds reasonable though and a feature we should add. I'll see what kind of solution I can come up with.

minuscorp commented 8 years ago

Ooooooh is that why! I didn't even realized It was a conflict with the way the data checked the existance of an object! You might think of me as a nighmare from now on... :dizzy_face:

rexmas commented 8 years ago

Lol! No, it's totally a good thing you're bringing up points like this I hadn't considered. Just makes the library more robust.

We could probably add a transform to the primaryKeys var

public var primaryKeys: Dictionary<String, CRMappingKey>? {
    return [ "identifier": .Mapping("id_hash", PrimaryKeyMapping()) ]
}

We may even want to enhance primaryKeys further to avoid Primary key cannot be modified after object is inserted. I came up with this design over the last hour:

public var primaryKeys: Dictionary<String, CRMappingKey>? {
   return [ "identifier": .Primary("id_hash", PrimaryKeyMapping(),
   { tomap: MappedObject, map: PrimaryMapping in
        tomap.identifier <- map
    }) ]
}

PrimaryMapping would be a new type constructed at runtime with the "identifier" and PrimaryKeyMapping you supplied. The mapping is wrapped in a closure and would only be executed in the case of generating a new object to map the primary key to. We'd avoid having to use inout for tomap in this case because only a reference type would ever need to follow these semantics.

It's a little complex, but should cover every edge case. This work for you?

minuscorp commented 8 years ago

It seems fine to me, I actually like closures... haha. I think that with this we solve the issue where the inout with the primary key throws the exception. And I think that this kind of usage is acceptable in the library, with documentation doesn't add up anything too hard to understand.