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

fix: Evict policies from cache after disable or enable #1711

Merged
merged 1 commit into from
Jul 24, 2023
Merged

fix: Evict policies from cache after disable or enable #1711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/compile/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (c *Manager) processUpdateQueue(ctx context.Context) {
case storage.EventReload:
c.log.Info("Purging compile cache")
c.cache.Purge()
case storage.EventAddOrUpdatePolicy, storage.EventDeletePolicy:
case storage.EventAddOrUpdatePolicy, storage.EventDeleteOrDisablePolicy:
if err := c.recompile(evt); err != nil {
c.log.Warnw("Error while processing storage event", "event", evt, "error", err)
}
Expand All @@ -106,7 +106,7 @@ func (c *Manager) processUpdateQueue(ctx context.Context) {

func (c *Manager) recompile(evt storage.Event) error {
// if this is a delete event, remove the module from the cache
if evt.Kind == storage.EventDeletePolicy {
if evt.Kind == storage.EventDeleteOrDisablePolicy {
c.evict(evt.PolicyID)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/compile/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func TestManager(t *testing.T) {
require.NotNil(t, rps1)

// send event to trigger recompiliation
mockStore.subscriber.OnStorageEvent(storage.Event{Kind: storage.EventDeletePolicy, PolicyID: dr.ID})
mockStore.subscriber.OnStorageEvent(storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: dr.ID})

yield()

Expand Down
2 changes: 1 addition & 1 deletion internal/storage/blob/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func TestStore_updateIndex(t *testing.T) {
Kind: storage.EventAddOrUpdatePolicy,
}
deleteEvent := storage.Event{
Kind: storage.EventDeletePolicy,
Kind: storage.EventDeleteOrDisablePolicy,
}
store.idx = &mockIndex{
addOrUpdate: func(entry index.Entry) (storage.Event, error) {
Expand Down
8 changes: 4 additions & 4 deletions internal/storage/db/internal/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@

events := make([]storage.Event, len(policyKey))
for i, pk := range policyKey {
events[i] = storage.Event{Kind: storage.EventAddOrUpdatePolicy, PolicyID: namer.GenModuleIDFromFQN(namer.FQNFromPolicyKey(pk))}
events[i] = storage.NewPolicyEvent(storage.EventDeleteOrDisablePolicy, namer.GenModuleIDFromFQN(namer.FQNFromPolicyKey(pk)))

Check warning on line 597 in internal/storage/db/internal/db.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/db/internal/db.go#L597 

Added line #L597 was not covered by tests
}
res, err := s.db.Update(PolicyTbl).Prepared(true).
Set(goqu.Record{PolicyTblDisabledCol: true}).
Expand All @@ -618,7 +618,7 @@
events := make([]storage.Event, len(policyKey))
for idx, pk := range policyKey {
mIDs[idx] = namer.GenModuleIDFromFQN(namer.FQNFromPolicyKey(pk))
events[idx] = storage.Event{Kind: storage.EventAddOrUpdatePolicy, PolicyID: namer.GenModuleIDFromFQN(namer.FQNFromPolicyKey(pk))}
events[idx] = storage.NewPolicyEvent(storage.EventAddOrUpdatePolicy, namer.GenModuleIDFromFQN(namer.FQNFromPolicyKey(pk)))

Check warning on line 621 in internal/storage/db/internal/db.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/db/internal/db.go#L621 

Added line #L621 was not covered by tests
}

res, err := s.db.Update(PolicyTbl).Prepared(true).
Expand Down Expand Up @@ -647,7 +647,7 @@
return err
}

s.NotifySubscribers(storage.NewPolicyEvent(storage.EventDeletePolicy, ids[0]))
s.NotifySubscribers(storage.NewPolicyEvent(storage.EventDeleteOrDisablePolicy, ids[0]))

Check warning on line 650 in internal/storage/db/internal/db.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/db/internal/db.go#L650 

Added line #L650 was not covered by tests

return nil
}
Expand All @@ -657,7 +657,7 @@

for i, id := range ids {
idList[i] = id
events[i] = storage.Event{Kind: storage.EventDeletePolicy, PolicyID: id}
events[i] = storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: id}

Check warning on line 660 in internal/storage/db/internal/db.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/db/internal/db.go#L660 

Added line #L660 was not covered by tests
}
_, err := s.db.Delete(PolicyTbl).Prepared(true).
Where(goqu.C(PolicyTblIDCol).In(idList...)).
Expand Down
2 changes: 1 addition & 1 deletion internal/storage/db/internal/tests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@
require.NoError(t, err)
require.Empty(t, have)

checkEvents(t, timeout, storage.Event{Kind: storage.EventDeletePolicy, PolicyID: rpx.ID})
checkEvents(t, timeout, storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: rpx.ID})

Check warning on line 275 in internal/storage/db/internal/tests.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/db/internal/tests.go#L275 

Added line #L275 was not covered by tests
})

t.Run("add_schema", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions internal/storage/disk/dirwatch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestDirWatch(t *testing.T) {
haveEntries := make(chan index.Entry, 8)
mockIdx.On("Delete", mock.Anything).Return(func(entry index.Entry) storage.Event {
haveEntries <- entry
return storage.Event{Kind: storage.EventDeletePolicy, PolicyID: entry.Policy.ID}
return storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: entry.Policy.ID}
}, nil)

checkEvents := storage.TestSubscription(subMgr)
Expand All @@ -111,7 +111,7 @@ func TestDirWatch(t *testing.T) {
// Check expectations
mockIdx.AssertExpectations(t)

wantEvent := storage.Event{Kind: storage.EventDeletePolicy}
wantEvent := storage.Event{Kind: storage.EventDeleteOrDisablePolicy}
checkEvents(t, timeOut, wantEvent)
})

Expand Down
2 changes: 1 addition & 1 deletion internal/storage/git/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func (s *Store) updateIndex(ctx context.Context) error {
switch fromType {
case util.FileTypePolicy:
s.log.Debugf("Removing policy %s", fromPath)
if err := s.applyIndexUpdate(c.From, storage.EventDeletePolicy); err != nil {
if err := s.applyIndexUpdate(c.From, storage.EventDeleteOrDisablePolicy); err != nil {
return err
}

Expand Down
6 changes: 3 additions & 3 deletions internal/storage/git/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func TestUpdateStore(t *testing.T) {

wantEvents := make([]storage.Event, 0, len(pset))
for _, p := range pset {
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventDeletePolicy, PolicyID: namer.GenModuleID(p)})
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: namer.GenModuleID(p)})
}

checkEvents(t, timeout, wantEvents...)
Expand Down Expand Up @@ -390,7 +390,7 @@ func TestUpdateStore(t *testing.T) {

wantEvents := make([]storage.Event, 0, len(pset))
for _, p := range pset {
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventDeletePolicy, PolicyID: namer.GenModuleID(p)})
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: namer.GenModuleID(p)})
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventAddOrUpdatePolicy, PolicyID: namer.GenModuleID(p)})
}

Expand Down Expand Up @@ -435,7 +435,7 @@ func TestUpdateStore(t *testing.T) {

wantEvents := make([]storage.Event, 0, len(pset))
for _, p := range pset {
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventDeletePolicy, PolicyID: namer.GenModuleID(p)})
wantEvents = append(wantEvents, storage.Event{Kind: storage.EventDeleteOrDisablePolicy, PolicyID: namer.GenModuleID(p)})
}

checkEvents(t, timeout, wantEvents...)
Expand Down
2 changes: 1 addition & 1 deletion internal/storage/index/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@
// nothing to do because we don't have that file in the index.
return storage.Event{Kind: storage.EventNop}, nil
}
evt := storage.NewPolicyEvent(storage.EventDeletePolicy, modID)
evt := storage.NewPolicyEvent(storage.EventDeleteOrDisablePolicy, modID)

Check warning on line 339 in internal/storage/index/index.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/index/index.go#L339 

Added line #L339 was not covered by tests

// go through the dependencies and remove self from the dependents list for each dependency.
if deps, ok := idx.dependencies[modID]; ok {
Expand Down
6 changes: 3 additions & 3 deletions internal/storage/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@

const (
EventAddOrUpdatePolicy EventKind = iota
EventDeletePolicy
EventDeleteOrDisablePolicy
EventAddOrUpdateSchema
EventDeleteSchema
EventReload
Expand All @@ -210,8 +210,8 @@
switch evt.Kind {
case EventAddOrUpdatePolicy:
kind = "ADD/UPDATE"
case EventDeletePolicy:
kind = "DELETE"
case EventDeleteOrDisablePolicy:
kind = "DELETE/DISABLE"

Check warning on line 214 in internal/storage/store.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/store.go#L213-L214 

Added lines #L213 - L214 were not covered by tests
case EventAddOrUpdateSchema:
kind = "ADD/UPDATE SCHEMA"
id = evt.SchemaFile
Expand Down