This repository has been archived by the owner. It is now read-only.

JERSEY-2171 jersey-spring: fix field injections on spring proxies #35

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants
@nikku

nikku commented Oct 27, 2013

  • Fields get properly unwrapped before the spring component provider
    performs hk2 injection on them
jersey-spring: fix field injections on spring proxies
* Fields get properly unwrapped before the spring component provider
  performs hk2 injection on them

Closes JERSEY-2171
@buildhive

This comment has been minimized.

Show comment
Hide comment
+ private void inject(Object bean) {
+ Object injectTarget = bean;
+
+ // correctly handle adviced (proxied) spring beans

This comment has been minimized.

@mpotociar

mpotociar Oct 30, 2013

Member

Fix comment typo - "advised"

@mpotociar

mpotociar Oct 30, 2013

Member

Fix comment typo - "advised"

+ try {
+ injectTarget = ((Advised) bean).getTargetSource().getTarget();
+ } catch (Exception e) {
+ throw new BeanCreationException("Failed to resolve injectTarget for " + bean, e);

This comment has been minimized.

@mpotociar

mpotociar Oct 30, 2013

Member

This needs to be localized. (See localization.properties in the module's resources)

@mpotociar

mpotociar Oct 30, 2013

Member

This needs to be localized. (See localization.properties in the module's resources)

This comment has been minimized.

@mpotociar

mpotociar Oct 30, 2013

Member

Also, it is not clear to me, why is HK2 factory throwing a Spring exception?

@mpotociar

mpotociar Oct 30, 2013

Member

Also, it is not clear to me, why is HK2 factory throwing a Spring exception?

@mpotociar

This comment has been minimized.

Show comment
Hide comment
@mpotociar

mpotociar Oct 30, 2013

Member

This pull requests claims to fix an issue. In order to accept the fix, we need to make sure that it really fixes the issue. This is why we require a reproducer unit test to accompany any new bug fix commit (external or internal).

Please provide the reproducer unit test. One potentially suitable location for the test would be to add a new test module into Jersey integration tests.

Member

mpotociar commented Oct 30, 2013

This pull requests claims to fix an issue. In order to accept the fix, we need to make sure that it really fixes the issue. This is why we require a reproducer unit test to accompany any new bug fix commit (external or internal).

Please provide the reproducer unit test. One potentially suitable location for the test would be to add a new test module into Jersey integration tests.

@nikku

This comment has been minimized.

Show comment
Hide comment
@nikku

nikku Oct 30, 2013

Hi @mpotociar. Thanks for pointing me to your contribution guide. I will update my pull request accordingly.

nikku commented Oct 30, 2013

Hi @mpotociar. Thanks for pointing me to your contribution guide. I will update my pull request accordingly.

@shamoh

This comment has been minimized.

Show comment
Hide comment
@shamoh

shamoh Nov 13, 2013

Member

@nikku In order to be able to accept your contribution, we need you to sign Oracle Contributor Agreement (OCA). You can find more details here:
https://jersey.java.net/scm.html#/Submitting_Patches_and_Contribute_Code
http://www.oracle.com/technetwork/community/oca-486395.html

Member

shamoh commented Nov 13, 2013

@nikku In order to be able to accept your contribution, we need you to sign Oracle Contributor Agreement (OCA). You can find more details here:
https://jersey.java.net/scm.html#/Submitting_Patches_and_Contribute_Code
http://www.oracle.com/technetwork/community/oca-486395.html

@nikku

This comment has been minimized.

Show comment
Hide comment
@nikku

nikku Nov 13, 2013

@shamoh: Thanks for the hint. I'll get the pull request acceptance ready first.

nikku commented Nov 13, 2013

@shamoh: Thanks for the hint. I'll get the pull request acceptance ready first.

@mpotociar

This comment has been minimized.

Show comment
Hide comment
@mpotociar

mpotociar Feb 8, 2014

Member

Hi @nikku, do you plan to update the pull request? Unless I hear from you within next few days, I'm going to close this pull request.

Member

mpotociar commented Feb 8, 2014

Hi @nikku, do you plan to update the pull request? Unless I hear from you within next few days, I'm going to close this pull request.

@nikku

This comment has been minimized.

Show comment
Hide comment
@nikku

nikku Feb 8, 2014

Hi @mpotociar, unfortunately I have no time to work on this pull request at the moment. Feel free to close it due to inactivity. I may file a new one some time.

nikku commented Feb 8, 2014

Hi @mpotociar, unfortunately I have no time to work on this pull request at the moment. Feel free to close it due to inactivity. I may file a new one some time.

@mpotociar

This comment has been minimized.

Show comment
Hide comment
@mpotociar

mpotociar Feb 8, 2014

Member

Fair enough :)

Please feel free to file a new, update pull request as your time allows.

Member

mpotociar commented Feb 8, 2014

Fair enough :)

Please feel free to file a new, update pull request as your time allows.

@mpotociar mpotociar closed this Feb 8, 2014

@trophies

This comment has been minimized.

Show comment
Hide comment
@trophies

trophies Jul 24, 2015

I ran into a similar issue as described in https://java.net/jira/browse/JERSEY-2171, I tested out this change and it resolves my issue (I filed a separate bug for the specific issue I had though - https://java.net/jira/browse/JERSEY-2919).

Is there any chance of this fix being included in a future jersey-spring3 release? I attempted to write a test case for it, but the tests passed even without this fix in place.

I ran into a similar issue as described in https://java.net/jira/browse/JERSEY-2171, I tested out this change and it resolves my issue (I filed a separate bug for the specific issue I had though - https://java.net/jira/browse/JERSEY-2919).

Is there any chance of this fix being included in a future jersey-spring3 release? I attempted to write a test case for it, but the tests passed even without this fix in place.

@bric3

This comment has been minimized.

Show comment
Hide comment
@bric3

bric3 Nov 22, 2016

For those interested a pull request got merged that partly fix the problem :
in #226 , relevant ticket https://java.net/jira/browse/JERSEY-3126

i.e. if you register an bean instance then context injection won't happen, the patched code is not even called.

bric3 commented Nov 22, 2016

For those interested a pull request got merged that partly fix the problem :
in #226 , relevant ticket https://java.net/jira/browse/JERSEY-3126

i.e. if you register an bean instance then context injection won't happen, the patched code is not even called.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.