From 4772d10cbd80558c5d2a846b06d91fd50336d050 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 22 Aug 2016 15:55:00 +0100 Subject: [PATCH] Join webhook rooms on Register() Also fix a NPE when there was no old service. --- src/github.com/matrix-org/go-neb/api.go | 10 +++--- .../matrix-org/go-neb/database/db.go | 3 +- .../matrix-org/go-neb/services/echo/echo.go | 8 ++--- .../go-neb/services/github/github.go | 36 +++++++++++++------ .../matrix-org/go-neb/services/jira/jira.go | 2 +- .../matrix-org/go-neb/types/types.go | 2 +- 6 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/github.com/matrix-org/go-neb/api.go b/src/github.com/matrix-org/go-neb/api.go index b06f663..c5592cb 100644 --- a/src/github.com/matrix-org/go-neb/api.go +++ b/src/github.com/matrix-org/go-neb/api.go @@ -248,16 +248,16 @@ func (s *configureServiceHandler) OnIncomingRequest(req *http.Request) (interfac return nil, &errors.HTTPError{err, "Error loading old service", 500} } - if err = service.Register(old); err != nil { - return nil, &errors.HTTPError{err, "Failed to register service: " + err.Error(), 500} - } - client, err := s.clients.Client(service.ServiceUserID()) if err != nil { return nil, &errors.HTTPError{err, "Unknown matrix client", 400} } - oldService, err := s.db.StoreService(service, client) + if err = service.Register(old, client); err != nil { + return nil, &errors.HTTPError{err, "Failed to register service: " + err.Error(), 500} + } + + oldService, err := s.db.StoreService(service) if err != nil { return nil, &errors.HTTPError{err, "Error storing service", 500} } diff --git a/src/github.com/matrix-org/go-neb/database/db.go b/src/github.com/matrix-org/go-neb/database/db.go index 7b4c394..07a29b8 100644 --- a/src/github.com/matrix-org/go-neb/database/db.go +++ b/src/github.com/matrix-org/go-neb/database/db.go @@ -2,7 +2,6 @@ package database import ( "database/sql" - "github.com/matrix-org/go-neb/matrix" "github.com/matrix-org/go-neb/types" "time" ) @@ -104,7 +103,7 @@ func (d *ServiceDB) LoadServicesForUser(serviceUserID string) (services []types. // StoreService stores a service into the database either by inserting a new // service or updating an existing service. Returns the old service if there // was one. -func (d *ServiceDB) StoreService(service types.Service, client *matrix.Client) (oldService types.Service, err error) { +func (d *ServiceDB) StoreService(service types.Service) (oldService types.Service, err error) { err = runTransaction(d.db, func(txn *sql.Tx) error { oldService, err = selectServiceTxn(txn, service.ServiceID()) if err == sql.ErrNoRows { diff --git a/src/github.com/matrix-org/go-neb/services/echo/echo.go b/src/github.com/matrix-org/go-neb/services/echo/echo.go index 3e0d77a..83a2c74 100644 --- a/src/github.com/matrix-org/go-neb/services/echo/echo.go +++ b/src/github.com/matrix-org/go-neb/services/echo/echo.go @@ -13,10 +13,10 @@ type echoService struct { serviceUserID string } -func (e *echoService) ServiceUserID() string { return e.serviceUserID } -func (e *echoService) ServiceID() string { return e.id } -func (e *echoService) ServiceType() string { return "echo" } -func (e *echoService) Register(oldService types.Service) error { return nil } +func (e *echoService) ServiceUserID() string { return e.serviceUserID } +func (e *echoService) ServiceID() string { return e.id } +func (e *echoService) ServiceType() string { return "echo" } +func (e *echoService) Register(oldService types.Service, client *matrix.Client) error { return nil } func (e *echoService) Plugin(roomID string) plugin.Plugin { return plugin.Plugin{ Commands: []plugin.Command{ diff --git a/src/github.com/matrix-org/go-neb/services/github/github.go b/src/github.com/matrix-org/go-neb/services/github/github.go index 3b19c24..4973859 100644 --- a/src/github.com/matrix-org/go-neb/services/github/github.go +++ b/src/github.com/matrix-org/go-neb/services/github/github.go @@ -221,7 +221,7 @@ func (s *githubService) OnReceiveWebhook(w http.ResponseWriter, req *http.Reques // // Hooks can get out of sync if a user manually deletes a hook in the Github UI. In this case, toggling the repo configuration will // force NEB to recreate the hook. -func (s *githubService) Register(oldService types.Service) error { +func (s *githubService) Register(oldService types.Service, client *matrix.Client) error { if s.RealmID == "" { return fmt.Errorf("RealmID is required") } @@ -250,15 +250,17 @@ func (s *githubService) Register(oldService types.Service) error { // Fetch the old service list and work out the difference between the two. var oldRepos []string - old, ok := oldService.(*githubService) - if !ok { - log.WithFields(log.Fields{ - "service_id": oldService.ServiceID(), - "service_type": oldService.ServiceType(), - }).Print("Cannot case old github service to GithubService") - // non-fatal though, we'll just make the hooks - } else { - oldRepos = old.repoList() + if oldService != nil { + old, ok := oldService.(*githubService) + if !ok { + log.WithFields(log.Fields{ + "service_id": oldService.ServiceID(), + "service_type": oldService.ServiceType(), + }).Print("Cannot cast old github service to GithubService") + // non-fatal though, we'll just make the hooks + } else { + oldRepos = old.repoList() + } } // Add the repos in the new service but not the old service @@ -272,6 +274,10 @@ func (s *githubService) Register(oldService types.Service) error { } logger.Info("Created webhook") } + + if err := s.joinWebhookRooms(client); err != nil { + return err + } } log.Infof("%+v", s) @@ -279,6 +285,16 @@ func (s *githubService) Register(oldService types.Service) error { return nil } +func (s *githubService) joinWebhookRooms(client *matrix.Client) error { + for roomID := range s.Rooms { + if _, err := client.JoinRoom(roomID, ""); err != nil { + // TODO: Leave the rooms we successfully joined? + return err + } + } + return nil +} + func (s *githubService) repoList() []string { var repos []string if s.Rooms == nil { diff --git a/src/github.com/matrix-org/go-neb/services/jira/jira.go b/src/github.com/matrix-org/go-neb/services/jira/jira.go index ed400f4..a7440f0 100644 --- a/src/github.com/matrix-org/go-neb/services/jira/jira.go +++ b/src/github.com/matrix-org/go-neb/services/jira/jira.go @@ -41,7 +41,7 @@ type jiraService struct { func (s *jiraService) ServiceUserID() string { return s.serviceUserID } func (s *jiraService) ServiceID() string { return s.id } func (s *jiraService) ServiceType() string { return "jira" } -func (s *jiraService) Register(oldService types.Service) error { +func (s *jiraService) Register(oldService types.Service, client *matrix.Client) error { // We only ever make 1 JIRA webhook which listens for all projects and then filter // on receive. So we simply need to know if we need to make a webhook or not. We // need to do this for each unique realm. diff --git a/src/github.com/matrix-org/go-neb/types/types.go b/src/github.com/matrix-org/go-neb/types/types.go index dadea80..2f36ce6 100644 --- a/src/github.com/matrix-org/go-neb/types/types.go +++ b/src/github.com/matrix-org/go-neb/types/types.go @@ -38,7 +38,7 @@ type Service interface { ServiceType() string Plugin(roomID string) plugin.Plugin OnReceiveWebhook(w http.ResponseWriter, req *http.Request, cli *matrix.Client) - Register(oldService Service) error + Register(oldService Service, client *matrix.Client) error } var baseURL = ""