From 0e34c572b48e15d1f8a38555c265fc61ec7de910 Mon Sep 17 00:00:00 2001 From: Vasiliy Tolstov Date: Thu, 4 Jul 2019 00:15:44 +0300 Subject: [PATCH] export registry util function to safe copy registry data Signed-off-by: Vasiliy Tolstov --- registry/cache/rcache.go | 39 +-------------- registry/gossip/gossip.go | 8 +-- registry/memory/helper.go | 76 ----------------------------- registry/memory/helper_test.go | 78 ------------------------------ registry/memory/memory.go | 46 +++++++++--------- registry/{gossip => }/util.go | 58 +++++++++++----------- registry/{gossip => }/util_test.go | 20 ++++---- 7 files changed, 66 insertions(+), 259 deletions(-) delete mode 100644 registry/memory/helper.go delete mode 100644 registry/memory/helper_test.go rename registry/{gossip => }/util.go (59%) rename registry/{gossip => }/util_test.go (72%) diff --git a/registry/cache/rcache.go b/registry/cache/rcache.go index 09ff3c48..cc5ca319 100644 --- a/registry/cache/rcache.go +++ b/registry/cache/rcache.go @@ -80,41 +80,6 @@ func (c *cache) quit() bool { } } -// cp copies a service. Because we're caching handing back pointers would -// create a race condition, so we do this instead its fast enough -func (c *cache) cp(current []*registry.Service) []*registry.Service { - var services []*registry.Service - - for _, service := range current { - // copy service - s := new(registry.Service) - *s = *service - - // copy nodes - var nodes []*registry.Node - for _, node := range service.Nodes { - n := new(registry.Node) - *n = *node - nodes = append(nodes, n) - } - s.Nodes = nodes - - // copy endpoints - var eps []*registry.Endpoint - for _, ep := range service.Endpoints { - e := new(registry.Endpoint) - *e = *ep - eps = append(eps, e) - } - s.Endpoints = eps - - // append service - services = append(services, s) - } - - return services -} - func (c *cache) del(service string) { delete(c.cache, service) delete(c.ttls, service) @@ -132,7 +97,7 @@ func (c *cache) get(service string) ([]*registry.Service, error) { // got services && within ttl so return cache if c.isValid(services, ttl) { // make a copy - cp := c.cp(services) + cp := registry.CopyServices(services) // unlock the read c.RUnlock() // return servics @@ -149,7 +114,7 @@ func (c *cache) get(service string) ([]*registry.Service, error) { // cache results c.Lock() - c.set(service, c.cp(services)) + c.set(service, registry.CopyServices(services)) c.Unlock() return services, nil diff --git a/registry/gossip/gossip.go b/registry/gossip/gossip.go index 45506392..508c9961 100644 --- a/registry/gossip/gossip.go +++ b/registry/gossip/gossip.go @@ -626,7 +626,7 @@ func (g *gossipRegistry) run() { g.services[u.Service.Name] = []*registry.Service{u.Service} } else { - g.services[u.Service.Name] = addServices(service, []*registry.Service{u.Service}) + g.services[u.Service.Name] = registry.AddServices(service, []*registry.Service{u.Service}) } g.Unlock() @@ -645,7 +645,7 @@ func (g *gossipRegistry) run() { case actionTypeDelete: g.Lock() if service, ok := g.services[u.Service.Name]; ok { - if services := delServices(service, []*registry.Service{u.Service}); len(services) == 0 { + if services := registry.DelServices(service, []*registry.Service{u.Service}); len(services) == 0 { delete(g.services, u.Service.Name) } else { g.services[u.Service.Name] = services @@ -706,7 +706,7 @@ func (g *gossipRegistry) Register(s *registry.Service, opts ...registry.Register if service, ok := g.services[s.Name]; !ok { g.services[s.Name] = []*registry.Service{s} } else { - g.services[s.Name] = addServices(service, []*registry.Service{s}) + g.services[s.Name] = registry.AddServices(service, []*registry.Service{s}) } g.Unlock() @@ -754,7 +754,7 @@ func (g *gossipRegistry) Deregister(s *registry.Service) error { g.Lock() if service, ok := g.services[s.Name]; ok { - if services := delServices(service, []*registry.Service{s}); len(services) == 0 { + if services := registry.DelServices(service, []*registry.Service{s}); len(services) == 0 { delete(g.services, s.Name) } else { g.services[s.Name] = services diff --git a/registry/memory/helper.go b/registry/memory/helper.go deleted file mode 100644 index 2308dde7..00000000 --- a/registry/memory/helper.go +++ /dev/null @@ -1,76 +0,0 @@ -package memory - -import ( - "github.com/micro/go-micro/registry" -) - -func addNodes(old, neu []*registry.Node) []*registry.Node { - for _, n := range neu { - var seen bool - for i, o := range old { - if o.Id == n.Id { - seen = true - old[i] = n - break - } - } - if !seen { - old = append(old, n) - } - } - return old -} - -func addServices(old, neu []*registry.Service) []*registry.Service { - for _, s := range neu { - var seen bool - for i, o := range old { - if o.Version == s.Version { - s.Nodes = addNodes(o.Nodes, s.Nodes) - seen = true - old[i] = s - break - } - } - if !seen { - old = append(old, s) - } - } - return old -} - -func delNodes(old, del []*registry.Node) []*registry.Node { - var nodes []*registry.Node - for _, o := range old { - var rem bool - for _, n := range del { - if o.Id == n.Id { - rem = true - break - } - } - if !rem { - nodes = append(nodes, o) - } - } - return nodes -} - -func delServices(old, del []*registry.Service) []*registry.Service { - var services []*registry.Service - for i, o := range old { - var rem bool - for _, s := range del { - if o.Version == s.Version { - old[i].Nodes = delNodes(o.Nodes, s.Nodes) - if len(old[i].Nodes) == 0 { - rem = true - } - } - } - if !rem { - services = append(services, o) - } - } - return services -} diff --git a/registry/memory/helper_test.go b/registry/memory/helper_test.go deleted file mode 100644 index f19d1c52..00000000 --- a/registry/memory/helper_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package memory - -import ( - "testing" - - "github.com/micro/go-micro/registry" -) - -func TestDelServices(t *testing.T) { - services := []*registry.Service{ - { - Name: "foo", - Version: "1.0.0", - Nodes: []*registry.Node{ - { - Id: "foo-123", - Address: "localhost", - Port: 9999, - }, - }, - }, - { - Name: "foo", - Version: "1.0.0", - Nodes: []*registry.Node{ - { - Id: "foo-123", - Address: "localhost", - Port: 6666, - }, - }, - }, - } - - servs := delServices([]*registry.Service{services[0]}, []*registry.Service{services[1]}) - if i := len(servs); i > 0 { - t.Errorf("Expected 0 nodes, got %d: %+v", i, servs) - } - t.Logf("Services %+v", servs) -} - -func TestDelNodes(t *testing.T) { - services := []*registry.Service{ - { - Name: "foo", - Version: "1.0.0", - Nodes: []*registry.Node{ - { - Id: "foo-123", - Address: "localhost", - Port: 9999, - }, - { - Id: "foo-321", - Address: "localhost", - Port: 6666, - }, - }, - }, - { - Name: "foo", - Version: "1.0.0", - Nodes: []*registry.Node{ - { - Id: "foo-123", - Address: "localhost", - Port: 6666, - }, - }, - }, - } - - nodes := delNodes(services[0].Nodes, services[1].Nodes) - if i := len(nodes); i != 1 { - t.Errorf("Expected only 1 node, got %d: %+v", i, nodes) - } - t.Logf("Nodes %+v", nodes) -} diff --git a/registry/memory/memory.go b/registry/memory/memory.go index e159ce61..a978e1ac 100644 --- a/registry/memory/memory.go +++ b/registry/memory/memory.go @@ -22,17 +22,6 @@ var ( timeout = time.Millisecond * 10 ) -/* -// Setup sets mock data -func (m *Registry) Setup() { - m.Lock() - defer m.Unlock() - - // add some memory data - m.Services = Data -} -*/ - func (m *Registry) watch(r *registry.Result) { var watchers []*Watcher @@ -66,7 +55,7 @@ func (m *Registry) Init(opts ...registry.Option) error { m.Lock() for k, v := range getServices(m.options.Context) { s := m.Services[k] - m.Services[k] = addServices(s, v) + m.Services[k] = registry.AddServices(s, v) } m.Unlock() return nil @@ -76,20 +65,20 @@ func (m *Registry) Options() registry.Options { return m.options } -func (m *Registry) GetService(service string) ([]*registry.Service, error) { +func (m *Registry) GetService(name string) ([]*registry.Service, error) { m.RLock() - s, ok := m.Services[service] - if !ok || len(s) == 0 { - m.RUnlock() + service, ok := m.Services[name] + m.RUnlock() + if !ok { return nil, registry.ErrNotFound } - m.RUnlock() - return s, nil + + return service, nil } func (m *Registry) ListServices() ([]*registry.Service, error) { - m.RLock() var services []*registry.Service + m.RLock() for _, service := range m.Services { services = append(services, service...) } @@ -99,11 +88,14 @@ func (m *Registry) ListServices() ([]*registry.Service, error) { func (m *Registry) Register(s *registry.Service, opts ...registry.RegisterOption) error { go m.watch(®istry.Result{Action: "update", Service: s}) - m.Lock() - services := addServices(m.Services[s.Name], []*registry.Service{s}) - m.Services[s.Name] = services + if service, ok := m.Services[s.Name]; !ok { + m.Services[s.Name] = []*registry.Service{s} + } else { + m.Services[s.Name] = registry.AddServices(service, []*registry.Service{s}) + } m.Unlock() + return nil } @@ -111,9 +103,15 @@ func (m *Registry) Deregister(s *registry.Service) error { go m.watch(®istry.Result{Action: "delete", Service: s}) m.Lock() - services := delServices(m.Services[s.Name], []*registry.Service{s}) - m.Services[s.Name] = services + if service, ok := m.Services[s.Name]; ok { + if service := registry.DelServices(service, []*registry.Service{s}); len(service) == 0 { + delete(m.Services, s.Name) + } else { + m.Services[s.Name] = service + } + } m.Unlock() + return nil } diff --git a/registry/gossip/util.go b/registry/util.go similarity index 59% rename from registry/gossip/util.go rename to registry/util.go index 0fa8116d..7954cb0d 100644 --- a/registry/gossip/util.go +++ b/registry/util.go @@ -1,30 +1,26 @@ -package gossip +package registry -import ( - "github.com/micro/go-micro/registry" -) - -func cp(current []*registry.Service) []*registry.Service { - var services []*registry.Service +func CopyServices(current []*Service) []*Service { + var services []*Service for _, service := range current { // copy service - s := new(registry.Service) + s := new(Service) *s = *service // copy nodes - var nodes []*registry.Node + var nodes []*Node for _, node := range service.Nodes { - n := new(registry.Node) + n := new(Node) *n = *node nodes = append(nodes, n) } s.Nodes = nodes // copy endpoints - var eps []*registry.Endpoint + var eps []*Endpoint for _, ep := range service.Endpoints { - e := new(registry.Endpoint) + e := new(Endpoint) *e = *ep eps = append(eps, e) } @@ -37,8 +33,8 @@ func cp(current []*registry.Service) []*registry.Service { return services } -func addNodes(old, neu []*registry.Node) []*registry.Node { - var nodes []*registry.Node +func addServiceNodes(old, neu []*Node) []*Node { + var nodes []*Node // add all new nodes for _, n := range neu { @@ -69,35 +65,39 @@ func addNodes(old, neu []*registry.Node) []*registry.Node { return nodes } -func addServices(old, neu []*registry.Service) []*registry.Service { - var srv []*registry.Service +func AddServices(olist []*Service, nlist []*Service) []*Service { + var srv []*Service - for _, s := range neu { + for _, n := range nlist { var seen bool - for _, o := range old { - if o.Version == s.Version { - sp := new(registry.Service) + for _, o := range olist { + if o.Version == n.Version { + sp := new(Service) // make copy *sp = *o // set nodes - sp.Nodes = addNodes(o.Nodes, s.Nodes) + sp.Nodes = addServiceNodes(o.Nodes, n.Nodes) // mark as seen seen = true srv = append(srv, sp) break + } else { + sp := new(Service) + // make copy + *sp = *o + srv = append(srv, sp) } } if !seen { - srv = append(srv, cp([]*registry.Service{s})...) + srv = append(srv, CopyServices([]*Service{n})...) } } - return srv } -func delNodes(old, del []*registry.Node) []*registry.Node { - var nodes []*registry.Node +func delServiceNodes(old, del []*Node) []*Node { + var nodes []*Node for _, o := range old { var rem bool for _, n := range del { @@ -113,18 +113,18 @@ func delNodes(old, del []*registry.Node) []*registry.Node { return nodes } -func delServices(old, del []*registry.Service) []*registry.Service { - var services []*registry.Service +func DelServices(old, del []*Service) []*Service { + var services []*Service for _, o := range old { - srv := new(registry.Service) + srv := new(Service) *srv = *o var rem bool for _, s := range del { if srv.Version == s.Version { - srv.Nodes = delNodes(srv.Nodes, s.Nodes) + srv.Nodes = delServiceNodes(srv.Nodes, s.Nodes) if len(srv.Nodes) == 0 { rem = true diff --git a/registry/gossip/util_test.go b/registry/util_test.go similarity index 72% rename from registry/gossip/util_test.go rename to registry/util_test.go index a77dda52..6e4dccd5 100644 --- a/registry/gossip/util_test.go +++ b/registry/util_test.go @@ -1,17 +1,15 @@ -package gossip +package registry import ( "testing" - - "github.com/micro/go-micro/registry" ) func TestDelServices(t *testing.T) { - services := []*registry.Service{ + services := []*Service{ { Name: "foo", Version: "1.0.0", - Nodes: []*registry.Node{ + Nodes: []*Node{ { Id: "foo-123", Address: "localhost", @@ -22,7 +20,7 @@ func TestDelServices(t *testing.T) { { Name: "foo", Version: "1.0.0", - Nodes: []*registry.Node{ + Nodes: []*Node{ { Id: "foo-123", Address: "localhost", @@ -32,7 +30,7 @@ func TestDelServices(t *testing.T) { }, } - servs := delServices([]*registry.Service{services[0]}, []*registry.Service{services[1]}) + servs := DelServices([]*Service{services[0]}, []*Service{services[1]}) if i := len(servs); i > 0 { t.Errorf("Expected 0 nodes, got %d: %+v", i, servs) } @@ -40,11 +38,11 @@ func TestDelServices(t *testing.T) { } func TestDelNodes(t *testing.T) { - services := []*registry.Service{ + services := []*Service{ { Name: "foo", Version: "1.0.0", - Nodes: []*registry.Node{ + Nodes: []*Node{ { Id: "foo-123", Address: "localhost", @@ -60,7 +58,7 @@ func TestDelNodes(t *testing.T) { { Name: "foo", Version: "1.0.0", - Nodes: []*registry.Node{ + Nodes: []*Node{ { Id: "foo-123", Address: "localhost", @@ -70,7 +68,7 @@ func TestDelNodes(t *testing.T) { }, } - nodes := delNodes(services[0].Nodes, services[1].Nodes) + nodes := delServiceNodes(services[0].Nodes, services[1].Nodes) if i := len(nodes); i != 1 { t.Errorf("Expected only 1 node, got %d: %+v", i, nodes) }