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

Overridden methods are injected twice #67

Closed
gissuebot opened this issue Jul 7, 2014 · 8 comments
Closed

Overridden methods are injected twice #67

gissuebot opened this issue Jul 7, 2014 · 8 comments

Comments

@gissuebot
Copy link

From crazyboblee on March 14, 2007 18:47:12

If you override a method with @Inject with another method with @Inject,
Guice will call it twice. This usually isn't a big deal, but we really
should filter out overridden methods.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=67

@gissuebot
Copy link
Author

From kevinb9n on March 15, 2007 07:47:27

bogus.

Labels: 1.1

@gissuebot
Copy link
Author

From robbie.vanbrabant on August 10, 2007 01:39:38

Take a look if this is what you meant. Patch attached.

Binary attachments: guice_overrides.zip

@gissuebot
Copy link
Author

From crazyboblee on September 09, 2007 14:07:36

Two things. 1) You can't override fields (you can only shadow them). Given:

class Super {
  @Inject int foo;
}

class Sub extends Super {
  @Inject int foo;
}

Super.foo and Sub.foo are separate fields and we should inject both.

  1. I think we can detect overridden methods a little more efficiently and accurately.
    I'd rather the "overridden" check be a single, simple hash lookup, not require
    iterating over all the methods in all the superclasses, i.e. O(1) for each method,
    not O(N*M).

There's also the question of what does overriding mean semantically? If I have
@Inject on a super class method, and I override the method in a subclass but don't
use @Inject, should we still inject it, or no?

Finally, detecting true overriding is a little trickier. You have to take the
accessibility of the method and whether or not the parent and child classes are in
the same package or not into account. For example, I could have a private method in
the parent and a public method with the same signature in the child, but we should
inject both separately. If a parent has a package-private method, a child in the same
package can override it, while if the child is in a separate package, the parent
class's method might as well be private (in which case we should inject both).

@gissuebot
Copy link
Author

From robbie.vanbrabant on September 09, 2007 16:12:10

You're right Bob.

  1. Stupid mistake
  2. (a) I'll take a second look once I get back from vacation.
       (b) @Inject(cascade=true) on the superclass? Should not be the default.
       (c) Correct.

@gissuebot
Copy link
Author

From robbie.vanbrabant on April 13, 2008 09:42:41

Found the time to look at this again. Attached.

Attachment: gist
   guice_overrides.diff

@gissuebot
Copy link
Author

From limpbizkit on May 29, 2008 23:53:49

(No comment was entered for this change.)

Labels: -1.1 Milestone-Release2.0

@gissuebot
Copy link
Author

From limpbizkit on October 31, 2008 23:42:49

(No comment was entered for this change.)

Labels: -Milestone-Release2.0 Milestone-Release2.1

@gissuebot
Copy link
Author

From crazyboblee on September 29, 2009 11:22:00

This is fixed. Modified Guice to have the same behavior as JSR-330. This shouldn't
break anyone (since you shouldn't have depended on injection ordering), but if it does
and the existing code is too difficult to fix, I can add a compatibility mode that uses the
old ordering. Please review: https://code.google.com/p/google- guice/source/browse/trunk/src/com/google/inject/spi/InjectionPoint.java

Status: Fixed

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

No branches or pull requests

1 participant