Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

API simplification #118

Merged
merged 48 commits into from
May 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
2d3a354
Introduce the RulesetVersion type
asdine Mar 22, 2019
91d3a05
NewRuleset creates one version
asdine Mar 22, 2019
11c0bbe
Ruleset Eval method evaluates latest ruleset
asdine Mar 22, 2019
b91bb05
Remove unused methods
asdine Mar 22, 2019
ce9dbbe
Remove ErrTypeMismatch
asdine Mar 27, 2019
ddbf012
Define constant sentinel errors
asdine Mar 27, 2019
ad44c43
Add EvalVersion to Ruleset type
asdine Mar 27, 2019
62d9ad6
Simplify RulesetBuffer
asdine Mar 27, 2019
740a1d6
Fix engine and example tests
asdine Mar 27, 2019
b503702
Remove unused ErrSignatureMismatch
asdine Mar 27, 2019
70278fa
Adapt API Get to the new Ruleset format
asdine Mar 28, 2019
aac381a
Simplify API Put
asdine Mar 28, 2019
7339a87
API Put returns the new version string
asdine Mar 28, 2019
5caccfe
Add method to ListOptions
asdine Mar 28, 2019
ed10363
Test GetLimit
asdine Mar 28, 2019
99e3fe0
Rework list endpoint
asdine Mar 28, 2019
6fc2c06
Fix Get test
asdine Mar 28, 2019
b9f18c1
Add LatestVersion method to Ruleset
asdine Mar 29, 2019
d77902f
Split etcd List method
asdine Apr 8, 2019
df06ca5
Simplify list implementation
asdine May 7, 2019
1e23409
Extract put tests
asdine May 7, 2019
a691b68
Extract get tests
asdine May 7, 2019
09a7b5a
Extract watch tests
asdine May 7, 2019
880e4a5
Extract eval tests
asdine May 7, 2019
60426f1
Change List signature
asdine May 16, 2019
db2675c
Simplifyied ruleset definition
asdine May 16, 2019
02c2b2d
Add support for versions in RulesetBuffer
asdine May 16, 2019
770e915
Fix tests
asdine May 16, 2019
fedfe26
Simplify list API
asdine May 16, 2019
b659547
Change Get signature
asdine May 16, 2019
aaec095
Support version in the Get endpoint
asdine May 16, 2019
0e493ae
Split tests into multiple files
asdine May 16, 2019
caaa850
Fix list tests
asdine May 17, 2019
0ea6323
Fix mocks
asdine May 17, 2019
1c065f5
Reworked API Get and List tests
asdine May 21, 2019
dd87d53
Reworked API Eval tests
asdine May 21, 2019
c0d8333
Reworked API Watch tests
asdine May 21, 2019
c83a418
Reworked API Put tests
asdine May 21, 2019
90601f3
Reworked API Create tests
asdine May 21, 2019
fb18c94
Fix put endpoint
asdine May 21, 2019
77c62d6
Split server tests
asdine May 21, 2019
e0e2beb
Remove debug print
asdine May 21, 2019
0d6b179
Apply suggestions from code review
asdine May 22, 2019
d2484b4
Simplify etcd response reading
asdine May 22, 2019
2dfb3d4
Remove unnecessary docstring
asdine May 22, 2019
f6f6d02
Improve etcd get test
asdine May 22, 2019
7f652a1
Improve GetLimit behaviour
asdine May 22, 2019
cd5448a
Improve etcd get docstring
asdine May 22, 2019
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
35 changes: 35 additions & 0 deletions api/etcd/create_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package etcd

import (
"context"
"testing"

"github.com/gogo/protobuf/proto"
"github.com/heetch/regula"
"github.com/heetch/regula/api"
pb "github.com/heetch/regula/api/etcd/proto"
"github.com/stretchr/testify/require"
)

func TestCreate(t *testing.T) {
s, cleanup := newEtcdRulesetService(t)
defer cleanup()

sig := regula.Signature{ReturnType: "bool", Params: map[string]string{"id": "string"}}

// create a ruleset
err := s.Create(context.Background(), "a", &sig)
require.NoError(t, err)

// verify creation
resp, err := s.Client.Get(context.Background(), s.signaturesPath("a"))
require.NoError(t, err)
var pbsig pb.Signature
err = proto.Unmarshal(resp.Kvs[0].Value, &pbsig)
require.NoError(t, err)
require.EqualValues(t, &sig, signatureFromProtobuf(&pbsig))

// create with the same path
err = s.Create(context.Background(), "a", &sig)
require.Error(t, api.ErrAlreadyExists, err)
}
62 changes: 62 additions & 0 deletions api/etcd/eval_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package etcd

import (
"context"
"testing"

"github.com/heetch/regula"
"github.com/heetch/regula/errors"
"github.com/heetch/regula/rule"
"github.com/stretchr/testify/require"
)

func TestEval(t *testing.T) {
s, cleanup := newEtcdRulesetService(t)
defer cleanup()

sig := &regula.Signature{ReturnType: "bool", Params: map[string]string{"id": "string"}}
require.NoError(t, s.Create(context.Background(), "a", sig))

r := rule.New(
rule.Eq(
rule.StringParam("id"),
rule.StringValue("123"),
),
rule.BoolValue(true),
)

ruleset := createRuleset(t, s, "a", r)
version := ruleset.Version

t.Run("SpecificVersion", func(t *testing.T) {
res, err := s.Eval(context.Background(), "a", version, regula.Params{
"id": "123",
})
require.NoError(t, err)
require.Equal(t, version, res.Version)
require.Equal(t, rule.BoolValue(true), res.Value)
})

t.Run("Latest", func(t *testing.T) {
res, err := s.Eval(context.Background(), "a", "", regula.Params{
"id": "123",
})
require.NoError(t, err)
require.Equal(t, version, res.Version)
require.Equal(t, rule.BoolValue(true), res.Value)
})

t.Run("NotFound", func(t *testing.T) {
_, err := s.Eval(context.Background(), "b", version, regula.Params{
"id": "123",
})
require.Equal(t, errors.ErrRulesetNotFound, err)
})

t.Run("BadVersion", func(t *testing.T) {
_, err := s.Eval(context.Background(), "a", "someversion", regula.Params{
"id": "123",
})
require.Equal(t, errors.ErrRulesetNotFound, err)
})
}
34 changes: 21 additions & 13 deletions api/etcd/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/pkg/errors"
)

// Get returns the ruleset related to the given path. By default, it returns the latest one.
// Get returns the ruleset related to the given path.By default, it returns the latest one.
// It returns the related ruleset version if it's specified.
func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula.Ruleset, error) {
if path == "" {
Expand All @@ -20,14 +20,15 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula

ops := []clientv3.Op{
clientv3.OpGet(s.signaturesPath(path)),
clientv3.OpGet(s.versionsPath(path)),
clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithPrefix(), clientv3.WithKeysOnly()),
}

if version == "" {
ops = append(ops, clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithLastKey()...))
} else {
ops = append(ops, clientv3.OpGet(s.rulesPath(path, version)))
}

// running all the requests within a single transaction so only one network round trip is performed.
resp, err := s.Client.KV.Txn(ctx).Then(ops...).Commit()
if err != nil {
Expand All @@ -49,28 +50,35 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula
return nil, errors.Wrap(err, "failed to unmarshal signature")
}

// decode versions
var versions pb.Versions
err = proto.Unmarshal(resp.Responses[1].GetResponseRange().Kvs[0].Value, &versions)
if err != nil {
return nil, errors.Wrap(err, "failed to unmarshal versions")
// the versions must be parsed from the keys returned by the second operation
asdine marked this conversation as resolved.
Show resolved Hide resolved
kvs := resp.Responses[1].GetResponseRange().Kvs
versions := make([]string, len(kvs))
for i, kv := range kvs {
_, versions[i] = s.pathVersionFromKey(string(kv.Key))
}

// decode rules
// decode rules, might not be filled if only the signature was created
var rules pb.Rules
err = proto.Unmarshal(resp.Responses[2].GetResponseRange().Kvs[0].Value, &rules)
if err != nil {
return nil, errors.Wrap(err, "failed to unmarshal ruleset")
if resp.Responses[2].GetResponseRange().Count > 0 {
err = proto.Unmarshal(resp.Responses[2].GetResponseRange().Kvs[0].Value, &rules)
if err != nil {
return nil, errors.Wrap(err, "failed to unmarshal rules")
}
} else if version != "" {
// if no rules are returned but a version is specified, it means that the version doesn't exist
return nil, api.ErrRulesetNotFound
}

// if the version wasn't specified, get the latest returned
if version == "" {
_, version = s.pathVersionFromKey(string(resp.Responses[2].GetResponseRange().Kvs[0].Key))
version = versions[len(versions)-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also have stored the result of the len we did on the kvs above. Micro-optimisation, mind you ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, IIRC len is constant time, unlike C's strlen which is O(n) and justifies caching the length to reuse it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not just constant time - it can even be registerised if the compiler decides it's worth it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I knew it was constant time, but pushing it to the register, I didn't.

}

return &regula.Ruleset{
Path: path,
Version: version,
Rules: rulesFromProtobuf(&rules),
Signature: signatureFromProtobuf(&sig),
Versions: versions.Versions,
Versions: versions,
}, nil
}
76 changes: 76 additions & 0 deletions api/etcd/get_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package etcd

import (
"context"
"testing"
"time"

"github.com/heetch/regula"
"github.com/heetch/regula/api"
"github.com/heetch/regula/rule"
"github.com/stretchr/testify/require"
)

func TestGet(t *testing.T) {
t.Parallel()

s, cleanup := newEtcdRulesetService(t)
defer cleanup()

path := "p/a/t/h"
sig := &regula.Signature{ReturnType: "bool", Params: make(map[string]string)}

t.Run("OK", func(t *testing.T) {
rules1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))}
createBoolRuleset(t, s, path, rules1...)

ruleset1, err := s.Get(context.Background(), path, "")
require.NoError(t, err)
require.Equal(t, path, ruleset1.Path)
require.Len(t, ruleset1.Versions, 1)
require.NotEmpty(t, rules1, ruleset1.Version)
require.Equal(t, rules1, ruleset1.Rules)
require.Equal(t, sig, ruleset1.Signature)

// we are waiting 1 second here to avoid creating the new version in the same second as the previous one
asdine marked this conversation as resolved.
Show resolved Hide resolved
// ksuid gives a sorting with a one second precision
time.Sleep(time.Second)
rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("foo")), rule.BoolValue(true))}
createBoolRuleset(t, s, path, rules2...)

// it should return the second version
ruleset2, err := s.Get(context.Background(), path, "")
require.NoError(t, err)
require.Equal(t, path, ruleset2.Path)
require.Len(t, ruleset2.Versions, 2)
require.Equal(t, ruleset1.Version, ruleset2.Versions[0])
require.Equal(t, ruleset2.Version, ruleset2.Versions[1])
require.Equal(t, rules2, ruleset2.Rules)
require.Equal(t, sig, ruleset2.Signature)

// it should return the first version
rs, err := s.Get(context.Background(), path, ruleset1.Version)
require.NoError(t, err)
require.Equal(t, ruleset1.Path, rs.Path)
require.Equal(t, ruleset1.Version, rs.Version)
require.Equal(t, ruleset1.Rules, rs.Rules)

// it should return the second version
rs, err = s.Get(context.Background(), path, ruleset2.Version)
require.NoError(t, err)
require.Equal(t, ruleset2, rs)
})

t.Run("Path Not found", func(t *testing.T) {
_, err := s.Get(context.Background(), "doesntexist", "")
require.Equal(t, err, api.ErrRulesetNotFound)
})

t.Run("Version Not found", func(t *testing.T) {
rules := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))}
createBoolRuleset(t, s, path, rules...)

_, err := s.Get(context.Background(), path, "doesntexist")
require.Equal(t, err, api.ErrRulesetNotFound)
})
}
Loading