Skip to content

Commit

Permalink
review changes
Browse files Browse the repository at this point in the history
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
  • Loading branch information
Kavindu-Dodan committed May 8, 2024
1 parent 44811cc commit 275a505
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 66 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ func (h MyHook) Error(context context.Context, hookContext openfeature.HookConte
## Testing

To test interactions with OpenFeature API and Client, you can rely on `openfeature.IOFApi` & `openfeature.IClient` interfaces.
To test interactions with OpenFeature API and Client, you can rely on `openfeature.IEvaluation` & `openfeature.IClient` interfaces.

While you may use global methods to interact with the API, it is recommended to obtain the singleton API instance so that you can use appropriate mocks for your testing needs,

Expand All @@ -381,7 +381,7 @@ apiInstance := openfeature.GetApiInstance()
apiInstance.SetProvider(myProvider)
```

Similarly, while you have options (due to historical reasons) to create a client with `openfeature.NewClient()` helper, it is recommended to use API to generate the client which returns an `IClient` instance.
Similarly, while you have option (due to historical reasons) to create a client with `openfeature.NewClient()` helper, it is recommended to use API to generate the client which returns an `IClient` instance.

```go
// global helper
Expand Down
8 changes: 4 additions & 4 deletions openfeature/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (cm ClientMetadata) Name() string {

// Client implements the behaviour required of an openfeature client
type Client struct {
api ofApiImpl
api evaluationImpl
clientEventing clientEvent
metadata ClientMetadata
hooks []Hook
Expand All @@ -44,12 +44,12 @@ type Client struct {
var _ IClient = (*Client)(nil)

// NewClient returns a new Client. Name is a unique identifier for this client
// This helper exists for historical reasons. It is recommended to interact with IOFApi to derive IClient instances.
// This helper exists for historical reasons. It is recommended to interact with IEvaluation to derive IClient instances.
func NewClient(name string) *Client {
return newClient(name, api, eventing, logger)
}

func newClient(name string, apiRef ofApiImpl, eventRef clientEvent, log logr.Logger) *Client {
func newClient(name string, apiRef evaluationImpl, eventRef clientEvent, log logr.Logger) *Client {
return &Client{
api: apiRef,
clientEventing: eventRef,
Expand Down Expand Up @@ -84,7 +84,7 @@ func (c *Client) AddHooks(hooks ...Hook) {

// AddHandler allows to add Client level event handler
func (c *Client) AddHandler(eventType EventType, callback EventCallback) {
c.clientEventing.RegisterClientHandler(c.metadata.Name(), eventType, callback)
c.clientEventing.AddClientHandler(c.metadata.Name(), eventType, callback)
}

// RemoveHandler allows to remove Client level event handler
Expand Down
20 changes: 10 additions & 10 deletions openfeature/event_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type eventingImpl interface {

// clientEvent is an internal reference for OpenFeature Client events
type clientEvent interface {
RegisterClientHandler(clientName string, t EventType, c EventCallback)
AddClientHandler(clientName string, t EventType, c EventCallback)
RemoveClientHandler(name string, t EventType, c EventCallback)
}

Expand Down Expand Up @@ -85,7 +85,7 @@ type eventPayload struct {
}

// providerReference is a helper struct to store FeatureProvider with EventHandler capability along with their
// Shutdown semaphore
// shutdown semaphore
type providerReference struct {
featureProvider FeatureProvider
shutdownSemaphore chan interface{}
Expand Down Expand Up @@ -134,7 +134,7 @@ func (e *EventExecutor) RemoveHandler(t EventType, c EventCallback) {
}

// RegisterClientHandler registers a client level handler
func (e *EventExecutor) RegisterClientHandler(clientName string, t EventType, c EventCallback) {
func (e *EventExecutor) AddClientHandler(clientName string, t EventType, c EventCallback) {
e.mu.Lock()
defer e.mu.Unlock()

Expand Down Expand Up @@ -233,7 +233,7 @@ func (e *EventExecutor) registerDefaultProvider(provider FeatureProvider) error
e.mu.Lock()
defer e.mu.Unlock()

// register Shutdown semaphore for new default provider
// register shutdown semaphore for new default provider
sem := make(chan interface{})

newProvider := providerReference{
Expand All @@ -252,7 +252,7 @@ func (e *EventExecutor) registerNamedEventingProvider(associatedClient string, p
e.mu.Lock()
defer e.mu.Unlock()

// register Shutdown semaphore for new named provider
// register shutdown semaphore for new named provider
sem := make(chan interface{})

newProvider := providerReference{
Expand All @@ -266,7 +266,7 @@ func (e *EventExecutor) registerNamedEventingProvider(associatedClient string, p
return e.startListeningAndShutdownOld(newProvider, oldProvider)
}

// startListeningAndShutdownOld is a helper to start concurrent listening to new provider events and invoke Shutdown
// startListeningAndShutdownOld is a helper to start concurrent listening to new provider events and invoke shutdown
// hook of the old provider if it's not bound by another subscription
func (e *EventExecutor) startListeningAndShutdownOld(newProvider providerReference, oldReference providerReference) error {

Expand Down Expand Up @@ -295,7 +295,7 @@ func (e *EventExecutor) startListeningAndShutdownOld(newProvider providerReferen
}()
}

// Shutdown old provider handling
// shutdown old provider handling

// check if this provider is still bound - 1:N binding capability
if isBound(oldReference, e.defaultProviderReference, maps.Values(e.namedProviderReference)) {
Expand All @@ -311,16 +311,16 @@ func (e *EventExecutor) startListeningAndShutdownOld(newProvider providerReferen

_, ok := oldReference.featureProvider.(EventHandler)
if !ok {
// no Shutdown for non event handling provider
// no shutdown for non event handling provider
return nil
}

// avoid Shutdown lockouts
// avoid shutdown lockouts
select {
case oldReference.shutdownSemaphore <- "":
return nil
case <-time.After(200 * time.Millisecond):
return fmt.Errorf("old event handler %s timeout waiting for handler Shutdown",
return fmt.Errorf("old event handler %s timeout waiting for handler shutdown",
oldReference.featureProvider.Metadata().Name)
}
}
Expand Down
28 changes: 14 additions & 14 deletions openfeature/event_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1395,15 +1395,15 @@ func TestEventHandler_Registration(t *testing.T) {
executor := NewEventExecutor(logger)

// Add multiple - client a
executor.RegisterClientHandler("a", ProviderReady, &h1)
executor.RegisterClientHandler("a", ProviderReady, &h2)
executor.RegisterClientHandler("a", ProviderReady, &h3)
executor.RegisterClientHandler("a", ProviderReady, &h4)
executor.AddClientHandler("a", ProviderReady, &h1)
executor.AddClientHandler("a", ProviderReady, &h2)
executor.AddClientHandler("a", ProviderReady, &h3)
executor.AddClientHandler("a", ProviderReady, &h4)

// Add single for rest of the client
executor.RegisterClientHandler("b", ProviderError, &h2)
executor.RegisterClientHandler("c", ProviderStale, &h3)
executor.RegisterClientHandler("d", ProviderConfigChange, &h4)
executor.AddClientHandler("b", ProviderError, &h2)
executor.AddClientHandler("c", ProviderStale, &h3)
executor.AddClientHandler("d", ProviderConfigChange, &h4)

readyLen := len(executor.scopedRegistry["a"].callbacks[ProviderReady])
if readyLen != 4 {
Expand Down Expand Up @@ -1485,15 +1485,15 @@ func TestEventHandler_APIRemoval(t *testing.T) {
executor := NewEventExecutor(logger)

// Add multiple - client a
executor.RegisterClientHandler("a", ProviderReady, &h1)
executor.RegisterClientHandler("a", ProviderReady, &h2)
executor.RegisterClientHandler("a", ProviderReady, &h3)
executor.RegisterClientHandler("a", ProviderReady, &h4)
executor.AddClientHandler("a", ProviderReady, &h1)
executor.AddClientHandler("a", ProviderReady, &h2)
executor.AddClientHandler("a", ProviderReady, &h3)
executor.AddClientHandler("a", ProviderReady, &h4)

// Add single
executor.RegisterClientHandler("b", ProviderError, &h2)
executor.RegisterClientHandler("c", ProviderStale, &h3)
executor.RegisterClientHandler("d", ProviderConfigChange, &h4)
executor.AddClientHandler("b", ProviderError, &h2)
executor.AddClientHandler("c", ProviderStale, &h3)
executor.AddClientHandler("d", ProviderConfigChange, &h4)

// removal
executor.RemoveClientHandler("a", ProviderReady, &h1)
Expand Down
6 changes: 3 additions & 3 deletions openfeature/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package openfeature

import "context"

// IOFApi defines the OpenFeature API contract
type IOFApi interface {
// IEvaluation defines the OpenFeature API contract
type IEvaluation interface {
SetProvider(provider FeatureProvider) error
SetProviderAndWait(provider FeatureProvider) error
GetProviderMetadata() Metadata
Expand Down Expand Up @@ -39,7 +39,7 @@ type IClient interface {
Float(ctx context.Context, flag string, defaultValue float64, evalCtx EvaluationContext, options ...Option) float64
Int(ctx context.Context, flag string, defaultValue int64, evalCtx EvaluationContext, options ...Option) int64
Object(ctx context.Context, flag string, defaultValue interface{}, evalCtx EvaluationContext, options ...Option) interface{}

IEventing
}

Expand Down
10 changes: 5 additions & 5 deletions openfeature/openfeature.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"github.com/open-feature/go-sdk/openfeature/internal"
)

// api is the global ofApiImpl implementation. This is a singleton and there can only be one instance.
var api ofApiImpl
// api is the global evaluationImpl implementation. This is a singleton and there can only be one instance.
var api evaluationImpl
var eventing eventingImpl
var logger logr.Logger

Expand All @@ -21,12 +21,12 @@ func initSingleton() {
var exec = NewEventExecutor(logger)
eventing = exec

api = NewEvaluationAPI(exec, logger)
api = newEvaluationAPI(exec, logger)
}

// GetApiInstance returns the current singleton IOFApi instance.
// GetApiInstance returns the current singleton IEvaluation instance.
// This is the preferred interface to interact with OpenFeature functionalities
func GetApiInstance() IOFApi {
func GetApiInstance() IEvaluation {
return api
}

Expand Down

0 comments on commit 275a505

Please sign in to comment.