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

Modify is not working properly #27

Open
voronianski opened this issue Dec 12, 2016 · 7 comments
Open

Modify is not working properly #27

voronianski opened this issue Dec 12, 2016 · 7 comments

Comments

@voronianski
Copy link

I noticed a critical bug in driver behavior. Modify functionality is not working as expected. I think issue is connected to #25 but I want to describe it in more detail. Steps to reproduce (you can run locally example app - https://github.com/voronianski/vapor-server-example/ to repro):

  • save 2 documents in db via endpoint like curl -H "Content-Type: application/json" -X POST -d '{"text":"some"}' POST https://localhost:8080/posts
  • check in database
{ "_id" : ObjectId("5f214f58d66cdc2b53b99b99"), "id" : null, "text" : "some" }
{ "_id" : ObjectId("63214f58d66cdc2b54b99b99"), "id" : null, "text" : "some 2" }
  • send modify request to one of the items curl -H "Content-Type: application/json" -X POST -d '{"text":"some change"}' PATCH https://localhost:8080/posts/63214f58d66cdc2b54b99b99
  • check in database:
{ "_id" : ObjectId("5f214f58d66cdc2b53b99b99"), "id" : "63214f58d66cdc2b54b99b99", "text" : "some change" }
{ "_id" : ObjectId("63214f58d66cdc2b54b99b99"), "id" : null, "text" : "some 2" }

Observe that "text" was updated in a wrong document. It looks like driver makes wrong query to update document and matches first one without id field.

Implementation of Controller and Model:

@voronianski
Copy link
Author

Ok, it looks like I've found a workaround for this problem.

MongoDriver doesn't care about syncing Fluent's required id key and Mongo's _id. So you need to do it by yourself - voronianski/vapor-server-example@d372780

Required changes:

 final class Post: Model {
+   var id: Node? // required to conform Fluent protocol
+   var _id: Node? // required to store correct value in db
    var text: String

   init(node: Node, in context: Context) throws {
+     id = try node.extract("id")
+     _id = try node.extract("_id")
      text = try node.extract("text")
    }

   func makeNode(context: Context) throws -> Node {
      return try Node(node: [
-       "id": id, // we don't want to double values in response
+       "_id": id,
        "text": text
      ])
    }

But I think this issue still needs to be addressed on driver level.

@tanner0101
Copy link
Contributor

This should work by just having id = try node.extract("_id") and "_id": id right?

@voronianski
Copy link
Author

@tannernelson probably yes, but I decided to keep track of _id in class instance for safety.

@tanner0101
Copy link
Contributor

What kind of safety? Fluent expects that the model.id be the same id as what is in the database. Storing them as separate values might cause issues elsewhere.

@voronianski
Copy link
Author

Yeah, your example is cleaner, just thought _id maybe needed somewhere.

@osmarogo
Copy link

osmarogo commented Apr 7, 2017

Hi Every one!

I'm facing an issue when I fetch my documents by Entity.All(), this happens when I get a Document with another embedded document.

In my model, I'm setting a relationship between User Entity that contains an embedded Token document in the userTokenId property, bellow you can see how I wrote my objects.

User Model.

final class User: Model {
    var id: Node?
    var _id: Node?
    var username: String
    var exists: Bool = false
    var userTokenId : Token?
    
    init(username: String, userTokenId:Token? = nil) {
        self.username = username
        self.userTokenId = userTokenId
    }
    
    init(node: Node, in context: Context) throws {
        id = try node.extract("id")
        _id = try node.extract("_id")
        username = try node.extract("username")
        userTokenId = try node.extract("userTokenId")
    }

    func makeNode(context: Context) throws -> Node {
        return try Node(node: [
            "_id": id,
            "username": username,
            "userTokenId": userTokenId
            ])
    }
}

extension UserNout: Preparation {
    static func prepare(_ database: Database) throws {
        //
    }
    
    static func revert(_ database: Database) throws {
        //
    }
}

Token Model

final class Token: Model {
    var id: Node?
    var _id: Node?
    var exists: Bool = false
    var token: String
    
    init(token: String) {
        self.token = token
    }
    
    init(node: Node, in context: Context) throws {
        id = try node.extract("id")
        _id = try node.extract("_id")
        token = try node.extract("token")
    }
    
    func makeNode(context: Context) throws -> Node {
        return try Node(node: [
            "_id": id,
            "token": token
            ])
    }
}

extension Token: Preparation {
    static func prepare(_ database: Database) throws {
        //
    }
    
    static func revert(_ database: Database) throws {
        //
    }
}

When I save one user at the first time I had this response.

screen shot 2017-04-07 at 10 57 43 am

The relation seems works fine when I save the object through Modify Fluent method as you can see below.

screen shot 2017-04-07 at 10 47 12 am

On the DB the collection looks ok as well.

User Document.

screen shot 2017-04-07 at 10 49 35 am

Token Document.

screen shot 2017-04-07 at 10 49 26 am

However the issue comes up when I get my document through User.all(), the id of the token Entity comes nil.

screen shot 2017-04-07 at 10 51 13 am

I'm not sure if this is an issue on my model, I appreciate some advice to know what can I do about that.

@valeriomazzeo
Copy link
Contributor

you guys should try again after #48 is merged

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

No branches or pull requests

4 participants