Generate ID on Entity.save()

[stapel]

Currently Entity depends on the database the create a value for id. This is nice for Longs or Ints as most databases provide autoincrement or something like that. Now I'd "want" to use UUIDs as id, this is hard to make work with e.g. MySQL in a convenient or simple way. I'd really like to see the functionality to provide a generator-method or something like this to make it into Entity. Any ideas or comments appreciated.

[mvysny]

Currently the Entity.save() does SQL INSERT if the ID is null, or SQL UPDATE if the ID is not null. Which is kinda incompatible with app-generated as you found out :(

The generator method sounds great. I'm thinking of having a Entity.generateID() function which would return null by default but you could override it to return UUID.create(); the save() method could then consult the generateID() function when the ID is null, and do an INSERT with ID pre-provided.

However, there is the problem of pre-provided IDs (e.g. if the ID is pre-known, say, a state person identifier). The pre-provided ID kinda rules out the current save() algorithm, since it would always do insert. Yet this could be solved by UPSERT maybe...

[mvysny]

I wonder if it's simply easier to replace save() with two other methods, create() and update(), then let the vok-orm user to figure out the correct operation to call. Or, we keep save() and pile up UPSERT and generateID() on top of that. I can't kinda decide.

In my simple apps I am always able to tell whether I'm creating or updating, and the first option sounds a lot simpler, both to explain in the API and to implement (no need to figure out how to do UPSERT on multiple databases), so I'm a bit biased that way. However, I'd love to draw from your experience @stapel - what do you think? Which way would work best for your app?

[mvysny]

Alternatively, we can perhaps keep all three methods. That way, you can use save() with IDs generated by the database (the common case), and create()/update() with both app-generated and pre-provided IDs... However that would require a proper explanation both in kdoc and in the documentation.

[stapel]

Personally I'd like the generateId()-method (I fiddled with it locally, I still need to find a way to add a custom UUID-Converter for sql2o).

If going the create/update-way, I'd keep all three, because, as you said, save() will probably be perfect for most use cases.

I wouldn't go in the UPSERT-direction, it sounds cumbersome to check for different databases with different versions.. and IIRC there is very little database-dependency (maybe a little pgsql-preference) in the vok-subprojects.

What are your thoughs on always generating UUIDs on any Entity<UUID> with id = null?

[mvysny]

What are your thoughs on always generating UUIDs on any Entity with id = null?

Hmm, it could only work if the type of the ID column was UUID. I feel it's too magicky, in a sense that the behavior suddenly changes for UUID-typed column. Therefore I'd advise against it, I'd rather make it explicit.

Since generateId() doesn't work with pre-provided IDs, I'd rather go with create() which works for both cases. Let me prototype this a bit :)

I wouldn't go in the UPSERT-direction, it sounds cumbersome to check for different databases with different versions..

I agree, I'll avoid UPSERT.

I still need to find a way to add a custom UUID-Converter for sql2o).

I thought it was natively supported... Let me test that as well.

[stapel]

Great!

Regarding UUIDs, in mysql you'd have to use binary(16) to properly save a UUID, this will be mapped to ByteArray in sql2o. And the default UUIDConverter does not check for a bytearray. So, writing works, reading doesn't.

[mvysny]

Yup, binary(16) is the way to go. However, sql2o doesn't map UUID to ByteArray for me automatically, since mysql fails with Caused by: com.mysql.jdbc.MysqlDataTruncation: Data truncation: Data too long for column 'id' at row 1. I'm guessing it's trying to use toString and insert the UUID as a String, which takes 36 characters. So writing doesn't work for me. I'll investigate a bit more.

[mvysny]

Fixed in vok-orm 0.17

[mvysny]

Hmm, not fixed yet. Sql2o sometimes picks up the ID type as Object instead of UUID, doesn't activate the Converter and then fails that it can't set ByteArray value to a UUID field.

[stapel]

This is a really nice solution, having only save() and create(). A generating wrapper-class is easily created and more expressive then the generator-variant!

[mvysny]

@stapel you don't even need to create the wrapper class perhaps; you can simply override the create() function as follows:

override fun create(validate: Boolean) {
    id = UUID.randomUUID()
    super.create(validate)
}

then you can either call create() directly, or you can perhaps simply continue using save() as before.

Yet I'd love to hear about your wrapper-class solution - could you paste a bit of info please?

[mvysny]

Reopened: rather than a nasty workaround in vok-orm it should be fixed in Sql2o: https://github.com/aaberg/sql2o/issues/314

[mvysny]

This bug report is about generating IDs; i've created a separate bug ticket regarding the failing converters: #10

[stapel]

@mvysny: It's nothing really, and I am not sure that this is the final form, but it keeps me from too much refactoring. ;) (And we probably want to use UUIDs for all Entities anyhow) (Edit: I probably used the wrong name with wrapper-class)

interface UUIDEntity : Entity<UUID> {
    override fun save(validate: Boolean) {
        if (id == null) {
            id = UUID.randomUUID()
            super.create(validate)
        } else {
            super.save(validate)
        }
    }
}

(Good catch with sql2o!)

[mvysny]

@stapel Hey, that's a really awesome idea, thanks! I've updated the docs to mention this possibility; we can probably simplify it to:

interface UuidEntity : Entity<UUID> {
    override fun create(validate: Boolean) {
        id = UUID.randomUUID()
        super.create(validate)
    }
}