Spring-cloud-contract: AutoConfigureStubRunner doesn't work with Ribbon listOfServers configuration

[jrobison153]

Hi,

I'm having an issue with stub runner since upgrading from Edgware to Finchley. It appears as if the enabling AutoConfigureStubRunner clears out any other Ribbon configuration, attempts to use my Feign client results in the following error:

Load balancer does not have available server for client: cool-service

Here's how to reproduce the issue

mock profile for configuring ribbon for use by a feign client

cool-service:
  ribbon:
    NIWSServerListClassName: com.netflix.loadbalancer.ConfigurationBasedServerList
    listOfServers: localhost:${local.server.port}

stubrunner:
  idsToServiceIds:
    nifty-spec: "nifty-service"
  ids: "com.foo:nifty-spec"

eureka:
  client:
    enabled: true
    register-with-eureka: false
    fetch-registry: false

feign:
  hystrix:
    enabled: false

Test code

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles({ "mock" })
@EnableFeignClients(clients = { GreetingApiClient.class })
@DirtiesContext
@AutoConfigureStubRunner
@RunWith(SpringRunner.class)
public class StubRunnerIssue {

    @Autowired
    private CoolApiClient client;

    @Test
    public void stubRunnerIssueExample() {

        this.client.getCool();
    }
}

Feign client and service interface/implementation

@FeignClient(name="cool-service", configuration = CoolClientConfiguration.class)
public interface CoolServiceApiClient extends CoolApi {
}

public interface CoolApi {

    @RequestMapping(value = "/cool",
        produces = "application/json",
        consumes = "",
        method = RequestMethod.GET)
    ResponseEntity<CoolResponse> getCool();

}

@RestController
public class CoolController implements CoolApi {

    @Autowired
    private NiftyClient niftyClient;

    @Override
    public ResponseEntity<CoolResponse> getCool() {

        try {

            // call service that should be stubbed by stub runner
            ResponseEntity<NiftyResponse> niftyResponse = this.niftyClient.getSomethingNifty();
        }
        catch (NullPointerException e) {
            throw e;
        }
        return ResponseEntity.ok();
    }
}

In the unit test code, if you remove @AutoConfigureStubRunner then ribbon is able to find the configured server and the rest service is called, the test will eventually fail because the service then tries to call the client that was supposed to be stubbed out by the stub runner, but we disabled it.

Please note that this configuration worked in the Edgware release train, but since upgrading to Finchley.RELEASE this problem presents.

[marcingrzejszczak]

Your cool-service is not mapped to ant stub id. Your ids mapping is invalid

[jrobison153]

That's intentional, cool service is not to be stubbed by stub runner. I want to consume that service with my feign client. However the ribbon config is wiped out when AutoConfigureStubRunner is enabled

[marcingrzejszczak]

I don't understand what you're trying to achieve. So don't use stub runner for this test.

I remember the issue that we fixed which was related to not delegating to ribbon cause it doesn't make any sense really.

You should have separate tests that test the integration of contract related stuff separately to other things.

[jrobison153]

These are acceptance tests that run against virtualized dependencies. The example here is overly simplified, but the gist is that in many circumstances the stubs generated by Spring Cloud Contract work well for this purpose.

Regardless of the test strategy being employed, the question is should I be able to use Stub Runner and Ribbon at the same time? It looks like we cannot and it sounds like you are saying it was intentionally 'fixed'.

[marcingrzejszczak]

Yes it was fixed.

You can run stub runner in a mode that disables the service discovery stubbing. It will try to register the stubs in a service discovery tool.

[jrobison153]

I've tried every permutation of configuration options that I can find, none of them enable the use of StubRunner with pre-existing Ribbon configuration.

It looks like the StubRunnerRibbonBeanPostProcessor is overriding the ConfigurationBasedServerList bean with the StubRunnerRibbonServerList bean.

class StubRunnerRibbonBeanPostProcessor implements BeanPostProcessor {
...
    @Override
    public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
        if (bean instanceof ServerList && !(bean instanceof StubRunnerRibbonServerList)) {
            return new StubRunnerRibbonServerList(stubFinder(), stubMapperProperties(), clientConfig());
        }
        return bean;
    }
...
}

Would an acceptable enhancement be to merge the Servers from ConfigurationBasedServerList (or whatever other implementation of ServerList<Server> comes in) into the StubRunnerRibbonServerList? This could be made configurable, default behavior is how it works today, optionally turn on the 'merging' similar to what stubrunner.cloud.delegate.enabled=true would do for discovery clients.

[jrobison153]

Actually, that looks like how it used to work in 1.2.x release. Removing the delegate must be the fix you referred to.

Bean post processing code from the 1.2.5.RELEASE tag

@Override
    public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
        if (bean instanceof ServerList && !(bean instanceof StubRunnerRibbonServerList)) {
            return new StubRunnerRibbonServerList(stubFinder(), stubMapperProperties(), clientConfig(), (ServerList<?>) bean);
        }
        return bean;
    }

EDIT: Support was officially removed in 1.2.2.RELEASE which corresponds to the Edgware.SR1 release.

[jrobison153]

If this is the expected behavior then this issue can be closed. I would like to make the final argument that this behavior limits the usage of SCC, specifically stub runner, when the consumer is writing integration tests. If my consuming service has non-Spring Cloud Contract integration points (services that do not have contracts/stubs) and we are using Ribbon in any capacity to integrate with them then using the stub runner is not possible. This is a very real scenario, many services we build must integrate with legacy services as well as nice new services that use SCC.

For anyone else who has this issue a work around would be to remove use of stub runner and replace it with programatic configuration of wiremock from your test code. You should be able to load the SCC wiremock stubs from the classpath.

[marcingrzejszczak]

Let's continue this conversation :)

I think that you're trying to fix a wrong problem.

If you have onion layered architecture, you can test the contract pieces in one test (with spring cloud contract), and in other tests you can create a mock that will replace the client calling the endpoints that you tested with contract tests. WDYT?

[jrobison153]

Yes, I'd started a similar conversation with the team. Thinking about this more it seems like we may be stretching the purpose/intent of stub runner by using it in our more advanced behavioral testing where we need to virtualize/mock multiple external dependencies (the outer layer of onion). What you are suggesting lines up with some of our other test strategy conversations.

[marcingrzejszczak]

I think that it's better to make things as simple as possible. Two different sets of test for different purposes.

I'll close the issue for now but feel free to post any comments & ideas here.