From 7dae3def51f63a7e3048889d0016bedf51a15ba5 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Mon, 10 Jul 2023 23:27:18 +0200 Subject: [PATCH 01/10] Adds the GetYearsOld function to the templates --- server/server.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/server.go b/server/server.go index ead52aa..4d1caad 100644 --- a/server/server.go +++ b/server/server.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "text/template" + "time" "git.ctrlz.es/mgdelacroix/birthdaybot/model" "git.ctrlz.es/mgdelacroix/birthdaybot/notification" @@ -94,6 +95,12 @@ func New(options ...Option) (*Server, error) { if err != nil { return nil, fmt.Errorf("cannot parse template file %q: %w", srv.Config.Birthdays.Template, err) } + + srv.tmpl.Funcs(template.FuncMap{ + "GetYearsOld": func(yearOfBirth int) int { + return time.Now().Year() - yearOfBirth + }, + }) } return srv, nil From 1918740563291f33ed6b59b0b97cf036958f086e Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 10:10:56 +0200 Subject: [PATCH 02/10] Properly parses the template functions, and adds some tests --- sample.tmpl | 2 +- server/helpers_test.go | 10 +++++--- server/server.go | 18 ++++++++------ server/server_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/sample.tmpl b/sample.tmpl index 54ebaef..0b40e71 100644 --- a/sample.tmpl +++ b/sample.tmpl @@ -1 +1 @@ -¡Mañana es el cumpleaños de {{.Name}}! Puedes felicitarle o bien escribiendo a {{.Email}} o bien llamando al número {{.Phone}} \ No newline at end of file +¡Mañana es el cumpleaños de {{.Name}}! Cumple {{getYearsOld .YearOfBirth}} años, puedes felicitarle o bien escribiendo a {{.Email}} o bien llamando al número {{.Phone}} \ No newline at end of file diff --git a/server/helpers_test.go b/server/helpers_test.go index 1cdc576..e73e2c7 100644 --- a/server/helpers_test.go +++ b/server/helpers_test.go @@ -31,7 +31,7 @@ func testConfig(t *testing.T) *model.Config { return &model.Config{Birthdays: &model.BirthdaysConfig{File: f.Name()}} } -func SetupTestHelper(t *testing.T) *TestHelper { +func SetupTestHelper(t *testing.T, opts ...Option) *TestHelper { th := &TestHelper{t: t} th.ctrl = gomock.NewController(t) @@ -54,14 +54,16 @@ func SetupTestHelper(t *testing.T) *TestHelper { }, } - var err error - th.srv, err = New( + serverOpts := append([]Option{ WithConfig(testConfig(t)), WithLogger(log.New(os.Stderr)), WithBirthdays(birthdays), WithNotificationServices(notificationServices), WithWorkers(workers), - ) + }, opts...) + + var err error + th.srv, err = New(serverOpts...) require.NoError(t, err) th.srv.Start() diff --git a/server/server.go b/server/server.go index 4d1caad..1c0e715 100644 --- a/server/server.go +++ b/server/server.go @@ -3,6 +3,7 @@ package server import ( "errors" "fmt" + "path" "text/template" "time" @@ -90,17 +91,20 @@ func New(options ...Option) (*Server, error) { if srv.Config.Birthdays.Template != "" { srv.Logger.Debug("parsing birthday template", "file", srv.Config.Birthdays.Template) + funcs := template.FuncMap{ + "getYearsOld": func(yearOfBirth int) int { + return time.Now().Year() - yearOfBirth + }, + } + var err error - srv.tmpl, err = template.ParseFiles(srv.Config.Birthdays.Template) + srv.tmpl, err = template. + New(path.Base(srv.Config.Birthdays.Template)). + Funcs(funcs). + ParseFiles(srv.Config.Birthdays.Template) if err != nil { return nil, fmt.Errorf("cannot parse template file %q: %w", srv.Config.Birthdays.Template, err) } - - srv.tmpl.Funcs(template.FuncMap{ - "GetYearsOld": func(yearOfBirth int) int { - return time.Now().Year() - yearOfBirth - }, - }) } return srv, nil diff --git a/server/server_test.go b/server/server_test.go index 6d8d5d9..3da18b9 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1,9 +1,14 @@ package server import ( + "bytes" "errors" + "fmt" + "os" "testing" + "time" + "git.ctrlz.es/mgdelacroix/birthdaybot/model" "github.com/stretchr/testify/require" ) @@ -35,3 +40,53 @@ func TestNotify(t *testing.T) { require.ErrorIs(t, err, mockErr) }) } + +func TestTemplate(t *testing.T) { + t.Run("template should work with birthday data", func(t *testing.T) { + // create a template file and populate it + f, err := os.CreateTemp("", "birthdaybot-config-") + require.NoError(t, err) + _, werr := fmt.Fprint(f, "My name is {{.Name}}") + require.NoError(t, werr) + require.NoError(t, f.Close()) + + // create a test config and set the template + config := testConfig(t) + config.Birthdays.Template = f.Name() + + // create the test helper with the custom config + th := SetupTestHelper(t, WithConfig(config)) + defer th.TearDown() + + birthday := &model.Birthday{Name: "Jane Doe"} + expectedString := "My name is Jane Doe" + + var stringBuffer bytes.Buffer + require.NoError(t, th.srv.tmpl.Execute(&stringBuffer, birthday)) + require.Equal(t, expectedString, stringBuffer.String()) + }) + + t.Run("template should work with custom functions", func(t *testing.T) { + // create a template file and populate it + f, err := os.CreateTemp("", "birthdaybot-config-") + require.NoError(t, err) + _, werr := fmt.Fprint(f, "I'm getting {{getYearsOld .YearOfBirth}} years old") + require.NoError(t, werr) + require.NoError(t, f.Close()) + + // create a test config and set the template + config := testConfig(t) + config.Birthdays.Template = f.Name() + + // create the test helper with the custom config + th := SetupTestHelper(t, WithConfig(config)) + defer th.TearDown() + + birthday := &model.Birthday{YearOfBirth: 1980} + expectedString := fmt.Sprintf("I'm getting %d years old", time.Now().Year()-birthday.YearOfBirth) + + var stringBuffer bytes.Buffer + require.NoError(t, th.srv.tmpl.Execute(&stringBuffer, birthday)) + require.Equal(t, expectedString, stringBuffer.String()) + }) +} From 7cd86ed42993a5c82c91bd171bab91a850df0289 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 10:21:53 +0200 Subject: [PATCH 03/10] Remove the ToMap birthday method --- README.md | 5 +++++ model/birthdays.go | 11 ----------- notification/service_telegram.go | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index e55c522..24bb886 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,11 @@ Template Format, and has the following properties available: - `.MonthOfBirth`: the month that the person was born, as number. - `.DayOfBirth`: the day that the person was born, as number. +There are as well some functions available to be used: + +- `getYeardsOld`: receives the year that a user was born, and returns + how old the user is getting. + ### Pictures Alongside the notification for each birthday, the bot can send a diff --git a/model/birthdays.go b/model/birthdays.go index cbd64d6..e4a311a 100644 --- a/model/birthdays.go +++ b/model/birthdays.go @@ -19,17 +19,6 @@ func (b *Birthday) Filename() string { return fmt.Sprintf("%d_%d_%d_%s.png", b.YearOfBirth, b.MonthOfBirth, b.DayOfBirth, b.Phone) } -func (b *Birthday) ToMap() map[string]any { - return map[string]any{ - "Name": b.Name, - "Email": b.Email, - "Phone": b.Phone, - "YearOfBirth": b.YearOfBirth, - "MonthOfBirth": b.MonthOfBirth, - "DayOfBirth": b.DayOfBirth, - } -} - func NewBirthdayFromRecord(record []string) (*Birthday, error) { if len(record) != 4 { return nil, fmt.Errorf("invalid length %d for record", len(record)) diff --git a/notification/service_telegram.go b/notification/service_telegram.go index f41c9f4..0dffdb8 100644 --- a/notification/service_telegram.go +++ b/notification/service_telegram.go @@ -43,7 +43,7 @@ func (tns *TelegramNotificationService) Notify(birthday *model.Birthday, templat var msgText string if template != nil { var stringBuffer bytes.Buffer - if err := template.Execute(&stringBuffer, birthday.ToMap()); err != nil { + if err := template.Execute(&stringBuffer, birthday); err != nil { return fmt.Errorf("cannot execute template for birthday: %w", err) } From 8087e91d69f5cead6edfa189d6daae28965f09a1 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 11:30:29 +0200 Subject: [PATCH 04/10] Fix readme typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 24bb886..e615f22 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ Template Format, and has the following properties available: There are as well some functions available to be used: -- `getYeardsOld`: receives the year that a user was born, and returns +- `getYearsOld`: receives the year that a user was born, and returns how old the user is getting. ### Pictures From 045c8760fe8b5f715ec91f3d597b0853a2eec52c Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 12:09:08 +0200 Subject: [PATCH 05/10] Add new tasks to the bot --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index e615f22..dbe72c4 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,10 @@ $ make run - [X] Reduce logger verbosity (through levels) - [X] Add pictures to birthday notifications - [X] Create a configurable template to fill with each notification +- [ ] Add some endpoints + - [ ] Health endpoint + - [ ] Next birthday endpoint + - [ ] Birthday list endpoint - [ ] Create different message systems to use with the bot - [X] Telegram - [ ] Email From 0bd05b6efe01a009919e4955b85ce26156263aa7 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 12:37:56 +0200 Subject: [PATCH 06/10] Adds web endpoint --- cmd/birthdaybot/main.go | 12 +++++++-- example-config.yml | 3 +++ model/config.go | 25 ++++++++++++++++++ server/server.go | 27 +++++++++++++++++-- server/web.go | 58 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 server/web.go diff --git a/cmd/birthdaybot/main.go b/cmd/birthdaybot/main.go index dbbe9cc..fbdb1c6 100644 --- a/cmd/birthdaybot/main.go +++ b/cmd/birthdaybot/main.go @@ -47,8 +47,16 @@ func main() { c := make(chan os.Signal, 1) signal.Notify(c, syscall.SIGINT, syscall.SIGTERM) - srv.Start() + if err := srv.Start(); err != nil { + fmt.Fprintf(os.Stderr, "ERROR: cannot start server: %s\n", err) + os.Exit(1) + } + s := <-c srv.Logger.Info("received signal, stopping", "signal", s) - srv.Stop() + + if err := srv.Stop(); err != nil { + fmt.Fprintf(os.Stderr, "ERROR: cannot stop server: %s\n", err) + os.Exit(1) + } } diff --git a/example-config.yml b/example-config.yml index 34d1a22..57b9ca2 100644 --- a/example-config.yml +++ b/example-config.yml @@ -1,4 +1,7 @@ --- +web: + port: 8080 + birthdays: file: birthdays.csv template: ./birthday_message.tmpl diff --git a/model/config.go b/model/config.go index a0adc7e..469cd4c 100644 --- a/model/config.go +++ b/model/config.go @@ -19,6 +19,7 @@ var ( type Config struct { Birthdays *BirthdaysConfig `yaml:"birthdays"` Logger *LoggerConfig `yaml:"logger"` + Web *WebConfig `yaml:"web"` TelegramNotifications *TelegramNotificationsConfig `yaml:"telegram_notifications"` } @@ -31,6 +32,10 @@ func (c *Config) IsValid() error { return fmt.Errorf("invalid logger config: %w", err) } + if err := c.Web.IsValid(); err != nil { + return fmt.Errorf("invalid web config: %w", err) + } + if c.TelegramNotifications != nil { if err := c.TelegramNotifications.IsValid(); err != nil { return fmt.Errorf("invalid telegram notifications config: %w", err) @@ -53,6 +58,12 @@ func (c *Config) SetDefaults() { c.Logger.SetDefaults() + if c.Web == nil { + c.Web = &WebConfig{} + } + + c.Web.SetDefaults() + if c.TelegramNotifications != nil { c.TelegramNotifications.SetDefaults() } @@ -99,6 +110,20 @@ func (lc *LoggerConfig) IsValid() error { return nil } +type WebConfig struct { + Port int `yaml:"port"` +} + +func (wc *WebConfig) SetDefaults() { + if wc.Port == 0 { + wc.Port = 8080 + } +} + +func (wc *WebConfig) IsValid() error { + return nil +} + type TelegramNotificationsConfig struct { BotToken string `yaml:"bot_token"` ChannelID string `yaml:"channel_id"` diff --git a/server/server.go b/server/server.go index 1c0e715..a54d5db 100644 --- a/server/server.go +++ b/server/server.go @@ -22,6 +22,7 @@ var ( type Server struct { Logger *log.Logger Config *model.Config + WebServer *WebServer workers []Worker birthdays []*model.Birthday notificationServices []notification.NotificationService @@ -107,23 +108,45 @@ func New(options ...Option) (*Server, error) { } } + if srv.WebServer == nil { + srv.Logger.Debug("creating web server") + + srv.WebServer = NewWebServer(srv) + } + return srv, nil } -func (s *Server) Start() { +func (s *Server) Start() error { s.Logger.Info("starting server") + + if err := s.WebServer.Start(); err != nil { + return fmt.Errorf("cannot start web server: %w", err) + } + for _, worker := range s.workers { worker.Start() } + s.Logger.Debug("server started", "workers", len(s.workers)) + + return nil } -func (s *Server) Stop() { +func (s *Server) Stop() error { s.Logger.Info("stopping server") + + if err := s.WebServer.Stop(); err != nil { + return fmt.Errorf("cannot stop web server: %w", err) + } + for _, worker := range s.workers { worker.Stop() } + s.Logger.Debug("server stopped", "workers", len(s.workers)) + + return nil } func (s *Server) Notify(birthday *model.Birthday) error { diff --git a/server/web.go b/server/web.go new file mode 100644 index 0000000..8c5c489 --- /dev/null +++ b/server/web.go @@ -0,0 +1,58 @@ +package server + +import ( + "errors" + "fmt" + "net/http" + + "github.com/charmbracelet/log" +) + +type WebServer struct { + server *Server + logger *log.Logger + httpServer *http.Server +} + +func NewWebServer(server *Server) *WebServer { + ws := &WebServer{ + server: server, + logger: server.Logger, + httpServer: &http.Server{ + Addr: fmt.Sprintf(":%d", server.Config.Web.Port), + }, + } + + mux := http.NewServeMux() + mux.HandleFunc("/health", ws.healthHandler) + + ws.httpServer.Handler = mux + + return ws +} + +func (ws *WebServer) Start() error { + ws.logger.Debug("starting web server") + + go func() { + if err := ws.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + ws.logger.Fatal("cannot start web server", "error", err) + } + }() + + return nil +} + +func (ws *WebServer) Stop() error { + ws.logger.Debug("stopping web server") + + if err := ws.httpServer.Close(); err != nil { + return fmt.Errorf("cannot stop web server: %w", err) + } + + return nil +} + +func (ws *WebServer) healthHandler(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "OK") +} From 1cc687395c54424788dbbb7195dc0489ee0cdab5 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 13:27:50 +0200 Subject: [PATCH 07/10] Adds next birthdays endpoint --- README.md | 8 +++-- birthdays.csv | 1 + model/birthdays.go | 38 ++++++++++++++++++++++++ model/birthdays_test.go | 66 +++++++++++++++++++++++++++++++++++++++++ server/helpers_test.go | 6 +++- server/server.go | 4 +++ server/web.go | 18 +++++++++++ 7 files changed, 137 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index dbe72c4..0c4a277 100644 --- a/README.md +++ b/README.md @@ -70,10 +70,12 @@ $ make run - [X] Reduce logger verbosity (through levels) - [X] Add pictures to birthday notifications - [X] Create a configurable template to fill with each notification -- [ ] Add some endpoints - - [ ] Health endpoint - - [ ] Next birthday endpoint +- [X] Add some endpoints + - [X] Health endpoint + - [X] Next birthdays endpoint - [ ] Birthday list endpoint +- [ ] Allow to use a random port in web tests +- [ ] Web server should be optional - [ ] Create different message systems to use with the bot - [X] Telegram - [ ] Email diff --git a/birthdays.csv b/birthdays.csv index 1450480..eda11db 100644 --- a/birthdays.csv +++ b/birthdays.csv @@ -1,2 +1,3 @@ John Doe,john@doe.com,12345,17/04/2192 +John Doe The Second,john@doesecond.com,12543,17/04/2192 Jane Doe,jane@doe.com,54321,10/11/2020 \ No newline at end of file diff --git a/model/birthdays.go b/model/birthdays.go index e4a311a..818d646 100644 --- a/model/birthdays.go +++ b/model/birthdays.go @@ -4,6 +4,7 @@ import ( "fmt" "strconv" "strings" + "time" ) type Birthday struct { @@ -19,6 +20,10 @@ func (b *Birthday) Filename() string { return fmt.Sprintf("%d_%d_%d_%s.png", b.YearOfBirth, b.MonthOfBirth, b.DayOfBirth, b.Phone) } +func (b *Birthday) Time() time.Time { + return time.Date(b.YearOfBirth, time.Month(b.MonthOfBirth), b.DayOfBirth, 0, 0, 0, 0, time.Now().Location()) +} + func NewBirthdayFromRecord(record []string) (*Birthday, error) { if len(record) != 4 { return nil, fmt.Errorf("invalid length %d for record", len(record)) @@ -97,3 +102,36 @@ func FilterByDate(birthdays []*Birthday, day, month, year int) []*Birthday { } return filteredBirthdays } + +func NextBirthdayDate(birthdays []*Birthday, now time.Time) (int, int, int) { + nowRounded := now.Round(24 * time.Hour) + + var nextBirthday *Birthday + for _, birthday := range birthdays { + if nextBirthday == nil { + nextBirthday = birthday + continue + } + + birthdayTime := birthday.Time() + nextBirthdayTime := nextBirthday.Time() + + if nextBirthdayTime.Before(nowRounded) && birthdayTime.After(nowRounded) { + nextBirthday = birthday + continue + } + + if birthdayTime.Before(nextBirthdayTime) { + if birthdayTime.After(nowRounded) || nextBirthdayTime.Before(nowRounded) { + nextBirthday = birthday + } + } + } + + return nextBirthday.DayOfBirth, nextBirthday.MonthOfBirth, nextBirthday.YearOfBirth +} + +func NextBirthdays(birthdays []*Birthday, now time.Time) []*Birthday { + day, month, year := NextBirthdayDate(birthdays, now) + return FilterByDate(birthdays, day, month, year) +} diff --git a/model/birthdays_test.go b/model/birthdays_test.go index 2570b45..260ed7a 100644 --- a/model/birthdays_test.go +++ b/model/birthdays_test.go @@ -2,6 +2,7 @@ package model import ( "testing" + "time" "github.com/stretchr/testify/require" ) @@ -55,3 +56,68 @@ func TestFilename(t *testing.T) { require.Equal(t, "2022_4_6_123456789.png", birthday.Filename()) } + +func TestNextBirthdayDate(t *testing.T) { + firstBirthday := &Birthday{ + YearOfBirth: 1900, + MonthOfBirth: 2, + DayOfBirth: 1, + } + + secondBirthday := &Birthday{ + YearOfBirth: 1900, + MonthOfBirth: 8, + DayOfBirth: 1, + } + + birthdays := []*Birthday{firstBirthday, secondBirthday} + birthdaysReversed := []*Birthday{secondBirthday, firstBirthday} + + testCases := []struct { + Name string + Now time.Time + Birthdays []*Birthday + ExpectedDay int + ExpectedMonth int + ExpectedYear int + }{ + { + Name: "should find first birthday", + Now: time.Date(1900, time.Month(1), 1, 0, 0, 0, 0, time.Now().Location()), + ExpectedDay: 1, + ExpectedMonth: 2, + ExpectedYear: 1900, + }, + { + Name: "should find second birthday", + Now: time.Date(1900, time.Month(4), 1, 0, 0, 0, 0, time.Now().Location()), + ExpectedDay: 1, + ExpectedMonth: 8, + ExpectedYear: 1900, + }, + { + Name: "should find first birthday for next year", + Now: time.Date(1900, time.Month(10), 1, 0, 0, 0, 0, time.Now().Location()), + ExpectedDay: 1, + ExpectedMonth: 2, + ExpectedYear: 1900, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + t.Run("with birthdays sorted", func(t *testing.T) { + day, month, year := NextBirthdayDate(birthdays, tc.Now) + require.Equal(t, tc.ExpectedDay, day) + require.Equal(t, tc.ExpectedMonth, month) + require.Equal(t, tc.ExpectedYear, year) + }) + t.Run("with birthdays reversed", func(t *testing.T) { + day, month, year := NextBirthdayDate(birthdaysReversed, tc.Now) + require.Equal(t, tc.ExpectedDay, day) + require.Equal(t, tc.ExpectedMonth, month) + require.Equal(t, tc.ExpectedYear, year) + }) + }) + } +} diff --git a/server/helpers_test.go b/server/helpers_test.go index e73e2c7..c9b6af4 100644 --- a/server/helpers_test.go +++ b/server/helpers_test.go @@ -28,7 +28,11 @@ func testConfig(t *testing.T) *model.Config { require.NoError(t, f.Close()) require.NoError(t, os.Remove(f.Name())) - return &model.Config{Birthdays: &model.BirthdaysConfig{File: f.Name()}} + // ToDo: allow for a random port to be used + return &model.Config{ + Web: &model.WebConfig{Port: 9090}, + Birthdays: &model.BirthdaysConfig{File: f.Name()}, + } } func SetupTestHelper(t *testing.T, opts ...Option) *TestHelper { diff --git a/server/server.go b/server/server.go index a54d5db..9b88f74 100644 --- a/server/server.go +++ b/server/server.go @@ -167,3 +167,7 @@ func (s *Server) Notify(birthday *model.Birthday) error { func (s *Server) Birthdays() []*model.Birthday { return s.birthdays } + +func (s *Server) NextBirthdays() []*model.Birthday { + return model.NextBirthdays(s.birthdays, time.Now()) +} diff --git a/server/web.go b/server/web.go index 8c5c489..6709699 100644 --- a/server/web.go +++ b/server/web.go @@ -1,6 +1,7 @@ package server import ( + "encoding/json" "errors" "fmt" "net/http" @@ -25,6 +26,7 @@ func NewWebServer(server *Server) *WebServer { mux := http.NewServeMux() mux.HandleFunc("/health", ws.healthHandler) + mux.HandleFunc("/next_birthdays", ws.nextBirthdayHandler) ws.httpServer.Handler = mux @@ -56,3 +58,19 @@ func (ws *WebServer) Stop() error { func (ws *WebServer) healthHandler(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, "OK") } + +func (ws *WebServer) nextBirthdayHandler(w http.ResponseWriter, r *http.Request) { + ws.JSON(w, http.StatusOK, ws.server.NextBirthdays()) +} + +func (ws *WebServer) JSON(w http.ResponseWriter, statusCode int, data any) { + b, err := json.Marshal(data) + if err != nil { + ws.logger.Error("cannot marshal data", "error", err) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + + w.WriteHeader(statusCode) + w.Write(b) +} From eaca9c1691dcf835e79a272ce4da729c02a58158 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 15:47:01 +0200 Subject: [PATCH 08/10] Make web server optional and allow to use a random port --- README.md | 4 ++-- example-config.yml | 1 + model/config.go | 9 +++------ server/helpers_test.go | 3 +-- server/server.go | 21 +++++++++++++++------ server/server_test.go | 1 + server/web.go | 27 ++++++++++++++++++--------- server/web_test.go | 15 +++++++++++++++ 8 files changed, 56 insertions(+), 25 deletions(-) create mode 100644 server/web_test.go diff --git a/README.md b/README.md index 0c4a277..74daf59 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,8 @@ $ make run - [X] Health endpoint - [X] Next birthdays endpoint - [ ] Birthday list endpoint -- [ ] Allow to use a random port in web tests -- [ ] Web server should be optional +- [X] Allow to use a random port in web tests +- [X] Web server should be optional - [ ] Create different message systems to use with the bot - [X] Telegram - [ ] Email diff --git a/example-config.yml b/example-config.yml index 57b9ca2..d59be78 100644 --- a/example-config.yml +++ b/example-config.yml @@ -1,5 +1,6 @@ --- web: + enabled: true port: 8080 birthdays: diff --git a/model/config.go b/model/config.go index 469cd4c..9ec6a18 100644 --- a/model/config.go +++ b/model/config.go @@ -111,14 +111,11 @@ func (lc *LoggerConfig) IsValid() error { } type WebConfig struct { - Port int `yaml:"port"` + Enabled bool `yaml:"enabled"` + Port int `yaml:"port"` } -func (wc *WebConfig) SetDefaults() { - if wc.Port == 0 { - wc.Port = 8080 - } -} +func (wc *WebConfig) SetDefaults() {} func (wc *WebConfig) IsValid() error { return nil diff --git a/server/helpers_test.go b/server/helpers_test.go index c9b6af4..fec1ce9 100644 --- a/server/helpers_test.go +++ b/server/helpers_test.go @@ -28,9 +28,8 @@ func testConfig(t *testing.T) *model.Config { require.NoError(t, f.Close()) require.NoError(t, os.Remove(f.Name())) - // ToDo: allow for a random port to be used return &model.Config{ - Web: &model.WebConfig{Port: 9090}, + Web: &model.WebConfig{Enabled: true, Port: 0}, Birthdays: &model.BirthdaysConfig{File: f.Name()}, } } diff --git a/server/server.go b/server/server.go index 9b88f74..46254df 100644 --- a/server/server.go +++ b/server/server.go @@ -108,10 +108,15 @@ func New(options ...Option) (*Server, error) { } } - if srv.WebServer == nil { + if srv.WebServer == nil && srv.Config.Web.Enabled { srv.Logger.Debug("creating web server") - srv.WebServer = NewWebServer(srv) + ws, err := NewWebServer(srv) + if err != nil { + return nil, fmt.Errorf("cannot create web server: %w", err) + } + + srv.WebServer = ws } return srv, nil @@ -120,8 +125,10 @@ func New(options ...Option) (*Server, error) { func (s *Server) Start() error { s.Logger.Info("starting server") - if err := s.WebServer.Start(); err != nil { - return fmt.Errorf("cannot start web server: %w", err) + if s.WebServer != nil { + if err := s.WebServer.Start(); err != nil { + return fmt.Errorf("cannot start web server: %w", err) + } } for _, worker := range s.workers { @@ -136,8 +143,10 @@ func (s *Server) Start() error { func (s *Server) Stop() error { s.Logger.Info("stopping server") - if err := s.WebServer.Stop(); err != nil { - return fmt.Errorf("cannot stop web server: %w", err) + if s.WebServer != nil { + if err := s.WebServer.Stop(); err != nil { + return fmt.Errorf("cannot stop web server: %w", err) + } } for _, worker := range s.workers { diff --git a/server/server_test.go b/server/server_test.go index 3da18b9..8d76cb1 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -15,6 +15,7 @@ import ( func TestNotify(t *testing.T) { th := SetupTestHelper(t) defer th.TearDown() + t.Run("should correctly use the notification services to notify", func(t *testing.T) { birthday := th.srv.birthdays[0] th.mockNotificationService. diff --git a/server/web.go b/server/web.go index 6709699..d2e1dfd 100644 --- a/server/web.go +++ b/server/web.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "net" "net/http" "github.com/charmbracelet/log" @@ -11,33 +12,37 @@ import ( type WebServer struct { server *Server + listener net.Listener logger *log.Logger httpServer *http.Server } -func NewWebServer(server *Server) *WebServer { +func NewWebServer(server *Server) (*WebServer, error) { + listener, err := net.Listen("tcp", fmt.Sprintf(":%d", server.Config.Web.Port)) + if err != nil { + return nil, fmt.Errorf("cannot create listener: %w", err) + } + ws := &WebServer{ - server: server, - logger: server.Logger, - httpServer: &http.Server{ - Addr: fmt.Sprintf(":%d", server.Config.Web.Port), - }, + server: server, + listener: listener, + logger: server.Logger, } mux := http.NewServeMux() mux.HandleFunc("/health", ws.healthHandler) mux.HandleFunc("/next_birthdays", ws.nextBirthdayHandler) - ws.httpServer.Handler = mux + ws.httpServer = &http.Server{Handler: mux} - return ws + return ws, nil } func (ws *WebServer) Start() error { ws.logger.Debug("starting web server") go func() { - if err := ws.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + if err := ws.httpServer.Serve(ws.listener); err != nil && !errors.Is(err, http.ErrServerClosed) { ws.logger.Fatal("cannot start web server", "error", err) } }() @@ -55,6 +60,10 @@ func (ws *WebServer) Stop() error { return nil } +func (ws *WebServer) Port() int { + return ws.listener.Addr().(*net.TCPAddr).Port +} + func (ws *WebServer) healthHandler(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, "OK") } diff --git a/server/web_test.go b/server/web_test.go new file mode 100644 index 0000000..14d41ac --- /dev/null +++ b/server/web_test.go @@ -0,0 +1,15 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPort(t *testing.T) { + th := SetupTestHelper(t) + defer th.TearDown() + + port := th.srv.WebServer.Port() + require.NotEmpty(t, port) +} From 3335cfe79550218f37b9932366a5ebebdc5c4d30 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 16:29:57 +0200 Subject: [PATCH 09/10] Adds a client and tests for the handlers --- client/client.go | 91 ++++++++++++++++++++++++++++++++++++++++++ client/options.go | 17 ++++++++ server/helpers_test.go | 7 ++++ server/web_test.go | 24 +++++++++++ 4 files changed, 139 insertions(+) create mode 100644 client/client.go create mode 100644 client/options.go diff --git a/client/client.go b/client/client.go new file mode 100644 index 0000000..e4246a9 --- /dev/null +++ b/client/client.go @@ -0,0 +1,91 @@ +package client + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "strings" + + "git.ctrlz.es/mgdelacroix/birthdaybot/model" +) + +var ( + ErrEmptyURL = errors.New("URL cannot be empty") +) + +type Client struct { + url string + httpClient *http.Client + headers map[string]string +} + +func New(opts ...Option) (*Client, error) { + client := &Client{ + httpClient: &http.Client{}, + } + + for _, opt := range opts { + client = opt(client) + } + + if client.url == "" { + return nil, ErrEmptyURL + } + + return client, nil +} + +func (c *Client) Do(ctx context.Context, method, path, data string, headers map[string]string) (*http.Response, error) { + url, err := url.JoinPath(c.url, path) + if err != nil { + return nil, fmt.Errorf("cannot build request url: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, method, url, strings.NewReader(data)) + if err != nil { + return nil, fmt.Errorf("cannot create request: %w", err) + } + + for k, v := range c.headers { + req.Header.Set(k, v) + } + + for k, v := range headers { + req.Header.Set(k, v) + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("cannot do request: %w", err) + } + + return resp, nil +} + +func (c *Client) Health(ctx context.Context) (bool, error) { + resp, err := c.Do(ctx, http.MethodGet, "/health", "", nil) + if err != nil { + return false, err + } + defer resp.Body.Close() + + return resp.StatusCode == 200, nil +} + +func (c *Client) NextBirthdays(ctx context.Context) ([]*model.Birthday, error) { + resp, err := c.Do(ctx, http.MethodGet, "/next_birthdays", "", nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + var birthdays []*model.Birthday + if err := json.NewDecoder(resp.Body).Decode(&birthdays); err != nil { + return nil, fmt.Errorf("cannot decode birthdays: %w", err) + } + + return birthdays, nil +} diff --git a/client/options.go b/client/options.go new file mode 100644 index 0000000..80554ba --- /dev/null +++ b/client/options.go @@ -0,0 +1,17 @@ +package client + +type Option func(*Client) *Client + +func WithURL(url string) Option { + return func(client *Client) *Client { + client.url = url + return client + } +} + +func WithHeaders(headers map[string]string) Option { + return func(client *Client) *Client { + client.headers = headers + return client + } +} diff --git a/server/helpers_test.go b/server/helpers_test.go index fec1ce9..9757bc0 100644 --- a/server/helpers_test.go +++ b/server/helpers_test.go @@ -1,10 +1,12 @@ package server import ( + "fmt" "io/ioutil" "os" "testing" + "git.ctrlz.es/mgdelacroix/birthdaybot/client" "git.ctrlz.es/mgdelacroix/birthdaybot/model" "git.ctrlz.es/mgdelacroix/birthdaybot/notification" notification_mocks "git.ctrlz.es/mgdelacroix/birthdaybot/notification/mocks" @@ -20,6 +22,7 @@ type TestHelper struct { mockNotificationService *notification_mocks.MockNotificationService mockWorker *server_mocks.MockWorker srv *Server + client *client.Client } func testConfig(t *testing.T) *model.Config { @@ -71,6 +74,10 @@ func SetupTestHelper(t *testing.T, opts ...Option) *TestHelper { th.srv.Start() + client, err := client.New(client.WithURL(fmt.Sprintf("http://127.0.0.1:%d", th.srv.WebServer.Port()))) + require.NoError(t, err) + th.client = client + return th } diff --git a/server/web_test.go b/server/web_test.go index 14d41ac..fc2c935 100644 --- a/server/web_test.go +++ b/server/web_test.go @@ -1,6 +1,7 @@ package server import ( + "context" "testing" "github.com/stretchr/testify/require" @@ -13,3 +14,26 @@ func TestPort(t *testing.T) { port := th.srv.WebServer.Port() require.NotEmpty(t, port) } + +func TestHealthHandler(t *testing.T) { + th := SetupTestHelper(t) + defer th.TearDown() + + t.Run("should return ok if the server is up and running", func(t *testing.T) { + health, err := th.client.Health(context.Background()) + require.NoError(t, err) + require.True(t, health) + }) +} + +func TestNextBirthdaysHandler(t *testing.T) { + th := SetupTestHelper(t) + defer th.TearDown() + + t.Run("should return a list if the server is up and running", func(t *testing.T) { + birthdays, err := th.client.NextBirthdays(context.Background()) + require.NoError(t, err) + require.Len(t, birthdays, 1) + require.Equal(t, "john@doe.com", birthdays[0].Email) + }) +} From 1747612b86977de369ec9c9467d48ceb2c21725a Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 11 Jul 2023 16:43:20 +0200 Subject: [PATCH 10/10] Adds golangci-lint --- .gitlab-ci.yml | 3 ++- Makefile | 3 +++ model/config.go | 4 ++-- model/config_test.go | 5 ++--- parser/csv_parser_test.go | 4 ++-- server/helpers_test.go | 7 +++---- server/web.go | 5 ++++- server/worker.go | 4 +++- shell.nix | 1 + 9 files changed, 22 insertions(+), 14 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 11967c5..dd9788c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -9,11 +9,12 @@ check-generate: script: - nix-shell --run "make generate && git diff --quiet" -check-fmt: +check-lint: stage: format image: nixos/nix:latest script: - nix-shell --run "make fmt && git diff --quiet" + - nix-shell --run "make lint" check-gomod: stage: format diff --git a/Makefile b/Makefile index 428f401..7060845 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,9 @@ test-watch: fmt: go fmt ./... +lint: + golangci-lint run ./... + run: go run ./cmd/birthdaybot -config example-config.yml diff --git a/model/config.go b/model/config.go index 9ec6a18..c732011 100644 --- a/model/config.go +++ b/model/config.go @@ -3,7 +3,7 @@ package model import ( "errors" "fmt" - "io/ioutil" + "os" "strings" "gopkg.in/yaml.v3" @@ -141,7 +141,7 @@ func (tnc *TelegramNotificationsConfig) IsValid() error { } func ReadConfig(path string) (*Config, error) { - fileBytes, err := ioutil.ReadFile(path) + fileBytes, err := os.ReadFile(path) if err != nil { return nil, err } diff --git a/model/config_test.go b/model/config_test.go index b744133..81f88af 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -2,7 +2,6 @@ package model import ( "io" - "io/ioutil" "os" "testing" @@ -11,7 +10,7 @@ import ( func TestReadConfig(t *testing.T) { t.Run("should correctly read a configuration file", func(t *testing.T) { - f, err := ioutil.TempFile("", "birthdaybot-") + f, err := os.CreateTemp("", "birthdaybot-") require.NoError(t, err) defer os.Remove(f.Name()) @@ -25,7 +24,7 @@ func TestReadConfig(t *testing.T) { }) t.Run("should fail if the file doesn't exist", func(t *testing.T) { - f, err := ioutil.TempFile("", "birthdaybot-") + f, err := os.CreateTemp("", "birthdaybot-") require.NoError(t, err) f.Close() os.Remove(f.Name()) diff --git a/parser/csv_parser_test.go b/parser/csv_parser_test.go index d0b977b..f4ae075 100644 --- a/parser/csv_parser_test.go +++ b/parser/csv_parser_test.go @@ -2,7 +2,7 @@ package parser import ( "io" - "io/ioutil" + "os" "testing" "github.com/stretchr/testify/require" @@ -10,7 +10,7 @@ import ( func TestParseCsv(t *testing.T) { t.Run("should correctly parse a valid CSV file", func(t *testing.T) { - f, err := ioutil.TempFile("", "birthdaybot-") + f, err := os.CreateTemp("", "birthdaybot-") require.NoError(t, err) _, werr := io.WriteString(f, "John Doe , john@doe.com, 1234, 17/04/2192\nJane Doe,jane@doe.com,4321,15/01/2020\n") diff --git a/server/helpers_test.go b/server/helpers_test.go index 9757bc0..d198be6 100644 --- a/server/helpers_test.go +++ b/server/helpers_test.go @@ -2,7 +2,6 @@ package server import ( "fmt" - "io/ioutil" "os" "testing" @@ -26,7 +25,7 @@ type TestHelper struct { } func testConfig(t *testing.T) *model.Config { - f, err := ioutil.TempFile("", "birthdaybot-") + f, err := os.CreateTemp("", "birthdaybot-") require.NoError(t, err) require.NoError(t, f.Close()) require.NoError(t, os.Remove(f.Name())) @@ -72,7 +71,7 @@ func SetupTestHelper(t *testing.T, opts ...Option) *TestHelper { th.srv, err = New(serverOpts...) require.NoError(t, err) - th.srv.Start() + require.NoError(t, th.srv.Start()) client, err := client.New(client.WithURL(fmt.Sprintf("http://127.0.0.1:%d", th.srv.WebServer.Port()))) require.NoError(t, err) @@ -82,6 +81,6 @@ func SetupTestHelper(t *testing.T, opts ...Option) *TestHelper { } func (th *TestHelper) TearDown() { - th.srv.Stop() + require.NoError(th.t, th.srv.Stop()) th.ctrl.Finish() } diff --git a/server/web.go b/server/web.go index d2e1dfd..176ffc6 100644 --- a/server/web.go +++ b/server/web.go @@ -81,5 +81,8 @@ func (ws *WebServer) JSON(w http.ResponseWriter, statusCode int, data any) { } w.WriteHeader(statusCode) - w.Write(b) + if _, err := w.Write(b); err != nil { + ws.logger.Error("cannot write to response writer", "error", err) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + } } diff --git a/server/worker.go b/server/worker.go index 1676454..99b7658 100644 --- a/server/worker.go +++ b/server/worker.go @@ -48,7 +48,9 @@ func (w *SimpleWorker) notifyDay(year, month, day int) { for _, b := range birthdays { w.logger.Info("notifying for birthday", "name", b.Name) - w.server.Notify(b) + if err := w.server.Notify(b); err != nil { + w.logger.Error("error notifying for birthday", "name", b.Name, "error", err) + } } } diff --git a/shell.nix b/shell.nix index dd84f05..c83ebbf 100644 --- a/shell.nix +++ b/shell.nix @@ -15,6 +15,7 @@ pkgs.mkShell { gnumake modd mockgen + golangci-lint ]; shellHook = ''