Skip to content

Find DataSourceProxy even when it is wrapped or proxied #15206

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

Closed
wants to merge 1 commit into from

Conversation

ttddyy
Copy link
Contributor

@ttddyy ttddyy commented Nov 18, 2018

In DataSourceJmxConfiguration with tomcat DataSource, it can only find DataSourceProxy if given DataSource is a direct child of it.

The check logic uses instanceof. When target DataSource is wrapped(delegated) or proxied, it cannot find DataSourceProxy.

Since DataSourceProxy#unwrap() always returns null, here cannot use this method to directly obtain DataSourceProxy.

In this PR, updated the check logic to perform the best effort to retrieve DataSourceProxy. If given DataSource is wrapped or proxied by spring, tries to unwrap or get target datasource recursively.

Prior to this commit, `DataSourceJmxConfiguration` with tomcat
`DataSource`, it can only find `DataSourceProxy` if the given
`DataSource` is a direct child of it.  Since it uses `instanceof`, it
could not find `DataSourceProxy` if the `DataSource` is
wrapped(delegated) or proxied.

This is because `DataSourceProxy#unwrap()` always returns null; thus
cannot use this method to directly obtain `DataSourceProxy`.

In this commit, updated the check logic to perform the best effort to
retrieve `DataSourceProxy`. If given `DataSource` is wrapped or proxied
by spring, tries to unwrap or get target datasource recursively to find
`DataSourceProxy`.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 18, 2018
@philwebb philwebb added type: bug A general bug for: team-attention An issue we'd like other members of the team to review and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 19, 2018
@philwebb philwebb added this to the 2.0.x milestone Nov 19, 2018
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Nov 21, 2018
@snicoll snicoll self-assigned this Nov 22, 2018
snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Nov 23, 2018
Prior to this commit, `DataSourceJmxConfiguration` with tomcat
`DataSource`, it can only find `DataSourceProxy` if the given
`DataSource` is a direct child of it.  Since it uses `instanceof`, it
could not find `DataSourceProxy` if the `DataSource` is
wrapped(delegated) or proxied.

This is because `DataSourceProxy#unwrap()` always returns null; thus
cannot use this method to directly obtain `DataSourceProxy`.

In this commit, updated the check logic to perform the best effort to
retrieve `DataSourceProxy`. If given `DataSource` is wrapped or proxied
by spring, tries to unwrap or get target datasource recursively to find
`DataSourceProxy`.

See spring-projectsgh-15206
snicoll added a commit to snicoll/spring-boot that referenced this pull request Nov 23, 2018
snicoll pushed a commit that referenced this pull request Nov 26, 2018
Prior to this commit, `DataSourceJmxConfiguration` with tomcat
`DataSource`, it can only find `DataSourceProxy` if the given
`DataSource` is a direct child of it.  Since it uses `instanceof`, it
could not find `DataSourceProxy` if the `DataSource` is
wrapped(delegated) or proxied.

This is because `DataSourceProxy#unwrap()` always returns null; thus
cannot use this method to directly obtain `DataSourceProxy`.

In this commit, updated the check logic to perform the best effort to
retrieve `DataSourceProxy`. If given `DataSource` is wrapped or proxied
by spring, tries to unwrap or get target datasource recursively to find
`DataSourceProxy`.

See gh-15206
snicoll added a commit that referenced this pull request Nov 26, 2018
* pr/15206:
  Polish "Perform best effort to retrieve DataSourceProxy"
  Perform best effort to retrieve DataSourceProxy
@snicoll snicoll closed this in e424dfb Nov 26, 2018
@snicoll snicoll modified the milestones: 2.0.x, 2.0.7 Nov 26, 2018
@snicoll
Copy link
Member

snicoll commented Nov 26, 2018

Thanks for the PR @ttddyy. I've merged it with a polish commit.

@ttddyy ttddyy deleted the tomcat-datasource-jmx branch January 3, 2019 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants