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

DelegatingDataSource incorrectly implements java.sql.Wrapper [SPR-9770] #14404

Closed
spring-projects-issues opened this issue Sep 6, 2012 · 6 comments
Assignees
Labels
in: data type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Sep 6, 2012

Juan M. Tula opened SPR-9770 and commented

Status Quo

Spring's DelegatingDataSource does not correctly implement the java.sql.Wrapper interface.

Analysis

According to the documentation for the unwrap() and isWrapper() methods, if the receiver implements the received interface, it should return itself, something like this:

public <T> T unwrap(Class<T> iface) throws SQLException {
	T result;
	if(iface.isInstance(this)) {
		result = iface.cast(this);
	} else {
		result = getTargetDataSource().unwrap(iface);
	}
	return result;
}

But the implementation is currently this:

public <T> T unwrap(Class<T> iface) throws SQLException {
	return getTargetDataSource().unwrap(iface);
}

So, a call to new DelegatingDataSource().unwrap(DelegatingDataSource.class) will fail instead of returning itself.


Affects: 3.1 GA

Reference URL: http://docs.oracle.com/javase/6/docs/api/java/sql/Wrapper.html?is-external=true

Issue Links:

  • #14489 AbstractRoutingDataSource does not allow to unwrap the underlying datasource

0 votes, 6 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 7, 2012

Phil Webb commented

#140

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 10, 2012

Juergen Hoeller commented

Good catch! AbstractDataSource and DelegatingDataSource consistently implement JDBC 4.0's Wrapper interface now.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 3, 2012

Hernan J. Gonzalez commented

Shouldn't we also look at AbstractRoutingDataSource.isWrapper()? I think it should override the parent, so that the isInstance matching is done, not with respect to the class itself (well, perhaps also with the class itself) but also (mainly) with the currently routed datasource.

(it might be debatable if the Wrapper interface behaviour can change for a given object in runtime, i'd say yes)

Case in point: I have a RoutingDataSource than can route to a org.apache.commons.dbcp.BasicDataSource pool or a plain jdbc datasource. I expected I can ask myRoutingDataSource.isWrapperFor(org.apache.commons.dbcp.BasicDataSource.class) to check if I'm using the pooled data source, and to retrieve the wrapped dbcp object. Currently, I can't.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 3, 2012

Sam Brannen commented

Hernan,

Could you please create a new JIRA issue to address your concerns regarding AbstractRoutingDataSource?

Thanks,

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 3, 2012

Hernan J. Gonzalez commented

Sam: done, I hope it's ok: https://jira.springsource.org/browse/DATAJDBC-35

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 4, 2012

Sam Brannen commented

Hernan,

Thanks for creating that JIRA issue.

Please note that I've moved it from the Spring Data JDBC project to the Spring Framework project where it belongs (#14489).

Sam

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

No branches or pull requests

2 participants