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

Docs: Extend entities definition docs and reference code from snippets #2264

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

vnikolova
Copy link
Collaborator

Description

Extend documentation for "Defining entity instances" and replace code examples with references from snippets.

Defining entity instances documentation

Extended the documentation for defining entities instances in the DAO's 'Table Types' topic.

  • Added cross-references to the 'CRUD operations' topic.
  • Elaborated on keywords.
  • Moved the section to the bottom of the topic.

Referencing code from snippets

Extended the exposed-dao project sample to include the following:

  • atables directory, including table definitions referenced in the DAO's 'Table Types' and 'CRUD operations' topics.
  • an entities directory, including entities definitions also referenced in the DAO's 'Table Types' and 'CRUD operations' topics.
  • Separate functions in App.kt for CRUD operations, as referenced in the DAO 'CRUD operations' topic.
  • README.md files with instructions how to run individual projects and how to reference code from snippets.

Notes

  1. In the process of replacing the code blocks, I found that the findById query in this example was not working, so I removed it. Please let me know if this was a mistake.
val directorId = CompositeID {
    it[Directors.name] = "George Lucas"
    it[Directors.guildId] = "..."
}

val director = Director.findById(directorId) // not working, removed from example
val directors = Director.find { Directors.id eq directorId } // this is fine
  1. Not sure if we should also rename the 'Table types' topic to 'Table types and Entity definition' or something like that. Or perhaps move the entities definition to 'CRUD operations'. What do you think makes more sense?

Type of Change

Please mark the relevant options with an "X":

  • Documentation update

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)

Related Issues

EXPOSED-535 Improve documentation for Table types and definition

Copy link
Member

@bog-walk bog-walk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left comments in details. My primary points are:

  • Entity class definition examples that have been removed (or at least I can't seem to find them)
  • Placement of entity definition section
  • All snippet functions being placed together in App.kt & what kind of cascade effect that may have with any future updates/additions/refactoring to lines of code.

Comment on lines 112 to 113
</chapter>
<chapter title="Defining entity instances" id="define-entity-instance">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're planning ahead, I'd actually suggest moving this section to it's own standalone topic, like "Entity definitions", after this "Table types". Mostly because there's a lot more content that can be provided specific to DAO entities.
For example:

  • The composite id section and the custom table section above now only show how to set up the tables. A new topic could have respective sections showing how to make their matching entities.
  • How to define class method overrides. A lot of power can be gained by implementing overrides of both Entity methods and companion object EntityClass methods, so it'd be worth showing examples of that.
  • EntityClass technically has 2 other properties that are rarely used, but which can be defined to gain more control over the associated entity type and construction. To this day there's no documentation for how to take advantage of these properties, just some vague ones in test.
  • There is also no documentation about special ImmutableEntityClass and ImmutableCachedEntityClass, which can be used as companion objects when defining entities that are meant to be immutable.

If a new topic was made, I'd also consider moving "Field transformations" inside it as a subsection.

I think the CRUD ops topic already has a lot of content.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestions. I've created EXPOSED-578 for the missing documentation. For the time being, I will move the existing content to a new topic to be extended upon in a separate PR later.

companion object : IntEntityClass<StarWarsWFilmWithRankEntity>(StarWarsWFilmsWithRankTable) {
override fun searchQuery(op: Op<Boolean>): Query {
return super.searchQuery(op).adjustSelect {
select(columns + StarWarsWFilmsWithRankTable.rank)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bog-walk In the original example line 21 has .set in the end. I removed it to pass the detekt check because otherwise it produced the following error:
Type mismatch: inferred type is FieldSet but Query was expected
As it is now, the ReadExamples.readComputedField example (currently commented out) results in this error:

Exception in thread "main" java.lang.IllegalStateException: RANK() OVER( ORDER BY STARWARSWFILMSWITHRANK.RATING DESC) is not in record set
        at org.jetbrains.exposed.sql.ResultRow.getExpressionIndex(ResultRow.kt:128)
        at org.jetbrains.exposed.sql.ResultRow.getRaw(ResultRow.kt:109)
        at org.jetbrains.exposed.sql.ResultRow.getInternal$lambda$5(ResultRow.kt:72)
        at org.jetbrains.exposed.sql.ResultRow$ResultRowCache.cached(ResultRow.kt:207)
        at org.jetbrains.exposed.sql.ResultRow.getInternal(ResultRow.kt:71)
        at org.jetbrains.exposed.sql.ResultRow.get(ResultRow.kt:40)
        at org.example.entities.StarWarsWFilmWithRankEntity.getRank(StarWarsWFilmWithRankEntity.kt:16)
        at org.example.examples.ReadExamples.readComputedField(ReadExamples.kt:117)

Can you advise how to fix it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entityCache.clear() from the original example is what's needed to fix this (or perform each operation in a separate transaction).

When new() creates an entity, it's cached. Then using find() first checks the cache for the entity and, only if it isn't found, then pings the database to find the record and returns it as a new entity.
So the current setup means the entity is in the cache, and is returned by find(). But it throws that exception because the cached entity only stores the data provided to new(). The window function rank is a database generated value so we need to make sure the entity is being retrieved from the database instead of the cache.

So we have to manually clear the cache since we're running these samples in a single transaction:

fun readComputedField() {
    transaction {
        StarWarsWFilmWithRankEntity.new {
            sequelId = MOVIE_SEQUELID
            name = "The Last Jedi"
            rating = MOVIE_RATING
        }

        entityCache.clear()

        StarWarsWFilmWithRankEntity
            .find { StarWarsWFilmsWithRankTable.name like "The%" }
            .map { it.name to it.rank }
    }
}

The alternative would be to simulate what a user more likely does, which is create entities in 1 transaction, then perform queries in a separate transaction:

// the way App.main() is set up, this would require changing the project's database config
Database.connect(
    "jdbc:h2:mem:test",
    "org.h2.Driver",
    databaseConfig = DatabaseConfig { useNestedTransactions = true }
)

fun readComputedField() {
    transaction {
        StarWarsWFilmWithRankEntity.new {
            sequelId = MOVIE_SEQUELID
            name = "The Last Jedi"
            rating = MOVIE_RATING
        }
    }

    transaction {
        StarWarsWFilmWithRankEntity
            .find { StarWarsWFilmsWithRankTable.name like "The%" }
            .map { it.name to it.rank }
    }
}

Comment on lines 9 to 10
val deletedMovie = movie.delete()
println(deletedMovie)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do another review whenever you're ready to request a new one, but I saw this when troubleshooting.

Entity.delete() doesn't return a value , but using line 9 val deletedMovie = movie.delete() gives the impression that it might return the entity or something.
This might be a case to use individual lines in code block, or change the entire function:

fun deleteFilm() {
    val movie = StarWarsWFilmEntity.findById(2)
    movie?.delete()
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I've fixed the examples as suggested and updated the code refs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants