fluent 4 api discussion about fluent-kit HOT 74 CLOSED

So initial thoughts:

The most important question, in terms of raw database terms, how does a table in Fluent 3 look compared to Fluent 4 - i.e. will upgrading require custom scripts when upgrading?

Aside from that, my main overriding concern is models have become massively more complex than they were. I also agree with @mcdappdev in that they just feel 'unSwifty' (and comparing to CoreData is not a comparison that should be made to justify it! 😅 ) I think Fluent (and Vapor) needs to decide at this point if it is a Swift-like server framework, or a high performance server framework. If it's aiming for a high performance framework then sure, make it as complicated as possible but I don't feel like that has been Vapor's sole goal.

To reiterate what I said on Discord - one of the big selling points of Vapor 3 was having hugely simplified code and models, where models could be only a few lines of code, especially with the different helpers to automatically set the database type and ID. If there's any way at all for the framework to hide the complexities to the end user, then it absolutely should. Even if that includes horrible hacks to enable Reflection.

Overall, it just seems like everything is more complicated. From the looks of it, doing something like return req.content.decode(Planet.self).save(on: req) doesn't look possible any more since you'd need to set each parameter of the model. It all seems like a bit of a step backwards for what I see as the majority of use cases (which could well be wrong, but from my experience is just basic CRUD APIs) to allow a small minority of use cases (things like partial retrieves and eager loading) prosper. I absolutely agree that Fluent should provide an API to do these, but I'm not sure it should be at the expense of the ease of use we've come to expect from Vapor. I really don't want to come across as negative here! I'm just concerned about the effect this may have on newcomers and put people off using the framework.

I think ideally there'd be a way to support both use cases, with something like a BasicModel that allows you to use properties like normal but doesn't allow any of the advanced features.

I'll add more thoughts as they come to me!

from fluent-kit.

I think making Vapor easy to pick up and learn is important, yes. But I don't think that should come at the expense of usability for real, production-size apps. Fluent 3's Codable models are great for getting started, but in my experience they really fall apart and become difficult to use once you start creating big apps.

So when you ask "what direction does Vapor want to take", I definitely don't want Vapor to just be useful for toy / getting started projects. I want it to be powerful enough to tackle any project.

At the end of the day, the core team's motto has always been: Build something that we would want to use. That is at the heart of this change to how models are defined. While Fluent 3's API seemed great and simple at first, once we started applying it to large cases the problems began to show and it started to feel clunky and hard to use.

I feel like this is a critical piece that is being missed in this conversation, so I will repeat it again:

This change is meant to greatly simplify using Fluent overall (at the expense of a slightly more complex Model definition)

For example, again, doing simple eager loading in Fluent 3:

struct GalaxyWithPlanet {
    var id: Int
    var name: String
    var planet: Planet
}
let galaxies = self.db.query(Galaxy.self)
    .join(\Planet.galaxyID, to: \.id)
    .alsoDecode(Planet.self)
    .all().map 
{ res in
    return try res.map { (galaxy, planet) in
        return try GalaxyPlanet(
            id: galaxy.requireID(), 
            name: galaxy.name, 
            planet: planet
        )
    }
}

IMO this is unacceptably complex for something that you might need to do even 5-6 times, nested, for large models / routes.

W/ proposed Fluent 4, it's just one line of code:

let galaxies = db.query(Galaxy.self).with(\.planets).all()

We are making a tradeoff of the Model being more complex, but getting huge simplicity wins where we actual use the model--which tends to make up a lot more of the code than just the model definition. And again, this isn't just about eager loading. There are tons of other examples (some of which I described above) that are radically simplified w/ Fluent 4's models.

Furthermore, compare to how other ORMs work:

Laravel (Eloquent):

$galaxies = App\Galaxy::with(['planets'])->get();

SQLAlchemy:

galaxies = session.query(Galaxy).options(joinedload(Galaxy.planets)).all()

Django:

galaxies = Galaxy.objects.prefetch_related('planets').all()

Rails (Active Record):

galaxies = Galaxy.includes(:planets).all

Express.js (Sequelize):

Galaxy.findAll({ include: [{ model: Planet, as: 'Planets' }] }).then(galaxies => { })

There's a clear pattern here in how other, popular web frameworks have solved this problem. And all of them are enabled by having a dynamic model type.

Hopefully this and the previous comments address the broad argument that this change "makes things more complex". I think going forward it would be more productive to discuss particular aspects / concerns about the proposed Model API so that they can be addressed one by one.

For example, there could be ways to make Fluent 4's model definition more closely resemble Fluent 3, at the cost of some additional internal complexity / performance. For example, maybe something like this (no nested Properties type) would be an improvement:

final class Planet: Model {
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let galaxy = Parent<Galaxy>(id: Field("galaxyID"))

    var storage: Storage
    init(storage: Storage) {
        self.storage = storage
    }

    convenience init(name: String, galaxy: Galaxy) {
        self.set(\.name, to: name)
        self.set(\.galaxy, to: galaxy)
    }
}

In Fluent 3, there was no lazy decoding, which means (sheer deduction, forgive me if I'm wrong) that one had to define a custom model for partial selections which do not load all columns. (Maybe optional properties were an option too). But again, once loaded without error, this custom model could also be passed around without error checking, because it was guaranteed to contain the fetched columns, and to not contain the columns that were not fetched.

You can still do something like this in Fluent 4. The Model offers a method for decoding itself into a keyed container (struct).

struct GalaxyJSON: Codable {
    var id: Int
    var name: String
}

let galaxies = try db.query(Galaxy.self).all().wait()
print(galaxies) // [Galaxy]
let json = try galaxies.map { try $0.decode(GalaxyJSON.self) }
print(json) // [GalaxyJSON]

However, I would recommend against doing this Codable mapping unless you know for sure that you will use all of the data in the struct. A couple of reasons:

• Decoding keyed containers is expensive. (If you encode the Model directly to a format like JSON, you encode the DB row directly without ever needing to do keyed decoding.)
• Decoding each property lazily as needed saves memory and CPU time. For example, if a function using Galaxy is refactored at some point to no longer use name, that key may no longer need to be decoded at all, increasing the performance and reducing memory usage for the route in question.

Maybe a recommended practice, to avoid the proliferation of throwing methods, will be to "map" Fluent models into custom structs immediately after the fetch.

You need an extra Codable struct here, but that's really no different than Fluent 3 where you already need to map to several, separate Codable structs to do most things.

Fixing bugs has turned into a test coverage game.

I'm not sure I understand this part. If your Model.Properties do not match your database schema, you will get an error just like if your Model's codable properties do not match in Fluent 3.

from fluent-kit.

@twof on a positive note, one of the pros for "getting started" use cases is that you can now easily hide fields from being encoded. A common use case for this being passwordHash on a user model:

final class User: Model {
    struct Properties: ModelProperties {
        let id = Field<Int>("id")
        let name = Field<String>("name")
        let passwordHash = Field<String>("passwordHash", visibility: .hidden)
    }
    ...
}

In this case, passwordHash would still be sent to/from the DB, but when encoded to JSON, the user would look like:

{ "id": 1, "name": "Vapor User" }

from fluent-kit.

Model definitions will need to change, which is a burden. But they aren't any less concise. In my example above, the model is only 1 line longer.

The issue is not the length. The issue is that implementation details are bleeding into userland, and models are significantly less abstract. Previously users could write plain old swift objects just like they were used to, conform to a few protocols, and trust everything would work. There was no need to think about implementation because users weren't being exposed to it.

Now if I was going to be teaching someone Fluent, I've got to explain why things are different than what they're used to in 3 or 4 different regards. The mental models people have to establish before they can use models in a comfortable way are more complex.

I want to be clear that I that I believe this level of flexibility and performance (the new Fluent 4 level) is the correct direction to go in. I'm fully aware of the pros and cons and I am on board to pursue this direction. However, a huge reason many folks choose Vapor over other frameworks is due to Swiftyness and up front ease of use. You'd be abandoning that use case if fluent-kit replaced fluent and with it any new users who would have adopted Vapor had it supported that use case.

This is something that's important to me (idk about others). I don't know the internals of Fluent or Fluent-Kit well, so I'm going to go learn them and figure out what it would take to run both styles of Model in parallel.

from fluent-kit.

You could do something like this:

final class Planet: Model {
    struct Properties: ModelProperties {
        let id = Field<Int>("id")
        let name = Field<String>("name")
        let galaxy = Parent<Galaxy>(id: Field("galaxyID"))
    }

    static let properties = Properties()

    var id: String {
        get { return try! self.get(\. id) }
        set { self.set(\. id, to: newValue) }
    }

    var name: String {
        get { return try! self.get(\.name) }
        set { self.set(\.name, to: newValue) }
    }

    var galaxy: String {
        get { return try! self.get(\.galaxy) }
        set { self.set(\. galaxy, to: newValue) }
    }

    var storage: Storage
    init(storage: Storage) {
        self.storage = storage
    }
    convenience init(name: String, galaxy: Galaxy) {
        self.name = name
        self.galaxy = galaxy
    }
}

let planet: Planet // pulled from DB
print(planet.name)

I'm not a huge fan of that since we have to throw away potential missing value / decoding errors though. It also increases the boilerplate for models a lot.

However, there are some interesting things happening with Swift like: Pitch: Key-Path Member Lookup. If that pitch were to be accepted, I think it could be possible for us to simulate properties (in a type-safe way) that ultimately redirect to the function calls get / set.

I think, generally, we should have faith that Swift will improve and not try to design Vapor's APIs around what is the most concise / easy today. For example, we know that Swift will get async / await sometime soon, which is why we embraced Futures as a temporary solution, even though they are relatively verbose and not easy to use. The win here is that Vapor's architecture, and all the packages designed to work w/ Vapor, will eventually have all the incredible benefits of non-blocking with a beautiful synchronous API. All good things come with time.

I think the same thing applies with models. It's likely that Swift will eventually get better reflection, attributes, dynamism, etc. When Swift does get those things, we can have all the benefits of choosing the most powerful solution and get the best API. And we as part of the server-side Swift community can help pitch, propose, and implement those features. Swift is a very young language that has only really been used for application /client programming up until this point. It makes sense that some things don't translate well for server use cases yet--and that's OK. Swift's mission is to become better at those things.

from fluent-kit.

Some code paths will not express the error, some bugs will become latent.

Ah okay, I see what you mean now. Yeah that seems like a fundamental downside to lazy decoding. A good point worth adding to the downsides of this change, thank you. 👍

from fluent-kit.

I put my two cents:

From a beginner perspective (let's say someone who comes from iOS or from another backend framework Laravel, Flask, Rails,etc) I believe this new way how to define and use fluent's models it doesn't feel "natural". a lot of people would expect to make the mutations directly on the properties instead of use keypaths and write plains swift models without nested intermediate types for declare the properties to use.

But the benefits that this new api give to us is great and I think the tradeoff of this new level of complexity worth it, I know that with this new design we lose a little bit of the type safety from fluent 3 but I think is better to use the best design based on what the language offer instead of trying to force to use something that at the end of the day just will cause more problem that what is trying to solve. Hopefully one day swift will provide great reflections capabilities and helps to comeback an even better api like Fluent 3.

Saying this IMHO we should keep with this api

final class Planet: Model {
   struct Properties: ModelProperties {
       let id = Field<Int>("id")
       let name = Field<String>("name")
       let galaxy = Parent<Galaxy>(id: Field("galaxyID"))
   }
   static let properties = Properties()
   var storage: Storage
   init(storage: Storage) {
       self.storage = storage
   }
   convenience init(name: String, galaxy: Galaxy) {
       self.set(\.name, to: name)
       self.set(\.galaxy, to: galaxy)
   }
}

I would rather to keep my initializers inside of the type for what are intended for, instead of write it inside of a where extension from another type (like Instance example). Also I would rather use directly my types instead of have a kind of wrapper like Instance, I think this add more complexity than we actually need.

from fluent-kit.

I was asked on Twitter to provide my feedback, even though I think it has all been said already in this thread :) My thoughts as an experienced Swift dev, but only just started using Vapor: I'd gladly have a slightly more complex model definition if using the models gets easier. Right now my biggest problem with Vapor, by far, is the lack of eager loading of relationships. Also it's currently too hard to specify things like indexes or unique fields, and non-public fields means having to have a separate public version of your model, and translate data from one to the other. This seems like it could solve all (or most) of those things. I am super excited to see these changes, even though yes planet.get(\.name) is not as nice as planet.name.

It's been mentioned that this suggested change would make Vapor harder to learn for beginners, but in my opinion as a Vapor beginner, I'd rather have to learn once how to write a model, but then actually using them would be a lot easier, compared to all the work I currently have to do to return a full model with all its nested relationships as JSON.

For example, this is what a Django model looks like:

class Book(models.Model):
    title = models.CharField(max_length=255)
    date_created = models.DateTimeField(auto_now_add=True)
    date_modified = models.DateTimeField(auto_now=True)
    owner = models.ForeignKey(User, related_name='books', on_delete=models.CASCADE)

Yes, not as concise as a Vapor 3 model with pure Strings and Ints and all that, but then I need to add a Migration extension to handle the relations on a DB level, the onDelete handling, etc. There is currently no way to describe model properties in an easy manner, to add extra behavior to them. This proposal would bring a lot of power to models I think. I'm all for it!

from fluent-kit.

Moving my comment over from the other thread.

IMO it would be worth it to support both model styles and let folks upgrade their models to the Fluent 4 style as they decide the tradeoffs become worth it.

The other thing you lose is the ability to share models between the frontend and backend out of the gate, but by the time users decide to upgrade a model, they've probably already created a public model and shared that instead, so that shouldn't be a problem if you're supporting both styles.

Any idea how much work it would be to have both styles exist at the same time?

from fluent-kit.

Oh ok, that's slightly better. I guess one of my main worries is that without a proper init it'll be really easy to add a new property and then forget to set it somewhere. So if we can support that that'll alleviate that concern.

from fluent-kit.

@groue thanks, I will definitely take a look. It's nice to have a Swift example besides CoreData to reference (even if it is client-focused). I do like this model, but the one thing I don't think Fluent could live without is key paths. Those are crucial to our query building API:

db.query(Planet.self).filter(\.mass > 3).with(\.galaxy).sort(by: \.name, .descending).all()

Maybe with Swift 5's ABI being stable there is a way Fluent could have the best of both worlds. I will do some more digging...

from fluent-kit.

Since Models are no longer associated with a specific DB, how would DB specific types work? Postgres' geometric types for example.

from fluent-kit.

I've confirmed that if @dyanmicMemberLookup gains key path support, it should work with the Planet.Row based API. See the comment here: https://forums.swift.org/t/pitch-key-path-member-lookup/21579/42

from fluent-kit.

@tanner0101 I really like this proposal. In Fluent 3 I always found the.alsoDecode(Planet.self) method convoluted, glad to see that simplified. I Also like the simplified way of defining a relationship in the proposal let galaxy = Parent<Galaxy>(id: .init("galaxyID")). Finally +1 for not having the inner struct Properties: ModelProperties

from fluent-kit.

I'm late to the party, after the first few comments I wasn't overly keen on the look of the new Models, because they are more complex to understand and simply don't look as pretty and Swifty. That being said, with the right documentation and clear example sets of some real world style model scenarios I think they could be picked up. The key is just that, quality in-depth docs, so people don't feel there's unknown magic happening. Would be good to maybe have some FAQs as people come across them to help new comers.

The reason I'm good with the complexity, is that as an ORM, fluent is behind other popular ones as mentioned and people coming from big web based frameworks like Django, Spring, Rails, Laravel, Yii expect to be able to do 1 line queries for joins etc like:

 return Wastage::find()->innerJoinWith('Facilities')->where(['FACILITY_WASTAGE.DEL_DT' => NULL])->all()
// or multiple joins and clauses

It seems that kind of query would now return a simpler more expected result set as opposed to alsoDecode(), which returns the [(x), (y)]. So larger, more complex apps would benefit from this improved simplicity in getting the results. If fluent is more powerful, then the underlying SQL can be more complex and the method to get the data in the controllers simpler then I think that's a win. I feel like for me and I work on pretty large Oracle and MSSQL Dbs that the SQL writing should be the hard part and you shouldn't have to write some complex SQL, then have to write some complex closure to get the data in a form that you expect. The new approach seems to fix this :)

from fluent-kit.

I'm largely in favor of this, but I'm having a hard time getting over the .get and .set syntax. It feels so... gross. Is there a way to move those actions into the getter and setter of the individual properties?

from fluent-kit.

Where is the DB specified in the Fluent 4 model? Or do we no longer need to do that?

from fluent-kit.

Is there a way to move those actions into the getter and setter of the individual properties?

@mcdappdev computed property getters can't throw in Swift, so no :(

FWIW, this is how CoreData works, so if anything this change should actually make Fluent more familiar to iOS engineers.

import CoreData

let person: NSManagedObject // from db
person.value(forKeyPath: "name") as? String
person.setValue(name, forKeyPath: "name")

Where is the DB specified in the Fluent 4 model? Or do we no longer need to do that?

@twof Database no longer has associated types so you can use it abstractly now. For example:

final class GalaxyController {
    let db: Database
    func index(_ req: HTTPRequest) -> EventLoopFuture<[Galaxy]> {
        return self.db.query(Galaxy.self).all()
    }
}

When initializing this controller, you can pass in any type that conforms to Database, whether that be a connection pool, single connection, postgres, mysql, sqlite, mongo, or even a dummy / mock database. No need to be generic anymore, so Model doesn't need the associated type.

from fluent-kit.

"When initializing this controller, you can pass in any type that conforms to Database, whether that be a connection pool, single connection, postgres, mysql, sqlite, mongo, or even a dummy / mock database. No need to be generic anymore, so Model doesn't need the associated type."

Wow that's really nice. Swapping databases doesn't come up super often, but if it does that ought to make it mad easy.

from fluent-kit.

@tanner0101 that’s sad :( is there any way to make a variant of that method that doesn’t throw? And what about creating a new object? Do you have to set each value independently? Or can that still be wrapped up in an init?

from fluent-kit.

IMO it would be worth it to support both model styles and let folks upgrade their models to the Fluent 4 style as they decide the tradeoffs become worth it.

@twof I tried a couple approaches for this, but haven't found anything that works yet. :\ The wrench in the works seems to be that Model gets its Codable conformance dynamically now (not compiler synthesized, but synthesized by Fluent introspecting the model properties and doing the encoding / decoding). So the Codable struct / classes clash with that.

The other thing you lose is the ability to share models between the frontend and backend out of the gate

Yeah that is another unfortunate thing. Especially since it affects "getting started" use cases.

You can still have Codable structs that you share between your iOS app and your server, it's just that you will need to have a separate model and translate between them. Like is common in Fluent 3 anyway for cases where you want to change the data structure.

struct GalaxyJSON: Codable {
    var id: Int?
    var name: String
    var planets: [PlanetJSON]
}

let galaxy: Galaxy // fetched from DB
let json = try galaxy.decode(GalaxyJSON.self)
// or
let json = try GalaxyJSON(
    id: galaxy.get(\.id), 
    name: galaxy.get(\.name), 
    planets: galaxy.get(\.planets).map { ... }
)

I think that almost all real world apps will want this anyway because you really don't want to strictly tie your DB structure to your public JSON API in production use cases. But yeah, it's a trade-off for sure.

And what about creating a new object? Do you have to set each value independently? Or can that still be wrapped up in an init?

@mcdappdev you can create a convenience init if you want, like I did in the example:

final class Planet: Model {
    // ...
    convenience init(name: String, galaxy: Galaxy) {
        self.set(\.name, to: name)
        self.set(\.galaxy, to: galaxy)
    }
}

otherwise, you will need to create an empty object and set them manually:

let planet = Planet()
try planet.set(\.name, to: "Earth")
try planet.set(\.galaxy.id, to: 1)
planet.save(on: self.db)

from fluent-kit.

The most important question, in terms of raw database terms, how does a table in Fluent 3 look compared to Fluent 4 - i.e. will upgrading require custom scripts when upgrading?

This change in how models are declared shouldn't have any effect on DB structure.

Aside from that, my main overriding concern is models have become massively more complex than they were.

Could you elaborate more on which parts are more complex? The way I see it there are trade-offs, but ultimately things become less complex.

Increased complexity (cons):

• Getting / setting individual properties is more verbose: planet.name becomes planet.get(\.name).
• Model definitions will need to change, which is a burden. But they aren't any less concise. In my example above, the model is only 1 line longer.

Decreased complexity (pros):

• Many situations that required a custom migration can now be done with one small change.
• Many situations that required manual joins and decoding can now be done automatically.
• Many situations that required one-off structs can now re-use the model type:
  • Public / private models with different structures.
  • Models w/ or w/o relations loaded in
• Declaring nested / custom types no longer requires complex ReflectionDecodable conformance. Things "just work".
• Migrations that change / remove model properties can now be done without needing a copy of the old model structure:

let planet: Planet
// assume mass has been removed from the model, but we 
// still need to interface with it to perform this migration:
let mass = try planet.storage.get("mass", as: Double.self)
print(mass) // Double

Improved performance (pros):

• Un-used properties are no longer decoded.
• Only changed properties are sent over the wire during model saves.

From the looks of it, doing something like return req.content.decode(Planet.self).save(on: req) doesn't look possible any more since you'd need to set each parameter of the model.

Model still conforms to Codable. You can still decode instances of Models from things like JSON. (And encode them, too)

from fluent-kit.

Regarding the last point there, is it still possible to chain on save() to that decoding call? Because that’s one of my concerns as well.

from fluent-kit.

@mcdappdev I've been told that chaining a save() onto a decode() will work fine.

I think @twof hits the nail on the head, it's the lack of Swift types and having to learn new ways of accessing properties on models that seem a) confusing and b) very unSwifty for anyone looking at it the first time.

Looking at it, it doesn't look like there can be any helpers (such as PostgreSQLModel) that we had in Fluent 3 to cut down on all the boilerplate

from fluent-kit.

To put my two cents in - it took a few minutes to read all comments 😊

My first impression

"ohw bummer.. it's becoming more complex".
Especially with a "beginner mind". I think @twof has pointed out the "why" very good.

When speaking of simplicity I think it's less about code length and more what you know intuitively. In Fluent 3 you could just define your properties as you would do with normal structs and classes - the swift default way. Nothing new to learn.

New vs. Old

With Fluent 4 you now have to learn that defining properties was taken on a new level, more abstract level and not only would you have to learn that but you also have to know about "more advanced" coding-concepts like "Generics" and "Keypath".

Issue

With Vapor 3 the beginner-friendliness decreased quite a bit with the introduction of futures. For stuff that you would want to do after 30 minutes using vapor for the first time in your life - querying a database for a model and return it as JSON - it required you to face your first challenge: Futures. Even more if server-side is the first time you are actually touching swift.

In my opinion that beginner-friendliness would decrease even more now with the new way of how you define models. I mean that's something you would want to do after 30 minutes using Vapor for the first time in your life, too, right?

In conclusion:

To me it feels like "starting out with vapor" would overwhelm a lot of developer with having to wrap their heads around a lot of new (non-straight forward) concepts that at times require more advanced knowledge about swift development. Which leads to being un-swifty when thinking of that swift aims to appeal to beginners as you can build things pretty quick but can become more advanced later in the learning-process (not having to at the beginning).

I usually don't like to declare something as an issue without a solution to it. However I can't come up with something that would reduce the complexity "at the beginning" while introducing simplicity "later on".

That's how I feel about the new Model. It became more complex in the beginning to become more simple later on. Thinking of how peeps actually "learn" I think having it rather "simple in the beginning" and more complex "later on" is more bearable - less frustrating thus reduces the likeliness of giving up.

PS

I have the feeling you (@tanner0101) have put in a lot of thoughts in that new concept to solve a lot of current issues/requested features. And I really don't want to sound negative either - like @0xTim said - the question really is what direction does vapor want to go. Does maybe solving the current issue introduces another one (of same size or bigger) ?

✌🏻😊

from fluent-kit.

Hello,

I'm chiming in because I expect to use Vapor in the near future, and because I am interested in "Swifty" ORM apis myself, as the main author of GRDB, the SQLite toolkit for applications.

I'm wondering if the changes described in the "Eager loading" and "Lazy Decoding" chapters are for the best. Something has been lost: the ability to catch errors early when the decoded model does not match the query.

In Fluent 3, eager loading would use a "custom model". Thanks to this custom model, the user is confident about the structure of the loaded values. Once loaded without error, the values can be consumed with no thinking at all: Swift provides static guarantees that the expected data is available. You can pass custom models around without try/catch management.

In Fluent 3, there was no lazy decoding, which means (sheer deduction, forgive me if I'm wrong) that one had to define a custom model for partial selections which do not load all columns. (Maybe optional properties were an option too). But again, once loaded without error, this custom model could also be passed around without error checking, because it was guaranteed to contain the fetched columns, and to not contain the columns that were not fetched.

Early failure is no longer possible with Fluent 4. Because new models do not use Swift static checks, no function can be sure a model contains the needed information. All functions that read inside a model may throw. Fixing bugs has turned into a test coverage game.

Maybe a recommended practice, to avoid the proliferation of throwing methods, will be to "map" Fluent models into custom structs immediately after the fetch. This would restore both early errors and static checks. In such a setup, Fluent models would be pushed in a "DAO" layer, not really designed for application consumption. And this would surely raise the amount of boilerplate code.

from fluent-kit.

+1 for removing the nested Properties. I think that alone removes a lot of cognitive strain for beginners while keeping the benefit of the stated goals.

May I suggest, once again, that we should be looking to find a way to make get() non-throwing - or perhaps offer a version that doesn't throw. I think a ton of the concerns around readability/usability can be fixed if we can wrap .get() and .set() inside of a property's getter and setter.

Before:

post.name.set("Jimmy")
try post.name.get() // Jimmy

After:

// Post.swift
final class Post: Model {
    let id = Field<Int>("id")
    let name = Field<String>("name") {
        get {
            return name.get()
        }
        set {
            name.set(newValue) // I forget the exact syntax here
        }
    }

    // rest of the model
}

post.name = "Jimmy"
post.name // Jimmy

from fluent-kit.

Fixing bugs has turned into a test coverage game.

I'm not sure I understand this part. If your Model.Properties do not match your database schema, you will get an error just like if your Model's codable properties do not match in Fluent 3.

Yes, but you will get a late error, not an early error. The late error happens only when you attempt to access a missing information, long after the data has been fetched. Some code paths will not express the error, some bugs will become latent.

This is the direct consequence of the new dynamic api. What is no longer checked for free by the compiler is just no longer free. Experience (from frameworks written in dynamic languages) shows that the cost is new tests.

In case you ask: what the compiler no longer provides is the guaranteed access to a property: planet.name doesn't throw, when planet.get(\.name) can.

from fluent-kit.

Furthermore, compare to how other ORMs work:

[...] Rails (Active Record):

galaxies = Galaxy.includes(:planets).all

[...]

I'm not here to advertise vaporware, but I do certainly hope that GRDB will soon let people write:

struct GalaxyInfo: Decodable {
    let galaxy: Galaxy
    let planets: [Planet]
}

// Single point of failure
let infos: [GalaxyInfo] = try Galaxy
    .including(all: Galaxy.planets) // Galaxy.planets is a "HasMany" association
    .asRequest(of: GalaxyInfo.self)
    .fetchAll(db)

// No need to catch errors from now on:
use(infos)

And there is nothing dynamic here.

This is not totally vaporware, because support for "to-one" associations is already implemented and working well:

// Currently works
struct PlanetInfo: Decodable {
    let planet: Planet
    let galaxy: Galaxy
}

let infos: [PlanetInfo] = try Planet
    .including(required: Planet.galaxy) // Planet.galaxy is a to-one "BelongsTo" association
    .asRequest(of: PlanetInfo.self)
    .fetchAll(db)

use(infos)

My point is that dynamism is maybe not as required as it may seem. And as you wrote above, Swift does improve over time. Rust's Diesel is very interesting, too.

from fluent-kit.

Here's a potential idea following on the remove Model.Properties idea, but inverting it. Instead of having users declare their own instance of the model class each time, we could have users declare only the model properties. Then Fluent could re-use a generic model definition. Naming aside, this could look like:

struct Planet: Model {
    static let default = Planet()
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let galaxy = Parent<Galaxy>(id: Field("galaxyID"))
}

Instances of planet would become:

let planet: Instance<Planet> // pulled from DB
let name = try planet.get(\.name)

Querying would look the same, but return a different type:

final class GalaxyController {
    let db: Database
    func index(_ req: HTTPRequest) -> EventLoopFuture<[Galaxy.Instance]> {
        return self.db.query(Galaxy.self).with(\planets).all()
    }
}

Alternate ideas for naming Instance:

• Ref / Reference
• Object
• Entity
• Row

from fluent-kit.

@groue how do you apply filters to that data? For example, can you do something like:

// Currently works
struct PlanetInfo: Decodable {
    let planet: Planet
    let galaxy: Galaxy
}

let infos: [PlanetInfo] = try Planet
    .including(required: Planet.galaxy) // Planet.galaxy is a to-one "BelongsTo" association
    .asRequest(of: PlanetInfo.self)
    .filter(\.name == "Earth")
    .fetchAll(db)

use(infos)

If so, how do you get the string "name" from \.name?

from fluent-kit.

@tanner0101 I could get behind that. How would you save a new object with that model?

from fluent-kit.

@mcdappdev something like this would work:

let planet = Planet.new() // Instance<Planet>
planet.set(\.name, to: "Earth")
planet.set(\.galaxy.id, to: 42)
planet.save(on: db)

We might be able to get a more convenient init working, though. Maybe something like:

let planet = Planet([\.name: "Earth", \.galaxy.id: 42])
planet.save(on: db)

I would have to try a bit to see if I could get that one working, though.

from fluent-kit.

Nevermind. I'm now against it :)

from fluent-kit.

Haha. You could also add a convenience init if you wanted:

extension Instance where Model == Planet {
    convenience init(name: String, galaxy: Galaxy) {
        self.init()
        self.set(\.name, to: name)
        self.set(\.galaxy, to: galaxy)
    }
}

let planet = Planet.Instance(name: "Earth", galaxy: ...)

I'm not really in love with that either, though.

from fluent-kit.

@groue how do you apply filters to that data? For example, can you do something like:

let infos: [PlanetInfo] = try Planet
    .including(required: Planet.galaxy) // Planet.galaxy is a to-one "BelongsTo" association
    .asRequest(of: PlanetInfo.self)
    .filter(\.name == "Earth")
    .fetchAll(db)

If so, how do you get the string "name" from \.name?

GRDB doesn't use key paths, and prefers strings or coding keys instead. But this is not your question. As you have correctly analysed, the .asRequest(of: PlanetInfo.self) part has to go last, so that the Planet type is not lost (unless you opt in for strings, in which case the order does not matter).

from fluent-kit.

@groue what would the definition of Planet look like in this case?

from fluent-kit.

what would the definition of Planet look like in this case?

In its shortest form, for read/write access:

// A regular struct...
struct Planet {
    var id: Int64
    var name: String
    var galaxyID: Int64
}

// ... now with database support
extension Planet: FetchableRecord, PersistableRecord, Codable {
    static let galaxy = belongsTo(Galaxy.self) 
}

Have a look at GRDB, one day, it is a cool (client-focused) library! But I'm sure Fluent is pretty cool, too. I'm not here to advertise my work. Just to spot a few questions in the original post.

from fluent-kit.

Removing Properties

I'm all in on to "closely resemble Fluent 3, at the cost of some additional internal complexity / performance." and removing the nested properties 🙌

Updated opinion

I didn't address it previously but I do like the reduction of the code-clutter when crafting more complex queries. 😊

Not having the properties struct had a big impact on my thoughts about the new model. It starts growing on me.

PS

Out of curiousity @tanner0101 - defining a parent relation does not need a type for the id?

let galaxy = Parent<Galaxy>(id: Field("galaxyID")) // your example

vs

let galaxy = Parent<Galaxy>(id: Field<Int>("galaxyID")) // my question :)

from fluent-kit.

@martinlasek Yeah just an idea though, would consider it at the project level but definitely not at the Fluent level. I guess it's a tradeoff between complexity (I use that word lightly as .set and .get aren't exactly complex) at the model level or the access level

from fluent-kit.

Given that this change is targeted at folks using Vapor on large, serious projects, it'd be great to get feedback from more folks at Nodes. @calebkleveter would be good too.

from fluent-kit.

Out of curiousity @tanner0101 - defining a parent relation does not need a type for the id?

@martinlasek You can write Parent<Galaxy>(id: Field<Int>("galaxyID")), too. Both work. Swift is able to infer <Int> because it knows Galaxy.ID == Int.

Given that this change is targeted at folks using Vapor on large, serious projects, it'd be great to get feedback

@twof I agree. I'd like to get people trying this out sooner than later to see how it holds up.

from fluent-kit.

I put up a proof of concept of the generic instance idea here: #12

It was pretty easy to implement, and the diff is even less lines of code. Maybe this can be a good compromise?

from fluent-kit.

Do you have to specify the keys when querying? Will it default to not lazy-loading properties unless keys are specified?

from fluent-kit.

@ScottRobbins hmm not sure what you mean by that, can you elaborate?

from fluent-kit.

You had an example like

// fluent 4
let planets = self.db.query(Planet.self).keys(\.name).all()
for planet in planets {
    // works
    let name = try planet.get(\.name)
    // throws since ID was not selected
    let id = try planet.get(\.id)
}

If I changed this to

// fluent 4
let planets = self.db.query(Planet.self).all() // no more .keys
for planet in planets {
    let name = try planet.get(\.name)
    let id = try planet.get(\.id)
}

Does that end up making 2 queries?

Edit: Realized half of my questions you answered right above where i was reading this example and I missed it.

from fluent-kit.

@ScottRobbins ah I see what you mean. Two things:

1: If you don't write .keys, then all of them will be selected by default. Note that this doesn't mean SELECT *, it means SELECT id, name, etc
2: Lazy decoding only means decoding data from the row (converting it from your DB's format to Swift types). If the column wasn't fetched in the query, then the decoding will fail with "no column named x found". It's actually impossible for us to do a query to get a single key lazily because the get method is synchronous. We can only access data that is already available.

from fluent-kit.

@guseducampos thanks for the comments. My replies inline:

But the benefits that this new api give to us is great and I think the tradeoff of this new level of complexity worth it, I know that with this new design we lose a little bit of the type safety from fluent 3 but I think is better to use the best design based on what the language offer instead of trying to force to use something that at the end of the day just will cause more problem that what is trying to solve. Hopefully one day swift will provide great reflections capabilities and helps to comeback an even better api like Fluent 3.

Yeah those are my thoughts exactly. I think Pitch: Key-Path Member Lookup specifically could make the syntax a lot more bearable, and that seems like it could be merged in the near future (if the proposal goes well). I've added a small note about how that could affect the API positively in #12.

Saying this IMHO we should keep with this api

I agree with your reasons here. But also, I think that using @keyPathMemberLookup will not be easy to add if we don't own the type. The best we could do with a protocol (maybe) is provide a default implementation of the subscript method, but people would still need to add the attribute to each of their models.

I would rather to keep my initializers inside of the type for what are intended for

An alternative is doing something like this:

struct Foo: Model {
    static let `default` = Foo()
    let id = Field<Int>("id")
    let bar = Field<String>("bar")
    let baz = Field<Int>("baz")
    static func new(bar: String, baz: Int) -> Row<Foo> {
        let new = self.new()
        new.set(\.bar, to: bar)
        new.set(\.baz, to: baz)
        return new
    }
}

I know it's not as nice as a direct init, but it works.

Also I would rather use directly my types instead of have a kind of wrapper like Instance, I think this add more complexity than we actually need.

#12 has renamed the type from Instance -> Row. The added concision and specificity makes the change more bearable to me at least. Thoughts?

from fluent-kit.

1: If you don't write .keys, then all of them will be selected by default. Note that this doesn't mean SELECT *, it means SELECT id, name, etc
2: Lazy decoding only means decoding data from the row (converting it from your DB's format to Swift types). If the column wasn't fetched in the query, then the decoding will fail with "no column named x found". It's actually impossible for us to do a query to get a single key lazily because the get method is synchronous. We can only access data that is already available.

That makes sense, thank you 👍

from fluent-kit.

I'm wondering if putting the storage in a Schema and then setting the Schema of a model might open some doors 🤔.

final class PlanetSchema: Schema {
    let id = Field<Int>("id", identifier: true)
    let name = Field<String>("name")
    let galaxy = Field<Galaxy>(field: Field("galaxyID"))
    
    var storage: Storage
    
    init(storage: Storage) {
        self.storage = storage
    }
}

final class Planet: Model {
    let schema: PlanetSchema
    
    // `Column<T>` typaliased to `Result<Error, T>`
    var id: Column<Int> {
        return self.schema.get(\.id)
    }
    
    var galaxy: Column<Galaxy> {
        return self.schema.get(\.galaxy)   
    }
    
    var name: Column<String> {
        get { return self.schema.get(\.name) }
        set { self.schema.set(\.name, to: newValue) }
    }
    
    init(schema: PlanetSchema) {
        self.schema = schema
    }
}

from fluent-kit.

Is it possible to use keypaths inside of Field? So: Field<Int>(\.id, identifier: true)? Can it even be self-referential like that? I just struggle with the stringly typed API there, even if it is only in that one place.

Also, I've seen this syntax referred to a few times now: planet.get(\.name). Will that be available in addition to planet.name.get()? Because I think the latter is a lot more natural.

from fluent-kit.

For context, I've collected what a theoretical Planet model would look like in some other popular ORMs. Interestingly, there seems to be two main approaches.

1. Empty subclass + migration

One method is to use an empty sub-class of a model class. This method relies on defining a separate migration since there is no schema info on the model itself. Everything is looked up dynamically (no type safety of course).

ORMs: Eloquent, ActiveRecord

2. Schema declaration

The method other is to have the model be a schema declaration. Here you declare what the table should look like, which allows the framework to generate the migration on your behalf. I'm not sure what type is actually returned here when you query the model, that is less easy to find in documentation. Given that these are all dynamic languages, it's probably just an untyped dictionary.

ORMs: Django, SQLAlchemy, Sequelize

Which is Fluent?

What we have in Fluent 4 (and #12) seems like a reasonable mix between these strategies that adds type safety. Our models are simple definitions of the schema / table, like some of the other ORMs. But with Swift, we can take that a step further and use the model schema generically to make accessing the db output type-safe (Row<Planet>).

Examples

Below are the model examples for Planet in each ORM.

Rails (ActiveRecord)

class Planet < ApplicationRecord
  self.table_name = "planets"
  belongs_to :galaxy
end

class CreatePlanets < ActiveRecord::Migration[5.0]
  def change
    create_table :planets do |t|
      t.string :name
      t.belongs_to :galaxy, index: true
    end
  end
end

Laravel (Eloquent)

namespace App;

use Illuminate\Database\Eloquent\Model;

class Planet extends Model
{
    protected $table = 'planets';
    public function galaxy()
    {
        return $this->belongsTo('App\Galaxy');
    }
}

use Illuminate\Support\Facades\Schema;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;

class CreatePlanetsTable extends Migration
{
    public function up()
    {
        Schema::create('planets', function (Blueprint $table) {
            $table->increments('id');
            $table->string('name');
            $table->bigInteger('galaxyID');
        });
    }

    public function down()
    {
        Schema::drop('planets');
    }
}

SQLAlchemy

from sqlalchemy import Column, Integer, String
class Planet(Base):
    __tablename__ = 'planets'

    id = Column(Integer, primary_key=True)
    name = Column(String)
    galaxy_id = Column(Integer, ForeignKey('galaxy.id'))
    galaxy = relationship("Galaxy", back_populates="galaxies")

Django

from django.db import models

class Planet(models.Model):
    id = models.IntegerField(max_length=30)
    name = models.CharField(max_length=30)
    galaxy = models.ForeignKey(Galaxy, related_name='galaxies', on_delete=models.CASCADE)

Sequelize

const Planet = sequelize.define('planets', {
  id: Sequelize.INT,
  name: Sequelize.TEXT
})
Planet.belongsTo(Galaxy); // Will also add galaxyId to Planet model

Fluent 4

struct Planet: Model {
    static let `default` = Planet()
    static let entity = "planets"
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let galaxy = Parent<Galaxy>(id: .init("galaxyID"))
}

from fluent-kit.

@mcdappdev

Is it possible to use keypaths inside of Field

Yes. One side of the relation needs to declare the string of course, but the other side could use a key path to reference it. For example, we could have the Child declare:

let galaxy = Parent<Galaxy>(id: "galaxyID")

But the parent declares:

let planets = Children<Planet>(\.galaxy)

Also, I've seen this syntax referred to a few times now: planet.get(.name). Will that be available in addition to planet.name.get()? Because I think the latter is a lot more natural.

It's either one or the other. The benefit of the get(...) syntax is that we may get dot-syntax in the future with @keyPathMemberLookup . So I think we should definitely go that route. (See the note in #12 for more info)

from fluent-kit.

Alright, it's really starting to grow on me. I will say though that I prefer static let default = Planet() be spelled as static let shared = Planet(). That may just be a mental holdover from my Objc days though :)

What do we think the chances of Key-Path member lookup being merged before Fluent 4 is? Because if we get that then I think we get the best of both worlds - natural/typed API with dynamic backing.

from fluent-kit.

@tanner0101 I mentioned previously that it starts growing on me (edit: @mcdappdev just read it's growing on you, too 😄) and following the discussion and getting a better idea of it overall with that - I started to like it (especially when compared to how other Frameworks implemented it) 😊

struct Planet: Model {
    static let `default` = Planet()
    static let entity = "planets"
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let galaxy = Parent<Galaxy>(id: Field("galaxyID"))
}

I am wondering is there a specific reason we switched from final class to struct?

from fluent-kit.

@mcdappdev

I will say though that I prefer static let default = Planet() be spelled as static let shared = Planet().

I think shared would be better, too. default is not ideal because it needs to be escaped. (default is a Swift keyword).

What do we think the chances of Key-Path member lookup being merged before Fluent 4 is?

Very low. It's probably going to be Swift 5.2 at best, if not Swift 6. The get syntax will always work though, so it should be an additive improvement when Swift adds it.

@martinlasek

I am wondering is there a specific reason we switched from final class to struct

The model should be a reference type, since we want to make it cheap to pass around and easy to mutate as it goes through async closures. But since the instance of a model is Row now, that is the reference type. The Model is just information about the schema, and we don't need that to be a reference.

You could use either class or struct here. Maybe one is more performant than the other, I'd have to test. But it doesn't need to be a reference type anymore, so Swift style says default to a struct.

Alright, it's really starting to grow on me.

I mentioned previously that it starts growing on me

Yay 🤠

from fluent-kit.

Personally I also like

final class Planet: Model {
   struct Properties: ModelProperties {
       let id = Field<Int>("id")
       let name = Field<String>("name")
       let galaxy = Parent<Galaxy>(id: Field("galaxyID"))
   }
   static let properties = Properties()
   var storage: Storage
   init(storage: Storage) {
       self.storage = storage
   }
   convenience init(name: String, galaxy: Galaxy) {
       self.set(\.name, to: name)
       self.set(\.galaxy, to: galaxy)
   }
}

more then

struct Planet: Model {
    static let `default` = Planet()
    static let entity = "planets"
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let galaxy = Parent<Galaxy>(id: Field("galaxyID"))
}

Although I don't really understand why that first example has the Storage property and related initializer? Anyway, using this model seems easier since it has normal initializers instead of having to use .new(). What is the use of that default (or shared) property by the way?

from fluent-kit.

@kevinrenskers

Although I don't really understand why that first example has the Storage property and related initializer?

In that case, those are requirements of the Model protocol. Fluent needs to know how to create instances of your model. The Storage property is where information like the database output, unsaved input, eager-loads, joined models, etc is stored. It's where Fluent looks for data when you do planet.get(\.name). That call redirects basically to:

return try planet.storage.output!.decode("name", as: String.self)

What is the use of that default (or shared) property by the way?

To use key-paths, the properties must be non static. KeyPaths only work with instances atm. So, to actually convert a key path to the Field struct with the information we need, we need an instance of the struct, not just the type name.

When you do planet.get(\.name) the does something like this internally to get the string "name":

extension Row { // has generic Model type we can use
    func get<T>(_ key: KeyPath<Model, Field<T>>) throws -> T {
        let field = Model.default[keyPath: key]
        print(field.name) // String
        ...
    }
}

from fluent-kit.

@tanner0101 @mcdappdev It should be possible to create the field names based on the property name using Mirror, so that is worth looking into. I hope to make that how arguments and options work in ConsoleKit 4.

struct Planet: Model {
    static let `default` = Planet()
    static let entity = "planets"
    let id = Field<Int>()
    let name = Field<String>()
    let galaxy = Parent<Galaxy>(id: .init("galaxyID"))
}

from fluent-kit.

Ah, I see. And Model can’t be a class that needs to be subclasses instead of a protocol?

from fluent-kit.

@kevinrenskers it might be possible, but I'm not sure how you'd do it. For example, a sub-class can't override an associated type of a base class and Model.Properties is an associated type.

Generally, sub-classing and protocols don't mix well together. Vapor has totally avoided sub-classing in favor of protocols for this reason.

from fluent-kit.

@twof the idea is that it would work like this:

// fluent 4
struct Planet: Model {
    let id = Field<Int>("id")
    let name = Field<String>("name")
    let location: Field<String>("location", .psql(.point))
}

psql would be a static method on DatabaseSchema.DataType that puts a postgres-specific type into the custom(Any). When postgres goes to serialize the migration for that type, it will see the custom Any, and attempt to cast it to its known supported types.

from fluent-kit.

Will changes in Fluent 4 preclude ever having polymorphic models? @tanner0101 and I were having this discussion on Discord and here: #8

There are strong reasons for needing polymorphism such as a social app where a Like can be on multiple objects such as a Post, Comment, Photo, etc. Ideally you'd like to have one Like model and a base LikeableEntity model that Post, Comment, Photo, etc. models inherit from. The advantage is that you can have foreign keys and indexing so you can for example list all likes across multiple objects.

Postgres supports INHERITS: https://www.postgresql.org/docs/10/tutorial-inheritance.html

from fluent-kit.

@sherwinzadeh I think if anything this change would make it easier to implement polymorphism. Just based on the fact that being polymorphic is a kind a dynamism. But it's hard to say for sure what polymorphism would look like in Fluent without digging into that as a separate issue.

from fluent-kit.

@tanner0101, wouldn't using structs instead of classes preclude using inheritance for polymorphism?

from fluent-kit.

@sherwinzadeh classes work, too. I'm just using struct for that example since I don't explicitly need a reference type.

from fluent-kit.

Adding another posible future directions for fluent 4, looks like will be allowed use try on getter/setters, thanks to apple/swift#22749. seems like the implementation is almost done but I don't know if will need go through swift evolution though. cc @tanner0101

from fluent-kit.

@guseducampos That'd be awesome, but I don't think that would come earlier than Swift 5.1, so we'd have to do something else in the mean time.

from fluent-kit.

@guseducampos thanks for sharing. If throwing accessors works w/ the dynamic key path subscript proposed here: https://forums.swift.org/t/pitch-key-path-member-lookup/21579/42, that would be awesome.

Then you should be able to do this with no added code in your models:

try print(planet.name)
planet.name = "Earth"

from fluent-kit.

Major win - apple/swift#23436 (comment)

from fluent-kit.

Hello,

I know 1) I am very late to this party, and 2) I am a very inexperienced developer...but I managed to write the platform for my start-up using Vapor 2.4.

The reason I want to chip in here, is as the voice of the average programmer.

TL:DR - Having more complexity, and therefore flexibility, in the model definition is a good thing for some use cases.

The thing that I like about Vapor 2.4 is that, yes whilst there is quite a lot of boiler plating required when defining models, it did allow lots of flexibility, and allowed me to cover two of my use cases:

a) Enable models to produce different JSON encodings: my end users should not all see all of the model's fields, depending on their role, or where I want to save bandwidth and say only send back an array of id’s and names for a dropdown select list.

I wrote a master decode function for each model in which I would pass a context variable to alter the JSON output. My controller functions could then choose which output version to use.

b) It was easy to include data from a related model in the JSON output. Not the whole model but just a few fields.

Here’s a toy example of that tries to encapsulate what I’ve been doing:

final class User: Model {
	struct Props {
		static let organisation_id = "organisation_id"
		static let name = "name"
		static let age = "age"”
	}

	let storage = Storage()
	var organisation_id : Identifier? = nil
	var name : String
	var age : Int
}


extension User : JSONConvertible
{
	func makeJSONForUSer() throws -> JSON { return try buildJSON(scope: .user) }
	func makeJSONForManager() throws -> JSON { return try buildJSON(scope: .manager) }
	func makeJSON() throws -> JSON { return try buildJSON(scope: .list) }

	
	func buildJSON(scope: JSONScope) throws -> JSON
	{
		var json = JSON()

        		try json.set(CoreProps.id, self.id)
		try json.set(CoreProps.id, self.id)

        		if scope == .user || scope == .manager
        		{
			try json.set(User.Props.name, self.name)
			
			if let organisation = Organisation.find(self.organisation_id)
			{
				try json.set(User.Props.organisation, organisation.companyName)
			}	

		}

		if scope == .user
		{
			try json.set(User.Props.age, self.age)
		}
	}
}

And then a couple of example methods in the controller

        func userList(request: Request) throws -> ResponseRepresentable
	{
	 	let users = try User.makeQuery().all()
		return try users { try $0.makeJSONForList() }
	}


         func user(request: Request) throws -> ResponseRepresentable
	{
	 	return try request.parameters.next(User.self).makeJSONForUser()
	}

I wasn’t sure how I could achieve the same effect in Vapor 3 / Codable w/o having to define multiple structs for each different json version I needed

c) I also created a protocol that allowed me to write common functions such as basic record views, lists etc

protocol CoreFunctionCompatibleController
{
	associatedtype ModelType : CoreFunctionCompatibleModel, JSONConvertible, ResponseRepresentable
	
	var database : Database { get }
}


extension CoreFunctionCompatibleController
{

	
	func coreGet(request: Request) throws -> ResponseRepresentable
	{
		return try request.parameters.next(ModelType.self).makeJSON()
	}

}

d) Finally, I also wrote a set of functions to provide some common functionality to all my models:
i) I had a routine which would always be used instead of a save() call. It did a few extra things, like checking some security credentials, ensuring instance version in db matched that which was sent to UI, and adding some logging (for user use)
ii) I also have a routine which is added to every user search query, which adds additional constraints, to ensure that I user can only see records they have the right to see.

So if this proposal would enable this I would be very much in favour. And from what I have experienced to date adding more complexity in the model’s, can ease the code in the controllers. Removing too much control to make things "easy" can be a false economy - as later requirements become harder to implement.

Happy to share more (or be quiet!) if it is helpful - I am currently pondering how to go about an upgrade as staying on Vapor 2.4 for too long doesn’t should like a good option!

from fluent-kit.

Hello everyone,

I know I'm late in the discussion but I want to offer my point of view.

I my opinion it's very important to have data structures as light and pure as possible. Since working in Vapor 2 I've found myself using Fluent models as data wrappers of my "pure" data structures, in a single codable property called data. Other code parts working in bussines logic don't get tied on using Fluent models.

a sketch:

struct Planet: Codable{
 let id: Int?
 let name: String
let galaxy: Galaxy
}
// make it with default values and getting advanatge of new autogenerated partial initializers (allowing specify only some props in it without manually defining all cases)

struct PlanetModel: Model {
    static let shared = Planet()
    static let entity = "planets"
    static let dataMapping = [ 
                              ( \.id, Field<Int>("id") ),
                              ( \.name, Field<String>("name") ),
                              ( \.galaxy, Parent<Galaxy>(id: Field("falaxyID")) )
                      ]

    let data: Planet
}

I think with the improvements made in Swift (especially with codable) it's even more interessting to decouple the functionallity of being a bridge to the database as Fluent core goal and let go away json parsing etc...

Daniel

from fluent-kit.

Actually there is many good improvements with this new way of setting up the Model. But with the WWDC sessions about SwiftUI there is obviously a paradigm shift on how to define the UI from code, where the dynamism is returning but now in a type safe way. Isn't it what can be done with the Model too.
With a "VStack" like structure the whole Model and its schema can be defined in a simple but generic
and powerful way. Maybe a little late for 4.0 but nice for the future. Just my 2c.

from fluent-kit.

A bit of an addendum, but I would love to see inheritance included in Fluent 4 to allow for polymorphism as discussed above, and to allow Fluent models be imported and subclassed as necessary, as I discussed in #636 over at fluent. Right now there's no good way for a business to structure multiple Vapor applications around a base package of models that can be inherited on a per-app basis.

from fluent-kit.