open-feature/java-sdk

Implement events

Closed this issue ยท 12 comments

I'd like to implement events as mentioned here.

At the moment, I'm trying to decide on the most "Java-ish" way of implementing an event listener. In the web-sdk, we add event handlers like this:

client.addHandler(ProviderEvents.Ready, myClientOnReadyHandler);

Something very like this is possible in Java, but it doesn't feel very "Java-y". I think the spec gives us the flexibility to do something more like this:

client.addEventObserver(new EventObserver() { // anonymous class implementing possible EventObserver interface

  @Override
  public onProviderReady(EventDetails eventDetails) {
    // do stuff
  }
  
  @Override
  public onProviderConfigurationChanged(EventDetails eventDetails) {
    // do stuff
  }
  
  @Override
  public onProviderError(EventDetails eventDetails) {
    // do stuff
  }

});

What are people's thoughts?

cc @open-feature/sdk-java-approvers @open-feature/sdk-java-maintainers

I am not super engaged in the Java ecosystem, but how do people feel about SAM interfaces these days? For me it seems like the ability to provide a lambda when I just care about one aspect is nice, especially when they seem like reasonably different concerns. Though I imagine that would mean 3 registration methods.

I am not super engaged in the Java ecosystem, but how do people feel about SAM interfaces these days? For me it seems like the ability to provide a lambda when I just care about one aspect is nice, especially when they seem like reasonably different concerns. Though I imagine that would mean 3 registration methods.

I like functional interfaces, and I think that's basically how I would tackle the first example above... I fear I might be in the minority there though ๐Ÿ˜… . I'm hoping that a discussion here can help us decide.

gRPC decided to go with something like my second proposal, FWIW: https://grpc.io/docs/languages/java/basics/#bidirectional-streaming-rpc https://grpc.github.io/grpc-java/javadoc/io/grpc/stub/StreamObserver.html

I'll defer to others. I don't have experience with eventing within Java like this. I would naturally reach for unlined lambdas as others mention.

I am also in favor of lambda, even if that requires individual registrations. This gives the developer the freedom to register only on interested events.

Besides, I think we will only need a single functional interface definition from SDK ๐Ÿค”

@FunctionalInterface
public interface EventHandler {
    void handle(EventDetails details);
}

p.s - I think we can't fully compare the grpc stream observer to OF events. The grpc interface is a contract for the full request streaming lifecycle (new response, error and done), where as OF is purely individual events.

OK I think I'll go with the functional interface direction then!

lopitz commented

๐Ÿ‘ to the functional interface direction.

I happen to love the project reactor api (e.g. Mono) over the last couple of years and I think, it would be a wonderful fit for this implementation as well. The signature of the event listener registration methods would be something like

Client doOnProviderReady(Consumer<EventDetails> eventHandler)

or we could also (additionally?) go the route

Client doOnProviderEvent(Predicate<ProviderEvent> predicate, Consumer<EventDetails> eventHandler)

do allow for new event types.

A code example could be:

client
    .doOnProviderReady(eventDetails -> doSomethingWhenTheProviderIsReady(eventDetails))
    .doOnProviderError(this::handleProviderError) //alternative notation
    .doOnProviderEvent(eventType -> eventType == ProviderEvents.ConfigurationChanged, this::providerConfigurationChanged);

To make the handling of the predicates easier, we could even move the predicates into the ProviderEvents, so that it might read like

client
    .doOnProviderEvent(ProviderEvents::isConfigurationChanged, this::providerConfigurationChanged);
lopitz commented

I am also in favor of lambda, even if that requires individual registrations. This gives the developer the freedom to register only on interested events.

Besides, I think we will only need a single functional interface definition from SDK ๐Ÿค”

@FunctionalInterface
public interface EventHandler {
    void handle(EventDetails details);
}

p.s - I think we can't fully compare the grpc stream observer to OF events. The grpc interface is a contract for the full request streaming lifecycle (new response, error and done), where as OF is purely individual events.

I'm not sure, whether we need a dedicated interface definition at all. The JDK already contains a Consumer<T> interface, which should be sufficient for this job.

I am also in favor of lambda, even if that requires individual registrations. This gives the developer the freedom to register only on interested events.
Besides, I think we will only need a single functional interface definition from SDK ๐Ÿค”

@FunctionalInterface
public interface EventHandler {
    void handle(EventDetails details);
}

p.s - I think we can't fully compare the grpc stream observer to OF events. The grpc interface is a contract for the full request streaming lifecycle (new response, error and done), where as OF is purely individual events.

I'm not sure, whether we need a dedicated interface definition at all. The JDK already contains a Consumer<T> interface, which should be sufficient for this job.

Yes, agree with this. I just showed the possibility of minimalizing the contract.

I happen to love the project reactor api (e.g. Mono) over the last couple of years and I think, it would be a wonderful fit for this implementation as well. The signature of the event listener registration methods would be something like

@lopitz I haven't used Mono personally but checked the referenced docs. It is interesting but I have doubts.

API does provide many features, but do you see how this fits especially to a library scenario? The API we have accepts an event handler and the internals of the handlers are beyond our scope (ex:- accepting an event through a simple logger, running a background thread, etc...). Whereas Mono focuses on reacting to data streams (per my understanding and in general) with extra operations (ex:- delay, take(x))

lopitz commented

@Kavindu-Dodan i just meant, that the Mono API reads well. We of course don't need all the stuff that is part of the Mono API. so, it's really just the naming of methods we need, not trying to pull in methods, which are available. Hence, with the current spec I see 4 methods, which would be beneficial. They would be:

public Client doOnProviderReady(Consumer<EventDetails> eventConsumer);
public Client doOnProviderError(Consumer<EventDetails> eventConsumer);
public Client doOnProviderConfigurationChanged(Consumer<EventDetails> eventConsumer);

//the following method could also be omitted
public Client doOnProviderEvent(Predicated<ProviderEventType> condition, Consumer<EventDetails> eventConsumer);

@Kavindu-Dodan i just meant, that the Mono API reads well. We of course don't need all the stuff that is part of the Mono API. so, it's really just the naming of methods we need, not trying to pull in methods, which are available. Hence, with the current spec I see 4 methods, which would be beneficial. They would be:

public Client doOnProviderReady(Consumer<EventDetails> eventConsumer);
public Client doOnProviderError(Consumer<EventDetails> eventConsumer);
public Client doOnProviderConfigurationChanged(Consumer<EventDetails> eventConsumer);

//the following method could also be omitted
public Client doOnProviderEvent(Predicated<ProviderEventType> condition, Consumer<EventDetails> eventConsumer);

Thanks for the explanantion @lopitz , Yes, agree with the naming and parameters. Plus the chaining capability by returning the Client

I've started on this, based on #456, with an APi similar to what @lopitz has proposed above.