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

Make LastInsertIDInitializable public #199

Closed
t-ae opened this issue Mar 17, 2021 · 2 comments
Closed

Make LastInsertIDInitializable public #199

t-ae opened this issue Mar 17, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@t-ae
Copy link

t-ae commented Mar 17, 2021

I'm planning to have indivisual ID types for each models.
For example:

struct MyModelID: Codable, Hashable, RawRepresenatable, LosslessStringConvertible {
    var id: Int

    ...
}

final class MyModel: Model {
    @ID(custom: "id", generatedBy: .database)
    var id: MyModelID?

    ...
}

It seems most Fluent futures works well, but I found a problem.

id of MyModel is auto incrementing INT column, so it should be set after the model is saved.
Unfortunately, if the type of id does not conform to LastInsertIDInitializable, the program crashes at here:

if let lastInsertIDInitializable = T.self as? LastInsertIDInitializable.Type {
return lastInsertIDInitializable.init(lastInsertID: self.metadata.lastInsertID!) as! T
} else {
fatalError("Unsupported database generated identifier type: \(T.self).")
}

To make matter worse, MyModelID cannot confrom to LastInsertIDInitializable because it is internal.
protocol LastInsertIDInitializable {
init(lastInsertID: UInt64)
}

I want it to be public.

Also I want to hear your opinion about using custom types as ID.
I think its good idea but haven't gone deep yet.

@t-ae t-ae added the enhancement New feature or request label Mar 17, 2021
@0xTim
Copy link
Member

0xTim commented Mar 27, 2021

Why do you want to use a custom type for the ID? I can see this breaking a lot of Fluent, including the relationships and querying since Fluent won't know what underlying DB type to map to in the encoder/decoder

@t-ae
Copy link
Author

t-ae commented Mar 27, 2021

The main reason is that it is easy to misuse if all models have same ID type.
The IDs of ModelA and ModelB is similar but they are different. ModelA.find(idOfModelB, on: db) is totally incorrect operation.
I want to forbid it with type system.

I can see this breaking a lot of Fluent, including the relationships and querying since Fluent won't know what underlying DB type to map to in the encoder/decoder

That's correct. I didn't noticed simply making LastInsertIDInitializable public causes several problem.
There's no restriction LastInsertIDInitializable will be encoded to integer.
As far as I know there's no way in type system to assure the underlying type so protocol approach won't work.

The only way that can achieve my demand is having ID type in this repository but it's going too far.
I give up this issue.
If someone has good idea, please tell me.

@t-ae t-ae closed this as completed Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants