Git: code refactor. replace Sleep with Ticker

config/setup/git_test.go

@@ -87,7 +87,7 @@ No new changes.`
 		}
 
 		// stop background thread monitor
-		git.Monitor.StopAndWait(repo.URL, 1)
+		git.Services.Stop(repo.URL, 1)
 
 	}
 

middleware/git/git.go

@@ -33,8 +33,9 @@ var initMutex = sync.Mutex{}
 // Logger is used to log errors; if nil, the default log.Logger is used.
 var Logger *log.Logger
 
-// Monitor listens for halt signal to stop repositories from auto pulling.
-var Monitor = &monitor{}
+// Services holds all git pulling services and provides the function to
+// stop them.
+var Services = &services{}
 
 // logger is an helper function to retrieve the available logger
 func logger() *log.Logger {

middleware/git/service.go

@@ -5,106 +5,81 @@ import (
 	"time"
 )
 
-// RepoService is the repository service that runs in background and
-// periodic pull from the repository.
-type RepoService struct {
+// repoService is the service that runs in background and periodically
+// pull from the repository.
+type repoService struct {
 	repo    *Repo
+	ticker  *time.Ticker  // ticker to tick at intervals
 	running bool          // whether service is running.
 	halt    chan struct{} // channel to notify service to halt and stop pulling.
-	exit    chan struct{} // channel to notify on exit.
 }
 
-// Start starts a new RepoService in background and adds it to monitor.
+// Start starts a new background service to pull periodically.
 func Start(repo *Repo) {
-	service := &RepoService{
+	service := &repoService{
 		repo,
+		time.NewTicker(repo.Interval),
 		true,
 		make(chan struct{}),
-		make(chan struct{}),
 	}
-
-	// start service
-	go func(s *RepoService) {
+	go func(s *repoService) {
 		for {
-			// if service is halted
-			if !s.running {
-				// notify exit channel
-				service.exit <- struct{}{}
-				break
-			}
-			time.Sleep(repo.Interval)
-
-			err := repo.Pull()
-			if err != nil {
-				logger().Println(err)
+			select {
+			case <-s.ticker.C:
+				err := repo.Pull()
+				if err != nil {
+					logger().Println(err)
+				}
+			case <-s.halt:
+				s.ticker.Stop()
+				return
 			}
 		}
 	}(service)
 
-	// add to monitor to enable halting
-	Monitor.add(service)
+	// add to services to make it stoppable
+	Services.add(service)
 }
 
-// monitor monitors running services (RepoService)
-// and can halt them.
-type monitor struct {
-	services []*RepoService
+// services stores all repoServices
+type services struct {
+	services []*repoService
 	sync.Mutex
 }
 
-// add adds a new service to the monitor.
-func (m *monitor) add(service *RepoService) {
-	m.Lock()
-	defer m.Unlock()
+// add adds a new service to list of services.
+func (s *services) add(r *repoService) {
+	s.Lock()
+	defer s.Unlock()
 
-	m.services = append(m.services, service)
-
-	// start a goroutine to listen for halt signal
-	service.running = true
-	go func(r *RepoService) {
-		<-r.halt
-		r.running = false
-	}(service)
+	s.services = append(s.services, r)
 }
 
-// Stop stops at most `limit` currently running services that is pulling from git repo at
-// repoURL. It returns list of exit channels for the services. A wait for message on the
-// channels guarantees exit. If limit is less than zero, it is ignored.
+// Stop stops at most `limit` running services pulling from git repo at
+// repoURL. It waits until the service is terminated before returning.
+// If limit is less than zero, it is ignored.
 // TODO find better ways to identify repos
-func (m *monitor) Stop(repoURL string, limit int) []chan struct{} {
-	m.Lock()
-	defer m.Unlock()
+func (s *services) Stop(repoURL string, limit int) {
+	s.Lock()
+	defer s.Unlock()
 
-	var chans []chan struct{}
-
-	// locate services
-	for i, j := 0, 0; i < len(m.services) && ((limit >= 0 && j < limit) || limit < 0); i++ {
-		s := m.services[i]
-		if s.repo.URL == repoURL {
+	// locate repos
+	for i, j := 0, 0; i < len(s.services) && ((limit >= 0 && j < limit) || limit < 0); i++ {
+		service := s.services[i]
+		if service.repo.URL == repoURL {
 			// send halt signal
-			s.halt <- struct{}{}
-			chans = append(chans, s.exit)
+			service.halt <- struct{}{}
+			s.services[i] = nil
 			j++
-			m.services[i] = nil
 		}
 	}
 
-	// remove them from services list
-	services := m.services[:0]
-	for _, s := range m.services {
+	// remove them from repos list
+	services := s.services[:0]
+	for _, s := range s.services {
 		if s != nil {
 			services = append(services, s)
 		}
 	}
-	m.services = services
-	return chans
-}
-
-// StopAndWait is similar to stop but it waits for the services to terminate before
-// returning.
-func (m *monitor) StopAndWait(repoUrl string, limit int) {
-	chans := m.Stop(repoUrl, limit)
-	for _, c := range chans {
-		<-c
-	}
+	s.services = services
 }

middleware/git/service_test.go

@@ -16,45 +16,45 @@ func Test(t *testing.T) {
 	repo := &Repo{URL: "git@github.com", Interval: time.Second}
 
 	Start(repo)
-	if len(Monitor.services) != 1 {
-		t.Errorf("Expected 1 service, found %v", len(Monitor.services))
+	if len(Services.services) != 1 {
+		t.Errorf("Expected 1 service, found %v", len(Services.services))
 	}
 
-	Monitor.StopAndWait(repo.URL, 1)
-	if len(Monitor.services) != 0 {
-		t.Errorf("Expected 1 service, found %v", len(Monitor.services))
+	Services.Stop(repo.URL, 1)
+	if len(Services.services) != 0 {
+		t.Errorf("Expected 1 service, found %v", len(Services.services))
 	}
 
 	repos := make([]*Repo, 5)
 	for i := 0; i < 5; i++ {
 		repos[i] = &Repo{URL: fmt.Sprintf("test%v", i), Interval: time.Second * 2}
 		Start(repos[i])
-		if len(Monitor.services) != i+1 {
-			t.Errorf("Expected %v service(s), found %v", i+1, len(Monitor.services))
+		if len(Services.services) != i+1 {
+			t.Errorf("Expected %v service(s), found %v", i+1, len(Services.services))
 		}
 	}
 
 	time.Sleep(time.Second * 5)
-	Monitor.StopAndWait(repos[0].URL, 1)
-	if len(Monitor.services) != 4 {
-		t.Errorf("Expected %v service(s), found %v", 4, len(Monitor.services))
+	Services.Stop(repos[0].URL, 1)
+	if len(Services.services) != 4 {
+		t.Errorf("Expected %v service(s), found %v", 4, len(Services.services))
 	}
 
 	repo = &Repo{URL: "git@github.com", Interval: time.Second}
 	Start(repo)
-	if len(Monitor.services) != 5 {
-		t.Errorf("Expected %v service(s), found %v", 5, len(Monitor.services))
+	if len(Services.services) != 5 {
+		t.Errorf("Expected %v service(s), found %v", 5, len(Services.services))
 	}
 
 	repo = &Repo{URL: "git@github.com", Interval: time.Second * 2}
 	Start(repo)
-	if len(Monitor.services) != 6 {
-		t.Errorf("Expected %v service(s), found %v", 6, len(Monitor.services))
+	if len(Services.services) != 6 {
+		t.Errorf("Expected %v service(s), found %v", 6, len(Services.services))
 	}
 
 	time.Sleep(time.Second * 5)
-	Monitor.StopAndWait(repo.URL, -1)
-	if len(Monitor.services) != 4 {
-		t.Errorf("Expected %v service(s), found %v", 4, len(Monitor.services))
+	Services.Stop(repo.URL, -1)
+	if len(Services.services) != 4 {
+		t.Errorf("Expected %v service(s), found %v", 4, len(Services.services))
 	}
 }