From d6167980f4130892a3690e8397994f92650ef4e5 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Sun, 13 Jun 2021 00:43:25 +0200 Subject: [PATCH] remove ability to set telegram API, clarify params --- backend/app/cmd/server.go | 13 +++-- backend/app/main.go | 7 ++- backend/app/notify/telegram.go | 46 ++++++++++-------- backend/app/notify/telegram_test.go | 74 ++++++++++++++++++++++++----- 4 files changed, 102 insertions(+), 38 deletions(-) diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 4b3e4934..f9b5cfdf 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -216,7 +216,7 @@ type NotifyGroup struct { QueueSize int `long:"queue" env:"QUEUE" description:"size of notification queue" default:"100"` Telegram struct { Channel string `long:"chan" env:"CHAN" description:"telegram channel for admin notifications"` - API string `long:"api" env:"API" default:"https://api.telegram.org/bot" description:"telegram api prefix"` + API string `long:"api" env:"API" default:"https://api.telegram.org/bot" description:"[deprecated, not used] telegram api prefix"` Token string `long:"token" env:"TOKEN" description:"[deprecated, use --telegram.token] telegram token"` Timeout time.Duration `long:"timeout" env:"TIMEOUT" default:"5s" description:"[deprecated, use --telegram.timeout] telegram timeout"` } `group:"telegram" namespace:"telegram" env-namespace:"TELEGRAM"` @@ -361,6 +361,9 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { s.Telegram.Token = s.Notify.Telegram.Token result = append(result, DeprecatedFlag{Old: "notify.telegram.timeout", New: "telegram.timeout", Version: "1.9"}) } + if s.Notify.Telegram.API != "https://api.telegram.org/bot" { + result = append(result, DeprecatedFlag{Old: "notify.telegram.api", Version: "1.9"}) + } return result } @@ -877,8 +880,12 @@ func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator * } destinations = append(destinations, slack) case "telegram": - tg, err := notify.NewTelegram(s.Telegram.Token, s.Notify.Telegram.Channel, - s.Telegram.Timeout, s.Notify.Telegram.API) + telegramParams := notify.TelegramParams{ + AdminChannelID: s.Notify.Telegram.Channel, + Token: s.Telegram.Token, + Timeout: s.Telegram.Timeout, + } + tg, err := notify.NewTelegram(telegramParams) if err != nil { return nil, errors.Wrap(err, "failed to create telegram notification destination") } diff --git a/backend/app/main.go b/backend/app/main.go index 60a8f053..e3861d89 100644 --- a/backend/app/main.go +++ b/backend/app/main.go @@ -46,8 +46,11 @@ func main() { Revision: revision, }) for _, entry := range c.HandleDeprecatedFlags() { - log.Printf("[WARN] --%s is deprecated since v%s and will be removed in the future, please use --%s instead", - entry.Old, entry.Version, entry.New) + deprecationNote := fmt.Sprintf("[WARN] --%s is deprecated since v%s and will be removed in the future", entry.Old, entry.Version) + if entry.New != "" { + deprecationNote += fmt.Sprintf(", please use --%s instead", entry.New) + } + log.Print(deprecationNote) } err := c.Execute(args) if err != nil { diff --git a/backend/app/notify/telegram.go b/backend/app/notify/telegram.go index dcf5e127..46434ceb 100644 --- a/backend/app/notify/telegram.go +++ b/backend/app/notify/telegram.go @@ -16,38 +16,44 @@ import ( "github.com/pkg/errors" ) +// TelegramParams contain settings for telegram notifications +type TelegramParams struct { + AdminChannelID string // unique identifier for the target chat or username of the target channel (in the format @channelusername) + Token string // token for telegram bot API interactions + Timeout time.Duration // http client timeout + + apiPrefix string // changed only in tests +} + // Telegram implements notify.Destination for telegram type Telegram struct { - channelID string // unique identifier for the target chat or username of the target channel (in the format @channelusername) - token string - apiPrefix string - timeout time.Duration + TelegramParams } const telegramTimeOut = 5000 * time.Millisecond const telegramAPIPrefix = "https://api.telegram.org/bot" // NewTelegram makes telegram bot for notifications -func NewTelegram(token, channelID string, timeout time.Duration, api string) (*Telegram, error) { - if _, err := strconv.ParseInt(channelID, 10, 64); err != nil { - channelID = "@" + channelID // if channelID not a number enforce @ prefix +func NewTelegram(params TelegramParams) (*Telegram, error) { + res := Telegram{TelegramParams: params} + if _, err := strconv.ParseInt(res.AdminChannelID, 10, 64); err != nil { + res.AdminChannelID = "@" + res.AdminChannelID // if channelID not a number enforce @ prefix } - res := Telegram{channelID: channelID, token: token, apiPrefix: api, timeout: timeout} if res.apiPrefix == "" { res.apiPrefix = telegramAPIPrefix } - if res.timeout == 0 { - res.timeout = telegramTimeOut + if res.Timeout == 0 { + res.Timeout = telegramTimeOut } - log.Printf("[DEBUG] create new telegram notifier for chan %s, timeout=%s, api=%s", channelID, res.timeout, res.timeout) + log.Printf("[DEBUG] create new telegram notifier for api=%s, timeout=%s", res.apiPrefix, res.Timeout) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() err := repeater.NewDefault(5, time.Millisecond*250).Do(ctx, func() error { - client := http.Client{Timeout: res.timeout} - resp, err := client.Get(fmt.Sprintf("%s%s/getMe", res.apiPrefix, token)) + client := http.Client{Timeout: res.Timeout} + resp, err := client.Get(fmt.Sprintf("%s%s/getMe", res.apiPrefix, res.Token)) if err != nil { return errors.Wrap(err, "can't initialize telegram notifications") } @@ -88,9 +94,9 @@ func NewTelegram(token, channelID string, timeout time.Duration, api string) (*T func (t *Telegram) Send(ctx context.Context, req Request) error { var err error - if t.channelID != "" { + if t.AdminChannelID != "" { err = t.sendAdminNotification(ctx, req) - if err != nil{ + if err != nil { return errors.Wrapf(err, "problem sending admin telegram notification") } } @@ -99,14 +105,14 @@ func (t *Telegram) Send(ctx context.Context, req Request) error { } func (t *Telegram) sendAdminNotification(ctx context.Context, req Request) error { - log.Printf("[DEBUG] send admin telegram notification to %s, comment id %s", t.channelID, req.Comment.ID) + log.Printf("[DEBUG] send admin telegram notification to %s, comment id %s", t.AdminChannelID, req.Comment.ID) msg, err := buildTelegramMessage(req) if err != nil { return errors.Wrap(err, "failed to make telegram message body") } - err = t.sendMessage(ctx, msg, t.channelID) + err = t.sendMessage(ctx, msg, t.AdminChannelID) if err != nil { return errors.Wrapf(err, "failed to send admin notification about %s", req.Comment.ID) } @@ -115,14 +121,14 @@ func (t *Telegram) sendAdminNotification(ctx context.Context, req Request) error func (t *Telegram) sendMessage(ctx context.Context, b []byte, chatID string) error { u := fmt.Sprintf("%s%s/sendMessage?chat_id=%s&parse_mode=Markdown&disable_web_page_preview=true", - t.apiPrefix, t.token, chatID) + t.apiPrefix, t.Token, chatID) r, err := http.NewRequest("POST", u, bytes.NewReader(b)) if err != nil { return errors.Wrap(err, "failed to make telegram request") } r.Header.Set("Content-Type", "application/json; charset=utf-8") - client := http.Client{Timeout: t.timeout} + client := http.Client{Timeout: t.Timeout} r = r.WithContext(ctx) resp, err := client.Do(r) if err != nil { @@ -186,5 +192,5 @@ func (t *Telegram) SendVerification(_ context.Context, _ VerificationRequest) er } func (t *Telegram) String() string { - return "telegram: " + t.channelID + return "telegram: " + t.AdminChannelID } diff --git a/backend/app/notify/telegram_test.go b/backend/app/notify/telegram_test.go index 84d31514..97ea5821 100644 --- a/backend/app/notify/telegram_test.go +++ b/backend/app/notify/telegram_test.go @@ -20,45 +20,85 @@ func TestTelegram_New(t *testing.T) { ts := mockTelegramServer() defer ts.Close() - tb, err := NewTelegram("good-token", "remark_test", 2*time.Second, ts.URL+"/") + tb, err := NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "good-token", + apiPrefix: ts.URL + "/", + }) assert.NoError(t, err) assert.NotNil(t, tb) - assert.Equal(t, "@remark_test", tb.channelID, "@ added") + assert.Equal(t, tb.Timeout, time.Second*5) + assert.Equal(t, "@remark_test", tb.AdminChannelID, "@ added") st := time.Now() - _, err = NewTelegram("bad-resp", "remark_test", 2*time.Second, ts.URL+"/") + _, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "bad-resp", + apiPrefix: ts.URL + "/", + }) assert.EqualError(t, err, "unexpected telegram response {OK:false Result:{FirstName:comments_test ID:707381019 IsBot:false UserName:remark42_test_bot}}") assert.True(t, time.Since(st) >= 250*5*time.Millisecond) - _, err = NewTelegram("non-json-resp", "remark_test", 2*time.Second, ts.URL+"/") + _, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "non-json-resp", + Timeout: 2 * time.Second, + apiPrefix: ts.URL + "/", + }) assert.Error(t, err) assert.Contains(t, err.Error(), "can't decode response:") - _, err = NewTelegram("404", "remark_test", 2*time.Second, ts.URL+"/") + _, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "404", + Timeout: 2 * time.Second, + apiPrefix: ts.URL + "/", + }) assert.EqualError(t, err, "unexpected telegram status code 404") - _, err = NewTelegram("no-such-thing", "remark_test", 2*time.Second, "http://127.0.0.1:4321/") + _, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "no-such-thing", + apiPrefix: "http://127.0.0.1:4321/", + }) require.Error(t, err) assert.Contains(t, err.Error(), "can't initialize telegram notifications") assert.Contains(t, err.Error(), "dial tcp 127.0.0.1:4321: connect: connection refused") - _, err = NewTelegram("good-token", "remark_test", 2*time.Second, "") + _, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "no-such-thing", + apiPrefix: "", + }) assert.Error(t, err, "empty api url not allowed") - _, err = NewTelegram("good-token", "remark_test", 0, ts.URL+"/") + _, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "good-token", + Timeout: 2 * time.Second, + apiPrefix: ts.URL + "/", + }) assert.NoError(t, err, "0 timeout allowed as default") - tb, err = NewTelegram("good-token", "1234567890", 2*time.Second, ts.URL+"/") + tb, err = NewTelegram(TelegramParams{ + AdminChannelID: "1234567890", + Token: "good-token", + apiPrefix: ts.URL + "/", + }) assert.NoError(t, err) assert.NotNil(t, tb) - assert.Equal(t, "1234567890", tb.channelID, "no @ prefix") + assert.Equal(t, "1234567890", tb.AdminChannelID, "no @ prefix") } func TestTelegram_Send(t *testing.T) { ts := mockTelegramServer() defer ts.Close() - tb, err := NewTelegram("good-token", "remark_test", 2*time.Second, ts.URL+"/") + tb, err := NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "good-token", + apiPrefix: ts.URL + "/", + }) assert.NoError(t, err) assert.NotNil(t, tb) c := store.Comment{Text: "some text", ParentID: "1", ID: "999"} @@ -78,7 +118,11 @@ func TestTelegram_Send(t *testing.T) { err = tb.Send(context.TODO(), Request{Comment: c, parent: cp}) assert.NoError(t, err) - tb, err = NewTelegram("non-json-resp", "remark_test", 2*time.Second, ts.URL+"/") + tb, err = NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "non-json-resp", + apiPrefix: ts.URL + "/", + }) assert.Error(t, err, "should fail") err = tb.Send(context.TODO(), Request{Comment: c, parent: cp}) require.Error(t, err) @@ -96,7 +140,11 @@ func TestTelegram_SendVerification(t *testing.T) { ts := mockTelegramServer() defer ts.Close() - tb, err := NewTelegram("good-token", "remark_test", 2*time.Second, ts.URL+"/") + tb, err := NewTelegram(TelegramParams{ + AdminChannelID: "remark_test", + Token: "good-token", + apiPrefix: ts.URL + "/", + }) assert.NoError(t, err) assert.NotNil(t, tb)