mvysny / vok-orm

Mapping rows from a SQL database to POJOs in its simplest form
MIT License
21 stars 4 forks source link

Drop DataLoader API #18

Closed mvysny closed 7 months ago

mvysny commented 7 months ago

In practice, vok-orm is always used in connection with Vaadin; the DataLoader part is rarely used. However, this creates a huge abstraction mess when using with Vaadin: Vaadin needs DataProvider which is adapted to DataLoader which then finally polls vok-orm for data. We shall remove the data loader parts.

  1. The DataLoader filter API will be inlined into vok-orm and modified to directly take advantage of the new jdbi-orm Condition API. Discussion below.
  2. The EntityDataLoader and SqlDataLoader will be retrofitted to implement Vaadin 24 DataProvider and moved to a separate project. Alternatively, they can be moved to jdbi-orm-vaadin if it's tested that jdbi-orm-vaadin works with vok-orm entities.

Removal of DataLoader Filters

The JDBI-ORM Condition API provides direct replacement for the filter implementation. All that's left is to evaluate the replacement Kotlin API.

Currently it's possible to write the following expressions in Kotlin: Person.findAllBy { !(Person::age eq 26) }. The JDBI-ORM counterpart is Person.findAllBy(Person.AGE.eq(26)) which is actually pretty good but requires you to define all fields as val AGE = TableProperty.of<Person, Long>(Person::class.java, "age") which is a bit tedious. It would be great if Person::age of type KProperty1 could automatically convert to JDBI-ORM's TableProperty somehow, but that's not possible. Possible solutions below:

1: One way would be to define an infix invoke operator for all KProperty1 somehow so that you could write Person.findAllBy(Person::age { eq(26) }). Disadvantages: the syntax is too strange/fancy and harder to type than the option 4.

2: Another way is to keep the Filters API and create an infix function for all operators available in the Condition API. Backwards compatible, but we're not that keen in protecting backward compatibility here since it's going to be a major release anyway, but still might make the upgrade of existing code easier.

3: Another way is to add all operators as extension functions to KProperty, allowing you to write Person.findAllBy(Person::age.eq(26)). Here we might be polluting KProperty1 too much though, but perhaps it's okay. This is actually the same thing as the FilterBuilder yet FilterBuilder scopes the extension functions to the block, and thus doesn't pollute KProperty1 API. So this is very close to solution 2. Do note that when using outside of Dao's functions, you need to wrap the block with the buildFilter{} call which could be counter-intuitive but could be OK if properly documented. However that makes the API non-extensible.

4: Another way is to add an extension function KProperty1.col which creates TableProperty; you can then write Person.findAllBy(Person::age.col.eq(26)). A bit uglier than the one above, but we would be reusing the existing API to its fullest. The alternatives for col would be column, condition, asCondition, property, jdbi.

5: Alternatively we could unify 4 and 1 and use operators. Examples: the invoke operator: Person::age().eq(26). Possibly some other operator, see https://kotlinlang.org/docs/operator-overloading.html#increments-and-decrements . invoke() can not be used: KPerson::age() fails to compile with Kotlin error that this kind of syntax is reserved for future use. No other unary operator works: we don't want prefix operators like +KPerson::age and ++ doesn't work. So this option is not available.

mvysny commented 7 months ago

First steps:

  1. Unify EntityDataLoader with jdbi-orm-vaadin EntityDataProvider; port tests from vok-orm and make sure everything is working properly with Kotlin entities. Update documentation in vok-orm. DONE.
  2. Port SqlDataLoader to jdbi-orm-vaadin as SqlDataProvider and update documentation both in jdbi-orm-vaadin and vok-orm - DONE
  3. Add a new DataProvider which is able to load any object and uses DaoOfAny. Possible names: DaoOfAnyDataProvider, AnyDataProvider (shorter but too generic and could be easily mistaken in a bigger project for something else). Alternatively, it could be possible to reuse EntityDataProvider to load anything too - if it works, add a bunch of tests for this scenario as well: https://gitlab.com/mvysny/jdbi-orm-vaadin/-/issues/1 - DONE
mvysny commented 7 months ago

Solution: let's go with the solution that's very similar to the existing one: buildCondition{} and KProperty1.exp and KProperty1.asc and KProperty1.desc.

mvysny commented 7 months ago

Fixed in vok-orm 3.0