Repository navigation
Conversation
`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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nothing migrates Commons DBCP 1.x yet, and changing the dependency and the package is not enough. DBCP 2 renamed the pool settings, and
BasicDataSourceFactoryignores the old names without a warning:UpgradeApacheCommonsDbcp_1_2migrates the build, the Java sources and that configuration, in repositories that depend oncommons-dbcp.Changes
UpgradeApacheCommonsDbcp_1_2(new, YAML):commons-dbcp:commons-dbcpbecomesorg.apache.commons:commons-dbcp2:2.x.commons-pool1.x is kept where code still uses it, since DBCP 2 no longer brings it.org.apache.commons.dbcpmoves toorg.apache.commons.dbcp2, and the renamed methods ofBasicDataSource,SharedPoolDataSourceandAbandonedConfigfollow.SQLNestedExceptionbecomesSQLException,DbcpExceptionbecomesRuntimeException, andAbandonedConfigbecomes the one incommons-pool2.classattributes, and bean properties, includingp:attributes.MigrateBasicDataSourceFactoryProperties(new, Java) renamesmaxActive,maxWaitandremoveAbandonedin each.propertiesfile that a class callingcreateDataSourcenames in a string. Other properties files are left alone, as their keys can be the application's own.rewrite-propertiesjoins 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.recipes.csvrows are added by hand, as the generator rewrites every row. They match what it generates for these recipes, andrecipeCsvValidatepasses.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 checkpasses: 361 tests, 21 of them new.UpgradeApacheCommonsDbcp_1_2in Moderne, over the 307 repositories a search found using DBCP 1.x by dependency, type or bean definition:Of the 67 left alone, 66 have no
commons-dbcpin their build model. The other islinkedin/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:
What this does not show:
ddal-configfiles.apache/inlongfails 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:
isValid(), which is anAbstractMethodErroron HSQLDB 1.8 (datacleaner/DataCleaner).log4j-api: DBCP 2.14 bringscommons-logging1.3.5, which calls a method an oldlog4j-apidoes not have (hdiv/insecure-bank).jakarta.transaction-api1.3.3, which still holdsjavax.*classes. Being nearer in the tree it replaces the 2.x Hibernate needs, andjakarta.transaction.TransactionManagergoes missing (CloudSlang/score). Up to 10 of the 240 repositories use those Jakarta types.Also worth a look:
setMaxWaitbecomessetMaxWaitMillis, notsetMaxWait(Duration). It is in every 2.x release, but deprecated from 2.10, so expect deprecation warnings.BasicDataSourceFactorystill silently ignores the old keys where they are set in code, or in a properties file named outside of the class callingcreateDataSource. The recipe's description says so.alibaba/druid's shims are moved over its existingdbcp2shims, so that repository should be excluded. Sakai's classes move as well, and those were not built.mraible/ajax-loginandnmyphp/tbschedulehave 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:
PoolableConnectionFactoryto a commons-poolObjectPoolby hand needs a real port to commons-pool2:AlexeiStrug/EPAM,h-thurow/Simple-JNDIandKualiCo/rice. About 9 repositories in the organization do this, each differently.taskurotta/taskurotta, andwhite-cat/jeeweboutside of the sample.sample/pom.xmlinsogou-biztech/compass.