Join webhook rooms on Register()

Also fix a NPE when there was no old service.

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}
 	}

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 {

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{

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 {

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.

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 = ""