sdk-java icon indicating copy to clipboard operation
sdk-java copied to clipboard

[restful-ws] Allow configuring default writer encoding for CloudEventsProvider

Open cardil opened this issue 2 years ago • 4 comments
trafficstars

The CloudEventsProvider defaults to binary encoding. Only when there is an StructuredEncoding annotation present, this mode will be set. See the logic:

https://github.com/cloudevents/sdk-java/blob/3614a4f5f4982e6ed38332c2cc73b1a558326274/http/restful-ws/src/main/java/io/cloudevents/http/restful/ws/CloudEventsProvider.java#L95-L112

This makes the BinaryEncoding useless, confirmed by no usages in the code.

The default to the binary mode is unfortunate. Some implementations like Quarkus RESTEasy Reactive Server Sent Events (SSE) don't provide the annotations. This is problematic, as the binary mode doesn't make sense in SSE - the headers set by CloudEventsProvider are being dropped.

See the code:

https://github.com/quarkusio/quarkus/blob/247736226a8c8a55fa88a662eda05963803905eb/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/SseUtil.java#L141-L142

See repro: https://github.com/cardil/openshift-knative-showcase/commit/15e00429c459abf655fb483a27f467d598ae173e Related issue: https://github.com/quarkusio/quarkus/issues/31559

The fact the Quarkus doesn't pass the method annotations, is probably a bug in Quarkus. But, I feel the CloudEvents SDK should allow setting the default mode. Then, the BinaryEncoding could have a meaning, of changing that default encoding per-method.

/kind bug

cardil avatar Mar 03 '23 13:03 cardil

With @JemDay we were discussing configurations like this in the context of XML format writer but there is no concrete design or proposal yet.

pierDipi avatar Mar 03 '23 13:03 pierDipi

How about we provide settings like interface, which people could define using Java's ServiceLoader. It may be something like:

interface Settings {
  default Encoding defaultEncoding() {
    return Encoding.BINARY;
  }
}

cardil avatar Mar 03 '23 13:03 cardil

I created a Quarkus issue about this: https://github.com/quarkusio/quarkus/issues/31587

cardil avatar Mar 03 '23 14:03 cardil

As evidenced in https://github.com/cardil/openshift-knative-showcase/pull/2 the fixing of https://github.com/quarkusio/quarkus/issues/31587 (PR https://github.com/quarkusio/quarkus/pull/32815) didn't help completely.

cardil avatar Jul 25 '23 14:07 cardil