Skip to content

Commit d045d9c

Browse files
Kavindu-DodanKavindu-Dodan
committed
tests, improvements and documentation
Signed-off-by: Kavindu Dodanduwa <[email protected]>
1 parent 62f7a16 commit d045d9c

File tree

8 files changed

+138
-71
lines changed

8 files changed

+138
-71
lines changed

‎providers/ofrep/README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
This is the Go implementation of the OFREP provider.
44
The provider works by evaluating flags against OFREP single flag evaluation endpoint.
5-
And there is no caching layer in this version.
65

76
## Installation
87

‎providers/ofrep/internal/evaluate/flags.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,18 @@ import (
88
of "github.com/open-feature/go-sdk/openfeature"
99
)
1010

11-
type resolver interface {
12-
resolveSingle(ctx context.Context, key string, evalCtx map[string]interface{}) (*successDto, *of.ResolutionError)
13-
}
14-
11+
// Flags is the flag evaluator implementation. It contains domain logic of the OpenFeature flag evaluation.
1512
type Flags struct {
1613
resolver resolver
1714
}
1815

19-
func NewFlagsEvaluator(cfg outbound.Configuration) *Flags {
20-
client := outbound.NewHttp(cfg)
16+
type resolver interface {
17+
resolveSingle(ctx context.Context, key string, evalCtx map[string]interface{}) (*successDto, *of.ResolutionError)
18+
}
2119

20+
func NewFlagsEvaluator(cfg outbound.Configuration) *Flags {
2221
return &Flags{
23-
resolver: NewOutboundResolver(client),
22+
resolver: NewOutboundResolver(cfg),
2423
}
2524
}
2625

‎providers/ofrep/internal/evaluate/flags_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package evaluate
22

33
import (
44
"context"
5-
"fmt"
65
"reflect"
76
"testing"
87

@@ -302,19 +301,19 @@ func genericValidator[T knownTypes](test testDefinition[T], resolvedValue T, rea
302301
}
303302

304303
if !reflect.DeepEqual(test.defaultValue, resolvedValue) {
305-
t.Error(fmt.Sprintf("expected deafault value %v, but got %v", test.defaultValue, resolvedValue))
304+
t.Errorf("expected deafault value %v, but got %v", test.defaultValue, resolvedValue)
306305
}
307306

308307
if reason != of.ErrorReason {
309-
t.Error(fmt.Sprintf("expected reason %v, but got %v", of.ErrorReason, reason))
308+
t.Errorf("expected reason %v, but got %v", of.ErrorReason, reason)
310309
}
311310
} else {
312311
if err != nil {
313-
t.Fatal(fmt.Sprintf("expected no error, but got none nil error: %v", err))
312+
t.Fatalf("expected no error, but got none nil error: %v", err)
314313
}
315314

316315
if !reflect.DeepEqual(test.expect, resolvedValue) {
317-
t.Error(fmt.Sprintf("expected value %v, but got %v", test.expect, resolvedValue))
316+
t.Errorf("expected value %v, but got %v", test.expect, resolvedValue)
318317
}
319318
}
320319
}

‎providers/ofrep/internal/evaluate/resolver.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,22 @@ import (
99
"strconv"
1010
"time"
1111

12+
"github.com/open-feature/go-sdk-contrib/providers/ofrep/internal/outbound"
1213
of "github.com/open-feature/go-sdk/openfeature"
1314
)
1415

15-
type Outbound interface {
16-
PostSingle(ctx context.Context, key string, payload []byte) (*http.Response, error)
17-
}
18-
16+
// OutboundResolver is responsible for resolving flags with outbound communications.
17+
// It contains domain logic of the OFREP specification.
1918
type OutboundResolver struct {
2019
client Outbound
2120
}
2221

23-
func NewOutboundResolver(client Outbound) *OutboundResolver {
24-
return &OutboundResolver{client}
22+
type Outbound interface {
23+
PostSingle(ctx context.Context, key string, payload []byte) (*http.Response, error)
24+
}
25+
26+
func NewOutboundResolver(cfg outbound.Configuration) *OutboundResolver {
27+
return &OutboundResolver{client: outbound.NewHttp(cfg)}
2528
}
2629

2730
func (g *OutboundResolver) resolveSingle(ctx context.Context, key string, evalCtx map[string]interface{}) (
@@ -116,7 +119,7 @@ func parse429(rsp *http.Response) time.Duration {
116119
return 0
117120
}
118121

119-
return parsed.Sub(time.Now())
122+
return time.Until(parsed)
120123
}
121124

122125
func parseError500(body io.ReadCloser) *of.ResolutionError {

‎providers/ofrep/internal/evaluate/resolver_test.go

Lines changed: 34 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"context"
66
"encoding/json"
77
"errors"
8-
"fmt"
98
"io"
109
"net/http"
1110
"strings"
@@ -36,45 +35,49 @@ func TestSuccess200(t *testing.T) {
3635
t.Fatal(err)
3736
}
3837

39-
client := mockOutbound{
38+
resolver := OutboundResolver{client: mockOutbound{
4039
rsp: http.Response{
4140
StatusCode: http.StatusOK,
4241
Body: io.NopCloser(bytes.NewReader(successBytes)),
4342
},
44-
}
43+
}}
4544

46-
successDto, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
45+
successDto, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
4746

4847
if resolutionError != nil {
49-
t.Error(fmt.Sprintf("expected no errors, but got error: %v", err))
48+
t.Errorf("expected no errors, but got error: %v", err)
5049
}
5150

5251
if successDto == nil {
5352
t.Fatal("expected non empty success response")
5453
}
5554

5655
if successDto.Value != success.Value {
57-
t.Errorf(fmt.Sprintf("expected value %v, but got %v", success.Value, successDto.Value))
56+
t.Errorf("expected value %v, but got %v", success.Value, successDto.Value)
5857
}
5958

6059
if successDto.Variant != success.Variant {
61-
t.Errorf(fmt.Sprintf("expected variant %v, but got %v", success.Variant, successDto.Variant))
60+
t.Errorf("expected variant %v, but got %v", success.Variant, successDto.Variant)
6261
}
6362

6463
if successDto.Reason != success.Reason {
65-
t.Errorf(fmt.Sprintf("expected reason %s, but got %s", success.Reason, successDto.Reason))
64+
t.Errorf("expected reason %s, but got %s", success.Reason, successDto.Reason)
65+
}
66+
67+
if successDto.Metadata["key"] != "value" {
68+
t.Errorf("expected key to contain value %s, but got %s", "value", successDto.Metadata["key"])
6669
}
6770
})
6871

6972
t.Run("invalid payload type results in general error", func(t *testing.T) {
70-
client := mockOutbound{
73+
resolver := OutboundResolver{client: mockOutbound{
7174
rsp: http.Response{
7275
StatusCode: http.StatusOK,
7376
Body: io.NopCloser(bytes.NewReader([]byte("some payload"))),
7477
},
75-
}
78+
}}
79+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
7680

77-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
7881
validateErrorCode(success, resolutionError, of.GeneralCode, t)
7982
})
8083
}
@@ -116,8 +119,9 @@ func TestResolveGeneralErrors(t *testing.T) {
116119

117120
for _, test := range tests {
118121
t.Run(test.name, func(t *testing.T) {
119-
// when
120-
success, resolutionError := NewOutboundResolver(test.client).resolveSingle(context.Background(), "key", map[string]interface{}{})
122+
resolver := OutboundResolver{client: test.client}
123+
success, resolutionError := resolver.resolveSingle(context.Background(), "key", map[string]interface{}{})
124+
121125
validateErrorCode(success, resolutionError, of.GeneralCode, t)
122126
})
123127
}
@@ -166,31 +170,27 @@ func TestEvaluationError4xx(t *testing.T) {
166170
t.Fatal(err)
167171
}
168172

169-
client := mockOutbound{
173+
resolver := OutboundResolver{client: mockOutbound{
170174
rsp: http.Response{
171175
StatusCode: http.StatusBadRequest,
172176
Body: io.NopCloser(bytes.NewReader(errBytes)),
173177
},
174-
}
175-
// when
176-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
178+
}}
179+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
177180

178-
// then
179181
validateErrorCode(success, resolutionError, test.expectCode, t)
180182
})
181183
}
182184
}
183185

184186
func TestFlagNotFound404(t *testing.T) {
185-
client := mockOutbound{
187+
resolver := OutboundResolver{client: mockOutbound{
186188
rsp: http.Response{
187189
StatusCode: http.StatusNotFound,
188190
},
189-
}
190-
// when
191-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
191+
}}
192+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
192193

193-
// then
194194
validateErrorCode(success, resolutionError, of.FlagNotFoundCode, t)
195195
}
196196

@@ -225,29 +225,24 @@ func Test429(t *testing.T) {
225225
}
226226
}
227227

228-
client := mockOutbound{
229-
rsp: response,
230-
}
228+
resolver := OutboundResolver{client: mockOutbound{rsp: response}}
229+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
231230

232-
// when
233-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
234231
validateErrorCode(success, resolutionError, of.GeneralCode, t)
235232
})
236233
}
237234
}
238235

239236
func TestEvaluationError5xx(t *testing.T) {
240237
t.Run("without body", func(t *testing.T) {
241-
client := mockOutbound{
238+
resolver := OutboundResolver{client: mockOutbound{
242239
rsp: http.Response{
243240
StatusCode: http.StatusInternalServerError,
244241
Body: io.NopCloser(bytes.NewReader([]byte{})),
245242
},
246-
}
247-
// when
248-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
243+
}}
244+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
249245

250-
// then
251246
validateErrorCode(success, resolutionError, of.GeneralCode, t)
252247
})
253248

@@ -257,30 +252,26 @@ func TestEvaluationError5xx(t *testing.T) {
257252
t.Fatal(err)
258253
}
259254

260-
client := mockOutbound{
255+
resolver := OutboundResolver{client: mockOutbound{
261256
rsp: http.Response{
262257
StatusCode: http.StatusInternalServerError,
263258
Body: io.NopCloser(bytes.NewReader(errorBytes)),
264259
},
265-
}
266-
// when
267-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
260+
}}
261+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
268262

269-
// then
270263
validateErrorCode(success, resolutionError, of.GeneralCode, t)
271264
})
272265

273266
t.Run("with invalid body", func(t *testing.T) {
274-
client := mockOutbound{
267+
resolver := OutboundResolver{client: mockOutbound{
275268
rsp: http.Response{
276269
StatusCode: http.StatusInternalServerError,
277270
Body: io.NopCloser(bytes.NewReader([]byte("some error"))),
278271
},
279-
}
280-
// when
281-
success, resolutionError := NewOutboundResolver(client).resolveSingle(context.Background(), "", make(map[string]interface{}))
272+
}}
273+
success, resolutionError := resolver.resolveSingle(context.Background(), "", make(map[string]interface{}))
282274

283-
// then
284275
validateErrorCode(success, resolutionError, of.GeneralCode, t)
285276
})
286277
}

‎providers/ofrep/internal/outbound/http_test.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,58 @@ import (
55
"fmt"
66
"net/http"
77
"testing"
8+
"time"
89
)
910

1011
func TestHttpOutbound(t *testing.T) {
1112
// given
13+
host := "localhost:18181"
1214
key := "flag"
15+
1316
server := http.Server{
14-
Addr: "localhost:18181",
17+
Addr: host,
1518
Handler: mockHandler{
1619
t: t,
1720
key: key,
1821
},
1922
}
2023

2124
go func() {
22-
server.ListenAndServe()
25+
err := server.ListenAndServe()
26+
if err != nil {
27+
t.Logf("error starting mock server: %v", err)
28+
return
29+
}
2330
}()
2431

32+
<-time.After(3 * time.Second)
33+
2534
outbound := NewHttp(Configuration{
2635
Callbacks: []HeaderCallback{
2736
func() (string, string) {
2837
return "Authorization", "Token"
2938
},
3039
},
31-
BaseURI: "http://localhost:18181",
40+
BaseURI: fmt.Sprintf("http://%s", host),
3241
})
3342

3443
// when
3544
response, err := outbound.PostSingle(context.Background(), key, []byte{})
3645
if err != nil {
46+
t.Fatalf("error from request: %v", err)
3747
return
3848
}
3949

40-
// then - expect a ok response
50+
// then - expect an ok response
4151
if response.StatusCode != http.StatusOK {
4252
t.Errorf("expected 200, but got %d", response.StatusCode)
4353
}
4454

4555
// cleanup
46-
server.Close()
47-
server.Shutdown(context.Background())
56+
err = server.Shutdown(context.Background())
57+
if err != nil {
58+
t.Errorf("error shuttting down mock server: %v", err)
59+
}
4860
}
4961

5062
type mockHandler struct {
@@ -73,5 +85,4 @@ func (r mockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
7385
}
7486

7587
resp.WriteHeader(http.StatusOK)
76-
return
7788
}

‎providers/ofrep/provider.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ type Provider struct {
1616

1717
type option func(*outbound.Configuration)
1818

19-
// NewProvider returns a provider configured with provided configuration
19+
// NewProvider returns an OFREP provider configured with provided configuration.
20+
// The only mandatory configuration is the baseUri, which is the base path of the OFREP service implementation.
2021
func NewProvider(baseUri string, options ...option) *Provider {
2122
cfg := outbound.Configuration{
2223
BaseURI: baseUri,

0 commit comments

Comments
 (0)