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

Jukito shouldn't inject optional injection points #61

Open
tbroyer opened this issue Nov 6, 2014 · 3 comments
Open

Jukito shouldn't inject optional injection points #61

tbroyer opened this issue Nov 6, 2014 · 3 comments
Assignees

Comments

@tbroyer
Copy link

tbroyer commented Nov 6, 2014

I'm using a third-party library that uses Guice (Closure Templates) and makes use of optional injection:

  @Inject(optional = true)
  void setCheckConformance(CheckConformance checkConformance) {
    this.checkConformance = checkConformance;
  }

Unfortunately, Jukito will inject a mock CheckConformance which will later cause issues because the contract for the interface is to never return a null, causing a NullPointerException when the mock breaks that contract:

    if (checkConformance != null) {
      ImmutableList<SoySyntaxException> violations = checkConformance.getViolations(soyTree);
      if (!violations.isEmpty()) {

Jukito shouldn't inject such injection points with mocks unless bind() or forceMock() has been called for the key.

The workaround in my case is to explicitly bind CheckConformance to a dummy instance (or configure the mock) to make sure it never return nulls.

@christiangoudreau
Copy link
Member

Thanks for this great issue Thomas!

One question to think about, what if, in the dependency chain, you have a CheckConformance that isn't optional? What would be the expected behaviour?

@tbroyer
Copy link
Author

tbroyer commented Nov 6, 2014

Ah indeed, if Jukito has to inject a mock somewhere in the graph, then it should inject one in the optional injection point I suppose. I'd say Jukito should follow whatever Guice does for JIT bindings.

If that's not possible, or too difficult, I could also live with a way to disable "auto mocking" for some classes (kind of the opposite of forceMock()).
In my case, the workaround was easy, but I'm sure there are times when it's not so simple. This would be particularly true when the optionally-injected classes are really internal (e.g. package private) to a third-party lib.

@christiangoudreau
Copy link
Member

Good point. Right now they only way to avoid auto-mocking is to specifically bind the abstraction to something.

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

3 participants