Exposed icon indicating copy to clipboard operation
Exposed copied to clipboard

feat!: EXPOSED-320 Many-to-many relation with extra columns

Open bog-walk opened this issue 1 year ago • 1 comments

Description

Summary of the change: Provides the functionality for DAO entities to set/get additional data to/from extra columns (non-referencing) in the intermediate table of a many-to-many relation.

Detailed description:

  • Why: Additional columns (beyond the 2 reference columns) are possible on intermediate tables defined in many-to-many relations. But these columns cannot be accessed by the referencing DAO entities involved because via() creates an InnerTableLink class that ignores these columns. Workarounds include adding a 'fake' id primary key to the intermediate table and creating an associated entity, or duplicating inner logic with custom classes. The latter only partially works, however, because internal cache logic requires that the target of via() is an Entity.

  • What:

    • This PR allows via() to be used in the same way as before by introducing a new entity class InnerTableLinkEntity that wraps the referenced entity (and delegates to it's id and table) along with any additional data in the intermediate table row.
    • Additional columns are no longer ignored by default and values are required to be used in the generated SQL whenever a new reference collection is set.
    • Additional column data can be accessed from either (or both) the source or target entity.
  • How:

    • Add abstract InnerTableLinkEntity, with associated InnerTableLinkEntityClass, that forces 2 overrides so users define how to properly set and get any additional data. Subclass can be a class or data class as needed.
    • via() and InnerTableLink now accept a list of columns as arguments to specify which additional columns should be included. This allows users to opt-out of new functionality, by providing emptyList(), if it breaks any current workaround.
    • InnerTableLink internal logic uses additional columns to generate triggered delete and insert SQL. Previously, statements would only be generated if there was a change in the reference id. Now they will be generated even if the reference id is identical, as long as any of the additional data changes. This is necessary to allow reference collections to be updated properly.
    • Add new internal EntityCache map just for InnerTableLinkEntity. Because the latter delegates to the wrapped entity, if the regular data cache is used, this special entity may override its cached wrapped entity or be incorrectly retrieved on find(). This map stores each entity by its target column and source id, as the intermediate table is expected to have a contract of uniqueness on the 2 reference columns.

Note: The original plan was to have this implementation alongside a more standard approach, which would get/set the additional data as a delegate field on an existing entity, for example:

class Project(id: EntityID<Int>) : IntEntity(id) {
    // ...
    var tasks by Task via ProjectTasks
}
class Task(id: EntityID<Int>) : IntEntity(id) {
    // ...
    var approved by ProjectTasks.approved
}

This worked well for safe setting and getting, but started raising questions when updates were introduced:

  1. Should it be possible to set the field approved in isolation? Meaning not as part of SizedCollection, but in new {} or through a standard task.approved = true?
  2. Would setting the field in isolation be considered an update and should it then trigger a cascade by causing the reference field to also update? And if so, how?
  3. If a Task was already cached with its approved field set by references loaded from the intermediate table, then something like Task.all() was loaded, the new task would override the cached task and trying to access the approved field would cause an exception as it would rightly not have any value. Would it be expected that this shouldn't happen?

So I opted to go for the safer implementation and if users come forward requesting the design above, I'm hopeful that their use cases will answer some of these questions.


Type of Change

Please mark the relevant options with an "X":

  • [X] New feature

Updates/remove existing public API methods:

  • [X] Is breaking change

Affected databases:

  • [X] All

Checklist

  • [X] Unit tests are in place
  • [X] The build is green (including the Detekt check)
  • [X] All public methods affected by my PR has up to date API docs
  • [X] Documentation for my change is up to date

Related Issues

EXPOSED-320, EXPOSED-443

bog-walk avatar Aug 16 '24 02:08 bog-walk

@obabichevjb I refactored this PR (and added more tests) as original cache was failing if the same wrapped entity was used with different additional data (for example, updating TaskWithData(Task(11), true, 1) to TaskWithData(Task(11), false, 1) would not trigger the cache to update). Now the cache stores these special entities based on target column, source id, and the target (wrapped) id.

Please let me know if any API improvements could be considered, and what you think about overriding the standard entity functions to throw an error (like new() etc) so that the entity isn't accidentally used like a standard entity.

bog-walk avatar Sep 20 '24 03:09 bog-walk