git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request #93 from 18F/watcher-done

Provide graceful shutdown of file watcher in tests

commitaca1fe81f4bc73a30f92362ba5d344fcfcaa6a6b authorJehiah Czebotar <jehiah@gmail.com> dateMon 18 May 2015, 17:16 (11 years ago) parent254b26d020a35e
5 files changed+41−23
compare with master
unifiedsplit
validator.go+5−5 validator_test.go+20−5 validator_watcher_copy_test.go+1−2 validator_watcher_test.go+2−4 watcher.go+13−7
validator.go+5−5
@@ -15,13 +15,13 @@ type UserMap struct {
1515 m unsafe.Pointer
1616}
1717
18−func NewUserMap(usersFile string, onUpdate func()) *UserMap {
18+func NewUserMap(usersFile string, done <-chan bool, onUpdate func()) *UserMap {
1919 um := &UserMap{usersFile: usersFile}
2020 m := make(map[string]bool)
2121 atomic.StorePointer(&um.m, unsafe.Pointer(&m))
2222 if usersFile != "" {
2323 log.Printf("using authenticated emails file %s", usersFile)
24− started := WatchForUpdates(usersFile, func() {
24+ started := WatchForUpdates(usersFile, done, func() {
2525 um.LoadAuthenticatedEmailsFile()
2626 onUpdate()
2727 })
@@ -62,8 +62,8 @@ func (um *UserMap) LoadAuthenticatedEmailsFile() {
6262}
6363
6464func newValidatorImpl(domains []string, usersFile string,
65− onUpdate func()) func(string) bool {
66− validUsers := NewUserMap(usersFile, onUpdate)
65+ done <-chan bool, onUpdate func()) func(string) bool {
66+ validUsers := NewUserMap(usersFile, done, onUpdate)
6767
6868 for i, domain := range domains {
6969 domains[i] = fmt.Sprintf("@%s", strings.ToLower(domain))
@@ -85,5 +85,5 @@ func newValidatorImpl(domains []string, usersFile string,
8585}
8686
8787func NewValidator(domains []string, usersFile string) func(string) bool {
88− return newValidatorImpl(domains, usersFile, func() {})
88+ return newValidatorImpl(domains, usersFile, nil, func() {})
8989}
validator_test.go+20−5
@@ -9,6 +9,8 @@ import (
99
1010type ValidatorTest struct {
1111 auth_email_file *os.File
12+ done chan bool
13+ update_seen bool
1214}
1315
1416func NewValidatorTest(t *testing.T) *ValidatorTest {
@@ -18,13 +20,26 @@ func NewValidatorTest(t *testing.T) *ValidatorTest {
1820 if err != nil {
1921 t.Fatal("failed to create temp file: " + err.Error())
2022 }
23+ vt.done = make(chan bool)
2124 return vt
2225}
2326
2427func (vt *ValidatorTest) TearDown() {
28+ vt.done <- true
2529 os.Remove(vt.auth_email_file.Name())
2630}
2731
32+func (vt *ValidatorTest) NewValidator(domains []string,
33+ updated chan<- bool) func(string) bool {
34+ return newValidatorImpl(domains, vt.auth_email_file.Name(),
35+ vt.done, func() {
36+ if vt.update_seen == false {
37+ updated <- true
38+ vt.update_seen = true
39+ }
40+ })
41+}
42+
2843// This will close vt.auth_email_file.
2944func (vt *ValidatorTest) WriteEmails(t *testing.T, emails []string) {
3045 defer vt.auth_email_file.Close()
@@ -41,7 +56,7 @@ func TestValidatorEmpty(t *testing.T) {
4156
4257 vt.WriteEmails(t, []string(nil))
4358 domains := []string(nil)
44− validator := NewValidator(domains, vt.auth_email_file.Name())
59+ validator := vt.NewValidator(domains, nil)
4560
4661 if validator("foo.bar@example.com") {
4762 t.Error("nothing should validate when the email and " +
@@ -55,7 +70,7 @@ func TestValidatorSingleEmail(t *testing.T) {
5570
5671 vt.WriteEmails(t, []string{"foo.bar@example.com"})
5772 domains := []string(nil)
58− validator := NewValidator(domains, vt.auth_email_file.Name())
73+ validator := vt.NewValidator(domains, nil)
5974
6075 if !validator("foo.bar@example.com") {
6176 t.Error("email should validate")
@@ -72,7 +87,7 @@ func TestValidatorSingleDomain(t *testing.T) {
7287
7388 vt.WriteEmails(t, []string(nil))
7489 domains := []string{"example.com"}
75− validator := NewValidator(domains, vt.auth_email_file.Name())
90+ validator := vt.NewValidator(domains, nil)
7691
7792 if !validator("foo.bar@example.com") {
7893 t.Error("email should validate")
@@ -91,7 +106,7 @@ func TestValidatorMultipleEmailsMultipleDomains(t *testing.T) {
91106 "plugh@example.com",
92107 })
93108 domains := []string{"example0.com", "example1.com"}
94− validator := NewValidator(domains, vt.auth_email_file.Name())
109+ validator := vt.NewValidator(domains, nil)
95110
96111 if !validator("foo.bar@example0.com") {
97112 t.Error("email from first domain should validate")
@@ -117,7 +132,7 @@ func TestValidatorComparisonsAreCaseInsensitive(t *testing.T) {
117132
118133 vt.WriteEmails(t, []string{"Foo.Bar@Example.Com"})
119134 domains := []string{"Frobozz.Com"}
120− validator := NewValidator(domains, vt.auth_email_file.Name())
135+ validator := vt.NewValidator(domains, nil)
121136
122137 if !validator("foo.bar@example.com") {
123138 t.Error("loaded email addresses are not lower-cased")
validator_watcher_copy_test.go+1−2
@@ -34,8 +34,7 @@ func TestValidatorOverwriteEmailListViaCopyingOver(t *testing.T) {
3434 vt.WriteEmails(t, []string{"xyzzy@example.com"})
3535 domains := []string(nil)
3636 updated := make(chan bool)
37− validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
38− func() { updated <- true })
37+ validator := vt.NewValidator(domains, updated)
3938
4039 if !validator("xyzzy@example.com") {
4140 t.Error("email in list should validate")
validator_watcher_test.go+2−4
@@ -51,8 +51,7 @@ func TestValidatorOverwriteEmailListDirectly(t *testing.T) {
5151 })
5252 domains := []string(nil)
5353 updated := make(chan bool)
54− validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
55− func() { updated <- true })
54+ validator := vt.NewValidator(domains, updated)
5655
5756 if !validator("xyzzy@example.com") {
5857 t.Error("first email in list should validate")
@@ -89,8 +88,7 @@ func TestValidatorOverwriteEmailListViaRenameAndReplace(t *testing.T) {
8988 vt.WriteEmails(t, []string{"xyzzy@example.com"})
9089 domains := []string(nil)
9190 updated := make(chan bool)
92− validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
93− func() { updated <- true })
91+ validator := vt.NewValidator(domains, updated)
9492
9593 if !validator("xyzzy@example.com") {
9694 t.Error("email in list should validate")
watcher.go+13−7
@@ -12,17 +12,18 @@ import (
1212 "gopkg.in/fsnotify.v1"
1313)
1414
15−func WaitForReplacement(event fsnotify.Event, watcher *fsnotify.Watcher) {
15+func WaitForReplacement(filename string, op fsnotify.Op,
16+ watcher *fsnotify.Watcher) {
1617 const sleep_interval = 50 * time.Millisecond
1718
1819 // Avoid a race when fsnofity.Remove is preceded by fsnotify.Chmod.
19− if event.Op&fsnotify.Chmod != 0 {
20+ if op&fsnotify.Chmod != 0 {
2021 time.Sleep(sleep_interval)
2122 }
2223 for {
23− if _, err := os.Stat(event.Name); err == nil {
24− if err := watcher.Add(event.Name); err == nil {
25− log.Printf("watching resumed for %s", event.Name)
24+ if _, err := os.Stat(filename); err == nil {
25+ if err := watcher.Add(filename); err == nil {
26+ log.Printf("watching resumed for %s", filename)
2627 return
2728 }
2829 }
@@ -30,7 +31,7 @@ func WaitForReplacement(event fsnotify.Event, watcher *fsnotify.Watcher) {
3031 }
3132}
3233
33−func WatchForUpdates(filename string, action func()) bool {
34+func WatchForUpdates(filename string, done <-chan bool, action func()) bool {
3435 filename = filepath.Clean(filename)
3536 watcher, err := fsnotify.NewWatcher()
3637 if err != nil {
@@ -40,6 +41,10 @@ func WatchForUpdates(filename string, action func()) bool {
4041 defer watcher.Close()
4142 for {
4243 select {
44+ case _ = <-done:
45+ log.Printf("Shutting down watcher for: %s",
46+ filename)
47+ return
4348 case event := <-watcher.Events:
4449 // On Arch Linux, it appears Chmod events precede Remove events,
4550 // which causes a race between action() and the coming Remove event.
@@ -48,7 +53,8 @@ func WatchForUpdates(filename string, action func()) bool {
4853 // can't be opened.
4954 if event.Op&(fsnotify.Remove|fsnotify.Rename|fsnotify.Chmod) != 0 {
5055 log.Printf("watching interrupted on event: %s", event)
51− WaitForReplacement(event, watcher)
56+ watcher.Remove(filename)
57+ WaitForReplacement(filename, event.Op, watcher)
5258 }
5359 log.Printf("reloading after event: %s", event)
5460 action()