Assembly validation

[cescoffier]

Implementation must check the validity of the graph at startup time.

This PR:

• add a section in the spec about that
• add TCK tests verifying that the implementation validates the graph

Because of that, the BeanMissingChannel test should fail at startup as there is no downstream channel for the emitter. It's a definition exception (equivalent to injecting a missing bean).

The ChannelInjectionDifferentFlavourSameChannelTest is invalid as a single upstream cannot, by default, emit values to multiple (6 in the case of the tests) downstream. Implementations could support such behavior, but the TCK should not assume that this mechanism exists or is the default behavior. This is now checked by the TCK

[Azquelt]

On the BeanMissingChannel, the spec currently says

An IllegalStateException will be thrown if no consumer is found by the time a message is emitted to the channel.

I don't object to changing the behaviour, but we need to change the spec as well.

[cescoffier]

Oh, I missed that. In a regular CDI application, it's a definition exception. Do you remember why we choose a different path?

[Azquelt]

No. However, I also don't see anywhere in the spec where we require an exception to be thrown if there is an @Incoming("channel") method with no corresponding @Outgoing("channel") method or configured connector.

We do require an exception to be thrown if a method has an invalid shape for its annotations, or if it has an invalid acknowledgement policy, but not if it's just not connected to anything.

[Emily-Jiang]

Oh, I missed that. In a regular CDI application, it's a definition exception. Do you remember why we choose a different path?

I think since the annotations are only valid on CDI beans. DefinitationException is ok, since it is a configuration error made by a developer. IIRC, the IllegalStateException might be defined in SmallRye config. Anyway, in the test, eventually it will be all wrapped under DeploymentException.

[cescoffier]

@Emily-Jiang do you suggest rewriting the test to catch a DeploymentException, easier during the container creation or during the attempt to send the message?

[Azquelt]

It would be odd if having a disconnected Emitter caused a deployment exception, but a disconnected @Incoming or @Outgoing method did not.

[cescoffier]

I would say it should always fail, as it's a user issue. Not failing would produce applications not doing anything, without any hint.

[Azquelt]

I agree with you on ChannelInjectionDifferentFlavourSameChannelTest, there's nothing that says you should be able to inject a Publisher with @Channel in more than one place or what should happen if you do.

However, we might want to consider what should happen if you inject it into a RequestScoped bean. Do you get the same Publisher injected into each bean instance?

[Azquelt]

I would say it should always fail, as it's a user issue. Not failing would produce applications not doing anything, without any hint.

I think I agree, though I remember some discussion about wanting to allow classes with @Incoming and @Outgoing annotations to be left in the application even if they were unused.

On the one hand, it might be helpful to get an error pointing out that they're not connected to anything but on the other, during development it means that you have to actually remove the annotations if you want to deploy the application with those elements disabled.

Either way, if the TCK asserts one behaviour or the other, it needs to be clear in the spec which behaviour is expected.

[cescoffier]

Any idea what CDI does in this case? We should have the same behavior.

[Emily-Jiang]

@Emily-Jiang do you suggest rewriting the test to catch a DeploymentException, easier during the container creation or during the attempt to send the message?

I think it will be a better user experience the error thrown during the container creation. This is what DeploymentException mandates.

[cescoffier]

let me update this PR with that change (tomorrow). I may add at least one test in the TCK.

[cescoffier]

@Azquelt @Emily-Jiang Spec and TCK tests updates (lots of new tests!)

[cescoffier]

(restarting CI - Maven Central issue)

[Emily-Jiang]

@cescoffier I like the tests you added to test the misconfiguration. Can you put the description in this PR to an issue and then it will make it easier to produce a release note? Also, can you also clarify in the spec on multiple channel, many upstream, downstreams, etc?

[cescoffier]

@Emily-Jiang Done.