Clarify search order of Encoders / Decoders

[joakime]

As outlined in #39 (comment) ...

Danny Coward states ...

then try to find a Decoder.Text or a Decoder.TextStream. Look, in order, through developer provided ones (single list, may contain both Text and TextStream types). Then look through platform provided ones. Use the first match to decode it. If the decode works, call the MessageHandler and stop, if not, call the onError/@websocket method with the decode exception and stop.

What is the search order when the "developer provided ones" comes from 2 places (annotations and configuration)?

@ServerEndpoint(
    value = "/purchase",
    decoders = {AppleDecoder.class, PearDecoder.class, FruitDecoder.class},
    configurator = FruitStandConfigurator.class
)
public class PurchaseEndpoint
{
}

and in the container init ...

        ServerContainer container = servletContext.getAttribuet(javax.websocket.server.ServerContainer.class.getName());
        ServerEndpointConfig config = ServerEndpointConfig.Builder.create(PurchaseEndpoint.class, "/floo")
                .decoders(Arrays.asList(CherryDecoder.class, BananaDecoder.class)).build();
        container.addEndpoint(config);

What is the resulting search order of decoders?

Option 1: (EndpointConfig wins over Annotations)

1. CherryDecoder.class
2. BananaDecoder.class
3. AppleDecoder.class
4. PearDecoder.class
5. FruitDecoder.class
6. Platform Provided Decoders

Option 2: (Annotation wins over EndpointConfig)

1. AppleDecoder.class
2. PearDecoder.class
3. FruitDecoder.class
4. CherryDecoder.class
5. BananaDecoder.class
6. Platform Provided Decoders

(Does the order of the Platform Decoders matter? I don't think so)

[joakime]

Then there is the issue with merging decoders in common between the annotations and configuration.

It could be argued that just like @ServerEndpoint(value="/echo") is overridden if using ServerEndpointConfig.getPath(), why wouldn't the decoders/encoders from the config also override/replace the ones from the annotation.

The javadoc is unclear on this behavior.
And the TCK doesn't seem to have a test for this scenario as well.

[markt-asf]

Clarification is (potentially) a change in behaviour so this will need to wait for Jakarta EE 10. As a note to self I don't think the API will need to change (but haven't thought about this much) but don't forget to update the spec document and the Javadoc.

[joakime]

I don't think the API will need to change (but haven't thought about this much)

I agree. This seems like a need for updated documentation and/or javadoc, along with some new TCK tests.

[volosied]

To me, it makes sense for the "/purchase" endpoint to use "AppleDecoder.class, PearDecoder.class, FruitDecoder.class" while the "/floo" endpoint to use "CherryDecoder.class, BananaDecoder.class". Shouldn't each URI path be mapped to a particular config?

However, I'm not familiar with this scenario: "could be argued that just like @serverendpoint(value="/echo") is overridden if using ServerEndpointConfig.getPath()". @joakime Could you provide an example? Overriding encoders / decoders could make sense.

[joakime]

@volosied In the original example ...

ServerEndpointConfig.Builder.create(PurchaseEndpoint.class, "/floo")

The PurchaseEndpoint.class has an annotation defining the the decoders.
Then when you use the ServerEndpointConfig.Builder.create(class, ....) you have the ability to change the values coming from the annotation.

Since the annotation doesn't support multiple paths, or having multiple annotations, the end user has to create the config manually to add.
The annotation entries for decoders/encoders/subprotocols/extensions/configurator are viewed as requirements for the class to function.

I wish we had either defined the annotation in a way that multiple configurations could be defined (use annotations only), or that it only applied to 1 configuration (never mix with manual endpoint config).
But we didn't, and this scenario (using annotated endpoint in ServerEndpointConfig.Builder) is now quite common, so we cannot fix it.

Frankly, the "platform" decoders / encoders should have also been mandatory to declare in your decoders/encoders to use them (as their behaviors have never been fully defined)
OR, in my opinion, the ENTIRE decoders / encoders concept shouldn't even be here on the websocket spec/api (that's a higher level concern)

[volosied]

I see. Thanks for the explanation and the background information.

I'm not sure what the official proposal is, but It makes sense to use the Annotated Endpoints values for the encoders and decoders by default as you suggested before. If any encoders / decoders are set via the ServerEndpointConfig.Builder then they should override the ones set via annotation.

Since you mentioned the subprotocols, extensions, and configurator. Should this overriding process be extended to them, too?

[markt-asf]

How about this then:

• EndpointConfig Encoders/Decoders are in addition to those configured via Annotation
• EndpointConfig Encoders/Decoders take priority over those configured via Annotation
• Annotation Encoders/Decoders take priority over the platform defaults

Same rules for sub-protocols, extensions and configurators (except there is only a single configurator)

Is deprecating everything encode and decoder related a step too far?

[markt-asf]

See also #213 and make sure any fix for this issue also addresses the problems described in #213