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

AbstractBuilder doesn't provide ability to add alternatives as well as beans #22

Closed
goodidea-kp opened this issue Dec 19, 2017 · 9 comments · Fixed by #23
Closed

AbstractBuilder doesn't provide ability to add alternatives as well as beans #22

goodidea-kp opened this issue Dec 19, 2017 · 9 comments · Fixed by #23
Milestone

Comments

@goodidea-kp
Copy link

I suggest to add this method:
public T addAlternatives(Class<?>... clz) { for(Class cl:clz){ weld.addAlternative(cl); } return self(); }

@manovotn
Copy link
Collaborator

Hello,

such method is already present on org.jboss.weld.environment.se.Weld, therefore accessible while you bootstrap container by providing Weld object. The method is called alternatives(), e.g.:

@WeldSetup
public WeldInitiator weld = WeldInitiator.of(WeldInitiator.createWeld().alternatives(...).beanClasses(...));

Is this what you were looking for?

@goodidea-kp
Copy link
Author

goodidea-kp commented Dec 20, 2017

It is close. In your case combination is : ALTERNATIVES + CLASSES.
I need: ALTERNATIVES + BEANS (MOCKS)
How to do that without a change in Builder?
Example:
weld = WeldInitiator.of(WeldInitiator.from(MyServiceA.class).addBeans(MockBean.of(Mockito.mock(MyService.class), MyService.class)).addAlternatives(MyServiceA.class).build());

@mkouba
Copy link
Member

mkouba commented Dec 20, 2017

@goodidea-kp Well, Matej is correct that Weld.alternatives() does exactly what you suggest in the description, .i.e. selects the given alternatives for the synthetic bean archive.

Do you want to add and select an alternative mock bean using AbstractBuilder.addBeans(Bean<?>...)? If so, there are several solutions (none of them is optimal):

  • if using CDI 2/Weld 3 the Bean could implement javax.enterprise.inject.spi.Prioritized and then it's selected globally, for the application
  • if using org.jboss.weld.junit.MockBean the synthetic bean archive must have org.jboss.weld.junit.WeldCDIExtension alternative class selected

We should improve this and allow to specify the bean class in MockBean.Builder because currently all mock beans produced by MockBean return org.jboss.weld.junit.WeldCDIExtension in javax.enterprise.inject.spi.Bean.getBeanClass().

@manovotn
Copy link
Collaborator

I am not sure I fully grasp what you are after - whether the question is how to compose WeldInitiator to include mocked beans and alternatives at the same time, or whether it is about the alternatives not being selected?
What Martin says is right, there will be difficulties activating such mock bean alternatives ATM; we need to improve that.
If it's more about how to compose WeldInitiator in this case, then it could look something like this:
public WeldInitiator weld = WeldInitiator.from(WeldInitiator.createWeld().beanClasses(MyServiceA.class).alternatives(MyServiceA.class)).addBeans(MockBean.of(...)).build();

@goodidea-kp
Copy link
Author

@manovotn your proposed solution is what I am looking for.
public WeldInitiator weld = WeldInitiator.from(WeldInitiator.createWeld().beanClasses(MyServiceA.class).alternatives(MyServiceA.class)).addBeans(MockBean.of(...)).build(); Should work for me. Problem is not alternative are not selected. The problem I want to see more convenient builder use. See title of this ticket "AbstractBuilder doesn't provide an ability to add alternatives as well as beans". As far as I understood "Builder" pattern it is a pure responsibility of any builder. After build() you should be able to get fully ready to use an object without additional "massaging". The issue I claimed is a poor set of builder functionality for some uncovered combination. Make sense?

mkouba added a commit to mkouba/weld-junit that referenced this issue Dec 20, 2017
- also add convenient MockBean.Builder.selectedAlternative()
- resolves weld#22
@mkouba
Copy link
Member

mkouba commented Dec 20, 2017

@goodidea-kp The problem is we're working with several builders at once: org.jboss.weld.environment.se.Weld, org.jboss.weld.junit4.WeldInitiator.Builder, org.jboss.weld.junit.MockBean.Builder, etc. And it's not always easy to find the best combination to fit all use cases.

In any case, this proposal adds a convenient MockBean.Builder.selectedAlternative() method:
https://github.com/weld/weld-junit/pull/23/files#diff-24125e473c00c04d0b0e118085160441R36

@manovotn
Copy link
Collaborator

@goodidea-kp Yes, makes sense, but we also need to avoid duplicating code and we are basically making a builder (WeldInitiator.Builder) on top of a builder (Weld) which can possibly use optional builder (MockBean.Builder)... So this was the best we could come up with as first version, we should give it some more thoughts though.

@goodidea-kp
Copy link
Author

agreed.
I took a snapshot with a change..it works well. Thanks! Great job and quick.
I can't take most recent from maven repo though. How much time usually it takes to be propagated?

@manovotn
Copy link
Collaborator

manovotn commented Dec 20, 2017

Snapshot builds aren't automatically pushed to maven central, you will have to stick to your own build or wait for next release (which isn't yet scheduled).

@mkouba mkouba added this to the 1.2.1 milestone Dec 21, 2017
mkouba added a commit to mkouba/weld-junit that referenced this issue Dec 21, 2017
- also add convenient MockBean.Builder.selectedAlternative()
- resolves weld#22
mkouba added a commit to mkouba/weld-junit that referenced this issue Dec 22, 2017
- also add convenient MockBean.Builder.selectedAlternative()
- resolves weld#22
@mkouba mkouba closed this as completed in #23 Jan 3, 2018
mkouba added a commit that referenced this issue Jan 3, 2018
- also add convenient MockBean.Builder.selectedAlternative()
- resolves #22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants