Skip to content

refactor: deduplicate shared logic and remove dead code across modules - #526

Merged
zantvoort merged 1 commit into
mainfrom
refactor/dedup-cleanup
Aug 29, 2026
Merged

refactor: deduplicate shared logic and remove dead code across modules#526
zantvoort merged 1 commit into
mainfrom
refactor/dedup-cleanup

Conversation

@zantvoort

Copy link
Copy Markdown
Collaborator

Behavior-preserving cleanup across the repository: duplicated logic collapses into shared helpers, dead code goes, and redundant dialect overrides are dropped. 91 files, net −1,299 lines. No public API is added; the JPMS surface only narrows.

storm-core

  • New OnConflictEntityRepositoryImpl holds the INSERT ... ON CONFLICT upsert machinery previously duplicated between the PostgreSQL and SQLite repositories; the version-column timestamp resolves through dialect().currentTimestamp(), which renders the same literal PostgreSQL previously hardcoded.
  • EntityRepositoryImpl gains the id-returning upsert loop (upsertAndFetchIdsPartitioned + upsertPartitionAndFetchIds + the SeqPartitionKey types) previously copied into the PostgreSQL, MariaDB, and MSSQL repositories, and insertAndFetchIdReturning for the INSERT ... RETURNING single-insert PostgreSQL and MariaDB shared verbatim.
  • MergeEntityRepositoryImpl.doUpsert/doUpsertBatch delegate to their id-returning twins; EntityRepositoryImpl.update(List, PreparedQuery, cache) delegates to updateAndFetchIds, and the insert pair shares an executeInsertBatch prologue. No subclass splits any of these pairs, so dispatch is unchanged (MySQL, whose pair is split by MariaDB, is untouched).
  • Template engine: SqlTemplateImpl's twin shape/compilation key builders share buildKey; the join/WHERE rendering flagged DuplicatedCode in DeleteBuilderImpl/SelectBuilderImpl moves to QueryBuilderImpl.appendJoinsAndWhere; TemplateProcessor binds through one bindAll; TemplatePreparation gains whereOrParam and resolveFromAlias; RecordMapper gains pkColumnOffset; SqlInterceptorManager's four lock bodies share add/remove helpers; Providers.getSqlDialect overloads share sqlDialectFor.
  • New package-private BaseRef carries the state and accessors RefImpl and ScalarRefImpl duplicated; AbstractRef keeps the cached row identity, ScalarRefImpl stays cache-free.
  • Provably unused members and dead branches removed (QueryBuilder.asMetamodels, setObjectOr, two REFLECTION constants, PredicateBuilderFactory.createRefWithId, the unreachable kClass.isData() re-check).

Dialect modules

  • Overrides identical to DefaultSqlDialect are removed: quote-literal/identifier/valid-identifier pattern constants, supportsDeleteAlias/supportsMultiValueTuples, forUpdateLockHint, and the redundant implements SqlDialect clause. Genuinely divergent code (SQLite's quote regex, MySQL/MSSQL identifier patterns, every forShareLockHint) stays.
  • storm-mariadb drops a dangling requires org.jetbrains.annotations;; the Oracle dialect's mangled license header is restored; SQLite's quote-literal javadoc no longer claims backslash handling its regex does not do.

storm-kotlin

  • The ~30 entity/projection dispatch bodies in RepositoryLookup.kt route through the typed select<T>()/selectRef<T>() helpers plus a new @PublishedApi internal selectCount<T>(); every erased cast in those functions is gone and the compiler now checks the chains.
  • ORMTemplateImpl's twin repository scanners share createRepositoryFor; QueryBuilder.kt's three column-list loops share columnList; TransactionCallbacks settles through one fire.

storm-spring and starters

  • exports st.orm.spring.impl is now qualified to storm.kotlin.spring and the Spring modules that reflectively instantiate the auto-configuration, registrar, and runtime-hints classes, matching the sealing of storm-core's impl packages.
  • New EntityCaches dedupes the entity-cache methods of SpringTransactionContext and SpringLinkedTransactionContext; PerformanceLog owns the st.orm.sql.perf logger both boundaries previously created; SpringTransactionContext shares its deadline and timeout-translation logic; the DataStormTest context key compares annotationState directly instead of building throwaway filters.

Metamodel processors

  • APT: the record and sealed-interface class emitters share one renderMetamodelClassSource; interfaces share writeMetamodelInterface; every generation site uses writeSourceFile; rootIsSameBody and reportInlineKeyAnnotations are single-sourced. Regenerating storm-core's 264 generated metamodel files produces byte-identical output.
  • KSP: renderResolvedKotlinName, primitiveEq, reportInlineKeyAnnotations, and dependenciesOf collapse the equivalent copies.

Test support

  • TestSpringConnectionProvider, previously copied into all seven dialect modules, lives once in storm-test under st.orm.test.spring. The package is deliberately not exported: the dialect suites run classpath-mode, where their META-INF/services registration reaches it. spring-jdbc joins storm-test in provided scope, so nothing leaks transitively.
  • IntegrationConfig stays per module on purpose: @EntityScan/@EnableJpaRepositories scan from the annotated class's package, so each copy does different work.

Verification

  • Full suites green: storm-core (2,659), storm-kotlin (1,646), storm-foundation, storm-test, storm-spring, both starters, storm-micrometer, storm-spring-boot-test-autoconfigure, storm-jackson2/3, storm-java21, and both metamodel processors.
  • All seven dialect suites against real databases via Testcontainers: H2, SQLite, MySQL, PostgreSQL, MariaDB, MSSQL Server, and Oracle (1,180 tests), which also exercises the relocated Spring test connection provider in every suite.
  • The APT emitter refactor is byte-diffed against a pre-change baseline of all generated sources: identical.

}
}, getMaxShapes(), new SeqUpdateKey()).forEach(partition -> {
switch (partition.key()) {
case SeqNoOpKey ignore -> result.addAll(partition.chunk().stream().map(E::id).toList());
}, getMaxShapes(), new SeqUpdateKey()).forEach(partition -> {
switch (partition.key()) {
case SeqNoOpKey ignore -> result.addAll(partition.chunk().stream().map(E::id).toList());
case SeqUpsertKey ignore -> result.addAll(upsertPartition.apply(partition.chunk(), entityCache));
@zantvoort
zantvoort merged commit 812acf6 into main Aug 29, 2026
9 checks passed
@zantvoort
zantvoort deleted the refactor/dedup-cleanup branch August 29, 2026 22:15
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.

1 participant