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

@FeignClient and manual Feign Client #1852

Closed
thorstenfrank opened this issue Apr 11, 2017 · 18 comments
Closed

@FeignClient and manual Feign Client #1852

thorstenfrank opened this issue Apr 11, 2017 · 18 comments
Labels

Comments

@thorstenfrank
Copy link

thorstenfrank commented Apr 11, 2017

Here's my scenario:

I have a library used by various Spring Boot applications. The library has a dependency to some ServiceProvider REST API. I can't simply create a @FeignClient inside my lib, because if the App that's using it also declares one to the same interface, it doesn't start up because of a NoUniqueBeanDefinitionException.

I help myself around this by using auto configuration in the lib like so:

@Configuration
@Import(FeignClientsConfiguration.class)
public class LibAutoConfig {

@Bean
@ConditionalOnMissingBean(ServiceProvider.class)
public ServiceProvider serviceProvider(Client client, Encoder encoder, Decoder decoder) {
	System.out.println("CREATING NEW SERVICE PROVIDER");
	return HystrixFeign.builder()
			.client(client)
			.encoder(encoder)
			.decoder(decoder)
			.contract(new SpringMvcContract())
			.target(ServiceProvider.class, "http://service-provider"); 
}

}

Which works, at the end of the day. BUT - I can see in the logs that the manually built client is always created, even if the host application supplies something like

@FeignClient(name = "service-provider")
public interface ProviderClient extends ServiceProvider {}

I can use the ServiceProvider from both inside the app and the library, I just don't know how this works... shouldn't the library be re-using the @FeignClient from the host?

Full example available here: https://github.com/thorstenfrank/multi-feign-clients

@ryanjbaxter
Copy link
Contributor

Maybe it is an order issue? The auto configuration may be running before we create the Feign clients.

@thorstenfrank
Copy link
Author

Possible, yes - but still, I have two feign clients (one created automatically, one manually) for the same endpoint. And I can autowire the endpoint interface anywhere throughout my application and it works - so how come there's no clash?

Like I said - I'm glad it works, just wanting to understand.

@thorstenfrank
Copy link
Author

The @FeignClient bean is created first. Even though the auto configuration bean method is annotated as @ConditionalOnMissingBean(type = ServiceProvider.class), when doing this inside the method body (with an autowired ApplicationContext):

ctx.getBeansOfType(ServiceProvider.class)

it returns the automatically created @FeignClient, to be more exact a Proxy with a HystrixInvocationHandler and the target is of type ProviderClient(which is the interface annotated with @FeignClient)

@ryanjbaxter
Copy link
Contributor

If you created the Feign client manually and return it as a bean in the application using the lib does it work as you expect?

@thorstenfrank
Copy link
Author

@ryanjbaxter if I create the Feign client manually, everything behaves as expected - the conditional in the library recognizes its existence and so does not create a second one.

Can you point me to the place in the Feign implementation where the annotated beans are created?

@spencergibb
Copy link
Member

spencergibb commented Apr 18, 2017

The bean definitions are created in FeignClientsRegistrar.

@thorstenfrank
Copy link
Author

thorstenfrank commented Apr 18, 2017

Well, I love a good puzzle, but I'm close to just letting this one rest... since it works, somehow.

The FeignClientsRegistrar definitely registers the @FeignClient before any of my custom code is run. Turning on Spring's debug logging, though, I can see that:

@ConditionalOnMissingBean (types: de.tfsw.spring.cloud.provider.ServiceProvider; 
SearchStrategy: all) found no beans (OnBeanCondition)

even though inside the method with the conditional, I can retrieve the very Feign client automatically created.

When creating the client manually through the Builder, Spring will discover it and not jump into the conditional method:

@ConditionalOnMissingBean (types: de.tfsw.spring.cloud.provider.ServiceProvider; 
SearchStrategy: all) found the following [providerClient] (OnBeanCondition)

My best guess right now is that this is more an issue within the Spring framework itself (bean factory), not so much the Spring Cloud Netflix Feign implementation.

If I have some time in the future, I might investigate a little further - use newer versions, try out comparable behaviour of other ImportBeanDefinitionRegistrar implementations.

@ryanjbaxter
Copy link
Contributor

@thorstenfrank can you try specifying an order on your auto configuration using @AutoconfigureOrder? I think that if we make sure your configuration runs before the spring cloud netflix configuration it might work.

@spencergibb
Copy link
Member

Not sure the order will help, since the registrar is @Imported from @EnableFeignClients.

@thorstenfrank
Copy link
Author

thorstenfrank commented Apr 19, 2017

No love, but it was worth a try - both lowest and highest precedence have no effect, the results are identical.

Also, not really sure that's what I would want, since I do want the Feign clients to be created first (if they exist), and to only have my auto configuration run if the host application does not already provide a valid Feign client.

As mentioned before, the puzzling thing to me isn't the registrar or the ordering, but the fact that the bean factory doesn't recognize the @FeignClient as a valid candidate for the @ConditionalOnMissingBean annotation. But I admit I'm no expert that deep into the Spring framework.

@spencergibb
Copy link
Member

I didn't think it would. We'd need to restructure @EnableFeignClients to trigger an auto-configuration, then the order might work.

@thorstenfrank
Copy link
Author

Just out of curiosity, I tried to place the @ConditionalOnMissingBean at the class level (i.e. on the auto configuration class in the library), but still had the same results.

I tried this because I had a similar situation at work today where the conditional only worked this way, not at the method level. But in that specific scenario, the bean the conditional was based on was also created by an auto configuration, not a registrar.

@spencergibb
Copy link
Member

Again, I think the problem is that the registrar is NOT created by auto configuration

@thorstenfrank
Copy link
Author

I agree, I just wanted to double-check. As mentioned, I don't understand the Spring loading mechanisms well enough to judge this effect...

@thorstenfrank
Copy link
Author

Is there a plan to have the Feign registrar trigger an auto configuration?

@spencergibb
Copy link
Member

Not currently

@spring-projects-issues
Copy link

Closing due to age of the question. If you would like us to look at this issue, please comment and we will look at re-opening the issue.

@thorstenfrank
Copy link
Author

Just a follow-up/work-around/solution:
the situation arises because the @ConditionalOnMissingBean is evaluated before the FeignClientsRegistrar actually creates the bean definitions for declarative clients. And the registrar doesn't check (or doesn't have access to?) said bean.
Strangely, using the primary attribute in the annotation has zero effect, so I think this whole issue is actually one of the Spring Framework itself, because apparently the bean creation through component scans and auto configuration seems to be in a parallel universe compared to the registrar.

The only solution around this specific issue is to only use one of the two approaches throughout: annotations (where the primary attribute then actually has an effect) or manual clients using Feign.Builder (or Hystrix, ...), but mix-and-match simply doesn't work.

We've decided to go with all annotations and marking the library clients as non-primary.

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

No branches or pull requests

4 participants