refactor!: Rename IStatementBuilder interface and add executable converter #2562
Conversation
| @Deprecated( | ||
| message = "This interface has been renamed and will be removed in future releases.", | ||
| replaceWith = ReplaceWith("StatementBuilder", "org.jetbrains.exposed.v1.core.statements.StatementBuilder"), | ||
| level = DeprecationLevel.WARNING | ||
| ) | ||
| interface IStatementBuilder { |
There was a problem hiding this comment.
Let me know if either option would be better:
- Immediately set level to ERROR
- Instead of deprecating, just rename (since it is new as of 1.0.0)
There was a problem hiding this comment.
It looks like many people use the 1.0.0-demo-x just like the next versions of Exposed, so I'd say that it's better to keep it like this, and could be better to mark as deprecated and remove later. We still can do breaking changes, but if avoiding it is cheap, we could avoid it.
| @Suppress("ForbiddenComment", "AnnotationSpacing") | ||
| fun <S> buildStatement(body: IStatementBuilder.() -> S): S = body(StatementBuilder) |
There was a problem hiding this comment.
Could not deprecate this, as lambda parameter would cause overload ambiguity error.
There was a problem hiding this comment.
Also, just pointing out that the type parameter is not restricted to something like <T, S : Statement<T>>, so technically it could be used like this:
val x: String = buildStatement {
"hello world"
}Are we ok with this? Or is there a reason to not type it?
| fun JdbcTransaction.explain( | ||
| analyze: Boolean = false, | ||
| options: String? = null, | ||
| body: IStatementBuilder.() -> Statement<*> | ||
| body: StatementBuilder.() -> Statement<*> |
There was a problem hiding this comment.
Could not deprecate this, as lambda parameter would cause overload ambiguity error. Same with r2dbc variant.
| * | ||
| * @throws IllegalStateException If the invoking statement does not have a corresponding built-in executable. | ||
| */ | ||
| fun <T : Any, S : Statement<T>> S.toExecutable(): BlockingExecutable<T, S> { |
There was a problem hiding this comment.
Decided to add this when writing up the related documentation, because I thought it would be good to be able to take a Statement and then eventually make it into an executable to send to the DB. And it seems odd to force the user to have to look up what the possible executable classes are.
| // TODO: StatementBuilder -> StatementBuilderImpl, and IStatementBuilder -> StatementBuilder | ||
| private object StatementBuilder : IStatementBuilder | ||
|
|
||
| // TODO: add documentation for building statements without execution, like in the old DSL |
| @Suppress("ForbiddenComment", "AnnotationSpacing") | ||
| fun <S> buildStatement(body: IStatementBuilder.() -> S): S = body(StatementBuilder) |
| fun JdbcTransaction.explain( | ||
| analyze: Boolean = false, | ||
| options: String? = null, | ||
| body: IStatementBuilder.() -> Statement<*> | ||
| body: StatementBuilder.() -> Statement<*> |
…erter - Rename IStatementBuilder to StatementBuilder - Adjust associated methods - Add toExecutable() to allow easier conversion - Add unit tests
bog-walk
force-pushed
the
bog-walk/refactor-statement-builder
branch
from
c596812 to
1b2f6c4
Compare
Description
Summary of the change: Rename
IStatementBuildertoStatementBuilderand replace usage and add way to get built-in executable instance from statement instance.Detailed description:
IStatementBuildertoStatementBuilder+ deprecate originalbuildStatement()andexplain()Statement.toExecutable()in both modules to avoid user needing to know what executable classes areDocumentation update -> PR #2563
Type of Change
Please mark the relevant options with an "X":
Updates/remove existing public API methods:
Affected databases:
Checklist