Existing Spring Tests are failling with dapr-sdk 1.6.0

[javageek79]

Expected Behavior

All existing tests still run with the new dapr-sdk 1.6.0 as they do with 1.5.0

Actual Behavior

Some tests in our test suite are failing due to

java.lang.IllegalStateException: Failed to load ApplicationContext
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'backChannelController' defined in file [.../BackChannelController.class]: Initialization of bean failed; nested exception is java.lang.RuntimeException: a default route is already set for topic ---.---.backchannel on pubsub messagebus
Caused by: java.lang.RuntimeException: a default route is already set for topic ---.---.backchannel on pubsub messagebus

Steps to Reproduce the Problem

Run two spring extended test classes testing a spring rest controller like

@RestController
@RequiredArgsConstructor
public class BackChannelController {
    private final BackChannelService backchannelService;

    @Topic(name = TOPIC_BACKCHANNEL, pubsubName = DEFAULT_PUBSUB_NAME)
    @PostMapping(path = {PATH_BACKCHANNEL})
    public Mono<DaprResponse> handleBackchannel(@RequestBody CloudEvent<BackChannelPayload> event) {
        // Business Logic
    }
// . . .
}

A test might looks like this

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@AutoConfigureMockMvc
@ContextConfiguration(classes = {
        WebConfig.class,
        MyApplication.class,
        // other classes
})
class BackChannelServiceTest {
}

the tests are running fine if test classes are executed standalone. If all tests are running, they are failing with the named error above.

Release Note

RELEASE NOTE: FIX Bug in java sdk.

RELEASE NOTE:

[DeepanshuA]

Was able to reproduce the issue:
Used multiple controllers where-in the Topic is same and both were trying to set some default path for this topic.

As per Dapr proto definition in runtime, default path can be only one as string: https://github.com/dapr/dapr/blob/81fc87deae04713a78416009060eb35e8ae3bf9c/dapr/proto/runtime/v1/appcallback.proto#L180

Following lines are invoked in java-sdk in such a case (thanks to @mukundansundar for helping me identify exact code-piece in java-sdk):

java-sdk/sdk-springboot/src/main/java/io/dapr/springboot/DaprSubscriptionBuilder.java

Line 49 in 5a2833c

if (defaultPath != null) {

@javageek79 Please confirm, if that is the scenario: in your test(s), you are trying to set default paths for the same topic via multiple tests. And, that is why when you run test separately, it should run fine.

Assuming that is the case - Ideally, it would be good, if we can avoid using same topic and setting default path for it via multiple tests - that doesn't seem like a prod scenario.

@artursouza @mukundansundar, One discussable stuff here: Should we allow updation of default path and not throw a RuntimeException. means, if somebody tries to really update it in real scenario?

[Gonzoe79]

Hi,

let me jump in for @javageek79 and answer your question, since i took over the bug in our project.

we are running multiple parallel springboot tests. when we configure surefire to forkmode=always (create a new jvm for each test) we don't get the runtime excception. running parallel springboot tests worked fine in 1.5.0 and is the default behaviour when buiulding spring applications with maven.

executing tests consecutively (surefire configuration forkMode=always) is solving the behaviour but almost doubles the build time (which is not a good option in the production pipeline).

looking at the code i wonder if it might be a solution by altering setDefaultPath like this:

 DaprSubscriptionBuilder setDefaultPath(String path) {
    
    if(Objects.equal(path, defaultPath) {
          return;
    }
    
    if (defaultPath != null) {
      throw new RuntimeException(
              String.format(
                      "a default route is already set for topic %s on pubsub %s",
                      this.topic, this.pubsubName));
    }
    defaultPath = path;
    return this;
  }

allowing setDefaultPath if it is the same path as before.

[artursouza]

We can make a change where it does not throw an exception if the defaultPath is already set to the same value.

[Gonzoe79]

thanks, i can confirm that the problem no longer exists with the new master version.
best regards