Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat!: Support transient identities and traits #133

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat!: Support transient identities and traits #133

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d4d2a1
feat: Support transient identities and traits
khvn26 Jul 24, 2024
dc0b89c
Update lint config
khvn26 Jul 24, 2024
7b73f08
linting
khvn26 Jul 24, 2024
7435704
Add evaluation context support
khvn26 Aug 2, 2024
208484c
update schema
khvn26 Aug 9, 2024
8ceb613
fix typo, try stuff
khvn26 Aug 9, 2024
d91e2a4
avoid mutation
khvn26 Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ func (c *Client) GetEnvironmentFlags(ctx context.Context) (f Flags, err error) {
return Flags{}, &FlagsmithClientError{msg: fmt.Sprintf("Failed to fetch flags with error: %s", err)}
}

type GetIdentityFlagsOpts struct {
Transient bool `json:"transient,omitempty"`
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gagantrivedi Keen to know if this approach is redundant. Maybe we could just go with a transient bool argument to respective interfaces.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd prefer the latter approach

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having implemented the bool argument, I don't quite like not being able to provide nil as the value. Since Golang doesn't support optional args, I feel we're forcing the majority of users to make the decision they don't need to make.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a bool pointer should be able to achieve that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should, in theory, but then there's no way to provide a pointer to a constant (that I know of, at least).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like this: https://github.com/open-feature/java-sdk?tab=readme-ov-file#targeting

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... yep, ok, that's a good point. Perhaps, for now, we should add that method and mark getEnvironmentFlags and getIdentityFlags as deprecated? To use transient identities, you will need to move to use the getFlags method.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also like implementing the OF interface with an evaluation context that can contain the identity. Actually, an immediate benefit of doing that is that we could optionally send the environment or another environment key in this call providing an easy way to "switch context" I guess, but that's just an example and another discussion.

Copy link
Member

@kyle-ssg kyle-ssg Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoyingly I don't think it'll be as straight forward on the client-side with OF. In JS there's a single OF context. I wonder if this could be achieved using their concept of domains,

const domainScopedClient = OpenFeature.getClient("my-domain");
domainScopedClient.setContext(context1)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyle-ssg Domain-specific contexts look like the exact solution for this.


// Returns `Flags` struct holding all the flags for the current environment for
// a given identity.
//
Expand All @@ -118,13 +122,13 @@ func (c *Client) GetEnvironmentFlags(ctx context.Context) (f Flags, err error) {
// If local evaluation is enabled this function will not call the Flagsmith API
// directly, but instead read the asynchronously updated local environment or
// use the default flag handler in case it has not yet been updated.
func (c *Client) GetIdentityFlags(ctx context.Context, identifier string, traits []*Trait) (f Flags, err error) {
func (c *Client) GetIdentityFlags(ctx context.Context, identifier string, traits []*Trait, opts *GetIdentityFlagsOpts) (f Flags, err error) {
if c.config.localEvaluation || c.config.offlineMode {
if f, err = c.getIdentityFlagsFromEnvironment(identifier, traits); err == nil {
return f, nil
}
} else {
if f, err = c.GetIdentityFlagsFromAPI(ctx, identifier, traits); err == nil {
if f, err = c.GetIdentityFlagsFromAPI(ctx, identifier, traits, opts); err == nil {
return f, nil
}
}
Expand Down Expand Up @@ -191,11 +195,15 @@ func (c *Client) GetEnvironmentFlagsFromAPI(ctx context.Context) (Flags, error)

// GetIdentityFlagsFromAPI tries to contact the Flagsmith API to get the latest identity flags.
// Will return an error in case of failure or unexpected response.
func (c *Client) GetIdentityFlagsFromAPI(ctx context.Context, identifier string, traits []*Trait) (Flags, error) {
func (c *Client) GetIdentityFlagsFromAPI(ctx context.Context, identifier string, traits []*Trait, opts *GetIdentityFlagsOpts) (Flags, error) {
body := struct {
Identifier string `json:"identifier"`
Traits []*Trait `json:"traits,omitempty"`
GetIdentityFlagsOpts
}{Identifier: identifier, Traits: traits}
if opts != nil {
body.Transient = opts.Transient
}
resp, err := c.client.NewRequest().
SetBody(&body).
SetContext(ctx).
Expand Down
103 changes: 63 additions & 40 deletions client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@
// Then
assert.NoError(t, err)

flags, err := client.GetIdentityFlags(ctx, "test_identity", nil)
flags, err := client.GetIdentityFlags(ctx, "test_identity", nil, nil)

assert.NoError(t, err)

Expand All @@ -257,7 +257,7 @@
// Then
assert.NoError(t, err)

flags, err := client.GetIdentityFlags(ctx, "overridden-id", nil)
flags, err := client.GetIdentityFlags(ctx, "overridden-id", nil, nil)

assert.NoError(t, err)

Expand All @@ -272,55 +272,78 @@

func TestGetIdentityFlagsCallsAPIWhenLocalEnvironmentNotAvailableWithTraits(t *testing.T) {
// Given
stringTrait := flagsmith.Trait{TraitKey: "stringTrait", TraitValue: "trait_value"}
intTrait := flagsmith.Trait{TraitKey: "intTrait", TraitValue: 1}
floatTrait := flagsmith.Trait{TraitKey: "floatTrait", TraitValue: 1.11}
boolTrait := flagsmith.Trait{TraitKey: "boolTrait", TraitValue: true}
nillTrait := flagsmith.Trait{TraitKey: "NoneTrait", TraitValue: nil}
transientTrait := flagsmith.Trait{TraitKey: "TransientTrait", TraitValue: "not_persisted", Transient: true}

testCases := []struct {
Identifier string
Traits []*flagsmith.Trait
Opts *flagsmith.GetIdentityFlagsOpts
ExpectedRequestBody string
}{
{
"test_identity",
[]*flagsmith.Trait{&stringTrait, &intTrait, &floatTrait, &boolTrait, &nillTrait, &transientTrait},
nil,
`{"identifier":"test_identity","traits":[{"trait_key":"stringTrait","trait_value":"trait_value"},` +
`{"trait_key":"intTrait","trait_value":1},` +
`{"trait_key":"floatTrait","trait_value":1.11},` +
`{"trait_key":"boolTrait","trait_value":true},` +
`{"trait_key":"NoneTrait","trait_value":null},` +
`{"trait_key":"TransientTrait","trait_value":"not_persisted","transient":true}]}`,
},
{
"test_transient_identity",
[]*flagsmith.Trait{},
&flagsmith.GetIdentityFlagsOpts{Transient: true},
`{"identifier":"test_transient_identity","transient":true}`,
},
}

ctx := context.Background()
expectedRequestBody := `{"identifier":"test_identity","traits":[{"trait_key":"stringTrait","trait_value":"trait_value"},` +
`{"trait_key":"intTrait","trait_value":1},` +
`{"trait_key":"floatTrait","trait_value":1.11},` +
`{"trait_key":"boolTrait","trait_value":true},` +
`{"trait_key":"NoneTrait","trait_value":null}]}`

server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
assert.Equal(t, req.URL.Path, "/api/v1/identities/")
assert.Equal(t, fixtures.EnvironmentAPIKey, req.Header.Get("X-Environment-Key"))
for _, tc := range testCases {
server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
assert.Equal(t, req.URL.Path, "/api/v1/identities/")
assert.Equal(t, fixtures.EnvironmentAPIKey, req.Header.Get("X-Environment-Key"))

// Test that we sent the correct body
rawBody, err := io.ReadAll(req.Body)
assert.NoError(t, err)
assert.Equal(t, expectedRequestBody, string(rawBody))
// Test that we sent the correct body
rawBody, err := io.ReadAll(req.Body)
assert.NoError(t, err)
assert.Equal(t, tc.ExpectedRequestBody, string(rawBody))

rw.Header().Set("Content-Type", "application/json")
rw.Header().Set("Content-Type", "application/json")

rw.WriteHeader(http.StatusOK)
_, err = io.WriteString(rw, fixtures.IdentityResponseJson)
rw.WriteHeader(http.StatusOK)
_, err = io.WriteString(rw, fixtures.IdentityResponseJson)

assert.NoError(t, err)
}))
defer server.Close()
// When
client := flagsmith.NewClient(fixtures.EnvironmentAPIKey,
flagsmith.WithBaseURL(server.URL+"/api/v1/"))
assert.NoError(t, err)
}))
defer server.Close()
// When
client := flagsmith.NewClient(fixtures.EnvironmentAPIKey,
flagsmith.WithBaseURL(server.URL+"/api/v1/"))

stringTrait := flagsmith.Trait{TraitKey: "stringTrait", TraitValue: "trait_value"}
intTrait := flagsmith.Trait{TraitKey: "intTrait", TraitValue: 1}
floatTrait := flagsmith.Trait{TraitKey: "floatTrait", TraitValue: 1.11}
boolTrait := flagsmith.Trait{TraitKey: "boolTrait", TraitValue: true}
nillTrait := flagsmith.Trait{TraitKey: "NoneTrait", TraitValue: nil}
// When

traits := []*flagsmith.Trait{&stringTrait, &intTrait, &floatTrait, &boolTrait, &nillTrait}
// When
flags, err := client.GetIdentityFlags(ctx, tc.Identifier, tc.Traits, tc.Opts)

flags, err := client.GetIdentityFlags(ctx, "test_identity", traits)
// Then
assert.NoError(t, err)

// Then
assert.NoError(t, err)
allFlags := flags.AllFlags()

allFlags := flags.AllFlags()
assert.Equal(t, 1, len(allFlags))

assert.Equal(t, 1, len(allFlags))
assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName)
assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID)
assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value)

assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName)
assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID)
assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value)
}

Check failure on line 346 in client_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

unnecessary trailing newline (whitespace)
}

func TestDefaultHandlerIsUsedWhenNoMatchingEnvironmentFlagReturned(t *testing.T) {
Expand Down Expand Up @@ -608,7 +631,7 @@
assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value)

// And GetIdentityFlags works as well
flags, err = client.GetIdentityFlags(ctx, "test_identity", nil)
flags, err = client.GetIdentityFlags(ctx, "test_identity", nil, nil)
assert.NoError(t, err)

allFlags = flags.AllFlags()
Expand Down Expand Up @@ -650,7 +673,7 @@
assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value)

// And GetIdentityFlags works as well
flags, err = client.GetIdentityFlags(ctx, "test_identity", nil)
flags, err = client.GetIdentityFlags(ctx, "test_identity", nil, nil)
assert.NoError(t, err)

allFlags = flags.AllFlags()
Expand Down
2 changes: 2 additions & 0 deletions models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ type Flag struct {
type Trait struct {
TraitKey string `json:"trait_key"`
TraitValue interface{} `json:"trait_value"`
Transient bool `json:"transient,omitempty"`
}

type IdentityTraits struct {
Identifier string `json:"identifier"`
Traits []*Trait `json:"traits"`
Transient bool `json:"transient,omitempty"`
}

func (t *Trait) ToTraitModel() *traits.TraitModel {
Expand Down
Loading