Visitar URL original
Migrate Commons DBCP 1.x to DBCP 2 by krlittle · Pull Request #179 · openrewrite/rewrite-apache · GitHub
Skip to content

Migrate Commons DBCP 1.x to DBCP 2 - #179

Open
krlittle wants to merge 1 commit into
mainfrom
commons-dbcp-2-migration
Open

krlittle wants to merge 1 commit into
mainfrom
commons-dbcp-2-migration

Conversation

@krlittle

@krlittle krlittle commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Nothing migrates Commons DBCP 1.x yet, and changing the dependency and the package is not enough. DBCP 2 renamed the pool settings, and BasicDataSourceFactory ignores the old names without a warning:

# a file given to BasicDataSourceFactory.createDataSource
maxActive=500    # ignored by DBCP 2, so the pool stays at its default of 8
maxWait=5000     # ignored by DBCP 2, so a caller waits forever

UpgradeApacheCommonsDbcp_1_2 migrates the build, the Java sources and that configuration, in repositories that depend on commons-dbcp.

Changes

  • UpgradeApacheCommonsDbcp_1_2 (new, YAML):
    • Dependency: commons-dbcp:commons-dbcp becomes org.apache.commons:commons-dbcp2:2.x. commons-pool 1.x is kept where code still uses it, since DBCP 2 no longer brings it.
    • Java: org.apache.commons.dbcp moves to org.apache.commons.dbcp2, and the renamed methods of BasicDataSource, SharedPoolDataSource and AbandonedConfig follow. SQLNestedException becomes SQLException, DbcpException becomes RuntimeException, and AbandonedConfig becomes the one in commons-pool2.
    • Configuration: class names in string literals and in XML class attributes, and bean properties, including p: attributes.
  • MigrateBasicDataSourceFactoryProperties (new, Java) renames maxActive, maxWait and removeAbandoned in each .properties file that a class calling createDataSource names in a string. Other properties files are left alone, as their keys can be the application's own.
  • rewrite-properties joins the dependencies, and DBCP and commons-pool 1.x and 2.x join the test classpath. That is the binary type table in this diff, so another PR changing it needs it regenerated rather than merged.
  • The two recipes.csv rows are added by hand, as the generator rewrites every row. They match what it generates for these recipes, and recipeCsvValidate passes.

This is deliberately not part of ApacheCommonsBestPractices. That composite runs broadly, and this recipe leaves some repositories not compiling until someone finishes the port, see below.

Verification

./gradlew check passes: 361 tests, 21 of them new.

UpgradeApacheCommonsDbcp_1_2 in Moderne, over the 307 repositories a search found using DBCP 1.x by dependency, type or bean definition:

Repositories changed 240
Files changed 676
Repositories with errors 0

Of the 67 left alone, 66 have no commons-dbcp in their build model. The other is linkedin/data-integration-library, which declares it through a Gradle map the recipe cannot reach, and nothing in it changed.

32 of the repositories, one shard of the run, were then built before and after the migration:

Compile Repositories
Still compile 26
Stop compiling 5
Already broken 1
Tests of those 26 Repositories
Pass 14
Newly fail 3
Fail or time out the same way before 9

What this does not show:

  • The search index covers 49,525 of the organization's 57,096 repositories, so some users of DBCP 1.x may be missing.
  • The sample was built from an earlier run, which differs from this recipe only in properties and ddal-config files.
  • apache/inlong fails before and after with different tests. The names look unrelated, but it was not rerun.

Behaviour to review

The 3 new test failures all come from DBCP 2 itself, not from the recipe's edits:

  • Drivers from before JDBC 4: DBCP 2 validates connections with isValid(), which is an AbstractMethodError on HSQLDB 1.8 (datacleaner/DataCleaner).
  • Old log4j-api: DBCP 2.14 brings commons-logging 1.3.5, which calls a method an old log4j-api does not have (hdiv/insecure-bank).
  • Hibernate 6 and later: DBCP 2.14 brings jakarta.transaction-api 1.3.3, which still holds javax.* classes. Being nearer in the tree it replaces the 2.x Hibernate needs, and jakarta.transaction.TransactionManager goes missing (CloudSlang/score). Up to 10 of the 240 repositories use those Jakarta types.

Also worth a look:

  • setMaxWait becomes setMaxWaitMillis, not setMaxWait(Duration). It is in every 2.x release, but deprecated from 2.10, so expect deprecation warnings.
  • BasicDataSourceFactory still silently ignores the old keys where they are set in code, or in a properties file named outside of the class calling createDataSource. The recipe's description says so.
  • A repository defining its own classes in these packages has them moved too. alibaba/druid's shims are moved over its existing dbcp2 shims, so that repository should be excluded. Sakai's classes move as well, and those were not built.
  • mraible/ajax-login and nmyphp/tbschedule have their XML moved with no change to a build file, so they need the dependency added by hand.

Not in this PR

The 5 repositories that stop compiling:

  • Code wiring PoolableConnectionFactory to a commons-pool ObjectPool by hand needs a real port to commons-pool2: AlexeiStrug/EPAM, h-thurow/Simple-JNDI and KualiCo/rice. About 9 repositories in the organization do this, each differently.
  • Sources the LST could not type keep their imports on the old package: taskurotta/taskurotta, and white-cat/jeeweb outside of the sample.
  • A module with no Maven model is out of a recipe's reach: sample/pom.xml in sogou-biztech/compass.

`UpgradeApacheCommonsDbcp_1_2` changes the dependency, moves the package,
renames the methods and bean properties that changed, and replaces the
classes DBCP 2 removed. `MigrateBasicDataSourceFactoryProperties` renames the
keys `BasicDataSourceFactory` would otherwise silently ignore.

It is not part of `ApacheCommonsBestPractices`, as it leaves code that wires
commons-pool by hand not compiling until it is ported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant