From 23d65145e648f658ed721b697ee205fa236ef33e Mon Sep 17 00:00:00 2001
From: Milos Gajdos <milosgajdos83@gmail.com>
Date: Fri, 17 Jan 2020 16:25:18 +0000
Subject: [PATCH] Use the same logic for advertising routes in Router and
 Network

router.Query() allows to query the routes with given router.Strategy.
It uses the same logic as was implemented in flushRoutes but the code
was never updated. This way we are consistent across both router and
network packages.
---
 router/default.go      | 75 ++++++++----------------------------------
 router/default_test.go |  1 -
 2 files changed, 13 insertions(+), 63 deletions(-)

diff --git a/router/default.go b/router/default.go
index 688d5337..07bddc0d 100644
--- a/router/default.go
+++ b/router/default.go
@@ -88,9 +88,10 @@ func (r *router) Init(opts ...Option) error {
 
 // Options returns router options
 func (r *router) Options() Options {
-	r.Lock()
+	r.RLock()
+	defer r.RUnlock()
+
 	options := r.options
-	r.Unlock()
 
 	return options
 }
@@ -733,74 +734,24 @@ func (r *router) Process(a *Advert) error {
 
 // flushRouteEvents returns a slice of events, one per each route in the routing table
 func (r *router) flushRouteEvents(evType EventType) ([]*Event, error) {
-	// Do not advertise anything
-	if r.options.Advertise == AdvertiseNone {
-		return []*Event{}, nil
+	// get a list of routes for each service in our routing table
+	// for the configured advertising strategy
+	q := []QueryOption{
+		QueryStrategy(r.options.Advertise),
 	}
 
-	// list all routes
-	routes, err := r.table.List()
-	if err != nil {
-		return nil, fmt.Errorf("failed listing routes: %s", err)
+	routes, err := r.Table().Query(q...)
+	if err != nil && err != ErrRouteNotFound {
+		return nil, err
 	}
 
-	// Return all the routes
-	if r.options.Advertise == AdvertiseAll {
-		// build a list of events to advertise
-		events := make([]*Event, len(routes))
-		for i, route := range routes {
-			event := &Event{
-				Type:      evType,
-				Timestamp: time.Now(),
-				Route:     route,
-			}
-			events[i] = event
-		}
-		return events, nil
-	}
-
-	// routeMap stores the routes we're going to advertise
-	bestRoutes := make(map[string]Route)
-
-	// set whether we're advertising only local
-	advertiseLocal := r.options.Advertise == AdvertiseLocal
-
-	// go through all routes found in the routing table and collapse them to optimal routes
-	for _, route := range routes {
-		// if we're only advertising local routes
-		if advertiseLocal && route.Link != "local" {
-			continue
-		}
-
-		// now we're going to find the best routes
-
-		routeKey := route.Service + "@" + route.Network
-		current, ok := bestRoutes[routeKey]
-		if !ok {
-			bestRoutes[routeKey] = route
-			continue
-		}
-		// if the current optimal route metric is higher than routing table route, replace it
-		if current.Metric > route.Metric {
-			bestRoutes[routeKey] = route
-			continue
-		}
-		// if the metrics are the same, prefer advertising your own route
-		if current.Metric == route.Metric {
-			if route.Router == r.options.Id {
-				bestRoutes[routeKey] = route
-				continue
-			}
-		}
-	}
-
-	log.Debugf("Router advertising %d %s routes out of %d", len(bestRoutes), r.options.Advertise, len(routes))
+	log.Debugf("Router advertising %d routes with strategy %s", len(routes), r.options.Advertise)
 
 	// build a list of events to advertise
-	events := make([]*Event, len(bestRoutes))
+	events := make([]*Event, len(routes))
 	var i int
 
-	for _, route := range bestRoutes {
+	for _, route := range routes {
 		event := &Event{
 			Type:      evType,
 			Timestamp: time.Now(),
diff --git a/router/default_test.go b/router/default_test.go
index d3edf67d..9427ecbc 100644
--- a/router/default_test.go
+++ b/router/default_test.go
@@ -18,7 +18,6 @@ func routerTestSetup() Router {
 func TestRouterStartStop(t *testing.T) {
 	r := routerTestSetup()
 
-	log.Debugf("TestRouterStartStop STARTING")
 	if err := r.Start(); err != nil {
 		t.Errorf("failed to start router: %v", err)
 	}