Remove Clients' Dependency on Concrete Implementations of EventFormat
skepticoitusInteruptus opened this issue · 6 comments
Context
The intent of Java™ SE's Service Provider Interface (SPI) mechanism is to facilitate OO best practice. The value the SPI facility is meant to add to the design of a Java™ library, is to decouple clients from concrete implementations of services.
I ❤️ well-designed libraries like Java SDK for Cloud Events that leverage SPI to follow OO best practice:
- Program to an interface; not to an implementation
- Open for extension; closed for modification
- etc.
The Issue
In the usage example [1], the import io.cloudevents.jackson.JsonFormat
of the concrete implementation of EventFormat
effectively defeats the purpose of using the SPI service abstraction facility.
Current Design
import io.cloudevents.CloudEvent;
import io.cloudevents.core.format.EventFormatProvider;
import io.cloudevents.core.builder.CloudEventBuilder;
import io.cloudevents.jackson.JsonFormat; // Tightly couples this client to a concrete implementation
CloudEvent event = CloudEventBuilder.v1()
.withId("hello")
.withType("example.vertx")
.withSource(URI.create("http://localhost"))
.build();
byte[]serialized = EventFormatProvider
.getInstance() // Since clients need the concrete impl in spite of SPI, why not do this:
.resolveFormat(JsonFormat.CONTENT_TYPE) // var json = new JsonFormat();
.serialize(event); // var serialized = json.serialize(event);
Proposed Enhancement
import io.cloudevents.CloudEvent;
import io.cloudevents.core.format.EventFormatProvider;
import io.cloudevents.core.builder.CloudEventBuilder;
import io.cloudevents.core.format.ContentTypes; // Introduce constant to decouple clients from implementations
CloudEvent event = CloudEventBuilder.v1()
.withId("hello")
.withType("example.vertx")
.withSource(URI.create("http://localhost"))
.build();
byte[] serialized = EventFormatProvider
.getInstance()
.resolveFormat(ContentTypes.JSON;) // No longer coupled to concrete io.cloudevents.jackson.JsonFormat
.serialize(event);
——
Thanks for the suggestion @skepticoitusInteruptus, feel free to propose a PR.
cc @JemDay in case you have opinions on the proposed approach
Technically this is already possible with passing a string but I see how a constant in the core library might facilitate that
I'd be in favor of cleanly separating these constructs, having a core 'ContentTypes' construct makes perfect sense to me.
I'd be curious is there's any advantage to using an enum; I suspect most of the usage will be string based - resolving the format during message reception based on a transport header, and populating the transport header during message sending.
I guess that'll become clear as you work your proposal ;-)
I'd be curious is there's any advantage to using an enum;
Off the top of my head, from a performance angle enum
constants would probably be my preferred option (vs String
literals) as expressions in a switch
statement, say.
Performance differences might or might not be significant, String
constants (i.e., literals) are more prone to hard-to-notice typos though:
var contentType = receiveMessage( )
.body( )
.contentTypeAsString();
switch(contentType) {
case "applicatoin/cloudevents+json":
// ...
break;
...
}
...vs...
var contentType = receiveMessage( )
.body( )
.contentTypeAsEnum();
switch(contentType) {
case JSON:
// ...
break;
...
}
I wouldn't insist on enum
s, by the way. The above is just for the sake of folks' curiosity.