diff --git a/overlord/registrystate/registrystate.go b/overlord/registrystate/registrystate.go index c83b77ae6ee5..ccc8dd2f37ef 100644 --- a/overlord/registrystate/registrystate.go +++ b/overlord/registrystate/registrystate.go @@ -213,13 +213,20 @@ func updateDatabags(st *state.State, databag registry.JSONDataBag, reg *registry return nil } -type cachedRegistryTx struct{} +type cachedRegistryTx struct { + account string + registry string +} // RegistryTransaction returns the registry.Transaction cached in the context // or creates one and caches it, if none existed. The context must be locked by // the caller. func RegistryTransaction(ctx *hookstate.Context, reg *registry.Registry) (*registry.Transaction, error) { - tx, ok := ctx.Cached(cachedRegistryTx{}).(*registry.Transaction) + key := cachedRegistryTx{ + account: reg.Account, + registry: reg.Name, + } + tx, ok := ctx.Cached(key).(*registry.Transaction) if ok { return tx, nil } @@ -233,6 +240,6 @@ func RegistryTransaction(ctx *hookstate.Context, reg *registry.Registry) (*regis return tx.Commit() }) - ctx.Cache(cachedRegistryTx{}, tx) + ctx.Cache(key, tx) return tx, nil } diff --git a/overlord/registrystate/registrystate_test.go b/overlord/registrystate/registrystate_test.go index 893589fb8879..417a17280ecd 100644 --- a/overlord/registrystate/registrystate_test.go +++ b/overlord/registrystate/registrystate_test.go @@ -296,31 +296,73 @@ func (s *registryTestSuite) TestRegistrystateGetEntireView(c *C) { } func (s *registryTestSuite) TestRegistryTransaction(c *C) { - reg, err := registry.New("acc", "foo", map[string]interface{}{ - "bar": map[string]interface{}{ - "rules": []interface{}{ - map[string]interface{}{"request": "foo", "storage": "foo"}, + mkRegistry := func(account, name string) *registry.Registry { + reg, err := registry.New(account, name, map[string]interface{}{ + "bar": map[string]interface{}{ + "rules": []interface{}{ + map[string]interface{}{"request": "foo", "storage": "foo"}, + }, }, - }, - }, registry.NewJSONSchema()) - c.Assert(err, IsNil) + }, registry.NewJSONSchema()) + c.Assert(err, IsNil) + return reg + } s.state.Lock() task := s.state.NewTask("test-task", "my test task") setup := &hookstate.HookSetup{Snap: "test-snap", Revision: snap.R(1), Hook: "test-hook"} s.state.Unlock() - mockHandler := hooktest.NewMockHandler() - ctx, err := hookstate.NewContext(task, task.State(), setup, mockHandler, "") - c.Assert(err, IsNil) - ctx.Lock() - defer ctx.Unlock() + type testcase struct { + acc1, acc2 string + reg1, reg2 string + equals bool + } - tx, err := registrystate.RegistryTransaction(ctx, reg) - c.Assert(err, IsNil) + tcs := []testcase{ + { + // same transaction + acc1: "acc-1", reg1: "reg-1", + acc2: "acc-1", reg2: "reg-1", + equals: true, + }, + { + // different registry name, different transaction + acc1: "acc-1", reg1: "reg-1", + acc2: "acc-1", reg2: "reg-2", + }, + { + // different account, different transaction + acc1: "acc-1", reg1: "reg-1", + acc2: "acc-2", reg2: "reg-1", + }, + { + // both different, different transaction + acc1: "acc-1", reg1: "reg-1", + acc2: "acc-2", reg2: "reg-2", + }, + } - sameTx, err := registrystate.RegistryTransaction(ctx, reg) - c.Assert(err, IsNil) - c.Assert(tx, Equals, sameTx) + for _, tc := range tcs { + ctx, err := hookstate.NewContext(task, task.State(), setup, mockHandler, "") + c.Assert(err, IsNil) + ctx.Lock() + + reg1 := mkRegistry(tc.acc1, tc.reg1) + reg2 := mkRegistry(tc.acc2, tc.reg2) + + tx1, err := registrystate.RegistryTransaction(ctx, reg1) + c.Assert(err, IsNil) + + tx2, err := registrystate.RegistryTransaction(ctx, reg2) + c.Assert(err, IsNil) + + if tc.equals { + c.Assert(tx1, Equals, tx2) + } else { + c.Assert(tx1, Not(Equals), tx2) + } + ctx.Unlock() + } }