vapor-community / node

A formatted data encapsulation meant to facilitate the transformation from one object to another
MIT License
25 stars 20 forks source link

Understanding on Identifier #91

Closed dalu93 closed 6 years ago

dalu93 commented 7 years ago

Hi,

I'm spending some time trying out Vapor2 a bit and I have to say that, imo, the code is now much better than the version 1.0. Thank you all guys :)

Going to the question, I'm a bit confused with the Identifier concept. By implementing a Model using Fluent, the idea is to hide any database-related logic from our code, to make it more maintainable and easy to change to other engines. But the Identifier class is still a bit unclear to me.

I'm now using a postgres database, so a normal table id value is an Int. In mongo it will probably be an object or a String. So far everything is ok.

My Todo object as an Identifier and the router should handle a GET /todos/:id. Here is the problem: how I'm supposed to handle the id type?

What I ended up is this

todos.get("", ":id") { request in
  guard let todoId = request.parameters["id"]?.string else {
    throw Abort.badRequest
  }

  let identifier = Identifier(.string(todoId), in: nil)
  return try _todoController.todoWith(identifier, for: try request.getUserId())
}

In my case, an Int is convertible to a String type, so I hope that, when Fluent will be comparing Identifiers, the equality will be verified correctly. Am I correct? Is there a more elegant way to handle this without passing by a String object? I would have expected something similar to

guard let todoId = request.parameters["id"]?.identifier else {
    throw Abort.badRequest
}

which will abstract the Identifier type at all. What do you think? Also because the subscript value is already a StructuredDataWrapper object.

0xTim commented 7 years ago

@dalu93 any particular reason why you're manually pulling out the Identifier and then passing it to the controller as oppose to just using Parameterizable and then getting the Todo object?

dalu93 commented 7 years ago

The reason is that I need the userId which is encoded in the access_token parameter. Then access the database by looking for the specific todo for the specific user. @0xTim

tanner0101 commented 7 years ago

@dalu93 Just use String if you want to support all identifier types. That's what we would do with a .identifier property anyway. (The equality operator should work fine between ex: 42, "42", and 42.0). Also it's possible to add the .identifier property as an extension in your code if you want it.

We're working on moving away from the StructuredDataWrapper type in the next version of Node so hopefully that will make things more clear.

0xTim commented 6 years ago

Closing as Fluent 3 has no more Identifier!