websocket
websocket copied to clipboard
Define that user provided encoders and decoders override the platform provided ones (e.g. for Integer-> String conversions)
Otherwise, what would be the point ?
For encoders, the javadoc on the RemoteEndpoint send operations does define the override for primitive types, but doesn't mention String-> String encoders which should be allowed.
For decoders, there isn't anything, so currently its undefined.
Affected Versions
[1.0]
- Issue Imported From: https://github.com/javaee/websocket-spec/issues/213
- Original Issue Raised By:@glassfishrobot
- Original Issue Assigned To: @pavelbucek
@glassfishrobot Commented Reported by dannycoward
@glassfishrobot Commented This issue was imported from java.net JIRA WEBSOCKET_SPEC-213
Otherwise, what would be the point ?
Indeed. I think this is sufficiently obvious that we can clarify it in 2.0. I'll work on some updates.
Ah. As I look into this it gets more complex. I'm using String
throughout here but the issue also includes Reader
, ByteBuffer
, InputStream
and any other type explicitly mentioned when discussing message handling.
First of all Encoder
s. These are used with sendObject()
but they are also implicitly used with the return value from a MessageHandler
or a method annotated with @OnMessage
.
The sendObject()
case is relatively simple. The only container provided encoders are the Java primitives and equivalents and updating the Javadoc is simple enough for this case. String
is just another object in this case so if there is an Encoder
it will be used and if there isn't there will be an error. I have a Javadoc update for this that I'll submit a PR for shortly.
Return values are less obvious. If the method returns a String
and there is an Encoder
registered for String
should sendObject()
be used with the Encoder
or should sendText()
be used?
Similar questions exist for Decoder
s. Does MessageHandler.Whole<String>
use a Encoder
registered for String
or not? And essentially the same question when considering methods annotated with @OnMessage
.
The final part of the puzzle is the search order of annotations and configurations. See #289. I mention that here for completeness but I think it can be handled in that issue.
For this issue I am currently thinking that a registered Encoder always takes priority when determining how to handle a message. This allows developers to override the default container provided handling for any primitive or object. My thinking is that this is the logical extension of what we already have for sendObject()
. Thoughts?
Whatever we decide for this will have implications for the specification document, Javadoc and the TCK so this is definitely a Jakarta EE 10 issue.
I really wish this spec never had Encoders and Decoders.
I've always interpreted the spec as the implementation always having primitive encoders as default implementations.
-
java.lang.Boolean
(boolean
) as TEXT -
java.lang.Byte
(byte
) as TEXT -
java.lang.Character
(char
) as TEXT -
java.lang.Double
(double
) as TEXT -
java.lang.Float
(float
) as TEXT -
java.lang.Integer
(int
) as TEXT -
java.lang.Long
(long
) as TEXT -
java.lang.Short
(short
) as TEXT -
java.lang.String
(String
) as TEXT -
java.nio.ByteBuffer
as BINARY -
byte[]
as BINARY
Then when a Endpoint is registered, the declared Encoders replace the previous set for the same type.
So if they had a MyStringEncoder extends Encoder<String>
then that would be the new String
encoder for all Object
based actions.
The use of sendObject()
and the return types from MessageHandler
and the return types from methods annotated with @OnMessage
are always Object
based actions, never using sendBinary()
or sendText()
.
In fact, wouldn't it be swell to just have each of the primitive encoders and decoders declared in the websocket API and ask the implementations to use them?
That way the other vague pieces about supporting primitives could be standardized as well (such as how many decimal places do we support in double or float, what does a "byte" look like in text form? what about out of bands usage for that format?)
I like this idea. It is simple and clear.
Can someone elaborate on parts of this issue post as I don't quite follow it.
Then when a Endpoint is registered, the declared Encoders replace the previous set for the same type.
So if they had a MyStringEncoder extends Encoder<String> then that would be the new String encoder for all Object based actions.
How is this different from the new current spec? The primitive would be autoboxed via the sendObject(Object data) call, so wouldn't the custom encoder be picked first?
- In the original issue text. when could a
Integer -> String
or aString -> String
encoder be needed? Isn't thetoString
called on the Object before it converted to byte data?
Once again, wouldn't the custom encoder be used first (assuming the class type is applicable) before the platform encoder, per spec?
The 2.1 javadoc for sendObject
states that: A developer-provided encoder for a Java primitive type and its object equivalent overrides the container default encoder.
As for the other comments:
The use of sendObject() and the return types from MessageHandler and the return types from methods annotated with @OnMessage are always Object based actions, never using sendBinary() or sendText().
I also agree here.
Thanks!
-
Should this be titled Define that user provided encoders and decoders for primitive types override the platform provided ones
-
Testing on Open Liberty and Tomcat, I saw that custom primitives encoders are handled before platform encoders, but platform decoders for primitives are handled first. Custom / Platform decoders should be switched around?
Tomcat Decoder: https://github.com/apache/tomcat/blob/a1cc4e94a17589f2edb28c4d6807fb28252130e9/java/org/apache/tomcat/websocket/pojo/PojoMessageHandlerWholeText.java#L93-L109
Tomcat Encoder: https://github.com/apache/tomcat/blob/a1cc4e94a17589f2edb28c4d6807fb28252130e9/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java#L604-L614
Have you filed this as a bug at Tomcat?
I have not. Before I do, let me ask @markt-asf. Would this be considered a bug -- looks like it's been this way for 10 years?
Looked at this more, and I don't see decoders registered for primitives or Strings (decoders only added when nothing else has a match) https://github.com/apache/tomcat/blob/f4fb5a923d061d45794dfa64eec91af213c77c8d/java/org/apache/tomcat/websocket/pojo/PojoMethodMapping.java#L451
Jetty doesn't add the Primitive decoders to the decoder list on the either the EndpointConfig.decoders or the ServerEndpointConfig.decoders
Those are used internally, when nothing else will handle the object.