feat!: EXPOSED-388 Support for column type converters

[obabichevjb]

Description

Here is the PR that allows to transform columns from their original types to another.

Transformations could be nested, and could be applied to nullable columns. Transformation of a non-nullable column produces a non-nullable column, and transformation of a nullable column produces a nullable one. Creating nullable columns requires not only types changing, but also set field nullable into true, so still the method nullable() must be used.

We already have transform() for columns on the DAO layer, so I took naming from there (toColumn, toReal methods):

.transform(
    toColumn: (TReal) -> TColumn,
    toReal: (TColumn) -> TReal
)

But probably it's not the perfect variant. I was initially thinking about Target/Source instead of TReal/TColumn respectively. If you prefer the first variant (or have other suggestions) I'd be happy to know. It would be breaking change, but it could be changed for DAO as well if needed.

Breaking changes

• DAO Entity Transformation Changes
  • Parameter Renaming: transform() and memoizedTransform() now use wrap and unwrap instead of toColumn and toReal.

    // Old:
    var name by EmployeeTable.name.transform(toColumn = { it.uppercase() }, toReal = { it.lowercase() })
    // New:
    var name by EmployeeTable.name.transform(wrap = { it.uppercase() }, unwrap = { it.lowercase() })

  • Class Renaming: ColumnWithTransform is now EntityFieldWithTransform, consolidating properties into a single transformer.

    EntityFieldWithTransform(column, object : ColumnTransformer<String, Int> {
            override fun unwrap(value: Int): String = value.toString()
            override fun wrap(value: String): Int = value.toInt()
        })

  • Entity transformation via DAO is deprecated and should be replaced with DSL transformation.

    val tester = object : Table() {
            val value = integer("value")
                .transform(wrap = { ... }, unwrap = { ... })
        }

[bog-walk]

What would be the plan for this feature in relation to the existing DAO ColumnWithTransform? Could that class make use of this transformer instead?

And given how transform already works for Entity field columns, if the user does not choose to use memoizedTransform() for example, could defining the transformation on the table object actually replace defining the transformation on the entity class?

Also, would using the 2 in parallel work ok? For example, defining a column with core/DSL transform() in table object. Then defining entity class with a field for that column, which then uses a DAO transform() on every read. Would it create similar behavior as in the nested transformations test?

[obabichevjb]

Actually, DAO transform and DSL transform are independent.

The main difference is that on the DSL level it's necessary to make column replacement with a new column type. It's needed because during reading/writing values go through the underlying ColumnType, so table object must contain the column with the proper column type.

I was trying to make the transform without replacing column but haven't found a way to do that, if you probably have any ideas how it could be done, I'd be happy to know about it).

From the DAO's perspective transformed DSL column is still just a column, so DAO works as before. I added a test testDaoTransformWithDslTransform() to show that DAO and DSL transforms could be mixed.

[e5l]

We need to define the Real type. It's not clear what it is in the air.

I would also check what names are used in applications for that. There are already some known, like Domain, Model, ...

[obabichevjb]

I took these names from the existing ColumnWithTransform class. But we may think about complete renaming of these types. What do you think about TOuter/TInner (and methods toOuter()/toInner() respectively), or TTransformed/TOriginal, or TWrapper/TColumn or something different?

[e5l]

please consider making it private

[obabichevjb]

It's used in both: DSL transform, and DAO transform which are in different packages. I could copy it and make private in both packages.

[e5l]

or it can be internal

[obabichevjb]

It's the first thing I tried to do, but it is not reachable from the exposed-dao module..

[obabichevjb]

Another variant is in ktorm:

fun <C : Any, R : Any> Column<C>.transform(
    fromUnderlyingValue: (C) -> R,
    toUnderlyingValue: (R) -> C
): Column<R>

Hibernate also has similar concept, but it applies modification directly to the sql, and looks like the following annotation:

@ColumnTransformer(
  read="decrypt(credit_card_num)" 
  write="encrypt(?)"
)
String creditCardNumber;

In SqlDelight there is a similar to our transform thing called ColumnAdapter:

object : ColumnAdapter<List<String>, String> {
  override fun decode(databaseValue: String) =
    if (databaseValue.isEmpty()) listOf() else databaseValue.split(",")
  override fun encode(value: List<String>) = value.joinToString(separator = ",")
}

[obabichevjb]

Updated PR. I tried to check how similar APIs look in other libraries and tried to make it more consistent inside Exposed.

At the current moment I stopped at the variant of type names for transformation as Source/Dest pair; we already have such namings across other methods, and it looks reasonable for generic transformation.

But for more specific cases there are changes:

• When transformation is applied to the column API looks like fun <TColumn : Any, Target : Any> Column<TColumn>.transform(transformer: ColumnTransformer<TColumn, Target>): Column<Target>, so it's clearer where is the original column value, and where is the transformed one
• When transformation applied to already transformed entity field API looks like fun <TColumn : Any?, CurrentTarget : Any?, NextTarget : Any?> EntityFieldWithTransform<TColumn, CurrentTarget>.transform(toCurrentTarget: (NextTarget) -> CurrentTarget, toNextTarget: (CurrentTarget) -> NextTarget): EntityFieldWithTransform<TColumn, NextTarget>. In this case Target is split into CurrentTarget and NextTarget.

Also in all the generic interfaces/methods the source/column type is the first. It was not aligned before.

One more change is the renaming of base transformation class for DSL into ColumnWithTransform, and for DAO into EntityFieldWithTransform.

[bog-walk]

Personally I think this may be quite confusing. Reading the function names alone, it's not immediately clear which value is meant for the database. With the KDocs it does help imply (but I had to really think) that Source means what is actually stored in the db and Target means the value used client-side?

I get that the original toReal() was also vague, but maybe we could still consider other options?
Something like toColumnValue + toTransformedValue,toColumnValue + fromColumnValue or some variant of read + write like readFromColumn + writeToColumn?

[bog-walk]

I understand that transform() doesn't actually have to be used with a ColumnTransformer object, but it still seems misaligned to me that the parameter names and interface method names are only partially matched:
toTarget -> toTarget()
toColumn -> toSource()

Maybe if both were completely different it would be better, but since 1 of them is the same, shouldn't they both match?

[bog-walk]

With the introduction of transform() in DSL, I'm still not sure that I see the value of DAO's transform() being kept. They serve the same purpose (except for DAO's additional caching to avoid multiple expensive transformations) and they have the same name. At the moment, I can't think of any other example in the API that has separate DSL and DAO components that are identically named and also so close in functionality.

For example, using the DAO test file ColumnWithTransformTest.kt (of course without the new DSL+DAO unit tests):
If I cut & paste all the transform blocks from each entity class and place them in the respective table object instead, all tests still pass. So our current tests at least get the same result using the new functionality on the DSL table instead of the old functionality on the entity class.

I guess I just still don't have an answer to my original question: Why would/should a user choose transform on Entity instead of transform on Table?
If the answer is, just for potential memoisation, we could still enable that through the new DSL implementation by leaving the actual transformation logic in the core module only, deprecating the DAO transform(), & reducing the DAO EntityFieldWithTransform to only having the caching logic (& renaming function to something like withTransformation()).

[e5l]

For me it looks the opposite way: I usually choose if I need Entity or Table and next stick with what I have. Let's discuss it anyway

[e5l]

LGTM, except for two questions for the design:

• naming
• Entity and Table transform behavior

@obabichevjb, could you find a slot for us to discuss this?

[e5l]

Let's discuss the naming. Please define the Target upfront

[e5l]

If you want to have a symmetric name for toColumn, I would go with toUserType.

There are also other options we can discuss:

• pack / unpack
• save / load
• encode / decode
• wrap / unwrap
• persist / retrieve
• store / fetch

[e5l]

For me it looks the opposite way: I usually choose if I need Entity or Table and next stick with what I have. Let's discuss it anyway

[obabichevjb]

I would sum up the questions for the discussion:

• Do we need transformation for both DSL and DAO, probably we need it only for DSL?
• Naming:
  • Transformation methods
  • Generic types (first transform and chained transform cases)

Naming

The naming toSource/toTarget most chosen as a most generic to fit dsl column transform, entity column transform, and chained transformations. Another variant I had before (but finally changed) was toColumn/toTransformed, but it was looking a bit weird in the case of chained transformations.

So I made a generic transformation interface and specific parameters for specific cases. That's why transform() methods have toColumn/toTarget for the first call and toCurrentTarget/toNextTarget for the second one. But it looks like it's over complicated and not so meaningful.

The method name toColumn I chose by analogy with IColumnType::valueToDB, so the second variant I liked was to name it like in IColumnType, it could toColumn/fromColumn or valueToColumn/valueFromColumn. One more question in this case would be in the names of generic types. There are also several options (during implementation I wanted several times to write TTransformed but it looks weird..) like TTransformed,TransformedType,TransformedValue/TColumn,ColumnType,ColumnValue. TColumn is a good variant because we already use it in several places, but paired TTransformed looks weird.

From the comment above the variant wrap/unwrap looks interesting. It could be used with types TWrapped/TColumn.

DSL/DAO

I also do not see something blocking from removing DAO's transform. Anyway we could mark it as deprecated and check if somebody complains about the case that could not be covered with DSL's only variant.

Potential reasons to keep both variants I see:

• Using both DSL and DAO layers independently. Exposed allows to mis DSL and DAO in the code, so it's possible that for some cases (like scripting over the database) user may want to have original types, but for application logic transformed, in this case DAO one would be still needed.
• Adding DAO to the existing application. In this case it could be complicated to add transformation to the DSL table, but could be added without problems to the DAO entity.

[obabichevjb]

Updated PR.

Methods toTarget()/toColumn(), toTarget()/toSource() (and original toReal()/toColumn()) were renamed to wrap/unwrap.
Generic types Source/Target, TColumn/TReal were renamed to Unwrapped/Wrapped

[bog-walk]

Looks good, just some minor KDocs comments. Oh and potential breaking change point.

[bog-walk]

The double "transform" and double "to'" is a bit confusing. I'd just suggest something like any of the following:

• "Returns the underlying column value without a transformation applied"
• "Reverts the value of the transformed type and returns the underlying column type"
• "Applies a reverse transformation on the value of the transformed type and returns the underlying column type"
• "Applies a reverse transformation on the value of the transformed type returning it to the column type"

[bog-walk]

I would change this to be consistent with all the other @params in the functions below:
@param Unwrapped The type of the original column.

It's also more clear.

[bog-walk]

If the goal is to see what feedback we get from users, I'd also suggest adding a line like:
"Please log a request on YouTrack if a use-case cannot be sufficiently covered by the DSL transform()."

[sschuberth]

As a remark, the ReplaceWith code seems to be completely useless to convert code like

var finishedAt by AdvisorJobsTable.finishedAt.transform({ it?.toDatabasePrecision() }, { it })

which creates

var finishedAt by run {
    { it?.toDatabasePrecision() }
    { it }
    object : Table() {
        val c = column().transform(wrap, unwrap)
    }
}

that does not compile.

[yassenb]

This is not a proper replacement for the DAO transform! I have code that transforms one column's value using another column's value (or any DAO property for that matter). The point of having the DAO is moving the complex business domain logic there and I don't see why transformations shouldn't be able to take advantage of that. Please revert the depracation of the DAO transform

[obabichevjb]

@yassenb Thank you a lot for the comment, to be honest it was not expected that transform allows to make transformation based on the whole entity. I'll take it into account and rethink on what we'll do with the whole feature.