Skip to content

Commit

Permalink
Remove storage calls from local.Driver.alterModules (#184)
Browse files Browse the repository at this point in the history
* Remove storage calls from local.Driver.alterModules

Signed-off-by: davis-haba <davishaba@google.com>

* Remove PutModules StorageError tests, since PutModules no longer writes to storage

Signed-off-by: davis-haba <davishaba@google.com>

* Remove unused helper func getModules from client.drivers.local

Signed-off-by: davis-haba <davishaba@google.com>

* remove unused test helpers in driver.local

Signed-off-by: davis-haba <davishaba@google.com>
  • Loading branch information
davis-haba committed Jan 21, 2022
1 parent cb43228 commit 5d06ded
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 110 deletions.
29 changes: 1 addition & 28 deletions constraint/pkg/client/drivers/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,6 @@ func (d *Driver) putModules(namePrefix string, srcs []string) error {
// the provided modules then returns the count of modules removed.
// alterModules expects that the caller is holding the modulesMux lock.
func (d *Driver) alterModules(insert insertParam, remove []string) (int, error) {
// TODO(davis-haba): Remove this Context once it is no longer necessary.
ctx := context.TODO()

updatedModules := copyModules(d.modules)
for _, name := range remove {
delete(updatedModules, name)
Expand All @@ -240,38 +237,14 @@ func (d *Driver) alterModules(insert insertParam, remove []string) (int, error)
updatedModules[name] = mod.parsed
}

txn, err := d.storage.NewTransaction(ctx, storage.WriteParams)
if err != nil {
return 0, err
}

for _, name := range remove {
if err := d.storage.DeletePolicy(ctx, txn, name); err != nil {
d.storage.Abort(ctx, txn)
return 0, err
}
}

c := ast.NewCompiler().WithPathConflictsCheck(storage.NonEmpty(ctx, d.storage, txn)).
c := ast.NewCompiler().
WithCapabilities(d.capabilities).
WithEnablePrintStatements(d.printEnabled)

if c.Compile(updatedModules); c.Failed() {
d.storage.Abort(ctx, txn)
return 0, fmt.Errorf("%w: %v", ErrCompile, c.Errors)
}

for name, mod := range insert {
if err := d.storage.UpsertPolicy(ctx, txn, name, []byte(mod.text)); err != nil {
d.storage.Abort(ctx, txn)
return 0, err
}
}

if err := d.storage.Commit(ctx, txn); err != nil {
return 0, err
}

d.compiler = c
d.modules = updatedModules

Expand Down
82 changes: 0 additions & 82 deletions constraint/pkg/client/drivers/local/local_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,67 +276,6 @@ func TestDriver_PutModules(t *testing.T) {
}
}

func TestDriver_PutModules_StorageErrors(t *testing.T) {
testCases := []struct {
name string
storage storage.Store

wantErr bool
}{
{
name: "success",
storage: &fakeStorage{},
wantErr: false,
},
{
name: "failure to create transaction",
storage: &transactionErrorStorage{},
wantErr: true,
},
{
name: "failure to upsert policy",
storage: &upsertErrorStorage{},
wantErr: true,
},
{
name: "failure to commit policy",
storage: &commitErrorStorage{},
wantErr: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
d := New(Storage(tc.storage))

err := d.PutModule("foo", Module)

if tc.wantErr && err == nil {
t.Fatalf("got PutModule() err %v, want error", nil)
} else if !tc.wantErr && err != nil {
t.Fatalf("got PutModule() err %v, want %v", err, nil)
}

dr, ok := d.(*Driver)
if !ok {
t.Fatalf("got New() type = %T, want %T",
d, &Driver{})
}

gotModules := getModules(dr)

var wantModules []string
if !tc.wantErr {
wantModules = []string{"foo"}
}

if diff := cmp.Diff(wantModules, gotModules, cmpopts.EquateEmpty()); diff != "" {
t.Errorf(diff)
}
})
}
}

func TestDriver_DeleteModules(t *testing.T) {
testCases := []struct {
name string
Expand Down Expand Up @@ -868,19 +807,6 @@ func TestDriver_DeleteData_StorageErrors(t *testing.T) {
}
}

func getModules(dr *Driver) []string {
result := make([]string, len(dr.modules))

idx := 0
for module := range dr.modules {
result[idx] = module
idx++
}

sort.Strings(result)
return result
}

type fakeStorage struct {
storage.Store

Expand Down Expand Up @@ -947,14 +873,6 @@ func (s *transactionErrorStorage) NewTransaction(_ context.Context, _ ...storage
return nil, errors.New("error making new transaction")
}

type upsertErrorStorage struct {
fakeStorage
}

func (s *upsertErrorStorage) UpsertPolicy(_ context.Context, _ storage.Transaction, _ string, _ []byte) error {
return errors.New("error upserting policy")
}

type commitErrorStorage struct {
fakeStorage
}
Expand Down

0 comments on commit 5d06ded

Please sign in to comment.